Merge pull request #25 from cyberark/policy-delete

Added policy deletion capabilities to the client

README.md

@@ -125,6 +125,13 @@ Replaces a named policy with one from the provided file. This is
 usually a destructive invocation. Result is a dictionary object
 constructed from the returned JSON data.
 
+#### `delete_policy_file(policy_name, policy_file)`
+
+Modifies an existing Conjur policy. Data may be explicitly
+deleted using the !delete, !revoke, and !deny statements. Unlike
+"replace" mode, no data is ever implicitly deleted. Result is a
+dictionary object constructed from the returned JSON data.
+
 #### `list()`
 
 Returns a Python list of all the available resources for the current

conjur/api.py

@@ -221,9 +221,10 @@ def set_variable(self, variable_id, value):
                                value, api_token=self.api_token,
                                ssl_verify=self._ssl_verify).text
 
-    def apply_policy_file(self, policy_id, policy_file):
+
+    def _load_policy_file(self, policy_id, policy_file, http_verb):
         """
-        This method is used to load a file-based policy into the desired
+        This method is used to load, replace or delete a file-based policy into the desired
         name.
         """
 
@@ -236,31 +237,33 @@ def apply_policy_file(self, policy_id, policy_file):
         with open(policy_file, 'r') as content_file:
             policy_data = content_file.read()
 
-        json_response = invoke_endpoint(HttpVerb.POST, ConjurEndpoint.POLICIES, params,
+        json_response = invoke_endpoint(http_verb, ConjurEndpoint.POLICIES, params,
                                         policy_data, api_token=self.api_token,
                                         ssl_verify=self._ssl_verify).text
 
         policy_changes = json.loads(json_response)
         return policy_changes
 
+    def apply_policy_file(self, policy_id, policy_file):
+        """
+        This method is used to load a file-based policy into the desired
+        name.
+        """
+
+        return self._load_policy_file(policy_id, policy_file, HttpVerb.POST)
+
     def replace_policy_file(self, policy_id, policy_file):
         """
         This method is used to replace a file-based policy into the desired
         policy ID.
         """
 
-        params = {
-            'identifier': policy_id,
-        }
-        params.update(self._default_params)
-
-        policy_data = None
-        with open(policy_file, 'r') as content_file:
-            policy_data = content_file.read()
+        return self._load_policy_file(policy_id, policy_file, HttpVerb.PUT)
 
-        json_response = invoke_endpoint(HttpVerb.PUT, ConjurEndpoint.POLICIES, params,
-                                        policy_data, api_token=self.api_token,
-                                        ssl_verify=self._ssl_verify).text
+    def delete_policy_file(self, policy_id, policy_file):
+        """
+        This method is used to delete a file-based policy into the desired
+        policy ID.
+        """
 
-        policy_changes = json.loads(json_response)
-        return policy_changes
+        return self._load_policy_file(policy_id, policy_file, HttpVerb.PATCH)

conjur/cli.py

@@ -71,6 +71,13 @@ def run(self, *args):
         replace_policy_parser.add_argument('policy',
             help='File containing the YAML policy')
 
+        delete_policy_parser = policy_subparsers.add_parser('delete',
+            help='Delete a policy file')
+        delete_policy_parser.add_argument('name',
+            help='Name of the policy (usually "root")')
+        delete_policy_parser.add_argument('policy',
+            help='File containing the YAML policy')
+
 
         parser.add_argument('-v', '--version', action='version',
             version='%(prog)s v' + __version__)
@@ -150,6 +157,9 @@ def run_client_action(resource, args):
             if args.action == 'replace':
                 resources = client.replace_policy_file(args.name, args.policy)
                 print(json.dumps(resources, indent=4))
+            elif args.action == 'delete':
+                resources = client.delete_policy_file(args.name, args.policy)
+                print(json.dumps(resources, indent=4))
             else:
                 resources = client.apply_policy_file(args.name, args.policy)
                 print(json.dumps(resources, indent=4))

conjur/client.py

@@ -147,3 +147,9 @@ def replace_policy_file(self, policy_name, policy_file):
         Replaces a file-based policy defined in the Conjur instance
         """
         return self._api.replace_policy_file(policy_name, policy_file)
+
+    def delete_policy_file(self, policy_name, policy_file):
+        """
+        Replaces a file-based policy defined in the Conjur instance
+        """
+        return self._api.delete_policy_file(policy_name, policy_file)

conjur/http.py

@@ -23,6 +23,7 @@ class HttpVerb(Enum):
     POST = auto()
     PUT = auto()
     DELETE = auto()
+    PATCH = auto()
 
 
 #pylint: disable=too-many-locals

test/test_api.py

@@ -381,6 +381,54 @@ def mock_auth():
                               identifier='mypolicyname',
                               ssl_verify='ssl_verify')
 
+    # Policy delete
+
+    @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse(text='{}'))
+    def test_delete_policy_invokes_http_client_correctly(self, mock_http_client):
+        api = Api(url='http://localhost', login_id='mylogin', api_key='apikey')
+        def mock_auth():
+            return 'apitoken'
+        api.authenticate = mock_auth
+
+        api.delete_policy_file('mypolicyname', self.POLICY_FILE)
+
+        policy_data = None
+        with open(self.POLICY_FILE, 'r') as content_file:
+            policy_data = content_file.read()
+
+        self.verify_http_call(mock_http_client, HttpVerb.PATCH, ConjurEndpoint.POLICIES,
+                              policy_data,
+                              identifier='mypolicyname',
+                              ssl_verify=True)
+
+    @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse(text=json.dumps(MOCK_POLICY_CHANGE_OBJECT)))
+    def test_delete_policy_converts_returned_data_to_expected_objects(self, mock_http_client):
+        api = Api(url='http://localhost', login_id='mylogin', api_key='apikey')
+        def mock_auth():
+            return 'apitoken'
+        api.authenticate = mock_auth
+
+        output = api.delete_policy_file('mypolicyname', self.POLICY_FILE)
+        self.assertEqual(output, MOCK_POLICY_CHANGE_OBJECT)
+
+    @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse(text='{}'))
+    def test_delete_policy_passes_down_ssl_verify_parameter(self, mock_http_client):
+        api = Api(url='http://localhost', login_id='mylogin', api_key='apikey', ssl_verify='ssl_verify')
+        def mock_auth():
+            return 'apitoken'
+        api.authenticate = mock_auth
+
+        api.delete_policy_file('mypolicyname', self.POLICY_FILE)
+
+        policy_data = None
+        with open(self.POLICY_FILE, 'r') as content_file:
+            policy_data = content_file.read()
+
+        self.verify_http_call(mock_http_client, HttpVerb.PATCH, ConjurEndpoint.POLICIES,
+                              policy_data,
+                              identifier='mypolicyname',
+                              ssl_verify='ssl_verify')
+
     # Get variables
 
     @patch('conjur.api.invoke_endpoint', return_value=MockClientResponse(content='{"foo": "a", "bar": "b"}'))

test/test_cli.py

@@ -152,6 +152,18 @@ def test_cli_policy_replace_doesnt_break_on_empty_input(self, cli_invocation, ou
     def test_cli_policy_replace_outputs_formatted_json(self, cli_invocation, output, client):
         self.assertEquals('{\n    "foo": "A",\n    "bar": "B"\n}\n', output)
 
+    @cli_test(["policy", "delete", "foo", "foopolicy"])
+    def test_cli_invokes_policy_delete_correctly(self, cli_invocation, output, client):
+        client.delete_policy_file.assert_called_once_with('foo', 'foopolicy')
+
+    @cli_test(["policy", "delete", "foo", "foopolicy"], policy_change_output={})
+    def test_cli_policy_delete_doesnt_break_on_empty_input(self, cli_invocation, output, client):
+        self.assertEquals('{}\n', output)
+
+    @cli_test(["policy", "delete", "foo", "foopolicy"], policy_change_output={"foo": "A", "bar": "B"})
+    def test_cli_policy_delete_outputs_formatted_json(self, cli_invocation, output, client):
+        self.assertEquals('{\n    "foo": "A",\n    "bar": "B"\n}\n', output)
+
     @cli_test(["list"], list_output=RESOURCE_LIST)
     def test_cli_invokes_resource_listing_correctly(self, cli_invocation, output, client):
         client.list.assert_called_once_with()

test/test_cli_integration.py

@@ -67,6 +67,17 @@ def apply_policy(self, policy_path):
         return invoke_cli(self, self.cli_auth_params,
             ['policy', 'apply', 'root', policy_path])
 
+    def apply_policy_from_string(self, policy):
+        output = None
+        with tempfile.NamedTemporaryFile() as temp_policy_file:
+            temp_policy_file.write(policy.encode('utf-8'))
+            temp_policy_file.flush()
+
+            # Run the new apply that should not result in newly created roles
+            output = self.apply_policy(temp_policy_file.name)
+
+        return output
+
     def replace_policy(self, policy_path):
         return invoke_cli(self, self.cli_auth_params,
             ['policy', 'replace', 'root', policy_path])
@@ -77,22 +88,26 @@ def replace_policy_from_string(self, policy):
             temp_policy_file.write(policy.encode('utf-8'))
             temp_policy_file.flush()
 
-            # Run the new apply that should not result in newly created roles
+            # Run the new replace that should not result in newly created roles
             output = self.replace_policy(temp_policy_file.name)
 
         return output
 
-    def apply_policy_from_string(self, policy):
+    def delete_policy(self, policy_path):
+        return invoke_cli(self, self.cli_auth_params,
+            ['policy', 'delete', 'root', policy_path])
+
+    def delete_policy_from_string(self, policy):
         output = None
         with tempfile.NamedTemporaryFile() as temp_policy_file:
             temp_policy_file.write(policy.encode('utf-8'))
             temp_policy_file.flush()
 
-            # Run the new apply that should not result in newly created roles
-            output = self.apply_policy(temp_policy_file.name)
-
+            # Run the new delete that should not result in newly created roles
+            output = self.delete_policy(temp_policy_file.name)
+        
         return output
-
+    
     def get_variable(self, *variable_ids):
         return invoke_cli(self, self.cli_auth_params,
             ['variable', 'get', *variable_ids])
@@ -336,3 +351,72 @@ def test_https_replace_policy_doesnt_break_if_no_created_roles(self):
         }
 
         self.assertDictEqual(json_result, expected_object)
+
+
+    @integration_test
+    def test_https_can_delete_policy(self):
+        self.setup_cli_params({
+            **self.HTTPS_ENV_VARS,
+            **self.HTTPS_CA_BUNDLE_ENV_VAR
+        })
+
+        policy, variables = self.generate_policy_string()
+        self.delete_policy_from_string(policy)
+
+        for variable in variables:
+            self.assert_set_and_get(variable)
+
+    @integration_test
+    def test_https_delete_policy_can_output_returned_data(self):
+        self.setup_cli_params({
+            **self.HTTPS_ENV_VARS,
+            **self.HTTPS_CA_BUNDLE_ENV_VAR
+        })
+
+        user_id1 = uuid.uuid4().hex
+        user_id2 = uuid.uuid4().hex
+        policy = "- !user {user_id1}\n- !user {user_id2}\n".format(user_id1=user_id1,
+                                                                   user_id2=user_id2)
+
+        # Run the new delete that should not result in newly created roles
+        json_result = json.loads(self.delete_policy_from_string(policy))
+
+        expected_object = {
+            'version': json_result['version'],
+            'created_roles': {
+                'dev:user:' + user_id1: {
+                    'id': 'dev:user:' + user_id1,
+                    'api_key': json_result['created_roles']['dev:user:' + user_id1]['api_key'],
+                },
+                'dev:user:' + user_id2: {
+                    'id': 'dev:user:' + user_id2,
+                    'api_key': json_result['created_roles']['dev:user:' + user_id2]['api_key'],
+                }
+            }
+        }
+
+        self.assertDictEqual(json_result, expected_object)
+
+    @integration_test
+    def test_https_delete_policy_doesnt_break_if_no_created_roles(self):
+        self.setup_cli_params({
+            **self.HTTPS_ENV_VARS,
+            **self.HTTPS_CA_BUNDLE_ENV_VAR
+        })
+
+        user_id1 = uuid.uuid4().hex
+        user_id2 = uuid.uuid4().hex
+        policy = "- !user {user_id1}\n- !user {user_id2}\n".format(user_id1=user_id1,
+                                                                   user_id2=user_id2)
+        # Ensure that the accounts exist
+        self.delete_policy_from_string(policy)
+
+        # Run the new apply that should not result in newly created roles
+        json_result = json.loads(self.delete_policy_from_string(policy))
+
+        expected_object = {
+            'version': json_result['version'],
+            'created_roles': {}
+        }
+
+        self.assertDictEqual(json_result, expected_object)

test/test_client.py

@@ -313,6 +313,28 @@ def test_client_returns_replace_policy_result(self, mock_api_instance,
         return_value = Client().replace_policy_file('name', 'policy')
         self.assertEquals(return_value, replace_policy_result)
 
+
+    @patch('conjur.client.ApiConfig', return_value=MockApiConfig())
+    @patch('conjur.client.Api')
+    def test_client_passes_through_api_delete_policy_params(self, mock_api_instance,
+            mock_api_config):
+        Client().delete_policy_file('name', 'policy')
+
+        mock_api_instance.return_value.delete_policy_file.assert_called_once_with(
+            'name',
+            'policy'
+        )
+
+    @patch('conjur.client.ApiConfig', return_value=MockApiConfig())
+    @patch('conjur.client.Api')
+    def test_client_returns_delete_policy_result(self, mock_api_instance,
+            mock_api_config):
+        delete_policy_result = uuid.uuid4().hex
+        mock_api_instance.return_value.delete_policy_file.return_value = delete_policy_result
+
+        return_value = Client().delete_policy_file('name', 'policy')
+        self.assertEquals(return_value, delete_policy_result)
+
     @patch('conjur.client.ApiConfig', return_value=MockApiConfig())
     @patch('conjur.client.Api')
     def test_client_passes_through_resource_list_method(self, mock_api_instance,

test/test_http.py

@@ -15,6 +15,7 @@ def test_http_verb_has_all_the_verbs_expected(self):
         self.assertTrue(HttpVerb.PUT)
         self.assertTrue(HttpVerb.POST)
         self.assertTrue(HttpVerb.DELETE)
+        self.assertTrue(HttpVerb.PATCH)
 
 
 class HttpInvokeEndpointTest(unittest.TestCase):

test/util/cli_helpers.py

@@ -43,6 +43,7 @@ def test_wrapper_func(self, *inner_args, **inner_kwargs):
             client_instance_mock.list.return_value = list_output
             client_instance_mock.apply_policy_file.return_value = policy_change_output
             client_instance_mock.replace_policy_file.return_value = policy_change_output
+            client_instance_mock.delete_policy_file.return_value = policy_change_output
 
             with self.assertRaises(SystemExit) as sys_exit:
                 with redirect_stdout(capture_stream):