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 all commits
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
80 changes: 30 additions & 50 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,23 @@
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.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;

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(..)");
private static final MethodMatcher CSRF_MATCHERS = new MethodMatcher("org.springframework.security.config.annotation.web.configurers.CsrfConfigurer ignoringAntMatchers(..)");


@Override
Expand All @@ -57,46 +56,27 @@ 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);
boolean isCsrfMatcher = CSRF_MATCHERS.matches(mi);
if ((ANT_MATCHERS.matches(mi) || MVC_MATCHERS.matches(mi) || REGEX_MATCHERS.matches(mi) || isCsrfMatcher)
&& mi.getSelect() != null) {
String parametersTemplate = mi.getArguments().stream().map(arg -> "#{any()}").collect(joining(", "));
String replacementMethodName = isCsrfMatcher ? "ignoringRequestMatchers" : "requestMatchers";
JavaTemplate template = JavaTemplate.builder(String.format(replacementMethodName + "(%s)", parametersTemplate))
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "spring-security-config-5.8"))
.build();
J.MethodInvocation apply = template.apply(getCursor(), mi.getCoordinates().replaceMethod(), mi.getArguments().toArray());
return apply.withSelect(mi.getSelect())
.withName(mi.getName().withSimpleName(replacementMethodName));
}
return mi;
}
});
}
}
68 changes: 59 additions & 9 deletions ...hSpringSecurity_5_7/java/org/openrewrite/spring/security5/UseNewSecurityMatchersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
package org.openrewrite.spring.security5;

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.ExecutionContext;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.Tree;
import org.openrewrite.DocumentExample;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaVisitor;
Expand All @@ -39,7 +39,7 @@ public void defaults(RecipeSpec spec) {
spec.recipe(new UseNewSecurityMatchers())
.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.+"));
.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.+"));
}

@DocumentExample
Expand Down Expand Up @@ -99,18 +99,68 @@ SecurityFilterChain securityFilterChain(HttpSecurity http) {
);
}

@Test
void chainedCalls() {
rewriteRun(
//language=java
java("""
package com.example.demo;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpMethod;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.web.SecurityFilterChain;

@Configuration
@EnableWebSecurity
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http.antMatcher("/**").authorizeRequests()
.antMatchers(HttpMethod.POST, "/verify").access("hasRole('ROLE_USER')")
.anyRequest().authenticated();
return http.build();
}
}
""","""
package com.example.demo;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpMethod;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.web.SecurityFilterChain;

@Configuration
@EnableWebSecurity
class SecurityConfig {
@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http) {
http.securityMatcher("/**").authorizeRequests()
.antMatchers(HttpMethod.POST, "/verify").access("hasRole('ROLE_USER')")
.anyRequest().authenticated();
return http.build();
}
}
""")
);
}

@Test
void togetherWithRequestMatchers() {
//language=java
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public @Nullable J visit(@Nullable Tree tree, ExecutionContext ctx) {
tree = new UseNewRequestMatchers().getVisitor().visit(tree, ctx);
tree = new UseNewSecurityMatchers().getVisitor().visit(tree, ctx);
return (J) tree;
}
})),
@Override
public @Nullable J visit(@Nullable Tree tree, ExecutionContext ctx) {
tree = new UseNewRequestMatchers().getVisitor().visit(tree, ctx);
tree = new UseNewSecurityMatchers().getVisitor().visit(tree, ctx);
return (J) tree;
}
})),
java(
"""
package com.example;
Expand Down
Loading