Use RequestMatcherEntry

Closes gh-11046
This commit is contained in:
Josh Cummings 2022-03-30 14:31:11 -06:00
parent 51a99701ad
commit 1edfa07d27
No known key found for this signature in database
GPG Key ID: A306A51F43B8E5A5
3 changed files with 20 additions and 28 deletions

View File

@ -16,7 +16,6 @@
package org.springframework.security.config.annotation.web.configurers; package org.springframework.security.config.annotation.web.configurers;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequest;
@ -37,6 +36,7 @@ import org.springframework.security.web.access.intercept.RequestAuthorizationCon
import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager; import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager;
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcherEntry;
import org.springframework.util.Assert; import org.springframework.util.Assert;
/** /**
@ -129,14 +129,7 @@ public final class AuthorizeHttpRequestsConfigurer<H extends HttpSecurityBuilder
private void addFirst(RequestMatcher matcher, AuthorizationManager<RequestAuthorizationContext> manager) { private void addFirst(RequestMatcher matcher, AuthorizationManager<RequestAuthorizationContext> manager) {
this.unmappedMatchers = null; this.unmappedMatchers = null;
this.managerBuilder.mappings((m) -> { this.managerBuilder.mappings((m) -> m.add(0, new RequestMatcherEntry<>(matcher, manager)));
LinkedHashMap<RequestMatcher, AuthorizationManager<RequestAuthorizationContext>> reorderedMap = new LinkedHashMap<>(
m.size() + 1);
reorderedMap.put(matcher, manager);
reorderedMap.putAll(m);
m.clear();
m.putAll(reorderedMap);
});
this.mappingCount++; this.mappingCount++;
} }

View File

@ -16,8 +16,8 @@
package org.springframework.security.web.access.intercept; package org.springframework.security.web.access.intercept;
import java.util.LinkedHashMap; import java.util.ArrayList;
import java.util.Map; import java.util.List;
import java.util.function.Consumer; import java.util.function.Consumer;
import java.util.function.Supplier; import java.util.function.Supplier;
@ -31,6 +31,7 @@ import org.springframework.security.authorization.AuthorizationManager;
import org.springframework.security.core.Authentication; import org.springframework.security.core.Authentication;
import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher.MatchResult; import org.springframework.security.web.util.matcher.RequestMatcher.MatchResult;
import org.springframework.security.web.util.matcher.RequestMatcherEntry;
import org.springframework.util.Assert; import org.springframework.util.Assert;
/** /**
@ -45,10 +46,10 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho
private final Log logger = LogFactory.getLog(getClass()); private final Log logger = LogFactory.getLog(getClass());
private final Map<RequestMatcher, AuthorizationManager<RequestAuthorizationContext>> mappings; private final List<RequestMatcherEntry<AuthorizationManager<RequestAuthorizationContext>>> mappings;
private RequestMatcherDelegatingAuthorizationManager( private RequestMatcherDelegatingAuthorizationManager(
Map<RequestMatcher, AuthorizationManager<RequestAuthorizationContext>> mappings) { List<RequestMatcherEntry<AuthorizationManager<RequestAuthorizationContext>>> mappings) {
Assert.notEmpty(mappings, "mappings cannot be empty"); Assert.notEmpty(mappings, "mappings cannot be empty");
this.mappings = mappings; this.mappings = mappings;
} }
@ -67,13 +68,12 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho
if (this.logger.isTraceEnabled()) { if (this.logger.isTraceEnabled()) {
this.logger.trace(LogMessage.format("Authorizing %s", request)); this.logger.trace(LogMessage.format("Authorizing %s", request));
} }
for (Map.Entry<RequestMatcher, AuthorizationManager<RequestAuthorizationContext>> mapping : this.mappings for (RequestMatcherEntry<AuthorizationManager<RequestAuthorizationContext>> mapping : this.mappings) {
.entrySet()) {
RequestMatcher matcher = mapping.getKey(); RequestMatcher matcher = mapping.getRequestMatcher();
MatchResult matchResult = matcher.matcher(request); MatchResult matchResult = matcher.matcher(request);
if (matchResult.isMatch()) { if (matchResult.isMatch()) {
AuthorizationManager<RequestAuthorizationContext> manager = mapping.getValue(); AuthorizationManager<RequestAuthorizationContext> manager = mapping.getEntry();
if (this.logger.isTraceEnabled()) { if (this.logger.isTraceEnabled()) {
this.logger.trace(LogMessage.format("Checking authorization on %s using %s", request, manager)); this.logger.trace(LogMessage.format("Checking authorization on %s using %s", request, manager));
} }
@ -98,7 +98,7 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho
*/ */
public static final class Builder { public static final class Builder {
private final Map<RequestMatcher, AuthorizationManager<RequestAuthorizationContext>> mappings = new LinkedHashMap<>(); private final List<RequestMatcherEntry<AuthorizationManager<RequestAuthorizationContext>>> mappings = new ArrayList<>();
/** /**
* Maps a {@link RequestMatcher} to an {@link AuthorizationManager}. * Maps a {@link RequestMatcher} to an {@link AuthorizationManager}.
@ -109,7 +109,7 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho
public Builder add(RequestMatcher matcher, AuthorizationManager<RequestAuthorizationContext> manager) { public Builder add(RequestMatcher matcher, AuthorizationManager<RequestAuthorizationContext> manager) {
Assert.notNull(matcher, "matcher cannot be null"); Assert.notNull(matcher, "matcher cannot be null");
Assert.notNull(manager, "manager cannot be null"); Assert.notNull(manager, "manager cannot be null");
this.mappings.put(matcher, manager); this.mappings.add(new RequestMatcherEntry<>(matcher, manager));
return this; return this;
} }
@ -122,7 +122,7 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho
* @since 5.7 * @since 5.7
*/ */
public Builder mappings( public Builder mappings(
Consumer<Map<RequestMatcher, AuthorizationManager<RequestAuthorizationContext>>> mappingsConsumer) { Consumer<List<RequestMatcherEntry<AuthorizationManager<RequestAuthorizationContext>>>> mappingsConsumer) {
Assert.notNull(mappingsConsumer, "mappingsConsumer cannot be null"); Assert.notNull(mappingsConsumer, "mappingsConsumer cannot be null");
mappingsConsumer.accept(this.mappings); mappingsConsumer.accept(this.mappings);
return this; return this;

View File

@ -27,6 +27,7 @@ import org.springframework.security.authorization.AuthorizationDecision;
import org.springframework.security.core.Authentication; import org.springframework.security.core.Authentication;
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
import org.springframework.security.web.util.matcher.AnyRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcherEntry;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@ -90,10 +91,12 @@ public class RequestMatcherDelegatingAuthorizationManagerTests {
public void checkWhenMultipleMappingsConfiguredWithConsumerThenDelegatesMatchingManager() { public void checkWhenMultipleMappingsConfiguredWithConsumerThenDelegatesMatchingManager() {
RequestMatcherDelegatingAuthorizationManager manager = RequestMatcherDelegatingAuthorizationManager.builder() RequestMatcherDelegatingAuthorizationManager manager = RequestMatcherDelegatingAuthorizationManager.builder()
.mappings((m) -> { .mappings((m) -> {
m.put(new MvcRequestMatcher(null, "/grant"), (a, o) -> new AuthorizationDecision(true)); m.add(new RequestMatcherEntry<>(new MvcRequestMatcher(null, "/grant"),
m.put(AnyRequestMatcher.INSTANCE, AuthorityAuthorizationManager.hasRole("ADMIN")); (a, o) -> new AuthorizationDecision(true)));
m.put(new MvcRequestMatcher(null, "/deny"), (a, o) -> new AuthorizationDecision(false)); m.add(new RequestMatcherEntry<>(AnyRequestMatcher.INSTANCE,
m.put(new MvcRequestMatcher(null, "/afterAny"), (a, o) -> new AuthorizationDecision(true)); AuthorityAuthorizationManager.hasRole("ADMIN")));
m.add(new RequestMatcherEntry<>(new MvcRequestMatcher(null, "/afterAny"),
(a, o) -> new AuthorizationDecision(true)));
}).build(); }).build();
Supplier<Authentication> authentication = () -> new TestingAuthenticationToken("user", "password", "ROLE_USER"); Supplier<Authentication> authentication = () -> new TestingAuthenticationToken("user", "password", "ROLE_USER");
@ -103,10 +106,6 @@ public class RequestMatcherDelegatingAuthorizationManagerTests {
assertThat(grant).isNotNull(); assertThat(grant).isNotNull();
assertThat(grant.isGranted()).isTrue(); assertThat(grant.isGranted()).isTrue();
AuthorizationDecision deny = manager.check(authentication, new MockHttpServletRequest(null, "/deny"));
assertThat(deny).isNotNull();
assertThat(deny.isGranted()).isFalse();
AuthorizationDecision afterAny = manager.check(authentication, new MockHttpServletRequest(null, "/afterAny")); AuthorizationDecision afterAny = manager.check(authentication, new MockHttpServletRequest(null, "/afterAny"));
assertThat(afterAny).isNotNull(); assertThat(afterAny).isNotNull();
assertThat(afterAny.isGranted()).isFalse(); assertThat(afterAny.isGranted()).isFalse();