Fix the UserOpenIDAdmin to set the permission on the user whose OpenID has just…

Fix the UserOpenIDAdmin to set the permission on the user whose OpenID has just been updated, not the user making the request; Save a db query in authenticate by returning the UserOpenID from create_user_from_openid; Set user permissions in update_user_details.

django_openid_auth/admin.py

@@ -70,9 +70,9 @@ class UserOpenIDAdmin(admin.ModelAdmin):
     def save_model(self, request, obj, form, change):
         permission = Permission.objects.get(codename='account_verified')
         if obj.account_verified:
-            request.user.user_permissions.add(permission)
+            obj.user.user_permissions.add(permission)
         else:
-            request.user.user_permissions.remove(permission)
+            obj.user.user_permissions.remove(permission)
         obj.save()
 
     def log_deletion(self, request, obj, object_repr):

django_openid_auth/auth.py

@@ -80,9 +80,8 @@ class OpenIDBackend:
                 claimed_id__exact=openid_response.identity_url)
         except UserOpenID.DoesNotExist:
             if getattr(settings, 'OPENID_CREATE_USERS', False):
-                user = self.create_user_from_openid(openid_response)
-                user_openid = UserOpenID.objects.get(
-                    claimed_id__exact=openid_response.identity_url)
+                user, user_openid = self.create_user_from_openid(
+                    openid_response)
         else:
             user = user_openid.user
 
@@ -91,8 +90,7 @@ class OpenIDBackend:
 
         if getattr(settings, 'OPENID_UPDATE_DETAILS_FROM_SREG', False):
             details = self._extract_user_details(openid_response)
-            self.update_user_details(user, details, openid_response)
-            self.update_user_openid(user_openid, details)
+            self.update_user_details(user_openid, details, openid_response)
 
         if getattr(settings, 'OPENID_PHYSICAL_MULTIFACTOR_REQUIRED', False):
             pape_response = pape.Response.fromSuccessResponse(openid_response)
@@ -277,10 +275,10 @@ class OpenIDBackend:
             openid_response.identity_url)
 
         user = User.objects.create_user(username, email, password=None)
-        self.associate_openid(user, openid_response)
-        self.update_user_details(user, details, openid_response)
+        user_openid = self.associate_openid(user, openid_response)
+        self.update_user_details(user_openid, details, openid_response)
 
-        return user
+        return user, user_openid
 
     def associate_openid(self, user, openid_response):
         """Associate an OpenID with a user account."""
@@ -302,15 +300,8 @@ class OpenIDBackend:
 
         return user_openid
 
-    def update_user_openid(self, user_openid, details):
-        updated = False
-        if details.get('account_verified', None) is not None:
-            user_openid.account_verified = details['account_verified']
-            updated = True
-        if updated:
-            user_openid.save()
-
-    def update_user_details(self, user, details, openid_response):
+    def update_user_details(self, user_openid, details, openid_response):
+        user = user_openid.user
         updated = False
         if details['first_name']:
             user.first_name = details['first_name'][:30]
@@ -324,9 +315,19 @@ class OpenIDBackend:
         if getattr(settings, 'OPENID_FOLLOW_RENAMES', False):
             user.username = self._get_available_username(details['nickname'], openid_response.identity_url)
             updated = True
+        account_verified = details.get('account_verified', None)
+        if account_verified is not None:
+            permission = Permission.objects.get(codename='account_verified')
+            user_openid.account_verified = account_verified
+            if account_verified:
+                user.user_permissions.add(permission)
+            else:
+                user.user_permissions.remove(permission)
+            updated = True
 
         if updated:
             user.save()
+            user_openid.save()
 
     def get_teams_mapping(self):
         teams_mapping_auto = getattr(settings, 'OPENID_LAUNCHPAD_TEAMS_MAPPING_AUTO', False)
@@ -369,17 +370,3 @@ class OpenIDBackend:
                 break
 
         user.save()
-
-    def has_perm(self, user_obj, perm, instance=None):
-        return perm in self.get_all_permissions(user_obj, instance)
-
-    def get_all_permissions(self, user_obj, instance=None):
-        try:
-            user_openid = UserOpenID.objects.get(user=user_obj)
-        except UserOpenID.DoesNotExist:
-            return set()
-        if user_openid.account_verified:
-            permission = Permission.objects.get(codename='account_verified')
-            return set(['%s.%s' % (permission.content_type.app_label,
-                                   permission.codename)])
-        return set()

django_openid_auth/tests/test_auth.py

@@ -178,19 +178,25 @@ class OpenIDBackendTests(TestCase):
         response = self.make_response_ax()
         user = User.objects.create_user('someuser', 'someuser@example.com',
             password=None)
+        user_openid, created = UserOpenID.objects.get_or_create(
+            user=user,
+            claimed_id='http://example.com/existing_identity',
+            display_id='http://example.com/existing_identity',
+            account_verified=False)
         data = dict(first_name=u"Some56789012345678901234567890123",
             last_name=u"User56789012345678901234567890123",
             email=u"someotheruser@example.com", account_verified=False)
 
-        self.backend.update_user_details(user, data, response)
+        self.backend.update_user_details(user_openid, data, response)
 
         self.assertEqual("Some56789012345678901234567890",  user.first_name)
         self.assertEqual("User56789012345678901234567890",  user.last_name)
 
     def test_update_user_openid_unverified(self):
+        response = self.make_response_ax()
         user = User.objects.create_user('someuser', 'someuser@example.com',
             password=None)
-        user_openid = UserOpenID.objects.get_or_create(
+        user_openid, created = UserOpenID.objects.get_or_create(
             user=user,
             claimed_id='http://example.com/existing_identity',
             display_id='http://example.com/existing_identity',
@@ -199,14 +205,14 @@ class OpenIDBackendTests(TestCase):
             last_name=u"User56789012345678901234567890123",
             email=u"someotheruser@example.com", account_verified=False)
 
-        user_openid = UserOpenID.objects.get(user=user)
-        self.backend.update_user_openid(user_openid, data)
+        self.backend.update_user_details(user_openid, data, response)
         self.assertFalse(user_openid.account_verified)
 
     def test_update_user_openid_verified(self):
+        response = self.make_response_ax()
         user = User.objects.create_user('someuser', 'someuser@example.com',
             password=None)
-        user_openid = UserOpenID.objects.get_or_create(
+        user_openid, created = UserOpenID.objects.get_or_create(
             user=user,
             claimed_id='http://example.com/existing_identity',
             display_id='http://example.com/existing_identity',
@@ -215,8 +221,7 @@ class OpenIDBackendTests(TestCase):
             last_name=u"User56789012345678901234567890123",
             email=u"someotheruser@example.com", account_verified=True)
 
-        user_openid = UserOpenID.objects.get(user=user)
-        self.backend.update_user_openid(user_openid, data)
+        self.backend.update_user_details(user_openid, data, response)
         self.assertTrue(user_openid.account_verified)
 
     def test_extract_user_details_name_with_trailing_space(self):