Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ability to match Endpoint requests by http method #29596

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add ability to match Endpoint requests by http method #29596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b15617
Add ability to match Endpoint requests by http method
onobc Jan 29, 2022
1e73e22
Fix checkstyle
onobc Jan 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 26 additions & 9 deletions ...ava/org/springframework/boot/actuate/autoconfigure/security/reactive/EndpointRequest.java
View file
Open in desktop
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2022 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.
Expand Down Expand Up @@ -39,6 +39,7 @@
import org.springframework.context.ApplicationContext;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.http.HttpMethod;
import org.springframework.security.web.server.util.matcher.OrServerWebExchangeMatcher;
import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
Expand All @@ -52,6 +53,7 @@
* endpoint locations.
*
* @author Madhura Bhave
* @author Chris Bono
* @since 2.0.0
*/
public final class EndpointRequest {
Expand Down Expand Up @@ -157,41 +159,55 @@ public static final class EndpointServerWebExchangeMatcher extends AbstractWebEx

private final boolean includeLinks;

private final HttpMethod httpMethod;

private volatile ServerWebExchangeMatcher delegate;

private EndpointServerWebExchangeMatcher(boolean includeLinks) {
this(Collections.emptyList(), Collections.emptyList(), includeLinks);
this(Collections.emptyList(), Collections.emptyList(), includeLinks, null);
}

private EndpointServerWebExchangeMatcher(Class<?>[] endpoints, boolean includeLinks) {
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks);
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null);
}

private EndpointServerWebExchangeMatcher(String[] endpoints, boolean includeLinks) {
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks);
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null);
}

private EndpointServerWebExchangeMatcher(List<Object> includes, List<Object> excludes, boolean includeLinks) {
private EndpointServerWebExchangeMatcher(List<Object> includes, List<Object> excludes, boolean includeLinks,
HttpMethod httpMethod) {
super(PathMappedEndpoints.class);
this.includes = includes;
this.excludes = excludes;
this.includeLinks = includeLinks;
this.httpMethod = httpMethod;
}

public EndpointServerWebExchangeMatcher excluding(Class<?>... endpoints) {
List<Object> excludes = new ArrayList<>(this.excludes);
excludes.addAll(Arrays.asList((Object[]) endpoints));
return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks);
return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks, null);
}

public EndpointServerWebExchangeMatcher excluding(String... endpoints) {
List<Object> excludes = new ArrayList<>(this.excludes);
excludes.addAll(Arrays.asList((Object[]) endpoints));
return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks);
return new EndpointServerWebExchangeMatcher(this.includes, excludes, this.includeLinks, null);
}

public EndpointServerWebExchangeMatcher excludingLinks() {
return new EndpointServerWebExchangeMatcher(this.includes, this.excludes, false);
return new EndpointServerWebExchangeMatcher(this.includes, this.excludes, false, null);
}

/**
* Restricts the matcher to only consider requests with a particular http method.
* @param httpMethod the http method to include
* @return a copy of the matcher further restricted to only match requests with
* the specified http method
*/
public EndpointServerWebExchangeMatcher withHttpMethod(HttpMethod httpMethod) {
return new EndpointServerWebExchangeMatcher(this.includes, this.excludes, false, httpMethod);
}

@Override
Expand Down Expand Up @@ -246,7 +262,8 @@ private EndpointId getEndpointId(Class<?> source) {
}

private List<ServerWebExchangeMatcher> getDelegateMatchers(Set<String> paths) {
return paths.stream().map((path) -> new PathPatternParserServerWebExchangeMatcher(path + "/**"))
return paths.stream()
.map((path) -> new PathPatternParserServerWebExchangeMatcher(path + "/**", this.httpMethod))
.collect(Collectors.toList());
}

Expand Down
51 changes: 35 additions & 16 deletions ...java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java
View file
Open in desktop
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2021 the original author or authors.
* Copyright 2012-2022 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.
Expand Down Expand Up @@ -40,6 +40,8 @@
import org.springframework.boot.web.context.WebServerApplicationContext;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.http.HttpMethod;
import org.springframework.lang.Nullable;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.OrRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
Expand All @@ -53,6 +55,7 @@
*
* @author Madhura Bhave
* @author Phillip Webb
* @author Chris Bono
* @since 2.0.0
*/
public final class EndpointRequest {
Expand Down Expand Up @@ -164,8 +167,8 @@ protected abstract RequestMatcher createDelegate(WebApplicationContext context,
protected List<RequestMatcher> getLinksMatchers(RequestMatcherFactory requestMatcherFactory,
RequestMatcherProvider matcherProvider, String basePath) {
List<RequestMatcher> linksMatchers = new ArrayList<>();
linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, basePath));
linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, basePath, "/"));
linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, null, basePath));
linksMatchers.add(requestMatcherFactory.antPath(matcherProvider, null, basePath, "/"));
return linksMatchers;
}

