Skip to content

Commit cce0c0d

Browse files
JeongJaeSoonclaudegithub-actions[bot]him0
authored
fix(remote): translate jose token errors to InvalidTokenError (401) (#398)
* fix(remote): translate jose token errors to InvalidTokenError for RFC-compliant 401 When a Remote mode access token expires, jose's `JWTExpired` (and related errors like `JWSSignatureVerificationFailed`, `JWTInvalid`, `JWSInvalid`, `JWTClaimValidationFailed`) propagated from `verifyAccessToken` uncaught. MCP SDK's `bearerAuth` maps unknown exceptions to a generic 500 `server_error`, breaking RFC 6750 clients (e.g. Anthropic Managed Agents Vault) that rely on a 401 `invalid_token` response to trigger transparent `refresh_token` reuse. - Catch jose errors in `FreeeOAuthProvider.verifyAccessToken` and translate them to `InvalidTokenError` so `bearerAuth` emits HTTP 401 + `WWW-Authenticate: Bearer error="invalid_token"` as required by RFC 6750 - Call `getCurrentRecorder()?.recordError({ source: 'auth', ... })` before throwing so the canonical log captures the real jose error name/message instead of the PR #392 `UnrecordedError` safety net fallback - Add 5 test cases covering expired / wrong-signature / malformed / wrong-issuer paths and the `RequestRecorder` enrichment Fixes #394 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: resolve conflicts with main (gh skill section + contributors) - README.md: restore gh skill / APM install section added in #397 - README.md: add Kitamura777 and yuyohi contributors from main - CLAUDE.md: add FREEE_API_BASE_URL env var entry from #407 Co-authored-by: Hiroki Nakashima <him0@users.noreply.github.com> * docs(changeset): CHANGELOG ガイドラインに合わせて要約に整理 * docs(changeset): Fixes 行を削除(既存 CHANGELOG の慣習に揃える) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Hiroki Nakashima <him0@users.noreply.github.com> Co-authored-by: him0 <him0@freee.co.jp>
1 parent 1c75d88 commit cce0c0d

4 files changed

Lines changed: 146 additions & 6 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"freee-mcp": patch
3+
---
4+
5+
Remote モードで期限切れアクセストークンが 500 ではなく 401 を返すように修正
6+
7+
- jose のトークン検証例外を `InvalidTokenError` に変換し、`WWW-Authenticate: Bearer error="invalid_token"` 付き HTTP 401 を返す
8+
- これにより RFC 6750 準拠クライアント(Anthropic Managed Agents Vault など)の `refresh_token` 自動再発行が動作するようになる

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ npx --package=freee-mcp -- freee-sign-mcp configure
283283
<a href="https://github.com/ryuuuuma"><img src="https://github.com/ryuuuuma.png" width="40" height="40" alt="@ryuuuuma"></a>
284284
<a href="https://github.com/toyamagu-2021"><img src="https://github.com/toyamagu-2021.png" width="40" height="40" alt="@toyamagu-2021"></a>
285285
<a href="https://github.com/YasuakiOmokawa"><img src="https://github.com/YasuakiOmokawa.png" width="40" height="40" alt="@YasuakiOmokawa"></a>
286+
<a href="https://github.com/Ryosuke-Watanabe9"><img src="https://github.com/Ryosuke-Watanabe9.png" width="40" height="40" alt="@Ryosuke-Watanabe9"></a>
286287
<a href="https://github.com/Kitamura777"><img src="https://github.com/Kitamura777.png" width="40" height="40" alt="@Kitamura777"></a>
287288
<a href="https://github.com/yuyohi"><img src="https://github.com/yuyohi.png" width="40" height="40" alt="@yuyohi"></a>
288289
<!-- CONTRIBUTORS-END -->

src/server/oauth-provider.test.ts

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
1-
import { InvalidGrantError } from '@modelcontextprotocol/sdk/server/auth/errors.js';
1+
import {
2+
InvalidGrantError,
3+
InvalidTokenError,
4+
} from '@modelcontextprotocol/sdk/server/auth/errors.js';
25
import type { AuthorizationParams } from '@modelcontextprotocol/sdk/server/auth/provider.js';
36
import type { OAuthClientInformationFull } from '@modelcontextprotocol/sdk/shared/auth.js';
7+
import { SignJWT } from 'jose';
48
import { describe, expect, it, vi } from 'vitest';
59
import type { TokenStore } from '../storage/token-store.js';
610
import type { RedisClientStore } from './client-store.js';
711
import type { FreeeOAuthProviderDeps } from './oauth-provider.js';
812
import { FreeeOAuthProvider } from './oauth-provider.js';
913
import type { AuthCodeData, OAuthStateStore, RefreshTokenData } from './oauth-store.js';
14+
import { RequestRecorder, withRequestRecorder } from './request-context.js';
1015

1116
const TEST_SECRET = 'test-jwt-secret-long-enough-for-hmac-signing';
1217
const TEST_ISSUER = 'https://mcp.example.com';
@@ -287,10 +292,91 @@ describe('FreeeOAuthProvider', () => {
287292
expect(authInfo.extra?.tokenStore).toBe(tokenStore);
288293
});
289294

290-
it('rejects invalid JWT', async () => {
295+
it('rejects a malformed (non-JWT) token as InvalidTokenError', async () => {
291296
const { provider } = createProvider();
292297

293-
await expect(provider.verifyAccessToken('invalid-jwt')).rejects.toThrow();
298+
await expect(provider.verifyAccessToken('invalid-jwt')).rejects.toThrow(InvalidTokenError);
299+
});
300+
301+
it('maps jose JWTExpired to InvalidTokenError("Token has expired")', async () => {
302+
const { provider } = createProvider();
303+
const pastTime = Math.floor(Date.now() / 1000) - 7200;
304+
const expiredToken = await new SignJWT({ scope: 'mcp:read', client_id: 'test-client-id' })
305+
.setProtectedHeader({ alg: 'HS256' })
306+
.setSubject('user-1')
307+
.setIssuer(TEST_ISSUER)
308+
.setIssuedAt(pastTime)
309+
.setExpirationTime(pastTime + 3600)
310+
.sign(new TextEncoder().encode(TEST_SECRET));
311+
312+
await expect(provider.verifyAccessToken(expiredToken)).rejects.toThrow(
313+
new InvalidTokenError('Token has expired'),
314+
);
315+
});
316+
317+
it('maps jose JWSSignatureVerificationFailed to InvalidTokenError', async () => {
318+
const { provider } = createProvider();
319+
const foreignToken = await new SignJWT({ scope: 'mcp:read', client_id: 'test-client-id' })
320+
.setProtectedHeader({ alg: 'HS256' })
321+
.setSubject('user-1')
322+
.setIssuer(TEST_ISSUER)
323+
.setIssuedAt()
324+
.setExpirationTime('1h')
325+
.sign(new TextEncoder().encode('different-jwt-secret-long-enough-for-hmac-signing'));
326+
327+
await expect(provider.verifyAccessToken(foreignToken)).rejects.toThrow(
328+
new InvalidTokenError('Invalid token signature'),
329+
);
330+
});
331+
332+
it('maps jose JWTClaimValidationFailed (wrong issuer) to InvalidTokenError', async () => {
333+
const { provider } = createProvider();
334+
const wrongIssuerToken = await new SignJWT({ scope: 'mcp:read', client_id: 'test-client-id' })
335+
.setProtectedHeader({ alg: 'HS256' })
336+
.setSubject('user-1')
337+
.setIssuer('https://wrong.example.com')
338+
.setIssuedAt()
339+
.setExpirationTime('1h')
340+
.sign(new TextEncoder().encode(TEST_SECRET));
341+
342+
await expect(provider.verifyAccessToken(wrongIssuerToken)).rejects.toThrowError(
343+
InvalidTokenError,
344+
);
345+
});
346+
347+
it('records auth error to the current RequestRecorder on expired token', async () => {
348+
const { provider } = createProvider();
349+
const pastTime = Math.floor(Date.now() / 1000) - 7200;
350+
const expiredToken = await new SignJWT({ scope: 'mcp:read', client_id: 'test-client-id' })
351+
.setProtectedHeader({ alg: 'HS256' })
352+
.setSubject('user-1')
353+
.setIssuer(TEST_ISSUER)
354+
.setIssuedAt(pastTime)
355+
.setExpirationTime(pastTime + 3600)
356+
.sign(new TextEncoder().encode(TEST_SECRET));
357+
358+
const recorder = new RequestRecorder({
359+
request_id: 'req-1',
360+
source_ip: '127.0.0.1',
361+
method: 'POST',
362+
path: '/mcp',
363+
});
364+
365+
await withRequestRecorder(recorder, async () => {
366+
await expect(provider.verifyAccessToken(expiredToken)).rejects.toThrow(InvalidTokenError);
367+
});
368+
369+
const payload = recorder.buildPayload({ status: 401, duration_ms: 1 });
370+
expect(payload.errors).toHaveLength(1);
371+
expect(payload.errors[0]).toMatchObject({
372+
source: 'auth',
373+
status_code: 401,
374+
error_type: 'invalid_token',
375+
});
376+
expect(payload.errors[0]?.chain[0]).toMatchObject({
377+
name: 'JWTExpired',
378+
message: 'Token has expired',
379+
});
294380
});
295381
});
296382

src/server/oauth-provider.ts

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { randomUUID } from 'node:crypto';
22
import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/server/auth/clients.js';
3-
import { InvalidGrantError } from '@modelcontextprotocol/sdk/server/auth/errors.js';
3+
import {
4+
InvalidGrantError,
5+
InvalidTokenError,
6+
} from '@modelcontextprotocol/sdk/server/auth/errors.js';
47
import type {
58
AuthorizationParams,
69
OAuthServerProvider,
@@ -16,8 +19,10 @@ import { generatePKCE } from '../auth/oauth.js';
1619
import { FREEE_CALLBACK_PATH } from '../constants.js';
1720
import type { TokenStore } from '../storage/token-store.js';
1821
import type { RedisClientStore } from './client-store.js';
19-
import { signAccessToken, verifyAccessToken as verifyJwt } from './jwt.js';
22+
import { makeErrorChain } from './error-serializer.js';
23+
import { joseErrors, signAccessToken, verifyAccessToken as verifyJwt } from './jwt.js';
2024
import type { OAuthStateStore } from './oauth-store.js';
25+
import { getCurrentRecorder } from './request-context.js';
2126

2227
export interface FreeeOAuthProviderDeps {
2328
clientStore: RedisClientStore;
@@ -138,7 +143,27 @@ export class FreeeOAuthProvider implements OAuthServerProvider {
138143
}
139144

140145
async verifyAccessToken(token: string): Promise<AuthInfo> {
141-
const payload = await verifyJwt(token, this.deps.jwtSecret, this.deps.issuerUrl);
146+
let payload: Awaited<ReturnType<typeof verifyJwt>>;
147+
try {
148+
payload = await verifyJwt(token, this.deps.jwtSecret, this.deps.issuerUrl);
149+
} catch (err) {
150+
const mapped = mapJoseErrorToInvalidToken(err);
151+
if (mapped) {
152+
// Explicitly record so the canonical log surfaces the real jose cause
153+
// instead of the PR #392 `UnrecordedError` safety net fallback.
154+
getCurrentRecorder()?.recordError({
155+
source: 'auth',
156+
status_code: 401,
157+
error_type: 'invalid_token',
158+
chain: makeErrorChain(
159+
err instanceof Error ? err.name : 'UnknownJoseError',
160+
mapped.message,
161+
),
162+
});
163+
throw mapped;
164+
}
165+
throw err;
166+
}
142167
return {
143168
token,
144169
clientId: payload.client_id,
@@ -186,3 +211,23 @@ export class FreeeOAuthProvider implements OAuthServerProvider {
186211
};
187212
}
188213
}
214+
215+
// jose throws its own error classes that the MCP SDK bearerAuth middleware
216+
// does not recognize, so we translate the common token-validity failures into
217+
// `InvalidTokenError` to produce a spec-compliant 401 + WWW-Authenticate
218+
// response. See issue #394.
219+
function mapJoseErrorToInvalidToken(err: unknown): InvalidTokenError | null {
220+
if (err instanceof joseErrors.JWTExpired) {
221+
return new InvalidTokenError('Token has expired');
222+
}
223+
if (err instanceof joseErrors.JWSSignatureVerificationFailed) {
224+
return new InvalidTokenError('Invalid token signature');
225+
}
226+
if (err instanceof joseErrors.JWTInvalid || err instanceof joseErrors.JWSInvalid) {
227+
return new InvalidTokenError('Malformed token');
228+
}
229+
if (err instanceof joseErrors.JWTClaimValidationFailed) {
230+
return new InvalidTokenError(`Invalid token claim: ${err.message}`);
231+
}
232+
return null;
233+
}

0 commit comments

Comments
 (0)