Merge lp:~canonical-isd-hackers/canonical-identity-provider/split-oauth into lp:canonical-identity-provider/release

[Łukasz Czyżykowski]

Extracting oauth_backend as separate Django app.

Extracting oauth_backend as separate Django app.

[Ricardo Kirkner (ricardokirkner)]

The changes seem to be alright. No new tests are failing (I get 3 test failures, but those are also failing in trunk, so it's not something purely related to this branch). Good work!

review: Approve

=== modified file 'identityprovider/auth.py'
--- identityprovider/auth.py	2010-09-17 04:23:40 +0000
+++ identityprovider/auth.py	2011-01-11 15:53:23 +0000
@@ -3,8 +3,9 @@
 
 from datetime import datetime
 
-from identityprovider.models import (Account, AccountPassword, EmailAddress,
-    Consumer, Token)
+from oauth_backend.models import Consumer, Token
+
+from identityprovider.models import Account, AccountPassword, EmailAddress
 from identityprovider.models.api import APIUser
 from identityprovider.models.const import EmailStatus
 from identityprovider.utils import validate_launchpad_password
@@ -68,17 +69,18 @@
     """Currently only checks that given consumer and token are in database"""
     try:
         consumer = Consumer.objects.get(
-            account__openid_identifier=oauth_consumer.key)
+            user__username=oauth_consumer.key)
         token = Token.objects.get(token=oauth_token.key)
 
     except (Token.DoesNotExist, Consumer.DoesNotExist):
         return None
     else:
         # only allow authentication if account is active
-        if not consumer.account.is_active:
+        account = Account.objects.get(openid_identifier=consumer.key)
+        if not account.is_active:
             return None
 
         if token.consumer.key == oauth_consumer.key:
-            return consumer.account
+            return account
         else:
             return None

=== modified file 'identityprovider/bin/accounts_merge.py'
--- identityprovider/bin/accounts_merge.py	2010-11-09 14:07:02 +0000
+++ identityprovider/bin/accounts_merge.py	2011-01-11 15:53:23 +0000
@@ -67,7 +67,7 @@
 
     def merge_accounts(self, destination, source):
         from identityprovider.models.const import AccountStatus
