Browse Source

Merge "Fix getting share networks and security services error" into stable/ocata

tags/4.0.2
Zuul 9 months ago
parent
commit
6363f83027

+ 1
- 1
manila/api/v1/security_service.py View File

@@ -105,7 +105,7 @@ class SecurityServiceController(wsgi.Controller):
105 105
             security_services = share_nw['security_services']
106 106
             del search_opts['share_network_id']
107 107
         else:
108
-            if 'all_tenants' in search_opts:
108
+            if 'all_tenants' in search_opts and context.is_admin:
109 109
                 policy.check_policy(context, RESOURCE_NAME,
110 110
                                     'get_all_security_services')
111 111
                 security_services = db.security_service_get_all(context)

+ 3
- 9
manila/api/v2/share_networks.py View File

@@ -111,20 +111,13 @@ class ShareNetworkController(wsgi.Controller):
111 111
         search_opts = {}
112 112
         search_opts.update(req.GET)
113 113
 
114
-        if ('all_tenants' in search_opts or
115
-            ('project_id' in search_opts and
116
-             search_opts['project_id'] != context.project_id)):
117
-                policy.check_policy(context, RESOURCE_NAME,
118
-                                    'get_all_share_networks')
119
-
120 114
         if 'security_service_id' in search_opts:
121 115
             networks = db_api.share_network_get_all_by_security_service(
122 116
                 context, search_opts['security_service_id'])
123
-        elif ('project_id' in search_opts and
124
-              search_opts['project_id'] != context.project_id):
117
+        elif context.is_admin and 'project_id' in search_opts:
125 118
             networks = db_api.share_network_get_all_by_project(
126 119
                 context, search_opts['project_id'])
127
-        elif 'all_tenants' in search_opts:
120
+        elif context.is_admin and 'all_tenants' in search_opts:
128 121
             networks = db_api.share_network_get_all(context)
129 122
         else:
130 123
             networks = db_api.share_network_get_all_by_project(
@@ -159,6 +152,7 @@ class ShareNetworkController(wsgi.Controller):
159 152
             'limit',
160 153
             'offset',
161 154
             'security_service_id',
155
+            'project_id'
162 156
         ]
163 157
         for opt in opts_to_remove:
164 158
             search_opts.pop(opt, None)

+ 7
- 9
manila/tests/api/v1/test_security_service.py View File

@@ -302,18 +302,16 @@ class ShareApiTest(test.TestCase):
302 302
         db.security_service_get_all.assert_called_once_with(
303 303
             req.environ['manila.context'])
304 304
 
305
-    @mock.patch.object(db, 'security_service_get_all', mock.Mock())
305
+    @mock.patch.object(db, 'security_service_get_all_by_project', mock.Mock())
306 306
     def test_security_services_list_all_tenants_non_admin_context(self):
307
-        self.check_policy_patcher.stop()
308
-        db.security_service_get_all.return_value = [
309
-            self.ss_active_directory,
310
-            self.ss_ldap,
311
-        ]
307
+        db.security_service_get_all_by_project.return_value = []
312 308
         req = fakes.HTTPRequest.blank(
313 309
             '/security-services?all_tenants=1')
314
-        self.assertRaises(exception.PolicyNotAuthorized, self.controller.index,
315
-                          req)
316
-        self.assertFalse(db.security_service_get_all.called)
310
+        fake_context = req.environ['manila.context']
311
+        self.controller.index(req)
312
+        db.security_service_get_all_by_project.assert_called_once_with(
313
+            fake_context, fake_context.project_id
314
+        )
317 315
 
318 316
     @mock.patch.object(db, 'security_service_get_all_by_project', mock.Mock())
319 317
     def test_security_services_list_admin_context_invalid_opts(self):

+ 13
- 11
manila/tests/api/v2/test_share_networks.py View File

@@ -384,13 +384,15 @@ class ShareNetworkAPITest(test.TestCase):
384 384
             result[share_networks.RESOURCES_NAME][0],
385 385
             fake_sn_with_ss_shortened)
386 386
 
387
-    @mock.patch.object(db_api, 'share_network_get_all', mock.Mock())
387
+    @mock.patch.object(db_api, 'share_network_get_all_by_project', mock.Mock())
388 388
     def test_index_all_tenants_non_admin_context(self):
389 389
         req = fakes.HTTPRequest.blank(
390 390
             '/share_networks?all_tenants=1')
391
-        self.assertRaises(exception.PolicyNotAuthorized, self.controller.index,
392
-                          req)
393
-        self.assertFalse(db_api.share_network_get_all.called)
391
+        fake_context = req.environ['manila.context']
392
+        db_api.share_network_get_all_by_project.return_value = []
393
+        self.controller.index(req)
394
+        db_api.share_network_get_all_by_project.assert_called_with(
395
+            fake_context, fake_context.project_id)
394 396
 
395 397
     @mock.patch.object(db_api, 'share_network_get_all', mock.Mock())
396 398
     def test_index_all_tenants_admin_context(self):
@@ -410,15 +412,16 @@ class ShareNetworkAPITest(test.TestCase):
410 412
     def test_index_filter_by_project_id_non_admin_context(self):
411 413
         req = fakes.HTTPRequest.blank(
412 414
             '/share_networks?project_id=fake project')
413
-        self.assertRaises(exception.PolicyNotAuthorized, self.controller.index,
414
-                          req)
415
-        self.assertFalse(db_api.share_network_get_all_by_project.called)
415
+        fake_context = req.environ['manila.context']
416
+        db_api.share_network_get_all_by_project.return_value = []
417
+        self.controller.index(req)
418
+        db_api.share_network_get_all_by_project.assert_called_with(
419
+            fake_context, fake_context.project_id)
416 420
 
