Skip to content

Commit

Permalink
Fix csrf token with ensure_logout. Fixes FriendsOfSymfony#628
Browse files Browse the repository at this point in the history
  • Loading branch information
jdeniau committed Mar 31, 2021
1 parent 446f0c5 commit 41a8157
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 4 deletions.
85 changes: 81 additions & 4 deletions Controller/AuthorizeController.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Csrf\CsrfToken;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Twig\Environment as TwigEnvironment;

/**
Expand Down Expand Up @@ -95,6 +98,11 @@ class AuthorizeController
*/
private $eventDispatcher;

/**
* @var CsrfTokenManagerInterface
*/
private $csrfTokenManager;

/**
* This controller had been made as a service due to support symfony 4 where all* services are private by default.
* Thus, this is considered a bad practice to fetch services directly from container.
Expand All @@ -113,6 +121,7 @@ public function __construct(
ClientManagerInterface $clientManager,
EventDispatcherInterface $eventDispatcher,
TwigEnvironment $twig,
CsrfTokenManagerInterface $csrfTokenManager,
SessionInterface $session = null
) {
$this->requestStack = $requestStack;
Expand All @@ -124,6 +133,7 @@ public function __construct(
$this->router = $router;
$this->clientManager = $clientManager;
$this->eventDispatcher = $eventDispatcher;
$this->csrfTokenManager = $csrfTokenManager;
$this->twig = $twig;
}

Expand All @@ -134,17 +144,21 @@ public function authorizeAction(Request $request)
{
$user = $this->tokenStorage->getToken()->getUser();

$form = $this->authorizeForm;
$formHandler = $this->authorizeFormHandler;

if (!$user instanceof UserInterface) {
throw new AccessDeniedException('This user does not have access to this section.');
}

if ($this->session && true === $this->session->get('_fos_oauth_server.ensure_logout')) {
$this->checkCsrfTokenBeforeInvalidingTheSession($form, $request);

$this->session->invalidate(600);
$this->session->set('_fos_oauth_server.ensure_logout', true);
}

$form = $this->authorizeForm;
$formHandler = $this->authorizeFormHandler;
$this->regenerateTokenForInvalidatedSession($form, $request);
}

/** @var PreAuthorizationEvent $event */
$event = $this->eventDispatcher->dispatch(new PreAuthorizationEvent($user, $this->getClient()));
Expand Down Expand Up @@ -216,7 +230,7 @@ protected function getClient()

if (null === $clientId = $request->get('client_id')) {
$formData = $request->get($this->authorizeForm->getName(), []);
$clientId = isset($formData['client_id']) ? $formData['client_id'] : null;
$clientId = $formData['client_id'] ?? null;
}

$this->client = $this->clientManager->findClientByPublicId($clientId);
Expand Down Expand Up @@ -247,4 +261,67 @@ private function getCurrentRequest()

return $request;
}

/**
* Validate if the current POST CSRF token is valid.
* We need to do this now as the session will be regenerated due to the `ensure_logout` parameter.
*/
private function checkCsrfTokenBeforeInvalidingTheSession(Form $form, Request $request): void
{
if (!$request->isMethod('POST')) {
// no need to check the CSRF token if we are not on a POST request (ie. submitting the form)
return;
}

if (!$form->getConfig()->getOption('csrf_protection')) {
// no csrf security, no need to validate token
return;
}

$tokenFieldName = $form->getConfig()->getOption('csrf_field_name');
$tokenId = $form->getConfig()->getOption('csrf_token_id') ?? $form->getName();

$formData = $request->request->get($form->getName());
$tokenValue = $formData[$tokenFieldName] ?? null;

$token = new CsrfToken($tokenId, $tokenValue);

if (!$this->csrfTokenManager->isTokenValid($token)) {
throw new InvalidCsrfTokenException();
}
}

/**
* This method will inject a newly regenerated CSRF token into the actual form
* as Symfony's form manager will check this token upon the current session.
*
* As we have regenerate a session, we need to inject the newly generated token into
* the form data.
*
* It does bypass Symfony form CSRF protection, but the CSRF token is validated
* in the `checkCsrfTokenBeforeInvalidingTheSession` method
*/
private function regenerateTokenForInvalidatedSession(Form $form, Request $request): void
{
if (!$request->isMethod('POST')) {
// no need to check the CSRF token if we are not on a POST request (ie. submitting the form)
return;
}

if (!$form->getConfig()->getOption('csrf_protection')) {
// no csrf security, no need to regenerate a valid token
return;
}

$tokenFieldName = $form->getConfig()->getOption('csrf_field_name');
$tokenId = $form->getConfig()->getOption('csrf_token_id') ?? $form->getName();

// regenerate a new token and replace the form data as Symfony's form manager will check this token.
// the request token has already been checked.
$newToken = $this->csrfTokenManager->refreshToken($tokenId);

$formData = $request->request->get($form->getName());
$formData[$tokenFieldName] = $newToken->getValue();
$request->request->set($form->getName(), $formData);
}
}
1 change: 1 addition & 0 deletions Resources/config/authorize.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
<argument type="service" id="fos_oauth_server.client_manager" />
<argument type="service" id="event_dispatcher" />
<argument type="service" id="twig" />
<argument type="service" id="security.csrf.token_manager" />
<argument type="service" id="session" on-invalid="null" />
</service>
</services>
Expand Down
11 changes: 11 additions & 0 deletions Tests/Controller/AuthorizeControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Twig\Environment as TwigEnvironment;

class AuthorizeControllerTest extends \PHPUnit\Framework\TestCase
Expand Down Expand Up @@ -132,6 +133,11 @@ class AuthorizeControllerTest extends \PHPUnit\Framework\TestCase
*/
protected $formView;

/**
* @var \Symfony\Component\Security\Csrf\CsrfTokenManagerInterface
*/
protected $csrfTokenManager;

public function setUp(): void
{
$this->requestStack = $this->getMockBuilder(RequestStack::class)
Expand Down Expand Up @@ -170,6 +176,10 @@ public function setUp(): void
->disableOriginalConstructor()
->getMock()
;
$this->csrfTokenManager = $this->getMockBuilder(CsrfTokenManagerInterface::class)
->disableOriginalConstructor()
->getMock()
;
$this->session = $this->getMockBuilder(SessionInterface::class)
->disableOriginalConstructor()
->getMock()
Expand All @@ -185,6 +195,7 @@ public function setUp(): void
$this->clientManager,
$this->eventDispatcher,
$this->twig,
$this->csrfTokenManager,
$this->session
);

Expand Down

0 comments on commit 41a8157

Please sign in to comment.