Merge "Allow Barbican Client Secret Update Functionality"

12 files changed, 260 insertions, 38 deletions

diff --git a/README.rst b/README.rst
index f93c6cb..ac34477 100644
--- a/README.rst
+++ b/README.rst
@@ -166,6 +166,7 @@ The command line client is self-documenting. Use the --help flag to access the u
       secret get     Retrieve a secret by providing its URI.
       secret list    List secrets.
       secret store   Store a secret in Barbican.
+      secret update  Update a secret with no payload in Barbican.
 
 * License: Apache License, Version 2.0
 * Documentation: http://docs.openstack.org/developer/python-barbicanclient
diff --git a/barbicanclient/barbican_cli/secrets.py b/barbicanclient/barbican_cli/secrets.py
index 4daaabd..24f5890 100644
--- a/barbicanclient/barbican_cli/secrets.py
+++ b/barbicanclient/barbican_cli/secrets.py
@@ -71,6 +71,21 @@ class GetSecret(show.ShowOne):
             return entity._get_formatted_entity()
 
 
+class UpdateSecret(show.ShowOne):
+    """Update a secret with no payload in Barbican."""
+
+    def get_parser(self, prog_name):
+        parser = super(UpdateSecret, self).get_parser(prog_name)
+        parser.add_argument('URI', help='The URI reference for the secret.')
+        parser.add_argument('payload', help='the unencrypted secret')
+
+        return parser
+
+    def take_action(self, args):
+        self.app.client.secrets.update(args.URI,
+                                       args.payload)
+
+
 class ListSecret(lister.Lister):
     """List secrets."""
 
diff --git a/barbicanclient/client.py b/barbicanclient/client.py
index e74e5e5..3bf1771 100644
--- a/barbicanclient/client.py
+++ b/barbicanclient/client.py
@@ -71,11 +71,15 @@ class _HTTPClient(adapter.Adapter):
         return super(_HTTPClient, self).get(*args, **kwargs).json()
 
     def post(self, path, *args, **kwargs):
-        if not path[-1] == '/':
-            path += '/'
+        path = self._fix_path(path)
 
         return super(_HTTPClient, self).post(path, *args, **kwargs).json()
 
+    def _fix_path(self, path):
+        if not path[-1] == '/':
+            path += '/'
+        return path
+
     def _get_raw(self, path, *args, **kwargs):
         return self.request(path, 'GET', *args, **kwargs).content
 
diff --git a/barbicanclient/secrets.py b/barbicanclient/secrets.py
index fe8dbfa..2ed41d7 100644
--- a/barbicanclient/secrets.py
+++ b/barbicanclient/secrets.py
@@ -185,7 +185,11 @@ class Secret(SecretFormatter):
         Lazy-loaded property that holds the unencrypted data
         """
         if self._payload is None and self.secret_ref is not None:
-            self._fetch_payload()
+            try:
+                self._fetch_payload()
+            except ValueError:
+                LOG.warning("Secret does not contain a payload")
+                return None
         return self._payload
 
     @name.setter
@@ -219,7 +223,6 @@ class Secret(SecretFormatter):
         self._mode = value
 
     @payload.setter
-    @immutable_after_save
     def payload(self, value):
         self._payload = value
 
@@ -279,10 +282,15 @@ class Secret(SecretFormatter):
             'expiration': self.expiration
         }
 
-        if not self.payload:
-            raise exceptions.PayloadException("Missing Payload")
-        if not isinstance(self.payload, (six.text_type, six.binary_type)):
+        if self.payload == '':
+            raise exceptions.PayloadException("Invalid Payload: "
+                                              "Cannot Be Empty String")
+
+        if self.payload is not None and not isinstance(self.payload,
+                                                       (six.text_type,
+                                                        six.binary_type)):
             raise exceptions.PayloadException("Invalid Payload Type")
+
         if self.payload_content_type or self.payload_content_encoding:
             """
             Setting the payload_content_type and payload_content_encoding
@@ -322,6 +330,27 @@ class Secret(SecretFormatter):
             self._secret_ref = response.get('secret_ref')
         return self.secret_ref
 
