Skip to content

Standard RoleHierarchyVoter always return 1 #16358

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
sergio-ivanuzzo opened this issue Oct 27, 2015 · 3 comments
Closed

Standard RoleHierarchyVoter always return 1 #16358

sergio-ivanuzzo opened this issue Oct 27, 2015 · 3 comments
Labels
SecurityBundle

Comments

@sergio-ivanuzzo
Copy link

I found this bug in the process of solving the problem http://stackoverflow.com/questions/33346543/symfony2-how-to-disable-default-voter

Code of method "createRoleHierarchy" in the Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

if (!isset($config['role_hierarchy'])) {
    $container->removeDefinition('security.access.role_hierarchy_voter');
    return;
 }

not work properly, because role_hierarchy contains empty array, when I remove role_hierarchy from my config. so, isset($config['role_hierarchy']) always returns true

And according to comments at #Symfony irc channel from user dantleec1:

yeah, the Configuration class adds the key as an array: https://github.com/symfony/symfony/blob/2.3/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php#L136
so I guess NULL resolves to an empty array

To reprocude this bug you need to add code like

var_dump(get_class($voter) . ":" . $result);

to decideConsensus or another decide method of AccessDecisionManager.

I'm use linux, have Symfony version 2.7.5, PHP 5.5.9
@wouterj wouterj mentioned this issue Nov 4, 2015
Closed
@wouterj
Copy link
Member

wouterj commented Nov 4, 2015

For disabling the role_hierarchy voter, you had to completely remove the role_hierarchy setting. I've created a pull request to also disable the voter when an empty array (or null) is passed: #16460

In that PR, I also added tests for the RoleHierarchyVoter when passing an empty hierarchy. As you can see, it just behaves like the normal RoleVoter, so I cannot reproduce the "it always returns 1" statement. Can you provide code (e.g. fork the standard edition) to reproduce this issue?

@wouterj
Copy link
Member

wouterj commented Nov 4, 2015

Status: Works for me

@sergio-ivanuzzo
Copy link
Author

Hello, WouterJ. Thanks for answer. The description of your pull request is what I actually meant. I'm sorry for incorrect description. This issue can be closed.

fabpot added a commit that referenced this issue Nov 28, 2015
@fabpot
minor #16460 [SecurityBundle] Fix disabling of RoleHierarchyVoter whe…
6ae61f6 
…n passing empty hierarchy (WouterJ)

This PR was submitted for the 2.3 branch but it was merged into the 2.8 branch instead (closes #16460).

Discussion
----------

[SecurityBundle] Fix disabling of RoleHierarchyVoter when passing empty hierarchy

| 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

Commits
-------

96afff6 [SecurityBundle] Fix disabling of RoleHierarchyVoter when passing empty hierarchy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
SecurityBundle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@wouterj @xabbuh @sergio-ivanuzzo @carsonbot