-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Allow setting realm for Http Basic #6279
Conversation
769d136
to
71f73c0
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 PR! I provided feedback inline
* @since 5.2.0 | ||
* @author Ankur Pathak | ||
*/ | ||
public HttpBasicSpec realmName(String realmName){ |
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.
I think that rather than specifying realmName we should allow injection of the entryPoint instead. This gives the user more flexibility and we don't need this additional logic which does an instance of check.
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.
I do agree, allowing setting entryPoint will give user more flexibility. So method to set realmName
removed in its favour. The new method authenticationEntryPoint added in its place.
71f73c0
to
0b66c7a
Compare
1. Added method authenticationEntryPoint in ServerHttpSecurity to allow setting authenticationEntryPoint. 2. Added test in ServerHttpSecurityTests to check if if specified realm name set by authenticationEntryPoint is returned Fixes: spring-projectsgh-6270
0b66c7a
to
e210b93
Compare
Thanks for the PR @ankurpathak! This is now merged into master |
specifying realm name.
specified realm name is returned.
Pull request for issue: #6270