launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06269
[Merge] lp:~rvb/maas/maas-oauth into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/maas-oauth into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/maas-oauth/+merge/91863
This branch makes the API use OAuth for authentication.
It also:
- introduces a UserProfile class (see https://docs.djangoproject.com/en/dev/topics/auth/#storing-additional-information-about-users) that we will use for our own user related methods (and fields if any).
- uses Django's signal mechanism to have the UserProfile and the Token/Consumer objects created when a User object gets created.
- adds the token/consumer objects related to admin and test in the sampledata. This way, the users's keys won't change.
- fixes a few tests so that they don't rely on data created in setUp.
--
https://code.launchpad.net/~rvb/maas/maas-oauth/+merge/91863
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-oauth into lp:maas.
=== modified file 'buildout.cfg'
--- buildout.cfg 2012-02-06 16:22:27 +0000
+++ buildout.cfg 2012-02-07 15:54:01 +0000
@@ -38,6 +38,7 @@
django-nose
django-piston
docutils
+ oauth2
oops
oops-datedir-repo
oops-wsgi
=== modified file 'docs/models.rst'
--- docs/models.rst 2012-02-04 22:20:08 +0000
+++ docs/models.rst 2012-02-07 15:54:01 +0000
@@ -27,3 +27,8 @@
:show-inheritance:
:members:
+.. autoclass:: UserProfile
+ :show-inheritance:
+ :members:
+
+
=== modified file 'src/maas/development.py'
--- src/maas/development.py 2012-01-25 14:24:52 +0000
+++ src/maas/development.py 2012-02-07 15:54:01 +0000
@@ -166,6 +166,7 @@
'maastesting',
'debug_toolbar',
'django_nose',
+ 'piston',
)
# A sample logging configuration. The only tangible logging
=== modified file 'src/maas/settings.py'
--- src/maas/settings.py 2012-01-27 14:30:12 +0000
+++ src/maas/settings.py 2012-02-07 15:54:01 +0000
@@ -41,6 +41,8 @@
YUI_VERSION = '3.4.1'
STATIC_LOCAL_SERVE = DEBUG
+AUTH_PROFILE_MODULE = 'maasserver.UserProfile'
+
AUTHENTICATION_BACKENDS = (
'maasserver.models.MaaSAuthorizationBackend',
)
@@ -171,6 +173,7 @@
'django.contrib.admin',
'django_nose',
'maasserver',
+ 'piston',
# Uncomment the next line to enable admin documentation:
# 'django.contrib.admindocs',
)
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-02-06 17:17:47 +0000
+++ src/maasserver/api.py 2012-02-07 15:54:01 +0000
@@ -36,15 +36,15 @@
MACAddress,
Node,
)
-from piston.authentication import HttpBasicAuthentication
+from piston.authentication import OAuthAuthentication
from piston.doc import generate_doc
from piston.handler import BaseHandler
from piston.utils import rc
-class DjangoHttpBasicAuthentication(HttpBasicAuthentication):
+class MaasAPIAuthentication(OAuthAuthentication):
"""A piston authentication class that uses the currently logged-in user
- if there is one, and defaults to piston's HttpBasicAuthentication if not.
+ if there is one, and defaults to piston's OAuthAuthentication if not.
"""
@@ -53,7 +53,10 @@
return request.user
else:
return super(
- DjangoHttpBasicAuthentication, self).is_authenticated(request)
+ MaasAPIAuthentication, self).is_authenticated(request)
+
+ def challenge(self):
+ return rc.FORBIDDEN
def validate_and_save(obj):
=== modified file 'src/maasserver/fixtures/dev_fixture.yaml'
--- src/maasserver/fixtures/dev_fixture.yaml 2012-01-24 17:21:45 +0000
+++ src/maasserver/fixtures/dev_fixture.yaml 2012-02-07 15:54:01 +0000
@@ -132,3 +132,21 @@
username: test
model: auth.user
pk: 6
+- fields: {description: '', key: RfGASHsLT6XwuRunC6, name: Generic consumer, secret: 99cpqUcPxJ8GP4RZdfgRHBnTr7uTcQm5, status: accepted,
+ user: 1}
+ model: piston.consumer
+ pk: 1
+- fields: {description: '', key: JcaW54bSSN66wj5M3d, name: Generic consumer, secret: wVbGPSzZ4YyNrCpST7WYR7Av9yUhZHJF, status: accepted,
+ user: 5}
+ model: piston.consumer
+ pk: 2
+- fields: {callback: null, callback_confirmed: false, consumer: 1, is_approved: true,
+ key: EswkkXrcungmh4wjgN, secret: Z7xkk6LBjfsEDnXpL93ksTRWn9jMBXDg, timestamp: 1328622205,
+ token_type: 2, user: 1, verifier: ''}
+ model: piston.token
+ pk: 1
+- fields: {callback: null, callback_confirmed: false, consumer: 1, is_approved: true,
+ key: e6jDkAJzAP7SZnpkQq, secret: mkWmXM62Xv6p73sF8uZNdu6kpLcdktpp, timestamp: 1328622494,
+ token_type: 2, user: 5, verifier: ''}
+ model: piston.token
+ pk: 2
=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py 2012-02-04 22:24:18 +0000
+++ src/maasserver/models.py 2012-02-07 15:54:01 +0000
@@ -25,8 +25,13 @@
from django.contrib.auth.models import User
from django.core.exceptions import PermissionDenied
from django.db import models
+from django.db.models.signals import post_save
from django.shortcuts import get_object_or_404
from maasserver.macaddress import MACAddressField
+from piston.models import (
+ Consumer,
+ Token,
+ )
class CommonInfo(models.Model):
@@ -265,9 +270,80 @@
return self.mac_address
+GENERIC_CONSUMER = 'Generic consumer'
+
+
+class UserProfile(models.Model):
+ """A custom UserProfile_
+
+ :ivar user: The related User_.
+
+ .. _UserProfile: https://docs.djangoproject.com/
+ en/dev/topics/auth/
+ #storing-additional-information-about-users
+
+ """
+
+ user = models.OneToOneField(User)
+
+ def reset_authorisation_token(self):
+ """Reset the key and the secret key of the OAuth authorisation token
+ attached to the related User_.
+
+ :return: The resetted Token.
+ :rtype: piston.models.Token
+
+ """
+ consumer, _ = Consumer.objects.get_or_create(
+ user=self.user, name=GENERIC_CONSUMER,
+ defaults={'status': 'accepted'})
+ consumer.generate_random_codes()
+ token, _ = Token.objects.get_or_create(
+ user=self.user, token_type=Token.ACCESS, consumer=consumer,
+ defaults={'is_approved': True})
+ token.generate_random_codes()
+ return token
+
+ def get_authorisation_consumer(self):
+ """Returns the OAuth Consumer attached to the related User_.
+
+ :return: The attached Consumer.
+ :rtype: piston.models.Consumer
+
+ """
+ return Consumer.objects.get(user=self.user, name=GENERIC_CONSUMER)
+
+ def get_authorisation_token(self):
+ """Returns the OAuth authorisation token attached to the related User_.
+
+ :return: The attached Token.
+ :rtype: piston.models.Token
+
+ """
+ consumer = self.get_authorisation_consumer()
+ token = Token.objects.get(
+ user=self.user, token_type=Token.ACCESS, consumer=consumer)
+ return token
+
+
+# When a user is created: create the related profile and the default
+# consumer/token.
+def create_user(sender, instance, created, **kwargs):
+ if created:
+ # Create related UserProfile.
+ profile = UserProfile.objects.create(user=instance)
+
+ # Create initial authorisation token.
+ profile.reset_authorisation_token()
+
+
+post_save.connect(create_user, sender=User)
+
+
# Register the models in the admin site.
admin.site.register(Node)
admin.site.register(MACAddress)
+admin.site.register(Consumer)
class MaaSAuthorizationBackend(ModelBackend):
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-02-06 11:13:25 +0000
+++ src/maasserver/tests/test_api.py 2012-02-07 15:54:01 +0000
@@ -11,9 +11,9 @@
__metaclass__ = type
__all__ = []
-import base64
import httplib
import json
+import time
from django.test.client import Client
from maasserver.models import (
@@ -26,6 +26,7 @@
TestCase,
)
from maasserver.testing.factory import factory
+import oauth2 as oauth
class NodeAnonAPITest(TestCase):
@@ -43,29 +44,35 @@
self.assertEqual(httplib.OK, response.status_code)
-class AuthenticatedClient(Client):
+class OAuthAuthenticatedClient(Client):
+ def __init__(self, user):
+ super(OAuthAuthenticatedClient, self).__init__()
+ self.consumer = user.get_profile().get_authorisation_consumer()
+ self.token = user.get_profile().get_authorisation_token()
- def __init__(self, auth):
- super(AuthenticatedClient, self).__init__()
- self.extra = {
- 'HTTP_AUTHORIZATION': auth,
+ def get_extra(self, path):
+ params = {
+ 'oauth_version': "1.0",
+ 'oauth_nonce': oauth.generate_nonce(),
+ 'oauth_timestamp': int(time.time()),
+ 'oauth_token': self.token.key,
+ 'oauth_consumer_key': self.consumer.key,
}
-
- def get(self, *args, **kwargs):
- kwargs.update(self.extra)
- return super(AuthenticatedClient, self).get(*args, **kwargs)
-
- def put(self, *args, **kwargs):
- kwargs.update(self.extra)
- return super(AuthenticatedClient, self).put(*args, **kwargs)
-
- def post(self, *args, **kwargs):
- kwargs.update(self.extra)
- return super(AuthenticatedClient, self).post(*args, **kwargs)
-
- def delete(self, *args, **kwargs):
- kwargs.update(self.extra)
- return super(AuthenticatedClient, self).delete(*args, **kwargs)
+ req = oauth.Request(method="GET", url=path, parameters=params)
+ signature_method = oauth.SignatureMethod_PLAINTEXT()
+ req.sign_request(signature_method, self.consumer, self.token)
+ return req.to_header()
+
+ def request(self, **kwargs):
+ # Get test url.
+ environ = self._base_environ()
+ url = '%s://%s' % (environ['wsgi.url_scheme'], kwargs['PATH_INFO'])
+ # Addi OAuth authorization information to the request.
+ extra = self.get_extra(url)
+ extra['HTTP_AUTHORIZATION'] = extra['Authorization']
+ kwargs.update(extra)
+
+ return super(OAuthAuthenticatedClient, self).request(**kwargs)
class APITestMixin(TestCase):
@@ -74,20 +81,7 @@
super(APITestMixin, self).setUp()
self.logged_in_user = factory.make_user(
username='test', password='test')
- auth = '%s:%s' % ('test', 'test')
- auth = 'Basic %s' % base64.encodestring(auth)
- auth = auth.strip()
- self.extra = {
- 'HTTP_AUTHORIZATION': auth,
- }
- self.client = AuthenticatedClient(auth)
- self.other_user = factory.make_user()
- self.other_node = factory.make_node(
- status=NODE_STATUS.DEPLOYED, owner=self.other_user)
-
- self.other_user = factory.make_user()
- self.other_node = factory.make_node(
- status=NODE_STATUS.DEPLOYED, owner=self.other_user)
+ self.client = OAuthAuthenticatedClient(self.logged_in_user)
class NodeAPILoggedInTest(LoggedInTestCase):
@@ -132,8 +126,10 @@
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.
- response = self.client.get(
- '/api/nodes/%s/' % self.other_node.system_id)
+ other_node = factory.make_node(
+ status=NODE_STATUS.DEPLOYED, owner=factory.make_user())
+
+ response = self.client.get('/api/nodes/%s/' % other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
@@ -256,8 +252,10 @@
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.
- response = self.client.put(
- '/api/nodes/%s/' % self.other_node.system_id)
+ other_node = factory.make_node(
+ status=NODE_STATUS.DEPLOYED, owner=factory.make_user())
+
+ response = self.client.put('/api/nodes/%s/' % other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
@@ -271,8 +269,10 @@
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.
- response = self.client.delete(
- '/api/nodes/%s/' % self.other_node.system_id)
+ other_node = factory.make_node(
+ status=NODE_STATUS.DEPLOYED, owner=factory.make_user())
+
+ response = self.client.delete('/api/nodes/%s/' % other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
@@ -317,8 +317,10 @@
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.
+ other_node = factory.make_node(
+ status=NODE_STATUS.DEPLOYED, owner=factory.make_user())
response = self.client.get(
- '/api/nodes/%s/macs/' % self.other_node.system_id)
+ '/api/nodes/%s/macs/' % other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
@@ -340,8 +342,10 @@
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.
+ other_node = factory.make_node(
+ status=NODE_STATUS.DEPLOYED, owner=factory.make_user())
response = self.client.get(
- '/api/nodes/%s/macs/0-aa-22-cc-44-dd/' % self.other_node.system_id)
+ '/api/nodes/%s/macs/0-aa-22-cc-44-dd/' % other_node.system_id)
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
@@ -396,9 +400,11 @@
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.
+ other_node = factory.make_node(
+ status=NODE_STATUS.DEPLOYED, owner=factory.make_user())
response = self.client.delete(
'/api/nodes/%s/macs/%s/' % (
- self.other_node.system_id, self.mac1.mac_address))
+ other_node.system_id, self.mac1.mac_address))
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py 2012-01-25 14:16:04 +0000
+++ src/maasserver/tests/test_models.py 2012-02-07 15:54:01 +0000
@@ -16,12 +16,19 @@
ValidationError,
)
from maasserver.models import (
+ GENERIC_CONSUMER,
MACAddress,
Node,
NODE_STATUS,
+ UserProfile,
)
from maasserver.testing.factory import factory
from maastesting import TestCase
+from piston.models import (
+ Consumer,
+ KEY_SIZE,
+ Token,
+ )
class NodeTest(TestCase):
@@ -120,5 +127,32 @@
self.assertEqual([mac], list(MACAddress.objects.all()))
def test_invalid_address_raises_validation_error(self):
- mac = self.make_MAC('AA:BB:CCXDD:EE:FF')
+ mac = self.make_MAC('aa:bb:ccxdd:ee:ff')
self.assertRaises(ValidationError, mac.full_clean)
+
+
+class UserProfileTest(TestCase):
+
+ def test_profile_creation(self):
+ # A profile is created each time a user is created.
+ user = factory.make_user()
+ self.assertTrue(isinstance(user.get_profile(), UserProfile))
+ self.assertEqual(user, user.get_profile().user)
+
+ def test_consumer_creation(self):
+ # A generic consumer is created each time a user is created.
+ user = factory.make_user()
+ consumers = Consumer.objects.filter(user=user, name=GENERIC_CONSUMER)
+ self.assertEqual([user], [consumer.user for consumer in consumers])
+ self.assertEqual(GENERIC_CONSUMER, consumers[0].name)
+ self.assertTrue(isinstance(consumers[0].key, unicode))
+ self.assertEqual(KEY_SIZE, len(consumers[0].key))
+
+ def test_token_creation(self):
+ # A token is created each time a user is created.
+ user = factory.make_user()
+ tokens = Token.objects.filter(user=user)
+ self.assertEqual([user], [token.user for token in tokens])
+ self.assertTrue(isinstance(tokens[0].key, unicode))
+ self.assertEqual(KEY_SIZE, len(tokens[0].key))
+ self.assertEqual(Token.ACCESS, tokens[0].token_type)
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-02-05 14:56:29 +0000
+++ src/maasserver/urls.py 2012-02-07 15:54:01 +0000
@@ -22,7 +22,7 @@
)
from maasserver.api import (
api_doc,
- DjangoHttpBasicAuthentication,
+ MaasAPIAuthentication,
NodeHandler,
NodeMacHandler,
NodeMacsHandler,
@@ -61,7 +61,7 @@
)
# API.
-auth = DjangoHttpBasicAuthentication(realm="MaaS API")
+auth = MaasAPIAuthentication(realm="MaaS API")
node_handler = Resource(NodeHandler, authentication=auth)
nodes_handler = Resource(NodesHandler, authentication=auth)
=== modified file 'versions.cfg'
--- versions.cfg 2012-02-06 17:37:55 +0000
+++ versions.cfg 2012-02-07 15:54:01 +0000
@@ -10,6 +10,7 @@
ipython =
nose =
nose-subunit =
+oauth2 =
oops =
oops-datedir-repo =
oops-twisted =
Follow ups
