From 7ef389bf7776fb8a898bafaf44d2b32897d5b399 Mon Sep 17 00:00:00 2001 From: Jonathan Perret Date: Thu, 24 Oct 2024 16:10:51 +0200 Subject: [PATCH] OIDConfig.getLogoutRedirectUrl: use redirectURL when possible When GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT is true, we can actually use the requested destination URL since there is no constraint imposed by the OIDC provider. Co-authored-by: fflorent --- app/server/lib/OIDCConfig.ts | 13 ++++++------- test/server/lib/OIDCConfig.ts | 5 +++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index b14a269350..d8e3af33f3 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -279,20 +279,19 @@ export class OIDCConfig { }); } - public async getLogoutRedirectUrl(req: express.Request): Promise { - const session: SessionObj|undefined = (req as RequestWithLogin).session; - const stableRedirectUri = new URL('/signed-out', getOriginUrl(req)).href; - // For IdPs that don't have end_session_endpoint, we just redirect to the logout page. + public async getLogoutRedirectUrl(req: express.Request, redirectUrl: URL): Promise { + // For IdPs that don't have end_session_endpoint, we just redirect to the requested page. if (this._skipEndSessionEndpoint) { - // Ignore redirectUrl because OIDC providers don't allow variable redirect URIs - return stableRedirectUri; + return redirectUrl.href; } // Alternatively, we could use a logout URL specified by configuration. if (this._endSessionEndpoint) { return this._endSessionEndpoint; } + // Ignore redirectUrl because OIDC providers don't allow variable redirect URIs + const stableRedirectUri = new URL('/signed-out', getOriginUrl(req)).href; + const session: SessionObj|undefined = (req as RequestWithLogin).session; return this._client.endSessionUrl({ - // Ignore redirectUrl because OIDC providers don't allow variable redirect URIs post_logout_redirect_uri: stableRedirectUri, id_token_hint: session?.oidc?.idToken, }); diff --git a/test/server/lib/OIDCConfig.ts b/test/server/lib/OIDCConfig.ts index d7f46814d2..99e1dfee51 100644 --- a/test/server/lib/OIDCConfig.ts +++ b/test/server/lib/OIDCConfig.ts @@ -752,6 +752,7 @@ describe('OIDCConfig', () => { }); describe('getLogoutRedirectUrl', () => { + const REDIRECT_URL = new URL('http://localhost:8484/docs/signed-out'); const STABLE_LOGOUT_URL = new URL('http://localhost:8484/signed-out'); const URL_RETURNED_BY_CLIENT = 'http://localhost:8484/logout_url_from_issuer'; const ENV_VALUE_GRIST_OIDC_IDP_END_SESSION_ENDPOINT = 'http://localhost:8484/logout'; @@ -767,7 +768,7 @@ describe('OIDCConfig', () => { env: { GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT: 'true', }, - expectedUrl: STABLE_LOGOUT_URL.href, + expectedUrl: REDIRECT_URL.href, }, { itMsg: 'should use the GRIST_OIDC_IDP_END_SESSION_ENDPOINT when it is set', env: { @@ -803,7 +804,7 @@ describe('OIDCConfig', () => { }, session: 'session' in ctx ? ctx.session : FAKE_SESSION } as unknown as RequestWithLogin; - const url = await config.getLogoutRedirectUrl(req); + const url = await config.getLogoutRedirectUrl(req, REDIRECT_URL); assert.equal(url, ctx.expectedUrl); if (ctx.expectedLogoutParams) { assert.isTrue(clientStub.endSessionUrl.calledOnce);