From b7201e64f2134d0c9eaf92bf1effb874e669b6d7 Mon Sep 17 00:00:00 2001 From: Mickael Gaillard Date: Thu, 4 Jun 2026 14:57:05 +0200 Subject: [PATCH] fix(oauth): add client_id to exchangeCodeForToken + extend unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add client_id param to api_token grant in exchangeCodeForToken() to align with PHP SDK (RFC 6749 §4.4 / platform-specific requirement) - Add 14 new tests to upsun-client.test.ts covering: middleware retry logic (401/non-401/double-retry/bearer-only), getUserId caching, getToken bearer/throw paths, authenticate success/no-key, cloneHeaders (undefined/Headers/array/object) - Add 1 new test + update 1 existing test in oauth2-client.test.ts: assert client_id present in api_token grant body; add refresh-token missing guard coverage 618 unit tests passing, 0 failures Coverage: oauth-provider.ts 100%, upsun.ts 100%/96% branches --- src/core/oauth-provider.ts | 5 +- tests/unit/core/oauth2-client.test.ts | 20 +++- tests/unit/upsun-client.test.ts | 160 ++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 4 deletions(-) diff --git a/src/core/oauth-provider.ts b/src/core/oauth-provider.ts index 0973e02..c65ea08 100644 --- a/src/core/oauth-provider.ts +++ b/src/core/oauth-provider.ts @@ -1,7 +1,7 @@ type TokenResponse = { access_token: string; refresh_token?: string; - expires_in: number; // in secondes + expires_in: number; // in seconds token_type: string; }; @@ -21,7 +21,7 @@ export class OAuth2Client { private typeToken: string | null = null; private accessToken: string | null = null; private refreshToken: string | null = null; - private tokenExpiry: number = 0; // timestamp en ms + private tokenExpiry: number = 0; // timestamp in ms /** Deduplicates concurrent token acquisition calls (thundering-herd protection). */ private pendingToken: Promise | null = null; @@ -51,6 +51,7 @@ export class OAuth2Client { const params = new URLSearchParams({ grant_type: 'api_token', api_token: this.clientSecret, + client_id: this.clientId, }); const response = await fetch(this.tokenEndpoint, { diff --git a/tests/unit/core/oauth2-client.test.ts b/tests/unit/core/oauth2-client.test.ts index 34c4866..65eecc5 100644 --- a/tests/unit/core/oauth2-client.test.ts +++ b/tests/unit/core/oauth2-client.test.ts @@ -31,17 +31,21 @@ describe('OAuth2Client', () => { expect(typeof result).toBe('boolean'); }); - it('should send Authorization: Basic header', async () => { + it('should send Authorization: Basic header and include client_id in body', async () => { let capturedAuth: string | undefined; + let capturedBody: string | undefined; nock('https://auth.upsun.com') .post('/oauth2/token') - .reply(function () { + .reply(function (_uri, body) { capturedAuth = this.req.headers['authorization'] as string; + capturedBody = body as string; return [200, { access_token: 'tok', token_type: 'Bearer', expires_in: 3600 }]; }); await oauth2Client.exchangeCodeForToken(); expect(capturedAuth).toMatch(/^Basic /); + expect(capturedBody).toContain('client_id=test-client-id'); + expect(capturedBody).toContain('grant_type=api_token'); }); it('should handle token exchange errors', async () => { @@ -252,4 +256,16 @@ describe('OAuth2Client', () => { expect(token).toBe('refreshed-via-separate-endpoint'); }); }); + + // --------------------------------------------------------------------------- + // refreshAccessToken defensive guard (private method, line 121) + // --------------------------------------------------------------------------- + describe('refreshAccessToken (defensive guard)', () => { + it('should throw when called without a stored refresh token', async () => { + // oauth2Client has no refresh token because no exchange has been performed + await expect((oauth2Client as any).refreshAccessToken()).rejects.toThrow( + 'No refresh token available', + ); + }); + }); }); diff --git a/tests/unit/upsun-client.test.ts b/tests/unit/upsun-client.test.ts index e826810..201c9b6 100644 --- a/tests/unit/upsun-client.test.ts +++ b/tests/unit/upsun-client.test.ts @@ -73,4 +73,164 @@ describe('UpsunClient', () => { expect(typeof client.getUserId).toBe('function'); }); }); + + // --------------------------------------------------------------------------- + // createAuthRetryMiddleware — RFC 6750 §3.1 P1 fix + // --------------------------------------------------------------------------- + describe('createAuthRetryMiddleware', () => { + const AUTH_URL = 'https://auth.upsun.com'; + const API_URL = 'https://api.upsun.com'; + const TOKEN_PATH = '/oauth2/token'; + const ME_PATH = '/users/me'; + + const mockUser = { id: 'test-user-id' }; + const tokenReply = (token: string) => ({ + access_token: token, + token_type: 'Bearer', + expires_in: 3600, + }); + + it('should pass through non-401 responses without retry', async () => { + nock(AUTH_URL).post(TOKEN_PATH).reply(200, tokenReply('token-1')); + nock(API_URL).get(ME_PATH).reply(200, mockUser); + + const result = await client.users.me(); + expect(result.id).toBe('test-user-id'); + expect(nock.pendingMocks()).toHaveLength(0); + }); + + it('should call forceRefresh and retry with new token after 401 (P1 fix)', async () => { + // Initial token acquisition + nock(AUTH_URL).post(TOKEN_PATH).reply(200, tokenReply('token-1')); + // First API call → 401 (expired / revoked token) + nock(API_URL).get(ME_PATH).reply(401, { error: 'invalid_token' }); + // forceRefresh() re-acquires a new token + nock(AUTH_URL).post(TOKEN_PATH).reply(200, tokenReply('token-2')); + // Retry with new token → success + nock(API_URL).get(ME_PATH).reply(200, mockUser); + + const result = await client.users.me(); + expect(result.id).toBe('test-user-id'); + expect(nock.pendingMocks()).toHaveLength(0); + }); + + it('should not retry a second time when __upsunRetry guard is set', async () => { + nock(AUTH_URL).post(TOKEN_PATH).reply(200, tokenReply('token-1')); + nock(API_URL).get(ME_PATH).reply(401, { error: 'invalid_token' }); + // forceRefresh is called once + nock(AUTH_URL).post(TOKEN_PATH).reply(200, tokenReply('token-2')); + // Retry still returns 401 — must not loop again + nock(API_URL).get(ME_PATH).reply(401, { error: 'invalid_token' }); + + await expect(client.users.me()).rejects.toThrow(); + expect(nock.pendingMocks()).toHaveLength(0); + }); + + it('should return 401 without retry in Bearer-only mode (no auth client)', async () => { + const bearerClient = new UpsunClient(); + bearerClient.setBearerToken('static-bearer-token'); + + // Only one request — no retry, no auth call + nock(API_URL).get(ME_PATH).reply(401, { error: 'invalid_token' }); + + await expect(bearerClient.users.me()).rejects.toThrow(); + expect(nock.pendingMocks()).toHaveLength(0); + }); + }); + + // --------------------------------------------------------------------------- + // getUserId + // --------------------------------------------------------------------------- + describe('getUserId', () => { + it('should fetch user ID from API and cache it for subsequent calls', async () => { + nock('https://auth.upsun.com').post('/oauth2/token').reply(200, { + access_token: 'token-1', + token_type: 'Bearer', + expires_in: 3600, + }); + nock('https://api.upsun.com').get('/users/me').reply(200, { id: 'user-abc' }); + + const userId = await client.getUserId(); + expect(userId).toBe('user-abc'); + + // Second call must use the cached value — no new network request + const cachedId = await client.getUserId(); + expect(cachedId).toBe('user-abc'); + expect(nock.pendingMocks()).toHaveLength(0); + }); + }); + + // --------------------------------------------------------------------------- + // getToken + // --------------------------------------------------------------------------- + describe('getToken', () => { + it('should return bearer token when set via setBearerToken', async () => { + const unauthClient = new UpsunClient(); + unauthClient.setBearerToken('my-bearer-token'); + + const token = await (unauthClient as any).getToken(); + expect(token).toBe('my-bearer-token'); + }); + + it('should throw when neither apiKey nor bearer token is configured', async () => { + const unauthClient = new UpsunClient(); + + await expect((unauthClient as any).getToken()).rejects.toThrow( + 'No authentication method available', + ); + }); + }); + + // --------------------------------------------------------------------------- + // authenticate + // --------------------------------------------------------------------------- + describe('authenticate', () => { + it('should throw when no API key is configured', async () => { + const unauthClient = new UpsunClient(); + + await expect(unauthClient.authenticate()).rejects.toThrow('API Key is not defined'); + }); + + it('should return true on successful token exchange', async () => { + nock('https://auth.upsun.com').post('/oauth2/token').reply(200, { + access_token: 'fresh-token', + token_type: 'Bearer', + expires_in: 3600, + }); + + await expect(client.authenticate()).resolves.toBe(true); + }); + }); + + // --------------------------------------------------------------------------- + // cloneHeaders (private — tests via cast to any) + // --------------------------------------------------------------------------- + describe('cloneHeaders', () => { + it('should return empty object when headers is undefined', () => { + const result = (client as any).cloneHeaders(undefined); + expect(result).toEqual({}); + }); + + it('should clone a Headers instance (keys normalised to lower-case)', () => { + const headers = new Headers({ 'Content-Type': 'application/json', 'X-Custom': 'value' }); + const result = (client as any).cloneHeaders(headers); + expect(result['content-type']).toBe('application/json'); + expect(result['x-custom']).toBe('value'); + }); + + it('should clone an array of [key, value] header pairs', () => { + const headers: [string, string][] = [ + ['Authorization', 'Bearer test'], + ['Accept', 'application/json'], + ]; + const result = (client as any).cloneHeaders(headers); + expect(result).toEqual({ Authorization: 'Bearer test', Accept: 'application/json' }); + }); + + it('should clone a plain-object header map', () => { + const headers = { Authorization: 'Bearer test', 'Content-Type': 'application/json' }; + const result = (client as any).cloneHeaders(headers); + expect(result).toEqual({ Authorization: 'Bearer test', 'Content-Type': 'application/json' }); + }); + }); });