417 421
     @mock.patch.object(db_api, 'share_network_get_all_by_project', mock.Mock())
418 422
     def test_index_filter_by_project_id_admin_context(self):
419 423
         db_api.share_network_get_all_by_project.return_value = [
420
-            fake_share_network,
421
-            fake_share_network_with_ss,
424
+            fake_share_network_with_ss
422 425
         ]
423 426
         req = fakes.HTTPRequest.blank(
424 427
             '/share_networks?project_id=fake',
@@ -435,8 +438,7 @@ class ShareNetworkAPITest(test.TestCase):
435 438
                        mock.Mock())
436 439
     def test_index_filter_by_ss_and_project_id_admin_context(self):
437 440
         db_api.share_network_get_all_by_security_service.return_value = [
438
-            fake_share_network,
439
-            fake_share_network_with_ss,
441
+            fake_share_network_with_ss
440 442
         ]
441 443
         req = fakes.HTTPRequest.blank(
442 444
             '/share_networks?security_service_id=fake-ss-id&project_id=fake',

+ 8
- 0
manila_tempest_tests/tests/api/test_security_services.py View File

@@ -200,3 +200,11 @@ class SecurityServicesTest(base.BaseSharesTest,
200 200
         self.assertTrue(any(self.ss_ldap['id'] == ss['id'] for ss in listed))
201 201
         self.assertTrue(any(self.ss_kerberos['id'] == ss['id']
202 202
                             for ss in listed))
203
+
204
+    @tc.attr(base.TAG_POSITIVE, base.TAG_API)
205
+    def test_try_list_security_services_all_tenants(self):
206
+        listed = self.shares_client.list_security_services(
207
+            params={'all_tenants': 1})
208
+        self.assertTrue(any(self.ss_ldap['id'] == ss['id'] for ss in listed))
209
+        self.assertTrue(any(self.ss_kerberos['id'] == ss['id']
210
+                            for ss in listed))

+ 0
- 6
manila_tempest_tests/tests/api/test_security_services_negative.py View File

@@ -120,9 +120,3 @@ class SecurityServicesNegativeTest(base.BaseSharesTest):
120 120
         self.assertRaises(lib_exc.NotFound,
121 121
                           self.shares_client.get_security_service,
122 122
                           ss["id"])
123
-
124
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API)
125
-    def test_try_list_security_services_all_tenants(self):
126
-        self.assertRaises(lib_exc.Forbidden,
127
-                          self.shares_client.list_security_services,
128
-                          params={'all_tenants': 1})

+ 20
- 0
manila_tempest_tests/tests/api/test_share_networks.py View File

@@ -35,6 +35,26 @@ class ShareNetworkListMixin(object):
35 35
         keys = ["name", "id"]
36 36
         [self.assertIn(key, sn.keys()) for sn in listed for key in keys]
37 37
 
38
+    @tc.attr(base.TAG_POSITIVE, base.TAG_API)
39
+    def test_try_list_share_networks_all_tenants(self):
40
+        listed = self.shares_client.list_share_networks_with_detail(
41
+            params={'all_tenants': 1})
42
+        any(self.sn_with_ldap_ss["id"] in sn["id"] for sn in listed)
43
+
44
+        # verify keys
45
+        keys = ["name", "id"]
46
+        [self.assertIn(key, sn.keys()) for sn in listed for key in keys]
47
+
48
+    @tc.attr(base.TAG_POSITIVE, base.TAG_API)
49
+    def test_try_list_share_networks_project_id(self):
50
+        listed = self.shares_client.list_share_networks_with_detail(
51
+            params={'project_id': 'some_project'})
52
+        any(self.sn_with_ldap_ss["id"] in sn["id"] for sn in listed)
53
+
54
+        # verify keys
55
+        keys = ["name", "id"]
56
+        [self.assertIn(key, sn.keys()) for sn in listed for key in keys]
57
+
38 58
     @tc.attr(base.TAG_POSITIVE, base.TAG_API)
39 59
     def test_list_share_networks_with_detail(self):
40 60
         listed = self.shares_v2_client.list_share_networks_with_detail()

+ 0
- 12
manila_tempest_tests/tests/api/test_share_networks_negative.py View File

@@ -81,18 +81,6 @@ class ShareNetworksNegativeTest(base.BaseSharesTest):
81 81
                           self.shares_client.get_security_service,
82 82
                           sn["id"])
83 83
 
84
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API)
85
-    def test_try_list_share_networks_all_tenants(self):
86
-        self.assertRaises(lib_exc.Forbidden,
87
-                          self.shares_client.list_share_networks_with_detail,
88
-                          params={'all_tenants': 1})
89
-
90
-    @tc.attr(base.TAG_NEGATIVE, base.TAG_API)
91
-    def test_try_list_share_networks_project_id(self):
92
-        self.assertRaises(lib_exc.Forbidden,
93
-                          self.shares_client.list_share_networks_with_detail,
94
-                          params={'project_id': 'some_project'})
95
-
96 84
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API)
97 85
     def test_try_list_share_networks_wrong_created_since_value(self):
98 86
         self.assertRaises(

+ 6
- 0
releasenotes/notes/bug-1721787-fix-getting-share-networks-and-security-services-error-7e5e7981fcbf2b53.yaml View File

@@ -0,0 +1,6 @@
1
+---
2
+fixes:
3
+  - Non admin users may invoke GET /share-networks and GET /security-services
4
+    APIs with the 'all-tenants' flag in the query, however, the flag is
5
+    ignored, and only resources belonging to the project will be served. This
6
+    API change was made to fix bug 1721787 in the manila client project.

Loading…
Cancel
Save