paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-13 10:26:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

management: Fix password reset emails being sent to deactivated users.

Browse Source 
Apparently, the filters written for the send_password_reset_email (and
some other management commands) didn't correctly consider the case of
deactivated users.

While some commands, like syncing LDAP data (which can include whether
a user should be deactivated) want to process all users, other
commands generally only want to interact with active users.  We fix
this and add some tests.
This commit is contained in:
Tim Abbott
2019-08-14 10:48:54 -07:00
parent cbae51db63
commit 2ada0a9bad
4 changed files with 32 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
zerver/lib/management.py
View File

@@ -88,7 +88,8 @@ You can use the command list_realms to find ID of the realms in this server."""
(options["realm_id"],)) (options["realm_id"],))
def get_users(self, options: Dict[str, Any], realm: Optional[Realm], def get_users(self, options: Dict[str, Any], realm: Optional[Realm],
is_bot: Optional[bool]=None) -> List[UserProfile]: is_bot: Optional[bool]=None,
include_deactivated: bool=False) -> List[UserProfile]:
if "all_users" in options: if "all_users" in options:
all_users = options["all_users"] all_users = options["all_users"]
@@ -103,6 +104,8 @@ You can use the command list_realms to find ID of the realms in this server."""
if all_users: if all_users:
user_profiles = UserProfile.objects.filter(realm=realm) user_profiles = UserProfile.objects.filter(realm=realm)
if not include_deactivated:
user_profiles = user_profiles.filter(is_active=True)
if is_bot is not None: if is_bot is not None:
return user_profiles.filter(is_bot=is_bot) return user_profiles.filter(is_bot=is_bot)
return user_profiles return user_profiles

2
zerver/management/commands/send_password_reset_email.py
View File

@@ -26,7 +26,7 @@ class Command(ZulipBaseCommand):
else: else:
realm = self.get_realm(options) realm = self.get_realm(options)
try: try:
users = self.get_users(options, realm) users = self.get_users(options, realm, is_bot=False)
except CommandError as error: except CommandError as error:
if str(error) == "You have to pass either -u/--users or -a/--all-users.": if str(error) == "You have to pass either -u/--users or -a/--all-users.":
raise CommandError("You have to pass -u/--users or -a/--all-users or --entire-server.") raise CommandError("You have to pass -u/--users or -a/--all-users or --entire-server.")

3
zerver/management/commands/sync_ldap_user_data.py
View File

@@ -41,7 +41,8 @@ class Command(ZulipBaseCommand):
def handle(self, *args: Any, **options: Any) -> None: def handle(self, *args: Any, **options: Any) -> None:
if options.get('realm_id') is not None: if options.get('realm_id') is not None:
realm = self.get_realm(options) realm = self.get_realm(options)
user_profiles = self.get_users(options, realm, is_bot=False) user_profiles = self.get_users(options, realm, is_bot=False,
include_deactivated=True)
else: else:
user_profiles = UserProfile.objects.select_related().filter(is_bot=False) user_profiles = UserProfile.objects.select_related().filter(is_bot=False)
sync_ldap_user_data(user_profiles) sync_ldap_user_data(user_profiles)

29
zerver/tests/test_management_commands.py
View File

@@ -82,8 +82,8 @@ class TestZulipBaseCommand(ZulipTestCase):
self.assertEqual(get_user_profile_by_email(email), user_profile) self.assertEqual(get_user_profile_by_email(email), user_profile)
def get_users_sorted(self, options: Dict[str, Any], realm: Optional[Realm], def get_users_sorted(self, options: Dict[str, Any], realm: Optional[Realm],
is_bot: Optional[bool]=None) -> List[UserProfile]: **kwargs: Any) -> List[UserProfile]:
user_profiles = self.command.get_users(options, realm, is_bot=is_bot) user_profiles = self.command.get_users(options, realm, **kwargs)
return sorted(user_profiles, key = lambda x: x.email) return sorted(user_profiles, key = lambda x: x.email)
def test_get_users(self) -> None: def test_get_users(self) -> None:
@@ -116,9 +116,30 @@ class TestZulipBaseCommand(ZulipTestCase):
with self.assertRaisesRegex(CommandError, error_message): with self.assertRaisesRegex(CommandError, error_message):
self.command.get_users(dict(users=user_emails, all_users=True), None) self.command.get_users(dict(users=user_emails, all_users=True), None)
expected_user_profiles = sorted(UserProfile.objects.filter(realm=self.zulip_realm), # Test the default mode excluding bots and deactivated users
expected_user_profiles = sorted(UserProfile.objects.filter(realm=self.zulip_realm,
is_active=True, is_bot=False),
key = lambda x: x.email) key = lambda x: x.email)
user_profiles = self.get_users_sorted(dict(users=None, all_users=True), self.zulip_realm) user_profiles = self.get_users_sorted(dict(users=None, all_users=True),
self.zulip_realm,
is_bot=False)
self.assertEqual(user_profiles, expected_user_profiles)
# Test the default mode excluding bots and deactivated users
expected_user_profiles = sorted(UserProfile.objects.filter(realm=self.zulip_realm,
is_active=True),
key = lambda x: x.email)
user_profiles = self.get_users_sorted(dict(users=None, all_users=True),
self.zulip_realm)
self.assertEqual(user_profiles, expected_user_profiles)
# Test include_deactivated
expected_user_profiles = sorted(UserProfile.objects.filter(realm=self.zulip_realm,
is_bot=False),
key = lambda x: x.email)
user_profiles = self.get_users_sorted(dict(users=None, all_users=True),
self.zulip_realm,
is_bot=False, include_deactivated=True)
self.assertEqual(user_profiles, expected_user_profiles) self.assertEqual(user_profiles, expected_user_profiles)
error_message = "You have to pass either -u/--users or -a/--all-users." error_message = "You have to pass either -u/--users or -a/--all-users."