Merge ~ltrager/maas:lp1870171_2.6 into maas:2.6

Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: 82258bc1a20a18d6c45b8a97bf4b8aaf88dbefbf
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:lp1870171_2.6
Merge into: maas:2.6
Diff against target: 820 lines (+361/-128)
13 files modified
src/maasserver/api/account.py (+1/-1)
src/maasserver/api/machines.py (+12/-9)
src/maasserver/api/tests/test_api.py (+15/-2)
src/maasserver/api/tests/test_machine.py (+40/-16)
src/maasserver/api/tests/test_machines.py (+23/-54)
src/maasserver/models/node.py (+11/-7)
src/maasserver/models/tests/test_node.py (+29/-32)
src/maasserver/models/tests/test_userprofile.py (+9/-1)
src/maasserver/models/userprofile.py (+7/-2)
src/maasserver/node_action.py (+2/-2)
src/maasserver/tests/test_node_action.py (+1/-2)
src/maasserver/websockets/handlers/tests/test_token.py (+115/-0)
src/maasserver/websockets/handlers/token.py (+96/-0)
Reviewer Review Type Date Requested Status
Lee Trager (community) Approve
Review via email: mp+382095@code.launchpad.net

Commit message

Backport of d0653c9 LP: #1870171 - Remove the token column from the Node table.

The token column is only used by the API when allocating and machine and
listing allocated machines. When a User Token is deleted this association
causes the Node to be deleted as well. Allocated machines are already
filtered by owner and status so the token column isn't needed.

In the backport Node.token is no longer used but the column remains to
avoid creating a migration.

