From 224dd38fe1d43bf646c017bdfa7eaac3f3ef1518 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 19 Feb 2021 00:16:32 +0100 Subject: [PATCH] refactor: Removed OpenID Connect Session Management BREAKING CHANGE: OpenID Connect Session Management draft implementation was removed. This is due to front-channel becoming more and more unreliable due to browsers blocking third-party cookie access. --- .github/workflows/test.yml | 1 - certification/configuration.js | 1 - docs/README.md | 42 +--- lib/actions/authorization/respond.js | 5 - lib/actions/check_session.js | 188 ------------------ lib/actions/discovery.js | 1 - lib/actions/end_session.js | 29 +-- lib/actions/index.js | 2 - lib/helpers/defaults.js | 32 +-- lib/helpers/features.js | 6 - lib/helpers/initialize_app.js | 7 +- lib/helpers/process_session_state.js | 49 ----- lib/models/client.js | 13 -- lib/models/session.js | 10 - lib/shared/authorization_error_handler.js | 5 - lib/shared/session.js | 3 +- test/interaction/interaction.config.js | 1 - test/interaction/interaction.test.js | 11 +- test/provider/provider_instance.test.js | 6 +- test/session_management/authorization.test.js | 138 ------------- .../check_session_endpoint.test.js | 122 ------------ test/session_management/discovery.test.js | 18 -- test/session_management/end_session.test.js | 130 ------------ .../session_management.config.js | 25 --- test/test_helper.js | 16 +- types/index.d.ts | 8 - types/oidc-provider-tests.ts | 5 - 27 files changed, 17 insertions(+), 857 deletions(-) delete mode 100644 lib/actions/check_session.js delete mode 100644 lib/helpers/process_session_state.js delete mode 100644 test/session_management/authorization.test.js delete mode 100644 test/session_management/check_session_endpoint.test.js delete mode 100644 test/session_management/discovery.test.js delete mode 100644 test/session_management/end_session.test.js delete mode 100644 test/session_management/session_management.config.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6604d0988..d7e9c66c4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -93,7 +93,6 @@ jobs: - '{"plan":"oidcc-dynamic-certification-test-plan","response_type":"code id_token token","skip":"oidcc-server-rotate-keys"}' - '{"plan":"oidcc-backchannel-rp-initiated-logout-certification-test-plan","response_type":"code","client_registration":"dynamic_client"}' - '{"plan":"oidcc-rp-initiated-logout-certification-test-plan","response_type":"code","client_registration":"dynamic_client"}' - - '{"plan":"oidcc-session-management-certification-test-plan","response_type":"code","client_registration":"dynamic_client","skip":"oidcc-session-management-rp-initiated-logout"}' - '{"plan":"fapi-rw-id2-test-plan","configuration":"./certification/fapi/pkjwt.json","client_auth_type":"private_key_jwt","fapi_profile":"plain_fapi","fapi_response_mode":"plain_response"}' - '{"plan":"fapi-rw-id2-test-plan","configuration":"./certification/fapi/pkjwt.json","client_auth_type":"private_key_jwt","fapi_profile":"plain_fapi","fapi_response_mode":"jarm"}' - '{"plan":"fapi-rw-id2-test-plan","configuration":"./certification/fapi/mtls.json","client_auth_type":"mtls","fapi_profile":"plain_fapi","fapi_response_mode":"plain_response"}' diff --git a/certification/configuration.js b/certification/configuration.js index 12aff4cee..e293450f2 100644 --- a/certification/configuration.js +++ b/certification/configuration.js @@ -94,7 +94,6 @@ module.exports = { mode: 'strict', }, revocation: { enabled: true }, - sessionManagement: { enabled: true }, }, jwks: { keys: [ diff --git a/docs/README.md b/docs/README.md index 4e785391d..271e1e73f 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1675,7 +1675,7 @@ _**default value**_: #### logoutSource -HTML source rendered when session management feature renders a confirmation prompt for the User-Agent. +HTML source rendered when RP-Initiated Logout renders a confirmation prompt for the User-Agent. _**default value**_: @@ -1703,7 +1703,7 @@ async function logoutSource(ctx, form) { #### postLogoutSuccessSource -HTML source rendered when session management feature concludes a logout but there was no `post_logout_redirect_uri` provided by the client. +HTML source rendered when RP-Initiated Logout concludes a logout but there was no `post_logout_redirect_uri` provided by the client. _**default value**_: @@ -1731,40 +1731,6 @@ async function postLogoutSuccessSource(ctx) { -### features.sessionManagement - -[Session Management 1.0 - draft 30](https://openid.net/specs/openid-connect-session-1_0-30.html) - -Enables Session Management features. - Note: Browsers blocking access to cookies from a third party context hinder the reliability of this standard. - - -_**default value**_: -```js -{ - ack: undefined, - enabled: false, - keepHeaders: false -} -``` - -
(Click to expand) features.sessionManagement options details
- - -#### keepHeaders - -Enables/Disables removing frame-ancestors from Content-Security-Policy and X-Frame-Options headers. - -_**recommendation**_: Only enable this if you know what you're doing either in a followup middleware or your app server, otherwise you shouldn't have the need to touch this option. - - -_**default value**_: -```js -false -``` - -
- ### features.userinfo [Core 1.0](https://openid.net/specs/openid-connect-core-1_0.html#UserInfo) - UserInfo Endpoint @@ -2002,8 +1968,7 @@ _**default value**_: { interaction: '_interaction', resume: '_interaction_resume', - session: '_session', - state: '_state' + session: '_session' } ``` @@ -2871,7 +2836,6 @@ _**default value**_: ```js { authorization: '/auth', - check_session: '/session/check', code_verification: '/device', device_authorization: '/device/auth', end_session: '/session/end', diff --git a/lib/actions/authorization/respond.js b/lib/actions/authorization/respond.js index a77b410f5..db7f811a8 100644 --- a/lib/actions/authorization/respond.js +++ b/lib/actions/authorization/respond.js @@ -1,5 +1,4 @@ const instance = require('../../helpers/weak_cache'); -const processSessionState = require('../../helpers/process_session_state'); /* * Based on the authorization request response mode either redirects with parameters in query or @@ -19,10 +18,6 @@ module.exports = async function respond(ctx, next) { out.state = params.state; } - if (instance(ctx.oidc.provider).configuration('features.sessionManagement.enabled')) { - out.session_state = processSessionState(ctx, params.redirect_uri); - } - if (instance(ctx.oidc.provider).configuration('features.issAuthResp.enabled')) { out.iss = ctx.oidc.provider.issuer; } diff --git a/lib/actions/check_session.js b/lib/actions/check_session.js deleted file mode 100644 index 9ce81503e..000000000 --- a/lib/actions/check_session.js +++ /dev/null @@ -1,188 +0,0 @@ -const instance = require('../helpers/weak_cache'); -const { json: parseBody } = require('../shared/selective_body'); -const noCache = require('../shared/no_cache'); -const paramsMiddleware = require('../shared/assemble_params'); -const presence = require('../helpers/validate_presence'); -const { InvalidRequest, InvalidClient } = require('../helpers/errors'); -const pushInlineSha = require('../helpers/script_src_sha'); - -const PARAM_LIST = new Set(['client_id', 'origin']); - -module.exports = { - get: async function checkSessionIframe(ctx, next) { - const { keepHeaders } = instance(ctx.oidc.provider).configuration('features.sessionManagement'); - const csp = ctx.response.get('Content-Security-Policy'); - if (!keepHeaders) { - ctx.response.remove('X-Frame-Options'); - if (csp.includes('frame-ancestors')) { - ctx.response.set('Content-Security-Policy', csp.replace(/ ?frame-ancestors [^;]+;/, '')); - } - } - - ctx.type = 'html'; - ctx.body = ` - - - -Session Management - OP iframe - - - - - - -`; - await next(); - }, - post: [ - noCache, - parseBody, - paramsMiddleware.bind(undefined, PARAM_LIST), - async function checkClientOrigin(ctx, next) { - presence(ctx, 'origin', 'client_id'); - const { client_id: clientId, origin } = ctx.oidc.params; - [clientId, origin].forEach((value) => { - if (typeof value !== 'string') { - throw new InvalidRequest('only string parameter values are expected'); - } - }); - const client = await ctx.oidc.provider.Client.find(clientId); - ctx.oidc.entity('Client', client); - if (!client) { - throw new InvalidClient('client is invalid', 'client not found'); - } - if (!client.checkSessionOriginAllowed(origin)) { - throw new InvalidRequest('origin not allowed', 403); - } - ctx.status = 204; - await next(); - }, - ], -}; diff --git a/lib/actions/discovery.js b/lib/actions/discovery.js index a0908a026..b52291ce9 100644 --- a/lib/actions/discovery.js +++ b/lib/actions/discovery.js @@ -15,7 +15,6 @@ module.exports = function discovery(ctx, next) { claims_supported: [...config.claimsSupported], code_challenge_methods_supported: config.pkce.methods, end_session_endpoint: features.rpInitiatedLogout.enabled ? ctx.oidc.urlFor('end_session') : undefined, - check_session_iframe: features.sessionManagement.enabled ? ctx.oidc.urlFor('check_session') : undefined, grant_types_supported: [...config.grantTypes], id_token_signing_alg_values_supported: config.idTokenSigningAlgValues, issuer: ctx.oidc.issuer, diff --git a/lib/actions/end_session.js b/lib/actions/end_session.js index 5c30cd09e..227bedb1e 100644 --- a/lib/actions/end_session.js +++ b/lib/actions/end_session.js @@ -124,7 +124,7 @@ module.exports = { const { state } = session; const { - features: { backchannelLogout, sessionManagement }, + features: { backchannelLogout }, cookies: { long: opts }, } = instance(ctx.oidc.provider).configuration(); @@ -176,25 +176,6 @@ module.exports = { await session.destroy(); - if (sessionManagement.enabled) { - // get all cookies matching _state.[clientId](.sig) and drop them - const STATES = new RegExp(`${ctx.oidc.provider.cookieName('state')}\\.[^=]+=`, 'g'); - const cookieNames = ctx.get('cookie').match(STATES); - if (cookieNames) { - cookieNames.forEach((val) => { - const name = val.slice(0, -1); - if (!name.endsWith('.legacy')) { - ssHandler.set( - ctx.oidc.cookies, - name, - null, - opts, - ); - } - }); - } - } - ssHandler.set( ctx.oidc.cookies, ctx.oidc.provider.cookieName('session'), @@ -212,14 +193,6 @@ module.exports = { if (session.authorizations) { delete session.authorizations[state.clientId]; } - if (sessionManagement.enabled) { - ssHandler.set( - ctx.oidc.cookies, - `${ctx.oidc.provider.cookieName('state')}.${state.clientId}`, - null, - opts, - ); - } session.resetIdentifier(); } diff --git a/lib/actions/index.js b/lib/actions/index.js index 66024b89c..09c452713 100644 --- a/lib/actions/index.js +++ b/lib/actions/index.js @@ -6,7 +6,6 @@ const registration = require('./registration'); const getRevocation = require('./revocation'); const getIntrospection = require('./introspection'); const discovery = require('./discovery'); -const checkSession = require('./check_session'); const endSession = require('./end_session'); const codeVerification = require('./code_verification'); @@ -19,7 +18,6 @@ module.exports = { getRevocation, getIntrospection, discovery, - checkSession, endSession, codeVerification, }; diff --git a/lib/helpers/defaults.js b/lib/helpers/defaults.js index 97d4f970d..635687a6a 100644 --- a/lib/helpers/defaults.js +++ b/lib/helpers/defaults.js @@ -628,7 +628,6 @@ function getDefaults() { session: '_session', // used for main session reference interaction: '_interaction', // used by the interactions for interaction session reference resume: '_interaction_resume', // used when interactions resume authorization for interaction session reference - state: '_state', // prefix for sessionManagement state cookies => _state.{clientId} }, /* @@ -1044,7 +1043,7 @@ function getDefaults() { /* * features.rpInitiatedLogout.postLogoutSuccessSource * - * description: HTML source rendered when session management feature concludes a logout but there + * description: HTML source rendered when RP-Initiated Logout concludes a logout but there * was no `post_logout_redirect_uri` provided by the client. */ postLogoutSuccessSource, @@ -1052,7 +1051,7 @@ function getDefaults() { /* * features.rpInitiatedLogout.logoutSource * - * description: HTML source rendered when session management feature renders a confirmation + * description: HTML source rendered when RP-Initiated Logout renders a confirmation * prompt for the User-Agent. */ logoutSource, @@ -1476,32 +1475,6 @@ function getDefaults() { */ revocation: { enabled: false }, - /* - * features.sessionManagement - * - * title: [Session Management 1.0 - draft 30](https://openid.net/specs/openid-connect-session-1_0-30.html) - * - * description: Enables Session Management features. - * - * Note: Browsers blocking access to cookies from a third party context hinder the reliability - * of this standard. - */ - sessionManagement: { - enabled: false, - - ack: undefined, - - /* - * features.sessionManagement.keepHeaders - * - * description: Enables/Disables removing frame-ancestors from Content-Security-Policy and - * X-Frame-Options headers. - * recommendation: Only enable this if you know what you're doing either in a followup - * middleware or your app server, otherwise you shouldn't have the need to touch this option. - */ - keepHeaders: false, - }, - /* * features.userinfo * @@ -1842,7 +1815,6 @@ function getDefaults() { */ routes: { authorization: '/auth', - check_session: '/session/check', code_verification: '/device', device_authorization: '/device/auth', end_session: '/session/end', diff --git a/lib/helpers/features.js b/lib/helpers/features.js index 4d8e01681..a634f2501 100644 --- a/lib/helpers/features.js +++ b/lib/helpers/features.js @@ -58,12 +58,6 @@ const DRAFTS = new Map(Object.entries({ url: 'https://tools.ietf.org/html/draft-ietf-oauth-resource-indicators-08', version: [2, 3, 4, 5, 6, 7, 'draft-07', 'draft-08'], }, - sessionManagement: { - name: 'OpenID Connect Session Management 1.0 - draft 30', - type: 'OIDF AB/Connect Working Group draft', - url: 'https://openid.net/specs/openid-connect-session-1_0-30.html', - version: [28, 'draft-28', 'draft-29', 'draft-30'], - }, webMessageResponseMode: { name: 'OAuth 2.0 Web Message Response Mode - draft 00', type: 'Individual draft', diff --git a/lib/helpers/initialize_app.js b/lib/helpers/initialize_app.js index 2289a909a..1d8dcab71 100644 --- a/lib/helpers/initialize_app.js +++ b/lib/helpers/initialize_app.js @@ -4,7 +4,7 @@ const querystring = require('querystring'); const Router = require('../router'); const { getAuthorization, userinfo, getToken, jwks, registration, getRevocation, - getIntrospection, discovery, checkSession, endSession, codeVerification, + getIntrospection, discovery, endSession, codeVerification, } = require('../actions'); const getInteraction = require('../actions/interaction'); const cors = require('../shared/cors'); @@ -184,11 +184,6 @@ module.exports = function initializeApp() { options('cors.introspection', routes.introspection, CORS.client); } - if (configuration.features.sessionManagement.enabled) { - get('check_session', routes.check_session, error(this, 'check_session.error'), checkSession.get); - post('check_session_origin', routes.check_session, error(this, 'check_session_origin.error'), ...checkSession.post); - } - post('end_session_confirm', `${routes.end_session}/confirm`, error(this, 'end_session_confirm.error'), ...endSession.confirm); if (configuration.features.rpInitiatedLogout.enabled) { diff --git a/lib/helpers/process_session_state.js b/lib/helpers/process_session_state.js deleted file mode 100644 index 2c833dd27..000000000 --- a/lib/helpers/process_session_state.js +++ /dev/null @@ -1,49 +0,0 @@ -const { randomBytes, createHash } = require('crypto'); -const { URL } = require('url'); - -const base64url = require('./base64url'); -const instance = require('./weak_cache'); -const ssHandler = require('./samesite_handler'); - -function processSessionState(ctx, redirectUri, salt) { - const { oidc } = ctx; - const parsed = new URL(redirectUri); - const { origin } = parsed; - - const { clientId } = oidc.client; - const state = oidc.session.stateFor(clientId); - - const shasum = createHash('sha256') - .update(clientId) - .update(' ') - .update(origin) - .update(' ') - .update(state); - - if (salt) { - shasum.update(' '); - shasum.update(salt); - } - - const sessionState = base64url.encodeBuffer(shasum.digest()); - - const stateCookieName = `${oidc.provider.cookieName('state')}.${clientId}`; - ssHandler.set( - oidc.cookies, - stateCookieName, - state, - { - ...instance(oidc.provider).configuration('cookies.long'), - httpOnly: false, - signed: false, - }, - ); - - return salt ? `${sessionState}.${salt}` : sessionState; -} - -module.exports = processSessionState; -module.exports.salted = function saltedProcessSessionState(ctx, redirectUri) { - const salt = base64url.encodeBuffer(randomBytes(8)); - return processSessionState(ctx, redirectUri, salt); -}; diff --git a/lib/models/client.js b/lib/models/client.js index ecadabfbe..f08142348 100644 --- a/lib/models/client.js +++ b/lib/models/client.js @@ -484,19 +484,6 @@ module.exports = function getClient(provider) { }); } - checkSessionOriginAllowed(origin) { - if (!('checkSessionOriginAllowed' in instance(this))) { - instance(this).checkSessionOriginAllowed = this.redirectUris.reduce((acc, uri) => { - const { origin: redirectUriOrigin } = new URL(uri); - acc.add(redirectUriOrigin); - return acc; - }, new Set()); - } - - const origins = instance(this).checkSessionOriginAllowed; - return origins.has(origin); - } - webMessageUriAllowed(webMessageUri) { return this.webMessageUris && this.webMessageUris.includes(webMessageUri); } diff --git a/lib/models/session.js b/lib/models/session.js index 5f0ee3f35..db8edcefb 100644 --- a/lib/models/session.js +++ b/lib/models/session.js @@ -192,16 +192,6 @@ module.exports = function getSession(provider) { return this.authorizations[clientId]; } - stateFor(clientId) { - return base64url.encodeBuffer(hash(this.authorizationFor(clientId), { - algorithm: 'sha256', - ignoreUnknown: true, - unorderedArrays: true, - unorderedSets: true, - encoding: 'buffer', - })); - } - sidFor(clientId, value) { const authorization = this.authorizationFor(...arguments); diff --git a/lib/shared/authorization_error_handler.js b/lib/shared/authorization_error_handler.js index 4492888a6..2aba248d5 100644 --- a/lib/shared/authorization_error_handler.js +++ b/lib/shared/authorization_error_handler.js @@ -3,7 +3,6 @@ const Debug = require('debug'); const { InvalidRedirectUri, WebMessageUriMismatch } = require('../helpers/errors'); const instance = require('../helpers/weak_cache'); const errOut = require('../helpers/err_out'); -const processSessionState = require('../helpers/process_session_state'); const resolveResponseMode = require('../helpers/resolve_response_mode'); const oneRedirectUriClients = require('../actions/authorization/one_redirect_uri_clients'); @@ -100,10 +99,6 @@ module.exports = (provider) => { const renderError = instance(provider).configuration('renderError'); await renderError(ctx, out, err); } else { - if (instance(provider).configuration('features.sessionManagement.enabled')) { - out.session_state = processSessionState.salted(ctx, params.redirect_uri); - } - let mode = safe(params.response_mode); if (!instance(provider).responseModes.has(mode)) { mode = resolveResponseMode(safe(params.response_type)); diff --git a/lib/shared/session.js b/lib/shared/session.js index 97c972394..303ddf64d 100644 --- a/lib/shared/session.js +++ b/lib/shared/session.js @@ -29,8 +29,7 @@ module.exports = async function sessionHandler(ctx, next) { await next(); } finally { const sessionCookieName = ctx.oidc.provider.cookieName('session'); - const stateCookieName = ctx.oidc.provider.cookieName('state'); - const longRegExp = new RegExp(`^(${sessionCookieName}|${stateCookieName}\\.[^=]+)(?:\\.legacy)?(?:\\.sig)?=`); + const longRegExp = new RegExp(`^${sessionCookieName}(?:\\.legacy)?(?:\\.sig)?=`); // refresh the session duration if ((!ctx.oidc.session.new || ctx.oidc.session.touched) && !ctx.oidc.session.destroyed) { diff --git a/test/interaction/interaction.config.js b/test/interaction/interaction.config.js index d716e3f77..a705ab3ce 100644 --- a/test/interaction/interaction.config.js +++ b/test/interaction/interaction.config.js @@ -6,7 +6,6 @@ const { Check, Prompt, base } = require('../../lib/helpers/interaction_policy'); config.extraParams = ['triggerCustomFail', 'triggerUnrequestable']; merge(config.features, { - sessionManagement: { enabled: true }, rpInitiatedLogout: { enabled: false }, }); diff --git a/test/interaction/interaction.test.js b/test/interaction/interaction.test.js index be9426eb9..2eacbd396 100644 --- a/test/interaction/interaction.test.js +++ b/test/interaction/interaction.test.js @@ -431,7 +431,7 @@ describe('resume after consent', () => { .expect('set-cookie', /expires/) // expect a permanent cookie .expect(auth.validateState) .expect(auth.validateClientLocation) - .expect(auth.validatePresence(['code', 'state', 'session_state'])) + .expect(auth.validatePresence(['code', 'state'])) .expect(() => { expect(this.getSession()).to.be.ok.and.not.have.property('transient'); }); @@ -457,7 +457,7 @@ describe('resume after consent', () => { .expect('set-cookie', /expires/) // expect a permanent cookie .expect(auth.validateState) .expect(auth.validateClientLocation) - .expect(auth.validatePresence(['code', 'state', 'session_state'])) + .expect(auth.validatePresence(['code', 'state'])) .expect(() => { expect(this.getSession()).to.be.ok.and.not.have.property('transient'); }); @@ -482,9 +482,8 @@ describe('resume after consent', () => { .expect(302) .expect(auth.validateState) .expect('set-cookie', /_session=((?!expires).)+,/) // expect a transient session cookie - .expect('set-cookie', /_state\.client=((?!expires).)+,/) // expect a transient session cookie .expect(auth.validateClientLocation) - .expect(auth.validatePresence(['code', 'state', 'session_state'])) + .expect(auth.validatePresence(['code', 'state'])) .expect(() => { expect(this.getSession()).to.be.ok.and.have.property('transient'); }); @@ -994,7 +993,7 @@ describe('resume after consent', () => { return this.agent.get('/auth/resume') .expect(302) .expect(auth.validateState) - .expect(auth.validatePresence(['error', 'state', 'session_state'])) + .expect(auth.validatePresence(['error', 'state'])) .expect(auth.validateError('access_denied')); }); @@ -1012,7 +1011,7 @@ describe('resume after consent', () => { return this.agent.get('/auth/resume') .expect(302) .expect(auth.validateState) - .expect(auth.validatePresence(['error', 'state', 'session_state'])) + .expect(auth.validatePresence(['error', 'state'])) .expect(auth.validateError('access_denied')); }); diff --git a/test/provider/provider_instance.test.js b/test/provider/provider_instance.test.js index 5d1b2f8e0..3353612f4 100644 --- a/test/provider/provider_instance.test.js +++ b/test/provider/provider_instance.test.js @@ -20,7 +20,7 @@ describe('provider instance', () => { it('it warns when draft/experimental specs are enabled', () => { new Provider('http://localhost', { // eslint-disable-line no-new - features: { sessionManagement: { enabled: true } }, + features: { backchannelLogout: { enabled: true } }, }); expect(console.info.called).to.be.true; @@ -28,7 +28,7 @@ describe('provider instance', () => { it('it is silent when a version is acknowledged', () => { new Provider('http://localhost', { // eslint-disable-line no-new - features: { sessionManagement: { enabled: true, ack: 28 } }, + features: { backchannelLogout: { enabled: true, ack: 'draft-06' } }, }); expect(console.info.called).to.be.false; @@ -45,7 +45,7 @@ describe('provider instance', () => { it('throws when an acked feature has breaking changes since', () => { expect(() => { new Provider('http://localhost', { // eslint-disable-line no-new - features: { sessionManagement: { enabled: true, ack: 27 } }, + features: { backchannelLogout: { enabled: true, ack: 3 } }, }); }).to.throw('An unacknowledged version of a draft feature is included in this oidc-provider version.'); expect(console.info.called).to.be.true; diff --git a/test/session_management/authorization.test.js b/test/session_management/authorization.test.js deleted file mode 100644 index ea4ae7feb..000000000 --- a/test/session_management/authorization.test.js +++ /dev/null @@ -1,138 +0,0 @@ -const { parse } = require('url'); - -const { expect } = require('chai'); - -const bootstrap = require('../test_helper'); - -const route = '/auth'; - -describe('session management', () => { - before(bootstrap(__dirname)); - - ['get', 'post'].forEach((verb) => { - describe(`[session_management] ${verb} ${route} with session`, () => { - describe('success responses', () => { - before(function () { return this.login(); }); - it('provides session_state in the response', async function () { - const auth = new this.AuthorizationRequest({ - response_type: 'code', - scope: 'openid', - }); - - let sessionState; - await this.wrap({ route, verb, auth }) - .expect(302) - .expect(auth.validatePresence(['code', 'state', 'session_state'])) - .expect(auth.validateState) - .expect(auth.validateClientLocation) - .expect(({ headers: { location } }) => { - sessionState = parse(location, true).query.session_state; - }); - - await this.wrap({ route, verb, auth }) - .expect(302) - .expect(auth.validatePresence(['code', 'state', 'session_state'])) - .expect(auth.validateState) - .expect(auth.validateClientLocation) - .expect(({ headers: { location } }) => { - expect(parse(location, true).query.session_state).to.equal(sessionState); - }); - }); - - bootstrap.passInteractionChecks('native_client_prompt', () => { - it('doesn\'t omit the session_state for native applications', function () { - const auth = new this.AuthorizationRequest({ - client_id: 'client-native-claimed', - response_type: 'code', - scope: 'openid', - code_challenge_method: 'S256', - code_challenge: 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM', - }); - - return this.wrap({ route, verb, auth }) - .expect(302) - .expect(auth.validatePresence(['code', 'state', 'session_state'])) - .expect(auth.validateState) - .expect(auth.validateClientLocation); - }); - }); - - it('sets a _state.clientId cookies', function () { - const auth = new this.AuthorizationRequest({ - response_type: 'code', - scope: 'openid', - }); - - return this.wrap({ route, verb, auth }) - .expect(() => { - const state = this.agent.jar.getCookie('_state.client', { path: '/' }); - expect(state).to.be.ok; - }); - }); - }); - - describe('error responses', () => { - before(function () { return this.logout(); }); - - it('provides salted session_state in the response', async function () { - const auth = new this.AuthorizationRequest({ - prompt: 'none', - response_type: 'code', - scope: 'openid', - }); - - let sessionState; - await this.wrap({ route, verb, auth }) - .expect(302) - .expect(auth.validatePresence(['error', 'error_description', 'session_state', 'state'])) - .expect(auth.validateState) - .expect(auth.validateClientLocation) - .expect(({ headers: { location } }) => { - sessionState = parse(location, true).query.session_state; - expect(sessionState).to.contain('.'); - }); - - await this.wrap({ route, verb, auth }) - .expect(302) - .expect(auth.validatePresence(['error', 'error_description', 'session_state', 'state'])) - .expect(auth.validateState) - .expect(auth.validateClientLocation) - .expect(({ headers: { location } }) => { - expect(parse(location, true).query.session_state).not.to.equal(sessionState); - }); - }); - - it('doesn\'t omit the session_state for native applications', function () { - const auth = new this.AuthorizationRequest({ - prompt: 'none', - client_id: 'client-native-claimed', - response_type: 'code', - scope: 'openid', - code_challenge_method: 'S256', - code_challenge: 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM', - }); - - return this.wrap({ route, verb, auth }) - .expect(302) - .expect(auth.validatePresence(['error', 'error_description', 'state', 'session_state'])) - .expect(auth.validateState) - .expect(auth.validateClientLocation); - }); - - it('sets a _state.clientId cookies', function () { - const auth = new this.AuthorizationRequest({ - prompt: 'none', - response_type: 'code', - scope: 'openid', - }); - - return this.wrap({ route, verb, auth }) - .expect(() => { - const state = this.agent.jar.getCookie('_state.client', { path: '/' }); - expect(state).to.be.ok; - }); - }); - }); - }); - }); -}); diff --git a/test/session_management/check_session_endpoint.test.js b/test/session_management/check_session_endpoint.test.js deleted file mode 100644 index 092ced4cd..000000000 --- a/test/session_management/check_session_endpoint.test.js +++ /dev/null @@ -1,122 +0,0 @@ -const { expect } = require('chai'); - -const bootstrap = require('../test_helper'); - -describe('check_session_iframe', () => { - before(bootstrap(__dirname)); - before(function () { - this.provider.use(async (ctx, next) => { - ctx.response.set('X-Frame-Options', 'SAMEORIGIN'); - ctx.response.set('Content-Security-Policy', "default-src 'none'; frame-ancestors 'self' example.com *.example.net; script-src 'self'; connect-src 'self'; img-src 'self'; style-src 'self';"); - await next(); - }); - }); - - it('responds with frameable html', async function () { - await this.agent.get('/session/check') - .expect(200) - .expect('content-type', /text\/html/) - .expect((response) => { - expect(response.headers['x-frame-options']).not.to.be.ok; - expect(response.headers['content-security-policy']).not.to.match(/frame-ancestors/); - }); - - return this.agent.get('/session/check') - .expect(200) - .expect('content-type', /text\/html/) - .expect((response) => { - expect(response.headers['x-frame-options']).not.to.be.ok; - expect(response.headers['content-security-policy']).not.to.match(/frame-ancestors/); - }); - }); - - it('does not populate ctx.oidc.entities', function (done) { - this.provider.use(this.assertOnce((ctx) => { - expect(ctx.oidc.entities).to.be.empty; - }, done)); - - this.agent.get('/session/check').end(() => {}); - }); -}); - -describe('check_session_endpoint origin check', () => { - before(bootstrap(__dirname)); - - it('responds with a 204 when origin is allowed for a client_id', function () { - return this.agent.post('/session/check') - .send({ client_id: 'client', origin: 'https://client.example.com' }) - .expect(204) - .expect(''); - }); - - it('does populate ctx.oidc.entities', function (done) { - this.provider.use(this.assertOnce((ctx) => { - expect(ctx.oidc.entities).to.have.keys('Client'); - }, done)); - - this.agent.post('/session/check') - .send({ client_id: 'client', origin: 'https://client.example.com' }) - .end(() => {}); - }); - - it('checks that origin and client_id are provided', async function () { - await this.agent.post('/session/check') - .send({ client_id: 'client' }) - .expect(400) - .expect({ - error: 'invalid_request', - error_description: "missing required parameter 'origin'", - }); - await this.agent.post('/session/check') - .send({ origin: 'https://client.example.com' }) - .expect(400) - .expect({ - error: 'invalid_request', - error_description: "missing required parameter 'client_id'", - }); - await this.agent.post('/session/check') - .send({}) - .expect(400) - .expect({ - error: 'invalid_request', - error_description: "missing required parameters 'origin' and 'client_id'", - }); - }); - - it('checks that origin and client_id are strings', async function () { - await this.agent.post('/session/check') - .send({ client_id: 1, origin: 'https://client.example.com' }) - .expect(400) - .expect({ - error: 'invalid_request', - error_description: 'only string parameter values are expected', - }); - await this.agent.post('/session/check') - .send({ client_id: 'client', origin: 1 }) - .expect(400) - .expect({ - error: 'invalid_request', - error_description: 'only string parameter values are expected', - }); - }); - - it('checks the client is a valid one', async function () { - await this.agent.post('/session/check') - .send({ client_id: 'not-found-client', origin: 'https://client.example.com' }) - .expect(400) - .expect({ - error: 'invalid_client', - error_description: 'client is invalid', - }); - }); - - it('checks the client has a given origin amongst its redirect_uris origins', async function () { - await this.agent.post('/session/check') - .send({ client_id: 'client', origin: 'https://example.com' }) - .expect(403) - .expect({ - error: 'invalid_request', - error_description: 'origin not allowed', - }); - }); -}); diff --git a/test/session_management/discovery.test.js b/test/session_management/discovery.test.js deleted file mode 100644 index b4e63adce..000000000 --- a/test/session_management/discovery.test.js +++ /dev/null @@ -1,18 +0,0 @@ -const { expect } = require('chai'); - -const bootstrap = require('../test_helper'); - -describe('configuration features.encryption', () => { - before(bootstrap(__dirname)); - - it('extends discovery', function () { - return this.agent.get('/.well-known/openid-configuration') - .expect(200) - .expect((response) => { - expect(response.body).to.contain.keys( - 'check_session_iframe', - 'end_session_endpoint', - ); - }); - }); -}); diff --git a/test/session_management/end_session.test.js b/test/session_management/end_session.test.js deleted file mode 100644 index dbff4c6e2..000000000 --- a/test/session_management/end_session.test.js +++ /dev/null @@ -1,130 +0,0 @@ -const { parse: parseUrl } = require('url'); - -const sinon = require('sinon').createSandbox(); -const { expect } = require('chai'); - -const bootstrap = require('../test_helper'); - -describe('[session_management]', () => { - before(bootstrap(__dirname)); - - beforeEach(function () { return this.login(); }); - afterEach(function () { return this.logout(); }); - afterEach(sinon.restore); - - beforeEach(function () { - return this.agent.get('/auth') - .query({ - client_id: 'client', - scope: 'openid', - nonce: String(Math.random()), - response_type: 'id_token', - redirect_uri: 'https://client.example.com/cb', - }) - .expect(302) - .expect((response) => { - const { query: { id_token: idToken } } = parseUrl(response.headers.location.replace('#', '?'), true); - this.idToken = idToken; - }); - }); - - describe('POST end_session', () => { - it('destroys complete session if user wants to', function () { - const sessionId = this.getSessionId(); - const adapter = this.TestAdapter.for('Session'); - sinon.spy(adapter, 'destroy'); - sinon.spy(adapter, 'upsert'); - - this.getSession().state = { secret: '123', postLogoutRedirectUri: '/', clientId: 'client' }; - - return this.agent.post('/session/end/confirm') - .send({ xsrf: '123', logout: 'yes' }) - .type('form') - .expect(302) - .expect((response) => { - expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; samesite=none; httponly'); - expect(response.headers['set-cookie']).to.contain('_state.client.legacy=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly'); - expect(adapter.destroy.called).to.be.true; - expect(adapter.upsert.called).not.to.be.true; - expect(adapter.destroy.withArgs(sessionId).calledOnce).to.be.true; - }); - }); - - it('only clears one clients session if user doesnt wanna log out', function () { - const adapter = this.TestAdapter.for('Session'); - sinon.spy(adapter, 'destroy'); - let session = this.getSession(); - const oldId = this.getSessionId(); - session.state = { secret: '123', postLogoutRedirectUri: '/', clientId: 'client' }; - - expect(session.authorizations.client).to.be.ok; - - return this.agent.post('/session/end/confirm') - .send({ xsrf: '123' }) - .type('form') - .expect(302) - .expect((response) => { - session = this.getSession(); - expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; samesite=none; httponly'); - expect(response.headers['set-cookie']).to.contain('_state.client.legacy=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly'); - expect(session.authorizations.client).to.be.undefined; - expect(session.state).to.be.undefined; - expect(this.getSessionId()).not.to.eql(oldId); - expect(adapter.destroy.calledOnceWith(oldId)).to.be.true; - }); - }); - - it('follows a domain if configured', function () { - this.getSession().state = { secret: '123', postLogoutRedirectUri: '/', clientId: 'client' }; - - i(this.provider).configuration().cookies.long.domain = '.oidc.dev'; - - return this.agent.post('/session/end/confirm') - .send({ xsrf: '123', logout: 'yes' }) - .type('form') - .expect(() => { - delete i(this.provider).configuration().cookies.long.domain; - }) - .expect(302) - .expect((response) => { - expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; samesite=none; httponly'); - expect(response.headers['set-cookie']).to.contain('_state.client.legacy=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; httponly'); - }); - }); - - it('forwards the state too', function () { - this.getSession().state = { - secret: '123', postLogoutRedirectUri: '/', clientId: 'client', state: 'foobar', - }; - - i(this.provider).configuration().cookies.long.domain = '.oidc.dev'; - - return this.agent.post('/session/end/confirm') - .send({ xsrf: '123', logout: 'yes' }) - .type('form') - .expect(() => { - delete i(this.provider).configuration().cookies.long.domain; - }) - .expect(302) - .expect('location', '/?state=foobar') - .expect((response) => { - expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; samesite=none; httponly'); - expect(response.headers['set-cookie']).to.contain('_state.client.legacy=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; httponly'); - }); - }); - - it('handles a no existing session state', async function () { - Object.assign(this.getSession(), { - state: { - secret: '123', postLogoutRedirectUri: '/', clientId: 'client', state: 'foobar', - }, - authorizations: undefined, - }); - - return this.agent.post('/session/end/confirm') - .send({ xsrf: '123' }) - .type('form') - .expect(302); - }); - }); -}); diff --git a/test/session_management/session_management.config.js b/test/session_management/session_management.config.js deleted file mode 100644 index 0381445cc..000000000 --- a/test/session_management/session_management.config.js +++ /dev/null @@ -1,25 +0,0 @@ -const cloneDeep = require('lodash/cloneDeep'); -const merge = require('lodash/merge'); - -const config = cloneDeep(require('../default.config')); - -merge(config.features, { sessionManagement: { enabled: true } }); - -module.exports = { - config, - clients: [ - { - client_id: 'client', - client_secret: 'secret', - response_types: ['code', 'id_token'], - grant_types: ['authorization_code', 'implicit'], - redirect_uris: ['https://client.example.com/cb'], - }, - { - client_id: 'client-native-claimed', - client_secret: 'secret', - application_type: 'native', - redirect_uris: ['https://client.example.com/cb'], - }, - ], -}; diff --git a/test/test_helper.js b/test/test_helper.js index 9e0148e1a..651fcd767 100644 --- a/test/test_helper.js +++ b/test/test_helper.js @@ -107,13 +107,6 @@ module.exports = function testHelper(dir, { `_session.legacy.sig=; path=/; expires=${expire.toGMTString()}; httponly`, ]; - clients.forEach((cl) => { - cookies.push(`_state.${cl.client_id}=; path=/; expires=${expire.toGMTString()}; httponly`); - cookies.push(`_state.${cl.client_id}.sig=; path=/; expires=${expire.toGMTString()}; httponly`); - cookies.push(`_state.${cl.client_id}.legacy=; path=/; expires=${expire.toGMTString()}; httponly`); - cookies.push(`_state.${cl.client_id}.legacy.sig=; path=/; expires=${expire.toGMTString()}; httponly`); - }); - return agent._saveCookies.bind(agent)({ headers: { 'set-cookie': cookies } }); } @@ -132,7 +125,7 @@ module.exports = function testHelper(dir, { const sessionCookie = `_session=${sessionId}; path=/; expires=${expire.toGMTString()}; httponly`; const cookies = [sessionCookie]; - let [pre, ...post] = sessionCookie.split(';'); + const [pre, ...post] = sessionCookie.split(';'); cookies.push([`_session.sig=${keys.sign(pre)}`, ...post].join(';')); session.authorizations = {}; @@ -152,13 +145,6 @@ module.exports = function testHelper(dir, { rejectedScopes, rejectedClaims, }; - - if (i(provider).configuration('features.sessionManagement.enabled')) { - const cookie = `_state.${cl.client_id}=${session.stateFor(cl.client_id)}; path=/; expires=${expire.toGMTString()}`; - cookies.push(cookie); - [pre, ...post] = cookie.split(';'); - cookies.push([`_state.${cl.client_id}.sig=${keys.sign(pre)}`, ...post].join(';')); - } }); let ttl = i(provider).configuration('ttl.Session'); diff --git a/types/index.d.ts b/types/index.d.ts index 0532b0130..a10c31f2c 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -193,7 +193,6 @@ declare class Session extends BaseModel { transient?: boolean; }): void; authorizationFor(clientId: string): ClientAuthorizationState | void; - stateFor(clientId: string): string; sidFor(clientId: string): string; sidFor(clientId: string, value: string): void; grantIdFor(clientId: string): string; @@ -524,7 +523,6 @@ declare class Client { responseTypeAllowed(type: ResponseType): boolean; grantTypeAllowed(type: string): boolean; redirectUriAllowed(redirectUri: string): boolean; - checkSessionOriginAllowed(origin: string): boolean; webMessageUriAllowed(webMessageUri: string): boolean; requestUriAllowed(requestUri: string): boolean; postLogoutRedirectUriAllowed(postLogoutRedirectUri: string): boolean; @@ -883,12 +881,6 @@ export interface Configuration { ack?: 'draft-01' }, - sessionManagement?: { - enabled?: boolean, - keepHeaders?: boolean, - ack?: 28 | 'draft-28' | 'draft-29' | 'draft-30', - }, - backchannelLogout?: { enabled?: boolean, ack?: 4 | 'draft-04' | 'draft-05' | 'draft-06' diff --git a/types/oidc-provider-tests.ts b/types/oidc-provider-tests.ts index 24f4dec2b..5eb988499 100644 --- a/types/oidc-provider-tests.ts +++ b/types/oidc-provider-tests.ts @@ -341,7 +341,6 @@ const provider = new Provider('https://op.example.com', { jwtUserinfo: { enabled: false }, webMessageResponseMode: { enabled: false, ack: 'id-00' }, revocation: { enabled: false }, - sessionManagement: { enabled: false, ack: 28, keepHeaders: false }, jwtIntrospection: { enabled: false, ack: 'draft-09' }, jwtResponseModes: { enabled: false, ack: 2 }, pushedAuthorizationRequests: { enabled: false, ack: 'draft-02' }, @@ -558,10 +557,6 @@ provider.Client.prototype.postLogoutRedirectUriAllowed = function (postLogoutRed postLogoutRedirectUri.substring(0); return true; } -provider.Client.prototype.checkSessionOriginAllowed = function (checkSessionOrigin) { - checkSessionOrigin.substring(0); - return true; -} provider.Client.prototype.webMessageUriAllowed = function (webMessageUri) { webMessageUri.substring(0); return true;