Merge lp:~nataliabidart/canonical-identity-provider/user-person-in-team into lp:canonical-identity-provider/release

[Natalia Bidart]

- Added log messages to try to pin point why authenticate backends are returning a user instead of an account.

[Matt Goodall (matt-goodall)]

lgtm

review: Approve

[Daniel Manrique (roadmr)]

Looks OK to me, I'd say to put a FIXME or TODO as part of the "remove the ModelBackend later" comment so it's easier to find, but I don't think it's a big deal since this should be resolved before we forget about that :)

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~nataliabidart/canonical-identity-provider/user-person-in-team into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Bootstrapping...
rm -rf /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env
rm -rf branches/wheels
rm -rf branches/*
rm -rf staticfiles
rm -f lib/versioninfo.py
find -name '*.pyc' -delete
find -name '*.~*' -delete
Not deleting /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin
New python executable in /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin/python
Installing setuptools, pip...done.
/usr/lib/config-manager/cm.py update config-manager.txt
touch branches/last_build
[ -d branches/wheels ] && (cd branches/wheels && bzr pull) || (bzr branch lp:~ubuntuone-pqm-team/canonical-identity-provider/dependencies branches/wheels)
bzr version-info --format=python > lib/versioninfo.py
/mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin/pip install --verbose --find-links=branches/wheels --no-index -r requirements.txt
Ignoring indexes: https://pypi.python.org/simple/
Downloading/unpacking bson==0.3.3 (from -r requirements.txt (line 1))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/bson-0.3.3-py2-none-any.whl
Downloading/unpacking convoy==0.4.1 (from -r requirements.txt (line 2))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.4.1-py2-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.2.2-py2-none-any.whl
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.2.2-py2-none-any.whl, version 0.2.2 doesn't match ==0.4.1
Downloading/unpacking dj-static==0.0.6.dev1 (from -r requirements.txt (line 3))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/dj_static-0.0.6.dev1-py2-none-any.whl
Downloading/unpacking django==1.7.9 (from -r requirements.txt (line 4))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.8.3-py2.py3-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.9-py2.py3-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.8-py2.py3-none-any.whl
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.8.3-py2.py3-none-any.whl, version 1.8.3 doesn't match ==1.7.9
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.8-py2.py3-none-any.whl, version 1.7.8 doesn't match ==1.7.9
Downloading/unpacking django-honeypot==0.4.0 (from -r requirements.txt (line 5))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/django_honeypot-0.4.0-py2-none-any.whl
Downloading/unpacking django-jsonfield==0.9.13 (from -r requirements.txt (line 6))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/django_jsonfield-0.9.13-py2-none-any.whl
Downloading/unpacking d...

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~nataliabidart/canonical-identity-provider/user-person-in-team into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Bootstrapping...
rm -rf /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env
rm -rf branches/wheels
rm -rf branches/*
rm -rf staticfiles
rm -f lib/versioninfo.py
find -name '*.pyc' -delete
find -name '*.~*' -delete
Not deleting /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin
New python executable in /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin/python
Installing setuptools, pip...done.
/usr/lib/config-manager/cm.py update config-manager.txt
touch branches/last_build
[ -d branches/wheels ] && (cd branches/wheels && bzr pull) || (bzr branch lp:~ubuntuone-pqm-team/canonical-identity-provider/dependencies branches/wheels)
bzr version-info --format=python > lib/versioninfo.py
/mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin/pip install --verbose --find-links=branches/wheels --no-index -r requirements.txt
Ignoring indexes: https://pypi.python.org/simple/
Downloading/unpacking bson==0.3.3 (from -r requirements.txt (line 1))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/bson-0.3.3-py2-none-any.whl
Downloading/unpacking convoy==0.4.1 (from -r requirements.txt (line 2))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.4.1-py2-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.2.2-py2-none-any.whl
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.2.2-py2-none-any.whl, version 0.2.2 doesn't match ==0.4.1
Downloading/unpacking dj-static==0.0.6.dev1 (from -r requirements.txt (line 3))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/dj_static-0.0.6.dev1-py2-none-any.whl
Downloading/unpacking django==1.7.9 (from -r requirements.txt (line 4))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.8.3-py2.py3-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.9-py2.py3-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.8-py2.py3-none-any.whl
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.8.3-py2.py3-none-any.whl, version 1.8.3 doesn't match ==1.7.9
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.8-py2.py3-none-any.whl, version 1.7.8 doesn't match ==1.7.9
Downloading/unpacking django-honeypot==0.4.0 (from -r requirements.txt (line 5))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/django_honeypot-0.4.0-py2-none-any.whl
Downloading/unpacking django-jsonfield==0.9.13 (from -r requirements.txt (line 6))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/django_jsonfield-0.9.13-py2-none-any.whl
Downloading/unpacking d...

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~nataliabidart/canonical-identity-provider/user-person-in-team into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Bootstrapping...
rm -rf /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env
rm -rf branches/wheels
rm -rf branches/*
rm -rf staticfiles
rm -f lib/versioninfo.py
find -name '*.pyc' -delete
find -name '*.~*' -delete
Not deleting /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin
New python executable in /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin/python
Installing setuptools, pip...done.
/usr/lib/config-manager/cm.py update config-manager.txt
touch branches/last_build
[ -d branches/wheels ] && (cd branches/wheels && bzr pull) || (bzr branch lp:~ubuntuone-pqm-team/canonical-identity-provider/dependencies branches/wheels)
bzr version-info --format=python > lib/versioninfo.py
/mnt/tarmac/cache/canonical-identity-provider/merges/trunk/env/bin/pip install --verbose --find-links=branches/wheels --no-index -r requirements.txt
Ignoring indexes: https://pypi.python.org/simple/
Downloading/unpacking bson==0.3.3 (from -r requirements.txt (line 1))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/bson-0.3.3-py2-none-any.whl
Downloading/unpacking convoy==0.4.1 (from -r requirements.txt (line 2))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.4.1-py2-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.2.2-py2-none-any.whl
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/convoy-0.2.2-py2-none-any.whl, version 0.2.2 doesn't match ==0.4.1
Downloading/unpacking dj-static==0.0.6.dev1 (from -r requirements.txt (line 3))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/dj_static-0.0.6.dev1-py2-none-any.whl
Downloading/unpacking django==1.7.9 (from -r requirements.txt (line 4))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.8.3-py2.py3-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.9-py2.py3-none-any.whl, /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.8-py2.py3-none-any.whl
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.8.3-py2.py3-none-any.whl, version 1.8.3 doesn't match ==1.7.9
  Ignoring link file:///mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/Django-1.7.8-py2.py3-none-any.whl, version 1.7.8 doesn't match ==1.7.9
Downloading/unpacking django-honeypot==0.4.0 (from -r requirements.txt (line 5))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/django_honeypot-0.4.0-py2-none-any.whl
Downloading/unpacking django-jsonfield==0.9.13 (from -r requirements.txt (line 6))
  Local files found: /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/branches/wheels/django_jsonfield-0.9.13-py2-none-any.whl
Downloading/unpacking d...

=== modified file 'django_project/settings_base.py'
--- django_project/settings_base.py	2015-06-30 21:39:29 +0000
+++ django_project/settings_base.py	2015-07-20 19:43:14 +0000
@@ -42,6 +42,8 @@
 APP_SERVERS = []
 AUTHENTICATION_BACKENDS = [
     'identityprovider.auth.LaunchpadBackend',
+    # Once we debug LP: # 1476255, we should remove the ModelBackend
+    # from this list
     'django.contrib.auth.backends.ModelBackend',
     'django_openid_auth.auth.OpenIDBackend',
 ]

=== modified file 'src/identityprovider/login.py'
--- src/identityprovider/login.py	2015-07-01 15:16:55 +0000
+++ src/identityprovider/login.py	2015-07-20 19:43:14 +0000
@@ -1,5 +1,8 @@
 # Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+
+import logging
+
 from django.contrib import auth
 from django.template.loader import render_to_string
 from django.utils.translation import ugettext_lazy as _
@@ -12,6 +15,7 @@
 from identityprovider.validators import ValidationError, get_password_validator
 
 LOGIN_INCORRECT = _('Incorrect email/password combination')
+logger = logging.getLogger(__name__)
 
 
 class AuthenticationError(Exception):
@@ -71,6 +75,16 @@
 
         raise AuthenticationError(LOGIN_INCORRECT)
 
+    # Adding debug information for LP: #1476255
+    # AttributeError: 'User' object has no attribute 'person_in_team'
+    if not isinstance(account, Account):
+        logger.warning(
+            'authenticate_user: call to auth.authenticate returned a non '
+            'Account instance (%s, id: %r, backend: %r) for email %r, '
+            'and rpconfig %r.',
+            account.__class__.__name__, getattr(account, 'id', None),
+            getattr(account, 'backend', None), email, rpconfig)
+
     validate_policy = get_password_validator(account=account)
     try:
         validate_policy(password)

=== modified file 'src/identityprovider/tests/test_admin.py'
--- src/identityprovider/tests/test_admin.py	2015-06-24 23:20:24 +0000
+++ src/identityprovider/tests/test_admin.py	2015-07-20 19:43:14 +0000
@@ -139,13 +139,22 @@
             'devices-INITIAL_FORMS': '0',
         }
         parameters.update(kwargs)
-        r = self.client.post(change_view, parameters)
+        response = self.client.post(change_view, parameters)
 
         if success:
+            status_code = response.status_code
+            # If view returns with 200 on POST, it means the form has errors.
+            if status_code == 200:
+                self.assertEqual(response.context['errors'], [])
+
             # Any non-error status code would be fine here, but right now
             # it redirects with a 302.
-            self.assertEqual(r.status_code, 302)
-        return r
+            self.assertEqual(
+                status_code, 302,
+                'Response did not redirect as expected, response is: %s' %
+                response)
+
+        return response
 
     def test_account_change(self):
         new_email = '[e-mail]'

=== modified file 'src/identityprovider/tests/test_models_account.py'
--- src/identityprovider/tests/test_models_account.py	2015-07-01 19:20:49 +0000
+++ src/identityprovider/tests/test_models_account.py	2015-07-20 19:43:14 +0000
@@ -1251,3 +1251,16 @@
 
         self.assertIn('there was a security breach', str(ctx.exception))
         self.assertEqual(ctx.exception.account, self.account)
+
+    def test_non_account_instance(self):
+        mock_logger = self._apply_patch('identityprovider.login.logger')
+        user = User.objects.create_user(username='test', password='12345678')
+
+        with self.assertRaises(AttributeError):
+            authenticate_user(user.username, '12345678')
+
+        mock_logger.warning.assert_called_once_with(
+            'authenticate_user: call to auth.authenticate returned a non '
+            'Account instance (%s, id: %r, backend: %r) for email %r, and '
+            'rpconfig %r.', 'User', user.id,
+            'django.contrib.auth.backends.ModelBackend', 'test', None)