-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Fix/role security identity #5171
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
Conversation
@schmittjoh As you previously rejected such change saying it was a security issue, please explain why (and add a comment in the code to avoid later changes) |
similar PR: #5076 |
We're also using RoleInterface. This would be really nice to have, as it makes Symfony2 even more flexible. Please @schmittjoh why is it a security vulnerability? |
A quick and dirty workaround that worked for me: In my app/config/config.xml <parameters>
<parameter key="security.acl.security_identity_retrieval_strategy.class">{my bundle}\Security\Acl\Domain\SecurityIdentityRetrievalStrategy</parameter>
</parameters> Copy SecurityIdentityRetrievalStrategy to {my bundle}\Security\Acl\Domain\SecurityIdentityRetrievalStrategy /**
* {@inheritDoc}
*/
public function equals(SecurityIdentityInterface $sid)
{
if (!$sid instanceof RoleSecurityIdentity && !$sid instanceof SymfonyRoleSecurityIdentity) {
return false;
}
return $this->role === $sid->getRole();
} |
Any news about this? |
@schmittjoh ping |
Closing as this is a duplicate. |
The documentation seems to assume the implementation present in commit symfony/symfony#1673, which reverted soon after due to a potential, but undisclosed security hole (citation @schmittjoh in symfony/symfony@af70ac8). This incorrect documentation has likely been the source of many of the following issues: * symfony/symfony#1538 - [ACL RoleSecurityIdentity] check if instance of Role * symfony/symfony#1748 - Replace Role to RoleInterface for RoleSecurityIdentity * symfony/symfony#4309 - Issue related to custom group (role) and ACL/ACE * symfony/symfony#5026 - potential bug in Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity * symfony/symfony#5076 - [Acl] altered the behaviour of RoleSecurityIdentity * symfony/symfony#5171 - Fix/role security identity * symfony/symfony#5303 - [Security] Check for RoleInterface instead of Role object in RoleSecurityIdentity * symfony/symfony#5909 - Allow Custom Roles to implement the RoleInterface * symfony/symfony#6012 - Securityidentity fix
We're using our own Role object that implements RoleInterface and the Acl didn't work because of the fact that RoleSecurityIdentity checked on the objects passed being an instance of Role instead of RoleInterface...
I also added some extra sanity checks in the constructor and tests for them.