Merge branch 'main' into main
This commit is contained in:
commit
d897ee8718
14 changed files with 200 additions and 49 deletions
|
|
@ -15,5 +15,12 @@ export default tseslint.config(
|
|||
{ "argsIgnorePattern": "^_" }
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
files: ["src/client/**/*.ts", "src/server/**/*.ts"],
|
||||
ignores: ["**/*.test.ts"],
|
||||
rules: {
|
||||
"no-console": "error"
|
||||
}
|
||||
}
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1041,9 +1041,70 @@ describe("OAuth Authorization", () => {
|
|||
|
||||
// Verify custom validation method was called
|
||||
expect(mockValidateResourceURL).toHaveBeenCalledWith(
|
||||
"https://api.example.com/mcp-server",
|
||||
new URL("https://api.example.com/mcp-server"),
|
||||
"https://different-resource.example.com/mcp-server"
|
||||
);
|
||||
});
|
||||
|
||||
it("uses prefix of server URL from PRM resource as resource parameter", async () => {
|
||||
// Mock successful metadata discovery with resource URL that is a prefix of requested URL
|
||||
mockFetch.mockImplementation((url) => {
|
||||
const urlString = url.toString();
|
||||
|
||||
if (urlString.includes("/.well-known/oauth-protected-resource")) {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
// Resource is a prefix of the requested server URL
|
||||
resource: "https://api.example.com/",
|
||||
authorization_servers: ["https://auth.example.com"],
|
||||
}),
|
||||
});
|
||||
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
issuer: "https://auth.example.com",
|
||||
authorization_endpoint: "https://auth.example.com/authorize",
|
||||
token_endpoint: "https://auth.example.com/token",
|
||||
response_types_supported: ["code"],
|
||||
code_challenge_methods_supported: ["S256"],
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
return Promise.resolve({ ok: false, status: 404 });
|
||||
});
|
||||
|
||||
// Mock provider methods
|
||||
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
|
||||
client_id: "test-client",
|
||||
client_secret: "test-secret",
|
||||
});
|
||||
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
|
||||
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
|
||||
(mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);
|
||||
|
||||
// Call auth with a URL that has the resource as prefix
|
||||
const result = await auth(mockProvider, {
|
||||
serverUrl: "https://api.example.com/mcp-server/endpoint",
|
||||
});
|
||||
|
||||
expect(result).toBe("REDIRECT");
|
||||
|
||||
// Verify the authorization URL includes the resource parameter from PRM
|
||||
expect(mockProvider.redirectToAuthorization).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
searchParams: expect.any(URLSearchParams),
|
||||
})
|
||||
);
|
||||
|
||||
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
|
||||
const authUrl: URL = redirectCall[0];
|
||||
// Should use the PRM's resource value, not the full requested URL
|
||||
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import pkceChallenge from "pkce-challenge";
|
|||
import { LATEST_PROTOCOL_VERSION } from "../types.js";
|
||||
import type { OAuthClientMetadata, OAuthClientInformation, OAuthTokens, OAuthMetadata, OAuthClientInformationFull, OAuthProtectedResourceMetadata } from "../shared/auth.js";
|
||||
import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js";
|
||||
import { resourceUrlFromServerUrl } from "../shared/auth-utils.js";
|
||||
import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js";
|
||||
|
||||
/**
|
||||
* Implements an end-to-end OAuth client to be used with one MCP server.
|
||||
|
|
@ -116,8 +116,8 @@ export async function auth(
|
|||
if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) {
|
||||
authorizationServerUrl = resourceMetadata.authorization_servers[0];
|
||||
}
|
||||
} catch (error) {
|
||||
console.warn("Could not load OAuth Protected Resource metadata, falling back to /.well-known/oauth-authorization-server", error)
|
||||
} catch {
|
||||
// Ignore errors and fall back to /.well-known/oauth-authorization-server
|
||||
}
|
||||
|
||||
const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata);
|
||||
|
|
@ -175,8 +175,8 @@ export async function auth(
|
|||
|
||||
await provider.saveTokens(newTokens);
|
||||
return "AUTHORIZED";
|
||||
} catch (error) {
|
||||
console.error("Could not refresh OAuth tokens:", error);
|
||||
} catch {
|
||||
// Could not refresh OAuth tokens
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -197,14 +197,17 @@ export async function auth(
|
|||
return "REDIRECT";
|
||||
}
|
||||
|
||||
async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
|
||||
export async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
|
||||
let resource = resourceUrlFromServerUrl(serverUrl);
|
||||
if (provider.validateResourceURL) {
|
||||
return await provider.validateResourceURL(serverUrl, resourceMetadata?.resource);
|
||||
}
|
||||
|
||||
const resource = resourceUrlFromServerUrl(typeof serverUrl === "string" ? new URL(serverUrl) : serverUrl);
|
||||
if (resourceMetadata && resourceMetadata.resource !== resource.href) {
|
||||
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource}`);
|
||||
return await provider.validateResourceURL(resource, resourceMetadata?.resource);
|
||||
} else if (resourceMetadata) {
|
||||
if (checkResourceAllowed({ requestedResource: resource, configuredResource: resourceMetadata.resource })) {
|
||||
// If the resource mentioned in metadata is valid, prefer it since it is what the server is telling us to request.
|
||||
resource = new URL(resourceMetadata.resource);
|
||||
} else {
|
||||
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource} (or origin)`);
|
||||
}
|
||||
}
|
||||
|
||||
return resource;
|
||||
|
|
@ -222,7 +225,6 @@ export function extractResourceMetadataUrl(res: Response): URL | undefined {
|
|||
|
||||
const [type, scheme] = authenticateHeader.split(' ');
|
||||
if (type.toLowerCase() !== 'bearer' || !scheme) {
|
||||
console.log("Invalid WWW-Authenticate header format, expected 'Bearer'");
|
||||
return undefined;
|
||||
}
|
||||
const regex = /resource_metadata="([^"]*)"/;
|
||||
|
|
@ -235,7 +237,6 @@ export function extractResourceMetadataUrl(res: Response): URL | undefined {
|
|||
try {
|
||||
return new URL(match[1]);
|
||||
} catch {
|
||||
console.log("Invalid resource metadata url: ", match[1]);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -486,8 +486,8 @@ export class Client<
|
|||
try {
|
||||
const validator = this._ajv.compile(tool.outputSchema);
|
||||
this._cachedToolOutputValidators.set(tool.name, validator);
|
||||
} catch (error) {
|
||||
console.warn(`Failed to compile output schema for tool ${tool.name}: ${error}`);
|
||||
} catch {
|
||||
// Ignore schema compilation errors
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -117,23 +117,35 @@ export class SSEClientTransport implements Transport {
|
|||
}
|
||||
|
||||
private _startOrAuth(): Promise<void> {
|
||||
const fetchImpl = (this?._eventSourceInit?.fetch || fetch) as typeof fetch
|
||||
return new Promise((resolve, reject) => {
|
||||
this._eventSource = new EventSource(
|
||||
this._url.href,
|
||||
this._eventSourceInit ?? {
|
||||
fetch: (url, init) => this._commonHeaders().then((headers) => fetch(url, {
|
||||
...init,
|
||||
headers: {
|
||||
...headers,
|
||||
Accept: "text/event-stream"
|
||||
{
|
||||
...this._eventSourceInit,
|
||||
fetch: async (url, init) => {
|
||||
const headers = await this._commonHeaders()
|
||||
const response = await fetchImpl(url, {
|
||||
...init,
|
||||
headers: new Headers({
|
||||
...headers,
|
||||
Accept: "text/event-stream"
|
||||
})
|
||||
})
|
||||
|
||||
if (response.status === 401 && response.headers.has('www-authenticate')) {
|
||||
this._resourceMetadataUrl = extractResourceMetadataUrl(response);
|
||||
}
|
||||
})),
|
||||
|
||||
return response
|
||||
},
|
||||
},
|
||||
);
|
||||
this._abortController = new AbortController();
|
||||
|
||||
this._eventSource.onerror = (event) => {
|
||||
if (event.code === 401 && this._authProvider) {
|
||||
|
||||
this._authThenStart().then(resolve, reject);
|
||||
return;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import { CallToolResult, GetPromptResult, isInitializeRequest, PrimitiveSchemaDe
|
|||
import { InMemoryEventStore } from '../shared/inMemoryEventStore.js';
|
||||
import { setupAuthServer } from './demoInMemoryOAuthProvider.js';
|
||||
import { OAuthMetadata } from 'src/shared/auth.js';
|
||||
import { checkResourceAllowed } from 'src/shared/auth-utils.js';
|
||||
|
||||
// Check for OAuth flag
|
||||
const useOAuth = process.argv.includes('--oauth');
|
||||
|
|
@ -463,7 +464,7 @@ if (useOAuth) {
|
|||
if (!data.aud) {
|
||||
throw new Error(`Resource Indicator (RFC8707) missing`);
|
||||
}
|
||||
if (data.aud !== mcpServerUrl.href) {
|
||||
if (!checkResourceAllowed({ requestedResource: data.aud, configuredResource: mcpServerUrl })) {
|
||||
throw new Error(`Expected resource indicator ${mcpServerUrl}, got: ${data.aud}`);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -99,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());
|
||||
}
|
||||
|
|
@ -146,7 +145,6 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
|
|||
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));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -80,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");
|
||||
}
|
||||
|
||||
|
|
@ -143,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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { resourceUrlFromServerUrl } from './auth-utils.js';
|
||||
import { resourceUrlFromServerUrl, checkResourceAllowed } from './auth-utils.js';
|
||||
|
||||
describe('auth-utils', () => {
|
||||
describe('resourceUrlFromServerUrl', () => {
|
||||
|
|
@ -27,4 +27,35 @@ describe('auth-utils', () => {
|
|||
expect(resourceUrlFromServerUrl(new URL('https://example.com/path/')).href).toBe('https://example.com/path/');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('resourceMatches', () => {
|
||||
it('should match identical URLs', () => {
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/path', configuredResource: 'https://example.com/path' })).toBe(true);
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/', configuredResource: 'https://example.com/' })).toBe(true);
|
||||
});
|
||||
|
||||
it('should not match URLs with different paths', () => {
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/path1', configuredResource: 'https://example.com/path2' })).toBe(false);
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/', configuredResource: 'https://example.com/path' })).toBe(false);
|
||||
});
|
||||
|
||||
it('should not match URLs with different domains', () => {
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/path', configuredResource: 'https://example.org/path' })).toBe(false);
|
||||
});
|
||||
|
||||
it('should not match URLs with different ports', () => {
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com:8080/path', configuredResource: 'https://example.com/path' })).toBe(false);
|
||||
});
|
||||
|
||||
it('should not match URLs where one path is a sub-path of another', () => {
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/mcpxxxx', configuredResource: 'https://example.com/mcp' })).toBe(false);
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/subfolder' })).toBe(false);
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/api/v1', configuredResource: 'https://example.com/api' })).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle trailing slashes vs no trailing slashes', () => {
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/mcp/', configuredResource: 'https://example.com/mcp' })).toBe(true);
|
||||
expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/' })).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,8 +7,48 @@
|
|||
* RFC 8707 section 2 states that resource URIs "MUST NOT include a fragment component".
|
||||
* Keeps everything else unchanged (scheme, domain, port, path, query).
|
||||
*/
|
||||
export function resourceUrlFromServerUrl(url: URL): URL {
|
||||
const resourceURL = new URL(url.href);
|
||||
export function resourceUrlFromServerUrl(url: URL | string ): URL {
|
||||
const resourceURL = typeof url === "string" ? new URL(url) : new URL(url.href);
|
||||
resourceURL.hash = ''; // Remove fragment
|
||||
return resourceURL;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a requested resource URL matches a configured resource URL.
|
||||
* A requested resource matches if it has the same scheme, domain, port,
|
||||
* and its path starts with the configured resource's path.
|
||||
*
|
||||
* @param requestedResource The resource URL being requested
|
||||
* @param configuredResource The resource URL that has been configured
|
||||
* @returns true if the requested resource matches the configured resource, false otherwise
|
||||
*/
|
||||
export function checkResourceAllowed(
|
||||
{ requestedResource, configuredResource }: {
|
||||
requestedResource: URL | string;
|
||||
configuredResource: URL | string
|
||||
}
|
||||
): boolean {
|
||||
const requested = typeof requestedResource === "string" ? new URL(requestedResource) : new URL(requestedResource.href);
|
||||
const configured = typeof configuredResource === "string" ? new URL(configuredResource) : new URL(configuredResource.href);
|
||||
|
||||
// Compare the origin (scheme, domain, and port)
|
||||
if (requested.origin !== configured.origin) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Handle cases like requested=/foo and configured=/foo/
|
||||
if (requested.pathname.length < configured.pathname.length) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check if the requested path starts with the configured path
|
||||
// Ensure both paths end with / for proper comparison
|
||||
// This ensures that if we have paths like "/api" and "/api/users",
|
||||
// we properly detect that "/api/users" is a subpath of "/api"
|
||||
// By adding a trailing slash if missing, we avoid false positives
|
||||
// where paths like "/api123" would incorrectly match "/api"
|
||||
const requestedPath = requested.pathname.endsWith('/') ? requested.pathname : requested.pathname + '/';
|
||||
const configuredPath = configured.pathname.endsWith('/') ? configured.pathname : configured.pathname + '/';
|
||||
|
||||
return requestedPath.startsWith(configuredPath);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue