From 8c358561f2032e68c5b07d2b86170e110fe84950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Thu, 25 Jul 2019 21:15:21 -0500 Subject: [PATCH 1/3] Add match result for servlet requests Fixes gh-7148 --- ...ultOAuth2AuthorizationRequestResolver.java | 3 +- ...ilterInvocationSecurityMetadataSource.java | 5 +- .../util/matcher/MvcRequestMatcher.java | 25 +++++--- .../util/matcher/AntPathRequestMatcher.java | 12 +++- .../web/util/matcher/RequestMatcher.java | 63 ++++++++++++++++++- .../matcher/RequestVariablesExtractor.java | 3 +- .../util/matcher/MvcRequestMatcherTests.java | 9 ++- 7 files changed, 104 insertions(+), 16 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java index 81e423b2020..84e2ec47be8 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java @@ -50,6 +50,7 @@ * * @author Joe Grandja * @author Rob Winch + * @author Eddú Meléndez * @since 5.1 * @see OAuth2AuthorizationRequestResolver * @see OAuth2AuthorizationRequestRedirectFilter @@ -147,7 +148,7 @@ private OAuth2AuthorizationRequest resolve(HttpServletRequest request, String re private String resolveRegistrationId(HttpServletRequest request) { if (this.authorizationRequestMatcher.matches(request)) { return this.authorizationRequestMatcher - .extractUriTemplateVariables(request).get(REGISTRATION_ID_URI_VARIABLE_NAME); + .matcher(request).getVariables().get(REGISTRATION_ID_URI_VARIABLE_NAME); } return null; } diff --git a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java index c8938715745..da0f3e35f75 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -40,6 +40,7 @@ * Expression-based {@code FilterInvocationSecurityMetadataSource}. * * @author Luke Taylor + * @author Eddú Meléndez * @since 3.0 */ public final class ExpressionBasedFilterInvocationSecurityMetadataSource @@ -111,7 +112,7 @@ public AntPathMatcherEvaluationContextPostProcessor( @Override Map extractVariables(HttpServletRequest request) { - return this.matcher.extractUriTemplateVariables(request); + return this.matcher.matcher(request).getVariables(); } } diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java index a455d1ca7a9..168a77c6817 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2019 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. @@ -16,7 +16,6 @@ package org.springframework.security.web.servlet.util.matcher; -import java.util.Collections; import java.util.Map; import javax.servlet.http.HttpServletRequest; @@ -43,6 +42,7 @@ *

* * @author Rob Winch + * @author Eddú Meléndez * @since 4.1.1 */ public class MvcRequestMatcher implements RequestMatcher, RequestVariablesExtractor { @@ -93,13 +93,18 @@ private MatchableHandlerMapping getMapping(HttpServletRequest request) { */ @Override public Map extractUriTemplateVariables(HttpServletRequest request) { + return matcher(request).getVariables(); + } + + @Override + public MatchResult matcher(HttpServletRequest request) { MatchableHandlerMapping mapping = getMapping(request); if (mapping == null) { - return this.defaultMatcher.extractUriTemplateVariables(request); + return this.defaultMatcher.matcher(request); } RequestMatchResult result = mapping.match(request, this.pattern); - return result == null ? Collections.emptyMap() - : result.extractUriTemplateVariables(); + return result == null ? MatchResult.notMatch() + : MatchResult.match(result.extractUriTemplateVariables()); } /** @@ -160,12 +165,18 @@ private boolean matches(String lookupPath) { @Override public Map extractUriTemplateVariables( HttpServletRequest request) { + return matcher(request).getVariables(); + } + + @Override + public MatchResult matcher(HttpServletRequest request) { String lookupPath = this.pathHelper.getLookupPathForRequest(request); if (matches(lookupPath)) { - return this.pathMatcher.extractUriTemplateVariables( + Map variables = this.pathMatcher.extractUriTemplateVariables( MvcRequestMatcher.this.pattern, lookupPath); + return MatchResult.match(variables); } - return Collections.emptyMap(); + return MatchResult.notMatch(); } } } diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java index f734b4c6d22..7861e983ef9 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -49,6 +49,7 @@ * * @author Luke Taylor * @author Rob Winch + * @author Eddú Meléndez * @since 3.1 * * @see org.springframework.util.AntPathMatcher @@ -182,11 +183,16 @@ public boolean matches(HttpServletRequest request) { @Override public Map extractUriTemplateVariables(HttpServletRequest request) { + return matcher(request).getVariables(); + } + + @Override + public MatchResult matcher(HttpServletRequest request) { if (this.matcher == null || !matches(request)) { - return Collections.emptyMap(); + return MatchResult.notMatch(); } String url = getRequestPath(request); - return this.matcher.extractUriTemplateVariables(url); + return MatchResult.match(this.matcher.extractUriTemplateVariables(url)); } private String getRequestPath(HttpServletRequest request) { diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java index 84a5018eac1..61ce2d08428 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2019 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. @@ -15,12 +15,16 @@ */ package org.springframework.security.web.util.matcher; +import java.util.Collections; +import java.util.Map; + import javax.servlet.http.HttpServletRequest; /** * Simple strategy to match an HttpServletRequest. * * @author Luke Taylor + * @author Eddú Meléndez * @since 3.0.2 */ public interface RequestMatcher { @@ -33,4 +37,61 @@ public interface RequestMatcher { */ boolean matches(HttpServletRequest request); + /** + * @since 5.2 + */ + default MatchResult matcher(HttpServletRequest request) { + boolean match = matches(request); + return new MatchResult(match, Collections.emptyMap()); + } + + /** + * The result of matching + */ + class MatchResult { + private final boolean match; + private final Map variables; + + MatchResult(boolean match, Map variables) { + this.match = match; + this.variables = variables; + } + + public boolean isMatch() { + return this.match; + } + + public Map getVariables() { + return this.variables; + } + + /** + * Creates an instance of {@link MatchResult} that is a match with no variables + * + * @return + */ + public static MatchResult match() { + return new MatchResult(true, Collections.emptyMap()); + } + + /** + * Creates an instance of {@link MatchResult} that is a match with the specified variables + * + * @param variables + * @return + */ + public static MatchResult match(Map variables) { + return new MatchResult(true, variables); + } + + /** + * Creates an instance of {@link MatchResult} that is not a match. + * + * @return + */ + public static MatchResult notMatch() { + return new MatchResult(false, Collections.emptyMap()); + } + } + } diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java index 93d4dbe83a1..c3444d0e70c 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2019 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. @@ -25,6 +25,7 @@ * * @author Rob Winch * @since 4.1.1 + * @deprecated */ public interface RequestVariablesExtractor { diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherTests.java index 7e49daf1aa2..591d3fe539f 100644 --- a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2019 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. @@ -40,6 +40,7 @@ /** * @author Rob Winch + * @author Eddú Meléndez */ @RunWith(MockitoJUnitRunner.class) public class MvcRequestMatcherTests { @@ -73,6 +74,8 @@ public void extractUriTemplateVariablesSuccess() throws Exception { assertThat(this.matcher.extractUriTemplateVariables(this.request)) .containsEntry("p", "path"); + assertThat(this.matcher.matcher(this.request).getVariables()) + .containsEntry("p", "path"); } @Test @@ -85,6 +88,7 @@ public void extractUriTemplateVariablesFail() throws Exception { .thenReturn(this.result); assertThat(this.matcher.extractUriTemplateVariables(this.request)).isEmpty(); + assertThat(this.matcher.matcher(this.request).getVariables()).isEmpty(); } @Test @@ -94,6 +98,8 @@ public void extractUriTemplateVariablesDefaultSuccess() throws Exception { assertThat(this.matcher.extractUriTemplateVariables(this.request)) .containsEntry("p", "path"); + assertThat(this.matcher.matcher(this.request).getVariables()) + .containsEntry("p", "path"); } @Test @@ -102,6 +108,7 @@ public void extractUriTemplateVariablesDefaultFail() throws Exception { when(this.introspector.getMatchableHandlerMapping(this.request)).thenReturn(null); assertThat(this.matcher.extractUriTemplateVariables(this.request)).isEmpty(); + assertThat(this.matcher.matcher(this.request).getVariables()).isEmpty(); } @Test From fc26d11a2bf76255f9d528a7cb8ac02f7191c73f Mon Sep 17 00:00:00 2001 From: Filip Hanik Date: Thu, 1 Aug 2019 10:52:02 -0700 Subject: [PATCH 2/3] Add RequestMatcher.matcher(HttpServletRequest) Step 3 - Usage of RequestVariablesExtractor or types that are assigned to AntPathRequestMatcher should be replaced with the new method. [closes #7148] --- ...edFilterInvocationSecurityMetadataSource.java | 16 +++++----------- .../servlet/util/matcher/MvcRequestMatcher.java | 9 ++------- .../web/util/matcher/AntPathRequestMatcher.java | 3 ++- .../util/matcher/RequestVariablesExtractor.java | 2 +- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java index da0f3e35f75..02b2183d128 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java @@ -33,7 +33,6 @@ import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.security.web.util.matcher.RequestVariablesExtractor; import org.springframework.util.Assert; /** @@ -92,13 +91,8 @@ private static LinkedHashMap> proces return requestToExpressionAttributesMap; } - private static AbstractVariableEvaluationContextPostProcessor createPostProcessor( - Object request) { - if (request instanceof RequestVariablesExtractor) { - return new RequestVariablesExtractorEvaluationContextPostProcessor( - (RequestVariablesExtractor) request); - } - return null; + private static AbstractVariableEvaluationContextPostProcessor createPostProcessor(RequestMatcher request) { + return new RequestVariablesExtractorEvaluationContextPostProcessor(request); } static class AntPathMatcherEvaluationContextPostProcessor @@ -118,16 +112,16 @@ Map extractVariables(HttpServletRequest request) { static class RequestVariablesExtractorEvaluationContextPostProcessor extends AbstractVariableEvaluationContextPostProcessor { - private final RequestVariablesExtractor matcher; + private final RequestMatcher matcher; public RequestVariablesExtractorEvaluationContextPostProcessor( - RequestVariablesExtractor matcher) { + RequestMatcher matcher) { this.matcher = matcher; } @Override Map extractVariables(HttpServletRequest request) { - return this.matcher.extractUriTemplateVariables(request); + return this.matcher.matcher(request).getVariables(); } } diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java index 168a77c6817..7a667df5898 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java @@ -92,6 +92,7 @@ private MatchableHandlerMapping getMapping(HttpServletRequest request) { * extractUriTemplateVariables(javax.servlet.http.HttpServletRequest) */ @Override + @Deprecated public Map extractUriTemplateVariables(HttpServletRequest request) { return matcher(request).getVariables(); } @@ -146,7 +147,7 @@ public String toString() { return sb.toString(); } - private class DefaultMatcher implements RequestMatcher, RequestVariablesExtractor { + private class DefaultMatcher implements RequestMatcher { private final UrlPathHelper pathHelper = new UrlPathHelper(); @@ -162,12 +163,6 @@ private boolean matches(String lookupPath) { return this.pathMatcher.match(MvcRequestMatcher.this.pattern, lookupPath); } - @Override - public Map extractUriTemplateVariables( - HttpServletRequest request) { - return matcher(request).getVariables(); - } - @Override public MatchResult matcher(HttpServletRequest request) { String lookupPath = this.pathHelper.getLookupPathForRequest(request); diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java index 7861e983ef9..b77be2390b7 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java @@ -182,6 +182,7 @@ public boolean matches(HttpServletRequest request) { } @Override + @Deprecated public Map extractUriTemplateVariables(HttpServletRequest request) { return matcher(request).getVariables(); } @@ -264,7 +265,7 @@ private static HttpMethod valueOf(String method) { return null; } - private static interface Matcher { + private interface Matcher { boolean matches(String path); Map extractUriTemplateVariables(String path); diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java index c3444d0e70c..db26547e0d0 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestVariablesExtractor.java @@ -25,7 +25,7 @@ * * @author Rob Winch * @since 4.1.1 - * @deprecated + * @deprecated use {@link RequestMatcher.MatchResult} from {@link RequestMatcher#matcher(HttpServletRequest)} */ public interface RequestVariablesExtractor { From e1612ec166df675d7034eb7632265747775f70dc Mon Sep 17 00:00:00 2001 From: Filip Hanik Date: Fri, 2 Aug 2019 12:29:30 -0700 Subject: [PATCH 3/3] Add Javadoc --- .../web/util/matcher/RequestMatcher.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java index 61ce2d08428..ef8d3076d81 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcher.java @@ -38,7 +38,12 @@ public interface RequestMatcher { boolean matches(HttpServletRequest request); /** - * @since 5.2 + * Returns a MatchResult for this RequestMatcher + * The default implementation returns {@link Collections#emptyMap()} + * when {@link MatchResult#getVariables()} is invoked. + * + * @return the MatchResult from comparing this RequestMatcher against the HttpServletRequest + * @since 5.2 */ default MatchResult matcher(HttpServletRequest request) { boolean match = matches(request); @@ -46,7 +51,10 @@ default MatchResult matcher(HttpServletRequest request) { } /** - * The result of matching + * The result of matching against an HttpServletRequest + * Contains the status, true or false, of the match and + * if present, any variables extracted from the match + * @since 5.2 */ class MatchResult { private final boolean match; @@ -57,10 +65,18 @@ class MatchResult { this.variables = variables; } + /** + * @return true if the comparison against the HttpServletRequest produced a successful match + */ public boolean isMatch() { return this.match; } + /** + * Returns the extracted variable values where the key is the variable name and the value is the variable value + * + * @return a map containing key-value pairs representing extracted variable names and variable values + */ public Map getVariables() { return this.variables; }