From aba96c12f223713a17b97b2d88b3e90f5d152d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Mon, 3 Feb 2025 12:22:49 +0100 Subject: [PATCH 1/9] chore: Update keboola/azure-key-vault-client to ^4.0 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ca5f540..d4601b6 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,7 @@ "aws/aws-sdk-php": "^3.209", "defuse/php-encryption": "^2.3", "google/cloud-kms": "^1.20", - "keboola/azure-key-vault-client": "^3.0", + "keboola/azure-key-vault-client": "^4.0", "keboola/common-exceptions": "^1.2", "vkartaviy/retry": "^0.2" }, From c1d417053974d46d5f1b01e656bff3dacc67931d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Tue, 28 Jan 2025 09:04:27 +0100 Subject: [PATCH 2/9] feat (csas-migration): Add $encryptorId in EncryptorOptions --- src/EncryptorOptions.php | 14 ++++++++++++++ tests/EncryptorOptionsTest.php | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/EncryptorOptions.php b/src/EncryptorOptions.php index 3dbc98b..be7c05a 100644 --- a/src/EncryptorOptions.php +++ b/src/EncryptorOptions.php @@ -24,6 +24,9 @@ class EncryptorOptions private ?string $kmsRole; private int $backoffMaxTries; + /** @var non-empty-string|null */ + private ?string $encryptorId; + /** * @param non-empty-string $stackId * @param non-empty-string|null $kmsKeyId @@ -32,6 +35,7 @@ class EncryptorOptions * @param non-empty-string|null $akvUrl * @param non-empty-string|null $gkmsKeyId * @param int|null $backoffMaxTries + * @param non-empty-string|null $encryptorId */ public function __construct( string $stackId, @@ -41,6 +45,7 @@ public function __construct( ?string $akvUrl = null, ?string $gkmsKeyId = null, ?int $backoffMaxTries = null, + ?string $encryptorId = null, ) { $this->stackId = $stackId; $this->kmsKeyId = $kmsKeyId; @@ -49,6 +54,7 @@ public function __construct( $this->akvUrl = $akvUrl; $this->gkmsKeyId = $gkmsKeyId; $this->backoffMaxTries = $backoffMaxTries ?? self::DEFAULT_BACKOFF_MAX_TRIES; + $this->encryptorId = $encryptorId; $this->validateState(); } @@ -102,6 +108,14 @@ public function getBackoffMaxTries(): int return $this->backoffMaxTries; } + /** + * @return non-empty-string|null + */ + public function getEncryptorId(): ?string + { + return $this->encryptorId; + } + /** * @throws ApplicationException */ diff --git a/tests/EncryptorOptionsTest.php b/tests/EncryptorOptionsTest.php index 721fde2..0b755a3 100644 --- a/tests/EncryptorOptionsTest.php +++ b/tests/EncryptorOptionsTest.php @@ -20,6 +20,7 @@ public function testAccessors(): void akvUrl: 'akv-url', gkmsKeyId: 'gkms-key-id', backoffMaxTries: 1, + encryptorId: 'internal', ); self::assertSame('my-stack', $options->getStackId()); self::assertSame('my-kms-id', $options->getKmsKeyId()); @@ -28,6 +29,7 @@ public function testAccessors(): void self::assertSame('akv-url', $options->getAkvUrl()); self::assertSame('gkms-key-id', $options->getGkmsKeyId()); self::assertSame(1, $options->getBackoffMaxTries()); + self::assertSame('internal', $options->getEncryptorId()); } public function testConstructEmptyConfig(): void From fb1b3549327b9e16ace7db698fead029c3f2416d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Tue, 28 Jan 2025 16:19:41 +0100 Subject: [PATCH 3/9] feat (csas-migration): Add TransClientCredentialsEnvironmentAuthenticator & TransAuthenticatorFactory --- src/Temporary/TransAuthenticatorFactory.php | 24 +++++ ...entCredentialsEnvironmentAuthenticator.php | 14 +++ .../TransClientNotAvailableException.php | 11 +++ .../TransAuthenticatorFactoryTest.php | 89 +++++++++++++++++++ 4 files changed, 138 insertions(+) create mode 100644 src/Temporary/TransAuthenticatorFactory.php create mode 100644 src/Temporary/TransClientCredentialsEnvironmentAuthenticator.php create mode 100644 src/Temporary/TransClientNotAvailableException.php create mode 100644 tests/Temporary/TransAuthenticatorFactoryTest.php diff --git a/src/Temporary/TransAuthenticatorFactory.php b/src/Temporary/TransAuthenticatorFactory.php new file mode 100644 index 0000000..ac56a71 --- /dev/null +++ b/src/Temporary/TransAuthenticatorFactory.php @@ -0,0 +1,24 @@ +checkUsability(); + return $authenticator; + } catch (ClientException) { + throw new TransClientNotAvailableException; + } + } +} diff --git a/src/Temporary/TransClientCredentialsEnvironmentAuthenticator.php b/src/Temporary/TransClientCredentialsEnvironmentAuthenticator.php new file mode 100644 index 0000000..f1e73ee --- /dev/null +++ b/src/Temporary/TransClientCredentialsEnvironmentAuthenticator.php @@ -0,0 +1,14 @@ +getAuthenticator( + new GuzzleClientFactory(new NullLogger()), + 'https://vault.azure.net', + ); + self::assertTrue(true); + } catch (TransClientNotAvailableException) { + self::fail('Test should not have thrown an exception'); + } + } + + public static function provideInvalidTransEnvs(): iterable + { + yield 'missing all' => [ + [], + ]; + + yield 'missing TRANS_AZURE_TENANT_ID' => [ + [ + 'TRANS_AZURE_TENANT_ID' => '', + 'TRANS_AZURE_CLIENT_ID' => 'client-id', + 'TRANS_AZURE_CLIENT_SECRET' => 'client-secret', + ], + ]; + + yield 'missing TRANS_AZURE_CLIENT_ID' => [ + [ + 'TRANS_AZURE_TENANT_ID' => 'tenant-id', + 'TRANS_AZURE_CLIENT_ID' => '', + 'TRANS_AZURE_CLIENT_SECRET' => 'client-secret', + ], + ]; + + yield 'missing TRANS_AZURE_CLIENT_SECRET' => [ + [ + 'TRANS_AZURE_TENANT_ID' => 'tenant-id', + 'TRANS_AZURE_CLIENT_ID' => 'client-id', + 'TRANS_AZURE_CLIENT_SECRET' => '', + ], + ]; + } + + /** @dataProvider provideInvalidTransEnvs */ + public function testTransAuthenticatorIsUnavailable(array $invalidEnvs): void + { + foreach ($invalidEnvs as $name => $value) { + putenv(sprintf('%s=%s', $name, $value)); + } + + $transAuthFactory = new TransAuthenticatorFactory(); + + $this->expectException(TransClientNotAvailableException::class); + + $transAuthFactory->getAuthenticator( + new GuzzleClientFactory(new NullLogger()), + 'https://vault.azure.net', + ); + } +} From cd29f94d2865e5dc4ffb6fdff61aad34a57164b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Tue, 28 Jan 2025 19:41:00 +0100 Subject: [PATCH 4/9] feat (csas-migration): Add TransClient --- src/Temporary/TransClient.php | 39 +++++++ tests/Temporary/TransClientTest.php | 166 ++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 src/Temporary/TransClient.php create mode 100644 tests/Temporary/TransClientTest.php diff --git a/src/Temporary/TransClient.php b/src/Temporary/TransClient.php new file mode 100644 index 0000000..be7df10 --- /dev/null +++ b/src/Temporary/TransClient.php @@ -0,0 +1,39 @@ + [ + 'encryptorId' => null, + 'expectedEnvName' => 'TRANS_AZURE_KEY_VAULT_URL', + ]; + yield 'encryptorId is empty' => [ + 'encryptorId' => '', + 'expectedEnvName' => 'TRANS_AZURE_KEY_VAULT_URL', + ]; + yield 'encryptorId = "internal"' => [ + 'encryptorId' => 'internal', + 'expectedEnvName' => 'TRANS_AZURE_KEY_VAULT_URL_INTERNAL', + ]; + yield 'encryptorId = "job_queue"' => [ + 'encryptorId' => 'job_queue', + 'expectedEnvName' => 'TRANS_AZURE_KEY_VAULT_URL_JOB_QUEUE', + ]; + yield 'encryptorId = "job__queue_"' => [ + 'encryptorId' => 'job__queue_', + 'expectedEnvName' => 'TRANS_AZURE_KEY_VAULT_URL_JOB_QUEUE', + ]; + yield 'encryptorId = "job-queue"' => [ + 'encryptorId' => 'job-queue', + 'expectedEnvName' => 'TRANS_AZURE_KEY_VAULT_URL_JOB_QUEUE', + ]; + yield 'encryptorId = "job queue"' => [ + 'encryptorId' => 'job queue', + 'expectedEnvName' => 'TRANS_AZURE_KEY_VAULT_URL_JOB_QUEUE', + ]; + } + + /** @dataProvider provideDeterminateVaultUrlEnvNameTestData */ + public function testDeterminateVaultUrlEnvName( + ?string $encryptorId, + string $expectedEnvName, + ): void { + self::assertSame( + $expectedEnvName, + TransClient::determinateVaultUrlEnvName($encryptorId), + ); + } + + public static function provideTransClientUrlTestData(): iterable + { + yield 'encryptorId is null' => [ + 'encryptorId' => null, + 'envs' => [ + 'TRANS_AZURE_KEY_VAULT_URL' => 'https://vault-url', + ], + 'expectedClientUrl' => 'https://vault-url', + ]; + + yield 'encryptorId = "internal"' => [ + 'encryptorId' => 'internal', + 'envs' => [ + 'TRANS_AZURE_KEY_VAULT_URL_INTERNAL' => 'https://internal-vault-url', + ], + 'expectedClientUrl' => 'https://internal-vault-url', + ]; + } + + /** @dataProvider provideTransClientUrlTestData */ + public function testTransClientUrl( + ?string $encryptorId, + array $envs, + string $expectedVaultUrl, + ): void { + putenv('TRANS_AZURE_TENANT_ID=tenant-id'); + putenv('TRANS_AZURE_CLIENT_ID=client-id'); + putenv('TRANS_AZURE_CLIENT_SECRET=client-secret'); + foreach ($envs as $envName => $envValue) { + putenv(sprintf('%s=%s', $envName, $envValue)); + } + + $guzzleClientFactoryCounter = self::exactly(2); + $guzzleClientFactoryMock = $this->createMock(GuzzleClientFactory::class); + $guzzleClientFactoryMock->expects($guzzleClientFactoryCounter) + ->method('getClient') + ->with( + self::callback(fn($url) => match ($guzzleClientFactoryCounter->getInvocationCount()) { + 1 => $url === $expectedVaultUrl, + 2 => $url === 'https://management.azure.com/metadata/endpoints?api-version=2020-01-01', + default => self::fail('Unexpected url: ' . $url), + }), + self::isType('array'), + ); + + try { + new TransClient( + $guzzleClientFactoryMock, + $encryptorId, + ); + } catch (TransClientNotAvailableException) { + self::fail('Test should not have thrown an exception'); + } + } + + public static function provideTransClientMismatchEnvsTestData(): iterable + { + yield 'encryptorId is null, env has suffix' => [ + 'encryptorId' => null, + 'envs' => [ + 'TRANS_AZURE_KEY_VAULT_URL_SOMETHING' => 'https://vault-url', + ], + ]; + + yield 'encryptorId = "internal", env suffix is missing' => [ + 'encryptorId' => 'internal', + 'envs' => [ + 'TRANS_AZURE_KEY_VAULT_URL' => 'https://vault-url', + ], + ]; + } + + /** @dataProvider provideTransClientMismatchEnvsTestData */ + public function testTransClientMismatchEnvs( + ?string $encryptorId, + array $envs, + ): void { + putenv('TRANS_AZURE_TENANT_ID=tenant-id'); + putenv('TRANS_AZURE_CLIENT_ID=client-id'); + putenv('TRANS_AZURE_CLIENT_SECRET=client-secret'); + foreach ($envs as $envName => $envValue) { + putenv(sprintf('%s=%s', $envName, $envValue)); + } + + $this->expectException(TransClientNotAvailableException::class); + + new TransClient( + new GuzzleClientFactory(new NullLogger()), + $encryptorId, + ); + } +} From 4dc955a25c86d1f055045b64a5e0281216d81388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Tue, 28 Jan 2025 19:48:17 +0100 Subject: [PATCH 5/9] feat (csas-migration): Extend GenericAKVWrapper with trans client --- src/Wrapper/GenericAKVWrapper.php | 23 +++ .../AKVWrappersWithTransClientTest.php | 150 ++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 tests/Temporary/AKVWrappersWithTransClientTest.php diff --git a/src/Wrapper/GenericAKVWrapper.php b/src/Wrapper/GenericAKVWrapper.php index 576e497..1bbbb1a 100644 --- a/src/Wrapper/GenericAKVWrapper.php +++ b/src/Wrapper/GenericAKVWrapper.php @@ -15,6 +15,8 @@ use Keboola\ObjectEncryptor\EncryptorOptions; use Keboola\ObjectEncryptor\Exception\ApplicationException; use Keboola\ObjectEncryptor\Exception\UserException; +use Keboola\ObjectEncryptor\Temporary\TransClient; +use Keboola\ObjectEncryptor\Temporary\TransClientNotAvailableException; use Psr\Log\NullLogger; use Retry\BackOff\ExponentialBackOffPolicy; use Retry\Policy\SimpleRetryPolicy; @@ -37,6 +39,9 @@ class GenericAKVWrapper implements CryptoWrapperInterface private string $keyVaultURL; private ?Client $client = null; + private TransClient|false|null $transClient = null; + private ?string $encryptorId = null; + public function __construct(EncryptorOptions $encryptorOptions) { // there is no way to pass backOffMaxTries option to the Azure Key Vault client. Yet. @@ -44,6 +49,8 @@ public function __construct(EncryptorOptions $encryptorOptions) if (empty($this->keyVaultURL)) { throw new ApplicationException('Cipher key settings are invalid.'); } + + $this->encryptorId = $encryptorOptions->getEncryptorId(); } public function getClient(): Client @@ -58,6 +65,22 @@ public function getClient(): Client return $this->client; } + public function getTransClient(): ?TransClient + { + if ($this->transClient === null) { + try { + $this->transClient = new TransClient( + new GuzzleClientFactory(new NullLogger()), + $this->encryptorId, + ); + } catch (TransClientNotAvailableException) { + $this->transClient = false; + } + } + + return $this->transClient ?: null; + } + private function getRetryProxy(): RetryProxy { $retryPolicy = new SimpleRetryPolicy(3); diff --git a/tests/Temporary/AKVWrappersWithTransClientTest.php b/tests/Temporary/AKVWrappersWithTransClientTest.php new file mode 100644 index 0000000..af8b372 --- /dev/null +++ b/tests/Temporary/AKVWrappersWithTransClientTest.php @@ -0,0 +1,150 @@ + [ + 'wrapperClass' => $className, + ]; + } + } + + /** + * @dataProvider provideAKVWrappers + * @param class-string $wrapperClass + */ + public function testWrappersDoNotHaveTransClientInitializedWhenTransEnvsMissing( + string $wrapperClass, + ): void { + $encryptorOptions = new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ); + + $wrapper = new $wrapperClass($encryptorOptions); + + self::assertNull($wrapper->getTransClient()); + } + + /** + * @dataProvider provideAKVWrappers + * @param class-string $wrapperClass + */ + public function testWrappersHaveTransClientInitialized( + string $wrapperClass, + ): void { + putenv('TRANS_AZURE_TENANT_ID=tenant-id'); + putenv('TRANS_AZURE_CLIENT_ID=client-id'); + putenv('TRANS_AZURE_CLIENT_SECRET=client-secret'); + putenv('TRANS_AZURE_KEY_VAULT_URL=https://vault-url'); + + $encryptorOptions = new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ); + + $wrapper = new $wrapperClass($encryptorOptions); + + $transClient = $wrapper->getTransClient(); + self::assertInstanceOf(TransClient::class, $transClient); + + // ensure getter returns a single instance of the TransClient + self::assertSame($transClient, $wrapper->getTransClient()); + } + + /** + * @dataProvider provideAKVWrappers + * @param class-string $wrapperClass + */ + public function testWrappersHaveTransClientWhenEncryptorIdMatches( + string $wrapperClass, + ): void { + putenv('TRANS_AZURE_TENANT_ID=tenant-id'); + putenv('TRANS_AZURE_CLIENT_ID=client-id'); + putenv('TRANS_AZURE_CLIENT_SECRET=client-secret'); + putenv('TRANS_AZURE_KEY_VAULT_URL='); + putenv('TRANS_AZURE_KEY_VAULT_URL_EXTRA_BRATWURST=https://german-vault-url'); + + $encryptorOptions = new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + encryptorId: 'extra-bratwurst', + ); + + $wrapper = new $wrapperClass($encryptorOptions); + + $transClient = $wrapper->getTransClient(); + self::assertInstanceOf(TransClient::class, $transClient); + + // ensure getter returns a single instance of the TransClient + self::assertSame($transClient, $wrapper->getTransClient()); + } + + /** + * @dataProvider provideAKVWrappers + * @param class-string $wrapperClass + */ + public function testWrappersDoNotHaveTransClientWhenEncryptorIdMismatches( + string $wrapperClass, + ): void { + putenv('TRANS_AZURE_TENANT_ID=tenant-id'); + putenv('TRANS_AZURE_CLIENT_ID=client-id'); + putenv('TRANS_AZURE_CLIENT_SECRET=client-secret'); + putenv('TRANS_AZURE_KEY_VAULT_URL='); + putenv('TRANS_AZURE_KEY_VAULT_URL_EXTRA_BRATWURST=https://german-vault-url'); + + // null encryptorId + $wrapper = new $wrapperClass(new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + encryptorId: null, + )); + self::assertNull($wrapper->getTransClient()); + + // encryptorId does not match env suffix ('extra-sausage' vs. _EXTRA_BRATWURST) + $wrapper = new $wrapperClass(new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + encryptorId: 'extra-sausage', + )); + self::assertNull($wrapper->getTransClient()); + } +} From 66624054d79f6f54ac8ce28e81cc1417df16baf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Mon, 3 Feb 2025 16:38:36 +0100 Subject: [PATCH 6/9] feat (csas-migration): Check & backfill secret in trans AKV --- src/Temporary/CallbackRetryPolicy.php | 33 +++ src/Wrapper/GenericAKVWrapper.php | 62 +++- .../AKVWrappersWithTransClientTest.php | 280 +++++++++++++++++- 3 files changed, 357 insertions(+), 18 deletions(-) create mode 100644 src/Temporary/CallbackRetryPolicy.php 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..58fd320 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) { + // do not retry if trans AKV response is 404 + $retryDecider = fn($e) => !$e instanceof ClientException || $e->getCode() !== 404; + $retryPolicy = new CallbackRetryPolicy($retryDecider); + 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..49f2c7d 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,195 @@ 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); + } + + public function testRetrieveFromPrimaryVaultWhenTransVaultRetrievalFails(): void + { + $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 => [], + 1 => $key->saveToAsciiSafeString(), + ]), + 'attributes' => [], + ])); + + $mockTransClient = $this->createMock(TransClient::class); + $mockTransClient->expects(self::exactly(3)) + ->method('getSecret') + ->with('secret-name') + ->willThrowException(new ClientException('something failed', 500)); + $mockTransClient->expects(self::never())->method('setSecret'); + + /** @var GenericAKVWrapper|MockObject $mockWrapper */ + $mockWrapper = $this->getMockBuilder(GenericAKVWrapper::class) + ->setConstructorArgs([ + new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ), + ]) + ->onlyMethods(['getClient', 'getTransClient']) + ->getMock(); + $mockWrapper->expects(self::once()) + ->method('getClient') + ->willReturn($mockClient); + $mockWrapper->expects(self::exactly(4)) + ->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))); + } } From edd3a5b820d55c0b7b8a937eef451db27b6e5103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Wed, 5 Feb 2025 20:50:56 +0100 Subject: [PATCH 7/9] feat (csas-migration): Log both successful & failed secret migrations in trans AKV --- src/ObjectEncryptor.php | 11 +- src/ObjectEncryptorFactory.php | 19 ++- src/Wrapper/GenericAKVWrapper.php | 21 +++- .../AKVWrappersWithTransClientTest.php | 108 +++++++++++++++++- 4 files changed, 142 insertions(+), 17 deletions(-) diff --git a/src/ObjectEncryptor.php b/src/ObjectEncryptor.php index 2d7443f..d7908ad 100644 --- a/src/ObjectEncryptor.php +++ b/src/ObjectEncryptor.php @@ -35,6 +35,7 @@ use Keboola\ObjectEncryptor\Wrapper\ProjectWideAKVWrapper; use Keboola\ObjectEncryptor\Wrapper\ProjectWideGKMSWrapper; use Keboola\ObjectEncryptor\Wrapper\ProjectWideKMSWrapper; +use Psr\Log\LoggerInterface; use stdClass; use Throwable; @@ -47,7 +48,7 @@ class ObjectEncryptor private ?KmsClient $kmsClient = null; private ?KeyManagementServiceClient $gkmsClient = null; - public function __construct(EncryptorOptions $encryptorOptions) + public function __construct(EncryptorOptions $encryptorOptions, private readonly ?LoggerInterface $logger = null) { $this->encryptorOptions = $encryptorOptions; } @@ -622,14 +623,17 @@ private function getAKVWrappers( ): array { $wrappers = []; $wrapper = new GenericAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrappers[] = $wrapper; if ($this->encryptorOptions->getStackId()) { if ($projectId) { $wrapper = new ProjectWideAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrapper->setProjectId($projectId); $wrappers[] = $wrapper; if ($branchType) { $wrapper = new BranchTypeProjectWideAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrapper->setProjectId($projectId); $wrapper->setBranchType($branchType); $wrappers[] = $wrapper; @@ -637,21 +641,25 @@ private function getAKVWrappers( } if ($componentId) { $wrapper = new ComponentAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrapper->setComponentId($componentId); $wrappers[] = $wrapper; if ($projectId) { $wrapper = new ProjectAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrapper->setComponentId($componentId); $wrapper->setProjectId($projectId); $wrappers[] = $wrapper; if ($configurationId) { $wrapper = new ConfigurationAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrapper->setComponentId($componentId); $wrapper->setProjectId($projectId); $wrapper->setConfigurationId($configurationId); $wrappers[] = $wrapper; if ($branchType) { $wrapper = new BranchTypeConfigurationAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrapper->setComponentId($componentId); $wrapper->setProjectId($projectId); $wrapper->setConfigurationId($configurationId); @@ -661,6 +669,7 @@ private function getAKVWrappers( } if ($branchType) { $wrapper = new BranchTypeProjectAKVWrapper($this->encryptorOptions); + $wrapper->logger = $this->logger; $wrapper->setComponentId($componentId); $wrapper->setProjectId($projectId); $wrapper->setBranchType($branchType); diff --git a/src/ObjectEncryptorFactory.php b/src/ObjectEncryptorFactory.php index b702679..205231f 100644 --- a/src/ObjectEncryptorFactory.php +++ b/src/ObjectEncryptorFactory.php @@ -4,6 +4,8 @@ namespace Keboola\ObjectEncryptor; +use Psr\Log\LoggerInterface; + class ObjectEncryptorFactory { /** @@ -28,10 +30,13 @@ public static function getAwsEncryptor( * @param non-empty-string $keyVaultUrl * @return ObjectEncryptor */ - public static function getAzureEncryptor(string $stackId, string $keyVaultUrl): ObjectEncryptor - { + public static function getAzureEncryptor( + string $stackId, + string $keyVaultUrl, + ?LoggerInterface $logger = null, + ): ObjectEncryptor { $encryptOptions = new EncryptorOptions($stackId, null, null, null, $keyVaultUrl); - return self::getEncryptor($encryptOptions); + return self::getEncryptor($encryptOptions, $logger); } /** @@ -45,8 +50,10 @@ public static function getGcpEncryptor(string $stackId, string $gkmsKeyId): Obje return self::getEncryptor($encryptOptions); } - public static function getEncryptor(EncryptorOptions $encryptorOptions): ObjectEncryptor - { - return new ObjectEncryptor($encryptorOptions); + public static function getEncryptor( + EncryptorOptions $encryptorOptions, + ?LoggerInterface $logger = null, + ): ObjectEncryptor { + return new ObjectEncryptor($encryptorOptions, $logger); } } diff --git a/src/Wrapper/GenericAKVWrapper.php b/src/Wrapper/GenericAKVWrapper.php index 58fd320..1704342 100644 --- a/src/Wrapper/GenericAKVWrapper.php +++ b/src/Wrapper/GenericAKVWrapper.php @@ -19,6 +19,7 @@ use Keboola\ObjectEncryptor\Temporary\CallbackRetryPolicy; use Keboola\ObjectEncryptor\Temporary\TransClient; use Keboola\ObjectEncryptor\Temporary\TransClientNotAvailableException; +use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Retry\BackOff\ExponentialBackOffPolicy; use Retry\Policy\RetryPolicyInterface; @@ -44,6 +45,7 @@ class GenericAKVWrapper implements CryptoWrapperInterface private TransClient|false|null $transClient = null; private ?string $encryptorId = null; + public ?LoggerInterface $logger = null; public function __construct(EncryptorOptions $encryptorOptions) { @@ -197,12 +199,12 @@ public function decrypt(string $encryptedData): string if ($decryptedContext !== null && isset($this->metadata['stackId']) && self::getTransStackId()) { $metadata['stackId'] = self::getTransStackId(); } - } catch (ClientException $e) { - if ($e->getCode() === 404) { + } catch (Throwable $e) { + if ($e instanceof ClientException && $e->getCode() === 404) { $doBackfill = true; + } else { + throw new ApplicationException('Deciphering failed.', $e->getCode(), $e); } - } catch (Throwable) { - // intentionally suppress all errors to prevent decrypt() from failing } } @@ -244,8 +246,17 @@ public function decrypt(string $encryptedData): string $encrypted[self::SECRET_NAME], ); }); - } catch (Throwable) { + $this->logger?->info('Secret "{secretName}" migrated in {stackId} AKV.', [ + 'secretName' => $encrypted[self::SECRET_NAME], + 'stackId' => self::getTransStackId() ?? 'trans', + ]); + } catch (Throwable $e) { // intentionally suppress all errors to prevent decrypt() from failing + $this->logger?->error('Migration of secret "{secretName}" in {stackId} AKV failed.', [ + 'secretName' => $encrypted[self::SECRET_NAME], + 'stackId' => self::getTransStackId() ?? 'trans', + 'exception' => $e, + ]); } } } diff --git a/tests/Temporary/AKVWrappersWithTransClientTest.php b/tests/Temporary/AKVWrappersWithTransClientTest.php index 49f2c7d..7507c9a 100644 --- a/tests/Temporary/AKVWrappersWithTransClientTest.php +++ b/tests/Temporary/AKVWrappersWithTransClientTest.php @@ -11,6 +11,9 @@ use Keboola\AzureKeyVaultClient\Requests\SetSecretRequest; use Keboola\AzureKeyVaultClient\Responses\SecretBundle; use Keboola\ObjectEncryptor\EncryptorOptions; +use Keboola\ObjectEncryptor\Exception\ApplicationException; +use Keboola\ObjectEncryptor\ObjectEncryptor; +use Keboola\ObjectEncryptor\ObjectEncryptorFactory; use Keboola\ObjectEncryptor\Temporary\TransClient; use Keboola\ObjectEncryptor\Wrapper\BranchTypeConfigurationAKVWrapper; use Keboola\ObjectEncryptor\Wrapper\BranchTypeProjectAKVWrapper; @@ -22,6 +25,8 @@ use Keboola\ObjectEncryptor\Wrapper\ProjectWideAKVWrapper; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\Test\TestLogger; +use ReflectionClass; class AKVWrappersWithTransClientTest extends TestCase { @@ -259,6 +264,8 @@ public function testDecryptWithBackfillWhenSecretNotFoundInTransVault( 'secret-name', ); + $logger = new TestLogger(); + /** @var GenericAKVWrapper|MockObject $mockWrapper */ $mockWrapper = $this->getMockBuilder($wrapperClass) ->setConstructorArgs([ @@ -269,6 +276,7 @@ public function testDecryptWithBackfillWhenSecretNotFoundInTransVault( ]) ->onlyMethods(['getClient', 'getTransClient']) ->getMock(); + $mockWrapper->logger = $logger; foreach ($metadata as $metaKey => $metaValue) { $mockWrapper->setMetadataValue($metaKey, $metaValue); } @@ -288,6 +296,13 @@ public function testDecryptWithBackfillWhenSecretNotFoundInTransVault( $secret = $mockWrapper->decrypt($encryptedSecret); self::assertSame('something very secret', $secret); + self::assertTrue($logger->hasInfo([ + 'message' => 'Secret "{secretName}" migrated in {stackId} AKV.', + 'context' => [ + 'secretName' => 'secret-name', + 'stackId' => 'trans-stack', + ], + ])); } /** @@ -343,8 +358,49 @@ public function testDecryptOmitsPrimaryVaultWhenSecretFoundInTransVault( self::assertSame('something very secret', $secret); } - public function testRetrieveFromPrimaryVaultWhenTransVaultRetrievalFails(): void + public function testTransVaultGetSecretFails(): void + { + $key = Key::createNewRandomKey(); + + $mockTransClient = $this->createMock(TransClient::class); + $mockTransClient->expects(self::exactly(3)) + ->method('getSecret') + ->with('secret-name') + ->willThrowException(new ClientException('something failed', 500)); + $mockTransClient->expects(self::never())->method('setSecret'); + + /** @var GenericAKVWrapper|MockObject $mockWrapper */ + $mockWrapper = $this->getMockBuilder(GenericAKVWrapper::class) + ->setConstructorArgs([ + new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ), + ]) + ->onlyMethods(['getClient', 'getTransClient']) + ->getMock(); + $mockWrapper->expects(self::never())->method('getClient'); + $mockWrapper->expects(self::exactly(4)) + ->method('getTransClient') + ->willReturn($mockTransClient); + + $encryptedSecret = self::encode([ + 2 => Crypto::encrypt('something very secret', $key, true), + 3 => 'secret-name', + 4 => 'secret-version', + ]); + + $this->expectException(ApplicationException::class); + $this->expectExceptionCode(500); + $this->expectExceptionMessage('Deciphering failed.'); + + $mockWrapper->decrypt($encryptedSecret); + } + + public function testLogErrorWhenTransVaultSetSecretFails(): void { + putenv('TRANS_ENCRYPTOR_STACK_ID=trans-stack'); + $key = Key::createNewRandomKey(); $mockClient = $this->createMock(Client::class); @@ -361,11 +417,17 @@ public function testRetrieveFromPrimaryVaultWhenTransVaultRetrievalFails(): void ])); $mockTransClient = $this->createMock(TransClient::class); - $mockTransClient->expects(self::exactly(3)) + $mockTransClient->expects(self::once()) ->method('getSecret') ->with('secret-name') - ->willThrowException(new ClientException('something failed', 500)); - $mockTransClient->expects(self::never())->method('setSecret'); + ->willThrowException(new ClientException('not found', 404)); + + $setSecretException = new ClientException('something failed', 500); + $mockTransClient->expects(self::exactly(3)) + ->method('setSecret') + ->willThrowException($setSecretException); + + $logger = new TestLogger(); /** @var GenericAKVWrapper|MockObject $mockWrapper */ $mockWrapper = $this->getMockBuilder(GenericAKVWrapper::class) @@ -377,10 +439,11 @@ public function testRetrieveFromPrimaryVaultWhenTransVaultRetrievalFails(): void ]) ->onlyMethods(['getClient', 'getTransClient']) ->getMock(); + $mockWrapper->logger = $logger; $mockWrapper->expects(self::once()) ->method('getClient') ->willReturn($mockClient); - $mockWrapper->expects(self::exactly(4)) + $mockWrapper->expects(self::exactly(5)) ->method('getTransClient') ->willReturn($mockTransClient); @@ -393,6 +456,41 @@ public function testRetrieveFromPrimaryVaultWhenTransVaultRetrievalFails(): void $secret = $mockWrapper->decrypt($encryptedSecret); self::assertSame('something very secret', $secret); + self::assertTrue($logger->hasError([ + 'message' => 'Migration of secret "{secretName}" in {stackId} AKV failed.', + 'context' => [ + 'secretName' => 'secret-name', + 'stackId' => 'trans-stack', + 'exception' => $setSecretException, + ], + ])); + } + + public function testObjectEncryptorFactoryInjectsLoggerInAKVWrappers(): void + { + $logger = new TestLogger(); + + $encryptor = ObjectEncryptorFactory::getEncryptor( + new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ), + $logger, + ); + + $reflection = new ReflectionClass(ObjectEncryptor::class); + $method = $reflection->getMethod('getWrappers'); + $method->setAccessible(true); + + /** @var array $wrappers */ + $wrappers = $method->invokeArgs($encryptor, ['component-id', 'project-id', 'config-id', 'branch-type']); + + self::assertIsArray($wrappers); + self::assertCount(8, $wrappers); + + foreach ($wrappers as $wrapper) { + self::assertSame($logger, $wrapper->logger); + } } private static function encode(mixed $data): string From 01eb3061deff6eca9625cc3bb0ef7848d2e2164d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Thu, 6 Feb 2025 12:43:04 +0100 Subject: [PATCH 8/9] feat (csas-migration): Skip backfill and log error if TRANS_ENCRYPTOR_STACK_ID env is not set --- src/Wrapper/GenericAKVWrapper.php | 13 +++-- .../AKVWrappersWithTransClientTest.php | 58 +++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/Wrapper/GenericAKVWrapper.php b/src/Wrapper/GenericAKVWrapper.php index 1704342..a386694 100644 --- a/src/Wrapper/GenericAKVWrapper.php +++ b/src/Wrapper/GenericAKVWrapper.php @@ -38,6 +38,7 @@ class GenericAKVWrapper implements CryptoWrapperInterface private const PAYLOAD_INDEX = 2; private const SECRET_NAME = 3; private const SECRET_VERSION = 4; + private const TRANS_STACK_ID_ENV = 'TRANS_ENCRYPTOR_STACK_ID'; private array $metadata = []; private string $keyVaultURL; @@ -88,7 +89,7 @@ public function getTransClient(): ?TransClient private static function getTransStackId(): ?string { - return (string) getenv('TRANS_ENCRYPTOR_STACK_ID') ?: null; + return (string) getenv(self::TRANS_STACK_ID_ENV) ?: null; } private function getRetryProxy(?RetryPolicyInterface $retryPolicy = null): RetryProxy @@ -234,8 +235,12 @@ public function decrypt(string $encryptedData): string $doBackfill = false; throw new ApplicationException('Deciphering failed.', $e->getCode(), $e); } finally { + if (!self::getTransStackId()) { + $doBackfill = false; + $this->logger?->error(sprintf('Env %s not set.', self::TRANS_STACK_ID_ENV)); + } if ($doBackfill) { - if (isset($this->metadata['stackId']) && self::getTransStackId()) { + if (isset($this->metadata['stackId'])) { $decryptedContext[self::METADATA_INDEX]['stackId'] = self::getTransStackId(); } try { @@ -248,13 +253,13 @@ public function decrypt(string $encryptedData): string }); $this->logger?->info('Secret "{secretName}" migrated in {stackId} AKV.', [ 'secretName' => $encrypted[self::SECRET_NAME], - 'stackId' => self::getTransStackId() ?? 'trans', + 'stackId' => self::getTransStackId(), ]); } catch (Throwable $e) { // intentionally suppress all errors to prevent decrypt() from failing $this->logger?->error('Migration of secret "{secretName}" in {stackId} AKV failed.', [ 'secretName' => $encrypted[self::SECRET_NAME], - 'stackId' => self::getTransStackId() ?? 'trans', + 'stackId' => self::getTransStackId(), 'exception' => $e, ]); } diff --git a/tests/Temporary/AKVWrappersWithTransClientTest.php b/tests/Temporary/AKVWrappersWithTransClientTest.php index 7507c9a..2bfe0f0 100644 --- a/tests/Temporary/AKVWrappersWithTransClientTest.php +++ b/tests/Temporary/AKVWrappersWithTransClientTest.php @@ -466,6 +466,64 @@ public function testLogErrorWhenTransVaultSetSecretFails(): void ])); } + public function testSkipBackfillWhenTransStackIdEnvNotSet(): void + { + putenv('TRANS_ENCRYPTOR_STACK_ID='); + + $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 => [], + 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::never())->method('setSecret'); + + $logger = new TestLogger(); + + /** @var GenericAKVWrapper|MockObject $mockWrapper */ + $mockWrapper = $this->getMockBuilder(GenericAKVWrapper::class) + ->setConstructorArgs([ + new EncryptorOptions( + stackId: 'some-stack', + akvUrl: 'some-url', + ), + ]) + ->onlyMethods(['getClient', 'getTransClient']) + ->getMock(); + $mockWrapper->logger = $logger; + $mockWrapper->expects(self::once()) + ->method('getClient') + ->willReturn($mockClient); + $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); + self::assertTrue($logger->hasError('Env TRANS_ENCRYPTOR_STACK_ID not set.')); + } + public function testObjectEncryptorFactoryInjectsLoggerInAKVWrappers(): void { $logger = new TestLogger(); From d421583a5e3dbb3a8de0ceacab9ed4be11fc62ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pi=C5=A1t=C4=9Bk?= Date: Mon, 10 Feb 2025 14:54:39 +0100 Subject: [PATCH 9/9] fix (csas-migration): Fix TRANS_ENCRYPTOR_STACK_ID env check --- src/Wrapper/GenericAKVWrapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Wrapper/GenericAKVWrapper.php b/src/Wrapper/GenericAKVWrapper.php index a386694..fa98c09 100644 --- a/src/Wrapper/GenericAKVWrapper.php +++ b/src/Wrapper/GenericAKVWrapper.php @@ -235,7 +235,7 @@ public function decrypt(string $encryptedData): string $doBackfill = false; throw new ApplicationException('Deciphering failed.', $e->getCode(), $e); } finally { - if (!self::getTransStackId()) { + if ($doBackfill && !self::getTransStackId()) { $doBackfill = false; $this->logger?->error(sprintf('Env %s not set.', self::TRANS_STACK_ID_ENV)); }