-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Adding a shortcuts for the main security functionality #24337
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bundle\SecurityBundle\Tests\Functional; | ||
|
||
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; | ||
use Symfony\Component\Security\Core\User\User; | ||
|
||
class SecurityTest extends WebTestCase | ||
{ | ||
public function testServiceIsFunctional() | ||
{ | ||
$kernel = self::createKernel(array('test_case' => 'SecurityHelper', 'root_config' => 'config.yml')); | ||
$kernel->boot(); | ||
$container = $kernel->getContainer(); | ||
|
||
// put a token into the storage so the final calls can function | ||
$user = new User('foo', 'pass'); | ||
$token = new UsernamePasswordToken($user, '', 'provider', array('ROLE_USER')); | ||
$container->get('security.token_storage')->setToken($token); | ||
|
||
$security = $container->get('functional_test.security.helper'); | ||
$this->assertTrue($security->isGranted('ROLE_USER')); | ||
$this->assertSame($token, $security->getToken()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
use Symfony\Bundle\TwigBundle\TwigBundle; | ||
use Symfony\Bundle\SecurityBundle\SecurityBundle; | ||
use Symfony\Bundle\FrameworkBundle\FrameworkBundle; | ||
|
||
return array( | ||
new FrameworkBundle(), | ||
new SecurityBundle(), | ||
new TwigBundle(), | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
imports: | ||
- { resource: ./../config/default.yml } | ||
|
||
services: | ||
# alias the service so we can access it in the tests | ||
functional_test.security.helper: | ||
alias: security.helper | ||
public: true | ||
|
||
security: | ||
providers: | ||
in_memory: | ||
memory: | ||
users: [] | ||
|
||
firewalls: | ||
default: | ||
anonymous: ~ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,15 +11,63 @@ | |
|
||
namespace Symfony\Component\Security\Core; | ||
|
||
use Psr\Container\ContainerInterface; | ||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; | ||
use Symfony\Component\Security\Core\User\UserInterface; | ||
|
||
/** | ||
* This class holds security information. | ||
* | ||
* @author Johannes M. Schmitt <schmittjoh@gmail.com> | ||
* Helper class for commonly-needed security tasks. | ||
*/ | ||
final class Security | ||
{ | ||
const ACCESS_DENIED_ERROR = '_security.403_error'; | ||
const AUTHENTICATION_ERROR = '_security.last_error'; | ||
const LAST_USERNAME = '_security.last_username'; | ||
const MAX_USERNAME_LENGTH = 4096; | ||
|
||
private $container; | ||
|
||
public function __construct(ContainerInterface $container) | ||
{ | ||
$this->container = $container; | ||
} | ||
|
||
/** | ||
* @return UserInterface|null | ||
*/ | ||
public function getUser() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return type instead of phpdoc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this targets 3.4 which supports PHP 5.6. |
||
{ | ||
if (!$token = $this->getToken()) { | ||
return null; | ||
} | ||
|
||
$user = $token->getUser(); | ||
if (!is_object($user)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't the user be a simple string in some cases? /**
* Returns a user representation.
*
* @return mixed Can be a UserInterface instance, an object implementing a __toString method,
* or the username as a regular string
*
* @see AbstractToken::setUser()
*/
public function getUser(); So instead, this should return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, but I'm purposely not allowing for people who use a string User to make use of this feature. We do that in other places. Allowing it to be a string makes everyone's user experience worse. So, you can do this, but you need to use the lower-level tools :) |
||
return null; | ||
} | ||
|
||
return $user; | ||
} | ||
|
||
/** | ||
* Checks if the attributes are granted against the current authentication token and optionally supplied subject. | ||
* | ||
* @param mixed $attributes | ||
* @param mixed $subject | ||
* | ||
* @return bool | ||
*/ | ||
public function isGranted($attributes, $subject = null) | ||
{ | ||
return $this->container->get('security.authorization_checker') | ||
->isGranted($attributes, $subject); | ||
} | ||
|
||
/** | ||
* @return TokenInterface|null | ||
*/ | ||
public function getToken() | ||
{ | ||
return $this->container->get('security.token_storage')->getToken(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Security\Core\Tests; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Psr\Container\ContainerInterface; | ||
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; | ||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; | ||
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; | ||
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; | ||
use Symfony\Component\Security\Core\Security; | ||
use Symfony\Component\Security\Core\User\User; | ||
|
||
class SecurityTest extends TestCase | ||
{ | ||
public function testGetToken() | ||
{ | ||
$token = new UsernamePasswordToken('foo', 'bar', 'provider'); | ||
$tokenStorage = $this->getMockBuilder(TokenStorageInterface::class)->getMock(); | ||
|
||
$tokenStorage->expects($this->once()) | ||
->method('getToken') | ||
->will($this->returnValue($token)); | ||
|
||
$container = $this->createContainer('security.token_storage', $tokenStorage); | ||
|
||
$security = new Security($container); | ||
$this->assertSame($token, $security->getToken()); | ||
} | ||
|
||
/** | ||
* @dataProvider getUserTests | ||
*/ | ||
public function testGetUser($userInToken, $expectedUser) | ||
{ | ||
$token = $this->getMockBuilder(TokenInterface::class)->getMock(); | ||
$token->expects($this->any()) | ||
->method('getUser') | ||
->will($this->returnValue($userInToken)); | ||
$tokenStorage = $this->getMockBuilder(TokenStorageInterface::class)->getMock(); | ||
|
||
$tokenStorage->expects($this->once()) | ||
->method('getToken') | ||
->will($this->returnValue($token)); | ||
|
||
$container = $this->createContainer('security.token_storage', $tokenStorage); | ||
|
||
$security = new Security($container); | ||
$this->assertSame($expectedUser, $security->getUser()); | ||
} | ||
|
||
public function getUserTests() | ||
{ | ||
yield array(null, null); | ||
|
||
yield array('string_username', null); | ||
|
||
$user = new User('nice_user', 'foo'); | ||
yield array($user, $user); | ||
} | ||
|
||
public function testIsGranted() | ||
{ | ||
$authorizationChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); | ||
|
||
$authorizationChecker->expects($this->once()) | ||
->method('isGranted') | ||
->with('SOME_ATTRIBUTE', 'SOME_SUBJECT') | ||
->will($this->returnValue(true)); | ||
|
||
$container = $this->createContainer('security.authorization_checker', $authorizationChecker); | ||
|
||
$security = new Security($container); | ||
$this->assertTrue($security->isGranted('SOME_ATTRIBUTE', 'SOME_SUBJECT')); | ||
} | ||
|
||
private function createContainer($serviceId, $serviceObject) | ||
{ | ||
$container = $this->getMockBuilder(ContainerInterface::class)->getMock(); | ||
|
||
$container->expects($this->atLeastOnce()) | ||
->method('get') | ||
->with($serviceId) | ||
->will($this->returnValue($serviceObject)); | ||
|
||
return $container; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should make the Security class implement ServiceSubscriberInterface instead. But maybe you prefer no dep on the DI component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea... that was the basic idea: it felt better to keep the
Security
class from the component coupled only topsr/container
. And actually, that wasn't listed as a dep incomposer.json
. I'm going to add that now... tests must've passed because another dep ofsymfony/security
already required it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, or because I don't have a test for that class... fixing that now :)