Skip to content

Commit ed0212f

Browse files
committed
fix(client): preserve AS-bound client during code exchange
1 parent e0a8b3f commit ed0212f

2 files changed

Lines changed: 103 additions & 5 deletions

File tree

packages/client/src/client/auth.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,9 @@ async function authInternal(
770770
const staleClientInformation = await Promise.resolve(provider.clientInformation());
771771
// CIMD (URL-based) client IDs are portable across authorization servers
772772
// (SEP-991/SEP-2352) — no client invalidation or re-registration is needed.
773-
if (staleClientInformation && !isHttpsUrl(staleClientInformation.client_id)) {
773+
// During code exchange, keep the client registered by the redirect flow
774+
// that produced this authorization code.
775+
if (staleClientInformation && !isHttpsUrl(staleClientInformation.client_id) && authorizationCode === undefined) {
774776
await provider.invalidateCredentials?.('client');
775777
}
776778
}
@@ -948,12 +950,16 @@ export function isHttpsUrl(value?: string): boolean {
948950
/**
949951
* SEP-2352: Normalizes an authorization server identity (issuer identifier or
950952
* authorization server URL) for comparison, so that textual variations of the
951-
* same URL (e.g. a missing trailing slash on an origin-only issuer) do not
953+
* same URL (e.g. a missing trailing slash on an issuer URL) do not
952954
* register as an authorization server change.
953955
*/
954956
function normalizeAuthorizationServerIdentity(value: string): string {
955957
try {
956-
return new URL(value).href;
958+
const url = new URL(value);
959+
if (url.pathname !== '/') {
960+
url.pathname = url.pathname.replace(/\/+$/, '') || '/';
961+
}
962+
return url.href;
957963
} catch {
958964
return value;
959965
}

packages/client/test/client/auth.test.ts

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4192,6 +4192,7 @@ describe('SEP-2352: authorization server binding', () => {
41924192
provider: OAuthClientProvider;
41934193
invalidateCredentials: Mock;
41944194
saveClientInformation: Mock;
4195+
saveTokens: Mock;
41954196
redirectToAuthorization: Mock;
41964197
} {
41974198
let clientInformation: { client_id: string; client_secret?: string } | undefined = initialClientInformation;
@@ -4204,6 +4205,7 @@ describe('SEP-2352: authorization server binding', () => {
42044205
const saveClientInformation = vi.fn(async (info: { client_id: string; client_secret?: string }) => {
42054206
clientInformation = info;
42064207
});
4208+
const saveTokens = vi.fn();
42074209
const redirectToAuthorization = vi.fn();
42084210

42094211
const provider: OAuthClientProvider = {
@@ -4219,21 +4221,22 @@ describe('SEP-2352: authorization server binding', () => {
42194221
clientInformation: vi.fn(async () => clientInformation),
42204222
saveClientInformation,
42214223
tokens: vi.fn().mockResolvedValue(undefined),
4222-
saveTokens: vi.fn(),
4224+
saveTokens,
42234225
redirectToAuthorization,
42244226
saveCodeVerifier: vi.fn(),
42254227
codeVerifier: vi.fn().mockResolvedValue('test_verifier'),
42264228
authorizationServerUrl: vi.fn().mockResolvedValue(oldAuthServerUrl),
42274229
invalidateCredentials
42284230
};
42294231

4230-
return { provider, invalidateCredentials, saveClientInformation, redirectToAuthorization };
4232+
return { provider, invalidateCredentials, saveClientInformation, saveTokens, redirectToAuthorization };
42314233
}
42324234

42334235
function mockDiscoveryAndRegistration(options: {
42344236
resourceMetadata: { resource: string; authorization_servers: string[] };
42354237
authMetadata: { issuer: string };
42364238
registeredClient?: { client_id: string; client_secret?: string };
4239+
tokens?: OAuthTokens;
42374240
}): void {
42384241
mockFetch.mockImplementation((url, init) => {
42394242
const urlString = url.toString();
@@ -4268,6 +4271,17 @@ describe('SEP-2352: authorization server binding', () => {
42684271
});
42694272
}
42704273

4274+
if (urlString.includes('/token') && init?.method === 'POST') {
4275+
if (!options.tokens) {
4276+
return Promise.reject(new Error(`Unexpected token request: ${urlString}`));
4277+
}
4278+
return Promise.resolve({
4279+
ok: true,
4280+
status: 200,
4281+
json: async () => options.tokens
4282+
});
4283+
}
4284+
42714285
return Promise.reject(new Error(`Unexpected fetch: ${urlString}`));
42724286
});
42734287
}
@@ -4309,6 +4323,46 @@ describe('SEP-2352: authorization server binding', () => {
43094323
expect(redirectUrl.searchParams.get('client_id')).toBe('new-client-id');
43104324
});
43114325

4326+
it('keeps the re-registered client while exchanging the authorization code after an AS migration', async () => {
4327+
const { provider, invalidateCredentials, saveClientInformation, saveTokens, redirectToAuthorization } = createBoundProvider({
4328+
client_id: 'old-client-id',
4329+
client_secret: 'old-client-secret'
4330+
});
4331+
4332+
mockDiscoveryAndRegistration({
4333+
resourceMetadata: newResourceMetadata,
4334+
authMetadata: newAuthMetadata,
4335+
registeredClient: { client_id: 'new-client-id', client_secret: 'new-client-secret' },
4336+
tokens: { access_token: 'new-access-token', token_type: 'Bearer' }
4337+
});
4338+
4339+
const redirectResult = await auth(provider, { serverUrl: 'https://resource.example.com' });
4340+
4341+
expect(redirectResult).toBe('REDIRECT');
4342+
expect(invalidateCredentials).toHaveBeenCalledWith('client');
4343+
expect(saveClientInformation).toHaveBeenCalledWith(expect.objectContaining({ client_id: 'new-client-id' }));
4344+
4345+
invalidateCredentials.mockClear();
4346+
mockFetch.mockClear();
4347+
4348+
const exchangeResult = await auth(provider, {
4349+
serverUrl: 'https://resource.example.com',
4350+
authorizationCode: 'returned-code'
4351+
});
4352+
4353+
expect(exchangeResult).toBe('AUTHORIZED');
4354+
expect(invalidateCredentials).toHaveBeenCalledWith('tokens');
4355+
expect(invalidateCredentials).not.toHaveBeenCalledWith('client');
4356+
expect(saveTokens).toHaveBeenCalledWith({ access_token: 'new-access-token', token_type: 'Bearer' });
4357+
expect(redirectToAuthorization).toHaveBeenCalledTimes(1);
4358+
4359+
const registrationCalls = mockFetch.mock.calls.filter(call => call[0].toString().includes('/register'));
4360+
expect(registrationCalls).toHaveLength(0);
4361+
const tokenCalls = mockFetch.mock.calls.filter(call => call[0].toString().includes('/token'));
4362+
expect(tokenCalls).toHaveLength(1);
4363+
expect(tokenCalls[0]![0].toString()).toBe('https://new-auth.example.com/token');
4364+
});
4365+
43124366
it('refreshes cached discovery from an explicit resource metadata challenge before comparing authorization servers', async () => {
43134367
const { provider, invalidateCredentials, saveClientInformation, redirectToAuthorization } = createBoundProvider({
43144368
client_id: 'old-client-id',
@@ -4778,6 +4832,44 @@ describe('SEP-2352: authorization server binding', () => {
47784832
expect(redirectUrl.searchParams.get('client_id')).toBe('old-client-id');
47794833
});
47804834

4835+
it('does not invalidate credentials when path-bearing authorization server identities only differ by a trailing slash', async () => {
4836+
const tenantAuthServerUrl = 'https://auth.example.com/tenant1';
4837+
const tenantAuthMetadata = {
4838+
issuer: tenantAuthServerUrl,
4839+
authorization_endpoint: `${tenantAuthServerUrl}/authorize`,
4840+
token_endpoint: `${tenantAuthServerUrl}/token`,
4841+
registration_endpoint: `${tenantAuthServerUrl}/register`,
4842+
response_types_supported: ['code'],
4843+
code_challenge_methods_supported: ['S256']
4844+
};
4845+
const { provider, invalidateCredentials, redirectToAuthorization } = createBoundProvider({
4846+
client_id: 'tenant-client-id',
4847+
client_secret: 'tenant-client-secret'
4848+
});
4849+
4850+
provider.authorizationServerUrl = vi.fn().mockResolvedValue(`${tenantAuthServerUrl}/`);
4851+
4852+
mockDiscoveryAndRegistration({
4853+
resourceMetadata: {
4854+
resource: 'https://resource.example.com',
4855+
authorization_servers: [tenantAuthServerUrl]
4856+
},
4857+
authMetadata: tenantAuthMetadata
4858+
});
4859+
4860+
const result = await auth(provider, { serverUrl: 'https://resource.example.com' });
4861+
4862+
expect(result).toBe('REDIRECT');
4863+
expect(invalidateCredentials).not.toHaveBeenCalled();
4864+
4865+
const registrationCalls = mockFetch.mock.calls.filter(call => call[0].toString().includes('/register'));
4866+
expect(registrationCalls).toHaveLength(0);
4867+
4868+
const redirectUrl: URL = redirectToAuthorization.mock.calls[0]![0];
4869+
expect(redirectUrl.toString()).toContain(`${tenantAuthServerUrl}/authorize`);
4870+
expect(redirectUrl.searchParams.get('client_id')).toBe('tenant-client-id');
4871+
});
4872+
47814873
it('does not invalidate credentials when the authorization server is unchanged', async () => {
47824874
const { provider, invalidateCredentials, redirectToAuthorization } = createBoundProvider({
47834875
client_id: 'old-client-id',

0 commit comments

Comments
 (0)