Propagate Variables in And and OrRequestMatcher

Closes gh-12847

Author: jzheaux

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 boolean matches(HttpServletRequest request) {
 		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<String, String> 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;

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 boolean matches(HttpServletRequest request) {
 		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;

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,13 +19,16 @@
 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;
 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 void matchesMultiSingleFalse() {
 		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"));
+	}
+
 }

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,17 +19,21 @@
 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;
 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 void matchesMultiSingleFalse() {
 		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"));
+	}
+
 }