Specify default security user provider.

[drupol]

Hello,

I published a Symfony bundle ecphp/cas-bundle, that bundle has a Symfony flex recipe.

When users are trying the bundle against the Symfony demo (symfony new project --demo), it fails during installation because the flex recipe copies configuration files from the bundle into the app config directory.
Among the copied files, there is this one in particular, which contains a new security user provider.

The demo app has a firewall already configured to use the only existing provider: database_users.

When installing the demo app and the ecphp/cas-bundle bundle, it fails because multiple user providers exists in the application and the firewall do not know which one to use for the form_login listener.

My proposal is to explicitly specify which user provider to use for the form_login listener so we don't have this issue anymore with bundles providing their own security user provider.

When modifying the security.yaml file before running the composer require ecphp/cas-bundle, it works without any issue.

Log

$ symfony new test-ecas --demo
* Creating a new Symfony Demo project with Composer
  (running /home/pol/bin/composer create-project symfony/symfony-demo /home/pol/dev/git/symfony-demo/test-ecas)

* Setting up the project under Git version control
  (running git init /home/pol/dev/git/symfony-demo/test-ecas)
                                                                                                                        
 [OK] Your project is now ready in /home/pol/dev/git/symfony-demo/test-ecas                                                   
 
 $ cd test-ecas/
$ composer require ecphp/cas-bundle
Using version ^2.1 for ecphp/cas-bundle
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Restricting packages listed in "symfony/symfony" to "5.1.*"
Package operations: 11 installs, 0 updates, 0 removals
  - Installing psr/http-message (1.0.1): Loading from cache
  - Installing symfony/psr-http-message-bridge (v2.0.1): Loading from cache
  - Installing symfony/http-client-contracts (v2.1.3): Loading from cache
  - Installing symfony/http-client (v5.1.2): Loading from cache
  - Installing psr/http-factory (1.0.1): Loading from cache
  - Installing php-http/message-factory (v1.0.2): Loading from cache
  - Installing nyholm/psr7 (1.3.0): Loading from cache
  - Installing psr/http-client (1.0.1): Loading from cache
  - Installing league/uri-query-parser (1.0.1): Loading from cache
  - Installing ecphp/cas-lib (1.0.5): Loading from cache
  - Installing ecphp/cas-bundle (2.1.0): Loading from cache
Package zendframework/zend-code is abandoned, you should avoid using it. Use laminas/laminas-code instead.
Package zendframework/zend-eventmanager is abandoned, you should avoid using it. Use laminas/laminas-eventmanager instead.
Writing lock file
Generating autoload files
70 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Symfony operations: 2 recipes (dd529a900427cb02dbeddf6e23d4992a)
  - Configuring nyholm/psr7 (>=1.0): From github.com/symfony/recipes:master
  - Configuring ecphp/cas-bundle (>=2.0): From github.com/symfony/recipes-contrib:master
ocramius/package-versions:  Generating version class...
ocramius/package-versions: ...done generating version class
Executing script cache:clear [KO]
 [KO]
Script cache:clear returned with error code 1
!!
!!  In SecurityExtension.php line 626:
!!
!!    Not configuring explicitly the provider for the "form_login" listener on "m
!!    ain" firewall is ambiguous as there is more than one registered provider.
!!
!!
!!
Script @auto-scripts was called via post-update-cmd

Installation failed, reverting ./composer.json to its original content.
$

[javiereguiluz]

Thanks for this proposal Pol ... I don't know much about this (@wouterj knows everything about the new security system) but I think this proposal makes sense (I even think that Symfony made it "mandatory" a while ago ... or maybe it was a related change, I can't remember exactly).

In any case, I like the overall idea ... and I like it even more because it solves your issue. But let's wait for more comments to see if there's some technical drawback. Thanks!

[drupol]

Waiting for @wouterj 's feedback.

[wouterj]

👍 The implicit default is nice for quick bootstrapping, but it makes more sense to explicitly specify the user providers for each authentication provider. The MakerBundle was also recently updated to always specify the user provider, so I think it makes sense to update the demo app as well.

I think it might make more sense to have this option as last in the list instead of first (the first ones are specific for the form_login).

Btw, I think you're talking about the entry_point being mandatory in the new security system, Javier. However, the entry_point DX when using the old system is quite bad - so we should not add it atm (demo is not using the new system yet).

[drupol]

Going to make the changes now.

[drupol]

@javiereguiluz and @wouterj Ready for review again now.

[drupol]

Do you think we can move this forward now @javiereguiluz ?

[wouterj]

It's barely 23 hours since the last comment on this PR. There is a lot more to do in the life of open source maintainers than merging PRs in one of the 10+ packages they maintain :)

[drupol]

That is true as well :-) Didn't meant to push, sorry ✌️

[javiereguiluz]

And this is now merge 🎉 Thanks Pol for the contribution and thanks Wouter for the quick review!

[drupol]

Merci, hope you don't feel offended by my previous message!

[javiereguiluz]

Of course not!!! We usually merge things fast ... but these days I'm a bit busy with work-related stuff. But I completely understand the excitement to get some contribution merged fast (I feel the same all the time with my contributions 😊 )