diff --git a/clients/web/src/test/core/auth/providers.test.ts b/clients/web/src/test/core/auth/providers.test.ts index 3998f118..cef8985a 100644 --- a/clients/web/src/test/core/auth/providers.test.ts +++ b/clients/web/src/test/core/auth/providers.test.ts @@ -637,43 +637,8 @@ describe("OAuthNavigation", () => { const ISSUER = "https://as.example.com"; const METADATA_URL = "https://app.example.com/client-metadata.json"; - it("keeps cimd when the client_id is the configured metadata document URL", async () => { - const storage = makeStorage(); - const provider = makeProvider(storage, vi.fn(), { - clientMetadataUrl: METADATA_URL, - }); - - await provider.saveClientInformation( - { client_id: METADATA_URL }, - { issuer: ISSUER }, - ); - - expect(storage.saveClientInformation).toHaveBeenCalledWith( - SERVER, - { client_id: METADATA_URL }, - { registrationKind: "cimd", issuer: ISSUER }, - ); - }); - - it("still records dcr for a server-minted client_id while CIMD is configured", async () => { - const storage = makeStorage(); - const provider = makeProvider(storage, vi.fn(), { - clientMetadataUrl: METADATA_URL, - }); - - await provider.saveClientInformation( - { client_id: "dcr-minted-id" }, - { issuer: ISSUER }, - ); - - expect(storage.saveClientInformation).toHaveBeenCalledWith( - SERVER, - { client_id: "dcr-minted-id" }, - { registrationKind: "dcr", issuer: ISSUER }, - ); - }); - - it("preserves the stored kind when the stored client_id matches", async () => { + /** Storage already holding the CIMD pre-registration for METADATA_URL. */ + function makeCimdStorage(): OAuthStorage { const storage = makeStorage(); vi.mocked(storage.getClientInformation).mockImplementation( async (_url: string, preregistered?: boolean) => @@ -682,16 +647,22 @@ describe("OAuthNavigation", () => { vi.mocked(storage.getClientRegistrationKind).mockResolvedValue( "cimd", ); - // No `clientMetadataUrl` on the provider — the provenance comes from - // storage alone, so a config cleared since the registration was made - // does not silently demote it. - const provider = makeProvider(storage); + return storage; + } + + it("keeps cimd when CIMD is configured and the stored registration matches", async () => { + const storage = makeCimdStorage(); + const provider = makeProvider(storage, vi.fn(), { + clientMetadataUrl: METADATA_URL, + }); await provider.saveClientInformation( { client_id: METADATA_URL }, { issuer: ISSUER }, ); + // Reads the dynamic slot for this issuer, which falls back to the + // unkeyed slot the pre-registration wrote. expect(storage.getClientInformation).toHaveBeenCalledWith( SERVER, false, @@ -704,52 +675,108 @@ describe("OAuthNavigation", () => { ); }); - it("does not leak a stored cimd kind onto a different client_id", async () => { + it("records dcr for a server-minted client_id while CIMD is configured", async () => { + const storage = makeCimdStorage(); + const provider = makeProvider(storage, vi.fn(), { + clientMetadataUrl: METADATA_URL, + }); + + await provider.saveClientInformation( + { client_id: "dcr-minted-id" }, + { issuer: ISSUER }, + ); + + // The id is not the metadata URL, so nothing is read and nothing is + // carried forward. + expect(storage.getClientInformation).not.toHaveBeenCalled(); + expect(storage.getClientRegistrationKind).not.toHaveBeenCalled(); + expect(storage.saveClientInformation).toHaveBeenCalledWith( + SERVER, + { client_id: "dcr-minted-id" }, + { registrationKind: "dcr", issuer: ISSUER }, + ); + }); + + it("records dcr when CIMD is not configured, even if storage says cimd", async () => { + const storage = makeCimdStorage(); + const provider = makeProvider(storage); + + await provider.saveClientInformation( + { client_id: METADATA_URL }, + { issuer: ISSUER }, + ); + + expect(storage.saveClientInformation).toHaveBeenCalledWith( + SERVER, + { client_id: METADATA_URL }, + { registrationKind: "dcr", issuer: ISSUER }, + ); + }); + + it("records dcr when the metadata URL differs from the configured one", async () => { + const storage = makeCimdStorage(); + const provider = makeProvider(storage, vi.fn(), { + clientMetadataUrl: "https://other.example.com/client-metadata.json", + }); + + await provider.saveClientInformation( + { client_id: METADATA_URL }, + { issuer: ISSUER }, + ); + + expect(storage.saveClientInformation).toHaveBeenCalledWith( + SERVER, + { client_id: METADATA_URL }, + { registrationKind: "dcr", issuer: ISSUER }, + ); + }); + + it("records dcr when no CIMD registration was ever stored", async () => { + // The AS returns the configured metadata URL from a real registration + // (RFC 7591 §3.2 leaves the id opaque). With nothing recorded as CIMD + // under that id, the save is still DCR. + const storage = makeStorage(); + const provider = makeProvider(storage, vi.fn(), { + clientMetadataUrl: METADATA_URL, + }); + + await provider.saveClientInformation( + { client_id: METADATA_URL }, + { issuer: ISSUER }, + ); + + expect(storage.saveClientInformation).toHaveBeenCalledWith( + SERVER, + { client_id: METADATA_URL }, + { registrationKind: "dcr", issuer: ISSUER }, + ); + }); + + it("records dcr when the stored kind under that id is not cimd", async () => { const storage = makeStorage(); vi.mocked(storage.getClientInformation).mockImplementation( async (_url: string, preregistered?: boolean) => preregistered ? undefined : { client_id: METADATA_URL }, ); - vi.mocked(storage.getClientRegistrationKind).mockResolvedValue( - "cimd", - ); - const provider = makeProvider(storage); + vi.mocked(storage.getClientRegistrationKind).mockResolvedValue("dcr"); + const provider = makeProvider(storage, vi.fn(), { + clientMetadataUrl: METADATA_URL, + }); await provider.saveClientInformation( - { client_id: "freshly-registered" }, - { issuer: ISSUER }, - ); - - expect(storage.getClientRegistrationKind).not.toHaveBeenCalled(); - expect(storage.saveClientInformation).toHaveBeenCalledWith( - SERVER, - { client_id: "freshly-registered" }, - { registrationKind: "dcr", issuer: ISSUER }, - ); - }); - - it("falls back to dcr when storage has a matching id but no recorded kind", async () => { - const storage = makeStorage(); - vi.mocked(storage.getClientInformation).mockImplementation( - async (_url: string, preregistered?: boolean) => - preregistered ? undefined : { client_id: "legacy-id" }, - ); - const provider = makeProvider(storage); - - await provider.saveClientInformation( - { client_id: "legacy-id" }, + { client_id: METADATA_URL }, { issuer: ISSUER }, ); expect(storage.saveClientInformation).toHaveBeenCalledWith( SERVER, - { client_id: "legacy-id" }, + { client_id: METADATA_URL }, { registrationKind: "dcr", issuer: ISSUER }, ); }); it("an explicit registrationKind wins and consults no storage reads", async () => { - const storage = makeStorage(); + const storage = makeCimdStorage(); const provider = makeProvider(storage, vi.fn(), { clientMetadataUrl: METADATA_URL, }); diff --git a/core/auth/providers.ts b/core/auth/providers.ts index 5469e96b..ed7b3556 100644 --- a/core/auth/providers.ts +++ b/core/auth/providers.ts @@ -315,18 +315,29 @@ export class BaseOAuthClientProvider implements OAuthClientProvider { /** * Resolve the registration kind for a save that carries no explicit one — that - * is, one the SDK made. Three SDK call sites reach here: back-stamping an - * existing registration with its `issuer`, the SDK's own CIMD write (whose - * `client_id` *is* the metadata-document URL), and a real dynamic - * registration. Only the last is `"dcr"`, so defaulting every unstamped save - * to it relabels a CIMD registration `Dynamic (DCR)` in Connection Info the - * moment the SDK binds it to an issuer — reported as #2242, where no - * `POST /register` was ever made. + * is, one the SDK made. SDK v2's `saveClientInformation` contract passes only + * `{ issuer }`, so the mechanism cannot be handed to us; treating every such + * save as DCR is what relabeled a CIMD registration `Dynamic (DCR)` in + * Connection Info the moment the SDK bound it to an issuer (#2242). * - * `client_id` is what tells the cases apart, so match on it rather than on the - * stored kind alone: a DCR `client_id` is minted by the authorization server, - * so a later DCR registration for the same server cannot inherit the earlier - * CIMD provenance. + * The claim is deliberately narrow — three conditions must all hold, and the + * decisive one is a registration *we ourselves recorded* as CIMD, not an + * inference about what the authorization server returned: + * + * 1. CIMD is configured for this connection right now, and + * 2. the incoming `client_id` is exactly that metadata-document URL, and + * 3. the registration already stored under that same `client_id` is recorded + * as `cimd` — written by `ensureCimdClientRegistration`, which reaches that + * line only after confirming the AS advertises + * `client_id_metadata_document_supported`. + * + * RFC 7591 §3.2 makes a dynamically issued `client_id` opaque, so a client may + * not assume its format — which is why (2) is not load-bearing on its own. For + * a `registerClient` result to be mislabeled here, the AS would have to mint an + * identifier byte-identical to the HTTPS URL we configured *and* we would have + * to already hold a CIMD registration recorded under it. Anything else — a + * fresh DCR, a different id, CIMD switched off, no prior CIMD registration — + * falls through to `"dcr"`. */ private async resolveSdkRegistrationKind( clientInformation: OAuthClientInformation, @@ -334,23 +345,19 @@ export class BaseOAuthClientProvider implements OAuthClientProvider { ): Promise { const clientMetadataUrl = this.clientMetadataUrl?.trim(); if ( - clientMetadataUrl && - clientInformation.client_id === clientMetadataUrl + !clientMetadataUrl || + clientInformation.client_id !== clientMetadataUrl ) { - return "cimd"; + return "dcr"; } - // Falls back to the unkeyed slot our own pre-registration wrote, since the - // issuer slot does not exist yet on the save that creates it. Reading it - // covers a CIMD registration whose `clientMetadataUrl` config has since - // been cleared, so the provenance is not silently demoted. + // Reads through to the unkeyed slot our own pre-registration wrote, since + // the issuer slot does not exist yet on the save that creates it. const stored = await this.storage.getClientInformation( this.serverUrl, false, issuer, ); - if (!stored || stored.client_id !== clientInformation.client_id) { - return "dcr"; - } + if (stored?.client_id !== clientMetadataUrl) return "dcr"; const storedKind = await this.storage.getClientRegistrationKind( this.serverUrl, issuer,