Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1334338

Browse filesBrowse files
fabpotfabpot
committed
feature #11690 [Security] Split of the SecurityContext (iltar)
This PR was merged into the 2.6-dev branch. Discussion ---------- [Security] Split of the SecurityContext ~~_As a reminder, this PR is not ready to be merged. It's merely a proof of concept in which I'm trying to fix a circular dependency with the SecurityContext and the entity manager for Symfony 2.6 and/or 3.0_~~ PR Info ====== | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#4188 TODO List ========= - [x] Split tests for SecurityContext/AuthorizationChecker/TokenStorage - [x] Fix tests for security usages (only the component has been successfully tested at this point) - [x] Submit changes to the documentation - [x] Document the BC breaks Main Problem for my use case ======================== I've build a bunch of event listeners on `doctrine.event_manager`. They include a Blamable, Revision and Mutation annotation on entities. It works by creating a custom event listener on preFlush which then throws an entityChanged event (also a doctrine hooked up event). To make it configurable and flexible, we have written a provider for Blamable to provide the username/user-id and a date time (updated-by, updated-at). In order to get that information, we need to look into the SecurityContext to get the current user and ask the user id (custom user implementation). However, injecting the SecurityContext - or services depending on the SecurityContext - creates a circular reference and causes the container to blurt out an Exception. This is because the SecurityContext uses a UserProvider (indirectly) which has a dependency on doctrine (em, connection). Because it needs doctrine, it's impossible for my listener to inject the SecurityContext as it becomes this: - SecurityContext requires AuthenticationProvider - (Simple)AuthenticationProvider requires UserProvider - UserProvider requires EntityManager - EntityManager requires _insert connection name here_ - My custom Listener calls addEvent (or something similar) in doctrine which causes a dependency from the EM/Connection to my Listener - My Listener requires SecurityContext... which finishes the circle. I've googled for this problem and it wasn't hard to find similar issues, it seems to be a quite common issue regarding the SecurityContext and the EntityManager - http://stackoverflow.com/questions/7561013/injecting-securitycontext-into-a-listener-prepersist-or-preupdate-in-symfony2-to - http://stackoverflow.com/questions/8708822/circular-reference-when-injecting-security-context-into-entity-listener-class - http://stackoverflow.com/questions/17020733/how-to-get-userid-from-eventlistener-which-are-called-from-ajax - You can find more simply by googling. The main solution seems to be to lazy load using an additional bundle or as recommended in the above topics, inject the container. Neither of them is really a solution I'm happy with. I don't want my code to know about the Container(Interface), nor do I want to use a another bundle just to get around an issue that a lot of people seem to have with the SecurityContext and EntityManager. Possible Solutions ============== I've been thinking about several solutions: - I could write a service that listens to `kernel.request` and when possible injects the username/user-id into my provider which then can provide it to my listener - I could use the Container directly - I can use a lazy service with `symfony/proxy-manager-bridge` - I can store the user-id in my request However, those solutions are just not it for me. Depending on an event like `kernel.request` is a bad practice in my opinion, I shouldn't depend on what listeners might be registered. Using the container directly inverses the dependency which is also wrong in my opinion. Using a lazy service will only work around the problem and storing the user-id in my request means I might not always have it (say commands). Long story short, not what I'm looking for. Splitting the SecurityContext ====================== So, I ended up at the SecurityContext. Digging back to the real problem, I started asking myself the following questions: why do I have that dependency? Why do I need to have the EntityManager when the only thing I want, is the currently logged in User object? (which is not related to a database). I came to the conclusion that the SecurityContext gives me too many dependencies in order to retrieve a simple Token/User object, which is not really what I want. Most of the times I need the SecurityContext to get the token/user and not for isGranted. Personally I use `@Security` and `access_control` for that. I came to the conclusion that storing the Token within the SecurityContext wasn't what I found useful due to the dependencies of the SecurityContext. I figured I'd want a storage class with a dependency on the SessionInterface which could autonomously retrieve and store the TokenInterface (`@session` in this case). It would also be handling the storage within the session using get/setToken. I have proposed this change and had a small discussion with @wouterj on IRC about my proposal to take out the Token (can be read here http://pastebin.com/8kSvVZtj). Based on his feedback, I have split the isGranted to the AuthorizationChecker(Interface), which now has those dependencies. I have also moved the set/getToken to a TokenStorage. tldr; - The getToken en setToken are moved to the TokenStorage(Interface). - ~~If this idea is feasible, I will also try to get the SecurityContext to actually store and retrieve it from the session instead of `ContextListener::onKernelResponse`. This will just do `$context->setToken($token);` which will handle this storage itself.~~ I still chase this idea but I will create a new PR for this in the future if I find time. - isGranted is moved to AuthorizationChecker(Interface) so that you don't have a bunch of dependencies you don't need when retrieving the Token/User. Draft ==== ~~This PR is just a draft. I'm looking for feedback if this proposal is A) desired and B) in-line with the developer's ideas regarding the SecurityContext.~~ Changed Components/bundles ========================= [FrameworkBundle] Updated GlobalVariables, added test for GlobalVariables [SecurityBundle] Updated service definitions [Security Component] Deprecated SecurityContext(Interface), added AuthorizationChecker(Interface) and TokenStorage(Interface) Commits ------- b967787 Split of the SecurityContext to AuthorizationChecker and TokenStorage
2 parents 4ee2e93 + b967787 commit 1334338
Copy full SHA for 1334338

File tree

Expand file treeCollapse file tree

18 files changed

+586
-117
lines changed
Filter options
Expand file treeCollapse file tree

18 files changed

+586
-117
lines changed

‎UPGRADE-2.6.md

Copy file name to clipboardExpand all lines: UPGRADE-2.6.md
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,27 @@ Validator
7777
```
7878
value == null or (YOUR_EXPRESSION)
7979
```
80+
81+
Security
82+
--------
83+
84+
* The `SecurityContextInterface` is marked as deprecated in favor of the
85+
`Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface` and
86+
`Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface`.
87+
```
88+
isGranted => AuthorizationCheckerInterface
89+
getToken => TokenStorageInterface
90+
setToken => TokenStorageInterface
91+
```
92+
The Implementations have moved too, The `SecurityContext` is marked as
93+
deprecated and has been split to use the `AuthorizationCheckerInterface`
94+
and `TokenStorage`. This change is 100% Backwards Compatible as the SecurityContext
95+
delegates the methods.
96+
97+
* The service `security.context` is deprecated along with the above change. Recommended
98+
to use instead:
99+
```
100+
@security.authorization_checker => isGranted()
101+
@security.token_storage => getToken()
102+
@security.token_storage => setToken()
103+
```

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ CHANGELOG
1010
* Added `Controller::addFlash` helper
1111
* Added `Controller::isGranted` helper
1212
* Added `Controller::denyAccessUnlessGranted` helper
13+
* Deprecated `app.security` in twig as `app.user` and `is_granted()` are already available
1314

1415
2.5.0
1516
-----

‎src/Symfony/Bundle/FrameworkBundle/Templating/GlobalVariables.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Templating/GlobalVariables.php
+5-4Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
namespace Symfony\Bundle\FrameworkBundle\Templating;
1313

1414
use Symfony\Component\DependencyInjection\ContainerInterface;
15+
use Symfony\Component\HttpFoundation\Request;
1516
use Symfony\Component\HttpFoundation\Session\Session;
16-
use Symfony\Component\Security\Core\SecurityContext;
1717
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
18-
use Symfony\Component\HttpFoundation\Request;
18+
use Symfony\Component\Security\Core\SecurityContext;
1919