+    def update(self):
+        """
+        Updates the secret in Barbican.
+        """
+
+        if not self.payload:
+            raise exceptions.PayloadException("Missing Payload")
+        if not self.secret_ref:
+            raise LookupError("Secret is not yet stored.")
+
+        if type(self.payload) is six.binary_type:
+            headers = {'content-type': "application/octet-stream"}
+        elif type(self.payload) is six.text_type:
+            headers = {'content-type': "text/plain"}
+        else:
+            raise exceptions.PayloadException("Invalid Payload Type")
+
+        self._api.put(self._secret_ref,
+                      headers=headers,
+                      data=self.payload)
+
     def delete(self):
         """
         Deletes the Secret from Barbican
@@ -426,6 +455,32 @@ class SecretManager(base.BaseEntityManager):
             secret_ref=secret_ref
         )
 
+    def update(self, secret_ref, payload=None):
+        """
+        Update an existing Secret from Barbican
+
+        :param str secret_ref: Full HATEOAS reference to a Secret
+        :param str payload: New payload to add to secret
+        :raises barbicanclient.exceptions.HTTPAuthError: 401 Responses
+        :raises barbicanclient.exceptions.HTTPClientError: 4xx Responses
+        :raises barbicanclient.exceptions.HTTPServerError: 5xx Responses
+        """
+
+        base.validate_ref(secret_ref, 'Secret')
+        if not secret_ref:
+            raise ValueError('secret_ref is required.')
+
+        if type(payload) is six.binary_type:
+            headers = {'content-type': "application/octet-stream"}
+        elif type(payload) is six.text_type:
+            headers = {'content-type': "text/plain"}
+        else:
+            raise exceptions.PayloadException("Invalid Payload Type")
+
+        self._api.put(secret_ref,
+                      headers=headers,
+                      data=payload)
+
     def create(self, name=None, payload=None,
                payload_content_type=None, payload_content_encoding=None,
                algorithm=None, bit_length=None, secret_type=None,
diff --git a/barbicanclient/tests/test_client.py b/barbicanclient/tests/test_client.py
index d2e1879..a27cbc6 100644
--- a/barbicanclient/tests/test_client.py
+++ b/barbicanclient/tests/test_client.py
@@ -105,7 +105,38 @@ class WhenTestingClientPost(TestClient):
     def test_post_checks_status_code(self):
         self.httpclient._check_status_code = mock.MagicMock()
         self.httpclient.post(path='secrets', json={'test_data': 'test'})
-        self.assertTrue(self.httpclient._check_status_code.called)
+        self.httpclient._check_status_code.assert_has_calls([])
+
+
+class WhenTestingClientPut(TestClient):
+
+    def setUp(self):
+        super(WhenTestingClientPut, self).setUp()
+        self.httpclient = client._HTTPClient(session=self.session,
+                                             endpoint=self.endpoint)
+        self.href = 'http://test_href/'
+        self.put_mock = self.responses.put(self.href, status_code=204)
+
+    def test_put_uses_href_as_is(self):
+        self.httpclient.put(self.href)
+        self.assertTrue(self.put_mock.called)
+
+    def test_put_passes_data(self):
+        data = "test"
+        self.httpclient.put(self.href, data=data)
+        self.assertEqual(self.put_mock.last_request.text, "test")
+
+    def test_put_includes_default_headers(self):
+        self.httpclient._default_headers = {'Test-Default-Header': 'test'}
+        self.httpclient.put(self.href)
+        self.assertEqual(
+            'test',
+            self.put_mock.last_request.headers['Test-Default-Header'])
+
+    def test_put_checks_status_code(self):
+        self.httpclient._check_status_code = mock.MagicMock()
+        self.httpclient.put(self.href, data='test')
+        self.httpclient._check_status_code.assert_has_calls([])
 
 
 class WhenTestingClientGet(TestClient):
@@ -144,7 +175,7 @@ class WhenTestingClientGet(TestClient):
     def test_get_checks_status_code(self):
         self.httpclient._check_status_code = mock.MagicMock()
         self.httpclient.get(self.href)
-        self.assertTrue(self.httpclient._check_status_code.called)
+        self.httpclient._check_status_code.assert_has_calls([])
 
     def test_get_raw_uses_href_as_is(self):
         self.httpclient._get_raw(self.href, headers=self.headers)
@@ -163,7 +194,7 @@ class WhenTestingClientGet(TestClient):
     def test_get_raw_checks_status_code(self):
         self.httpclient._check_status_code = mock.MagicMock()
         self.httpclient._get_raw(self.href, headers=self.headers)
-        self.assertTrue(self.httpclient._check_status_code.called)
+        self.httpclient._check_status_code.assert_has_calls([])
 
 
 class WhenTestingClientDelete(TestClient):
@@ -194,7 +225,7 @@ class WhenTestingClientDelete(TestClient):
     def test_delete_checks_status_code(self):
         self.httpclient._check_status_code = mock.MagicMock()
         self.httpclient.delete(self.href)
-        self.assertTrue(self.httpclient._check_status_code.called)
+        self.httpclient._check_status_code.assert_has_calls([])
 
 
 class WhenTestingCheckStatusCodes(TestClient):
diff --git a/barbicanclient/tests/test_secrets.py b/barbicanclient/tests/test_secrets.py
index 0bd3f4a..91beb8b 100644
--- a/barbicanclient/tests/test_secrets.py
+++ b/barbicanclient/tests/test_secrets.py
@@ -195,9 +195,8 @@ class WhenTestingSecrets(test_client.BaseEntityResource):
 
         # Verify that attributes are immutable after store.
         attributes = [
-            "name", "expiration", "algorithm", "bit_length", "mode", "payload",
-            "payload_content_type", "payload_content_encoding", "secret_type"
-        ]
+            "name", "expiration", "algorithm", "bit_length", "mode",
+            "payload_content_type", "payload_content_encoding", "secret_type"]
         for attr in attributes:
             try:
                 setattr(secret, attr, "test")
@@ -372,6 +371,29 @@ class WhenTestingSecrets(test_client.BaseEntityResource):
         # Verify the correct URL was used to make the call.
         self.assertEqual(self.entity_href, self.responses.last_request.url)
 
+    def test_should_update(self):
+        data = {'secret_ref': self.entity_href}
+        self.responses.post(self.entity_base + '/', json=data)
+
+        secret = self.manager.create()
+        secret.payload = None
+        secret.store()
+
+        # This literal will have type(unicode) in Python 2, but will have
+        # type(str) in Python 3.  It is six.text_type in both cases.
+        text_payload = u'time for an ice cold \U0001f37a'
+
+        self.responses.put(self.entity_href, status_code=204)
+
+        secret.payload = text_payload
+        secret.update()
+
+        # Verify the correct URL was used to make the call.
+        self.assertEqual(self.entity_href, self.responses.last_request.url)
+
+        # Verify that the data has been updated
+        self.assertEqual(text_payload, secret.payload)
+
     def test_should_get_list(self):
         secret_resp = self.secret.get_dict(self.entity_href)
 
@@ -400,11 +422,7 @@ class WhenTestingSecrets(test_client.BaseEntityResource):
         self.responses.get(self.entity_href, json=data)
         secret = self.manager.get(secret_ref=self.entity_href)
 
-        try:
-            secret.payload
-            self.fail("didn't raise a ValueError exception")
-        except ValueError:
-            pass
+        self.assertIsNone(secret.payload)
 
     def test_should_fail_decrypt_no_default_content_type(self):
         content_types_dict = {'no-default': 'application/octet-stream'}
@@ -412,11 +430,7 @@ class WhenTestingSecrets(test_client.BaseEntityResource):
         self.responses.get(self.entity_href, json=data)
 
         secret = self.manager.get(secret_ref=self.entity_href)
-        try:
-            secret.payload
-            self.fail("didn't raise a ValueError exception")
-        except ValueError:
-            pass
+        self.assertIsNone(secret.payload)
 
     def test_should_fail_delete_no_href(self):
         self.assertRaises(ValueError, self.manager.delete, None)
diff --git a/doc/source/cli_usage.rst b/doc/source/cli_usage.rst
index 3aa9930..2bbc0b0 100644
--- a/doc/source/cli_usage.rst
+++ b/doc/source/cli_usage.rst
@@ -157,6 +157,15 @@ Secret Delete
 
     $ barbican secret delete http://localhost:9311/v1/secrets/a70a45d8-4076-42a2-b111-8893d3b92a3e
 
+Secret Update
+~~~~~~~~~~~~~
+
+.. code-block:: bash
+
+    $ barbican secret update http://localhost:9311/v1/secrets/a70a45d8-4076-42a2-b111-8893d3b92a3e ``my_payload``
+
+In order for a secret to be updated it must have been created without a payload.
+``my_payload`` will be added as the secret's payload.
 
 Secret List
 ~~~~~~~~~~~
diff --git a/functionaltests/cli/v1/behaviors/secret_behaviors.py b/functionaltests/cli/v1/behaviors/secret_behaviors.py
index 778e1d0..2361029 100644
--- a/functionaltests/cli/v1/behaviors/secret_behaviors.py
+++ b/functionaltests/cli/v1/behaviors/secret_behaviors.py
@@ -25,6 +25,22 @@ class SecretBehaviors(base_behaviors.BaseBehaviors):
         self.LOG = logging.getLogger(type(self).__name__)
         self.secret_hrefs_to_delete = []
 
+    def update_secret(self,
+                      secret_href,
+                      payload):
+        """ Update a secret
+
+        :param secret_href the href to the secret to update.
+        :param payload the payload to put into the secret.
+        :param payload_content_type the payload content type.
+        """
+        argv = ['secret', 'update']
+        self.add_auth_and_endpoint(argv)
+        argv.extend([secret_href])
+        argv.extend([payload])
+
+        stdout, stderr = self.issue_barbican_command(argv)
+
     def delete_secret(self, secret_href):
         """ Delete a secret
 
