Skip to content

Commit 9258b0a

Browse files
committed
fix: rename routePrefix to issuerPath and add issuer-mismatch scenario
The previous fix (#152) used routePrefix for the issuer path, conflating two concepts: (1) where OAuth endpoints are mounted and (2) the issuer path component per RFC 8414. This worked for metadata-var2/var3 where both were /tenant1, but broke auth/2025-03-26-oauth-metadata-backcompat where routePrefix was /oauth (endpoint namespacing) but the issuer should be root. Changes: - Rename routePrefix -> issuerPath in createAuthServer. Endpoints are mounted under the issuer path (the real-world multi-tenant model). - Drop the /oauth prefix from march-spec-backcompat entirely. No route collision exists with createServer, and test integrity against fallback-bypass is already guaranteed by expectedSlugs requiring authorization-server-metadata. - Rename authRoutePrefix -> authIssuerPath in discovery-metadata config. - Add issuerOverride option to createAuthServer for negative testing. - Add auth/issuer-mismatch scenario: AS returns a mismatched issuer (https://evil.example.com) and the client is expected to reject per RFC 8414 section 3.3. Skipped in CI for now as the TS SDK reference client does not yet validate issuer. Fixes #140
1 parent cb0f4c3 commit 9258b0a

8 files changed

Lines changed: 150 additions & 20 deletions

File tree

examples/clients/typescript/everything-client.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ registerScenarios(
147147
'auth/token-endpoint-auth-none',
148148
// Resource mismatch (client should error when PRM resource doesn't match)
149149
'auth/resource-mismatch',
150+
// Issuer mismatch (client should error when AS metadata issuer doesn't match, RFC 8414 §3.3)
151+
'auth/issuer-mismatch',
150152
// SEP-2207: Offline access / refresh token guidance (draft)
151153
'auth/offline-access-scope',
152154
'auth/offline-access-not-supported'

src/scenarios/client/auth/discovery-metadata.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ interface MetadataScenarioConfig {
2222
prmLocation: string;
2323
inWwwAuth: boolean;
2424
oauthMetadataLocation: string;
25-
/** Route prefix for the auth server (e.g., '/tenant1') */
26-
authRoutePrefix?: string;
25+
/** Issuer path component for the auth server (e.g., '/tenant1' for multi-tenant) */
26+
authIssuerPath?: string;
2727
/** If true, add a trap for root PRM requests */
2828
trapRootPrm?: boolean;
2929
}
@@ -57,14 +57,14 @@ const SCENARIO_CONFIGS: MetadataScenarioConfig[] = [
5757
prmLocation: '/.well-known/oauth-protected-resource',
5858
inWwwAuth: false,
5959
oauthMetadataLocation: '/.well-known/oauth-authorization-server/tenant1',
60-
authRoutePrefix: '/tenant1'
60+
authIssuerPath: '/tenant1'
6161
},
6262
{
6363
name: 'metadata-var3',
6464
prmLocation: '/custom/metadata/location.json',
6565
inWwwAuth: true,
6666
oauthMetadataLocation: '/tenant1/.well-known/openid-configuration',
67-
authRoutePrefix: '/tenant1'
67+
authIssuerPath: '/tenant1'
6868
}
6969
];
7070

@@ -76,7 +76,7 @@ function createMetadataScenario(config: MetadataScenarioConfig): Scenario {
7676
const server = new ServerLifecycle();
7777
let checks: ConformanceCheck[] = [];
7878

79-
const routePrefix = config.authRoutePrefix || '';
79+
const issuerPath = config.authIssuerPath || '';
8080
const isOpenIdConfiguration = config.oauthMetadataLocation.includes(
8181
'openid-configuration'
8282
);
@@ -100,11 +100,11 @@ function createMetadataScenario(config: MetadataScenarioConfig): Scenario {
100100
const authApp = createAuthServer(checks, authServer.getUrl, {
101101
metadataPath: config.oauthMetadataLocation,
102102
isOpenIdConfiguration,
103-
...(routePrefix && { routePrefix })
103+
...(issuerPath && { issuerPath })
104104
});
105105

106106
// If path-based OAuth metadata, trap root requests
107-
if (routePrefix) {
107+
if (issuerPath) {
108108
authApp.get('/.well-known/oauth-authorization-server', (req, res) => {
109109
checks.push({
110110
id: 'authorization-server-metadata-wrong-path',
@@ -127,8 +127,8 @@ function createMetadataScenario(config: MetadataScenarioConfig): Scenario {
127127

128128
await authServer.start(authApp);
129129

130-
const getAuthServerUrl = routePrefix
131-
? () => `${authServer.getUrl()}${routePrefix}`
130+
const getAuthServerUrl = issuerPath
131+
? () => `${authServer.getUrl()}${issuerPath}`
132132
: authServer.getUrl;
133133

134134
const app = createServer(checks, server.getUrl, getAuthServerUrl, {

src/scenarios/client/auth/helpers/createAuthServer.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,18 @@ export interface AuthServerOptions {
3333
metadataPath?: string;
3434
isOpenIdConfiguration?: boolean;
3535
loggingEnabled?: boolean;
36-
routePrefix?: string;
36+
/**
37+
* Path component of the issuer identifier (e.g., '/tenant1' for multi-tenant).
38+
* Per RFC 8414, this must match the path used to construct the metadata URL.
39+
* OAuth endpoints (/authorize, /token, /register) are mounted under this path.
40+
*/
41+
issuerPath?: string;
42+
/**
43+
* Override the issuer value in the metadata response. For negative testing
44+
* of RFC 8414 §3.3 issuer validation — clients MUST reject when the issuer
45+
* in the response doesn't match the one used to construct the metadata URL.
46+
*/
47+
issuerOverride?: string;
3748
scopesSupported?: string[];
3849
grantTypesSupported?: string[];
3950
tokenEndpointAuthMethodsSupported?: string[];
@@ -78,7 +89,8 @@ export function createAuthServer(
7889
metadataPath = '/.well-known/oauth-authorization-server',
7990
isOpenIdConfiguration = false,
8091
loggingEnabled = true,
81-
routePrefix = '',
92+
issuerPath = '',
93+
issuerOverride,
8294
scopesSupported,
8395
grantTypesSupported = ['authorization_code', 'refresh_token'],
8496
tokenEndpointAuthMethodsSupported = ['none'],
@@ -98,9 +110,9 @@ export function createAuthServer(
98110
let storedCodeChallenge: string | undefined;
99111

100112
const authRoutes = {
101-
authorization_endpoint: `${routePrefix}/authorize`,
102-
token_endpoint: `${routePrefix}/token`,
103-
registration_endpoint: `${routePrefix}/register`
113+
authorization_endpoint: `${issuerPath}/authorize`,
114+
token_endpoint: `${issuerPath}/token`,
115+
registration_endpoint: `${issuerPath}/register`
104116
};
105117

106118
const app = express();
@@ -134,7 +146,7 @@ export function createAuthServer(
134146
});
135147

136148
const metadata: any = {
137-
issuer: `${getAuthBaseUrl()}${routePrefix}`,
149+
issuer: issuerOverride ?? `${getAuthBaseUrl()}${issuerPath}`,
138150
authorization_endpoint: `${getAuthBaseUrl()}${authRoutes.authorization_endpoint}`,
139151
token_endpoint: `${getAuthBaseUrl()}${authRoutes.token_endpoint}`,
140152
...(!disableDynamicRegistration && {

src/scenarios/client/auth/index.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,19 @@ beforeAll(() => {
2222
});
2323

2424
const skipScenarios = new Set<string>([
25-
// Add scenarios that should be skipped here
25+
// TS SDK does not yet validate that AS metadata issuer matches the issuer
26+
// used to construct the metadata URL (RFC 8414 §3.3). Unskip once
27+
// typescript-sdk implements validation.
28+
'auth/issuer-mismatch'
2629
]);
2730

2831
const allowClientErrorScenarios = new Set<string>([
2932
// Client is expected to give up (error) after limited retries, but check should pass
3033
'auth/scope-retry-limit',
3134
// Client is expected to error when PRM resource doesn't match server URL
32-
'auth/resource-mismatch'
35+
'auth/resource-mismatch',
36+
// Client is expected to error when AS metadata issuer doesn't match (RFC 8414 §3.3)
37+
'auth/issuer-mismatch'
3338
]);
3439

3540
describe('Client Auth Scenarios', () => {
@@ -68,6 +73,9 @@ describe('Client Back-compat Scenarios', () => {
6873
describe('Client Draft Scenarios', () => {
6974
for (const scenario of draftScenariosList) {
7075
test(`${scenario.name} passes`, async () => {
76+
if (skipScenarios.has(scenario.name)) {
77+
return;
78+
}
7179
const clientFn = getHandler(scenario.name);
7280
if (!clientFn) {
7381
throw new Error(`No handler registered for scenario: ${scenario.name}`);

src/scenarios/client/auth/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
ClientCredentialsBasicScenario
2323
} from './client-credentials';
2424
import { ResourceMismatchScenario } from './resource-mismatch';
25+
import { IssuerMismatchScenario } from './issuer-mismatch';
2526
import { PreRegistrationScenario } from './pre-registration';
2627
import { CrossAppAccessCompleteFlowScenario } from './cross-app-access';
2728
import {
@@ -60,6 +61,7 @@ export const extensionScenariosList: Scenario[] = [
6061
// Draft scenarios (informational - not scored for tier assessment)
6162
export const draftScenariosList: Scenario[] = [
6263
new ResourceMismatchScenario(),
64+
new IssuerMismatchScenario(),
6365
new OfflineAccessScopeScenario(),
6466
new OfflineAccessNotSupportedScenario()
6567
];
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import type { Scenario, ConformanceCheck } from '../../../types.js';
2+
import { ScenarioUrls, SpecVersion } from '../../../types.js';
3+
import { createAuthServer } from './helpers/createAuthServer.js';
4+
import { createServer } from './helpers/createServer.js';
5+
import { ServerLifecycle } from './helpers/serverLifecycle.js';
6+
import { SpecReferences } from './spec-references.js';
7+
8+
/**
9+
* Scenario: Authorization Server Issuer Mismatch Detection
10+
*
11+
* Tests that clients correctly detect and reject when the Authorization
12+
* Server metadata response contains an `issuer` value that doesn't match
13+
* the issuer identifier used to construct the metadata URL.
14+
*
15+
* Per RFC 8414 §3.3, clients MUST validate that the issuer in the metadata
16+
* response matches the issuer used to construct the well-known metadata URL.
17+
* Failing to do so enables mix-up attacks where a malicious AS impersonates
18+
* another.
19+
*
20+
* Setup:
21+
* - PRM advertises authorization server at http://localhost:<port> (root issuer)
22+
* - Client constructs metadata URL /.well-known/oauth-authorization-server
23+
* - AS responds with issuer: "https://evil.example.com" (mismatch)
24+
*
25+
* Expected behavior:
26+
* - Client should NOT proceed with authorization
27+
* - Client should abort due to issuer mismatch
28+
* - Test passes if client does NOT make an authorization request
29+
*/
30+
export class IssuerMismatchScenario implements Scenario {
31+
name = 'auth/issuer-mismatch';
32+
specVersions: SpecVersion[] = ['draft'];
33+
description =
34+
'Tests that client rejects when AS metadata issuer does not match the issuer used to construct the metadata URL (RFC 8414 §3.3)';
35+
allowClientError = true;
36+
37+
private authServer = new ServerLifecycle();
38+
private server = new ServerLifecycle();
39+
private checks: ConformanceCheck[] = [];
40+
private authorizationRequestMade = false;
41+
42+
async start(): Promise<ScenarioUrls> {
43+
this.checks = [];
44+
this.authorizationRequestMade = false;
45+
46+
const authApp = createAuthServer(this.checks, this.authServer.getUrl, {
47+
// Root issuer: metadata at /.well-known/oauth-authorization-server,
48+
// so the expected issuer is just the base URL. Override it to a
49+
// different origin to trigger the mismatch.
50+
issuerOverride: 'https://evil.example.com',
51+
onAuthorizationRequest: () => {
52+
// If we get here, the client incorrectly proceeded past issuer validation
53+
this.authorizationRequestMade = true;
54+
}
55+
});
56+
await this.authServer.start(authApp);
57+
58+
const app = createServer(
59+
this.checks,
60+
this.server.getUrl,
61+
this.authServer.getUrl,
62+
{
63+
prmPath: '/.well-known/oauth-protected-resource/mcp'
64+
}
65+
);
66+
await this.server.start(app);
67+
68+
return { serverUrl: `${this.server.getUrl()}/mcp` };
69+
}
70+
71+
async stop() {
72+
await this.authServer.stop();
73+
await this.server.stop();
74+
}
75+
76+
getChecks(): ConformanceCheck[] {
77+
const timestamp = new Date().toISOString();
78+
79+
if (!this.checks.some((c) => c.id === 'issuer-mismatch-rejected')) {
80+
const correctlyRejected = !this.authorizationRequestMade;
81+
this.checks.push({
82+
id: 'issuer-mismatch-rejected',
83+
name: 'Client rejects mismatched issuer',
84+
description: correctlyRejected
85+
? 'Client correctly rejected authorization when AS metadata issuer does not match the metadata URL'
86+
: 'Client MUST validate that the issuer in AS metadata matches the issuer used to construct the metadata URL (RFC 8414 §3.3)',
87+
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
88+
timestamp,
89+
specReferences: [SpecReferences.RFC_AUTH_SERVER_METADATA_VALIDATION],
90+
details: {
91+
metadataIssuer: 'https://evil.example.com',
92+
expectedIssuer: this.authServer.getUrl(),
93+
expectedBehavior: 'Client should NOT proceed with authorization',
94+
authorizationRequestMade: this.authorizationRequestMade
95+
}
96+
});
97+
}
98+
99+
return this.checks;
100+
}
101+
}

src/scenarios/client/auth/march-spec-backcompat.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,12 @@ export class Auth20250326OAuthMetadataBackcompatScenario implements Scenario {
1818
this.checks = [];
1919
// Legacy server, so we create the auth server endpoints on the
2020
// same URL as the main server (rather than separating AS / RS).
21+
// Metadata at root well-known → issuer is the root URL (no path).
22+
// Test integrity against fallback-bypass is ensured by expectedSlugs
23+
// requiring 'authorization-server-metadata'.
2124
const authApp = createAuthServer(this.checks, this.server.getUrl, {
2225
// Disable logging since the main server will already have logging enabled
23-
loggingEnabled: false,
24-
// Add a prefix to auth endpoints to avoid being caught by auth fallbacks
25-
routePrefix: '/oauth'
26+
loggingEnabled: false
2627
});
2728
const app = createServer(
2829
this.checks,

src/scenarios/client/auth/spec-references.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ export const SpecReferences: { [key: string]: SpecReference } = {
99
id: 'RFC-8414-metadata-request',
1010
url: 'https://www.rfc-editor.org/rfc/rfc8414.html#section-3.1'
1111
},
12+
RFC_AUTH_SERVER_METADATA_VALIDATION: {
13+
id: 'RFC-8414-metadata-validation',
14+
url: 'https://www.rfc-editor.org/rfc/rfc8414.html#section-3.3'
15+
},
1216
LEGACY_2025_03_26_AUTH_DISCOVERY: {
1317
id: 'MCP-2025-03-26-Authorization-metadata-discovery',
1418
url: 'https://modelcontextprotocol.io/specification/2025-03-26/basic/authorization#server-metadata-discovery'

0 commit comments

Comments
 (0)