2020
/**
2121
* GlobalVariables is the entry point for Symfony global variables in Twig templates.
@@ -37,6 +37,7 @@ public function __construct(ContainerInterface $container)
3737
/**
3838
* Returns the security context service.
3939
*
40+
* @deprecated Deprecated since version 2.6, to be removed in 3.0.
4041
* @return SecurityContext|null The security context
4142
*/
4243
public function getSecurity()
@@ -55,11 +56,11 @@ public function getSecurity()
5556
*/
5657
public function getUser()
5758
{
58-
if (!$security = $this->getSecurity()) {
59+
if (!$tokenStorage = $this->container->get('security.token_storage')) {
5960
return;
6061
}
6162

62-
if (!$token = $security->getToken()) {
63+
if (!$token = $tokenStorage->getToken()) {
6364
return;
6465
}
6566

‎src/Symfony/Bundle/FrameworkBundle/Tests/Templating/GlobalVariablesTest.php

Copy file name to clipboard
+79Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
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\Bundle\FrameworkBundle\Tests\Templating;
13+
14+
use Symfony\Bundle\FrameworkBundle\Templating\GlobalVariables;
15+
use Symfony\Bundle\FrameworkBundle\Tests\TestCase;
16+
use Symfony\Component\DependencyInjection\Container;
17+
18+
class GlobalVariablesTest extends TestCase
19+
{
20+
private $container;
21+
private $globals;
22+
23+
public function setUp()
24+
{
25+
$this->container = new Container();
26+
$this->globals = new GlobalVariables($this->container);
27+
}
28+
29+
public function testGetSecurity()
30+
{
31+
$securityContext = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
32+
33+
$this->assertNull($this->globals->getSecurity());
34+
$this->container->set('security.context', $securityContext);
35+
$this->assertSame($securityContext, $this->globals->getSecurity());
36+
}
37+
38+
public function testGetUser()
39+
{
40+
// missing test cases to return null, only happy flow tested
41+
$securityContext = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface');
42+
$token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
43+
$user = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
44+
45+
$this->container->set('security.token_storage', $securityContext);
46+
47+
$token
48+
->expects($this->once())
49+
->method('getUser')
50+
->will($this->returnValue($user));
51+
52+
$securityContext
53+
->expects($this->once())
54+
->method('getToken')
55+
->will($this->returnValue($token));
56+
57+
$this->assertSame($user, $this->globals->getUser());
58+
}
59+
60+
public function testGetRequest()
61+
{
62+
$this->markTestIncomplete();
63+
}
64+
65+
public function testGetSession()
66+
{
67+
$this->markTestIncomplete();
68+
}
69+
70+
public function testGetEnvironment()
71+
{
72+
$this->markTestIncomplete();
73+
}
74+
75+
public function testGetDubug()
76+
{
77+
$this->markTestIncomplete();
78+
}
79+
}

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
CHANGELOG
22
=========
33

4+
2.6.0
5+
-----
6+
7+
* Deprecated the `security.context` service for the `security.token_storage` and
8+
`security.authorization_checker` services.
9+
410
2.4.0
511
-----
612

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
<parameters>
88
<parameter key="security.context.class">Symfony\Component\Security\Core\SecurityContext</parameter>
9+
<parameter key="security.authorization_checker.class">Symfony\Component\Security\Core\Authorization\AuthorizationChecker</parameter>
10+
<parameter key="security.token_storage.class">Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage</parameter>
911

1012
<parameter key="security.user_checker.class">Symfony\Component\Security\Core\User\UserChecker</parameter>
1113

@@ -54,11 +56,19 @@
5456

5557
<services>
5658
<service id="security.context" class="%security.context.class%">
59+
<argument type="service" id="security.token_storage" />
60+
<argument type="service" id="security.authorization_checker" />
61+
</service>
62+
63+
<service id="security.authorization_checker" class="%security.authorization_checker.class%">
64+
<argument type="service" id="security.token_storage" />
5765
<argument type="service" id="security.authentication.manager" />
5866
<argument type="service" id="security.access.decision_manager" />
5967
<argument>%security.access.always_authenticate_before_granting%</argument>
6068
</service>
6169

70+
<service id="security.token_storage" class="%security.token_storage.class%" />
71+
6272
<!-- Authentication related services -->
6373
<service id="security.authentication.manager" class="%security.authentication.manager.class%" public="false">
6474
<argument type="collection" />

‎src/Symfony/Component/Security/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/CHANGELOG.md
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CHANGELOG
55
-----
66

77
* added Symfony\Component\Security\Http\Authentication\AuthenticationUtils
8+
* Deprecated the `SecurityContext` class in favor of the `AuthorizationChecker` and `TokenStorage` classes
89

910
2.4.0
1011
-----

‎src/Symfony/Component/Security/Core/Authentication/Token/Storage/TokenStorage.php

Copy file name to clipboard
+43Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
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\Authentication\Token\Storage;
13+
14+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
15+
16+
/**
17+
* TokenStorage contains a TokenInterface
18+
*
19+
* It gives access to the token representing the current user authentication.
20+
*
21+
* @author Fabien Potencier <fabien@symfony.com>
22+
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
23+
*/
24+
class TokenStorage implements TokenStorageInterface
25+
{
26+
private $token;
27+
28+
/**
29+
* {@inheritdoc}
30+
*/
31+
public function getToken()
32+
{
33+
return $this->token;
34+
}
35+
36+
/**
37+
* {@inheritdoc}
38+
*/
39+
public function setToken(TokenInterface $token = null)
40+
{
41+
$this->token = $token;
42+
}
43+
}

‎src/Symfony/Component/Security/Core/Authentication/Token/Storage/TokenStorageInterface.php

Copy file name to clipboard
+36Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
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\Authentication\Token\Storage;
13+
14+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
15+
16+
/**
17+
* The TokenStorageInterface.
18+
*
19+
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
20+
*/
21+
interface TokenStorageInterface
22+
{
23+
/**
24+
* Returns the current security token.
25+
*
26+
* @return TokenInterface|null A TokenInterface instance or null if no authentication information is available
27+
*/
28+
public function getToken();
29+
30+
/**
31+
* Sets the authentication token.
32+
*
33+
* @param TokenInterface $token A TokenInterface token, or null if no further authentication information should be stored
34+
*/
35+
public function setToken(TokenInterface $token = null);
36+
}

‎src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php

Copy file name to clipboard
+70Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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\Authorization;
13+
14+
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
15+
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
16+
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
17+
18+
/**
19+
* AuthorizationChecker is the main authorization point of the Security component.
20+
*
21+
* It gives access to the token representing the current user authentication.
22+
*
23+
* @author Fabien Potencier <fabien@symfony.com>
24+
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
25+
*/
26+
class AuthorizationChecker implements AuthorizationCheckerInterface
27+
{
28+
private $tokenStorage;
29+
private $accessDecisionManager;
30+
private $authenticationManager;
31+
private $alwaysAuthenticate;
32+
33+
/**
34+
* Constructor.
35+
*
36+
* @param TokenStorageInterface $tokenStorage
37+
* @param AuthenticationManagerInterface $authenticationManager An AuthenticationManager instance
38+
* @param AccessDecisionManagerInterface $accessDecisionManager An AccessDecisionManager instance
39+
* @param bool $alwaysAuthenticate
40+
*/
41+
public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, AccessDecisionManagerInterface $accessDecisionManager, $alwaysAuthenticate = false)
42+
{
43+
$this->tokenStorage = $tokenStorage;
44+
$this->authenticationManager = $authenticationManager;
45+
$this->accessDecisionManager = $accessDecisionManager;
46+
$this->alwaysAuthenticate = $alwaysAuthenticate;
47+
}
48+
49+
/**
50+
* {@inheritdoc}
51+
*
52+
* @throws AuthenticationCredentialsNotFoundException when the token storage has no authentication token.
53+
*/
54+
final public function isGranted($attributes, $object = null)
55+
{
56+
if (null === ($token = $this->tokenStorage->getToken())) {
57+
throw new AuthenticationCredentialsNotFoundException('The token storage contains no authentication token. One possible reason may be that there is no firewall configured for this URL.');
58+
}
59+
60+
if ($this->alwaysAuthenticate || !$token->isAuthenticated()) {
61+
$this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token));
62+
}
63+
64+
if (!is_array($attributes)) {
65+
$attributes = array($attributes);
66+
}
67+
68+
return $this->accessDecisionManager->decide($token, $attributes, $object);
69+
}
70+
}

‎src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php

Copy file name to clipboard
+30Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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\Authorization;
13+
14+
/**
15+
* The AuthorizationCheckerInterface.
16+
*
17+
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
18+
*/
19+
interface AuthorizationCheckerInterface
20+
{
21+
/**
22+
* Checks if the attributes are granted against the current authentication token and optionally supplied object.
23+
*
24+
* @param mixed $attributes
25+
* @param mixed $object
26+
*
27+
* @return bool
28+
*/
29+
public function isGranted($attributes, $object = null);
30+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.