Skip to content

Commit

Permalink
Issue #5824 Durable ConstraintMappings. (#5842)
Browse files Browse the repository at this point in the history 
* Issue #5824 Durable ConstraintMappings.

Signed-off-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
@janbartel
janbartel committed Jan 11, 2021
1 parent 403d5ec commit 26ef233
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 19 deletions.
55 changes: 37 additions & 18 deletions jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -259,9 +261,6 @@ public static List<ConstraintMapping> createConstraintsWithMappingsForPath(Strin
return mappings;
}

/**
* @return Returns the constraintMappings.
*/
@Override
public List<ConstraintMapping> getConstraintMappings()
{
Expand Down Expand Up @@ -308,8 +307,15 @@ public void setConstraintMappings(ConstraintMapping[] constraintMappings)
@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)
{
Expand All @@ -331,10 +337,7 @@ public void setConstraintMappings(List<ConstraintMapping> constraintMappings, Se

if (isStarted())
{
for (ConstraintMapping mapping : _constraintMappings)
{
processConstraintMapping(mapping);
}
_constraintMappings.stream().forEach(m -> processConstraintMapping(m));
}
}

Expand All @@ -358,6 +361,10 @@ public void setRoles(Set<String> roles)
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
Expand All @@ -371,9 +378,7 @@ public void addConstraintMapping(ConstraintMapping mapping)
}

if (isStarted())
{
processConstraintMapping(mapping);
}
}

/**
Expand Down Expand Up @@ -404,14 +409,7 @@ public void addRole(String role)
@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();
Expand All @@ -424,7 +422,9 @@ protected void doStop() throws Exception
{
super.doStop();
_constraintMap.clear();
}
_constraintMappings.clear();
_constraintMappings.addAll(_durableConstraintMappings);
}

/**
* Create and combine the constraint with the existing processed
Expand Down Expand Up @@ -857,4 +857,23 @@ protected Set<String> 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());
}
}
75 changes: 74 additions & 1 deletion jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -236,6 +237,78 @@ 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
{
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
Expand Down Expand Up @@ -655,7 +728,7 @@ public static Stream<Arguments> basicScenarios()
@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);
Expand Down

0 comments on commit 26ef233

Please sign in to comment.