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.

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

Recipe to address unresolvable circular reference upgrading to Spring Boot 2.7.18 #668

Open
nickdala opened this issue Jan 15, 2025 · 2 comments
Labels
boot-2.7 enhancement New feature or request recipe Recipe requested spring-security

Comments

@nickdala
Copy link

nickdala commented Jan 15, 2025

What problem are you trying to solve?

Improve the Spring Security recipe that updates the class that extends WebSecurityConfigurerAdapter.

Sample application

The sample application is the spring-boot-blog-app. This is a fork of skarware/spring-boot-blog-app. The OpenRewrite maven plugin is defined in the pom.xml.

Branch before the upgrade:

https://github.com/nickdala/spring-boot-blog-app-java-copilot-upgrade-demo/tree/openrewrite-circular-reference-before

Branch after the upgrade using OpenRewrite

https://github.com/nickdala/spring-boot-blog-app-java-copilot-upgrade-demo/tree/openrewrite-circular-reference-after

What precondition(s) should be checked before applying this recipe?

A PasswordEncoder bean like the following defined in the class that extends WebSecurityConfigurerAdapter.

@Configuration
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
  @Bean
  public BCryptPasswordEncoder bcryptEncoder() {
      return new BCryptPasswordEncoder();
  }
}

Steps to reproduce

  1. Clone the repo spring-boot-blog-app-java-copilot-upgrade-demo
git clone https://github.com/nickdala/spring-boot-blog-app-java-copilot-upgrade-demo
cd spring-boot-blog-app-java-copilot-upgrade-demo
  1. (Optional) Open the project with VS Code and start the Dev Container. Docker needs to be running.
code .
  1. Check out the branch openrewrite-circular-reference-before
git checkout openrewrite-circular-reference-before
  1. Upgrade using maven
./mvnw rewrite:run

or

mvnw.cmd rewrite:run
  1. (Optional) Use Java 21. Below is an example of installing Java 21 using SDKMAN!. This is not optional if you're using the dev container.
sdk install java 21.0.5-tem
  1. Build and run the application
./mvnw clean package

./mvnw spring-boot:run

You will see the following error.

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  11.282 s
[INFO] Finished at: 2025-01-15T12:39:31-05:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.springframework.boot:spring-boot-maven-plugin:2.7.18:run (default-cli) on project spring-boot-blog-app: Application finished with exit code: 1 -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
Caused by: org.springframework.beans.factory.BeanCurrentlyInCreationException: Error creating bean with name 'webSecurityConfig': Requested bean is currently in creation: Is there an unresolvable circular reference?
        at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.beforeSingletonCreation(DefaultSingletonBeanRegistry.java:355) ~[spring-beans-5.3.31.jar:5.3.31]
        at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:227) ~[spring-beans-5.3.31.jar:5.3.31]

Describe the situation after applying the recipe

Step 1:

Move the method bcryptEncoder() in WebSecurityConfig.java to a separate class to create the BCryptPasswordEncoder instance.

@Configuration
public class PasswordEncoderConfig {
    @Bean
    BCryptPasswordEncoder bcryptEncoder() {
        return new BCryptPasswordEncoder();
    }   
}

Step 2:

Pass the BCryptPasswordEncoder to the configureGlobal() method in WebSecurityConfig.

@Autowired
    public void configureGlobal(AuthenticationManagerBuilder authenticationManagerBuilder, BCryptPasswordEncoder passwordEncoder) throws Exception {

        authenticationManagerBuilder
                .jdbcAuthentication()
                .usersByUsernameQuery(USERS_SQL_QUERY) // not really necessary, as users table follows default Spring Security User schema
                .authoritiesByUsernameQuery(AUTHORITIES_SQL_QUERY)  // a must as using customized authorities table, many to many variation
                .dataSource(dataSource)
                .passwordEncoder(passwordEncoder);
}

The complete code is in the branch openrewrite-circular-reference-after

Are you interested in [contributing this recipe to OpenRewrite]

Yes. I'm interested in collaborating with the community to verify the recipe and to ensure that we're accounting for any edge cases. Once that's done, I can lead the coding effort.

@timtebeek
Copy link
Contributor

hi @nickdala ; thanks for the detailed analysis here! Help accounting for this case would indeed be appreciated. I wonder if this is perhaps best tackled with a separate ScanningRecipes that runs before WebSecurityConfigurerAdapter to move that password bean into a new configuration class, and replace the method invocation with an injected bean constructor argument. That way we can isolate this bit of logic, and it's valuable on it's own as well. @nmck257 might have some insights here as well based on his prior involvement with that Spring Security recipes.

@timtebeek timtebeek added enhancement New feature or request recipe Recipe requested boot-2.7 spring-security labels Jan 16, 2025
@timtebeek timtebeek moved this to Recipes Wanted in OpenRewrite Jan 16, 2025
@nmck257
Copy link
Collaborator

nmck257 commented Jan 16, 2025

Yeah, I believe that moving the encoder bean can happen independently of the broader WebSecurityConfigurerAdapter change, so putting it in a separate recipe feels valid.

Splitting a little further, maybe there should be:

  • A reusable Recipe / Visitor for "prefer bean parameter injection over bean method invocation injection"
  • A ScanningRecipe for "move password encoder beans to separate configuration class", which can also queue up an instance of the above visitor, configured to target this specific scenario

Or maybe the second item can even be genericized as a MoveBeanMethod recipe, with options such as bean name, bean type, declaring class, target declaring class, etc?
And maybe there are other beans besides the password encoder which might fall in this same category for the WebSecurityConfigurerAdapter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
boot-2.7 enhancement New feature or request recipe Recipe requested spring-security
Projects
Status: Recipes Wanted
Development

No branches or pull requests

3 participants