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 af84870a44dddbaf90a524161dd1828f8248eebc Author: James Bognar <[email protected]> AuthorDate: Sun Aug 16 18:49:06 2026 -0400 READY-383: Don't union roles across distinct principals in AuthFilterChain AuthResultAccumulator previously merged the role sets from every successful auth-provider outcome regardless of which principal each outcome authenticated, letting a request end up with the union of roles from unrelated identities. Roles are now accumulated only for the principal that AuthFilterChain ultimately selects. --- .../AuthFilterChain_GuardIntegration_Test.java | 10 +++---- .../rest/server/auth/AuthFilterChain_Test.java | 6 ++--- .../juneau/rest/server/auth/AuthFilterChain.java | 6 +++-- .../rest/server/auth/AuthResultAccumulator.java | 27 +++++++++++++++---- .../rest/server/auth/AuthFilterChain_Test.java | 31 ++++++++++++++++++++-- .../server/auth/AuthResultAccumulator_Test.java | 14 ++++++++-- 6 files changed, 75 insertions(+), 19 deletions(-) diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_GuardIntegration_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_GuardIntegration_Test.java index aee281ffcd..a9fa73b997 100644 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_GuardIntegration_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_GuardIntegration_Test.java @@ -157,16 +157,16 @@ class AuthFilterChain_GuardIntegration_Test extends TestBase { assertFalse(w.isUserInRole("user")); } - @Test void a03_bothCredentials_bearerPrincipalWins_rolesUnion() throws Exception { - // Both bearer-user (user role) and apikey-admin (admin role) present. - // Bearer is registered first → alice wins for principal; roles = union. + @Test void a03_bothCredentials_bearerPrincipalWins_rolesNotUnionedAcrossDistinctPrincipals() throws Exception { + // Both bearer-user (alice, user role) and apikey-admin (bob, admin role) present. + // Bearer is registered first → alice wins for principal; bob is a DIFFERENT principal, so his + // admin role must NOT be unioned onto alice's identity. var r = runChain(buildChain(), "Bearer bearer-user", "apikey-admin"); assertNotNull(r.captured); var w = (AuthenticatedRequestWrapper) r.captured; assertEquals("alice", w.getUserPrincipal().getName()); - // Union: user (from bearer) + admin (from api-key) assertTrue(w.isUserInRole("user")); - assertTrue(w.isUserInRole("admin")); + assertFalse(w.isUserInRole("admin")); } @Test void a04_noCredentials_passThroughUnchanged() throws Exception { diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java index 58568d6903..532a255562 100644 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java @@ -165,10 +165,10 @@ class AuthFilterChain_Test extends TestBase { var capturing = new CapturingChain(); chain.doFilter(req("/"), capturingResponse(), capturing); var w = (AuthenticatedRequestWrapper) capturing.captured; - // All roles from all successful filters must be present + // Bob is a distinct principal from alice — his roles must NOT be unioned onto alice's identity. assertTrue(w.isUserInRole("user")); - assertTrue(w.isUserInRole("admin")); - assertTrue(w.isUserInRole("billing")); + assertFalse(w.isUserInRole("admin")); + assertFalse(w.isUserInRole("billing")); } @Test void a06_allMatchingFiltersFail_returns401() throws Exception { diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthFilterChain.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthFilterChain.java index 34ebed37f3..b79faa7172 100644 --- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthFilterChain.java +++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthFilterChain.java @@ -46,8 +46,10 @@ import jakarta.servlet.http.*; * <ul> * <li>{@link Optional#empty()} — filter doesn't apply this request; continue. * <li>{@link Optional#of(Object) Optional.of(AuthResult)} — success. The first successful filter's - * {@link Principal} wins for identity. All subsequent successful filters contribute their roles to the - * union. + * {@link Principal} wins for identity. Subsequent successful filters contribute their roles to the + * union only when they carry no principal of their own or the <b>same</b> principal (by + * {@link Principal#getName()}); a filter that authenticates a <i>different</i> principal does not + * contribute its roles, so a second identity cannot silently elevate the first. * <li>throw {@link AuthenticationException} — credentials present but invalid; record as a failure. * </ul> * <li>After iterating: diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator.java index 9f88d2f20c..c92268a476 100644 --- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator.java +++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator.java @@ -20,14 +20,19 @@ import static org.apache.juneau.commons.utils.Shorts.*; import java.security.*; import java.util.*; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Mutable helper that folds a sequence of {@link AuthResult}s into a single result, honoring * {@link AuthResult.MergeMode}. * * <p> - * {@link AuthResult.MergeMode#ADD ADD} unions roles and keeps the first non-<jk>null</jk> principal; - * {@link AuthResult.MergeMode#REPLACE REPLACE} resets the accumulated principal + roles. Used by both + * {@link AuthResult.MergeMode#ADD ADD} keeps the first non-<jk>null</jk> principal and unions roles from + * later results <b>only when</b> those results carry no principal of their own or the <b>same</b> principal + * (compared by {@link Principal#getName()}, null-safe). A later result with a <i>different</i> non-<jk>null</jk> + * principal does not contribute its roles — a second authenticated identity must not silently elevate the + * first. {@link AuthResult.MergeMode#REPLACE REPLACE} resets the accumulated principal + roles. Used by both * {@link AuthFilterChain} and the resource-level fold in {@link org.apache.juneau.rest.server.RestContext}. * * <h5 class='section'>See Also:</h5><ul> @@ -40,6 +45,8 @@ import java.util.*; */ public final class AuthResultAccumulator { + private static final Logger LOG = Logger.getLogger(AuthResultAccumulator.class.getName()); + private Principal principal; private final Set<String> roles = new LinkedHashSet<>(); private boolean any; @@ -58,10 +65,20 @@ public final class AuthResultAccumulator { principal = r.getPrincipal(); roles.clear(); roles.addAll(r.getRoles()); - } else { - if (principal == null) - principal = r.getPrincipal(); // may still be null (roles-only) + return this; + } + if (principal == null) { + principal = r.getPrincipal(); // may still be null (roles-only) roles.addAll(r.getRoles()); + return this; + } + var otherPrincipal = r.getPrincipal(); + if (otherPrincipal == null || Objects.equals(principal.getName(), otherPrincipal.getName())) { + roles.addAll(r.getRoles()); + } else { + var establishedName = principal.getName(); + LOG.log(Level.WARNING, () -> "Ignoring roles from a distinct authenticated principal ('" + + otherPrincipal.getName() + "'); request identity remains '" + establishedName + "'."); } return this; } diff --git a/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java index e9a9fdec84..83fa8c268d 100644 --- a/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java +++ b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthFilterChain_Test.java @@ -166,10 +166,24 @@ class AuthFilterChain_Test extends TestBase { var capturing = new CapturingChain(); chain.doFilter(req("/"), capturingResponse(), capturing); var w = (AuthenticatedRequestWrapper) capturing.captured; - // All roles from all successful filters must be present + // Bob is a distinct principal from alice — his roles must NOT be unioned onto alice's identity. + assertTrue(w.isUserInRole("user")); + assertFalse(w.isUserInRole("admin")); + assertFalse(w.isUserInRole("billing")); + } + + @Test void a05b_roleAggregation_unionAcrossFiltersForSamePrincipal() throws Exception { + Principal aliceAgain = () -> "alice"; // distinct instance, same getName() — same subject + var chain = AuthFilterChain.create(null) + .append(succeeds(ALICE, "user")) + .append(succeeds(aliceAgain, "admin")) + .build(); + var capturing = new CapturingChain(); + chain.doFilter(req("/"), capturingResponse(), capturing); + var w = (AuthenticatedRequestWrapper) capturing.captured; + // Same subject authenticated by two schemes — roles still union. assertTrue(w.isUserInRole("user")); assertTrue(w.isUserInRole("admin")); - assertTrue(w.isUserInRole("billing")); } @Test void a06_allMatchingFiltersFail_returns401() throws Exception { @@ -285,6 +299,19 @@ class AuthFilterChain_Test extends TestBase { var r = chain.authenticate(req("/")).orElseThrow(); assertSame(ALICE, r.getPrincipal()); assertTrue(r.getRoles().contains("user")); + // Bob is a distinct principal from alice — his role must NOT be unioned onto alice's identity. + assertFalse(r.getRoles().contains("admin")); + } + + @Test void e05b_authenticateRoleUnionForSamePrincipal() { + Principal aliceAgain = () -> "alice"; // distinct instance, same getName() — same subject + var chain = AuthFilterChain.create(null) + .append(succeeds(ALICE, "user")) + .append(succeeds(aliceAgain, "admin")) + .build(); + var r = chain.authenticate(req("/")).orElseThrow(); + assertSame(ALICE, r.getPrincipal()); + assertTrue(r.getRoles().contains("user")); assertTrue(r.getRoles().contains("admin")); } diff --git a/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator_Test.java b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator_Test.java index 5a63139e41..134317f1e8 100644 --- a/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator_Test.java +++ b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/auth/AuthResultAccumulator_Test.java @@ -34,10 +34,20 @@ class AuthResultAccumulator_Test extends TestBase { private static final Principal ALICE = () -> "alice"; private static final Principal BOB = () -> "bob"; - @Test void a01_addUnionsRoles_firstPrincipalWins() { + @Test void a01_addFromDifferentPrincipal_rolesNotUnioned_firstPrincipalWins() { var acc = new AuthResultAccumulator(); acc.add(AuthResult.of(ALICE, "r1")); - acc.add(AuthResult.of(BOB, "r2")); // ADD: principal stays alice, roles union + acc.add(AuthResult.of(BOB, "r2")); // ADD: principal stays alice; bob's roles are NOT unioned (different subject) + var r = acc.result().orElseThrow(); + assertSame(ALICE, r.getPrincipal()); + assertEquals(Set.of("r1"), r.getRoles()); + } + + @Test void a01b_addFromSameNamedPrincipal_rolesUnion() { + var acc = new AuthResultAccumulator(); + Principal aliceAgain = () -> "alice"; // distinct instance, same getName() — same subject + acc.add(AuthResult.of(ALICE, "r1")); + acc.add(AuthResult.of(aliceAgain, "r2")); var r = acc.result().orElseThrow(); assertSame(ALICE, r.getPrincipal()); assertEquals(Set.of("r1", "r2"), r.getRoles());
