Merge ~ack/maas:rbac-edit-owned-devices-1811665 into maas:master

[Alberto Donato]

LP: #1811665 - allow non-admins to edit their devices

[MAAS Lander (maas-lander)]

UNIT TESTS
-b rbac-edit-owned-devices-1811665 lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 04aa1bcc936869b85ce1c9b58cda97ab30d2a95d

review: Approve

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

diff --git a/src/maasserver/api/tests/test_devices.py b/src/maasserver/api/tests/test_devices.py
index fd1ec46..a122d67 100644
--- a/src/maasserver/api/tests/test_devices.py
+++ b/src/maasserver/api/tests/test_devices.py
@@ -8,6 +8,7 @@ __all__ = []
 import http.client
 import random
 
+from maasserver.api import auth
 from maasserver.enum import (
     NODE_STATUS,
     NODE_TYPE,
@@ -19,6 +20,7 @@ from maasserver.models import (
 )
 from maasserver.testing.api import APITestCase
 from maasserver.testing.factory import factory
+from maasserver.testing.fixtures import RBACEnabled
 from maasserver.utils.converters import json_load_bytes
 from maasserver.utils.django_urls import reverse
 from maasserver.utils.orm import reload_object
@@ -336,6 +338,23 @@ class TestDeviceAPI(APITestCase.ForUser):
         self.assertEqual(http.client.FORBIDDEN, response.status_code)
         self.assertEqual(old_hostname, reload_object(device).hostname)
 
+    def test_PUT_updates_with_rbac(self):
+        self.patch(auth, 'validate_user_external_auth').return_value = True
+        self.useFixture(RBACEnabled())
+        self.become_non_local()
+
+        device = factory.make_Node(
+            node_type=NODE_TYPE.DEVICE, owner=self.user)
+        new_hostname = factory.make_name('hostname')
+
+        response = self.client.put(
+            get_device_uri(device), {'hostname': new_hostname})
+        self.assertEqual(
+            http.client.OK, response.status_code, response.content)
+
+        device = reload_object(device)
+        self.assertEqual(new_hostname, device.hostname)
+
     def test_DELETE_removes_device(self):
         device = factory.make_Node(
             node_type=NODE_TYPE.DEVICE, owner=self.user)
diff --git a/src/maasserver/models/__init__.py b/src/maasserver/models/__init__.py
index ba2c084..6abec61 100644
--- a/src/maasserver/models/__init__.py
+++ b/src/maasserver/models/__init__.py
@@ -520,7 +520,10 @@ class MAASAuthorizationBackend(ModelBackend):
         if rbac_enabled:
             can_admin = self._can_admin(
                 rbac_enabled, user, machine, admin_pools)
-            can_edit = (machine.pool_id in deploy_pools) or can_admin
+            can_edit = (
+                machine.pool_id in deploy_pools or
+                (machine.pool_id is None and machine.owner == user) or
+                can_admin)
             return (editable and can_edit) or can_admin
         return editable or user.is_superuser
 
diff --git a/src/maasserver/websockets/handlers/tests/test_device.py b/src/maasserver/websockets/handlers/tests/test_device.py
index 36169e6..ac3e083 100644
--- a/src/maasserver/websockets/handlers/tests/test_device.py
+++ b/src/maasserver/websockets/handlers/tests/test_device.py
@@ -27,7 +27,10 @@ from maasserver.models.staticipaddress import StaticIPAddress
 from maasserver.node_action import compile_node_actions
 from maasserver.permissions import NodePermission
 from maasserver.testing.factory import factory
-from maasserver.testing.fixtures import RBACForceOffFixture
+from maasserver.testing.fixtures import (
+    RBACEnabled,
+    RBACForceOffFixture,
+)
 from maasserver.testing.testcase import MAASTransactionServerTestCase
 from maasserver.utils.orm import (
     reload_object,
@@ -1014,6 +1017,18 @@ class TestDeviceHandler(MAASTransactionServerTestCase):
             HandlerDoesNotExistError, handler.update, node_data)
 
     @transactional
+    def test_update_owned_with_rbac(self):
+        self.useFixture(RBACEnabled())
+        user = factory.make_User(is_local=False)
+        node = factory.make_Node(owner=user, node_type=NODE_TYPE.DEVICE)
+        handler = DeviceHandler(user, {}, None)
+        new_hostname = factory.make_name("hostname")
+        updated_node = handler.update(
+            {"system_id": node.system_id,
+             'hostname': new_hostname})
+        self.assertEqual(updated_node['hostname'], new_hostname)
+
+    @transactional
     def test_delete_interface_admin(self):
         user = factory.make_admin()
         node = factory.make_Node(node_type=NODE_TYPE.DEVICE)