Skip to content

[SecurityBundle] Fix disabling of RoleHierarchyVoter when passing empty hierarchy #16460

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
wants to merge 2 commits into from

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Nov 4, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #16358
License MIT
Doc PR -
  • When passing role_hierarchy: ~ in the config, the role hierarchy voter was still enabled. I've now changed this so that an empty hierarchy also results in disabling this voter. With an empty hierarchy, the voter behaves exactly the same as the RoleVoter, so no BC break is introduced here.
  • Added some tests for the RoleHierarchyVoter when passing an empty hierarchy. As it then behaves exactly like RoleVoter, the question is whether we shouldn't just always return ACCESS_ABSTAIN when the hierarchy is empty

@@ -157,7 +157,7 @@ private function configureDbalAclProvider(array $config, ContainerBuilder $conta
*/
private function createRoleHierarchy($config, ContainerBuilder $container)
{
if (!isset($config['role_hierarchy'])) {
if (!isset($config['role_hierarchy']) || 0 === count($config['role_hierarchy'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think it is always set due to the way the configuration class works

@fabpot
Copy link
Member

fabpot commented Nov 23, 2015

Is it really a bug or just a simplification? If the latter, we need to merge this into 2.8 instead.

@wouterj
Copy link
Member Author

wouterj commented Nov 28, 2015

@fabpot I think we can call this a simplification.

@fabpot
Copy link
Member

fabpot commented Nov 28, 2015

Thank you @wouterj.

@fabpot fabpot closed this in 6ae61f6 Nov 28, 2015
@fabpot fabpot reopened this Nov 28, 2015
@fabpot fabpot closed this Nov 28, 2015
@wouterj wouterj deleted the issue_16358 branch November 28, 2015 15:47
nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull request Nov 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants