8000 fixes #1538 by yktd26 · Pull Request #1673 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

fixes #1538 #1673

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

Merged
merged 1 commit into from
Jul 13, 2011
Merged

fixes #1538 #1673

merged 1 commit into from
Jul 13, 2011

Conversation

yktd26
Copy link
Contributor
@yktd26 yktd26 commented Jul 13, 2011

Constructor of Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity

currently it check if the argument is instance of Symfony\Component\Security\Core\Role\Role by

if ($role instanceof Role)

Maybe it should be changed to

if ($role instanceof RoleInterface)

Because if we use another Role class which implements RoleInterface

it dosen't work when we check access, it will throw a NoAceFoundException when vote

fabpot added a commit that referenced this pull request Jul 13, 2011
Commits
-------

26ff05b fixes #1538

Discussion
----------

fixes #1538

Constructor of  Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity
--------------------------------------------------------------------------------------------------------

currently it check if the argument is instance of Symfony\Component\Security\Core\Role\Role by

``if ($role instanceof Role)``

Maybe it should be changed to

``if ($role instanceof RoleInterface)``

Because if we use another Role class which implements RoleInterface

it dosen't work when we check access, it will throw a *NoAceFoundException* when vote
@fabpot fabpot merged commit 26ff05b into symfony:master Jul 13, 2011
fabpot added a commit that referenced this pull request Jul 13, 2011
This reverts commit af70ac8, reversing
changes made to c881379.
@mltrx
Copy link
mltrx commented Sep 6, 2011

Oh, why it was reverted? Mistake?
Without that fix ACL is not working properly with own Roles.

@jalliot
Copy link
Contributor
jalliot commented Sep 6, 2011

@halmit Because @schmittjoh said it introduced a security vulnerability (but never said what)

@mltrx
Copy link
mltrx commented Sep 6, 2011

For now im extending my Role class with Symfony\Component\Security\Core\Role\Role instead of raw implementing RoleInterface (i need that fix otherwise). Im not using private parent "role" field at all, so extending with Symfony Role class doesn't have any sense. I have own private field called "name".

77web pushed a commit to 77web/symfony-docs that referenced this pull request Nov 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0