Skip to content

Add a method named authenticationProviders() in HttpSecurityBuilder #14586

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
ghusta opened this issue Feb 12, 2024 · 3 comments
Closed

Add a method named authenticationProviders() in HttpSecurityBuilder #14586

ghusta opened this issue Feb 12, 2024 · 3 comments
Assignees
Labels
in: config An issue in spring-security-config status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@ghusta
Copy link
Contributor

ghusta commented Feb 12, 2024

Could you consider adding a method authenticationProviders() in interface HttpSecurityBuilder ?

It could take varargs or List as parameter.

Expected Behavior

It would make it more obvious that HttpSecurity can accept multiple Authentication Providers.

Current Behavior

httpSecurity.authenticationProvider() must be called multiple times. The order is of course important.

Currently, ProviderManager already supports this behaviour, with constructors like :

  • public ProviderManager(AuthenticationProvider... providers)
  • public ProviderManager(List<AuthenticationProvider> providers)

Related with #11428 ?

@ghusta ghusta added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Feb 12, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Feb 12, 2024

Hi, @ghusta. I see your point about how taking a varargs would make the ordering clearer. I suppose the concern would be now it might seem like authenticationProvider and authenticationProviders would conflict. It wouldn't get rid of the ordering issue since I suppose you could reasonably call authenticationProviders and authenticationProvider in the same stack unless authenticationProvider is also deprecated.

Just curious, have you considered using .authenticationManager instead? As you pointed out, ProviderManager already communicates the possibility of multiple providers. Using this method also aligns with #11428 since that ticket indicates that .authenticationProvider may eventually go away.

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 12, 2024
@ghusta
Copy link
Contributor Author

ghusta commented Feb 12, 2024

I don't really think that authenticationProvider and authenticationProviders would conflict.

As authentication providers are stored in a List in AuthenticationManagerBuilder, I imagine it would not be different to using List.add() and List.addAll(), in whatever order.

I'm just wondering if it's worth working on it, depending of the plans to deprecate AuthenticationProvider.

And yes, maybe it's more logical or natural to configure HttpSecurity following this pattern :

httpSecurity > authenticationManager( new ProviderManager( List of AuthenticationProvider ) )

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 12, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Feb 12, 2024

@ghusta, good points. I think let's hold off, given that folks can already use authenticationManager.

@jzheaux jzheaux closed this as completed Feb 12, 2024
@jzheaux jzheaux added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided labels Feb 12, 2024
@jzheaux jzheaux self-assigned this Feb 12, 2024
@jzheaux jzheaux added the in: config An issue in spring-security-config label Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants