sts-sponsors team mailing list archive

[Igor Brovtsin <mp+433293@xxxxxxxxxxxxxxxxxx>]

Igor Brovtsin has proposed merging ~igor-brovtsin/maas:lp-1811799 into maas:master.

Commit message:
Use `self.read -> get_node_or_404` in `details` handler of nodes API to check permissions

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1811799 in MAAS: "[2.5, API] Normal users can read machine details of owned machines"
  https://bugs.launchpad.net/maas/+bug/1811799

For more details, see:
https://code.launchpad.net/~igor-brovtsin/maas/+git/maas/+merge/433293

Straightforward fix for LP:#1811799, use `self.read` to call `get_node_or_404` with proper permissions (`read` seems appropriate in that context, but I might be wrong).
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/api/nodes.py b/src/maasserver/api/nodes.py
index 04c2457..18127ba 100644
--- a/src/maasserver/api/nodes.py
+++ b/src/maasserver/api/nodes.py
@@ -13,7 +13,6 @@ import bson
 from django.db.models import Prefetch
 from django.db.models.functions import Coalesce
 from django.http import HttpResponse
-from django.shortcuts import get_object_or_404
 from formencode.validators import Int, StringBool
 from piston3.utils import rc
 
@@ -540,12 +539,18 @@ class NodeHandler(OperationsHandler):
         ASCII using ``bsondump example.bson``.
         @success-example "success-content" [exkey=details] placeholder text
 
+        @error (http-status-code) "403" 403
+        @error (content) "no-perms" The user does not have permission to see
+        the node details.
+        @error-example "no-perms"
+            Forbidden
+
         @error (http-status-code) "404" 404
         @error (content) "not-found" The requested node is not found.
         @error-example "not-found"
             No Node matches the given query.
         """
-        node = get_object_or_404(self.model, system_id=system_id)
+        node = self.read(request, system_id)
         probe_details = get_single_probed_details(node)
         probe_details_report = {
             name: None if data is None else bson.Binary(data)
diff --git a/src/maasserver/api/tests/test_nodes.py b/src/maasserver/api/tests/test_nodes.py
index 88fa7e6..702edfc 100644
--- a/src/maasserver/api/tests/test_nodes.py
+++ b/src/maasserver/api/tests/test_nodes.py
@@ -746,6 +746,49 @@ class TestNodesAPI(APITestCase.ForUser):
         machine = reload_object(machine)
         self.assertEqual(zone, machine.zone)
 
+    def test_GET_details_returns_details(self):
+        node = factory.make_Node(owner=self.user)
+        response = self.client.get(
+            reverse("nodes_handler"),
+            {
+                "op": "details",
+                "system_id": node.system_id,
+            },
+        )
+        self.assertEqual(http.client.OK, response.status_code)
+
+    def test_GET_details_returns_details_to_admin(self):
+        self.become_admin()
+        node = factory.make_Node(owner=self.user)
+        other_user_node = factory.make_Node(owner=factory.make_User())
+        response = self.client.get(
+            reverse("nodes_handler"),
+            {
+                "op": "details",
+                "system_id": node.system_id,
+            },
+        )
+        self.assertEqual(http.client.OK, response.status_code)
+        response = self.client.get(
+            reverse("nodes_handler"),
+            {
+                "op": "details",
+                "system_id": other_user_node.system_id,
+            },
+        )
+        self.assertEqual(http.client.OK, response.status_code)
+
+    def test_GET_details_returns_forbidden_to_another_user(self):
+        node = factory.make_Node(owner=factory.make_User())
+        response = self.client.get(
+            reverse("nodes_handler"),
+            {
+                "op": "details",
+                "system_id": node.system_id,
+            },
+        )
+        self.assertEqual(http.client.FORBIDDEN, response.status_code)
+
     def test_CREATE_disabled(self):
         response = self.client.post(reverse("nodes_handler"), {})
         self.assertEqual(http.client.BAD_REQUEST, response.status_code)