openstack/manila
Code Issues Proposed changes

Fix access rule visibility locks

Browse Source 
Access rule visibility and deletion locks were not being properly
retrieved when listing, showing and deleting access rules, leading
to an unexpected behavior.

Fixes that issue by elevating the context and making sure that
we are looking for all of the locks placed against the access
rule.

Closes-Bug: #2089061
Change-Id: Ib6667df25c8935826e673f180848887fe4fff8d6
Signed-off-by: Carlos Eduardo <ces.eduardo98@gmail.com>
(cherry picked from commit 3d80e8668b)
(cherry picked from commit 112791a8ea)
(cherry picked from commit 57408d5abf)
This commit is contained in:
Carlos Eduardo
2024-11-19 19:53:11 -03:00
committed by Goutham Pacha Ravi
parent fcd3ddb058
commit 3f9240a8f5
5 changed files with 87 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
manila/api/v1/shares.py
View File

@@ -585,12 +585,14 @@ class ShareMixin(object):
unrestrict = access_data.get('unrestrict', False)
search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
locks, locks_count = (
self.resource_locks_api.get_all(
context, search_opts=search_opts, show_count=True) or []
context.elevated(), search_opts=search_opts,
show_count=True) or []
)
# no locks placed, nothing to do
@@ -616,7 +618,9 @@ class ShareMixin(object):
try:
self.resource_locks_api.ensure_context_can_delete_lock(
context, lock['id'])
except exception.NotAuthorized:
except (exception.NotAuthorized, exception.ResourceLockNotFound):
# If it is not found, then it means that the context doesn't
# have access to this resource and should be denied.
non_deletable_locks.append(lock)
if non_deletable_locks:

5
manila/api/v2/share_accesses.py
View File

@@ -55,10 +55,11 @@ class ShareAccessesController(wsgi.Controller, wsgi.AdminActionsMixin):
search_opts = {
'resource_id': id,
'resource_action': constants.RESOURCE_ACTION_SHOW,
'resource_type': 'access_rule'
'resource_type': 'access_rule',
'all_projects': True,
}
locks, count = self.resource_locks_api.get_all(
context, search_opts, show_count=True)
context.elevated(), search_opts, show_count=True)
if count:
return self.resource_locks_api.access_is_restricted(context,

34
manila/tests/api/v1/test_shares.py
View File

@@ -1282,6 +1282,7 @@ class ShareActionsTest(test.TestCase):
self.mock_object(share_api.API, "deny_access", _stub_deny_access)
self.mock_object(share_api.API, "access_get", _fake_access_get)
self.mock_object(self.controller, '_check_for_access_rule_locks')
id = 'fake_share_id'
body = {"os-deny_access": {"access_id": 'fake_acces_id'}}
@@ -1299,14 +1300,20 @@ class ShareActionsTest(test.TestCase):
share_network = db_utils.create_share_network()
share = db_utils.create_share(share_network_id=share_network['id'])
self.mock_object(share_api.API, 'get', mock.Mock(return_value=share))
self.mock_object(self.controller, '_check_for_access_rule_locks')
id = 'fake_share_id'
body = {"os-deny_access": {"access_id": 'fake_acces_id'}}
access_data = {"access_id": 'fake_acces_id'}
body = {"os-deny_access": access_data}
req = fakes.HTTPRequest.blank('/v1/tenant1/shares/%s/action' % id)
res = self.controller._deny_access(req, id, body)
self.assertEqual(202, res.status_int)
self.controller._check_for_access_rule_locks.assert_called_once_with(
req.environ['manila.context'], access_data,
access_data['access_id'], id
)
self.mock_policy_check.assert_called_once_with(
req.environ['manila.context'], 'share', 'deny_access')
@@ -1316,6 +1323,7 @@ class ShareActionsTest(test.TestCase):
self.mock_object(share_api.API, "deny_access", _stub_deny_access)
self.mock_object(share_api.API, "access_get", _fake_access_get)
self.mock_object(self.controller, '_check_for_access_rule_locks')
id = 'super_fake_share_id'
body = {"os-deny_access": {"access_id": 'fake_acces_id'}}
@@ -1363,12 +1371,14 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.controller._check_for_access_rule_locks(
context, {}, access_id, share_id)
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
@@ -1387,6 +1397,7 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.assertRaises(
webob.exc.HTTPForbidden,
self.controller._check_for_access_rule_locks,
@@ -1394,7 +1405,8 @@ class ShareActionsTest(test.TestCase):
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
@@ -1419,6 +1431,7 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.assertRaises(
webob.exc.HTTPForbidden,
self.controller._check_for_access_rule_locks,
@@ -1426,7 +1439,8 @@ class ShareActionsTest(test.TestCase):
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
@@ -1457,6 +1471,7 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.assertRaises(
webob.exc.HTTPForbidden,
self.controller._check_for_access_rule_locks,
@@ -1464,7 +1479,8 @@ class ShareActionsTest(test.TestCase):
)
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
context, search_opts=delete_search_opts, show_count=True
@@ -1491,15 +1507,19 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.controller._check_for_access_rule_locks(
context, {'unrestrict': True}, access_id, share_id)
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
context, search_opts=delete_search_opts, show_count=True)
context.elevated(), search_opts=delete_search_opts,
show_count=True
)
(resource_locks.API.ensure_context_can_delete_lock
.assert_called_once_with(
context, locks[0]['id']))

42
manila/tests/lock/test_api.py
View File

@@ -517,3 +517,45 @@ class ResourceLockApiTest(test.TestCase):
utils.IsAMatcher(context.RequestContext),
'd767d3cd-1187-404a-a91f-8b172e0e768e'
)
def test_ensure_context_can_delete_lock_policy_fails(self):
lock = {'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'}
self.mock_object(
self.lock_api.db, 'resource_lock_get', mock.Mock(return_value=lock)
)
self.mock_object(
policy,
'check_policy',
mock.Mock(side_effect=exception.PolicyNotAuthorized(
action="resource_lock:delete")),
)
self.assertRaises(
exception.NotAuthorized,
self.lock_api.ensure_context_can_delete_lock,
self.ctxt,
'd767d3cd-1187-404a-a91f-8b172e0e768e')
self.lock_api.db.resource_lock_get.assert_called_once_with(
self.ctxt, 'd767d3cd-1187-404a-a91f-8b172e0e768e'
)
policy.check_policy.assert_called_once_with(
self.ctxt, 'resource_lock', 'delete', lock)
def test_ensure_context_can_delete_lock(self):
lock = {'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'}
self.mock_object(
self.lock_api.db, 'resource_lock_get', mock.Mock(return_value=lock)
)
self.mock_object(policy, 'check_policy')
self.mock_object(self.lock_api, '_check_allow_lock_manipulation')
self.lock_api.ensure_context_can_delete_lock(
self.ctxt,
'd767d3cd-1187-404a-a91f-8b172e0e768e')
self.lock_api.db.resource_lock_get.assert_called_once_with(
self.ctxt, 'd767d3cd-1187-404a-a91f-8b172e0e768e'
)
policy.check_policy.assert_called_once_with(
self.ctxt, 'resource_lock', 'delete', lock)

8
releasenotes/notes/bug-2089061-fix-access-rules-locks-lookup-b5efbd41397acba3.yaml Normal file
View File

@@ -0,0 +1,8 @@
---
fixes:
- |
While displaying and deleting access rules, manila was limiting the search
for locks to the context of the request. Now, manila will search within
all of the projects for locks and properly apply visibility and deletion
restrictions. For more details, please refer to
`launchpad bug #2089061 <https://bugs.launchpad.net/manila/+bug/2089061>`_.