openstack/manila
Code Issues Proposed changes

Merge "Allow restricting access rules fields and deletion"

Browse Source
This commit is contained in:
Zuul 2023-08-28 02:40:24 +00:00 committed by Gerrit Code Review
commit 44014e6827
27 changed files with 1262 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
api-ref/source/parameters.yaml
View File

@ -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``.
@ -3927,6 +3943,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

5
api-ref/source/samples/share-actions-grant-access-request.json
View File

@ -6,6 +6,9 @@
"metadata":{
"key1": "value1",
"key2": "value2"
}
},
"lock_visibility": false,
"lock_deletion": true,
"lock_reason": "Locked for deletion until year end audit."
}
}

3
api-ref/source/samples/share-actions-revoke-access-request.json
View File

@ -1,5 +1,6 @@
{
"deny_access": {
"access_id": "a25b2df3-90bd-4add-afa6-5f0dbbd50452"
"access_id": "a25b2df3-90bd-4add-afa6-5f0dbbd50452",
"unrestrict": true
}
}

12
api-ref/source/share-access-rules.inc
View File

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

15
api-ref/source/share-actions.inc
View File

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

18
doc/source/admin/shared-file-systems-crud-share.rst
View File

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

60
doc/source/user/create-and-manage-shares.rst
View File

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

3
manila/api/openstack/api_version_request.py
View File

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

5
manila/api/openstack/rest_api_version_history.rst
View File

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

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

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

42
manila/api/v2/resource_locks.py
View File

@ -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)
self._check_body(body, lock_to_update=resource_lock)
lock_data = body['resource_lock']
try:
resource_lock = self.resource_locks_api.update(
context,
id,
resource_lock,
lock_data,
)
except exception.InvalidInput as e:
raise exc.HTTPBadRequest(explanation=e.msg)
return self._view_builder.detail(req, resource_lock)

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

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

9
manila/api/v2/shares.py
View File

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

9
manila/api/views/share_accesses.py
View File

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

26
manila/common/constants.py
View File

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

5
manila/db/api.py
View File

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

35
manila/db/sqlalchemy/api.py
View File

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

9
manila/db/sqlalchemy/models.py
View File

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

4
manila/exception.py
View File

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

88
manila/lock/api.py
View File

@ -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])
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)
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."
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)

27
manila/policies/resource_lock.py
View File

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

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

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

85
manila/tests/api/v2/test_resource_locks.py
View File

@ -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', {
'body': {
'resource_lock': {
'resource_action': 'invalid_action',
}
},
'resource_type': 'share'
},
),
test_utils.annotated(
'invalid_reason', {
'body': {
'resource_lock': {
'lock_reason': 'xyzzyspoon!' * 94,
}
},
'resource_type': 'share'
},
),
test_utils.annotated(
'disallowed_attributes', {
'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', {
'body': {
'resource_lock': {
'resource_action': 'delete',
'lock_reason': 'the reason is you',
}
},
'resource_type': 'share',
},
),
test_utils.annotated(
'lock_reason', {
'body': {
'resource_lock': {
'lock_reason': 'tienes razon',
}
},
'resource_type': 'share',
},
),
test_utils.annotated(
'resource_action', {
'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)

75
manila/tests/api/v2/test_share_accesses.py
View File

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

17
manila/tests/db/sqlalchemy/test_api.py
View File

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

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

@ -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'},
)

13
releasenotes/notes/add-access-visibility-and-delete-locks-52a7ef235813d147.yaml Normal file
View File

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