diff --git a/functionaltests/cli/v1/smoke/test_secret.py b/functionaltests/cli/v1/smoke/test_secret.py
index 416027f..7db17a6 100644
--- a/functionaltests/cli/v1/smoke/test_secret.py
+++ b/functionaltests/cli/v1/smoke/test_secret.py
@@ -29,6 +29,7 @@ class SecretTestCase(CmdLineTestCase):
         super(SecretTestCase, self).setUp()
         self.secret_behaviors = SecretBehaviors()
         self.expected_payload = "Top secret payload for secret smoke tests"
+        self.payload_content_type = "text/plain"
 
     def tearDown(self):
         super(SecretTestCase, self).tearDown()
@@ -85,6 +86,19 @@ class SecretTestCase(CmdLineTestCase):
         self.assertEqual(secret_href, secret['Secret href'])
 
     @testcase.attr('positive')
+    def test_secret_update(self):
+        secret_href = self.secret_behaviors.store_secret(
+            payload=None)
+
+        payload = 'time for an ice cold!!!'
+        self.assertIsNotNone(secret_href)
+        self.secret_behaviors.update_secret(secret_href,
+                                            payload)
+
+        payload_update = self.secret_behaviors.get_secret_payload(secret_href)
+        self.assertEqual(payload, payload_update)
+
+    @testcase.attr('positive')
     def test_secret_list(self):
         secrets_to_create = 10
         for _ in range(secrets_to_create):
diff --git a/functionaltests/client/v1/behaviors/secret_behaviors.py b/functionaltests/client/v1/behaviors/secret_behaviors.py
index 98d27f9..7540bfe 100644
--- a/functionaltests/client/v1/behaviors/secret_behaviors.py
+++ b/functionaltests/client/v1/behaviors/secret_behaviors.py
@@ -59,6 +59,17 @@ class SecretBehaviors(base_behaviors.BaseBehaviors):
 
         return resp
 
+    def update_secret(self, secret_ref, payload):
+        """Updates a secret.
+
+        :param secret_ref: HATEOS ref of the secret to be updated
+        :return: It will return a string
+        """
+
+        resp = self.client.secrets.update(secret_ref, payload)
+
+        return resp
+
     def get_secrets(self, limit=10, offset=0):
         """Handles getting a list of secrets.
 
diff --git a/functionaltests/client/v1/functional/test_secrets.py b/functionaltests/client/v1/functional/test_secrets.py
index f02c156..a5fbd0b 100644
--- a/functionaltests/client/v1/functional/test_secrets.py
+++ b/functionaltests/client/v1/functional/test_secrets.py
@@ -145,24 +145,80 @@ class SecretsTestCase(base.TestCase):
 
         self.assertEqual(e.status_code, 404)
 
+    @utils.parameterized_dataset({
+        'text/plain_payload': {
+            'payload': 'time for an ice cold!!'},
+        'application/octet-stream_payload': {
+            'payload': 'abcdefg=='}
+    })
+    @testcase.attr('positive')
+    def test_secret_update_nones_content_type(self,
+                                              payload):
+        """Update secret, secret will start with payload of None.
+
+        Secret will be created with even though
+        the payload is None, then it will be updated.
+        """
+
+        test_model = self.behaviors.create_secret(
+            secret_create_defaults_data)
+        test_model.payload = None
+
+        secret_ref = self.behaviors.store_secret(test_model)
+        self.assertIsNotNone(secret_ref)
+
+        test_model.payload = payload
+        resp = self.behaviors.update_secret(secret_ref, payload)
+
+        secret = self.behaviors.get_secret(secret_ref)
+        payload = secret.payload
+        payload_content_type = secret.payload_content_type
+        self.assertEqual(secret.payload, payload)
+        self.assertEqual(secret.payload_content_type, payload_content_type)
+
+    @utils.parameterized_dataset({
+        'text/plain_payload': {
+            'payload_update': 'time for the Texas heat!!!',
+            'payload': 'time for an ice cold!!!'},
+        'application/octet-stream_payload': {
+            'payload_update': 'hijklmn==',
+            'payload': 'abcdefg=='}
+    })
     @testcase.attr('negative')
+    def test_secret_update_provided_content_type(self,
+                                                 payload_update,
+                                                 payload):
+        """Update secret, secret will start with a payload that is not None.
+
+        Secret will be created with a not None payload, then it
+        will try to update. Update will fail.
+        """
+        test_model = self.behaviors.create_secret(
+            secret_create_defaults_data)
+        test_model.payload = payload
+
+        secret_ref = self.behaviors.store_secret(test_model)
+        self.assertIsNotNone(secret_ref)
+
+        self.assertRaises(exceptions.HTTPClientError,
+                          self.behaviors.update_secret,
+                          secret_ref,
+                          payload_update)
+
+    @testcase.attr('positive')
     def test_secret_create_nones_content_type(self):
         """Create secret with valid content type but no payload.
 
