fix(client/auth): bind negative-check verdicts to an observed prerequisite (#483)

A negative check that reads only the final verdict scores SUCCESS whenever
the client fails to reach the requirement at all: "declined" and "never got
far enough to decide" are the same observation. auth/resource-mismatch
decided on !authorizationRequestMade alone, so a client that threw before
making a single HTTP request passed it.

Gate each verdict on the prerequisite the harness already records:
prm-pathbased-requested for the PRM resource comparison, and
authorization-server-metadata for the RFC 9207 iss comparisons, which
SEP-2468 conditions on the issuer recorded from the validated metadata
document. When the prerequisite is not observed the requirement was never
exercised, so report it via notTestable() (#248) rather than SUCCESS.

auth/metadata-issuer-mismatch already gated on the metadata fetch but
reported the unreached case as a plain violation; it now reports untestable
too. All six checks record propertyReached and stopReason in details.

Adds an inert client and a vitest case, so the checks are shown to catch
something and not merely to avoid false-positiving.
This commit is contained in:
Abhinav Singh 2026-09-07 00:44:00 +05:30 • committed by GitHub
commit 31c5e7aa55
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 320 additions and 102 deletions

View file

@ -0,0 +1,22 @@
#!/usr/bin/env node
import { runAsCli } from './helpers/cliRunner';
/**
* Broken client that gives up before performing any discovery request.
*
* BUG: it never fetches Protected Resource Metadata, so it never reads — let
* alone validates — the `resource` value the scenario mismatches on purpose.
*
* It exists to pin issue #467. `auth/resource-mismatch` decides its verdict
* from `!authorizationRequestMade` alone, and a client that does nothing at
* all satisfies that verdict, so the check scores SUCCESS for a client that
* cannot possibly have performed the validation under test.
*/
export async function runClient(_serverUrl: string): Promise<void> {
throw new Error(
'inert client: aborted before any discovery request (no PRM fetch, no authorization request)'
);
}
runAsCli(runClient, import.meta.url, 'auth-test-inert <server-url>');

View file

@ -26,6 +26,7 @@ import { runClient as noPkceClient } from '../../../../examples/clients/typescri
import { runClient as reuseCredsClient } from '../../../../examples/clients/typescript/auth-test-reuse-credentials';
import { runClient as noAppTypeClient } from '../../../../examples/clients/typescript/auth-test-no-application-type';
import { runClient as noIssValidationClient } from '../../../../examples/clients/typescript/auth-test';
import { runClient as inertClient } from '../../../../examples/clients/typescript/auth-test-inert';
import { runClient as issNormalizeClient } from '../../../../examples/clients/typescript/auth-test-iss-normalize';
import { runClient as echoScopeClient } from '../../../../examples/clients/typescript/auth-test-echo-scope';
import { runClient as dpopBearerClient } from '../../../../examples/clients/typescript/auth-test-dpop-bearer';
@ -476,3 +477,36 @@ describe('DPoP client nonce-less baseline (SEP-1932)', () => {
expect(count('pkce-verifier-matches-challenge')).toBe(1);
});
});
// Reason-bound negative checks (issue #467).
//
// A negative check that reads only the final verdict scores SUCCESS whenever
// the client fails to reach the requirement at all: "did not proceed" and
// "never got far enough to decide" are the same observation. These tests pin
// that the harness distinguishes them, so a client that cannot have performed
// the validation cannot bank a pass for it.
describe('Reason-bound negative checks (#467)', () => {
test('auth/resource-mismatch: an inert client does not pass by doing nothing', async () => {
// The inert client throws before any discovery request, so it never reads
// the mismatched `resource` it is required to validate. Bound only to the
// verdict (`!authorizationRequestMade`) this scored SUCCESS.
const runner = new InlineClientRunner(inertClient);
const checks = await runClientAgainstScenario(
runner,
'auth/resource-mismatch',
{
allowClientError: true,
expectedFailureSlugs: ['resource-mismatch-rejected']
}
);
const check = checks.find((c) => c.id === 'resource-mismatch-rejected');
expect(check).toBeDefined();
// Reported as untestable (#248), not as a plain violation: the client did
// not break the requirement, it never exercised it.
expect(check?.details?.untestable).toBe(true);
expect(check?.details?.propertyReached).toBe(false);
expect(check?.details?.stopReason).toBe('prm-not-requested');
expect(check?.errorMessage).toMatch(/^Not testable: /);
});
});

View file