To post a comment you must log in.
Revision history for this message
Lee Trager (ltrager) wrote :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/api/account.py b/src/maasserver/api/account.py
2index 5a5e1f5..cf9ea03 100644
3--- a/src/maasserver/api/account.py
4+++ b/src/maasserver/api/account.py
5@@ -1,4 +1,4 @@
6-# Copyright 2014-2018 Canonical Ltd. This software is licensed under the
7+# Copyright 2014-2020 Canonical Ltd. This software is licensed under the
8 # GNU Affero General Public License version 3 (see the file LICENSE).
9
10 """API handler: `Account`."""
11diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
12index ab5ab67..6e6d001 100644
13--- a/src/maasserver/api/machines.py
14+++ b/src/maasserver/api/machines.py
15@@ -1,4 +1,4 @@
16-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
17+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
18 # GNU Affero General Public License version 3 (see the file LICENSE).
19
20 __all__ = [
21@@ -46,7 +46,6 @@ from maasserver.api.support import (
22 )
23 from maasserver.api.utils import (
24 get_mandatory_param,
25- get_oauth_token,
26 get_optional_list,
27 get_optional_param,
28 )
29@@ -708,8 +707,9 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
30 "Request from user %s to acquire machine: %s (%s)",
31 request.user.username, machine.fqdn, machine.system_id)
32 machine.acquire(
33- request.user, get_oauth_token(request),
34- agent_name=options.agent_name, comment=options.comment,
35+ request.user,
36+ agent_name=options.agent_name,
37+ comment=options.comment,
38 bridge_all=options.bridge_all,
39 bridge_stp=options.bridge_stp, bridge_fd=options.bridge_fd)
40 if NODE_STATUS.DEPLOYING not in NODE_TRANSITIONS[machine.status]:
41@@ -2004,7 +2004,7 @@ class MachinesHandler(NodesHandler, PowersMixin):
42 @operation(idempotent=True)
43 def list_allocated(self, request):
44 """@description-title List allocated
45- @description List machines that were allocated to the User/oauth token.
46+ @description List machines that were allocated to the User.
47
48 @success (http-status-code) "200" 200
49 @success (json) "success-json" A JSON object containing a list of
50@@ -2014,8 +2014,10 @@ class MachinesHandler(NodesHandler, PowersMixin):
51 """
52 # limit to machines that the user can view
53 machines = Machine.objects.get_nodes(request.user, NodePermission.view)
54- machines = machines.filter(token=get_oauth_token(request))
55- system_ids = get_optional_list(request.GET, 'id')
56+ machines = machines.filter(
57+ owner=request.user, status=NODE_STATUS.ALLOCATED
58+ )
59+ system_ids = get_optional_list(request.GET, "id")
60 if system_ids:
61 machines = machines.filter(system_id__in=system_ids)
62 return machines.order_by('id')
63@@ -2320,8 +2322,9 @@ class MachinesHandler(NodesHandler, PowersMixin):
64 raise NodesNotAvailable(message)
65 if not dry_run:
66 machine.acquire(
67- request.user, get_oauth_token(request),
68- agent_name=options.agent_name, comment=options.comment,
69+ request.user,
70+ agent_name=options.agent_name,
71+ comment=options.comment,
72 bridge_all=options.bridge_all,
73 bridge_stp=options.bridge_stp, bridge_fd=options.bridge_fd)
74 machine.constraint_map = storage.get(machine.id, {})
75diff --git a/src/maasserver/api/tests/test_api.py b/src/maasserver/api/tests/test_api.py
76index c918ff8..55f4368 100644
77--- a/src/maasserver/api/tests/test_api.py
78+++ b/src/maasserver/api/tests/test_api.py
79@@ -1,4 +1,4 @@
80-# Copyright 2012-2019 Canonical Ltd. This software is licensed under the
81+# Copyright 2012-2020 Canonical Ltd. This software is licensed under the
82 # GNU Affero General Public License version 3 (see the file LICENSE).
83
84 """Test maasserver API."""
85@@ -50,7 +50,10 @@ from maasserver.testing.testclient import MAASSensibleOAuthClient
86 from maasserver.utils.converters import json_load_bytes
87 from maasserver.utils.django_urls import reverse
88 from maasserver.utils.keys import ImportSSHKeysError
89-from maasserver.utils.orm import get_one
90+from maasserver.utils.orm import (
91+ get_one,
92+ reload_object,
93+)
94 from maastesting.matchers import (
95 MockCalledOnce,
96 MockCalledOnceWith,
97@@ -328,6 +331,16 @@ class AccountAPITest(APITestCase.ForUser):
98 self.assertEqual(http.client.BAD_REQUEST, response.status_code)
99 self.assertThat(mock_create_audit_event, MockNotCalled())
100
101+ def test_delete_authorisation_token_doesnt_delete_node(self):
102+ token = self.user.tokens.first()
103+ node = factory.make_Node(owner=self.user, token=token)
104+ response = self.client.post(
105+ reverse("account_handler"),
106+ {"op": "delete_authorisation_token", "token_key": token.key},
107+ )
108+ self.assertEqual(http.client.NO_CONTENT, response.status_code)
109+ self.assertIsNotNone(reload_object(node))
110+
111 def test_update_authorisation_token(self):
112 token_name_orig = 'Test_Token'
113 token_name_updated = 'Test_Token update'
114diff --git a/src/maasserver/api/tests/test_machine.py b/src/maasserver/api/tests/test_machine.py
115index 6eb2cd6..e3ed7e9 100644
116--- a/src/maasserver/api/tests/test_machine.py
117+++ b/src/maasserver/api/tests/test_machine.py
118@@ -797,10 +797,16 @@ class TestMachineAPI(APITestCase.ForUser):
119 }
120 self.client.post(self.get_machine_uri(machine), request)
121 self.assertThat(
122- machine_method, MockCalledOnceWith(
123- ANY, ANY, agent_name=ANY,
124- bridge_all=False, bridge_fd=False,
125- bridge_stp=False, comment=request['comment']))
126+ machine_method,
127+ MockCalledOnceWith(
128+ ANY,
129+ agent_name=ANY,
130+ bridge_all=False,
131+ bridge_fd=0,
132+ bridge_stp=False,
133+ comment=request["comment"],
134+ ),
135+ )
136
137 def test_POST_deploy_passes_bridge_settings(self):
138 self.patch(node_module.Node, "_start")
139@@ -821,10 +827,16 @@ class TestMachineAPI(APITestCase.ForUser):
140 }
141 self.client.post(self.get_machine_uri(machine), request)
142 self.assertThat(
143- machine_method, MockCalledOnceWith(
144- ANY, ANY, agent_name=ANY,
145- bridge_all=True, bridge_fd=7,
146- bridge_stp=True, comment=None))
147+ machine_method,
148+ MockCalledOnceWith(
149+ ANY,
150+ agent_name=ANY,
151+ bridge_all=True,
152+ bridge_fd=7,
153+ bridge_stp=True,
154+ comment=None,
155+ ),
156+ )
157
158 def test_POST_deploy_stores_vcenter_registration_by_default(self):
159 self.become_admin()
160@@ -1148,10 +1160,16 @@ class TestMachineAPI(APITestCase.ForUser):
161 reverse('machines_handler'),
162 {'op': 'allocate', 'comment': comment})
163 self.assertThat(
164- machine_method, MockCalledOnceWith(
165- ANY, ANY, agent_name=ANY,
166- bridge_all=False, bridge_fd=False,
167- bridge_stp=False, comment=comment))
168+ machine_method,
169+ MockCalledOnceWith(
170+ ANY,
171+ agent_name=ANY,
172+ bridge_all=False,
173+ bridge_fd=False,
174+ bridge_stp=False,
175+ comment=comment,
176+ ),
177+ )
178
179 def test_POST_allocate_handles_missing_comment(self):
180 factory.make_Node(
181@@ -1161,10 +1179,16 @@ class TestMachineAPI(APITestCase.ForUser):
182 self.client.post(
183 reverse('machines_handler'), {'op': 'allocate'})
184 self.assertThat(
185- machine_method, MockCalledOnceWith(
186- ANY, ANY, agent_name=ANY,
187- bridge_all=False, bridge_fd=False,
188- bridge_stp=False, comment=None))
189+ machine_method,
190+ MockCalledOnceWith(
191+ ANY,
192+ agent_name=ANY,
193+ bridge_all=False,
194+ bridge_fd=0,
195+ bridge_stp=False,
196+ comment=None,
197+ ),
198+ )
199
200 def test_POST_release_frees_hwe_kernel(self):
201 self.patch(node_module.Machine, '_stop')
202diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
203index 7849678..9199cce 100644
204--- a/src/maasserver/api/tests/test_machines.py
205+++ b/src/maasserver/api/tests/test_machines.py
206@@ -1,4 +1,4 @@
207-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
208+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
209 # GNU Affero General Public License version 3 (see the file LICENSE).
210
211 """Tests for the machines API."""
212@@ -43,10 +43,6 @@ from maasserver.models import (
213 node as node_module,
214 )
215 from maasserver.models.node import RELEASABLE_STATUSES
216-from maasserver.models.user import (
217- create_auth_token,
218- get_auth_tokens,
219-)
220 from maasserver.node_constraint_filter_forms import AcquireNodeForm
221 from maasserver.rpc.testing.fixtures import MockLiveRegionToClusterRPCFixture
222 from maasserver.testing.api import (
223@@ -634,29 +630,11 @@ class TestMachinesAPI(APITestCase.ForUser):
224 [machine.system_id for machine in machines],
225 extract_system_ids(parsed_result))
226
227- def test_GET_list_allocated_returns_only_allocated_with_user_token(self):
228- # If the user's allocated machines have different session tokens,
229- # list_allocated should only return the machines that have the
230- # current request's token on them.
231- machine_1 = factory.make_Node(
232- status=NODE_STATUS.ALLOCATED, owner=self.user,
233- token=get_auth_tokens(self.user)[0])
234- second_token = create_auth_token(self.user)
235- factory.make_Node(
236- owner=self.user, status=NODE_STATUS.ALLOCATED,
237- token=second_token)
238-
239- user_2 = factory.make_User()
240- create_auth_token(user_2)
241- factory.make_Node(
242- owner=self.user, status=NODE_STATUS.ALLOCATED,
243- token=second_token)
244-
245- # At this point we have two machines owned by the same user but
246- # allocated with different tokens, and a third machine allocated to
247- # someone else entirely. We expect list_allocated to
248- # return the machine with the same token as the one used in
249- # self.client, which is the one we set on machine_1 above.
250+ def test_GET_list_allocated_returns_only_allocated_with_user(self):
251+ machine = factory.make_Node(
252+ status=NODE_STATUS.ALLOCATED, owner=self.user
253+ )
254+ factory.make_Node(status=NODE_STATUS.ALLOCATED)
255
256 response = self.client.get(reverse('machines_handler'), {
257 'op': 'list_allocated'})
258@@ -664,17 +642,17 @@ class TestMachinesAPI(APITestCase.ForUser):
259 parsed_result = json.loads(
260 response.content.decode(settings.DEFAULT_CHARSET))
261 self.assertItemsEqual(
262- [machine_1.system_id], extract_system_ids(parsed_result))
263+ [machine.system_id], extract_system_ids(parsed_result)
264+ )
265
266 def test_GET_list_allocated_filters_by_id(self):
267- # list_allocated takes an optional list of 'id' parameters to
268- # filter returned results.
269- current_token = get_auth_tokens(self.user)[0]
270 machines = []
271 for _ in range(3):
272- machines.append(factory.make_Node(
273- status=NODE_STATUS.ALLOCATED,
274- owner=self.user, token=current_token))
275+ machines.append(
276+ factory.make_Node(
277+ status=NODE_STATUS.ALLOCATED, owner=self.user
278+ )
279+ )
280
281 required_machine_ids = [machines[0].system_id, machines[1].system_id]
282 response = self.client.get(reverse('machines_handler'), {
283@@ -700,15 +678,19 @@ class TestMachinesAPI(APITestCase.ForUser):
284 rbac.store.add_pool(pool)
285 rbac.store.allow(self.user.username, pool, 'view')
286
287- token = get_auth_tokens(self.user)[0]
288 factory.make_Node(
289- hostname='viewable', owner=self.user, token=token, pool=pool,
290- status=NODE_STATUS.ALLOCATED)
291- # a machine with the same token but not accesssible to the user (not in
292+ hostname="viewable",
293+ owner=self.user,
294+ pool=pool,
295+ status=NODE_STATUS.ALLOCATED,
296+ )
297+ # a machine with the same user but not accesssible to the user (not in
298 # the allowed pool)
299 factory.make_Node(
300- hostname='not-accessible', owner=self.user, token=token,
301- status=NODE_STATUS.ALLOCATED)
302+ hostname="not-accessible",
303+ owner=self.user,
304+ status=NODE_STATUS.ALLOCATED,
305+ )
306 # a machine owned by another user in the accessible pool
307 factory.make_Node(
308 hostname='other-user', owner=factory.make_User(),
309@@ -1933,19 +1915,6 @@ class TestMachinesAPI(APITestCase.ForUser):
310 response.content.decode(settings.DEFAULT_CHARSET))['system_id']
311 self.assertEqual(node2.system_id, system_id)
312
313- def test_POST_allocate_sets_a_token(self):
314- # "acquire" should set the Token being used in the request on
315- # the Machine that is allocated.
316- available_status = NODE_STATUS.READY
317- machine = factory.make_Node(
318- status=available_status, owner=None, with_boot_disk=True)
319- response = self.client.post(
320- reverse('machines_handler'), {'op': 'allocate'})
321- self.assertThat(response, HasStatusCode(http.client.OK))
322- machine = Machine.objects.get(system_id=machine.system_id)
323- oauth_key = self.client.token.key
324- self.assertEqual(oauth_key, machine.token.key)
325-
326 def test_POST_accept_gets_machine_out_of_declared_state(self):
327 # This will change when we add provisioning. Until then,
328 # acceptance gets a machine straight to Ready state.
329diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
330index e810045..2a42679 100644
331--- a/src/maasserver/models/node.py
332+++ b/src/maasserver/models/node.py
333@@ -2810,11 +2810,17 @@ class Node(CleanSave, TimestampedModel):
334 "s" if len(missing_packages) > 1 else ""))
335
336 def acquire(
337- self, user, token=None, agent_name='', comment=None,
338- bridge_all=False, bridge_stp=None, bridge_fd=None):
339- """Mark commissioned node as acquired by the given user and token."""
340+ self,
341+ user,
342+ agent_name="",
343+ comment=None,
344+ bridge_all=False,
345+ bridge_type=None,
346+ bridge_stp=None,
347+ bridge_fd=None,
348+ ):
349+ """Mark commissioned node as acquired by the given user."""
350 assert self.owner is None or self.owner == user
351- assert token is None or token.user == user
352
353 self._create_acquired_filesystems()
354 self._register_request_event(
355@@ -2823,7 +2829,6 @@ class Node(CleanSave, TimestampedModel):
356 self.status = NODE_STATUS.ALLOCATED
357 self.owner = user
358 self.agent_name = agent_name
359- self.token = token
360 if bridge_all:
361 self._create_acquired_bridges(
362 bridge_stp=bridge_stp, bridge_fd=bridge_fd)
363@@ -3095,8 +3100,7 @@ class Node(CleanSave, TimestampedModel):
364 finalize_release = True
365
366 self.status = NODE_STATUS.RELEASING
367- self.token = None
368- self.agent_name = ''
369+ self.agent_name = ""
370 self.set_netboot()
371 self.set_ephemeral_deploy()
372 self.osystem = ''
373diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
374index 3dd51f0..b8b54c6 100644
375--- a/src/maasserver/models/tests/test_node.py
376+++ b/src/maasserver/models/tests/test_node.py
377@@ -1,4 +1,4 @@
378-# Copyright 2012-2019 Canonical Ltd. This software is licensed under the
379+# Copyright 2012-2020 Canonical Ltd. This software is licensed under the
380 # GNU Affero General Public License version 3 (see the file LICENSE).
381
382 """Test maasserver models."""
383@@ -119,7 +119,6 @@ from maasserver.models.partitiontable import PARTITION_TABLE_EXTRA_SPACE
384 from maasserver.models.resourcepool import ResourcePool
385 from maasserver.models.signals import power as node_query
386 from maasserver.models.timestampedmodel import now
387-from maasserver.models.user import create_auth_token
388 from maasserver.node_status import (
389 COMMISSIONING_LIKE_STATUSES,
390 get_node_timeout,
391@@ -1245,12 +1244,6 @@ class TestNode(MAASServerTestCase):
392 node = factory.make_Node(bios_boot_method=factory.make_name("boot"))
393 self.assertEqual("pxe", node.get_bios_boot_method())
394
395- def test_add_node_with_token(self):
396- user = factory.make_User()
397- token = create_auth_token(user)
398- node = factory.make_Node(token=token)
399- self.assertEqual(token, node.token)
400-
401 def test_add_physical_interface(self):
402 mac = factory.make_mac_address()
403 node = factory.make_Node()
404@@ -1715,9 +1708,8 @@ class TestNode(MAASServerTestCase):
405 def test_acquire(self):
406 node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
407 user = factory.make_User()
408- token = create_auth_token(user)
409- agent_name = factory.make_name('agent-name')
410- node.acquire(user, token, agent_name)
411+ agent_name = factory.make_name("agent-name")
412+ node.acquire(user, agent_name)
413 self.assertEqual(
414 (user, NODE_STATUS.ALLOCATED, agent_name),
415 (node.owner, node.status, node.agent_name))
416@@ -1725,36 +1717,44 @@ class TestNode(MAASServerTestCase):
417 def test_acquire_calls__create_acquired_filesystems(self):
418 node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
419 user = factory.make_User()
420- token = create_auth_token(user)
421- agent_name = factory.make_name('agent-name')
422+ agent_name = factory.make_name("agent-name")
423 mock_create_acquired_filesystems = self.patch_autospec(
424- node, "_create_acquired_filesystems")
425- node.acquire(user, token, agent_name)
426+ node, "_create_acquired_filesystems"
427+ )
428+ node.acquire(user, agent_name)
429 self.assertThat(mock_create_acquired_filesystems, MockCalledOnceWith())
430
431 def test_acquire_logs_user_request(self):
432 node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
433 user = factory.make_User()
434- token = create_auth_token(user)
435- agent_name = factory.make_name('agent-name')
436- register_event = self.patch(node, '_register_request_event')
437- node.acquire(user, token, agent_name)
438- self.assertThat(register_event, MockCalledOnceWith(
439- user, EVENT_TYPES.REQUEST_NODE_ACQUIRE, action='acquire',
440- comment=None))
441+ agent_name = factory.make_name("agent-name")
442+ register_event = self.patch(node, "_register_request_event")
443+ node.acquire(user, agent_name)
444+ self.assertThat(
445+ register_event,
446+ MockCalledOnceWith(
447+ user,
448+ EVENT_TYPES.REQUEST_NODE_ACQUIRE,
449+ action="acquire",
450+ comment=None,
451+ ),
452+ )
453
454 def test_acquire_calls__create_acquired_bridges(self):
455 node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
456 user = factory.make_User()
457- token = create_auth_token(user)
458- agent_name = factory.make_name('agent-name')
459+ agent_name = factory.make_name("agent-name")
460 mock_create_acquired_bridges = self.patch_autospec(
461 node, "_create_acquired_bridges")
462 bridge_stp = factory.pick_bool()
463 bridge_fd = random.randint(0, 500)
464 node.acquire(
465- user, token, agent_name,
466- bridge_all=True, bridge_stp=bridge_stp, bridge_fd=bridge_fd)
467+ user,
468+ agent_name,
469+ bridge_all=True,
470+ bridge_stp=bridge_stp,
471+ bridge_fd=bridge_fd,
472+ )
473 self.assertThat(
474 mock_create_acquired_bridges,
475 MockCalledOnceWith(bridge_stp=bridge_stp, bridge_fd=bridge_fd))
476@@ -2266,8 +2266,7 @@ class TestNode(MAASServerTestCase):
477 MockCalledOnceWith(node.system_id, NODE_STATUS.RELEASING))
478 self.expectThat(node.status, Equals(NODE_STATUS.RELEASING))
479 self.expectThat(node.owner, Equals(owner))
480- self.expectThat(node.agent_name, Equals(''))
481- self.expectThat(node.token, Is(None))
482+ self.expectThat(node.agent_name, Equals(""))
483 self.expectThat(node.netboot, Is(True))
484 self.expectThat(node.ephemeral_deploy, Is(False))
485 self.expectThat(node.osystem, Equals(''))
486@@ -2306,8 +2305,7 @@ class TestNode(MAASServerTestCase):
487 self.expectThat(Node._set_status_expires, MockNotCalled())
488 self.expectThat(node.status, Equals(NODE_STATUS.RELEASING))
489 self.expectThat(node.owner, Equals(owner))
490- self.expectThat(node.agent_name, Equals(''))
491- self.expectThat(node.token, Is(None))
492+ self.expectThat(node.agent_name, Equals(""))
493 self.expectThat(node.netboot, Is(True))
494 self.expectThat(node.ephemeral_deploy, Is(False))
495 self.expectThat(node.osystem, Equals(''))
496@@ -2338,8 +2336,7 @@ class TestNode(MAASServerTestCase):
497 self.expectThat(Node._set_status_expires, MockNotCalled())
498 self.expectThat(node.status, Equals(NODE_STATUS.READY))
499 self.expectThat(node.owner, Equals(None))
500- self.expectThat(node.agent_name, Equals(''))
501- self.expectThat(node.token, Is(None))
502+ self.expectThat(node.agent_name, Equals(""))
503 self.expectThat(node.netboot, Is(True))
504 self.expectThat(node.ephemeral_deploy, Is(False))
505 self.expectThat(node.osystem, Equals(''))
506diff --git a/src/maasserver/models/tests/test_userprofile.py b/src/maasserver/models/tests/test_userprofile.py
507index 9fa2ba1..1a9bdc3 100644
508--- a/src/maasserver/models/tests/test_userprofile.py
509+++ b/src/maasserver/models/tests/test_userprofile.py
510@@ -1,4 +1,4 @@
511-# Copyright 2012-2016 Canonical Ltd. This software is licensed under the
512+# Copyright 2012-2020 Canonical Ltd. This software is licensed under the
513 # GNU Affero General Public License version 3 (see the file LICENSE).
514
515 """Tests for the UserProfile model."""
516@@ -85,6 +85,14 @@ class UserProfileTest(MAASServerTestCase):
517 self.assertFalse(Consumer.objects.filter(id__in=consumer_ids).exists())
518 self.assertFalse(Token.objects.filter(id__in=token_ids).exists())
519
520+ def test_delete_doesnt_delete_node(self):
521+ user = factory.make_User()
522+ profile = user.userprofile
523+ _, token = profile.create_authorisation_token()
524+ node = factory.make_Node(owner=user, token=token)
525+ profile.delete_authorisation_token(token.key)
526+ self.assertIsNotNone(reload_object(node))
527+
528 def test_delete_deletes_related_filestorage_objects(self):
529 # Deleting a profile deletes the related filestorage objects.
530 profile = factory.make_User().userprofile
531diff --git a/src/maasserver/models/userprofile.py b/src/maasserver/models/userprofile.py
532index e87b484..be6a208 100644
533--- a/src/maasserver/models/userprofile.py
534+++ b/src/maasserver/models/userprofile.py
535@@ -1,4 +1,4 @@
536-# Copyright 2012-2016 Canonical Ltd. This software is licensed under the
537+# Copyright 2012-2020 Canonical Ltd. This software is licensed under the
538 # GNU Affero General Public License version 3 (see the file LICENSE).
539
540 """UserProfile model."""
541@@ -20,6 +20,7 @@ from django.db.models import (
542 from django.shortcuts import get_object_or_404
543 from maasserver import DefaultMeta
544 from maasserver.exceptions import CannotDeleteUserException
545+from maasserver.models import Node
546 from maasserver.models.cleansave import CleanSave
547 from piston3.models import Token
548
549@@ -145,7 +146,11 @@ class UserProfile(CleanSave, Model):
550
551 """
552 token = get_object_or_404(
553- Token, user=self.user, token_type=Token.ACCESS, key=token_key)
554+ Token, user=self.user, token_type=Token.ACCESS, key=token_key,
555+ )
556+ # LP:1870171 - Make sure the key being deleted isn't assoicated with
557+ # any node. In MAAS 2.8+ Node.token has been removed.
558+ Node.objects.filter(token=token).update(token=None)
559 token.consumer.delete()
560 token.delete()
561
562diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
563index a64cf57..60232d7 100644
564--- a/src/maasserver/node_action.py
565+++ b/src/maasserver/node_action.py
566@@ -418,7 +418,7 @@ class Acquire(NodeAction):
567 """See `NodeAction.execute`."""
568 with locks.node_acquire:
569 try:
570- self.node.acquire(self.user, token=None)
571+ self.node.acquire(self.user)
572 except ValidationError as e:
573 raise NodeActionError(e)
574
575@@ -450,7 +450,7 @@ class Deploy(NodeAction):
576 if self.node.owner is None:
577 with locks.node_acquire:
578 try:
579- self.node.acquire(self.user, token=None)
580+ self.node.acquire(self.user)
581 except ValidationError as e:
582 raise NodeActionError(e)
583 if install_kvm:
584diff --git a/src/maasserver/tests/test_node_action.py b/src/maasserver/tests/test_node_action.py
585index 06f80f7..8aa8373 100644
586--- a/src/maasserver/tests/test_node_action.py
587+++ b/src/maasserver/tests/test_node_action.py
588@@ -724,8 +724,7 @@ class TestDeployAction(MAASServerTestCase):
589 Deploy(node, user, request).execute(**extra)
590 self.expectThat(mock_get_curtin_config, MockCalledOnceWith(ANY, node))
591 # Make sure we set bridge_all when we acquire the Node.
592- self.expectThat(
593- mock_node_acquire, MockCalledOnceWith(user, token=None))
594+ self.expectThat(mock_node_acquire, MockCalledOnceWith(user))
595 self.expectThat(mock_node_start, MockCalledOnceWith(user))
596 # Make sure ubuntu/bionic is selected if KVM pod deployment has been
597 # selected.
598diff --git a/src/maasserver/websockets/handlers/tests/test_token.py b/src/maasserver/websockets/handlers/tests/test_token.py
599new file mode 100644
600index 0000000..26e79f2
601--- /dev/null
602+++ b/src/maasserver/websockets/handlers/tests/test_token.py
603@@ -0,0 +1,115 @@
604+# Copyright 2019-2020 Canonical Ltd. This software is licensed under the
605+# GNU Affero General Public License version 3 (see the file LICENSE).
606+
607+"""Tests for `maasserver.websockets.handlers.token`"""
608+
609+__all__ = []
610+
611+from maasserver.models.event import Event
612+from maasserver.models.user import (
613+ create_auth_token,
614+ get_auth_tokens,
615+)
616+from maasserver.testing.factory import factory
617+from maasserver.testing.testcase import MAASServerTestCase
618+from maasserver.utils.orm import (
619+ get_one,
620+ reload_object,
621+)
622+from maasserver.websockets.base import HandlerDoesNotExistError
623+from maasserver.websockets.handlers.token import TokenHandler
624+from provisioningserver.events import AUDIT
625+from testtools.matchers import KeysEqual
626+
627+
628+class TestTokenHandler(MAASServerTestCase):
629+ def dehydrate_token(self, token):
630+ return {
631+ "id": token.id,
632+ "key": token.key,
633+ "secret": token.secret,
634+ "consumer": {
635+ "key": token.consumer.key,
636+ "name": token.consumer.name,
637+ },
638+ }
639+
640+ def test_get(self):
641+ user = factory.make_User()
642+ handler = TokenHandler(user, {}, None)
643+ token = create_auth_token(user)
644+ self.assertEqual(
645+ self.dehydrate_token(token), handler.get({"id": token.id})
646+ )
647+
648+ def test_get_doesnt_work_if_not_owned(self):
649+ user = factory.make_User()
650+ handler = TokenHandler(user, {}, None)
651+ not_owned_token = create_auth_token(factory.make_User())
652+ self.assertRaises(
653+ HandlerDoesNotExistError, handler.get, {"id": not_owned_token.id}
654+ )
655+
656+ def test_list(self):
657+ user = factory.make_User()
658+ handler = TokenHandler(user, {}, None)
659+ create_auth_token(user)
660+ expected_tokens = [
661+ self.dehydrate_token(token) for token in get_auth_tokens(user)
662+ ]
663+ self.assertItemsEqual(expected_tokens, handler.list({}))
664+
665+ def test_create_no_name(self):
666+ user = factory.make_User()
667+ handler = TokenHandler(user, {}, None)
668+ new_token = handler.create({})
669+ self.assertThat(
670+ new_token, KeysEqual("id", "key", "secret", "consumer")
671+ )
672+ event = Event.objects.get(type__level=AUDIT)
673+ self.assertIsNotNone(event)
674+ self.assertEqual(event.description, "Created token.")
675+
676+ def test_create_with_name(self):
677+ user = factory.make_User()
678+ handler = TokenHandler(user, {}, None)
679+ name = factory.make_name("name")
680+ new_token = handler.create({"name": name})
681+ self.assertThat(
682+ new_token, KeysEqual("id", "key", "secret", "consumer")
683+ )
684+ self.assertEqual(name, new_token["consumer"]["name"])
685+ event = Event.objects.get(type__level=AUDIT)
686+ self.assertIsNotNone(event)
687+ self.assertEqual(event.description, "Created token.")
688+
689+ def test_update(self):
690+ user = factory.make_User()
691+ handler = TokenHandler(user, {}, None)
692+ name = factory.make_name("name")
693+ token = create_auth_token(user, name)
694+ new_name = factory.make_name("name")
695+ updated_token = handler.update({"id": token.id, "name": new_name})
696+ self.assertThat(
697+ updated_token, KeysEqual("id", "key", "secret", "consumer")
698+ )
699+ self.assertEqual(new_name, updated_token["consumer"]["name"])
700+ event = Event.objects.get(type__level=AUDIT)
701+ self.assertIsNotNone(event)
702+ self.assertEqual(event.description, "Modified consumer name of token.")
703+
704+ def test_delete(self):
705+ user = factory.make_User()
706+ token = create_auth_token(user)
707+ handler = TokenHandler(user, {}, None)
708+ handler.delete({"id": token.id})
709+ self.assertIsNone(get_one(get_auth_tokens(user).filter(id=token.id)))
710+
711+ def test_delete_doesnt_delete_node(self):
712+ user = factory.make_User()
713+ token = create_auth_token(user)
714+ node = factory.make_Node(owner=user, token=token)
715+ handler = TokenHandler(user, {}, None)
716+ handler.delete({"id": token.id})
717+ self.assertIsNone(get_one(get_auth_tokens(user).filter(id=token.id)))
718+ self.assertIsNotNone(reload_object(node))
719diff --git a/src/maasserver/websockets/handlers/token.py b/src/maasserver/websockets/handlers/token.py
720new file mode 100644
721index 0000000..f450d36
722--- /dev/null
723+++ b/src/maasserver/websockets/handlers/token.py
724@@ -0,0 +1,96 @@
725+# Copyright 2019-2020 Canonical Ltd. This software is licensed under the
726+# GNU Affero General Public License version 3 (see the file LICENSE).
727+
728+"""The Token handler for the WebSocket connection."""
729+
730+__all__ = ["TokenHandler"]
731+
732+from django.http import HttpRequest
733+from maasserver.audit import create_audit_event
734+from maasserver.enum import ENDPOINT
735+from maasserver.models import Node
736+from maasserver.models.user import (
737+ create_auth_token,
738+ get_auth_tokens,
739+)
740+from maasserver.websockets.base import (
741+ Handler,
742+ HandlerDoesNotExistError,
743+)
744+from piston3.models import Token
745+from provisioningserver.events import EVENT_TYPES
746+
747+
748+class TokenHandler(Handler):
749+ class Meta:
750+ queryset = Token.objects.none() # Overridden in `get_querset`.
751+ allowed_methods = ["list", "get", "create", "update", "delete"]
752+ listen_channels = ["token"]
753+
754+ def get_queryset(self, for_list=False):
755+ """Return `QuerySet` for `Token` owned by `user`."""
756+ return get_auth_tokens(self.user)
757+
758+ def get_object(self, params, permission=None):
759+ """Only allow getting keys owned by the user."""
760+ obj = super(TokenHandler, self).get_object(
761+ params, permission=permission
762+ )
763+ if obj.user != self.user:
764+ raise HandlerDoesNotExistError(params[self._meta.pk])
765+ else:
766+ return obj
767+
768+ def full_dehydrate(self, obj, for_list=False):
769+ """Return the representation for the object."""
770+ return {
771+ "id": obj.id,
772+ "key": obj.key,
773+ "secret": obj.secret,
774+ "consumer": {"key": obj.consumer.key, "name": obj.consumer.name},
775+ }
776+
777+ def create(self, params):
778+ """Create a Token."""
779+ token = create_auth_token(self.user, params.get("name"))
780+ request = HttpRequest()
781+ request.user = self.user
782+ create_audit_event(
783+ EVENT_TYPES.AUTHORISATION,
784+ ENDPOINT.UI,
785+ request,
786+ None,
787+ "Created token.",
788+ )
789+ return self.full_dehydrate(token)
790+
791+ def update(self, params):
792+ """Update a Token.
793+
794+ Only the name can be updated.
795+ """
796+ token = self.get_object(params)
797+ name = params.get("name")
798+ if name != token.consumer.name:
799+ token.consumer.name = params.get("name")
800+ token.consumer.save()
801+ request = HttpRequest()
802+ request.user = self.user
803+ create_audit_event(
804+ EVENT_TYPES.AUTHORISATION,
805+ ENDPOINT.UI,
806+ request,
807+ None,
808+ "Modified consumer name of token.",
809+ )
810+ return self.full_dehydrate(token)
811+
812+ def delete(self, params):
813+ """Delete a token."""
814+ # LP:1870171 - Make sure the key being deleted isn't assoicated with
815+ # any node. In MAAS 2.8+ Node.token has been removed.
816+ token = self.get_object(
817+ params, permission=self._meta.delete_permission
818+ )
819+ Node.objects.filter(token=token).update(token=None)
820+ token.delete()

Subscribers

People subscribed via source and target branches