Skip to content

Commit fd14bf8

Browse files
author
Jordi Romera
committed
ldap_clean_users - Does not catch Exception and fix flake8 warning
1 parent 62d5cbc commit fd14bf8

File tree

2 files changed

+24
-20
lines changed

2 files changed

+24
-20
lines changed

django_python3_ldap/management/commands/ldap_clean_users.py

+8-12
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from django.contrib.auth import get_user_model
22
from django.core.management.base import BaseCommand, CommandError
33
from django.db import transaction
4+
from django.db.models import ProtectedError
45

56
from django_python3_ldap import ldap
67
from django_python3_ldap.conf import settings
@@ -54,12 +55,11 @@ def _iter_local_users(User, lookups, superuser, staff):
5455
yield User.objects.get(**lookup,
5556
is_superuser=superuser,
5657
is_staff=staff)
57-
except Exception as e:
58+
except User.DoesNotExist:
5859
raise CommandError("Could not find user with lookup : {lookup}".format(
5960
lookup=lookup,
6061
))
6162

62-
6363
@staticmethod
6464
def _remove(user, purge):
6565
"""
@@ -69,19 +69,15 @@ def _remove(user, purge):
6969
# Delete local user
7070
try:
7171
user.delete()
72-
except Exception as e:
73-
raise CommandError("Could not purge user {user}".format(
72+
except ProtectedError as e:
73+
raise CommandError("Could not purge user {user} : {e}".format(
7474
user=user,
75+
e=e
7576
))
7677
else:
7778
# Deactivate local user
78-
try:
79-
user.is_active = False
80-
user.save()
81-
except Exception as e:
82-
raise CommandError("Could not deactivate user {user}".format(
83-
user=user,
84-
))
79+
user.is_active = False
80+
user.save()
8581

8682
@transaction.atomic()
8783
def handle(self, *args, **kwargs):
@@ -101,7 +97,7 @@ def handle(self, *args, **kwargs):
10197
for user in self._iter_local_users(User, lookups, superuser, staff):
10298
# For each local users
10399
# Check if user still exists
104-
user_kwargs = {
100+
user_kwargs = {
105101
User.USERNAME_FIELD: getattr(user, User.USERNAME_FIELD)
106102
}
107103
if connection.has_user(**user_kwargs):

django_python3_ldap/tests.py

+16-8
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,10 @@ def testImportFunc(self):
262262
with self.settings(LDAP_AUTH_SYNC_USER_RELATIONS='django.contrib.auth.get_user_model'):
263263
self.assertTrue(callable(import_func(settings.LDAP_AUTH_SYNC_USER_RELATIONS)))
264264

265-
# User cleaning.
266-
267265
def testCleanUsersDeactivate(self):
266+
"""
267+
ldap_clean_users management command test
268+
"""
268269
from django.contrib.auth import get_user_model
269270
User = get_user_model()
270271
_username = "nonldap{user}".format(user=settings.LDAP_AUTH_TEST_USER_USERNAME)
@@ -280,7 +281,9 @@ def testCleanUsersDeactivate(self):
280281
self.assertEqual(user_count_1, user_count_2)
281282
self.assertEqual(User.objects.get(username=_username).is_active, False)
282283

283-
### Test with lookup
284+
"""
285+
Test with lookup
286+
"""
284287
# Reactivate user
285288
user = User.objects.get(username=_username)
286289
user.is_active = True
@@ -309,12 +312,12 @@ def testCleanUsersDeactivate(self):
309312
self.assertEqual(User.objects.get(username=_username).is_active, False)
310313
self.assertEqual(User.objects.get(username=_usernameLookup).is_active, True)
311314
# Lookup a non existing user (raise a CommandError)
312-
try:
315+
with self.assertRaises(CommandError):
313316
call_command("ldap_clean_users", 'doesnonexist', verbosity=0)
314-
except Exception as e:
315-
print(e)
316317

317-
### Test with superuser
318+
"""
319+
Test with superuser
320+
"""
318321
# Reactivate first user and promote to superuser
319322
user = User.objects.get(username=_username)
320323
user.is_active = True
@@ -330,7 +333,9 @@ def testCleanUsersDeactivate(self):
330333
call_command("ldap_clean_users", superuser=True, verbosity=0)
331334
self.assertEqual(User.objects.get(username=_username).is_active, False)
332335

333-
### Test with staff user
336+
"""
337+
Test with staff user
338+
"""
334339
# Reactivate first user and promote to staff
335340
user = User.objects.get(username=_username)
336341
user.is_active = True
@@ -348,6 +353,9 @@ def testCleanUsersDeactivate(self):
348353
self.assertEqual(User.objects.get(username=_username).is_active, False)
349354

350355
def testCleanUsersPurge(self):
356+
"""
357+
ldap_clean_users management command test with purge argument
358+
"""
351359
from django.contrib.auth import get_user_model
352360
User = get_user_model()
353361
user = User.objects.create_user(

0 commit comments

Comments
 (0)