Skip to content

Commit 971b0e3

Browse files
joshtrichardsjoshtrichards
authored
test(Session): refactor and expand CryptoSessionDataTest tests
Signed-off-by: Josh <[email protected]>
1 parent 3294b9c commit 971b0e3

File tree

1 file changed

+108
-64
lines changed

1 file changed

+108
-64
lines changed

‎tests/lib/Session/CryptoSessionDataTest.php‎

Lines changed: 108 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -13,102 +13,159 @@
1313
use OCP\ISession;
1414
use OCP\Security\ICrypto;
1515
use OCP\Security\ISecureRandom;
16+
use PHPUnit\Framework\Attributes\CoversClass;
17+
use PHPUnit\Framework\Attributes\DataProvider;
18+
use PHPUnit\Framework\Attributes\UsesClass;
1619
use PHPUnit\Framework\MockObject\MockObject;
1720

1821
/**
19-
* Test case for OC\Session\CryptoSessionData using in-memory session storage.
20-
* Reuses session contract tests but verifies they hold with encrypted storage
21-
* (i.e., session values are encrypted/decrypted transparently).
22+
* Unit tests for CryptoSessionData, verifying encrypted session storage,
23+
* tamper resistance, passphrase boundaries, and round-trip data integrity.
24+
* Covers edge cases and crypto-specific behaviors beyond the base session contract.
25+
*
26+
* Note: ISession API conformity/contract tests are inherited from the parent
27+
* (Test\Session\Session). Only crypto-specific (and pre-wrapper) additions are
28+
* defined here.
2229
*/
30+
#[CoversClass(CryptoSessionData::class)]
31+
#[UsesClass(Memory::class)]
2332
class CryptoSessionDataTest extends Session {
33+
private const DUMMY_PASSPHRASE = 'dummyPassphrase';
34+
private const TAMPERED_BLOB = 'garbage-data';
35+
private const MALFORMED_JSON_BLOB = '{not:valid:json}';
36+
2437
protected ICrypto|MockObject $crypto;
2538
protected ISession $session;
2639

2740
protected function setUp(): void {
2841
parent::setUp();
2942

30-
$this->session = new Memory();
31-
3243
$this->crypto = $this->createMock(ICrypto::class);
33-
$this->crypto->method('encrypt')
34-
->willReturnCallback(fn($input) =>
35-
'#' . $input . '#');
36-
$this->crypto->method('decrypt')
37-
->willReturnCallback(fn($input) =>
38-
($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1));
39-
40-
$this->instance = new CryptoSessionData($this->session, $this->crypto, 'PASS');
41-
}
4244

43-
/* Basic API conformity/contract tests are in parent class; these are crypto specific pre-wrapper additions */
45+
$this->crypto->method('encrypt')->willReturnCallback(
46+
fn($input) => '#' . $input . '#'
47+
);
48+
$this->crypto->method('decrypt')->willReturnCallback(
49+
fn($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1)
50+
);
51+
52+
$this->session = new Memory();
53+
$this->instance = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE);
54+
}
4455

56+
/**
57+
* Ensure backend never stores plaintext at-rest.
58+
*/
4559
public function testSessionDataStoredEncrypted(): void {
4660
$keyName = 'secret';
4761
$unencryptedValue = 'superSecretValue123';
4862

49-
$this->instance->set('secret', 'superSecretValue123');
63+
$this->instance->set($keyName, $unencryptedValue);
5064
$this->instance->close();
5165

5266
$unencryptedSessionDataJson = json_encode(["$keyName" => "$unencryptedValue"]);
53-
$expectedEncryptedSessionDataBlob = $this->crypto->encrypt($unencryptedSessionDataJson, 'PASS');
67+
$expectedEncryptedSessionDataBlob = $this->crypto->encrypt($unencryptedSessionDataJson, self::DUMMY_PASSPHRASE);
5468

55-
// Retrieve the CryptoSessionData blob directly from lower level session layer to guarantee bypass of crypto layer
69+
// Retrieve the CryptoSessionData blob directly from lower level session layer to bypass crypto decryption layer
5670
$encryptedSessionDataBlob = $this->session->get('encrypted_session_data'); // should contain raw encrypted blob not the decrypted data
5771
// Definitely encrypted?
58-
$this->assertStringStartsWith('#', $encryptedSessionDataBlob); // Must match mocked crypto->encrypt()
72+
$this->assertStringStartsWith('#', $encryptedSessionDataBlob); // Must match stubbed crypto->encrypt()
5973
$this->assertStringEndsWith('#', $encryptedSessionDataBlob); // ditto
60-
$this->assertFalse($expectedEncryptedSessionDataBlob === $unencryptedSessionDataJson);
61-
// Expected before/after?
74+
$this->assertNotSame($unencryptedSessionDataJson, $expectedEncryptedSessionDataBlob);
6275
$this->assertSame($expectedEncryptedSessionDataBlob, $encryptedSessionDataBlob);
6376
}
6477

65-
public function testLargeAndUnicodeValuesRoundTrip() {
66-
$unicodeValue = "héllo 🌍";
67-
$largeValue = str_repeat('x', 4096);
68-
$this->instance->set('unicode', $unicodeValue);
69-
$this->instance->set('big', $largeValue);
78+
/**
79+
* Ensure various key/value types are storable/retrievable
80+
*/
81+
#[DataProvider('roundTripValuesProvider')]
82+
public function testRoundTripValue($key, $value): void {
83+
$this->instance->set($key, $value);
7084
$this->instance->close();
71-
// Simulate reload
72-
$instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS');
73-
$this->assertSame($unicodeValue, $instance2->get('unicode'));
74-
$this->assertSame($largeValue, $instance2->get('big'));
85+
// Simulate reload
86+
$instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE);
87+
$this->assertSame($value, $instance2->get($key));
7588
}
7689

