launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06219
[Merge] lp:~rvb/maas/maas-add-node-piston-auth into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/maas-add-node-piston-auth into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/maas-add-node-piston-auth/+merge/91034
This branch adds a custom piston authentication backend that acts exactly like piston's HttpBasicAuthentication except for the fact that (Django's) logged-in users are automatically authenticated for the API.
Drive-by fixes:
- add a few assertions to test the content type of API responses.
- reformat tests' comments.
--
https://code.launchpad.net/~rvb/maas/maas-add-node-piston-auth/+merge/91034
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-add-node-piston-auth into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-01-31 12:15:08 +0000
+++ src/maasserver/api.py 2012-02-01 09:47:56 +0000
@@ -36,11 +36,26 @@
MACAddress,
Node,
)
+from piston.authentication import HttpBasicAuthentication
from piston.doc import generate_doc
from piston.handler import BaseHandler
from piston.utils import rc
+class DjangoHttpBasicAuthentication(HttpBasicAuthentication):
+ """A piston authentication class that uses the currently logged-in user
+ if there is one, and defaults to piston's HttpBasicAuthentication if not.
+
+ """
+
+ def is_authenticated(self, request):
+ if request.user.is_authenticated():
+ return request.user
+ else:
+ return super(
+ DjangoHttpBasicAuthentication, self).is_authenticated(request)
+
+
def validate_and_save(obj):
try:
obj.full_clean()
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-01-31 18:23:35 +0000
+++ src/maasserver/tests/test_api.py 2012-02-01 09:47:56 +0000
@@ -21,20 +21,23 @@
Node,
NODE_STATUS,
)
-from maasserver.testing import TestCase
+from maasserver.testing import (
+ LoggedInTestCase,
+ TestCase,
+ )
from maasserver.testing.factory import factory
class NodeAnonAPITest(TestCase):
def test_anon_nodes_GET(self):
- """Anonymous requests to the API are denied."""
+ # Anonymous requests to the API are denied.
response = self.client.get('/api/nodes/')
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_anon_api_doc(self):
- """The documentation is accessible to anon users."""
+ # The documentation is accessible to anon users.
response = self.client.get('/api/doc/')
self.assertEqual(httplib.OK, response.status_code)
@@ -87,13 +90,23 @@
status=NODE_STATUS.DEPLOYED, owner=self.other_user)
+class NodeAPILoggedInTest(LoggedInTestCase):
+
+ def test_nodes_GET_logged_in(self):
+ # A (Django) logged-in user can access the API.
+ node = factory.make_node()
+ response = self.client.get('/api/nodes/')
+ parsed_result = json.loads(response.content)
+
+ self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(1, len(parsed_result))
+ self.assertEqual(node.system_id, parsed_result[0]['system_id'])
+
+
class NodeAPITest(APITestMixin):
def test_nodes_GET(self):
- """
- The api allows for fetching the list of Nodes.
-
- """
+ # The api allows for fetching the list of Nodes.
node1 = factory.make_node()
node2 = factory.make_node(
set_hostname=True, status=NODE_STATUS.DEPLOYED,
@@ -107,10 +120,7 @@
self.assertEqual(node2.system_id, parsed_result[1]['system_id'])
def test_node_GET(self):
- """
- The api allows for fetching a single Node (using system_id).
-
- """
+ # The api allows for fetching a single Node (using system_id).
node = factory.make_node(set_hostname=True)
response = self.client.get('/api/nodes/%s/' % node.system_id)
parsed_result = json.loads(response.content)
@@ -120,31 +130,23 @@
self.assertEqual(node.system_id, parsed_result['system_id'])
def test_node_GET_non_visible_node(self):
- """
- The request to fetch a single node is denied if the node isn't visible
- by the user.
-
- """
+ # The request to fetch a single node is denied if the node isn't
+ # visible by the user.
response = self.client.get(
'/api/nodes/%s/' % self.other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_node_GET_not_found(self):
- """
- When fetching a Node, the api returns a 'Not Found' (404) error
- if no node is found.
-
- """
+ # When fetching a Node, the api returns a 'Not Found' (404) error
+ # if no node is found.
response = self.client.get('/api/nodes/invalid-uuid/')
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_nodes_POST(self):
- """
- The API allows a Node to be created and associated with MAC Addresses.
-
- """
+ # The API allows a Node to be created and associated with MAC
+ # Addresses.
response = self.client.post(
'/api/nodes/',
{
@@ -156,6 +158,8 @@
parsed_result = json.loads(response.content)
self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(
+ 'application/json; charset=utf-8', response['Content-Type'])
self.assertEqual('diane', parsed_result['hostname'])
self.assertEqual(41, len(parsed_result.get('system_id')))
self.assertEqual(1, Node.objects.filter(hostname='diane').count())
@@ -166,11 +170,8 @@
[mac.mac_address for mac in node.macaddress_set.all()])
def test_nodes_POST_no_operation(self):
- """
- If there is no operation ('op=operation_name') specified in the
- request data, a 'Bad request' response is returned.
-
- """
+ # If there is no operation ('op=operation_name') specified in the
+ # request data, a 'Bad request' response is returned.
response = self.client.post(
'/api/nodes/',
{
@@ -179,14 +180,13 @@
})
self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+ self.assertEqual(
+ 'text/html; charset=utf-8', response['Content-Type'])
self.assertEqual('Unknown operation.', response.content)
def test_nodes_POST_bad_operation(self):
- """
- If the operation ('op=operation_name') specified in the
- request data is unknown, a 'Bad request' response is returned.
-
- """
+ # If the operation ('op=operation_name') specified in the
+ # request data is unknown, a 'Bad request' response is returned.
response = self.client.post(
'/api/nodes/',
{
@@ -200,11 +200,8 @@
"Unknown operation: 'invalid_operation'.", response.content)
def test_nodes_POST_invalid(self):
- """
- If the data provided to create a node with MAC Addresse is invalid,
- a 'Bad request' response is returned.
-
- """
+ # If the data provided to create a node with MAC Addresse is invalid,
+ # a 'Bad request' response is returned.
response = self.client.post(
'/api/nodes/',
{
@@ -215,16 +212,15 @@
parsed_result = json.loads(response.content)
self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+ self.assertEqual(
+ 'application/json; charset=utf-8', response['Content-Type'])
self.assertEqual(['mac_addresses'], list(parsed_result))
self.assertEqual(
["One or more MAC Addresses is invalid."],
parsed_result['mac_addresses'])
def test_node_PUT(self):
- """
- The api allows to update a Node.
-
- """
+ # The api allows to update a Node.
node = factory.make_node(hostname='diane')
response = self.client.put(
'/api/nodes/%s/' % node.system_id, {'hostname': 'francis'})
@@ -236,11 +232,8 @@
self.assertEqual(1, Node.objects.filter(hostname='francis').count())
def test_node_resource_uri(self):
- """
- When a Node is returned by the API, the field 'resource_uri' provides
- the URI for this Node.
-
- """
+ # When a Node is returned by the API, the field 'resource_uri'
+ # provides the URI for this Node.
node = factory.make_node(hostname='diane')
response = self.client.put(
'/api/nodes/%s/' % node.system_id, {'hostname': 'francis'})
@@ -251,11 +244,8 @@
parsed_result['resource_uri'])
def test_node_PUT_invalid(self):
- """
- If the data provided to update a node is invalid, a 'Bad request'
- response is returned.
-
- """
+ # If the data provided to update a node is invalid, a 'Bad request'
+ # response is returned.
node = factory.make_node(hostname='diane')
response = self.client.put(
'/api/nodes/%s/' % node.system_id,
@@ -264,42 +254,30 @@
self.assertEqual(httplib.BAD_REQUEST, response.status_code)
def test_node_PUT_non_visible_node(self):
- """
- The request to update a single node is denied if the node isn't visible
- by the user.
-
- """
+ # The request to update a single node is denied if the node isn't
+ # visible by the user.
response = self.client.put(
'/api/nodes/%s/' % self.other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_node_PUT_not_found(self):
- """
- When updating a Node, the api returns a 'Not Found' (404) error
- if no node is found.
-
- """
+ # When updating a Node, the api returns a 'Not Found' (404) error
+ # if no node is found.
response = self.client.put('/api/nodes/no-node-here/')
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_node_DELETE_non_visible_node(self):
- """
- The request to delete a single node is denied if the node isn't visible
- by the user.
-
- """
+ # The request to delete a single node is denied if the node isn't
+ # visible by the user.
response = self.client.delete(
'/api/nodes/%s/' % self.other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_node_DELETE(self):
- """
- The api allows to delete a Node.
-
- """
+ # The api allows to delete a Node.
node = factory.make_node(set_hostname=True)
system_id = node.system_id
response = self.client.delete('/api/nodes/%s/' % node.system_id)
@@ -309,11 +287,8 @@
[], list(Node.objects.filter(system_id=system_id)))
def test_node_DELETE_not_found(self):
- """
- When deleting a Node, the api returns a 'Not Found' (404) error
- if no node is found.
-
- """
+ # When deleting a Node, the api returns a 'Not Found' (404) error
+ # if no node is found.
response = self.client.delete('/api/nodes/no-node-here/')
self.assertEqual(httplib.NOT_FOUND, response.status_code)
@@ -328,10 +303,7 @@
self.mac2 = self.node.add_mac_address('22:bb:cc:dd:aa:ff')
def test_macs_GET(self):
- """
- The api allows for fetching the list of the MAC Addresss for a node.
-
- """
+ # The api allows for fetching the list of the MAC Addresss for a node.
response = self.client.get('/api/nodes/%s/macs/' % self.node.system_id)
parsed_result = json.loads(response.content)
@@ -343,64 +315,46 @@
self.mac2.mac_address, parsed_result[1]['mac_address'])
def test_macs_GET_forbidden(self):
- """
- When fetching MAC Addresses, the api returns a 'Unauthorized' (401)
- error if the node is not visible to the logged-in user.
-
- """
+ # When fetching MAC Addresses, the api returns a 'Unauthorized' (401)
+ # error if the node is not visible to the logged-in user.
response = self.client.get(
'/api/nodes/%s/macs/' % self.other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_macs_GET_not_found(self):
- """
- When fetching MAC Addresses, the api returns a 'Not Found' (404)
- error if no node is found.
-
- """
+ # When fetching MAC Addresses, the api returns a 'Not Found' (404)
+ # error if no node is found.
response = self.client.get('/api/nodes/invalid-id/macs/')
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_macs_GET_node_not_found(self):
- """
- When fetching a MAC Address, the api returns a 'Not Found' (404)
- error if the MAC Address does not exist.
-
- """
+ # When fetching a MAC Address, the api returns a 'Not Found' (404)
+ # error if the MAC Address does not exist.
response = self.client.get(
'/api/nodes/%s/macs/00-aa-22-cc-44-dd/' % self.node.system_id)
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_macs_GET_node_forbidden(self):
- """
- When fetching a MAC Address, the api returns a 'Unauthorized' (401)
- error if the node is not visible to the logged-in user.
-
- """
+ # When fetching a MAC Address, the api returns a 'Unauthorized' (401)
+ # error if the node is not visible to the logged-in user.
response = self.client.get(
'/api/nodes/%s/macs/0-aa-22-cc-44-dd/' % self.other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_macs_GET_node_bad_request(self):
- """
- When fetching a MAC Address, the api returns a 'Bad Request' (400)
- error if the MAC Address is not valid.
-
- """
+ # When fetching a MAC Address, the api returns a 'Bad Request' (400)
+ # error if the MAC Address is not valid.
response = self.client.get(
'/api/nodes/%s/macs/invalid-mac/' % self.node.system_id)
self.assertEqual(400, response.status_code)
def test_macs_POST_add_mac(self):
- """
- The api allows to add a MAC Address to an existing node.
-
- """
+ # The api allows to add a MAC Address to an existing node.
nb_macs = MACAddress.objects.filter(node=self.node).count()
response = self.client.post(
'/api/nodes/%s/macs/' % self.node.system_id,
@@ -414,11 +368,8 @@
MACAddress.objects.filter(node=self.node).count())
def test_macs_POST_add_mac_invalid(self):
- """
- A 'Bad Request' response is returned if one tries to add an invalid
- MAC Address to a node.
-
- """
+ # A 'Bad Request' response is returned if one tries to add an invalid
+ # MAC Address to a node.
response = self.client.post(
'/api/nodes/%s/macs/' % self.node.system_id,
{'mac_address': 'invalid-mac'})
@@ -431,10 +382,7 @@
parsed_result['mac_address'])
def test_macs_DELETE_mac(self):
- """
- The api allows to delete a MAC Address.
-
- """
+ # The api allows to delete a MAC Address.
nb_macs = self.node.macaddress_set.count()
response = self.client.delete(
'/api/nodes/%s/macs/%s/' % (
@@ -446,11 +394,8 @@
self.node.macaddress_set.count())
def test_macs_DELETE_mac_forbidden(self):
- """
- When deleting a MAC Address, the api returns a 'Unauthorized' (401)
- error if the node is not visible to the logged-in user.
-
- """
+ # When deleting a MAC Address, the api returns a 'Unauthorized' (401)
+ # error if the node is not visible to the logged-in user.
response = self.client.delete(
'/api/nodes/%s/macs/%s/' % (
self.other_node.system_id, self.mac1.mac_address))
@@ -458,11 +403,8 @@
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_macs_DELETE_not_found(self):
- """
- When deleting a MAC Address, the api returns a 'Not Found' (404)
- error if no existing MAC Address is found.
-
- """
+ # When deleting a MAC Address, the api returns a 'Not Found' (404)
+ # error if no existing MAC Address is found.
response = self.client.delete(
'/api/nodes/%s/macs/%s/' % (
self.node.system_id, '00-aa-22-cc-44-dd'))
@@ -470,11 +412,8 @@
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_macs_DELETE_bad_request(self):
- """
- When deleting a MAC Address, the api returns a 'Bad Request' (400)
- error if the provided MAC Address is not valid.
-
- """
+ # When deleting a MAC Address, the api returns a 'Bad Request' (400)
+ # error if the provided MAC Address is not valid.
response = self.client.delete(
'/api/nodes/%s/macs/%s/' % (
self.node.system_id, 'invalid-mac'))
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-01-31 18:23:35 +0000
+++ src/maasserver/urls.py 2012-02-01 09:47:56 +0000
@@ -22,6 +22,7 @@
)
from maasserver.api import (
api_doc,
+ DjangoHttpBasicAuthentication,
NodeHandler,
NodeMacHandler,
NodeMacsHandler,
@@ -33,7 +34,6 @@
NodeListView,
NodesCreateView,
)
-from piston.authentication import HttpBasicAuthentication
from piston.resource import Resource
# Urls accessible to anonymous users.
@@ -61,7 +61,7 @@
)
# API
-auth = HttpBasicAuthentication(realm="MaaS API")
+auth = DjangoHttpBasicAuthentication(realm="MaaS API")
node_handler = Resource(NodeHandler, authentication=auth)
nodes_handler = Resource(NodesHandler, authentication=auth)
