Merge lp:~jtv/maas/api-add-user into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Allow creation of user accounts through the API.

Scott says his life will be easier with this capability. It's mainly for field-testing and such, so I did not bother with a secure way of exchanging the password. Instead, the name clearly indicates that it's not secure.

Jeroen

[Scott Moser (smoser)]

seems probably sufficent.
i'd save a step if i could specify ssh keys to that, but other than that that is fine.

this magically gets callable by the maas-cli?

[Julian Edwards (julian-edwards)]

> + production, unless you are confident that you can eavesdroppers from

Missing "prevent" ... ?

46 + if is_admin:
47 + User.objects.create_superuser(
48 + username=username, password=password, email=email)
49 + else:
50 + User.objects.create_user(
51 + username=username, password=password, email=email)
52 + return rc.ALL_OK

Do the create_* methods always succeed? Surely this needs some error checking? Or do they raise an exception?

Everything else looks great.

review: Approve

[Julian Edwards (julian-edwards)]

On Wednesday 23 Oct 2013 21:49:45 you wrote:
> this magically gets callable by the maas-cli?

Yes.

[Raphaël Badin (rvb)]

I'm a bit surprised that you decided to put this on the MAAS handler. I think it really deserves both a UserHandler and a UsersHandler. Right now, the UI has functions (to manage users) not available through the API and we probably should fix that.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2013-10-18 09:32:57 +0000
+++ src/maasserver/api.py	2013-10-24 03:23:13 +0000
@@ -105,6 +105,7 @@
 import bson
 from celery.app import app_or_default
 from django.conf import settings
+from django.contrib.auth.models import User
 from django.core.exceptions import (
     PermissionDenied,
     ValidationError,
@@ -129,6 +130,7 @@
     OperationsHandler,
     )
 from maasserver.api_utils import (
+    extract_bool,
     extract_oauth_key,
     get_list_from_dict_or_multidict,
     get_mandatory_param,
@@ -1967,6 +1969,36 @@
     # about the available configuration items.
     get_config.__doc__ %= get_config_doc(indentation=8)
 
+    @operation(idempotent=False)
+    def unsafe_create_user(self, request):
+        """Create a MAAS user account.
+
+        This is not safe: the password is sent in plaintext.  Avoid it for
+        production, unless you are confident that you can prevent eavesdroppers
+        from observing the request.
+
+        :param username: Identifier-style username for the new user.
+        :type username: unicode
+        :param email: Email address for the new user.
+        :type email: unicode
+        :param password: Password for the new user.
+        :type password: unicode
+        :param is_admin: Whether the new user is to be an administrator.
+        :type is_admin: bool ('0' for False, '1' for True)
+        """
+        username = get_mandatory_param(request.data, 'username')
+        email = get_mandatory_param(request.data, 'email')
+        password = get_mandatory_param(request.data, 'password')
+        is_admin = extract_bool(get_mandatory_param(request.data, 'is_admin'))
+
+        if is_admin:
+            User.objects.create_superuser(
+                username=username, password=password, email=email)
+        else:
+            User.objects.create_user(
+                username=username, password=password, email=email)
+        return rc.ALL_OK
+
     @classmethod
     def resource_uri(cls, *args, **kwargs):
         return ('maas_handler', [])
 
=== modified file 'src/maasserver/api_utils.py'
--- src/maasserver/api_utils.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/api_utils.py	2013-10-24 03:23:13 +0000
@@ -13,6 +13,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'extract_bool',
     'extract_oauth_key',
     'get_list_from_dict_or_multidict',
     'get_mandatory_param',
@@ -28,6 +29,28 @@
 from piston.models import Token
 
 
+def extract_bool(value):
+    """Extract a boolean from an API request argument.
+
+    Boolean arguments to API requests are passed as string values: "0" for
+    False, or "1" for True.  This helper converts the string value to the
+    native Boolean value.
+
+    :param value: Value of a request parameter.
+    :type value: unicode
+    :return: Boolean value encoded in `value`.
+    :rtype bool:
+    :raise ValueError: If `value` is not an accepted encoding of a boolean.
+    """
+    assert isinstance(value, unicode)
+    if value == '0':
+        return False
+    elif value == '1':
+        return True
+    else:
+        raise ValueError("Not a valid Boolean value (0 or 1): '%s'" % value)
+
+
 def get_mandatory_param(data, key, validator=None):
     """Get the parameter from the provided data dict or raise a ValidationError
     if this parameter is not present.
 
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2013-10-18 16:35:07 +0000
+++ src/maasserver/tests/test_api.py	2013-10-24 03:23:13 +0000
@@ -21,6 +21,7 @@
 
 from apiclient.maas_client import MAASClient
 from django.conf import settings
+from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from fixtures import EnvironmentVariableFixture
 from maasserver import api
@@ -441,6 +442,61 @@
             ),
             (response.status_code, json.loads(response.content)))
 
+    def test_unsafe_create_user_creates_user(self):
+        self.become_admin()
+        username = factory.make_name('user')
+        email = factory.getRandomEmail()
+        password = factory.getRandomString()
+
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {
+                'op': 'unsafe_create_user',
+                'username': username,
+                'email': email,
+                'password': password,
+                'is_admin': '0',
+            })
+        self.assertEqual(httplib.OK, response.status_code, response.content)
+
+        created_user = User.objects.get(username=username)
+        self.assertEqual(email, created_user.email)
+        self.assertEqual(False, created_user.is_superuser)
+
+    def test_unsafe_create_user_creates_admin(self):
+        self.become_admin()
+        username = factory.make_name('user')
+        email = factory.getRandomEmail()
+        password = factory.getRandomString()
+
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {
+                'op': 'unsafe_create_user',
+                'username': username,
+                'email': email,
+                'password': password,
+                'is_admin': '1',
+            })
+        self.assertEqual(httplib.OK, response.status_code, response.content)
+
+        created_user = User.objects.get(username=username)
+        self.assertEqual(email, created_user.email)
+        self.assertEqual(True, created_user.is_superuser)
+
+    def test_unsafe_create_user_requires_admin(self):
+        response = self.client.post(
+            self.get_uri('maas/'),
+            {
+                'op': 'unsafe_create_user',
+                'username': factory.make_name('user'),
+                'email': factory.getRandomEmail(),
+                'password': factory.getRandomString(),
+                'is_admin': '1' if factory.getRandomBoolean() else '0',
+            })
+        self.assertEqual(
+            httplib.FORBIDDEN, response.status_code, response.content)
+
 
 class APIErrorsTest(APIv10TestMixin, TransactionTestCase):
 
 
=== modified file 'src/maasserver/tests/test_api_utils.py'
--- src/maasserver/tests/test_api_utils.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/tests/test_api_utils.py	2013-10-24 03:23:13 +0000
@@ -18,6 +18,7 @@
 
 from django.http import QueryDict
 from maasserver.api_utils import (
+    extract_bool,
     extract_oauth_key,
     extract_oauth_key_from_auth_header,
     get_oauth_token,
@@ -28,6 +29,31 @@
 from maasserver.testing.testcase import MAASServerTestCase
 
 
+class TestExtractBool(MAASServerTestCase):
+    def test_asserts_against_raw_bytes(self):
+        self.assertRaises(AssertionError, extract_bool, b'0')
+
+    def test_asserts_against_None(self):
+        self.assertRaises(AssertionError, extract_bool, None)
+
+    def test_asserts_against_number(self):
+        self.assertRaises(AssertionError, extract_bool, 0)
+
+    def test_0_means_False(self):
+        self.assertEquals(extract_bool('0'), False)
+
+    def test_1_means_True(self):
+        self.assertEquals(extract_bool('1'), True)
+
+    def test_rejects_other_numeric_strings(self):
+        self.assertRaises(ValueError, extract_bool, '00')
+        self.assertRaises(ValueError, extract_bool, '2')
+        self.assertRaises(ValueError, extract_bool, '-1')
+
+    def test_rejects_empty_string(self):
+        self.assertRaises(ValueError, extract_bool, '')
+
+
 class TestGetOverridedQueryDict(MAASServerTestCase):
 
     def test_returns_QueryDict(self):