fix(agent-bff): send client_id on the Forest refresh grant

packages/agent-bff/package.json

@@ -51,6 +51,7 @@
     "@types/koa": "^2.13.5",
     "@types/sanitize-html": "^2.16.1",
     "@types/supertest": "^6.0.2",
+    "joi": "^17.13.6",
     "openapi3-ts": "4.6.1",
     "redoc": "2.5.3",
     "supertest": "^7.1.3"

packages/agent-bff/src/ai/ai-routes-middleware.ts

@@ -91,7 +91,7 @@ async function resolveSessionAccessToken(
   logger: Logger,
 ): Promise<string> {
   try {
-    return await ensureFreshServerAccess({ sid: principal.sid, store, serverClient });
+    return await ensureFreshServerAccess({ sid: principal.sid, store, serverClient, logger });
   } catch (error) {
     if (error instanceof OAuthRequestError && error.status >= 500) {
       logger('Warn', 'AI query refused: the Forest server could not refresh the session', {

packages/agent-bff/src/auth/forest-server-token-middleware.ts

@@ -57,6 +57,7 @@ async function resolveToken(
       sid: principal.sid,
       store: session.store,
       serverClient: session.serverClient,
+      logger,
     });
   } catch (error) {
     // The errors below carry neither the cause nor a `cause` field, so this line is the only place

packages/agent-bff/src/oauth/forest-server-client.ts

@@ -29,6 +29,11 @@ export interface ExchangeCodeParams {
   clientId: string;
 }
 
+export interface RefreshServerTokenParams {
+  refreshToken: string;
+  clientId: string;
+}
+
 export interface ForestServerClientOptions {
   forestServerUrl: string;
   envSecret: string;
@@ -142,8 +147,14 @@ export default class ForestServerClient {
     );
   }
 
-  async refreshServerToken(saasRefreshToken: string): Promise<ServerTokens> {
-    return this.postToken({ grant_type: 'refresh_token', refresh_token: saasRefreshToken }, false);
+  async refreshServerToken({
+    refreshToken,
+    clientId,
+  }: RefreshServerTokenParams): Promise<ServerTokens> {
+    return this.postToken(
+      { grant_type: 'refresh_token', refresh_token: refreshToken, client_id: clientId },
+      false,
+    );
   }
 
   private async postToken(

packages/agent-bff/src/oauth/oauth-routes.ts

@@ -349,6 +349,7 @@ async function handleAuthorizationCodeGrant(
     saasRefreshToken: serverTokens.saasRefreshToken,
     renderingId: serverTokens.renderingId,
     userId: user.id,
+    clientId: request.clientId,
   });
 
   const accessToken = issueBffAccessToken({
@@ -404,6 +405,7 @@ async function issueAccessForSession(
     sid,
     store: options.sessionStore,
     serverClient: options.serverClient,
+    logger: options.logger,
   });
 
   const expiresInSeconds = expiresInFromAccessToken(saasAccessToken);

packages/agent-bff/src/oauth/session-lifecycle.ts

@@ -1,5 +1,6 @@
 import type ForestServerClient from './forest-server-client';
-import type { SessionStore } from './session-store';
+import type { SessionStore, StoredSession } from './session-store';
+import type { Logger } from '../ports/logger-port';
 
 import jsonwebtoken from 'jsonwebtoken';
 
@@ -10,6 +11,7 @@ export interface EnsureFreshServerAccessParams {
   sid: string;
   store: SessionStore;
   serverClient: ForestServerClient;
+  logger: Logger;
 }
 
 const CLIENT_ERROR_CODES = new Set(['invalid_grant', 'invalid_request', 'invalid_client']);
@@ -23,10 +25,21 @@ function accessTokenExpiry(saasAccessToken: string): number {
 }
 
 async function refreshAndPersist(
-  sid: string,
-  store: SessionStore,
-  serverClient: ForestServerClient,
+  session: StoredSession,
+  { sid, store, serverClient, logger }: EnsureFreshServerAccessParams,
 ): Promise<string> {
+  const sessionContext = { renderingId: session.renderingId, userId: session.userId };
+
+  if (!session.clientId) {
+    logger(
+      'Warn',
+      'The session has no client id to refresh the Forest server token',
+      sessionContext,
+    );
+
+    throw sessionExpired('Session not found or expired');
+  }
+
   const currentRefresh = store.getSaasRefreshToken(sid);
 
   if (currentRefresh === undefined) {
@@ -36,9 +49,18 @@ async function refreshAndPersist(
   let rotated: Awaited<ReturnType<ForestServerClient['refreshServerToken']>>;
 
   try {
-    rotated = await serverClient.refreshServerToken(currentRefresh);
+    rotated = await serverClient.refreshServerToken({
+      refreshToken: currentRefresh,
+      clientId: session.clientId,
+    });
   } catch (error) {
     if (error instanceof OAuthExchangeError && CLIENT_ERROR_CODES.has(error.error)) {
+      logger('Warn', 'The Forest server rejected the session refresh', {
+        ...sessionContext,
+        error: error.error,
+        errorDescription: error.message,
+      });
+
       throw sessionExpired('The Forest server rejected the refresh token');
     }
 
@@ -57,11 +79,10 @@ async function refreshAndPersist(
   return rotated.saasAccessToken;
 }
 
-export default async function ensureFreshServerAccess({
-  sid,
-  store,
-  serverClient,
-}: EnsureFreshServerAccessParams): Promise<string> {
+export default async function ensureFreshServerAccess(
+  params: EnsureFreshServerAccessParams,
+): Promise<string> {
+  const { sid, store } = params;
   const session = store.get(sid);
 
   if (!session) {
@@ -75,7 +96,7 @@ export default async function ensureFreshServerAccess({
   const existing = inFlightRefreshesBySid.get(sid);
   if (existing) return existing;
 
-  const refresh = refreshAndPersist(sid, store, serverClient).finally(() => {
+  const refresh = refreshAndPersist(session, params).finally(() => {
     inFlightRefreshesBySid.delete(sid);
   });
   inFlightRefreshesBySid.set(sid, refresh);

packages/agent-bff/src/oauth/session-store.ts

@@ -7,6 +7,7 @@ export interface CreateSessionInput {
   saasRefreshToken: string;
   renderingId: number;
   userId: number;
+  clientId: string;
 }
 
 export interface StoredSession {
@@ -15,6 +16,7 @@ export interface StoredSession {
   refreshTokenHash: string;
   renderingId: number;
   userId: number;
+  clientId: string;
   expiresAt: number;
 }
 
@@ -145,6 +147,7 @@ export default function createInMemorySessionStore({
         refreshTokenHash,
         renderingId: input.renderingId,
         userId: input.userId,
+        clientId: input.clientId,
         expiresAt: now() + sessionTtlSeconds * 1000,
       });
       activeRefreshHashToSid.set(refreshTokenHash, sid);

packages/agent-bff/test/ai/ai-routes-middleware.test.ts

@@ -139,7 +139,10 @@ describe('createAiRoutesMiddleware', () => {
 
   describe('when refreshing the session fails on a transient error', () => {
     it('should answer 502 network_error rather than the oauth server_error type', async () => {
-      const store = makeSessionStore({ saasAccessToken: 'expired.token.value' });
+      const store = makeSessionStore({
+        saasAccessToken: 'expired.token.value',
+        clientId: 'client-1',
+      });
       const serverClient = {
         refreshServerToken: jest.fn().mockRejectedValue(new Error('socket hang up')),
       } as unknown as ForestServerClient;
@@ -158,7 +161,10 @@ describe('createAiRoutesMiddleware', () => {
       const transport = Object.assign(new TypeError('fetch failed'), {
         cause: new Error('connect ECONNREFUSED 10.0.0.1:443'),
       });
-      const store = makeSessionStore({ saasAccessToken: 'expired.token.value' });
+      const store = makeSessionStore({
+        saasAccessToken: 'expired.token.value',
+        clientId: 'client-1',
+      });
       const serverClient = {
         refreshServerToken: jest.fn().mockRejectedValue(transport),
       } as unknown as ForestServerClient;

packages/agent-bff/test/auth/forest-server-token-middleware.test.ts

@@ -111,7 +111,7 @@ describe('forest server token middleware', () => {
 
     it('should refuse with audit_unavailable when the Forest server cannot be reached', async () => {
       const store = {
-        get: () => ({ saasAccessToken: expiredAccessToken() }),
+        get: () => ({ saasAccessToken: expiredAccessToken(), clientId: 'client-1' }),
         getSaasRefreshToken: () => 'refresh-token',
       } as unknown as SessionStore;
       const serverClient = {
@@ -160,7 +160,7 @@ describe('forest server token middleware', () => {
 
     it('should refuse with session_expired when the Forest server rejects the refresh token', async () => {
       const store = {
-        get: () => ({ saasAccessToken: expiredAccessToken() }),
+        get: () => ({ saasAccessToken: expiredAccessToken(), clientId: 'client-1' }),
         getSaasRefreshToken: () => 'refresh-token',
       } as unknown as SessionStore;
       const serverClient = {

packages/agent-bff/test/oauth/fixtures/forestadmin-server-oauth-route-validator-issue-token.ts

@@ -0,0 +1,33 @@
+import Joi from 'joi';
+
+const issueTokenBodySchema = Joi.object({
+  client_id: Joi.string().required(),
+  grant_type: Joi.string().valid('authorization_code', 'refresh_token').required(),
+  code: Joi.string().when('grant_type', {
+    is: 'authorization_code',
+    then: Joi.required(),
+    otherwise: Joi.forbidden(),
+  }),
+  redirect_uri: Joi.string().uri().when('grant_type', {
+    is: 'authorization_code',
+    then: Joi.required(),
+    otherwise: Joi.forbidden(),
+  }),
+  code_verifier: Joi.string().when('grant_type', {
+    is: 'authorization_code',
+    then: Joi.required(),
+    otherwise: Joi.forbidden(),
+  }),
+  refresh_token: Joi.string().when('grant_type', {
+    is: 'refresh_token',
+    then: Joi.required(),
+    otherwise: Joi.forbidden(),
+  }),
+  scope: Joi.string().when('grant_type', {
+    is: 'refresh_token',
+    then: Joi.optional(),
+    otherwise: Joi.forbidden(),
+  }),
+}).required();
+
+export default issueTokenBodySchema;

packages/agent-bff/test/oauth/forest-server-client.test.ts

@@ -91,7 +91,7 @@ describe('ForestServerClient', () => {
       });
 
       const client = new ForestServerClient({ forestServerUrl: SERVER_URL, envSecret: ENV_SECRET });
-      const tokens = await client.refreshServerToken('R1');
+      const tokens = await client.refreshServerToken({ refreshToken: 'R1', clientId: 'client-1' });
 
       expect(tokens.saasAccessToken).toBe(newAccess);
       expect(tokens.saasRefreshToken).toBe('R2');
@@ -103,7 +103,9 @@ describe('ForestServerClient', () => {
 
       const client = new ForestServerClient({ forestServerUrl: SERVER_URL, envSecret: ENV_SECRET });
 
-      await expect(client.refreshServerToken('R1')).rejects.toBeInstanceOf(OAuthExchangeError);
+      await expect(
+        client.refreshServerToken({ refreshToken: 'R1', clientId: 'client-1' }),
+      ).rejects.toBeInstanceOf(OAuthExchangeError);
     });
 
     it('should not require renderingId in the refresh-grant access token', async () => {
@@ -118,7 +120,9 @@ describe('ForestServerClient', () => {
 
       const client = new ForestServerClient({ forestServerUrl: SERVER_URL, envSecret: ENV_SECRET });
 
-      await expect(client.refreshServerToken('R1')).resolves.toMatchObject({
+      await expect(
+        client.refreshServerToken({ refreshToken: 'R1', clientId: 'client-1' }),
+      ).resolves.toMatchObject({
         saasRefreshToken: 'R2',
       });
     });
@@ -202,7 +206,9 @@ describe('ForestServerClient', () => {
 
       const client = new ForestServerClient({ forestServerUrl: SERVER_URL, envSecret: ENV_SECRET });
 
-      await expect(client.refreshServerToken('R1')).resolves.toMatchObject({ renderingId: 0 });
+      await expect(
+        client.refreshServerToken({ refreshToken: 'R1', clientId: 'client-1' }),
+      ).resolves.toMatchObject({ renderingId: 0 });
     });
   });
 

packages/agent-bff/test/oauth/forest-server-token-contract.test.ts

@@ -0,0 +1,65 @@
+import jsonwebtoken from 'jsonwebtoken';
+
+import issueTokenBodySchema from './fixtures/forestadmin-server-oauth-route-validator-issue-token';
+import ForestServerClient from '../../src/oauth/forest-server-client';
+
+jest.mock('@forestadmin/forestadmin-client', () => ({
+  __esModule: true,
+  default: jest.fn(() => ({ authService: { getUserInfo: jest.fn() } })),
+}));
+
+function captureTokenRequestBody(): jest.Mock {
+  const access = jsonwebtoken.sign(
+    { meta: { renderingId: 17 }, exp: Math.floor(Date.now() / 1000) + 3600 },
+    'irrelevant',
+  );
+  const fetchMock = jest.fn().mockResolvedValue({
+    ok: true,
+    status: 200,
+    json: async () => ({ access_token: access, refresh_token: 'R2' }),
+  });
+  global.fetch = fetchMock as unknown as typeof fetch;
+
+  return fetchMock;
+}
+
+function sentBody(fetchMock: jest.Mock): unknown {
+  return JSON.parse(fetchMock.mock.calls[0][1].body);
+}
+
+describe('Forest server /oauth/token contract', () => {
+  const client = new ForestServerClient({
+    forestServerUrl: 'https://api.forestadmin.com',
+    envSecret: 'env-secret',
+  });
+
+  it('should send a refresh grant body the Forest server accepts', async () => {
+    const fetchMock = captureTokenRequestBody();
+
+    await client.refreshServerToken({ refreshToken: 'R1', clientId: 'client-1' });
+
+    expect(issueTokenBodySchema.validate(sentBody(fetchMock)).error).toBeUndefined();
+  });
+
+  it('should send an authorization code body the Forest server accepts', async () => {
+    const fetchMock = captureTokenRequestBody();
+
+    await client.exchangeCode({
+      code: 'code-1',
+      codeVerifier: 'verifier-1',
+      redirectUri: 'http://localhost/callback',
+      clientId: 'client-1',
+    });
+
+    expect(issueTokenBodySchema.validate(sentBody(fetchMock)).error).toBeUndefined();
+  });
+
+  it('should be rejected by the Forest server schema when client_id is missing', () => {
+    const { error } = issueTokenBodySchema.validate({
+      grant_type: 'refresh_token',
+      refresh_token: 'R1',
+    });
+
+    expect(error?.message).toBe('"client_id" is required');
+  });
+});

packages/agent-bff/test/oauth/oauth-routes.test.ts

@@ -370,6 +370,15 @@ describe('oauth-routes POST /oauth/token', () => {
       expect(JSON.stringify(logs)).not.toContain(SENTINEL_ACCESS);
       expect(JSON.stringify(logs)).not.toContain(SENTINEL_REFRESH);
     });
+
+    it('should store the exchanged client id in the session for later refreshes', async () => {
+      const { app, sessionStore } = buildApp(stubServerClient());
+
+      const response = await request(app.callback()).post('/oauth/token').send(TOKEN_BODY);
+      const { sid } = jsonwebtoken.decode(response.body.access_token) as { sid: string };
+
+      expect(sessionStore.get(sid)?.clientId).toBe(CLIENT_ID);
+    });
   });
 
   describe('when the redirect_uri does not match the registered client', () => {