Skip to content

ServerHttpSecurity: oauth2Login() ignores securityContextRepository() #7222

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

Closed
ilgrosso opened this issue Aug 6, 2019 · 2 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@ilgrosso
Copy link
Contributor

ilgrosso commented Aug 6, 2019

In org.springframework.security.config.web.server.ServerHttpSecurity.OAuth2LoginSpec#configure, the line

authenticationFilter.setSecurityContextRepository(new WebSessionServerSecurityContextRepository());

does not take into account ServerHttpSecurity.this.securityContextRepository (which has the same default value, but can be customized by invoking securityContextRepository()) and blindly creates a new instance of WebSessionServerSecurityContextRepository.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 6, 2019
@jzheaux
Copy link
Contributor

jzheaux commented Aug 8, 2019

The JavaDoc for ServerHttpSecurity#securityContextRepository states:

It does not impact how the {@code SecurityContext} is saved which is configured on a per {@link AuthenticationWebFilter} basis

The way that HttpBasicSpec and FormLoginSpec are configured is as follows:

http
    .httpBasic()
        .securityContextRepository(...)

and

http
    .formLogin()
        .securityContextRepository(...)

Given the JavaDoc and the way that other authentication mechanisms work, it would seem to be preferable instead to add a securityContextRepository method to the oauth2Login DSL:

http
    .oauth2Login()
        .securityContextRepository(...)

Would you be willing to provide a PR along those lines instead? It would be one that introduces securityContextRepository(...) to OAuth2LoginSpec.

@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 8, 2019
@jzheaux jzheaux added this to the 5.2.0.RC1 milestone Aug 8, 2019
@jzheaux jzheaux self-assigned this Aug 8, 2019
@ilgrosso
Copy link
Contributor Author

ilgrosso commented Aug 9, 2019

@jzheaux thanks for review; here's the new PR: #7244

kostya05983 pushed a commit to kostya05983/spring-security that referenced this issue Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants