diff --git a/src/server/auth/clients.ts b/src/server/auth/clients.ts index 30e4085d..1b61a4de 100644 --- a/src/server/auth/clients.ts +++ b/src/server/auth/clients.ts @@ -12,7 +12,7 @@ export interface OAuthRegisteredClientsStore { /** * Registers a new client with the server. The client ID and secret will be automatically generated by the library. A modified version of the client information can be returned to reflect specific values enforced by the server. * - * NOTE: Implementations must ensure that client secrets, if present, are expired in accordance with the `client_secret_expires_at` field. + * NOTE: Implementations should NOT delete expired client secrets in-place. Auth middleware provided by this library will automatically check the `client_secret_expires_at` field and reject requests with expired secrets. Any custom logic for authenticating clients should check the `client_secret_expires_at` field as well. * * If unimplemented, dynamic client registration is unsupported. */ diff --git a/src/server/auth/middleware/clientAuth.test.ts b/src/server/auth/middleware/clientAuth.test.ts index 4a71c596..0cfe2247 100644 --- a/src/server/auth/middleware/clientAuth.test.ts +++ b/src/server/auth/middleware/clientAuth.test.ts @@ -15,11 +15,18 @@ describe('clientAuth middleware', () => { redirect_uris: ['https://example.com/callback'] }; } else if (clientId === 'expired-client') { - // Client with expired secret + // Client with no secret return { client_id: 'expired-client', redirect_uris: ['https://example.com/callback'] - // No client_secret due to expiration + }; + } else if (clientId === 'client-with-expired-secret') { + // Client with an expired secret + return { + client_id: 'client-with-expired-secret', + client_secret: 'expired-secret', + client_secret_expires_at: Math.floor(Date.now() / 1000) - 3600, // Expired 1 hour ago + redirect_uris: ['https://example.com/callback'] }; } return undefined; @@ -101,9 +108,22 @@ describe('clientAuth middleware', () => { client_id: 'expired-client' }); - // Since the expired client has no secret, this should pass without providing one + // Since the client has no secret, this should pass without providing one expect(response.status).toBe(200); }); + + it('rejects request when client secret has expired', async () => { + const response = await supertest(app) + .post('/protected') + .send({ + client_id: 'client-with-expired-secret', + client_secret: 'expired-secret' + }); + + expect(response.status).toBe(400); + expect(response.body.error).toBe('invalid_client'); + expect(response.body.error_description).toBe('Client secret has expired'); + }); it('handles malformed request body', async () => { const response = await supertest(app) diff --git a/src/server/auth/middleware/clientAuth.ts b/src/server/auth/middleware/clientAuth.ts index 78aa595b..76049c11 100644 --- a/src/server/auth/middleware/clientAuth.ts +++ b/src/server/auth/middleware/clientAuth.ts @@ -39,8 +39,22 @@ export function authenticateClient({ clientsStore }: ClientAuthenticationMiddlew throw new InvalidClientError("Invalid client_id"); } - if (client.client_secret !== client_secret) { - throw new InvalidClientError("Invalid client_secret"); + // If client has a secret, validate it + if (client.client_secret) { + // Check if client_secret is required but not provided + if (!client_secret) { + throw new InvalidClientError("Client secret is required"); + } + + // Check if client_secret matches + if (client.client_secret !== client_secret) { + throw new InvalidClientError("Invalid client_secret"); + } + + // Check if client_secret has expired + if (client.client_secret_expires_at && client.client_secret_expires_at < Math.floor(Date.now() / 1000)) { + throw new InvalidClientError("Client secret has expired"); + } } req.client = client;