diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java index 2917f00f80..60a384ebd6 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurer.java @@ -16,7 +16,6 @@ package org.springframework.security.config.annotation.web.configurers; -import java.util.LinkedHashMap; import java.util.List; import javax.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.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcherEntry; import org.springframework.util.Assert; /** @@ -129,14 +129,7 @@ public final class AuthorizeHttpRequestsConfigurer manager) { this.unmappedMatchers = null; - this.managerBuilder.mappings((m) -> { - LinkedHashMap> reorderedMap = new LinkedHashMap<>( - m.size() + 1); - reorderedMap.put(matcher, manager); - reorderedMap.putAll(m); - m.clear(); - m.putAll(reorderedMap); - }); + this.managerBuilder.mappings((m) -> m.add(0, new RequestMatcherEntry<>(matcher, manager))); this.mappingCount++; } diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java b/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java index 5a7266964b..e19285c1ef 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManager.java @@ -16,8 +16,8 @@ package org.springframework.security.web.access.intercept; -import java.util.LinkedHashMap; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; import java.util.function.Consumer; import java.util.function.Supplier; @@ -32,6 +32,7 @@ import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.core.Authentication; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher.MatchResult; +import org.springframework.security.web.util.matcher.RequestMatcherEntry; import org.springframework.util.Assert; /** @@ -46,10 +47,10 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho private final Log logger = LogFactory.getLog(getClass()); - private final Map> mappings; + private final List>> mappings; private RequestMatcherDelegatingAuthorizationManager( - Map> mappings) { + List>> mappings) { Assert.notEmpty(mappings, "mappings cannot be empty"); this.mappings = mappings; } @@ -68,13 +69,12 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho if (this.logger.isTraceEnabled()) { this.logger.trace(LogMessage.format("Authorizing %s", request)); } - for (Map.Entry> mapping : this.mappings - .entrySet()) { + for (RequestMatcherEntry> mapping : this.mappings) { - RequestMatcher matcher = mapping.getKey(); + RequestMatcher matcher = mapping.getRequestMatcher(); MatchResult matchResult = matcher.matcher(request); if (matchResult.isMatch()) { - AuthorizationManager manager = mapping.getValue(); + AuthorizationManager manager = mapping.getEntry(); if (this.logger.isTraceEnabled()) { this.logger.trace(LogMessage.format("Checking authorization on %s using %s", request, manager)); } @@ -99,7 +99,7 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho */ public static final class Builder { - private final Map> mappings = new LinkedHashMap<>(); + private final List>> mappings = new ArrayList<>(); /** * Maps a {@link RequestMatcher} to an {@link AuthorizationManager}. @@ -110,7 +110,7 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho public Builder add(RequestMatcher matcher, AuthorizationManager manager) { Assert.notNull(matcher, "matcher cannot be null"); Assert.notNull(manager, "manager cannot be null"); - this.mappings.put(matcher, manager); + this.mappings.add(new RequestMatcherEntry<>(matcher, manager)); return this; } @@ -123,7 +123,7 @@ public final class RequestMatcherDelegatingAuthorizationManager implements Autho * @since 5.7 */ public Builder mappings( - Consumer>> mappingsConsumer) { + Consumer>>> mappingsConsumer) { Assert.notNull(mappingsConsumer, "mappingsConsumer cannot be null"); mappingsConsumer.accept(this.mappings); return this; diff --git a/web/src/test/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManagerTests.java b/web/src/test/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManagerTests.java index a67c1740df..624a6ecee8 100644 --- a/web/src/test/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManagerTests.java +++ b/web/src/test/java/org/springframework/security/web/access/intercept/RequestMatcherDelegatingAuthorizationManagerTests.java @@ -27,6 +27,7 @@ import org.springframework.security.authorization.AuthorizationDecision; import org.springframework.security.core.Authentication; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; 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.assertThatIllegalArgumentException; @@ -90,10 +91,12 @@ public class RequestMatcherDelegatingAuthorizationManagerTests { public void checkWhenMultipleMappingsConfiguredWithConsumerThenDelegatesMatchingManager() { RequestMatcherDelegatingAuthorizationManager manager = RequestMatcherDelegatingAuthorizationManager.builder() .mappings((m) -> { - m.put(new MvcRequestMatcher(null, "/grant"), (a, o) -> new AuthorizationDecision(true)); - m.put(AnyRequestMatcher.INSTANCE, AuthorityAuthorizationManager.hasRole("ADMIN")); - m.put(new MvcRequestMatcher(null, "/deny"), (a, o) -> new AuthorizationDecision(false)); - m.put(new MvcRequestMatcher(null, "/afterAny"), (a, o) -> new AuthorizationDecision(true)); + m.add(new RequestMatcherEntry<>(new MvcRequestMatcher(null, "/grant"), + (a, o) -> new AuthorizationDecision(true))); + m.add(new RequestMatcherEntry<>(AnyRequestMatcher.INSTANCE, + AuthorityAuthorizationManager.hasRole("ADMIN"))); + m.add(new RequestMatcherEntry<>(new MvcRequestMatcher(null, "/afterAny"), + (a, o) -> new AuthorizationDecision(true))); }).build(); Supplier authentication = () -> new TestingAuthenticationToken("user", "password", "ROLE_USER"); @@ -103,10 +106,6 @@ public class RequestMatcherDelegatingAuthorizationManagerTests { assertThat(grant).isNotNull(); 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")); assertThat(afterAny).isNotNull(); assertThat(afterAny.isGranted()).isFalse();