From 1bf1725a5103a86769f0e2665faf61f2c21e1228 Mon Sep 17 00:00:00 2001
From: Christian Foidl <christian@wunderwerk.io>
Date: Wed, 8 Jan 2025 15:11:51 +0100
Subject: [PATCH 1/3] Issue #3498190 by chfoidl: Implement lock to avoid token
request race conditions
---
src/Controller/Oauth2Token.php | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/src/Controller/Oauth2Token.php b/src/Controller/Oauth2Token.php
index 954cab8..dfbcc36 100644
--- a/src/Controller/Oauth2Token.php
+++ b/src/Controller/Oauth2Token.php
@@ -3,6 +3,7 @@
namespace Drupal\simple_oauth\Controller;
use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\Lock\LockBackendInterface;
use Drupal\simple_oauth\Server\AuthorizationServerFactoryInterface;
use GuzzleHttp\Psr7\Response;
use League\OAuth2\Server\Exception\OAuthServerException;
@@ -40,6 +41,13 @@ class Oauth2Token extends ControllerBase {
*/
protected ClientRepositoryInterface $clientRepository;
+ /**
+ * The lock backend.
+ *
+ * @var \Drupal\Core\Lock\LockBackendInterface
+ */
+ protected LockBackendInterface $lock;
+
/**
* The simple_oauth logger channel.
*
@@ -56,6 +64,8 @@ class Oauth2Token extends ControllerBase {
* The PSR-7 converter.
* @param \League\OAuth2\Server\Repositories\ClientRepositoryInterface $client_repository
* The client repository service.
+ * @param \Drupal\Core\Lock\LockBackendInterface $lock
+ * The lock backend.
* @param \Psr\Log\LoggerInterface $logger
* The simple_oauth logger channel.
*/
@@ -63,11 +73,13 @@ class Oauth2Token extends ControllerBase {
AuthorizationServerFactoryInterface $authorization_server_factory,
HttpMessageFactoryInterface $http_message_factory,
ClientRepositoryInterface $client_repository,
+ LockBackendInterface $lock,
LoggerInterface $logger,
) {
$this->authorizationServerFactory = $authorization_server_factory;
$this->httpMessageFactory = $http_message_factory;
$this->clientRepository = $client_repository;
+ $this->lock = $lock;
$this->logger = $logger;
}
@@ -79,6 +91,7 @@ class Oauth2Token extends ControllerBase {
$container->get('simple_oauth.server.authorization_server.factory'),
$container->get('psr7.http_message_factory'),
$container->get('simple_oauth.repositories.client'),
+ $container->get('lock'),
$container->get('logger.channel.simple_oauth')
);
}
@@ -99,6 +112,17 @@ class Oauth2Token extends ControllerBase {
$server_response = new Response();
$client_id = $request->get('client_id');
+ $req_key = hash('sha256', $request->getContent());
+
+ // Try to acquire the lock.
+ while (!$this->lock->acquire($req_key)) {
+ // If we can't acquire the lock, wait for it.
+ if ($this->lock->wait($req_key, 30)) {
+ // Timeout reached after 30 seconds.
+ return new Response(408);
+ }
+ }
+
try {
if (empty($client_id)) {
throw OAuthServerException::invalidRequest('client_id');
@@ -120,6 +144,10 @@ class Oauth2Token extends ControllerBase {
);
$response = $exception->generateHttpResponse($server_response);
}
+ finally {
+ // Release the lock.
+ $this->lock->release($req_key);
+ }
return $response;
}
--
GitLab
From aa5c08f98a77ed8e5519b83d160fbc6479446353 Mon Sep 17 00:00:00 2001
From: Christian Foidl <christian@wunderwerk.io>
Date: Sat, 25 Jan 2025 13:54:13 +0100
Subject: [PATCH 2/3] Use more sophisticated lock key generation; use
OAuthServerException for lock timeout #3498190
---
src/Controller/Oauth2Token.php | 38 +++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/Controller/Oauth2Token.php b/src/Controller/Oauth2Token.php
index dfbcc36..14d9562 100644
--- a/src/Controller/Oauth2Token.php
+++ b/src/Controller/Oauth2Token.php
@@ -2,6 +2,8 @@
namespace Drupal\simple_oauth\Controller;
+use Drupal\Component\Serialization\Json;
+use Drupal\Component\Utility\Crypt;
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Lock\LockBackendInterface;
use Drupal\simple_oauth\Server\AuthorizationServerFactoryInterface;
@@ -112,18 +114,18 @@ class Oauth2Token extends ControllerBase {
$server_response = new Response();
$client_id = $request->get('client_id');
- $req_key = hash('sha256', $request->getContent());
+ $lock_key = $this->createLockKey($request);
- // Try to acquire the lock.
- while (!$this->lock->acquire($req_key)) {
- // If we can't acquire the lock, wait for it.
- if ($this->lock->wait($req_key, 30)) {
- // Timeout reached after 30 seconds.
- return new Response(408);
+ try {
+ // Try to acquire the lock.
+ while (!$this->lock->acquire($lock_key)) {
+ // If we can't acquire the lock, wait for it.
+ if ($this->lock->wait($lock_key)) {
+ // Timeout reached after 30 seconds.
+ throw OAuthServerException::accessDenied('Request timed out. Could not aquire lock.');
+ }
}
- }
- try {
if (empty($client_id)) {
throw OAuthServerException::invalidRequest('client_id');
}
@@ -146,10 +148,26 @@ class Oauth2Token extends ControllerBase {
}
finally {
// Release the lock.
- $this->lock->release($req_key);
+ $this->lock->release($lock_key);
}
return $response;
}
+ /**
+ * Creates a lock key for the request.
+ *
+ * The key consists of the request content, in this case the
+ * grant payload.
+ *
+ * @param \Symfony\Component\HttpFoundation\Request $request
+ * The request.
+ *
+ * @return string
+ * The lock key.
+ */
+ protected function createLockKey(Request $request): string {
+ return Crypt::hashBase64(Json::encode($request->request->all()));
+ }
+
}
--
GitLab
From d13a881258df2f47d81e014ae6cf92bd6dc3c12d Mon Sep 17 00:00:00 2001
From: Bojan Bogdanovic <info@bojanbogdanovic.nl>
Date: Mon, 27 Jan 2025 08:47:14 +0100
Subject: [PATCH 3/3] Correct spelling
---
src/Controller/Oauth2Token.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Controller/Oauth2Token.php b/src/Controller/Oauth2Token.php
index 14d9562..c0f31e2 100644
--- a/src/Controller/Oauth2Token.php
+++ b/src/Controller/Oauth2Token.php
@@ -122,7 +122,7 @@ class Oauth2Token extends ControllerBase {
// If we can't acquire the lock, wait for it.
if ($this->lock->wait($lock_key)) {
// Timeout reached after 30 seconds.
- throw OAuthServerException::accessDenied('Request timed out. Could not aquire lock.');
+ throw OAuthServerException::accessDenied('Request timed out. Could not acquire lock.');
}
}
--
GitLab