From ce0c06d538cf925c198070b469c378a0fd92ef7f Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 26 Aug 2019 09:39:58 +0200 Subject: [PATCH] fix: do not use mounted app's ctx.cookies using ctx.cookies means cookie keys from the parent app need to be the same, this fix removes that requirement closes #517 --- lib/actions/authorization/interactions.js | 4 ++-- lib/actions/authorization/resume.js | 4 ++-- lib/actions/end_session.js | 6 +++--- lib/helpers/oidc_context.js | 2 ++ lib/helpers/process_session_state.js | 2 +- lib/provider.js | 10 +++------- lib/shared/session.js | 2 +- test/test_helper.js | 2 +- 8 files changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/actions/authorization/interactions.js b/lib/actions/authorization/interactions.js index 661ff73dd..7a71756d7 100644 --- a/lib/actions/authorization/interactions.js +++ b/lib/actions/authorization/interactions.js @@ -119,13 +119,13 @@ module.exports = async function interactions(resumeRouteName, ctx, next) { const destination = await interactionUrl(ctx, interactionSession); - ctx.cookies.set( + ctx.oidc.cookies.set( oidc.provider.cookieName('interaction'), oidc.uid, { path: url.parse(destination).pathname, ...cookieOptions }, ); - ctx.cookies.set( + ctx.oidc.cookies.set( oidc.provider.cookieName('resume'), oidc.uid, { ...cookieOptions, path: url.parse(returnTo).pathname }, diff --git a/lib/actions/authorization/resume.js b/lib/actions/authorization/resume.js index c58b3fe49..099142c45 100644 --- a/lib/actions/authorization/resume.js +++ b/lib/actions/authorization/resume.js @@ -18,7 +18,7 @@ module.exports = async function resumeAction(whitelist, resumeRouteName, ctx, ne 'expires', ); - const cookieId = ctx.cookies.get(ctx.oidc.provider.cookieName('resume'), cookieOptions); + const cookieId = ctx.oidc.cookies.get(ctx.oidc.provider.cookieName('resume'), cookieOptions); if (!cookieId || cookieId !== ctx.oidc.uid) { throw new errors.SessionNotFound('authorization request has expired'); } @@ -68,7 +68,7 @@ module.exports = async function resumeAction(whitelist, resumeRouteName, ctx, ne ...(ctx.params.user_code ? { user_code: ctx.params.user_code } : undefined), })).pathname, }; - ctx.cookies.set(ctx.oidc.provider.cookieName('resume'), null, clearOpts); + ctx.oidc.cookies.set(ctx.oidc.provider.cookieName('resume'), null, clearOpts); if (result && result.error) { const className = upperFirst(camelCase(result.error)); diff --git a/lib/actions/end_session.js b/lib/actions/end_session.js index 6f1beb584..837c5a335 100644 --- a/lib/actions/end_session.js +++ b/lib/actions/end_session.js @@ -186,12 +186,12 @@ module.exports = { if (cookies) { cookies.forEach((val) => { const name = val.slice(0, -1); - if (!name.endsWith('.sig')) ctx.cookies.set(name, null, opts); + if (!name.endsWith('.sig')) ctx.oidc.cookies.set(name, null, opts); }); } } - ctx.cookies.set(ctx.oidc.provider.cookieName('session'), null, opts); + ctx.oidc.cookies.set(ctx.oidc.provider.cookieName('session'), null, opts); } else if (state.clientId) { const grantId = session.grantIdFor(state.clientId); if (grantId) { @@ -204,7 +204,7 @@ module.exports = { delete session.authorizations[state.clientId]; } if (sessionManagement.enabled) { - ctx.cookies.set(`${ctx.oidc.provider.cookieName('state')}.${state.clientId}`, null, opts); + ctx.oidc.cookies.set(`${ctx.oidc.provider.cookieName('state')}.${state.clientId}`, null, opts); } session.resetIdentifier(); } diff --git a/lib/helpers/oidc_context.js b/lib/helpers/oidc_context.js index acefd59d7..79ffe2238 100644 --- a/lib/helpers/oidc_context.js +++ b/lib/helpers/oidc_context.js @@ -19,6 +19,7 @@ const resolveResponseMode = require('./resolve_response_mode'); module.exports = function getContext(provider) { const { clockTolerance, features: { dPoP: dPoPConfig } } = instance(provider).configuration(); + const { app } = provider; class OIDCContext extends events.EventEmitter { constructor(ctx) { @@ -31,6 +32,7 @@ module.exports = function getContext(provider) { this.uid = (ctx.params && ctx.params.uid) || nanoid(); this.entities = {}; this.claims = {}; + this.cookies = app.createContext(ctx.req, ctx.res).cookies; } get issuer() { // eslint-disable-line class-methods-use-this diff --git a/lib/helpers/process_session_state.js b/lib/helpers/process_session_state.js index 30778e3da..d2ab5cea2 100644 --- a/lib/helpers/process_session_state.js +++ b/lib/helpers/process_session_state.js @@ -5,7 +5,7 @@ const base64url = require('./base64url'); const instance = require('./weak_cache'); function processSessionState(ctx, redirectUri, salt) { - const { oidc: { session, client }, cookies } = ctx; + const { oidc: { session, client, cookies } } = ctx; const parsed = new URL(redirectUri); const { origin } = parsed; diff --git a/lib/provider.js b/lib/provider.js index f6966f17d..f7a7606a6 100644 --- a/lib/provider.js +++ b/lib/provider.js @@ -24,10 +24,6 @@ const { SessionNotFound } = require('./helpers/errors'); const models = require('./models'); const nanoid = require('./helpers/nanoid'); -function getCookie(name, opts, cookies) { - return cookies.get(name, opts); -} - function assertReqRes(req, res) { assert( req instanceof IncomingMessage || req instanceof Http2ServerRequest, @@ -43,10 +39,10 @@ function assertReqRes(req, res) { async function getInteraction(req, res) { assertReqRes.apply(undefined, arguments); // eslint-disable-line prefer-spread, prefer-rest-params - const { cookies } = this.app.createContext(req, res); - const id = getCookie.call(this, this.cookieName('interaction'), { + const ctx = this.app.createContext(req, res); + const id = ctx.cookies.get(this.cookieName('interaction'), { signed: instance(this).configuration('cookies.short.signed'), - }, cookies); + }); if (!id) { throw new SessionNotFound('interaction session id cookie not found'); } diff --git a/lib/shared/session.js b/lib/shared/session.js index 8ba69b1e9..ae1a45765 100644 --- a/lib/shared/session.js +++ b/lib/shared/session.js @@ -27,7 +27,7 @@ module.exports = async function sessionHandler(ctx, next) { // refresh the session duration if ((!ctx.oidc.session.new || ctx.oidc.session.touched) && !ctx.oidc.session.destroyed) { - ctx.cookies.set(sessionCookieName, ctx.oidc.session.id, instance(ctx.oidc.provider).configuration('cookies.long')); + ctx.oidc.cookies.set(sessionCookieName, ctx.oidc.session.id, instance(ctx.oidc.provider).configuration('cookies.long')); await ctx.oidc.session.save(); } diff --git a/test/test_helper.js b/test/test_helper.js index 370a864d0..643622037 100644 --- a/test/test_helper.js +++ b/test/test_helper.js @@ -104,7 +104,7 @@ module.exports = function testHelper(dir, { config: base = path.basename(dir), m session.authorizations = {}; clients.forEach((cl) => { - const ctx = new provider.OIDCContext({}); + const ctx = new provider.OIDCContext({ req: { socket: {} }, res: {} }); ctx.params = { scope, claims }; if (ctx.params.claims && typeof ctx.params.claims !== 'string') {