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

UseNewRequestMatchers should also replace with 5.7 on classpath #398

Merged
merged 6 commits into from
Jul 28, 2023
Merged

UseNewRequestMatchers should also replace with 5.7 on classpath #398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8bbe02c
UseNewRequestMatchers should also replace with 5.7 on classpath
timtebeek Jul 27, 2023
dad84cb
Also migrate ignoringAntMatchers to ignoringRequestMatchers
timtebeek Jul 27, 2023
aee9455
Ignore type validation issues on combined test
timtebeek Jul 27, 2023
33998c6
Use getCoordinates().replaceMethod()
timtebeek Jul 28, 2023
6ef8cd9
Update name to remove need for typeValidationOptions
timtebeek Jul 28, 2023
2935560
Remove unused imports
timtebeek Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 30 additions & 48 deletions src/main/java/org/openrewrite/java/spring/security5/UseNewRequestMatchers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,26 @@
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;

@Value
@EqualsAndHashCode(callSuper = false)
public class UseNewRequestMatchers extends Recipe {

private static final MethodMatcher ANT_MATCHERS = new MethodMatcher("org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry antMatchers(..)");
private static final MethodMatcher MVC_MATCHERS = new MethodMatcher("org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry mvcMatchers(..)", true);
private static final MethodMatcher REGEX_MATCHERS = new MethodMatcher("org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry regexMatchers(..)");
private static final String CLAZZ = "org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry";
private static final MethodMatcher ANT_MATCHERS = new MethodMatcher(CLAZZ + " antMatchers(..)");
private static final MethodMatcher MVC_MATCHERS = new MethodMatcher(CLAZZ + " mvcMatchers(..)", true);
private static final MethodMatcher REGEX_MATCHERS = new MethodMatcher(CLAZZ + " regexMatchers(..)");


@Override
Expand All @@ -57,46 +59,26 @@ public String getDescription() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(Preconditions.or(
new UsesMethod<>(ANT_MATCHERS),
new UsesMethod<>(MVC_MATCHERS),
new UsesMethod<>(REGEX_MATCHERS)), new JavaIsoVisitor<ExecutionContext>() {

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
if (ANT_MATCHERS.matches(mi) || MVC_MATCHERS.matches(mi) || REGEX_MATCHERS.matches(mi)) {
mi = maybeChangeMethodInvocation(mi);
}
return mi;
}
});

}

private J.MethodInvocation maybeChangeMethodInvocation(J.MethodInvocation mi) {
return findRequestMatchersMethodWithMatchingParameterTypes(mi)
.map(requestMatchersMethod -> mi
.withMethodType(requestMatchersMethod)
.withName(mi.getName().withSimpleName("requestMatchers")))
.orElse(mi);
}

private Optional<JavaType.Method> findRequestMatchersMethodWithMatchingParameterTypes(J.MethodInvocation mi) {
JavaType.Method methodType = mi.getMethodType();
if (methodType == null) {
return Optional.empty();
}
List<JavaType> parameterTypes = methodType.getParameterTypes();
List<JavaType.Method> methods;
boolean isOverride = TypeUtils.isOverride(mi.getMethodType());
if (isOverride) {
methods = requireNonNull(methodType.getDeclaringType().getSupertype(), "superType").getMethods();
} else {
methods = methodType.getDeclaringType().getMethods();
}
return methods.stream()
.filter(m -> m.getName().equals("requestMatchers"))
.filter(m -> m.getParameterTypes().equals(parameterTypes))
Comment on lines -97 to -99
Copy link
Contributor Author

@timtebeek timtebeek Jul 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This lookup failed, as at recipe runtime only the 5.7 class is available, meaning no replacement method was found. The recipes tests hid that fact, by using classpathResources of 5.8+.

.findFirst();
new UsesMethod<>(ANT_MATCHERS),
new UsesMethod<>(MVC_MATCHERS),
new UsesMethod<>(REGEX_MATCHERS)),
new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
if ((ANT_MATCHERS.matches(mi) || MVC_MATCHERS.matches(mi) || REGEX_MATCHERS.matches(mi))
&& mi.getSelect() != null) {
String parametersTemplate = mi.getArguments().stream().map(arg -> "#{any()}").collect(joining(", "));
List<Expression> parameters = new ArrayList<>();
parameters.add(mi.getSelect());
parameters.addAll(mi.getArguments());
JavaTemplate template = JavaTemplate.builder(String.format("#{any()}\n.requestMatchers(%s)", parametersTemplate))
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "spring-security-config-5.8"))
.build();
return template.apply(getCursor(), mi.getCoordinates().replace(), parameters.toArray());
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
}
return mi;
}
});
}
}
84 changes: 25 additions & 59 deletions .../security5/UseNewRequestMatchersTest.java → .../security5/UseNewRequestMatchersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.openrewrite.java.spring.security5.UseNewRequestMatchers;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import static org.openrewrite.java.Assertions.java;

