Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement support for AWS SecretsManager #4

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Implement support for AWS SecretsManager #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9eda652
Implement support for AWS SecretsManager
natepage Aug 8, 2023
269868d
Fix test and cs
natepage Aug 8, 2023
13e6914
Fix minimum version for symfony/polyfill-uuid
natepage Aug 8, 2023
6b8ceba
Implement json processor for bref-secretsmanager
natepage Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
"async-aws/ssm": "^1.3"
},
"require-dev": {
"async-aws/secrets-manager": "^1.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it OK to put this in the require-dev instead of in the require section?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@f-lombardo my thinking was people don't need this dependency unless they are working with secrets manager, and because ssm is the default implementation of this library it's most likely to be the only use in a majority of cases. But I do not have strong opinions about it 😄

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO since async-aws/ssm is in the require section, the same should hold for async-aws/secrets-manager

"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": {
Expand Down
127 changes: 107 additions & 20 deletions src/Secrets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Bref\Secrets;

use AsyncAws\SecretsManager\SecretsManagerClient;
use AsyncAws\Ssm\SsmClient;
use Closure;
use JsonException;
Expand All @@ -13,63 +14,91 @@ 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,string>|string|false $envVars */
$envVars = getenv(local_only: true); // @phpstan-ignore-line PHPStan is wrong
if (! is_array($envVars)) {
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;
}

// 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);
$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:'));
}
}

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, $secretsManagerNames);
});

foreach ($secretsManagerParameters as $parameterName => $parameterValue) {
$_SERVER[$parameterName] = $_ENV[$parameterName] = $parameterValue;
putenv("$parameterName=$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);
}
}

/**
* 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<string, string> $paramResolver
* @return array<string, string> 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)) {
Expand All @@ -86,6 +115,64 @@ private static function readParametersFromCacheOr(Closure $paramResolver): array
return $parameters;
}

/**
* @param string[] $secretNames
* @return array<string, string> Map of parameter name -> value
* @throws JsonException
*/
private static function retrieveParametersFromSecretsManager(
?SecretsManagerClient $secretsManagerClient,
array $secretNames
): array {
$secretsManager = $secretsManagerClient ?? new SecretsManagerClient([
'region' => $_ENV['AWS_REGION'] ?? $_ENV['AWS_DEFAULT_REGION'],
]);

/** @var array<string, string> $parameters Map of parameter name -> value */
$parameters = [];

foreach ($secretNames as $originalEnvVar => $secretId) {
$isJson = false;

if (str_starts_with($secretId, 'json:')) {
$isJson = true;
$secretId = substr($secretId, strlen('json:'));
}

try {
$result = $secretsManager->getSecretValue([
'SecretId' => $secretId,
]);
$secretString = $result->getSecretString();
} catch (RuntimeException $e) {
if ($e->getCode() === 400) {
// Extra descriptive error message for the most common error
throw new RuntimeException(
"Bref was not able to resolve secrets contained in environment variables from SecretsManager because of a permissions issue with the SecretsManager API. Did you add IAM permissions in serverless.yml to allow Lambda to access SecretsManager? (docs: https://bref.sh/docs/environment/variables.html#at-deployment-time).\nFull exception message: {$e->getMessage()}",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be cool to try to get all the secret values first and afterwards throw the error (including indicating which secrets have failed).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of a permission issue it's more likely that permissions are not good for all secrets. If not, that's not a common case, I'm fine keeping it simple honestly (easier to maintain).

$e->getCode(),
$e,
);
}
throw $e;
}

// If json processor not set, replace original env var with secretString value
if ($isJson === false) {
$parameters[$originalEnvVar] = $secretString;

continue;
}

// Otherwise JSON decode secretString and add parameters from decoded array
$secretValues = json_decode($secretString, true, 512, JSON_THROW_ON_ERROR);
foreach ($secretValues as $name => $value) {
$parameters[$name] = $value;
}
}

return $parameters;
}

