diff --git a/deckhand/barbican/driver.py b/deckhand/barbican/driver.py index 6ed6443f..82d77009 100644 --- a/deckhand/barbican/driver.py +++ b/deckhand/barbican/driver.py @@ -200,3 +200,17 @@ class BarbicanDriver(object): secret = payload return secret + + def delete_secret(self, secret_ref): + """Delete a secret.""" + try: + return self.barbicanclient.call("secrets.delete", secret_ref) + except (barbicanclient.exceptions.HTTPAuthError, + barbicanclient.exceptions.HTTPServerError) as e: + LOG.exception(str(e)) + raise errors.BarbicanException(details=str(e)) + except barbicanclient.exceptions.HTTPClientError as e: + if e.status_code == 404: + LOG.warning('Could not delete secret %s because it was not ' + 'found. Assuming it no longer exists.', secret_ref) + raise diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index 4e988b38..8b8e8242 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -21,6 +21,7 @@ from deckhand.control import base as api_base from deckhand.control import common from deckhand.control.views import revision as revision_view from deckhand.db.sqlalchemy import api as db_api +from deckhand.engine import secrets_manager from deckhand import errors from deckhand import policy @@ -74,7 +75,19 @@ class RevisionsResource(api_base.BaseResource): resp.status = falcon.HTTP_200 resp.body = self.view_builder.list(revisions) + def _delete_all_barbican_secrets(self): + filters = {'metadata.storagePolicy': 'encrypted'} + # NOTE(felipemonteiro): Don't pass `unique_only` because we want + # all unique secret references (just the data section), not unique + # documents, which considers all attributes. + encrypted_documents = db_api.document_get_all(**filters) + + for document in encrypted_documents: + secrets_manager.SecretsManager.delete(document) + @policy.authorize('deckhand:delete_revisions') def on_delete(self, req, resp): + self._delete_all_barbican_secrets() + db_api.revision_delete_all() resp.status = falcon.HTTP_204 diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 9c8bb221..4e334b62 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -586,6 +586,7 @@ def revision_delete_all(): :returns: None """ engine = get_engine() + if engine.name == 'postgresql': # NOTE(fmontei): While cascade should delete all data from all tables, # we also need to reset the index to 1 for each table. @@ -658,7 +659,7 @@ def revision_documents_get(revision_id=None, include_history=True, ID is ``None``, then retrieve the latest revision, if one exists. :param include_history: Return all documents for revision history prior and up to current revision, if ``True``. Default is ``True``. - :param unique_only: Return only unique documents if ``True. Default is + :param unique_only: Return only unique documents if ``True``. Default is ``True``. :param session: Database session object. :param filters: Key-value pairs used for filtering out revision documents. diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index 6788c541..a6ebc75e 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -91,6 +91,26 @@ class SecretsManager(object): LOG.debug('Successfully retrieved Barbican secret using reference.') return secret + @classmethod + def delete(cls, document): + """Delete a secret from Barbican. + + :param dict document: Document with secret_ref in ``data`` section with + format: "https://{barbican_host}/v1/secrets/{secret_uuid}" + :returns: None + + """ + if not isinstance(document, document_wrapper.DocumentDict): + document = document_wrapper.DocumentDict(document) + + secret_ref = document.data + if document.is_encrypted and document.has_barbican_ref: + LOG.debug('Deleting Barbican secret: %s.', secret_ref) + cls.barbican_driver.delete_secret(secret_ref=secret_ref) + else: + LOG.warning('%s is not a valid Barbican secret. Could not delete.', + secret_ref) + class SecretsSubstitution(object): """Class for document substitution logic for YAML files.""" diff --git a/deckhand/tests/common/test_gabbi.py b/deckhand/tests/common/test_gabbi.py index d795f39e..9dac1237 100644 --- a/deckhand/tests/common/test_gabbi.py +++ b/deckhand/tests/common/test_gabbi.py @@ -21,6 +21,7 @@ import tempfile import yaml from gabbi import driver +from gabbi.handlers import core from gabbi.handlers import jsonhandler TEST_DIR = None @@ -105,5 +106,9 @@ def load_tests(loader, tests, pattern): # the same content-type, the one that is earliest in the list will be # used. Thus, we cannot specify multiple content handlers for handling # list/dictionary responses from the server using different handlers. - content_handlers=[MultidocJsonpaths], + content_handlers=[ + core.StringResponseHandler, # For parsing text/plain + jsonhandler.JSONHandler, # For parsing application/json + MultidocJsonpaths # For parsing application/x-yaml + ], verbose=True) diff --git a/deckhand/tests/integration/gabbits/revision-delete-all-secrets.yaml b/deckhand/tests/integration/gabbits/revision-delete-all-secrets.yaml new file mode 100644 index 00000000..2b9b1768 --- /dev/null +++ b/deckhand/tests/integration/gabbits/revision-delete-all-secrets.yaml @@ -0,0 +1,87 @@ +# Tests success paths for deleting all revisions: +# +# 1. Tests that deleting all revisions purges secret references from +# Barbican. + +defaults: + verbose: true + +tests: + - name: purge + desc: Begin testing from known state. + DELETE: /api/v1.0/revisions + status: 204 + request_headers: + content-type: application/x-yaml + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: null + + - name: create_encrypted_passphrase + desc: Create passphrase with storagePolicy=encrypted + PUT: /api/v1.0/buckets/secret/documents + status: 200 + request_headers: + content-type: application/x-yaml + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: + content-type: application/x-yaml + data: |- + --- + schema: deckhand/Passphrase/v1 + metadata: + schema: metadata/Document/v1 + name: my-passphrase + layeringDefinition: + layer: fake + storagePolicy: encrypted + data: not-a-real-password + ... + response_multidoc_jsonpaths: + $.`len`: 1 + # NOTE(fmontei): jsonpath-rw-ext uses a 1 character separator (rather than allowing a string) + # leading to this nastiness: + $.[0].data.`split(:, 0, 1)` + "://" + $.[0].data.`split(/, 2, 3)`: $ENVIRON['TEST_BARBICAN_URL'] + + - name: validate_secret_exists_in_barbican + desc: Validate that the secret ref exists in Barbican + GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets/$HISTORY['create_encrypted_passphrase'].$RESPONSE['$.[0].data.`split(/, 5, -1)`'] + status: 200 + request_headers: + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: + content-type: application/json; charset=UTF-8 + response_json_paths: + $.status: ACTIVE + + - name: validate_secret_payload_matches_in_barbican + desc: Validate that the secret itself matches in Barbican + GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets/$HISTORY['create_encrypted_passphrase'].$RESPONSE['$.[0].data.`split(/, 5, -1)`']/payload + status: 200 + request_headers: + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: + content-type: application/octet-stream; charset=UTF-8 + response_strings: + - not-a-real-password + + - name: delete_all_revisions + desc: Delete all revisions from Deckhand, which should delete all secrets. + DELETE: /api/v1.0/revisions + status: 204 + request_headers: + content-type: application/x-yaml + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: null + + - name: validate_all_secrets_deleted_from_barbican + desc: |- + Validate that deleting all revisions deletes all secrets from Barbican. + GET: $ENVIRON['TEST_BARBICAN_URL']/v1/secrets + status: 200 + request_headers: + X-Auth-Token: $ENVIRON['TEST_AUTH_TOKEN'] + response_headers: + content-type: application/json; charset=UTF-8 + response_json_paths: + $.secrets.`len`: 0 + $.secrets: [] diff --git a/deckhand/tests/unit/control/test_revisions_controller.py b/deckhand/tests/unit/control/test_revisions_controller.py index 9c4d4c3e..cc38c45f 100644 --- a/deckhand/tests/unit/control/test_revisions_controller.py +++ b/deckhand/tests/unit/control/test_revisions_controller.py @@ -12,12 +12,53 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os + +import mock import yaml +from deckhand.engine import secrets_manager from deckhand import factories +from deckhand.tests import test_utils from deckhand.tests.unit.control import base as test_base +class TestRevisionsController(test_base.BaseControllerTest): + + def test_delete_revisions_also_deletes_barbican_secrets(self): + rules = {'deckhand:create_cleartext_documents': '@', + 'deckhand:create_encrypted_documents': '@', + 'deckhand:delete_revisions': '@'} + self.policy.set_rules(rules) + + resource_path = os.path.join(os.getcwd(), 'deckhand', 'tests', 'unit', + 'resources', 'sample_passphrase.yaml') + with open(resource_path) as f: + encrypted_document = f.read() + + fake_secret_ref = test_utils.rand_barbican_ref() + with mock.patch.object(secrets_manager, 'SecretsManager', + autospec=True) as mock_secrets_mgr: + mock_secrets_mgr.create.return_value = fake_secret_ref + + resp = self.app.simulate_put( + '/api/v1.0/buckets/mop/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=encrypted_document) + self.assertEqual(200, resp.status_code) + + with mock.patch.object(secrets_manager.SecretsManager, + 'barbican_driver', autospec=True) \ + as m_barbican_driver: + resp = self.app.simulate_delete( + '/api/v1.0/revisions', + headers={'Content-Type': 'application/x-yaml'}) + + self.assertEqual(204, resp.status_code) + m_barbican_driver.delete_secret.assert_called_once_with( + fake_secret_ref) + + class TestRevisionsControllerNegativeRBAC(test_base.BaseControllerTest): """Test suite for validating negative RBAC scenarios for revisions controller.