mirror of
https://github.com/zulip/zulip.git
synced 2025-10-23 04:52:12 +00:00
update_user_backend: Allow authorized org owners to change user emails.
This adds a new special UserProfile flag can_change_user_emails(disabled by default) and the ability for changing the email address of users in the realm via update_user_backend. This is useful for allowing organizations to update user emails without needing to set up a SCIM integration, but since it gives the ability to hijack user accounts, it needs to be behind this additional permission and can't be just given to organization owners by default. Analogical to how the create_user_backend endpoint works.
This commit is contained in:
committed by
Tim Abbott
parent
8e9c592ce3
commit
389b851f81
@@ -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(
|
||||
|
@@ -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
|
||||
|
@@ -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
|
||||
|
@@ -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"]:
|
||||
|
17
zerver/migrations/0616_userprofile_can_change_user_emails.py
Normal file
17
zerver/migrations/0616_userprofile_can_change_user_emails.py
Normal file
@@ -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),
|
||||
),
|
||||
]
|
@@ -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)
|
||||
|
@@ -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
|
||||
|
@@ -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
|
||||
|
@@ -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)
|
||||
|
||||
|
||||
|
Reference in New Issue
Block a user