/**
* @param string[] $ssmNames
* @return array<string, string> Map of parameter name -> value
Expand Down
117 changes: 117 additions & 0 deletions tests/SecretsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -36,8 +41,99 @@ public function test decrypts env variables(): void
$this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));
}

public function test decrypts env variables from secretsmanager not json(): 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('{"SOME_VARIABLE_1":"foobar_1","SOME_VARIABLE_2":"foobar_2"}', 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 secretsmanager(): void
{
putenv('SOME_VARIABLE=bref-secretsmanager:json:/some/parameter');
putenv('SOME_OTHER_VARIABLE=helloworld');

// Sanity checks
$this->assertSame('bref-secretsmanager:json:/some/parameter', getenv('SOME_VARIABLE'));
$this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));

Secrets::loadSecretEnvironmentVariables(null, $this->mockSecretsManagerClient());

$this->assertSame('foobar_1', getenv('SOME_VARIABLE_1'));
$this->assertSame('foobar_1', $_SERVER['SOME_VARIABLE_1']);
$this->assertSame('foobar_1', $_ENV['SOME_VARIABLE_1']);
$this->assertSame('foobar_2', getenv('SOME_VARIABLE_2'));
$this->assertSame('foobar_2', $_SERVER['SOME_VARIABLE_2']);
$this->assertSame('foobar_2', $_ENV['SOME_VARIABLE_2']);
// 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 secretsmanager(): void
{
putenv('SOME_VARIABLE=bref-ssm:/some/parameter');
putenv('SOME_VARIABLE_1=bref-secretsmanager:json:/some/parameter');
putenv('SOME_OTHER_VARIABLE=helloworld');

// Sanity checks
$this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE'));
$this->assertSame('bref-secretsmanager:json:/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 secretsmanager
$this->assertSame('foobar_1', getenv('SOME_VARIABLE_1'));
$this->assertSame('foobar_1', $_SERVER['SOME_VARIABLE_1']);
$this->assertSame('foobar_1', $_ENV['SOME_VARIABLE_1']);
$this->assertSame('foobar_2', getenv('SOME_VARIABLE_2'));
$this->assertSame('foobar_2', $_SERVER['SOME_VARIABLE_2']);
$this->assertSame('foobar_2', $_ENV['SOME_VARIABLE_2']);
// Check that the other variable was not modified
$this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));
}

public function test env variables from secretsmanager overrides ssm(): void
{
putenv('SOME_VARIABLE=bref-ssm:/some/parameter');
putenv('SOME_VARIABLE_1=bref-secretsmanager:json:/some/parameter');
putenv('SOME_OTHER_VARIABLE=helloworld');

// Sanity checks
$this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE'));
$this->assertSame('bref-secretsmanager:json:/some/parameter', getenv('SOME_VARIABLE_1'));
$this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));

Secrets::loadSecretEnvironmentVariables(
$this->mockSsmClient(),
$this->mockSecretsManagerClient('{"SOME_VARIABLE":"foobar_from_secretsmanager"}')
);

// Check value from ssm
$this->assertSame('foobar_from_secretsmanager', getenv('SOME_VARIABLE'));
$this->assertSame('foobar_from_secretsmanager', $_SERVER['SOME_VARIABLE']);
$this->assertSame('foobar_from_secretsmanager', $_ENV['SOME_VARIABLE']);
// 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
Expand All @@ -64,6 +160,27 @@ public function test throws a clear error message on missing permissions
Secrets::loadSecretEnvironmentVariables($ssmClient);
}

private function mockSecretsManagerClient(?string $secretString = null): SecretsManagerClient
{
$secretsManagerClient = $this->getMockBuilder(SecretsManagerClient::class)
->disableOriginalConstructor()
->onlyMethods(['getSecretValue'])
->getMock();

$result = ResultMockFactory::create(GetSecretValueResponse::class, [
'SecretString' => $secretString ?? '{"SOME_VARIABLE_1":"foobar_1","SOME_VARIABLE_2":"foobar_2"}',
]);

$secretsManagerClient->expects($this->once())
->method('getSecretValue')
->with([
'SecretId' => '/some/parameter',
])
->willReturn($result);

return $secretsManagerClient;
}

private function mockSsmClient(): SsmClient
{
$ssmClient = $this->getMockBuilder(SsmClient::class)
Expand Down