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

@@ -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())

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)

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)