Skip to content
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

Authorization voters missing in 3.1 #20211

Closed
dspiegel opened this issue Oct 12, 2016 · 9 comments

Comments

Projects
None yet
6 participants
@dspiegel
Copy link

commented Oct 12, 2016

Previously Posted issue in symfony/symfony-standard, but was told it belongs here

Just upgraded from 3.0.9 to 3.1 and was not expecting any problems, but my app can no longer log in. I've looked for others having same type of issue and spent several days trying to determine what might have changed.

I'm really not sure where the problem starts, but what occurs is:

Token does not have the required roles.
403 Forbidden - AccessDeniedHttpException
1 linked Exception: AccessDeniedException »

I am using GuardAuthenticator with a token. It appears as if all the Roles and User are being created as expected, but then Authorization fails. The user has proper Roles to access page.

I traced the actually point of failure at JMS\SecurityExtraBundle\Security\Authorization\Interception
accessDecisionManager->decide call. I see no changes that were done to this code from 3.0.9 to 3.1.5.

if (!empty($metadata->roles) && false === $this->accessDecisionManager->decide($token, $metadata->roles, $method)) {
            throw new AccessDeniedException('Token does not have the required roles.');
 }

accessDecisionManager->decide() resides in Symfony\Component\Security\Core\Authorization class
which calls decideAffirmative(TokenInterface $token, array $attributes, $object = null)

Version 3.0.9 has 4 different Voters at this point (decideAffirmative) which provide a true (success) response.

array:4 [▼
  0 => LazyLoadingExpressionVoter {#458 ▶}
  1 => ExpressionVoter {#454 ▶}
  2 => RoleHierarchyVoter {#431 ▶}
  3 => AuthenticatedVoter {#434 ▶}
]

While Version 3.1.5 has 0 Voters which caused false return and failed login.

So something happened from 3.0 -> 3.1 which caused BC because the 4 default voters from 3.0.9 are gone.

Perhaps I missed something when setting up Guard, but I cannot find any demo's or examples for Guard other than using Symfony 2.8 or 3.0.

Any help to understand what might have caused this would be appreciated. Do I need to now create these Voters for my app?

Thanks, Dave

@dspiegel

This comment has been minimized.

Copy link
Author

commented Oct 12, 2016

I just tracked down to the exact function in appDevDebugProjectContainer.php cache that causes this issue:
getSecurity_Access_DecisionManagerService

I replaced the changed code in 3.1 with code from 3.0.9 and my app is working again. I'm having trouble finding the actual piece of code that does this, but hopefully the cache function name will help.

Symfony 3.0.9 working code =

  protected function getSecurity_Access_DecisionManagerService()
    {
        $a = $this->get('security.authentication.trust_resolver');
        $b = $this->get('security.role_hierarchy');

        $c = new \JMS\SecurityExtraBundle\Security\Authorization\Expression\LazyLoadingExpressionVoter($this->get('security.expressions.handler'), $this->get('monolog.logger.security', ContainerInterface::NULL_ON_INVALID_REFERENCE));
        $c->setLazyCompiler($this, 'security.expressions.compiler');
        $c->setCacheDir((__DIR__.'/jms_security/expressions'));

        $d = new \Symfony\Component\Security\Core\Authorization\AccessDecisionManager(array(), 'affirmative', false, true);
        $d->setVoters(array(0 => $c, 1 => new \Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter(new \Symfony\Component\Security\Core\Authorization\ExpressionLanguage(), $a, $b), 2 => new \Symfony\Component\Security\Core\Authorization\Voter\RoleHierarchyVoter($b), 3 => new \Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter($a)));

        return $this->services['security.access.decision_manager'] = new \JMS\SecurityExtraBundle\Security\Authorization\RememberingAccessDecisionManager($d);
    }

Symfony 3.1.5 broken code =

    protected function getSecurity_Access_DecisionManagerService()
    {
        $a = $this->get('security.authentication.trust_resolver');
        $b = $this->get('security.role_hierarchy');

        $c = new \JMS\SecurityExtraBundle\Security\Authorization\Expression\LazyLoadingExpressionVoter($this->get('security.expressions.handler'), $this->get('monolog.logger.security', ContainerInterface::NULL_ON_INVALID_REFERENCE));
        $c->setLazyCompiler($this, 'security.expressions.compiler');
        $c->setCacheDir((__DIR__.'/jms_security/expressions'));

        $this->services['security.access.decision_manager'] = $instance = new \Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager(new \JMS\SecurityExtraBundle\Security\Authorization\RememberingAccessDecisionManager(new \Symfony\Component\Security\Core\Authorization\AccessDecisionManager(array(), 'affirmative', false, true)));

        $instance->setVoters(array(0 => $c, 1 => new \Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter(new \Symfony\Component\Security\Core\Authorization\ExpressionLanguage(), $a, $b), 2 => new \Symfony\Component\Security\Core\Authorization\Voter\RoleHierarchyVoter($b), 3 => new \Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter($a)));

        return $instance;
    }

@dspiegel

This comment has been minimized.

Copy link
Author

commented Oct 12, 2016

@fabpot and @HeahDude

Looks like the BC break occurred as part of merge in April.
Commit 53c78fe
File = src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSecurityVotersPass.php

When I revert that file back to the 3.0.9 version, my 3.1.5 app works again.

If this just requires some change on my part, please let me know.

Thanks, Dave

@HeahDude

This comment has been minimized.

Copy link
Member

commented Oct 12, 2016

@dspiegel

This comment has been minimized.

Copy link
Author

commented Oct 13, 2016

@HeahDude not sure if it is same issue, as the file I reverted to make my app work was the AddSecurityVotersPass.php which does not seem to be part of JMSSecurityBundle. I had looked at JMS issue before and it seemed to be about RemeberMe functionality, which I am not using.

I will agree that it is in the same mess, but since JMS has not touched his code for Years, not sure how it started affecting 3.1.5. I don't know, you guys understand all this better than I do. I will just continue using the 3.0.9 version of AddSecurityVotersPass.php file while someone sorts this all out.

Thanks, Dave

@xabbuh

This comment has been minimized.

Copy link
Member

commented Oct 14, 2016

@dspiegel I guess the issue is that in all releases of the JMSSecurityExtraBundle there is no setVoters() method in the RememberingAccessDecisionManager class which means that Symfony's DebugAccessDecisionManager does not forward calls to setVoters() here. A setVoters() method has been added in schmittjoh/JMSSecurityExtraBundle#209, but this change was not part of any release yet (you can try the patch by changing your version constraint for the JMSSecurityExtraBundle to ~1.6@dev).

@dspiegel

This comment has been minimized.

Copy link
Author

commented Oct 14, 2016

Thanks @xabbuh, I will try it. I'm not sure understand how remember me affects my app as I don't use that functionality. As far as I know.

  • Dave
@javiereguiluz

This comment has been minimized.

Copy link
Member

commented Nov 8, 2016

Let's close this one as "fixed". The issue was fixed and merged in the related repository and now it's a matter of waiting until @GuilhemN prepares a new stable release of that bundle. Then, you just need to run composer update jms/security-extra-bundle to upgrade that particular bundle in your Symfony app. Thanks!

@GuilhemN

This comment has been minimized.

Copy link
Contributor

commented Nov 11, 2016

@javiereguiluz this is part of 1.6.1 :)

@javiereguiluz

This comment has been minimized.

Copy link
Member

commented Nov 11, 2016

@GuilhemN thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.