From f6046ba4f30424932980ec4564e256d7a5e28bbb Mon Sep 17 00:00:00 2001 From: Joshua Thijssen Date: Fri, 9 Jan 2015 11:12:42 +0100 Subject: [PATCH] [security] Fetching current stored context when not explicitly specified --- .../Resources/config/templating_php.xml | 3 +- .../Templating/Helper/LogoutUrlHelper.php | 47 +++++++++++++++---- .../views/Login/after_login.html.twig | 10 ++++ .../Tests/Functional/FormLoginTest.php | 34 ++++++++++++++ .../app/StandardFormLogin/config.yml | 9 ++++ .../Twig/Extension/LogoutUrlExtension.php | 8 ++-- 6 files changed, 96 insertions(+), 15 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml index 033cba0ef9d9..02ffa595828b 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/templating_php.xml @@ -12,8 +12,9 @@ - + + diff --git a/src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php b/src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php index 3532c2cb780b..8ef5ee7a4236 100644 --- a/src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php +++ b/src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php @@ -14,7 +14,9 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderAdapter; use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface; use Symfony\Component\Templating\Helper\Helper; @@ -25,20 +27,33 @@ */ class LogoutUrlHelper extends Helper { - private $container; + private $requestStack; private $listeners = array(); private $router; + private $tokenStorage; /** * Constructor. * - * @param ContainerInterface $container A ContainerInterface instance - * @param UrlGeneratorInterface $router A Router instance + * @param ContainerInterface|RequestStack $requestStack A ContainerInterface instance or RequestStack + * @param UrlGeneratorInterface $router The router service + * @param TokenStorageInterface|null $tokenStorage The token storage service + * + * @deprecated Passing a ContainerInterface as a first argument is deprecated since 2.7 and will be removed in 3.0. */ - public function __construct(ContainerInterface $container, UrlGeneratorInterface $router) + public function __construct($requestStack, UrlGeneratorInterface $router, TokenStorageInterface $tokenStorage = null) { - $this->container = $container; + if ($requestStack instanceof ContainerInterface) { + $this->requestStack = $requestStack->get('request_stack'); + trigger_error('The '.__CLASS__.' constructor will require a RequestStack instead of a ContainerInterface instance in 3.0.', E_USER_DEPRECATED); + } elseif ($requestStack instanceof RequestStack) { + $this->requestStack = $requestStack; + } else { + throw new \InvalidArgumentException(sprintf('%s takes either a RequestStack or a ContainerInterface object as its first argument.', __METHOD__)); + } + $this->router = $router; + $this->tokenStorage = $tokenStorage; } /** @@ -64,7 +79,7 @@ public function registerListener($key, $logoutPath, $csrfTokenId, $csrfParameter /** * Generates the absolute logout path for the firewall. * - * @param string $key The firewall key + * @param string|null $key The firewall key or null to use the current firewall key * * @return string The logout path */ @@ -76,7 +91,7 @@ public function getLogoutPath($key) /** * Generates the absolute logout URL for the firewall. * - * @param string $key The firewall key + * @param string|null $key The firewall key or null to use the current firewall key * * @return string The logout URL */ @@ -88,15 +103,27 @@ public function getLogoutUrl($key) /** * Generates the logout URL for the firewall. * - * @param string $key The firewall key + * @param string|null $key The firewall key or null to use the current firewall key * @param bool|string $referenceType The type of reference (one of the constants in UrlGeneratorInterface) * * @return string The logout URL * - * @throws \InvalidArgumentException if no LogoutListener is registered for the key + * @throws \InvalidArgumentException if no LogoutListener is registered for the key or the key could not be found automatically. */ private function generateLogoutUrl($key, $referenceType) { + // Fetch the current provider key from token, if possible + if (null === $key && null !== $this->tokenStorage) { + $token = $this->tokenStorage->getToken(); + if (null !== $token && method_exists($token, 'getProviderKey')) { + $key = $token->getProviderKey(); + } + } + + if (null === $key) { + throw new \InvalidArgumentException('Unable to find the current firewall LogoutListener, please provide the provider key manually.'); + } + if (!array_key_exists($key, $this->listeners)) { throw new \InvalidArgumentException(sprintf('No LogoutListener found for firewall key "%s".', $key)); } @@ -106,7 +133,7 @@ private function generateLogoutUrl($key, $referenceType) $parameters = null !== $csrfTokenManager ? array($csrfParameter => (string) $csrfTokenManager->getToken($csrfTokenId)) : array(); if ('/' === $logoutPath[0]) { - $request = $this->container->get('request_stack')->getCurrentRequest(); + $request = $this->requestStack->getCurrentRequest(); $url = UrlGeneratorInterface::ABSOLUTE_URL === $referenceType ? $request->getUriForPath($logoutPath) : $request->getBasePath().$logoutPath; diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/views/Login/after_login.html.twig b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/views/Login/after_login.html.twig index 9972b48ae730..3ef1f9c7bd18 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/views/Login/after_login.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/views/Login/after_login.html.twig @@ -3,4 +3,14 @@ {% block body %} Hello {{ app.user.username }}!

You're browsing to path "{{ app.request.pathInfo }}". + + Log out. + Log out. + + Log out. + Log out. + + Log out. + Log out. + {% endblock %} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php index 0dc038e1df26..6119b45803e9 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/FormLoginTest.php @@ -36,6 +36,40 @@ public function testFormLogin($config) $this->assertContains('You\'re browsing to path "/profile".', $text); } + /** + * @dataProvider getConfigs + */ + public function testFormLogout($config) + { + $client = $this->createClient(array('test_case' => 'StandardFormLogin', 'root_config' => $config)); + $client->insulate(); + + $form = $client->request('GET', '/login')->selectButton('login')->form(); + $form['_username'] = 'johannes'; + $form['_password'] = 'test'; + $client->submit($form); + + $this->assertRedirect($client->getResponse(), '/profile'); + + $crawler = $client->followRedirect(); + $text = $crawler->text(); + + $this->assertContains('Hello johannes!', $text); + $this->assertContains('You\'re browsing to path "/profile".', $text); + + $logoutLinks = $crawler->selectLink('Log out')->links(); + $this->assertCount(6, $logoutLinks); + $this->assertSame($logoutLinks[0]->getUri(), $logoutLinks[1]->getUri()); + $this->assertSame($logoutLinks[2]->getUri(), $logoutLinks[3]->getUri()); + $this->assertSame($logoutLinks[4]->getUri(), $logoutLinks[5]->getUri()); + + $this->assertNotSame($logoutLinks[0]->getUri(), $logoutLinks[2]->getUri()); + $this->assertNotSame($logoutLinks[1]->getUri(), $logoutLinks[3]->getUri()); + + $this->assertSame($logoutLinks[0]->getUri(), $logoutLinks[4]->getUri()); + $this->assertSame($logoutLinks[1]->getUri(), $logoutLinks[5]->getUri()); + } + /** * @dataProvider getConfigs */ diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml index 624637b0c82a..19b9d8952ec5 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml @@ -23,8 +23,17 @@ security: form_login: check_path: /login_check default_target_path: /profile + logout: ~ anonymous: ~ + # This firewall is here just to check its the logout functionality + second_area: + http_basic: ~ + anonymous: ~ + logout: + target: /second/target + path: /second/logout + access_control: - { path: ^/unprotected_resource$, roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: ^/secure-but-not-covered-by-access-control$, roles: IS_AUTHENTICATED_ANONYMOUSLY } diff --git a/src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php b/src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php index 8d28b3f246f7..3473f97cad49 100644 --- a/src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/Twig/Extension/LogoutUrlExtension.php @@ -41,11 +41,11 @@ public function getFunctions() /** * Generates the relative logout URL for the firewall. * - * @param string $key The firewall key + * @param string|null $key The firewall key or null to use the current firewall key * * @return string The relative logout URL */ - public function getLogoutPath($key) + public function getLogoutPath($key = null) { return $this->helper->getLogoutPath($key); } @@ -53,11 +53,11 @@ public function getLogoutPath($key) /** * Generates the absolute logout URL for the firewall. * - * @param string $key The firewall key + * @param string|null $key The firewall key or null to use the current firewall key * * @return string The absolute logout URL */ - public function getLogoutUrl($key) + public function getLogoutUrl($key = null) { return $this->helper->getLogoutUrl($key); }