diff --git a/src/main/java/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapter.java b/src/main/java/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapter.java index 9a39b819e..f45ceb4b9 100644 --- a/src/main/java/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapter.java +++ b/src/main/java/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapter.java @@ -24,7 +24,6 @@ import org.openrewrite.marker.Markers; import java.util.*; -import java.util.concurrent.atomic.AtomicBoolean; /** * @author Alex Boyko @@ -51,13 +50,12 @@ public class WebSecurityConfigurerAdapter extends Recipe { private static final MethodMatcher CONFIGURE_AUTH_MANAGER_SECURITY_METHOD_MATCHER = new MethodMatcher("org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter configure(org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder)", true); - private static final List APPLICABLE_METHODS_MATCHERS = Arrays.asList( - CONFIGURE_HTTP_SECURITY_METHOD_MATCHER, - CONFIGURE_WEB_SECURITY_METHOD_MATCHER -// CONFIGURE_AUTH_MANAGER_SECURITY_METHOD_MATCHER - ); + private static final MethodMatcher USER_DETAILS_SERVICE_BEAN_METHOD_MATCHER = + new MethodMatcher("org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter userDetailsServiceBean()", true); + private static final MethodMatcher AUTHENTICATION_MANAGER_BEAN_METHOD_MATCHER = + new MethodMatcher("org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter authenticationManagerBean()", true); - private static final String MSG_QUALIFIES = "qualifies"; + private static final String HAS_CONFLICT = "has-conflict"; @Override public String getDisplayName() { @@ -74,19 +72,37 @@ protected TreeVisitor getVisitor() { return new JavaIsoVisitor() { @Override public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext context) { - if (isApplicableClass(classDecl)) { - getCursor().putMessage(MSG_QUALIFIES, true); - maybeRemoveImport(FQN_WEB_SECURITY_CONFIGURER_ADAPTER); - return super.visitClassDeclaration(classDecl, context).withExtends(null); + if (TypeUtils.isAssignableTo(FQN_WEB_SECURITY_CONFIGURER_ADAPTER, classDecl.getType()) + && classDecl.getLeadingAnnotations().stream().anyMatch(a -> TypeUtils.isOfClassType(a.getType(), FQN_CONFIGURATION))) { + boolean hasConflict = false; + for (JavaType.Method method : classDecl.getType().getMethods()) { + if (isConflictingMethod(method, method.getName())) { + hasConflict = true; + } + } + getCursor().putMessage(HAS_CONFLICT, hasConflict); + if (!hasConflict) { + maybeRemoveImport(FQN_WEB_SECURITY_CONFIGURER_ADAPTER); + classDecl = classDecl.withExtends(null); + } } return super.visitClassDeclaration(classDecl, context); } + private boolean isConflictingMethod(@Nullable JavaType.Method methodType, String methodName) { + return (methodType == null && "authenticationManagerBean".equals(methodName) || "userDetailsServiceBean".equals(methodName)) || + (USER_DETAILS_SERVICE_BEAN_METHOD_MATCHER.matches(methodType) + || AUTHENTICATION_MANAGER_BEAN_METHOD_MATCHER.matches(methodType) + || CONFIGURE_AUTH_MANAGER_SECURITY_METHOD_MATCHER.matches(methodType)); + } + @Override public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext context) { J.MethodDeclaration m = super.visitMethodDeclaration(method, context); Cursor classCursor = getCursor().dropParentUntil(J.ClassDeclaration.class::isInstance); - if (classCursor.getMessage(MSG_QUALIFIES, false)) { + if (isConflictingMethod(m.getMethodType(), method.getSimpleName())) { + m = m.withMarkers(m.getMarkers().searchResult("Migrate manually based on https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter")); + } else if (!classCursor.getMessage(HAS_CONFLICT, true)) { if (CONFIGURE_HTTP_SECURITY_METHOD_MATCHER.matches(m, classCursor.getValue())) { JavaType securityChainType = JavaType.buildType(FQN_SECURITY_FILTER_CHAIN); JavaType.Method type = m.getMethodType(); @@ -131,9 +147,6 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex m = addBeanAnnotation(m, getCursor()); maybeAddImport(FQN_WEB_SECURITY_CUSTOMIZER); - } else if (CONFIGURE_AUTH_MANAGER_SECURITY_METHOD_MATCHER.matches(m, classCursor.getValue())) { - // TODO: implement this case - m = m.withMarkers(m.getMarkers().searchResult("Migrate manually based on https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter")); } } return m; @@ -146,7 +159,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext context) { J.MethodDeclaration parentMethod = getCursor().getParent().getValue(); Cursor classDeclCursor = getCursor().dropParentUntil(J.ClassDeclaration.class::isInstance); J.ClassDeclaration classDecl = classDeclCursor.getValue(); - if (classDeclCursor.getMessage(MSG_QUALIFIES, false)) { + if (!classDeclCursor.getMessage(HAS_CONFLICT, true)) { if (CONFIGURE_HTTP_SECURITY_METHOD_MATCHER.matches(parentMethod, classDecl)) { JavaTemplate template = JavaTemplate.builder(this::getCursor, "return #{any(org.springframework.security.config.annotation.SecurityBuilder)}.build();") .javaParser(() -> JavaParser.fromJavaVersion() @@ -180,55 +193,6 @@ private J.MethodDeclaration addBeanAnnotation(J.MethodDeclaration m, Cursor c) { return m.withTemplate(template, m.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::getSimpleName))); } - private boolean isApplicableClass(J.ClassDeclaration classDecl) { - - if (TypeUtils.isAssignableTo(FQN_WEB_SECURITY_CONFIGURER_ADAPTER, classDecl.getType()) - && classDecl.getLeadingAnnotations().stream().anyMatch(a -> TypeUtils.isOfClassType(a.getType(), FQN_CONFIGURATION))) { - - AtomicBoolean foundNonApplicableMethodUsage = new AtomicBoolean(); - new JavaIsoVisitor() { - - private void checkNonApplicableMethod(@Nullable JavaType.Method m, AtomicBoolean found) { - if (m != null && !found.get()) { - J.ClassDeclaration enclosingClassDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - if (enclosingClassDecl == classDecl || - (enclosingClassDecl != null && !enclosingClassDecl.hasModifier(J.Modifier.Type.Static)) - ) { - if (TypeUtils.isAssignableTo(FQN_WEB_SECURITY_CONFIGURER_ADAPTER, m.getDeclaringType()) - && APPLICABLE_METHODS_MATCHERS.stream().noneMatch(matcher -> matcher.matches(m))) { - found.set(true); - } - } - } - } - - @Override - public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean found) { - checkNonApplicableMethod(method.getMethodType(), found); - return found.get() ? method : super.visitMethodInvocation(method, found); - } - - @Override - public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, AtomicBoolean found) { - checkNonApplicableMethod(method.getMethodType(), found); - return found.get() ? method : super.visitMethodDeclaration(method, found); - } - - @Override - public J.MemberReference visitMemberReference(J.MemberReference memberRef, AtomicBoolean found) { - checkNonApplicableMethod(memberRef.getMethodType(), found); - return found.get() ? memberRef : super.visitMemberReference(memberRef, found); - } - - }.visit(classDecl, foundNonApplicableMethodUsage); - - return !foundNonApplicableMethodUsage.get(); - } - return false; - - } - }; } - } diff --git a/src/testWithSpringBoot_2_4/kotlin/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapterTest.kt b/src/testWithSpringBoot_2_4/kotlin/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapterTest.kt index 8880f25b0..476ca64fb 100644 --- a/src/testWithSpringBoot_2_4/kotlin/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapterTest.kt +++ b/src/testWithSpringBoot_2_4/kotlin/org/openrewrite/java/spring/boot2/WebSecurityConfigurerAdapterTest.kt @@ -56,6 +56,8 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { ) .httpBasic(withDefaults()); } + + void someMethod() {} } """, @@ -81,6 +83,8 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { .httpBasic(withDefaults()); return http.build(); } + + void someMethod() {} } """ @@ -152,42 +156,67 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { ) @Test - fun configurAuthManagerMethod() = assertUnchanged( + fun configurAuthManagerMethod() = assertChanged( before = """ package com.example.websecuritydemo; import org.springframework.context.annotation.Configuration; - import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; - import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; - import org.springframework.security.ldap.userdetails.PersonContextMapper; + import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; + import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; + import org.springframework.security.ldap.userdetails.PersonContextMapper; + + @Configuration + public class SecurityConfiguration extends WebSecurityConfigurerAdapter { - @Configuration - public class SecurityConfiguration extends WebSecurityConfigurerAdapter { + @Override + protected void configure(AuthenticationManagerBuilder auth) { + auth + .ldapAuthentication() + .userDetailsContextMapper(new PersonContextMapper()) + .userDnPatterns("uid={0},ou=people") + .contextSource() + .port(0); + } - @Override - protected void configure(AuthenticationManagerBuilder auth) { - auth - .ldapAuthentication() - .userDetailsContextMapper(new PersonContextMapper()) - .userDnPatterns("uid={0},ou=people") - .contextSource() - .port(0); - } - + } + """, + after = """ + package com.example.websecuritydemo; + + import org.springframework.context.annotation.Configuration; + import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; + import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; + import org.springframework.security.ldap.userdetails.PersonContextMapper; + + @Configuration + public class SecurityConfiguration extends WebSecurityConfigurerAdapter { + + /*~~(Migrate manually based on https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter)~~>*/@Override + protected void configure(AuthenticationManagerBuilder auth) { + auth + .ldapAuthentication() + .userDetailsContextMapper(new PersonContextMapper()) + .userDnPatterns("uid={0},ou=people") + .contextSource() + .port(0); } + + } """ ) @Test - fun overideUnapplicableMethod() = assertUnchanged( + fun overideUnapplicableMethod() = assertChanged( before = """ package com.example.websecuritydemo; import static org.springframework.security.config.Customizer.withDefaults; + import org.springframework.context.annotation.Configuration; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.authentication.AuthenticationManager; + import org.springframework.security.core.userdetails.UserDetailsService; @Configuration public class SecurityConfiguration extends WebSecurityConfigurerAdapter { @@ -202,30 +231,33 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { } @Override - protected AuthenticationManager authenticationManager() throws Exception { - throw new Exception("Not Implemented"); + public UserDetailsService userDetailsServiceBean() throws Exception { + return null; + } + + @Override + public AuthenticationManager authenticationManagerBean() throws Exception { + return null; } } - """ - ) - - @Test - fun unapplicableMethodInvocation() = assertUnchanged( - before = """ + """, + after = """ package com.example.websecuritydemo; import static org.springframework.security.config.Customizer.withDefaults; + import org.springframework.context.annotation.Configuration; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; import org.springframework.security.config.annotation.web.builders.HttpSecurity; - + import org.springframework.security.authentication.AuthenticationManager; + import org.springframework.security.core.userdetails.UserDetailsService; + @Configuration public class SecurityConfiguration extends WebSecurityConfigurerAdapter { @Override - protected void configure(HttpSecurity http) { - System.out.println(getApplicationContext()); + protected void configure(HttpSecurity http) throws Exception { http .authorizeHttpRequests((authz) -> authz .anyRequest().authenticated() @@ -233,12 +265,22 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { .httpBasic(withDefaults()); } + /*~~(Migrate manually based on https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter)~~>*/@Override + public UserDetailsService userDetailsServiceBean() throws Exception { + return null; + } + + /*~~(Migrate manually based on https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter)~~>*/@Override + public AuthenticationManager authenticationManagerBean() throws Exception { + return null; + } + } """ ) @Test - fun configureHttpSecurityMethodInnerClass() = assertChanged( + fun unapplicableMethodInvocation() = assertChanged( before = """ package com.example.websecuritydemo; @@ -251,7 +293,7 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { public class SecurityConfiguration extends WebSecurityConfigurerAdapter { @Override - protected void configure(HttpSecurity http) throws Exception { + protected void configure(HttpSecurity http) { System.out.println(getApplicationContext()); http .authorizeHttpRequests((authz) -> authz @@ -259,62 +301,41 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { ) .httpBasic(withDefaults()); } - - @Configuration - public static class InnerSecurityConfiguration extends WebSecurityConfigurerAdapter { - @Override - protected void configure(HttpSecurity http) throws Exception { - http - .authorizeHttpRequests((authz) -> authz - .anyRequest().authenticated() - ) - .httpBasic(withDefaults()); - } - } + + public void someMethod() {} } """, after = """ package com.example.websecuritydemo; - + import static org.springframework.security.config.Customizer.withDefaults; - + import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; - import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; - import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.config.annotation.web.builders.HttpSecurity; - + import org.springframework.security.web.SecurityFilterChain; + @Configuration - public class SecurityConfiguration extends WebSecurityConfigurerAdapter { + public class SecurityConfiguration { - @Override - protected void configure(HttpSecurity http) throws Exception { + @Bean + SecurityFilterChain filterChain(HttpSecurity http) { System.out.println(getApplicationContext()); http .authorizeHttpRequests((authz) -> authz .anyRequest().authenticated() ) .httpBasic(withDefaults()); + return http.build(); } - - @Configuration - public static class InnerSecurityConfiguration { - @Bean - SecurityFilterChain filterChain(HttpSecurity http) throws Exception { - http - .authorizeHttpRequests((authz) -> authz - .anyRequest().authenticated() - ) - .httpBasic(withDefaults()); - return http.build(); - } - } + + public void someMethod() {} } """ ) @Test - fun configureHttpSecurityMethodInnerClassWithNotApplicableMethod() = assertChanged( + fun configureHttpSecurityMethodWithNotApplicableMethodInNonStaticInnerClass() = assertChanged( before = """ package com.example.websecuritydemo; @@ -336,15 +357,9 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { } @Configuration - public static class InnerSecurityConfiguration extends WebSecurityConfigurerAdapter { - @Override - protected void configure(HttpSecurity http) throws Exception { + public class InnerSecurityConfiguration { + protected void configure() throws Exception { System.out.println(getApplicationContext()); - http - .authorizeHttpRequests((authz) -> authz - .anyRequest().authenticated() - ) - .httpBasic(withDefaults()); } } } @@ -353,13 +368,12 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { package com.example.websecuritydemo; import static org.springframework.security.config.Customizer.withDefaults; - + import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; - import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; - import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.config.annotation.web.builders.HttpSecurity; - + import org.springframework.security.web.SecurityFilterChain; + @Configuration public class SecurityConfiguration { @@ -373,44 +387,6 @@ class WebSecurityConfigurerAdapterTest : JavaRecipeTest { return http.build(); } - @Configuration - public static class InnerSecurityConfiguration extends WebSecurityConfigurerAdapter { - @Override - protected void configure(HttpSecurity http) throws Exception { - System.out.println(getApplicationContext()); - http - .authorizeHttpRequests((authz) -> authz - .anyRequest().authenticated() - ) - .httpBasic(withDefaults()); - } - } - } - """ - ) - - @Test - fun configureHttpSecurityMethodWithNotApplicableMethodInNonStaticInnerClass() = assertUnchanged( - before = """ - package com.example.websecuritydemo; - - import static org.springframework.security.config.Customizer.withDefaults; - import org.springframework.context.annotation.Configuration; - import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; - import org.springframework.security.config.annotation.web.builders.HttpSecurity; - - @Configuration - public class SecurityConfiguration extends WebSecurityConfigurerAdapter { - - @Override - protected void configure(HttpSecurity http) throws Exception { - http - .authorizeHttpRequests((authz) -> authz - .anyRequest().authenticated() - ) - .httpBasic(withDefaults()); - } - @Configuration public class InnerSecurityConfiguration { protected void configure() throws Exception {