Expand All @@ -30,6 +31,7 @@ class UseNewRequestMatchersTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.recipe(new UseNewRequestMatchers())
.typeValidationOptions(TypeValidation.builder().methodInvocations(false).build())
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
.parser(JavaParser.fromJavaVersion()
.logCompilationWarningsAndErrors(true)
.classpathFromResources(new InMemoryExecutionContext(),"spring-context-5.3.+", "spring-beans-5.3.+", "spring-web-5.3.+", "spring-security-web-5.8.+", "spring-security-config-5.8.+"));
Expand All @@ -55,10 +57,7 @@ void migratesMvcMatchersWithMvcPatterns() {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.mvcMatchers("/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz.mvcMatchers("/static/**").permitAll());
return http.build();
}
}
Expand All @@ -78,10 +77,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.requestMatchers("/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz
.requestMatchers("/static/**").permitAll());
return http.build();
}
}
Expand Down Expand Up @@ -110,10 +107,7 @@ void migratesMvcMatchersWithMvcPatternsAndHttpMethod() {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.mvcMatchers(HttpMethod.GET, "/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz.mvcMatchers(HttpMethod.GET, "/static/**").permitAll());
return http.build();
}
}
Expand All @@ -134,10 +128,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.requestMatchers(HttpMethod.GET, "/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz
.requestMatchers(HttpMethod.GET, "/static/**").permitAll());
return http.build();
}
}
Expand Down Expand Up @@ -165,10 +157,7 @@ void migratesRegexMatchersWithRegexPatterns() {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.regexMatchers("/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz.regexMatchers("/static/**").permitAll());
return http.build();
}
}
Expand All @@ -188,10 +177,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.requestMatchers("/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz
.requestMatchers("/static/**").permitAll());
return http.build();
}
}
Expand Down Expand Up @@ -220,10 +207,7 @@ void migratesRegexMatchersWithRegexPatternsAndHttpMethod() {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.regexMatchers(HttpMethod.GET, "/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz.regexMatchers(HttpMethod.GET, "/static/**").permitAll());
return http.build();
}
}
Expand All @@ -244,10 +228,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.requestMatchers(HttpMethod.GET, "/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz
.requestMatchers(HttpMethod.GET, "/static/**").permitAll());
return http.build();
}
}
Expand Down Expand Up @@ -275,10 +257,7 @@ void migratesAntMatchersWithAntPatterns() {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.antMatchers("/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz.antMatchers("/static/**").permitAll());
return http.build();
}
}
Expand All @@ -298,10 +277,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.requestMatchers("/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz
.requestMatchers("/static/**").permitAll());
return http.build();
}
}
Expand Down Expand Up @@ -330,10 +307,7 @@ void migratesAntMatchersWithHttpMethod() {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.antMatchers(HttpMethod.GET).permitAll()
);
http.authorizeHttpRequests(authz -> authz.antMatchers(HttpMethod.GET).permitAll());
return http.build();
}
}
Expand All @@ -354,10 +328,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.requestMatchers(HttpMethod.GET).permitAll()
);
http.authorizeHttpRequests(authz -> authz
.requestMatchers(HttpMethod.GET).permitAll());
return http.build();
}
}
Expand Down Expand Up @@ -386,10 +358,7 @@ void migratesAntMatchersWithAntPatternsAndHttpMethod() {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.antMatchers(HttpMethod.GET, "/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz.antMatchers(HttpMethod.GET, "/static/**").permitAll());
return http.build();
}
}
Expand All @@ -410,10 +379,8 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.requestMatchers(HttpMethod.GET, "/static/**").permitAll()
);
http.authorizeHttpRequests(authz -> authz
.requestMatchers(HttpMethod.GET, "/static/**").permitAll());
return http.build();
}
}
Expand Down Expand Up @@ -443,8 +410,7 @@ class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.regexMatchers("/api/admin/**").hasRole("ADMIN")
.authorizeHttpRequests(authz -> authz.regexMatchers("/api/admin/**").hasRole("ADMIN")
.antMatchers(HttpMethod.GET, "/api/user/**").hasRole("USER")
.mvcMatchers(HttpMethod.PATCH).denyAll()
.anyRequest().authenticated()
Expand All @@ -470,7 +436,7 @@ class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http
.authorizeHttpRequests((authz) -> authz
.authorizeHttpRequests(authz -> authz
.requestMatchers("/api/admin/**").hasRole("ADMIN")
.requestMatchers(HttpMethod.GET, "/api/user/**").hasRole("USER")
.requestMatchers(HttpMethod.PATCH).denyAll()
Expand Down
Loading