Expand All @@ -174,7 +177,7 @@ protected RequestMatcherProvider getRequestMatcherProvider(WebApplicationContext
return context.getBean(RequestMatcherProvider.class);
}
catch (NoSuchBeanDefinitionException ex) {
return AntPathRequestMatcher::new;
return (pattern, method) -> new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null);
}
}

Expand All @@ -191,38 +194,53 @@ public static final class EndpointRequestMatcher extends AbstractRequestMatcher

private final boolean includeLinks;

@Nullable
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: @Nullable... You will notice I did not use it in the reactive counterpart - on purpose. I wanted to see what the recommendation from the team is in this area. I do not see it widely used in the SB codebase other than mostly on a few params to @Endpoint methods (selectors, params).

Whatever direction we land I will make both imperative and reactive impls consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently don't use them in our own codebase so we may as well strip them out for now. We've got #10712 open to add them consistently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you handle that in your polish commit @philwebb ? I don't mind doing it - just let me know.

private final HttpMethod httpMethod;

private EndpointRequestMatcher(boolean includeLinks) {
this(Collections.emptyList(), Collections.emptyList(), includeLinks);
this(Collections.emptyList(), Collections.emptyList(), includeLinks, null);
}

private EndpointRequestMatcher(Class<?>[] endpoints, boolean includeLinks) {
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks);
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null);
}

private EndpointRequestMatcher(String[] endpoints, boolean includeLinks) {
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks);
this(Arrays.asList((Object[]) endpoints), Collections.emptyList(), includeLinks, null);
}

private EndpointRequestMatcher(List<Object> includes, List<Object> excludes, boolean includeLinks) {
private EndpointRequestMatcher(List<Object> includes, List<Object> excludes, boolean includeLinks,
@Nullable HttpMethod httpMethod) {
this.includes = includes;
this.excludes = excludes;
this.includeLinks = includeLinks;
this.httpMethod = httpMethod;
}

public EndpointRequestMatcher excluding(Class<?>... endpoints) {
List<Object> excludes = new ArrayList<>(this.excludes);
excludes.addAll(Arrays.asList((Object[]) endpoints));
return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks);
return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks, null);
}

public EndpointRequestMatcher excluding(String... endpoints) {
List<Object> excludes = new ArrayList<>(this.excludes);
excludes.addAll(Arrays.asList((Object[]) endpoints));
return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks);
return new EndpointRequestMatcher(this.includes, excludes, this.includeLinks, null);
}

public EndpointRequestMatcher excludingLinks() {
return new EndpointRequestMatcher(this.includes, this.excludes, false);
return new EndpointRequestMatcher(this.includes, this.excludes, false, null);
}

/**
* Restricts the matcher to only consider requests with a particular http method.
* @param httpMethod the http method to include
* @return a copy of the matcher further restricted to only match requests with
* the specified http method
*/
public EndpointRequestMatcher withHttpMethod(HttpMethod httpMethod) {
return new EndpointRequestMatcher(this.includes, this.excludes, false, httpMethod);
}

@Override
Expand All @@ -236,7 +254,8 @@ protected RequestMatcher createDelegate(WebApplicationContext context,
}
streamPaths(this.includes, pathMappedEndpoints).forEach(paths::add);
streamPaths(this.excludes, pathMappedEndpoints).forEach(paths::remove);
List<RequestMatcher> delegateMatchers = getDelegateMatchers(requestMatcherFactory, matcherProvider, paths);
List<RequestMatcher> delegateMatchers = getDelegateMatchers(requestMatcherFactory, matcherProvider, paths,
this.httpMethod);
String basePath = pathMappedEndpoints.getBasePath();
if (this.includeLinks && StringUtils.hasText(basePath)) {
delegateMatchers.addAll(getLinksMatchers(requestMatcherFactory, matcherProvider, basePath));
Expand Down Expand Up @@ -268,8 +287,8 @@ private EndpointId getEndpointId(Class<?> source) {
}

private List<RequestMatcher> getDelegateMatchers(RequestMatcherFactory requestMatcherFactory,
RequestMatcherProvider matcherProvider, Set<String> paths) {
return paths.stream().map((path) -> requestMatcherFactory.antPath(matcherProvider, path, "/**"))
RequestMatcherProvider matcherProvider, Set<String> paths, HttpMethod httpMethod) {
return paths.stream().map((path) -> requestMatcherFactory.antPath(matcherProvider, httpMethod, path, "/**"))
.collect(Collectors.toList());
}

Expand Down Expand Up @@ -299,12 +318,12 @@ protected RequestMatcher createDelegate(WebApplicationContext context,
*/
private static class RequestMatcherFactory {

RequestMatcher antPath(RequestMatcherProvider matcherProvider, String... parts) {
RequestMatcher antPath(RequestMatcherProvider matcherProvider, HttpMethod httpMethod, String... parts) {
StringBuilder pattern = new StringBuilder();
for (String part : parts) {
pattern.append(part);
}
return matcherProvider.getRequestMatcher(pattern.toString());
return matcherProvider.getRequestMatcher(pattern.toString(), httpMethod);
}

}
Expand Down