Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Propagate Authorities From Previous Factors #17790

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

Open
jzheaux wants to merge 11 commits into spring-projects:main
base: main
Choose a base branch
Loading
from jzheaux:authentication-builder

Conversation

Copy link
Contributor

@jzheaux jzheaux commented Aug 21, 2025 •
edited
Loading

Applications can use AuthenticationBuilder to apply existing authentications to new ones.

For example, if the current logged in user is represented by:

Authentication firstFactor = ...

And they provide a second set of authenticated credentials, represented by:

Authentication secondFactor = ...

Then the first factor can be applied to the second factor as follows:

secondFactor = secondFactor.toBuilder().apply(firstFactor).build();

This draft PR adds a basic builder to each Authentication implementation that implements Authentication.Builder. In order to simplify upgrades, toBuilder by default returns a no-op implementation of Authentication.Builder that ultimately returns the same authentication unchanged.

@jzheaux jzheaux changed the title (削除) Authentication Builder (削除ここまで) (追記) Propagate Authorities From Previous Factors (追記ここまで) Aug 21, 2025
@jzheaux jzheaux force-pushed the authentication-builder branch 6 times, most recently from 6eb00d0 to b48b10a Compare August 22, 2025 22:25
Copy link
Member

@rwinch rwinch left a 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 @jzheaux! I've provided feedback inline, but my main pieces of feedback are:

  • The AuthenticationManager should not be accessing the SecurityContext. Instead, we should have the controller (e.g. Filter) that invokes the AuthenticationManager perform the merging of the two Authentication instances.
  • I think that the builder APIs should function independently of MFA and should work for any properties on the Authentication. Doing this would also allow deprecation of the setAuthenticated method.
  • I don't think we should have an Authentication.apply(Authentication) method. Especially so if it is only applying the authorities and ignoring many other properties that are on the Authentication object.

@@ -54,6 +57,9 @@
*/
public interface Authentication extends Principal, Serializable {

@Serial
long serialVersionUID = -3884394378624019849L;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me to understand why this is specified?

Copy link
Contributor Author

@jzheaux jzheaux Aug 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deserialization of concrete implementations fails otherwise. With the inclusion of a default method, Java now treats it like an abstract class from a serialization perspective. From what I understand, any class, abstract or otherwise, should have a version id if it extends Serializable.

@jzheaux jzheaux force-pushed the authentication-builder branch from b48b10a to 41398f0 Compare August 29, 2025 20:28
jzheaux added 9 commits August 30, 2025 10:00 
The sample better matches a value that would be used in the constructor
Issue spring-projectsgh-16276 
Given that 7e3bf96 changes
the InteractiveAuthenticationSuccessEvent serialization sample,
this commit syncs up the 6.5.x version to match.
Issue spring-projectsgh-16276 
Given that 7e3bf96 changes
the InteractiveAuthenticationSuccessEvent serialization sample,
this commit syncs up the 7.0.x version to match.
Closes spring-projectsgh-16276 
This commit adds a new default method to Authentication
for the purposes of creating a Builder based on the current
authentication, allowing other authentications to be
applied to it as a composite.
It also adds Builders for each one of the authentication
result classes.
This commit allows looking up the current authentication and applying
it to the latest authentication. This is specifically handy when
collecting authorities gained from each authentication factor.
This commit provides the SecurityContextHolderStrategy bean to
ProviderManager instances that the HttpSecurity DSL constructs.
- Added remaining properties
- Removed apply method since Spring Security isn't using
it right now
- Made builders extensible since the authentications are
extensible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwinch rwinch rwinch requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants

AltStyle によって変換されたページ (->オリジナル) /