Merge lp:~matiasb/canonical-identity-provider/phoneid-as-emails into lp:canonical-identity-provider/release

[Matias Bordese]

Added new login API (behind a flag) to allow login with a phone id.

Added new login API (behind a flag) to allow login with a phone id.

[Natalia Bidart (nataliabidart)]

Looks great, thanks!

review: Approve

=== modified file 'api/urls.py'
--- api/urls.py	2013-02-04 11:13:04 +0000
+++ api/urls.py	2013-02-18 16:43:22 +0000
@@ -42,6 +42,7 @@
     handler=v20.AccountsHandler, authentication=ApiOAuthAuthentication())
 v2emails = Resource(handler=v20.EmailsHandler)
 v2login = Resource(handler=v20.AccountLoginHandler)
+v2login_phone = Resource(handler=v20.AccountPhoneLoginHandler)
 v2registration = Resource(handler=v20.AccountRegistrationHandler)
 v2requests = Resource(handler=v20.RequestsHandler)
 
@@ -78,6 +79,8 @@
     url(r'^v2/tokens$', v2login, name='api-login'),
     url(r'^v2/accounts/(\w+)$', v2accounts, name='api-account'),
     url(r'^v2/requests/validate$', v2requests, name='api-requests'),
+    # login from phone, with a phone user id
+    url(r'^v2/tokens/phone$', v2login_phone, name='api-login-phone'),
     # temporarily hooked up so we can do reverse()
     url(r'^v2/emails/(.*)$', v2emails, name='api-email'),
     url(r'v2/tokens/(.*)$', v2login, name='api-token'),
 
=== modified file 'api/v20/handlers.py'
--- api/v20/handlers.py	2013-02-06 15:14:39 +0000
+++ api/v20/handlers.py	2013-02-18 16:43:22 +0000
@@ -20,7 +20,7 @@
     AuthenticationError,
     authenticate_user,
 )
-from identityprovider.models import Account, twofactor
+from identityprovider.models import Account, EmailAddress, twofactor
 from identityprovider.models.captcha import Captcha, VerifyCaptchaError
 from identityprovider.store import SSODataStore
 
@@ -137,6 +137,69 @@
         return response
 
 
+class AccountPhoneLoginHandler(BaseHandler):
+    allowed_methods = ('POST', )
+
+    @require_mime('json')
+    def create(self, request):
+        if not gargoyle.is_active('LOGIN_BY_PHONE'):
+            return errors.RESOURCE_NOT_FOUND()
+
+        data = request.data
+        # email should be available the first time, to bind account/phone
+        # after that, get email from phoneid in the token name
+        email = data.get('email')
+        try:
+            phone_id = data['phone_id']
+            password = data['password']
+            token_name = data['token_name']
+        except KeyError:
+            expected = set(('phone_id', 'password', 'token_name'))
+            missing = dict((k, 'field required') for k in expected - set(data))
+            return errors.INVALID_DATA(**missing)
+
+        try:
+            phone_email = EmailAddress.objects.get_from_phone_id(phone_id)
+            login_email = phone_email.email
+        except EmailAddress.DoesNotExist:
+            phone_email = None
+            login_email = email
+
+        if login_email is None:
+            return errors.INVALID_DATA(phone_id='invalid value')
+
+        try:
+            account = authenticate_user(login_email, password)
+        except AccountSuspended:
+            return errors.ACCOUNT_SUSPENDED()
+        except AccountDeactivated:
+            return errors.ACCOUNT_DEACTIVATED()
+        except AuthenticationError:
+            return errors.INVALID_CREDENTIALS()
+
+        # TODO: here we should check for 2fa, avoiding for demo
+
+        if phone_email is None:
+            # user is authenticated through email,
+            # adding phone id as email for future login
+            EmailAddress.objects.create_from_phone_id(phone_id, account)
+
+        token = account.get_or_create_oauth_token(token_name)
+        response = rc.ALL_OK
+        response.content = {
+            "consumer_key": token.consumer.key,
+            "consumer_secret": token.consumer.secret,
+            "token_key": token.token,
+            "token_secret": token.token_secret,
+            "token_name": token_name,
+            "date_created": token.created_at,
+            "date_updated": token.updated_at,
+            "href": reverse('api-token', args=(token.token,)),
+            "openid": account.openid_identifier,
+        }
+        return response
+
+
 class EmailsHandler(BaseHandler):
     allowed_methods = ('GET',)
 
 
