From 55e0b344b59901018215e36056eecf37cb34813a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Mon, 4 Nov 2024 11:28:25 +0100 Subject: [PATCH] Add caching capabilities to UserRepository --- config-templates/module_oidc.php | 7 ++++ routing/services/services.yml | 6 +++- src/Controller/Federation/Test.php | 2 ++ src/ModuleConfig.php | 17 +++++++++ src/Repositories/UserRepository.php | 36 ++++++++++++++++--- .../RevokeTokenByAuthCodeIdTraitTest.php | 2 +- 6 files changed, 63 insertions(+), 7 deletions(-) diff --git a/config-templates/module_oidc.php b/config-templates/module_oidc.php index 0b915221..bd61a5de 100644 --- a/config-templates/module_oidc.php +++ b/config-templates/module_oidc.php @@ -288,6 +288,13 @@ // 60 * 60 * 6, // Default lifetime in seconds (used when particular cache item doesn't define its own lifetime) //], + // Cache duration for user entities (authenticated users data). If not set, cache duration will be the same as + // session duration. This is used to avoid fetching user data from database on every authentication event. + // This is only relevant if protocol cache adapter is set up. For duration format info, check + // https://www.php.net/manual/en/dateinterval.construct.php. +// ModuleConfig::OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION => 'PT1H', // 1 hour + ModuleConfig::OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION => null, // fallback to session duration + /** * Cron related options. */ diff --git a/routing/services/services.yml b/routing/services/services.yml index 0d5e8a48..0b4ea060 100644 --- a/routing/services/services.yml +++ b/routing/services/services.yml @@ -115,4 +115,8 @@ services: SimpleSAML\OpenID\Federation: factory: [ '@SimpleSAML\Module\oidc\Factories\FederationFactory', 'build' ] SimpleSAML\OpenID\Jwks: - factory: [ '@SimpleSAML\Module\oidc\Factories\JwksFactory', 'build' ] \ No newline at end of file + factory: [ '@SimpleSAML\Module\oidc\Factories\JwksFactory', 'build' ] + + # SSP + SimpleSAML\Database: + factory: [ 'SimpleSAML\Database', 'getInstance' ] diff --git a/src/Controller/Federation/Test.php b/src/Controller/Federation/Test.php index 64d579cf..0f4477b6 100644 --- a/src/Controller/Federation/Test.php +++ b/src/Controller/Federation/Test.php @@ -6,6 +6,7 @@ namespace SimpleSAML\Module\oidc\Controller\Federation; +use SimpleSAML\Database; use SimpleSAML\Module\oidc\Codebooks\RegistrationTypeEnum; use SimpleSAML\Module\oidc\Factories\CoreFactory; use SimpleSAML\Module\oidc\Factories\Entities\ClientEntityFactory; @@ -33,6 +34,7 @@ public function __construct( protected ?FederationCache $federationCache, protected LoggerService $loggerService, protected Jwks $jwks, + protected Database $database, protected ClientEntityFactory $clientEntityFactory, protected CoreFactory $coreFactory, protected \DateInterval $maxCacheDuration = new \DateInterval('PT30S'), diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index eec9669c..91331c01 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -80,6 +80,7 @@ class ModuleConfig final public const OPTION_FEDERATION_ENTITY_STATEMENT_CACHE_DURATION = 'federation_entity_statement_cache_duration'; final public const OPTION_PROTOCOL_CACHE_ADAPTER = 'protocol_cache_adapter'; final public const OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS = 'protocol_cache_adapter_arguments'; + final public const OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION = 'protocol_user_entity_cache_duration'; protected static array $standardScopes = [ ScopesEnum::OpenId->value => [ @@ -630,4 +631,20 @@ public function getProtocolCacheAdapterArguments(): array { return $this->config()->getOptionalArray(self::OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS, []); } + + /** + * Get cache duration for user entities (user data). If not set in configuration, it will fall back to SSP session + * duration. + * + * @throws \Exception + */ + public function getUserEntityCacheDuration(): DateInterval + { + return new DateInterval( + $this->config()->getOptionalString( + self::OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION, + null, + ) ?? "PT{$this->sspConfig()->getInteger('session.duration')}S", + ); + } } diff --git a/src/Repositories/UserRepository.php b/src/Repositories/UserRepository.php index 61b7fcba..ae0fb21f 100644 --- a/src/Repositories/UserRepository.php +++ b/src/Repositories/UserRepository.php @@ -48,6 +48,11 @@ public function getTableName(): string return $this->database->applyPrefix(self::TABLE_NAME); } + public function getCacheKey(string $identifier): string + { + return $this->getTableName() . '_' . $identifier; + } + /** * @param string $identifier * @@ -56,6 +61,13 @@ public function getTableName(): string */ public function getUserEntityByIdentifier(string $identifier): ?UserEntity { + /** @var ?array $cachedState */ + $cachedState = $this->protocolCache?->get(null, $this->getCacheKey($identifier)); + + if (is_array($cachedState)) { + return $this->userEntityFactory->fromState($cachedState); + } + $stmt = $this->database->read( "SELECT * FROM {$this->getTableName()} WHERE id = :id", [ @@ -99,21 +111,29 @@ public function add(UserEntity $userEntity): void $stmt, $userEntity->getState(), ); + + $this->protocolCache?->set( + $userEntity->getState(), + $this->moduleConfig->getUserEntityCacheDuration(), + $this->getCacheKey($userEntity->getIdentifier()), + ); } - public function delete(UserEntity $user): void + public function delete(UserEntity $userEntity): void { $this->database->write( "DELETE FROM {$this->getTableName()} WHERE id = :id", [ - 'id' => $user->getIdentifier(), + 'id' => $userEntity->getIdentifier(), ], ); + + $this->protocolCache?->delete($this->getCacheKey($userEntity->getIdentifier())); } - public function update(UserEntity $user, ?DateTimeImmutable $updatedAt = null): void + public function update(UserEntity $userEntity, ?DateTimeImmutable $updatedAt = null): void { - $user->setUpdatedAt($updatedAt ?? $this->helpers->dateTime()->getUtc()); + $userEntity->setUpdatedAt($updatedAt ?? $this->helpers->dateTime()->getUtc()); $stmt = sprintf( "UPDATE %s SET claims = :claims, updated_at = :updated_at, created_at = :created_at WHERE id = :id", @@ -122,7 +142,13 @@ public function update(UserEntity $user, ?DateTimeImmutable $updatedAt = null): $this->database->write( $stmt, - $user->getState(), + $userEntity->getState(), + ); + + $this->protocolCache?->set( + $userEntity->getState(), + $this->moduleConfig->getUserEntityCacheDuration(), + $this->getCacheKey($userEntity->getIdentifier()), ); } } diff --git a/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php b/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php index ddb89984..aa015884 100644 --- a/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php +++ b/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php @@ -168,7 +168,7 @@ public function getDatabase(): Database $moduleConfig, $database, null, - $clientEntityFactoryMock + $clientEntityFactoryMock, ); $this->accessTokenRepository = new AccessTokenRepository(