-        Secret will not create due to None in the payload even if content
-        type is valid.
+        Secret will be created with valid content even though
+        the payload is None.
         """
         test_model = self.behaviors.create_secret(
             secret_create_defaults_data)
         test_model.payload = None
 
-        self.assertRaises(
-            exceptions.PayloadException,
-            self.behaviors.store_secret,
-            test_model
-        )
+        self.assertEqual(test_model.payload, None)
 
-    @testcase.attr('negative')
+    @testcase.attr('positive')
     def test_secret_create_nones(self):
         """Cover case of posting with all nones in the Secret object."""
         test_model = self.behaviors.create_secret(
@@ -171,11 +227,7 @@ class SecretsTestCase(base.TestCase):
         test_model.payload_content_encoding = None
         test_model.payload_content_type = None
 
-        self.assertRaises(
-            exceptions.PayloadException,
-            self.behaviors.store_secret,
-            test_model
-        )
+        self.assertEqual(test_model.payload, None)
 
     @testcase.attr('negative')
     def test_secret_get_secret_doesnt_exist(self):
@@ -618,7 +670,6 @@ class SecretsTestCase(base.TestCase):
 
     @utils.parameterized_dataset({
         'empty': [''],
-        'none': [None],
         'zero': [0]
     })
     @testcase.attr('negative')
diff --git a/setup.cfg b/setup.cfg
index 37838f7..311ff63 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -38,6 +38,7 @@ barbican.client =
     secret_get = barbicanclient.barbican_cli.secrets:GetSecret
     secret_list = barbicanclient.barbican_cli.secrets:ListSecret
     secret_store = barbicanclient.barbican_cli.secrets:StoreSecret
+    secret_update = barbicanclient.barbican_cli.secrets:UpdateSecret
 
     container_delete = barbicanclient.barbican_cli.containers:DeleteContainer
     container_get = barbicanclient.barbican_cli.containers:GetContainer