From 3f86310b44105b53846820d262d82091cf58f8f0 Mon Sep 17 00:00:00 2001 From: MonicaJoshi Date: Wed, 11 Oct 2017 05:41:58 -0400 Subject: [PATCH] Raise PolicyNotAuthorized exception on consistency Group snapshot In role based access control test case, a consistencyGroup snapshot delete when attempted by Non-admin role/user does not return an informative policy not authorized message. The proposed fix catches the PolicyNotAuthorized exception explicitly to raise the appropriate message and error code to the caller method, includes a new unit test case. Closes-bug:#1722736 Change-Id: I263c3f5201ac76773a9c962ec958f02f601c6adc --- cinder/api/contrib/cgsnapshots.py | 10 +++--- .../unit/api/contrib/test_cgsnapshots.py | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/cinder/api/contrib/cgsnapshots.py b/cinder/api/contrib/cgsnapshots.py index 16344671b27..02b37a07de4 100644 --- a/cinder/api/contrib/cgsnapshots.py +++ b/cinder/api/contrib/cgsnapshots.py @@ -57,17 +57,17 @@ class CgsnapshotsController(wsgi.Controller): context = req.environ['cinder.context'] LOG.info('Delete cgsnapshot with id: %s', id) - try: cgsnapshot = self._get_cgsnapshot(context, id) self.group_snapshot_api.delete_group_snapshot(context, cgsnapshot) - except exception.GroupSnapshotNotFound: - # Not found exception will be handled at the wsgi level - raise except exception.InvalidGroupSnapshot as e: raise exc.HTTPBadRequest(explanation=six.text_type(e)) + except (exception.GroupSnapshotNotFound, + exception.PolicyNotAuthorized) as e: + # Exceptions will be handled at the wsgi level + raise except Exception: - msg = _("Failed cgsnapshot") + msg = _('Failed to delete the cgsnapshot') raise exc.HTTPBadRequest(explanation=msg) return webob.Response(status_int=http_client.ACCEPTED) diff --git a/cinder/tests/unit/api/contrib/test_cgsnapshots.py b/cinder/tests/unit/api/contrib/test_cgsnapshots.py index e7025b5114a..1fe0b1f899a 100644 --- a/cinder/tests/unit/api/contrib/test_cgsnapshots.py +++ b/cinder/tests/unit/api/contrib/test_cgsnapshots.py @@ -529,3 +529,34 @@ class CgsnapshotsAPITestCase(test.TestCase): cgsnapshot.destroy() db.volume_destroy(context.get_admin_context(), volume_id) consistencygroup.destroy() + + @mock.patch('cinder.group.API.delete_group_snapshot') + def test_delete_cgsnapshot_delete_policy_not_auth(self, mock_delete): + vol_type = utils.create_volume_type(context.get_admin_context(), + self, name='my_vol_type') + consistencygroup = utils.create_group( + self.context, + group_type_id=fake.GROUP_TYPE_ID, + volume_type_ids=[vol_type['id']]) + volume_id = utils.create_volume(self.context, + volume_type_id=vol_type['id'], + group_id= + consistencygroup.id)['id'] + cgsnapshot = utils.create_group_snapshot( + self.context, group_id=consistencygroup.id, + group_type_id=fake.GROUP_TYPE_ID, + status='available') + mock_delete.side_effect = exception.PolicyNotAuthorized( + message='PolicyNotAuthorized') + req = webob.Request.blank('/v2/%s/cgsnapshots/%s' % + (fake.PROJECT_ID, cgsnapshot.id)) + req.method = 'DELETE' + req.headers['Content-Type'] = 'application/json' + res = req.get_response(fakes.wsgi_app( + fake_auth_context=self.user_ctxt)) + res_dict = jsonutils.loads(res.body) + self.assertEqual('PolicyNotAuthorized', + res_dict['forbidden']['message']) + cgsnapshot.destroy() + db.volume_destroy(context.get_admin_context(), volume_id) + consistencygroup.destroy()