From 8585170885af5dc39b64cda209d307b7b81d5060 Mon Sep 17 00:00:00 2001 From: itsrubberduck Date: Tue, 28 Jul 2026 19:14:20 +0200 Subject: [PATCH] fix(sso): hand the issuer /auth/callback so the code gets redeemed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auth guard asked the issuer to come back to the page the visitor wanted. The issuer appends ?code= to whatever URL it is given, but only /auth/callback redeems a code — so it arrived on the target page, sat there unread, the guard found no session and bounced back for a fresh code. The browser ping-ponged between the two hosts until the user gave up. The guard now hands over /auth/callback and carries the wanted page in its redirect parameter, which is exactly what that page already expected. A spent code in the URL is dropped rather than carried along, so a stale link cannot turn into a redemption error one hop later. URL building moved into shared/utils/ssoHandoff.ts because the same mistake existed twice — the retry button on the callback page had it too — and because a redirect loop deserves a regression test that does not need a browser. Co-Authored-By: Claude Opus 5 --- app/middleware/require-auth.ts | 6 ++-- app/pages/auth/callback.vue | 6 ++-- shared/utils/ssoHandoff.ts | 32 +++++++++++++++++ tests/shared/ssoHandoff.test.ts | 62 +++++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 shared/utils/ssoHandoff.ts create mode 100644 tests/shared/ssoHandoff.test.ts diff --git a/app/middleware/require-auth.ts b/app/middleware/require-auth.ts index 388843f..b60bc7a 100644 --- a/app/middleware/require-auth.ts +++ b/app/middleware/require-auth.ts @@ -1,5 +1,6 @@ import { defineNuxtRouteMiddleware, navigateTo, useRuntimeConfig } from '#app' import { useAuthStore } from '~/stores/auth' +import { buildIssuerLoginUrl } from '~~/shared/utils/ssoHandoff' export default defineNuxtRouteMiddleware(async (to) => { const config = useRuntimeConfig() @@ -31,10 +32,7 @@ export default defineNuxtRouteMiddleware(async (to) => { return navigateTo(`/login?redirect=${encodeURIComponent(target)}`) } - // The issuer needs an absolute URL to come back to, and it will only accept - // one whose origin is on its allowlist. - const redirect = new URL(target, window.location.origin).toString() - return navigateTo(`${issuer}/login?redirect=${encodeURIComponent(redirect)}`, { + return navigateTo(buildIssuerLoginUrl(issuer, window.location.origin, target), { external: true, }) }) diff --git a/app/pages/auth/callback.vue b/app/pages/auth/callback.vue index 9566bad..9af83ae 100644 --- a/app/pages/auth/callback.vue +++ b/app/pages/auth/callback.vue @@ -18,6 +18,7 @@ import { onMounted, ref } from 'vue' import { useRoute, useRouter, useRuntimeConfig, navigateTo } from '#app' import { useAuthStore } from '~/stores/auth' +import { buildIssuerLoginUrl } from '~~/shared/utils/ssoHandoff' // Consumer end of the SSO handoff: the issuer sent the browser here with a // one-time code. Redeeming it happens server-side (the code alone is useless @@ -41,11 +42,12 @@ function safeRedirectTarget(): string { function retry() { const issuer = String(config.public.authIssuer || '').replace(/\/+$/, '') - const target = new URL(safeRedirectTarget(), window.location.origin).toString() if (!issuer) { return router.replace(`/login?redirect=${encodeURIComponent(safeRedirectTarget())}`) } - return navigateTo(`${issuer}/login?redirect=${encodeURIComponent(target)}`, { external: true }) + return navigateTo(buildIssuerLoginUrl(issuer, window.location.origin, safeRedirectTarget()), { + external: true, + }) } onMounted(async () => { diff --git a/shared/utils/ssoHandoff.ts b/shared/utils/ssoHandoff.ts new file mode 100644 index 0000000..cab3abd --- /dev/null +++ b/shared/utils/ssoHandoff.ts @@ -0,0 +1,32 @@ +/** + * Builds the URL that sends an unauthenticated visitor to the SSO issuer. + * + * The one rule that matters: the issuer appends `?code=` to whatever URL it is + * handed, and only `/auth/callback` redeems that code. Handing it the page the + * user actually wanted leaves the code unread in the address bar — the auth + * guard then bounces back to the issuer for a fresh one, and the browser ping + * pongs between the two hosts forever. + */ + +/** + * Drops a `code` already present in a path. Codes are single-use, so carrying + * a spent one through the round trip can only produce a redemption error on + * the far side. + */ +export function stripSsoCode(path: string, origin: string): string { + if (!path.includes('code=')) return path + const url = new URL(path, origin) + url.searchParams.delete('code') + return `${url.pathname}${url.search}${url.hash}` +} + +/** + * @param issuer the website origin, without a trailing slash + * @param origin this app's own origin + * @param target the in-app path the visitor was trying to reach + */ +export function buildIssuerLoginUrl(issuer: string, origin: string, target: string): string { + const callback = new URL('/auth/callback', origin) + callback.searchParams.set('redirect', stripSsoCode(target || '/', origin)) + return `${issuer.replace(/\/+$/, '')}/login?redirect=${encodeURIComponent(callback.toString())}` +} diff --git a/tests/shared/ssoHandoff.test.ts b/tests/shared/ssoHandoff.test.ts new file mode 100644 index 0000000..ece2d30 --- /dev/null +++ b/tests/shared/ssoHandoff.test.ts @@ -0,0 +1,62 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' + +import { buildIssuerLoginUrl, stripSsoCode } from '~~/shared/utils/ssoHandoff' + +const ISSUER = 'https://opensquawk.de' +const ORIGIN = 'https://app.opensquawk.de' + +function redirectParam(url: string): string { + return new URL(url).searchParams.get('redirect') || '' +} + +describe('SSO handoff URL', () => { + it('sends the issuer to /auth/callback, not to the page the user wanted', () => { + // The redirect loop: the issuer appends ?code= to whatever it is handed, + // and only /auth/callback redeems it. Handing it '/' left the code unread + // and the guard bounced straight back for another one. + const url = buildIssuerLoginUrl(ISSUER, ORIGIN, '/') + const target = new URL(redirectParam(url)) + + assert.equal(target.origin, ORIGIN) + assert.equal(target.pathname, '/auth/callback') + }) + + it('carries the wanted page along so the callback can finish the trip', () => { + const url = buildIssuerLoginUrl(ISSUER, ORIGIN, '/classroom?lesson=3') + const target = new URL(redirectParam(url)) + + assert.equal(target.searchParams.get('redirect'), '/classroom?lesson=3') + }) + + it('points at the issuer login page and encodes the target as one parameter', () => { + const url = buildIssuerLoginUrl(ISSUER, ORIGIN, '/live-atc') + + assert.ok(url.startsWith('https://opensquawk.de/login?redirect=')) + // Exactly one query parameter — an unencoded '?' in the value would split + // the target into a second parameter and lose it. + assert.deepEqual([...new URL(url).searchParams.keys()], ['redirect']) + }) + + it('tolerates a trailing slash on the issuer', () => { + const url = buildIssuerLoginUrl('https://opensquawk.de/', ORIGIN, '/') + assert.ok(url.startsWith('https://opensquawk.de/login?')) + }) + + it('does not carry a spent code back to the issuer', () => { + const url = buildIssuerLoginUrl(ISSUER, ORIGIN, '/?code=already-used') + const target = new URL(redirectParam(url)) + + assert.equal(target.searchParams.get('redirect'), '/') + }) + + it('treats an empty target as the front page', () => { + const target = new URL(redirectParam(buildIssuerLoginUrl(ISSUER, ORIGIN, ''))) + assert.equal(target.searchParams.get('redirect'), '/') + }) + + it('leaves other query parameters alone when stripping the code', () => { + assert.equal(stripSsoCode('/classroom?lesson=3&code=x', ORIGIN), '/classroom?lesson=3') + assert.equal(stripSsoCode('/classroom?lesson=3', ORIGIN), '/classroom?lesson=3') + }) +})