From 2e65da88135ef57084a177cf744661d8ef15ff16 Mon Sep 17 00:00:00 2001 From: bdj Date: Wed, 6 May 2026 20:16:06 -0700 Subject: [PATCH] fix(oauth): only re-register on invalid_client, not all 4xx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The introspection and code-exchange retry paths were treating every 401/403 as "client credentials are bad" and re-registering. But 401/403 has many other causes (invalid_token, invalid_grant, AS hiccups, empty bodies). Each false-positive register rotates the shared client_secret and amplifies failure across users in multi-user deployments (atxp-pics, atxp-video, atxp-music — see auth dataset showing ~1.2M /register/day with oauth4webapi user-agent). Fix: introduce isInvalidClientError(response) that parses the response body and WWW-Authenticate header per RFC 6749 §5.2 and RFC 6750. Re-register only when the failure is explicitly identified as `error=invalid_client`. Other 4xx errors propagate to the caller as before. Auto-DCR on first use and self-healing on genuine credential failure are both preserved. Tests: 4 unit tests + 4 real-HTTP integration tests + 2 shared-client multi-user scenarios verifying that 50 requests with 10 false-positive 401s produce zero /register calls (pre-fix would have produced ~10). 2 new false-positive guard tests on the client-side token-exchange path. Existing "should re-register if code exchange fails" test updated to use proper invalid_client error body (matching the live auth.atxp.ai contract, verified via curl probes). --- packages/atxp-client/src/oAuth.ts | 6 +- packages/atxp-client/src/oauth.test.ts | 65 ++++- packages/atxp-common/src/index.ts | 5 +- ...source.introspectRetry.integration.test.ts | 249 ++++++++++++++++++ .../src/oAuthResource.introspectRetry.test.ts | 164 ++++++++++++ packages/atxp-common/src/oAuthResource.ts | 41 ++- 6 files changed, 517 insertions(+), 13 deletions(-) create mode 100644 packages/atxp-common/src/oAuthResource.introspectRetry.integration.test.ts create mode 100644 packages/atxp-common/src/oAuthResource.introspectRetry.test.ts diff --git a/packages/atxp-client/src/oAuth.ts b/packages/atxp-client/src/oAuth.ts index ed4a3ee1..66dda515 100644 --- a/packages/atxp-client/src/oAuth.ts +++ b/packages/atxp-client/src/oAuth.ts @@ -1,5 +1,5 @@ import * as oauth from 'oauth4webapi'; -import { OAuthResourceClient } from '@atxp/common'; +import { OAuthResourceClient, isInvalidClientError } from '@atxp/common'; import { crypto } from '@atxp/common'; import { AccessToken, ClientCredentials, FetchLike, Logger, OAuthDb, PKCEValues } from '@atxp/common'; import { ConsoleLogger } from '@atxp/common'; @@ -290,8 +290,8 @@ export class OAuthClient extends OAuthResourceClient { // Get the client credentials let credentials = await this.getClientCredentials(authorizationServer); let [response, client] = await this.makeTokenRequestAndClient(authorizationServer, credentials, codeVerifier, authResponse); - if(response.status === 403 || response.status === 401) { - this.logger.info(`Bad response status exchanging code for token: ${response.statusText}. Could be due to bad client credentials - trying to re-register`); + if (await isInvalidClientError(response)) { + this.logger.info(`Token endpoint rejected our client credentials (invalid_client) — re-registering`); credentials = await this.registerClient(authorizationServer); [response, client] = await this.makeTokenRequestAndClient(authorizationServer, credentials, codeVerifier, authResponse); } diff --git a/packages/atxp-client/src/oauth.test.ts b/packages/atxp-client/src/oauth.test.ts index 6e0ddebf..d1a33578 100644 --- a/packages/atxp-client/src/oauth.test.ts +++ b/packages/atxp-client/src/oauth.test.ts @@ -467,12 +467,12 @@ describe('oauthClient', () => { expect(registerCall).toBeDefined(); }); - it('should re-register client if code exchange fails with bad credentials', async () => { + it('should re-register client if code exchange fails with invalid_client (self-healing)', async () => { const f = fetchMock.createInstance(); mockResourceServer(f, 'https://example.com', '/mcp'); mockAuthorizationServer(f, DEFAULT_AUTHORIZATION_SERVER) - .modifyRoute(`${DEFAULT_AUTHORIZATION_SERVER}/token`, {method: 'post', response: {status: 401, body: {}}}) - .postOnce(`${DEFAULT_AUTHORIZATION_SERVER}/token`, + .modifyRoute(`${DEFAULT_AUTHORIZATION_SERVER}/token`, {method: 'post', response: {status: 401, body: {error: 'invalid_client', error_description: 'bad client creds'}}}) + .postOnce(`${DEFAULT_AUTHORIZATION_SERVER}/token`, { access_token: 'test-access-token', refresh_token: 'test-refresh-token', @@ -500,6 +500,65 @@ describe('oauthClient', () => { await client.handleCallback(authCallbackUrl); }); + it('should NOT re-register client if code exchange fails with invalid_grant (false-positive guard)', async () => { + // RFC 6749 §5.2: invalid_grant means the user's authorization code is bad. + // Re-registering the client would not help — re-issuing client_id/secret + // does not fix a bad auth code, it only rotates the shared client and + // amplifies failure across all users. + const f = fetchMock.createInstance(); + mockResourceServer(f, 'https://example.com', '/mcp'); + mockAuthorizationServer(f, DEFAULT_AUTHORIZATION_SERVER) + .modifyRoute(`${DEFAULT_AUTHORIZATION_SERVER}/token`, {method: 'post', response: {status: 400, body: {error: 'invalid_grant', error_description: 'auth code expired'}}}); + + const db = new MemoryOAuthDb(); + db.savePKCEValues('bdj', 'test-state', { + url: 'https://example.com/mcp', + codeVerifier: 'test-code-verifier', + codeChallenge: 'test-code-challenge', + resourceUrl: 'https://example.com/mcp' + }); + db.saveClientCredentials(DEFAULT_AUTHORIZATION_SERVER, { + clientId: 'good-client-id', + clientSecret: 'good-client-secret', + redirectUri: 'https://atxp.ai' + }); + + const client = oauthClient(f.fetchHandler, db); + const authCallbackUrl = `https://example.com/callback?code=test-code&state=test-state`; + await expect(client.handleCallback(authCallbackUrl)).rejects.toThrow(); + + // /register must NOT have been called — invalid_grant doesn't mean bad client creds + const registerCalls = f.callHistory.calls(`${DEFAULT_AUTHORIZATION_SERVER}/register`); + expect(registerCalls).toHaveLength(0); + }); + + it('should NOT re-register client if code exchange returns 401 with empty body (ambiguous, default safe)', async () => { + const f = fetchMock.createInstance(); + mockResourceServer(f, 'https://example.com', '/mcp'); + mockAuthorizationServer(f, DEFAULT_AUTHORIZATION_SERVER) + .modifyRoute(`${DEFAULT_AUTHORIZATION_SERVER}/token`, {method: 'post', response: {status: 401, body: {}}}); + + const db = new MemoryOAuthDb(); + db.savePKCEValues('bdj', 'test-state', { + url: 'https://example.com/mcp', + codeVerifier: 'test-code-verifier', + codeChallenge: 'test-code-challenge', + resourceUrl: 'https://example.com/mcp' + }); + db.saveClientCredentials(DEFAULT_AUTHORIZATION_SERVER, { + clientId: 'good-client-id', + clientSecret: 'good-client-secret', + redirectUri: 'https://atxp.ai' + }); + + const client = oauthClient(f.fetchHandler, db); + const authCallbackUrl = `https://example.com/callback?code=test-code&state=test-state`; + await expect(client.handleCallback(authCallbackUrl)).rejects.toThrow(); + + const registerCalls = f.callHistory.calls(`${DEFAULT_AUTHORIZATION_SERVER}/register`); + expect(registerCalls).toHaveLength(0); + }); + it('should throw if authorization server authorization endpoint returns an error', async () => { // We can't save this - the authorization URL was constructed using the client_id, so diff --git a/packages/atxp-common/src/index.ts b/packages/atxp-common/src/index.ts index d82c1314..87f65c2c 100644 --- a/packages/atxp-common/src/index.ts +++ b/packages/atxp-common/src/index.ts @@ -11,9 +11,10 @@ export { } from './memoryOAuthDb.js'; // OAuth resource client -export { +export { OAuthResourceClient, - type OAuthResourceClientConfig + isInvalidClientError, + type OAuthResourceClientConfig } from './oAuthResource.js'; // Payment error handling diff --git a/packages/atxp-common/src/oAuthResource.introspectRetry.integration.test.ts b/packages/atxp-common/src/oAuthResource.introspectRetry.integration.test.ts new file mode 100644 index 00000000..1d2e9260 --- /dev/null +++ b/packages/atxp-common/src/oAuthResource.introspectRetry.integration.test.ts @@ -0,0 +1,249 @@ +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import express from 'express'; +import type { Server } from 'http'; +import { OAuthResourceClient } from './oAuthResource.js'; +import { MemoryOAuthDb } from './memoryOAuthDb.js'; +import type { ClientCredentials, Logger } from './types.js'; + +// Real-HTTP integration test: a tiny local Express AS lets us verify end-to-end +// that introspectToken's retry logic (1) does not re-register on non-credential +// 401/403s (the dominant /register volume driver), and (2) does still self-heal +// on `invalid_client`. This is one rung up from the mocked-fetch unit tests: +// it exercises the actual oauth4webapi HTTP path. + +let server: Server; +let baseUrl: string; +let issuer: string; +let registerCalls = 0; +let introspectCalls = 0; +let introspectResponder: () => { status: number; body: unknown; contentType?: string } = () => ({ + status: 200, + body: { active: true, sub: 'user-123' }, +}); + +const seededCredentials: ClientCredentials = { + clientId: 'seeded-client-id', + clientSecret: 'seeded-client-secret', + redirectUri: 'http://localhost:3000/cb', +}; + +const silentLogger: Logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +}; + +beforeAll(async () => { + const app = express(); + app.use(express.urlencoded({ extended: true })); + app.use(express.json()); + + // Set up base URL placeholder; rewritten after listen() + app.get('/.well-known/oauth-authorization-server', (_req, res) => { + res.json({ + issuer, + authorization_endpoint: `${issuer}/authorize`, + token_endpoint: `${issuer}/token`, + introspection_endpoint: `${issuer}/introspect`, + registration_endpoint: `${issuer}/register`, + introspection_endpoint_auth_methods_supported: ['client_secret_basic'], + response_types_supported: ['code'], + }); + }); + + app.post('/introspect', (_req, res) => { + introspectCalls++; + const r = introspectResponder(); + const ct = r.contentType ?? 'application/json'; + res.status(r.status).type(ct); + if (ct === 'application/json') { + res.send(JSON.stringify(r.body)); + } else { + res.send(String(r.body)); + } + }); + + app.post('/register', (_req, res) => { + registerCalls++; + res.status(201).json({ + client_id: `rotated-client-${registerCalls}`, + client_secret: `rotated-secret-${registerCalls}`, + client_secret_expires_at: 0, + redirect_uris: [seededCredentials.redirectUri], + }); + }); + + await new Promise((resolve) => { + server = app.listen(0, () => resolve()); + }); + const port = (server.address() as { port: number }).port; + baseUrl = `http://127.0.0.1:${port}`; + issuer = baseUrl; +}); + +afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); +}); + +beforeEach(() => { + registerCalls = 0; + introspectCalls = 0; +}); + +async function makeClient(): Promise { + const db = new MemoryOAuthDb({ logger: silentLogger }); + await db.saveClientCredentials(issuer, seededCredentials); + return new OAuthResourceClient({ + db, + logger: silentLogger, + allowInsecureRequests: true, + }); +} + +describe('introspectToken — real-HTTP retry behavior', () => { + it('does NOT call /register when AS returns 401 invalid_token (false-positive)', async () => { + introspectResponder = () => ({ + status: 401, + body: { error: 'invalid_token', error_description: 'token revoked' }, + }); + const client = await makeClient(); + + await expect(client.introspectToken(issuer, 'bad-user-token')).rejects.toThrow(); + + expect(registerCalls).toBe(0); + }); + + it('does NOT call /register when AS returns 401 with empty body', async () => { + introspectResponder = () => ({ status: 401, body: '', contentType: 'text/plain' }); + const client = await makeClient(); + + await expect(client.introspectToken(issuer, 'whatever')).rejects.toThrow(); + + expect(registerCalls).toBe(0); + }); + + it('DOES call /register exactly once when AS returns 401 invalid_client, then succeeds on retry', async () => { + introspectResponder = () => + introspectCalls === 1 + ? { status: 401, body: { error: 'invalid_client' } } + : { status: 200, body: { active: true, sub: 'user-456' } }; + + const client = await makeClient(); + const result = await client.introspectToken(issuer, 'good-user-token'); + + expect(registerCalls).toBe(1); + expect(result.active).toBe(true); + }); + + it('does NOT call /register on the happy path (200 active=true)', async () => { + introspectResponder = () => ({ + status: 200, + body: { active: true, sub: 'user-789' }, + }); + const client = await makeClient(); + + const result = await client.introspectToken(issuer, 'good-user-token'); + + expect(registerCalls).toBe(0); + expect(result.active).toBe(true); + }); +}); + +describe('introspectToken — shared-client (atxp-pics-style multi-user) behavior', () => { + // Mirrors the shape of atxp-pics/src/lib/mcp.ts: one shared OAuthDb (the + // singleton), many OAuthResourceClient instances (one per user request). + // Verifies that after the bug fix, sustained per-request usage does NOT + // produce sustained /register calls — one call at cold start per issuer, + // then zero on every subsequent request. + + it('cold start: many concurrent users with empty shared db produce at most one register burst, then zero', async () => { + introspectResponder = () => ({ + status: 200, + body: { active: true, sub: 'shared-test-user' }, + }); + // Empty shared db — first wave of users will trigger DCR + const sharedDb = new MemoryOAuthDb({ logger: silentLogger }); + + // 10 concurrent first-time users, each in their own OAuthResourceClient + // (atxp-pics creates a fresh atxpFetcher → OAuthClient per request) + const firstWave = await Promise.all( + Array.from({ length: 10 }, async () => { + const client = new OAuthResourceClient({ + db: sharedDb, + logger: silentLogger, + allowInsecureRequests: true, + }); + return client.introspectToken(issuer, 'token-' + Math.random()); + }), + ); + + expect(firstWave.every((r) => r.active === true)).toBe(true); + + // Cold-start race: per-instance registrationLocks means up to N concurrent + // registers can fire before the shared db gets populated. We document the + // current upper bound (it should be small and bounded, not unbounded). + const coldStartRegisters = registerCalls; + expect(coldStartRegisters).toBeGreaterThan(0); + expect(coldStartRegisters).toBeLessThanOrEqual(10); + + // After the cold-start storm, the shared db is populated. + // Now: 50 more requests over time. None should trigger /register because + // the happy path returns 200, and shared db has cached credentials. + registerCalls = 0; + const steadyState = await Promise.all( + Array.from({ length: 50 }, async () => { + const client = new OAuthResourceClient({ + db: sharedDb, + logger: silentLogger, + allowInsecureRequests: true, + }); + return client.introspectToken(issuer, 'token-' + Math.random()); + }), + ); + expect(steadyState.every((r) => r.active === true)).toBe(true); + + // This is the key assertion: in the happy case, sustained traffic produces + // ZERO additional /register calls — because the AS returns 200 and we + // never enter the false-positive retry path. + expect(registerCalls).toBe(0); + }); + + it('steady-state with intermittent 401 invalid_token (false-positive scenario): ZERO registers', async () => { + // This is the load pattern that was producing 1.2M /register/day in prod: + // many users, occasional introspect 401 because the user token is bad, + // and the SDK was wrongly treating each one as bad client creds. + let i = 0; + introspectResponder = () => { + i++; + // Every 5th request returns 401 invalid_token (a bad user token) + return i % 5 === 0 + ? { status: 401, body: { error: 'invalid_token' } } + : { status: 200, body: { active: true, sub: 'u' + i } }; + }; + + const sharedDb = new MemoryOAuthDb({ logger: silentLogger }); + await sharedDb.saveClientCredentials(issuer, seededCredentials); + + const results = await Promise.allSettled( + Array.from({ length: 50 }, async () => { + const client = new OAuthResourceClient({ + db: sharedDb, + logger: silentLogger, + allowInsecureRequests: true, + }); + return client.introspectToken(issuer, 'token-' + Math.random()); + }), + ); + + const fulfilled = results.filter((r) => r.status === 'fulfilled').length; + const rejected = results.filter((r) => r.status === 'rejected').length; + expect(fulfilled).toBe(40); + expect(rejected).toBe(10); + + // CRITICAL: zero /register calls despite 10 401s. + // Pre-fix: this would have been ~10 /register calls per 50 introspects. + // At 1.2M/day in prod, that ratio matches the observed volume. + expect(registerCalls).toBe(0); + }); +}); diff --git a/packages/atxp-common/src/oAuthResource.introspectRetry.test.ts b/packages/atxp-common/src/oAuthResource.introspectRetry.test.ts new file mode 100644 index 00000000..572991da --- /dev/null +++ b/packages/atxp-common/src/oAuthResource.introspectRetry.test.ts @@ -0,0 +1,164 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { OAuthResourceClient } from './oAuthResource.js'; +import { MemoryOAuthDb } from './memoryOAuthDb.js'; +import type { ClientCredentials, Logger } from './types.js'; + +// These tests verify that introspectToken's retry-on-401/403 path only +// re-registers the client when the failure is *actually* a client-credential +// problem (`error: invalid_client`). Other 401/403 reasons (invalid_token, +// invalid_grant, network blip, AS hiccup) must NOT trigger DCR — re-registering +// rotates the shared client_secret and amplifies the failure across users. + +const ISSUER = 'https://auth.example.com'; + +const mockAuthServerMetadata = { + issuer: ISSUER, + authorization_endpoint: `${ISSUER}/authorize`, + token_endpoint: `${ISSUER}/token`, + introspection_endpoint: `${ISSUER}/introspect`, + registration_endpoint: `${ISSUER}/register`, + // oauth4webapi's introspectionRequest checks this + introspection_endpoint_auth_methods_supported: ['client_secret_basic'], +}; + +const seededCredentials: ClientCredentials = { + clientId: 'seeded-client-id', + clientSecret: 'seeded-client-secret', + redirectUri: 'http://localhost:3000/cb', +}; + +const silentLogger: Logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +}; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +async function makeClient(fetchImpl: typeof fetch): Promise<{ + client: OAuthResourceClient; + db: MemoryOAuthDb; + registerSpy: ReturnType; +}> { + const db = new MemoryOAuthDb({ logger: silentLogger }); + await db.saveClientCredentials(ISSUER, seededCredentials); + const client = new OAuthResourceClient({ + db, + sideChannelFetch: fetchImpl, + logger: silentLogger, + allowInsecureRequests: true, + }); + // Stub registerClient so we can assert call counts without making it call out + const registerSpy = vi + .spyOn(client as any, 'registerClient') + .mockImplementation(async () => { + const rotated: ClientCredentials = { + clientId: 'rotated-client-id', + clientSecret: 'rotated-client-secret', + redirectUri: 'http://localhost:3000/cb', + }; + await db.saveClientCredentials(ISSUER, rotated); + return rotated; + }); + return { client, db, registerSpy }; +} + +describe('introspectToken — re-register only on invalid_client', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('does NOT re-register when introspect returns 401 with error=invalid_token (the false-positive case)', async () => { + const fetchMock = vi.fn(async (input: RequestInfo | URL): Promise => { + const url = input.toString(); + if (url.includes('/.well-known/oauth-authorization-server') || url === ISSUER || url === ISSUER + '/') { + return jsonResponse(mockAuthServerMetadata); + } + if (url.includes('/introspect')) { + // The token is bad, not the client creds. AS returns 401 with invalid_token. + return jsonResponse({ error: 'invalid_token', error_description: 'token revoked' }, 401); + } + throw new Error(`Unexpected fetch: ${url}`); + }); + + const { client, registerSpy } = await makeClient(fetchMock as unknown as typeof fetch); + + await expect( + client.introspectToken(ISSUER, 'bad-user-token'), + ).rejects.toThrow(); + + expect(registerSpy).not.toHaveBeenCalled(); + }); + + it('does NOT re-register when introspect returns 401 with empty body (ambiguous, default safe)', async () => { + const fetchMock = vi.fn(async (input: RequestInfo | URL): Promise => { + const url = input.toString(); + if (url.includes('/.well-known/oauth-authorization-server') || url === ISSUER || url === ISSUER + '/') { + return jsonResponse(mockAuthServerMetadata); + } + if (url.includes('/introspect')) { + return new Response('', { status: 401 }); + } + throw new Error(`Unexpected fetch: ${url}`); + }); + + const { client, registerSpy } = await makeClient(fetchMock as unknown as typeof fetch); + + await expect( + client.introspectToken(ISSUER, 'some-token'), + ).rejects.toThrow(); + + expect(registerSpy).not.toHaveBeenCalled(); + }); + + it('DOES re-register when introspect returns 401 with error=invalid_client (self-healing path)', async () => { + let introspectCalls = 0; + const fetchMock = vi.fn(async (input: RequestInfo | URL): Promise => { + const url = input.toString(); + if (url.includes('/.well-known/oauth-authorization-server') || url === ISSUER || url === ISSUER + '/') { + return jsonResponse(mockAuthServerMetadata); + } + if (url.includes('/introspect')) { + introspectCalls++; + if (introspectCalls === 1) { + return jsonResponse({ error: 'invalid_client', error_description: 'bad client creds' }, 401); + } + return jsonResponse({ active: true, sub: 'user-123' }); + } + throw new Error(`Unexpected fetch: ${url}`); + }); + + const { client, registerSpy } = await makeClient(fetchMock as unknown as typeof fetch); + + const result = await client.introspectToken(ISSUER, 'good-user-token'); + + expect(registerSpy).toHaveBeenCalledTimes(1); + expect(result.active).toBe(true); + }); + + it('does NOT re-register on the happy path (200 active=true)', async () => { + const fetchMock = vi.fn(async (input: RequestInfo | URL): Promise => { + const url = input.toString(); + if (url.includes('/.well-known/oauth-authorization-server') || url === ISSUER || url === ISSUER + '/') { + return jsonResponse(mockAuthServerMetadata); + } + if (url.includes('/introspect')) { + return jsonResponse({ active: true, sub: 'user-123' }); + } + throw new Error(`Unexpected fetch: ${url}`); + }); + + const { client, registerSpy } = await makeClient(fetchMock as unknown as typeof fetch); + + const result = await client.introspectToken(ISSUER, 'good-user-token'); + + expect(registerSpy).not.toHaveBeenCalled(); + expect(result.active).toBe(true); + }); +}); diff --git a/packages/atxp-common/src/oAuthResource.ts b/packages/atxp-common/src/oAuthResource.ts index fa4f2813..f214296a 100644 --- a/packages/atxp-common/src/oAuthResource.ts +++ b/packages/atxp-common/src/oAuthResource.ts @@ -3,6 +3,28 @@ import * as oauth from 'oauth4webapi'; import { ClientCredentials, FetchLike, OAuthResourceDb, OAuthDb, TokenData, Logger } from './types.js'; import { ConsoleLogger } from './logger.js'; +/** + * Returns true only when a 401/403 response explicitly identifies the failure + * as `error=invalid_client` (per RFC 6749 §5.2 / RFC 6750), via either the + * JSON body or the WWW-Authenticate header. Other 401/403 reasons + * (`invalid_token`, `invalid_grant`, AS hiccups, empty bodies) return false: + * re-registering the client wouldn't fix them and rotating the shared + * client_secret amplifies failure across users in multi-user deployments. + */ +export async function isInvalidClientError(response: Response): Promise { + if (response.status !== 401 && response.status !== 403) return false; + const wwwAuth = response.headers.get('www-authenticate'); + if (wwwAuth && /error\s*=\s*"?invalid_client"?/i.test(wwwAuth)) return true; + try { + const text = await response.clone().text(); + if (!text) return false; + const body = JSON.parse(text) as { error?: unknown }; + return body?.error === 'invalid_client'; + } catch { + return false; + } +} + export interface OAuthResourceClientConfig { db: OAuthDb; callbackUrl?: string; @@ -82,6 +104,15 @@ export class OAuthResourceClient { return res === url ? null : res; } + /** + * Returns true only when the response is a 401/403 whose body or + * WWW-Authenticate header explicitly identifies the failure as + * `error=invalid_client` (RFC 6749 §5.2 / RFC 6750). Other 401/403 reasons + * (`invalid_token`, `invalid_grant`, transient network/AS hiccups, empty + * bodies) MUST NOT trigger DCR — re-registering rotates the shared + * client_secret and amplifies failure across users. + */ + introspectToken = async (authorizationServerUrl: string, token: string, additionalParameters?: Record): Promise => { // Don't use getAuthorizationServer here, because we're not using the resource server url const authorizationServer = await this.authorizationServerFromUrl(new URL(authorizationServerUrl)); @@ -110,8 +141,8 @@ export class OAuthResourceClient { } ); - if(introspectionResponse.status === 403 || introspectionResponse.status === 401) { - this.logger.info(`Bad response status doing token introspection: ${introspectionResponse.statusText}. Could be due to bad client credentials - trying to re-register`); + if (await isInvalidClientError(introspectionResponse)) { + this.logger.info(`Token introspection rejected our client credentials (invalid_client) — re-registering`); clientCredentials = await this.registerClient(authorizationServer); client = { client_id: clientCredentials.clientId, @@ -123,9 +154,9 @@ export class OAuthResourceClient { client, clientAuth, token, - { - additionalParameters, - [oauth.customFetch]: this.sideChannelFetch, + { + additionalParameters, + [oauth.customFetch]: this.sideChannelFetch, [oauth.allowInsecureRequests]: this.allowInsecureRequests } );