-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[SecurityBundle] Merging configuration requires access_decision_manager to be in every configuration #25609
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
Comments
access_decision_manager
to be in every configuration
This bug is still present in 4.2, is there any way we can help to solve this? I'm not sure where to start to fix this by myself. 😕 |
This issue seem's to be setted in the Normalization of Symfony\Bundle\SecurityBundle\DependencyInjection\MainConfiguration::getConfigTreeBuilder
If reproduce this issue by adding the config/package/security.yaml to the following:
If you set something in the config/package/dev/security.yaml for instance but don't set the access_decision_managers, it will override if with the default value set in the MainConfiguration when processing the second (dev config) and override the main config with the default normalization value. |
This bug is still present in Symfony 4.3 |
This bug is still present in Symfony 5.2, and when you wan to use own Access decision manager:
it also returns an error related to this issue:
ifTrue() returns false, but then() is still called. |
Also reported in #39939 status: reviewed |
Hey, thanks for your report! |
I don't think this was solved. |
I agree. The bug i.m.o. is very relevant and can cause security incidents. In our case, we created a custom voter, denying access to a certain resource for a user in 'demo-mode'. however, in the prod environment, we enabled login throttling in config/packages/prod/security.yaml using debug:config security told us that the decision strategy was now suddenly affermative a workaround would be to copy/paste settings to the prod/security.yaml configuration. That did solve the issue in this case, but makes you wondering what other settings are overwritten by defaults. On version 5.3.9 |
Same issue here, in 5.3.10 - and I quite agree with @Gielovic, in my case it was causing tests to behave differently that the app, which could lead to (authorization) issues being pushed to production. I'm not sure how to easily fix it (in Symfony code), though. |
…option with merge (biozshock) This PR was merged into the 4.4 branch. Discussion ---------- [SecurityBundle] Default access_decision_manager.strategy option with merge | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #25609 | License | MIT Security bundle should set default `access_decision_manager.strategy` in extension instead of configuration. Otherwise merging configurations will override first set option if the next configurations do not have the option set. Commits ------- f253995 Default access_decision_manager.strategy option with merge.
In an application i need to change
access_decision_manager.strategy
to "unanimous".After the change symfony/security-bundle@06393da#diff-5382c0b1af9e9c09693d49cd3b00aac9R62 i can not have a configuration in config_test.yml that just adds another firewall:
Because here https://github.com/symfony/symfony/blob/v3.4.2/src/Symfony/Component/Config/Definition/BaseNode.php#L269 config variable
$rightSide
for the second file will contain[access_decision_manager][strategy] = 'affirmative'
and after the merge it will overwrite any value insecurity.yml
foraccess_decision_manager.strategy
path.Test case:
The text was updated successfully, but these errors were encountered: