Skip to content

Commit 2f1a850

Browse files
committed
Fix csrf token with ensure_logout. Fixes #628
1 parent 446f0c5 commit 2f1a850

File tree

2 files changed

+82
-4
lines changed

2 files changed

+82
-4
lines changed

Diff for: Controller/AuthorizeController.php

+81-4
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@
3030
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
3131
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
3232
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
33+
use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException;
3334
use Symfony\Component\Security\Core\User\UserInterface;
35+
use Symfony\Component\Security\Csrf\CsrfToken;
36+
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
3437
use Twig\Environment as TwigEnvironment;
3538

3639
/**
@@ -95,6 +98,11 @@ class AuthorizeController
9598
*/
9699
private $eventDispatcher;
97100

101+
/**
102+
* @var CsrfTokenManagerInterface
103+
*/
104+
private $csrfTokenManager;
105+
98106
/**
99107
* This controller had been made as a service due to support symfony 4 where all* services are private by default.
100108
* Thus, this is considered a bad practice to fetch services directly from container.
@@ -113,6 +121,7 @@ public function __construct(
113121
ClientManagerInterface $clientManager,
114122
EventDispatcherInterface $eventDispatcher,
115123
TwigEnvironment $twig,
124+
CsrfTokenManagerInterface $csrfTokenManager,
116125
SessionInterface $session = null
117126
) {
118127
$this->requestStack = $requestStack;
@@ -124,6 +133,7 @@ public function __construct(
124133
$this->router = $router;
125134
$this->clientManager = $clientManager;
126135
$this->eventDispatcher = $eventDispatcher;
136+
$this->csrfTokenManager = $csrfTokenManager;
127137
$this->twig = $twig;
128138
}
129139

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

147+
$form = $this->authorizeForm;
148+
$formHandler = $this->authorizeFormHandler;
149+
137150
if (!$user instanceof UserInterface) {
138151
throw new AccessDeniedException('This user does not have access to this section.');
139152
}
140153

141154
if ($this->session && true === $this->session->get('_fos_oauth_server.ensure_logout')) {
155+
$this->checkCsrfTokenBeforeInvalidingTheSession($form, $request);
156+
142157
$this->session->invalidate(600);
143158
$this->session->set('_fos_oauth_server.ensure_logout', true);
144-
}
145159

146-
$form = $this->authorizeForm;
147-
$formHandler = $this->authorizeFormHandler;
160+
$this->regenerateTokenForInvalidatedSession($form, $request);
161+
}
148162

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

217231
if (null === $clientId = $request->get('client_id')) {
218232
$formData = $request->get($this->authorizeForm->getName(), []);
219-
$clientId = isset($formData['client_id']) ? $formData['client_id'] : null;
233+
$clientId = $formData['client_id'] ?? null;
220234
}
221235

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

248262
return $request;
249263
}
264+
265+
/**
266+
* Validate if the current POST CSRF token is valid.
267+
* We need to do this now as the session will be regenerated due to the `ensure_logout` parameter.
268+
*/
269+
private function checkCsrfTokenBeforeInvalidingTheSession(Form $form, Request $request): void
270+
{
271+
if (!$request->isMethod('POST')) {
272+
// no need to check the CSRF token if we are not on a POST request (ie. submitting the form)
273+
return;
274+
}
275+
276+
if (!$form->getConfig()->getOption('csrf_protection')) {
277+
// no csrf security, no need to validate token
278+
return;
279+
}
280+
281+
$tokenFieldName = $form->getConfig()->getOption('csrf_field_name');
282+
$tokenId = $form->getConfig()->getOption('csrf_token_id') ?? $form->getName();
283+
284+
$formData = $request->request->get($form->getName());
285+
$tokenValue = $formData[$tokenFieldName] ?? null;
286+
287+
$token = new CsrfToken($tokenId, $tokenValue);
288+
289+
if (!$this->csrfTokenManager->isTokenValid($token)) {
290+
throw new InvalidCsrfTokenException();
291+
}
292+
}
293+
294+
/**
295+
* This method will inject a newly regenerated CSRF token into the actual form
296+
* as Symfony's form manager will check this token upon the current session.
297+
*
298+
* As we have regenerate a session, we need to inject the newly generated token into
299+
* the form data.
300+
*
301+
* It does bypass Symfony form CSRF protection, but the CSRF token is validated
302+
* in the `checkCsrfTokenBeforeInvalidingTheSession` method
303+
*/
304+
private function regenerateTokenForInvalidatedSession(Form $form, Request $request): void
305+
{
306+
if (!$request->isMethod('POST')) {
307+
// no need to check the CSRF token if we are not on a POST request (ie. submitting the form)
308+
return;
309+
}
310+
311+
if (!$form->getConfig()->getOption('csrf_protection')) {
312+
// no csrf security, no need to regenerate a valid token
313+
return;
314+
}
315+
316+
$tokenFieldName = $form->getConfig()->getOption('csrf_field_name');
317+
$tokenId = $form->getConfig()->getOption('csrf_token_id') ?? $form->getName();
318+
319+
// regenerate a new token and replace the form data as Symfony's form manager will check this token.
320+
// the request token has already been checked.
321+
$newToken = $this->csrfTokenManager->refreshToken($tokenId);
322+
323+
$formData = $request->request->get($form->getName());
324+
$formData[$tokenFieldName] = $newToken->getValue();
325+
$request->request->set($form->getName(), $formData);
326+
}
250327
}

Diff for: Resources/config/authorize.xml

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
<argument type="service" id="fos_oauth_server.client_manager" />
3434
<argument type="service" id="event_dispatcher" />
3535
<argument type="service" id="twig" />
36+
<argument type="service" id="security.csrf.token_manager" />
3637
<argument type="service" id="session" on-invalid="null" />
3738
</service>
3839
</services>

0 commit comments

Comments
 (0)