From be0c8561c9788e4a2b10c4abd3e30d62d84b5156 Mon Sep 17 00:00:00 2001
From: Kiran Pawar <kinpaa@gmail.com>
Date: Mon, 27 May 2024 14:52:31 +0000
Subject: [PATCH] Fix share/snapshot show for deferred deletion

Deferred deletion states deferred_deleting, error_deferred_deleting are
visible for non-admin user in 'share/snapshot show' command. Fixed this.

partially-implements: bp/deferred-deletion
Closes-bug: #2067456
Change-Id: I42ddda3144a3f52b4bc0420d5acde1e5e7560264
---
 manila/policies/share_snapshot.py             |  6 ++-
 manila/policies/shares.py                     |  6 ++-
 manila/share/api.py                           | 28 +++++++++--
 manila/tests/share/test_api.py                | 49 +++++++++++++++++++
 ...or-deferred-deletion-37654e034eabccc6.yaml |  6 +++
 5 files changed, 88 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/bug-2067456-handle-share-and-snapshot-show-for-deferred-deletion-37654e034eabccc6.yaml

diff --git a/manila/policies/share_snapshot.py b/manila/policies/share_snapshot.py
index f60dfbb90e..d074e489ab 100644
--- a/manila/policies/share_snapshot.py
+++ b/manila/policies/share_snapshot.py
@@ -278,11 +278,15 @@ share_snapshot_policies = [
         name=BASE_POLICY_NAME % 'list_snapshots_in_deferred_deletion_states',
         check_str=base.ADMIN,
         scope_types=['project'],
-        description="List share snapshots whose deletion has been deferred",
+        description="List (or get) snapshots whose deletion has been deferred",
         operations=[
             {
                 'method': 'GET',
                 'path': '/v2/snapshots',
+            },
+            {
+                'method': 'GET',
+                'path': '/snapshots/{snapshot_id}'
             }
         ],
         deprecated_rule=deprecated_list_snapshots_in_deferred_deletion_states
diff --git a/manila/policies/shares.py b/manila/policies/shares.py
index c0895988c3..90bff62399 100644
--- a/manila/policies/shares.py
+++ b/manila/policies/shares.py
@@ -669,12 +669,16 @@ shares_policies = [
         name=BASE_POLICY_NAME % 'list_shares_in_deferred_deletion_states',
         check_str=base.ADMIN,
         scope_types=['project'],
-        description="List shares whose deletion has been deferred",
+        description="List (or get) shares whose deletion has been deferred",
         operations=[
             {
                 'method': 'GET',
                 'path': '/v2/shares',
             },
+            {
+                'method': 'GET',
+                'path': '/shares/{share_id}',
+            }
         ],
         deprecated_rule=deprecated_list_shares_in_deferred_deletion_states
     ),
diff --git a/manila/share/api.py b/manila/share/api.py
index 19df4f6257..f76858a33f 100644
--- a/manila/share/api.py
+++ b/manila/share/api.py
@@ -2208,13 +2208,21 @@ class API(base.Base):
         return self.db.share_snapshot_update(context, snapshot['id'], fields)
 
     def get(self, context, share_id):
-        rv = self.db.share_get(context, share_id)
-        if not rv['is_public']:
+        share = self.db.share_get(context, share_id)
+        if not share['is_public']:
             authorized = policy.check_policy(
-                context, 'share', 'get', rv, do_raise=False)
+                context, 'share', 'get', share, do_raise=False)
             if not authorized:
                 raise exception.NotFound()
-        return rv
+        if share['status'] in (
+            constants.STATUS_DEFERRED_DELETING,
+                constants.STATUS_ERROR_DEFERRED_DELETING):
+            policy_str = "list_shares_in_deferred_deletion_states"
+            authorized = policy.check_policy(
+                context, 'share', policy_str, share, do_raise=False)
+            if not authorized:
+                raise exception.NotFound()
+        return share
 
     def get_all(self, context, search_opts=None, sort_key='created_at',
                 sort_dir='desc'):
@@ -2324,7 +2332,17 @@ class API(base.Base):
 
     def get_snapshot(self, context, snapshot_id):
         policy.check_policy(context, 'share_snapshot', 'get_snapshot')