-        from identityprovider.models.oauthtoken import Consumer
+        from oauth_backend.models import Consumer
         source.status = AccountStatus.DEACTIVATED
         source.authtoken_set.all().delete()
         source.status_comment = (
@@ -163,9 +163,7 @@
 
     def test_merge_accounts_with_oauth_tokens(self):
         s, d = self.create_account_pair()
-        from identityprovider.models.oauthtoken import (
-            create_oauth_token_for_account)
-        create_oauth_token_for_account(s, "test")
+        s.create_oauth_token("test")
         self.merger.merge_accounts(d, s)
         self.assertEqual(s.oauth_consumer.token_set.count(), 0)
 

=== modified file 'identityprovider/models/__init__.py'
--- identityprovider/models/__init__.py	2010-04-21 15:29:24 +0000
+++ identityprovider/models/__init__.py	2011-01-11 15:53:23 +0000
@@ -8,5 +8,4 @@
 from authtoken import *
 from openidmodels import *
 from team import *
-from oauthtoken import *
 from api import *

=== modified file 'identityprovider/models/account.py'
--- identityprovider/models/account.py	2010-12-20 16:17:59 +0000
+++ identityprovider/models/account.py	2011-01-11 15:53:23 +0000
@@ -10,6 +10,8 @@
 from django.utils.translation import ugettext_lazy as _
 from zope.interface import classImplements
 
+from oauth_backend.models import Consumer
+
 from identityprovider.models.const import (AccountCreationRationale,
                                            AccountStatus, EmailStatus)
 from identityprovider.utils import (encrypt_launchpad_password,
@@ -252,6 +254,24 @@
                     return True
         return False
 
+    def create_oauth_token(self, token_name):
+        user, _ = User.objects.get_or_create(username=self.openid_identifier)
+        try:
+            consumer = user.oauth_consumer
+        except Consumer.DoesNotExist:
+            consumer = Consumer.objects.create(user=user)
+        token = consumer.token_set.create(name=token_name)
+        return token
+
+    def oauth_tokens(self):
+        user, _ = User.objects.get_or_create(username=self.openid_identifier)
+        try:
+            consumer = user.oauth_consumer
+            return consumer.token_set.all()
+        except Consumer.DoesNotExist:
+            return []
+
+
     def save(self, force_insert=False, force_update=False):
         if settings.READ_ONLY_MODE:
             return

=== modified file 'identityprovider/tests/test_auth.py'
--- identityprovider/tests/test_auth.py	2010-05-28 22:03:16 +0000
+++ identityprovider/tests/test_auth.py	2011-01-11 15:53:23 +0000
@@ -1,11 +1,12 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+from django.contrib.auth.models import User
+
+from oauth_backend.models import Consumer
 
 from identityprovider.models.account import Account
 from identityprovider.models.const import AccountStatus, EmailStatus
 from identityprovider.models.emailaddress import EmailAddress
-from identityprovider.models.oauthtoken import (Consumer,
-    create_oauth_token_for_account)
 from identityprovider.tests.utils import BasicAccountTestCase
 from identityprovider.auth import LaunchpadBackend, oauth_authenticate
 
@@ -74,17 +75,18 @@
     def test_authenticate_account_no_password(self):
         account = Account.objects.get_by_email('[e-mail]')
         account.accountpassword.delete()
-        
+
         response = self.backend.authenticate('[e-mail]', 'test')
-        
+
         self.assertTrue(response is None)
         account = Account.objects.get_by_email('[e-mail]')
         self.assertTrue(account is not None)
 
     def test_oauth_authenticate_account_active(self):
         account = Account.objects.get_by_email('[e-mail]')
-        consumer, created = Consumer.objects.get_or_create(account=account)
-        token = create_oauth_token_for_account(account, 'new-token')
+        user, _ = User.objects.get_or_create(username=account.openid_identifier)
+        consumer, created = Consumer.objects.get_or_create(user=user)
+        token = account.create_oauth_token('new-token')
         oauth_token = token.oauth_token()
 
         # make sure the account is active
@@ -97,9 +99,10 @@
     def test_oauth_authenticate_account_inactive(self):
         account = Account.objects.get_by_email('[e-mail]')
         _status = account.status
+        user, _ = User.objects.get_or_create(username=account.openid_identifier)
 
-        consumer, created = Consumer.objects.get_or_create(account=account)
-        token = create_oauth_token_for_account(account, 'new-token')
+        consumer, created = Consumer.objects.get_or_create(user=user)
+        token = account.create_oauth_token('new-token')
         oauth_token = token.oauth_token()
 
         for status, _ in AccountStatus._get_choices():
@@ -123,11 +126,13 @@
 
     def test_oauth_authenticate_stolen_token(self):
         victim_account = Account.objects.get_by_email('[e-mail]')
-        token = create_oauth_token_for_account(victim_account, 'new-token')
+        token = victim_account.create_oauth_token('new-token')
         oauth_token = token.oauth_token()
 
         malicious_account = Account.objects.get_by_email('[e-mail]')
-        consumer, created = Consumer.objects.get_or_create(account=malicious_account)
+        malicious_user, _ = User.objects.get_or_create(
+            username=malicious_account.openid_identifier)
+        consumer, created = Consumer.objects.get_or_create(user=malicious_user)
 
         # make sure the accounts are active
         self.assertTrue(victim_account.status, AccountStatus.ACTIVE)

=== modified file 'identityprovider/tests/test_models_account.py'
--- identityprovider/tests/test_models_account.py	2010-12-20 18:22:53 +0000
+++ identityprovider/tests/test_models_account.py	2011-01-11 15:53:23 +0000
@@ -4,6 +4,9 @@
 from datetime import datetime, timedelta
 from django.conf import settings
 from django.contrib.auth.models import User
+
+from oauth_backend.models import Consumer
+
 from identityprovider.tests.utils import BasicAccountTestCase
 from identityprovider.models.account import (Account, AccountPassword,
     AccountStatus)
@@ -322,3 +325,28 @@
         password = AccountPassword(account=account, password='password')
         self.assertEqual(unicode(password), u'Password for displayname')
 
+
+class CreateOAuthTokenForAccountTestCase(BasicAccountTestCase):
+
+    def setUp(self):
+        self.account = Account.objects.create_account(
+            'test', '[e-mail]', 'password'
+        )
+        self.user = User.objects.create_user(
+            self.account.openid_identifier,
+            '[e-mail]', 'password'
+        )
+
+    def test_when_account_has_associated_consumer(self):
+        consumer, _ = Consumer.objects.get_or_create(user=self.user)
+
+        token = self.account.create_oauth_token('new-token')
+
+        self.assertEquals(token.consumer.id, consumer.id)
+
+    def test_when_account_has_no_associated_consumer(self):
+        Consumer.objects.filter(user=self.user).delete()
+
+        token = self.account.create_oauth_token('new-token')
+
+        self.assertEquals(token.consumer.user.id, self.user.id)

=== modified file 'identityprovider/tests/test_views_account.py'
--- identityprovider/tests/test_views_account.py	2010-07-12 22:49:50 +0000
+++ identityprovider/tests/test_views_account.py	2011-01-11 15:53:23 +0000
@@ -5,9 +5,9 @@
 
 from django.core import mail
 
+from oauth_backend.models import Token
+
 from identityprovider.models import Account, EmailAddress
-from identityprovider.models.oauthtoken import (create_oauth_token_for_account,
-                                                Token)
 from identityprovider.models.const import AccountStatus, EmailStatus
 from identityprovider.utils import validate_launchpad_password
 
@@ -199,8 +199,8 @@
 
     def test_revoking_token_removes_it_from_being_displayed(self):
         account = Account.objects.get_by_email("[e-mail]")
-        token_1 = create_oauth_token_for_account(account, "Token-1")
-        token_2 = create_oauth_token_for_account(account, "Token-2")
+        token_1 = account.create_oauth_token("Token-1")
+        token_2 = account.create_oauth_token("Token-2")
 
         r = self.client.get('/+applications')
 
@@ -217,7 +217,7 @@
 
     def test_revoking_token_which_does_not_belogs_to_an_account(self):
         account = Account.objects.get_by_email("[e-mail]")
-        token = create_oauth_token_for_account(account, "Token")
+        token = account.create_oauth_token("Token")
 
         r = self.client.post('/+applications', {'token_id': token.token})
 

=== modified file 'identityprovider/views/account.py'
--- identityprovider/views/account.py	2010-09-18 05:38:59 +0000
+++ identityprovider/views/account.py	2011-01-11 15:53:23 +0000
@@ -2,11 +2,11 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from openid.yadis.constants import YADIS_HEADER_NAME
-import urllib
 
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
-from django.http import HttpResponse, HttpResponseRedirect
+from django.contrib.auth.models import User
+from django.http import HttpResponseRedirect
 from django.shortcuts import render_to_response, get_object_or_404
 from django.template import RequestContext
 from django.utils.translation import ugettext as _
@@ -15,7 +15,7 @@
 from identityprovider.forms import EditAccountForm, LoginForm, NewEmailForm
 from identityprovider.models import (send_validation_email_request,
     EmailAddress)
-from identityprovider.models.oauthtoken import Consumer, Token
+from oauth_backend.models import Consumer, Token
 from identityprovider.models.const import EmailStatus
 from identityprovider.views.utils import (redirection_url_for_token,
     set_session_token_info)
@@ -201,18 +201,15 @@
         token_id = request.POST.get('token_id')
         if token_id:
             try:
-                token = request.user.oauth_consumer.token_set.get(pk=token_id)
+                token = request.user.oauth_tokens().get(pk=token_id)
                 message = _("'%s' token was revoked successfully") % token.name
                 token.delete()
                 request.session['message'] = message
-            except (Consumer.DoesNotExist, Token.DoesNotExist):
+            except (AttributeError, Token.DoesNotExist):
                 pass
         return HttpResponseRedirect('/+applications')
     else:
-        try:
-            tokens = request.user.oauth_consumer.token_set.all()
-        except Consumer.DoesNotExist:
-            tokens = []
+        tokens = request.user.oauth_tokens()
         context = RequestContext(request, {
             'current_section': 'applications',
             'tokens': tokens,

=== modified file 'identityprovider/webservice/models.py'
--- identityprovider/webservice/models.py	2010-12-20 18:20:12 +0000
+++ identityprovider/webservice/models.py	2011-01-11 15:53:23 +0000
@@ -12,12 +12,11 @@
 from lazr.restful.utils import get_current_browser_request
 from lazr.restful.declarations import webservice_error
 
+from oauth_backend.models import Token
 from identityprovider.models import (Account, EmailAddress,
-                 AccountPassword, Token, APIUser, Person)
+                 AccountPassword, APIUser, Person)
 from identityprovider.models.const import (
     AccountCreationRationale, AccountStatus, EmailStatus, LoginTokenType)
-from identityprovider.models.oauthtoken import (
-    create_oauth_token_for_account)
 from identityprovider.models.authtoken import AuthToken, AuthTokenFactory
 from identityprovider.utils import encrypt_launchpad_password
 from identityprovider.utils import password_policy_compliant
@@ -159,7 +158,7 @@
     def authenticate(self, token_name):
         request = get_current_browser_request()
         account = request.environment['authenticated_user']
-        token = create_oauth_token_for_account(account, token_name)
+        token = account.create_oauth_token(token_name)
         application_token_created.send(
             sender=self, openid_identifier=account.openid_identifier)
         return token.serialize()

=== modified file 'identityprovider/wsgi.py'
--- identityprovider/wsgi.py	2010-11-15 16:24:53 +0000
+++ identityprovider/wsgi.py	2011-01-11 15:53:23 +0000
@@ -9,7 +9,7 @@
 from lazr.restful.wsgi import WSGIApplication
 from lazr.authentication.wsgi import BasicAuthMiddleware, OAuthMiddleware
 from identityprovider.auth import basic_authenticate, oauth_authenticate
-from identityprovider.models.oauthtoken import DataStore
+from oauth_backend.models import DataStore
 
 
 

=== added directory 'oauth_backend'
=== added file 'oauth_backend/__init__.py'
=== renamed file 'identityprovider/models/oauthtoken.py' => 'oauth_backend/models.py'
--- identityprovider/models/oauthtoken.py	2010-04-21 15:29:24 +0000
+++ oauth_backend/models.py	2011-01-11 15:53:23 +0000
@@ -10,13 +10,21 @@
 
 from functools import partial
 from django.db import models
-
-from identityprovider.models import Account
-
-
-TOKEN_LENGTH = 50
-TOKEN_SECRET_LENGTH = 50
-CONSUMER_SECRET_LENGTH = 30
+from django.conf import settings
+from django.contrib.auth.models import User
+
+
+__all__ = [
+    'Token',
+    'Consumer',
+    'Nonce',
+    'DataStore',
+]
+
+
+TOKEN_LENGTH = getattr(settings, 'OAUTH_TOKEN_LENGTH', 50)
+TOKEN_SECRET_LENGTH = getattr(settings, 'OAUTH_TOKEN_SECRET_LENGTH', 50)
+CONSUMER_SECRET_LENGTH = getattr(settings, 'OAUTH_CONSUMER_SECRET_LENGTH', 30)
 
 
 def _set_seed():
@@ -68,16 +76,15 @@
         return self.token
 
     class Meta:
-        app_label = 'identityprovider'
         db_table = 'oauth_token'
 
 
 class Consumer(models.Model):
-    account = models.OneToOneField(Account, related_name='oauth_consumer')
+    user = models.OneToOneField(User, related_name='oauth_consumer')
 
     @property
     def key(self):
-        return self.account.openid_identifier
+        return self.user.username
 
     secret = models.CharField(
         max_length=255,
@@ -97,7 +104,6 @@
         return OAuthConsumer(self.key, self.secret)
 
     class Meta:
-        app_label = 'identityprovider'
         db_table = 'oauth_consumer'
 
 
@@ -115,12 +121,11 @@
         Create new nonce object linked to given consumer and token
         """
         consumer = Consumer.objects.get(
-            account__openid_identifier=consumer_key)
+            user__username=consumer_key)
         token = consumer.token_set.get(token=token_value)
         return consumer.nonce_set.create(token=token, nonce=nonce)
 
     class Meta:
-        app_label = 'identityprovider'
         db_table = 'oauth_nonce'
 
 
@@ -152,7 +157,7 @@
         """
         try:
             consumer = Consumer.objects.get(
-                account__openid_identifier=consumer_key)
+                user__username=consumer_key)
             return consumer.oauth_consumer()
         except Consumer.DoesNotExist:
             return None
@@ -165,7 +170,7 @@
 
         """
         count = Nonce.objects.filter(
-            consumer__account__openid_identifier=consumer.key,
+            consumer__user__username=consumer.key,
             token__token=token.key,
             nonce=nonce).count()
         if count > 0:
@@ -174,12 +179,3 @@
             Nonce.create(consumer.key, token.key, nonce)
             return False
 
-
-def create_oauth_token_for_account(account, token_name):
-    try:
-        consumer = account.oauth_consumer
-    except Consumer.DoesNotExist:
-        consumer = Consumer.objects.create(account=account)
-
-    token = consumer.token_set.create(name=token_name)
-    return token

=== renamed file 'identityprovider/tests/test_models_oauthtoken.py' => 'oauth_backend/tests.py'
--- identityprovider/tests/test_models_oauthtoken.py	2010-05-20 17:01:43 +0000
+++ oauth_backend/tests.py	2011-01-11 15:53:23 +0000
@@ -1,45 +1,27 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-from identityprovider.tests.utils import BasicAccountTestCase
-from identityprovider.models.account import Account
-from identityprovider.models.oauthtoken import (Consumer, DataStore, Nonce,
-    Token, OAuthToken, create_oauth_token_for_account)
-
-
-class ConsumerTestCase(BasicAccountTestCase):
-
-    def test_one_to_one_relationship_with_account(self):
-        account = Account.objects.get_by_email('[e-mail]')
-        consumer, _ = Consumer.objects.get_or_create(account=account)
-
-        self.assertEquals(account.id, consumer.account.id)
-        self.assertEquals(consumer.id, account.oauth_consumer.id)
-
-
-class CreateOAuthTokenForAccountTestCase(BasicAccountTestCase):
-
-    def test_when_account_has_associated_consumer(self):
-        account = Account.objects.get_by_email('[e-mail]')
-        consumer, _ = Consumer.objects.get_or_create(account=account)
-
-        token = create_oauth_token_for_account(account, 'new-token')
-
-        self.assertEquals(token.consumer.id, consumer.id)
-
-    def test_when_account_has_no_associated_consumer(self):
-        account = Account.objects.get_by_email('[e-mail]')
-        Consumer.objects.filter(account=account).delete()
-
-        token = create_oauth_token_for_account(account, 'new-token')
-
-        self.assertEquals(token.consumer.account.id, account.id)
-
-
-class TokenTestCase(BasicAccountTestCase):
+from django.test import TestCase
+from django.contrib.auth.models import User
+
+from .models import Consumer, DataStore, Nonce, Token, OAuthToken
+
+
+class BaseTestCase(TestCase):
+
+    @property
+    def user(self):
+        if not hasattr(self, '_user'):
+            self._user = User.objects.create_user(
+                'test_username', '[e-mail]', 'password'
+            )
+        return self._user
+
+
+class TokenTestCase(BaseTestCase):
+
     def setUp(self):
-        self.account = Account.objects.get_by_email('[e-mail]')
-        self.consumer, _ = Consumer.objects.get_or_create(account=self.account)
+        self.consumer, _ = Consumer.objects.get_or_create(user=self.user)
         self.token = Token(consumer=self.consumer, token='token',
                            token_secret='secret', name='name')
     def test_unicode(self):
@@ -54,18 +36,25 @@
         self.assertEqual(self.token.serialize(), expected)
 
 
-class ConsumerTestCase(BasicAccountTestCase):
+class ConsumerTestCase(BaseTestCase):
+
     def test_unicode(self):
-        account = Account.objects.get_by_email('[e-mail]')
-        consumer, _ = Consumer.objects.get_or_create(account=account)
-        self.assertEqual(unicode(consumer), unicode(account.openid_identifier))
-
-
-class DataStoreTestCase(BasicAccountTestCase):
+        consumer, _ = Consumer.objects.get_or_create(user=self.user)
+        self.assertEqual(unicode(consumer),
+                         unicode(self.user.username))
+
+    def test_one_to_one_relationship_with_user(self):
+        consumer, _ = Consumer.objects.get_or_create(user=self.user)
+
+        self.assertEquals(self.user.id, consumer.user.id)
+        self.assertEquals(consumer.id, self.user.oauth_consumer.id)
+
+
+class DataStoreTestCase(BaseTestCase):
+
     def setUp(self):
         self.ds = DataStore()
-        account = Account.objects.get_by_email('[e-mail]')
-        self.consumer, _ = Consumer.objects.get_or_create(account=account)
+        self.consumer, _ = Consumer.objects.get_or_create(user=self.user)
         self.token, _ = Token.objects.get_or_create(consumer=self.consumer,
             token='token', token_secret='secret', name='name')
         self.nonce, _ = Nonce.objects.get_or_create(consumer=self.consumer,