diff --git a/tools/test-api b/tools/test-api index 73fa6acd1b..b3b2310973 100755 --- a/tools/test-api +++ b/tools/test-api @@ -59,7 +59,8 @@ with test_server_running( # Prepare the admin client email = "iago@zulip.com" # Iago is an admin realm = get_realm("zulip") - user = get_user(email, realm) + iago = get_user(email, realm) + user = iago # Iago needs permission to manage all user groups. admins_group = NamedUserGroup.objects.get( @@ -69,9 +70,10 @@ with test_server_running( realm, "can_manage_all_groups", admins_group, acting_user=None ) - # Required to test can_create_users endpoints. + # Required to test can_create_users and can_change_user_emails endpoints. user.can_create_users = True - user.save(update_fields=["can_create_users"]) + user.can_change_user_emails = True + user.save(update_fields=["can_create_users", "can_change_user_emails"]) api_key = get_api_key(user) site = "http://zulip.zulipdev.com:9981" @@ -85,6 +87,7 @@ with test_server_running( email = "desdemona@zulip.com" # desdemona is an owner realm = get_realm("zulip") user = get_user(email, realm) + api_key = get_api_key(user) site = "http://zulip.zulipdev.com:9981" owner_client = Client( diff --git a/version.py b/version.py index c0c47a6a89..3e74f3ec6a 100644 --- a/version.py +++ b/version.py @@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 312 # Last bumped for adding 'realm_export_consent' event type. +API_FEATURE_LEVEL = 313 # Last bumped for adding `new_email` to /users/{user_id} # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/actions/users.py b/zerver/actions/users.py index 81ae6a0ef7..ac8fb55c61 100644 --- a/zerver/actions/users.py +++ b/zerver/actions/users.py @@ -749,6 +749,28 @@ def do_change_can_create_users(user_profile: UserProfile, value: bool) -> None: ) +@transaction.atomic(savepoint=False) +def do_change_can_change_user_emails(user_profile: UserProfile, value: bool) -> None: + event_time = timezone_now() + old_value = user_profile.can_change_user_emails + + user_profile.can_change_user_emails = value + user_profile.save(update_fields=["can_change_user_emails"]) + + RealmAuditLog.objects.create( + realm=user_profile.realm, + event_type=AuditLogEventType.USER_SPECIAL_PERMISSION_CHANGED, + event_time=event_time, + acting_user=None, + modified_user=user_profile, + extra_data={ + RealmAuditLog.OLD_VALUE: old_value, + RealmAuditLog.NEW_VALUE: value, + "property": "can_change_user_emails", + }, + ) + + @transaction.atomic(durable=True) def do_update_outgoing_webhook_service( bot_profile: UserProfile, service_interface: int, service_payload_url: str diff --git a/zerver/management/commands/change_user_role.py b/zerver/management/commands/change_user_role.py index e0c49256b1..b8ff9518e5 100644 --- a/zerver/management/commands/change_user_role.py +++ b/zerver/management/commands/change_user_role.py @@ -6,6 +6,7 @@ from django.core.management.base import CommandError from typing_extensions import override from zerver.actions.users import ( + do_change_can_change_user_emails, do_change_can_create_users, do_change_can_forge_sender, do_change_is_billing_admin, @@ -23,6 +24,7 @@ ROLE_CHOICES = [ "guest", "can_forge_sender", "can_create_users", + "can_change_user_emails", "is_billing_admin", ] @@ -65,7 +67,12 @@ ONLY perform this on customer request from an authorized person. "guest": UserProfile.ROLE_GUEST, } - if options["new_role"] not in ["can_forge_sender", "can_create_users", "is_billing_admin"]: + if options["new_role"] not in [ + "can_forge_sender", + "can_create_users", + "can_change_user_emails", + "is_billing_admin", + ]: new_role = user_role_map[options["new_role"]] if not options["grant"]: raise CommandError( @@ -109,6 +116,12 @@ ONLY perform this on customer request from an authorized person. elif not user.can_create_users and not options["grant"]: raise CommandError("User can't create users for this realm.") do_change_can_create_users(user, options["grant"]) + elif options["new_role"] == "can_change_user_emails": + if user.can_change_user_emails and options["grant"]: + raise CommandError("User can already change user emails for this realm.") + elif not user.can_change_user_emails and not options["grant"]: + raise CommandError("User can't change user emails for this realm.") + do_change_can_change_user_emails(user, options["grant"]) else: assert options["new_role"] == "is_billing_admin" if user.is_billing_admin and options["grant"]: diff --git a/zerver/migrations/0616_userprofile_can_change_user_emails.py b/zerver/migrations/0616_userprofile_can_change_user_emails.py new file mode 100644 index 0000000000..b29d1ca843 --- /dev/null +++ b/zerver/migrations/0616_userprofile_can_change_user_emails.py @@ -0,0 +1,17 @@ +# Generated by Django 5.0.8 on 2024-09-02 20:36 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0615_system_bot_avatars"), + ] + + operations = [ + migrations.AddField( + model_name="userprofile", + name="can_change_user_emails", + field=models.BooleanField(db_index=True, default=False), + ), + ] diff --git a/zerver/models/users.py b/zerver/models/users.py index b32e3efe4c..406271b38f 100644 --- a/zerver/models/users.py +++ b/zerver/models/users.py @@ -571,6 +571,8 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): can_forge_sender = models.BooleanField(default=False, db_index=True) # Users with this flag set can create other users via API. can_create_users = models.BooleanField(default=False, db_index=True) + # Users with this flag can change email addresses of users in the realm via the API. + can_change_user_emails = models.BooleanField(default=False, db_index=True) # Used for rate-limiting certain automated messages generated by bots last_reminder = models.DateTimeField(default=None, null=True) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index f3b3762193..268d9394c8 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -12779,6 +12779,16 @@ paths: type: object example: [{"id": 4, "value": "0"}, {"id": 5, "value": "1909-04-05"}] + new_email: + description: | + New email address for the user. Requires the user making + the request to be an organization administrator and + additionally have the `.can_change_user_emails` special + permission. + + **Changes**: New in Zulip 10.0 (feature level 313). + type: string + example: username@example.com encoding: role: contentType: application/json diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index cb51ea802d..bbdf86fa96 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -23,6 +23,7 @@ from zerver.actions.user_settings import bulk_regenerate_api_keys, do_change_use from zerver.actions.user_topics import do_set_user_topic_visibility_policy from zerver.actions.users import ( change_user_is_active, + do_change_can_change_user_emails, do_change_can_create_users, do_change_can_forge_sender, do_change_is_billing_admin, @@ -1120,6 +1121,56 @@ class BulkCreateUserTest(ZulipTestCase): ) +class AdminChangeUserEmailTest(ZulipTestCase): + def test_change_user_email_backend(self) -> None: + cordelia = self.example_user("cordelia") + realm_admin = self.example_user("iago") + self.login_user(realm_admin) + + valid_params = dict(new_email="cordelia_new@zulip.com") + + self.assertEqual(realm_admin.can_change_user_emails, False) + result = self.client_patch(f"/json/users/{cordelia.id}", valid_params) + self.assert_json_error(result, "User not authorized to change user emails") + + do_change_can_change_user_emails(realm_admin, True) + # can_change_user_emails is insufficient without being a realm administrator: + do_change_user_role(realm_admin, UserProfile.ROLE_MEMBER, acting_user=None) + result = self.client_patch(f"/json/users/{cordelia.id}", valid_params) + self.assert_json_error(result, "Insufficient permission") + + do_change_user_role(realm_admin, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None) + result = self.client_patch( + f"/json/users/{cordelia.id}", + dict(new_email="invalid"), + ) + self.assert_json_error(result, "Invalid new email address.") + + result = self.client_patch( + f"/json/users/{UserProfile.objects.latest('id').id + 1}", + dict(new_email="new@zulip.com"), + ) + self.assert_json_error(result, "No such user") + + result = self.client_patch( + f"/json/users/{cordelia.id}", + dict(new_email=realm_admin.delivery_email), + ) + self.assert_json_error(result, "New email value error: Already has an account.") + + result = self.client_patch(f"/json/users/{cordelia.id}", valid_params) + self.assert_json_success(result) + + cordelia.refresh_from_db() + self.assertEqual(cordelia.delivery_email, "cordelia_new@zulip.com") + + last_realm_audit_log = RealmAuditLog.objects.last() + assert last_realm_audit_log is not None + self.assertEqual(last_realm_audit_log.event_type, AuditLogEventType.USER_EMAIL_CHANGED) + self.assertEqual(last_realm_audit_log.modified_user, cordelia) + self.assertEqual(last_realm_audit_log.acting_user, realm_admin) + + class AdminCreateUserTest(ZulipTestCase): def test_create_user_backend(self) -> None: # This test should give us complete coverage on diff --git a/zerver/views/users.py b/zerver/views/users.py index ce69de8feb..b16f84a298 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -4,6 +4,8 @@ from typing import Annotated, Any, TypeAlias from django.conf import settings from django.contrib.auth.models import AnonymousUser +from django.core import validators +from django.core.exceptions import ValidationError from django.core.files.uploadedfile import UploadedFile from django.db import transaction from django.http import HttpRequest, HttpResponse @@ -26,6 +28,7 @@ from zerver.actions.user_settings import ( check_change_bot_full_name, check_change_full_name, do_change_avatar_fields, + do_change_user_delivery_email, do_regenerate_api_key, ) from zerver.actions.users import ( @@ -39,7 +42,7 @@ from zerver.decorator import require_member_or_admin, require_realm_admin from zerver.forms import PASSWORD_TOO_WEAK_ERROR, CreateUserForm from zerver.lib.avatar import avatar_url, get_avatar_for_inaccessible_user, get_gravatar_url from zerver.lib.bot_config import set_bot_config -from zerver.lib.email_validation import email_allowed_for_realm +from zerver.lib.email_validation import email_allowed_for_realm, validate_email_not_already_in_realm from zerver.lib.exceptions import ( CannotDeactivateLastUserError, JsonableError, @@ -207,11 +210,17 @@ def update_user_backend( full_name: str | None = None, role: Json[RoleParamType] | None = None, profile_data: Json[list[ProfileDataElement]] | None = None, + new_email: str | None = None, ) -> HttpResponse: target = access_user_by_id( user_profile, user_id, allow_deactivated=True, allow_bots=True, for_admin=True ) + if new_email is not None and ( + not user_profile.can_change_user_emails or not user_profile.is_realm_admin + ): + raise JsonableError(_("User not authorized to change user emails")) + if role is not None and target.role != role: # Require that the current user has permissions to # grant/remove the role in question. @@ -261,6 +270,23 @@ def update_user_backend( ) do_update_user_custom_profile_data_if_changed(target, clean_profile_data) + if new_email is not None and target.delivery_email != new_email: + assert user_profile.can_change_user_emails and user_profile.is_realm_admin + try: + validators.validate_email(new_email) + except ValidationError: + raise JsonableError(_("Invalid new email address.")) + try: + validate_email_not_already_in_realm( + user_profile.realm, + new_email, + verbose=False, + ) + except ValidationError as e: + raise JsonableError(_("New email value error: {message}").format(message=e.message)) + + do_change_user_delivery_email(target, new_email, acting_user=user_profile) + return json_success(request)