Skip to content

Commit 3ab1e66

Browse files
committed
fix: address PR feedback and CI failures in TypeScript SDK
1 parent 1a833db commit 3ab1e66

7 files changed

Lines changed: 36 additions & 33 deletions

File tree

docs/auth.md

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,16 @@ const server = new McpServer({
2424
name: "my-authenticated-server",
2525
version: "1.0.0",
2626
}, {
27-
authenticator: new BearerTokenAuthenticator({
28-
validate: async (token) => {
29-
// Validate the token (e.g., verify with an OAuth provider)
30-
if (token === "valid-token") {
31-
return {
32-
name: "john_doe",
33-
scopes: ["read:resources", "execute:tools"]
34-
};
35-
}
36-
return undefined; // Invalid token
27+
authenticator: new BearerTokenAuthenticator(async (token) => {
28+
// Validate the token (e.g., verify with an OAuth provider)
29+
if (token === "valid-token") {
30+
return {
31+
token,
32+
clientId: "john_doe",
33+
scopes: ["read:resources", "execute:tools"]
34+
};
3735
}
36+
return undefined; // Invalid token
3837
})
3938
});
4039
```

docs/server.md

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -453,11 +453,9 @@ Quick example:
453453

454454
```ts
455455
const server = new McpServer({ name: 'my-server', version: '1.0.0' }, {
456-
authenticator: new BearerTokenAuthenticator({
457-
validate: async (token) => {
458-
if (token === 'secret') return { name: 'admin', scopes: ['all'] };
459-
return undefined;
460-
}
456+
authenticator: new BearerTokenAuthenticator(async (token) => {
457+
if (token === 'secret') return { token, clientId: 'admin', scopes: ['all'] };
458+
return undefined;
461459
})
462460
});
463461

packages/core/src/types/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ export enum ProtocolErrorCode {
233233
InternalError = -32_603,
234234

235235
// MCP-specific error codes
236+
Unauthorized = 401,
237+
Forbidden = 403,
236238
ResourceNotFound = -32_002,
237239
UrlElicitationRequired = -32_042
238240
}

packages/middleware/express/src/middleware/auth.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Request, Response, NextFunction } from 'express';
1+
import { Request, Response, NextFunction, RequestHandler } from 'express';
22
import { Authenticator, AuthInfo } from '@modelcontextprotocol/server';
33

44
/**
@@ -23,13 +23,11 @@ export interface AuthMiddlewareOptions {
2323
*
2424
* @example
2525
* ```ts
26-
* const authenticator = new BearerTokenAuthenticator({
27-
* validate: async (token) => ({ name: 'user', scopes: ['read'] })
28-
* });
26+
* const authenticator = new BearerTokenAuthenticator((token) => Promise.resolve({ token, clientId: 'user', scopes: ['read'] }));
2927
* app.use(auth({ authenticator }));
3028
* ```
3129
*/
32-
export function auth(options: AuthMiddlewareOptions) {
30+
export function auth(options: AuthMiddlewareOptions): RequestHandler {
3331
return async (req: Request & { auth?: AuthInfo }, res: Response, next: NextFunction) => {
3432
try {
3533
const headers: Record<string, string> = {};
@@ -53,6 +51,7 @@ export function auth(options: AuthMiddlewareOptions) {
5351
// If authentication fails, we let the MCP server handle it later,
5452
// or the developer can choose to reject here.
5553
// By default, we just proceed to allow the MCP server to decide (e.g., if auth is optional).
54+
console.error('[MCP Express Auth Middleware] Authentication failed:', error);
5655
next();
5756
}
5857
};

packages/server/src/server/mcp.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ export class McpServer {
211211
// Authorization check
212212
if (tool.scopes && tool.scopes.length > 0) {
213213
if (!ctx.http?.authInfo || !Authorizer.isAuthorized(ctx.http.authInfo, tool.scopes)) {
214-
throw new ProtocolError(ProtocolErrorCode.InvalidRequest, 'Forbidden');
214+
throw new ProtocolError(ProtocolErrorCode.Forbidden, 'Forbidden');
215215
}
216216
}
217217

@@ -229,10 +229,13 @@ export class McpServer {
229229
return result;
230230
} catch (error) {
231231
if (error instanceof ProtocolError) {
232-
if (error.message.includes('Forbidden') || error.message.includes('Unauthorized')) {
232+
if (
233+
error.code === ProtocolErrorCode.Forbidden ||
234+
error.code === ProtocolErrorCode.Unauthorized ||
235+
error.code === ProtocolErrorCode.UrlElicitationRequired
236+
) {
233237
throw error;
234238
}
235-
throw error;
236239
}
237240
return this.createToolError(error instanceof Error ? error.message : String(error));
238241
}

packages/server/src/server/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ export class Server extends Protocol<ServerContext> {
180180
});
181181

182182
if (!authInfo) {
183-
throw new ProtocolError(ProtocolErrorCode.InvalidRequest, 'Unauthorized');
183+
throw new ProtocolError(ProtocolErrorCode.Unauthorized, 'Unauthorized');
184184
}
185185

186186
// Inject authInfo into extra for buildContext

packages/server/src/server/streamableHttp.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -824,19 +824,23 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
824824
}));
825825
} catch (error) {
826826
if (error instanceof ProtocolError) {
827-
const message = error.message;
828-
if (message.includes('Unauthorized')) {
827+
if (error.code === ProtocolErrorCode.Unauthorized) {
829828
const response = this.createJsonErrorResponse(401, error.code, 'Unauthorized', { headers: { 'WWW-Authenticate': 'Bearer' } });
830829
this._streamMapping.delete(streamId);
831830
return response;
832831
}
833-
if (message.includes('Forbidden')) {
834-
const response = this.createJsonErrorResponse(403, error.code, message);
832+
if (error.code === ProtocolErrorCode.Forbidden) {
833+
const response = this.createJsonErrorResponse(403, error.code, error.message);
835834
this._streamMapping.delete(streamId);
836835
return response;
837836
}
837+
if (error.code === ProtocolErrorCode.UrlElicitationRequired) {
838+
throw error;
839+
}
838840
}
839841
console.error('Transport caught error in onmessage:', error);
842+
// Standard tools should return a CallToolResult with isError: true.
843+
// For onmessage we only rethrow auth-related errors and UrlElicitationRequired.
840844
throw error;
841845
}
842846
}
@@ -846,13 +850,11 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
846850
return new Response(readable, { status: 200, headers });
847851
} catch (error) {
848852
if (error instanceof ProtocolError) {
849-
const message = error.message;
850-
if (message.includes('Unauthorized')) {
853+
if (error.code === ProtocolErrorCode.Unauthorized) {
851854
return this.createJsonErrorResponse(401, error.code, 'Unauthorized', { headers: { 'WWW-Authenticate': 'Bearer' } });
852855
}
853-
if (message.includes('Forbidden')) {
854-
console.log('[Transport] Mapping Forbidden to 403');
855-
return this.createJsonErrorResponse(403, error.code, message);
856+
if (error.code === ProtocolErrorCode.Forbidden) {
857+
return this.createJsonErrorResponse(403, error.code, error.message);
856858
}
857859
}
858860
// return JSON-RPC formatted error

0 commit comments

Comments
 (0)