From f041606cb4f5def4837387812c6b4f379c857255 Mon Sep 17 00:00:00 2001 From: zhongjun Date: Tue, 1 Sep 2015 18:10:06 +0800 Subject: [PATCH] Do not allow to modify access for public share type Now public share type can be removed project access, but when listing access, manila returns 'Access list not available for public share types'. It's weird for users experience. We should check if a type is public and do not allow public share type to modify project access. APIImpact When modifying access for public share type, API will return 409 and message: "Invalid share type: Type access modification is not applicable to public share type." Change-Id: I94f580eeb7eef4263c631227cc3710eeae2b9706 Closes-Bug: #1490912 --- manila/api/v2/share_types.py | 27 ++++++++++------- manila/tests/api/v2/test_share_types.py | 28 ++++++++++++++++- .../api/admin/test_share_types_negative.py | 30 +++++++++++++++++++ 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/manila/api/v2/share_types.py b/manila/api/v2/share_types.py index 52afbade85..6615c98d3b 100644 --- a/manila/api/v2/share_types.py +++ b/manila/api/v2/share_types.py @@ -241,15 +241,7 @@ class ShareTypesController(wsgi.Controller): self._check_body(body, 'addProjectAccess') project = body['addProjectAccess']['project'] - try: - share_type = share_types.get_share_type(context, id) - - if share_type['is_public']: - msg = _("Project cannot be added to public share_type.") - raise webob.exc.HTTPForbidden(explanation=msg) - - except exception.ShareTypeNotFound as err: - raise webob.exc.HTTPNotFound(explanation=six.text_type(err)) + self._verify_if_non_public_share_type(context, id) try: share_types.add_share_type_access(context, id, project) @@ -265,13 +257,26 @@ class ShareTypesController(wsgi.Controller): self._check_body(body, 'removeProjectAccess') project = body['removeProjectAccess']['project'] + self._verify_if_non_public_share_type(context, id) + try: share_types.remove_share_type_access(context, id, project) - except (exception.ShareTypeNotFound, - exception.ShareTypeAccessNotFound) as err: + except exception.ShareTypeAccessNotFound as err: raise webob.exc.HTTPNotFound(explanation=six.text_type(err)) return webob.Response(status_int=202) + def _verify_if_non_public_share_type(self, context, share_type_id): + try: + share_type = share_types.get_share_type(context, share_type_id) + + if share_type['is_public']: + msg = _("Type access modification is not applicable to " + "public share type.") + raise webob.exc.HTTPConflict(explanation=msg) + + except exception.ShareTypeNotFound as err: + raise webob.exc.HTTPNotFound(explanation=six.text_type(err)) + def create_resource(): return wsgi.Resource(ShareTypesController()) diff --git a/manila/tests/api/v2/test_share_types.py b/manila/tests/api/v2/test_share_types.py index f61b109534..48f786ad29 100644 --- a/manila/tests/api/v2/test_share_types.py +++ b/manila/tests/api/v2/test_share_types.py @@ -624,7 +624,7 @@ class ShareTypeAccessTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/fake/types/2/action', use_admin_context=True) - self.assertRaises(webob.exc.HTTPForbidden, + self.assertRaises(webob.exc.HTTPConflict, self.controller._add_project_access, req, share_type_id, body) @@ -653,3 +653,29 @@ class ShareTypeAccessTest(test.TestCase): self.assertRaises(webob.exc.HTTPForbidden, self.controller._remove_project_access, req, '2', body) + + def test_remove_project_access_from_public_share_type(self): + share_type_id = '3' + body = {'removeProjectAccess': {'project': PROJ2_UUID}} + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value={"is_public": True})) + + req = fakes.HTTPRequest.blank('/v2/fake/types/2/action', + use_admin_context=True) + + self.assertRaises(webob.exc.HTTPConflict, + self.controller._remove_project_access, + req, share_type_id, body) + share_types.get_share_type.assert_called_once_with( + mock.ANY, share_type_id) + + def test_remove_project_access_by_nonexistent_share_type(self): + self.mock_object(share_types, 'get_share_type', + return_share_types_get_share_type) + body = {'removeProjectAccess': {'project': PROJ2_UUID}} + req = fakes.HTTPRequest.blank('/v2/fake/types/777/action', + use_admin_context=True) + + self.assertRaises(webob.exc.HTTPNotFound, + self.controller._remove_project_access, + req, '777', body) \ No newline at end of file diff --git a/manila_tempest_tests/tests/api/admin/test_share_types_negative.py b/manila_tempest_tests/tests/api/admin/test_share_types_negative.py index 62e4523af1..32c962019f 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types_negative.py @@ -68,3 +68,33 @@ class ShareTypesAdminNegativeTest(base.BaseSharesAdminTest): self.create_share_type, st["share_type"]["name"], extra_specs=self.add_required_extra_specs_to_dict()) + + @test.attr(type=["gate", "smoke", ]) + def test_add_share_type_allowed_for_public(self): + st = self._create_share_type() + self.assertRaises(lib_exc.Conflict, + self.shares_client.add_access_to_share_type, + st["share_type"]["id"], + self.shares_client.tenant_id) + + @test.attr(type=["gate", "smoke", ]) + def test_remove_share_type_allowed_for_public(self): + st = self._create_share_type() + self.assertRaises(lib_exc.Conflict, + self.shares_client.remove_access_from_share_type, + st["share_type"]["id"], + self.shares_client.tenant_id) + + @test.attr(type=["gate", "smoke", ]) + def test_add_share_type_by_nonexistent_id(self): + self.assertRaises(lib_exc.NotFound, + self.shares_client.add_access_to_share_type, + data_utils.rand_name("fake"), + self.shares_client.tenant_id) + + @test.attr(type=["gate", "smoke", ]) + def test_remove_share_type_by_nonexistent_id(self): + self.assertRaises(lib_exc.NotFound, + self.shares_client.remove_access_from_share_type, + data_utils.rand_name("fake"), + self.shares_client.tenant_id)