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

Authentication Mechanisms Should Default their ServerSecurityContextRepository #7249

Closed
jzheaux opened this issue Aug 10, 2019 · 2 comments · Fixed by #7275

Comments

@jzheaux
Copy link
Contributor

commented Aug 10, 2019

The Javadoc for ServerHttpSecurity#securityContextRepository states:

The strategy used with {@code ReactorContextWebFilter}. It does not impact how the {@code SecurityContext} is saved which is configured on a per {@link AuthenticationWebFilter} basis.

This is, however, outdated since form login does use this value if it's specified by the application:

if (this.securityContextRepository != null) {
	this.formLogin.securityContextRepository(this.securityContextRepository);
}

I had a chat with @rwinch, and the desired functionality is the following:

If an application wires a security context repository for a given authentication mechanisms, that takes precedence for that mechanism. For example:

http
    .securityContextRepository(a)
    .formLogin()
        .securityContextRepository(b);
http
    .formLogin()
        .securityContextRepository(b);

In both cases, b would be used for form login.

If not, then, the authentication mechanism should use the global one:

http
    .securityContextRepository(a)
        .formLogin();

In this case, a would be used for form login.

If none is specified by the application, each authentication mechanisms should use what it already defaults to.

So, to complete this task, there are three steps:

  1. Update the logic in form login, which currently accepts the global value regardless
  2. Update the logic in http basic and oauth2 login, which currently don't consider the global value at all
  3. Update the JavaDoc to indicate that setting the securityContextRepository does affect the underlying authentication mechanisms.
@eddumelendez

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2019

I will take a look at this one @jzheaux

@jzheaux

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2019

Sounds great, @eddumelendez, it's yours.

eddumelendez added a commit to eddumelendez/spring-security that referenced this issue Aug 18, 2019
eddumelendez added a commit to eddumelendez/spring-security that referenced this issue Aug 18, 2019
eddumelendez added a commit to eddumelendez/spring-security that referenced this issue Sep 2, 2019

@jzheaux jzheaux closed this in #7275 Sep 3, 2019

jzheaux added a commit that referenced this issue Sep 3, 2019

@jzheaux jzheaux self-assigned this Sep 4, 2019

AndreasKl added a commit to AndreasKl/spring-security that referenced this issue Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.