Skip to content

Commit

Permalink
feature #13342 [security] Fetching current stored context when not ex…
Browse files Browse the repository at this point in the history
…plicitly specified (jaytaph)

This PR was squashed before being merged into the 2.7 branch (closes #13342).

Discussion
----------

[security] Fetching current stored context when not explicitly specified

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12160
| License       | MIT
| Doc PR        | N/A

This patch will use the current stored context found in a token (provided, there is one), if none has been specified.

According to a quick scan of the code, this will be the only place where `getProviderKey()` is used outside a specific class (the authentication providers will check token type before calling `getProviderKey()`, but maybe it's be a good idea to implement a "providerKeyTokenInterface" or something. It's a nicer solution imho than the current `method_exists()`

Commits
-------

f6046ba [security] Fetching current stored context when not explicitly specified
  • Loading branch information
fabpot committed Jan 18, 2015
2 parents e756135 + f6046ba commit 388ae55
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 15 deletions.
Expand Up @@ -12,8 +12,9 @@
<services>
<service id="templating.helper.logout_url" class="%templating.helper.logout_url.class%">
<tag name="templating.helper" alias="logout_url" />
<argument type="service" id="service_container" />
<argument type="service" id="request_stack" />
<argument type="service" id="router" />
<argument type="service" id="security.token_storage" />
</service>

<service id="templating.helper.security" class="%templating.helper.security.class%">
Expand Down
Expand Up @@ -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;

Expand All @@ -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;
}

/**
Expand All @@ -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
*/
Expand All @@ -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
*/
Expand All @@ -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));
}
Expand All @@ -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;

Expand Down
Expand Up @@ -3,4 +3,14 @@
{% block body %}
Hello {{ app.user.username }}!<br /><br />
You're browsing to path "{{ app.request.pathInfo }}".

<a href="{{ logout_path('default') }}">Log out</a>.
<a href="{{ logout_url('default') }}">Log out</a>.

<a href="{{ logout_path('second_area') }}">Log out</a>.
<a href="{{ logout_url('second_area') }}">Log out</a>.

<a href="{{ logout_path() }}">Log out</a>.
<a href="{{ logout_url() }}">Log out</a>.

{% endblock %}
Expand Up @@ -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
*/
Expand Down
Expand Up @@ -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 }
Expand Down
Expand Up @@ -41,23 +41,23 @@ 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);
}

/**
* 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);
}
Expand Down

0 comments on commit 388ae55

Please sign in to comment.