@ -6,6 +6,7 @@ import { createServer } from './helpers/createServer.js';
import { ServerLifecycle } from './helpers/serverLifecycle.js';
import { SpecReferences } from './spec-references.js';
import { MockTokenVerifier } from './helpers/mockTokenVerifier.js';
import { untestableCheck } from '../../untestable.js';
const specRefs = [SpecReferences.RFC_9207_ISS_PARAMETER];
const metadataSpecRefs = [
@ -13,6 +14,93 @@ const metadataSpecRefs = [
SpecReferences.MCP_AUTH_DISCOVERY
];
/**
* Reason-bound verdict for the RFC 9207 `iss` rejection checks (issue #467).
*
* `authReached && !tokenRequestMade` is a verdict, not a reason. SEP-2468
* conditions every one of these requirements on the issuer the client recorded
* "from the selected authorization server validated metadata document", so a
* client that never retrieved that document cannot have performed the
* comparison under test — yet it satisfies the verdict, because not reaching
* the token endpoint is exactly what a client that fell over earlier also
* does. Absent the retrieval the requirement was never exercised, which is the
* untestable case (#248) rather than a pass or a violation.
*
* `auth/metadata-issuer-mismatch` in this same file already gates on the
* metadata fetch; the other checks did not. Keeping the policy in one function
* is deliberate: the duplication is what let five of six sites drift apart.
*
* Residual gap, deliberately not papered over: a client that receives the
* redirect and then aborts before the token request for an unrelated reason is
* still indistinguishable from one that rejected on `iss`. Closing that needs
* a signal from inside the client, which a black-box harness does not have.
* What this closes is the "never reached the requirement at all" class.
*/
function issRejectionCheck(opts: {
id: string;
name: string;
passDescription: string;
failDescription: string;
metadataRequested: boolean;
authReached: boolean;
tokenRequestMade: boolean;
observations: Record<string, unknown>;
timestamp: string;
}): ConformanceCheck {
const {
id,
name,
passDescription,
failDescription,
metadataRequested,
authReached,
tokenRequestMade,
observations,
timestamp
} = opts;
const reached = metadataRequested && authReached;
const observed = {
...observations,
metadataRequested,
authReached,
tokenRequestMade
};
if (!reached) {
const reason = !metadataRequested
? 'client never retrieved the authorization server metadata document, so it never recorded the issuer this comparison is made against'
: 'client never reached the authorization endpoint, so it never received an authorization response to validate';
const check = untestableCheck(id, name, failDescription, reason, specRefs);
check.details = {
...check.details,
...observed,
propertyReached: false,
stopReason: !metadataRequested
? 'as-metadata-not-requested'
: 'authorization-endpoint-not-reached'
};
return check;
}
const correctlyRejected = !tokenRequestMade;
return {
id,
name,
description: correctlyRejected ? passDescription : failDescription,
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: specRefs,
details: {
...observed,
propertyReached: true,
stopReason: correctlyRejected
? 'declined-before-token-request'
: 'transmitted-code-to-token-endpoint'
}
};
}
/**
* Scenario: ISS Parameter Supported (positive)
*
@ -217,23 +305,26 @@ export class IssParameterSupportedMissingScenario implements Scenario {
if (
!this.checks.some((c) => c.id === 'sep-2468-client-reject-missing-iss')
) {
const correctlyRejected = this.authReached && !this.tokenRequestMade;
this.checks.push({
id: 'sep-2468-client-reject-missing-iss',
name: 'Client rejects missing iss when required',
description: correctlyRejected
? 'Client correctly rejected authorization response missing required iss parameter'
: 'Client MUST reject authorization response when server advertised iss support but iss is absent from redirect',
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: specRefs,
details: {
serverAdvertisedSupport: true,
issSentInRedirect: false,
this.checks.push(
issRejectionCheck({
id: 'sep-2468-client-reject-missing-iss',
name: 'Client rejects missing iss when required',
passDescription:
'Client correctly rejected authorization response missing required iss parameter',
failDescription:
'Client MUST reject authorization response when server advertised iss support but iss is absent from redirect',
metadataRequested: this.checks.some(
(c) => c.id === 'authorization-server-metadata'
),
authReached: this.authReached,
tokenRequestMade: this.tokenRequestMade
}
});
tokenRequestMade: this.tokenRequestMade,
observations: {
serverAdvertisedSupport: true,
issSentInRedirect: false
},
timestamp
})
);
}
return this.checks;
@ -304,23 +395,26 @@ export class IssParameterWrongIssuerScenario implements Scenario {
if (
!this.checks.some((c) => c.id === 'sep-2468-client-compare-iss-supported')
) {
const correctlyRejected = this.authReached && !this.tokenRequestMade;
this.checks.push({
id: 'sep-2468-client-compare-iss-supported',
name: 'Client rejects mismatched iss',
description: correctlyRejected
? 'Client correctly rejected authorization response with mismatched iss parameter'
: 'Client MUST reject authorization response when iss does not match the authorization server issuer',
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: specRefs,
details: {
serverAdvertisedSupport: true,
issSentInRedirect: 'https://evil.example.com',
this.checks.push(
issRejectionCheck({
id: 'sep-2468-client-compare-iss-supported',
name: 'Client rejects mismatched iss',
passDescription:
'Client correctly rejected authorization response with mismatched iss parameter',
failDescription:
'Client MUST reject authorization response when iss does not match the authorization server issuer',
metadataRequested: this.checks.some(
(c) => c.id === 'authorization-server-metadata'
),
authReached: this.authReached,
tokenRequestMade: this.tokenRequestMade
}
});
tokenRequestMade: this.tokenRequestMade,
observations: {
serverAdvertisedSupport: true,
issSentInRedirect: 'https://evil.example.com'
},
timestamp
})
);
}
return this.checks;
@ -394,23 +488,26 @@ export class IssParameterUnexpectedScenario implements Scenario {
(c) => c.id === 'sep-2468-client-compare-iss-unadvertised'
)
) {
const correctlyRejected = this.authReached && !this.tokenRequestMade;
this.checks.push({
id: 'sep-2468-client-compare-iss-unadvertised',
name: 'Client compares unadvertised iss and rejects mismatch',
description: correctlyRejected
? 'Client correctly compared unadvertised iss against recorded issuer and rejected the mismatch'
: 'Client MUST compare a present iss against the recorded issuer regardless of metadata advertisement, and reject on mismatch',
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: specRefs,
details: {
serverAdvertisedSupport: false,
issSentInRedirect: 'https://evil.example.com',
this.checks.push(
issRejectionCheck({
id: 'sep-2468-client-compare-iss-unadvertised',
name: 'Client compares unadvertised iss and rejects mismatch',
passDescription:
'Client correctly compared unadvertised iss against recorded issuer and rejected the mismatch',
failDescription:
'Client MUST compare a present iss against the recorded issuer regardless of metadata advertisement, and reject on mismatch',
metadataRequested: this.checks.some(
(c) => c.id === 'authorization-server-metadata'
),
authReached: this.authReached,
tokenRequestMade: this.tokenRequestMade
}
});
tokenRequestMade: this.tokenRequestMade,
observations: {
serverAdvertisedSupport: false,
issSentInRedirect: 'https://evil.example.com'
},
timestamp
})
);
}
return this.checks;
@ -483,23 +580,26 @@ export class IssParameterNormalizedVariantScenario implements Scenario {
const timestamp = new Date().toISOString();
if (!this.checks.some((c) => c.id === 'sep-2468-client-no-normalization')) {
const correctlyRejected = this.authReached && !this.tokenRequestMade;
this.checks.push({
id: 'sep-2468-client-no-normalization',
name: 'Client compares iss without URL normalization',
description: correctlyRejected
? 'Client rejected an iss value that only matches the recorded issuer after URL normalization'
: 'Client MUST NOT apply scheme/host case folding, default-port elision, trailing-slash, or percent-encoding normalization to iss before comparison; a trailing-slash variant of the issuer must be treated as a mismatch',
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: specRefs,
details: {
recordedIssuer: this.authServer.getUrl(),
issSentInRedirect: `${this.authServer.getUrl()}/`,
this.checks.push(
issRejectionCheck({
id: 'sep-2468-client-no-normalization',
name: 'Client compares iss without URL normalization',
passDescription:
'Client rejected an iss value that only matches the recorded issuer after URL normalization',
failDescription:
'Client MUST NOT apply scheme/host case folding, default-port elision, trailing-slash, or percent-encoding normalization to iss before comparison; a trailing-slash variant of the issuer must be treated as a mismatch',
metadataRequested: this.checks.some(
(c) => c.id === 'authorization-server-metadata'
),
authReached: this.authReached,
tokenRequestMade: this.tokenRequestMade
}
});
tokenRequestMade: this.tokenRequestMade,
observations: {
recordedIssuer: this.authServer.getUrl(),
issSentInRedirect: `${this.authServer.getUrl()}/`
},
timestamp
})
);
}
return this.checks;
@ -589,26 +689,55 @@ export class MetadataIssuerMismatchScenario implements Scenario {
const metadataRequested = this.checks.some(
(c) => c.id === 'authorization-server-metadata'
);
const correctlyRejected =
metadataRequested && !this.metadataEndpointsUsed;
this.checks.push({
id: 'sep-2468-client-validate-metadata-issuer',
name: 'Client validates metadata issuer against well-known URL',
description: correctlyRejected
? 'Client rejected authorization server metadata whose issuer does not match the issuer identifier used to construct the well-known URL'
: metadataRequested
? 'Client MUST NOT use authorization server metadata whose issuer differs from the issuer identifier used to construct the well-known URL; client used endpoints from the mismatched metadata'
: 'Client never retrieved the authorization server metadata document, so issuer validation could not be observed',
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: metadataSpecRefs,
details: {
expectedIssuer: this.authServer.getUrl(),
metadataIssuer: 'https://attacker.example.com',
metadataRequested,
metadataEndpointsUsed: this.metadataEndpointsUsed
}
});
const observations = {
expectedIssuer: this.authServer.getUrl(),
metadataIssuer: 'https://attacker.example.com',
metadataRequested,
metadataEndpointsUsed: this.metadataEndpointsUsed
};
const failDescription =
'Client MUST NOT use authorization server metadata whose issuer differs from the issuer identifier used to construct the well-known URL; client used endpoints from the mismatched metadata';
// This check already bound its verdict to the metadata fetch, but
// reported the unreached case as a plain violation. Per #248 a
// requirement that could not be exercised is untestable, so the report
// distinguishes "client used the poisoned metadata" from "client never
// fetched it" instead of collapsing both into FAILURE (#467).
if (!metadataRequested) {
const check = untestableCheck(
'sep-2468-client-validate-metadata-issuer',
'Client validates metadata issuer against well-known URL',
failDescription,
'client never retrieved the authorization server metadata document, so issuer validation could not be observed',
metadataSpecRefs
);
check.details = {
...check.details,
...observations,
propertyReached: false,
stopReason: 'as-metadata-not-requested'
};
this.checks.push(check);
} else {
const correctlyRejected = !this.metadataEndpointsUsed;
this.checks.push({
id: 'sep-2468-client-validate-metadata-issuer',
name: 'Client validates metadata issuer against well-known URL',
description: correctlyRejected
? 'Client rejected authorization server metadata whose issuer does not match the issuer identifier used to construct the well-known URL'
: failDescription,
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: metadataSpecRefs,
details: {
...observations,
propertyReached: true,
stopReason: correctlyRejected
? 'declined-poisoned-metadata'
: 'used-poisoned-metadata-endpoints'
}
});
}
}
return this.checks;

View file

@ -6,6 +6,7 @@ import { createServer } from './helpers/createServer.js';
import { ServerLifecycle } from './helpers/serverLifecycle.js';
import { SpecReferences } from './spec-references.js';
import { MockTokenVerifier } from './helpers/mockTokenVerifier.js';
import { untestableCheck } from '../../untestable.js';
/**
* Scenario: Resource Mismatch Detection
@ -90,25 +91,57 @@ export class ResourceMismatchScenario implements Scenario {
SpecReferences.MCP_RESOURCE_PARAMETER
];
// The test passes if the client did NOT make an authorization request
// (meaning it correctly rejected the mismatched resource)
// Reason-bound verdict (#467). "Did not proceed with authorization" is not
// by itself evidence of validation: a client that never fetched the PRM
// document never read the mismatched `resource`, so it cannot have
// compared it. Absent that fetch the requirement was never exercised,
// which is the untestable case (#248) rather than a pass or a violation.
if (!this.checks.some((c) => c.id === 'resource-mismatch-rejected')) {
const correctlyRejected = !this.authorizationRequestMade;
this.checks.push({
id: 'resource-mismatch-rejected',
name: 'Client rejects mismatched resource',
description: correctlyRejected
? 'Client correctly rejected authorization when PRM resource does not match server URL'
: 'Client MUST validate that PRM resource matches the server URL before proceeding with authorization',
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: specRefs,
details: {
prmResource: 'https://evil.example.com/mcp',
expectedBehavior: 'Client should NOT proceed with authorization',
authorizationRequestMade: this.authorizationRequestMade
}
});
const prmRequested = this.checks.some(
(c) => c.id === 'prm-pathbased-requested'
);
const correctlyRejected = prmRequested && !this.authorizationRequestMade;
const observations = {
prmResource: 'https://evil.example.com/mcp',
expectedBehavior: 'Client should NOT proceed with authorization',
prmRequested,
authorizationRequestMade: this.authorizationRequestMade
};
if (!prmRequested) {
const check = untestableCheck(
'resource-mismatch-rejected',
'Client rejects mismatched resource',
'Client MUST validate that PRM resource matches the server URL before proceeding with authorization',
'client never requested the Protected Resource Metadata document, so it never read the resource value it was required to validate',
specRefs
);
check.details = {
...check.details,
...observations,
propertyReached: false,
stopReason: 'prm-not-requested'
};
this.checks.push(check);
} else {
this.checks.push({
id: 'resource-mismatch-rejected',
name: 'Client rejects mismatched resource',
description: correctlyRejected
? 'Client correctly rejected authorization when PRM resource does not match server URL'
: 'Client MUST validate that PRM resource matches the server URL before proceeding with authorization',
status: correctlyRejected ? 'SUCCESS' : 'FAILURE',
timestamp,
specReferences: specRefs,
details: {
...observations,
propertyReached: true,
stopReason: correctlyRejected
? 'declined-after-reading-prm'
: 'proceeded-to-authorization'
}
});
}
}
return this.checks;