diff --git a/src/Temporary/CallbackRetryPolicy.php b/src/Temporary/CallbackRetryPolicy.php new file mode 100644 index 0000000..648b2db --- /dev/null +++ b/src/Temporary/CallbackRetryPolicy.php @@ -0,0 +1,33 @@ +shouldRetryCallback = $shouldRetryCallback(...); + } + + public function canRetry(RetryContextInterface $context): bool + { + $e = $context->getLastException(); + + if (($this->shouldRetryCallback)($e, $context) !== true) { + return false; + } + + return parent::canRetry($context); + } +} diff --git a/src/Wrapper/GenericAKVWrapper.php b/src/Wrapper/GenericAKVWrapper.php index 1bbbb1a..c21a91d 100644 --- a/src/Wrapper/GenericAKVWrapper.php +++ b/src/Wrapper/GenericAKVWrapper.php @@ -8,6 +8,7 @@ use Defuse\Crypto\Key; use Keboola\AzureKeyVaultClient\Authentication\AuthenticatorFactory; use Keboola\AzureKeyVaultClient\Client; +use Keboola\AzureKeyVaultClient\Exception\ClientException; use Keboola\AzureKeyVaultClient\GuzzleClientFactory; use Keboola\AzureKeyVaultClient\Requests\SecretAttributes; use Keboola\AzureKeyVaultClient\Requests\SetSecretRequest; @@ -15,10 +16,12 @@ use Keboola\ObjectEncryptor\EncryptorOptions; use Keboola\ObjectEncryptor\Exception\ApplicationException; use Keboola\ObjectEncryptor\Exception\UserException; +use Keboola\ObjectEncryptor\Temporary\CallbackRetryPolicy; use Keboola\ObjectEncryptor\Temporary\TransClient; use Keboola\ObjectEncryptor\Temporary\TransClientNotAvailableException; use Psr\Log\NullLogger; use Retry\BackOff\ExponentialBackOffPolicy; +use Retry\Policy\RetryPolicyInterface; use Retry\Policy\SimpleRetryPolicy; use Retry\RetryProxy; use Throwable; @@ -81,9 +84,14 @@ public function getTransClient(): ?TransClient return $this->transClient ?: null; } - private function getRetryProxy(): RetryProxy + private static function getTransStackId(): ?string { - $retryPolicy = new SimpleRetryPolicy(3); + return (string) getenv('TRANS_ENCRYPTOR_STACK_ID') ?: null; + } + + private function getRetryProxy(?RetryPolicyInterface $retryPolicy = null): RetryProxy + { + $retryPolicy ??= new SimpleRetryPolicy(3); $backOffPolicy = new ExponentialBackOffPolicy(1000); return new RetryProxy($retryPolicy, $backOffPolicy); } @@ -171,8 +179,36 @@ public function decrypt(string $encryptedData): string ) { throw new UserException('Deciphering failed.'); } + + $metadata = $this->metadata; + $doBackfill = false; + + // try retrieve secret from trans AKV + if ($this->getTransClient() !== null) { + $retryPolicy = new CallbackRetryPolicy(fn($e) => + // do not retry if trans AKV response is 404 + !$e instanceof ClientException || $e->getCode() !== 404); + try { + $decryptedContext = $this->getRetryProxy($retryPolicy)->call(function () use ($encrypted) { + return $this->getTransClient() + ?->getSecret($encrypted[self::SECRET_NAME]) + ->getValue(); + }); + if ($decryptedContext !== null && isset($this->metadata['stackId']) && self::getTransStackId()) { + $metadata['stackId'] = self::getTransStackId(); + } + } catch (ClientException $e) { + if ($e->getCode() === 404) { + $doBackfill = true; + } + } catch (Throwable) { + // intentionally suppress all errors to prevent decrypt() from failing + } + } + try { - $decryptedContext = $this->getRetryProxy()->call(function () use ($encrypted) { + // retrieve only if not found at trans AKV + $decryptedContext ??= $this->getRetryProxy()->call(function () use ($encrypted) { return $this->getClient() ->getSecret($encrypted[self::SECRET_NAME]) ->getValue(); @@ -188,12 +224,30 @@ public function decrypt(string $encryptedData): string } catch (Throwable $e) { throw new ApplicationException('Deciphering failed.', $e->getCode(), $e); } - $this->verifyMetadata($decryptedContext[self::METADATA_INDEX], $this->metadata); + $this->verifyMetadata($decryptedContext[self::METADATA_INDEX], $metadata); try { $key = Key::loadFromAsciiSafeString($decryptedContext[self::KEY_INDEX]); return Crypto::decrypt($encrypted[self::PAYLOAD_INDEX], $key, true); } catch (Throwable $e) { + $doBackfill = false; throw new ApplicationException('Deciphering failed.', $e->getCode(), $e); + } finally { + if ($doBackfill) { + if (isset($this->metadata['stackId']) && self::getTransStackId()) { + $decryptedContext[self::METADATA_INDEX]['stackId'] = self::getTransStackId(); + } + try { + $this->getRetryProxy()->call(function () use ($encrypted, $decryptedContext) { + $context = $this->encode($decryptedContext); + $this->getTransClient()?->setSecret( + new SetSecretRequest($context, new SecretAttributes()), + $encrypted[self::SECRET_NAME], + ); + }); + } catch (Throwable) { + // intentionally suppress all errors to prevent decrypt() from failing + } + } } } diff --git a/tests/Temporary/AKVWrappersWithTransClientTest.php b/tests/Temporary/AKVWrappersWithTransClientTest.php index af8b372..b982f7b 100644 --- a/tests/Temporary/AKVWrappersWithTransClientTest.php +++ b/tests/Temporary/AKVWrappersWithTransClientTest.php @@ -4,6 +4,12 @@ namespace Keboola\ObjectEncryptor\Tests\Temporary; +use Defuse\Crypto\Crypto; +use Defuse\Crypto\Key; +use Keboola\AzureKeyVaultClient\Client; +use Keboola\AzureKeyVaultClient\Exception\ClientException; +use Keboola\AzureKeyVaultClient\Requests\SetSecretRequest; +use Keboola\AzureKeyVaultClient\Responses\SecretBundle; use Keboola\ObjectEncryptor\EncryptorOptions; use Keboola\ObjectEncryptor\Temporary\TransClient; use Keboola\ObjectEncryptor\Wrapper\BranchTypeConfigurationAKVWrapper; @@ -14,6 +20,7 @@ use Keboola\ObjectEncryptor\Wrapper\GenericAKVWrapper; use Keboola\ObjectEncryptor\Wrapper\ProjectAKVWrapper; use Keboola\ObjectEncryptor\Wrapper\ProjectWideAKVWrapper; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class AKVWrappersWithTransClientTest extends TestCase @@ -25,26 +32,80 @@ public function setUp(): void putenv('TRANS_AZURE_TENANT_ID='); putenv('TRANS_AZURE_CLIENT_ID='); putenv('TRANS_AZURE_CLIENT_SECRET='); + putenv('TRANS_ENCRYPTOR_STACK_ID='); } public static function provideAKVWrappers(): iterable { - $classes = [ - GenericAKVWrapper::class, - ComponentAKVWrapper::class, - ProjectAKVWrapper::class, - ConfigurationAKVWrapper::class, - ProjectWideAKVWrapper::class, - BranchTypeProjectAKVWrapper::class, - BranchTypeProjectWideAKVWrapper::class, - BranchTypeConfigurationAKVWrapper::class, + yield GenericAKVWrapper::class => [ + 'wrapperClass' => GenericAKVWrapper::class, + 'metadata' => [], ]; - foreach ($classes as $className) { - yield $className => [ - 'wrapperClass' => $className, - ]; - } + yield ComponentAKVWrapper::class => [ + 'wrapperClass' => ComponentAKVWrapper::class, + 'metadata' => [ + 'stackId' => 'some-stack', + 'componentId' => 'component-id', + ], + ]; + + yield ProjectAKVWrapper::class => [ + 'wrapperClass' => ProjectAKVWrapper::class, + 'metadata' => [ + 'stackId' => 'some-stack', + 'componentId' => 'component-id', + 'projectId' => 'project-id', + ], + ]; + + yield ConfigurationAKVWrapper::class => [ + 'wrapperClass' => ConfigurationAKVWrapper::class, + 'metadata' => [ + 'stackId' => 'some-stack', + 'componentId' => 'component-id', + 'projectId' => 'project-id', + 'configurationId' => 'config-id', + ], + ]; + + yield ProjectWideAKVWrapper::class => [ + 'wrapperClass' => ProjectWideAKVWrapper::class, + 'metadata' => [ + 'stackId' => 'some-stack', + 'projectId' => 'project-id', + ], + ]; + + yield BranchTypeProjectAKVWrapper::class => [ + 'wrapperClass' => BranchTypeProjectAKVWrapper::class, + 'metadata' => [ + 'stackId' => 'some-stack', + 'componentId' => 'component-id', + 'projectId' => 'project-id', + 'branchType' => 'branch-type', + ], + ]; + + yield BranchTypeConfigurationAKVWrapper::class => [ + 'wrapperClass' => BranchTypeConfigurationAKVWrapper::class, + 'metadata' => [ + 'stackId' => 'some-stack', + 'componentId' => 'component-id', + 'projectId' => 'project-id', + 'configurationId' => 'config-id', + 'branchType' => 'branch-type', + ], + ]; + + yield BranchTypeProjectWideAKVWrapper::class => [ + 'wrapperClass' => BranchTypeProjectWideAKVWrapper::class, + 'metadata' => [ + 'stackId' => 'some-stack', + 'projectId' => 'project-id', + 'branchType' => 'branch-type', + ], + ]; } /** @@ -147,4 +208,143 @@ public function testWrappersDoNotHaveTransClientWhenEncryptorIdMismatches( )); self::assertNull($wrapper->getTransClient()); } + + /** + * @dataProvider provideAKVWrappers + * @param class-string $wrapperClass + */ + public function testDecryptWithBackfillWhenSecretNotFoundInTransVault( + string $wrapperClass, + array $metadata, + ): void { + putenv('TRANS_ENCRYPTOR_STACK_ID=trans-stack'); + + $key = Key::createNewRandomKey(); + + $mockClient = $this->createMock(Client::class); + $mockClient->expects(self::once()) + ->method('getSecret') + ->with('secret-name') + ->willReturn(new SecretBundle([ + 'id' => 'secret-id', + 'value' => self::encode([ + 0 => $metadata, + 1 => $key->saveToAsciiSafeString(), + ]), + 'attributes' => [], + ])); + + $mockTransClient = $this->createMock(TransClient::class); + $mockTransClient->expects(self::once()) + ->method('getSecret') + ->with('secret-name') + ->willThrowException(new ClientException('not found', 404)); + $mockTransClient->expects(self::once()) + ->method('setSecret') + ->with( + self::callback(function ($arg) use ($key, $metadata) { + self::assertInstanceOf(SetSecretRequest::class, $arg); + + if (array_key_exists('stackId', $metadata)) { + $metadata['stackId'] = 'trans-stack'; + } + $encodedKey = self::encode([ + 0 => $metadata, + 1 => $key->saveToAsciiSafeString(), + ]); + + self::assertSame($encodedKey, $arg->getArray()['value']); + return true; + }), + 'secret-name', + ); + + /** @var GenericAKVWrapper|MockObject $mockWrapper */ + $mockWrapper = $this->getMockBuilder($wrapperClass) + ->setConstructorArgs([ + new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ), + ]) + ->onlyMethods(['getClient', 'getTransClient']) + ->getMock(); + foreach ($metadata as $metaKey => $metaValue) { + $mockWrapper->setMetadataValue($metaKey, $metaValue); + } + $mockWrapper->expects(self::once()) + ->method('getClient') + ->willReturn($mockClient); + $mockWrapper->expects(self::atLeast(3)) + ->method('getTransClient') + ->willReturn($mockTransClient); + + $encryptedSecret = self::encode([ + 2 => Crypto::encrypt('something very secret', $key, true), + 3 => 'secret-name', + 4 => 'secret-version', + ]); + + $secret = $mockWrapper->decrypt($encryptedSecret); + + self::assertSame('something very secret', $secret); + } + + /** + * @dataProvider provideAKVWrappers + * @param class-string $wrapperClass + */ + public function testDecryptOmitsPrimaryVaultWhenSecretFoundInTransVault( + string $wrapperClass, + array $metadata, + ): void { + $key = Key::createNewRandomKey(); + + $mockTransClient = $this->createMock(TransClient::class); + $mockTransClient->expects(self::once()) + ->method('getSecret') + ->with('secret-name') + ->willReturn(new SecretBundle([ + 'id' => 'secret-id', + 'value' => self::encode([ + 0 => $metadata, + 1 => $key->saveToAsciiSafeString(), + ]), + 'attributes' => [], + ])); + $mockTransClient->expects(self::never())->method('setSecret'); + + /** @var GenericAKVWrapper|MockObject $mockWrapper */ + $mockWrapper = $this->getMockBuilder($wrapperClass) + ->setConstructorArgs([ + new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ), + ]) + ->onlyMethods(['getClient', 'getTransClient']) + ->getMock(); + foreach ($metadata as $metaKey => $metaValue) { + $mockWrapper->setMetadataValue($metaKey, $metaValue); + } + $mockWrapper->expects(self::never())->method('getClient'); + $mockWrapper->expects(self::exactly(2)) + ->method('getTransClient') + ->willReturn($mockTransClient); + + $encryptedSecret = self::encode([ + 2 => Crypto::encrypt('something very secret', $key, true), + 3 => 'secret-name', + 4 => 'secret-version', + ]); + + $secret = $mockWrapper->decrypt($encryptedSecret); + + self::assertSame('something very secret', $secret); + } + + private static function encode(mixed $data): string + { + return base64_encode((string) gzcompress(serialize($data))); + } }