-        return self.db.share_snapshot_get(context, snapshot_id)
+        snapshot = self.db.share_snapshot_get(context, snapshot_id)
+        if snapshot.get('status') in (
+            constants.STATUS_DEFERRED_DELETING,
+                constants.STATUS_ERROR_DEFERRED_DELETING):
+            policy_str = "list_snapshots_in_deferred_deletion_states"
+            authorized = policy.check_policy(
+                context, 'share_snapshot', policy_str,
+                snapshot, do_raise=False)
+            if not authorized:
+                raise exception.NotFound()
+        return snapshot
 
     def get_all_snapshots(self, context, search_opts=None, limit=None,
                           offset=None, sort_key='share_id', sort_dir='desc'):
diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py
index 3707d3ef33..1df56ec1e8 100644
--- a/manila/tests/share/test_api.py
+++ b/manila/tests/share/test_api.py
@@ -2675,6 +2675,17 @@ class ShareAPITestCase(test.TestCase):
             db_api.share_snapshot_get.assert_called_once_with(
                 self.context, 'fakeid')
 
+    def test_get_snapshot_non_admin_deferred_state(self):
+        fake_get_snap = {
+            'fake_key': 'fake_val', 'status': 'deferred_deleting'
+        }
+        with mock.patch.object(db_api, 'share_snapshot_get',
+                               mock.Mock(return_value=fake_get_snap)):
+            self.mock_object(
+                policy, 'check_policy', mock.Mock(side_effect=[True, False]))
+            self.assertRaises(exception.NotFound, self.api.get_snapshot,
+                              self.context, 'fakeid')
+
     def test_create_from_snapshot_not_available(self):
         snapshot = db_utils.create_snapshot(
             with_share=True, status=constants.STATUS_ERROR)
@@ -2899,6 +2910,44 @@ class ShareAPITestCase(test.TestCase):
             db_api.share_get.assert_called_once_with(
                 self.context, 'fakeid')
 
+    def test_get_admin_deferred_state(self):
+        rv = {
+            'id': 'fake_id',
+            'is_public': False,
+            'name': 'bar',
+            'status': constants.STATUS_ERROR_DEFERRED_DELETING,
+            'project_id': 'fake_pid_2',
+            'share_server_id': 'fake_server_3',
+        }
+
+        self.mock_object(db_api, 'share_get',
+                         mock.Mock(return_value=rv))
+        ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True)
+        self.mock_object(
+            policy, 'check_policy', mock.Mock(side_effect=[True, True]))
+        share = self.api.get(ctx, 'fake_id')
+        self.assertEqual(rv, share)
+
+    def test_get_non_admin_deferred_state(self):
+        rv = {
+            'id': 'fake_id',
+            'is_public': False,
+            'name': 'bar',
+            'status': constants.STATUS_ERROR_DEFERRED_DELETING,
+            'project_id': 'fake_pid_2',
+            'share_server_id': 'fake_server_3',
+        }
+
+        self.mock_object(db_api, 'share_get',
+                         mock.Mock(return_value=rv))
+        ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=False)
+        self.mock_object(
+            policy, 'check_policy', mock.Mock(side_effect=[True, False]))
+
+        self.assertRaises(
+            exception.NotFound,
+            self.api.get, ctx, 'fake_id')
+
     def test_get_not_authorized(self):
         share = db_utils.create_share(
             is_public=False,
diff --git a/releasenotes/notes/bug-2067456-handle-share-and-snapshot-show-for-deferred-deletion-37654e034eabccc6.yaml b/releasenotes/notes/bug-2067456-handle-share-and-snapshot-show-for-deferred-deletion-37654e034eabccc6.yaml
new file mode 100644
index 0000000000..f880917ba1
--- /dev/null
+++ b/releasenotes/notes/bug-2067456-handle-share-and-snapshot-show-for-deferred-deletion-37654e034eabccc6.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Share and snapshot in deferred deletion states will be hidden from non-admin
+    user in show API along-with existing list API. For more details, please check
+    Launchpad `bug 2067456 <https://bugs.launchpad.net/manila/+bug/2067456>`_