77-
public function testLargeArrayRoundTrip() {
78-
$bigArray = [];
79-
for ($i = 0; $i < 1000; $i++) {
80-
$bigArray["key$i"] = "val$i";
81-
}
82-
$this->instance->set('thousand', json_encode($bigArray));
83-
$this->instance->close();
90+
public static function roundTripValuesProvider(): array {
91+
return [
92+
'simple string' => ['foo', 'bar'],
93+
'unicode value' => ['uni', "héllo 🌍"],
94+
'large value' => ['big', str_repeat('x', 4096)],
95+
'large array' => ['thousand', json_encode(self::makeLargeArray())],
96+
'empty string' => ['', ''],
97+
];
98+
}
8499

85-
$instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS');
86-
$this->assertSame(json_encode($bigArray), $instance2->get('thousand'));
100+
/* Helper */
101+
private static function makeLargeArray(int $size = 1000): array {
102+
$result = [];
103+
for ($i = 0; $i < $size; $i++) {
104+
$result["key$i"] = "val$i";
105+
}
106+
return $result;
87107
}
88108

89-
public function testRemovedValueIsGoneAfterClose() {
109+
/**
110+
* Ensure removed values are not accessible after flush/reload.
111+
*/
112+
public function testRemovedValueIsGoneAfterClose(): void {
90113
$this->instance->set('temp', 'gone soon');
91114
$this->instance->remove('temp');
92115
$this->instance->close();
93116

94-
$instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS');
117+
$instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE);
95118
$this->assertNull($instance2->get('temp'));
96119
}
97120

98-
public function testTamperedBlobReturnsNull() {
121+
/**
122+
* Ensure tampering is handled robustly.
123+
*/
124+
public function testTamperedBlobReturnsNull(): void {
99125
$this->instance->set('foo', 'bar');
100126
$this->instance->close();
101-
// Tamper the lower level blob
102-
$this->session->set('encrypted_session_data', 'garbage-data');
127+
// Bypass crypto layer and tamper the lower level blob
128+
$this->session->set('encrypted_session_data', self::TAMPERED_BLOB);
103129

104-
$instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS');
130+
$instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE);
105131
$this->assertNull($instance2->get('foo'));
106132
$this->assertNull($instance2->get('notfoo'));
107133
}
108134

109-
public function testWrongPassphraseGivesNoAccess() {
135+
/**
136+
* Ensure malformed JSON is handled robustly.
137+
*/
138+
public function testMalformedJsonBlobReturnsNull(): void {
139+
$this->instance->set('foo', 'bar');
140+
$this->instance->close();
141+
$this->session->set('encrypted_session_data', '#' . self::MALFORMED_JSON_BLOB . '#');
142+
$instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE);
143+
$this->assertNull($instance2->get('foo'));
144+
}
145+
146+
/**
147+
* Ensure an invalid passphrase is handled appropriately.
148+
*/
149+
public function testWrongPassphraseGivesNoAccess(): void {
110150
// Override ICrypto mock/stubs for this test only
151+
$crypto = $this->createPassphraseAwareCryptoMock();
152+
153+
// Override main instance with local ISession and local ICrypto mock/stubs
154+
$session = new Memory();
155+
$instance = new CryptoSessionData($session, $crypto, self::DUMMY_PASSPHRASE);
156+
157+
$instance->set('secure', 'yes');
158+
$instance->close();
159+
160+
$instance2 = new CryptoSessionData($session, $crypto, 'NOT_THE_DUMMY_PASSPHRASE');
161+
$this->assertNull($instance2->get('secure'));
162+
$this->assertFalse($instance2->exists('secure'));
163+
}
164+
165+
/* Helper */
166+
private function createPassphraseAwareCryptoMock(): ICrypto {
111167
$crypto = $this->createMock(ICrypto::class);
168+
112169
$crypto->method('encrypt')->willReturnCallback(function($plain, $passphrase = null) {
113170
// Set up: store a value with the passphrase embedded (fake encryption)
114171
return $passphrase . '#' . $plain . '#' . $passphrase;
@@ -123,26 +180,13 @@ public function testWrongPassphraseGivesNoAccess() {
123180
return '';
124181
});
125182

126-
// Override main instance with local ISession and local ICrypto mock/stubs
127-
$session = new Memory();
128-
$instance = new CryptoSessionData($session, $crypto, 'PASS');
129-
130-
$instance->set('secure', 'yes');
131-
$instance->close();
132-
133-
$instance2 = new CryptoSessionData($session, $crypto, 'DIFFERENT');
134-
$this->assertNull($instance2->get('secure'));
135-
$this->assertFalse($instance2->exists('secure'));
136-
}
137-
138-
public function testEmptyKeyValue() {
139-
$this->instance->set('', '');
140-
$this->instance->close();
141-
$instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS');
142-
$this->assertSame('', $instance2->get(''));
183+
return $crypto;
143184
}
144185

145-
public function testDoubleCloseDoesNotCorrupt() {
186+
/**
187+
* Ensure closes are idempotent and safe.
188+
*/
189+
public function testDoubleCloseDoesNotCorrupt(): void {
146190
$this->instance->set('safe', 'value');
147191
$this->instance->close();
148192
$blobBefore = $this->session->get('encrypted_session_data');

0 commit comments

Comments
 (0)