From 00147a7d700e6d0142161152137bbab0c39ce4c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20J=C3=B3zefczyk?= <maciej.jozefczyk@corp.ovh.com>
Date: Thu, 11 Oct 2018 08:57:29 +0000
Subject: [PATCH] Allow advsvc role to create port in foreign tenant

Change [1] introduced support for advsvc role. This added
possibility for user with role advsvc to make CRUD operations
on ports, subnets and networks in foreign tenants.
Due the check in _validate_privileges() it was not working.
This patch fixes that.

Closes-Bug: #1796854

[1] https://review.openstack.org/#/c/101281

Change-Id: I6a3f91337bf8dd32012a75916e3409e30f46b50d
---
 api-ref/source/v2/parameters.yaml             |  6 ++-
 neutron_lib/api/attributes.py                 |  5 ++-
 neutron_lib/tests/unit/api/test_attributes.py | 41 +++++++++++++++++++
 .../advsvc-role-support-d4f1c532264b729a.yaml |  4 ++
 4 files changed, 52 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/advsvc-role-support-d4f1c532264b729a.yaml

diff --git a/api-ref/source/v2/parameters.yaml b/api-ref/source/v2/parameters.yaml
index f64015e40..e55e3ef23 100644
--- a/api-ref/source/v2/parameters.yaml
+++ b/api-ref/source/v2/parameters.yaml
@@ -3732,7 +3732,8 @@ mac_address-request:
 mac_address-request-put:
   description: |
     The MAC address of the port.
-    By default, only administrative users can change this value.
+    By default, only administrative users and users with advsvc role
+    can change this value.
   in: body
   required: false
   type: string
@@ -4432,7 +4433,8 @@ project_id-body-required:
 project_id-request:
   description: |
     The ID of the project that owns the resource.
-    Only administrative users can specify a project ID other than their own.
+    Only administrative and users with advsvc role can specify
+    a project ID other than their own.
     You cannot change this value through authorization policies.
   in: body
   required: false
diff --git a/neutron_lib/api/attributes.py b/neutron_lib/api/attributes.py
index 0ddc383bb..6a19aae28 100644
--- a/neutron_lib/api/attributes.py
+++ b/neutron_lib/api/attributes.py
@@ -25,9 +25,10 @@ from neutron_lib import exceptions
 def _validate_privileges(context, res_dict):
     if ('project_id' in res_dict and
             res_dict['project_id'] != context.project_id and
-            not context.is_admin):
+            not (context.is_admin or context.is_advsvc)):
         msg = _("Specifying 'project_id' or 'tenant_id' other than the "
-                "authenticated project in request requires admin privileges")
+                "authenticated project in request requires admin or advsvc "
+                "privileges")
         raise exc.HTTPBadRequest(msg)
 
 
diff --git a/neutron_lib/tests/unit/api/test_attributes.py b/neutron_lib/tests/unit/api/test_attributes.py
index 16fc50670..c9d031399 100644
--- a/neutron_lib/tests/unit/api/test_attributes.py
+++ b/neutron_lib/tests/unit/api/test_attributes.py
@@ -280,3 +280,44 @@ class TestCoreResources(base.BaseTestCase):
         for r in TestCoreResources.CORE_DEFS:
             self.assertIs(r.RESOURCE_ATTRIBUTE_MAP[r.COLLECTION_NAME],
                           attributes.RESOURCES[r.COLLECTION_NAME])
+
+
+class TestValidatePriviliges(base.BaseTestCase):
+
+    def test__validate_privileges_same_tenant(self):
+        project_id = 'fake_project'
+        ctx = context.Context(project_id=project_id)
+        res_dict = {'project_id': project_id}
+        try:
+            attributes._validate_privileges(ctx, res_dict)
+        except exc.HTTPBadRequest:
+            self.fail("HTTPBadRequest exception should not be raised.")
+
+    def test__validate_privileges_user_other_tenant(self):
+        project_id = 'fake_project'
+        ctx = context.Context(project_id='fake_project2')
+        res_dict = {'project_id': project_id}
+        self.assertRaises(
+            exc.HTTPBadRequest,
+            attributes._validate_privileges,
+            ctx, res_dict)
+
+    def test__validate_privileges_admin_other_tenant(self):
+        project_id = 'fake_project'
+        ctx = context.Context(project_id='fake_project2',
+                              is_admin=True)
+        res_dict = {'project_id': project_id}
+        try:
+            attributes._validate_privileges(ctx, res_dict)
+        except exc.HTTPBadRequest:
+            self.fail("HTTPBadRequest exception should not be raised.")
+
+    def test__validate_privileges_advsvc_other_tenant(self):
+        project_id = 'fake_project'
+        ctx = context.Context(project_id='fake_project2',
+                              is_advsvc=True)
+        res_dict = {'project_id': project_id}
+        try:
+            attributes._validate_privileges(ctx, res_dict)
+        except exc.HTTPBadRequest:
+            self.fail("HTTPBadRequest exception should not be raised.")
diff --git a/releasenotes/notes/advsvc-role-support-d4f1c532264b729a.yaml b/releasenotes/notes/advsvc-role-support-d4f1c532264b729a.yaml
new file mode 100644
index 000000000..f5987536f
--- /dev/null
+++ b/releasenotes/notes/advsvc-role-support-d4f1c532264b729a.yaml
@@ -0,0 +1,4 @@
+---
+fixes:
+  - Bug `1796854 <https://bugs.launchpad.net/neutron/+bug/1796854>`_ is fixed
+    by validating if advsvc context is used.