=== added file 'api/v20/tests/test_phone_login.py'
--- api/v20/tests/test_phone_login.py	1970-01-01 00:00:00 +0000
+++ api/v20/tests/test_phone_login.py	2013-02-18 16:43:22 +0000
@@ -0,0 +1,155 @@
+
+from django.core.urlresolvers import reverse
+from gargoyle.testutils import switches
+from mock import patch
+
+from identityprovider.login import (
+    AuthenticationError,
+    AccountDeactivated,
+    AccountSuspended,
+)
+from identityprovider.models.emailaddress import PHONE_EMAIL_DOMAIN
+from identityprovider.tests import DEFAULT_USER_PASSWORD
+from identityprovider.tests.utils import (
+    SSOBaseTestCase,
+)
+
+from api.v20 import handlers
+from api.v20.tests.utils import call
+
+
+handler = handlers.AccountPhoneLoginHandler()
+API_URL = reverse('api-login-phone')
+API_DATA = dict(
+    email='[e-mail]', phone_id='tel:[phone]',
+    password='foobar', token_name='token_name')
+
+
+class PhoneLoginHandlerTestCase(SSOBaseTestCase):
+    fixtures = ['test']
+
+    def test_disabled_by_default(self):
+        response, json_body = call(handler.create, API_URL, API_DATA)
+        self.assertEqual(response.status_code, 404)
+
+    @switches(LOGIN_BY_PHONE=True)
+    def test_login_required_parameters(self):
+        response, json_body = call(handler.create, API_URL, {})
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(json_body, {
+            'code': 'INVALID_DATA',
+            'extra': {
+                'phone_id': 'field required', 'password': 'field required',
+                'token_name': 'field required'
+            },
+            'message': 'Invalid request data'})
+
+    @switches(LOGIN_BY_PHONE=True)
+    def test_account_suspended(self):
+        self.assert_failed_login('ACCOUNT_SUSPENDED', AccountSuspended)
+
+    @switches(LOGIN_BY_PHONE=True)
+    def test_account_deactivated(self):
+        self.assert_failed_login('ACCOUNT_DEACTIVATED', AccountDeactivated)
+
+    @switches(LOGIN_BY_PHONE=True)
+    def test_failed_login(self):
+        self.assert_failed_login('INVALID_CREDENTIALS', AuthenticationError)
+
+    @switches(LOGIN_BY_PHONE=True)
+    def assert_failed_login(self, code, exception):
+        status_code = 403
+        if exception is AuthenticationError:
+            status_code = 401
+
+        with patch('api.v20.handlers.authenticate_user') as mock_authenticate:
+            mock_authenticate.side_effect = exception
+            response, json_body = call(handler.create, API_URL, API_DATA)
+
+        self.assertEqual(response.status_code, status_code)
+        mock_authenticate.assert_called_once_with('[e-mail]', 'foobar')
+        self.assertEqual(json_body['code'], code)
+
+    @switches(LOGIN_BY_PHONE=True)
+    @switches(ALLOW_UNVALIDATED=True)
+    def test_login_add_phone_email(self):
+        account = self.factory.make_account()
+        email = account.preferredemail
+        # no unverified_emails
+        assert len(account.unverified_emails()) == 0
+
+        data = {'phone_id': 'tel:[phone]', 'email': email.email,
+                'password': DEFAULT_USER_PASSWORD, 'token_name': 'token_name'}
+        response, json_body = call(handler.create, API_URL, data)
+
+        self.assertEqual(response.status_code, 200)
+        token = account.get_or_create_oauth_token('token_name')
+        expected_response = {
+            "consumer_key": token.consumer.key,
+            "consumer_secret": token.consumer.secret,
+            "token_key": token.token,
+            "token_secret": token.token_secret,
+            "token_name": 'token_name',
+            "date_created": token.created_at,
+            "href": reverse('api-token', args=(token.token,)),
+            "openid": account.openid_identifier,
+        }
+        # ignore date_updated
+        json_body.pop('date_updated')
+        self.assertEqual(json_body, expected_response)
+        # check added phone email
+        unverified_emails = account.unverified_emails()
+        self.assertEqual(len(unverified_emails), 1)
+        phone_email = unverified_emails[0].email
+        self.assertTrue(phone_email.endswith(PHONE_EMAIL_DOMAIN))
+
+    @switches(LOGIN_BY_PHONE=True)
+    @switches(ALLOW_UNVALIDATED=True)
+    def test_login_without_email_or_added_phone_email(self):
+        account = self.factory.make_account()
+        # no unverified_emails
+        assert len(account.unverified_emails()) == 0
+
+        data = {'phone_id': 'tel:[phone]',
+                'password': DEFAULT_USER_PASSWORD, 'token_name': 'token_name'}
+        response, json_body = call(handler.create, API_URL, data)
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(json_body, {
+            'code': 'INVALID_DATA',
+            'extra': {'phone_id': 'invalid value'},
+            'message': 'Invalid request data'})
+
+    @switches(LOGIN_BY_PHONE=True)
+    @switches(ALLOW_UNVALIDATED=True)
+    def test_login_with_phone_email(self):
+        account = self.factory.make_account()
+        email = account.preferredemail
+        # no unverified_emails
+        assert len(account.unverified_emails()) == 0
+
+        # login first with email and phone id
+        data = {'phone_id': 'tel:[phone]', 'email': email.email,
+                'password': DEFAULT_USER_PASSWORD, 'token_name': 'token_name'}
+        response, json_body = call(handler.create, API_URL, data)
+        assert response.status_code == 200
+
+        # now we should be able to login with phone id only
+        data = {'phone_id': 'tel:[phone]',
+                'password': DEFAULT_USER_PASSWORD, 'token_name': 'token_name'}
+        response, json_body = call(handler.create, API_URL, data)
+        self.assertEqual(response.status_code, 200)
+        token = account.get_or_create_oauth_token('token_name')
+        expected_response = {
+            "consumer_key": token.consumer.key,
+            "consumer_secret": token.consumer.secret,
+            "token_key": token.token,
+            "token_secret": token.token_secret,
+            "token_name": 'token_name',
+            "date_created": token.created_at,
+            "href": reverse('api-token', args=(token.token,)),
+            "openid": account.openid_identifier,
+        }
+        # ignore date_updated
+        json_body.pop('date_updated')
+        self.assertEqual(json_body, expected_response)
 
=== modified file 'identityprovider/login.py'
--- identityprovider/login.py	2013-01-03 11:41:12 +0000
+++ identityprovider/login.py	2013-02-18 16:43:22 +0000
@@ -43,6 +43,7 @@
         else:
             raise AuthenticationError(_("Password didn't match."))
 
+    # also check for LOGIN_BY_PHONE flag conditional to canonical users
     if not gargoyle.is_active('ALLOW_UNVALIDATED'):
         email_obj = account.emailaddress_set.get(email__iexact=email)
         if email_obj.status == EmailStatus.NEW:
 
=== modified file 'identityprovider/models/emailaddress.py'
--- identityprovider/models/emailaddress.py	2013-01-25 20:39:28 +0000
+++ identityprovider/models/emailaddress.py	2013-02-18 16:43:22 +0000
@@ -3,6 +3,7 @@
 # LICENSE).
 
 import datetime
+import re
 
 from django.core.urlresolvers import reverse
 from django.core.validators import validate_email
@@ -18,6 +19,31 @@
 )
 
 
+PHONE_EMAIL_DOMAIN = 'phone.ubuntu'
+PHONE_EMAIL_INVALID_CHARS = re.compile(r"[^-!#$%&'*+/=?^_`{}|~0-9A-Z\.]",
+                                       re.IGNORECASE)
+
+
+class EmailAddressManager(models.Manager):
+
+    def _generate_email_from_phone_id(self, phone_id):
+        # replace chars not validated by django validate_email by #
+        email = '%s@%s' % (PHONE_EMAIL_INVALID_CHARS.sub('#', phone_id),
+                           PHONE_EMAIL_DOMAIN)
+        return email
+
+    def create_from_phone_id(self, phone_id, account):
+        email = self._generate_email_from_phone_id(phone_id)
+        email_address = EmailAddress.objects.create(
+            email=email, account=account, status=EmailStatus.NEW)
+        return email_address
+
+    def get_from_phone_id(self, phone_id):
+        email = self._generate_email_from_phone_id(phone_id)
+        email_address = self.get(email=email)
+        return email_address
+
+
 class EmailAddress(models.Model):
     email = models.TextField(validators=[validate_email])
     lp_person = models.IntegerField(
@@ -28,6 +54,8 @@
     account = models.ForeignKey(
         Account, db_column='account', blank=True, null=True)
 
+    objects = EmailAddressManager()
+
     class Meta:
         app_label = 'identityprovider'
         db_table = u'emailaddress'
@@ -39,6 +67,10 @@
     def is_preferred(self):
         return self.status == EmailStatus.PREFERRED
 
+    def is_verifiable(self):
+        suffix = '@%s' % PHONE_EMAIL_DOMAIN
+        return not self.email.endswith(suffix)
+
     def is_verified(self):
         return self.status in (EmailStatus.VALIDATED, EmailStatus.PREFERRED)
 
 
=== modified file 'identityprovider/tests/unit/test_models_emailaddress.py'
--- identityprovider/tests/unit/test_models_emailaddress.py	2013-01-25 21:32:21 +0000
+++ identityprovider/tests/unit/test_models_emailaddress.py	2013-02-18 16:43:22 +0000
@@ -2,9 +2,34 @@
 
 from identityprovider.models import Account, EmailAddress
 from identityprovider.models.const import EmailStatus
+from identityprovider.models.emailaddress import PHONE_EMAIL_DOMAIN
 from identityprovider.tests.utils import SSOBaseTestCase
 
 
+class EmailAddressManagerTestCase(SSOBaseTestCase):
+    fixtures = ['test']
+
+    def test_create_from_phone_id(self):
+        account = Account.objects.get_by_email('[e-mail]')
+        new_email = EmailAddress.objects.create_from_phone_id('tel:+123',
+                                                              account)
+        self.assertEqual(new_email.status, EmailStatus.NEW)
+        self.assertEqual(new_email.account, account)
+        self.assertEqual(new_email.email, 'tel#+123@%s' % PHONE_EMAIL_DOMAIN)
+
+    def test_get_from_phone_id(self):
+        account = Account.objects.get_by_email('[e-mail]')
+        new_email = EmailAddress.objects.create_from_phone_id('tel:+123',
+                                                              account)
+        email = EmailAddress.objects.get_from_phone_id('tel:+123')
+        self.assertEqual(email, new_email)
+
+    def test_get_from_phone_id_not_exist(self):
+        self.assertRaises(
+            EmailAddress.DoesNotExist,
+            EmailAddress.objects.get_from_phone_id, 'tel:+123')
+
+
 class EmailAddressTestCase(SSOBaseTestCase):
     fixtures = ['test']
 
@@ -13,6 +38,13 @@
         email = EmailAddress.objects.get(email='[e-mail]')
         self.assertEqual(account, email.account)
 
+    def test_emailaddress_is_verifiable(self):
+        email = EmailAddress.objects.get(email='[e-mail]')
+        self.assertTrue(email.is_verifiable())
+        unverifiable_email = 'test@%s' % PHONE_EMAIL_DOMAIN
+        email = EmailAddress(email=unverifiable_email)
+        self.assertFalse(email.is_verifiable())
+
     def test_emailaddress_is_verified(self):
         email = EmailAddress.objects.get(email='[e-mail]')
         assert email.status == EmailStatus.PREFERRED
 
=== modified file 'webui/templates/account/emails.html'
--- webui/templates/account/emails.html	2012-12-19 18:54:44 +0000
+++ webui/templates/account/emails.html	2013-02-18 16:43:22 +0000
@@ -69,7 +69,9 @@
         <td class="email">{{ email }}</td>
         {% if not readonly %}
           <td class="actions">
+            {% if email.is_verifiable %}
             <a href="./+verify-email?id={{ email.id }}" class="btn-sm"><span>{% trans "Verify" %}</span></a>
+            {% endif %}
             <a href="./+remove-email?id={{ email.id }}" class="btn-sm"><span>{% trans "Delete" %}</span></a>
           </td>
         {% endif %}
 
=== modified file 'webui/tests/test_views_account.py'
--- webui/tests/test_views_account.py	2013-01-25 20:40:15 +0000
+++ webui/tests/test_views_account.py	2013-02-18 16:43:22 +0000
@@ -36,6 +36,36 @@
 from identityprovider.utils import validate_launchpad_password
 
 
+class AccountEmailsViewTestCase(AuthenticatedTestCase):
+
+    def setUp(self):
+        super(AccountEmailsViewTestCase, self).setUp()
+        account = Account.objects.get(emailaddress__email=self.login_email)
+        self.phone_email = EmailAddress.objects.create_from_phone_id(
+            'tel:[phone]', account)
+
+    def test_phone_id_email_is_not_verifiable(self):
+        url = reverse('account-emails')
+        response = self.client.get(url)
+        tree = PyQuery(response.content)
+        # unverified emails -> second listing table
+        unverified_emails = tree.find('table.listing')[1]
+        emails_td = PyQuery(unverified_emails).find('td.email')
+        for email_td in emails_td:
+            if email_td.text == self.phone_email.email:
+                # get available actions
+                actions = email_td.getnext().getchildren()
+                # only delete option available, is not verifiable
+                self.assertEqual(len(actions), 1)
+                self.assertEqual(actions[0].get('href'),
+                                 '.%s?id=%d' % (reverse('delete_email'),
+                                                self.phone_email.id))
+            else:
+                # get available actions: verify, remove
+                actions = email_td.getnext().getchildren()
+                self.assertEqual(len(actions), 2)
+
+
 class AccountViewsUnauthenticatedTestCase(SSOBaseTestCase):
 
     def test_index_unauthenticated(self):