Fix: show volume transfer by name for non-admins
When we try to show a volume transfer as a non-admin, it doesn't work. The problem is that the 'all_tenants' key isn't parsable by the DB, we remove it for admins but not for non-admins. Since all_tenants is just used to redirect to the correct db method, it should be removed for non-admins as well. Change-Id: I79362ee6b819c6472fa5e69ea1a85a1185dda5a3 Closes-Bug: #1884268
This commit is contained in:
parent
b204df52cc
commit
1f733cdf2d
@ -14,6 +14,7 @@
|
|||||||
|
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
|
import ddt
|
||||||
from oslo_utils import timeutils
|
from oslo_utils import timeutils
|
||||||
|
|
||||||
from cinder import context
|
from cinder import context
|
||||||
@ -33,6 +34,7 @@ from cinder.transfer import api as transfer_api
|
|||||||
QUOTAS = quota.QUOTAS
|
QUOTAS = quota.QUOTAS
|
||||||
|
|
||||||
|
|
||||||
|
@ddt.ddt
|
||||||
class VolumeTransferTestCase(test.TestCase):
|
class VolumeTransferTestCase(test.TestCase):
|
||||||
"""Test cases for volume transfer code."""
|
"""Test cases for volume transfer code."""
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
@ -289,6 +291,38 @@ class VolumeTransferTestCase(test.TestCase):
|
|||||||
ts = tx_api.get_all(nctxt)
|
ts = tx_api.get_all(nctxt)
|
||||||
self.assertEqual(0, len(ts), 'Unexpected transfers listed.')
|
self.assertEqual(0, len(ts), 'Unexpected transfers listed.')
|
||||||
|
|
||||||
|
@ddt.data({'all_tenants': '1', 'name': 'transfer1'},
|
||||||
|
{'all_tenants': 'true', 'name': 'transfer1'},
|
||||||
|
{'all_tenants': 'false', 'name': 'transfer1'},
|
||||||
|
{'all_tenants': '0', 'name': 'transfer1'},
|
||||||
|
{'name': 'transfer1'})
|
||||||
|
@mock.patch.object(context.RequestContext, 'authorize')
|
||||||
|
@mock.patch('cinder.db.transfer_get_all')
|
||||||
|
@mock.patch('cinder.db.transfer_get_all_by_project')
|
||||||
|
def test_get_all_transfers_non_admin(self, search_opts, get_all_by_project,
|
||||||
|
get_all, auth_mock):
|
||||||
|
ctxt = context.RequestContext(user_id=None, is_admin=False,
|
||||||
|
project_id=mock.sentinel.project_id,
|
||||||
|
read_deleted='no', overwrite=False)
|
||||||
|
tx_api = transfer_api.API()
|
||||||
|
res = tx_api.get_all(ctxt, mock.sentinel.marker,
|
||||||
|
mock.sentinel.limit, mock.sentinel.sort_keys,
|
||||||
|
mock.sentinel.sort_dirs,
|
||||||
|
search_opts, mock.sentinel.offset)
|
||||||
|
|
||||||
|
auth_mock.assert_called_once_with(transfer_api.policy.GET_ALL_POLICY)
|
||||||
|
get_all.assert_not_called()
|
||||||
|
get_all_by_project.assert_called_once_with(
|
||||||
|
ctxt,
|
||||||
|
mock.sentinel.project_id,
|
||||||
|
filters={'name': 'transfer1'},
|
||||||
|
limit=mock.sentinel.limit,
|
||||||
|
marker=mock.sentinel.marker,
|
||||||
|
offset=mock.sentinel.offset,
|
||||||
|
sort_dirs=mock.sentinel.sort_dirs,
|
||||||
|
sort_keys=mock.sentinel.sort_keys)
|
||||||
|
self.assertEqual(get_all_by_project.return_value, res)
|
||||||
|
|
||||||
@mock.patch('cinder.volume.volume_utils.notify_about_volume_usage')
|
@mock.patch('cinder.volume.volume_utils.notify_about_volume_usage')
|
||||||
def test_delete_transfer_with_deleted_volume(self, mock_notify):
|
def test_delete_transfer_with_deleted_volume(self, mock_notify):
|
||||||
# create a volume
|
# create a volume
|
||||||
|
@ -25,6 +25,7 @@ import os
|
|||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
from oslo_utils import excutils
|
from oslo_utils import excutils
|
||||||
|
from oslo_utils import strutils
|
||||||
import six
|
import six
|
||||||
|
|
||||||
from cinder.db import base
|
from cinder.db import base
|
||||||
@ -84,8 +85,9 @@ class API(base.Base):
|
|||||||
sort_dirs=None, filters=None, offset=None):
|
sort_dirs=None, filters=None, offset=None):
|
||||||
filters = filters or {}
|
filters = filters or {}
|
||||||
context.authorize(policy.GET_ALL_POLICY)
|
context.authorize(policy.GET_ALL_POLICY)
|
||||||
if context.is_admin and 'all_tenants' in filters:
|
all_tenants = strutils.bool_from_string(filters.pop('all_tenants',
|
||||||
del filters['all_tenants']
|
'false'))
|
||||||
|
if context.is_admin and all_tenants:
|
||||||
transfers = self.db.transfer_get_all(context, marker=marker,
|
transfers = self.db.transfer_get_all(context, marker=marker,
|
||||||
limit=limit,
|
limit=limit,
|
||||||
sort_keys=sort_keys,
|
sort_keys=sort_keys,
|
||||||
|
@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
`Bug #1884268 <https://bugs.launchpad.net/cinder/+bug/1884268>`_:
|
||||||
|
Fixed issue where non-admin users could not show a volume transfer by name.
|
Loading…
x
Reference in New Issue
Block a user