Merge "Update get_snapshot to return 404 if snapshot is inaccessible" into stable/2025.1

manila/share/api.py

@@ -2426,8 +2426,12 @@ class API(base.Base):
         return result
 
     def get_snapshot(self, context, snapshot_id):
-        policy.check_policy(context, 'share_snapshot', 'get_snapshot')
         snapshot = self.db.share_snapshot_get(context, snapshot_id)
+        authorized = policy.check_policy(context, 'share_snapshot',
+                                         'get_snapshot', snapshot,
+                                         do_raise=False)
+        if not authorized:
+            raise exception.NotFound()
         if snapshot.get('status') in (
             constants.STATUS_DEFERRED_DELETING,
                 constants.STATUS_ERROR_DEFERRED_DELETING):

manila/tests/share/test_api.py

@@ -2659,6 +2659,8 @@ class ShareAPITestCase(test.TestCase):
         )
         share_type = fakes.fake_share_type()
 
+        self.mock_object(db_api, 'share_snapshot_get',
+                         mock.Mock(return_value=snapshot))
         mock_get_share_type_call = self.mock_object(
             share_types, 'get_share_type', mock.Mock(return_value=share_type))
 
@@ -2689,7 +2691,8 @@ class ShareAPITestCase(test.TestCase):
             snapshot_host=snapshot['share']['instance']['host'],
             scheduler_hints=None, mount_point_name=None)
         share_api.policy.check_policy.assert_called_once_with(
-            self.context, 'share_snapshot', 'get_snapshot')
+            self.context, 'share_snapshot', 'get_snapshot',
+            snapshot, do_raise=False)
         quota.QUOTAS.reserve.assert_called_once_with(
             self.context, share_type_id=share_type['id'],
             gigabytes=1, shares=1)
@@ -2802,7 +2805,8 @@ class ShareAPITestCase(test.TestCase):
             rule = self.api.get_snapshot(self.context, 'fakeid')
             self.assertEqual(fake_get_snap, rule)
             share_api.policy.check_policy.assert_called_once_with(
-                self.context, 'share_snapshot', 'get_snapshot')
+                self.context, 'share_snapshot', 'get_snapshot', rule,
+                do_raise=False)
             db_api.share_snapshot_get.assert_called_once_with(
                 self.context, 'fakeid')
 
@@ -2817,6 +2821,20 @@ class ShareAPITestCase(test.TestCase):
             self.assertRaises(exception.NotFound, self.api.get_snapshot,
                               self.context, 'fakeid')
 
+    def test_get_snapshot_not_authorized(self):
+        fake_get_snap = {'fake_key': 'fake_val'}
+        share_api.policy.check_policy.return_value = False
+        with mock.patch.object(db_api, 'share_snapshot_get',
+                               mock.Mock(return_value=fake_get_snap)):
+            self.assertRaises(exception.NotFound,
+                              self.api.get_snapshot,
+                              self.context, 'fakeid')
+            share_api.policy.check_policy.assert_called_once_with(
+                self.context, 'share_snapshot', 'get_snapshot',
+                fake_get_snap, do_raise=False)
+            db_api.share_snapshot_get.assert_called_once_with(
+                self.context, 'fakeid')
+
     def test_create_from_snapshot_not_available(self):
         snapshot = db_utils.create_snapshot(
             with_share=True, status=constants.STATUS_ERROR)

releasenotes/notes/bug-2120650-enforce-policy-check-for-share-snapshots-retrieval-b4c66a3e90bd38af.yaml

@@ -0,0 +1,6 @@
+---
+security:
+  - |
+    Closes a gap where a user can see the snapshots belonging to another
+    project if the uuid of the snapshot is leaked, stolen or (improbably)
+    guessed.