From 268a9074254c4e6b9c7b61cf58a346a599c829be Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 23 Jan 2020 09:56:43 -0800 Subject: [PATCH 1/8] Merge appSessionCookie schema into main config schema --- lib/config.js | 38 ++++++++---------------------------- test/invalid_params.tests.js | 10 +++++----- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/lib/config.js b/lib/config.js index ba4cc9db..7da50aec 100644 --- a/lib/config.js +++ b/lib/config.js @@ -16,23 +16,15 @@ const authorizationParamsSchema = Joi.object().keys({ scope: Joi.string().required() }).unknown(true); -const defaultAppSessionCookie = { - httpOnly: true, - sameSite: 'Lax', - ephemeral: false -}; - -const appSessionCookieSchema = Joi.object().keys({ - domain: Joi.string().optional(), - ephemeral: Joi.boolean().optional(), - httpOnly: Joi.boolean().optional(), - path: Joi.string().optional(), - sameSite: Joi.string().valid('Lax', 'Strict', 'None').optional(), - secure: Joi.boolean().optional() -}).unknown(false); - const paramsSchema = Joi.object().keys({ - appSessionCookie: Joi.object().optional(), + appSessionCookie: Joi.object({ + domain: Joi.string().optional(), + ephemeral: Joi.boolean().optional().default(false), + httpOnly: Joi.boolean().optional().default(true), + path: Joi.string().optional(), + sameSite: Joi.string().valid('Lax', 'Strict', 'None').optional().default('Lax'), + secure: Joi.boolean().optional() + }).optional().unknown(false).default(), appSessionDuration: Joi.number().integer().optional().default(7 * 24 * 60 * 60), appSessionName: Joi.string().token().optional().default('identity'), appSessionSecret: Joi.alternatives([ @@ -95,19 +87,6 @@ If the user provides authorizationParams then return authorizationParams; } -function buildAppSessionCookieConfig(cookieConfig) { - - cookieConfig = cookieConfig && Object.keys(cookieConfig).length ? cookieConfig : {}; - cookieConfig = Object.assign({}, defaultAppSessionCookie, cookieConfig); - const cookieConfigValidation = appSessionCookieSchema.validate(cookieConfig); - - if(cookieConfigValidation.error) { - throw new Error(cookieConfigValidation.error.details[0].message); - } - - return cookieConfig; -} - module.exports.get = function(params) { let config = typeof params == 'object' ? clone(params) : {}; @@ -121,7 +100,6 @@ module.exports.get = function(params) { config = paramsValidation.value; config.authorizationParams = buildAuthorizeParams(config.authorizationParams); - config.appSessionCookie = buildAppSessionCookieConfig(config.appSessionCookie); // Code grant requires a client secret to exchange the code for tokens const responseTypeHasCode = config.authorizationParams.response_type.split(' ').includes('code'); diff --git a/test/invalid_params.tests.js b/test/invalid_params.tests.js index b708de6f..a0e43a54 100644 --- a/test/invalid_params.tests.js +++ b/test/invalid_params.tests.js @@ -104,7 +104,7 @@ describe('invalid parameters', function() { httpOnly: '__invalid_httponly__' } })); - }, '"httpOnly" must be a boolean'); + }, '"appSessionCookie.httpOnly" must be a boolean'); }); it('should fail when app session cookie secure is not a boolean', function() { @@ -114,7 +114,7 @@ describe('invalid parameters', function() { secure: '__invalid_secure__' } })); - }, '"secure" must be a boolean'); + }, '"appSessionCookie.secure" must be a boolean'); }); it('should fail when app session cookie sameSite is invalid', function() { @@ -124,7 +124,7 @@ describe('invalid parameters', function() { sameSite: '__invalid_samesite__' } })); - }, '"sameSite" must be one of [Lax, Strict, None]'); + }, '"appSessionCookie.sameSite" must be one of [Lax, Strict, None]'); }); it('should fail when app session cookie domain is invalid', function() { @@ -134,7 +134,7 @@ describe('invalid parameters', function() { domain: false } })); - }, '"domain" must be a string'); + }, '"appSessionCookie.domain" must be a string'); }); it('should fail when app session cookie sameSite is an invalid value', function() { @@ -144,6 +144,6 @@ describe('invalid parameters', function() { path: 123 } })); - }, '"path" must be a string'); + }, '"appSessionCookie.path" must be a string'); }); }); From 5b274f4aff27eca6f496187016a359751d1dc89b Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 23 Jan 2020 10:17:49 -0800 Subject: [PATCH 2/8] Merge authorizationParams schema into main config schema --- lib/config.js | 52 ++++++++++------------------------------------ test/auth.tests.js | 4 ++-- 2 files changed, 13 insertions(+), 43 deletions(-) diff --git a/lib/config.js b/lib/config.js index 7da50aec..8b125840 100644 --- a/lib/config.js +++ b/lib/config.js @@ -4,18 +4,6 @@ const loadEnvs = require('./loadEnvs'); const getUser = require('./hooks/getUser'); const handleCallback = require('./hooks/handleCallback'); -const defaultAuthorizeParams = { - response_mode: 'form_post', - response_type: 'id_token', - scope: 'openid profile email' -}; - -const authorizationParamsSchema = Joi.object().keys({ - response_mode: [Joi.string().optional(), Joi.allow(null).optional()], - response_type: Joi.string().required(), - scope: Joi.string().required() -}).unknown(true); - const paramsSchema = Joi.object().keys({ appSessionCookie: Joi.object({ domain: Joi.string().optional(), @@ -36,7 +24,17 @@ const paramsSchema = Joi.object().keys({ Joi.boolean().valid(false) ]).required(), auth0Logout: Joi.boolean().optional().default(false), - authorizationParams: Joi.object().optional(), + authorizationParams: Joi.object({ + response_type: Joi.string().optional().default('id_token'), + scope: Joi.string().optional().default('openid profile email'), + response_mode: Joi.alternatives([ + Joi.string().optional(), + Joi.allow(null).optional() + ]).default(function(parent) { + const responseType = parent.response_type.split(' '); + return responseType.includes('id_token') || responseType.includes('token') ? 'form_post' : undefined; + }), + }).optional().unknown(true).default(), baseURL: Joi.string().uri().required(), clientID: Joi.string().required(), clientSecret: Joi.string().optional(), @@ -60,33 +58,6 @@ const paramsSchema = Joi.object().keys({ postLogoutRedirectUri: Joi.string().uri({allowRelative: true}).optional().default('/') }); -function buildAuthorizeParams(authorizationParams) { - /* -If the user does not provide authorizationParams we default to "defaultAuthorizeParams" (id_token/form_post). - -If the user provides authorizationParams then - - the default response_mode is DEFAULT (undefined), - - the default scope is defaultAuthorizeParams.scope - - response type is required - */ - - authorizationParams = authorizationParams && Object.keys(authorizationParams).length > 0 ? - authorizationParams : - clone(defaultAuthorizeParams); - - if (!authorizationParams.scope) { - authorizationParams.scope = defaultAuthorizeParams.scope; - } - - const authParamsValidation = authorizationParamsSchema.validate(authorizationParams); - - if(authParamsValidation.error) { - throw new Error(authParamsValidation.error.details[0].message); - } - - return authorizationParams; -} - module.exports.get = function(params) { let config = typeof params == 'object' ? clone(params) : {}; @@ -99,7 +70,6 @@ module.exports.get = function(params) { } config = paramsValidation.value; - config.authorizationParams = buildAuthorizeParams(config.authorizationParams); // Code grant requires a client secret to exchange the code for tokens const responseTypeHasCode = config.authorizationParams.response_type.split(' ').includes('code'); diff --git a/test/auth.tests.js b/test/auth.tests.js index cb59ae7c..3fa81466 100644 --- a/test/auth.tests.js +++ b/test/auth.tests.js @@ -200,14 +200,14 @@ describe('auth', function() { 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, undefined); + 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'); }); it('should contain the two callbacks route', function() { - assert.ok(router.stack.some(filterRoute('GET', '/callback'))); + assert.ok(router.stack.some(filterRoute('POST', '/callback'))); }); }); From 44894cef9d77491f1dfe7292f1fbfe6707aefc98 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 23 Jan 2020 10:24:02 -0800 Subject: [PATCH 3/8] Cleanup for postLogoutRedirectUri, authorizationParams --- API.md | 6 +++--- index.d.ts | 13 +++++++------ lib/config.js | 10 ++++------ test/logout.tests.js | 17 ++++++++--------- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/API.md b/API.md index dc130832..faba4800 100644 --- a/API.md +++ b/API.md @@ -37,14 +37,14 @@ Additional configuration keys that can be passed to `auth()` on initialization: - **`legacySameSiteCookie`** - Set a fallback cookie with no SameSite attribute when `authorizationParams.response_mode` is `form_post`. Default is `true`. - **`loginPath`** - Relative path to application login. Default is `/login`. - **`logoutPath`** - Relative path to application logout. Default is `/logout`. +- **`postLogoutRedirectUri`** - Either a relative path to the application or a valid URI to an external domain. The user will be redirected to this after a logout has been performed. This value must be registered at the authorization server/ Default is `baseUrl`. - **`redirectUriPath`** - Relative path to the application callback to process the response from the authorization server. This value is combined with the `baseUrl` and sent to the authorize endpoint as the `redirectUri` parameter. Default is `/callback`. -- **`postLogoutRedirectUri`** - Either a relative path to the application or a valid URI to an external domain. The user will be redirected to this after a logout has been performed. Default is `baseUrl`. - **`required`** - Use a boolean value to require authentication for all routes. Pass a function instead to base this value on the request. Default is `true`. - **`routes`** - Boolean value to automatically install the login and logout routes. See [the examples](EXAMPLES.md) for more information on how this key is used. Default is `true`. ### Authorization Params Key -The `authorizationParams` key defines the URL parameters used when redirecting users to the authorization server to log in. If this key is not provided by your application, its default value will be: +The `authorizationParams` key defines the URL parameters used when redirecting users to the authorization server to log in. If this key is not provided by your application, its default values will be: ```js { @@ -54,7 +54,7 @@ The `authorizationParams` key defines the URL parameters used when redirecting u } ``` -A new object can be passed in to change what is returned from the authorization server depending on your specific scenario. +New values can be passed in to change what is returned from the authorization server depending on your specific scenario. For example, to receive an access token for an API, you could initialize like the sample below. Note that `response_mode` can be omitted because the OAuth2 default mode of `query` is fine: diff --git a/index.d.ts b/index.d.ts index 4fa2dd89..2b51cc2c 100644 --- a/index.d.ts +++ b/index.d.ts @@ -24,6 +24,7 @@ interface ConfigParams { /** * REQUIRED. The secret(s) used to derive an encryption key for the user identity in a session cookie. + * Use a single string key or array of keys for an encrypted session cookie or false to skip. * Can use env key APP_SESSION_SECRET instead. */ appSessionSecret?: boolean | string | string[]; @@ -119,16 +120,16 @@ interface ConfigParams { logoutPath?: string; /** - * Relative path to the application callback to process the response from the authorization server. + * Either a relative path to the application or a valid URI to an external domain. + * This value must be registered on the authorization server. + * The user will be redirected to this after a logout has been performed. */ - redirectUriPath?: string; + postLogoutRedirectUri?: string; /** - * Either a relative path to the application - * or a valid URI to an external domain. - * The user will be redirected to this after a logout has been performed. + * Relative path to the application callback to process the response from the authorization server. */ - postLogoutRedirectUri?: string; + redirectUriPath?: string; /** * Require authentication for all routes. diff --git a/lib/config.js b/lib/config.js index 8b125840..d67f7058 100644 --- a/lib/config.js +++ b/lib/config.js @@ -4,7 +4,7 @@ const loadEnvs = require('./loadEnvs'); const getUser = require('./hooks/getUser'); const handleCallback = require('./hooks/handleCallback'); -const paramsSchema = Joi.object().keys({ +const paramsSchema = Joi.object({ appSessionCookie: Joi.object({ domain: Joi.string().optional(), ephemeral: Joi.boolean().optional().default(false), @@ -16,11 +16,8 @@ const paramsSchema = Joi.object().keys({ appSessionDuration: Joi.number().integer().optional().default(7 * 24 * 60 * 60), appSessionName: Joi.string().token().optional().default('identity'), appSessionSecret: Joi.alternatives([ - // Single string key. Joi.string(), - // Array of keys to allow for rotation. Joi.array().items(Joi.string()), - // False to stop client session from being created. Joi.boolean().valid(false) ]).required(), auth0Logout: Joi.boolean().optional().default(false), @@ -32,7 +29,8 @@ const paramsSchema = Joi.object().keys({ Joi.allow(null).optional() ]).default(function(parent) { const responseType = parent.response_type.split(' '); - return responseType.includes('id_token') || responseType.includes('token') ? 'form_post' : undefined; + const responseIncludesTokens = responseType.includes('id_token') || responseType.includes('token'); + return responseIncludesTokens ? 'form_post' : undefined; }), }).optional().unknown(true).default(), baseURL: Joi.string().uri().required(), @@ -52,10 +50,10 @@ const paramsSchema = Joi.object().keys({ legacySameSiteCookie: Joi.boolean().optional().default(true), loginPath: Joi.string().uri({relativeOnly: true}).optional().default('/login'), logoutPath: Joi.string().uri({relativeOnly: true}).optional().default('/logout'), + postLogoutRedirectUri: Joi.string().uri({allowRelative: true}).optional().default(''), redirectUriPath: Joi.string().uri({relativeOnly: true}).optional().default('/callback'), required: Joi.alternatives([ Joi.function(), Joi.boolean()]).optional().default(true), routes: Joi.boolean().optional().default(true), - postLogoutRedirectUri: Joi.string().uri({allowRelative: true}).optional().default('/') }); module.exports.get = function(params) { diff --git a/test/logout.tests.js b/test/logout.tests.js index 8949eeeb..ef55e75f 100644 --- a/test/logout.tests.js +++ b/test/logout.tests.js @@ -1,5 +1,4 @@ const { assert } = require('chai'); -const url = require('url'); const server = require('./fixture/server'); const { auth } = require('./..'); @@ -44,7 +43,7 @@ describe('logout route', function() { it('should redirect to the base url', function() { assert.equal(logoutResponse.statusCode, 302); - assert.equal(logoutResponse.headers.location, 'https://example.org/'); + assert.equal(logoutResponse.headers.location, 'https://example.org'); }); }); @@ -83,7 +82,7 @@ describe('logout route', function() { it('should redirect to the base url', function() { assert.equal(logoutResponse.statusCode, 302); - assert.equal(logoutResponse.headers.location, 'https://example.org/'); + assert.equal(logoutResponse.headers.location, 'https://example.org'); }); }); @@ -92,7 +91,7 @@ describe('logout route', function() { describe('should allow relative paths, and prepend with baseURL', () => { let baseUrl; const jar = request.jar(); - + before(async function() { const middleware = auth({ idpLogout: false, @@ -114,12 +113,12 @@ describe('logout route', function() { baseUrl, jar }); }); - + it('should redirect to postLogoutRedirectUri in auth() config', async function() { const logoutResponse = await request.get({uri: '/logout', baseUrl, jar, followRedirect: false}); assert.equal(logoutResponse.headers.location, 'https://example.org/after-logout-in-auth-config'); }); - + it('should redirect to returnTo in logout query', async function() { const logoutResponse = await request.get({uri: '/logout', qs: {returnTo: '/after-logout-in-logout-query'}, baseUrl, jar, followRedirect: false}); assert.equal(logoutResponse.headers.location, 'https://example.org/after-logout-in-logout-query'); @@ -129,7 +128,7 @@ describe('logout route', function() { describe('should allow absolute paths', () => { let baseUrl; const jar = request.jar(); - + before(async function() { const middleware = auth({ idpLogout: false, @@ -151,12 +150,12 @@ describe('logout route', function() { baseUrl, jar }); }); - + it('should redirect to postLogoutRedirectUri in auth() config', async function() { const logoutResponse = await request.get({uri: '/logout', baseUrl, jar, followRedirect: false}); assert.equal(logoutResponse.headers.location, 'https://external-domain.com/after-logout-in-auth-config'); }); - + it('should redirect to returnTo in logout query', async function() { const logoutResponse = await request.get({uri: '/logout', qs: {returnTo: 'https://external-domain.com/after-logout-in-logout-query'}, baseUrl, jar, followRedirect: false}); assert.equal(logoutResponse.headers.location, 'https://external-domain.com/after-logout-in-logout-query'); From 7deb960084d3924236f2d7d0d52d4d788ef8909a Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 23 Jan 2020 12:02:20 -0800 Subject: [PATCH 4/8] Allow configured response_type to fuzzy match issuer response_type --- lib/client.js | 15 ++++++++++----- test/config.tests.js | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/client.js b/lib/client.js index 5cb00ab0..6712dcec 100644 --- a/lib/client.js +++ b/lib/client.js @@ -12,24 +12,29 @@ const telemetryHeader = { } }; +function spacedStringsToAlphabetical(string) { + return string.split(' ').sort().join(' '); +} + async function get(config) { const issuer = await Issuer.discover(config.issuerBaseURL); - const issuerTokenAlgs = Array.isArray(issuer.id_token_signing_alg_values_supported) ? + const issuerTokenAlgs = Array.isArray(issuer.id_token_signing_alg_values_supported) ? issuer.id_token_signing_alg_values_supported : []; if (!issuerTokenAlgs.includes(config.idTokenAlg)) { throw new Error( - `ID token algorithm "${config.idTokenAlg}" is not supported by the issuer. ` + + `ID token algorithm "${config.idTokenAlg}" is not supported by the issuer. ` + `Supported ID token algorithms are: "${issuerTokenAlgs.join('", "')}". ` ); } - const configRespType = config.authorizationParams.response_type; + const configRespType = spacedStringsToAlphabetical(config.authorizationParams.response_type); const issuerRespTypes = Array.isArray(issuer.response_types_supported) ? issuer.response_types_supported : []; + issuerRespTypes.map(spacedStringsToAlphabetical); if (!issuerRespTypes.includes(configRespType)) { throw new Error( - `Response type "${configRespType}" is not supported by the issuer. ` + + `Response type "${configRespType}" is not supported by the issuer. ` + `Supported response types are: "${issuerRespTypes.join('", "')}". ` ); } @@ -38,7 +43,7 @@ async function get(config) { const issuerRespModes = Array.isArray(issuer.response_modes_supported) ? issuer.response_modes_supported : []; if (configRespMode && ! issuerRespModes.includes(configRespMode)) { throw new Error( - `Response mode "${configRespMode}" is not supported by the issuer. ` + + `Response mode "${configRespMode}" is not supported by the issuer. ` + `Supported response modes are "${issuerRespModes.join('", "')}". ` ); } diff --git a/test/config.tests.js b/test/config.tests.js index bd6a5417..0c62c713 100644 --- a/test/config.tests.js +++ b/test/config.tests.js @@ -51,6 +51,20 @@ describe('config', function() { }); }); + describe('when authorizationParams response_type fuzzy matches issuer', function() { + const customConfig = Object.assign({}, defaultConfig, { + clientSecret: '__test_client_secret__', + authorizationParams: { + response_type: 'token id_token code' + } + }); + const config = getConfig(customConfig); + + it('should keep token code', function() { + assert.equal(config.authorizationParams.response_type, 'token id_token code'); + }); + }); + describe('with auth0Logout', function() { const config = getConfig(Object.assign({}, defaultConfig, {auth0Logout: true})); From 323a67d0d7bffceb06273912eb4737780b8fc164 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 23 Jan 2020 14:28:32 -0800 Subject: [PATCH 5/8] Clean up test names and docs --- API.md | 2 +- test/config.tests.js | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/API.md b/API.md index faba4800..83187ca8 100644 --- a/API.md +++ b/API.md @@ -37,7 +37,7 @@ Additional configuration keys that can be passed to `auth()` on initialization: - **`legacySameSiteCookie`** - Set a fallback cookie with no SameSite attribute when `authorizationParams.response_mode` is `form_post`. Default is `true`. - **`loginPath`** - Relative path to application login. Default is `/login`. - **`logoutPath`** - Relative path to application logout. Default is `/logout`. -- **`postLogoutRedirectUri`** - Either a relative path to the application or a valid URI to an external domain. The user will be redirected to this after a logout has been performed. This value must be registered at the authorization server/ Default is `baseUrl`. +- **`postLogoutRedirectUri`** - Either a relative path to the application or a valid URI to an external domain. The user will be redirected to this after a logout has been performed. Adding a `returnTo` parameter on the logout route will override this value. The value used must be registered at the authorization server. Default is `baseUrl`. - **`redirectUriPath`** - Relative path to the application callback to process the response from the authorization server. This value is combined with the `baseUrl` and sent to the authorize endpoint as the `redirectUri` parameter. Default is `/callback`. - **`required`** - Use a boolean value to require authentication for all routes. Pass a function instead to base this value on the request. Default is `true`. - **`routes`** - Boolean value to automatically install the login and logout routes. See [the examples](EXAMPLES.md) for more information on how this key is used. Default is `true`. diff --git a/test/config.tests.js b/test/config.tests.js index 0c62c713..5d641446 100644 --- a/test/config.tests.js +++ b/test/config.tests.js @@ -20,16 +20,16 @@ describe('config', function() { assert.equal(config.authorizationParams.response_mode, 'form_post'); }); - it('should default to scope=openid profile email ', function() { + it('should default to scope=openid profile email', function() { assert.equal(config.authorizationParams.scope, 'openid profile email'); }); - it('should default to required true ', function() { + it('should default to required true', function() { assert.ok(config.required); }); }); - describe('when authorizationParams is response_type=x', function() { + describe('when authorizationParams is response_type=code', function() { const customConfig = Object.assign({}, defaultConfig, { clientSecret: '__test_client_secret__', authorizationParams: { @@ -38,15 +38,15 @@ describe('config', function() { }); const config = getConfig(customConfig); - it('should default to response_type=id_token', function() { + it('should set new response_type', function() { assert.equal(config.authorizationParams.response_type, 'code'); }); - it('should default to response_mode=form_post', function() { + it('should allow undefined response_mode', function() { assert.equal(config.authorizationParams.response_mode, undefined); }); - it('should default to scope=openid profile email ', function() { + it('should keep default to scope', function() { assert.equal(config.authorizationParams.scope, 'openid profile email'); }); }); From c88a5d4be92d584d1682baebd6f0fe2927691583 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Thu, 23 Jan 2020 14:32:47 -0800 Subject: [PATCH 6/8] Move loadEnvs to Joi default functions --- lib/config.js | 20 +++++++++----------- lib/loadEnvs.js | 16 ---------------- 2 files changed, 9 insertions(+), 27 deletions(-) delete mode 100644 lib/loadEnvs.js diff --git a/lib/config.js b/lib/config.js index d67f7058..32f8ac3b 100644 --- a/lib/config.js +++ b/lib/config.js @@ -1,6 +1,5 @@ const Joi = require('@hapi/joi'); const clone = require('clone'); -const loadEnvs = require('./loadEnvs'); const getUser = require('./hooks/getUser'); const handleCallback = require('./hooks/handleCallback'); @@ -19,7 +18,7 @@ const paramsSchema = Joi.object({ Joi.string(), Joi.array().items(Joi.string()), Joi.boolean().valid(false) - ]).required(), + ]).required().default(() => process.env.APP_SESSION_SECRET), auth0Logout: Joi.boolean().optional().default(false), authorizationParams: Joi.object({ response_type: Joi.string().optional().default('id_token'), @@ -33,20 +32,21 @@ const paramsSchema = Joi.object({ return responseIncludesTokens ? 'form_post' : undefined; }), }).optional().unknown(true).default(), - baseURL: Joi.string().uri().required(), - clientID: Joi.string().required(), - clientSecret: Joi.string().optional(), + baseURL: Joi.string().uri().required().default(() => process.env.BASE_URL), + clientID: Joi.string().required().default(() => process.env.CLIENT_ID), + clientSecret: Joi.string().optional().default(() => process.env.CLIENT_SECRET), clockTolerance: Joi.number().optional().default(60), errorOnRequiredAuth: Joi.boolean().optional().default(false), getUser: Joi.function().optional().default(() => getUser), handleCallback: Joi.function().optional().default(() => handleCallback), httpOptions: Joi.object().optional(), identityClaimFilter: Joi.array().optional().default(['aud', 'iss', 'iat', 'exp', 'nonce', 'azp', 'auth_time']), - idpLogout: Joi.boolean().optional().default(false).when( - 'auth0Logout', { is: true, then: Joi.boolean().optional().default(true) } - ), + idpLogout: Joi.boolean().optional().default((parent) => parent.auth0Logout || false), idTokenAlg: Joi.string().not('none').optional().default('RS256'), - issuerBaseURL: Joi.alternatives([ Joi.string().uri(), Joi.string().hostname() ]).required(), + issuerBaseURL: Joi.alternatives([ + Joi.string().uri(), + Joi.string().hostname() + ]).required().default(() => process.env.ISSUER_BASE_URL), legacySameSiteCookie: Joi.boolean().optional().default(true), loginPath: Joi.string().uri({relativeOnly: true}).optional().default('/login'), logoutPath: Joi.string().uri({relativeOnly: true}).optional().default('/logout'), @@ -59,8 +59,6 @@ const paramsSchema = Joi.object({ module.exports.get = function(params) { let config = typeof params == 'object' ? clone(params) : {}; - loadEnvs(config); - const paramsValidation = paramsSchema.validate(config); if(paramsValidation.error) { diff --git a/lib/loadEnvs.js b/lib/loadEnvs.js deleted file mode 100644 index 9cbb7e94..00000000 --- a/lib/loadEnvs.js +++ /dev/null @@ -1,16 +0,0 @@ -const fieldsEnvMap = { - 'issuerBaseURL': 'ISSUER_BASE_URL', - 'baseURL': 'BASE_URL', - 'clientID': 'CLIENT_ID', - 'clientSecret': 'CLIENT_SECRET', - 'appSessionSecret': 'APP_SESSION_SECRET', -}; - -module.exports = function(params) { - Object.keys(fieldsEnvMap).forEach(k => { - if (typeof params[k] === 'undefined') { - params[k] = process.env[fieldsEnvMap[k]]; - } - }); -}; - From a709b2d0e817244f21426fa2dc060473693b6f97 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Fri, 24 Jan 2020 14:57:22 -0800 Subject: [PATCH 7/8] Move clientSecret checks into Joi schema --- lib/config.js | 54 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/lib/config.js b/lib/config.js index 32f8ac3b..b01a856f 100644 --- a/lib/config.js +++ b/lib/config.js @@ -18,7 +18,7 @@ const paramsSchema = Joi.object({ Joi.string(), Joi.array().items(Joi.string()), Joi.boolean().valid(false) - ]).required().default(() => process.env.APP_SESSION_SECRET), + ]).required(), auth0Logout: Joi.boolean().optional().default(false), authorizationParams: Joi.object({ response_type: Joi.string().optional().default('id_token'), @@ -32,9 +32,26 @@ const paramsSchema = Joi.object({ return responseIncludesTokens ? 'form_post' : undefined; }), }).optional().unknown(true).default(), - baseURL: Joi.string().uri().required().default(() => process.env.BASE_URL), - clientID: Joi.string().required().default(() => process.env.CLIENT_ID), - clientSecret: Joi.string().optional().default(() => process.env.CLIENT_SECRET), + baseURL: Joi.string().uri().required(), + clientID: Joi.string().required(), + clientSecret: Joi.string().when( + Joi.ref('authorizationParams.response_type', {adjust: (value) => value && value.split(' ').includes('code')}), + { + is: true, + then: Joi.string().required().messages({ + 'any.required': '"clientSecret" is required for response_type code' + }), + otherwise: Joi.when( + Joi.ref('idTokenAlg', {adjust: (value) => value && 'HS' === value.substring(0,2)}), + { + is: true, + then: Joi.string().required().messages({ + 'any.required': '"clientSecret" is required for ID tokens with HS algorithms' + }) + } + ) + } + ), clockTolerance: Joi.number().optional().default(60), errorOnRequiredAuth: Joi.boolean().optional().default(false), getUser: Joi.function().optional().default(() => getUser), @@ -46,7 +63,7 @@ const paramsSchema = Joi.object({ issuerBaseURL: Joi.alternatives([ Joi.string().uri(), Joi.string().hostname() - ]).required().default(() => process.env.ISSUER_BASE_URL), + ]).required(), legacySameSiteCookie: Joi.boolean().optional().default(true), loginPath: Joi.string().uri({relativeOnly: true}).optional().default('/login'), logoutPath: Joi.string().uri({relativeOnly: true}).optional().default('/logout'), @@ -57,26 +74,19 @@ const paramsSchema = Joi.object({ }); module.exports.get = function(params) { - let config = typeof params == 'object' ? clone(params) : {}; + let config = (typeof params == 'object' ? clone(params) : {}); + config = Object.assign({ + issuerBaseURL: process.env.ISSUER_BASE_URL, + baseURL: process.env.BASE_URL, + clientID: process.env.CLIENT_ID, + clientSecret: process.env.CLIENT_SECRET, + appSessionSecret: process.env.APP_SESSION_SECRET, + }, config); const paramsValidation = paramsSchema.validate(config); - - if(paramsValidation.error) { + if (paramsValidation.error) { throw new Error(paramsValidation.error.details[0].message); } - config = paramsValidation.value; - - // Code grant requires a client secret to exchange the code for tokens - const responseTypeHasCode = config.authorizationParams.response_type.split(' ').includes('code'); - if (responseTypeHasCode && !config.clientSecret) { - throw new Error('"clientSecret" is required for response_type code'); - } - - // HS256 ID tokens require a client secret to validate the signature. - if ('HS' === config.idTokenAlg.substring(0,2) && !config.clientSecret) { - throw new Error('"clientSecret" is required for ID tokens with HS algorithms'); - } - - return config; + return paramsValidation.value; }; From 44cc859b1cd11cd46e68e420f8c2ff0a546b4b88 Mon Sep 17 00:00:00 2001 From: Josh Cunningham Date: Mon, 27 Jan 2020 07:40:03 -0800 Subject: [PATCH 8/8] Update test/config.tests.js Co-Authored-By: Steve Hobbs --- test/config.tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/config.tests.js b/test/config.tests.js index 5d641446..5f741c30 100644 --- a/test/config.tests.js +++ b/test/config.tests.js @@ -46,7 +46,7 @@ describe('config', function() { assert.equal(config.authorizationParams.response_mode, undefined); }); - it('should keep default to scope', function() { + it('should keep default scope', function() { assert.equal(config.authorizationParams.scope, 'openid profile email'); }); });