Merge ~ltrager/maas:lp1870171_2.6 into maas:2.6
- Git
- lp:~ltrager/maas
- lp1870171_2.6
- Merge into 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) |
||||
Related bugs: |
|
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.
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/src/maasserver/api/account.py b/src/maasserver/api/account.py |
2 | index 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`.""" |
11 | diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py |
12 | index 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, {}) |
75 | diff --git a/src/maasserver/api/tests/test_api.py b/src/maasserver/api/tests/test_api.py |
76 | index 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' |
114 | diff --git a/src/maasserver/api/tests/test_machine.py b/src/maasserver/api/tests/test_machine.py |
115 | index 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') |
202 | diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py |
203 | index 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. |
329 | diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py |
330 | index 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 = '' |
373 | diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py |
374 | index 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('')) |
506 | diff --git a/src/maasserver/models/tests/test_userprofile.py b/src/maasserver/models/tests/test_userprofile.py |
507 | index 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 |
531 | diff --git a/src/maasserver/models/userprofile.py b/src/maasserver/models/userprofile.py |
532 | index 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 | |
562 | diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py |
563 | index 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: |
584 | diff --git a/src/maasserver/tests/test_node_action.py b/src/maasserver/tests/test_node_action.py |
585 | index 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. |
598 | diff --git a/src/maasserver/websockets/handlers/tests/test_token.py b/src/maasserver/websockets/handlers/tests/test_token.py |
599 | new file mode 100644 |
600 | index 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)) |
719 | diff --git a/src/maasserver/websockets/handlers/token.py b/src/maasserver/websockets/handlers/token.py |
720 | new file mode 100644 |
721 | index 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() |
Approved in https:/ /code.launchpad .net/~ltrager/ maas/+git/ maas/+merge/ 381876