From 522f38870916e6d2ddbe07b040e96b1fbe85595d Mon Sep 17 00:00:00 2001 From: Pierre Gaxatte Date: Mon, 2 Jul 2018 09:26:45 +0200 Subject: [PATCH] Add a policy to control the right to publish resources This change intends to add the following policies: - actions:publicize - workflows:publicize A refactor of the policy tests is included to increase the coverage. Change-Id: I8bd637f7de63f02c63f3c304cb6f5198fc0c5f42 Closes-Bug: 1779293 --- mistral/api/controllers/v2/action.py | 4 + mistral/api/controllers/v2/workflow.py | 4 + mistral/policies/action.py | 15 + mistral/policies/workflow.py | 15 + mistral/tests/unit/api/test_policies.py | 78 ----- mistral/tests/unit/policies/__init__.py | 0 mistral/tests/unit/policies/test_actions.py | 249 ++++++++++++++++ mistral/tests/unit/policies/test_workflows.py | 279 ++++++++++++++++++ ...add-publicize-policy-d3b44590286c7fdd.yaml | 7 + 9 files changed, 573 insertions(+), 78 deletions(-) delete mode 100644 mistral/tests/unit/api/test_policies.py create mode 100644 mistral/tests/unit/policies/__init__.py create mode 100644 mistral/tests/unit/policies/test_actions.py create mode 100644 mistral/tests/unit/policies/test_workflows.py create mode 100644 releasenotes/notes/add-publicize-policy-d3b44590286c7fdd.yaml diff --git a/mistral/api/controllers/v2/action.py b/mistral/api/controllers/v2/action.py index 0f94ef815..660189707 100644 --- a/mistral/api/controllers/v2/action.py +++ b/mistral/api/controllers/v2/action.py @@ -83,6 +83,8 @@ class ActionsController(rest.RestController, hooks.HookController): scope = pecan.request.GET.get('scope', 'private') resources.Action.validate_scope(scope) + if scope == 'public': + acl.enforce('actions:publicize', context.ctx()) @rest_utils.rest_retry_on_db_error def _update_actions(): @@ -116,6 +118,8 @@ class ActionsController(rest.RestController, hooks.HookController): pecan.response.status = 201 resources.Action.validate_scope(scope) + if scope == 'public': + acl.enforce('actions:publicize', context.ctx()) LOG.debug("Create action(s) [definition=%s]", definition) diff --git a/mistral/api/controllers/v2/workflow.py b/mistral/api/controllers/v2/workflow.py index 0186367f9..4134b97b7 100644 --- a/mistral/api/controllers/v2/workflow.py +++ b/mistral/api/controllers/v2/workflow.py @@ -117,6 +117,8 @@ class WorkflowsController(rest.RestController, hooks.HookController): scope = pecan.request.GET.get('scope', 'private') resources.Workflow.validate_scope(scope) + if scope == 'public': + acl.enforce('workflows:publicize', context.ctx()) LOG.debug("Update workflow(s) [definition=%s]", definition) @@ -153,6 +155,8 @@ class WorkflowsController(rest.RestController, hooks.HookController): pecan.response.status = 201 resources.Workflow.validate_scope(scope) + if scope == 'public': + acl.enforce('workflows:publicize', context.ctx()) LOG.debug("Create workflow(s) [definition=%s]", definition) diff --git a/mistral/policies/action.py b/mistral/policies/action.py index c6eb91df8..e5ddf9b27 100644 --- a/mistral/policies/action.py +++ b/mistral/policies/action.py @@ -62,6 +62,21 @@ rules = [ } ] ), + policy.DocumentedRuleDefault( + name=ACTIONS % 'publicize', + check_str=base.RULE_ADMIN_OR_OWNER, + description='Make an action publicly available', + operations=[ + { + 'path': '/v2/actions', + 'method': 'POST' + }, + { + 'path': '/v2/actions', + 'method': 'PUT' + } + ] + ), policy.DocumentedRuleDefault( name=ACTIONS % 'update', check_str=base.RULE_ADMIN_OR_OWNER, diff --git a/mistral/policies/workflow.py b/mistral/policies/workflow.py index 161e585c2..5f1651dcc 100644 --- a/mistral/policies/workflow.py +++ b/mistral/policies/workflow.py @@ -73,6 +73,21 @@ rules = [ } ] ), + policy.DocumentedRuleDefault( + name=WORKFLOWS % 'publicize', + check_str=base.RULE_ADMIN_OR_OWNER, + description='Make a workflow publicly available', + operations=[ + { + 'path': '/v2/workflows', + 'method': 'POST' + }, + { + 'path': '/v2/workflows', + 'method': 'PUT' + } + ] + ), policy.DocumentedRuleDefault( name=WORKFLOWS % 'update', check_str=base.RULE_ADMIN_OR_OWNER, diff --git a/mistral/tests/unit/api/test_policies.py b/mistral/tests/unit/api/test_policies.py deleted file mode 100644 index db8211496..000000000 --- a/mistral/tests/unit/api/test_policies.py +++ /dev/null @@ -1,78 +0,0 @@ -# Copyright 2016 NEC Corporation. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - - -import datetime - -import mock - -from mistral.db.v2 import api as db_api -from mistral.db.v2.sqlalchemy import models -from mistral.tests.unit.api import base -from mistral.tests.unit.mstrlfixtures import policy_fixtures - -WF_DEFINITION = """ ---- -version: '2.0' - -flow: - type: direct - input: - - param1 - - tasks: - task1: - action: std.echo output="Hi" -""" - -WF_DB = models.WorkflowDefinition( - id='123e4567-e89b-12d3-a456-426655440000', - name='flow', - definition=WF_DEFINITION, - created_at=datetime.datetime(1970, 1, 1), - updated_at=datetime.datetime(1970, 1, 1), - spec={'input': ['param1']} -) - -WF = { - 'id': '123e4567-e89b-12d3-a456-426655440000', - 'name': 'flow', - 'definition': WF_DEFINITION, - 'created_at': '1970-01-01 00:00:00', - 'updated_at': '1970-01-01 00:00:00', - 'input': 'param1' -} - -MOCK_WF = mock.MagicMock(return_value=WF_DB) - - -class TestPolicies(base.APITest): - @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF) - def get(self): - resp = self.app.get('/v2/workflows/123', expect_errors=True) - return resp.status_int - - def test_disable_workflow_api(self): - self.policy = self.useFixture(policy_fixtures.PolicyFixture()) - rules = {"workflows:get": "role:FAKE"} - self.policy.change_policy_definition(rules) - response_value = self.get() - self.assertEqual(403, response_value) - - def test_enable_workflow_api(self): - self.policy = self.useFixture(policy_fixtures.PolicyFixture()) - rules = {"workflows:get": "role:FAKE or rule:admin_or_owner"} - self.policy.change_policy_definition(rules) - response_value = self.get() - self.assertEqual(200, response_value) diff --git a/mistral/tests/unit/policies/__init__.py b/mistral/tests/unit/policies/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/mistral/tests/unit/policies/test_actions.py b/mistral/tests/unit/policies/test_actions.py new file mode 100644 index 000000000..53fff3f85 --- /dev/null +++ b/mistral/tests/unit/policies/test_actions.py @@ -0,0 +1,249 @@ +# Copyright 2018 OVH SAS. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +import mock + +from mistral.db.v2 import api as db_api +from mistral.db.v2.sqlalchemy import models +from mistral.tests.unit.api import base +from mistral.tests.unit.mstrlfixtures import policy_fixtures + +MOCK_DELETE = mock.MagicMock(return_value=None) + +ACTION_DEFINITION = """ +--- +version: '2.0' + +my_action: + description: My super cool action. + tags: ['test', 'v2'] + base: std.echo + base-input: + output: "{$.str1}{$.str2}" +""" +ACTION_DB = models.ActionDefinition( + id='123e4567-e89b-12d3-a456-426655440000', + name='my_action', + is_system=False, + description='My super cool action.', + tags=['test', 'v2'], + definition=ACTION_DEFINITION +) +MOCK_ACTION = mock.MagicMock(return_value=ACTION_DB) + + +class TestActionPolicy(base.APITest): + """Test action related policies + + Policies to test: + - actions:create + - actions:delete + - actions:get + - actions:list + - actions:publicize (on POST & PUT) + - actions:update + """ + + def setUp(self): + self.policy = self.useFixture(policy_fixtures.PolicyFixture()) + super(TestActionPolicy, self).setUp() + + @mock.patch.object(db_api, "create_action_definition") + def test_action_create_not_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"actions:create": "role:FAKE"} + ) + resp = self.app.post( + '/v2/actions', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "create_action_definition") + def test_action_create_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"actions:create": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.post( + '/v2/actions', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(201, resp.status_int) + + @mock.patch.object(db_api, "create_action_definition") + def test_action_create_public_not_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "actions:create": "role:FAKE or rule:admin_or_owner", + "actions:publicize": "role:FAKE" + }) + resp = self.app.post( + '/v2/actions?scope=public', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "create_action_definition") + def test_action_create_public_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "actions:create": "role:FAKE or rule:admin_or_owner", + "actions:publicize": "role:FAKE or rule:admin_or_owner" + }) + resp = self.app.post( + '/v2/actions?scope=public', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(201, resp.status_int) + + @mock.patch.object(db_api, "delete_action_definition", MOCK_DELETE) + @mock.patch.object(db_api, "get_action_definition", MOCK_ACTION) + def test_action_delete_not_allowed(self): + self.policy.change_policy_definition( + {"actions:delete": "role:FAKE"} + ) + resp = self.app.delete( + '/v2/actions/123', + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "delete_action_definition", MOCK_DELETE) + @mock.patch.object(db_api, "get_action_definition", MOCK_ACTION) + def test_action_delete_allowed(self): + self.policy.change_policy_definition( + {"actions:delete": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.delete( + '/v2/actions/123', + expect_errors=True + ) + + self.assertEqual(204, resp.status_int) + + @mock.patch.object(db_api, "get_action_definition", MOCK_ACTION) + def test_action_get_not_allowed(self): + self.policy.change_policy_definition( + {"actions:get": "role:FAKE"} + ) + resp = self.app.get( + '/v2/actions/123', + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "get_action_definition", MOCK_ACTION) + def test_action_get_allowed(self): + self.policy.change_policy_definition( + {"actions:get": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.get( + '/v2/actions/123', + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) + + def test_action_list_not_allowed(self): + self.policy.change_policy_definition( + {"actions:list": "role:FAKE"} + ) + resp = self.app.get( + '/v2/actions', + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + def test_action_list_allowed(self): + self.policy.change_policy_definition( + {"actions:list": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.get( + '/v2/actions', + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) + + @mock.patch.object(db_api, "update_action_definition") + def test_action_update_not_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"actions:update": "role:FAKE"} + ) + resp = self.app.put( + '/v2/actions', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "update_action_definition") + def test_action_update_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"actions:update": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.put( + '/v2/actions', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) + + @mock.patch.object(db_api, "update_action_definition") + def test_action_update_public_not_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "actions:update": "role:FAKE or rule:admin_or_owner", + "actions:publicize": "role:FAKE" + }) + resp = self.app.put( + '/v2/actions?scope=public', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "update_action_definition") + def test_action_update_public_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "actions:update": "role:FAKE or rule:admin_or_owner", + "actions:publicize": "role:FAKE or rule:admin_or_owner" + }) + resp = self.app.put( + '/v2/actions?scope=public', + ACTION_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) diff --git a/mistral/tests/unit/policies/test_workflows.py b/mistral/tests/unit/policies/test_workflows.py new file mode 100644 index 000000000..71a3c8a09 --- /dev/null +++ b/mistral/tests/unit/policies/test_workflows.py @@ -0,0 +1,279 @@ +# Copyright 2016 NEC Corporation. All rights reserved. +# Copyright 2018 OVH SAS. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +import datetime + +import mock + +from mistral.db.v2 import api as db_api +from mistral.db.v2.sqlalchemy import models +from mistral.tests.unit.api import base +from mistral.tests.unit.mstrlfixtures import policy_fixtures + +MOCK_DELETE = mock.MagicMock(return_value=None) + +WF_DEFINITION = """ +--- +version: '2.0' + +flow: + type: direct + input: + - param1 + + tasks: + task1: + action: std.echo output="Hi" +""" +WF_DB = models.WorkflowDefinition( + id='123e4567-e89b-12d3-a456-426655440000', + name='flow', + definition=WF_DEFINITION, + created_at=datetime.datetime(1970, 1, 1), + updated_at=datetime.datetime(1970, 1, 1), + spec={'input': ['param1']} +) +MOCK_WF = mock.MagicMock(return_value=WF_DB) + + +class TestWorkflowPolicy(base.APITest): + """Test workflow related policies + + Policies to test: + - workflows:create + - workflows:delete + - workflows:get + - workflows:list + - workflows:list:all_projects + - workflows:publicize (on POST & PUT) + - workflows:update + """ + + def setUp(self): + self.policy = self.useFixture(policy_fixtures.PolicyFixture()) + super(TestWorkflowPolicy, self).setUp() + + @mock.patch.object(db_api, "create_workflow_definition") + def test_workflow_create_not_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"workflows:create": "role:FAKE"} + ) + resp = self.app.post( + '/v2/workflows', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "create_workflow_definition") + def test_workflow_create_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"workflows:create": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.post( + '/v2/workflows', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(201, resp.status_int) + + @mock.patch.object(db_api, "create_workflow_definition") + def test_workflow_create_public_not_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "workflows:create": "role:FAKE or rule:admin_or_owner", + "workflows:publicize": "role:FAKE" + }) + resp = self.app.post( + '/v2/workflows?scope=public', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "create_workflow_definition") + def test_workflow_create_public_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "workflows:create": "role:FAKE or rule:admin_or_owner", + "workflows:publicize": "role:FAKE or rule:admin_or_owner" + }) + resp = self.app.post( + '/v2/workflows?scope=public', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(201, resp.status_int) + + @mock.patch.object(db_api, "delete_workflow_definition", MOCK_DELETE) + @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF) + def test_workflow_delete_not_allowed(self): + self.policy.change_policy_definition( + {"workflows:delete": "role:FAKE"} + ) + resp = self.app.delete( + '/v2/workflows/123', + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "delete_workflow_definition", MOCK_DELETE) + @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF) + def test_workflow_delete_allowed(self): + self.policy.change_policy_definition( + {"workflows:delete": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.delete( + '/v2/workflows/123', + expect_errors=True + ) + + self.assertEqual(204, resp.status_int) + + @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF) + def test_workflow_get_not_allowed(self): + self.policy.change_policy_definition( + {"workflows:get": "role:FAKE"} + ) + resp = self.app.get( + '/v2/workflows/123', + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF) + def test_workflow_get_allowed(self): + self.policy.change_policy_definition( + {"workflows:get": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.get( + '/v2/workflows/123', + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) + + def test_workflow_list_not_allowed(self): + self.policy.change_policy_definition( + {"workflows:list": "role:FAKE"} + ) + resp = self.app.get( + '/v2/workflows', + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + def test_workflow_list_allowed(self): + self.policy.change_policy_definition( + {"workflows:list": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.get( + '/v2/workflows', + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) + + def test_workflow_list_all_not_allowed(self): + self.policy.change_policy_definition({ + "workflows:list": "role:FAKE or rule:admin_or_owner", + "workflows:list:all_projects": "role:FAKE" + }) + resp = self.app.get( + '/v2/workflows?all_projects=1', + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + def test_workflow_list_all_allowed(self): + self.policy.change_policy_definition({ + "workflows:list": "role:FAKE or rule:admin_or_owner", + "workflows:list:all_projects": "role:FAKE or rule:admin_or_owner" + }) + resp = self.app.get( + '/v2/workflows?all_projects=1', + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) + + @mock.patch.object(db_api, "update_workflow_definition") + def test_workflow_update_not_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"workflows:update": "role:FAKE"} + ) + resp = self.app.put( + '/v2/workflows', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "update_workflow_definition") + def test_workflow_update_allowed(self, mock_obj): + self.policy.change_policy_definition( + {"workflows:update": "role:FAKE or rule:admin_or_owner"} + ) + resp = self.app.put( + '/v2/workflows', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) + + @mock.patch.object(db_api, "update_workflow_definition") + def test_workflow_update_public_not_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "workflows:update": "role:FAKE or rule:admin_or_owner", + "workflows:publicize": "role:FAKE" + }) + resp = self.app.put( + '/v2/workflows?scope=public', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(403, resp.status_int) + + @mock.patch.object(db_api, "update_workflow_definition") + def test_workflow_update_public_allowed(self, mock_obj): + self.policy.change_policy_definition({ + "workflows:update": "role:FAKE or rule:admin_or_owner", + "workflows:publicize": "role:FAKE or rule:admin_or_owner" + }) + resp = self.app.put( + '/v2/workflows?scope=public', + WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(200, resp.status_int) diff --git a/releasenotes/notes/add-publicize-policy-d3b44590286c7fdd.yaml b/releasenotes/notes/add-publicize-policy-d3b44590286c7fdd.yaml new file mode 100644 index 000000000..9fc7533a5 --- /dev/null +++ b/releasenotes/notes/add-publicize-policy-d3b44590286c7fdd.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Mistral now supports a `publicize` policy on actions and workflows which + controls whether the users are allowed to create or update them. The + default policy does not change which means that everyone can publish + action or workflow unless specified differently in the policy.