From 9642aa525ff0c247810d6db10aee75b866e94008 Mon Sep 17 00:00:00 2001 From: David Badura Date: Fri, 13 Mar 2026 08:37:54 +0100 Subject: [PATCH] fix cipher key store & change api --- .../Cryptography/BaseCryptographer.php | 2 +- .../Cryptography/Store/CipherKeyStore.php | 4 +- .../Store/InMemoryCipherKeyStore.php | 42 ++-- .../Cryptography/BaseCryptographerTest.php | 2 +- .../Store/InMemoryCipherKeyStoreTest.php | 203 ++++++++++++++++-- 5 files changed, 203 insertions(+), 50 deletions(-) diff --git a/src/Extension/Cryptography/BaseCryptographer.php b/src/Extension/Cryptography/BaseCryptographer.php index 51dd0b5..2d19a64 100644 --- a/src/Extension/Cryptography/BaseCryptographer.php +++ b/src/Extension/Cryptography/BaseCryptographer.php @@ -54,7 +54,7 @@ public function encrypt(string $subjectId, mixed $value): array $cipherKey = $this->cipherKeyStore->currentKeyFor($subjectId); } catch (CipherKeyNotExists) { $cipherKey = ($this->cipherKeyFactory)($subjectId); - $this->cipherKeyStore->store($cipherKey->id, $cipherKey); + $this->cipherKeyStore->store($cipherKey); } $parameter = $this->cipher->encrypt($cipherKey, $value); diff --git a/src/Extension/Cryptography/Store/CipherKeyStore.php b/src/Extension/Cryptography/Store/CipherKeyStore.php index 82541c9..c0eefdf 100644 --- a/src/Extension/Cryptography/Store/CipherKeyStore.php +++ b/src/Extension/Cryptography/Store/CipherKeyStore.php @@ -13,9 +13,9 @@ interface CipherKeyStore public function currentKeyFor(string $subjectId): CipherKey; /** @throws CipherKeyNotExists */ - public function get(string $keyId): CipherKey; + public function get(string $id): CipherKey; - public function store(string $id, CipherKey $key): void; + public function store(CipherKey $key): void; public function remove(string $id): void; diff --git a/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php b/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php index 20a0322..70c8f66 100644 --- a/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php +++ b/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php @@ -14,7 +14,7 @@ final class InMemoryCipherKeyStore implements CipherKeyStore /** @var array */ private array $indexById = []; - /** @var array> */ + /** @var array> */ private array $indexBySubjectId = []; public function currentKeyFor(string $subjectId): CipherKey @@ -32,43 +32,31 @@ public function currentKeyFor(string $subjectId): CipherKey return $this->indexBySubjectId[$subjectId][$lastKey]; } - public function get(string $keyId): CipherKey + public function get(string $id): CipherKey { - return $this->indexById[$keyId] ?? throw CipherKeyNotExists::forKeyId($keyId); + return $this->indexById[$id] ?? throw CipherKeyNotExists::forKeyId($id); } - public function store(string $id, CipherKey $key): void + public function store(CipherKey $key): void { - $this->indexById[$id] = $key; + $this->remove($key->id); - if (!isset($this->indexBySubjectId[$key->subjectId])) { - $this->indexBySubjectId[$key->subjectId] = []; - } - - $this->indexBySubjectId[$key->subjectId][] = $key; + $this->indexById[$key->id] = $key; + $this->indexBySubjectId[$key->subjectId][$key->id] = $key; } public function remove(string $id): void { - unset($this->indexById[$id]); - - foreach ($this->indexBySubjectId as $subjectId => $keys) { - $filtered = []; - - foreach ($keys as $key) { - if ($key->id === $id) { - continue; - } + $key = $this->indexById[$id] ?? null; - $filtered[] = $key; - } - - if ($filtered === []) { - unset($this->indexBySubjectId[$subjectId]); - } else { - $this->indexBySubjectId[$subjectId] = $filtered; - } + if (!$key) { + return; } + + unset( + $this->indexBySubjectId[$key->subjectId][$id], + $this->indexById[$id], + ); } public function clear(): void diff --git a/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php b/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php index 296026a..2ee8ef5 100644 --- a/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php +++ b/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php @@ -70,7 +70,7 @@ public function testEncryptWithoutKey(): void $cipherKeyStore ->expects($this->once()) ->method('store') - ->with('key-456', $cipherKey); + ->with($cipherKey); $cipher = $this->createMock(Cipher::class); $cipher->expects($this->once())->method('encrypt')->with($cipherKey, 'info@patchlevel.de') diff --git a/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php b/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php index 99dcb44..0f19d62 100644 --- a/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php +++ b/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php @@ -17,17 +17,17 @@ final class InMemoryCipherKeyStoreTest extends TestCase public function testStoreAndLoad(): void { $key = new CipherKey( - 'foo', - 'bar', - 'baz', + 'key-1', + 'subject-1', + 'secret', 'aes-256-gcm', new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); - $store->store('foo', $key); + $store->store($key); - self::assertSame($key, $store->get('foo')); + self::assertSame($key, $store->get('key-1')); } public function testLoadFailed(): void @@ -35,50 +35,215 @@ public function testLoadFailed(): void $this->expectException(CipherKeyNotExists::class); $store = new InMemoryCipherKeyStore(); - $store->get('foo'); + $store->get('non-existent'); } public function testRemove(): void { $key = new CipherKey( - 'foo', - 'bar', - 'baz', + 'key-1', + 'subject-1', + 'secret', 'aes-256-gcm', new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); - $store->store('foo', $key); + $store->store($key); - self::assertSame($key, $store->get('foo')); + self::assertSame($key, $store->get('key-1')); - $store->remove('foo'); + $store->remove('key-1'); $this->expectException(CipherKeyNotExists::class); - $store->get('foo'); + $store->get('key-1'); } public function testClear(): void { $key = new CipherKey( - 'foo', - 'bar', - 'baz', + 'key-1', + 'subject-1', + 'secret', 'aes-256-gcm', new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); - $store->store('foo', $key); + $store->store($key); - self::assertSame($key, $store->get('foo')); + self::assertSame($key, $store->get('key-1')); $store->clear(); $this->expectException(CipherKeyNotExists::class); - $store->get('foo'); + $store->get('key-1'); + } + + public function testCurrentKeyFor(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-1', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + self::assertSame($key2, $store->currentKeyFor('subject-1')); + } + + public function testCurrentKeyForNotExists(): void + { + $this->expectException(CipherKeyNotExists::class); + + $store = new InMemoryCipherKeyStore(); + $store->currentKeyFor('non-existent'); + } + + public function testRemoveWithSubjectId(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-1', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key3 = new CipherKey( + 'key-3', + 'subject-2', + 'secret-3', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + $store->store($key3); + + $store->removeWithSubjectId('subject-1'); + + $this->expectException(CipherKeyNotExists::class); + $store->get('key-1'); + } + + public function testRemoveWithSubjectIdDoesNotAffectOtherSubjects(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-2', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + $store->removeWithSubjectId('subject-1'); + + self::assertSame($key2, $store->get('key-2')); + } + + public function testRemoveWithSubjectIdNonExistent(): void + { + $store = new InMemoryCipherKeyStore(); + $store->removeWithSubjectId('non-existent'); + + $this->expectNotToPerformAssertions(); + } + + public function testRemoveNonExistent(): void + { + $store = new InMemoryCipherKeyStore(); + $store->remove('non-existent'); + + $this->expectNotToPerformAssertions(); + } + + public function testStoreOverwritesExistingKey(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + self::assertSame($key2, $store->get('key-1')); + self::assertSame($key2, $store->currentKeyFor('subject-1')); + } + + public function testMultipleSubjects(): void + { + $key1 = new CipherKey( + 'key-1', + 'subject-1', + 'secret-1', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $key2 = new CipherKey( + 'key-2', + 'subject-2', + 'secret-2', + 'aes-256-gcm', + new DateTimeImmutable(), + ); + + $store = new InMemoryCipherKeyStore(); + $store->store($key1); + $store->store($key2); + + self::assertSame($key1, $store->currentKeyFor('subject-1')); + self::assertSame($key2, $store->currentKeyFor('subject-2')); } }