launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-bug-941751-api into lp:maas with lp:~rvb/maas/maas-fix-demo as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-bug-941751-api/+merge/95917

This branch introduces a new API handler to get/set config values.  The set_config method is required to enable us to set MaaS' title using javascript (the js side for this is in a follow-up branch).  I figured I could also create the get_config method that will be used when we will move the settings page to pure JS.
-- 
https://code.launchpad.net/~rvb/maas/maas-bug-941751-api/+merge/95917
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-bug-941751-api into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-03-05 10:20:41 +0000
+++ src/maasserver/api.py	2012-03-05 15:35:26 +0000
@@ -23,6 +23,7 @@
 
 from base64 import b64decode
 import httplib
+import json
 import sys
 import types
 
@@ -47,6 +48,7 @@
 from maasserver.fields import validate_mac
 from maasserver.forms import NodeWithMACAddressesForm
 from maasserver.models import (
+    Config,
     FileStorage,
     MACAddress,
     Node,
@@ -82,6 +84,17 @@
             return actor, anonymous
 
 
+class AdminRestrictedResource(RestrictedResource):
+
+    def authenticate(self, request, rm):
+        actor, anonymous = super(
+            AdminRestrictedResource, self).authenticate(request, rm)
+        if anonymous or not request.user.is_superuser:
+            raise PermissionDenied("User is not allowed access to this API.")
+        else:
+            return actor, anonymous
+
+
 def api_exported(operation_name=True, method='POST'):
     def _decorator(func):
         if method not in dispatch_methods:
@@ -198,6 +211,24 @@
     return cls
 
 
+def get_mandatory_param(data, key):
+    """Get the parameter from the provided data dict or raise a ValidationError
+    if this parameter is not present.
+
+    :param data: The data dict (usually request.data or request.GET where
+        request is a django.http.HttpRequest).
+    :param data: dict
+    :param key: The parameter's key.
+    :type key: basestring
+    :return: The value of the parameter.
+    :raises: ValidationError
+    """
+    value = data.get(key, None)
+    if value is None:
+        raise ValidationError("No provided %s!" % key)
+    return value
+
+
 NODE_FIELDS = (
     'system_id', 'hostname', ('macaddress_set', ('mac_address',)),
     'architecture')
@@ -500,13 +531,10 @@
         """Delete an authorisation OAuth token and the related OAuth consumer.
 
         :param token_key: The key of the token to be deleted.
-        :type token_key: str
-
+        :type token_key: basestring
         """
         profile = request.user.get_profile()
-        token_key = request.data.get('token_key', None)
-        if token_key is None:
-            raise ValidationError('No provided token_key!')
+        token_key = get_mandatory_param(request.data, 'token_key')
         profile.delete_authorisation_token(token_key)
         return rc.DELETED
 
@@ -515,6 +543,37 @@
         return ('account_handler', [])
 
 
+@api_operations
+class MaaSHandler(BaseHandler):
+    """Manage the MaaS' itself."""
+    allowed_methods = ('POST', 'GET')
+
+    @api_exported('set_config', method='POST')
+    def set_config(self, request):
+        """Set a config value.
+
+        :param name: The name of the config item to be set.
+        :type name: basestring
+        :param name: The value of the config item to be set.
+        :type value: json object
+        """
+        name = get_mandatory_param(request.data, 'name')
+        value = get_mandatory_param(request.data, 'value')
+        Config.objects.set_config(name, value)
+        return rc.ALL_OK
+
+    @api_exported('get_config', method='GET')
+    def get_config(self, request):
+        """Get a config value.
+
+        :param name: The name of the config item to be retrieved.
+        :type name: basestring
+        """
+        name = get_mandatory_param(request.GET, 'name')
+        value = Config.objects.get_config(name)
+        return HttpResponse(json.dumps(value), content_type='application/json')
+
+
 def generate_api_doc(add_title=False):
     # Fetch all the API Handlers (objects with the class
     # HandlerMetaClass).

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-03-05 10:04:43 +0000
+++ src/maasserver/tests/test_api.py	2012-03-05 15:35:26 +0000
@@ -20,6 +20,7 @@
 from django.conf import settings
 from maasserver.models import (
     ARCHITECTURE,
+    Config,
     MACAddress,
     Node,
     NODE_STATUS,
@@ -927,3 +928,103 @@
         self.assertEqual(httplib.NOT_FOUND, response.status_code)
         self.assertIn('text/plain', response['Content-Type'])
         self.assertEqual("File not found", response.content)
+
+
+class MaaSAPIAnonTest(APIv10TestMixin, TestCase):
+    # The MaaS' handler is not accessible to anon users.
+
+    def test_anon_get_config_forbidden(self):
+        response = self.client.get(
+            self.get_uri('maas/'),
+            {'op': 'get_config'})
+
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+    def test_anon_set_config_forbidden(self):
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {'op': 'set_config'})
+
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+
+class MaaSAPITest(APITestCase, MaaSAPIAnonTest):
+
+    def test_simple_user_get_config_forbidden(self):
+        response = self.client.get(
+            self.get_uri('maas/'),
+            {'op': 'get_config'})
+
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+    def test_simple_user_set_config_forbidden(self):
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {'op': 'set_config'})
+
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+    def test_get_config_requires_name_param(self):
+        self.become_admin()
+        response = self.client.get(
+            self.get_uri('maas/'),
+            {
+                'op': 'get_config',
+            })
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+
+    def test_get_config_returns_config(self):
+        self.become_admin()
+        name = factory.getRandomString()
+        value = factory.getRandomString()
+        Config.objects.set_config(name, value)
+        response = self.client.get(
+            self.get_uri('maas/'),
+            {
+                'op': 'get_config',
+                'name': name,
+            })
+
+        self.assertEqual(httplib.OK, response.status_code)
+        parsed_result = json.loads(response.content)
+        self.assertIn('application/json', response['Content-Type'])
+        self.assertEqual(value, parsed_result)
+
+    def test_set_config_requires_name_param(self):
+        self.become_admin()
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {
+                'op': 'set_config',
+                'value': factory.getRandomString(),
+            })
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+
+    def test_set_config_requires_value_param(self):
+        self.become_admin()
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {
+                'op': 'set_config',
+                'name': factory.getRandomString(),
+            })
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+
+    def test_admin_set_config(self):
+        self.become_admin()
+        name = factory.getRandomString()
+        value = factory.getRandomString()
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {
+                'op': 'set_config',
+                'name': name,
+                'value': value,
+            })
+
+        self.assertEqual(httplib.OK, response.status_code)
+        stored_value = Config.objects.get_config(name)
+        self.assertEqual(stored_value, value)

=== modified file 'src/maasserver/urls_api.py'
--- src/maasserver/urls_api.py	2012-02-24 15:23:10 +0000
+++ src/maasserver/urls_api.py	2012-03-05 15:35:26 +0000
@@ -18,14 +18,18 @@
 from maas.api_auth import api_auth
 from maasserver.api import (
     AccountHandler,
+    AdminRestrictedResource,
     api_doc,
     FilesHandler,
+    MaaSHandler,
     NodeHandler,
     NodeMacHandler,
     NodeMacsHandler,
     NodesHandler,
     RestrictedResource,
     )
+
+
 account_handler = RestrictedResource(AccountHandler, authentication=api_auth)
 files_handler = RestrictedResource(FilesHandler, authentication=api_auth)
 node_handler = RestrictedResource(NodeHandler, authentication=api_auth)
@@ -35,6 +39,10 @@
     NodeMacsHandler, authentication=api_auth)
 
 
+# Admin handlers.
+maas_handler = AdminRestrictedResource(MaaSHandler, authentication=api_auth)
+
+
 # API URLs accessible to anonymous users.
 urlpatterns = patterns('',
     url(r'doc/$', api_doc, name='api-doc'),
@@ -57,3 +65,9 @@
     url(r'files/$', files_handler, name='files_handler'),
     url(r'account/$', account_handler, name='account_handler'),
 )
+
+
+# API URLs for admin users.
+urlpatterns += patterns('',
+    url(r'maas/$', maas_handler, name='maas_handler'),
+)