Fix Infinite recursion WebSecurityConfigurerAdapter.java

[Milan-Toliya]

Fixed Infinite recursion issue during authenticate and loadUserByUsername method execution.

[pivotal-cla]

@Milan-Toliya Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-cla]

@Milan-Toliya Thank you for signing the Contributor License Agreement!

[marcusdacoregio]

Hi @Milan-Toliya, thanks for contributing to Spring Security. Have you opened an issue before working on the code changes?

Can you help me understand what this PR fixes and what is the use case?

[Milan-Toliya]

Hi @marcusdacoregio I did not open an issue before submitting this patch.

With respect to Use case, I was trying to implement security filter in the web app. I Implemented WebSecurityConfigurerAdapter and used InMemoryUserDetailsManager to store user details.

I did Implemented WebSecurityConfigurerAdapter twice in the same project. It was causing issue in boot up, I fixed that by adding @Order(0) in one class. Now It was time to test /authenticate API. During testing of authentication end point it was going in infinite recursion which is not acceptable in any use case.

I have eliminated Infinite recursion by throwing an Exception when this == this.delegate.

I may have missed important details to share here as I'm new to the spring community. Please Let me know if more details required.

[marcusdacoregio]

Thanks @Milan-Toliya.

Are you able to create a minimal, reproducible example so I can simulate your scenario here? There are many constraints in your use case that I'd like to verify before moving on with the PR.

It'd also be great if you could open an issue and add the example and all other details there.

[Milan-Toliya]

Hi @marcusdacoregio

I tried to reproduce but had no luck. I will share the code once I am able to reproduce it.

Steps to reproduce:

1. Implement JWT Token with the longest expiration time. Then did some changes in both implementations of WebSecurityConfigurerAdapter. (NOTE: I have two Implementation of WebSecurityConfigurerAdapter)
2. Those changes made in code lead to a state such that, UserDetailsService(Interface) is auto wired with UserDetailsServiceDelegator (Inner class of the WebSecurityConfigurerAdapter)
3. Used JWT token generated in step 1 to GET data from the secure END (i.e. /helloworld)
4. JWT Filter will perform this action userDetailsService.loadUserByUsername(username) (ISSUE Infinite recursion loop in the method userDetailsService.loadUserByUsername when userDetailsService is instance of UserDetailsServiceDelegator)
5. I am not able to simulate the 2nd step right now UserDetailsService(Interface) is auto wiring with InMemoryUserDetailsManager which it actually should in a positive case. Not sure how I managed to get in 2nd step's state earlier.

Refer Below Image for classes mention above.

NOTE: I did not get an issue with AuthenticationManagerDelegator.authenticate() but By looking at the code it clearly visible that it suffers from the same infinite recursion loop when this == this.delegate (refer to the code change I have proposed or look at the source code )

Thanks.

[Milan-Toliya]

Hi @Milan-Toliya, thanks for contributing to Spring Security. Have you opened an issue before working on the code changes?

Can you help me understand what this PR fixes and what is the use case?

Hi, @marcusdacoregio I have opened an issue for this PR.
#11088

[marcusdacoregio]

Hi @Milan-Toliya, while we cannot simulate the root problem that is happening, this was brought to the team's attention and we think that your changes make sense since it's "cheaper" to throw an IllegalStateException than waiting for the StackOverflowError to happen.

If users have configured things in an unsupported way, throwing an IllegalStateException feels like an improvement from an error handling perspective.

Did you try adding any tests?

[marcusdacoregio]

Closing in favor of #12343