-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Use the custom ServerRequestCache for Oauth2LoginSpec #7734
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
Conversation
Also, the |
Hi @sdoxsee I believe the success handler should be using the same as the others Lines 3115 to 3116 in c40a17b
|
Thanks @fhanik. I think that's for FormLoginSpec as opposed to OAuth2LoginSpec's configure? |
Correct. So yes, it's still an issue in the OAuth2LoginSpec. I'll update the PR to encapsulate both use scenarios. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @fhanik. Please see my comments.
config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java
Outdated
Show resolved
Hide resolved
config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java
Outdated
Show resolved
Hide resolved
config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @fhanik. Please go ahead and merge this after you remove the 2x unused imports.
@@ -76,9 +76,11 @@ | |||
import org.springframework.security.oauth2.client.web.server.AuthenticatedPrincipalServerOAuth2AuthorizedClientRepository; | |||
import org.springframework.security.oauth2.client.web.server.OAuth2AuthorizationCodeGrantWebFilter; | |||
import org.springframework.security.oauth2.client.web.server.OAuth2AuthorizationRequestRedirectWebFilter; | |||
import org.springframework.security.oauth2.client.web.server.ServerAuthorizationRequestRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused import
import org.springframework.security.oauth2.client.web.server.ServerOAuth2AuthorizationCodeAuthenticationTokenConverter; | ||
import org.springframework.security.oauth2.client.web.server.ServerOAuth2AuthorizationRequestResolver; | ||
import org.springframework.security.oauth2.client.web.server.ServerOAuth2AuthorizedClientRepository; | ||
import org.springframework.security.oauth2.client.web.server.WebSessionOAuth2ServerAuthorizationRequestRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused import
@fhanik I noticed you applied the "for: backport-to-5.2.x" to this PR, but it needs to be applied to the associated issue instead. The backport bot does not work on PR's. |
on for the default authentication entry point and authentication success handler Fixes spring-projectsgh-7721 spring-projects#7721 Set RequestCache on the Oauth2LoginSpec default authentication success handler import static ReflectionTestUtils.getField Feedback incorporated per spring-projects#7734 (review)
on for the default authentication entry point and authentication success handler Fixes spring-projectsgh-7721 spring-projects#7721 Set RequestCache on the Oauth2LoginSpec default authentication success handler import static ReflectionTestUtils.getField Feedback incorporated per spring-projects#7734 (review)
on for the default authentication entry point and authentication success handler Fixes gh-7721 #7721 Set RequestCache on the Oauth2LoginSpec default authentication success handler import static ReflectionTestUtils.getField Feedback incorporated per #7734 (review)
Fixes gh-7721
Test case provided