fix: Let an audience provider refuse a token whatever its order, and sort providers without overflow - EXO-90439

[boubaker]

Symptom: the MCP server refuses an access token to a user outside its audience by throwing from its OAuthAccessTokenAudienceProvider. That refusal was reached only while the MCP provider sorted before OAuthAccessTokenAudienceTokenRequestProvider, which answers with the RFC 8707 resource parameter that MCP clients send. With the order reversed, an excluded user would get a token and be refused only at the MCP server's door.

Cause: OAuthAccessTokenCustomizerService sorted its providers with (p1, p2) -> p2.getOrder() - p1.getOrder(). Between HIGHEST_PRECEDENCE and LOWEST_PRECEDENCE that subtraction overflows, and the overflow alone put the MCP provider first. computeJwtAudiences then stopped at the first non-empty answer, so a provider sorted after it was never consulted and its refusal never heard.

Fix:

• Both provider lists sort by ascending getOrder() through Comparator.comparingInt, the Spring Ordered convention. This keeps the order every shipped provider runs in today.
• Every audience provider is consulted for every access token. The first non-empty answer becomes aud, and a thrown OAuth2AuthenticationException refuses the token wherever its provider sorts. Authority providers keep stopping at the first answer.
• The lists are replaced by a new sorted copy, never mutated in place, and are read and written under the service's monitor: synchronized init(), addProvider(...) and the private accessors token requests read them through. A token request therefore never iterates a list being sorted while another webapp registers its provider.
• OAuthAccessTokenAudienceTokenRequestProvider logs a refused resource at debug. The value is client-supplied, and the provider now runs on every access token.
• The aud claim is set unconditionally, since computeJwtAudiences answers a non-empty list or throws.
• The two SPIs document the order and refusal contracts.
• OAuthAccessTokenCustomizerServiceTest covers the order, including the HIGHEST/LOWEST_PRECEDENCE pair, a refusal before and after an answering provider, providers added after startup, and a held list left untouched. Each pin fails against its mutant: the subtraction comparator, a descending Integer.compare, the short-circuit, in-place mutation, and a dropped authority provider.
• The full auth-server build passes, 126 tests. feat: gate MCP server access per user and group EXO-90439 mcp-server#36's token-gate tests, 33 of them, pass against this build.

Same task as Meeds-io/mcp-server#36, whose provider relies on this. That PR does not wait on this one.

Knowledge: Meeds-io/eng-standards#128, #129

This change is classified N1 (token issuance is an authorization decision on the trust boundary), computed at 6612c4f. Its approver must be an Architect or Senior Developer who knows it is N1, not an approval on AI review alone; author ≠ approver.

🤖 Generated with Claude Code

[boubaker]

Self-review close-out

Two AI review rounds by independent reviewer agents; the closing round was a fresh reviewer, re-checked at d86d093. Clean at head: nothing outstanding from the AI review side.

Round	Finding	Status
1	🟡 OAuthAccessTokenAudienceTokenRequestProvider's WARN now fires on MCP tokens with a non-canonical resource	✅ Logged at debug (client-supplied value, runs on every access token)
1	🟡 addProvider mutated the list request threads iterate	✅ volatile lists replaced copy-on-write under synchronized; pinned, and the in-place mutant fails the pin
1	🟡 mcp-server#36's provider Javadoc goes stale once this merges	➖ Not this PR: an mcp-server follow-up after this lands on develop (Architect's ruling)
1	🟢 Authority SPI silent on throwing	✅ Its Javadoc says refusal belongs to the audience SPI
1	🟢 A test display name overclaimed	✅ Javadoc names the mutant it pins
1	🟢 Endpoint-level pin in OAuthSecurityIntegrationTest (optional)	➖ Not taken: that context is shared across its tests and the SPI has no removal; the path to the token endpoint's 400 was verified in the Spring Authorization Server 7.1.1 sources
2	🟢 Authority addProvider never asserted	✅ addedAuthorityProviderTakesItsPlaceInTheOrder; the dropped-provider and unsorted mutants both fail it

Verified conform:

• Order kept for every shipped provider. They are OAuthAccessTokenAudienceTokenRequestProvider and OAuthAccessTokenAuthorityPrincipalProvider at LOWEST_PRECEDENCE, and mcp-server's provider at the default. No other implementation exists across the org's repos.
• Consulting every audience provider adds no failure mode. The resource provider cannot throw, and its allowed-audiences read is memoised.
• The customizer runs for both grants. It is set on both the JWT and the opaque generator. The authorization-code and refresh-token providers both call it, and OAuth2TokenEndpointFilter answers the exception with a 400.
• mcp-server#36's test still works. It injects both provider fields by reflection, and its 33 token-gate tests pass against this build.

Classification: N1, computed at d86d093: token issuance is an authorization decision on the trust boundary. Its approver must be an Architect or Senior Developer who knows it is N1, not an approval on AI review alone. Author ≠ approver.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[boubaker]

Self-review close-out — addendum at 6612c4f

On d86d093, Sonar's gate failed on reliability. It raised java:S3077 twice, for volatile on the two List fields, and java:S2589, for the audiences != null check that became always true.

Round	Finding	Status
3	Sonar S3077 ×2	✅ Fields are plain Lists, still copy-on-write, read and written under the service's monitor through synchronized accessors. The in-place mutant still fails addingAProviderLeavesTheHeldListUntouched.
3	Sonar S2589	✅ aud set unconditionally: computeJwtAudiences answers a non-empty list or throws
3	🟢 Chained-call indent	✅ Whitespace only
3	🟡 PR body still said volatile	✅ Body updated to the change as it stands
3	🟠 eng-standards#128 still said volatile	✅ Rewritten there and restamped at 6612c4f

Verified conform:

• Publication. Every write and every read of both lists holds the same monitor, so the writer's unlock happens-before the reader's lock. No list is modified after it is published.
• Contention. Each token request takes two uncontended monitor acquisitions, each held for one field load.
• Tests. The full auth-server build passes, 126 tests. mcp-server#36's reflection-based provider test, which injects these fields, passes against this head.
• Gates. Sonar's quality gate and PR Build both pass at 6612c4f.

Nothing outstanding from the AI review side.

Classification: N1, computed at 6612c4f. Its approver must be an Architect or Senior Developer who knows it is N1, not an approval on AI review alone. Author ≠ approver.

🤖 Generated with Claude Code