summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorjiaopengju <jiaopengju@cmss.chinamobile.com>2018-11-25 21:04:25 +0800
committerjiaopengju <jiaopengju@cmss.chinamobile.com>2018-11-25 21:09:11 +0800
commit37c197851a6a099d8c274920be62703c627b8a3d (patch)
treef493c9a6decf7bbea4cd32ae5ea5faa65779e16f
parent2df8d3400faa5c9a88d084714ed12dbf2cb4bcc7 (diff)
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
Notes
Notes (review): Code-Review+2: Jiao Pengju <jiaopengju@cmss.chinamobile.com> Workflow+1: Jiao Pengju <jiaopengju@cmss.chinamobile.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Mon, 26 Nov 2018 01:16:12 +0000 Reviewed-on: https://review.openstack.org/619926 Project: openstack/karbor Branch: refs/heads/master
-rw-r--r--karbor/api/v1/restores.py2
-rw-r--r--karbor/services/protection/manager.py8
-rw-r--r--karbor/tests/unit/api/v1/test_restores.py10
-rw-r--r--karbor/tests/unit/protection/test_manager.py14
4 files changed, 33 insertions, 1 deletions
diff --git a/karbor/api/v1/restores.py b/karbor/api/v1/restores.py
index fca30da..3dea082 100644
--- a/karbor/api/v1/restores.py
+++ b/karbor/api/v1/restores.py
@@ -232,6 +232,8 @@ class RestoresController(wsgi.Controller):
232 # call restore rpc API of protection service 232 # call restore rpc API of protection service
233 try: 233 try:
234 self.protection_api.restore(context, restoreobj, restore_auth) 234 self.protection_api.restore(context, restoreobj, restore_auth)
235 except exception.AccessCheckpointNotAllowed as error:
236 raise exc.HTTPForbidden(explanation=error.msg)
235 except Exception: 237 except Exception:
236 # update the status of restore 238 # update the status of restore
237 update_dict = { 239 update_dict = {
diff --git a/karbor/services/protection/manager.py b/karbor/services/protection/manager.py
index c3db38c..a2de22e 100644
--- a/karbor/services/protection/manager.py
+++ b/karbor/services/protection/manager.py
@@ -182,7 +182,8 @@ class ProtectionManager(manager.Manager):
182 exception.CheckpointNotFound, 182 exception.CheckpointNotFound,
183 exception.CheckpointNotAvailable, 183 exception.CheckpointNotAvailable,
184 exception.FlowError, 184 exception.FlowError,
185 exception.InvalidInput) 185 exception.InvalidInput,
186 exception.AccessCheckpointNotAllowed)
186 def restore(self, context, restore, restore_auth): 187 def restore(self, context, restore, restore_auth):
187 LOG.info("Starting restore service:restore action") 188 LOG.info("Starting restore service:restore action")
188 189
@@ -197,6 +198,11 @@ class ProtectionManager(manager.Manager):
197 checkpoint_collection = provider.get_checkpoint_collection() 198 checkpoint_collection = provider.get_checkpoint_collection()
198 checkpoint = checkpoint_collection.get(checkpoint_id) 199 checkpoint = checkpoint_collection.get(checkpoint_id)
199 200
201 if not context.is_admin and (
202 checkpoint.project_id != context.project_id):
203 raise exception.AccessCheckpointNotAllowed(
204 checkpoint_id=checkpoint_id)
205
200 if checkpoint.status != constants.CHECKPOINT_STATUS_AVAILABLE: 206 if checkpoint.status != constants.CHECKPOINT_STATUS_AVAILABLE:
201 raise exception.CheckpointNotAvailable( 207 raise exception.CheckpointNotAvailable(
202 checkpoint_id=checkpoint_id) 208 checkpoint_id=checkpoint_id)
diff --git a/karbor/tests/unit/api/v1/test_restores.py b/karbor/tests/unit/api/v1/test_restores.py
index 8d4b50c..79fb177 100644
--- a/karbor/tests/unit/api/v1/test_restores.py
+++ b/karbor/tests/unit/api/v1/test_restores.py
@@ -75,6 +75,16 @@ class RestoreApiTest(base.TestCase):
75 self.assertRaises(exception.ValidationError, self.controller.create, 75 self.assertRaises(exception.ValidationError, self.controller.create,
76 req, body=body) 76 req, body=body)
77 77
78 @mock.patch('karbor.services.protection.api.API.restore')
79 def test_restore_create_with_checkpoint_not_allowed_exception(
80 self, mock_restore):
81 mock_restore.side_effect = exception.AccessCheckpointNotAllowed
82 restore = self._restore_in_request_body()
83 body = {"restore": restore}
84 req = fakes.HTTPRequest.blank('/v1/restores')
85 self.assertRaises(exc.HTTPForbidden, self.controller.create,
86 req, body=body)
87
78 @mock.patch( 88 @mock.patch(
79 'karbor.api.v1.restores.RestoresController._get_all') 89 'karbor.api.v1.restores.RestoresController._get_all')
80 def test_restore_list_detail(self, moak_get_all): 90 def test_restore_list_detail(self, moak_get_all):
diff --git a/karbor/tests/unit/protection/test_manager.py b/karbor/tests/unit/protection/test_manager.py
index 7528f02..6ecf263 100644
--- a/karbor/tests/unit/protection/test_manager.py
+++ b/karbor/tests/unit/protection/test_manager.py
@@ -147,6 +147,20 @@ class ProtectionServiceTest(base.TestCase):
147 fakes.fake_protection_plan()) 147 fakes.fake_protection_plan())
148 148
149 @mock.patch.object(provider.ProviderRegistry, 'show_provider') 149 @mock.patch.object(provider.ProviderRegistry, 'show_provider')
150 def test_restore_with_project_id_not_same(self, mock_provider):
151 mock_provider.return_value = fakes.FakeProvider()
152 context = mock.MagicMock(project_id='fake_project_id_1',
153 is_admin=False)
154 fake_restore = {
155 'checkpoint_id': 'fake_checkpoint',
156 'provider_id': 'fake_provider_id',
157 'parameters': None
158 }
159 self.assertRaises(
160 oslo_messaging.ExpectedException, self.pro_manager.restore,
161 context, fake_restore, None)
162
163 @mock.patch.object(provider.ProviderRegistry, 'show_provider')
150 def test_list_checkpoints(self, mock_provider): 164 def test_list_checkpoints(self, mock_provider):
151 fake_provider = fakes.FakeProvider() 165 fake_provider = fakes.FakeProvider()
152 fake_provider.list_checkpoints = mock.MagicMock() 166 fake_provider.list_checkpoints = mock.MagicMock()