OAuth 2.0 Token Revocation #379
Conversation
jankapunkt
left a comment
jankapunkt
left a comment
There was a problem hiding this comment.
Thank you very much! This is a great addition to the library and I'd like to move forward with this, unless there are strong objections by @jorenvandeweyer @HappyZombies @shrihari-prakash @dhensby
My suggestion is to make unit and integration tests with focus on standard compliance, before further improving the code. I can support you especially with additional tests if you want to.
However, I am less of a help in regards to TypeScript related things. For this I'd like to add one of you guys for final review phase.
@wille feel free to continue in draft or review-ready mode whenever you like and ping me in the comment if you need anything.
Much appreciated!
| // attempted to authenticate via the "Authorization" request header. | ||
| // | ||
| // @see https://tools.ietf.org/html/rfc6749#section-5.2. | ||
| if (error instanceof InvalidClientError && request.get('authorization')) { |
There was a problem hiding this comment.
Can this even happen at any circumstance?
There was a problem hiding this comment.
Client might be including the client_id and client_secret using Basic authentication in the authorization header, but I don't think www-authenticate should be returned. I'll be removing this clause.
|
|
||
| // Validate token_type_hint if provided | ||
| if (tokenTypeHint && tokenTypeHint !== 'access_token' && tokenTypeHint !== 'refresh_token') { | ||
| throw new UnsupportedTokenTypeError('Unsupported token_type_hint: ' + tokenTypeHint); |
There was a problem hiding this comment.
Is this really the appropriate error type? From my understanding this error type is defined for the server not supporting the revocation at all? The RFC is a bit ambiguous here...
There was a problem hiding this comment.
I've just read it again and you're right.
|
Thank you for your reply. I will build proper unit tests for this feature asap. |
2 participants
Summary
Add support for OAuth 2.0 Token Revocation
Linked issue(s)
#343
Involved parts of the project
A new 'revoke' handler is added.
Added tests?
Not yet. This is a draft looking for feedback.
OAuth2 standard
https://datatracker.ietf.org/doc/html/rfc7009
Reproduction
I'm gonna add support in https://github.com/node-oauth/express-oauth-server and the examples repo