From e3a2a5da43dfe4d4d15cee23dbc0b5729a493bcd Mon Sep 17 00:00:00 2001 From: Nicolas Froidure Date: Tue, 18 Aug 2020 09:29:14 +0200 Subject: [PATCH] feat(oauth2): add pkce support fix #58 --- .../getOAuth2Authorize.test.ts.snap | 5 +- .../src/handlers/getOAuth2Authorize.ts | 53 ++++++- .../src/handlers/postOAuth2Token.ts | 4 + packages/whook-oauth2/src/index.test.ts | 4 + packages/whook-oauth2/src/index.ts | 13 +- .../src/services/oAuth2CodeGranter.test.ts | 139 +++++++++++++++++- .../src/services/oAuth2CodeGranter.ts | 72 +++++++-- .../src/services/oAuth2Granters.ts | 23 ++- 8 files changed, 288 insertions(+), 25 deletions(-) diff --git a/packages/whook-oauth2/src/handlers/__snapshots__/getOAuth2Authorize.test.ts.snap b/packages/whook-oauth2/src/handlers/__snapshots__/getOAuth2Authorize.test.ts.snap index d95cc530..c47998c4 100644 --- a/packages/whook-oauth2/src/handlers/__snapshots__/getOAuth2Authorize.test.ts.snap +++ b/packages/whook-oauth2/src/handlers/__snapshots__/getOAuth2Authorize.test.ts.snap @@ -11,7 +11,10 @@ Object { "redirectURI": "https://www.example.com", "scope": "user", }, - Object {}, + Object { + "codeChallenge": "", + "codeChallengeMethod": "plain", + }, ], ], "logCalls": Array [], diff --git a/packages/whook-oauth2/src/handlers/getOAuth2Authorize.ts b/packages/whook-oauth2/src/handlers/getOAuth2Authorize.ts index 33d5419f..f8b8240c 100644 --- a/packages/whook-oauth2/src/handlers/getOAuth2Authorize.ts +++ b/packages/whook-oauth2/src/handlers/getOAuth2Authorize.ts @@ -13,6 +13,7 @@ import type { OAuth2GranterService, } from '../services/oAuth2Granters'; import type { LogService } from 'common-services'; +import { CODE_CHALLENGE_METHODS } from '../services/oAuth2CodeGranter'; /* Architecture Note #1: OAuth2 authorize This endpoint simply redirect the user to the authentication @@ -78,6 +79,29 @@ export const stateParameter: WhookAPIParameterDefinition = { }, }, }; +export const codeChallengeParameter: WhookAPIParameterDefinition = { + name: 'code_challenge', + parameter: { + in: 'query', + name: 'code_challenge', + required: false, + schema: { + type: 'string', + }, + }, +}; +export const codeChallengeMethodParameter: WhookAPIParameterDefinition = { + name: 'code_challenge_method', + parameter: { + in: 'query', + name: 'code_challenge_method', + required: false, + schema: { + type: 'string', + enum: CODE_CHALLENGE_METHODS, + }, + }, +}; export const definition: WhookAPIHandlerDefinition = { method: 'get', @@ -103,6 +127,12 @@ export const definition: WhookAPIHandlerDefinition = { { $ref: `#/components/parameters/${stateParameter.name}`, }, + { + $ref: `#/components/parameters/${codeChallengeParameter.name}`, + }, + { + $ref: `#/components/parameters/${codeChallengeMethodParameter.name}`, + }, ], responses: { '302': { @@ -132,6 +162,8 @@ async function getOAuth2Authorize( redirect_uri: demandedRedirectURI = '', scope: demandedScope = '', state, + code_challenge: codeChallenge = '', + code_challenge_method: codeChallengeMethod = 'plain', ...authorizeParameters }: { response_type: string; @@ -139,6 +171,8 @@ async function getOAuth2Authorize( redirect_uri?: string; scope?: string; state: string; + code_challenge?: string; + code_challenge_method?: string; authorizeParameters?: { [name: string]: unknown }; }, ): Promise { @@ -153,6 +187,15 @@ async function getOAuth2Authorize( if (!granter) { throw new YError('E_UNKNOWN_AUTHORIZER_TYPE', responseType); } + if (responseType === 'code') { + if (!codeChallenge) { + if (OAUTH2.forcePKCE) { + throw new YError('E_PKCE_REQUIRED', responseType); + } + } + } else if (codeChallenge) { + throw new YError('E_PKCE_NOT_SUPPORTED', responseType); + } const { applicationId, @@ -164,7 +207,15 @@ async function getOAuth2Authorize( redirectURI: demandedRedirectURI, scope: demandedScope, }, - camelCaseObjectProperties(authorizeParameters), + camelCaseObjectProperties({ + ...authorizeParameters, + ...(responseType === 'code' + ? { + codeChallenge, + codeChallengeMethod, + } + : {}), + }), ); url.searchParams.set('type', responseType); diff --git a/packages/whook-oauth2/src/handlers/postOAuth2Token.ts b/packages/whook-oauth2/src/handlers/postOAuth2Token.ts index 4e0e1644..33afd24b 100644 --- a/packages/whook-oauth2/src/handlers/postOAuth2Token.ts +++ b/packages/whook-oauth2/src/handlers/postOAuth2Token.ts @@ -41,6 +41,10 @@ export const authorizationCodeTokenRequestBodySchema: WhookAPISchemaDefinition = type: 'string', format: 'uri', }, + code_verifier: { + type: 'string', + pattern: '^[\\d\\w\\-/\\._~]+$', + }, }, }, }; diff --git a/packages/whook-oauth2/src/index.test.ts b/packages/whook-oauth2/src/index.test.ts index a0d9a7f7..70424c70 100644 --- a/packages/whook-oauth2/src/index.test.ts +++ b/packages/whook-oauth2/src/index.test.ts @@ -23,6 +23,8 @@ import { getOAuth2AuthorizeRedirectURIParameter, getOAuth2AuthorizeScopeParameter, getOAuth2AuthorizeStateParameter, + getOAuth2AuthorizeCodeChallengeParameter, + getOAuth2AuthorizeCodeChallengeMethodParameter, initPostOAuth2Acknowledge, postOAuth2AcknowledgeDefinition, initPostOAuth2Token, @@ -108,6 +110,8 @@ describe('OAuth2 server', () => { getOAuth2AuthorizeRedirectURIParameter, getOAuth2AuthorizeScopeParameter, getOAuth2AuthorizeStateParameter, + getOAuth2AuthorizeCodeChallengeParameter, + getOAuth2AuthorizeCodeChallengeMethodParameter, ].reduce( (parametersHash, { name, parameter }) => ({ ...parametersHash, diff --git a/packages/whook-oauth2/src/index.ts b/packages/whook-oauth2/src/index.ts index cd8888a7..4e5c0cb6 100644 --- a/packages/whook-oauth2/src/index.ts +++ b/packages/whook-oauth2/src/index.ts @@ -5,6 +5,8 @@ import initGetOAuth2Authorize, { redirectURIParameter as getOAuth2AuthorizeRedirectURIParameter, scopeParameter as getOAuth2AuthorizeScopeParameter, stateParameter as getOAuth2AuthorizeStateParameter, + codeChallengeParameter as getOAuth2AuthorizeCodeChallengeParameter, + codeChallengeMethodParameter as getOAuth2AuthorizeCodeChallengeMethodParameter, } from './handlers/getOAuth2Authorize'; import initPostOAuth2Acknowledge, { definition as postOAuth2AcknowledgeDefinition, @@ -21,10 +23,14 @@ import initOAuth2Granters, { OAUTH2_ERRORS_DESCRIPTORS, } from './services/oAuth2Granters'; import initOAuth2ClientCredentialsGranter from './services/oAuth2ClientCredentialsGranter'; -import initOAuth2CodeGranter from './services/oAuth2CodeGranter'; +import initOAuth2CodeGranter, { + base64UrlEncode, + hashCodeVerifier, +} from './services/oAuth2CodeGranter'; import initOAuth2PasswordGranter from './services/oAuth2PasswordGranter'; import initOAuth2RefreshTokenGranter from './services/oAuth2RefreshTokenGranter'; import initOAuth2TokenGranter from './services/oAuth2TokenGranter'; +import type { CodeChallengeMethod } from './services/oAuth2CodeGranter'; import type { OAuth2CodeService, OAuth2PasswordService, @@ -56,6 +62,7 @@ import type { } from './services/authCookies'; export type { + CodeChallengeMethod, OAuth2CodeService, OAuth2PasswordService, OAuth2AccessTokenService, @@ -77,6 +84,10 @@ export { getOAuth2AuthorizeRedirectURIParameter, getOAuth2AuthorizeScopeParameter, getOAuth2AuthorizeStateParameter, + getOAuth2AuthorizeCodeChallengeParameter, + getOAuth2AuthorizeCodeChallengeMethodParameter, + base64UrlEncode, + hashCodeVerifier, initPostOAuth2Acknowledge, postOAuth2AcknowledgeDefinition, initPostOAuth2Token, diff --git a/packages/whook-oauth2/src/services/oAuth2CodeGranter.test.ts b/packages/whook-oauth2/src/services/oAuth2CodeGranter.test.ts index 19167280..9cb763e1 100644 --- a/packages/whook-oauth2/src/services/oAuth2CodeGranter.test.ts +++ b/packages/whook-oauth2/src/services/oAuth2CodeGranter.test.ts @@ -1,5 +1,7 @@ -import initOAuth2CodeGranter from './oAuth2CodeGranter'; -import type { BaseAuthenticationData } from '@whook/authorization'; +import initOAuth2CodeGranter, { + base64UrlEncode, + hashCodeVerifier, +} from './oAuth2CodeGranter'; describe('OAuth2CodeGranter', () => { const oAuth2Code = { @@ -32,11 +34,17 @@ describe('OAuth2CodeGranter', () => { scope: 'user', }); - const authorizerResult = await oAuth2CodeGranter.authorizer.authorize({ - clientId: 'abbacaca-abba-caca-abba-cacaabbacaca', - redirectURI: 'https://www.example.com/oauth2/code', - scope: 'user', - }); + const authorizerResult = await oAuth2CodeGranter.authorizer.authorize( + { + clientId: 'abbacaca-abba-caca-abba-cacaabbacaca', + redirectURI: 'https://www.example.com/oauth2/code', + scope: 'user', + }, + { + codeChallenge: '', + codeChallengeMethod: 'plain', + }, + ); const acknowledgerResult = await oAuth2CodeGranter.acknowledger.acknowledge( { applicationId: 'abbacaca-abba-caca-abba-cacaabbacaca', @@ -47,13 +55,17 @@ describe('OAuth2CodeGranter', () => { redirectURI: 'https://www.example.com/oauth2/code', scope: 'user', }, - {}, + { + codeChallenge: '', + codeChallengeMethod: 'plain', + }, ); const authenticatorResult = await oAuth2CodeGranter.authenticator.authenticate( { clientId: 'abbacaca-abba-caca-abba-cacaabbacaca', redirectURI: 'https://www.example.com/oauth2/code', code: 'yolo', + codeVerifier: '', }, { applicationId: 'abbacaca-abba-caca-abba-cacaabbacaca', @@ -79,6 +91,8 @@ describe('OAuth2CodeGranter', () => { }, "authorizerResult": Object { "applicationId": "abbacaca-abba-caca-abba-cacaabbacaca", + "codeChallenge": undefined, + "codeChallengeMethod": undefined, "redirectURI": "https://www.example.com", "scope": "user", }, @@ -92,3 +106,112 @@ describe('OAuth2CodeGranter', () => { }).toMatchSnapshot(); }); }); + +describe('base64UrlEncode()', () => { + test('should work like here https://tools.ietf.org/html/rfc7636#appendix-A', () => { + expect( + base64UrlEncode( + Buffer.from([ + 116, + 24, + 223, + 180, + 151, + 153, + 224, + 37, + 79, + 250, + 96, + 125, + 216, + 173, + 187, + 186, + 22, + 212, + 37, + 77, + 105, + 214, + 191, + 240, + 91, + 88, + 5, + 88, + 83, + 132, + 141, + 121, + ]), + ), + ).toEqual('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'); + }); +}); + +describe('base64UrlEncode()', () => { + test('should work with plain method', () => { + expect( + hashCodeVerifier( + Buffer.from('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'), + 'plain', + ), + ).toEqual(Buffer.from('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk')); + }); + + test('should work with S256 like here https://tools.ietf.org/html/rfc7636#appendix-A', () => { + expect( + hashCodeVerifier( + Buffer.from('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'), + 'S256', + ), + ).toEqual( + Buffer.from([ + 19, + 211, + 30, + 150, + 26, + 26, + 216, + 236, + 47, + 22, + 177, + 12, + 76, + 152, + 46, + 8, + 118, + 168, + 120, + 173, + 109, + 241, + 68, + 86, + 110, + 225, + 137, + 74, + 203, + 112, + 249, + 195, + ]), + ); + }); + + test('should work base64 url encode like here https://tools.ietf.org/html/rfc7636#appendix-A', () => { + expect( + base64UrlEncode( + hashCodeVerifier( + Buffer.from('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'), + 'S256', + ), + ), + ).toEqual('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM'); + }); +}); diff --git a/packages/whook-oauth2/src/services/oAuth2CodeGranter.ts b/packages/whook-oauth2/src/services/oAuth2CodeGranter.ts index dfa2675e..dcd4eb74 100644 --- a/packages/whook-oauth2/src/services/oAuth2CodeGranter.ts +++ b/packages/whook-oauth2/src/services/oAuth2CodeGranter.ts @@ -1,5 +1,6 @@ import { autoService } from 'knifecycle'; import { noop } from '@whook/whook'; +import { createHash } from 'crypto'; import YError from 'yerror'; import type { LogService } from 'common-services'; import type { @@ -10,23 +11,40 @@ import type { import type { BaseAuthenticationData } from '@whook/authorization'; export const CODE_GRANTER_TYPE = 'code'; +export const CODE_CHALLENGE_METHODS = ['plain', 'S256'] as const; +export type CodeChallengeMethod = typeof CODE_CHALLENGE_METHODS[number]; export type OAuth2CodeGranterDependencies = { oAuth2Code: OAuth2CodeService; checkApplication: CheckApplicationService; log?: LogService; }; -export type OAuth2CodeGranterParameters = { +export type OAuth2CodeGranterAuthorizeParameters = { + codeChallenge: string; + codeChallengeMethod: CodeChallengeMethod; +}; +export type OAuth2CodeGranterAcknowledgeParameters = { + codeChallenge: string; + codeChallengeMethod: CodeChallengeMethod; +}; +export type OAuth2CodeGranterGrantParameters = { code: string; redirectURI: string; clientId: string; + codeVerifier: string; }; export type OAuth2CodeGranterService< AUTHENTICATION_DATA extends BaseAuthenticationData = BaseAuthenticationData > = OAuth2GranterService< - unknown, - { [name: string]: unknown }, - OAuth2CodeGranterParameters, + OAuth2CodeGranterAuthorizeParameters & { + [name: string]: unknown; + }, + OAuth2CodeGranterAcknowledgeParameters & { + [name: string]: unknown; + }, + OAuth2CodeGranterGrantParameters & { + [name: string]: unknown; + }, AUTHENTICATION_DATA >; @@ -39,11 +57,10 @@ async function initOAuth2CodeGranter({ checkApplication, log = noop, }: OAuth2CodeGranterDependencies): Promise { - const authorizeWithCode: OAuth2CodeGranterService['authorizer']['authorize'] = async ({ - clientId, - redirectURI, - scope = '', - }) => { + const authorizeWithCode: OAuth2CodeGranterService['authorizer']['authorize'] = async ( + { clientId, redirectURI, scope = '' }, + { codeChallenge, codeChallengeMethod }, + ) => { const { redirectURI: finalRedirectURI } = await checkApplication({ applicationId: clientId, type: CODE_GRANTER_TYPE, @@ -55,6 +72,8 @@ async function initOAuth2CodeGranter({ applicationId: clientId, redirectURI: finalRedirectURI, scope, + codeChallenge, + codeChallengeMethod, }; }; @@ -63,12 +82,20 @@ async function initOAuth2CodeGranter({ const acknowledgeWithCode: OAuth2CodeGranterService['acknowledger']['acknowledge'] = async ( authenticationData, { clientId, redirectURI, scope }, - additionalParameters, + { + codeChallenge = '', + codeChallengeMethod = 'plain', + ...additionalParameters + }, ) => { const code = await oAuth2Code.create( { ...authenticationData, applicationId: clientId, scope }, redirectURI, - additionalParameters, + { + codeChallenge, + codeChallengeMethod, + ...additionalParameters, + }, ); return { @@ -80,7 +107,7 @@ async function initOAuth2CodeGranter({ }; const authenticateWithCode: OAuth2CodeGranterService['authenticator']['authenticate'] = async ( - { code, clientId, redirectURI }, + { code, clientId, redirectURI, codeVerifier }, authenticationData, ) => { // The client must be authenticated (for now, see below) @@ -89,7 +116,7 @@ async function initOAuth2CodeGranter({ } // This check is not really necessary atm but it acts - // as a reminde that this grant type could be used + // as a reminder that this grant type could be used // without authenticating the client. In this // scenario, the authenticationData should be deducted // from the clientId and the code given in parameters @@ -112,6 +139,7 @@ async function initOAuth2CodeGranter({ authenticationData, code, redirectURI, + codeVerifier, ); return newAuthenticationData; @@ -135,3 +163,21 @@ async function initOAuth2CodeGranter({ }, }; } + +// See https://tools.ietf.org/html/rfc7636#appendix-A +export function base64UrlEncode(buf: Buffer): string { + let s = buf.toString('base64'); + s = s.split('=')[0]; + s = s.replace('+', '-'); + s = s.replace('/', '_'); + return s; +} + +export function hashCodeVerifier( + codeVerifier: Buffer, + method: CodeChallengeMethod, +): Buffer { + return 'plain' === method + ? codeVerifier + : createHash('sha256').update(codeVerifier).digest(); +} diff --git a/packages/whook-oauth2/src/services/oAuth2Granters.ts b/packages/whook-oauth2/src/services/oAuth2Granters.ts index a213ddb4..f1a4ec58 100644 --- a/packages/whook-oauth2/src/services/oAuth2Granters.ts +++ b/packages/whook-oauth2/src/services/oAuth2Granters.ts @@ -2,6 +2,7 @@ import { initializer } from 'knifecycle'; import { DEFAULT_ERROR_URI, DEFAULT_HELP_URI } from '@whook/whook'; import type { WhookErrorsDescriptors } from '@whook/whook'; import type { BaseAuthenticationData } from '@whook/authorization'; +import { CodeChallengeMethod } from './oAuth2CodeGranter'; export const OAUTH2_ERRORS_DESCRIPTORS: WhookErrorsDescriptors = { E_UNKNOWN_AUTHORIZER_TYPE: { @@ -53,6 +54,20 @@ export const OAUTH2_ERRORS_DESCRIPTORS: WhookErrorsDescriptors = { uri: DEFAULT_ERROR_URI, help: DEFAULT_HELP_URI, }, + E_PKCE_REQUIRED: { + code: 'invalid_request', + status: 400, + description: 'Code challenge required', + uri: DEFAULT_ERROR_URI, + help: DEFAULT_HELP_URI, + }, + E_PKCE_NOT_SUPPORTED: { + code: 'invalid_request', + status: 400, + description: 'Code challenge not supported for that response type ($0)', + uri: DEFAULT_ERROR_URI, + help: DEFAULT_HELP_URI, + }, E_UNAUTHORIZED_CLIENT: { code: 'invalid_client', status: 401, @@ -129,12 +144,17 @@ export type OAuth2CodeService< create: ( authenticationData: AUTHENTICATION_DATA, redirectURI: string, - additionalParameters: { [name: string]: unknown }, + additionalParameters: { + codeChallenge: string; + codeChallengeMethod: CodeChallengeMethod; + [name: string]: unknown; + }, ) => Promise; check: ( authenticationData: AUTHENTICATION_DATA, code: CODE, redirectURI: string, + codeVerifier?: string, ) => Promise< AUTHENTICATION_DATA & { redirectURI: string; @@ -243,6 +263,7 @@ export type OAuth2GranterService< export type OAuth2Options = { authenticateURL: string; + forcePKCE?: boolean; }; export type OAuth2Config = {