Skip to content

Infinite recursion in WebSecurityConfigurerAdapter.java. #11088

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

Closed
Milan-Toliya opened this issue Apr 9, 2022 · 6 comments
Closed

Infinite recursion in WebSecurityConfigurerAdapter.java. #11088

Milan-Toliya opened this issue Apr 9, 2022 · 6 comments
Assignees
Labels
in: config An issue in spring-security-config status: declined A suggestion or change that we don't feel we should currently apply type: bug A general bug

Comments

@Milan-Toliya
Copy link

Milan-Toliya commented Apr 9, 2022

Bug
Infinite recursion in WebSecurityConfigurerAdapter.java.
Refer PR where this issue and its solution are committed.

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.
image

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 )

Expected behavior

Instead of going into the Infinite recursion, it should follow the positive flow or should throw any Exception (Which I have suggested as a solution in PR. I am throwing IllegalStateException). Code MUST NOT go into infinite recursion (i.e. Stack overflow state)

@Milan-Toliya Milan-Toliya added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 9, 2022
@marcusdacoregio marcusdacoregio self-assigned this Apr 11, 2022
@marcusdacoregio marcusdacoregio added status: waiting-for-triage An issue we've not yet triaged and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 11, 2022
@marcusdacoregio
Copy link
Contributor

Hi @Milan-Toliya, I could not simulate the issue here since there is a lot of unknown stuff going on.

I did research on the repository's issues and did not find anything related, we really need a minimal, reproducible sample so we can make sure that this is a problem with Spring Security and not with the application setup. Are you able to isolate the issue?

Thank you.

@marcusdacoregio marcusdacoregio added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 13, 2022
@Milan-Toliya
Copy link
Author

Hi @marcusdacoregio

I am able to simulate the issue of infinite recursion for the authentication method instead of the loadUserByUsername method.

I will share the code here so you have look.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 15, 2022
@Milan-Toliya
Copy link
Author

Milan-Toliya commented Apr 18, 2022

Hi @marcusdacoregio,

Apologies for the late reply.

As per the earlier discussion what I am able to reproduce is the authentication method infinite recursion instead of the loadUserByUsername method infinite recursion.

I have uploaded the code to my Github repository. Focus only on the package com.demo.springboot.config

Note: I have figured out the issue and fixed it in the actual project. The above project is just for the reference for the fix of #11071

Thanks.

@marcusdacoregio
Copy link
Contributor

Hi @Milan-Toliya, I took a look at your repository but there is a lot going on. Also, there are no steps to reproduce the issue so it became hard for me to figure out what is happening.

In order to be able to simulate the problem, we really need a minimal, reproducible sample with tests or steps to reproduce the issue.

@marcusdacoregio marcusdacoregio added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels May 6, 2022
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label May 13, 2022
@marcusdacoregio marcusdacoregio removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels May 13, 2022
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 13, 2022
@sjohnr sjohnr added in: config An issue in spring-security-config type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels May 19, 2022
@marcusdacoregio marcusdacoregio added this to the 5.8.x milestone Jun 2, 2022
@marcusdacoregio marcusdacoregio removed this from the 5.8.x milestone Dec 16, 2022
@marcusdacoregio marcusdacoregio added the status: declined A suggestion or change that we don't feel we should currently apply label Dec 16, 2022
@marcusdacoregio
Copy link
Contributor

Closing in favor of #12343

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config status: declined A suggestion or change that we don't feel we should currently apply type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants