Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return a 401 error when auth info is not provided in endpoint requiring default auth #91

Closed
gabrielliwerant opened this issue Jun 22, 2015 · 11 comments

Comments

@gabrielliwerant
Copy link

I created a fresh repo here that enables demonstration of the issue: https://github.com/gabrielliwerant/meteor-restivus-bug.

When attempting to use authentication with a GET route, we are presented with the following stack trace:

TypeError: Not a string or buffer at Hash.update (crypto.js:240:17) at Object.Accounts._hashLoginToken (packages/accounts-base/accounts_server.js:577:1) at [object Object].Restivus.config.auth.user (packages/nimble:restivus/lib/restivus.coffee:15:27) at Route.Route._authenticate (packages/nimble:restivus/lib/route.coffee:159:34) at Route.Route._authAccepted (packages/nimble:restivus/lib/route.coffee:146:8) at Route.Route._callEndpoint (packages/nimble:restivus/lib/route.coffee:124:9) at [object Object].Router.route.action (packages/nimble:restivus/lib/route.coffee:46:31) at boundNext (packages/iron:middleware-stack/lib/middleware_stack.js:251:1) at runWithEnvironment (packages/meteor/dynamics_nodejs.js:108:1) at packages/meteor/dynamics_nodejs.js:121:1

@kahmali
Copy link
Owner

kahmali commented Jun 23, 2015

Thanks for providing a test repo! That's always greatly appreciated.

