From 0f82690dddd9d28e33e1538e09c78a378a1eb9d0 Mon Sep 17 00:00:00 2001 From: silvacarloss Date: Wed, 5 Jul 2023 13:25:55 -0300 Subject: [PATCH] Allow restricting access rules fields and deletion Access rules rules allow API will now take three additional parameters: - lock_visibility: when True, only services, administrators and the same user will be able to see the content of ``access_to`` and access_key. - lock_deletion: when True, the access rule will be locked for deletion. Only services, administrators or the user that placed the lock will be able to drop the access rule. - lock_reason: a reason for the lock. This parameter should only be provided in the presence of at least one of the former parameters. In order to delete an access rule that is currently locked, the requester will need to specify ``unrestrict=True`` in the request. In case a service placed the restrictions, only the own service or the system administrator will be able to release it. This change also implements filters to the access list API. It is now possible to filter access rules based on `access_to`, `access_type`, `access_level` and `access_key`. DocImpact Change-Id: Iea422c9d6bc99a81cd88c5f4b7055d6a1cf97fdc --- api-ref/source/parameters.yaml | 24 ++ .../share-actions-grant-access-request.json | 5 +- .../share-actions-revoke-access-request.json | 3 +- api-ref/source/share-access-rules.inc | 12 + api-ref/source/share-actions.inc | 15 + .../admin/shared-file-systems-crud-share.rst | 18 + doc/source/user/create-and-manage-shares.rst | 60 ++- manila/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 5 + manila/api/v1/shares.py | 116 +++++- manila/api/v2/resource_locks.py | 50 ++- manila/api/v2/share_accesses.py | 76 +++- manila/api/v2/shares.py | 9 + manila/api/views/share_accesses.py | 9 + manila/common/constants.py | 26 ++ manila/db/api.py | 5 + manila/db/sqlalchemy/api.py | 35 ++ manila/db/sqlalchemy/models.py | 9 + manila/exception.py | 4 + manila/lock/api.py | 96 ++++- manila/policies/resource_lock.py | 27 ++ manila/tests/api/v1/test_shares.py | 393 +++++++++++++++++- manila/tests/api/v2/test_resource_locks.py | 115 +++-- manila/tests/api/v2/test_share_accesses.py | 75 +++- manila/tests/db/sqlalchemy/test_api.py | 17 + manila/tests/lock/test_api.py | 149 ++++++- ...ity-and-delete-locks-52a7ef235813d147.yaml | 13 + 27 files changed, 1262 insertions(+), 107 deletions(-) create mode 100644 releasenotes/notes/add-access-visibility-and-delete-locks-52a7ef235813d147.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index c75f598d7d..b32459e8fd 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1632,6 +1632,22 @@ links: in: body required: true type: array +lock_deletion: + description: | + Whether the resource should have its deletion locked or not. + in: body + required: false + type: string + min_version: 2.82 +lock_visibility: + description: | + Whether the resource should have its sensitive fields restricted or not. + When enabled, other users will see the "access_to" and "access_secret" + fields set to ****** + in: body + required: false + type: string + min_version: 2.82 manage_host: description: | The host of the destination back end, in this format: ``host@backend``. @@ -3917,6 +3933,14 @@ unit: in: body required: false type: string +unrestrict_access: + description: | + Whether the service should attempt to remove deletion restrictions during + the access rule deletion or not. + in: body + required: false + type: string + min_version: 2.82 updated_at: description: | The date and time stamp when the resource was last updated within the diff --git a/api-ref/source/samples/share-actions-grant-access-request.json b/api-ref/source/samples/share-actions-grant-access-request.json index 31b78ac743..bb26367ac8 100644 --- a/api-ref/source/samples/share-actions-grant-access-request.json +++ b/api-ref/source/samples/share-actions-grant-access-request.json @@ -6,6 +6,9 @@ "metadata":{ "key1": "value1", "key2": "value2" - } + }, + "lock_visibility": false, + "lock_deletion": true, + "lock_reason": "Locked for deletion until year end audit." } } diff --git a/api-ref/source/samples/share-actions-revoke-access-request.json b/api-ref/source/samples/share-actions-revoke-access-request.json index ca87e51ea0..410f053462 100644 --- a/api-ref/source/samples/share-actions-revoke-access-request.json +++ b/api-ref/source/samples/share-actions-revoke-access-request.json @@ -1,5 +1,6 @@ { "deny_access": { - "access_id": "a25b2df3-90bd-4add-afa6-5f0dbbd50452" + "access_id": "a25b2df3-90bd-4add-afa6-5f0dbbd50452", + "unrestrict": true } } diff --git a/api-ref/source/share-access-rules.inc b/api-ref/source/share-access-rules.inc index fad429064c..c83b1556a2 100644 --- a/api-ref/source/share-access-rules.inc +++ b/api-ref/source/share-access-rules.inc @@ -7,6 +7,12 @@ Share access rules (since API v2.45) Retrieve details about access rules +.. note:: + Starting from API version 2.82, access rule visibility can be restricted + by a project user, or any user with "service" or "admin" roles. When + restricted, the access_to and access_key fields will be redacted to other + users. This redaction applies irrespective of the API version. + Describe share access rule ~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -74,6 +80,12 @@ Lists the share access rules on a share. This API replaces the older :ref:`List share access rules ` API from version 2.45. +.. note:: + Starting from API version 2.82, access rule visibility can be restricted + by a project user, or any user with "service" or "admin" roles. When + restricted, the access_to and access_key fields will be redacted to other + users. This redaction applies irrespective of the API version. + Response codes -------------- diff --git a/api-ref/source/share-actions.inc b/api-ref/source/share-actions.inc index fc83e3f95d..31625aeda3 100644 --- a/api-ref/source/share-actions.inc +++ b/api-ref/source/share-actions.inc @@ -59,6 +59,13 @@ methods: IPv6 based access is only supported with API version 2.38 and beyond. +.. note:: + Starting from API version 2.82, it is possible to lock the deletion, + restrict the visibility of sensible fields of the access rules, and specify a + reason for such locks while invoking the grant access API through the + parameters ``lock_deletion``, ``lock_visibility`` and ``lock_reason`` + respectively. + - ``cert``. Authenticates an instance through a TLS certificate. Specify the TLS identity as the IDENTKEY. A valid value is any string up to 64 characters long in the common name (CN) of the @@ -99,6 +106,9 @@ Request - access_type: access_type - access_to: access_to - metadata: access_metadata_grant_access + - lock_visibility: lock_visibility + - lock_deletion: lock_deletion + - lock_reason: resource_lock_lock_reason Request example --------------- @@ -138,6 +148,10 @@ The shared file systems service stores each access rule in its database and assigns it a unique ID. This ID can be used to revoke access after access has been requested. +.. note:: + In case the access rule had its deletion locked, it will be necessary to + provide the ``unrestrict`` parameter in the revoke access request. + Response codes -------------- @@ -161,6 +175,7 @@ Request - share_id: share_id - deny_access: deny_access - access_id: access_id + - unrestrict: unrestrict_access Request example diff --git a/doc/source/admin/shared-file-systems-crud-share.rst b/doc/source/admin/shared-file-systems-crud-share.rst index 8ab2dcfd05..b15cf26172 100644 --- a/doc/source/admin/shared-file-systems-crud-share.rst +++ b/doc/source/admin/shared-file-systems-crud-share.rst @@ -740,6 +740,17 @@ Allow access to the share with ``user`` access type: features support mapping `_. +.. tip:: + + Starting from the 2023.2 (Bobcat) release, in case you want to restrict the + visibility of the sensitive fields (``access_to`` and ``access_key``), or + avoid the access rule being deleted by other users, you can specify + ``--lock-visibility`` and ``--lock-deletion`` in the Manila OpenStack command + for creating access rules. A reason (``--lock-reason``) can also be provided. + Only the user that placed the lock, system administrators and services will + be able to view sensitive fields of, or manipulate such access rules by + virtue of default RBAC. + To verify that the access rules (ACL) were configured correctly for a share, you list permissions for a share: @@ -766,3 +777,10 @@ access rule list: +--------------------------------------+-------------+-----------+--------------+-------+ | 4f391c6b-fb4f-47f5-8b4b-88c5ec9d568a | user | demo | rw | error | +--------------------------------------+-------------+-----------+--------------+-------+ + +.. note:: + + Starting from the 2023.2 (Bobcat) release, it is possible to prevent the + deletion of an access rule. In case the deletion was locked, the + ``--unrestrict`` argument from the Manila's OpenStack Client must be used + in the request to revoke the access. diff --git a/doc/source/user/create-and-manage-shares.rst b/doc/source/user/create-and-manage-shares.rst index a8d70a655a..5d9740c7f3 100644 --- a/doc/source/user/create-and-manage-shares.rst +++ b/doc/source/user/create-and-manage-shares.rst @@ -7,7 +7,7 @@ Create and manage shares .. contents:: :local: General Concepts -~~~~~~~~~~~~~~~~ +---------------- A ``share`` is filesystem storage that you can create with manila. You can pick a network protocol for the underlying storage, manage access and perform @@ -97,7 +97,7 @@ important terms: Usage and Limits -~~~~~~~~~~~~~~~~ +---------------- * List the resource limits and usages that apply to your project @@ -124,7 +124,7 @@ Usage and Limits +----------------------------+-------+ Share types -~~~~~~~~~~~ +----------- * List share types @@ -149,7 +149,7 @@ Share types +--------------------------------------+-----------------------------------+------------+------------+--------------------------------------+--------------------------------------------+---------------------------------------------------------+ Share networks -~~~~~~~~~~~~~~ +-------------- * Create a share network. @@ -191,7 +191,7 @@ Share networks +--------------------------------------+----------------+ Create a share -~~~~~~~~~~~~~~ +-------------- * Create a share @@ -366,6 +366,19 @@ Create a share | 40de4f4c-4588-4d9c-844b-f74d8951053a | myshare2 | 1 | NFS | available | False | default | nosb-devstack@lisboa#LISBOA | nova | +--------------------------------------+-----------+------+-------------+-----------+-----------+-----------------+-----------------------------+-------------------+ +Grant and revoke share access +----------------------------- + +.. tip:: + + Starting from the 2023.2 (Bobcat) release, in case you want to restrict the + visibility of the sensitive fields (``access_to`` and ``access_key``), or + avoid the access rule being deleted by other users, you can specify + ``--lock-visibility`` and ``--lock-deletion`` in the Manila OpenStack command + for creating access rules. A reason (``--lock-reason``) can also be provided. + Only the user that placed the lock, system administrators and services will + be able to manipulate such access rules. + Allow read-write access ~~~~~~~~~~~~~~~~~~~~~~~ @@ -448,8 +461,14 @@ Allow read-only access Another access rule is created. +.. note:: + + In case one or more access rules had its visibility locked, you might not be + able to see the content of the fields containing sensitive information + (``access_to`` and ``access_key``). + Update access rules metadata -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +---------------------------- #. Add a new metadata. @@ -494,7 +513,7 @@ Update access rules metadata +--------------+--------------------------------------+ Deny access -~~~~~~~~~~~ +----------- * Deny access. @@ -503,6 +522,13 @@ Deny access $ manila access-deny myshare 45b0a030-306a-4305-9e2a-36aeffb2d5b7 $ manila access-deny myshare e30bde96-9217-4f90-afdc-27c092af1c77 +.. note:: + + Starting from the 2023.2 (Bobcat) release, it is possible to prevent the + deletion of an access rule. In case you have placed a deletion lock during + the access rule creation, the ``--unrestrict`` argument from the Manila's + OpenStack Client must be used in the request to revoke the access. + * List access. .. code-block:: console @@ -516,7 +542,7 @@ Deny access The access rules are removed. Create snapshot -~~~~~~~~~~~~~~~ +--------------- * Create a snapshot. @@ -556,7 +582,7 @@ Create snapshot +--------------------------------------+--------------------------------------+-----------+------------+------------+ Create share from snapshot -~~~~~~~~~~~~~~~~~~~~~~~~~~ +-------------------------- * Create a share from a snapshot. @@ -661,7 +687,7 @@ Create share from snapshot +---------------------------------------+----------------------------------------------------------------------------------------------------------------------+ Delete share -~~~~~~~~~~~~ +------------ * Delete a share. @@ -684,7 +710,7 @@ Delete share The share is being deleted. Delete snapshot -~~~~~~~~~~~~~~~ +--------------- * Delete a snapshot. @@ -706,7 +732,7 @@ Delete snapshot The snapshot is deleted. Extend share -~~~~~~~~~~~~ +------------ * Extend share. @@ -803,7 +829,7 @@ Extend share +---------------------------------------+----------------------------------------------------------------------------------------------------------------------+ Shrink share -~~~~~~~~~~~~ +------------ * Shrink a share. @@ -900,7 +926,7 @@ Shrink share +---------------------------------------+----------------------------------------------------------------------------------------------------------------------+ Share metadata -~~~~~~~~~~~~~~ +-------------- * Set metadata items on your share @@ -938,7 +964,7 @@ Share metadata $ manila metadata myshare unset year_started Share revert to snapshot -~~~~~~~~~~~~~~~~~~~~~~~~ +------------------------ * Share revert to snapshot @@ -955,7 +981,7 @@ Share revert to snapshot $ manila revert-to-snapshot mysnapshot Share Transfer -~~~~~~~~~~~~~~ +-------------- * Transfer a share to a different project @@ -1032,7 +1058,7 @@ Share Transfer +------------------------+--------------------------------------+ Resource locks -~~~~~~~~~~~~~~ +-------------- * Prevent a share from being deleted by creating a ``resource lock``: diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 1071e9c0ef..228cfe6757 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -199,13 +199,14 @@ REST_API_VERSION_HISTORY = """ count info. * 2.80 - Added share backup APIs. * 2.81 - Added API methods, endpoint /resource-locks. + * 2.82 - Added lock and restriction to share access rules. """ # The minimum and maximum versions of the API supported # The default api version request is defined to be the # minimum version of the API supported. _MIN_API_VERSION = "2.0" -_MAX_API_VERSION = "2.81" +_MAX_API_VERSION = "2.82" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index b7ce842564..1d9f2b4db7 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -439,3 +439,8 @@ ____ ---- Introduce resource locks as a way users can restrict certain actions on resources. Only share deletion can be prevented at this version. + +2.82 +---- + Introduce the ability to lock access rules and restrict the visibility of + sensitive fields. diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index b36b9b00f9..8f7cde0e96 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -32,6 +32,7 @@ from manila.common import constants from manila import db from manila import exception from manila.i18n import _ +from manila.lock import api as resource_locks from manila import share from manila.share import share_types from manila import utils @@ -455,10 +456,61 @@ class ShareMixin(object): return True return False + def _create_access_locks( + self, context, access, lock_deletion=False, lock_visibility=False, + lock_reason=None): + """Creates locks for access rules and rollback if it fails.""" + + # We must populate project_id and user_id in the access object, as this + # is not in this entity + access['project_id'] = context.project_id + access['user_id'] = context.user_id + + def raise_lock_failed(access, lock_action): + word_mapping = { + constants.RESOURCE_ACTION_SHOW: 'visibility', + constants.RESOURCE_ACTION_DELETE: 'deletion' + } + msg = _("Failed to lock the %(action)s of the access rule " + "%(rule)s.") % { + 'action': word_mapping[lock_action], + 'rule': access['id'] + } + raise webob.exc.HTTPBadRequest(explanation=msg) + + deletion_lock = {} + + if lock_deletion: + try: + deletion_lock = self.resource_locks_api.create( + context, resource_id=access['id'], + resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_DELETE, + resource=access, lock_reason=lock_reason) + except Exception: + raise_lock_failed(access, constants.RESOURCE_ACTION_DELETE) + + if lock_visibility: + try: + self.resource_locks_api.create( + context, resource_id=access['id'], + resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_SHOW, + resource=access, lock_reason=lock_reason) + except Exception: + # If a deletion lock was placed and the visibility wasn't, + # we should rollback the deletion lock. + if deletion_lock: + self.resource_locks_api.delete( + context, deletion_lock['id']) + raise_lock_failed(access, constants.RESOURCE_ACTION_SHOW) + @wsgi.Controller.authorize('allow_access') def _allow_access(self, req, id, body, enable_ceph=False, allow_on_error_status=False, enable_ipv6=False, - enable_metadata=False, allow_on_error_state=False): + enable_metadata=False, allow_on_error_state=False, + lock_visibility=False, lock_deletion=False, + lock_reason=None): """Add share access rule.""" context = req.environ['manila.context'] access_data = body.get('allow_access', body.get('os-allow_access')) @@ -487,6 +539,11 @@ class ShareMixin(object): } raise webob.exc.HTTPBadRequest(explanation=msg) + if not (lock_visibility or lock_deletion) and lock_reason: + msg = _("Lock reason can only be specified when locking the " + "visibility or the deletion of an access rule.") + raise webob.exc.HTTPBadRequest(explanation=msg) + access_type = access_data['access_type'] access_to = access_data['access_to'] common.validate_access(access_type=access_type, @@ -507,15 +564,67 @@ class ShareMixin(object): except exception.InvalidMetadataSize as error: raise exc.HTTPBadRequest(explanation=error.msg) + if lock_deletion or lock_visibility: + self._create_access_locks( + context, access, lock_deletion=lock_deletion, + lock_visibility=lock_visibility, lock_reason=lock_reason) + return self._access_view_builder.view(req, access) + def _check_for_access_rule_locks(self, context, access_data, access_id, + share_id): + """Fetches locks for access rules and attempts deleting them.""" + + # ensure the requester is asking to remove the restrictions of the rule + unrestrict = access_data.get('unrestrict', False) + search_opts = { + 'resource_id': access_id, + 'resource_action': constants.RESOURCE_ACTION_DELETE + } + + locks, locks_count = ( + self.resource_locks_api.get_all( + context, search_opts=search_opts, show_count=True) or [] + ) + + # no locks placed, nothing to do + if not locks: + return + + def raise_rule_is_locked(share_id, unrestrict=False): + msg = _( + "Cannot deny access for share '%s' since it has been " + "locked. Please remove the locks and retry the " + "operation") % share_id + if unrestrict: + msg = _( + "Unable to drop access rule restrictions that are not " + "placed by you.") + raise exc.HTTPForbidden(explanation=msg) + + if locks_count and not unrestrict: + raise_rule_is_locked(share_id) + + non_deletable_locks = [] + for lock in locks: + try: + self.resource_locks_api.ensure_context_can_delete_lock( + context, lock['id']) + except exception.NotAuthorized: + non_deletable_locks.append(lock) + + if non_deletable_locks: + raise_rule_is_locked(share_id, unrestrict=unrestrict) + @wsgi.Controller.authorize('deny_access') def _deny_access(self, req, id, body, allow_on_error_state=False): """Remove share access rule.""" context = req.environ['manila.context'] - access_id = body.get( - 'deny_access', body.get('os-deny_access'))['access_id'] + access_data = body.get('deny_access', body.get('os-deny_access')) + access_id = access_data['access_id'] + + self._check_for_access_rule_locks(context, access_data, access_id, id) share = self.share_api.get(context, id) @@ -637,6 +746,7 @@ class ShareController(wsgi.Controller, ShareMixin, wsgi.AdminActionsMixin): def __init__(self): super(ShareController, self).__init__() self.share_api = share.API() + self.resource_locks_api = resource_locks.API() self._access_view_builder = share_access_views.ViewBuilder() @wsgi.action('os-reset_status') diff --git a/manila/api/v2/resource_locks.py b/manila/api/v2/resource_locks.py index ec68f3b8ae..17076d5ae4 100644 --- a/manila/api/v2/resource_locks.py +++ b/manila/api/v2/resource_locks.py @@ -45,13 +45,17 @@ class ResourceLocksController(wsgi.Controller): _view_builder_class = resource_locks_view.ViewBuilder resource_name = 'resource_lock' - def _check_body(self, body, for_update=False): + def _check_body(self, body, lock_to_update=None): if 'resource_lock' not in body: raise exc.HTTPBadRequest( explanation="Malformed request body.") lock_data = body['resource_lock'] + resource_type = ( + lock_to_update['resource_type'] + if lock_to_update + else lock_data.get('resource_type', constants.SHARE_RESOURCE_TYPE) + ) resource_id = lock_data.get('resource_id') or '' - resource_type = lock_data.get('resource_type') or '' resource_action = (lock_data.get('resource_action') or constants.RESOURCE_ACTION_DELETE) lock_reason = lock_data.get('lock_reason') or '' @@ -59,12 +63,20 @@ class ResourceLocksController(wsgi.Controller): if len(lock_reason) > 1023: msg = _("'lock_reason' can contain a maximum of 1023 characters.") raise exc.HTTPBadRequest(explanation=msg) - if resource_action not in constants.RESOURCE_LOCK_RESOURCE_ACTIONS: + if resource_type not in constants.RESOURCE_LOCK_RESOURCE_TYPES: + msg = _("'resource_type' is required and must be one " + "of %(resource_types)s") % { + 'resource_types': constants.RESOURCE_LOCK_RESOURCE_TYPES + } + raise exc.HTTPBadRequest(explanation=msg) + resource_type_lock_actions = ( + constants.RESOURCE_LOCK_ACTIONS_MAPPING[resource_type]) + if resource_action not in resource_type_lock_actions: msg = _("'resource_action' can only be one of %(actions)s" % - {'actions': constants.RESOURCE_LOCK_RESOURCE_ACTIONS}) + {'actions': resource_type_lock_actions}) raise exc.HTTPBadRequest(explanation=msg) - if for_update: + if lock_to_update: if set(lock_data.keys()) - {'resource_action', 'lock_reason'}: msg = _("Only 'resource_action' and 'lock_reason' " "can be updated.") @@ -73,10 +85,6 @@ class ResourceLocksController(wsgi.Controller): if not uuidutils.is_uuid_like(resource_id): msg = _("Resource ID is required and must be in uuid format.") raise exc.HTTPBadRequest(explanation=msg) - if resource_type not in constants.RESOURCE_LOCK_RESOURCE_TYPES: - msg = _("'resource_type' is required and must be one " - "of %s" % constants.RESOURCE_LOCK_RESOURCE_TYPES) - raise exc.HTTPBadRequest(explanation=msg) def __init__(self): self.resource_locks_api = resource_locks.API() @@ -165,6 +173,9 @@ class ResourceLocksController(wsgi.Controller): explanation="No such resource found.") except exception.InvalidInput as error: raise exc.HTTPConflict(explanation=error.msg) + except exception.ResourceVisibilityLockExists: + raise exc.HTTPConflict( + "Resource's visibility is already locked by other user.") return self._view_builder.detail(req, resource_lock) @wsgi.Controller.api_version(RESOURCE_LOCKS_MIN_API_VERSION) @@ -172,14 +183,21 @@ class ResourceLocksController(wsgi.Controller): def update(self, req, id, body): """Update an existing resource lock.""" context = req.environ['manila.context'] - self._check_body(body, for_update=True) - lock_data = body['resource_lock'] + try: + resource_lock = self.resource_locks_api.get(context, id) + except exception.NotFound as e: + raise exc.HTTPNotFound(explanation=e.msg) - resource_lock = self.resource_locks_api.update( - context, - id, - lock_data, - ) + self._check_body(body, lock_to_update=resource_lock) + lock_data = body['resource_lock'] + try: + resource_lock = self.resource_locks_api.update( + context, + resource_lock, + lock_data, + ) + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.msg) return self._view_builder.detail(req, resource_lock) diff --git a/manila/api/v2/share_accesses.py b/manila/api/v2/share_accesses.py index 2735cab068..64c074374b 100644 --- a/manila/api/v2/share_accesses.py +++ b/manila/api/v2/share_accesses.py @@ -19,10 +19,13 @@ import ast import webob +from manila.api import common from manila.api.openstack import wsgi from manila.api.views import share_accesses as share_access_views +from manila.common import constants from manila import exception from manila.i18n import _ +from manila.lock import api as resource_locks from manila import share @@ -35,6 +38,7 @@ class ShareAccessesController(wsgi.Controller, wsgi.AdminActionsMixin): def __init__(self): super(ShareAccessesController, self).__init__() self.share_api = share.API() + self.resource_locks_api = resource_locks.API() @wsgi.Controller.api_version('2.45') @wsgi.Controller.authorize('get') @@ -42,8 +46,25 @@ class ShareAccessesController(wsgi.Controller, wsgi.AdminActionsMixin): """Return data about the given share access rule.""" context = req.environ['manila.context'] share_access = self._get_share_access(context, id) + restricted = self._is_rule_restricted(context, id) + if restricted: + share_access['restricted'] = True return self._view_builder.view(req, share_access) + def _is_rule_restricted(self, context, id): + search_opts = { + 'resource_id': id, + 'resource_action': constants.RESOURCE_ACTION_SHOW, + 'resource_type': 'access_rule' + } + locks, count = self.resource_locks_api.get_all( + context, search_opts, show_count=True) + + if count: + return self.resource_locks_api.access_is_restricted(context, + locks[0]) + return False + def _get_share_access(self, context, share_access_id): try: return self.share_api.access_get(context, share_access_id) @@ -51,9 +72,34 @@ class ShareAccessesController(wsgi.Controller, wsgi.AdminActionsMixin): msg = _("Share access rule %s not found.") % share_access_id raise webob.exc.HTTPNotFound(explanation=msg) - @wsgi.Controller.api_version('2.45') - @wsgi.Controller.authorize - def index(self, req): + def _validate_search_opts(self, req, search_opts): + """Check if search opts parameters are valid.""" + access_type = search_opts.get('access_type', None) + access_to = search_opts.get('access_to', None) + + if access_type and access_type not in ['ip', 'user', 'cert', 'cephx']: + raise exception.InvalidShareAccessType(type=access_type) + + # If access_to is present but access type is not, it gets tricky to + # validate its content + if access_to and not access_type: + msg = _("'access_type' parameter must be provided when specifying " + "'access_to'.") + raise exception.InvalidInput(reason=msg) + + if access_type and access_to: + common.validate_access(access_type=access_type, + access_to=access_to, + enable_ceph=True, + enable_ipv6=True) + + access_level = search_opts.get('access_level') + if ('access_level' in search_opts and ( + search_opts['access_level'] not in constants.ACCESS_LEVELS)): + raise exception.InvalidShareAccessLevel(level=access_level) + + @wsgi.Controller.authorize('index') + def _index(self, req, support_for_access_filters=False): """Returns the list of access rules for a given share.""" context = req.environ['manila.context'] search_opts = {} @@ -66,6 +112,12 @@ class ShareAccessesController(wsgi.Controller, wsgi.AdminActionsMixin): if 'metadata' in search_opts: search_opts['metadata'] = ast.literal_eval( search_opts['metadata']) + if support_for_access_filters: + try: + self._validate_search_opts(req, search_opts) + except (exception.InvalidShareAccessLevel, + exception.InvalidShareAccessType) as e: + raise webob.exc.HTTPBadRequest(explanation=e.msg) try: share = self.share_api.get(context, share_id) except exception.NotFound: @@ -73,8 +125,24 @@ class ShareAccessesController(wsgi.Controller, wsgi.AdminActionsMixin): raise webob.exc.HTTPBadRequest(explanation=msg) access_rules = self.share_api.access_get_all( context, share, search_opts) + rule_list = [] + for rule in access_rules: + restricted = self._is_rule_restricted(context, rule['id']) + rule['restricted'] = restricted + if (('access_to' in search_opts or 'access_key' in search_opts) + and restricted): + continue + rule_list.append(rule) - return self._view_builder.list_view(req, access_rules) + return self._view_builder.list_view(req, rule_list) + + @wsgi.Controller.api_version('2.45', '2.81') + def index(self, req): + return self._index(req) + + @wsgi.Controller.api_version('2.82') + def index(self, req): # pylint: disable=function-redefined # noqa F811 + return self._index(req, support_for_access_filters=True) def create_resource(): diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index 05976d8424..6918c2b643 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -33,6 +33,7 @@ from manila.common import constants from manila import db from manila import exception from manila.i18n import _ +from manila.lock import api as resource_locks from manila import policy from manila import share from manila import utils @@ -53,6 +54,7 @@ class ShareController(wsgi.Controller, def __init__(self): super(ShareController, self).__init__() self.share_api = share.API() + self.resource_locks_api = resource_locks.API() self._access_view_builder = share_access_views.ViewBuilder() self._migration_view_builder = share_migration_views.ViewBuilder() @@ -474,6 +476,13 @@ class ShareController(wsgi.Controller, kwargs['enable_metadata'] = True if req.api_version_request >= api_version.APIVersionRequest("2.74"): kwargs['allow_on_error_state'] = True + if req.api_version_request >= api_version.APIVersionRequest("2.82"): + access_data = body.get('allow_access') + kwargs['lock_visibility'] = access_data.get( + 'lock_visibility', False) + kwargs['lock_deletion'] = access_data.get('lock_deletion', False) + kwargs['lock_reason'] = access_data.get('lock_reason') + return self._allow_access(*args, **kwargs) @wsgi.Controller.api_version('2.0', '2.6') diff --git a/manila/api/views/share_accesses.py b/manila/api/views/share_accesses.py index 0976a294b6..d96a74f0ec 100644 --- a/manila/api/views/share_accesses.py +++ b/manila/api/views/share_accesses.py @@ -34,6 +34,13 @@ class ViewBuilder(common.ViewBuilder): return {'access_list': [self.summary_view(request, access)['access'] for access in accesses]} + def _redact_restricted_fields(self, access, access_dict): + if access.get('restricted', False): + fields_to_redact = ['access_key', 'access_to'] + for field in fields_to_redact: + access_dict[field] = '******' + return access_dict + def summary_view(self, request, access): """Summarized view of a single share access.""" access_dict = { @@ -45,6 +52,7 @@ class ViewBuilder(common.ViewBuilder): } self.update_versioned_resource_dict( request, access_dict, access) + access_dict = self._redact_restricted_fields(access, access_dict) return {'access': access_dict} def view(self, request, access): @@ -59,6 +67,7 @@ class ViewBuilder(common.ViewBuilder): } self.update_versioned_resource_dict( request, access_dict, access) + access_dict = self._redact_restricted_fields(access, access_dict) return {'access': access_dict} def view_metadata(self, request, metadata): diff --git a/manila/common/constants.py b/manila/common/constants.py index 88422a1611..4b1f0a2e2e 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -54,6 +54,7 @@ STATUS_BACKUP_RESTORING_ERROR = 'backup_restoring_error' # Transfer resource type SHARE_RESOURCE_TYPE = 'share' +SHARE_ACCESS_RESOURCE_TYPE = 'access_rule' # Access rule states ACCESS_STATE_QUEUED_TO_APPLY = 'queued_to_apply' @@ -255,13 +256,38 @@ REPLICATION_TYPE_DR = 'dr' POLICY_EXTEND_BEYOND_MAX_SHARE_SIZE = 'extend_beyond_max_share_size_spec' RESOURCE_ACTION_DELETE = 'delete' # delete, soft-delete, unmanage +RESOURCE_ACTION_SHOW = 'show' RESOURCE_LOCK_RESOURCE_TYPES = ( SHARE_RESOURCE_TYPE, + SHARE_ACCESS_RESOURCE_TYPE, ) RESOURCE_LOCK_RESOURCE_ACTIONS = ( RESOURCE_ACTION_DELETE, + RESOURCE_ACTION_SHOW, +) + +RESOURCE_LOCK_ACTIONS_MAPPING = { + "share": [RESOURCE_ACTION_DELETE], + "access_rule": [RESOURCE_ACTION_DELETE, RESOURCE_ACTION_SHOW], +} + +DISALLOWED_STATUS_WHEN_LOCKING_SHARES = ( + STATUS_DELETING, + STATUS_ERROR_DELETING, + STATUS_UNMANAGING, + STATUS_MANAGE_ERROR_UNMANAGING, + STATUS_UNMANAGE_ERROR, + STATUS_UNMANAGED, # not possible, future proofing + STATUS_DELETED, # not possible, future proofing +) + +DISALLOWED_STATUS_WHEN_LOCKING_ACCESS_RULES = ( + ACCESS_STATE_QUEUED_TO_DENY, + ACCESS_STATE_DENYING, + ACCESS_STATE_ERROR, + ACCESS_STATE_DELETED, ) diff --git a/manila/db/api.py b/manila/db/api.py index 1e1f29f801..6a14161101 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -559,6 +559,11 @@ def share_access_get(context, access_id): return IMPL.share_access_get(context, access_id) +def share_access_get_with_context(context, access_id): + """Get share access rule.""" + return IMPL.share_access_get_with_context(context, access_id) + + def share_access_get_all_for_share(context, share_id, filters=None): """Get all access rules for given share.""" return IMPL.share_access_get_all_for_share(context, share_id, diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 54f9e94435..feabd75d14 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -2908,6 +2908,21 @@ def share_access_get(context, access_id, session=None): raise exception.NotFound() +@require_context +def share_access_get_with_context(context, access_id, session=None): + """Get access record.""" + session = session or get_session() + + access = _share_access_get_query( + context, session, + {'id': access_id}).options(joinedload('share')).first() + if access: + access['project_id'] = access['share']['project_id'] + return access + else: + raise exception.NotFound() + + @require_context def share_instance_access_get(context, access_id, instance_id, with_share_access_data=True): @@ -2930,16 +2945,23 @@ def share_access_get_all_for_share(context, share_id, filters=None, session=None): filters = filters or {} session = session or get_session() + share_access_mapping = models.ShareAccessMapping query = (_share_access_get_query( context, session, {'share_id': share_id}).filter( models.ShareAccessMapping.instance_mappings.any())) + legal_filter_keys = ('id', 'access_type', 'access_key', + 'access_to', 'access_level') + if 'metadata' in filters: for k, v in filters['metadata'].items(): query = query.filter( or_(models.ShareAccessMapping. share_access_rules_metadata.any(key=k, value=v))) + query = exact_filter( + query, share_access_mapping, filters, legal_filter_keys) + return query.all() @@ -3057,6 +3079,19 @@ def share_instance_access_delete(context, mapping_id): if not mapping: exception.NotFound() + filters = { + 'resource_id': mapping['access_id'], + 'all_projects': True + } + locks, __ = resource_lock_get_all( + context.elevated(), filters=filters + ) + if locks: + for lock in locks: + resource_lock_delete( + context.elevated(), lock['id'] + ) + mapping.soft_delete(session, update_status=True, status_field_name='state') diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 70cf1cba0c..11870dc9fd 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -577,6 +577,15 @@ class ShareAccessMapping(BASE, ManilaBase): 'ShareInstanceAccessMapping.deleted == "False")' ) ) + share = orm.relationship( + "Share", + primaryjoin=( + 'and_(' + 'ShareAccessMapping.share_id == ' + 'Share.id, ' + 'Share.deleted == "False")' + ) + ) class ShareAccessRulesMetadata(BASE, ManilaBase): diff --git a/manila/exception.py b/manila/exception.py index 1fc97bfe49..24f58ba568 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -213,6 +213,10 @@ class ResourceLockNotFound(NotFound): message = _("Resource lock %(lock_id)s could not be found.") +class ResourceVisibilityLockExists(ManilaException): + message = _("Resource %(resource_id)s is already locked.") + + class Found(ManilaException): message = _("Resource was found.") code = 302 diff --git a/manila/lock/api.py b/manila/lock/api.py index 387c199d2c..f5c08fea9e 100644 --- a/manila/lock/api.py +++ b/manila/lock/api.py @@ -28,6 +28,11 @@ class API(base.Base): resource_get = { "share": "share_get", + "access_rule": "share_access_get_with_context" + } + resource_lock_disallowed_statuses = { + "share": constants.DISALLOWED_STATUS_WHEN_LOCKING_SHARES, + "access_rule": constants.DISALLOWED_STATUS_WHEN_LOCKING_ACCESS_RULES } def _get_lock_context(self, context): @@ -73,6 +78,29 @@ class API(base.Base): "manipulated by user. Please " "contact the administrator.") + def access_is_restricted(self, context, resource_lock): + """Ensure the requester doesn't have visibility restrictions + + Call the check allow lock manipulation method as a first validation. + In case it fails, the requester should not have the access rules + fields entirely visible. In case it passes and the access visibility + is restricted, the users will have visibility of all fields only if + they have originally created the lock. + """ + try: + self._check_allow_lock_manipulation(context, resource_lock) + except exception.NotAuthorized: + return True + + try: + policy.check_policy( + context, 'resource_lock', 'bypass_locked_show_action', + resource_lock) + except exception.NotAuthorized: + return True + + return False + def get(self, context, lock_id): """Return resource lock with the specified id.""" return self.db.resource_lock_get(context, lock_id) @@ -109,22 +137,37 @@ class API(base.Base): return locks, count def create(self, context, resource_id=None, resource_type=None, - resource_action=None, lock_reason=None): + resource_action=None, lock_reason=None, resource=None): """Create a resource lock with the specified information.""" get_res_method = getattr(self.db, self.resource_get[resource_type]) - resource = get_res_method(context, resource_id) + if resource_action == constants.RESOURCE_ACTION_SHOW: + # We can't allow visibility locks to be placed more than once, + # otherwise the resource might become visible to someone else. + visibility_locks, __ = self.db.resource_lock_get_all( + context.elevated(), + filters={'resource_id': resource_id, + 'resource_action': resource_action, + 'all_projects': True}) + if visibility_locks: + raise exception.ResourceVisibilityLockExists( + resource_id=resource_id) + if resource is None: + resource = get_res_method(context, resource_id) policy.check_policy(context, 'resource_lock', 'create', resource) - self._check_resource_state_for_locking(resource_action, resource) + self._check_resource_state_for_locking( + resource_action, resource, resource_type=resource_type) lock_context_data = self._get_lock_context(context) resource_lock = lock_context_data.copy() resource_lock.update({ 'resource_id': resource_id, 'resource_action': resource_action, 'lock_reason': lock_reason, + 'resource_type': resource_type }) return self.db.resource_lock_create(context, resource_lock) - def _check_resource_state_for_locking(self, resource_action, resource): + def _check_resource_state_for_locking(self, resource_action, resource, + resource_type='share'): """Check if resource is in a "disallowed" state for locking. For example, deletion lock on a "deleting" resource would be futile. @@ -133,28 +176,37 @@ class API(base.Base): disallowed_statuses = () if resource_action == 'delete': disallowed_statuses = ( - constants.STATUS_DELETING, - constants.STATUS_ERROR_DELETING, - constants.STATUS_UNMANAGING, - constants.STATUS_MANAGE_ERROR_UNMANAGING, - constants.STATUS_UNMANAGE_ERROR, - constants.STATUS_UNMANAGED, # not possible, future proofing - constants.STATUS_DELETED, # not possible, future proofing - ) + self.resource_lock_disallowed_statuses[resource_type]) if resource_state in disallowed_statuses: msg = "Resource status not suitable for locking" raise exception.InvalidInput(reason=msg) - resource_is_soft_deleted = resource.get('is_soft_deleted', False) - if resource_is_soft_deleted: - msg = "Resource cannot be locked since it has been soft deleted." - raise exception.InvalidInput(reason=msg) + if resource_type == constants.SHARE_RESOURCE_TYPE: + resource_is_soft_deleted = resource.get('is_soft_deleted', False) + if resource_is_soft_deleted: + msg = ( + "Resource cannot be locked since it has been soft deleted." + ) + raise exception.InvalidInput(reason=msg) - def update(self, context, lock_id, updates): + def update(self, context, resource_lock, updates): """Update a resource lock with the specified information.""" - resource_lock = self.db.resource_lock_get(context, lock_id) + lock_id = resource_lock['id'] policy.check_policy(context, 'resource_lock', 'update', resource_lock) self._check_allow_lock_manipulation(context, resource_lock) if 'resource_action' in updates: + # A resource can have only one visibility lock + if (updates['resource_action'] == constants.RESOURCE_ACTION_SHOW + and resource_lock['resource_action'] != + constants.RESOURCE_ACTION_SHOW): + filters = { + "resource_id": resource_lock['resource_id'], + "resource_action": constants.RESOURCE_ACTION_SHOW + } + visibility_locks = self.get_all( + context.elevated(), search_opts=filters) + if visibility_locks: + msg = "The resource already has a visibility lock." + raise exception.InvalidInput(reason=msg) get_res_method = getattr( self.db, self.resource_get[resource_lock['resource_type']], @@ -164,9 +216,13 @@ class API(base.Base): updates['resource_action'], resource) return self.db.resource_lock_update(context, lock_id, updates) - def delete(self, context, lock_id): - """Delete resource lock with the specified id.""" + def ensure_context_can_delete_lock(self, context, lock_id): + """Ensure the requester is able to delete locks.""" resource_lock = self.db.resource_lock_get(context, lock_id) policy.check_policy(context, 'resource_lock', 'delete', resource_lock) self._check_allow_lock_manipulation(context, resource_lock) + + def delete(self, context, lock_id): + """Delete resource lock with the specified id.""" + self.ensure_context_can_delete_lock(context, lock_id) self.db.resource_lock_delete(context, lock_id) diff --git a/manila/policies/resource_lock.py b/manila/policies/resource_lock.py index 9ca23429f0..1e1fed24e8 100644 --- a/manila/policies/resource_lock.py +++ b/manila/policies/resource_lock.py @@ -57,7 +57,16 @@ deprecated_lock_delete = policy.DeprecatedRule( deprecated_reason=DEPRECATED_REASON, deprecated_since='2023.2/Bobcat', ) +deprecated_bypass_locked_show_action = policy.DeprecatedRule( + name=BASE_POLICY_NAME % 'bypass_locked_show_action', + check_str=base.RULE_ADMIN_OR_OWNER_USER, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='2023.2/Bobcat', +) +# We anticipate bypassing is desirable only for resource visibility locks. +# Without a bypass, the lock would have to be set aside each time the lock +# owner wants to view the resource. lock_policies = [ policy.DocumentedRuleDefault( @@ -147,6 +156,24 @@ lock_policies = [ ], deprecated_rule=deprecated_lock_delete, ), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'bypass_locked_show_action', + check_str=base.ADMIN_OR_SERVICE_OR_OWNER_USER, + scope_types=['project'], + description="Bypass a visibility lock placed in a resource.", + operations=[ + { + 'method': 'GET', + 'path': '/share-access-rules/{share_access_id}' + }, + { + 'method': 'GET', + 'path': ('/share-access-rules?share_id={share_id}' + '&key1=value1&key2=value2') + }, + ], + deprecated_rule=deprecated_bypass_locked_show_action, + ), ] diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index b3ee5154e6..40f0350100 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -28,6 +28,7 @@ from manila.common import constants from manila import context from manila import db from manila import exception +from manila.lock import api as resource_locks from manila import policy from manila.share import api as share_api from manila.share import share_types @@ -974,6 +975,12 @@ class ShareActionsTest(test.TestCase): {'access_type': 'cert', 'access_to': 'x'}, {'access_type': 'cert', 'access_to': 'tenant.example.com'}, {'access_type': 'cert', 'access_to': 'x' * 64}, + {'access_type': 'cert', 'access_to': 'x' * 64, + 'lock_visibility': True}, + {'access_type': 'cert', 'access_to': 'x' * 64, 'lock_deletion': True}, + {'access_type': 'cert', 'access_to': 'x' * 64, 'lock_deletion': True}, + {'access_type': 'cert', 'access_to': 'x' * 64, 'lock_deletion': True, + 'lock_visibility': True, 'lock_reason': 'locked_for_testing'}, ) def test_allow_access(self, access): self.mock_object(share_api.API, @@ -982,17 +989,218 @@ class ShareActionsTest(test.TestCase): self.mock_object(self.controller._access_view_builder, 'view', mock.Mock(return_value={'access': {'fake': 'fake'}})) + self.mock_object(self.controller, '_create_access_locks') id = 'fake_share_id' body = {'os-allow_access': access} expected = {'access': {'fake': 'fake'}} req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id) + lock_visibility = access.pop('lock_visibility', None) + lock_deletion = access.pop('lock_deletion', None) + lock_reason = access.pop('lock_reason', None) - res = self.controller._allow_access(req, id, body) + res = self.controller._allow_access( + req, id, body, lock_visibility=lock_visibility, + lock_deletion=lock_deletion, lock_reason=lock_reason + ) self.assertEqual(expected, res) self.mock_policy_check.assert_called_once_with( req.environ['manila.context'], 'share', 'allow_access') + if lock_visibility or lock_deletion: + self.controller._create_access_locks.assert_called_once_with( + req.environ['manila.context'], + expected['access'], + lock_deletion=lock_deletion, + lock_visibility=lock_visibility, + lock_reason=lock_reason + ) + + @ddt.data( + {'lock_visibility': True, 'lock_deletion': True, + 'lock_reason': 'test lock reason'}, + {'lock_visibility': True, 'lock_deletion': False, 'lock_reason': None}, + {'lock_visibility': False, 'lock_deletion': True, 'lock_reason': None}, + ) + @ddt.unpack + def test__create_access_locks(self, lock_visibility, lock_deletion, + lock_reason): + access = { + 'id': 'fake', + 'access_type': 'ip', + 'access_to': '127.0.0.1', + } + self.mock_object(resource_locks.API, 'create') + + id = 'fake_share_id' + req = fakes.HTTPRequest.blank( + '/tenant1/shares/%s/action' % id, version='2.82') + context = req.environ['manila.context'] + access['project_id'] = context.project_id + access['user_id'] = context.user_id + + self.controller._create_access_locks( + req.environ['manila.context'], + access, + lock_deletion=lock_deletion, + lock_visibility=lock_visibility, + lock_reason=lock_reason + ) + + restrict_calls = [] + if lock_deletion: + restrict_calls.append( + mock.call( + context, resource_id=access['id'], + resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_DELETE, + resource=access, + lock_reason=lock_reason + ) + ) + if lock_visibility: + restrict_calls.append( + mock.call( + context, resource_id=access['id'], + resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_SHOW, + resource=access, + lock_reason=lock_reason + ) + ) + resource_locks.API.create.assert_has_calls(restrict_calls) + + def test__create_access_visibility_locks_creation_failed(self): + access = { + 'id': 'fake', + 'access_type': 'ip', + 'access_to': '127.0.0.1', + } + lock_reason = 'locked for testing' + self.mock_object( + resource_locks.API, 'create', + mock.Mock(side_effect=exception.NotAuthorized) + ) + + id = 'fake_share_id' + req = fakes.HTTPRequest.blank( + '/tenant1/shares/%s/action' % id, version='2.82') + context = req.environ['manila.context'] + access['project_id'] = context.project_id + access['user_id'] = context.user_id + + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller._create_access_locks, + req.environ['manila.context'], + access, + lock_deletion=False, + lock_visibility=True, + lock_reason=lock_reason + ) + + resource_locks.API.create.assert_called_once_with( + context, resource_id=access['id'], resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_SHOW, resource=access, + lock_reason=lock_reason) + + def test__create_access_deletion_locks_creation_failed(self): + access = { + 'id': 'fake', + 'access_type': 'ip', + 'access_to': '127.0.0.1', + } + lock_reason = 'locked for testing' + self.mock_object( + resource_locks.API, 'create', + mock.Mock(side_effect=exception.NotAuthorized) + ) + + id = 'fake_share_id' + req = fakes.HTTPRequest.blank( + '/tenant1/shares/%s/action' % id, version='2.82') + context = req.environ['manila.context'] + access['project_id'] = context.project_id + access['user_id'] = context.user_id + + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller._create_access_locks, + req.environ['manila.context'], + access, + lock_deletion=True, + lock_visibility=False, + lock_reason=lock_reason + ) + + resource_locks.API.create.assert_called_once_with( + context, resource_id=access['id'], resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_DELETE, resource=access, + lock_reason=lock_reason) + + @ddt.data( + {'lock_visibility': True, 'lock_deletion': True, + 'lock_reason': 'test lock reason'}, + {'lock_visibility': True, 'lock_deletion': False, 'lock_reason': None}, + {'lock_visibility': False, 'lock_deletion': True, 'lock_reason': None}, + ) + @ddt.unpack + def test_allow_access_visibility_restrictions(self, lock_visibility, + lock_deletion, lock_reason): + access = {'id': 'fake'} + self.mock_object(share_api.API, + 'allow_access', + mock.Mock(return_value=access)) + self.mock_object(self.controller._access_view_builder, 'view', + mock.Mock(return_value={'access': {'fake': 'fake'}})) + self.mock_object(resource_locks.API, 'create') + + id = 'fake_share_id' + body = { + 'allow_access': { + 'access_type': 'ip', + 'access_to': '127.0.0.1', + 'lock_visibility': lock_visibility, + 'lock_deletion': lock_deletion, + 'lock_reason': lock_reason + } + } + expected = {'access': {'fake': 'fake'}} + req = fakes.HTTPRequest.blank( + '/tenant1/shares/%s/action' % id, version='2.82') + context = req.environ['manila.context'] + access['project_id'] = context.project_id + access['user_id'] = context.user_id + + res = self.controller._allow_access( + req, id, body, lock_visibility=lock_visibility, + lock_deletion=lock_deletion, lock_reason=lock_reason) + + self.assertEqual(expected, res) + self.mock_policy_check.assert_called_once_with( + context, 'share', 'allow_access') + restrict_calls = [] + if lock_deletion: + restrict_calls.append( + mock.call( + context, resource_id=access['id'], + resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_DELETE, + resource=access, + lock_reason=lock_reason + ) + ) + if lock_visibility: + restrict_calls.append( + mock.call( + context, resource_id=access['id'], + resource_type='access_rule', + resource_action=constants.RESOURCE_ACTION_SHOW, + resource=access, + lock_reason=lock_reason + ) + ) + resource_locks.API.create.assert_has_calls(restrict_calls) def test_allow_access_with_network_id(self): share_network = db_utils.create_share_network() @@ -1032,15 +1240,19 @@ class ShareActionsTest(test.TestCase): {'access_type': 'cert', 'access_to': ''}, {'access_type': 'cert', 'access_to': ' '}, {'access_type': 'cert', 'access_to': 'x' * 65}, - {'access_type': 'cephx', 'access_to': 'alice'} + {'access_type': 'cephx', 'access_to': 'alice'}, + {'access_type': 'ip', 'access_to': '127.0.0.0/24', + 'lock_reason': 'fake_lock_reason'}, ) def test_allow_access_error(self, access): id = 'fake_share_id' + lock_reason = access.pop('lock_reason', None) body = {'os-allow_access': access} req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id) self.assertRaises(webob.exc.HTTPBadRequest, - self.controller._allow_access, req, id, body) + self.controller._allow_access, req, id, body, + lock_reason=lock_reason) self.mock_policy_check.assert_called_once_with( req.environ['manila.context'], 'share', 'allow_access') @@ -1097,6 +1309,181 @@ class ShareActionsTest(test.TestCase): self.mock_policy_check.assert_called_once_with( req.environ['manila.context'], 'share', 'deny_access') + def test_deny_access_delete_locks(self): + def _stub_deny_access(*args, **kwargs): + pass + + 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_data = {"access_id": 'fake_acces_id'} + body = {"deny_access": body_data} + req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id, + version='2.82') + context = req.environ['manila.context'] + + res = self.controller._deny_access(req, id, body) + + self.assertEqual(202, res.status_int) + self.mock_policy_check.assert_called_once_with( + req.environ['manila.context'], 'share', 'deny_access') + self.controller._check_for_access_rule_locks.assert_called_once_with( + context, body['deny_access'], body_data['access_id'], id + ) + + def test__check_for_access_rule_locks_no_locks(self): + self.mock_object( + resource_locks.API, "get_all", mock.Mock(return_value=([], 0))) + + req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id, + version='2.82') + context = req.environ['manila.context'] + access_id = 'fake_access_id' + share_id = 'fake_share_id' + + 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_locks.API.get_all.assert_called_once_with( + context, search_opts=delete_search_opts, show_count=True + ) + + def test__check_for_access_rules_locks_too_many_locks(self): + locks = [{'id': f'lock_id_{i}'} for i in range(4)] + self.mock_object( + resource_locks.API, "get_all", + mock.Mock(return_value=(locks, len(locks)))) + + req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id, + version='2.82') + context = req.environ['manila.context'] + access_id = 'fake_access_id' + share_id = 'fake_share_id' + + self.assertRaises( + webob.exc.HTTPForbidden, + 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_locks.API.get_all.assert_called_once_with( + context, search_opts=delete_search_opts, show_count=True + ) + + def test__check_for_access_rules_cant_manipulate_lock(self): + locks = [{ + 'id': 'fake_lock_id', + 'resource_action': constants.RESOURCE_ACTION_DELETE + }] + self.mock_object( + resource_locks.API, "get_all", + mock.Mock(return_value=(locks, len(locks)))) + self.mock_object( + resource_locks.API, "ensure_context_can_delete_lock", + mock.Mock(side_effect=exception.NotAuthorized)) + + req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id, + version='2.82') + context = req.environ['manila.context'] + access_id = 'fake_access_id' + share_id = 'fake_share_id' + + self.assertRaises( + webob.exc.HTTPForbidden, + 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_locks.API.get_all.assert_called_once_with( + context, search_opts=delete_search_opts, show_count=True + ) + (resource_locks.API.ensure_context_can_delete_lock + .assert_called_once_with( + context, locks[0]['id'])) + + def test__check_for_access_rules_locks_unauthorized(self): + locks = [{ + 'id': 'fake_lock_id', + 'resource_action': constants.RESOURCE_ACTION_DELETE + }] + self.mock_object( + resource_locks.API, "get_all", + mock.Mock(return_value=(locks, len(locks)))) + self.mock_object( + resource_locks.API, "ensure_context_can_delete_lock", + mock.Mock(side_effect=exception.NotAuthorized)) + self.mock_object( + resource_locks.API, "delete", + mock.Mock(side_effect=exception.NotAuthorized)) + + req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id, + version='2.82') + context = req.environ['manila.context'] + access_id = 'fake_access_id' + share_id = 'fake_share_id' + + self.assertRaises( + webob.exc.HTTPForbidden, + 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_locks.API.get_all.assert_called_once_with( + context, search_opts=delete_search_opts, show_count=True + ) + (resource_locks.API.ensure_context_can_delete_lock + .assert_called_once_with( + context, locks[0]['id'])) + + def test_check_for_access_rules_locks(self): + locks = [{ + 'id': 'fake_lock_id', + 'resource_action': constants.RESOURCE_ACTION_DELETE + }] + self.mock_object( + resource_locks.API, "get_all", + mock.Mock(return_value=(locks, len(locks)))) + self.mock_object( + resource_locks.API, "ensure_context_can_delete_lock") + self.mock_object(resource_locks.API, "delete") + + req = fakes.HTTPRequest.blank('/tenant1/shares/%s/action' % id, + version='2.82') + context = req.environ['manila.context'] + access_id = 'fake_access_id' + share_id = 'fake_share_id' + + 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_locks.API.get_all.assert_called_once_with( + context, search_opts=delete_search_opts, show_count=True) + (resource_locks.API.ensure_context_can_delete_lock + .assert_called_once_with( + context, locks[0]['id'])) + @ddt.data('_allow_access', '_deny_access') def test_allow_access_deny_access_policy_not_authorized(self, method): req = fakes.HTTPRequest.blank('/tenant1/shares/someuuid/action') diff --git a/manila/tests/api/v2/test_resource_locks.py b/manila/tests/api/v2/test_resource_locks.py index d3690eda82..0194800350 100644 --- a/manila/tests/api/v2/test_resource_locks.py +++ b/manila/tests/api/v2/test_resource_locks.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy from unittest import mock import ddt @@ -46,37 +47,60 @@ class ResourceLockApiTest(test.TestCase): ) @ddt.data( - test_utils.annotated('no_body_content', {}), - test_utils.annotated('invalid_body', {'share': 'somedata'}), + test_utils.annotated( + 'no_body_content', { + 'body': {}, + 'resource_type': 'share' + } + ), + test_utils.annotated( + 'invalid_body', { + 'body': { + 'share': 'somedata' + }, + 'resource_type': 'share' + } + ), test_utils.annotated( 'invalid_action', { - 'resource_lock': { - 'resource_action': 'invalid_action', + 'body': { + 'resource_lock': { + 'resource_action': 'invalid_action', + } }, + 'resource_type': 'share' }, ), test_utils.annotated( 'invalid_reason', { - 'resource_lock': { - 'lock_reason': 'xyzzyspoon!' * 94, + 'body': { + 'resource_lock': { + 'lock_reason': 'xyzzyspoon!' * 94, + } }, + 'resource_type': 'share' }, ), test_utils.annotated( 'disallowed_attributes', { - 'resource_lock': { - 'lock_reason': 'the reason is you', - 'resource_action': 'delete', - 'resource_id': uuidutils.generate_uuid(), + 'body': { + 'resource_lock': { + 'lock_reason': 'the reason is you', + 'resource_action': 'delete', + 'resource_id': uuidutils.generate_uuid(), + } }, + 'resource_type': 'share' }, ), ) - def test__check_body_for_update_invalid(self, body): + @ddt.unpack + def test__check_body_for_update_invalid(self, body, resource_type): + current_lock = {'resource_type': resource_type} self.assertRaises(webob.exc.HTTPBadRequest, self.controller._check_body, body, - for_update=True) + lock_to_update=current_lock) @ddt.data( test_utils.annotated('no_body_content', {}), @@ -111,14 +135,6 @@ class ResourceLockApiTest(test.TestCase): }, }, ), - test_utils.annotated( - 'empty_resource_type', { - 'resource_lock': { - 'resource_id': uuidutils.generate_uuid(), - 'resource_type': '', - }, - }, - ), ) def test__check_body_for_create_invalid(self, body): self.assertRaises(webob.exc.HTTPBadRequest, @@ -128,29 +144,43 @@ class ResourceLockApiTest(test.TestCase): @ddt.data( test_utils.annotated( 'action_and_lock_reason', { - 'resource_lock': { - 'resource_action': 'delete', - 'lock_reason': 'the reason is you', + 'body': { + 'resource_lock': { + 'resource_action': 'delete', + 'lock_reason': 'the reason is you', + } }, + 'resource_type': 'share', }, ), test_utils.annotated( 'lock_reason', { - 'resource_lock': { - 'lock_reason': 'tienes razon', + 'body': { + 'resource_lock': { + 'lock_reason': 'tienes razon', + } }, + 'resource_type': 'share', }, ), test_utils.annotated( 'resource_action', { - 'resource_lock': { - 'resource_action': 'delete', + 'body': { + 'resource_lock': { + 'resource_action': 'delete', + } }, + 'resource_type': 'access_rule', }, ), ) - def test__check_body_for_update(self, body): - result = self.controller._check_body(body, for_update=True) + @ddt.unpack + def test__check_body_for_update(self, body, resource_type): + current_lock = copy.copy(body['resource_lock']) + current_lock['resource_type'] = resource_type + + result = self.controller._check_body( + body, lock_to_update=current_lock) self.assertIsNone(result) @@ -315,6 +345,27 @@ class ResourceLockApiTest(test.TestCase): self.req, body) + def test_create_visibility_already_locked(self): + self.mock_object(self.controller, '_check_body') + resource_id = '27e14086-16e1-445b-ad32-b2ebb07225a8' + body = { + 'resource_lock': { + 'resource_id': resource_id, + 'resource_type': 'share', + }, + } + self.mock_object( + self.controller.resource_locks_api, + 'create', + mock.Mock( + side_effect=exception.ResourceVisibilityLockExists( + resource_id=resource_id)) + ) + self.assertRaises(webob.exc.HTTPConflict, + self.controller.create, + self.req, + body) + def test_create(self): self.mock_object(self.controller, '_check_body') expected_lock = stubs.stub_lock( @@ -344,11 +395,13 @@ class ResourceLockApiTest(test.TestCase): self.assertIn('links', actual_lock) def test_update(self): - self.mock_object(self.controller, '_check_body') expected_lock = stubs.stub_lock( '04512dae-18c2-45b5-bbab-50b775ba6f1d', lock_reason=None, ) + self.mock_object(self.controller, '_check_body') + self.mock_object(self.controller.resource_locks_api, 'get', + mock.Mock(return_value=expected_lock)) self.mock_object(self.controller.resource_locks_api, 'update', mock.Mock(return_value=expected_lock)) @@ -367,7 +420,7 @@ class ResourceLockApiTest(test.TestCase): self.controller.resource_locks_api.update.assert_called_once_with( utils.IsAMatcher(context.RequestContext), - '04512dae-18c2-45b5-bbab-50b775ba6f1d', + expected_lock, {'lock_reason': None} ) self.assertSubDictMatch(expected_lock, actual_lock) diff --git a/manila/tests/api/v2/test_share_accesses.py b/manila/tests/api/v2/test_share_accesses.py index ab0586a9ee..c216145b1c 100644 --- a/manila/tests/api/v2/test_share_accesses.py +++ b/manila/tests/api/v2/test_share_accesses.py @@ -19,6 +19,7 @@ import ddt from webob import exc from manila.api.v2 import share_accesses +from manila.common import constants from manila import exception from manila import policy from manila import test @@ -102,6 +103,76 @@ class ShareAccessesAPITest(test.TestCase): for key in summary_keys: self.assertEqual(index_access[key], show_el[key]) + @ddt.data(True, False) + def test_list_accesses_restricted(self, restricted): + req = self._get_index_request(version='2.82') + rule_list = [{ + 'access_to': '0.0.0.0/0', + 'id': 'fakeid', + 'access_key': 'fake_key' + }] + self.mock_object( + self.controller.share_api, 'access_get_all', + mock.Mock(return_value=rule_list)) + self.mock_object( + self.controller, '_is_rule_restricted', + mock.Mock(return_value=restricted)) + + index_result = self.controller.index(req) + + self.assertIn('access_list', index_result) + self.controller._is_rule_restricted.assert_called_once_with( + req.environ['manila.context'], rule_list[0]['id']) + if restricted: + for access in index_result['access_list']: + self.assertEqual('******', access['access_key']) + self.assertEqual('******', access['access_to']) + + @ddt.data(True, False) + def test_show_restricted(self, restricted): + req = self._get_show_request( + version='2.82', use_admin_context=False) + self.mock_object( + self.controller, '_is_rule_restricted', + mock.Mock(return_value=restricted)) + + show_result = self.controller.show(req, self.access['id']) + + expected_access_to = ( + '******' if restricted else self.access['access_to']) + + self.assertEqual( + expected_access_to, show_result['access']['access_to']) + + @ddt.data(True, False) + def test__is_rule_restricted(self, is_rule_restricted): + req = self._get_show_request( + version='2.82', use_admin_context=False) + context = req.environ['manila.context'] + fake_lock = { + 'lock_context': 'user', + 'user_id': 'fake', + 'project_id': 'fake', + 'resource_id': 'fake', + 'resource_action': constants.RESOURCE_ACTION_DELETE, + 'lock_reason': 'fake reason', + } + lock = fake_lock if is_rule_restricted else {} + locks = [lock] + + self.mock_object( + self.controller.resource_locks_api, 'get_all', + mock.Mock(return_value=(locks, len(locks)))) + self.mock_object( + self.controller.resource_locks_api, 'access_is_restricted', + mock.Mock(return_value=is_rule_restricted)) + + result_rule_restricted = self.controller._is_rule_restricted( + context, self.access['id']) + + self.assertEqual( + is_rule_restricted, result_rule_restricted) + def test_list_accesses_share_not_found(self): self.assertRaises( exc.HTTPBadRequest, @@ -145,10 +216,12 @@ class ShareAccessesAPITest(test.TestCase): self.assertIs(False, share_being_checked['is_public']) def test_show_access_not_found(self): + req = self._get_show_request('inexistent_id') + print(req.environ) self.assertRaises( exc.HTTPNotFound, self.controller.show, - self._get_show_request('inexistent_id'), 'inexistent_id') + req, 'inexistent_id') @ddt.data('1.0', '2.0', '2.8', '2.44') def test_list_with_unsupported_version(self, version): diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 2f15730829..495358da46 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -330,6 +330,23 @@ class ShareAccessDatabaseAPITestCase(test.TestCase): metadata = {} self.assertEqual(new_metadata, metadata) + def test_share_access_get_with_context(self): + ctxt = context.RequestContext('demo', 'fake', False) + share = db_utils.create_share(project_id=ctxt.project_id) + rules = [db_utils.create_access(share_id=share['id'])] + + result = db_api.share_access_get_with_context(ctxt, rules[0]['id']) + + self.assertEqual(result['project_id'], ctxt.project_id) + + def test_share_access_get_with_context_not_found(self): + + self.assertRaises( + exception.NotFound, + db_api.share_access_get_with_context, + self.ctxt, + 'fake_rule_id') + @ddt.ddt class ShareDatabaseAPITestCase(test.TestCase): diff --git a/manila/tests/lock/test_api.py b/manila/tests/lock/test_api.py index 5cff6a0914..e3a6c09131 100644 --- a/manila/tests/lock/test_api.py +++ b/manila/tests/lock/test_api.py @@ -124,6 +124,71 @@ class ResourceLockApiTest(test.TestCase): ) self.assertIsNone(result) + @ddt.data( + test_utils.annotated( + 'service_manipulating_user_lock', + (context.RequestContext( + 'fake', 'fake', is_admin=False, + service_roles=['service']), + 'user', + 'user_b'), + ), + test_utils.annotated( + 'admin_manipulating_user_lock', + (context.RequestContext('fake', 'fake', is_admin=True), + 'admin', + 'user_a'), + ), + test_utils.annotated( + 'user_manipulating_locks_they_own', + (context.RequestContext('user_a', 'fake', is_admin=False), + 'user', + 'user_a'), + ), + test_utils.annotated( + 'user_manipulating_other_users_lock', + (context.RequestContext('user_a', 'fake', is_admin=False), + 'user', + 'user_b'), + ), + ) + @ddt.unpack + def test_access_is_restricted(self, ctxt, lock_ctxt, lock_user): + resource_lock = { + 'user_id': lock_user, + 'lock_context': lock_ctxt + } + is_restricted = ( + (not ctxt.is_admin and not ctxt.is_service) + and lock_user != ctxt.user_id) + expected_mock_policy = {} + if is_restricted: + expected_mock_policy['side_effect'] = exception.NotAuthorized + self.mock_object(self.lock_api, '_check_allow_lock_manipulation') + self.mock_object(policy, 'check_policy', + mock.Mock(**expected_mock_policy)) + + result = self.lock_api.access_is_restricted( + ctxt, + resource_lock + ) + self.assertEqual(is_restricted, result) + + def test_access_is_restricted_not_authorized(self): + resource_lock = { + 'user_id': 'fakeuserid', + 'lock_context': 'user' + } + ctxt = context.RequestContext('fake', 'fake') + self.mock_object(self.lock_api, '_check_allow_lock_manipulation', + mock.Mock(side_effect=exception.NotAuthorized())) + + result = self.lock_api.access_is_restricted( + ctxt, + resource_lock + ) + self.assertTrue(result) + def test_get_all_all_projects_ignored(self): self.mock_object(policy, 'check_policy', mock.Mock(return_value=False)) self.mock_object(self.lock_api.db, 'resource_lock_get_all', @@ -257,15 +322,84 @@ class ResourceLockApiTest(test.TestCase): 'project_id': 'fakeproject', 'lock_context': 'user', 'lock_reason': None, + 'resource_type': constants.SHARE_RESOURCE_TYPE } self.assertEqual(expected_create_arg, db_create_arg) + def test_create_access_show_lock(self): + self.mock_object(self.lock_api.db, 'resource_lock_create', + mock.Mock(return_value='created_obj')) + mock_access = { + 'id': 'cacac01c-853d-47f3-afcb-da4484bd09a5', + 'state': constants.STATUS_ACTIVE, + } + self.mock_object(self.lock_api.db, 'access_get', + mock.Mock(return_value=mock_access)) + self.mock_object(self.lock_api.db, 'resource_lock_get_all', + mock.Mock(return_value=['', 0])) + self.mock_object(self.ctxt, 'elevated', + mock.Mock(return_value=self.ctxt)) + + result = self.lock_api.create( + self.ctxt, + resource_id='cacac01c-853d-47f3-afcb-da4484bd09a5', + resource_action=constants.RESOURCE_ACTION_SHOW, + resource_type=constants.SHARE_ACCESS_RESOURCE_TYPE, + ) + + self.assertEqual('created_obj', result) + db_create_arg = self.lock_api.db.resource_lock_create.call_args[0][1] + resource_id = 'cacac01c-853d-47f3-afcb-da4484bd09a5' + expected_create_arg = { + 'resource_id': resource_id, + 'resource_action': constants.RESOURCE_ACTION_SHOW, + 'user_id': 'fakeuser', + 'project_id': 'fakeproject', + 'lock_context': 'user', + 'lock_reason': None, + 'resource_type': constants.SHARE_ACCESS_RESOURCE_TYPE + + } + self.assertEqual(expected_create_arg, db_create_arg) + filters = { + 'resource_id': resource_id, + 'resource_action': constants.RESOURCE_ACTION_SHOW, + 'all_projects': True + } + self.lock_api.db.resource_lock_get_all.assert_called_once_with( + self.ctxt, filters=filters) + + def test_create_visibility_lock_lock_exists(self): + self.mock_object(self.lock_api.db, 'resource_lock_create', + mock.Mock(return_value='created_obj')) + self.mock_object(self.lock_api.db, 'resource_lock_get_all', + mock.Mock(return_value=['visibility_lock', 1])) + self.mock_object(self.ctxt, 'elevated', + mock.Mock(return_value=self.ctxt)) + + self.assertRaises( + exception.ResourceVisibilityLockExists, + self.lock_api.create, + self.ctxt, + resource_id='cacac01c-853d-47f3-afcb-da4484bd09a5', + resource_action=constants.RESOURCE_ACTION_SHOW, + resource_type=constants.SHARE_ACCESS_RESOURCE_TYPE, + ) + + resource_id = 'cacac01c-853d-47f3-afcb-da4484bd09a5' + filters = { + 'resource_id': resource_id, + 'resource_action': constants.RESOURCE_ACTION_SHOW, + 'all_projects': True + } + self.lock_api.db.resource_lock_get_all.assert_called_once_with( + self.ctxt, filters=filters) + @ddt.data(True, False) def test_update_lock_resource_not_allowed_with_policy_failure( self, policy_fails): - self.mock_object(self.lock_api.db, 'resource_lock_get', mock.Mock( - return_value={'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'})) + lock = {'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'} if policy_fails: self.mock_object( policy, @@ -286,7 +420,7 @@ class ResourceLockApiTest(test.TestCase): self.assertRaises(exception.NotAuthorized, self.lock_api.update, self.ctxt, - 'd767d3cd-1187-404a-a91f-8b172e0e768e', + lock, {'foo': 'bar'}) @ddt.data(constants.STATUS_DELETING, @@ -303,8 +437,6 @@ class ResourceLockApiTest(test.TestCase): 'resource_action': 'something', 'resource_type': 'share', } - self.mock_object(self.lock_api.db, 'resource_lock_get', - mock.Mock(return_value=lock)) self.mock_object(self.lock_api, '_check_allow_lock_manipulation') self.mock_object(self.lock_api.db, 'share_get', @@ -313,21 +445,20 @@ class ResourceLockApiTest(test.TestCase): self.assertRaises(exception.InvalidInput, self.lock_api.update, self.ctxt, - 'd767d3cd-1187-404a-a91f-8b172e0e768e', + lock, {'resource_action': 'delete'}) self.lock_api.db.resource_lock_update.assert_not_called() def test_update(self): - self.mock_object(self.lock_api.db, 'resource_lock_get', mock.Mock( - return_value={'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'})) self.mock_object(self.lock_api, '_check_allow_lock_manipulation') self.mock_object(self.lock_api.db, 'resource_lock_update', mock.Mock(return_value='updated_obj')) + lock = {'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'} result = self.lock_api.update( self.ctxt, - 'd767d3cd-1187-404a-a91f-8b172e0e768e', + lock, {'foo': 'bar'}, ) diff --git a/releasenotes/notes/add-access-visibility-and-delete-locks-52a7ef235813d147.yaml b/releasenotes/notes/add-access-visibility-and-delete-locks-52a7ef235813d147.yaml new file mode 100644 index 0000000000..813ae5e614 --- /dev/null +++ b/releasenotes/notes/add-access-visibility-and-delete-locks-52a7ef235813d147.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + Added the possibility to lock the deletion of access rules, as well as the + visibility of the sensitive fields `access_to` and `access_type` while + creating share access rules. When the visibility is restricted, only the + owner or more privileged users will be able to visualize the context of + the sensitive fields. + Both locks can also be imposed by the recently introduced resource locks + APIs. + - | + It is now possible to filter access rules based on the `access_to`, + `access_type`, `access_key` and `access_level` keys.