From 87c21e2c4bf6d16858801712f8ad47e1131a0844 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 25 Jan 2026 10:29:24 -0500 Subject: [PATCH 01/19] test(Session): Add class docblock for Test\Session\Session Signed-off-by: Josh --- tests/lib/Session/Session.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/lib/Session/Session.php b/tests/lib/Session/Session.php index 1e2e9370825..b9c10234695 100644 --- a/tests/lib/Session/Session.php +++ b/tests/lib/Session/Session.php @@ -8,6 +8,11 @@ namespace Test\Session; +/** + * Abstract base test class defining the contract for session storage backends. + * Contains generic tests for set/get/remove/clear/array session API compliance. + * Extend this class to provide $this->instance and validate custom session implementations. + */ abstract class Session extends \Test\TestCase { /** * @var \OC\Session\Session From 028fbb29b22eb9bf1e2e41291f10739e70395096 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 25 Jan 2026 10:31:18 -0500 Subject: [PATCH 02/19] test(Session): Add class docblock for Test\Session\MemoryTest Signed-off-by: Josh --- tests/lib/Session/MemoryTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/lib/Session/MemoryTest.php b/tests/lib/Session/MemoryTest.php index 40977f1f3fb..daa79bb1c85 100644 --- a/tests/lib/Session/MemoryTest.php +++ b/tests/lib/Session/MemoryTest.php @@ -11,6 +11,10 @@ namespace Test\Session; use OC\Session\Memory; use OCP\Session\Exceptions\SessionNotAvailableException; +/** + * Concrete test case for OC\Session\Memory (in-memory session storage). + * Reuses session contract tests and adds in-memory specific assertions. + */ class MemoryTest extends Session { protected function setUp(): void { parent::setUp(); From 5223cc1003552c60fdf37f79c7c643987929ab9f Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 25 Jan 2026 10:40:15 -0500 Subject: [PATCH 03/19] test(Session): Add class docblock for Test\Session\CryptoSessionDataTest Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index a30a3297094..f2f3c0c6192 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -13,6 +13,11 @@ use OC\Session\Memory; use OCP\ISession; use OCP\Security\ICrypto; +/** + * Test case for OC\Session\CryptoSessionData using in-memory session storage. + * Reuses session contract tests but verifies they hold with encrypted storage + * (i.e., session values are encrypted/decrypted transparently). + */ class CryptoSessionDataTest extends Session { /** @var \PHPUnit\Framework\MockObject\MockObject|ICrypto */ protected $crypto; From ecf3269e47deb04741b6263feb813b5f4893eac7 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 25 Jan 2026 11:43:06 -0500 Subject: [PATCH 04/19] test(Session): Add class docblock for Test\Session\CryptoWrappingTest Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index 2bc09e0903a..b466b5647c0 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -13,6 +13,12 @@ use OCP\ISession; use OCP\Security\ICrypto; use Test\TestCase; +/** + * Test cases for the internal logic of OC\Session\CryptoSessionData. + * Focuses on correct encryption/decryption of session data and wrapping behavior. + * + * TODO: Should really be testing CryptoWrapper! + */ class CryptoWrappingTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|ICrypto */ protected $crypto; From fc79cf017d1be673e444698307e53a1769b1e5d1 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 25 Jan 2026 11:58:12 -0500 Subject: [PATCH 05/19] test(Session): test actual CryptoSessionData wrapper not mock The test as currently written does not test the actual CryptoSessionData logic. Instead it just calls and checks the mocks return value. It's also not setting and checking that correctly anyhow. This change makes the existing test confirm the wrapper sets/retrieves data from the encrypted session wrapper/handler. This just fixes the existing test, but CryptoWrappingTest should probably further refined to focus exclusively on testing CryptoWrapper itself. Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 59 ++++++++++-------------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index b466b5647c0..abde87cd503 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -9,8 +9,10 @@ namespace Test\Session; use OC\Session\CryptoSessionData; +use OC\Session\Memory; use OCP\ISession; use OCP\Security\ICrypto; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; /** @@ -20,52 +22,39 @@ use Test\TestCase; * TODO: Should really be testing CryptoWrapper! */ class CryptoWrappingTest extends TestCase { - /** @var \PHPUnit\Framework\MockObject\MockObject|ICrypto */ - protected $crypto; - - /** @var \PHPUnit\Framework\MockObject\MockObject|ISession */ - protected $wrappedSession; - - /** @var \OC\Session\CryptoSessionData */ - protected $instance; + protected ICrypto|MockObject $crypto; + protected ISession|MockObject $session; + protected CryptoSessionData $instance; + + protected string $passphrase = 'PASS'; protected function setUp(): void { parent::setUp(); - $this->wrappedSession = $this->getMockBuilder(ISession::class) - ->disableOriginalConstructor() - ->getMock(); - $this->crypto = $this->getMockBuilder('OCP\Security\ICrypto') - ->disableOriginalConstructor() - ->getMock(); - $this->crypto->expects($this->any()) - ->method('encrypt') - ->willReturnCallback(function ($input) { - return $input; - }); - $this->crypto->expects($this->any()) - ->method('decrypt') - ->willReturnCallback(function ($input) { - if ($input === '') { - return ''; - } - return substr($input, 1, -1); - }); + $this->session = new Memory(); + + $this->crypto = $this->createMock(ICrypto::class); + + $this->crypto->method('encrypt') + ->willReturnCallback(fn($input) => + '#' . $input . '#'); - $this->instance = new CryptoSessionData($this->wrappedSession, $this->crypto, 'PASS'); + $this->crypto->method('decrypt') + ->willReturnCallback(fn($input) => + ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1)); + + // Encrypted session handler under test + $this->instance = new CryptoSessionData($this->session, $this->crypto, $this->passphrase); } public function testUnwrappingGet(): void { + $keyName = 'someKey'; $unencryptedValue = 'foobar'; $encryptedValue = $this->crypto->encrypt($unencryptedValue); - $this->wrappedSession->expects($this->once()) - ->method('get') - ->with('encrypted_session_data') - ->willReturnCallback(function () use ($encryptedValue) { - return $encryptedValue; - }); + $this->instance->set($keyName, $unencryptedValue); - $this->assertSame($unencryptedValue, $this->wrappedSession->get('encrypted_session_data')); + $this->assertTrue($this->instance->exists($keyName)); + $this->assertSame($unencryptedValue, $this->instance->get($keyName)); } } From 184b9f43836018033781e64e780dd4fdf90ded0c Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 25 Jan 2026 12:44:07 -0500 Subject: [PATCH 06/19] test(Session): check that blob is encrypted in CryptoWrappingTest Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index abde87cd503..76c5c4cf4da 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -56,5 +56,13 @@ class CryptoWrappingTest extends TestCase { $this->assertTrue($this->instance->exists($keyName)); $this->assertSame($unencryptedValue, $this->instance->get($keyName)); + + // Trigger flush so encrypted_session_data blob gets written out + $this->instance->close(); // or unset($this->instance) + + // Confirm data is truly encrypted + $encryptedSessionDataBlob = $this->session->get('encrypted_session_data'); // should contain raw encrypted blob not the decrypted data + $expectedEncryptedSessionDataBlob = $this->crypto->encrypt(json_encode(["$keyName" => "$unencryptedValue"]), $this->passphrase); + $this->assertSame($expectedEncryptedSessionDataBlob, $encryptedSessionDataBlob); } } From 151b90f4f0751121b29ba55b5679ee9e4e705065 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 25 Jan 2026 17:31:31 -0500 Subject: [PATCH 07/19] test(Session): Refactor CryptoWrappingTest to focus on wrapper - Test CryptoWrapper for real (and not CryptoSessionData which has its own dedicated test class). - Cover various wrapper instantiation scenarios Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 98 ++++++++++++++++++------ 1 file changed, 73 insertions(+), 25 deletions(-) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index 76c5c4cf4da..77245367667 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -8,10 +8,12 @@ namespace Test\Session; +use OC\Session\CryptoWrapper; use OC\Session\CryptoSessionData; use OC\Session\Memory; -use OCP\ISession; +use OCP\IRequest; use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -23,46 +25,92 @@ use Test\TestCase; */ class CryptoWrappingTest extends TestCase { protected ICrypto|MockObject $crypto; - protected ISession|MockObject $session; - protected CryptoSessionData $instance; - - protected string $passphrase = 'PASS'; + protected ISecureRandom|MockObject $random; + protected IRequest|MockObject $request; protected function setUp(): void { parent::setUp(); - - $this->session = new Memory(); $this->crypto = $this->createMock(ICrypto::class); + $this->random = $this->createMock(ISecureRandom::class); + $this->request = $this->createMock(IRequest::class); + } + + public function testWrapSessionReturnsCryptoSessionData(): void { + $this->random->method('generate')->willReturn(str_repeat('Q', 128)); + $this->request->method('getCookie')->willReturn(null); + $this->request->method('getServerProtocol')->willReturn('https'); + + $session = new Memory(); + + $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); + $wrappedSession = $cryptoWrapper->wrapSession($session); + + $this->assertInstanceOf(CryptoSessionData::class, $wrappedSession); + } + + public function testWrapSessionDoesNotDoubleWrap(): void { + $alreadyWrapped = $this->createMock(CryptoSessionData::class); + + $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); + $wrappedSession = $cryptoWrapper->wrapSession($alreadyWrapped); + + $this->assertSame($alreadyWrapped, $wrappedSession); + } + + public function testPassphraseGeneratedIfNoCookie(): void { + $expectedPassphrase = str_repeat('z', 128); + $this->random->expects($this->once())->method('generate')->with(128)->willReturn($expectedPassphrase); + $this->request->method('getCookie')->willReturn(null); + $this->request->method('getServerProtocol')->willReturn('https'); + + $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); + $ref = new \ReflectionProperty($cryptoWrapper, 'passphrase'); + $ref->setAccessible(true); + $this->assertSame($expectedPassphrase, $ref->getValue($cryptoWrapper)); + } + + public function testPassphraseReusedIfCookiePresent(): void { + $cookieVal = 'pass_from_cookie'; + $this->request->method('getCookie')->willReturn($cookieVal); + $this->random->expects($this->never())->method('generate'); + $this->request->method('getServerProtocol')->willReturn('https'); + + $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); + $ref = new \ReflectionProperty($cryptoWrapper, 'passphrase'); + $ref->setAccessible(true); + $this->assertSame($cookieVal, $ref->getValue($cryptoWrapper)); + } + + public function testIntegrationWrapSetAndGet(): void { + $keyName = 'someKey'; + $unencryptedValue = 'foobar'; + $encryptedValue = $this->crypto->encrypt($unencryptedValue); $this->crypto->method('encrypt') ->willReturnCallback(fn($input) => '#' . $input . '#'); - $this->crypto->method('decrypt') ->willReturnCallback(fn($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1)); + $this->random->method('generate')->willReturn(str_repeat('C', 128)); + $this->request->method('getCookie')->willReturn(null); + $this->request->method('getServerProtocol')->willReturn('https'); - // Encrypted session handler under test - $this->instance = new CryptoSessionData($this->session, $this->crypto, $this->passphrase); - } + $session = new Memory(); + $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); + $wrappedSession = $cryptoWrapper->wrapSession($session); + + $wrappedSession->set($keyName, $unencryptedValue); - public function testUnwrappingGet(): void { - $keyName = 'someKey'; - $unencryptedValue = 'foobar'; - $encryptedValue = $this->crypto->encrypt($unencryptedValue); + $this->assertTrue($wrappedSession->exists($keyName)); + $this->assertSame($unencryptedValue, $wrappedSession->get($keyName)); - $this->instance->set($keyName, $unencryptedValue); + // Encrypted storage check + $wrappedSession->close(); // trigger flush so blob gets written out - $this->assertTrue($this->instance->exists($keyName)); - $this->assertSame($unencryptedValue, $this->instance->get($keyName)); - - // Trigger flush so encrypted_session_data blob gets written out - $this->instance->close(); // or unset($this->instance) - - // Confirm data is truly encrypted - $encryptedSessionDataBlob = $this->session->get('encrypted_session_data'); // should contain raw encrypted blob not the decrypted data - $expectedEncryptedSessionDataBlob = $this->crypto->encrypt(json_encode(["$keyName" => "$unencryptedValue"]), $this->passphrase); + $encryptedSessionDataBlob = $session->get('encrypted_session_data'); // should contain raw encrypted blob not the decrypted data + $expectedEncryptedSessionDataBlob = $this->crypto->encrypt(json_encode(["$keyName" => "$unencryptedValue"]), $this->random->generate(128)); $this->assertSame($expectedEncryptedSessionDataBlob, $encryptedSessionDataBlob); } } From 80fe8b0cebd4db2fe60b58fb6b7628127cffa693 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 26 Jan 2026 22:38:52 -0500 Subject: [PATCH 08/19] test(Session): additional crypto-specific sessionData tests Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 147 +++++++++++++++++--- 1 file changed, 126 insertions(+), 21 deletions(-) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index f2f3c0c6192..162aa43785d 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -12,6 +12,8 @@ use OC\Session\CryptoSessionData; use OC\Session\Memory; use OCP\ISession; use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; +use PHPUnit\Framework\MockObject\MockObject; /** * Test case for OC\Session\CryptoSessionData using in-memory session storage. @@ -19,31 +21,134 @@ use OCP\Security\ICrypto; * (i.e., session values are encrypted/decrypted transparently). */ class CryptoSessionDataTest extends Session { - /** @var \PHPUnit\Framework\MockObject\MockObject|ICrypto */ - protected $crypto; - - /** @var ISession */ - protected $wrappedSession; + protected ICrypto|MockObject $crypto; + protected ISession $session; protected function setUp(): void { parent::setUp(); - $this->wrappedSession = new Memory(); - $this->crypto = $this->createMock(ICrypto::class); - $this->crypto->expects($this->any()) - ->method('encrypt') - ->willReturnCallback(function ($input) { - return '#' . $input . '#'; - }); - $this->crypto->expects($this->any()) - ->method('decrypt') - ->willReturnCallback(function ($input) { - if ($input === '') { - return ''; - } - return substr($input, 1, -1); - }); + $this->session = new Memory(); - $this->instance = new CryptoSessionData($this->wrappedSession, $this->crypto, 'PASS'); + $this->crypto = $this->createMock(ICrypto::class); + $this->crypto->method('encrypt') + ->willReturnCallback(fn($input) => + '#' . $input . '#'); + $this->crypto->method('decrypt') + ->willReturnCallback(fn($input) => + ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1)); + + $this->instance = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + } + + /* Basic API conformity/contract tests are in parent class; these are crypto specific pre-wrapper additions */ + + public function testSessionDataStoredEncrypted(): void { + $keyName = 'secret'; + $unencryptedValue = 'superSecretValue123'; + $encryptedValue = $this->crypto->encrypt($unencryptedValue); + + $this->instance->set('secret', 'superSecretValue123'); + $this->instance->close(); + + $unencryptedSessionDataJson = json_encode(["$keyName" => "$unencryptedValue"]); + $expectedEncryptedSessionDataBlob = $this->crypto->encrypt($unencryptedSessionDataJson, 'PASS'); + + // Retrieve the CryptoSessionData blob directly from lower level session layer to guarantee bypass of crypto layer + $encryptedSessionDataBlob = $this->session->get('encrypted_session_data'); // should contain raw encrypted blob not the decrypted data + // Definitely encrypted? + $this->assertStringStartsWith('#', $encryptedSessionDataBlob); // Must match mocked crypto->encrypt() + $this->assertStringEndsWith('#', $encryptedSessionDataBlob); // ditto + $this->assertFalse($expectedEncryptedSessionDataBlob === $unencryptedSessionDataJson); + // Expected before/after? + $this->assertSame($expectedEncryptedSessionDataBlob, $encryptedSessionDataBlob); + } + + public function testLargeAndUnicodeValuesRoundTrip() { + $unicodeValue = "héllo 🌍"; + $largeValue = str_repeat('x', 4096); + $this->instance->set('unicode', $unicodeValue); + $this->instance->set('big', $largeValue); + $this->instance->close(); + // Simulate reload + $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $this->assertSame($unicodeValue, $instance2->get('unicode')); + $this->assertSame($largeValue, $instance2->get('big')); + } + + public function testLargeArrayRoundTrip() { + $bigArray = []; + for ($i = 0; $i < 1000; $i++) { + $bigArray["key$i"] = "val$i"; + } + $this->instance->set('thousand', json_encode($bigArray)); + $this->instance->close(); + + $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $this->assertSame(json_encode($bigArray), $instance2->get('thousand')); + } + + public function testRemovedValueIsGoneAfterClose() { + $this->instance->set('temp', 'gone soon'); + $this->instance->remove('temp'); + $this->instance->close(); + + $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $this->assertNull($instance2->get('temp')); + } + + public function testTamperedBlobReturnsNull() { + $this->instance->set('foo', 'bar'); + $this->instance->close(); + // Tamper the lower level blob + $this->session->set('encrypted_session_data', 'garbage-data'); + + $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $this->assertNull($instance2->get('foo')); + $this->assertNull($instance2->get('notfoo')); + } + + public function testWrongPassphraseGivesNoAccess() { + // Override ICrypto mock/stubs for this test only + $crypto = $this->createMock(ICrypto::class); + $crypto->method('encrypt')->willReturnCallback(function($plain, $passphrase = null) { + // Set up: store a value with the passphrase embedded (fake encryption) + return $passphrase . '#' . $plain . '#' . $passphrase; + }); + $crypto->method('decrypt')->willReturnCallback(function($input, $passphrase = null) { + // Only successfully decrypt if the embedded passphrase matches + if (strpos($input, $passphrase . '#') === 0 && strrpos($input, '#' . $passphrase) === strlen($input) - strlen('#' . $passphrase)) { + // Strip off passphrase markers and return the "decrypted" string + return substr($input, strlen($passphrase . '#'), -strlen('#' . $passphrase)); + } + // Fail to decrypt + return ''; + }); + + // Override main instance with local ISession and local ICrypto mock/stubs + $session = new Memory(); + $instance = new CryptoSessionData($session, $crypto, 'PASS'); + + $instance->set('secure', 'yes'); + $instance->close(); + + $instance2 = new CryptoSessionData($session, $crypto, 'DIFFERENT'); + $this->assertNull($instance2->get('secure')); + $this->assertFalse($instance2->exists('secure')); + } + + public function testEmptyKeyValue() { + $this->instance->set('', ''); + $this->instance->close(); + $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $this->assertSame('', $instance2->get('')); + } + + public function testDoubleCloseDoesNotCorrupt() { + $this->instance->set('safe', 'value'); + $this->instance->close(); + $blobBefore = $this->session->get('encrypted_session_data'); + $this->instance->close(); // Should do nothing harmful + $blobAfter = $this->session->get('encrypted_session_data'); + $this->assertSame($blobBefore, $blobAfter); } } From 41719f3e4e841cc2a8d4a76dadc34f8d3e87a41b Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 27 Jan 2026 10:37:50 -0500 Subject: [PATCH 09/19] test(Session): Refactor CryptoWrappingTest tests Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 100 +++++++++++++++++------ 1 file changed, 76 insertions(+), 24 deletions(-) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index 77245367667..961cf1de847 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -18,28 +18,42 @@ use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; /** - * Test cases for the internal logic of OC\Session\CryptoSessionData. - * Focuses on correct encryption/decryption of session data and wrapping behavior. + * Test cases for CryptoWrapper+CryptoSessionData wrapping logic. + * - Focuses on correct session wrapping. + * - Ensures passphrase handling (cookie, random generation). + * - Validates integration and robustness. * - * TODO: Should really be testing CryptoWrapper! + * @see Test\Session\CryptoSessionDataTest for crypto storage testing logic. */ +#[CoversClass(CryptoWrapper::class)] class CryptoWrappingTest extends TestCase { + private const DUMMY_PASSPHRASE = 'dummyPassphrase'; + private const COOKIE_PASSPHRASE = 'cookiePassphrase'; + private const GENERATED_PASSPHRASE = 'generatedPassphrase'; + private const SERVER_PROTOCOL = 'https'; + protected ICrypto|MockObject $crypto; protected ISecureRandom|MockObject $random; protected IRequest|MockObject $request; protected function setUp(): void { parent::setUp(); - + $this->crypto = $this->createMock(ICrypto::class); $this->random = $this->createMock(ISecureRandom::class); $this->request = $this->createMock(IRequest::class); } + /** + * Ensure wrapSession returns a CryptoSessionData when passed a basic session. + */ + #[CoversMethod(CryptoWrapper::class, 'wrapSession')] public function testWrapSessionReturnsCryptoSessionData(): void { - $this->random->method('generate')->willReturn(str_repeat('Q', 128)); + $generatedPassphrase128 = str_pad(self::GENERATED_PASSPHRASE, 128, '_' . __FUNCTION__, STR_PAD_RIGHT); + $this->random->method('generate')->willReturn($generatedPassphrase128); + $this->request->method('getCookie')->willReturn(null); - $this->request->method('getServerProtocol')->willReturn('https'); + $this->request->method('getServerProtocol')->willReturn(self::SERVER_PROTOCOL); $session = new Memory(); @@ -49,6 +63,10 @@ class CryptoWrappingTest extends TestCase { $this->assertInstanceOf(CryptoSessionData::class, $wrappedSession); } + /** + * Ensure wrapSession returns the same instance if already wrapped. + */ + #[CoversMethod(CryptoWrapper::class, 'wrapSession')] public function testWrapSessionDoesNotDoubleWrap(): void { $alreadyWrapped = $this->createMock(CryptoSessionData::class); @@ -58,59 +76,93 @@ class CryptoWrappingTest extends TestCase { $this->assertSame($alreadyWrapped, $wrappedSession); } + /** + * Ensure a passphrase is generated and stored if no cookie is present. + */ + #[CoversMethod(CryptoWrapper::class, '__construct__')] public function testPassphraseGeneratedIfNoCookie(): void { - $expectedPassphrase = str_repeat('z', 128); + $expectedPassphrase = str_pad(self::GENERATED_PASSPHRASE, 128, '_' . __FUNCTION__, STR_PAD_RIGHT); $this->random->expects($this->once())->method('generate')->with(128)->willReturn($expectedPassphrase); + $this->request->method('getCookie')->willReturn(null); - $this->request->method('getServerProtocol')->willReturn('https'); + $this->request->method('getServerProtocol')->willReturn(self::SERVER_PROTOCOL); $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); $ref = new \ReflectionProperty($cryptoWrapper, 'passphrase'); $ref->setAccessible(true); + + $this->assertTrue($ref->getValue($cryptoWrapper) !== null); $this->assertSame($expectedPassphrase, $ref->getValue($cryptoWrapper)); } + /** + * Ensure only the passphrase from cookie is used if present. + */ + #[CoversMethod(CryptoWrapper::class, '__construct__')] public function testPassphraseReusedIfCookiePresent(): void { - $cookieVal = 'pass_from_cookie'; + $cookieVal = self::COOKIE_PASSPHRASE; $this->request->method('getCookie')->willReturn($cookieVal); + $this->random->expects($this->never())->method('generate'); - $this->request->method('getServerProtocol')->willReturn('https'); + $this->request->method('getServerProtocol')->willReturn(self::SERVER_PROTOCOL); $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); $ref = new \ReflectionProperty($cryptoWrapper, 'passphrase'); $ref->setAccessible(true); + $this->assertSame($cookieVal, $ref->getValue($cryptoWrapper)); } + /** + * Ensure wrapSession throws if passed a non-ISession object (robustness). + */ + #[CoversMethod(CryptoWrapper::class, 'wrapSession')] + public function testWrapSessionThrowsTypeErrorOnInvalidInput(): void { + $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); + $this->expectException(\TypeError::class); + $cryptoWrapper->wrapSession(new \stdClass()); + } + + /** + * Full integration: wrap, set, get, flush, and encrypted blob. + */ + #[CoversMethod(CryptoWrapper::class, 'wrapSession')] + #[CoversClass(CryptoSessionData::class)] public function testIntegrationWrapSetAndGet(): void { $keyName = 'someKey'; $unencryptedValue = 'foobar'; - $encryptedValue = $this->crypto->encrypt($unencryptedValue); - - $this->crypto->method('encrypt') - ->willReturnCallback(fn($input) => - '#' . $input . '#'); - $this->crypto->method('decrypt') - ->willReturnCallback(fn($input) => - ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1)); - $this->random->method('generate')->willReturn(str_repeat('C', 128)); + $expectedPassphrase = str_pad(self::GENERATED_PASSPHRASE, 128, '_' . __FUNCTION__, STR_PAD_RIGHT); + + $this->crypto->method('encrypt')->willReturnCallback( + fn($input) => '#' . $input . '#' + ); + $this->crypto->method('decrypt')->willReturnCallback( + fn($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1) + ); + + $this->random->method('generate')->with(128)->willReturn($expectedPassphrase); $this->request->method('getCookie')->willReturn(null); - $this->request->method('getServerProtocol')->willReturn('https'); + $this->request->method('getServerProtocol')->willReturn(self::SERVER_PROTOCOL); $session = new Memory(); $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); $wrappedSession = $cryptoWrapper->wrapSession($session); $wrappedSession->set($keyName, $unencryptedValue); + $wrappedSession->close(); $this->assertTrue($wrappedSession->exists($keyName)); $this->assertSame($unencryptedValue, $wrappedSession->get($keyName)); - // Encrypted storage check - $wrappedSession->close(); // trigger flush so blob gets written out + $unencryptedSessionDataJson = json_encode(["$keyName" => "$unencryptedValue"]); + $expectedEncryptedSessionDataBlob = $this->crypto->encrypt($unencryptedSessionDataJson, $expectedPassphrase); - $encryptedSessionDataBlob = $session->get('encrypted_session_data'); // should contain raw encrypted blob not the decrypted data - $expectedEncryptedSessionDataBlob = $this->crypto->encrypt(json_encode(["$keyName" => "$unencryptedValue"]), $this->random->generate(128)); + // Retrieve the CryptoSessionData blob directly from lower level session layer to guarantee bypass of crypto layer + $encryptedSessionDataBlob = $session->get('encrypted_session_data'); + // Definitely encrypted? + $this->assertStringStartsWith('#', $encryptedSessionDataBlob); // Must match mocked crypto->encrypt() + $this->assertStringEndsWith('#', $encryptedSessionDataBlob); // ditto + $this->assertFalse($expectedEncryptedSessionDataBlob === $unencryptedSessionDataJson); $this->assertSame($expectedEncryptedSessionDataBlob, $encryptedSessionDataBlob); } } From 4feb4ca7eae07fa1ffb8d40c073c692473375099 Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 27 Jan 2026 10:38:07 -0500 Subject: [PATCH 10/19] test(Session): Drop unused variable Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index 162aa43785d..dbe83edbede 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -45,7 +45,6 @@ class CryptoSessionDataTest extends Session { public function testSessionDataStoredEncrypted(): void { $keyName = 'secret'; $unencryptedValue = 'superSecretValue123'; - $encryptedValue = $this->crypto->encrypt($unencryptedValue); $this->instance->set('secret', 'superSecretValue123'); $this->instance->close(); From 3294b9c6fc46187f2dde9be1540336c2e2a33c54 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 28 Jan 2026 10:02:42 -0500 Subject: [PATCH 11/19] test(Session): adjust CryptoWrappingTest attributes and docbock Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index 961cf1de847..20c7d2027c8 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -14,18 +14,25 @@ use OC\Session\Memory; use OCP\IRequest; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; /** - * Test cases for CryptoWrapper+CryptoSessionData wrapping logic. - * - Focuses on correct session wrapping. - * - Ensures passphrase handling (cookie, random generation). - * - Validates integration and robustness. + * Unit tests for CryptoWrapper, focusing on session wrapping logic, + * passphrase handling (cookie and generation), and integration with + * CryptoSessionData. Ensures robust construction and non-duplication + * of crypto-wrapped sessions. * - * @see Test\Session\CryptoSessionDataTest for crypto storage testing logic. + * Only wrapper-specific crypto behavior is tested here; + * core session encryption contract is covered in CryptoSessionDataTest. + * + * @see Test\Session\CryptoSessionDataTest For crypto storage testing logic. */ #[CoversClass(CryptoWrapper::class)] +#[UsesClass(Memory::class)] +#[UsesClass(CryptoSessionData::class)] class CryptoWrappingTest extends TestCase { private const DUMMY_PASSPHRASE = 'dummyPassphrase'; private const COOKIE_PASSPHRASE = 'cookiePassphrase'; @@ -47,7 +54,6 @@ class CryptoWrappingTest extends TestCase { /** * Ensure wrapSession returns a CryptoSessionData when passed a basic session. */ - #[CoversMethod(CryptoWrapper::class, 'wrapSession')] public function testWrapSessionReturnsCryptoSessionData(): void { $generatedPassphrase128 = str_pad(self::GENERATED_PASSPHRASE, 128, '_' . __FUNCTION__, STR_PAD_RIGHT); $this->random->method('generate')->willReturn($generatedPassphrase128); @@ -66,7 +72,6 @@ class CryptoWrappingTest extends TestCase { /** * Ensure wrapSession returns the same instance if already wrapped. */ - #[CoversMethod(CryptoWrapper::class, 'wrapSession')] public function testWrapSessionDoesNotDoubleWrap(): void { $alreadyWrapped = $this->createMock(CryptoSessionData::class); @@ -79,7 +84,6 @@ class CryptoWrappingTest extends TestCase { /** * Ensure a passphrase is generated and stored if no cookie is present. */ - #[CoversMethod(CryptoWrapper::class, '__construct__')] public function testPassphraseGeneratedIfNoCookie(): void { $expectedPassphrase = str_pad(self::GENERATED_PASSPHRASE, 128, '_' . __FUNCTION__, STR_PAD_RIGHT); $this->random->expects($this->once())->method('generate')->with(128)->willReturn($expectedPassphrase); @@ -98,7 +102,6 @@ class CryptoWrappingTest extends TestCase { /** * Ensure only the passphrase from cookie is used if present. */ - #[CoversMethod(CryptoWrapper::class, '__construct__')] public function testPassphraseReusedIfCookiePresent(): void { $cookieVal = self::COOKIE_PASSPHRASE; $this->request->method('getCookie')->willReturn($cookieVal); @@ -116,7 +119,6 @@ class CryptoWrappingTest extends TestCase { /** * Ensure wrapSession throws if passed a non-ISession object (robustness). */ - #[CoversMethod(CryptoWrapper::class, 'wrapSession')] public function testWrapSessionThrowsTypeErrorOnInvalidInput(): void { $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); $this->expectException(\TypeError::class); @@ -126,8 +128,6 @@ class CryptoWrappingTest extends TestCase { /** * Full integration: wrap, set, get, flush, and encrypted blob. */ - #[CoversMethod(CryptoWrapper::class, 'wrapSession')] - #[CoversClass(CryptoSessionData::class)] public function testIntegrationWrapSetAndGet(): void { $keyName = 'someKey'; $unencryptedValue = 'foobar'; From 971b0e32d5556fb103d266ea93458dc66a2ad9f0 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 28 Jan 2026 10:03:15 -0500 Subject: [PATCH 12/19] test(Session): refactor and expand CryptoSessionDataTest tests Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 170 ++++++++++++-------- 1 file changed, 107 insertions(+), 63 deletions(-) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index dbe83edbede..cab80796285 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -13,102 +13,159 @@ use OC\Session\Memory; use OCP\ISession; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\MockObject\MockObject; /** - * Test case for OC\Session\CryptoSessionData using in-memory session storage. - * Reuses session contract tests but verifies they hold with encrypted storage - * (i.e., session values are encrypted/decrypted transparently). + * Unit tests for CryptoSessionData, verifying encrypted session storage, + * tamper resistance, passphrase boundaries, and round-trip data integrity. + * Covers edge cases and crypto-specific behaviors beyond the base session contract. + * + * Note: ISession API conformity/contract tests are inherited from the parent + * (Test\Session\Session). Only crypto-specific (and pre-wrapper) additions are + * defined here. */ +#[CoversClass(CryptoSessionData::class)] +#[UsesClass(Memory::class)] class CryptoSessionDataTest extends Session { + private const DUMMY_PASSPHRASE = 'dummyPassphrase'; + private const TAMPERED_BLOB = 'garbage-data'; + private const MALFORMED_JSON_BLOB = '{not:valid:json}'; + protected ICrypto|MockObject $crypto; protected ISession $session; protected function setUp(): void { parent::setUp(); - $this->session = new Memory(); - $this->crypto = $this->createMock(ICrypto::class); - $this->crypto->method('encrypt') - ->willReturnCallback(fn($input) => - '#' . $input . '#'); - $this->crypto->method('decrypt') - ->willReturnCallback(fn($input) => - ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1)); - $this->instance = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $this->crypto->method('encrypt')->willReturnCallback( + fn($input) => '#' . $input . '#' + ); + $this->crypto->method('decrypt')->willReturnCallback( + fn($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1) + ); + + $this->session = new Memory(); + $this->instance = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE); } - /* Basic API conformity/contract tests are in parent class; these are crypto specific pre-wrapper additions */ - + /** + * Ensure backend never stores plaintext at-rest. + */ public function testSessionDataStoredEncrypted(): void { $keyName = 'secret'; $unencryptedValue = 'superSecretValue123'; - $this->instance->set('secret', 'superSecretValue123'); + $this->instance->set($keyName, $unencryptedValue); $this->instance->close(); $unencryptedSessionDataJson = json_encode(["$keyName" => "$unencryptedValue"]); - $expectedEncryptedSessionDataBlob = $this->crypto->encrypt($unencryptedSessionDataJson, 'PASS'); + $expectedEncryptedSessionDataBlob = $this->crypto->encrypt($unencryptedSessionDataJson, self::DUMMY_PASSPHRASE); - // Retrieve the CryptoSessionData blob directly from lower level session layer to guarantee bypass of crypto layer + // Retrieve the CryptoSessionData blob directly from lower level session layer to bypass crypto decryption layer $encryptedSessionDataBlob = $this->session->get('encrypted_session_data'); // should contain raw encrypted blob not the decrypted data // Definitely encrypted? - $this->assertStringStartsWith('#', $encryptedSessionDataBlob); // Must match mocked crypto->encrypt() + $this->assertStringStartsWith('#', $encryptedSessionDataBlob); // Must match stubbed crypto->encrypt() $this->assertStringEndsWith('#', $encryptedSessionDataBlob); // ditto - $this->assertFalse($expectedEncryptedSessionDataBlob === $unencryptedSessionDataJson); - // Expected before/after? + $this->assertNotSame($unencryptedSessionDataJson, $expectedEncryptedSessionDataBlob); $this->assertSame($expectedEncryptedSessionDataBlob, $encryptedSessionDataBlob); } - public function testLargeAndUnicodeValuesRoundTrip() { - $unicodeValue = "héllo 🌍"; - $largeValue = str_repeat('x', 4096); - $this->instance->set('unicode', $unicodeValue); - $this->instance->set('big', $largeValue); + /** + * Ensure various key/value types are storable/retrievable + */ + #[DataProvider('roundTripValuesProvider')] + public function testRoundTripValue($key, $value): void { + $this->instance->set($key, $value); $this->instance->close(); - // Simulate reload - $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); - $this->assertSame($unicodeValue, $instance2->get('unicode')); - $this->assertSame($largeValue, $instance2->get('big')); + // Simulate reload + $instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE); + $this->assertSame($value, $instance2->get($key)); } - public function testLargeArrayRoundTrip() { - $bigArray = []; - for ($i = 0; $i < 1000; $i++) { - $bigArray["key$i"] = "val$i"; + public static function roundTripValuesProvider(): array { + return [ + 'simple string' => ['foo', 'bar'], + 'unicode value' => ['uni', "héllo 🌍"], + 'large value' => ['big', str_repeat('x', 4096)], + 'large array' => ['thousand', json_encode(self::makeLargeArray())], + 'empty string' => ['', ''], + ]; + } + + /* Helper */ + private static function makeLargeArray(int $size = 1000): array { + $result = []; + for ($i = 0; $i < $size; $i++) { + $result["key$i"] = "val$i"; } - $this->instance->set('thousand', json_encode($bigArray)); - $this->instance->close(); - - $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); - $this->assertSame(json_encode($bigArray), $instance2->get('thousand')); + return $result; } - public function testRemovedValueIsGoneAfterClose() { + /** + * Ensure removed values are not accessible after flush/reload. + */ + public function testRemovedValueIsGoneAfterClose(): void { $this->instance->set('temp', 'gone soon'); $this->instance->remove('temp'); $this->instance->close(); - $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE); $this->assertNull($instance2->get('temp')); } - public function testTamperedBlobReturnsNull() { + /** + * Ensure tampering is handled robustly. + */ + public function testTamperedBlobReturnsNull(): void { $this->instance->set('foo', 'bar'); $this->instance->close(); - // Tamper the lower level blob - $this->session->set('encrypted_session_data', 'garbage-data'); + // Bypass crypto layer and tamper the lower level blob + $this->session->set('encrypted_session_data', self::TAMPERED_BLOB); - $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); + $instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE); $this->assertNull($instance2->get('foo')); $this->assertNull($instance2->get('notfoo')); } - public function testWrongPassphraseGivesNoAccess() { + /** + * Ensure malformed JSON is handled robustly. + */ + public function testMalformedJsonBlobReturnsNull(): void { + $this->instance->set('foo', 'bar'); + $this->instance->close(); + $this->session->set('encrypted_session_data', '#' . self::MALFORMED_JSON_BLOB . '#'); + $instance2 = new CryptoSessionData($this->session, $this->crypto, self::DUMMY_PASSPHRASE); + $this->assertNull($instance2->get('foo')); + } + + /** + * Ensure an invalid passphrase is handled appropriately. + */ + public function testWrongPassphraseGivesNoAccess(): void { // Override ICrypto mock/stubs for this test only + $crypto = $this->createPassphraseAwareCryptoMock(); + + // Override main instance with local ISession and local ICrypto mock/stubs + $session = new Memory(); + $instance = new CryptoSessionData($session, $crypto, self::DUMMY_PASSPHRASE); + + $instance->set('secure', 'yes'); + $instance->close(); + + $instance2 = new CryptoSessionData($session, $crypto, 'NOT_THE_DUMMY_PASSPHRASE'); + $this->assertNull($instance2->get('secure')); + $this->assertFalse($instance2->exists('secure')); + } + + /* Helper */ + private function createPassphraseAwareCryptoMock(): ICrypto { $crypto = $this->createMock(ICrypto::class); + $crypto->method('encrypt')->willReturnCallback(function($plain, $passphrase = null) { // Set up: store a value with the passphrase embedded (fake encryption) return $passphrase . '#' . $plain . '#' . $passphrase; @@ -123,26 +180,13 @@ class CryptoSessionDataTest extends Session { return ''; }); - // Override main instance with local ISession and local ICrypto mock/stubs - $session = new Memory(); - $instance = new CryptoSessionData($session, $crypto, 'PASS'); - - $instance->set('secure', 'yes'); - $instance->close(); - - $instance2 = new CryptoSessionData($session, $crypto, 'DIFFERENT'); - $this->assertNull($instance2->get('secure')); - $this->assertFalse($instance2->exists('secure')); + return $crypto; } - public function testEmptyKeyValue() { - $this->instance->set('', ''); - $this->instance->close(); - $instance2 = new CryptoSessionData($this->session, $this->crypto, 'PASS'); - $this->assertSame('', $instance2->get('')); - } - - public function testDoubleCloseDoesNotCorrupt() { + /** + * Ensure closes are idempotent and safe. + */ + public function testDoubleCloseDoesNotCorrupt(): void { $this->instance->set('safe', 'value'); $this->instance->close(); $blobBefore = $this->session->get('encrypted_session_data'); From f8756b1ef4d471d33aaeb63b76e6eb2d6e23bf93 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 28 Jan 2026 10:35:31 -0500 Subject: [PATCH 13/19] test(Session): lint/rector cleanup of CryptoWrappingTest Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index 20c7d2027c8..06acfad5e47 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -8,8 +8,8 @@ namespace Test\Session; -use OC\Session\CryptoWrapper; use OC\Session\CryptoSessionData; +use OC\Session\CryptoWrapper; use OC\Session\Memory; use OCP\IRequest; use OCP\Security\ICrypto; @@ -38,7 +38,7 @@ class CryptoWrappingTest extends TestCase { private const COOKIE_PASSPHRASE = 'cookiePassphrase'; private const GENERATED_PASSPHRASE = 'generatedPassphrase'; private const SERVER_PROTOCOL = 'https'; - + protected ICrypto|MockObject $crypto; protected ISecureRandom|MockObject $random; protected IRequest|MockObject $request; @@ -134,10 +134,10 @@ class CryptoWrappingTest extends TestCase { $expectedPassphrase = str_pad(self::GENERATED_PASSPHRASE, 128, '_' . __FUNCTION__, STR_PAD_RIGHT); $this->crypto->method('encrypt')->willReturnCallback( - fn($input) => '#' . $input . '#' + fn ($input) => '#' . $input . '#' ); $this->crypto->method('decrypt')->willReturnCallback( - fn($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1) + fn ($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1) ); $this->random->method('generate')->with(128)->willReturn($expectedPassphrase); @@ -145,9 +145,9 @@ class CryptoWrappingTest extends TestCase { $this->request->method('getServerProtocol')->willReturn(self::SERVER_PROTOCOL); $session = new Memory(); - $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); + $cryptoWrapper = new CryptoWrapper($this->crypto, $this->random, $this->request); $wrappedSession = $cryptoWrapper->wrapSession($session); - + $wrappedSession->set($keyName, $unencryptedValue); $wrappedSession->close(); From 504953fc8650475132aa4c283d25f3fccb23bbd9 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 28 Jan 2026 10:35:45 -0500 Subject: [PATCH 14/19] test(Session): lint/rector cleanup of CryptoSessionDataTest Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index cab80796285..449f0945b79 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -43,10 +43,10 @@ class CryptoSessionDataTest extends Session { $this->crypto = $this->createMock(ICrypto::class); $this->crypto->method('encrypt')->willReturnCallback( - fn($input) => '#' . $input . '#' + fn ($input) => '#' . $input . '#' ); $this->crypto->method('decrypt')->willReturnCallback( - fn($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1) + fn ($input) => ($input === '' || strlen($input) < 2) ? '' : substr($input, 1, -1) ); $this->session = new Memory(); @@ -59,7 +59,7 @@ class CryptoSessionDataTest extends Session { public function testSessionDataStoredEncrypted(): void { $keyName = 'secret'; $unencryptedValue = 'superSecretValue123'; - + $this->instance->set($keyName, $unencryptedValue); $this->instance->close(); @@ -89,11 +89,11 @@ class CryptoSessionDataTest extends Session { public static function roundTripValuesProvider(): array { return [ - 'simple string' => ['foo', 'bar'], - 'unicode value' => ['uni', "héllo 🌍"], - 'large value' => ['big', str_repeat('x', 4096)], - 'large array' => ['thousand', json_encode(self::makeLargeArray())], - 'empty string' => ['', ''], + 'simple string' => ['foo', 'bar'], + 'unicode value' => ['uni', "héllo 🌍"], + 'large value' => ['big', str_repeat('x', 4096)], + 'large array' => ['thousand', json_encode(self::makeLargeArray())], + 'empty string' => ['', ''], ]; } @@ -166,11 +166,11 @@ class CryptoSessionDataTest extends Session { private function createPassphraseAwareCryptoMock(): ICrypto { $crypto = $this->createMock(ICrypto::class); - $crypto->method('encrypt')->willReturnCallback(function($plain, $passphrase = null) { + $crypto->method('encrypt')->willReturnCallback(function ($plain, $passphrase = null) { // Set up: store a value with the passphrase embedded (fake encryption) return $passphrase . '#' . $plain . '#' . $passphrase; }); - $crypto->method('decrypt')->willReturnCallback(function($input, $passphrase = null) { + $crypto->method('decrypt')->willReturnCallback(function ($input, $passphrase = null) { // Only successfully decrypt if the embedded passphrase matches if (strpos($input, $passphrase . '#') === 0 && strrpos($input, '#' . $passphrase) === strlen($input) - strlen('#' . $passphrase)) { // Strip off passphrase markers and return the "decrypted" string From c6061be5a7bb7337c3bd13f069163372b41b9a9a Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 28 Jan 2026 11:05:34 -0500 Subject: [PATCH 16/19] test(Sesssion): fixup quoting CryptoSessionDataTest for lint/rector Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index 449f0945b79..03f39a00172 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -90,7 +90,7 @@ class CryptoSessionDataTest extends Session { public static function roundTripValuesProvider(): array { return [ 'simple string' => ['foo', 'bar'], - 'unicode value' => ['uni', "héllo 🌍"], + 'unicode value' => ['uni', 'héllo 🌍'], 'large value' => ['big', str_repeat('x', 4096)], 'large array' => ['thousand', json_encode(self::makeLargeArray())], 'empty string' => ['', ''], From 5e054a2270e99390c2acd232db9634bc01bf7827 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 29 Jan 2026 21:54:27 -0500 Subject: [PATCH 17/19] chore: fixup CryptoSessionDataTest Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index 03f39a00172..e4d6c0775b9 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -12,7 +12,6 @@ use OC\Session\CryptoSessionData; use OC\Session\Memory; use OCP\ISession; use OCP\Security\ICrypto; -use OCP\Security\ISecureRandom; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; From c4af7045528c32855a69eb3f2a479bcfce02ea02 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 2 Feb 2026 07:30:00 -0500 Subject: [PATCH 18/19] chore: fixup CryptoSessionDataTest from review Signed-off-by: Josh --- tests/lib/Session/CryptoSessionDataTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index e4d6c0775b9..e56d277f19e 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -33,7 +33,7 @@ class CryptoSessionDataTest extends Session { private const TAMPERED_BLOB = 'garbage-data'; private const MALFORMED_JSON_BLOB = '{not:valid:json}'; - protected ICrypto|MockObject $crypto; + protected ICrypto&MockObject $crypto; protected ISession $session; protected function setUp(): void { @@ -171,7 +171,7 @@ class CryptoSessionDataTest extends Session { }); $crypto->method('decrypt')->willReturnCallback(function ($input, $passphrase = null) { // Only successfully decrypt if the embedded passphrase matches - if (strpos($input, $passphrase . '#') === 0 && strrpos($input, '#' . $passphrase) === strlen($input) - strlen('#' . $passphrase)) { + if (str_starts_with($input, $passphrase . '#') && str_ends_with($input, '#' . $passphrase)) { // Strip off passphrase markers and return the "decrypted" string return substr($input, strlen($passphrase . '#'), -strlen('#' . $passphrase)); } From fe00757ca7214f9225eda4b7a95ee853ca2e3d0f Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 2 Feb 2026 07:31:00 -0500 Subject: [PATCH 19/19] chore: fixup CryptoWrappingTest.php from review feedback Signed-off-by: Josh --- tests/lib/Session/CryptoWrappingTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index 06acfad5e47..1395ddf9aa1 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -39,9 +39,9 @@ class CryptoWrappingTest extends TestCase { private const GENERATED_PASSPHRASE = 'generatedPassphrase'; private const SERVER_PROTOCOL = 'https'; - protected ICrypto|MockObject $crypto; - protected ISecureRandom|MockObject $random; - protected IRequest|MockObject $request; + protected ICrypto&MockObject $crypto; + protected ISecureRandom&MockObject $random; + protected IRequest&MockObject $request; protected function setUp(): void { parent::setUp();