You need to login and then pass a user ID and auth token to any authenticated routes (unless you provide custom auth). The repro steps you provided in your test repo cause an expected error (although I admit there's room for some much clearer error handling there). You are declaring the route as requiring authentication, but are not providing the user ID and auth token in the headers. This is the intended behavior. Please read more on authentication in Restivus. I'm going to close this issue, but if for some reason I've misunderstood you and you still feel this is unresolved, please feel free to reopen it and continue the discussion. Thanks!

@kahmali
Copy link
Owner

kahmali commented Jun 23, 2015

Actually, I'm going to reopen this until I return a 401 Unauthorized error from the default auth when no auth info is provided.

@kahmali kahmali reopened this Jun 23, 2015
@kahmali kahmali changed the title Authentication failing for GET request, leading to stack trace output Return a 401 error when auth info is not provided in endpoint requiring default auth Jun 23, 2015
@kahmali kahmali added this to the v0.8.0 milestone Jun 23, 2015
@gabrielliwerant
Copy link
Author

Thanks for the quickness of your response to this issue.

The problem may be a little deeper, or is perhaps a documentation issue. I seem unable to generate a userID and auth token (which I did attempt before I opened this issue).

According to the docs, I make a request to the login endpoint like so:
`curl http://localhost:3000/api/login/ -d "password=testpassword&user=test"

but I receive the following response:

{ "status": "error", "message": "Unauthorized"}

I also get the response above if I use a REST client or query params.

My understanding from the instructions ("The response will look like this, which you must save (for subsequent authenticated requests):") is that I should be able to make this request by submitting new credentials, and then get the generated authToken and userId, which I subsequently pass in the header to authenticate my requests. However, I receive the above error instead. So I am either not following the instructions correctly, or there is another issue.

By the way, I appreciate the work you've been doing on this meteor package. Other than the above issue, I have found the API to be very clean and easy to use.

@kahmali
Copy link
Owner

kahmali commented Jun 23, 2015

Ah, I think I see the issue. I will certainly review the authentication section of the docs and try to provide some more details there and make things clearer. The docs are assuming that you have already created that user. You can do that by just adding a POST collection endpoint for your Meteor.users collection (or by any other means of creating a user with the accounts packages). Please check out the section of the docs on generating endpoints for the Meteor.users collection. At the very least, I should put that link at the beginning of the "Authentication" section of the docs, and make it clear that you have to create a Meteor.user separately. Sorry for the confusion.

@gabrielliwerant
Copy link
Author

Ah, yes, that makes sense now that you point that out. I apologize for my confusion, but I agree it's an opportunity to make the docs more idiot-proof. Thank you.

@kahmali
Copy link
Owner

kahmali commented Jun 23, 2015

No problem! I agree. I'll definitely update that section of the docs. Thanks for reporting the issue!

kahmali added a commit that referenced this issue Jun 29, 2015
- Remove dependency on `useAuth` API config option for endpoint
  authentication (#49)
- Rename API config option `useAuth` to `useDefaultAuth` to better
  reflect it's purpose, which is to generate the default auth endpoints
  - `useAuth` will still work, for backwards-compatibility
- Return a 401 error when auth info is not provided in endpoint
  requiring default auth (#91)
kahmali added a commit that referenced this issue Jun 29, 2015
- Remove dependency on `useAuth` API config option for endpoint
  authentication (#49)
- Rename API config option `useAuth` to `useDefaultAuth` to better
  reflect it's purpose, which is to generate the default auth endpoints
  - `useAuth` will still work, for backwards-compatibility
- Return a 401 error when auth info is not provided in endpoint
  requiring default auth (#91)
@kahmali
Copy link
Owner

kahmali commented Jun 29, 2015

Resolved in 5bf092b

@kahmali kahmali closed this as completed Jun 29, 2015
kahmali added a commit that referenced this issue Jul 2, 2015
- Remove dependency on `useAuth` API config option for endpoint
  authentication (#49)
- Rename API config option `useAuth` to `useDefaultAuth` to better
  reflect it's purpose, which is to generate the default auth endpoints
  - `useAuth` will still work, for backwards-compatibility
- Return a 401 error when auth info is not provided in endpoint
  requiring default auth (#91)
@thierrysikora
Copy link

Thanks for the good work, however i have the same issue as gabrielliwerant reported
{ "status": "error", "message": "Unauthorized"}
, so perhaps you could clarify:

  1. when you write "Works alongside the alanning:roles package" this implies that restivus does not include this packages and this should be added to the project ( as well as meteor add accounts-password) right?
  2. There is no "user" field by default when using the Accounts.createUser() function, should it be added so that
    curl http://localhost:3000/api/login/ -d "password=testpassword&user=test" would work ? If i try to use the email and password to authenticate i get :
    curl http://localhost:3000/api/login/ -d "password= testpassword&email=[email protected]"
    Cannot call method 'indexOf' of undefined
    at [object Object].addRoute.post as action
    at Route.Route._callEndpoint (packages/nimble:restivus/lib/route.coffee:126:25)
    at [object Object].Router.route.action (packages/nimble:restivus/lib/route.coffee:46:31)
    at boundNext (packages/iron:middleware-stack/lib/middleware_stack.js:251:1)
    at runWithEnvironment (packages/meteor/dynamics_nodejs.js:108:1)
  3. I can see also that you "Return a 401 error when auth info is not provided in endpoint" in 0.8.0 branch, would it be soon available through the standard update procedure meteor update nimble:restivus ?

Thanks

@kahmali
Copy link
Owner

kahmali commented Jul 6, 2015

Hi @thierrysikora. To answer your questions:

  1. Yes, that's correct
  2. No, you don't have to add any field to the Meteor.user document. Just put the email in the user field of the request, and Restivus will (very naively) determine if it is an email or username, and validate against the proper field in the Meteor.user document. It won't recognize the email field that you provided, hence the error.
  3. YES! Restivus v0.8.0 is now available!

@markmacgillivray
Copy link

Hello! It's not worth re-opening this issue for this, but I am just commenting to say that I had a similar problem as above, but what I was doing wrong was using the user ID rather than the email address of the user I had created. It was not obvious from the docs that it would guess from the email which user I intended, rather than using the ID of the user object. I am new to Meteor though, so perhaps this is obvious to people usually. Otherwise, perhaps a recommendation for future docs updates would be that the "user" request parameter can be the email address. Thanks for the great repo by the way!

@kahmali
Copy link
Owner

kahmali commented Aug 11, 2015

@markmacgillivray I completely agree. This ambiguous field name was a carry-over from the original RestStop2 API that Restivus was inspired by. I just opened #121 to resolve this. Thanks for reporting this, and an even bigger thanks for the compliment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants