From 674e886ad0d314337ab85d5d4b200a89438170cf Mon Sep 17 00:00:00 2001 From: David Badura Date: Wed, 23 Sep 2026 16:11:03 +0200 Subject: [PATCH 1/2] Fix cache keys and eviction of the cipher key store decorators The cache keys used a colon, which is a reserved character in PSR-6 and PSR-16, so the decorators failed with any compliant cache like symfony/cache. Ids and subject ids are now hashed, and a cache hit is only accepted if the cached key really belongs to the requested id or subject. Removing keys also left entries behind: removeWithSubjectId() kept the cached keys by id and remove() kept the cached key by subject, so deleted personal data could still be decrypted until the cache expired. The decorators now remember the cached key ids per subject and evict all of them. --- composer.json | 1 + composer.lock | 383 +++++++++++++++++- src/Extension/Cryptography/Store/CacheKey.php | 31 ++ .../Store/Psr16CacheStoreDecorator.php | 75 +++- .../Store/Psr6CacheStoreDecorator.php | 96 ++++- .../Store/Psr16CacheStoreDecoratorTest.php | 175 +++++--- .../Store/Psr6CacheStoreDecoratorTest.php | 176 +++++--- 7 files changed, 778 insertions(+), 159 deletions(-) create mode 100644 src/Extension/Cryptography/Store/CacheKey.php diff --git a/composer.json b/composer.json index d1e52068..ba1d35b9 100644 --- a/composer.json +++ b/composer.json @@ -34,6 +34,7 @@ "phpstan/phpstan": "^2.1.39", "phpstan/phpstan-phpunit": "^2.0.15", "phpunit/phpunit": "^11.5.53", + "symfony/cache": "^6.4.0 || ^7.0.0 || ^8.0.0", "symfony/var-dumper": "^5.4.29 || ^6.4.0 || ^7.0.0 || ^8.0.0" }, "config": { diff --git a/composer.lock b/composer.lock index bf3fe207..8305d8ea 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "607eb78316379b3e1506540bda756c5c", + "content-hash": "f9b338659f01e2901633f1d62e9def68", "packages": [ { "name": "psr/cache", @@ -4319,6 +4319,185 @@ ], "time": "2024-10-20T05:08:20+00:00" }, + { + "name": "symfony/cache", + "version": "v8.1.7", + "source": { + "type": "git", + "url": "https://github.com/symfony/cache.git", + "reference": "47616ab077b5aff4d0f718b96680a48599fc1a3f" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/cache/zipball/47616ab077b5aff4d0f718b96680a48599fc1a3f", + "reference": "47616ab077b5aff4d0f718b96680a48599fc1a3f", + "shasum": "" + }, + "require": { + "php": ">=8.4.1", + "psr/cache": "^2.0|^3.0", + "psr/log": "^1.1|^2|^3", + "symfony/cache-contracts": "^3.6", + "symfony/service-contracts": "^2.5|^3", + "symfony/var-exporter": "^8.1" + }, + "conflict": { + "ext-redis": "<6.1", + "ext-relay": "<0.12.1" + }, + "provide": { + "psr/cache-implementation": "2.0|3.0", + "psr/simple-cache-implementation": "1.0|2.0|3.0", + "symfony/cache-implementation": "1.1|2.0|3.0" + }, + "require-dev": { + "cache/integration-tests": "^1.0.3", + "doctrine/dbal": "^4.3", + "predis/predis": "^1.1|^2.0", + "psr/simple-cache": "^1.0|^2.0|^3.0", + "symfony/clock": "^7.4|^8.0", + "symfony/config": "^7.4|^8.0", + "symfony/dependency-injection": "^7.4|^8.0", + "symfony/filesystem": "^7.4|^8.0", + "symfony/http-kernel": "^7.4|^8.0", + "symfony/messenger": "^7.4|^8.0", + "symfony/var-dumper": "^7.4|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Cache\\": "" + }, + "classmap": [ + "Traits/ValueWrapper.php" + ], + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides extended PSR-6, PSR-16 (and tags) implementations", + "homepage": "https://symfony.com", + "keywords": [ + "caching", + "psr6" + ], + "support": { + "source": "https://github.com/symfony/cache/tree/v8.1.7" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-09-08T13:39:13+00:00" + }, + { + "name": "symfony/cache-contracts", + "version": "v3.7.1", + "source": { + "type": "git", + "url": "https://github.com/symfony/cache-contracts.git", + "reference": "9789738bc19af1106dc54d6afba9a0b467516cf2" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/cache-contracts/zipball/9789738bc19af1106dc54d6afba9a0b467516cf2", + "reference": "9789738bc19af1106dc54d6afba9a0b467516cf2", + "shasum": "" + }, + "require": { + "php": ">=8.1", + "psr/cache": "^3.0" + }, + "type": "library", + "extra": { + "thanks": { + "url": "https://github.com/symfony/contracts", + "name": "symfony/contracts" + }, + "branch-alias": { + "dev-main": "3.7-dev" + } + }, + "autoload": { + "psr-4": { + "Symfony\\Contracts\\Cache\\": "" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Generic abstractions related to caching", + "homepage": "https://symfony.com", + "keywords": [ + "abstractions", + "contracts", + "decoupling", + "interfaces", + "interoperability", + "standards" + ], + "support": { + "source": "https://github.com/symfony/cache-contracts/tree/v3.7.1" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-06-05T06:23:12+00:00" + }, { "name": "symfony/console", "version": "v8.0.7", @@ -4411,16 +4590,16 @@ }, { "name": "symfony/deprecation-contracts", - "version": "v3.6.0", + "version": "v3.7.1", "source": { "type": "git", "url": "https://github.com/symfony/deprecation-contracts.git", - "reference": "63afe740e99a13ba87ec199bb07bbdee937a5b62" + "reference": "f3202fa1b5097b0af062dc978b32ecf63404e31d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/deprecation-contracts/zipball/63afe740e99a13ba87ec199bb07bbdee937a5b62", - "reference": "63afe740e99a13ba87ec199bb07bbdee937a5b62", + "url": "https://api.github.com/repos/symfony/deprecation-contracts/zipball/f3202fa1b5097b0af062dc978b32ecf63404e31d", + "reference": "f3202fa1b5097b0af062dc978b32ecf63404e31d", "shasum": "" }, "require": { @@ -4433,7 +4612,7 @@ "name": "symfony/contracts" }, "branch-alias": { - "dev-main": "3.6-dev" + "dev-main": "3.7-dev" } }, "autoload": { @@ -4458,7 +4637,7 @@ "description": "A generic function and convention to trigger deprecation notices", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/deprecation-contracts/tree/v3.6.0" + "source": "https://github.com/symfony/deprecation-contracts/tree/v3.7.1" }, "funding": [ { @@ -4469,12 +4648,16 @@ "url": "https://github.com/fabpot", "type": "github" }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, { "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", "type": "tidelift" } ], - "time": "2024-09-25T14:21:43+00:00" + "time": "2026-06-05T06:23:12+00:00" }, { "name": "symfony/filesystem", @@ -4768,6 +4951,93 @@ ], "time": "2024-09-09T11:45:10+00:00" }, + { + "name": "symfony/polyfill-deepclone", + "version": "v1.42.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/polyfill-deepclone.git", + "reference": "70ba0627efc68e97ea392843458a2dd9d6dbd156" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/polyfill-deepclone/zipball/70ba0627efc68e97ea392843458a2dd9d6dbd156", + "reference": "70ba0627efc68e97ea392843458a2dd9d6dbd156", + "shasum": "" + }, + "require": { + "php": ">=8.1" + }, + "provide": { + "ext-deepclone": "*" + }, + "suggest": { + "ext-deepclone": "For best performance" + }, + "type": "library", + "extra": { + "thanks": { + "url": "https://github.com/symfony/polyfill", + "name": "symfony/polyfill" + } + }, + "autoload": { + "files": [ + "bootstrap.php" + ], + "psr-4": { + "Symfony\\Polyfill\\DeepClone\\": "" + }, + "classmap": [ + "Resources/stubs" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony polyfill for the deepclone extension", + "homepage": "https://symfony.com", + "keywords": [ + "compatibility", + "deepclone", + "polyfill", + "portable", + "shim" + ], + "support": { + "source": "https://github.com/symfony/polyfill-deepclone/tree/v1.42.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-08-07T06:33:24+00:00" + }, { "name": "symfony/polyfill-intl-grapheme", "version": "v1.33.0", @@ -5167,16 +5437,16 @@ }, { "name": "symfony/service-contracts", - "version": "v3.6.1", + "version": "v3.7.3", "source": { "type": "git", "url": "https://github.com/symfony/service-contracts.git", - "reference": "45112560a3ba2d715666a509a0bc9521d10b6c43" + "reference": "15e6a07ec2a2c75ceb1b21dd98105ee8456d2257" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/service-contracts/zipball/45112560a3ba2d715666a509a0bc9521d10b6c43", - "reference": "45112560a3ba2d715666a509a0bc9521d10b6c43", + "url": "https://api.github.com/repos/symfony/service-contracts/zipball/15e6a07ec2a2c75ceb1b21dd98105ee8456d2257", + "reference": "15e6a07ec2a2c75ceb1b21dd98105ee8456d2257", "shasum": "" }, "require": { @@ -5194,7 +5464,7 @@ "name": "symfony/contracts" }, "branch-alias": { - "dev-main": "3.6-dev" + "dev-main": "3.7-dev" } }, "autoload": { @@ -5230,7 +5500,7 @@ "standards" ], "support": { - "source": "https://github.com/symfony/service-contracts/tree/v3.6.1" + "source": "https://github.com/symfony/service-contracts/tree/v3.7.3" }, "funding": [ { @@ -5250,7 +5520,7 @@ "type": "tidelift" } ], - "time": "2025-07-15T11:30:57+00:00" + "time": "2026-07-27T15:39:01+00:00" }, { "name": "symfony/string", @@ -5429,6 +5699,89 @@ ], "time": "2026-02-15T10:53:29+00:00" }, + { + "name": "symfony/var-exporter", + "version": "v8.1.6", + "source": { + "type": "git", + "url": "https://github.com/symfony/var-exporter.git", + "reference": "802362f41128c430fd6685491e1fb72127fae78a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/var-exporter/zipball/802362f41128c430fd6685491e1fb72127fae78a", + "reference": "802362f41128c430fd6685491e1fb72127fae78a", + "shasum": "" + }, + "require": { + "php": ">=8.4.1", + "symfony/deprecation-contracts": "^2.5|^3", + "symfony/polyfill-deepclone": "^1.40" + }, + "require-dev": { + "symfony/property-access": "^7.4|^8.0", + "symfony/serializer": "^7.4|^8.0", + "symfony/var-dumper": "^7.4|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\VarExporter\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides tools to export, instantiate, hydrate, clone and lazy-load PHP objects", + "homepage": "https://symfony.com", + "keywords": [ + "clone", + "construct", + "deep-clone", + "export", + "hydrate", + "instantiate", + "lazy-loading", + "proxy", + "serialize" + ], + "support": { + "source": "https://github.com/symfony/var-exporter/tree/v8.1.6" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-08-29T06:30:03+00:00" + }, { "name": "thecodingmachine/safe", "version": "v3.4.0", diff --git a/src/Extension/Cryptography/Store/CacheKey.php b/src/Extension/Cryptography/Store/CacheKey.php new file mode 100644 index 00000000..3529d00e --- /dev/null +++ b/src/Extension/Cryptography/Store/CacheKey.php @@ -0,0 +1,31 @@ +cache->get($key); + $entry = $this->cache->get(CacheKey::forSubjectId($subjectId)); - if ($entry instanceof CipherKey) { + if ($entry instanceof CipherKey && $entry->subjectId === $subjectId) { return $entry; } $entry = $this->cipherKeyStore->currentKeyFor($subjectId); - $this->cache->set($key, $entry, $this->ttl); + $this->cache->set(CacheKey::forSubjectId($subjectId), $entry, $this->ttl); + $this->rememberKeyId($entry); return $entry; } public function get(string $id): CipherKey { - $key = 'id:' . $id; - $entry = $this->cache->get($key); + $entry = $this->cache->get(CacheKey::forId($id)); - if ($entry instanceof CipherKey) { + if ($entry instanceof CipherKey && $entry->id === $id) { return $entry; } $entry = $this->cipherKeyStore->get($id); - $this->cache->set($key, $entry, $this->ttl); + $this->cache->set(CacheKey::forId($id), $entry, $this->ttl); + $this->rememberKeyId($entry); return $entry; } @@ -53,21 +60,65 @@ public function store(CipherKey $key): void { $this->cipherKeyStore->store($key); - $this->cache->set('id:' . $key->id, $key, $this->ttl); - $this->cache->set('subjectId:' . $key->subjectId, $key, $this->ttl); + $this->cache->set(CacheKey::forId($key->id), $key, $this->ttl); + $this->cache->set(CacheKey::forSubjectId($key->subjectId), $key, $this->ttl); + $this->rememberKeyId($key); } public function remove(string $id): void { + try { + $subjectId = $this->get($id)->subjectId; + } catch (CipherKeyNotExists) { + $subjectId = null; + } + $this->cipherKeyStore->remove($id); - $this->cache->delete('id:' . $id); + $this->cache->delete(CacheKey::forId($id)); + + if ($subjectId === null) { + return; + } + + $this->cache->delete(CacheKey::forSubjectId($subjectId)); } public function removeWithSubjectId(string $subjectId): void { $this->cipherKeyStore->removeWithSubjectId($subjectId); - $this->cache->delete('subjectId:' . $subjectId); + $this->cache->deleteMultiple([ + CacheKey::forSubjectId($subjectId), + CacheKey::forSubjectKeyIds($subjectId), + ...array_map(CacheKey::forId(...), $this->keyIds($subjectId)), + ]); + } + + /** + * Remember which key ids of a subject are cached, so that all of them can be evicted on removal. + * The list is saved after the key itself, so it never expires before one of its keys. + */ + private function rememberKeyId(CipherKey $key): void + { + $keyIds = $this->keyIds($key->subjectId); + + if (!in_array($key->id, $keyIds, true)) { + $keyIds[] = $key->id; + } + + $this->cache->set(CacheKey::forSubjectKeyIds($key->subjectId), $keyIds, $this->ttl); + } + + /** @return list */ + private function keyIds(string $subjectId): array + { + $keyIds = $this->cache->get(CacheKey::forSubjectKeyIds($subjectId)); + + if (!is_array($keyIds)) { + return []; + } + + return array_values(array_filter($keyIds, is_string(...))); } } diff --git a/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php b/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php index c8ca0451..5a096ce8 100644 --- a/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php +++ b/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php @@ -8,6 +8,13 @@ use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; use Psr\Cache\CacheItemPoolInterface; +use function array_filter; +use function array_map; +use function array_values; +use function in_array; +use function is_array; +use function is_string; + final readonly class Psr6CacheStoreDecorator implements CipherKeyStore { public function __construct( @@ -19,38 +26,32 @@ public function __construct( public function currentKeyFor(string $subjectId): CipherKey { - $key = 'subjectId:' . $subjectId; - $item = $this->cache->getItem($key); - $entry = $item->get(); + $entry = $this->fetch(CacheKey::forSubjectId($subjectId)); - if ($item->isHit() && $entry instanceof CipherKey) { + if ($entry instanceof CipherKey && $entry->subjectId === $subjectId) { return $entry; } $entry = $this->cipherKeyStore->currentKeyFor($subjectId); - $item->set($entry); - $item->expiresAfter($this->expiresAfter); - $this->cache->save($item); + $this->save(CacheKey::forSubjectId($subjectId), $entry); + $this->rememberKeyId($entry); return $entry; } public function get(string $id): CipherKey { - $key = 'id:' . $id; - $item = $this->cache->getItem($key); - $entry = $item->get(); + $entry = $this->fetch(CacheKey::forId($id)); - if ($item->isHit() && $entry instanceof CipherKey) { + if ($entry instanceof CipherKey && $entry->id === $id) { return $entry; } $entry = $this->cipherKeyStore->get($id); - $item->set($entry); - $item->expiresAfter($this->expiresAfter); - $this->cache->save($item); + $this->save(CacheKey::forId($id), $entry); + $this->rememberKeyId($entry); return $entry; } @@ -62,15 +63,78 @@ public function store(CipherKey $key): void public function remove(string $id): void { + try { + $subjectId = $this->get($id)->subjectId; + } catch (CipherKeyNotExists) { + $subjectId = null; + } + $this->cipherKeyStore->remove($id); - $this->cache->deleteItem('id:' . $id); + $this->cache->deleteItem(CacheKey::forId($id)); + + if ($subjectId === null) { + return; + } + + $this->cache->deleteItem(CacheKey::forSubjectId($subjectId)); } public function removeWithSubjectId(string $subjectId): void { $this->cipherKeyStore->removeWithSubjectId($subjectId); - $this->cache->deleteItem('subjectId:' . $subjectId); + $this->cache->deleteItems([ + CacheKey::forSubjectId($subjectId), + CacheKey::forSubjectKeyIds($subjectId), + ...array_map(CacheKey::forId(...), $this->keyIds($subjectId)), + ]); + } + + private function fetch(string $cacheKey): mixed + { + $item = $this->cache->getItem($cacheKey); + + if (!$item->isHit()) { + return null; + } + + return $item->get(); + } + + private function save(string $cacheKey, mixed $value): void + { + $item = $this->cache->getItem($cacheKey); + $item->set($value); + $item->expiresAfter($this->expiresAfter); + + $this->cache->save($item); + } + + /** + * Remember which key ids of a subject are cached, so that all of them can be evicted on removal. + * The list is saved after the key itself, so it never expires before one of its keys. + */ + private function rememberKeyId(CipherKey $key): void + { + $keyIds = $this->keyIds($key->subjectId); + + if (!in_array($key->id, $keyIds, true)) { + $keyIds[] = $key->id; + } + + $this->save(CacheKey::forSubjectKeyIds($key->subjectId), $keyIds); + } + + /** @return list */ + private function keyIds(string $subjectId): array + { + $keyIds = $this->fetch(CacheKey::forSubjectKeyIds($subjectId)); + + if (!is_array($keyIds)) { + return []; + } + + return array_values(array_filter($keyIds, is_string(...))); } } diff --git a/tests/Unit/Extension/Cryptography/Store/Psr16CacheStoreDecoratorTest.php b/tests/Unit/Extension/Cryptography/Store/Psr16CacheStoreDecoratorTest.php index bee4764f..e222787f 100644 --- a/tests/Unit/Extension/Cryptography/Store/Psr16CacheStoreDecoratorTest.php +++ b/tests/Unit/Extension/Cryptography/Store/Psr16CacheStoreDecoratorTest.php @@ -6,109 +6,178 @@ use DateTimeImmutable; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; +use Patchlevel\Hydrator\Extension\Cryptography\Store\CacheKey; +use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyNotExists; use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyStore; +use Patchlevel\Hydrator\Extension\Cryptography\Store\InMemoryCipherKeyStore; use Patchlevel\Hydrator\Extension\Cryptography\Store\Psr16CacheStoreDecorator; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Psr\SimpleCache\CacheInterface; +use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Cache\Psr16Cache; #[CoversClass(Psr16CacheStoreDecorator::class)] +#[CoversClass(CacheKey::class)] final class Psr16CacheStoreDecoratorTest extends TestCase { - public function testCurrentKeyForWithCacheHit(): void + public function testCurrentKeyForIsCached(): void { - $key = $this->createKey(); + $key = $this->createKey('key-1', 'subject-1'); - $cache = $this->createMock(CacheInterface::class); - $cache->expects(self::once())->method('get')->with('subjectId:subject-1')->willReturn($key); - $cache->expects(self::never())->method('set'); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($key); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::never())->method('currentKeyFor'); + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); + + self::assertEquals($key, $store->currentKeyFor('subject-1')); - $store = new Psr16CacheStoreDecorator($innerStore, $cache); + $innerStore->clear(); - self::assertSame($key, $store->currentKeyFor('subject-1')); + self::assertEquals($key, $store->currentKeyFor('subject-1')); } - public function testCurrentKeyForWithCacheMiss(): void + public function testGetIsCached(): void { - $key = $this->createKey(); + $key = $this->createKey('key-1', 'subject-1'); - $cache = $this->createMock(CacheInterface::class); - $cache->expects(self::once())->method('get')->with('subjectId:subject-1')->willReturn(null); - $cache->expects(self::once())->method('set')->with('subjectId:subject-1', $key, 42); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($key); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('currentKeyFor')->with('subject-1')->willReturn($key); + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); - $store = new Psr16CacheStoreDecorator($innerStore, $cache, 42); + self::assertEquals($key, $store->get('key-1')); - self::assertSame($key, $store->currentKeyFor('subject-1')); + $innerStore->clear(); + + self::assertEquals($key, $store->get('key-1')); } - public function testGetWithCacheMiss(): void + public function testKeysWithReservedCharacters(): void { - $key = $this->createKey(); + $key = $this->createKey('key:{1}', 'user@example.com/1'); - $cache = $this->createMock(CacheInterface::class); - $cache->expects(self::once())->method('get')->with('id:key-1')->willReturn(false); - $cache->expects(self::once())->method('set')->with('id:key-1', $key, null); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($key); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('get')->with('key-1')->willReturn($key); + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); - $store = new Psr16CacheStoreDecorator($innerStore, $cache); - - self::assertSame($key, $store->get('key-1')); + self::assertEquals($key, $store->currentKeyFor('user@example.com/1')); + self::assertEquals($key, $store->get('key:{1}')); } - public function testStoreWritesInnerStoreAndBothCacheEntries(): void + public function testStoreFillsCache(): void { - $key = $this->createKey(); - - $cache = $this->createMock(CacheInterface::class); - $cache->expects(self::exactly(2))->method('set')->willReturnMap([ - ['id:key-1', $key, 17, true], - ['subjectId:subject-1', $key, 17, true], - ]); + $key = $this->createKey('key-1', 'subject-1'); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('store')->with($key); + $innerStore = new InMemoryCipherKeyStore(); - $store = new Psr16CacheStoreDecorator($innerStore, $cache, 17); + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); $store->store($key); + + self::assertEquals($key, $innerStore->get('key-1')); + + $innerStore->clear(); + + self::assertEquals($key, $store->get('key-1')); + self::assertEquals($key, $store->currentKeyFor('subject-1')); } - public function testRemoveDeletesIdCacheEntry(): void + public function testRemoveEvictsKeyAndSubject(): void { - $cache = $this->createMock(CacheInterface::class); - $cache->expects(self::once())->method('delete')->with('id:key-1'); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($this->createKey('key-1', 'subject-1')); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('remove')->with('key-1'); + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); + $store->currentKeyFor('subject-1'); + $store->get('key-1'); + + $store->remove('key-1'); + + $this->assertKeyIdNotExists($store, 'key-1'); + $this->assertSubjectIdNotExists($store, 'subject-1'); + } - $store = new Psr16CacheStoreDecorator($innerStore, $cache); + public function testRemoveUnknownKey(): void + { + $innerStore = new InMemoryCipherKeyStore(); + + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); $store->remove('key-1'); + + $this->assertKeyIdNotExists($store, 'key-1'); + } + + public function testRemoveWithSubjectIdEvictsAllKeysOfSubject(): void + { + $otherKey = $this->createKey('key-3', 'subject-2'); + + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($this->createKey('key-1', 'subject-1')); + $innerStore->store($this->createKey('key-2', 'subject-1')); + $innerStore->store($otherKey); + + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); + $store->get('key-1'); + $store->get('key-2'); + $store->currentKeyFor('subject-1'); + $store->get('key-3'); + + $store->removeWithSubjectId('subject-1'); + + $this->assertKeyIdNotExists($store, 'key-1'); + $this->assertKeyIdNotExists($store, 'key-2'); + $this->assertSubjectIdNotExists($store, 'subject-1'); + self::assertEquals($otherKey, $store->get('key-3')); } - public function testRemoveWithSubjectIdDeletesSubjectCacheEntry(): void + public function testTtlIsPassedToCache(): void { $cache = $this->createMock(CacheInterface::class); - $cache->expects(self::once())->method('delete')->with('subjectId:subject-1'); + $cache->method('get')->willReturn(null); + $cache->expects(self::exactly(2))->method('set')->with(self::anything(), self::anything(), 42); $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('removeWithSubjectId')->with('subject-1'); + $innerStore->method('get')->willReturn($this->createKey('key-1', 'subject-1')); - $store = new Psr16CacheStoreDecorator($innerStore, $cache); - $store->removeWithSubjectId('subject-1'); + $store = new Psr16CacheStoreDecorator($innerStore, $cache, 42); + $store->get('key-1'); + } + + private function createCache(): CacheInterface + { + return new Psr16Cache(new ArrayAdapter()); + } + + private function assertKeyIdNotExists(CipherKeyStore $store, string $id): void + { + try { + $store->get($id); + self::fail('Expected CipherKeyNotExists for key id ' . $id); + } catch (CipherKeyNotExists) { + $this->addToAssertionCount(1); + } + } + + private function assertSubjectIdNotExists(CipherKeyStore $store, string $subjectId): void + { + try { + $store->currentKeyFor($subjectId); + self::fail('Expected CipherKeyNotExists for subject id ' . $subjectId); + } catch (CipherKeyNotExists) { + $this->addToAssertionCount(1); + } } - private function createKey(): CipherKey + /** + * @param non-empty-string $id + * @param non-empty-string $subjectId + */ + private function createKey(string $id, string $subjectId): CipherKey { return new CipherKey( - 'key-1', - 'subject-1', + $id, + $subjectId, 'secret', 'aes-256-gcm', new DateTimeImmutable(), diff --git a/tests/Unit/Extension/Cryptography/Store/Psr6CacheStoreDecoratorTest.php b/tests/Unit/Extension/Cryptography/Store/Psr6CacheStoreDecoratorTest.php index 99632d90..29932ed1 100644 --- a/tests/Unit/Extension/Cryptography/Store/Psr6CacheStoreDecoratorTest.php +++ b/tests/Unit/Extension/Cryptography/Store/Psr6CacheStoreDecoratorTest.php @@ -6,123 +6,173 @@ use DateTimeImmutable; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; +use Patchlevel\Hydrator\Extension\Cryptography\Store\CacheKey; +use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyNotExists; use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyStore; +use Patchlevel\Hydrator\Extension\Cryptography\Store\InMemoryCipherKeyStore; use Patchlevel\Hydrator\Extension\Cryptography\Store\Psr6CacheStoreDecorator; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; +use Symfony\Component\Cache\Adapter\ArrayAdapter; #[CoversClass(Psr6CacheStoreDecorator::class)] +#[CoversClass(CacheKey::class)] final class Psr6CacheStoreDecoratorTest extends TestCase { - public function testCurrentKeyForWithCacheHit(): void + public function testCurrentKeyForIsCached(): void { - $key = $this->createKey(); + $key = $this->createKey('key-1', 'subject-1'); - $item = $this->createMock(CacheItemInterface::class); - $item->expects(self::once())->method('get')->willReturn($key); - $item->expects(self::once())->method('isHit')->willReturn(true); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($key); - $cache = $this->createMock(CacheItemPoolInterface::class); - $cache->expects(self::once())->method('getItem')->with('subjectId:subject-1')->willReturn($item); + $store = new Psr6CacheStoreDecorator($innerStore, new ArrayAdapter()); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::never())->method('currentKeyFor'); + self::assertEquals($key, $store->currentKeyFor('subject-1')); - $store = new Psr6CacheStoreDecorator($innerStore, $cache); + $innerStore->clear(); - self::assertSame($key, $store->currentKeyFor('subject-1')); + self::assertEquals($key, $store->currentKeyFor('subject-1')); } - public function testCurrentKeyForWithCacheMiss(): void + public function testGetIsCached(): void { - $key = $this->createKey(); + $key = $this->createKey('key-1', 'subject-1'); - $item = $this->createMock(CacheItemInterface::class); - $item->expects(self::once())->method('get')->willReturn(null); - $item->expects(self::once())->method('isHit')->willReturn(false); - $item->expects(self::once())->method('set')->with($key)->willReturnSelf(); - $item->expects(self::once())->method('expiresAfter')->with(42)->willReturnSelf(); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($key); - $cache = $this->createMock(CacheItemPoolInterface::class); - $cache->expects(self::once())->method('getItem')->with('subjectId:subject-1')->willReturn($item); - $cache->expects(self::once())->method('save')->with($item); + $store = new Psr6CacheStoreDecorator($innerStore, new ArrayAdapter()); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('currentKeyFor')->with('subject-1')->willReturn($key); + self::assertEquals($key, $store->get('key-1')); - $store = new Psr6CacheStoreDecorator($innerStore, $cache, 42); + $innerStore->clear(); - self::assertSame($key, $store->currentKeyFor('subject-1')); + self::assertEquals($key, $store->get('key-1')); } - public function testGetWithCacheMiss(): void + public function testKeysWithReservedCharacters(): void { - $key = $this->createKey(); - - $item = $this->createMock(CacheItemInterface::class); - $item->expects(self::once())->method('get')->willReturn(null); - $item->expects(self::once())->method('isHit')->willReturn(false); - $item->expects(self::once())->method('set')->with($key)->willReturnSelf(); - $item->expects(self::once())->method('expiresAfter')->with(null)->willReturnSelf(); - - $cache = $this->createMock(CacheItemPoolInterface::class); - $cache->expects(self::once())->method('getItem')->with('id:key-1')->willReturn($item); - $cache->expects(self::once())->method('save')->with($item); + $key = $this->createKey('key:{1}', 'user@example.com/1'); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('get')->with('key-1')->willReturn($key); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($key); - $store = new Psr6CacheStoreDecorator($innerStore, $cache); + $store = new Psr6CacheStoreDecorator($innerStore, new ArrayAdapter()); - self::assertSame($key, $store->get('key-1')); + self::assertEquals($key, $store->currentKeyFor('user@example.com/1')); + self::assertEquals($key, $store->get('key:{1}')); } public function testStoreDelegatesToInnerStore(): void { - $key = $this->createKey(); - - $cache = $this->createMock(CacheItemPoolInterface::class); - $cache->expects(self::never())->method('getItem'); - $cache->expects(self::never())->method('save'); + $key = $this->createKey('key-1', 'subject-1'); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('store')->with($key); + $innerStore = new InMemoryCipherKeyStore(); - $store = new Psr6CacheStoreDecorator($innerStore, $cache); + $store = new Psr6CacheStoreDecorator($innerStore, new ArrayAdapter()); $store->store($key); + + self::assertEquals($key, $innerStore->get('key-1')); } - public function testRemoveDeletesIdCacheEntry(): void + public function testRemoveEvictsKeyAndSubject(): void { - $cache = $this->createMock(CacheItemPoolInterface::class); - $cache->expects(self::once())->method('deleteItem')->with('id:key-1'); + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($this->createKey('key-1', 'subject-1')); - $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('remove')->with('key-1'); + $store = new Psr6CacheStoreDecorator($innerStore, new ArrayAdapter()); + $store->currentKeyFor('subject-1'); + $store->get('key-1'); - $store = new Psr6CacheStoreDecorator($innerStore, $cache); $store->remove('key-1'); + + $this->assertKeyIdNotExists($store, 'key-1'); + $this->assertSubjectIdNotExists($store, 'subject-1'); } - public function testRemoveWithSubjectIdDeletesSubjectCacheEntry(): void + public function testRemoveUnknownKey(): void { + $innerStore = new InMemoryCipherKeyStore(); + + $store = new Psr6CacheStoreDecorator($innerStore, new ArrayAdapter()); + $store->remove('key-1'); + + $this->assertKeyIdNotExists($store, 'key-1'); + } + + public function testRemoveWithSubjectIdEvictsAllKeysOfSubject(): void + { + $otherKey = $this->createKey('key-3', 'subject-2'); + + $innerStore = new InMemoryCipherKeyStore(); + $innerStore->store($this->createKey('key-1', 'subject-1')); + $innerStore->store($this->createKey('key-2', 'subject-1')); + $innerStore->store($otherKey); + + $store = new Psr6CacheStoreDecorator($innerStore, new ArrayAdapter()); + $store->get('key-1'); + $store->get('key-2'); + $store->currentKeyFor('subject-1'); + $store->get('key-3'); + + $store->removeWithSubjectId('subject-1'); + + $this->assertKeyIdNotExists($store, 'key-1'); + $this->assertKeyIdNotExists($store, 'key-2'); + $this->assertSubjectIdNotExists($store, 'subject-1'); + self::assertEquals($otherKey, $store->get('key-3')); + } + + public function testExpiresAfterIsPassedToCacheItems(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->method('isHit')->willReturn(false); + $item->method('set')->willReturnSelf(); + $item->expects(self::exactly(2))->method('expiresAfter')->with(42)->willReturnSelf(); + $cache = $this->createMock(CacheItemPoolInterface::class); - $cache->expects(self::once())->method('deleteItem')->with('subjectId:subject-1'); + $cache->method('getItem')->willReturn($item); + $cache->expects(self::exactly(2))->method('save')->with($item); $innerStore = $this->createMock(CipherKeyStore::class); - $innerStore->expects(self::once())->method('removeWithSubjectId')->with('subject-1'); + $innerStore->method('get')->willReturn($this->createKey('key-1', 'subject-1')); - $store = new Psr6CacheStoreDecorator($innerStore, $cache); - $store->removeWithSubjectId('subject-1'); + $store = new Psr6CacheStoreDecorator($innerStore, $cache, 42); + $store->get('key-1'); + } + + private function assertKeyIdNotExists(CipherKeyStore $store, string $id): void + { + try { + $store->get($id); + self::fail('Expected CipherKeyNotExists for key id ' . $id); + } catch (CipherKeyNotExists) { + $this->addToAssertionCount(1); + } + } + + private function assertSubjectIdNotExists(CipherKeyStore $store, string $subjectId): void + { + try { + $store->currentKeyFor($subjectId); + self::fail('Expected CipherKeyNotExists for subject id ' . $subjectId); + } catch (CipherKeyNotExists) { + $this->addToAssertionCount(1); + } } - private function createKey(): CipherKey + /** + * @param non-empty-string $id + * @param non-empty-string $subjectId + */ + private function createKey(string $id, string $subjectId): CipherKey { return new CipherKey( - 'key-1', - 'subject-1', + $id, + $subjectId, 'secret', 'aes-256-gcm', new DateTimeImmutable(), From b00f23dd81f8e3b42a6e36a6c1fd77cb91bbfdb3 Mon Sep 17 00:00:00 2001 From: David Badura Date: Wed, 23 Sep 2026 19:34:48 +0200 Subject: [PATCH 2/2] Add tests for cache keys and eviction of stored keys Covers the exact cache key format and removeWithSubjectId() after store(), which were not caught by the existing tests. Also drops the rememberKeyId() call in currentKeyFor(), it only caches the entry by subject, which is evicted directly anyway. --- .../Store/Psr16CacheStoreDecorator.php | 1 - .../Store/Psr6CacheStoreDecorator.php | 1 - .../Cryptography/Store/CacheKeyTest.php | 28 +++++++++++++++++++ .../Store/Psr16CacheStoreDecoratorTest.php | 13 +++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 tests/Unit/Extension/Cryptography/Store/CacheKeyTest.php diff --git a/src/Extension/Cryptography/Store/Psr16CacheStoreDecorator.php b/src/Extension/Cryptography/Store/Psr16CacheStoreDecorator.php index 76694ac1..f48ba34d 100644 --- a/src/Extension/Cryptography/Store/Psr16CacheStoreDecorator.php +++ b/src/Extension/Cryptography/Store/Psr16CacheStoreDecorator.php @@ -35,7 +35,6 @@ public function currentKeyFor(string $subjectId): CipherKey $entry = $this->cipherKeyStore->currentKeyFor($subjectId); $this->cache->set(CacheKey::forSubjectId($subjectId), $entry, $this->ttl); - $this->rememberKeyId($entry); return $entry; } diff --git a/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php b/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php index 5a096ce8..88494b0c 100644 --- a/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php +++ b/src/Extension/Cryptography/Store/Psr6CacheStoreDecorator.php @@ -35,7 +35,6 @@ public function currentKeyFor(string $subjectId): CipherKey $entry = $this->cipherKeyStore->currentKeyFor($subjectId); $this->save(CacheKey::forSubjectId($subjectId), $entry); - $this->rememberKeyId($entry); return $entry; } diff --git a/tests/Unit/Extension/Cryptography/Store/CacheKeyTest.php b/tests/Unit/Extension/Cryptography/Store/CacheKeyTest.php new file mode 100644 index 00000000..5c26407f --- /dev/null +++ b/tests/Unit/Extension/Cryptography/Store/CacheKeyTest.php @@ -0,0 +1,28 @@ +get('key-3')); } + public function testRemoveWithSubjectIdEvictsStoredKeys(): void + { + $innerStore = new InMemoryCipherKeyStore(); + + $store = new Psr16CacheStoreDecorator($innerStore, $this->createCache()); + $store->store($this->createKey('key-1', 'subject-1')); + + $store->removeWithSubjectId('subject-1'); + + $this->assertKeyIdNotExists($store, 'key-1'); + $this->assertSubjectIdNotExists($store, 'subject-1'); + } + public function testTtlIsPassedToCache(): void { $cache = $this->createMock(CacheInterface::class);