Merge lp:~nataliabidart/canonical-identity-provider/no-django-modelbackend-auth into lp:canonical-identity-provider/release

[Natalia Bidart]

- Disable django.contrib.auth.backends.ModelBackend as one of the AUTHENTICATION_BACKENDS.
- Removed extra logging to debug LP: #1476225.

[Ricardo Kirkner (ricardokirkner)]

LGTM.

Looks like a possible next step would be to start using the custom user model support in django?

review: Approve

[Natalia Bidart (nataliabidart)]

> LGTM.
>
> Looks like a possible next step would be to start using the custom user model
> support in django?

Indeed!

The thing we need to think carefully is that we still have some Old-format tokens, which use the Consumer from oauth_backend, and that model has a FK to User.
Same for UserOpenID -- the main issue I see is how to remap the exiting FK to the proper account.

Any ideas?

=== modified file 'django_project/settings_base.py'
--- django_project/settings_base.py	2015-10-01 19:21:34 +0000
+++ django_project/settings_base.py	2015-10-09 20:18:42 +0000
@@ -43,9 +43,6 @@
 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',
 ]
 AUTH_USER_MODEL = 'auth.User'

=== modified file 'src/api/v20/auth.py'
--- src/api/v20/auth.py	2015-09-23 21:22:12 +0000
+++ src/api/v20/auth.py	2015-10-09 20:18:42 +0000
@@ -46,8 +46,7 @@
                     perm = 'identityprovider.api_view_account_details'
                 else:
                     perm = 'identityprovider.api_edit_account_details'
-                account = request.user
-                result = account.user.has_perm(perm)
+                result = request.user.has_perm(perm)
 
         return result
 

=== modified file 'src/identityprovider/auth.py'
--- src/identityprovider/auth.py	2015-10-05 22:09:03 +0000
+++ src/identityprovider/auth.py	2015-10-09 20:18:42 +0000
@@ -8,6 +8,7 @@
 import logging
 
 from django.conf import settings
+from django.contrib.auth.backends import ModelBackend
 from django.contrib.auth.hashers import BCryptPasswordHasher
 from django.contrib.auth.models import (
     check_password,
@@ -188,8 +189,7 @@
         self.password_dirty = True
 
     def get_user(self, user_id):
-        """Returns a Launchpad Account object instead of Django's built-in
-        User object. """
+        """Return the Account object instead of Django's User object."""
         return get_object_or_none(Account.objects.select_related(), pk=user_id)
 
     def supports_anonymous_user(self):
@@ -198,6 +198,9 @@
     def supports_object_permissions(self):
         return False
 
+    def has_perm(self, account, *args, **kwargs):
+        return ModelBackend().has_perm(account.user, *args, **kwargs)
+
 
 def decode(value):
     # try to decode as UTF-8

=== modified file 'src/identityprovider/login.py'
--- src/identityprovider/login.py	2015-10-06 22:41:40 +0000
+++ src/identityprovider/login.py	2015-10-09 20:18:42 +0000
@@ -76,16 +76,6 @@
 
         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, request=request)
     try:
         validate_policy(password)

=== modified file 'src/identityprovider/tests/test_admin.py'
--- src/identityprovider/tests/test_admin.py	2015-09-16 15:33:26 +0000
+++ src/identityprovider/tests/test_admin.py	2015-10-09 20:18:42 +0000
@@ -318,14 +318,15 @@
 
 class RPAdminTestCase(SSOBaseTestCase):
 
-    fixtures = ["admin"]
-
     def setUp(self):
         super(RPAdminTestCase, self).setUp()
 
         OpenIDRPConfig.objects.create(trust_root="test", displayname="test",
                                       description="test")
-        self.client.login(username="admin", password="admin007")
+        admin = self.factory.make_account(
+            is_superuser=True, password="admin007")
+        assert self.client.login(
+            username=admin.user.email, password="admin007")
 
     def _get_form(self):
         path = reverse("admin:identityprovider_openidrpconfig_add")

=== modified file 'src/identityprovider/tests/test_models_account.py'
--- src/identityprovider/tests/test_models_account.py	2015-10-06 22:41:40 +0000
+++ src/identityprovider/tests/test_models_account.py	2015-10-09 20:18:42 +0000
@@ -1285,14 +1285,7 @@
         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):
+        with self.assertRaises(AuthenticationError):
             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)