-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Reactive SwitchUserWebFilter for user impersonation #8687
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
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 @aotrzonsek!
I have left some feedback inline.
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java
Outdated
Show resolved
Hide resolved
b9d5eb5
to
5590f29
Compare
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 @aotrzonsek!
I have left a few more comments.
There are also some comments from the previous review that are unresolved (test names and Javadocs)
|
||
/** | ||
* Switch User processing filter responsible for user context switching. | ||
* It is a reactive reimplementation of the |
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.
We should have the full explanation of the filter here, rather than directing the users to look at SwitchUserFilter
.
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.
done
|
||
/** | ||
* @author Artur Otrzonsek | ||
* @since 5.2.6 |
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.
We don't need to add @since
for test classes.
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.
done
Before: It was not possible to switch the user context by using Spring WebFlux Security After: Added an appropriate filter that allows switch the user context in Spring WebFlux Security Closes spring-projectsgh-8599
5590f29
to
98b472b
Compare
Thanks @aotrzonsek! This is now merged into master via b22c50c. |
Before:
It was not possible to switch the user context by using Spring WebFlux Security
After:
Added an appropriate filter that allows switch the user context in Spring WebFlux Security
Closes gh-8599