From 2283b5fc916dbf26d98ba6b3e44e802921ecbdff Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Tue, 7 Feb 2017 19:39:55 -0800 Subject: [PATCH] users: Consolidate name change enforcement logic. This has the side effect of fixing an issue where one could edit a bot to have an invalid name. --- zerver/lib/users.py | 22 ++++++++++++++++++++++ zerver/views/user_settings.py | 12 ++++-------- zerver/views/users.py | 12 ++++-------- 3 files changed, 30 insertions(+), 16 deletions(-) create mode 100644 zerver/lib/users.py diff --git a/zerver/lib/users.py b/zerver/lib/users.py new file mode 100644 index 0000000000..ed05722707 --- /dev/null +++ b/zerver/lib/users.py @@ -0,0 +1,22 @@ +from __future__ import absolute_import +from typing import Text + +from django.utils.translation import ugettext as _ + +from zerver.lib.actions import do_change_full_name +from zerver.lib.request import JsonableError +from zerver.models import UserProfile + +def check_change_full_name(user_profile, full_name): + # type: (UserProfile, Text) -> Text + """Verifies that the user's proposed full name is valid. The caller + 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 = full_name.strip() + if len(new_full_name) > UserProfile.MAX_NAME_LENGTH: + raise JsonableError(_("Name too long!")) + if list(set(new_full_name).intersection(UserProfile.NAME_INVALID_CHARS)): + raise JsonableError(_("Invalid characters in name!")) + do_change_full_name(user_profile, new_full_name) + return new_full_name diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 2fdf5a5cac..574dd73cd2 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -9,7 +9,7 @@ from django.http import HttpRequest, HttpResponse from zerver.decorator import authenticated_json_post_view, has_request_variables, REQ from zerver.lib.actions import do_change_password, \ - do_change_full_name, do_change_enable_desktop_notifications, \ + do_change_enable_desktop_notifications, \ do_change_enter_sends, do_change_enable_sounds, \ do_change_enable_offline_email_notifications, do_change_enable_digest_emails, \ do_change_enable_offline_push_notifications, do_change_enable_online_push_notifications, \ @@ -24,6 +24,7 @@ from zerver.lib.response import json_success, json_error from zerver.lib.upload import upload_avatar_image from zerver.lib.validator import check_bool, check_string from zerver.lib.request import JsonableError +from zerver.lib.users import check_change_full_name from zerver.models import UserProfile, Realm, name_changes_disabled @has_request_variables @@ -87,13 +88,8 @@ def json_change_settings(request, user_profile, # they'd have to be trying to break the rules. pass else: - new_full_name = full_name.strip() - if len(new_full_name) > UserProfile.MAX_NAME_LENGTH: - return json_error(_("Name too long!")) - elif list(set(new_full_name).intersection(UserProfile.NAME_INVALID_CHARS)): - return json_error(_("Invalid characters in name!")) - do_change_full_name(user_profile, new_full_name) - result['full_name'] = new_full_name + # Note that check_change_full_name strips the passed name automatically + result['full_name'] = check_change_full_name(user_profile, full_name) return json_success(result) diff --git a/zerver/views/users.py b/zerver/views/users.py index 59b169259b..980cb7b5e4 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -14,7 +14,7 @@ from six.moves import map from zerver.decorator import has_request_variables, REQ, JsonableError, \ require_realm_admin from zerver.forms import CreateUserForm -from zerver.lib.actions import do_change_full_name, do_change_is_admin, \ +from zerver.lib.actions import do_change_is_admin, \ do_create_user, do_deactivate_user, do_reactivate_user, \ do_change_default_events_register_stream, do_change_default_sending_stream, \ do_change_default_all_public_streams, do_regenerate_api_key, do_change_avatar_source @@ -23,6 +23,7 @@ from zerver.lib.response import json_error, json_success from zerver.lib.streams import access_stream_by_name from zerver.lib.upload import upload_avatar_image from zerver.lib.validator import check_bool, check_string +from zerver.lib.users import check_change_full_name from zerver.lib.utils import generate_random_token from zerver.models import UserProfile, Stream, Realm, Message, get_user_profile_by_email, \ email_allowed_for_realm @@ -107,12 +108,7 @@ def update_user_backend(request, user_profile, email, full_name.strip() != ""): # We don't respect `name_changes_disabled` here because the request # is on behalf of the administrator. - new_full_name = full_name.strip() - if len(new_full_name) > UserProfile.MAX_NAME_LENGTH: - return json_error(_("Name too long!")) - elif list(set(new_full_name).intersection(UserProfile.NAME_INVALID_CHARS)): - return json_error(_("Invalid characters in name!")) - do_change_full_name(target, new_full_name) + check_change_full_name(target, full_name) return json_success() @@ -157,7 +153,7 @@ def patch_bot_backend(request, user_profile, email, return json_error(_('Insufficient permission')) if full_name is not None: - do_change_full_name(bot, full_name) + check_change_full_name(bot, full_name) if default_sending_stream is not None: if default_sending_stream == "": stream = None