@W-19861422 - token endpoint hardening

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@tableau/mcp-server",
   "description": "Helping agents see and understand data.",
-  "version": "1.17.17",
+  "version": "1.17.18",
   "repository": {
     "type": "git",
     "url": "git+https://github.com/tableau/tableau-mcp.git"

src/server/oauth/token.ts

@@ -87,6 +87,26 @@ export function token(
             return;
           }
 
+          // Validate redirect_uri matches what was used at authorization (OAuth 2.1 Section 4.1.3)
+          if (result.data.redirectUri !== authCode.redirectUri) {
+            res.status(400).json({
+              error: 'invalid_grant',
+              error_description: 'Redirect URI mismatch',
+            });
+            return;
+          }
+
+          // Validate client_id matches what was used at authorization (when provided).
+          // Fall back to the credential-verified identity from the Basic Auth path.
+          const effectiveClientId = result.data.clientId || clientCredentialClientId || undefined;
+          if (effectiveClientId && effectiveClientId !== authCode.clientId) {
+            res.status(400).json({
+              error: 'invalid_grant',
+              error_description: 'Client ID mismatch',
+            });
+            return;
+          }
+
           // Generate tokens
           const refreshTokenId = randomBytes(32).toString('hex');
           const accessToken = await createAccessToken(authCode, publicKey);

tests/oauth/embedded-authz/oauth.test.ts

@@ -5,6 +5,7 @@ import request from 'supertest';
 import { getConfig } from '../../../src/config.js';
 import { serverName } from '../../../src/server.js';
 import { startExpressServer } from '../../../src/server/express.js';
+import { generateCodeChallenge } from '../../../src/server/oauth/generateCodeChallenge.js';
 import { AwaitableWritableStream } from './awaitableWritableStream.js';
 import { exchangeAuthzCodeForAccessToken } from './exchangeAuthzCodeForAccessToken.js';
 import { resetEnv, setEnv } from './testEnv.js';
@@ -351,6 +352,246 @@ describe('OAuth', () => {
     expect(data).toMatchObject({ result: { tools: expect.any(Array) } });
   });
 
+  it('should reject token exchange when redirect_uri does not match authorization request', async () => {
+    const { app } = await startServer();
+
+    mocks.mockGetTokenResult.mockResolvedValue({
+      accessToken: 'test-access-token',
+      refreshToken: 'test-refresh-token',
+      expiresInSeconds: 3600,
+      originHost: '10ax.online.tableau.com',
+    });
+
+    const codeChallenge = 'test-code-challenge';
+    const authzResponse = await request(app)
+      .get('/oauth2/authorize')
+      .query({
+        client_id: 'test-client-id',
+        redirect_uri: 'http://localhost:3000',
+        response_type: 'code',
+        code_challenge: generateCodeChallenge(codeChallenge),
+        code_challenge_method: 'S256',
+        state: 'test-state',
+      });
+
+    const authzLocation = new URL(authzResponse.headers['location']);
+    const [authKey, tableauState] = authzLocation.searchParams.get('state')?.split(':') ?? [];
+
+    const callbackResponse = await request(app)
+      .get('/Callback')
+      .query({
+        code: 'test-code',
+        state: `${authKey}:${tableauState}`,
+      });
+
+    expect(callbackResponse.status).toBe(302);
+    const location = new URL(callbackResponse.headers['location']);
+    const code = location.searchParams.get('code');
+
+    const tokenResponse = await request(app).post('/oauth2/token').send({
+      grant_type: 'authorization_code',
+      code,
+      code_verifier: codeChallenge,
+      redirect_uri: 'http://localhost:9999/different',
+      client_id: 'test-client-id',
+      client_secret: 'test-client-secret',
+    });
+
+    expect(tokenResponse.status).toBe(400);
+    expect(tokenResponse.body).toEqual({
+      error: 'invalid_grant',
+      error_description: 'Redirect URI mismatch',
+    });
+  });
+
+  it('should succeed at token exchange when redirect_uri matches authorization request', async () => {
+    const { app } = await startServer();
+
+    mocks.mockGetTokenResult.mockResolvedValue({
+      accessToken: 'test-access-token',
+      refreshToken: 'test-refresh-token',
+      expiresInSeconds: 3600,
+      originHost: '10ax.online.tableau.com',
+    });
+
+    const tokenResponse = await exchangeAuthzCodeForAccessToken(app);
+
+    expect(tokenResponse.access_token).toBeDefined();
+    expect(tokenResponse.token_type).toBe('Bearer');
+  });
+
+  it('should reject token exchange when client_id does not match authorization request', async () => {
+    // Add a second client pair so 'other-client-id' passes the credential check,
+    // but it won't match the 'test-client-id' stored in the authorization code.
+    vi.stubEnv(
+      'OAUTH_CLIENT_ID_SECRET_PAIRS',
+      'test-client-id:test-client-secret,other-client-id:other-client-secret',
+    );
+
+    const { app } = await startServer();
+
+    mocks.mockGetTokenResult.mockResolvedValue({
+      accessToken: 'test-access-token',
+      refreshToken: 'test-refresh-token',
+      expiresInSeconds: 3600,
+      originHost: '10ax.online.tableau.com',
+    });
+
+    const codeChallenge = 'test-code-challenge';
+    const authzResponse = await request(app)
+      .get('/oauth2/authorize')
+      .query({
+        client_id: 'test-client-id',
+        redirect_uri: 'http://localhost:3000',
+        response_type: 'code',
+        code_challenge: generateCodeChallenge(codeChallenge),
+        code_challenge_method: 'S256',
+        state: 'test-state',
+      });
+
+    const authzLocation = new URL(authzResponse.headers['location']);
+    const [authKey, tableauState] = authzLocation.searchParams.get('state')?.split(':') ?? [];
+
+    const callbackResponse = await request(app)
+      .get('/Callback')
+      .query({
+        code: 'test-code',
+        state: `${authKey}:${tableauState}`,
+      });
+
+    expect(callbackResponse.status).toBe(302);
+    const location = new URL(callbackResponse.headers['location']);
+    const code = location.searchParams.get('code');
+
+    const tokenResponse = await request(app).post('/oauth2/token').send({
+      grant_type: 'authorization_code',
+      code,
+      code_verifier: codeChallenge,
+      redirect_uri: 'http://localhost:3000',
+      client_id: 'other-client-id',
+      client_secret: 'other-client-secret',
+    });
+
+    expect(tokenResponse.status).toBe(400);
+    expect(tokenResponse.body).toEqual({
+      error: 'invalid_grant',
+      error_description: 'Client ID mismatch',
+    });
+  });
+
+  it('should succeed at token exchange when client_id is absent from token request', async () => {
+    // Disable client credential pairs so the token endpoint doesn't require client_id.
+    // This tests the "client_id is optional" path: if absent, the mismatch check is skipped.
+    vi.stubEnv('OAUTH_CLIENT_ID_SECRET_PAIRS', '');
+
+    const { app } = await startServer();
+
+    mocks.mockGetTokenResult.mockResolvedValue({
+      accessToken: 'test-access-token',
+      refreshToken: 'test-refresh-token',
+      expiresInSeconds: 3600,
+      originHost: '10ax.online.tableau.com',
+    });
+
+    const codeChallenge = 'test-code-challenge';
+    const authzResponse = await request(app)
+      .get('/oauth2/authorize')
+      .query({
+        client_id: 'test-client-id',
+        redirect_uri: 'http://localhost:3000',
+        response_type: 'code',
+        code_challenge: generateCodeChallenge(codeChallenge),
+        code_challenge_method: 'S256',
+        state: 'test-state',
+      });
+
+    const authzLocation = new URL(authzResponse.headers['location']);
+    const [authKey, tableauState] = authzLocation.searchParams.get('state')?.split(':') ?? [];
+
+    const callbackResponse = await request(app)
+      .get('/Callback')
+      .query({
+        code: 'test-code',
+        state: `${authKey}:${tableauState}`,
+      });
+
+    expect(callbackResponse.status).toBe(302);
+    const location = new URL(callbackResponse.headers['location']);
+    const code = location.searchParams.get('code');
+
+    const tokenResponse = await request(app).post('/oauth2/token').send({
+      grant_type: 'authorization_code',
+      code,
+      code_verifier: codeChallenge,
+      redirect_uri: 'http://localhost:3000',
+    });
+
+    expect(tokenResponse.status).toBe(200);
+    expect(tokenResponse.body.access_token).toBeDefined();
+    expect(tokenResponse.body.token_type).toBe('Bearer');
+  });
+
+  it('should reject token exchange when client_id mismatches via Basic Auth (no body client_id)', async () => {
+    vi.stubEnv(
+      'OAUTH_CLIENT_ID_SECRET_PAIRS',
+      'test-client-id:test-client-secret,other-client-id:other-client-secret',
+    );
+
+    const { app } = await startServer();
+
+    mocks.mockGetTokenResult.mockResolvedValue({
+      accessToken: 'test-access-token',
+      refreshToken: 'test-refresh-token',
+      expiresInSeconds: 3600,
+      originHost: '10ax.online.tableau.com',
+    });
+
+    const codeChallenge = 'test-code-challenge';
+    const authzResponse = await request(app)
+      .get('/oauth2/authorize')
+      .query({
+        client_id: 'test-client-id',
+        redirect_uri: 'http://localhost:3000',
+        response_type: 'code',
+        code_challenge: generateCodeChallenge(codeChallenge),
+        code_challenge_method: 'S256',
+        state: 'test-state',
+      });
+
+    const authzLocation = new URL(authzResponse.headers['location']);
+    const [authKey, tableauState] = authzLocation.searchParams.get('state')?.split(':') ?? [];
+
+    const callbackResponse = await request(app)
+      .get('/Callback')
+      .query({
+        code: 'test-code',
+        state: `${authKey}:${tableauState}`,
+      });
+
+    expect(callbackResponse.status).toBe(302);
+    const location = new URL(callbackResponse.headers['location']);
+    const code = location.searchParams.get('code');
+
+    // Send token request with Basic Auth as `other-client-id` but no `client_id` in body.
+    // The effectiveClientId fallback should detect the mismatch.
+    const basicAuth = Buffer.from('other-client-id:other-client-secret').toString('base64');
+    const tokenResponse = await request(app)
+      .post('/oauth2/token')
+      .set('Authorization', `Basic ${basicAuth}`)
+      .send({
+        grant_type: 'authorization_code',
+        code,
+        code_verifier: codeChallenge,
+        redirect_uri: 'http://localhost:3000',
+      });
+
+    expect(tokenResponse.status).toBe(400);
+    expect(tokenResponse.body).toEqual({
+      error: 'invalid_grant',
+      error_description: 'Client ID mismatch',
+    });
+  });
+
   it('should reject if the access token is invalid or expired', async () => {
     const { app } = await startServer();