Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #5824 Durable ConstraintMappings. #5842

Merged
merged 5 commits into from Jan 11, 2021
Merged

Issue #5824 Durable ConstraintMappings. #5842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7955ebc
Issue #5824 Durable ConstraintMappings.
janbartel Dec 23, 2020
9d328ce
Issue #5824 Fix ConstraintTest
janbartel Dec 23, 2020
39d7083
Issue #5824 Return only durable mappings before ConstraintSecurityHan…
janbartel Dec 24, 2020
60b3508
Issue #5824 Change after review
janbartel Dec 30, 2020
65e53aa
Issue #5824 Changes after review
janbartel Dec 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 37 additions & 18 deletions jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
View file
Open in desktop
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
Open in desktop
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