Skip to content
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

Make Authentication/SecurityContext Immutable #8323

Open
5 tasks
rwinch opened this issue Apr 3, 2020 · 3 comments
Open
5 tasks

Make Authentication/SecurityContext Immutable #8323

rwinch opened this issue Apr 3, 2020 · 3 comments
Assignees
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement

Comments

@rwinch
Copy link
Member

rwinch commented Apr 3, 2020

Summary

NOTE: This was originally brought up as a question in gh-8322 this ticket is to resolve that question

Ideally we would provide Authentication implementations that are immutable to avoid race conditions. However, for historical reasons it is unfortunate that both Authentication and SecurityContext are mutable.

We should look into how we can make both of these immutable

  • Deprecate Authentication#setAuthenticated #16668
  • (breaking) Remove calls to setAuthenticated from implementations
  • Add SecurityContextHolder#setAuthentication
  • Deprecate SecurityContext#setAuthentication
  • Consider Adding SecurityContextHolderStrategy#createSecurityContext(Authentication)
@rwinch rwinch added in: core An issue in spring-security-core type: enhancement A general enhancement labels Apr 3, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Jul 1, 2021

Regarding SecurityContext, I think SecurityContextImpl should add a constructor that takes Authentication, and its setAuthentication method should be deprecated.

Then an application could do:

SecurityContext context = new SecurityContextImpl(authentication);
SecurityContextHolder.setContext(context);

whenever the authentication needs to change.

It's not really clear to me why the SecurityContextHolderStrategy has a createEmptyContext (instead of apps constructing a security context directly); however, if it's important to route construction through SecurityContextHolder, then adding something like SecurityContextHolder#createContext(Authentication) could address that.

@SpiReCZ
Copy link

SpiReCZ commented Apr 27, 2022

Security Context was already made immutable in #10032 and it created issue in: #11100

@rwinch
Copy link
Member Author

rwinch commented Jun 7, 2022

Given this is potentially a breaking change, we should consider it for 6.0.x

@rwinch rwinch self-assigned this Jun 14, 2022
@jzheaux jzheaux removed this from the 6.0.x milestone Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants