Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-989729 into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #989729 in MAAS: "“Start node” button breaks: no oauth key"
https://bugs.launchpad.net/maas/+bug/989729
For more details, see:
https://code.launchpad.net/~jtv/maas/bug-989729/+merge/103883
“Starting” a node, at least in the UI, now means acquiring and starting it.
But acquiring a node requires an OAuth key, and so this breaks.
Discussed this with Raphaël. We're not sure why we'd need that oauth key. The
Node.token field that the user's oauth token goes into seems to be completely
undocumented, and its name tells us nothing except that it's an authentication
token.
As a solution, then, I'm making the token an optional part of acquisition at
the model level. The API still requires it (and it could be relevant only to
that usage) but the UI will no pass it. The acquire() call also needs to know
the acquiring user, which it previously got from the token; I made that an
explicit parameter.
A separate branch solving this for 1.0 is underway. The two are identical in
principle, but quite different in where they affect the source tree. The two
branches have diverged too far for convenient parallel maintenance.
Jeroen
--
https://code.launchpad.net/~jtv/maas/bug-989729/+merge/103883
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/maas/bug-989729 into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-04-27 08:11:29 +0000
+++ src/maasserver/api.py 2012-04-27 14:14:24 +0000
@@ -591,7 +591,7 @@
request.user, constraints=extract_constraints(request.data))
if node is None:
raise NodesNotAvailable("No matching node is available.")
- node.acquire(get_oauth_token(request))
+ node.acquire(request.user, get_oauth_token(request))
return node
@classmethod
=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py 2012-04-27 08:11:29 +0000
+++ src/maasserver/models/__init__.py 2012-04-27 14:14:24 +0000
@@ -598,11 +598,12 @@
power_type = self.power_type
return power_type
- def acquire(self, token):
- """Mark commissioned node as acquired by the given user's token."""
+ def acquire(self, user, token=None):
+ """Mark commissioned node as acquired by the given user and token."""
assert self.owner is None
+ assert token is None or token.user == user
self.status = NODE_STATUS.ALLOCATED
- self.owner = token.user
+ self.owner = user
self.token = token
self.save()
=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py 2012-04-27 08:25:19 +0000
+++ src/maasserver/node_action.py 2012-04-27 14:14:24 +0000
@@ -183,12 +183,12 @@
return None
def execute(self):
- # Avoid circular imports.
- from maasserver.api import get_oauth_token
+ # The UI does not use OAuth, so there is no token to pass to the
+ # acquire() call.
+ self.node.acquire(self.user, token=None)
# Be sure to acquire before starting, or start_nodes will think
# the node ineligible based on its un-acquired status.
- self.node.acquire(get_oauth_token(self.request))
Node.objects.start_nodes([self.node.system_id], self.user)
return dedent("""\
This node is now allocated to you.
=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py 2012-04-25 12:35:02 +0000
+++ src/maasserver/tests/test_models.py 2012-04-27 14:14:24 +0000
@@ -281,7 +281,7 @@
node = factory.make_node(status=NODE_STATUS.READY)
user = factory.make_user()
token = create_auth_token(user)
- node.acquire(token)
+ node.acquire(user, token)
self.assertEqual(user, node.owner)
self.assertEqual(NODE_STATUS.ALLOCATED, node.status)
=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py 2012-04-27 08:25:19 +0000
+++ src/maasserver/tests/test_node_action.py 2012-04-27 14:14:24 +0000
@@ -15,7 +15,6 @@
from urlparse import urlparse
from django.core.urlresolvers import reverse
-import maasserver.api
from maasserver.enum import (
NODE_PERMISSION,
NODE_STATUS,
@@ -222,8 +221,6 @@
def test_StartNode_acquires_and_starts_node(self):
node = factory.make_node(status=NODE_STATUS.READY)
user = factory.make_user()
- consumer, token = user.get_profile().create_authorisation_token()
- self.patch(maasserver.api, 'get_oauth_token', lambda request: token)
StartNode(node, user).execute()
self.assertEqual(NODE_STATUS.ALLOCATED, node.status)
self.assertEqual(user, node.owner)
=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py 2012-04-27 10:56:37 +0000
+++ src/maasserver/tests/test_views_nodes.py 2012-04-27 14:14:24 +0000
@@ -26,11 +26,11 @@
)
from maasserver.exceptions import NoRabbit
from maasserver.forms import NodeActionForm
-from maasserver.node_action import StartNode
from maasserver.models import (
MACAddress,
Node,
)
+from maasserver.node_action import StartNode
from maasserver.testing import (
get_content_links,
reload_object,
@@ -328,14 +328,6 @@
"This node is now allocated to you.",
'\n'.join(msg.message for msg in response.context['messages']))
- def test_view_node_POST_without_oauth_returns_Unauthorized(self):
- factory.make_sshkey(self.logged_in_user)
- node = factory.make_node(status=NODE_STATUS.READY)
- response = self.client.post(
- reverse('node-view', args=[node.system_id]),
- data={NodeActionForm.input_name: StartNode.display})
- self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
-
class NodeDeleteMacTest(LoggedInTestCase):
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
