-
-
Notifications
You must be signed in to change notification settings - Fork 55
OAuth 2.0 Token Revocation #379
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| 'use strict'; | ||
|
|
||
| /** | ||
| * Module dependencies. | ||
| */ | ||
|
|
||
| const OAuthError = require('./oauth-error'); | ||
|
|
||
| /** | ||
| * Constructor. | ||
| * | ||
| * "The authorization server does not support | ||
| * the revocation of the presented token type. That is, the | ||
| * client tried to revoke an access token on a server not | ||
| * supporting this feature." | ||
| * | ||
| * @see https://www.rfc-editor.org/rfc/rfc7009#section-2.2.1 | ||
| */ | ||
|
|
||
| class UnsupportedTokenTypeError extends OAuthError { | ||
| constructor(message, properties) { | ||
| properties = { | ||
| code: 503, | ||
| name: 'unsupported_token_type', | ||
| ...properties | ||
| }; | ||
|
|
||
| super(message, properties); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Export constructor. | ||
| */ | ||
|
|
||
| module.exports = UnsupportedTokenTypeError; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,277 @@ | ||
| 'use strict'; | ||
|
|
||
| /** | ||
| * Module dependencies. | ||
| */ | ||
|
|
||
| const InvalidArgumentError = require('../errors/invalid-argument-error'); | ||
| const InvalidClientError = require('../errors/invalid-client-error'); | ||
| const InvalidRequestError = require('../errors/invalid-request-error'); | ||
| const OAuthError = require('../errors/oauth-error'); | ||
| const UnsupportedTokenTypeError = require('../errors/unsupported-token-type-error'); | ||
| const Request = require('../request'); | ||
| const Response = require('../response'); | ||
| const ServerError = require('../errors/server-error'); | ||
| const auth = require('basic-auth'); | ||
| const isFormat = require('@node-oauth/formats'); | ||
|
|
||
| /** | ||
| * Constructor. | ||
| */ | ||
|
|
||
| class RevokeHandler { | ||
| constructor (options) { | ||
| options = options || {}; | ||
|
|
||
| if (!options.model) { | ||
| throw new InvalidArgumentError('Missing parameter: `model`'); | ||
| } | ||
|
|
||
| if (!options.model.getClient) { | ||
| throw new InvalidArgumentError('Invalid argument: model does not implement `getClient()`'); | ||
| } | ||
|
|
||
| if (!options.model.revokeToken) { | ||
| throw new InvalidArgumentError('Invalid argument: model does not implement `revokeToken()`'); | ||
| } | ||
|
|
||
| this.model = options.model; | ||
| } | ||
|
|
||
| /** | ||
| * Revoke Handler. | ||
| * | ||
| * @see https://tools.ietf.org/html/rfc7009 | ||
| */ | ||
|
|
||
| async handle (request, response) { | ||
| if (!(request instanceof Request)) { | ||
| throw new InvalidArgumentError('Invalid argument: `request` must be an instance of Request'); | ||
| } | ||
|
|
||
| if (!(response instanceof Response)) { | ||
| throw new InvalidArgumentError('Invalid argument: `response` must be an instance of Response'); | ||
| } | ||
|
|
||
| if (request.method !== 'POST') { | ||
| throw new InvalidRequestError('Invalid request: method must be POST'); | ||
| } | ||
|
|
||
| try { | ||
| const client = await this.getClient(request, response); | ||
|
|
||
| if (!client) { | ||
| throw new InvalidClientError('Invalid client: client is invalid'); | ||
| } | ||
|
|
||
| const token = request.body.token; | ||
|
|
||
| // An invalid token type hint value is ignored by the authorization | ||
| // server and does not influence the revocation response. | ||
| const tokenTypeHint = request.body.token_type_hint; | ||
|
|
||
| if (!token) { | ||
| throw new InvalidRequestError('Missing parameter: `token`'); | ||
| } | ||
|
|
||
| if (!isFormat.vschar(token)) { | ||
| throw new InvalidRequestError('Invalid parameter: `token`'); | ||
| } | ||
|
|
||
| // Validate token_type_hint if provided | ||
| if (tokenTypeHint && tokenTypeHint !== 'access_token' && tokenTypeHint !== 'refresh_token') { | ||
| throw new UnsupportedTokenTypeError('Unsupported token_type_hint: ' + tokenTypeHint); | ||
| } | ||
|
|
||
| // Try to find and revoke the token | ||
| await this.revokeToken(token, tokenTypeHint, client); | ||
|
|
||
| // Per RFC 7009 section 2.2: return 200 OK even if token was invalid | ||
| // This prevents token enumeration attacks | ||
| this.updateSuccessResponse(response); | ||
| } catch (e) { | ||
| let error = e; | ||
|
|
||
| if (!(error instanceof OAuthError)) { | ||
| error = new ServerError(error); | ||
| } | ||
|
|
||
| // Include the "WWW-Authenticate" response header field if the client | ||
| // 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')) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this even happen at any circumstance?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| response.set('WWW-Authenticate', 'Basic realm="Service"'); | ||
| throw new InvalidClientError(error, { code: 401 }); | ||
| } | ||
|
|
||
| // For other errors, update the response but don't throw | ||
| // RFC 7009 says to return 200 OK even for invalid tokens, but we should | ||
| // still return errors for malformed requests or authentication failures | ||
| if (error instanceof InvalidRequestError || error instanceof InvalidClientError) { | ||
| this.updateErrorResponse(response, error); | ||
| throw error; | ||
| } | ||
|
|
||
| // For other errors (like server errors), still return 200 OK per RFC 7009 | ||
| // but log the error | ||
| this.updateSuccessResponse(response); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get the client from the model. | ||
| */ | ||
|
|
||
| async getClient (request, response) { | ||
| const credentials = await this.getClientCredentials(request); | ||
|
|
||
| if (!credentials.clientId) { | ||
| throw new InvalidRequestError('Missing parameter: `client_id`'); | ||
| } | ||
|
|
||
| if (!isFormat.vschar(credentials.clientId)) { | ||
| throw new InvalidRequestError('Invalid parameter: `client_id`'); | ||
| } | ||
|
|
||
| if (credentials.clientSecret && !isFormat.vschar(credentials.clientSecret)) { | ||
| throw new InvalidRequestError('Invalid parameter: `client_secret`'); | ||
| } | ||
|
|
||
| try { | ||
| const client = await this.model.getClient(credentials.clientId, credentials.clientSecret); | ||
|
|
||
| if (!client) { | ||
| throw new InvalidClientError('Invalid client: client is invalid'); | ||
| } | ||
|
|
||
| return client; | ||
| } catch (e) { | ||
| // Include the "WWW-Authenticate" response header field if the client | ||
| // attempted to authenticate via the "Authorization" request header. | ||
| // | ||
| // @see https://tools.ietf.org/html/rfc6749#section-5.2. | ||
| if ((e instanceof InvalidClientError) && request.get('authorization')) { | ||
| response.set('WWW-Authenticate', 'Basic realm="Service"'); | ||
| throw new InvalidClientError(e, { code: 401 }); | ||
| } | ||
|
|
||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get client credentials. | ||
| * | ||
| * The client credentials may be sent using the HTTP Basic authentication scheme or, alternatively, | ||
| * the `client_id` and `client_secret` can be embedded in the body. | ||
| * | ||
| * @see https://tools.ietf.org/html/rfc6749#section-2.3.1 | ||
| */ | ||
|
|
||
| getClientCredentials (request) { | ||
| const credentials = auth(request); | ||
|
|
||
| if (credentials) { | ||
| return { clientId: credentials.name, clientSecret: credentials.pass }; | ||
| } | ||
|
|
||
| if (request.body.client_id) { | ||
| return { clientId: request.body.client_id, clientSecret: request.body.client_secret }; | ||
| } | ||
|
|
||
| throw new InvalidClientError('Invalid client: cannot retrieve client credentials'); | ||
| } | ||
|
|
||
| /** | ||
| * Revoke the token. | ||
| * | ||
| * Attempts to find the token using the token_type_hint, then calls model.revokeToken(). | ||
| * Per RFC 7009, if the token cannot be found, we still return success to prevent | ||
| * token enumeration attacks. | ||
| */ | ||
|
|
||
| async revokeToken (token, tokenTypeHint, client) { | ||
| let tokenToRevoke = null; | ||
|
|
||
| // Try to find the token based on the hint | ||
| if (tokenTypeHint === 'refresh_token') { | ||
| // Try to get refresh token if model supports it | ||
| if (this.model.getRefreshToken) { | ||
| const refreshToken = await this.model.getRefreshToken(token); | ||
| if (refreshToken) { | ||
| // Verify the token belongs to the client | ||
| if (refreshToken.client && refreshToken.client.id === client.id) { | ||
| tokenToRevoke = refreshToken; | ||
| } | ||
| } | ||
| } | ||
| } else if (tokenTypeHint === 'access_token') { | ||
| // Try to get access token if model supports it | ||
| if (this.model.getAccessToken) { | ||
| const accessToken = await this.model.getAccessToken(token); | ||
| if (accessToken) { | ||
| // Verify the token belongs to the client | ||
| if (accessToken.client && accessToken.client.id === client.id) { | ||
| tokenToRevoke = accessToken; | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| // No hint provided, try both access token and refresh token | ||
| if (this.model.getAccessToken) { | ||
| const accessToken = await this.model.getAccessToken(token); | ||
| if (accessToken && accessToken.client && accessToken.client.id === client.id) { | ||
| tokenToRevoke = accessToken; | ||
| } | ||
| } | ||
|
|
||
| // If access token not found, try refresh token | ||
| if (!tokenToRevoke && this.model.getRefreshToken) { | ||
| const refreshToken = await this.model.getRefreshToken(token); | ||
| if (refreshToken && refreshToken.client && refreshToken.client.id === client.id) { | ||
| tokenToRevoke = refreshToken; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If we found a token, revoke it | ||
| if (tokenToRevoke) { | ||
| await this.model.revokeToken(tokenToRevoke); | ||
| } | ||
|
|
||
| // Per RFC 7009, we return success even if token was not found | ||
| // This prevents token enumeration attacks | ||
| } | ||
|
|
||
| /** | ||
| * Update response when token is revoked successfully. | ||
| */ | ||
|
|
||
| updateSuccessResponse (response) { | ||
| response.body = {}; | ||
| response.status = 200; | ||
| response.set('Cache-Control', 'no-store'); | ||
| response.set('Pragma', 'no-cache'); | ||
| } | ||
|
|
||
| /** | ||
| * Update response when an error is thrown. | ||
| */ | ||
|
|
||
| updateErrorResponse (response, error) { | ||
| response.body = { | ||
| error: error.name, | ||
| error_description: error.message | ||
| }; | ||
|
|
||
| response.status = error.code; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Export constructor. | ||
| */ | ||
|
|
||
| module.exports = RevokeHandler; | ||
|
|
||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just read it again and you're right.