This is an automated email from the ASF dual-hosted git repository. jamesbognar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/juneau.git
commit eed551345938ffd98940e559b5f8735588ea63e3 Author: James Bognar <[email protected]> AuthorDate: Sun Aug 16 13:40:23 2026 -0400 READY-380: Require exact OAuth scope match in MCP resource server McpResourceServerSupport.satisfies() treated a granted scope as implying any required scope under a "scope:"/"scope." prefix, letting a broad low-privilege grant authorize a more-privileged operation. Switch to exact-match set semantics. --- .../mcp/v20260728/McpResourceServerSupport.java | 25 ++++---------- .../v20260728/McpResourceServerBinding_Test.java | 28 +++++++++------- .../v20260728/McpResourceServerSupport_Test.java | 38 ++++++++++++++-------- 3 files changed, 49 insertions(+), 42 deletions(-) diff --git a/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java b/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java index c1b946d06f..b83e1df9cb 100644 --- a/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java +++ b/juneau-rest/juneau-rest-server-mcp-v20260728/src/main/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport.java @@ -513,32 +513,21 @@ public final class McpResourceServerSupport { } /** - * Returns whether the granted scopes satisfy every required scope, honoring OAuth scope hierarchies where a broader - * granted scope implies its narrower children. + * Returns whether the granted scopes satisfy every required scope, using exact string equality. * * <p> - * A required scope {@code r} is satisfied when some granted scope {@code g} either equals {@code r} exactly, or is a - * hierarchical <em>ancestor</em> of it — i.e. {@code r} begins with {@code g} followed by a {@code :} or - * {@code .} delimiter (so granted {@code repo} implies required {@code repo:read} / {@code repo.write}, but granted - * {@code repo:read} does <b>not</b> imply required {@code repo}). + * A required scope {@code r} is satisfied only when some granted scope {@code g} is exactly equal to it. OAuth + * scopes have no universal hierarchy, so a granted {@code repo} does <b>not</b> imply a required {@code repo:write} + * or {@code repo.admin} — a broad-but-low-privilege scope must never authorize a differently named privileged + * operation. Applications that want scope implication must expand their granted scopes themselves before calling + * this method; this method does not provide a hierarchical-implication SPI. * * @param grantedScopes The caller's granted scopes. Never <jk>null</jk>. * @param requiredScopes The scopes required to invoke the operation. Never <jk>null</jk>. * @return <jk>true</jk> if every required scope is satisfied. */ public static boolean satisfies(Collection<String> grantedScopes, Collection<String> requiredScopes) { - for (var r : requiredScopes) { - var ok = false; - for (var g : grantedScopes) { - if (g.equals(r) || r.startsWith(g + ":") || r.startsWith(g + ".")) { - ok = true; - break; - } - } - if (!ok) - return false; - } - return true; + return grantedScopes.containsAll(requiredScopes); } /** diff --git a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java index 660d5a352f..415cd660a6 100644 --- a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java +++ b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerBinding_Test.java @@ -72,11 +72,12 @@ class McpResourceServerBinding_Test extends TestBase { case "good4" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp.read mcp.write", "iss", "https://idp-b";, "sub", "alice")); case "noscope" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "other")); case "wrongaud" -> new ClaimsPrincipal("alice", Map.of("aud", "http://evil.example.com";, "scope", "mcp.read")); - // SEP-2350 step-up fixtures: baseline mcp.read + the per-operation tools.exec (exact and hierarchical grants). + // SEP-2350 step-up fixtures: baseline mcp.read + the per-operation tools.exec (exact grant vs. a + // broader-but-not-exact grant that must NOT satisfy it). case "opok" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp.read tools.exec")); case "broad" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp.read tools")); - // H1 baseline-hierarchy fixture: granted ONLY the broad ancestor "mcp", which satisfies the baseline - // "mcp.read" hierarchically (never exact-string). + // Baseline fixture: granted ONLY the broader "mcp" scope, which does NOT satisfy the exact-match baseline + // requirement "mcp.read" (OAuth scopes have no universal hierarchy). case "basehier" -> new ClaimsPrincipal("alice", Map.of("aud", RESOURCE, "scope", "mcp")); default -> throw new AuthenticationException("bad token"); }; @@ -201,13 +202,15 @@ class McpResourceServerBinding_Test extends TestBase { assertContains("alice", s); } - // H1: a token granted ONLY the broad ancestor "mcp" satisfies the baseline "mcp.read" hierarchically and - // dispatches. The old exact-string containsAll baseline gate would have 403'd this. - @Test void a10_baselineHierarchicalScope_dispatches() throws Exception { + // A token granted only the broader "mcp" scope does NOT satisfy the exact-match baseline requirement + // "mcp.read": a broad-but-differently-named scope must never authorize a differently named privileged operation. + @Test void a10_baselineBroaderScopeDoesNotSatisfy_403() throws Exception { clientA().post("/").contentString(body(1, "server/discover", null)) .header("Mcp-Method", "server/discover").header("Mcp-Name", "") .header("Authorization", "Bearer basehier") - .run().assertStatus(200); + .run() + .assertStatus(403) + .assertHeader("WWW-Authenticate").is("Bearer realm=\"mcp\", error=\"insufficient_scope\", scope=\"mcp.read\", resource_metadata=\"" + PRM_URL + "\""); } // M5: the wildcard well-known route must not serve this endpoint's PRM for an arbitrary suffix. @@ -218,7 +221,7 @@ class McpResourceServerBinding_Test extends TestBase { // --------------------------------------------------------------------------------------------- // SEP-2350 (F3) per-operation step-up: an end-to-end HTTP round-trip exercising the POST-parse enforcement // point in McpRevision.dispatch (a token satisfying the endpoint-wide baseline but not the operation's scope - // yields a scoped 403 insufficient_scope challenge; the correct scope, exact or hierarchical, dispatches). + // yields a scoped 403 insufficient_scope challenge; only the exact required scope dispatches). // --------------------------------------------------------------------------------------------- private static McpOptions rsEnabledWithOpScope() { @@ -266,12 +269,15 @@ class McpResourceServerBinding_Test extends TestBase { .run().assertStatus(200); } - // A broader ancestor scope (tools) satisfies the narrower required tools.exec (server MUST: hierarchy-aware). - @Test void d03_stepUp_hierarchicalScope_dispatches() throws Exception { + // A broader scope (tools) does NOT satisfy the exact required scope tools.exec: OAuth scopes have no universal + // hierarchy, so this must be a scoped 403 step-up challenge, not a dispatch. + @Test void d03_stepUp_broaderScopeDoesNotSatisfy_403() throws Exception { clientD().post("/").contentString(body(1, "tools/call", callEcho())) .header("Mcp-Method", "tools/call").header("Mcp-Name", "echo") .header("Authorization", "Bearer broad") - .run().assertStatus(200); + .run() + .assertStatus(403) + .assertHeader("WWW-Authenticate").is("Bearer realm=\"mcp\", error=\"insufficient_scope\", scope=\"tools.exec\", resource_metadata=\"" + PRM_URL + "\""); } // An operation with no per-operation scope configured is unaffected: baseline mcp.read alone dispatches it. diff --git a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java index 90338624bb..20f9446b53 100644 --- a/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java +++ b/juneau-rest/juneau-rest-server-mcp-v20260728/src/test/java/org/apache/juneau/rest/server/mcp/v20260728/McpResourceServerSupport_Test.java @@ -191,8 +191,9 @@ class McpResourceServerSupport_Test { } // --------------------------------------------------------------------------------------------- - // SEP-2350 per-operation step-up: scope satisfaction (hierarchy-aware), scoped challenge shape, and the - // POST-parse enforcement decision (throws 403 insufficient_scope + WWW-Authenticate, or is a no-op). + // SEP-2350 per-operation step-up: scope satisfaction (exact-match only, no hierarchy/prefix inheritance), + // scoped challenge shape, and the POST-parse enforcement decision (throws 403 insufficient_scope + + // WWW-Authenticate, or is a no-op). // --------------------------------------------------------------------------------------------- private static McpOperationContext op(String method, String name) { @@ -203,10 +204,12 @@ class McpResourceServerSupport_Test { assertTrue(McpResourceServerSupport.satisfies(Set.of("repo.delete"), Set.of("repo.delete"))); } - @Test void f02_satisfies_broaderAncestorImpliesNarrower() { - // Granted "repo" implies required "repo.delete" (dot) and "repo:delete" (colon). - assertTrue(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo.delete"))); - assertTrue(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo:delete"))); + @Test void f02_satisfies_broaderScopeDoesNotImplyNarrower() { + // OAuth scopes have no universal hierarchy: granted "repo" must NOT satisfy required "repo.delete" (dot) or + // "repo:delete" (colon) — a broad-but-low-privilege scope must never authorize a differently named, + // more-privileged operation. + assertFalse(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo.delete"))); + assertFalse(McpResourceServerSupport.satisfies(Set.of("repo"), Set.of("repo:delete"))); } @Test void f03_satisfies_narrowerDoesNotImplyBroader() { @@ -214,8 +217,8 @@ class McpResourceServerSupport_Test { assertFalse(McpResourceServerSupport.satisfies(Set.of("repo.read"), Set.of("repo"))); } - @Test void f04_satisfies_partialTokenIsNotAncestor() { - // "rep" is a string prefix of "repo.read" but NOT a hierarchical ancestor (no delimiter), so it must not satisfy. + @Test void f04_satisfies_partialTokenIsNotAMatch() { + // "rep" is a string prefix of "repo.read" but not an exact match, so it must not satisfy. assertFalse(McpResourceServerSupport.satisfies(Set.of("rep"), Set.of("repo.read"))); } @@ -224,11 +227,20 @@ class McpResourceServerSupport_Test { assertTrue(McpResourceServerSupport.satisfies(Set.of("a", "b"), Set.of("a", "b"))); } - @Test void f05b_satisfies_baselineParity_grantedAncestorSatisfiesBaseline() { - // H1 parity: the baseline required-scope gate uses satisfies(...) exactly like the per-operation gate, so a - // token granted "mcp" satisfies a baseline of "mcp.read" hierarchically (no exact-string containsAll needed). - assertTrue(McpResourceServerSupport.satisfies(Set.of("mcp"), Set.of("mcp.read"))); - assertFalse(Set.of("mcp").containsAll(Set.of("mcp.read"))); // the OLD containsAll gate would have 403'd this. + @Test void f05b_satisfies_baselineParity_grantedBroaderScopeDoesNotSatisfyBaseline() { + // Parity: the baseline required-scope gate uses satisfies(...) exactly like the per-operation gate, so a + // token granted only "mcp" does NOT satisfy a baseline of "mcp.read" — exact-match only, no + // prefix/hierarchy inheritance. + assertFalse(McpResourceServerSupport.satisfies(Set.of("mcp"), Set.of("mcp.read"))); + } + + @Test void f05c_satisfies_emptyRequired_alwaysTrue() { + assertTrue(McpResourceServerSupport.satisfies(Set.of(), Set.of())); + assertTrue(McpResourceServerSupport.satisfies(Set.of("a"), Set.of())); + } + + @Test void f05d_satisfies_emptyGrantedNonEmptyRequired_false() { + assertFalse(McpResourceServerSupport.satisfies(Set.of(), Set.of("a"))); } @Test void f06_stepUpChallengeShape() {
