diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 5707b09844..494affedb9 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 9.0 +**Feature level 246** + +* [`POST /register`](/api/register-queue), [`POST + /events`](/api/get-events): Added new `require_unique_names` setting + controlling whether users names can duplicate others. + **Feature level 245** * [`PATCH diff --git a/version.py b/version.py index e9bb94597f..4c092e7137 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 245 +API_FEATURE_LEVEL = 246 # 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/web/src/admin.js b/web/src/admin.js index 73a47ecb8b..7cc89e993a 100644 --- a/web/src/admin.js +++ b/web/src/admin.js @@ -50,6 +50,7 @@ const admin_settings_label = { realm_default_code_block_language: $t({defaultMessage: "Default language for code blocks"}), // Organization permissions + realm_require_unique_names: $t({defaultMessage: "Require unique names"}), realm_name_changes_disabled: $t({defaultMessage: "Prevent users from changing their name"}), realm_email_changes_disabled: $t({ defaultMessage: "Prevent users from changing their email address", @@ -118,6 +119,7 @@ export function build_page() { server_inline_url_embed_preview: realm.server_inline_url_embed_preview, realm_authentication_methods: realm.realm_authentication_methods, realm_name_changes_disabled: realm.realm_name_changes_disabled, + realm_require_unique_names: realm.realm_require_unique_names, realm_email_changes_disabled: realm.realm_email_changes_disabled, realm_avatar_changes_disabled: realm.realm_avatar_changes_disabled, realm_add_custom_emoji_policy: realm.realm_add_custom_emoji_policy, diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index c428e55633..2924a4ec0d 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -239,6 +239,7 @@ export function dispatch_normal_event(event) { org_type: noop, private_message_policy: compose_recipient.check_posting_policy_for_compose_box, push_notifications_enabled: noop, + require_unique_names: noop, send_welcome_emails: noop, message_content_allowed_in_email_notifications: noop, enable_spectator_access: noop, diff --git a/web/src/state_data.ts b/web/src/state_data.ts index 5d4c2fbce2..b8bcdccf62 100644 --- a/web/src/state_data.ts +++ b/web/src/state_data.ts @@ -179,6 +179,7 @@ export const realm_schema = z.object({ realm_presence_disabled: z.boolean(), realm_private_message_policy: z.number(), realm_push_notifications_enabled: z.boolean(), + realm_require_unique_names: z.boolean(), realm_signup_announcements_stream_id: z.number(), realm_upload_quota_mib: z.nullable(z.number()), realm_uri: z.string(), diff --git a/web/src/ui_init.js b/web/src/ui_init.js index 2702b4148c..f4701f2cf3 100644 --- a/web/src/ui_init.js +++ b/web/src/ui_init.js @@ -611,6 +611,7 @@ export function initialize_everything(state_data) { "realm_private_message_policy", "realm_push_notifications_enabled", "realm_push_notifications_enabled_end_timestamp", + "realm_require_unique_names", "realm_send_welcome_emails", "realm_signup_announcements_stream_id", "realm_upload_quota_mib", diff --git a/web/templates/settings/organization_permissions_admin.hbs b/web/templates/settings/organization_permissions_admin.hbs index 94a55666a4..b854f641e7 100644 --- a/web/templates/settings/organization_permissions_admin.hbs +++ b/web/templates/settings/organization_permissions_admin.hbs @@ -261,6 +261,12 @@ {{> settings_save_discard_widget section_name="user-identity" }}
+ {{> settings_checkbox + setting_name="realm_require_unique_names" + prefix="id_" + is_checked=realm_require_unique_names + label=admin_settings_label.realm_require_unique_names}} + {{> settings_checkbox setting_name="realm_name_changes_disabled" prefix="id_" diff --git a/zerver/actions/user_settings.py b/zerver/actions/user_settings.py index aa23990884..b948c844b3 100644 --- a/zerver/actions/user_settings.py +++ b/zerver/actions/user_settings.py @@ -249,7 +249,9 @@ def check_change_full_name( is responsible for checking check permissions. Returns the new full name, which may differ from what was passed in (because this function strips whitespace).""" - new_full_name = check_full_name(full_name_raw) + new_full_name = check_full_name( + full_name_raw=full_name_raw, user_profile=user_profile, realm=user_profile.realm + ) do_change_full_name(user_profile, new_full_name, acting_user) return new_full_name @@ -257,7 +259,9 @@ def check_change_full_name( def check_change_bot_full_name( user_profile: UserProfile, full_name_raw: str, acting_user: UserProfile ) -> None: - new_full_name = check_full_name(full_name_raw) + new_full_name = check_full_name( + full_name_raw=full_name_raw, user_profile=user_profile, realm=user_profile.realm + ) if new_full_name == user_profile.full_name: # Our web app will try to patch full_name even if the user didn't diff --git a/zerver/forms.py b/zerver/forms.py index ce56796561..9f38874ff0 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -180,6 +180,11 @@ class RegistrationForm(RealmDetailsForm): ) def __init__(self, *args: Any, **kwargs: Any) -> None: + # Since the superclass doesn't except random extra kwargs, we + # remove it from the kwargs dict before initializing. + self.realm_creation = kwargs["realm_creation"] + self.realm = kwargs.pop("realm", None) + super().__init__(*args, **kwargs) if settings.TERMS_OF_SERVICE_VERSION is not None: self.fields["terms"] = forms.BooleanField(required=True) @@ -211,7 +216,9 @@ class RegistrationForm(RealmDetailsForm): def clean_full_name(self) -> str: try: - return check_full_name(self.cleaned_data["full_name"]) + return check_full_name( + full_name_raw=self.cleaned_data["full_name"], user_profile=None, realm=self.realm + ) except JsonableError as e: raise ValidationError(e.msg) diff --git a/zerver/lib/users.py b/zerver/lib/users.py index a63d14b313..3d16dfec74 100644 --- a/zerver/lib/users.py +++ b/zerver/lib/users.py @@ -38,7 +38,7 @@ from zerver.models import ( UserProfile, ) from zerver.models.groups import SystemGroups -from zerver.models.realms import get_fake_email_domain +from zerver.models.realms import get_fake_email_domain, require_unique_names from zerver.models.users import ( active_non_guest_user_ids, active_user_ids, @@ -50,7 +50,9 @@ from zerver.models.users import ( ) -def check_full_name(full_name_raw: str) -> str: +def check_full_name( + full_name_raw: str, *, user_profile: Optional[UserProfile], realm: Optional[Realm] +) -> str: full_name = full_name_raw.strip() if len(full_name) > UserProfile.MAX_NAME_LENGTH: raise JsonableError(_("Name too long!")) @@ -65,6 +67,26 @@ def check_full_name(full_name_raw: str) -> str: # ban them. if re.search(r"\|\d+$", full_name_raw): raise JsonableError(_("Invalid format!")) + + if require_unique_names(realm): + normalized_user_full_name = unicodedata.normalize("NFKC", full_name).casefold() + users_query = UserProfile.objects.filter(realm=realm) + # We want to exclude the user's full name while checking for + # uniqueness. + if user_profile is not None: + existing_names = users_query.exclude(id=user_profile.id).values_list( + "full_name", flat=True + ) + else: + existing_names = users_query.values_list("full_name", flat=True) + + normalized_existing_names = [ + unicodedata.normalize("NFKC", full_name).casefold() for full_name in existing_names + ] + + if normalized_user_full_name in normalized_existing_names: + raise JsonableError(_("Unique names required in this organization.")) + return full_name diff --git a/zerver/migrations/0506_realm_require_unique_names.py b/zerver/migrations/0506_realm_require_unique_names.py new file mode 100644 index 0000000000..6cbebfb809 --- /dev/null +++ b/zerver/migrations/0506_realm_require_unique_names.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.11 on 2024-03-27 20:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0505_realmuserdefault_web_font_size_px_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="realm", + name="require_unique_names", + field=models.BooleanField(default=False), + ), + ] diff --git a/zerver/models/realms.py b/zerver/models/realms.py index e0136c5683..51e0a93ab7 100644 --- a/zerver/models/realms.py +++ b/zerver/models/realms.py @@ -167,6 +167,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub mandatory_topics = models.BooleanField(default=False) + require_unique_names = models.BooleanField(default=False) name_changes_disabled = models.BooleanField(default=False) email_changes_disabled = models.BooleanField(default=False) avatar_changes_disabled = models.BooleanField(default=False) @@ -627,6 +628,7 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub name_changes_disabled=bool, private_message_policy=int, push_notifications_enabled=bool, + require_unique_names=bool, send_welcome_emails=bool, user_group_edit_policy=int, video_chat_provider=int, @@ -964,6 +966,13 @@ def get_realm_by_id(realm_id: int) -> Realm: return Realm.objects.get(id=realm_id) +def require_unique_names(realm: Optional[Realm]) -> bool: + if realm is None: + # realm is None when a new realm is being created. + return False + return realm.require_unique_names + + def name_changes_disabled(realm: Optional[Realm]) -> bool: if realm is None: return settings.NAME_CHANGES_DISABLED diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 9ba052a0ae..e18195687d 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -4540,6 +4540,17 @@ paths: - 2 = Nobody **Changes**: New in Zulip 3.0 (feature level 1). + require_unique_names: + type: boolean + description: | + Indicates whether the organization is configured to require users to have + unique full names. If true, the server will reject attempts to create a + new user, or change the name of an existing user, where doing so would + lead to two users whose names are identical modulo case and unicode + normalization. + + **Changes**: New in Zulip 9.0 (feature level 246). Previously, the Zulip + server could not be configured to enforce unique names. send_welcome_emails: type: boolean description: | @@ -14640,6 +14651,17 @@ paths: Present if `realm` is present in `fetch_event_types`. The name of the organization, used in login pages etc. + realm_require_unique_names: + type: boolean + description: | + Indicates whether the organization is configured to require users + to have unique full names. If true, the server will reject attempts + to create a new user, or change the name of an existing user, where + doing so would lead to two users whose names are identical modulo + case and unicode normalization. + + **Changes**: New in Zulip 9.0 (feature level 246). Previously, the Zulip + server could not be configured to enforce unique names. realm_name_changes_disabled: type: boolean description: | diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 9bb7e2c39d..38a22c6285 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -187,6 +187,7 @@ class HomeTest(ZulipTestCase): "realm_private_message_policy", "realm_push_notifications_enabled", "realm_push_notifications_enabled_end_timestamp", + "realm_require_unique_names", "realm_send_welcome_emails", "realm_signup_announcements_stream_id", "realm_upload_quota_mib", diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index ecc18abb23..92e5cfba88 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -2421,6 +2421,25 @@ class UserSignUpTest(ZulipTestCase): # Verify that the user is asked for name and password self.assert_in_success_response(["id_password", "id_full_name"], result) + def test_signup_with_existing_name(self) -> None: + """ + Check if signing up with an existing name when organization + has set "Require Unique Names"is handled properly. + """ + + iago = self.example_user("iago") + email = "newguy@zulip.com" + password = "newpassword" + + do_set_realm_property(iago.realm, "require_unique_names", True, acting_user=None) + result = self.verify_signup(email=email, password=password, full_name="IaGo") + assert not isinstance(result, UserProfile) + self.assert_in_success_response(["Unique names required in this organization."], result) + + do_set_realm_property(iago.realm, "require_unique_names", False, acting_user=None) + result = self.verify_signup(email=email, password=password, full_name="IaGo") + assert isinstance(result, UserProfile) + def test_signup_without_password(self) -> None: """ Check if signing up without a password works properly when diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 7aea293cc6..ac29064afb 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -414,6 +414,45 @@ class PermissionTest(ZulipTestCase): result = self.client_patch("/json/users/{}".format(self.example_user("hamlet").id), req) self.assert_json_success(result) + def test_require_unique_names(self) -> None: + self.login("desdemona") + iago = self.example_user("iago") + hamlet = self.example_user("hamlet") + + do_set_realm_property(hamlet.realm, "require_unique_names", True, acting_user=None) + req = dict(full_name="IaGo") + result = self.client_patch(f"/json/users/{hamlet.id}", req) + self.assert_json_error(result, "Unique names required in this organization.") + + req = dict(full_name="π•šπ•’π•˜π• ") + result = self.client_patch(f"/json/users/{hamlet.id}", req) + self.assert_json_error(result, "Unique names required in this organization.") + + req = dict(full_name="ｉａｇｏ") + result = self.client_patch(f"/json/users/{hamlet.id}", req) + self.assert_json_error(result, "Unique names required in this organization.") + + req = dict(full_name="π’Ύπ’Άπ‘”π‘œ") + result = self.client_patch(f"/json/users/{hamlet.id}", req) + self.assert_json_error(result, "Unique names required in this organization.") + + # check for uniqueness including imported users + iago.is_mirror_dummy = True + req = dict(full_name="iago") + result = self.client_patch(f"/json/users/{hamlet.id}", req) + self.assert_json_error(result, "Unique names required in this organization.") + + # check for uniqueness including deactivated users + do_deactivate_user(iago, acting_user=None) + req = dict(full_name="iago") + result = self.client_patch(f"/json/users/{hamlet.id}", req) + self.assert_json_error(result, "Unique names required in this organization.") + + do_set_realm_property(hamlet.realm, "require_unique_names", False, acting_user=None) + req = dict(full_name="iago") + result = self.client_patch(f"/json/users/{hamlet.id}", req) + self.assert_json_success(result) + def test_not_allowed_format_complex(self) -> None: new_name = "Hello- 12iago|72" self.login("iago") diff --git a/zerver/views/realm.py b/zerver/views/realm.py index cada34a8f0..f59feb538a 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -82,6 +82,7 @@ def update_realm( create_multiuse_invite_group_id: Optional[int] = REQ( "create_multiuse_invite_group", json_validator=check_int, default=None ), + require_unique_names: Optional[bool] = REQ(json_validator=check_bool, default=None), name_changes_disabled: Optional[bool] = REQ(json_validator=check_bool, default=None), email_changes_disabled: Optional[bool] = REQ(json_validator=check_bool, default=None), avatar_changes_disabled: Optional[bool] = REQ(json_validator=check_bool, default=None), diff --git a/zerver/views/registration.py b/zerver/views/registration.py index 16478d3f06..2812c30077 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -394,7 +394,10 @@ def registration_helper( # so they can be directly registered without having to go through # this interstitial. form = RegistrationForm( - {"full_name": ldap_full_name}, initial=initial_data, realm_creation=realm_creation + {"full_name": ldap_full_name}, + initial=initial_data, + realm_creation=realm_creation, + realm=realm, ) request.session["authenticated_full_name"] = ldap_full_name name_validated = True @@ -407,6 +410,7 @@ def registration_helper( form = RegistrationForm( initial={"full_name": hesiod_name if "@" not in hesiod_name else ""}, realm_creation=realm_creation, + realm=realm, ) name_validated = True elif prereg_user is not None and prereg_user.full_name: @@ -417,21 +421,26 @@ def registration_helper( {"full_name": prereg_user.full_name}, initial=initial_data, realm_creation=realm_creation, + realm=realm, ) else: initial_data["full_name"] = prereg_user.full_name form = RegistrationForm( initial=initial_data, realm_creation=realm_creation, + realm=realm, ) elif form_full_name is not None: initial_data["full_name"] = form_full_name form = RegistrationForm( initial=initial_data, realm_creation=realm_creation, + realm=realm, ) else: - form = RegistrationForm(initial=initial_data, realm_creation=realm_creation) + form = RegistrationForm( + initial=initial_data, realm_creation=realm_creation, realm=realm + ) else: postdata = request.POST.copy() if name_changes_disabled(realm): @@ -443,7 +452,7 @@ def registration_helper( name_validated = True except KeyError: pass - form = RegistrationForm(postdata, realm_creation=realm_creation) + form = RegistrationForm(postdata, realm_creation=realm_creation, realm=realm) if not (password_auth_enabled(realm) and password_required): form["password"].field.required = False diff --git a/zerver/views/users.py b/zerver/views/users.py index 0c38b2caa7..319461f203 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -490,7 +490,9 @@ def add_bot_backend( if bot_type != UserProfile.INCOMING_WEBHOOK_BOT: service_name = service_name or short_name short_name += "-bot" - full_name = check_full_name(full_name_raw) + full_name = check_full_name( + full_name_raw=full_name_raw, user_profile=user_profile, realm=user_profile.realm + ) try: email = Address(username=short_name, domain=user_profile.realm.get_bot_domain()).addr_spec except InvalidFakeEmailDomainError: @@ -695,7 +697,9 @@ def create_user_backend( if not user_profile.can_create_users: raise JsonableError(_("User not authorized to create users")) - full_name = check_full_name(full_name_raw) + full_name = check_full_name( + full_name_raw=full_name_raw, user_profile=user_profile, realm=user_profile.realm + ) form = CreateUserForm({"full_name": full_name, "email": email}) if not form.is_valid(): raise JsonableError(_("Bad name or username")) diff --git a/zproject/backends.py b/zproject/backends.py index 46d9c0d86c..5dd1c2c592 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -930,7 +930,9 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): full_name = self.get_mapped_name(ldap_user) if full_name != user_profile.full_name: try: - full_name = check_full_name(full_name) + full_name = check_full_name( + full_name_raw=full_name, user_profile=user_profile, realm=user_profile.realm + ) except JsonableError as e: raise ZulipLDAPError(e.msg) do_change_full_name(user_profile, full_name, None) @@ -1148,7 +1150,9 @@ class ZulipLDAPAuthBackend(ZulipLDAPAuthBackendBase): # We have valid LDAP credentials; time to create an account. full_name = self.get_mapped_name(ldap_user) try: - full_name = check_full_name(full_name) + full_name = check_full_name( + full_name_raw=full_name, user_profile=None, realm=self._realm + ) except JsonableError as e: raise ZulipLDAPError(e.msg)