10000 merged branch jalliot/acl_proxies-2 (PR #3826) · hostingnuggets/symfony@dd5d7f2 · GitHub
[go: up one dir, main page]
Skip to content

Commit dd5d7f2

Browse files
fabpotfabpot
committed
merged branch jalliot/acl_proxies-2 (PR symfony#3826)
Commits ------- 6483d88 [Security][ACL] Fixed ObjectIdentity::fromDomainObject and UserSecurityIdentity::from(Account|Token) when working with proxies Discussion ---------- [WIP] Fixed ACL handling of Doctrine proxies Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/jalliot/symfony.png?branch=acl_proxies-2)](http://travis-ci.org/jalliot/symfony) Fixes the following tickets: symfony#3818, symfony#2611, symfony#2056, symfony#2048, symfony#2035 and probably others Todo: Fix tests, update changelog Hi, As per @fabpot's request, here is symfony#3818 ported to `master`. > Here is a new attempt to fix all the issues related to Symfony ACL identities and Doctrine proxies. > It only fixes the issue for Doctrine >=2.2 (older versions of Doctrine will still not work properly with ACL because `Doctrine\Common\Util\ClassUtils` didn't exist before and proxy naming strategy was not consistent between all Doctrine implementations (ORM/ODM/etc.)). /cc @schmittjoh @beberlei --------------------------------------------------------------------------- by schmittjoh at 2012-04-09T00:07:52Z I'm -1 on adding a dependency on the Doctrine class. The naming scheme was designed in a generic way, we should just copy the method. --------------------------------------------------------------------------- by jalliot at 2012-04-09T00:13:07Z @schmittjoh The ACL component requires Doctrine DBAL already (and as such Common) so I don't think this is really an issue at the moment. If (when?) the component is refactored to be decoupled from Doctrine, then maybe we will have to change that. But I can also copy the class (where?) if you think it is better :) --------------------------------------------------------------------------- by schmittjoh at 2012-04-09T01:01:27Z I'd suggest ``Symfony\Component\Security\Core\Util\ClassUtils``. --------------------------------------------------------------------------- by jalliot at 2012-04-09T11:16:19Z @fabpot @schmittjoh It's done: I've backported the ClassUtils class and its tests from Doctrine Common into the Security component. Maybe this PR can be merged into 2.0 as well now, what do you think? --------------------------------------------------------------------------- by oscherler at 2012-04-11T06:27:20Z There seems to be a consensus that ACLs don’t (fully?) work with Doctrine < 2.2, i.e. in Symfony 2.0. Can it therefore be documented somewhere, typically on the main documentation page on the subject? http://symfony.com/doc/current/cookbook/security/acl.html --------------------------------------------------------------------------- by jalliot at 2012-04-11T07:36:25Z @oscherler You can make ACL work with Doctrine < 2.2 and without this patch if you use something like this code (note this is for ORM only but it can be adapted for ODM; it also assumes that your identifier can be retrieved with `getId()`): ``` php <?php use Doctrine\ORM\Proxy\Proxy; use Symfony\Component\Acl\Domain\ObjectIdentity; $domainObject = ... // some Doctrine entity (maybe a proxy...) if ($domainObject instanceof Proxy) { $objectIdentity = new ObjectIdentity($domainObject->getId(), get_parent_class($domainObject)); } else { $objectIdentity = new ObjectIdentity($domainObject->getId(), get_class($domainObject)); } ``` It is ugly but it is the only way to get the correct identity for < 2.2. Never use `ObjectIdentity::fromDomainObject` with a proxy and without this patch! The same applies to `UserSecurityIdentity`. This should indeed be documented in the doc for 2.0. /cc @weaverryan --------------------------------------------------------------------------- by jalliot at 2012-04-11T22:34:37Z I just fixed the tests and squashed the commits. I didn't write a test for `UserSecurityIdentity::fromAccount` and `fromToken` because I didn't really have time to look for a clean solution (without creating a full UserInterface implementation for instance...). Anyway the test for `ObjectIdentity::fromDomainObject` should be enough I guess... Don't hesitate to add one if you think it is necessary. /cc @schmittjoh @fabpot Apart from @beberlei approval to use MIT license, I think it is ready for merge.
2 parents b47cb35 + 6483d88 commit dd5d7f2

File tree

6 files changed

+200
-68
lines changed

6 files changed

+200
-68
lines changed

‎src/Symfony/Component/Security/Acl/Domain/ObjectIdentity.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Security\Acl\Domain;
1313

14+
use Symfony\Component\Security\Core\Util\ClassUtils;
1415
use Symfony\Component\Security\Acl\Exception\InvalidDomainObjectException;
1516
use Symfony\Component\Security\Acl\Model\DomainObjectInterface;
1617
use Symfony\Component\Security\Acl\Model\ObjectIdentityInterface;
@@ -59,9 +60,9 @@ static public function fromDomainObject($domainObject)
5960

6061
try {
6162
if ($domainObject instanceof DomainObjectInterface) {
62-
return new self($domainObject->getObjectIdentifier(), get_class($domainObject));
63+
return new self($domainObject->getObjectIdentifier(), ClassUtils::getRealClass($domainObject));
6364
} elseif (method_exists($domainObject, 'getId')) {
64-
return new self($domainObject->getId(), get_class($domainObject));
65+
return new self($domainObject->getId(), ClassUtils::getRealClass($domainObject));
6566
}
6667
} catch (\InvalidArgumentException $invalid) {
6768
throw new InvalidDomainObjectException($invalid->getMessage(), 0, $invalid);

‎src/Symfony/Component/Security/Acl/Domain/UserSecurityIdentity.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1515
use Symfony\Component\Security\Core\User\UserInterface;
16+
use Symfony\Component\Security\Core\Util\ClassUtils;
1617
use Symfony\Component\Security\Acl\Model\SecurityIdentityInterface;
1718

1819
/**
@@ -52,7 +53,7 @@ public function __construct($username, $class)
5253
*/
5354
static public function fromAccount(UserInterface $user)
5455
{
55-
return new self($user->getUsername(), get_class($user));
56+
return new self($user->getUsername(), ClassUtils::getRealClass($user));
5657
}
5758

5859
/**
@@ -69,7 +70,7 @@ static public function fromToken(TokenInterface $token)
6970
return self::fromAccount($user);
7071
}
7172

72-
return new self((string) $user, is_object($user)? get_class($user) : get_class($token));
73+
return new self((string) $user, is_object($user) ? ClassUtils::getRealClass($user) : ClassUtils::getRealClass($token));
7374
}
7475

7576
/**

‎src/Symfony/Component/Security/Core/Util/ClassUtils.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Util;
13+
14+
/**
15+
* Class related functionality for objects that
16+
* might or might not be proxy objects at the moment.
17+
*
18+
* @see Doctrine\Common\Util\ClassUtils
19+
*
20+
* @author Benjamin Eberlei <kontakt@beberlei.de>
21+
* @author Johannes Schmitt <schmittjoh@gmail.com>
22+
*/
23+
class ClassUtils
24+
{
25+
/**
26+
* Marker for Proxy class names.
27+
*
28+
* @var string
29+
*/
30+
const MARKER = '__CG__';
31+
32+
/**
33+
* Length of the proxy marker
34+
*
35+
* @var int
36+
*/
37+
const MARKER_LENGTH = 6;
38+
39+
/**
40+
* Gets the real class name of a class name that could be a proxy.
41+
*
42+
* @param string|object
43+
* @return string
44+
*/
45+
public static function getRealClass($object)
46+
{
47+
$class = is_object($object) ? get_class($object) : $object;
48+
49+
if (false === $pos = strrpos($class, '\\'.self::MARKER.'\\')) {
50+
return $class;
51+
}
52+
53+
return substr($class, $pos + self::MARKER_LENGTH + 2);
54+
}
55+
}

‎src/Symfony/Component/Security/Tests/Acl/Domain/ObjectIdentityTest.php

Lines changed: 89 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -9,83 +9,108 @@
99
* file that was distributed with this source code.
1010
*/
1111

12-
namespace Symfony\Component\Security\Tests\Acl\Domain;
13-
14-
use Symfony\Component\Security\Acl\Domain\ObjectIdentity;
15-
16-
class ObjectIdentityTest extends \PHPUnit_Framework_TestCase
12+
namespace Symfony\Component\Security\Tests\Acl\Domain
1713
{
18-
public function testConstructor()
14+
use Symfony\Component\Security\Acl\Domain\ObjectIdentity;
15+
16+
class ObjectIdentityTest extends \PHPUnit_Framework_TestCase
1917
{
20-
$id = new ObjectIdentity('fooid', 'footype');
21-
22-
$this->assertEquals('fooid', $id->getIdentifier());
23-
$this->assertEquals('footype', $id->getType());
24-
}
25-
26-
public function testFromDomainObjectPrefersInterfaceOverGetId()
27-
{
28-
$domainObject = $this->getMock('Symfony\Component\Security\Acl\Model\DomainObjectInterface');
29-
$domainObject
30-
->expects($this->once())
31-
->method('getObjectIdentifier')
32-
->will($this->returnValue('getObjectIdentifier()'))
33-
;
34-
$domainObject
35-
->expects($this->never())
36-
->method('getId')
37-
->will($this->returnValue('getId()'))
38-
;
39-
40-
$id = ObjectIdentity::fromDomainObject($domainObject);
41-
$this->assertEquals('getObjectIdentifier()', $id->getIdentifier());
42-
}
18+
public function testConstructor()
19+
{
20+
$id = new ObjectIdentity('fooid', 'footype');
21+
22+
$this->assertEquals('fooid', $id->getIdentifier());
23+
$this->assertEquals('footype', $id->getType());
24+
}
4325

44-
public function testFromDomainObjectWithoutInterface()
45-
{
46-
$id = ObjectIdentity::fromDomainObject(new TestDomainObject());
47-
$this->assertEquals('getId()', $id->getIdentifier());
48-
}
26+
// Test that constructor never changes passed type, even with proxies
27+
public function testConstructorWithProxy()
28+
{
29+
$id = new ObjectIdentity('fooid', 'Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Acl\Domain\TestDomainObject');
4930

50-
/**
51-
* @dataProvider getCompareData
52-
*/
53-
public function testEquals($oid1, $oid2, $equal)
54-
{
55-
if ($equal) {
56-
$this->assertTrue($oid1->equals($oid2));
57-
} else {
58-
$this->assertFalse($oid1->equals($oid2));
31+
$this->assertEquals('fooid', $id->getIdentifier());
32+
$this->assertEquals('Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Acl\Domain\TestDomainObject', $id->getType());
33+
}
34+
35+
public function testFromDomainObjectPrefersInterfaceOverGetId()
36+
{
37+
$domainObject = $this->getMock('Symfony\Component\Security\Acl\Model\DomainObjectInterface');
38+
$domainObject
39+
->expects($this->once())
40+
->method('getObjectIdentifier')
41+
->will($this->returnValue('getObjectIdentifier()'))
42+
;
43+
$domainObject
44+
->expects($this->never())
45+
->method('getId')
46+
->will($this->returnValue('getId()'))
47+
;
48+
49+
$id = ObjectIdentity::fromDomainObject($domainObject);
50+
$this->assertEquals('getObjectIdentifier()', $id->getIdentifier());
51+
}
52+
53+
public function testFromDomainObjectWithoutInterface()
54+
{
55+
$id = ObjectIdentity::fromDomainObject(new TestDomainObject());
56+
$this->assertEquals('getId()', $id->getIdentifier());
57+
$this->assertEquals('Symfony\Component\Security\Tests\Acl\Domain\TestDomainObject', $id->getType());
5958
}
60-
}
6159

62-
public function getCompareData()
63-
{
64-
return array(
65-
array(new ObjectIdentity('123', 'foo'), new ObjectIdentity('123', 'foo'), true),
66-
array(new ObjectIdentity('123', 'foo'), new ObjectIdentity(123, 'foo'), true),
67-
array(new ObjectIdentity('1', 'foo'), new ObjectIdentity('2', 'foo'), false),
68-
array(new ObjectIdentity('1', 'bla'), new ObjectIdentity('1', 'blub'), false),
69-
);
60+
public function testFromDomainObjectWithProxy()
61+
{
62+
$id = ObjectIdentity::fromDomainObject(new \Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Acl\Domain\TestDomainObject());
63+
$this->assertEquals('getId()', $id->getIdentifier());
64+
$this->assertEquals('Symfony\Component\Security\Tests\Acl\Domain\TestDomainObject', $id->getType());
65+
}
66+
67+
/**
68+
* @dataProvider getCompareData
69+
*/
70+
public function testEquals($oid1, $oid2, $equal)
71+
{
72+
if ($equal) {
73+
$this->assertTrue($oid1->equals($oid2));
74+
} else {
75+
$this->assertFalse($oid1->equals($oid2));
76+
}
77+
}
78+
79+
public function getCompareData()
80+
{
81+
return array(
82+
array(new ObjectIdentity('123', 'foo'), new ObjectIdentity('123', 'foo'), true),
83+
array(new ObjectIdentity('123', 'foo'), new ObjectIdentity(123, 'foo'), true),
84+
array(new ObjectIdentity('1', 'foo'), new ObjectIdentity('2', 'foo'), false),
85+
array(new ObjectIdentity('1', 'bla'), new ObjectIdentity('1', 'blub'), false),
86+ 10000
);
87+
}
88+
89+
protected function setUp()
90+
{
91+
if (!class_exists('Doctrine\DBAL\DriverManager')) {
92+
$this->markTestSkipped('The Doctrine2 DBAL is required for this test');
93+
}
94+
}
7095
}
71-
72-
protected function setUp()
96+
97+
class TestDomainObject
7398
{
74-
if (!class_exists('Doctrine\DBAL\DriverManager')) {
75-
$this->markTestSkipped('The Doctrine2 DBAL is required for this test');
99+
public function getObjectIdentifier()
100+
{
101+
return 'getObjectIdentifier()';
102+
}
103+
104+
public function getId()
105+
{
106+
return 'getId()';
76107
}
77108
}
78109
}
79110

80-
class TestDomainObject
111+
namespace Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Acl\Domain
81112
{
82-
public function getObjectIdentifier()
83-
{
84-
return 'getObjectIdentifier()';
85-
}
86-
87-
public function getId()
113+
class TestDomainObject extends \Symfony\Component\Security\Tests\Acl\Domain\TestDomainObject
88114
{
89-
return 'getId()';
90115
}
91116
}

‎src/Symfony/Component/Security/Tests/Acl/Domain/UserSecurityIdentityTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ public function testConstructor()
2424
$this->assertEquals('Foo', $id->getClass());
2525
}
2626

27+
// Test that constructor never changes the type, even for proxies
28+
public function testContructorWithProxy()
29+
{
30+
$id = new UserSecurityIdentity('foo', 'Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Acl\Domain\Foo');
31+
32+
$this->assertEquals('foo', $id->getUsername());
33+
$this->assertEquals('Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Acl\Domain\Foo', $id->getClass());
34+
}
35+
2736
/**
2837
* @dataProvider getCompareData
2938
*/

‎src/Symfony/Component/Security/Tests/Core/Util/ClassUtilsTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
namespace Symfony\Component\Security\Tests\Core\Util
4+
{
5+
use Symfony\Component\Security\Core\Util\ClassUtils;
6+
7+
class ClassUtilsTest extends \PHPUnit_Framework_TestCase
8+
{
9+
static public function dataGetClass()
10+
{
11+
return array(
12+
array('stdClass', 'stdClass'),
13+
array('Symfony\Component\Security\Core\Util\ClassUtils', 'Symfony\Component\Security\Core\Util\ClassUtils'),
14+
array('MyProject\Proxies\__CG__\stdClass', 'stdClass'),
15+
array('MyProject\Proxies\__CG__\OtherProject\Proxies\__CG__\stdClass', 'stdClass'),
16+
array('MyProject\Proxies\__CG__\Symfony\Component\Security\Tests\Core\Util\ChildObject', 'Symfony\Component\Security\Tests\Core\Util\ChildObject'),
17+
array(new TestObject(), 'Symfony\Component\Security\Tests\Core\Util\TestObject'),
18+
array(new \Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Core\Util\TestObject(), 'Symfony\Component\Security\Tests\Core\Util\TestObject'),
19+
);
20+
}
21+
22+
/**
23+
* @dataProvider dataGetClass
24+
*/
25+
public function testGetRealClass($object, $expectedClassName)
26+
{
27+
$this->assertEquals($expectedClassName, ClassUtils::getRealClass($object));
28+
}
29+
}
30+
31+
class TestObject
32+
{
33+
}
34+
}
35+
36+
namespace Acme\DemoBundle\Proxy\__CG__\Symfony\Component\Security\Tests\Core\Util
37+
{
38+
class TestObject extends \Symfony\Component\Security\Tests\Core\Util\TestObject
39+
{
40+
}
41+
}

0 commit comments

Comments
 (0)
0