diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java b/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java index f8ba12f23ac4..2b774283d924 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java @@ -41,7 +41,7 @@ public class RoleInfo /** * List of permitted roles */ - private final Set _roles = new CopyOnWriteArraySet(); + private final Set _roles = new CopyOnWriteArraySet<>(); public RoleInfo() { @@ -140,26 +140,28 @@ public void combine(RoleInfo other) { if (other._forbidden) setForbidden(true); - else if (!other._checked) // TODO is this the right way around??? - setChecked(true); - else if (other._isAnyRole) - setAnyRole(true); - else if (other._isAnyAuth) - setAnyAuth(true); - else if (!_isAnyRole) + else if (other._checked) { - for (String r : other._roles) - { - _roles.add(r); - } + setChecked(true); + if (other._isAnyRole) + setAnyRole(true); + else if (other._isAnyAuth) + setAnyAuth(true); + else if (!_isAnyRole) + _roles.addAll(other._roles); } - setUserDataConstraint(other._userDataConstraint); } @Override public String toString() { - return "{RoleInfo" + (_forbidden ? ",F" : "") + (_checked ? ",C" : "") + (_isAnyRole ? ",*" : _roles) + (_userDataConstraint != null ? "," + _userDataConstraint : "") + "}"; + return String.format("RoleInfo@%x{%s%s%s%s,%s}", + hashCode(), + (_forbidden ? "Forbidden," : ""), + (_checked ? "Checked," : ""), + (_isAnyAuth ? "AnyAuth," : ""), + (_isAnyRole ? "*" : _roles), + _userDataConstraint); } } diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java index 02488e8cff7d..a686581f3e48 100644 --- a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java +++ b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java @@ -99,7 +99,7 @@ public class ConstraintTest private Constraint _authAdminConstraint; private Constraint _relaxConstraint; private Constraint _loginPageConstraint; - private Constraint _allowPostConstraint; + private Constraint _noAuthConstraint; private Constraint _confidentialDataConstraint; private Constraint _anyUserAuthConstraint; @@ -196,12 +196,12 @@ private List getConstraintMappings() mapping4.setPathSpec("/testLoginPage"); mapping4.setConstraint(_loginPageConstraint); - _allowPostConstraint = new Constraint(); - _allowPostConstraint.setAuthenticate(false); - _allowPostConstraint.setName("allow forbidden POST"); + _noAuthConstraint = new Constraint(); + _noAuthConstraint.setAuthenticate(false); + _noAuthConstraint.setName("allow forbidden"); ConstraintMapping mapping5 = new ConstraintMapping(); mapping5.setPathSpec("/forbid/post"); - mapping5.setConstraint(_allowPostConstraint); + mapping5.setConstraint(_noAuthConstraint); mapping5.setMethod("POST"); ConstraintMapping mapping5o = new ConstraintMapping(); mapping5o.setPathSpec("/forbid/post"); @@ -1836,7 +1836,11 @@ public void testForbidTraceAndOptions() throws Exception allowOmitOptions.setPathSpec("/"); allowOmitOptions.setConstraint(_relaxConstraint); - _security.setConstraintMappings(new ConstraintMapping[] {forbidTrace, allowOmitTrace, forbidOptions, allowOmitOptions}); + ConstraintMapping someConstraint = new ConstraintMapping(); + someConstraint.setPathSpec("/some/constaint/*"); + someConstraint.setConstraint(_noAuthConstraint); + + _security.setConstraintMappings(new ConstraintMapping[] {forbidTrace, allowOmitTrace, forbidOptions, allowOmitOptions, someConstraint}); _security.setAuthenticator(new BasicAuthenticator()); _server.start(); @@ -1852,6 +1856,12 @@ public void testForbidTraceAndOptions() throws Exception response = _connector.getResponse("GET /ctx/some/path HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 200 ")); + + response = _connector.getResponse("GET /ctx/some/constraint/info HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 200 ")); + + response = _connector.getResponse("OPTIONS /ctx/some/constraint/info HTTP/1.0\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 403 ")); } private static String authBase64(String authorization)