From 9bba1a1c6be9a891ec6633f373fd92fa7c46d376 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Mon, 27 Feb 2023 11:30:30 -0700 Subject: [PATCH] Propagate Variables in And and OrRequestMatcher Closes gh-12847 --- .../web/util/matcher/AndRequestMatcher.java | 26 +++++++++++++++- .../web/util/matcher/OrRequestMatcher.java | 21 ++++++++++++- .../util/matcher/AndRequestMatcherTests.java | 31 ++++++++++++++++++- .../util/matcher/OrRequestMatcherTests.java | 28 ++++++++++++++++- 4 files changed, 102 insertions(+), 4 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java index 383d390fd1..b28b69bbba 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/AndRequestMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,9 @@ package org.springframework.security.web.util.matcher; import java.util.Arrays; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; @@ -66,6 +68,28 @@ public final class AndRequestMatcher implements RequestMatcher { return true; } + /** + * Returns a {@link MatchResult} for this {@link HttpServletRequest}. In the case of a + * match, request variables are a composition of the request variables in underlying + * matchers. In the event that two matchers have the same key, the last key is the one + * propagated. + * @param request the HTTP request + * @return a {@link MatchResult} based on the given HTTP request + * @since 6.1 + */ + @Override + public MatchResult matcher(HttpServletRequest request) { + Map variables = new LinkedHashMap<>(); + for (RequestMatcher matcher : this.requestMatchers) { + MatchResult result = matcher.matcher(request); + if (!result.isMatch()) { + return MatchResult.notMatch(); + } + variables.putAll(result.getVariables()); + } + return MatchResult.match(variables); + } + @Override public String toString() { return "And " + this.requestMatchers; diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java index 3082d311c4..e3add8edf3 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/OrRequestMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -62,6 +62,25 @@ public final class OrRequestMatcher implements RequestMatcher { return false; } + /** + * Returns a {@link MatchResult} for this {@link HttpServletRequest}. In the case of a + * match, request variables are any request variables from the first underlying + * matcher. + * @param request the HTTP request + * @return a {@link MatchResult} based on the given HTTP request + * @since 6.1 + */ + @Override + public MatchResult matcher(HttpServletRequest request) { + for (RequestMatcher matcher : this.requestMatchers) { + MatchResult result = matcher.matcher(request); + if (result.isMatch()) { + return result; + } + } + return MatchResult.notMatch(); + } + @Override public String toString() { return "Or " + this.requestMatchers; diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java index 7c8240116f..9888aaf10e 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/AndRequestMatcherTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ package org.springframework.security.web.util.matcher; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import jakarta.servlet.http.HttpServletRequest; import org.junit.jupiter.api.Test; @@ -26,6 +27,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.web.util.matcher.RequestMatcher.MatchResult; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatNullPointerException; @@ -123,4 +126,30 @@ public class AndRequestMatcherTests { assertThat(this.matcher.matches(this.request)).isFalse(); } + @Test + public void matcherWhenMatchersHavePlaceholdersThenPropagatesMatches() { + this.matcher = new AndRequestMatcher(this.delegate, this.delegate2); + + given(this.delegate.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "value"))); + given(this.delegate2.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "othervalue"))); + MatchResult result = this.matcher.matcher(this.request); + assertThat(result.getVariables()).containsExactlyEntriesOf(Map.of("param", "othervalue")); + + given(this.delegate.matcher(this.request)).willReturn(MatchResult.match()); + given(this.delegate2.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "value"))); + result = this.matcher.matcher(this.request); + assertThat(result.getVariables()).containsExactlyEntriesOf(Map.of("param", "value")); + + given(this.delegate.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "value"))); + given(this.delegate2.matcher(this.request)).willReturn(MatchResult.notMatch()); + result = this.matcher.matcher(this.request); + assertThat(result.getVariables()).isEmpty(); + + given(this.delegate.matcher(this.request)).willReturn(MatchResult.match(Map.of("otherparam", "value"))); + given(this.delegate2.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "value"))); + result = this.matcher.matcher(this.request); + assertThat(result.getVariables()) + .containsExactlyInAnyOrderEntriesOf(Map.of("otherparam", "value", "param", "value")); + } + } diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java index 6ee601179a..bf94bf5de9 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/OrRequestMatcherTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ package org.springframework.security.web.util.matcher; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import jakarta.servlet.http.HttpServletRequest; import org.junit.jupiter.api.Test; @@ -26,10 +27,13 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.web.util.matcher.RequestMatcher.MatchResult; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatNullPointerException; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.verifyNoInteractions; /** * @author Rob Winch @@ -122,4 +126,26 @@ public class OrRequestMatcherTests { assertThat(this.matcher.matches(this.request)).isTrue(); } + @Test + public void matcherWhenMatchersHavePlaceholdersThenPropagatesFirstMatch() { + this.matcher = new OrRequestMatcher(this.delegate, this.delegate2); + + given(this.delegate.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "value"))); + given(this.delegate2.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "othervalue"))); + MatchResult result = this.matcher.matcher(this.request); + assertThat(result.getVariables()).containsExactlyEntriesOf(Map.of("param", "value")); + verifyNoInteractions(this.delegate2); + + given(this.delegate.matcher(this.request)).willReturn(MatchResult.match()); + given(this.delegate2.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "value"))); + result = this.matcher.matcher(this.request); + assertThat(result.getVariables()).isEmpty(); + verifyNoInteractions(this.delegate2); + + given(this.delegate.matcher(this.request)).willReturn(MatchResult.notMatch()); + given(this.delegate2.matcher(this.request)).willReturn(MatchResult.match(Map.of("param", "value"))); + result = this.matcher.matcher(this.request); + assertThat(result.getVariables()).containsExactlyEntriesOf(Map.of("param", "value")); + } + }