Skip to content

Replace Role to RoleInterface for RoleSecurityIdentity #1748

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
nfx opened this issue Jul 20, 2011 · 5 comments
Closed

Replace Role to RoleInterface for RoleSecurityIdentity #1748

nfx opened this issue Jul 20, 2011 · 5 comments
Labels

Comments

@nfx
Copy link
Contributor

nfx commented Jul 20, 2011

Class Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity should be able to be created from every object implementing RoleInterface.

@jalliot
Copy link
Contributor

jalliot commented Jul 20, 2011

Apparently, doing so would cause a security vulnerability (according to comment on af70ac8 which was later reverted in d80ee41).
BTW @schmittjoh, could you tell us a bit more on this?

@fabpot
Copy link
Member

fabpot commented Jul 20, 2011

This cannot be done because of security problems. See af70ac8

@fabpot fabpot closed this as completed Jul 20, 2011
@predakanga
Copy link

Was the security issue related to this ever resolved?
Just ran into this problem when implementing ACL using custom roles from the database (as described in the official documentation, at http://symfony.com/doc/current/cookbook/security/entity_provider.html#managing-roles-in-the-database)

Either RoleSecurityIdentity should use RoleInterface in a safe manner, or the documentation should be updated to warn that the UserInterface implementation should return only strings from getRoles(), so as to avoid further confusion for users.

@mevers47
Copy link
Contributor

With all due respect @schmittjoh and @fabpot can we reopen this issue? If there are security issues you can not provide details for I understand. But please let us know. If that's not the case, a unintentional kind of FUD must not be enough to stop process, even if it came from a respected core developer. Can we move on?

@celmaun
Copy link

celmaun commented Nov 1, 2012

+1 @mevers47 :/

m14t added a commit to m14t/symfony-docs that referenced this issue Apr 23, 2013
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants