Merge ~ack/maas:1812201-list-allocated-rbac-2.5 into maas:2.5

[Alberto Donato]

Backport ca0d2aebb1c8bdf4fa02ff529b0561512a6b3c9a - LP: #1812201 - filter allocated machines based on pools with RBAC

[Alberto Donato (ack)]

review: Approve

diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
index 0a0fbfc..2701784 100644
--- a/src/maasserver/api/machines.py
+++ b/src/maasserver/api/machines.py
@@ -1934,10 +1934,12 @@ class MachinesHandler(NodesHandler, PowersMixin):
         @success-example "success-json" [exkey=machines-placeholder]
         placeholder text
         """
-        token = get_oauth_token(request)
-        match_ids = get_optional_list(request.GET, 'id')
-        machines = Machine.objects.get_allocated_visible_machines(
-            token, match_ids)
+        # limit to machines that the user can view
+        machines = Machine.objects.get_nodes(request.user, NodePermission.view)
+        machines = machines.filter(token=get_oauth_token(request))
+        system_ids = get_optional_list(request.GET, 'id')
+        if system_ids:
+            machines = machines.filter(system_id__in=system_ids)
         return machines.order_by('id')
 
     @operation(idempotent=False)
diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
index 282c6bb..5eac395 100644
--- a/src/maasserver/api/tests/test_machines.py
+++ b/src/maasserver/api/tests/test_machines.py
@@ -629,6 +629,41 @@ class TestMachinesAPI(APITestCase.ForUser):
         self.assertItemsEqual(
             required_machine_ids, extract_system_ids(parsed_result))
 
+    def test_GET_list_allocated_with_rbac(self):
+        self.patch(auth, 'validate_user_external_auth').return_value = True
+        rbac = self.useFixture(RBACEnabled())
+        self.become_non_local()
+
+        user = factory.make_User()
+        pool = factory.make_ResourcePool()
+        rbac.store.allow(user.username, pool, 'view')
+
+        pool = factory.make_ResourcePool()
+        rbac.store.add_pool(pool)
+        rbac.store.allow(self.user.username, pool, 'view')
+
+        token = get_auth_tokens(self.user)[0]
+        factory.make_Node(
+            hostname='viewable', owner=self.user, token=token, pool=pool,
+            status=NODE_STATUS.ALLOCATED)
+        # a machine with the same token but not accesssible to the user (not in
+        # the allowed pool)
+        factory.make_Node(
+            hostname='not-accessible', owner=self.user, token=token,
+            status=NODE_STATUS.ALLOCATED)
+        # a machine owned by another user in the accessible pool
+        factory.make_Node(
+            hostname='other-user', owner=factory.make_User(),
+            status=NODE_STATUS.ALLOCATED, pool=pool)
+
+        response = self.client.get(
+            reverse('machines_handler'), {'op': 'list_allocated'})
+        self.assertEqual(http.client.OK, response.status_code)
+        parsed_result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        hostnames = [machine['hostname'] for machine in parsed_result]
+        self.assertEqual(['viewable'], hostnames)
+
     def test_POST_allocate_returns_available_machine(self):
         # The "allocate" operation returns an available machine.
         available_status = NODE_STATUS.READY
diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index 8a6d260..e49c649 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -604,27 +604,6 @@ class MachineManager(BaseNodeManager):
 
     extra_filters = {'node_type': NODE_TYPE.MACHINE}
 
-    def get_allocated_visible_machines(self, token, ids):
-        """Fetch Machines that were allocated to the User_/oauth token.
-
-        :param user: The user whose machines to fetch
-        :type user: User_
-        :param token: The OAuth token associated with the Machines.
-        :type token: piston3.models.Token.
-        :param ids: Optional set of IDs to filter by. If given, machines whose
-            system_ids are not in `ids` will be ignored.
-        :type param_ids: Sequence
-
-        .. _User: https://
-           docs.djangoproject.com/en/dev/topics/auth/
-           #django.contrib.auth.models.User
-        """
-        if ids is None:
-            machines = self.filter(token=token)
-        else:
-            machines = self.filter(token=token, system_id__in=ids)
-        return machines
-
     def get_available_machines_for_acquisition(self, for_user):
         """Find the machines that can be acquired by the given user.
 
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index c4ebd6e..361b6f8 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -432,6 +432,7 @@ class TestNodeManager(MAASServerTestCase):
 
 
 class TestMachineManager(MAASServerTestCase):
+
     def make_machine(self, user=None, **kwargs):
         """Create a machine, allocated to `user` if given."""
         if user is None: