From 749f4f63e6800f5f4127c6a5f38cb2dd6648f906 Mon Sep 17 00:00:00 2001 From: Chris Kilding Date: Thu, 18 Apr 2019 12:58:50 +0100 Subject: [PATCH] Allow soft deletion of secrets --- moto/secretsmanager/exceptions.py | 7 ++ moto/secretsmanager/models.py | 41 +++++++-- .../test_secretsmanager.py | 87 +++++++++++++++---- 3 files changed, 110 insertions(+), 25 deletions(-) diff --git a/moto/secretsmanager/exceptions.py b/moto/secretsmanager/exceptions.py index a72a3264..5ee96f12 100644 --- a/moto/secretsmanager/exceptions.py +++ b/moto/secretsmanager/exceptions.py @@ -27,3 +27,10 @@ class InvalidParameterException(SecretsManagerClientError): super(InvalidParameterException, self).__init__( 'InvalidParameterException', message) + + +class InvalidRequestException(SecretsManagerClientError): + def __init__(self, message): + super(InvalidRequestException, self).__init__( + 'InvalidRequestException', + message) \ No newline at end of file diff --git a/moto/secretsmanager/models.py b/moto/secretsmanager/models.py index 10b63635..c272957a 100644 --- a/moto/secretsmanager/models.py +++ b/moto/secretsmanager/models.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import time import json import uuid +import datetime import boto3 @@ -10,6 +11,7 @@ from moto.core import BaseBackend, BaseModel from .exceptions import ( ResourceNotFoundException, InvalidParameterException, + InvalidRequestException, ClientError ) from .utils import random_password, secret_arn @@ -36,11 +38,21 @@ class SecretsManagerBackend(BaseBackend): def _is_valid_identifier(self, identifier): return identifier in self.secrets + def _unix_time_secs(self, dt): + epoch = datetime.datetime.utcfromtimestamp(0) + return (dt - epoch).total_seconds() + def get_secret_value(self, secret_id, version_id, version_stage): if not self._is_valid_identifier(secret_id): raise ResourceNotFoundException() + if 'deleted_date' in self.secrets[secret_id]: + raise InvalidRequestException( + "An error occurred (InvalidRequestException) when calling the DeleteSecret operation: You tried to \ + perform the operation on a secret that's currently marked deleted." + ) + secret = self.secrets[secret_id] response = json.dumps({ @@ -101,7 +113,7 @@ class SecretsManagerBackend(BaseBackend): "LastRotatedDate": None, "LastChangedDate": None, "LastAccessedDate": None, - "DeletedDate": None, + "DeletedDate": secret.get('deleted_date', None), "Tags": secret['tags'] }) @@ -193,7 +205,7 @@ class SecretsManagerBackend(BaseBackend): secret_list = [{ "ARN": secret_arn(self.region, secret['secret_id']), - "DeletedDate": None, + "DeletedDate": secret.get('deleted_date', None), "Description": "", "KmsKeyId": "", "LastAccessedDate": None, @@ -218,10 +230,10 @@ class SecretsManagerBackend(BaseBackend): if not self._is_valid_identifier(secret_id): raise ResourceNotFoundException - if not force_delete_without_recovery: - raise InvalidParameterException( - "An error occurred (InvalidParameterException) when calling the DeleteSecret operation: \ - ForceDeleteWithoutRecovery must be true (Moto cannot simulate soft deletion with a recovery window)" + if 'deleted_date' in self.secrets[secret_id]: + raise InvalidRequestException( + "An error occurred (InvalidRequestException) when calling the DeleteSecret operation: You tried to \ + perform the operation on a secret that's currently marked deleted." ) if recovery_window_in_days and force_delete_without_recovery: @@ -230,9 +242,20 @@ class SecretsManagerBackend(BaseBackend): use ForceDeleteWithoutRecovery in conjunction with RecoveryWindowInDays." ) - secret = self.secrets.pop(secret_id, None) + if recovery_window_in_days and (recovery_window_in_days < 7 or recovery_window_in_days > 30): + raise InvalidParameterException( + "An error occurred (InvalidParameterException) when calling the DeleteSecret operation: The \ + RecoveryWindowInDays value must be between 7 and 30 days (inclusive)." + ) - deletion_date = int(time.time()) + deletion_date = datetime.datetime.utcnow() + + if force_delete_without_recovery: + secret = self.secrets.pop(secret_id, None) + else: + deletion_date += datetime.timedelta(days=recovery_window_in_days or 30) + self.secrets[secret_id]['deleted_date'] = self._unix_time_secs(deletion_date) + secret = self.secrets.get(secret_id, None) if not secret: raise ResourceNotFoundException @@ -240,7 +263,7 @@ class SecretsManagerBackend(BaseBackend): arn = secret_arn(self.region, secret['secret_id']) name = secret['name'] - return arn, name, deletion_date + return arn, name, self._unix_time_secs(deletion_date) available_regions = ( diff --git a/tests/test_secretsmanager/test_secretsmanager.py b/tests/test_secretsmanager/test_secretsmanager.py index f9a7d0d6..7ce8788e 100644 --- a/tests/test_secretsmanager/test_secretsmanager.py +++ b/tests/test_secretsmanager/test_secretsmanager.py @@ -6,7 +6,7 @@ from moto import mock_secretsmanager from botocore.exceptions import ClientError import sure # noqa import string -from datetime import datetime +from datetime import datetime, timezone import unittest from nose.tools import assert_raises @@ -35,6 +35,20 @@ def test_get_secret_that_does_not_match(): with assert_raises(ClientError): result = conn.get_secret_value(SecretId='i-dont-match') + +@mock_secretsmanager +def test_get_secret_value_that_is_marked_deleted(): + conn = boto3.client('secretsmanager', region_name='us-west-2') + + conn.create_secret(Name='test-secret', + SecretString='foosecret') + + deleted_secret = conn.delete_secret(SecretId='test-secret') + + with assert_raises(ClientError): + result = conn.get_secret_value(SecretId='test-secret') + + @mock_secretsmanager def test_create_secret(): conn = boto3.client('secretsmanager', region_name='us-east-1') @@ -67,16 +81,33 @@ def test_create_secret_with_tags(): def test_delete_secret(): conn = boto3.client('secretsmanager', region_name='us-west-2') + conn.create_secret(Name='test-secret', + SecretString='foosecret') + + deleted_secret = conn.delete_secret(SecretId='test-secret') + + assert deleted_secret['ARN'] + assert deleted_secret['Name'] == 'test-secret' + assert deleted_secret['DeletionDate'] > datetime.fromtimestamp(1, timezone.utc) + + secret_details = conn.describe_secret(SecretId='test-secret') + + assert secret_details['ARN'] + assert secret_details['Name'] == 'test-secret' + assert secret_details['DeletedDate'] > datetime.fromtimestamp(1, timezone.utc) + + +@mock_secretsmanager +def test_delete_secret_force(): + conn = boto3.client('secretsmanager', region_name='us-west-2') + conn.create_secret(Name='test-secret', SecretString='foosecret') result = conn.delete_secret(SecretId='test-secret', ForceDeleteWithoutRecovery=True) - deletion_date = result['DeletionDate'] assert result['ARN'] - - assert deletion_date > datetime.fromtimestamp(1, deletion_date.tzinfo) - + assert result['DeletionDate'] > datetime.fromtimestamp(1, timezone.utc) assert result['Name'] == 'test-secret' with assert_raises(ClientError): @@ -91,17 +122,6 @@ def test_delete_secret_that_does_not_exist(): result = conn.delete_secret(SecretId='i-dont-exist', ForceDeleteWithoutRecovery=True) -@mock_secretsmanager -def test_delete_secret_requires_force_delete_flag(): - conn = boto3.client('secretsmanager', region_name='us-west-2') - - conn.create_secret(Name='test-secret', - SecretString='foosecret') - - with assert_raises(ClientError): - result = conn.delete_secret(SecretId='test-secret', ForceDeleteWithoutRecovery=False) - - @mock_secretsmanager def test_delete_secret_fails_with_both_force_delete_flag_and_recovery_window_flag(): conn = boto3.client('secretsmanager', region_name='us-west-2') @@ -113,6 +133,41 @@ def test_delete_secret_fails_with_both_force_delete_flag_and_recovery_window_fla result = conn.delete_secret(SecretId='test-secret', RecoveryWindowInDays=1, ForceDeleteWithoutRecovery=True) +@mock_secretsmanager +def test_delete_secret_recovery_window_too_short(): + conn = boto3.client('secretsmanager', region_name='us-west-2') + + conn.create_secret(Name='test-secret', + SecretString='foosecret') + + with assert_raises(ClientError): + result = conn.delete_secret(SecretId='test-secret', RecoveryWindowInDays=6) + + +@mock_secretsmanager +def test_delete_secret_recovery_window_too_long(): + conn = boto3.client('secretsmanager', region_name='us-west-2') + + conn.create_secret(Name='test-secret', + SecretString='foosecret') + + with assert_raises(ClientError): + result = conn.delete_secret(SecretId='test-secret', RecoveryWindowInDays=31) + + +@mock_secretsmanager +def test_delete_secret_that_is_marked_deleted(): + conn = boto3.client('secretsmanager', region_name='us-west-2') + + conn.create_secret(Name='test-secret', + SecretString='foosecret') + + deleted_secret = conn.delete_secret(SecretId='test-secret') + + with assert_raises(ClientError): + result = conn.delete_secret(SecretId='test-secret') + + @mock_secretsmanager def test_get_random_password_default_length(): conn = boto3.client('secretsmanager', region_name='us-west-2')