From 686d1934fbf79aae05231984533f28b85c2365b6 Mon Sep 17 00:00:00 2001 From: Matt Bacalakis Date: Thu, 3 Nov 2022 18:52:26 -0400 Subject: [PATCH 1/4] Adding client id to _auth_verification cookie key to support multiple clients on same protocol and domain. --- lib/context.js | 16 +-- middleware/auth.js | 2 +- test/callback.tests.js | 285 ++++++++++++++++++++++++++--------------- test/login.tests.js | 7 +- 4 files changed, 192 insertions(+), 118 deletions(-) diff --git a/lib/context.js b/lib/context.js index 77ee5f3a..62f40ac9 100644 --- a/lib/context.js +++ b/lib/context.js @@ -58,13 +58,8 @@ function tokenSet() { const cachedTokenSet = weakRef(session); if (!('value' in cachedTokenSet)) { - const { - id_token, - access_token, - refresh_token, - token_type, - expires_at, - } = session; + const { id_token, access_token, refresh_token, token_type, expires_at } = + session; cachedTokenSet.value = new TokenSet({ id_token, access_token, @@ -215,9 +210,8 @@ class ResponseContext { stateValue.attemptingSilentLogin = true; } - const usePKCE = options.authorizationParams.response_type.includes( - 'code' - ); + const usePKCE = + options.authorizationParams.response_type.includes('code'); if (usePKCE) { debug( 'response_type includes code, the authorization request will use PKCE' @@ -258,7 +252,7 @@ class ResponseContext { ); } - transient.store('auth_verification', req, res, { + transient.store('auth_verification.' + config.clientID, req, res, { sameSite: options.authorizationParams.response_mode === 'form_post' ? 'None' diff --git a/middleware/auth.js b/middleware/auth.js index d4928bc1..5cd82ecd 100644 --- a/middleware/auth.js +++ b/middleware/auth.js @@ -89,7 +89,7 @@ const auth = function (params) { try { const callbackParams = client.callbackParams(req); const authVerification = transient.getOnce( - 'auth_verification', + 'auth_verification.' + config.clientID, req, res ); diff --git a/test/callback.tests.js b/test/callback.tests.js index f4948991..662cbaa3 100644 --- a/test/callback.tests.js +++ b/test/callback.tests.js @@ -30,8 +30,8 @@ const defaultConfig = { }; let server; -const generateCookies = (values) => ({ - auth_verification: JSON.stringify(values), +const generateCookies = (values, clientID) => ({ + ['auth_verification.' + clientID]: JSON.stringify(values), }); const setup = async (params) => { @@ -145,10 +145,13 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies({ - nonce: '__test_nonce__', - state: '__test_state__', - }), + cookies: generateCookies( + { + nonce: '__test_nonce__', + state: '__test_state__', + }, + clientID + ), body: true, }); assert.equal(statusCode, 400); @@ -179,10 +182,13 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies({ - nonce: '__test_nonce__', - state: '__valid_state__', - }), + cookies: generateCookies( + { + nonce: '__test_nonce__', + state: '__valid_state__', + }, + clientID + ), body: { state: '__invalid_state__', }, @@ -198,10 +204,13 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies({ - nonce: '__test_nonce__', - state: '__test_state__', - }), + cookies: generateCookies( + { + nonce: '__test_nonce__', + state: '__test_state__', + }, + clientID + ), body: { state: '__test_state__', id_token: '__invalid_token__', @@ -221,10 +230,13 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies({ - nonce: '__test_nonce__', - state: '__test_state__', - }), + cookies: generateCookies( + { + nonce: '__test_nonce__', + state: '__test_state__', + }, + clientID + ), body: { state: '__test_state__', id_token: jose.JWT.sign({ sub: '__test_sub__' }, 'secret', { @@ -243,10 +255,13 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies({ - nonce: '__test_nonce__', - state: '__test_state__', - }), + cookies: generateCookies( + { + nonce: '__test_nonce__', + state: '__test_state__', + }, + clientID + ), body: { state: '__test_state__', id_token: makeIdToken({ iss: undefined }), @@ -263,9 +278,12 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies({ - state: '__test_state__', - }), + cookies: generateCookies( + { + state: '__test_state__', + }, + clientID + ), body: { state: '__test_state__', id_token: makeIdToken(), @@ -287,7 +305,9 @@ describe('callback response_mode: form_post', () => { legacySameSiteCookie: false, }, cookies: { - _auth_verification: JSON.stringify({ state: '__test_state__' }), + ['_auth_verification.' + clientID]: JSON.stringify({ + state: '__test_state__', + }), }, body: { state: '__test_state__', @@ -324,7 +344,7 @@ describe('callback response_mode: form_post', () => { identityClaimFilter: [], }, cookies: { - _auth_verification: JSON.stringify({ + ['_auth_verification.' + clientID]: JSON.stringify({ state: expectedDefaultState, nonce: '__test_nonce__', }), @@ -343,10 +363,13 @@ describe('callback response_mode: form_post', () => { authOpts: { identityClaimFilter: [], }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: makeIdToken(), @@ -366,10 +389,13 @@ describe('callback response_mode: form_post', () => { currentUser, tokens, } = await setup({ - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -408,10 +434,13 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -446,10 +475,13 @@ describe('callback response_mode: form_post', () => { response_type: 'code', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -504,10 +536,13 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -588,10 +623,13 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -666,10 +704,13 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -747,10 +788,13 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -790,10 +834,13 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -824,10 +871,13 @@ describe('callback response_mode: form_post', () => { }, clientAssertionSigningKey: privateKey, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -859,10 +909,13 @@ describe('callback response_mode: form_post', () => { }, clientAuthMethod: 'client_secret_jwt', }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -886,11 +939,14 @@ describe('callback response_mode: form_post', () => { const jar = request.jar(); jar.setCookie('skipSilentLogin=true', baseUrl); await setup({ - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - skipSilentLogin: '1', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + skipSilentLogin: '1', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -945,10 +1001,13 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email', }, }, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -987,10 +1046,13 @@ describe('callback response_mode: form_post', () => { } = await setup({ router: auth(authOpts), authOpts, - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: idToken, @@ -1004,10 +1066,13 @@ describe('callback response_mode: form_post', () => { it('should replace the cookie session when a new user is logging in over an existing different user', async () => { const { currentSession, currentUser } = await setup({ - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'bar' }), @@ -1024,10 +1089,13 @@ describe('callback response_mode: form_post', () => { it('should preserve the cookie session when a new user is logging in over an anonymous session', async () => { const { currentSession, currentUser } = await setup({ - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'foo' }), @@ -1047,10 +1115,13 @@ describe('callback response_mode: form_post', () => { }); const { currentSession, currentUser, existingSessionCookie, jar } = await setup({ - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'foo' }), @@ -1084,10 +1155,13 @@ describe('callback response_mode: form_post', () => { }); const { currentSession, currentUser, existingSessionCookie, jar } = await setup({ - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'foo' }), @@ -1122,10 +1196,13 @@ describe('callback response_mode: form_post', () => { }); const { currentSession, currentUser, existingSessionCookie, jar } = await setup({ - cookies: generateCookies({ - state: expectedDefaultState, - nonce: '__test_nonce__', - }), + cookies: generateCookies( + { + state: expectedDefaultState, + nonce: '__test_nonce__', + }, + clientID + ), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'bar' }), diff --git a/test/login.tests.js b/test/login.tests.js index 8daf31f3..622f1365 100644 --- a/test/login.tests.js +++ b/test/login.tests.js @@ -20,7 +20,8 @@ const filterRoute = (method, path) => { const fetchAuthCookie = (res) => { const cookieHeaders = res.headers['set-cookie']; return cookieHeaders.filter( - (header) => header.split('=')[0] === 'auth_verification' + (header) => + header.split('=')[0] === 'auth_verification.' + defaultConfig.clientID )[0]; }; @@ -32,7 +33,9 @@ const fetchFromAuthCookie = (res, cookieName) => { } const decodedAuthCookie = querystring.decode(authCookie); - const cookieValuePart = decodedAuthCookie.auth_verification + const cookieValuePart = decodedAuthCookie[ + 'auth_verification.' + defaultConfig.clientID + ] .split('; ')[0] .split('.')[0]; const authCookieParsed = JSON.parse(cookieValuePart); From 8e72e25fb68fb253fb6397209dcfe1148acfbb5c Mon Sep 17 00:00:00 2001 From: Matt Bacalakis Date: Tue, 8 Nov 2022 15:01:40 -0500 Subject: [PATCH 2/4] Added support for applying custom name to txn cookie. --- lib/config.js | 1 + lib/context.js | 2 +- middleware/auth.js | 2 +- test/callback.tests.js | 311 ++++++++++++++++++----------------------- test/config.tests.js | 5 + test/login.tests.js | 136 ++++++++++++++++-- 6 files changed, 274 insertions(+), 183 deletions(-) diff --git a/lib/config.js b/lib/config.js index eb653328..d14a4feb 100644 --- a/lib/config.js +++ b/lib/config.js @@ -85,6 +85,7 @@ const paramsSchema = Joi.object({ .valid('Lax', 'Strict', 'None') .optional() .default(Joi.ref('...session.cookie.sameSite')), + name: Joi.string().optional().default('auth_verification'), }) .default() .unknown(false), diff --git a/lib/context.js b/lib/context.js index 62f40ac9..e717bbf5 100644 --- a/lib/context.js +++ b/lib/context.js @@ -252,7 +252,7 @@ class ResponseContext { ); } - transient.store('auth_verification.' + config.clientID, req, res, { + transient.store(config.transactionCookie.name, req, res, { sameSite: options.authorizationParams.response_mode === 'form_post' ? 'None' diff --git a/middleware/auth.js b/middleware/auth.js index 5cd82ecd..04195f58 100644 --- a/middleware/auth.js +++ b/middleware/auth.js @@ -89,7 +89,7 @@ const auth = function (params) { try { const callbackParams = client.callbackParams(req); const authVerification = transient.getOnce( - 'auth_verification.' + config.clientID, + config.transactionCookie.name, req, res ); diff --git a/test/callback.tests.js b/test/callback.tests.js index 662cbaa3..f79f7846 100644 --- a/test/callback.tests.js +++ b/test/callback.tests.js @@ -30,8 +30,8 @@ const defaultConfig = { }; let server; -const generateCookies = (values, clientID) => ({ - ['auth_verification.' + clientID]: JSON.stringify(values), +const generateCookies = (values, customTxnCookieName) => ({ + [customTxnCookieName || 'auth_verification']: JSON.stringify(values), }); const setup = async (params) => { @@ -145,13 +145,10 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies( - { - nonce: '__test_nonce__', - state: '__test_state__', - }, - clientID - ), + cookies: generateCookies({ + nonce: '__test_nonce__', + state: '__test_state__', + }), body: true, }); assert.equal(statusCode, 400); @@ -182,13 +179,10 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies( - { - nonce: '__test_nonce__', - state: '__valid_state__', - }, - clientID - ), + cookies: generateCookies({ + nonce: '__test_nonce__', + state: '__valid_state__', + }), body: { state: '__invalid_state__', }, @@ -204,13 +198,10 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies( - { - nonce: '__test_nonce__', - state: '__test_state__', - }, - clientID - ), + cookies: generateCookies({ + nonce: '__test_nonce__', + state: '__test_state__', + }), body: { state: '__test_state__', id_token: '__invalid_token__', @@ -230,13 +221,10 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies( - { - nonce: '__test_nonce__', - state: '__test_state__', - }, - clientID - ), + cookies: generateCookies({ + nonce: '__test_nonce__', + state: '__test_state__', + }), body: { state: '__test_state__', id_token: jose.JWT.sign({ sub: '__test_sub__' }, 'secret', { @@ -255,13 +243,10 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies( - { - nonce: '__test_nonce__', - state: '__test_state__', - }, - clientID - ), + cookies: generateCookies({ + nonce: '__test_nonce__', + state: '__test_state__', + }), body: { state: '__test_state__', id_token: makeIdToken({ iss: undefined }), @@ -278,12 +263,9 @@ describe('callback response_mode: form_post', () => { body: { err }, }, } = await setup({ - cookies: generateCookies( - { - state: '__test_state__', - }, - clientID - ), + cookies: generateCookies({ + state: '__test_state__', + }), body: { state: '__test_state__', id_token: makeIdToken(), @@ -344,7 +326,7 @@ describe('callback response_mode: form_post', () => { identityClaimFilter: [], }, cookies: { - ['_auth_verification.' + clientID]: JSON.stringify({ + auth_verification: JSON.stringify({ state: expectedDefaultState, nonce: '__test_nonce__', }), @@ -363,13 +345,10 @@ describe('callback response_mode: form_post', () => { authOpts: { identityClaimFilter: [], }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: makeIdToken(), @@ -383,6 +362,42 @@ describe('callback response_mode: form_post', () => { }); it('should expose the id token when id_token is valid', async () => { + const idToken = makeIdToken(); + const { + response: { statusCode, headers }, + currentUser, + tokens, + } = await setup({ + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), + body: { + state: expectedDefaultState, + id_token: idToken, + }, + }); + assert.equal(statusCode, 302); + assert.equal(headers.location, 'https://example.org'); + assert.ok(currentUser); + assert.equal(currentUser.sub, '__test_sub__'); + assert.equal(currentUser.nickname, '__test_nickname__'); + assert.notExists(currentUser.iat); + assert.notExists(currentUser.iss); + assert.notExists(currentUser.aud); + assert.notExists(currentUser.exp); + assert.notExists(currentUser.nonce); + assert.equal(tokens.isAuthenticated, true); + assert.equal(tokens.idToken, idToken); + assert.isUndefined(tokens.refreshToken); + assert.isUndefined(tokens.accessToken); + assert.include(tokens.idTokenClaims, { + sub: '__test_sub__', + }); + }); + + it('should succeed even if custom transaction cookie name used', async () => { + let customTxnCookieName = 'CustomTxnCookie'; const idToken = makeIdToken(); const { response: { statusCode, headers }, @@ -394,12 +409,15 @@ describe('callback response_mode: form_post', () => { state: expectedDefaultState, nonce: '__test_nonce__', }, - clientID + customTxnCookieName ), body: { state: expectedDefaultState, id_token: idToken, }, + authOpts: { + transactionCookie: { name: customTxnCookieName }, + }, }); assert.equal(statusCode, 302); assert.equal(headers.location, 'https://example.org'); @@ -434,13 +452,10 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -475,13 +490,10 @@ describe('callback response_mode: form_post', () => { response_type: 'code', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -536,13 +548,10 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -623,13 +632,10 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -704,13 +710,10 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -788,13 +791,10 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -834,13 +834,10 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email read:reports offline_access', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -871,13 +868,10 @@ describe('callback response_mode: form_post', () => { }, clientAssertionSigningKey: privateKey, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -909,13 +903,10 @@ describe('callback response_mode: form_post', () => { }, clientAuthMethod: 'client_secret_jwt', }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -939,14 +930,11 @@ describe('callback response_mode: form_post', () => { const jar = request.jar(); jar.setCookie('skipSilentLogin=true', baseUrl); await setup({ - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - skipSilentLogin: '1', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + skipSilentLogin: '1', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -1001,13 +989,10 @@ describe('callback response_mode: form_post', () => { scope: 'openid profile email', }, }, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -1046,13 +1031,10 @@ describe('callback response_mode: form_post', () => { } = await setup({ router: auth(authOpts), authOpts, - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: idToken, @@ -1066,13 +1048,10 @@ describe('callback response_mode: form_post', () => { it('should replace the cookie session when a new user is logging in over an existing different user', async () => { const { currentSession, currentUser } = await setup({ - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'bar' }), @@ -1089,13 +1068,10 @@ describe('callback response_mode: form_post', () => { it('should preserve the cookie session when a new user is logging in over an anonymous session', async () => { const { currentSession, currentUser } = await setup({ - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'foo' }), @@ -1115,13 +1091,10 @@ describe('callback response_mode: form_post', () => { }); const { currentSession, currentUser, existingSessionCookie, jar } = await setup({ - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'foo' }), @@ -1155,13 +1128,10 @@ describe('callback response_mode: form_post', () => { }); const { currentSession, currentUser, existingSessionCookie, jar } = await setup({ - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'foo' }), @@ -1196,13 +1166,10 @@ describe('callback response_mode: form_post', () => { }); const { currentSession, currentUser, existingSessionCookie, jar } = await setup({ - cookies: generateCookies( - { - state: expectedDefaultState, - nonce: '__test_nonce__', - }, - clientID - ), + cookies: generateCookies({ + state: expectedDefaultState, + nonce: '__test_nonce__', + }), body: { state: expectedDefaultState, id_token: makeIdToken({ sub: 'bar' }), diff --git a/test/config.tests.js b/test/config.tests.js index d7f1e503..b130b686 100644 --- a/test/config.tests.js +++ b/test/config.tests.js @@ -187,6 +187,7 @@ describe('get config', () => { secret: ['__test_session_secret_1__', '__test_session_secret_2__'], transactionCookie: { sameSite: 'Strict', + name: 'auth_verification', }, session: { name: '__test_custom_session_name__', @@ -247,6 +248,7 @@ describe('get config', () => { secret: ['__test_session_secret_1__', '__test_session_secret_2__'], transactionCookie: { sameSite: 'Lax', + name: 'auth_verification', }, }); }); @@ -266,6 +268,7 @@ describe('get config', () => { secret: ['__test_session_secret_1__', '__test_session_secret_2__'], transactionCookie: { sameSite: 'Strict', + name: 'auth_verification', }, }); }); @@ -276,6 +279,7 @@ describe('get config', () => { secret: ['__test_session_secret_1__', '__test_session_secret_2__'], transactionCookie: { sameSite: 'Strict', + name: 'CustomTxnCookie', }, session: { cookie: { @@ -288,6 +292,7 @@ describe('get config', () => { secret: ['__test_session_secret_1__', '__test_session_secret_2__'], transactionCookie: { sameSite: 'Strict', + name: 'CustomTxnCookie', }, }); }); diff --git a/test/login.tests.js b/test/login.tests.js index 622f1365..42d4b0f3 100644 --- a/test/login.tests.js +++ b/test/login.tests.js @@ -17,25 +17,24 @@ const filterRoute = (method, path) => { r.route && r.route.path === path && r.route.methods[method.toLowerCase()]; }; -const fetchAuthCookie = (res) => { +const fetchAuthCookie = (res, txnCookieName) => { + txnCookieName = txnCookieName || 'auth_verification'; const cookieHeaders = res.headers['set-cookie']; return cookieHeaders.filter( - (header) => - header.split('=')[0] === 'auth_verification.' + defaultConfig.clientID + (header) => header.split('=')[0] === txnCookieName )[0]; }; -const fetchFromAuthCookie = (res, cookieName) => { - const authCookie = fetchAuthCookie(res); +const fetchFromAuthCookie = (res, cookieName, txnCookieName) => { + txnCookieName = txnCookieName || 'auth_verification'; + const authCookie = fetchAuthCookie(res, txnCookieName); if (!authCookie) { return false; } const decodedAuthCookie = querystring.decode(authCookie); - const cookieValuePart = decodedAuthCookie[ - 'auth_verification.' + defaultConfig.clientID - ] + const cookieValuePart = decodedAuthCookie[txnCookieName] .split('; ')[0] .split('.')[0]; const authCookieParsed = JSON.parse(cookieValuePart); @@ -106,6 +105,39 @@ describe('auth', () => { assert.equal(fetchFromAuthCookie(res, 'state'), parsed.query.state); }); + it('should redirect to the authorize url for /login when txn cookie name is custom', async () => { + let customTxnCookieName = 'CustomTxnCookie'; + + server = await createServer( + auth({ + ...defaultConfig, + transactionCookie: { name: customTxnCookieName }, + }) + ); + const res = await request.get('/login', { baseUrl, followRedirect: false }); + assert.equal(res.statusCode, 302); + + const parsed = url.parse(res.headers.location, true); + assert.equal(parsed.hostname, 'op.example.com'); + assert.equal(parsed.pathname, '/authorize'); + assert.equal(parsed.query.client_id, '__test_client_id__'); + assert.equal(parsed.query.scope, 'openid profile email'); + assert.equal(parsed.query.response_type, 'id_token'); + assert.equal(parsed.query.response_mode, 'form_post'); + assert.equal(parsed.query.redirect_uri, 'https://example.org/callback'); + assert.property(parsed.query, 'nonce'); + assert.property(parsed.query, 'state'); + + assert.equal( + fetchFromAuthCookie(res, 'nonce', customTxnCookieName), + parsed.query.nonce + ); + assert.equal( + fetchFromAuthCookie(res, 'state', customTxnCookieName), + parsed.query.state + ); + }); + it('should redirect to the authorize url for any route if authRequired', async () => { server = await createServer( auth({ @@ -135,6 +167,22 @@ describe('auth', () => { assert.equal(res.statusCode, 302); }); + it('should redirect to the authorize url for any route with custom txn name if attemptSilentLogin ', async () => { + server = await createServer( + auth({ + ...defaultConfig, + authRequired: false, + attemptSilentLogin: true, + transactionCookie: { name: 'CustomTxnCookie' }, + }) + ); + const res = await request.get('/session', { + baseUrl, + followRedirect: false, + }); + assert.equal(res.statusCode, 302); + }); + it('should redirect to the authorize url for /login in code flow', async () => { server = await createServer( auth({ @@ -165,6 +213,44 @@ describe('auth', () => { assert.equal(fetchFromAuthCookie(res, 'state'), parsed.query.state); }); + it('should redirect to the authorize url for /login in code flow with custom txn cookie', async () => { + let customTxnCookieName = 'CustomTxnCookie'; + server = await createServer( + auth({ + ...defaultConfig, + clientSecret: '__test_client_secret__', + authorizationParams: { + response_type: 'code', + }, + transactionCookie: { name: customTxnCookieName }, + }) + ); + const res = await request.get('/login', { baseUrl, followRedirect: false }); + assert.equal(res.statusCode, 302); + + const parsed = url.parse(res.headers.location, true); + + assert.equal(parsed.hostname, 'op.example.com'); + assert.equal(parsed.pathname, '/authorize'); + assert.equal(parsed.query.client_id, '__test_client_id__'); + assert.equal(parsed.query.scope, 'openid profile email'); + assert.equal(parsed.query.response_type, 'code'); + assert.equal(parsed.query.response_mode, undefined); + assert.equal(parsed.query.redirect_uri, 'https://example.org/callback'); + assert.property(parsed.query, 'nonce'); + assert.property(parsed.query, 'state'); + assert.property(res.headers, 'set-cookie'); + + assert.equal( + fetchFromAuthCookie(res, 'nonce', customTxnCookieName), + parsed.query.nonce + ); + assert.equal( + fetchFromAuthCookie(res, 'state', customTxnCookieName), + parsed.query.state + ); + }); + it('should redirect to the authorize url for /login in id_token flow', async () => { server = await createServer( auth({ @@ -300,7 +386,39 @@ describe('auth', () => { }); it('should not allow an invalid response_type', async function () { - const router = auth({ ...defaultConfig, routes: { login: false } }); + const router = auth({ + ...defaultConfig, + routes: { login: false }, + }); + router.get('/login', (req, res) => { + res.oidc.login({ + authorizationParams: { + response_type: 'invalid', + }, + }); + }); + server = await createServer(router); + + const cookieJar = request.jar(); + const res = await request.get('/login', { + cookieJar, + baseUrl, + json: true, + followRedirect: false, + }); + assert.equal(res.statusCode, 500); + assert.equal( + res.body.err.message, + 'response_type should be one of id_token, code id_token, code' + ); + }); + + it('should not allow an invalid response_type when txn cookie name custom', async function () { + const router = auth({ + ...defaultConfig, + routes: { login: false }, + transactionCookie: { name: 'CustomTxnCookie' }, + }); router.get('/login', (req, res) => { res.oidc.login({ authorizationParams: { From 7a2b268b8169ff4e9ddb5613c026fb45159c336e Mon Sep 17 00:00:00 2001 From: Matt Bacalakis Date: Tue, 8 Nov 2022 15:05:55 -0500 Subject: [PATCH 3/4] cleaning up remnant from clientId approach --- test/callback.tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/callback.tests.js b/test/callback.tests.js index f79f7846..661b950b 100644 --- a/test/callback.tests.js +++ b/test/callback.tests.js @@ -287,7 +287,7 @@ describe('callback response_mode: form_post', () => { legacySameSiteCookie: false, }, cookies: { - ['_auth_verification.' + clientID]: JSON.stringify({ + ['_auth_verification.']: JSON.stringify({ state: '__test_state__', }), }, From 1657f335d2a1e3462b7d4e3db4d2cc8a3c74679c Mon Sep 17 00:00:00 2001 From: Matt Bacalakis Date: Wed, 9 Nov 2022 11:21:47 -0500 Subject: [PATCH 4/4] Added name to txnCookie type def, and cleaned up missed client id remnant. --- index.d.ts | 2 +- test/callback.tests.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/index.d.ts b/index.d.ts index a4279033..fe15618d 100644 --- a/index.d.ts +++ b/index.d.ts @@ -477,7 +477,7 @@ interface ConfigParams { /** * Configuration parameters used for the transaction cookie. */ - transactionCookie?: Pick; + transactionCookie?: Pick & { name?: string }; /** * String value for the client's authentication method. Default is `none` when using response_type='id_token', `private_key_jwt` when using a `clientAssertionSigningKey`, otherwise `client_secret_basic`. diff --git a/test/callback.tests.js b/test/callback.tests.js index 661b950b..aa8fdb9b 100644 --- a/test/callback.tests.js +++ b/test/callback.tests.js @@ -287,7 +287,7 @@ describe('callback response_mode: form_post', () => { legacySameSiteCookie: false, }, cookies: { - ['_auth_verification.']: JSON.stringify({ + ['_auth_verification']: JSON.stringify({ state: '__test_state__', }), },