[auth] Adjust scope management to line up with SEP-835 (#1133)
Co-authored-by: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com>
This commit is contained in:
parent
7e44b10401
commit
2e67eb552e
4 changed files with 210 additions and 10 deletions
|
|
@ -2174,6 +2174,135 @@ describe('OAuth Authorization', () => {
|
|||
expect(body.get('refresh_token')).toBe('refresh123');
|
||||
});
|
||||
|
||||
it('uses scopes_supported from PRM when scope is not provided', async () => {
|
||||
// Mock PRM with scopes_supported
|
||||
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: 'https://api.example.com/',
|
||||
authorization_servers: ['https://auth.example.com'],
|
||||
scopes_supported: ['mcp:read', 'mcp:write', 'mcp:admin']
|
||||
})
|
||||
});
|
||||
} 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',
|
||||
registration_endpoint: 'https://auth.example.com/register',
|
||||
response_types_supported: ['code'],
|
||||
code_challenge_methods_supported: ['S256']
|
||||
})
|
||||
});
|
||||
} else if (urlString.includes('/register')) {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
client_id: 'test-client-id',
|
||||
client_secret: 'test-client-secret',
|
||||
redirect_uris: ['http://localhost:3000/callback'],
|
||||
client_name: 'Test Client'
|
||||
})
|
||||
});
|
||||
}
|
||||
|
||||
return Promise.resolve({ ok: false, status: 404 });
|
||||
});
|
||||
|
||||
// Mock provider methods - no scope in clientMetadata
|
||||
(mockProvider.clientInformation as Mock).mockResolvedValue(undefined);
|
||||
(mockProvider.tokens as Mock).mockResolvedValue(undefined);
|
||||
mockProvider.saveClientInformation = vi.fn();
|
||||
(mockProvider.saveCodeVerifier as Mock).mockResolvedValue(undefined);
|
||||
(mockProvider.redirectToAuthorization as Mock).mockResolvedValue(undefined);
|
||||
|
||||
// Call auth without scope parameter
|
||||
const result = await auth(mockProvider, {
|
||||
serverUrl: 'https://api.example.com/'
|
||||
});
|
||||
|
||||
expect(result).toBe('REDIRECT');
|
||||
|
||||
// Verify the authorization URL includes the scopes from PRM
|
||||
const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0];
|
||||
const authUrl: URL = redirectCall[0];
|
||||
expect(authUrl.searchParams.get('scope')).toBe('mcp:read mcp:write mcp:admin');
|
||||
});
|
||||
|
||||
it('prefers explicit scope parameter over scopes_supported from PRM', async () => {
|
||||
// Mock PRM with scopes_supported
|
||||
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: 'https://api.example.com/',
|
||||
authorization_servers: ['https://auth.example.com'],
|
||||
scopes_supported: ['mcp:read', 'mcp:write', 'mcp:admin']
|
||||
})
|
||||
});
|
||||
} 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',
|
||||
registration_endpoint: 'https://auth.example.com/register',
|
||||
response_types_supported: ['code'],
|
||||
code_challenge_methods_supported: ['S256']
|
||||
})
|
||||
});
|
||||
} else if (urlString.includes('/register')) {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
client_id: 'test-client-id',
|
||||
client_secret: 'test-client-secret',
|
||||
redirect_uris: ['http://localhost:3000/callback'],
|
||||
client_name: 'Test Client'
|
||||
})
|
||||
});
|
||||
}
|
||||
|
||||
return Promise.resolve({ ok: false, status: 404 });
|
||||
});
|
||||
|
||||
// Mock provider methods
|
||||
(mockProvider.clientInformation as Mock).mockResolvedValue(undefined);
|
||||
(mockProvider.tokens as Mock).mockResolvedValue(undefined);
|
||||
mockProvider.saveClientInformation = vi.fn();
|
||||
(mockProvider.saveCodeVerifier as Mock).mockResolvedValue(undefined);
|
||||
(mockProvider.redirectToAuthorization as Mock).mockResolvedValue(undefined);
|
||||
|
||||
// Call auth with explicit scope parameter
|
||||
const result = await auth(mockProvider, {
|
||||
serverUrl: 'https://api.example.com/',
|
||||
scope: 'mcp:read'
|
||||
});
|
||||
|
||||
expect(result).toBe('REDIRECT');
|
||||
|
||||
// Verify the authorization URL uses the explicit scope, not scopes_supported
|
||||
const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0];
|
||||
const authUrl: URL = redirectCall[0];
|
||||
expect(authUrl.searchParams.get('scope')).toBe('mcp:read');
|
||||
});
|
||||
|
||||
it('fetches AS metadata with path from serverUrl when PRM returns external AS', async () => {
|
||||
// Mock PRM discovery that returns an external AS
|
||||
mockFetch.mockImplementation(url => {
|
||||
|
|
|
|||
|
|
@ -447,7 +447,7 @@ async function authInternal(
|
|||
clientInformation,
|
||||
state,
|
||||
redirectUrl: provider.redirectUrl,
|
||||
scope: scope || provider.clientMetadata.scope,
|
||||
scope: scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope,
|
||||
resource
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -316,6 +316,71 @@ describe('requireBearerAuth middleware', () => {
|
|||
expect(nextFunction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('with requiredScopes in WWW-Authenticate header', () => {
|
||||
it('should include scope in WWW-Authenticate header for 401 responses when requiredScopes is provided', async () => {
|
||||
mockRequest.headers = {};
|
||||
|
||||
const middleware = requireBearerAuth({
|
||||
verifier: mockVerifier,
|
||||
requiredScopes: ['read', 'write']
|
||||
});
|
||||
await middleware(mockRequest as Request, mockResponse as Response, nextFunction);
|
||||
|
||||
expect(mockResponse.status).toHaveBeenCalledWith(401);
|
||||
expect(mockResponse.set).toHaveBeenCalledWith(
|
||||
'WWW-Authenticate',
|
||||
'Bearer error="invalid_token", error_description="Missing Authorization header", scope="read write"'
|
||||
);
|
||||
expect(nextFunction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should include scope in WWW-Authenticate header for 403 insufficient scope responses', async () => {
|
||||
const authInfo: AuthInfo = {
|
||||
token: 'valid-token',
|
||||
clientId: 'client-123',
|
||||
scopes: ['read']
|
||||
};
|
||||
mockVerifyAccessToken.mockResolvedValue(authInfo);
|
||||
|
||||
mockRequest.headers = {
|
||||
authorization: 'Bearer valid-token'
|
||||
};
|
||||
|
||||
const middleware = requireBearerAuth({
|
||||
verifier: mockVerifier,
|
||||
requiredScopes: ['read', 'write']
|
||||
});
|
||||
|
||||
await middleware(mockRequest as Request, mockResponse as Response, nextFunction);
|
||||
|
||||
expect(mockResponse.status).toHaveBeenCalledWith(403);
|
||||
expect(mockResponse.set).toHaveBeenCalledWith(
|
||||
'WWW-Authenticate',
|
||||
'Bearer error="insufficient_scope", error_description="Insufficient scope", scope="read write"'
|
||||
);
|
||||
expect(nextFunction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should include both scope and resource_metadata in WWW-Authenticate header when both are provided', async () => {
|
||||
mockRequest.headers = {};
|
||||
|
||||
const resourceMetadataUrl = 'https://api.example.com/.well-known/oauth-protected-resource';
|
||||
const middleware = requireBearerAuth({
|
||||
verifier: mockVerifier,
|
||||
requiredScopes: ['admin'],
|
||||
resourceMetadataUrl
|
||||
});
|
||||
await middleware(mockRequest as Request, mockResponse as Response, nextFunction);
|
||||
|
||||
expect(mockResponse.status).toHaveBeenCalledWith(401);
|
||||
expect(mockResponse.set).toHaveBeenCalledWith(
|
||||
'WWW-Authenticate',
|
||||
`Bearer error="invalid_token", error_description="Missing Authorization header", scope="admin", resource_metadata="${resourceMetadataUrl}"`
|
||||
);
|
||||
expect(nextFunction).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('with resourceMetadataUrl', () => {
|
||||
const resourceMetadataUrl = 'https://api.example.com/.well-known/oauth-protected-resource';
|
||||
|
||||
|
|
@ -416,7 +481,7 @@ describe('requireBearerAuth middleware', () => {
|
|||
expect(mockResponse.status).toHaveBeenCalledWith(403);
|
||||
expect(mockResponse.set).toHaveBeenCalledWith(
|
||||
'WWW-Authenticate',
|
||||
`Bearer error="insufficient_scope", error_description="Insufficient scope", resource_metadata="${resourceMetadataUrl}"`
|
||||
`Bearer error="insufficient_scope", error_description="Insufficient scope", scope="read write", resource_metadata="${resourceMetadataUrl}"`
|
||||
);
|
||||
expect(nextFunction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -71,17 +71,23 @@ export function requireBearerAuth({ verifier, requiredScopes = [], resourceMetad
|
|||
req.auth = authInfo;
|
||||
next();
|
||||
} catch (error) {
|
||||
// Build WWW-Authenticate header parts
|
||||
const buildWwwAuthHeader = (errorCode: string, message: string): string => {
|
||||
let header = `Bearer error="${errorCode}", error_description="${message}"`;
|
||||
if (requiredScopes.length > 0) {
|
||||
header += `, scope="${requiredScopes.join(' ')}"`;
|
||||
}
|
||||
if (resourceMetadataUrl) {
|
||||
header += `, resource_metadata="${resourceMetadataUrl}"`;
|
||||
}
|
||||
return header;
|
||||
};
|
||||
|
||||
if (error instanceof InvalidTokenError) {
|
||||
const wwwAuthValue = resourceMetadataUrl
|
||||
? `Bearer error="${error.errorCode}", error_description="${error.message}", resource_metadata="${resourceMetadataUrl}"`
|
||||
: `Bearer error="${error.errorCode}", error_description="${error.message}"`;
|
||||
res.set('WWW-Authenticate', wwwAuthValue);
|
||||
res.set('WWW-Authenticate', buildWwwAuthHeader(error.errorCode, error.message));
|
||||
res.status(401).json(error.toResponseObject());
|
||||
} else if (error instanceof InsufficientScopeError) {
|
||||
const wwwAuthValue = resourceMetadataUrl
|
||||
? `Bearer error="${error.errorCode}", error_description="${error.message}", resource_metadata="${resourceMetadataUrl}"`
|
||||
: `Bearer error="${error.errorCode}", error_description="${error.message}"`;
|
||||
res.set('WWW-Authenticate', wwwAuthValue);
|
||||
res.set('WWW-Authenticate', buildWwwAuthHeader(error.errorCode, error.message));
|
||||
res.status(403).json(error.toResponseObject());
|
||||
} else if (error instanceof ServerError) {
|
||||
res.status(500).json(error.toResponseObject());
|
||||
|
|
|
|||
Loading…
Reference in a new issue