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

Update oauth2 middleware #587

Closed
wants to merge 3 commits into from
Closed

Update oauth2 middleware #587

wants to merge 3 commits into from

Conversation

etehtsea
Copy link
Contributor

@etehtsea etehtsea commented Mar 5, 2014

No description provided.

@@ -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')
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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?

Copy link
Member

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".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dblock , any thoughts?

Copy link
Member

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
@dblock
Copy link
Member

dblock commented Mar 5, 2014

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?

@etehtsea
Copy link
Contributor Author

etehtsea commented Mar 5, 2014

@dblock added spec version.

@dblock
Copy link
Member

dblock commented Mar 5, 2014

Thanks, merging.

@dblock
Copy link
Member

dblock commented Mar 5, 2014

Merged via 01f2590.

@dblock dblock closed this Mar 5, 2014
@etehtsea etehtsea deleted the fix-expired-token branch March 5, 2014 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants