Issue #5824 Durable ConstraintMappings. (#5842)

* Issue #5824 Durable ConstraintMappings.

Signed-off-by: Jan Bartel <janb@webtide.com>
This commit is contained in:
Jan Bartel 2021-01-11 10:30:23 +01:00 committed by GitHub
parent 403d5ec318
commit 26ef233e94
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 111 additions and 19 deletions

View File

@ -42,6 +42,7 @@ import org.eclipse.jetty.http.PathMap;
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<ConstraintMapping> _constraintMappings = new CopyOnWriteArrayList<>();
private final List<ConstraintMapping> _durableConstraintMappings = new CopyOnWriteArrayList<>();
private final Set<String> _roles = new CopyOnWriteArraySet<>();
private final PathMap<Map<String, RoleInfo>> _constraintMap = new PathMap<>();
private boolean _denyUncoveredMethods = false;
@ -259,9 +261,6 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
return mappings;
}
/**
* @return Returns the constraintMappings.
*/
@Override
public List<ConstraintMapping> getConstraintMappings()
{
@ -308,8 +307,15 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
@Override
public void setConstraintMappings(List<ConstraintMapping> constraintMappings, Set<String> roles)
{
_constraintMappings.clear();
_constraintMappings.addAll(constraintMappings);
_durableConstraintMappings.clear();
if (isInDurableState())
{
_durableConstraintMappings.addAll(constraintMappings);
}
if (roles == null)
{
@ -331,10 +337,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
if (isStarted())
{
for (ConstraintMapping mapping : _constraintMappings)
{
processConstraintMapping(mapping);
}
_constraintMappings.stream().forEach(m -> processConstraintMapping(m));
}
}
@ -358,6 +361,10 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
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
@ -371,9 +378,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
}
if (isStarted())
{
processConstraintMapping(mapping);
}
}
/**
@ -404,14 +409,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
@Override
protected void doStart() throws Exception
{
_constraintMap.clear();
if (_constraintMappings != null)
{
for (ConstraintMapping mapping : _constraintMappings)
{
processConstraintMapping(mapping);
}
}
_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,7 +422,9 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
{
super.doStop();
_constraintMap.clear();
}
_constraintMappings.clear();
_constraintMappings.addAll(_durableConstraintMappings);
}
/**
* Create and combine the constraint with the existing processed
@ -857,4 +857,23 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
}
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());
}
}

View File

@ -64,6 +64,7 @@ import org.eclipse.jetty.util.StringUtil;
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,78 @@ public class ConstraintTest
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
{
List<ConstraintMapping> mappings = _security.getConstraintMappings();
assertThat("before start", getConstraintMappings().size(), Matchers.equalTo(mappings.size()));
_server.start();
mappings = _security.getConstraintMappings();
assertThat("after start", getConstraintMappings().size(), Matchers.equalTo(mappings.size()));
_server.stop();
//After a stop, just the durable mappings are left
mappings = _security.getConstraintMappings();
assertThat("after stop", getConstraintMappings().size(), Matchers.equalTo(mappings.size()));
_server.start();
//Verify the constraints are just the durables
mappings = _security.getConstraintMappings();
assertThat("after restart", getConstraintMappings().size(), Matchers.equalTo(mappings.size()));
//Add a non-durable constraint
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();
//After a stop, only the durable mappings remain
mappings = _security.getConstraintMappings();
assertThat("after addition", getConstraintMappings().size(), Matchers.equalTo(mappings.size()));
//test that setConstraintMappings replaces all existing mappings whether durable or not
//test setConstraintMappings in durable state
_server.stop();
_security.setConstraintMappings(Collections.singletonList(mapping));
mappings = _security.getConstraintMappings();
assertThat("after set during stop", 1, Matchers.equalTo(mappings.size()));
_server.start();
mappings = _security.getConstraintMappings();
assertThat("after set after start", 1, Matchers.equalTo(mappings.size()));
//test setConstraintMappings not in durable state
_server.stop();
_server.start();
assertThat("no change after start", 1, Matchers.equalTo(mappings.size()));
_security.setConstraintMappings(getConstraintMappings());
mappings = _security.getConstraintMappings();
assertThat("durables lost", getConstraintMappings().size(), Matchers.equalTo(mappings.size()));
_server.stop();
mappings = _security.getConstraintMappings();
assertThat("no mappings", 0, Matchers.equalTo(mappings.size()));
}
/**
* Equivalent of Servlet Spec 3.1 pg 132, sec 13.4.1.1, Example 13-1
* &#064;ServletSecurity
@ -655,7 +728,7 @@ public class ConstraintTest
@MethodSource("basicScenarios")
public void testBasic(Scenario scenario) throws Exception
{
List<ConstraintMapping> list = new ArrayList<>(_security.getConstraintMappings());
List<ConstraintMapping> list = new ArrayList<>(getConstraintMappings());
Constraint constraint6 = new Constraint();
constraint6.setAuthenticate(true);