-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update oauth2 middleware #587
Conversation
@@ -54,7 +54,7 @@ def verify_token(token) | |||
token = token_class.verify(token) | |||
if token | |||
if token.respond_to?(:expired?) && token.expired? | |||
error_out(401, 'expired_token') | |||
error_out(401, 'invalid_token') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you clarify why you invalid_token
used here?
in rfc http://tools.ietf.org/html/rfc6749 possible response is invalid_grant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was typo. Fixed, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dblock @dm1try according to http://tools.ietf.org/html/draft-ietf-oauth-v2-bearer-23#section-3.1 error should be invalid_token
. Does anybody knows how it should look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etehtsea , sorry it's my fault..:pensive: rfc you provided seems more valid for our case. I used rfc link from oauth2 main page and just fluently search for differences but seems we should rely on this document that describes "Bearer Token Usage".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dblock , any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh i don't know what the 'right' thing to do here, oauth2 spec is always in flux. It would be great if you guys could figure it out and PR the "right thing to do".
In latest oauth2 spec versions oauth_token was replaced with access_token
It would be great to have a clearer CHANGELOG, "latest" spec will become not so latest soon :) Maybe a spec version or a link or something like that? |
@dblock added spec version. |
Thanks, merging. |
Merged via 01f2590. |
No description provided.