Add permission check when creating restore
Currently, general users have rights to restore other users' checkpoints. We should add permission check to avoid the security risk. Closes-Bug: #1805004 Change-Id: If0f957a3aa8f25778833d7611342fab6b8efa388
This commit is contained in:
parent
2df8d3400f
commit
37c197851a
@ -232,6 +232,8 @@ class RestoresController(wsgi.Controller):
|
|||||||
# call restore rpc API of protection service
|
# call restore rpc API of protection service
|
||||||
try:
|
try:
|
||||||
self.protection_api.restore(context, restoreobj, restore_auth)
|
self.protection_api.restore(context, restoreobj, restore_auth)
|
||||||
|
except exception.AccessCheckpointNotAllowed as error:
|
||||||
|
raise exc.HTTPForbidden(explanation=error.msg)
|
||||||
except Exception:
|
except Exception:
|
||||||
# update the status of restore
|
# update the status of restore
|
||||||
update_dict = {
|
update_dict = {
|
||||||
|
@ -182,7 +182,8 @@ class ProtectionManager(manager.Manager):
|
|||||||
exception.CheckpointNotFound,
|
exception.CheckpointNotFound,
|
||||||
exception.CheckpointNotAvailable,
|
exception.CheckpointNotAvailable,
|
||||||
exception.FlowError,
|
exception.FlowError,
|
||||||
exception.InvalidInput)
|
exception.InvalidInput,
|
||||||
|
exception.AccessCheckpointNotAllowed)
|
||||||
def restore(self, context, restore, restore_auth):
|
def restore(self, context, restore, restore_auth):
|
||||||
LOG.info("Starting restore service:restore action")
|
LOG.info("Starting restore service:restore action")
|
||||||
|
|
||||||
@ -197,6 +198,11 @@ class ProtectionManager(manager.Manager):
|
|||||||
checkpoint_collection = provider.get_checkpoint_collection()
|
checkpoint_collection = provider.get_checkpoint_collection()
|
||||||
checkpoint = checkpoint_collection.get(checkpoint_id)
|
checkpoint = checkpoint_collection.get(checkpoint_id)
|
||||||
|
|
||||||
|
if not context.is_admin and (
|
||||||
|
checkpoint.project_id != context.project_id):
|
||||||
|
raise exception.AccessCheckpointNotAllowed(
|
||||||
|
checkpoint_id=checkpoint_id)
|
||||||
|
|
||||||
if checkpoint.status != constants.CHECKPOINT_STATUS_AVAILABLE:
|
if checkpoint.status != constants.CHECKPOINT_STATUS_AVAILABLE:
|
||||||
raise exception.CheckpointNotAvailable(
|
raise exception.CheckpointNotAvailable(
|
||||||
checkpoint_id=checkpoint_id)
|
checkpoint_id=checkpoint_id)
|
||||||
|
@ -75,6 +75,16 @@ class RestoreApiTest(base.TestCase):
|
|||||||
self.assertRaises(exception.ValidationError, self.controller.create,
|
self.assertRaises(exception.ValidationError, self.controller.create,
|
||||||
req, body=body)
|
req, body=body)
|
||||||
|
|
||||||
|
@mock.patch('karbor.services.protection.api.API.restore')
|
||||||
|
def test_restore_create_with_checkpoint_not_allowed_exception(
|
||||||
|
self, mock_restore):
|
||||||
|
mock_restore.side_effect = exception.AccessCheckpointNotAllowed
|
||||||
|
restore = self._restore_in_request_body()
|
||||||
|
body = {"restore": restore}
|
||||||
|
req = fakes.HTTPRequest.blank('/v1/restores')
|
||||||
|
self.assertRaises(exc.HTTPForbidden, self.controller.create,
|
||||||
|
req, body=body)
|
||||||
|
|
||||||
@mock.patch(
|
@mock.patch(
|
||||||
'karbor.api.v1.restores.RestoresController._get_all')
|
'karbor.api.v1.restores.RestoresController._get_all')
|
||||||
def test_restore_list_detail(self, moak_get_all):
|
def test_restore_list_detail(self, moak_get_all):
|
||||||
|
@ -146,6 +146,20 @@ class ProtectionServiceTest(base.TestCase):
|
|||||||
None,
|
None,
|
||||||
fakes.fake_protection_plan())
|
fakes.fake_protection_plan())
|
||||||
|
|
||||||
|
@mock.patch.object(provider.ProviderRegistry, 'show_provider')
|
||||||
|
def test_restore_with_project_id_not_same(self, mock_provider):
|
||||||
|
mock_provider.return_value = fakes.FakeProvider()
|
||||||
|
context = mock.MagicMock(project_id='fake_project_id_1',
|
||||||
|
is_admin=False)
|
||||||
|
fake_restore = {
|
||||||
|
'checkpoint_id': 'fake_checkpoint',
|
||||||
|
'provider_id': 'fake_provider_id',
|
||||||
|
'parameters': None
|
||||||
|
}
|
||||||
|
self.assertRaises(
|
||||||
|
oslo_messaging.ExpectedException, self.pro_manager.restore,
|
||||||
|
context, fake_restore, None)
|
||||||
|
|
||||||
@mock.patch.object(provider.ProviderRegistry, 'show_provider')
|
@mock.patch.object(provider.ProviderRegistry, 'show_provider')
|
||||||
def test_list_checkpoints(self, mock_provider):
|
def test_list_checkpoints(self, mock_provider):
|
||||||
fake_provider = fakes.FakeProvider()
|
fake_provider = fakes.FakeProvider()
|
||||||
|
Loading…
Reference in New Issue
Block a user