Merge pull request #54207 from nextcloud/feat/noid/cache-user-keys

Cache IdentityProof keys

lib/private/Security/IdentityProof/Manager.php

@@ -11,6 +11,8 @@ namespace OC\Security\IdentityProof;
 use OC\Files\AppData\Factory;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
+use OCP\ICache;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IUser;
 use OCP\Security\ICrypto;
@@ -19,13 +21,17 @@ use Psr\Log\LoggerInterface;
 class Manager {
 	private IAppData $appData;
 
+	protected ICache $cache;
+
 	public function __construct(
 		Factory $appDataFactory,
 		private ICrypto $crypto,
 		private IConfig $config,
 		private LoggerInterface $logger,
+		private ICacheFactory $cacheFactory,
 	) {
 		$this->appData = $appDataFactory->get('identityproof');
+		$this->cache = $this->cacheFactory->createDistributed('identityproof::');
 	}
 
 	/**
@@ -96,12 +102,24 @@ class Manager {
 	 */
 	protected function retrieveKey(string $id): Key {
 		try {
+			$cachedPublicKey = $this->cache->get($id . '-public');
+			$cachedPrivateKey = $this->cache->get($id . '-private');
+
+			if ($cachedPublicKey !== null && $cachedPrivateKey !== null) {
+				$decryptedPrivateKey = $this->crypto->decrypt($cachedPrivateKey);
+
+				return new Key($cachedPublicKey, $decryptedPrivateKey);
+			}
+
 			$folder = $this->appData->getFolder($id);
-			$privateKey = $this->crypto->decrypt(
-				$folder->getFile('private')->getContent()
-			);
+			$privateKey = $folder->getFile('private')->getContent();
 			$publicKey = $folder->getFile('public')->getContent();
-			return new Key($publicKey, $privateKey);
+
+			$this->cache->set($id . '-public', $publicKey);
+			$this->cache->set($id . '-private', $privateKey);
+
+			$decryptedPrivateKey = $this->crypto->decrypt($privateKey);
+			return new Key($publicKey, $decryptedPrivateKey);
 		} catch (\Exception $e) {
 			return $this->generateKey($id);
 		}

tests/lib/Security/IdentityProof/ManagerTest.php

@@ -16,6 +16,8 @@ use OC\Security\IdentityProof\Manager;
 use OCP\Files\IAppData;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\Files\SimpleFS\ISimpleFolder;
+use OCP\ICache;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IUser;
 use OCP\Security\ICrypto;
@@ -24,18 +26,14 @@ use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class ManagerTest extends TestCase {
-	/** @var Factory|MockObject */
-	private $factory;
-	/** @var IAppData|MockObject */
-	private $appData;
-	/** @var ICrypto|MockObject */
-	private $crypto;
-	/** @var Manager|MockObject */
-	private $manager;
-	/** @var IConfig|MockObject */
-	private $config;
-	/** @var LoggerInterface|MockObject */
-	private $logger;
+	private Factory&MockObject $factory;
+	private IAppData&MockObject $appData;
+	private ICrypto&MockObject $crypto;
+	private Manager&MockObject $manager;
+	private IConfig&MockObject $config;
+	private LoggerInterface&MockObject $logger;
+	private ICacheFactory&MockObject $cacheFactory;
+	private ICache&MockObject $cache;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -49,6 +47,12 @@ class ManagerTest extends TestCase {
 			->with('identityproof')
 			->willReturn($this->appData);
 		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->cacheFactory = $this->createMock(ICacheFactory::class);
+		$this->cache = $this->createMock(ICache::class);
+
+		$this->cacheFactory->expects($this->any())
+			->method('createDistributed')
+			->willReturn($this->cache);
 
 		$this->crypto = $this->createMock(ICrypto::class);
 		$this->manager = $this->getManager(['generateKeyPair']);
@@ -66,7 +70,8 @@ class ManagerTest extends TestCase {
 				$this->factory,
 				$this->crypto,
 				$this->config,
-				$this->logger
+				$this->logger,
+				$this->cacheFactory,
 			);
 		} else {
 			return $this->getMockBuilder(Manager::class)
@@ -74,7 +79,8 @@ class ManagerTest extends TestCase {
 					$this->factory,
 					$this->crypto,
 					$this->config,
-					$this->logger
+					$this->logger,
+					$this->cacheFactory,
 				])
 				->onlyMethods($setMethods)
 				->getMock();
@@ -115,6 +121,33 @@ class ManagerTest extends TestCase {
 			->method('getFolder')
 			->with('user-MyUid')
 			->willReturn($folder);
+		$this->cache
+			->expects($this->exactly(2))
+			->method('get')
+			->willReturn(null);
+
+		$expected = new Key('MyPublicKey', 'MyPrivateKey');
+		$this->assertEquals($expected, $this->manager->getKey($user));
+	}
+
+	public function testGetKeyWithExistingKeyCached(): void {
+		$user = $this->createMock(IUser::class);
+		$user
+			->expects($this->once())
+			->method('getUID')
+			->willReturn('MyUid');
+		$this->crypto
+			->expects($this->once())
+			->method('decrypt')
+			->with('EncryptedPrivateKey')
+			->willReturn('MyPrivateKey');
+		$this->cache
+			->expects($this->exactly(2))
+			->method('get')
+			->willReturnMap([
+				['user-MyUid-public', 'MyPublicKey'],
+				['user-MyUid-private', 'EncryptedPrivateKey'],
+			]);
 
 		$expected = new Key('MyPublicKey', 'MyPrivateKey');
 		$this->assertEquals($expected, $this->manager->getKey($user));