From dbaed5d2d6be519e127a6664d81f49ac878b47f4 Mon Sep 17 00:00:00 2001 From: "smarcet@gmail.com" Date: Tue, 4 Oct 2022 08:38:02 -0500 Subject: [PATCH] Instrospection tweaks performance fixes for introspection endpoint Signed-off-by: smarcet@gmail.com Change-Id: I7928c53fe0885c83571ee5b5821a988f1752b1fb --- app/Models/OAuth2/AccessToken.php | 1 + app/Models/OAuth2/Client.php | 1 + app/Models/OAuth2/RefreshToken.php | 1 + .../DoctrineAccessTokenRepository.php | 19 +++++++++++++++++++ .../DoctrineOAuth2ClientRepository.php | 19 +++++++++++++++++++ .../DoctrineRefreshTokenRepository.php | 18 ++++++++++++++++++ app/Services/OAuth2/TokenService.php | 13 +++++++------ .../OAuth2/GrantTypes/AbstractGrantType.php | 4 ++-- .../ValidateBearerTokenGrantType.php | 4 ++-- .../Repositories/IAccessTokenRepository.php | 6 ++++++ .../OAuth2/Repositories/IClientRepository.php | 7 +++++++ run_tests.sh | 2 ++ 12 files changed, 85 insertions(+), 10 deletions(-) create mode 100755 run_tests.sh diff --git a/app/Models/OAuth2/AccessToken.php b/app/Models/OAuth2/AccessToken.php index 34c96d46..9f5445a5 100644 --- a/app/Models/OAuth2/AccessToken.php +++ b/app/Models/OAuth2/AccessToken.php @@ -20,6 +20,7 @@ /** * @ORM\Entity(repositoryClass="App\Repositories\DoctrineAccessTokenRepository") * @ORM\Table(name="oauth2_access_token") + * @ORM\Cache("NONSTRICT_READ_WRITE") * Class AccessToken * @package Models\OAuth2 */ diff --git a/app/Models/OAuth2/Client.php b/app/Models/OAuth2/Client.php index f2495c14..b1dfde3d 100644 --- a/app/Models/OAuth2/Client.php +++ b/app/Models/OAuth2/Client.php @@ -37,6 +37,7 @@ /** * @ORM\Entity(repositoryClass="App\Repositories\DoctrineOAuth2ClientRepository") * @ORM\Table(name="oauth2_client") + * @ORM\Cache("NONSTRICT_READ_WRITE") * Class Client * @package Models\OAuth2 */ diff --git a/app/Models/OAuth2/RefreshToken.php b/app/Models/OAuth2/RefreshToken.php index 02deb714..6c267c57 100644 --- a/app/Models/OAuth2/RefreshToken.php +++ b/app/Models/OAuth2/RefreshToken.php @@ -21,6 +21,7 @@ /** * @ORM\Entity(repositoryClass="App\Repositories\DoctrineRefreshTokenRepository") * @ORM\Table(name="oauth2_refresh_token") + * @ORM\Cache("NONSTRICT_READ_WRITE") * Class RefreshToken * Refresh Token Entity */ diff --git a/app/Repositories/DoctrineAccessTokenRepository.php b/app/Repositories/DoctrineAccessTokenRepository.php index ceb713c8..14c59a0f 100644 --- a/app/Repositories/DoctrineAccessTokenRepository.php +++ b/app/Repositories/DoctrineAccessTokenRepository.php @@ -14,6 +14,7 @@ use Doctrine\ORM\QueryBuilder; use Models\OAuth2\AccessToken; +use Models\OAuth2\RefreshToken; use OAuth2\Repositories\IAccessTokenRepository; /** * Class DoctrineAccessTokenRepository @@ -41,6 +42,24 @@ function getByValue(string $hashed_value):?AccessToken return $this->findOneBy(['value' => $hashed_value]); } + /** + * @param string $hashed_value + * @return AccessToken|null + */ + function getByValueCacheable(string $hashed_value):?AccessToken + { + return $this->getEntityManager() + ->createQueryBuilder() + ->select("e") + ->from($this->getBaseEntity(), "e") + ->where("e.value = (:value)") + ->setParameter("value", trim($hashed_value)) + ->setMaxResults(1) + ->getQuery() + ->setCacheable(true) + ->getOneOrNullResult(); + } + /** * @param string $hashed_value * @return AccessToken|null diff --git a/app/Repositories/DoctrineOAuth2ClientRepository.php b/app/Repositories/DoctrineOAuth2ClientRepository.php index 5e6f97cb..fbee1a1e 100644 --- a/app/Repositories/DoctrineOAuth2ClientRepository.php +++ b/app/Repositories/DoctrineOAuth2ClientRepository.php @@ -98,6 +98,25 @@ public function getClientById(string $client_id):?Client ->getOneOrNullResult(); } + /** + * @param string $client_id + * @return Client|null + * @throws \Doctrine\ORM\NonUniqueResultException + */ + public function getClientByIdCacheable(string $client_id):?Client + { + return $this->getEntityManager() + ->createQueryBuilder() + ->select("c") + ->from($this->getBaseEntity(), "c") + ->where("c.client_id = (:client_id)") + ->setParameter("client_id", trim($client_id)) + ->setMaxResults(1) + ->getQuery() + ->setCacheable(true) + ->getOneOrNullResult(); + } + /** * @param int $id * @return Client|null diff --git a/app/Repositories/DoctrineRefreshTokenRepository.php b/app/Repositories/DoctrineRefreshTokenRepository.php index 29f6a209..90a85c37 100644 --- a/app/Repositories/DoctrineRefreshTokenRepository.php +++ b/app/Repositories/DoctrineRefreshTokenRepository.php @@ -62,4 +62,22 @@ function getByValue(string $hashed_value):?RefreshToken { return $this->findOneBy(['value' => $hashed_value]); } + + /** + * @param string $hashed_value + * @return RefreshToken|null + */ + function getByValueCacheable(string $hashed_value):?RefreshToken + { + return $this->getEntityManager() + ->createQueryBuilder() + ->select("e") + ->from($this->getBaseEntity(), "e") + ->where("e.value = (:value)") + ->setParameter("value", trim($hashed_value)) + ->setMaxResults(1) + ->getQuery() + ->setCacheable(true) + ->getOneOrNullResult(); + } } \ No newline at end of file diff --git a/app/Services/OAuth2/TokenService.php b/app/Services/OAuth2/TokenService.php index 4fac6d85..53c66ad6 100644 --- a/app/Services/OAuth2/TokenService.php +++ b/app/Services/OAuth2/TokenService.php @@ -770,7 +770,7 @@ public function getAccessToken($value, $is_hashed = false) if (!$this->cache_service->exists($hashed_value)) { $this->lock_manager_service->lock('lock.get.accesstoken.' . $hashed_value, function () use ($value, $hashed_value) { // check on DB... - $access_token_db = $this->access_token_repository->getByValue($hashed_value); + $access_token_db = $this->access_token_repository->getByValueCacheable($hashed_value); if (is_null($access_token_db)) { if ($this->isAccessTokenRevoked($hashed_value)) { throw new RevokedAccessTokenException(sprintf('Access token %s is revoked!', $value)); @@ -830,8 +830,9 @@ public function getAccessToken($value, $is_hashed = false) $access_token->setRefreshToken($refresh_token); } } catch (UnacquiredLockException $ex1) { - throw new InvalidAccessTokenException(sprintf("access token %s ", $value)); + throw new InvalidAccessTokenException(sprintf("Access token %s. ", $value)); } + return $access_token; }); } @@ -920,11 +921,11 @@ public function createRefreshToken(AccessToken &$access_token, $refresh_cache = } /** - * @param \oauth2\services\refresh $value - * @param bool $is_hashed + * @param string $value + * @param false $is_hashed * @return RefreshToken * @throws InvalidGrantTypeException - * @throws ReplayAttackException + * @throws ReplayAttackRefreshTokenException * @throws RevokedRefreshTokenException */ public function getRefreshToken($value, $is_hashed = false) @@ -932,7 +933,7 @@ public function getRefreshToken($value, $is_hashed = false) //hash the given value, bc tokens values are stored hashed on DB $hashed_value = !$is_hashed ? Hash::compute('sha256', $value) : $value; - $refresh_token_db = $this->refresh_token_repository->getByValue($hashed_value); + $refresh_token_db = $this->refresh_token_repository->getByValueCacheable($hashed_value); if (is_null($refresh_token_db)) { if ($this->isRefreshTokenRevoked($hashed_value)) diff --git a/app/libs/OAuth2/GrantTypes/AbstractGrantType.php b/app/libs/OAuth2/GrantTypes/AbstractGrantType.php index a43077cc..2adfb13b 100644 --- a/app/libs/OAuth2/GrantTypes/AbstractGrantType.php +++ b/app/libs/OAuth2/GrantTypes/AbstractGrantType.php @@ -93,8 +93,8 @@ public function completeFlow(OAuth2Request $request) // get client credentials from request.. $this->client_auth_context = $this->client_service->getCurrentClientAuthInfo(); - // retrieve client from storage.. - $this->current_client = $this->client_repository->getClientById($this->client_auth_context->getId()); + // retrieve client from storage ... + $this->current_client = $this->client_repository->getClientByIdCacheable($this->client_auth_context->getId()); if (is_null($this->current_client)) throw new InvalidClientException diff --git a/app/libs/OAuth2/GrantTypes/ValidateBearerTokenGrantType.php b/app/libs/OAuth2/GrantTypes/ValidateBearerTokenGrantType.php index f66c7371..e78b2114 100644 --- a/app/libs/OAuth2/GrantTypes/ValidateBearerTokenGrantType.php +++ b/app/libs/OAuth2/GrantTypes/ValidateBearerTokenGrantType.php @@ -185,7 +185,7 @@ public function completeFlow(OAuth2Request $request) $strategy->validate($access_token, $this->current_client); - $issued_client = $this->client_repository->getClientById($access_token->getClientId()); + $issued_client = $this->client_repository->getClientByIdCacheable($access_token->getClientId()); if (is_null($issued_client)) { @@ -193,7 +193,7 @@ public function completeFlow(OAuth2Request $request) ( sprintf ( - 'access token %s does not belongs to client id %s', + 'Access token %s does not belongs to client id %s.', $token_value, $access_token->getClientId() ) diff --git a/app/libs/OAuth2/Repositories/IAccessTokenRepository.php b/app/libs/OAuth2/Repositories/IAccessTokenRepository.php index b90612f2..df2d6715 100644 --- a/app/libs/OAuth2/Repositories/IAccessTokenRepository.php +++ b/app/libs/OAuth2/Repositories/IAccessTokenRepository.php @@ -56,6 +56,12 @@ function getAllValidByUserId(int $user_id,PagingInfo $paging_info):PagingRespons */ function getByValue(string $hashed_value):?AccessToken; + /** + * @param string $hashed_value + * @return AccessToken|null + */ + function getByValueCacheable(string $hashed_value):?AccessToken; + /** * @param string $hashed_value * @return AccessToken diff --git a/app/libs/OAuth2/Repositories/IClientRepository.php b/app/libs/OAuth2/Repositories/IClientRepository.php index ebffdf36..82e231f0 100644 --- a/app/libs/OAuth2/Repositories/IClientRepository.php +++ b/app/libs/OAuth2/Repositories/IClientRepository.php @@ -31,6 +31,13 @@ public function getByApplicationName(string $app_name):?Client; */ public function getClientById(string $client_id):?Client; + /** + * @param string $client_id + * @return Client|null + * @throws \Doctrine\ORM\NonUniqueResultException + */ + public function getClientByIdCacheable(string $client_id):?Client; + /** * @param int $id * @return Client|null diff --git a/run_tests.sh b/run_tests.sh new file mode 100755 index 00000000..116b814c --- /dev/null +++ b/run_tests.sh @@ -0,0 +1,2 @@ +#!/usr/bin/bash +./vendor/bin/phpunit \ No newline at end of file