Skip to content

Commit 62ff552

Browse files
DavidBaduraDavidBadura
authored
Merge pull request #169 from patchlevel/fix-cipher-key-store
fix cipher key store & change api
2 parents 2566dd6 + 9642aa5 commit 62ff552

File tree

5 files changed

+203
-50
lines changed

5 files changed

+203
-50
lines changed

‎src/Extension/Cryptography/BaseCryptographer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public function encrypt(string $subjectId, mixed $value): array
5454
$cipherKey = $this->cipherKeyStore->currentKeyFor($subjectId);
5555
} catch (CipherKeyNotExists) {
5656
$cipherKey = ($this->cipherKeyFactory)($subjectId);
57-
$this->cipherKeyStore->store($cipherKey->id, $cipherKey);
57+
$this->cipherKeyStore->store($cipherKey);
5858
}
5959

6060
$parameter = $this->cipher->encrypt($cipherKey, $value);

‎src/Extension/Cryptography/Store/CipherKeyStore.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ interface CipherKeyStore
1313
public function currentKeyFor(string $subjectId): CipherKey;
1414

1515
/** @throws CipherKeyNotExists */
16-
public function get(string $keyId): CipherKey;
16+
public function get(string $id): CipherKey;
1717

18-
public function store(string $id, CipherKey $key): void;
18+
public function store(CipherKey $key): void;
1919

2020
public function remove(string $id): void;
2121

‎src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php‎

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ final class InMemoryCipherKeyStore implements CipherKeyStore
1414
/** @var array<string, CipherKey> */
1515
private array $indexById = [];
1616

17-
/** @var array<string, list<CipherKey>> */
17+
/** @var array<string, array<string, CipherKey>> */
1818
private array $indexBySubjectId = [];
1919

2020
public function currentKeyFor(string $subjectId): CipherKey
@@ -32,43 +32,31 @@ public function currentKeyFor(string $subjectId): CipherKey
3232
return $this->indexBySubjectId[$subjectId][$lastKey];
3333
}
3434

35-
public function get(string $keyId): CipherKey
35+
public function get(string $id): CipherKey
3636
{
37-
return $this->indexById[$keyId] ?? throw CipherKeyNotExists::forKeyId($keyId);
37+
return $this->indexById[$id] ?? throw CipherKeyNotExists::forKeyId($id);
3838
}
3939

40-
public function store(string $id, CipherKey $key): void
40+
public function store(CipherKey $key): void
4141
{
42-
$this->indexById[$id] = $key;
42+
$this->remove($key->id);
4343

44-
if (!isset($this->indexBySubjectId[$key->subjectId])) {
45-
$this->indexBySubjectId[$key->subjectId] = [];
46-
}
47-
48-
$this->indexBySubjectId[$key->subjectId][] = $key;
44+
$this->indexById[$key->id] = $key;
45+
$this->indexBySubjectId[$key->subjectId][$key->id] = $key;
4946
}
5047

5148
public function remove(string $id): void
5249
{
53-
unset($this->indexById[$id]);
54-
55-
foreach ($this->indexBySubjectId as $subjectId => $keys) {
56-
$filtered = [];
57-
58-
foreach ($keys as $key) {
59-
if ($key->id === $id) {
60-
continue;
61-
}
50+
$key = $this->indexById[$id] ?? null;
6251

63-
$filtered[] = $key;
64-
}
65-
66-
if ($filtered === []) {
67-
unset($this->indexBySubjectId[$subjectId]);
68-
} else {
69-
$this->indexBySubjectId[$subjectId] = $filtered;
70-
}
52+
if (!$key) {
53+
return;
7154
}
55+
56+
unset(
57+
$this->indexBySubjectId[$key->subjectId][$id],
58+
$this->indexById[$id],
59+
);
7260
}
7361

7462
public function clear(): void

‎tests/Unit/Extension/Cryptography/BaseCryptographerTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function testEncryptWithoutKey(): void
7070
$cipherKeyStore
7171
->expects($this->once())
7272
->method('store')
73-
->with('key-456', $cipherKey);
73+
->with($cipherKey);
7474

7575
$cipher = $this->createMock(Cipher::class);
7676
$cipher->expects($this->once())->method('encrypt')->with($cipherKey, 'info@patchlevel.de')

‎tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php‎

Lines changed: 184 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,68 +17,233 @@ final class InMemoryCipherKeyStoreTest extends TestCase
1717
public function testStoreAndLoad(): void
1818
{
1919
$key = new CipherKey(
20-
'foo',
21-
'bar',
22-
'baz',
20+
'key-1',
21+
'subject-1',
22+
'secret',
2323
'aes-256-gcm',
2424
new DateTimeImmutable(),
2525
);
2626

2727
$store = new InMemoryCipherKeyStore();
28-
$store->store('foo', $key);
28+
$store->store($key);
2929

30-
self::assertSame($key, $store->get('foo'));
30+
self::assertSame($key, $store->get('key-1'));
3131
}
3232

3333
public function testLoadFailed(): void
3434
{
3535
$this->expectException(CipherKeyNotExists::class);
3636

3737
$store = new InMemoryCipherKeyStore();
38-
$store->get('foo');
38+
$store->get('non-existent');
3939
}
4040

4141
public function testRemove(): void
4242
{
4343
$key = new CipherKey(
44-
'foo',
45-
'bar',
46-
'baz',
44+
'key-1',
45+
'subject-1',
46+
'secret',
4747
'aes-256-gcm',
4848
new DateTimeImmutable(),
4949
);
5050

5151
$store = new InMemoryCipherKeyStore();
52-
$store->store('foo', $key);
52+
$store->store($key);
5353

54-
self::assertSame($key, $store->get('foo'));
54+
self::assertSame($key, $store->get('key-1'));
5555

56-
$store->remove('foo');
56+
$store->remove('key-1');
5757

5858
$this->expectException(CipherKeyNotExists::class);
5959

60-
$store->get('foo');
60+
$store->get('key-1');
6161
}
6262

6363
public function testClear(): void
6464
{
6565
$key = new CipherKey(
66-
'foo',
67-
'bar',
68-
'baz',
66+
'key-1',
67+
'subject-1',
68+
'secret',
6969
'aes-256-gcm',
7070
new DateTimeImmutable(),
7171
);
7272

7373
$store = new InMemoryCipherKeyStore();
74-
$store->store('foo', $key);
74+
$store->store($key);
7575

76-
self::assertSame($key, $store->get('foo'));
76+
self::assertSame($key, $store->get('key-1'));
7777

7878
$store->clear();
7979

8080
$this->expectException(CipherKeyNotExists::class);
8181

82-
$store->get('foo');
82+
$store->get('key-1');
83+
}
84+
85+
public function testCurrentKeyFor(): void
86+
{
87+
$key1 = new CipherKey(
88+
'key-1',
89+
'subject-1',
90+
'secret-1',
91+
'aes-256-gcm',
92+
new DateTimeImmutable(),
93+
);
94+
95+
$key2 = new CipherKey(
96+
'key-2',
97+
'subject-1',
98+
'secret-2',
99+
'aes-256-gcm',
100+
new DateTimeImmutable(),
101+
);
102+
103+
$store = new InMemoryCipherKeyStore();
104+
$store->store($key1);
105+
$store->store($key2);
106+
107+
self::assertSame($key2, $store->currentKeyFor('subject-1'));
108+
}
109+
110+
public function testCurrentKeyForNotExists(): void
111+
{
112+
$this->expectException(CipherKeyNotExists::class);
113+
114+
$store = new InMemoryCipherKeyStore();
115+
$store->currentKeyFor('non-existent');
116+
}
117+
118+
public function testRemoveWithSubjectId(): void
119+
{
120+
$key1 = new CipherKey(
121+
'key-1',
122+
'subject-1',
123+
'secret-1',
124+
'aes-256-gcm',
125+
new DateTimeImmutable(),
126+
);
127+
128+
$key2 = new CipherKey(
129+
'key-2',
130+
'subject-1',
131+
'secret-2',
132+
'aes-256-gcm',
133+
new DateTimeImmutable(),
134+
);
135+
136+
$key3 = new CipherKey(
137+
'key-3',
138+
'subject-2',
139+
'secret-3',
140+
'aes-256-gcm',
141+
new DateTimeImmutable(),
142+
);
143+
144+
$store = new InMemoryCipherKeyStore();
145+
$store->store($key1);
146+
$store->store($key2);
147+
$store->store($key3);
148+
149+
$store->removeWithSubjectId('subject-1');
150+
151+
$this->expectException(CipherKeyNotExists::class);
152+
$store->get('key-1');
153+
}
154+
155+
public function testRemoveWithSubjectIdDoesNotAffectOtherSubjects(): void
156+
{
157+
$key1 = new CipherKey(
158+
'key-1',
159+
'subject-1',
160+
'secret-1',
161+
'aes-256-gcm',
162+
new DateTimeImmutable(),
163+
);
164+
165+
$key2 = new CipherKey(
166+
'key-2',
167+
'subject-2',
168+
'secret-2',
169+
'aes-256-gcm',
170+
new DateTimeImmutable(),
171+
);
172+
173+
$store = new InMemoryCipherKeyStore();
174+
$store->store($key1);
175+
$store->store($key2);
176+
177+
$store->removeWithSubjectId('subject-1');
178+
179+
self::assertSame($key2, $store->get('key-2'));
180+
}
181+
182+
public function testRemoveWithSubjectIdNonExistent(): void
183+
{
184+
$store = new InMemoryCipherKeyStore();
185+
$store->removeWithSubjectId('non-existent');
186+
187+
$this->expectNotToPerformAssertions();
188+
}
189+
190+
public function testRemoveNonExistent(): void
191+
{
192+
$store = new InMemoryCipherKeyStore();
193+
$store->remove('non-existent');
194+
195+
$this->expectNotToPerformAssertions();
196+
}
197+
198+
public function testStoreOverwritesExistingKey(): void
199+
{
200+
$key1 = new CipherKey(
201+
'key-1',
202+
'subject-1',
203+
'secret-1',
204+
'aes-256-gcm',
205+
new DateTimeImmutable(),
206+
);
207+
208+
$key2 = new CipherKey(
209+
'key-1',
210+
'subject-1',
211+
'secret-2',
212+
'aes-256-gcm',
213+
new DateTimeImmutable(),
214+
);
215+
216+
$store = new InMemoryCipherKeyStore();
217+
$store->store($key1);
218+
$store->store($key2);
219+
220+
self::assertSame($key2, $store->get('key-1'));
221+
self::assertSame($key2, $store->currentKeyFor('subject-1'));
222+
}
223+
224+
public function testMultipleSubjects(): void
225+
{
226+
$key1 = new CipherKey(
227+
'key-1',
228+
'subject-1',
229+
'secret-1',
230+
'aes-256-gcm',
231+
new DateTimeImmutable(),
232+
);
233+
234+
$key2 = new CipherKey(
235+
'key-2',
236+
'subject-2',
237+
'secret-2',
238+
'aes-256-gcm',
239+
new DateTimeImmutable(),
240+
);
241+
242+
$store = new InMemoryCipherKeyStore();
243+
$store->store($key1);
244+
$store->store($key2);
245+
246+
self::assertSame($key1, $store->currentKeyFor('subject-1'));
247+
self::assertSame($key2, $store->currentKeyFor('subject-2'));
83248
}
84249
}

0 commit comments

Comments
 (0)