From 7955ebcb3de5caa6d3351b524cb328743ea3bc6c Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 23 Dec 2020 18:32:54 +0100 Subject: [PATCH] Issue #5824 Durable ConstraintMappings. Signed-off-by: Jan Bartel --- .../security/ConstraintSecurityHandler.java | 43 ++++++++++++++----- .../jetty/security/ConstraintTest.java | 43 +++++++++++++++++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java index 5396e0b363f9..a82984f798f8 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.UserIdentity; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.URIUtil; @@ -64,6 +65,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr private static final String OMISSION_SUFFIX = ".omission"; private static final String ALL_METHODS = "*"; private final List _constraintMappings = new CopyOnWriteArrayList<>(); + private final List _durableConstraintMappings = new CopyOnWriteArrayList<>(); private final Set _roles = new CopyOnWriteArraySet<>(); private final PathMap> _constraintMap = new PathMap<>(); private boolean _denyUncoveredMethods = false; @@ -308,8 +310,11 @@ public void setConstraintMappings(ConstraintMapping[] constraintMappings) @Override public void setConstraintMappings(List constraintMappings, Set roles) { + _durableConstraintMappings.clear(); _constraintMappings.clear(); - _constraintMappings.addAll(constraintMappings); + + if (isInDurableState()) + _durableConstraintMappings.addAll(constraintMappings); if (roles == null) { @@ -331,6 +336,7 @@ public void setConstraintMappings(List constraintMappings, Se if (isStarted()) { + _constraintMappings.addAll(constraintMappings); for (ConstraintMapping mapping : _constraintMappings) { processConstraintMapping(mapping); @@ -357,7 +363,9 @@ public void setRoles(Set roles) @Override public void addConstraintMapping(ConstraintMapping mapping) { - _constraintMappings.add(mapping); + if (isInDurableState()) + _durableConstraintMappings.add(mapping); + if (mapping.getConstraint() != null && mapping.getConstraint().getRoles() != null) { //allow for lazy role naming: if a role is named in a security constraint, try and @@ -372,6 +380,7 @@ public void addConstraintMapping(ConstraintMapping mapping) if (isStarted()) { + _constraintMappings.add(mapping); processConstraintMapping(mapping); } } @@ -404,14 +413,8 @@ public void addRole(String role) @Override protected void doStart() throws Exception { - _constraintMap.clear(); - if (_constraintMappings != null) - { - for (ConstraintMapping mapping : _constraintMappings) - { - processConstraintMapping(mapping); - } - } + _constraintMappings.addAll(_durableConstraintMappings); + _constraintMappings.stream().forEach(m -> processConstraintMapping(m)); //Servlet Spec 3.1 pg 147 sec 13.8.4.2 log paths for which there are uncovered http methods checkPathsWithUncoveredHttpMethods(); @@ -424,6 +427,7 @@ protected void doStop() throws Exception { super.doStop(); _constraintMap.clear(); + _constraintMappings.clear(); } /** @@ -857,4 +861,23 @@ protected Set getOmittedMethods(String omission) } return methods; } + + /** + * Constraints can be added to the ConstraintSecurityHandler before the + * associated context is started. These constraints should persist across + * a stop/start. Others can be added after the associated context is starting + * (eg by a web.xml/web-fragment.xml, annotation or javax.servlet api call) - + * these should not be persisted across a stop/start as they will be re-added on + * the restart. + * + * @return true if the context with which this ConstraintSecurityHandler + * has not yet started, or if there is no context, the server has not yet started. + */ + private boolean isInDurableState() + { + ContextHandler context = ContextHandler.getContextHandler(null); + Server server = getServer(); + + return (context == null && server == null) || (context != null && !context.isRunning()) || (context == null && server != null && !server.isRunning()); + } } 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 9dfd30810200..d60c22afe51c 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 @@ -64,6 +64,7 @@ import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Password; +import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -236,6 +237,48 @@ public void testConstraints() throws Exception assertFalse(mappings.get(3).getConstraint().getAuthenticate()); } + /** + * Test that constraint mappings added before the context starts are + * retained, but those that are added after the context starts are not. + * + * @throws Exception + */ + @Test + public void testDurableConstraints() throws Exception + { + _server.start(); + + List mappings = _security.getConstraintMappings(); + assertThat("after start", getConstraintMappings().size(), Matchers.equalTo(mappings.size())); + + _server.stop(); + + mappings = _security.getConstraintMappings(); + assertThat("after restart", 0, Matchers.equalTo(mappings.size())); + + _server.start(); + mappings = _security.getConstraintMappings(); + assertThat("after restart", getConstraintMappings().size(), Matchers.equalTo(mappings.size())); + + ConstraintMapping mapping = new ConstraintMapping(); + mapping.setPathSpec("/xxxx/*"); + Constraint constraint = new Constraint(); + constraint.setAuthenticate(false); + constraint.setName("transient"); + mapping.setConstraint(constraint); + + _security.addConstraintMapping(mapping); + + mappings = _security.getConstraintMappings(); + assertThat("after addition", getConstraintMappings().size() + 1, Matchers.equalTo(mappings.size())); + + _server.stop(); + _server.start(); + + mappings = _security.getConstraintMappings(); + assertThat("after addition", getConstraintMappings().size(), Matchers.equalTo(mappings.size())); + } + /** * Equivalent of Servlet Spec 3.1 pg 132, sec 13.4.1.1, Example 13-1 * @ServletSecurity