diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bd79d1a8..e786263ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html The following changes have been implemented but not released yet: +### Bugfixes + +- Browser: recover from a stored dynamic client registration that the OIDC provider no longer + recognises. Previously, if the provider had dropped a client's registration (for example on a + restart), silent authentication would redirect to the provider, get a non-redirectable + "unknown client" error, and — because the `KEY_CURRENT_URL` marker was never cleared — retry + indefinitely with the same rejected client ID, leaving the user's session permanently broken. + On detecting an incomplete previous silent-authentication attempt, the stored client + registration is now discarded so the next login re-registers a fresh client. + ## [5.0.0](https://github.com/inrupt/solid-client-authn-js/releases/tag/v5.0.0) - 2026-06-03 ### Breaking changes diff --git a/packages/browser/src/ClientAuthentication.ts b/packages/browser/src/ClientAuthentication.ts index e05cb61c5..d7c9fc5ee 100644 --- a/packages/browser/src/ClientAuthentication.ts +++ b/packages/browser/src/ClientAuthentication.ts @@ -36,6 +36,7 @@ import { } from "@inrupt/solid-client-authn-core"; import { normalizeCallbackUrl } from "@inrupt/oidc-client-ext"; import type { EventEmitter } from "events"; +import type { SessionInfoManager } from "./sessionInfo/SessionInfoManager"; /** * Checks if a client's registration has expired. @@ -109,6 +110,16 @@ export default class ClientAuthentication extends ClientAuthenticationBase { return sessionInfo; }; + // Discards the stored dynamic client registration for a session so that the next login + // registers a fresh client. Used to recover when the OIDC provider no longer recognises the + // stored client (e.g. it dropped the registration on a restart), which would otherwise cause + // silent authentication to loop with a client ID the provider rejects. + clearClientRegistrationInfo = async (sessionId: string): Promise => { + await ( + this.sessionInfoManager as SessionInfoManager + ).clearClientRegistrationInfo(sessionId); + }; + handleIncomingRedirect = async ( url: string, eventEmitter: EventEmitter, diff --git a/packages/browser/src/Session.spec.ts b/packages/browser/src/Session.spec.ts index adf17462f..e21b699e8 100644 --- a/packages/browser/src/Session.spec.ts +++ b/packages/browser/src/Session.spec.ts @@ -566,6 +566,46 @@ describe("Session", () => { ).toBeInstanceOf(EventEmitter); }); + it("clears the stored client registration instead of looping when a previous silent authentication did not complete", async () => { + const sessionId = "mySession"; + mockLocalStorage({ + [KEY_CURRENT_SESSION]: sessionId, + // A leftover KEY_CURRENT_URL means a previous silent-auth attempt was started but never + // completed (e.g. the provider rejected a stale, dropped client with a non-redirectable error). + [KEY_CURRENT_URL]: "https://mock.current/location", + }); + mockLocation("https://mock.current/location"); + const clientAuthentication = mockClientAuthentication(); + clientAuthentication.clearClientRegistrationInfo = jest + .fn() + .mockResolvedValue(undefined); + clientAuthentication.login = jest.fn(); + clientAuthentication.validateCurrentSession = + jest.fn() as typeof clientAuthentication.validateCurrentSession; + const incomingRedirectPromise = Promise.resolve(); + clientAuthentication.handleIncomingRedirect = jest + .fn() + .mockReturnValueOnce( + incomingRedirectPromise, + ) as typeof clientAuthentication.handleIncomingRedirect; + + const mySession = new Session({ clientAuthentication }); + await mySession.handleIncomingRedirect({ + url: "https://some.redirect/url", + restorePreviousSession: true, + }); + await incomingRedirectPromise; + + // The stale registration is discarded so the next login re-registers. + expect( + clientAuthentication.clearClientRegistrationInfo, + ).toHaveBeenCalledWith(sessionId); + // It must NOT re-attempt silent auth with the same (rejected) client — that is the loop. + expect(clientAuthentication.login).not.toHaveBeenCalled(); + // The marker is cleared so a later, legitimate silent auth can proceed normally. + expect(window.localStorage.getItem(KEY_CURRENT_URL)).toBeNull(); + }); + it("resolves handleIncomingRedirect if silent authentication could not be started", async () => { const sessionId = "mySession"; mockLocalStorage({ diff --git a/packages/browser/src/Session.ts b/packages/browser/src/Session.ts index 425df6ac8..50fe5f49b 100644 --- a/packages/browser/src/Session.ts +++ b/packages/browser/src/Session.ts @@ -83,6 +83,18 @@ export async function silentlyAuthenticate( clientAuthn: ClientAuthentication, session: Session, ): Promise { + // KEY_CURRENT_URL is set just before a silent-authentication redirect and only cleared once the + // browser is redirected back after a *successful* attempt. If it is still present when we are + // about to start a new attempt, the previous one never completed — which happens when the OIDC + // provider no longer recognises the stored dynamic client (e.g. it dropped the registration on a + // restart) and responds with a non-redirectable error, stranding the user on an error page. + // Retrying with the same client ID would loop indefinitely, so discard the stored client + // registration and stop here; the next login will register a fresh client and recover. + if (window.localStorage.getItem(KEY_CURRENT_URL) !== null) { + window.localStorage.removeItem(KEY_CURRENT_URL); + await clientAuthn.clearClientRegistrationInfo(sessionId); + return false; + } const storedSessionInfo = await clientAuthn.validateCurrentSession(sessionId); if (storedSessionInfo !== null) { // It can be really useful to save the user's current browser location, diff --git a/packages/browser/src/sessionInfo/SessionInfoManager.ts b/packages/browser/src/sessionInfo/SessionInfoManager.ts index cb914cb90..8e535db56 100644 --- a/packages/browser/src/sessionInfo/SessionInfoManager.ts +++ b/packages/browser/src/sessionInfo/SessionInfoManager.ts @@ -151,4 +151,28 @@ export class SessionInfoManager async clear(sessionId: string): Promise { return clear(sessionId, this.storageUtility); } + + /** + * Removes only the stored dynamic client registration for a session (client ID, secret and + * related metadata), leaving the rest of the session information intact. This forces the next + * login to register a fresh client, which is used to recover when the OIDC provider no longer + * recognises the stored client (for example because it dropped the dynamic registration on a + * restart) and would otherwise reject it on every silent-authentication attempt. + * @param sessionId the session identifier + * @hidden + */ + async clearClientRegistrationInfo(sessionId: string): Promise { + await Promise.all( + [ + "clientId", + "clientSecret", + "clientType", + "clientName", + "expiresAt", + "idTokenSignedResponseAlg", + ].map((key) => + this.storageUtility.deleteForUser(sessionId, key, { secure: false }), + ), + ); + } }