From 95e609c5a7aeb3942be3fcbdf27320e725837cce Mon Sep 17 00:00:00 2001 From: Julian Hundeloh <5358638+jaulz@users.noreply.github.com> Date: Mon, 25 Sep 2023 22:35:12 +0200 Subject: [PATCH 1/4] feat: add support for secrets --- composer.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index fac18e5..c41c605 100644 --- a/composer.json +++ b/composer.json @@ -19,9 +19,11 @@ "async-aws/ssm": "^1.3" }, "require-dev": { + "async-aws/secrets-manager": "^1.0", "phpunit/phpunit": "^9.6.10", "mnapoli/hard-mode": "^0.3.0", - "phpstan/phpstan": "^1.10.26" + "phpstan/phpstan": "^1.10.26", + "symfony/polyfill-uuid": "^1.13.1" }, "config": { "allow-plugins": { From bbd7fc2fb693f1b9d3d2affffe0e9cea18d0256e Mon Sep 17 00:00:00 2001 From: Julian Hundeloh <5358638+jaulz@users.noreply.github.com> Date: Mon, 25 Sep 2023 20:52:55 +0000 Subject: [PATCH 2/4] feat: implement support for AWS Secrets Manager --- src/Secrets.php | 119 +++++++++++++++++++++++++++++++++++------- tests/SecretsTest.php | 88 ++++++++++++++++++++++++++++++- 2 files changed, 186 insertions(+), 21 deletions(-) diff --git a/src/Secrets.php b/src/Secrets.php index 639e815..93a3e15 100644 --- a/src/Secrets.php +++ b/src/Secrets.php @@ -2,8 +2,10 @@ namespace Bref\Secrets; +use AsyncAws\SecretsManager\SecretsManagerClient; use AsyncAws\Ssm\SsmClient; use Closure; +use Exception; use JsonException; use RuntimeException; @@ -13,9 +15,10 @@ class Secrets * Decrypt environment variables that are encrypted with AWS SSM. * * @param SsmClient|null $ssmClient To allow mocking in tests. + * @param SecretsManagerClient|null $secretsManagerClient To allow mocking in tests. * @throws JsonException */ - public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = null): void + public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = null, ?SecretsManagerClient $secretsManagerClient = null): void { /** @var array|string|false $envVars */ $envVars = getenv(local_only: true); // @phpstan-ignore-line PHPStan is wrong @@ -23,36 +26,64 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu return; } - // Only consider environment variables that start with "bref-ssm:" + // Only consider environment variables that start with "bref-ssm:" or "bref-secretsmanager:" $envVarsToDecrypt = array_filter($envVars, function (string $value): bool { - return str_starts_with($value, 'bref-ssm:'); + return str_starts_with($value, 'bref-ssm:') || str_starts_with($value, 'bref-secretsmanager:'); }); if (empty($envVarsToDecrypt)) { return; } + + $ssmNames = []; + $secretsManagerNames = []; + + // Extract the SSM and SecretsManager parameter names by removing the prefixes + foreach ($envVarsToDecrypt as $key => $envVar) { + if (str_starts_with($envVar, 'bref-ssm:')) { + $ssmNames[$key] = substr($envVar, strlen('bref-ssm:')); + } + if (str_starts_with($envVar, 'bref-secretsmanager:')) { + $secretsManagerNames[$key] = substr($envVar, strlen('bref-secretsmanager:')); + } + } - // Extract the SSM parameter names by removing the "bref-ssm:" prefix - $ssmNames = array_map(function (string $value): string { - return substr($value, strlen('bref-ssm:')); - }, $envVarsToDecrypt); + if (count($secretsManagerNames) > 0 && class_exists(SecretsManagerClient::class) === false) { + throw new RuntimeException('In order to load secrets from SecretsManager you must install "async-aws/secrets-manager" package'); + } $actuallyCalledSsm = false; - $parameters = self::readParametersFromCacheOr(function () use ($ssmClient, $ssmNames, &$actuallyCalledSsm) { - $actuallyCalledSsm = true; - return self::retrieveParametersFromSsm($ssmClient, array_values($ssmNames)); - }); + if (count($ssmNames) > 0) { + $ssmParameters = self::readParametersFromCacheOr('ssm', function () use ($ssmClient, $ssmNames, &$actuallyCalledSsm) { + $actuallyCalledSsm = true; + return self::retrieveParametersFromSsm($ssmClient, array_values($ssmNames)); + }); + + foreach ($ssmParameters as $parameterName => $parameterValue) { + $envVar = array_search($parameterName, $ssmNames, true); + $_SERVER[$envVar] = $_ENV[$envVar] = $parameterValue; + putenv("$envVar=$parameterValue"); + } + } - foreach ($parameters as $parameterName => $parameterValue) { - $envVar = array_search($parameterName, $ssmNames, true); - $_SERVER[$envVar] = $_ENV[$envVar] = $parameterValue; - putenv("$envVar=$parameterValue"); + $actuallyCalledSecretsManager = false; + if (count($secretsManagerNames) > 0) { + $secretsManagerParameters = self::readParametersFromCacheOr('secretsmanager', function () use ($secretsManagerClient, $secretsManagerNames, &$actuallyCalledSecretsManager) { + $actuallyCalledSecretsManager = true; + return self::retrieveParametersFromSecretsManager($secretsManagerClient, array_values($secretsManagerNames)); + }); + + foreach ($secretsManagerParameters as $parameterName => $parameterValue) { + $envVar = array_search($parameterName, $secretsManagerNames, true); + $_SERVER[$envVar] = $_ENV[$envVar] = $parameterValue; + putenv("$envVar=$parameterValue"); + } } // Only log once (when the cache was empty) else it might spam the logs in the function runtime // (where the process restarts on every invocation) - if ($actuallyCalledSsm) { + if ($actuallyCalledSsm || $actuallyCalledSecretsManager) { $stderr = fopen('php://stderr', 'ab'); - fwrite($stderr, '[Bref] Loaded these environment variables from SSM: ' . implode(', ', array_keys($envVarsToDecrypt)) . PHP_EOL); + fwrite($stderr, '[Bref] Loaded these environment variables from SSM/SecretsManager: ' . implode(', ', array_keys($envVarsToDecrypt)) . PHP_EOL); } } @@ -60,16 +91,16 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu * Cache the parameters in a temp file. * Why? Because on the function runtime, the PHP process might * restart on every invocation (or on error), so we don't want to - * call SSM every time. + * call SSM/SecretsManager every time. * * @param Closure(): array $paramResolver * @return array Map of parameter name -> value * @throws JsonException */ - private static function readParametersFromCacheOr(Closure $paramResolver): array + private static function readParametersFromCacheOr(string $paramType, Closure $paramResolver): array { // Check in cache first - $cacheFile = sys_get_temp_dir() . '/bref-ssm-parameters.php'; + $cacheFile = sprintf('%s/bref-%s-parameters.php', sys_get_temp_dir(), $paramType); if (is_file($cacheFile)) { $parameters = json_decode(file_get_contents($cacheFile), true, 512, JSON_THROW_ON_ERROR); if (is_array($parameters)) { @@ -86,6 +117,54 @@ private static function readParametersFromCacheOr(Closure $paramResolver): array return $parameters; } + /** + * @param string[] $secretIds + * @return array Map of parameter name -> value + * @throws JsonException + */ + private static function retrieveParametersFromSecretsManager( + ?SecretsManagerClient $secretsManagerClient, + array $secretIds + ): array { + if (! class_exists(SecretsManagerClient::class)) { + throw new Exception('The "async-aws/secrets-manager" package is required to load secrets from Secrets Manager via the "bref-secretsmanager:xxx" syntax in environment variables. Please add it to your "require" section in composer.json.'); + } + + $secretsManager = $secretsManagerClient ?? new SecretsManagerClient([ + 'region' => $_ENV['AWS_REGION'] ?? $_ENV['AWS_DEFAULT_REGION'], + ]); + + /** @var array $parameters Map of parameter name -> value */ + $parameters = []; + $parametersNotFound = []; + + foreach ($secretIds as $secretId) { + try { + $result = $secretsManager->getSecretValue([ + 'SecretId' => $secretId, + ]); + $secretString = $result->getSecretString(); + + $parameters[$secretId] = $secretString; + } catch (RuntimeException $e) { + $parametersNotFound[$secretId] = $e; + } + } + + if (count($parametersNotFound) > 0) { + array_walk($parametersNotFound, function(&$value, $key) { + $message = $value->getMessage(); + $value = "$key ($message)"; + }); + + throw new RuntimeException( + 'The following SecretsManager parameters could not be found: ' . implode(', ', $parametersNotFound) .'. Did you add IAM permissions in serverless.yml to allow Lambda to access SecretsManager?', + ); + } + + return $parameters; + } + /** * @param string[] $ssmNames * @return array Map of parameter name -> value diff --git a/tests/SecretsTest.php b/tests/SecretsTest.php index 6792ace..e2587e9 100644 --- a/tests/SecretsTest.php +++ b/tests/SecretsTest.php @@ -3,6 +3,8 @@ namespace Bref\Secrets\Test; use AsyncAws\Core\Test\ResultMockFactory; +use AsyncAws\SecretsManager\Result\GetSecretValueResponse; +use AsyncAws\SecretsManager\SecretsManagerClient; use AsyncAws\Ssm\Result\GetParametersResult; use AsyncAws\Ssm\SsmClient; use AsyncAws\Ssm\ValueObject\Parameter; @@ -16,6 +18,9 @@ public function setUp(): void if (file_exists(sys_get_temp_dir() . '/bref-ssm-parameters.php')) { unlink(sys_get_temp_dir() . '/bref-ssm-parameters.php'); } + if (file_exists(sys_get_temp_dir() . '/bref-secretsmanager-parameters.php')) { + unlink(sys_get_temp_dir() . '/bref-secretsmanager-parameters.php'); + } } public function test decrypts env variables(): void @@ -36,8 +41,54 @@ public function test decrypts env variables(): void $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); } + public function test decrypts env variables from Secrets Manager(): void + { + putenv('SOME_VARIABLE=bref-secretsmanager:/some/parameter'); + putenv('SOME_OTHER_VARIABLE=helloworld'); + + // Sanity checks + $this->assertSame('bref-secretsmanager:/some/parameter', getenv('SOME_VARIABLE')); + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + + Secrets::loadSecretEnvironmentVariables(null, $this->mockSecretsManagerClient()); + + $this->assertSame('foobar', getenv('SOME_VARIABLE')); + + // Check that the other variable was not modified + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + } + + public function test decrypts env variables from both SSM and Secrets Manager(): void + { + putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); + putenv('SOME_VARIABLE_1=bref-secretsmanager:/some/parameter'); + putenv('SOME_OTHER_VARIABLE=helloworld'); + + // Sanity checks + $this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE')); + $this->assertSame('bref-secretsmanager:/some/parameter', getenv('SOME_VARIABLE_1')); + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + + Secrets::loadSecretEnvironmentVariables($this->mockSsmClient(), $this->mockSecretsManagerClient()); + + // Check value from SSM + $this->assertSame('foobar', getenv('SOME_VARIABLE')); + $this->assertSame('foobar', $_SERVER['SOME_VARIABLE']); + $this->assertSame('foobar', $_ENV['SOME_VARIABLE']); + + // Check value from Secrets Manager + $this->assertSame('foobar', getenv('SOME_VARIABLE_1')); + $this->assertSame('foobar', $_SERVER['SOME_VARIABLE_1']); + $this->assertSame('foobar', $_ENV['SOME_VARIABLE_1']); + + // Check that the other variable was not modified + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + } + public function test caches parameters to call SSM only once(): void { + // Unset this env variable to prevent calling SecretsManager client because of previous test + putenv('SOME_VARIABLE_1'); putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); // Call twice, the mock will assert that SSM was only called once @@ -48,6 +99,20 @@ public function test caches parameters to call SSM only once(): void $this->assertSame('foobar', getenv('SOME_VARIABLE')); } + public function test caches parameters to call Secrets Manager only once(): void + { + // Unset this env variable to prevent calling SecretsManager client because of previous test + putenv('SOME_VARIABLE_1'); + putenv('SOME_VARIABLE=bref-secretsmanager:/some/parameter'); + + // Call twice, the mock will assert that Secrets Manager was only called once + $secretsManagerClient = $this->mockSecretsManagerClient(); + Secrets::loadSecretEnvironmentVariables(null, $secretsManagerClient); + Secrets::loadSecretEnvironmentVariables(null, $secretsManagerClient); + + $this->assertSame('foobar', getenv('SOME_VARIABLE')); + } + public function test throws a clear error message on missing permissions(): void { putenv('SOME_VARIABLE=bref-ssm:/app/test'); @@ -64,6 +129,27 @@ public function test throws a clear error message on missing permissions Secrets::loadSecretEnvironmentVariables($ssmClient); } + private function mockSecretsManagerClient(): SecretsManagerClient + { + $secretsManagerClient = $this->getMockBuilder(SecretsManagerClient::class) + ->disableOriginalConstructor() + ->onlyMethods(['getSecretValue']) + ->getMock(); + + $result = ResultMockFactory::create(GetSecretValueResponse::class, [ + 'SecretString' => 'foobar', + ]); + + $secretsManagerClient->expects($this->once()) + ->method('getSecretValue') + ->with([ + 'SecretId' => '/some/parameter', + ]) + ->willReturn($result); + + return $secretsManagerClient; + } + private function mockSsmClient(): SsmClient { $ssmClient = $this->getMockBuilder(SsmClient::class) @@ -90,4 +176,4 @@ private function mockSsmClient(): SsmClient return $ssmClient; } -} +} \ No newline at end of file From 79c8d6d3f4d9f301fdf7f9f8257fe97e43385f42 Mon Sep 17 00:00:00 2001 From: Julian Hundeloh <5358638+jaulz@users.noreply.github.com> Date: Mon, 25 Sep 2023 20:55:18 +0000 Subject: [PATCH 3/4] fix: fix namings --- src/Secrets.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Secrets.php b/src/Secrets.php index 93a3e15..38e091d 100644 --- a/src/Secrets.php +++ b/src/Secrets.php @@ -37,7 +37,7 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu $ssmNames = []; $secretsManagerNames = []; - // Extract the SSM and SecretsManager parameter names by removing the prefixes + // Extract the SSM and Secrets Manager parameter names by removing the prefixes foreach ($envVarsToDecrypt as $key => $envVar) { if (str_starts_with($envVar, 'bref-ssm:')) { $ssmNames[$key] = substr($envVar, strlen('bref-ssm:')); @@ -91,7 +91,7 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu * Cache the parameters in a temp file. * Why? Because on the function runtime, the PHP process might * restart on every invocation (or on error), so we don't want to - * call SSM/SecretsManager every time. + * call SSM/Secrets Manager every time. * * @param Closure(): array $paramResolver * @return array Map of parameter name -> value @@ -158,7 +158,7 @@ private static function retrieveParametersFromSecretsManager( }); throw new RuntimeException( - 'The following SecretsManager parameters could not be found: ' . implode(', ', $parametersNotFound) .'. Did you add IAM permissions in serverless.yml to allow Lambda to access SecretsManager?', + 'The following secrets from Secrets Manager could not be found: ' . implode(', ', $parametersNotFound) .'. Did you add IAM permissions in serverless.yml to allow Lambda to access Secrets Manager?', ); } From 686659f36349979a772c4c7149c8b22eecdc4cc6 Mon Sep 17 00:00:00 2001 From: Julian Hundeloh <5358638+jaulz@users.noreply.github.com> Date: Mon, 25 Sep 2023 20:59:24 +0000 Subject: [PATCH 4/4] test: remove obsolete lines from tests --- tests/SecretsTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/SecretsTest.php b/tests/SecretsTest.php index e2587e9..4b7439e 100644 --- a/tests/SecretsTest.php +++ b/tests/SecretsTest.php @@ -87,8 +87,6 @@ public function test decrypts env variables from both SSM and Secrets M public function test caches parameters to call SSM only once(): void { - // Unset this env variable to prevent calling SecretsManager client because of previous test - putenv('SOME_VARIABLE_1'); putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); // Call twice, the mock will assert that SSM was only called once @@ -101,8 +99,6 @@ public function test caches parameters to call SSM only once(): void public function test caches parameters to call Secrets Manager only once(): void { - // Unset this env variable to prevent calling SecretsManager client because of previous test - putenv('SOME_VARIABLE_1'); putenv('SOME_VARIABLE=bref-secretsmanager:/some/parameter'); // Call twice, the mock will assert that Secrets Manager was only called once