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

@@ -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

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."
     }
 }

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
     }
 }

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
   <get-access-rules-before-2-45>` 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
 --------------
 

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

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 <https://docs.openstack.org/manila/latest/admin
    /share_back_ends_feature_support_mapping.html>`_.
 
+.. 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.

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``:
 

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
 
 

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.

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(
+        access_data = body.get('deny_access', body.get('os-deny_access'))
-            'deny_access', body.get('os-deny_access'))['access_id']
+        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')

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)
+        try:
-        lock_data = body['resource_lock']
+            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(
+        self._check_body(body, lock_to_update=resource_lock)
-            context,
+        lock_data = body['resource_lock']
-            id,
+        try:
-            lock_data,
+            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)
 
 

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')
+    def _validate_search_opts(self, req, search_opts):
-    @wsgi.Controller.authorize
+        """Check if search opts parameters are valid."""
-    def index(self, req):
+        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():

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')

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):

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,
 )
 
 

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,

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')
 

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):

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

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,
+                self.resource_lock_disallowed_statuses[resource_type])
-                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
-            )
         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_type == constants.SHARE_RESOURCE_TYPE:
-        if resource_is_soft_deleted:
+            resource_is_soft_deleted = resource.get('is_soft_deleted', False)
-            msg = "Resource cannot be locked since it has been soft deleted."
+            if resource_is_soft_deleted:
-            raise exception.InvalidInput(reason=msg)
+                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):
+    def ensure_context_can_delete_lock(self, context, lock_id):
-        """Delete resource lock with the specified 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)

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,
+    ),
 ]
 
 

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')

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(
-        test_utils.annotated('invalid_body', {'share': 'somedata'}),
+            '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': {
+                'body': {
-                    'resource_action': 'invalid_action',
+                    'resource_lock': {
+                        'resource_action': 'invalid_action',
+                    }
                 },
+                'resource_type': 'share'
             },
         ),
         test_utils.annotated(
             'invalid_reason', {
-                'resource_lock': {
+                'body': {
-                    'lock_reason': 'xyzzyspoon!' * 94,
+                    'resource_lock': {
+                        'lock_reason': 'xyzzyspoon!' * 94,
+                    }
                 },
+                'resource_type': 'share'
             },
         ),
         test_utils.annotated(
             'disallowed_attributes', {
-                'resource_lock': {
+                'body': {
-                    'lock_reason': 'the reason is you',
+                    'resource_lock': {
-                    'resource_action': 'delete',
+                        'lock_reason': 'the reason is you',
-                    'resource_id': uuidutils.generate_uuid(),
+                        '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': {
+                'body': {
-                    'resource_action': 'delete',
+                    'resource_lock': {
-                    'lock_reason': 'the reason is you',
+                        'resource_action': 'delete',
+                        'lock_reason': 'the reason is you',
+                    }
                 },
+                'resource_type': 'share',
             },
         ),
         test_utils.annotated(
             'lock_reason', {
-                'resource_lock': {
+                'body': {
-                    'lock_reason': 'tienes razon',
+                    'resource_lock': {
+                        'lock_reason': 'tienes razon',
+                    }
                 },
+                'resource_type': 'share',
             },
         ),
         test_utils.annotated(
             'resource_action', {
-                'resource_lock': {
+                'body': {
-                    'resource_action': 'delete',
+                    'resource_lock': {
+                        'resource_action': 'delete',
+                    }
                 },
+                'resource_type': 'access_rule',
             },
         ),
     )
-    def test__check_body_for_update(self, body):
+    @ddt.unpack
-        result = self.controller._check_body(body, for_update=True)
+    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)

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):

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):

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(
+        lock = {'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'}
-            return_value={'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'},
         )
 

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.