From be1a401341c06d113a2d8f27ec687ce3551bec69 Mon Sep 17 00:00:00 2001 From: Sharon Stratsianis Date: Sun, 25 Oct 2026 16:08:57 +1100 Subject: [PATCH] temporary fix double slash issue when redirecting --- src/authSession/authSession.ts | 10 ++- src/authSession/loginCallback.ts | 36 ++++++++++ src/authn/SolidAuthnLogic.ts | 110 +++++++++++++++++-------------- test/loginCallback.test.ts | 35 ++++++++++ 4 files changed, 137 insertions(+), 54 deletions(-) create mode 100644 src/authSession/loginCallback.ts create mode 100644 test/loginCallback.test.ts diff --git a/src/authSession/authSession.ts b/src/authSession/authSession.ts index e6dbb14c..cc83b9b5 100644 --- a/src/authSession/authSession.ts +++ b/src/authSession/authSession.ts @@ -13,6 +13,7 @@ import type { Session as OidcSession } from '@uvdsl/solid-oidc-client-browser/core' import { _session } from './session' import { resolveIssuerForLogin } from './issuer' +import { loginCallbackUrl } from './loginCallback' import { SessionEvents } from './events' import { legacySessionInfo, restoreSession, subscribeIdentity, type SessionLike } from './identityState' @@ -72,18 +73,21 @@ const originalLogin = typeof sessionAny.login === 'function' : undefined if (originalLogin) { + // Container and root callbacks are moved to a child path; see loginCallback.ts. + const callbackUrl = (redirectUrl?: string) => + typeof redirectUrl === 'string' ? loginCallbackUrl(redirectUrl) : redirectUrl const compatLogin = async (idpOrOptions: any, redirectUri?: string): Promise => { if (idpOrOptions && typeof idpOrOptions === 'object' && !Array.isArray(idpOrOptions)) { const oidcIssuer = idpOrOptions.oidcIssuer ?? idpOrOptions.idp ?? idpOrOptions.issuer const redirectUrl = idpOrOptions.redirectUrl ?? idpOrOptions.redirect_uri ?? idpOrOptions.redirectUri if (typeof oidcIssuer === 'string' && typeof redirectUrl === 'string') { - return originalLogin(await resolveIssuerForLogin(oidcIssuer), redirectUrl) + return originalLogin(await resolveIssuerForLogin(oidcIssuer), callbackUrl(redirectUrl)) } } if (typeof idpOrOptions === 'string') { - return originalLogin(await resolveIssuerForLogin(idpOrOptions), redirectUri) + return originalLogin(await resolveIssuerForLogin(idpOrOptions), callbackUrl(redirectUri)) } - return originalLogin(idpOrOptions, redirectUri) + return originalLogin(idpOrOptions, callbackUrl(redirectUri)) } sessionAny.login = compatLogin as LoginCompat } diff --git a/src/authSession/loginCallback.ts b/src/authSession/loginCallback.ts new file mode 100644 index 00000000..617c8223 --- /dev/null +++ b/src/authSession/loginCallback.ts @@ -0,0 +1,36 @@ +/** + * Login callback URL workaround for Pivot / Community Solid Server setups. + * + * After login the identity provider sends the browser back to the redirect URL with + * `?code=…&state=…&iss=…` appended. Pivot's PivotResponseWriter answers a GET that is + * refused (401/403/404) on a URL not ending in `/` with a 301 to the same path plus `/`. + * It checks the raw URL, query included, and drops the query from the redirect. So a + * callback to a private container or server root, e.g. `http://localhost:3200/?code=…`, + * is redirected to `http://localhost:3200//` and the login is lost. + * + * When the page is a container (its path ends in `/`), the callback is therefore sent to + * a child path that does not exist. The server answers it with the data browser without + * redirecting and the query survives. Once the callback has been handled, the + * marker is removed from the address bar so the app shows the original page. + * + * Remove this once Pivot no longer redirects such URLs (see mydocs/pivotchange.md). + */ + +export const LOGIN_CALLBACK_SEGMENT = '.oidc-callback' + +/** The URL the identity provider should send the browser back to after login. */ +export function loginCallbackUrl (pageUrl: string): string { + const url = new URL(pageUrl) + url.hash = '' + if (url.pathname.endsWith('/')) url.pathname += LOGIN_CALLBACK_SEGMENT + return url.href +} + +/** After the callback has been handled, show the page the login started from again. */ +export function restorePageAfterLoginCallback (): void { + if (typeof window === 'undefined') return + const url = new URL(window.location.href) + if (!url.pathname.endsWith('/' + LOGIN_CALLBACK_SEGMENT)) return + url.pathname = url.pathname.slice(0, -LOGIN_CALLBACK_SEGMENT.length) + window.history.replaceState(window.history.state, document.title, url.href) +} diff --git a/src/authn/SolidAuthnLogic.ts b/src/authn/SolidAuthnLogic.ts index 621d8335..a313e606 100644 --- a/src/authn/SolidAuthnLogic.ts +++ b/src/authn/SolidAuthnLogic.ts @@ -1,6 +1,7 @@ import { namedNode, NamedNode, sym } from 'rdflib' import { appContext, offlineTestID } from './authUtil' import * as debug from '../util/debug' +import { restorePageAfterLoginCallback } from '../authSession/loginCallback' import { effectiveIdentity, restoreSession, @@ -144,62 +145,69 @@ export class SolidAuthnLogic implements AuthnLogic { private async resolveCurrentUser (): Promise { const sessionAny = this.session as any - /** - * Handle a successful authentication redirect - */ - const redirectUrl = new URL(window.location.href) - redirectUrl.hash = '' - if (typeof sessionAny?.handleIncomingRedirect === 'function') { - await sessionAny.handleIncomingRedirect({ - restorePreviousSession: true, - url: redirectUrl.href - }) - } else { - // uvdsl-style session (no handleIncomingRedirect): restore then handle redirect. - // - // The worker-backed session (WebWorkerSession) resolves restore() ONLY - // when the SharedWorker posts a message back. If the worker asset can't - // be fetched — local/dev servers that don't serve the RefreshWorker - // chunk at the resolved URL, wrong MIME type, CSP, or a worker that - // fails before `onconnect` — the promise never settles and the login - // UI would spin forever. Race it against a timeout and treat a stall - // as "no previous session" so the page can render the login button. - const wasActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) - // The shared restore lock also covers this call: a refocus resync can be - // in flight at the same time, and two overlapping restores could write an - // older identity back over a newer one. - const restoring = restoreSession(sessionAny) - if (restoring) { - try { - await withRestoreTimeout(restoring) - } catch (error) { - const message = error instanceof Error ? error.message : String(error) - // A failed restore on an inactive session just means "no usable - // session to restore" — whether that's "No session to restore.", - // a stale refresh token / dead client_id returning HTTP 400, or a - // missing session database. Never let it block the login UI: log - // and continue as logged-out so the page renders the login button. - // Only re-throw when the session actually became active, which is - // an unexpected refresh failure worth surfacing. + try { + /** + * Handle a successful authentication redirect + */ + const redirectUrl = new URL(window.location.href) + redirectUrl.hash = '' + if (typeof sessionAny?.handleIncomingRedirect === 'function') { + await sessionAny.handleIncomingRedirect({ + restorePreviousSession: true, + url: redirectUrl.href + }) + } else { + // uvdsl-style session (no handleIncomingRedirect): restore then handle redirect. + // + // The worker-backed session (WebWorkerSession) resolves restore() ONLY + // when the SharedWorker posts a message back. If the worker asset can't + // be fetched — local/dev servers that don't serve the RefreshWorker + // chunk at the resolved URL, wrong MIME type, CSP, or a worker that + // fails before `onconnect` — the promise never settles and the login + // UI would spin forever. Race it against a timeout and treat a stall + // as "no previous session" so the page can render the login button. + const wasActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) + // The shared restore lock also covers this call: a refocus resync can be + // in flight at the same time, and two overlapping restores could write an + // older identity back over a newer one. + const restoring = restoreSession(sessionAny) + if (restoring) { + try { + await withRestoreTimeout(restoring) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + // A failed restore on an inactive session just means "no usable + // session to restore" — whether that's "No session to restore.", + // a stale refresh token / dead client_id returning HTTP 400, or a + // missing session database. Never let it block the login UI: log + // and continue as logged-out so the page renders the login button. + // Only re-throw when the session actually became active, which is + // an unexpected refresh failure worth surfacing. + const isNowActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) + if (isNowActive && !/no session to restore/i.test(message)) { + throw error + } + debug.log(`Session restore failed, continuing logged-out: ${message}`) + } const isNowActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) - if (isNowActive && !/no session to restore/i.test(message)) { - throw error + if (!wasActive && isNowActive) { + sessionAny.events?.emit('sessionRestore', window.location.href) } - debug.log(`Session restore failed, continuing logged-out: ${message}`) - } - const isNowActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) - if (!wasActive && isNowActive) { - sessionAny.events?.emit('sessionRestore', window.location.href) } - } - if (typeof sessionAny?.handleRedirectFromLogin === 'function') { - const wasActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) - await sessionAny.handleRedirectFromLogin() - const isNowActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) - if (!wasActive && isNowActive) { - sessionAny.events?.emit('login') + if (typeof sessionAny?.handleRedirectFromLogin === 'function') { + const wasActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) + await sessionAny.handleRedirectFromLogin() + const isNowActive = sessionAny?.isActive ?? Boolean(sessionAny?.webId) + if (!wasActive && isNowActive) { + sessionAny.events?.emit('login') + } } } + + } finally { + // A container or root login came back on its callback path (see loginCallback.ts); + // show the original URL again before the page decides what to display. + restorePageAfterLoginCallback() } // Check to see if a hash was stored in local storage diff --git a/test/loginCallback.test.ts b/test/loginCallback.test.ts new file mode 100644 index 00000000..c6d5fe79 --- /dev/null +++ b/test/loginCallback.test.ts @@ -0,0 +1,35 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { loginCallbackUrl, restorePageAfterLoginCallback } from '../src/authSession/loginCallback' + +describe('loginCallbackUrl', () => { + it('moves a container or root callback to a child path', () => { + expect(loginCallbackUrl('http://localhost:3200/')).toBe('http://localhost:3200/.oidc-callback') + expect(loginCallbackUrl('http://localhost:3100/alice/Addressbook/#x')).toBe('http://localhost:3100/alice/Addressbook/.oidc-callback') + }) + + it('keeps the query of the page', () => { + expect(loginCallbackUrl('http://localhost:3100/alice/?a=1')).toBe('http://localhost:3100/alice/.oidc-callback?a=1') + }) + + it('leaves documents unchanged apart from the fragment', () => { + expect(loginCallbackUrl('http://localhost:3100/browse.html?uri=x#me')).toBe('http://localhost:3100/browse.html?uri=x') + }) +}) + +describe('restorePageAfterLoginCallback', () => { + const start = window.location.href + afterEach(() => window.history.replaceState(null, '', start)) + + it('removes the callback segment and keeps the query', () => { + window.history.replaceState(null, '', '/alice/Addressbook/.oidc-callback?a=1') + restorePageAfterLoginCallback() + expect(window.location.pathname).toBe('/alice/Addressbook/') + expect(window.location.search).toBe('?a=1') + }) + + it('leaves other pages alone', () => { + window.history.replaceState(null, '', '/alice/notes.oidc-callback') + restorePageAfterLoginCallback() + expect(window.location.pathname).toBe('/alice/notes.oidc-callback') + }) +})