Merge branch 'main' into fix/skip_validate_if_error

This commit is contained in:
ihrpr 2025-06-24 17:12:34 +01:00
commit 33e03f499e
No known key found for this signature in database
14 changed files with 1150 additions and 97 deletions

View file

@ -276,6 +276,34 @@ describe('Authorization Handler', () => {
});
});
describe('Resource parameter validation', () => {
it('propagates resource parameter', async () => {
const mockProviderWithResource = jest.spyOn(mockProvider, 'authorize');
const response = await supertest(app)
.get('/authorize')
.query({
client_id: 'valid-client',
redirect_uri: 'https://example.com/callback',
response_type: 'code',
code_challenge: 'challenge123',
code_challenge_method: 'S256',
resource: 'https://api.example.com/resource'
});
expect(response.status).toBe(302);
expect(mockProviderWithResource).toHaveBeenCalledWith(
validClient,
expect.objectContaining({
resource: new URL('https://api.example.com/resource'),
redirectUri: 'https://example.com/callback',
codeChallenge: 'challenge123'
}),
expect.any(Object)
);
});
});
describe('Successful authorization', () => {
it('handles successful authorization with all parameters', async () => {
const response = await supertest(app)

View file

@ -35,6 +35,7 @@ const RequestAuthorizationParamsSchema = z.object({
code_challenge_method: z.literal("S256"),
scope: z.string().optional(),
state: z.string().optional(),
resource: z.string().url().optional(),
});
export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: AuthorizationHandlerOptions): RequestHandler {
@ -98,7 +99,6 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
const status = error instanceof ServerError ? 500 : 400;
res.status(status).json(error.toResponseObject());
} else {
console.error("Unexpected error looking up client:", error);
const serverError = new ServerError("Internal Server Error");
res.status(500).json(serverError.toResponseObject());
}
@ -115,7 +115,7 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
throw new InvalidRequestError(parseResult.error.message);
}
const { scope, code_challenge } = parseResult.data;
const { scope, code_challenge, resource } = parseResult.data;
state = parseResult.data.state;
// Validate scopes
@ -138,13 +138,13 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
scopes: requestedScopes,
redirectUri: redirect_uri,
codeChallenge: code_challenge,
resource: resource ? new URL(resource) : undefined,
}, res);
} catch (error) {
// Post-redirect errors - redirect with error parameters
if (error instanceof OAuthError) {
res.redirect(302, createErrorRedirect(redirect_uri, error, state));
} else {
console.error("Unexpected error during authorization:", error);
const serverError = new ServerError("Internal Server Error");
res.redirect(302, createErrorRedirect(redirect_uri, serverError, state));
}

View file

@ -104,7 +104,6 @@ export function clientRegistrationHandler({
const status = error instanceof ServerError ? 500 : 400;
res.status(status).json(error.toResponseObject());
} else {
console.error("Unexpected error registering client:", error);
const serverError = new ServerError("Internal Server Error");
res.status(500).json(serverError.toResponseObject());
}

View file

@ -9,7 +9,7 @@ import {
InvalidRequestError,
ServerError,
TooManyRequestsError,
OAuthError
OAuthError,
} from "../errors.js";
export type RevocationHandlerOptions = {
@ -21,7 +21,10 @@ export type RevocationHandlerOptions = {
rateLimit?: Partial<RateLimitOptions> | false;
};
export function revocationHandler({ provider, rateLimit: rateLimitConfig }: RevocationHandlerOptions): RequestHandler {
export function revocationHandler({
provider,
rateLimit: rateLimitConfig,
}: RevocationHandlerOptions): RequestHandler {
if (!provider.revokeToken) {
throw new Error("Auth provider does not support revoking tokens");
}
@ -37,21 +40,25 @@ export function revocationHandler({ provider, rateLimit: rateLimitConfig }: Revo
// Apply rate limiting unless explicitly disabled
if (rateLimitConfig !== false) {
router.use(rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 50, // 50 requests per windowMs
standardHeaders: true,
legacyHeaders: false,
message: new TooManyRequestsError('You have exceeded the rate limit for token revocation requests').toResponseObject(),
...rateLimitConfig
}));
router.use(
rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 50, // 50 requests per windowMs
standardHeaders: true,
legacyHeaders: false,
message: new TooManyRequestsError(
"You have exceeded the rate limit for token revocation requests"
).toResponseObject(),
...rateLimitConfig,
})
);
}
// Authenticate and extract client details
router.use(authenticateClient({ clientsStore: provider.clientsStore }));
router.post("/", async (req, res) => {
res.setHeader('Cache-Control', 'no-store');
res.setHeader("Cache-Control", "no-store");
try {
const parseResult = OAuthTokenRevocationRequestSchema.safeParse(req.body);
@ -62,7 +69,6 @@ export function revocationHandler({ provider, rateLimit: rateLimitConfig }: Revo
const client = req.client;
if (!client) {
// This should never happen
console.error("Missing client information after authentication");
throw new ServerError("Internal Server Error");
}
@ -73,7 +79,6 @@ export function revocationHandler({ provider, rateLimit: rateLimitConfig }: Revo
const status = error instanceof ServerError ? 500 : 400;
res.status(status).json(error.toResponseObject());
} else {
console.error("Unexpected error revoking token:", error);
const serverError = new ServerError("Internal Server Error");
res.status(500).json(serverError.toResponseObject());
}

View file

@ -264,12 +264,14 @@ describe('Token Handler', () => {
});
it('returns tokens for valid code exchange', async () => {
const mockExchangeCode = jest.spyOn(mockProvider, 'exchangeAuthorizationCode');
const response = await supertest(app)
.post('/token')
.type('form')
.send({
client_id: 'valid-client',
client_secret: 'valid-secret',
resource: 'https://api.example.com/resource',
grant_type: 'authorization_code',
code: 'valid_code',
code_verifier: 'valid_verifier'
@ -280,6 +282,13 @@ describe('Token Handler', () => {
expect(response.body.token_type).toBe('bearer');
expect(response.body.expires_in).toBe(3600);
expect(response.body.refresh_token).toBe('mock_refresh_token');
expect(mockExchangeCode).toHaveBeenCalledWith(
validClient,
'valid_code',
undefined, // code_verifier is undefined after PKCE validation
undefined, // redirect_uri
new URL('https://api.example.com/resource') // resource parameter
);
});
it('passes through code verifier when using proxy provider', async () => {
@ -440,12 +449,14 @@ describe('Token Handler', () => {
});
it('returns new tokens for valid refresh token', async () => {
const mockExchangeRefresh = jest.spyOn(mockProvider, 'exchangeRefreshToken');
const response = await supertest(app)
.post('/token')
.type('form')
.send({
client_id: 'valid-client',
client_secret: 'valid-secret',
resource: 'https://api.example.com/resource',
grant_type: 'refresh_token',
refresh_token: 'valid_refresh_token'
});
@ -455,6 +466,12 @@ describe('Token Handler', () => {
expect(response.body.token_type).toBe('bearer');
expect(response.body.expires_in).toBe(3600);
expect(response.body.refresh_token).toBe('new_mock_refresh_token');
expect(mockExchangeRefresh).toHaveBeenCalledWith(
validClient,
'valid_refresh_token',
undefined, // scopes
new URL('https://api.example.com/resource') // resource parameter
);
});
it('respects requested scopes on refresh', async () => {

View file

@ -32,11 +32,13 @@ const AuthorizationCodeGrantSchema = z.object({
code: z.string(),
code_verifier: z.string(),
redirect_uri: z.string().optional(),
resource: z.string().url().optional(),
});
const RefreshTokenGrantSchema = z.object({
refresh_token: z.string(),
scope: z.string().optional(),
resource: z.string().url().optional(),
});
export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHandlerOptions): RequestHandler {
@ -78,7 +80,6 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
const client = req.client;
if (!client) {
// This should never happen
console.error("Missing client information after authentication");
throw new ServerError("Internal Server Error");
}
@ -89,7 +90,7 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
throw new InvalidRequestError(parseResult.error.message);
}
const { code, code_verifier, redirect_uri } = parseResult.data;
const { code, code_verifier, redirect_uri, resource } = parseResult.data;
const skipLocalPkceValidation = provider.skipLocalPkceValidation;
@ -107,7 +108,8 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
client,
code,
skipLocalPkceValidation ? code_verifier : undefined,
redirect_uri
redirect_uri,
resource ? new URL(resource) : undefined
);
res.status(200).json(tokens);
break;
@ -119,10 +121,10 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
throw new InvalidRequestError(parseResult.error.message);
}
const { refresh_token, scope } = parseResult.data;
const { refresh_token, scope, resource } = parseResult.data;
const scopes = scope?.split(" ");
const tokens = await provider.exchangeRefreshToken(client, refresh_token, scopes);
const tokens = await provider.exchangeRefreshToken(client, refresh_token, scopes, resource ? new URL(resource) : undefined);
res.status(200).json(tokens);
break;
}
@ -140,7 +142,6 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
const status = error instanceof ServerError ? 500 : 400;
res.status(status).json(error.toResponseObject());
} else {
console.error("Unexpected error exchanging token:", error);
const serverError = new ServerError("Internal Server Error");
res.status(500).json(serverError.toResponseObject());
}

View file

@ -88,7 +88,6 @@ export function requireBearerAuth({ verifier, requiredScopes = [], resourceMetad
} else if (error instanceof OAuthError) {
res.status(400).json(error.toResponseObject());
} else {
console.error("Unexpected error authenticating bearer token:", error);
const serverError = new ServerError("Internal Server Error");
res.status(500).json(serverError.toResponseObject());
}

View file

@ -64,7 +64,6 @@ export function authenticateClient({ clientsStore }: ClientAuthenticationMiddlew
const status = error instanceof ServerError ? 500 : 400;
res.status(status).json(error.toResponseObject());
} else {
console.error("Unexpected error authenticating client:", error);
const serverError = new ServerError("Internal Server Error");
res.status(500).json(serverError.toResponseObject());
}

View file

@ -8,6 +8,7 @@ export type AuthorizationParams = {
scopes?: string[];
codeChallenge: string;
redirectUri: string;
resource?: URL;
};
/**
@ -40,13 +41,14 @@ export interface OAuthServerProvider {
client: OAuthClientInformationFull,
authorizationCode: string,
codeVerifier?: string,
redirectUri?: string
redirectUri?: string,
resource?: URL
): Promise<OAuthTokens>;
/**
* Exchanges a refresh token for an access token.
*/
exchangeRefreshToken(client: OAuthClientInformationFull, refreshToken: string, scopes?: string[]): Promise<OAuthTokens>;
exchangeRefreshToken(client: OAuthClientInformationFull, refreshToken: string, scopes?: string[], resource?: URL): Promise<OAuthTokens>;
/**
* Verifies an access token and returns information about it.

View file

@ -88,6 +88,7 @@ describe("Proxy OAuth Server Provider", () => {
codeChallenge: "test-challenge",
state: "test-state",
scopes: ["read", "write"],
resource: new URL('https://api.example.com/resource'),
},
mockResponse
);
@ -100,6 +101,7 @@ describe("Proxy OAuth Server Provider", () => {
expectedUrl.searchParams.set("code_challenge_method", "S256");
expectedUrl.searchParams.set("state", "test-state");
expectedUrl.searchParams.set("scope", "read write");
expectedUrl.searchParams.set('resource', 'https://api.example.com/resource');
expect(mockResponse.redirect).toHaveBeenCalledWith(expectedUrl.toString());
});
@ -164,6 +166,41 @@ describe("Proxy OAuth Server Provider", () => {
expect(tokens).toEqual(mockTokenResponse);
});
it('includes resource parameter in authorization code exchange', async () => {
const tokens = await provider.exchangeAuthorizationCode(
validClient,
'test-code',
'test-verifier',
'https://example.com/callback',
new URL('https://api.example.com/resource')
);
expect(global.fetch).toHaveBeenCalledWith(
'https://auth.example.com/token',
expect.objectContaining({
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
},
body: expect.stringContaining('resource=' + encodeURIComponent('https://api.example.com/resource'))
})
);
expect(tokens).toEqual(mockTokenResponse);
});
it('handles authorization code exchange without resource parameter', async () => {
const tokens = await provider.exchangeAuthorizationCode(
validClient,
'test-code',
'test-verifier'
);
const fetchCall = (global.fetch as jest.Mock).mock.calls[0];
const body = fetchCall[1].body as string;
expect(body).not.toContain('resource=');
expect(tokens).toEqual(mockTokenResponse);
});
it("exchanges refresh token for new tokens", async () => {
const tokens = await provider.exchangeRefreshToken(
validClient,
@ -184,6 +221,26 @@ describe("Proxy OAuth Server Provider", () => {
expect(tokens).toEqual(mockTokenResponse);
});
it('includes resource parameter in refresh token exchange', async () => {
const tokens = await provider.exchangeRefreshToken(
validClient,
'test-refresh-token',
['read', 'write'],
new URL('https://api.example.com/resource')
);
expect(global.fetch).toHaveBeenCalledWith(
'https://auth.example.com/token',
expect.objectContaining({
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
},
body: expect.stringContaining('resource=' + encodeURIComponent('https://api.example.com/resource'))
})
);
expect(tokens).toEqual(mockTokenResponse);
});
});
describe("client registration", () => {

View file

@ -134,6 +134,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
// Add optional standard OAuth parameters
if (params.state) searchParams.set("state", params.state);
if (params.scopes?.length) searchParams.set("scope", params.scopes.join(" "));
if (params.resource) searchParams.set("resource", params.resource.href);
targetUrl.search = searchParams.toString();
res.redirect(targetUrl.toString());
@ -152,7 +153,8 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
client: OAuthClientInformationFull,
authorizationCode: string,
codeVerifier?: string,
redirectUri?: string
redirectUri?: string,
resource?: URL
): Promise<OAuthTokens> {
const params = new URLSearchParams({
grant_type: "authorization_code",
@ -172,6 +174,10 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
params.append("redirect_uri", redirectUri);
}
if (resource) {
params.append("resource", resource.href);
}
const response = await fetch(this._endpoints.tokenUrl, {
method: "POST",
headers: {
@ -192,7 +198,8 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
async exchangeRefreshToken(
client: OAuthClientInformationFull,
refreshToken: string,
scopes?: string[]
scopes?: string[],
resource?: URL
): Promise<OAuthTokens> {
const params = new URLSearchParams({
@ -209,6 +216,10 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
params.set("scope", scopes.join(" "));
}
if (resource) {
params.set("resource", resource.href);
}
const response = await fetch(this._endpoints.tokenUrl, {
method: "POST",
headers: {

View file

@ -22,6 +22,12 @@ export interface AuthInfo {
*/
expiresAt?: number;
/**
* The RFC 8707 resource server identifier for which this token is valid.
* If set, this MUST match the MCP server's resource identifier (minus hash fragment).
*/
resource?: URL;
/**
* Additional data associated with the token.
* This field should be used for any additional data that needs to be attached to the auth info.

View file

@ -505,7 +505,7 @@ test("should reject elicitation response with invalid data", async () => {
).rejects.toThrow(/does not match requested schema/);
});
test("should allow elicitation decline and cancel without validation", async () => {
test("should allow elicitation reject and cancel without validation", async () => {
const server = new Server(
{
name: "test server",
@ -524,7 +524,7 @@ test("should allow elicitation decline and cancel without validation", async ()
const client = new Client(
{
name: "test client",
name: "test client",
version: "1.0",
},
{
@ -538,7 +538,7 @@ test("should allow elicitation decline and cancel without validation", async ()
client.setRequestHandler(ElicitRequestSchema, (request) => {
requestCount++;
if (requestCount === 1) {
return { action: "decline" };
return { action: "reject" };
} else {
return { action: "cancel" };
}
@ -559,14 +559,14 @@ test("should allow elicitation decline and cancel without validation", async ()
required: ["name"],
};
// Test decline - should not validate
// Test reject - should not validate
await expect(
server.elicitInput({
message: "Please provide your name",
requestedSchema: schema,
}),
).resolves.toEqual({
action: "decline",
action: "reject",
});
// Test cancel - should not validate

View file

@ -4219,7 +4219,7 @@ describe("elicitInput()", () => {
// Mock availability check to return false
checkAvailability.mockResolvedValue(false);
// Set up client to decline alternative date checking
// Set up client to reject alternative date checking
client.setRequestHandler(ElicitRequestSchema, async () => {
return {
action: "accept",