Skip to content

[Security][DX] Allow omitting provider in firewall config, when each authenticator has a provider set explicitly #24980

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.

Sign up for GitHub

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

Jump to bottom
Closed
backbone87 opened this issue Nov 15, 2017 · 4 comments
Closed

[Security][DX] Allow omitting provider in firewall config, when each authenticator has a provider set explicitly #24980

backbone87 opened this issue Nov 15, 2017 · 4 comments
Assignees
@chalasr
Labels
Bug DX DX = Developer eXperience (anything that improves the experience of using Symfony) Security Status: Needs Review
Milestone
3.4

Comments

@backbone87
Copy link
Contributor

Given the following firewall config:

        main:
            pattern:   ^/
            stateless: true
            http_basic:
                provider: in_memory
            http_basic_ldap:
                provider:     ldap
                service:      Symfony\Component\Ldap\Ldap
                dn_string:    "%env(LDAP_USER_BASE_DN)%"
                query_string: "sAMAccountName={username}"

This config causes following deprecation notice:
Firewall "main" has no "provider" set but multiple providers exist. Using the first configured provider (in_memory) is deprecated since 3.4 and will throw an exception in 4.0, set the "provider" key on the firewall instead.
Because there is no "default" provider set as a child of the main key.

However each authenticator has a provider set explicitly, so there is no need for the "default" provider.

It would be nice, if this kind of config is allowed (no deprecation and working in 4.0)
@xabbuh xabbuh added this to the 3.4 milestone Nov 15, 2017
@chalasr chalasr self-assigned this Nov 15, 2017
@javiereguiluz javiereguiluz added the DX DX = Developer eXperience (anything that improves the experience of using Symfony) label Nov 19, 2017
@javiereguiluz
Copy link
Member

This makes a lot of sense to me ... but security is always a delicate matter, so let's wait and see if some security expert can verify that this wouldn't introduce any problem.

@chalasr
Copy link
Member

chalasr commented Nov 19, 2017 •
edited
Loading

Tricky one :/
I'd love to say that you're right and you should not get this deprecation, sadly that's not true.

The user provider configured on a firewall is the one injected in the security listeners configured on that firewall. Even if you are using your own ldap service which has its own user provider, the provider that will be used by the listener is the one configured on the firewall, meaning that depending on what your Ldap service does, it might deal with a different user than the one stored in the security token storage, potentially leading to weird behaviors in your security setup.
The issue was the same before 3.4, it was just hidden: the first configured user provider was automatically injected, so you were using 2 providers without being ever noticed.

The root issue is that the security system is too much tight to the user provider concept, which is itself tight to the configured firewalls (that's a per firewall/listener thing, not a service's internal thing, sadly). This is a known, hard issue which we plan to work on for 5.0 (#21998).

I think we can close. \cc @weaverryan

@chalasr
Copy link
Member

chalasr commented Nov 19, 2017

Oh sorry, I got it wrong. You do have providers configured per listener, but not at the firewall root level. We should fix that.

@chalasr chalasr added the Bug label Nov 19, 2017
@chalasr chalasr mentioned this issue Nov 20, 2017
Merged
@chalasr
Copy link
Member

chalasr commented Nov 20, 2017

See #25045

nicolas-grekas added a commit that referenced this issue Nov 20, 2017
@nicolas-grekas
bug #25045 [SecurityBundle] Don't trigger auto-picking notice if prov…
e3efa2f 
…ider is set per listener (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[SecurityBundle] Don't trigger auto-picking notice if provider is set per listener

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24980
| License       | MIT
| Doc PR        | n/a

Commits
-------

19e891a [SecurityBundle] Don't trigger auto-picking notice if provider is set per listener
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chalasr chalasr

Labels
Bug DX DX = Developer eXperience (anything that improves the experience of using Symfony) Security Status: Needs Review
Projects
None yet
Milestone
3.4
Development

No branches or pull requests
6 participants
@javiereguiluz @nicolas-grekas @backbone87 @xabbuh @chalasr @carsonbot