/json/users: Replace email with user_id in API to update/remove users.

This commit is contained in:
Yashashvi Dave
2018-05-17 23:06:33 +05:30
committed by Tim Abbott
parent b8e2339a65
commit 06e7e933cc
7 changed files with 42 additions and 51 deletions

View File

@@ -245,7 +245,8 @@ exports.on_load_success = function (realm_people_data) {
});
$("#do_deactivate_user_button").expectOne().click(function () {
var email = meta.current_deactivate_user_modal_row.find(".email").text();
var email = meta.current_deactivate_user_modal_row.attr("data-email");
var user_id = meta.current_deactivate_user_modal_row.attr("data-user-id");
if ($("#deactivation_user_modal .email").html() !== email) {
blueslip.error("User deactivation canceled due to non-matching fields.");
@@ -255,7 +256,7 @@ exports.on_load_success = function (realm_people_data) {
$("#deactivation_user_modal").modal("hide");
meta.current_deactivate_user_modal_row.find("button").eq(0).prop("disabled", true).text(i18n.t("Working…"));
channel.del({
url: '/json/users/' + encodeURIComponent(email),
url: '/json/users/' + encodeURIComponent(user_id),
error: function (xhr) {
var status = $("#organization-status").expectOne();
ui_report.error(i18n.t("Failed"), xhr, status);
@@ -316,11 +317,11 @@ exports.on_load_success = function (realm_people_data) {
e.preventDefault();
e.stopPropagation();
// Go up the tree until we find the user row, then grab the email element
// Go up the tree until we find the user row, then grab the user_id data
var row = $(e.target).closest(".user_row");
var email = get_email_for_user_row(row);
var user_id = row.attr("data-user-id");
var url = "/json/users/" + encodeURIComponent(email);
var url = "/json/users/" + encodeURIComponent(user_id);
var data = {
is_admin: JSON.stringify(true),
};
@@ -347,11 +348,11 @@ exports.on_load_success = function (realm_people_data) {
e.preventDefault();
e.stopPropagation();
// Go up the tree until we find the user row, then grab the email element
// Go up the tree until we find the user row, then grab the user_id data
var row = $(e.target).closest(".user_row");
var email = get_email_for_user_row(row);
var user_id = row.attr("data-user-id");
var url = "/json/users/" + encodeURIComponent(email);
var url = "/json/users/" + encodeURIComponent(user_id);
var data = {
is_admin: JSON.stringify(false),
};
@@ -424,7 +425,7 @@ exports.on_load_success = function (realm_people_data) {
data.bot_owner = owner_select.val();
}
} else {
url = "/json/users/" + encodeURIComponent(person.email);
url = "/json/users/" + encodeURIComponent(person.user_id);
data = {
full_name: JSON.stringify(full_name.val()),
};

View File

@@ -1524,6 +1524,9 @@ def get_user_profile_by_api_key(api_key: str) -> UserProfile:
def get_user(email: str, realm: Realm) -> UserProfile:
return UserProfile.objects.select_related().get(email__iexact=email.strip(), realm=realm)
def get_user_profile_by_id_in_realm(uid: int, realm: Realm) -> UserProfile:
return UserProfile.objects.select_related().get(id=uid, realm=realm)
def get_user_including_cross_realm(email: str, realm: Optional[Realm]=None) -> UserProfile:
if is_cross_realm_bot_email(email):
return get_system_bot(email)

View File

@@ -67,7 +67,7 @@ class TestRealmAuditLog(ZulipTestCase):
new_name = 'George Hamletovich'
self.login(self.example_email("iago"))
req = dict(full_name=ujson.dumps(new_name))
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user("hamlet").id), req)
self.assertTrue(result.status_code == 200)
query = RealmAuditLog.objects.filter(event_type='user_full_name_changed',
event_time__gte=start)

View File

@@ -513,7 +513,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(1)
# Cannot deactivate a bot as a user
result = self.client_delete("/json/users/{}".format(email))
result = self.client_delete("/json/users/{}".format(self.get_bot_user(email).id))
self.assert_json_error(result, 'No such user')
self.assert_num_bots_equal(1)

View File

@@ -88,7 +88,8 @@ class PermissionTest(ZulipTestCase):
admin = self.example_user('hamlet')
do_change_is_admin(admin, True)
result = self.client_patch('/json/users/nonexistentuser@zulip.com', {})
invalid_user_id = 1000
result = self.client_patch('/json/users/{}'.format(invalid_user_id), {})
self.assert_json_error(result, 'No such user')
def test_admin_api(self) -> None:
@@ -112,7 +113,7 @@ class PermissionTest(ZulipTestCase):
events = [] # type: List[Mapping[str, Any]]
with tornado_redirected_to_list(events):
result = self.client_patch('/json/users/othello@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user("othello").id), req)
self.assert_json_success(result)
admin_users = realm.get_admin_users()
self.assertTrue(user in admin_users)
@@ -124,7 +125,7 @@ class PermissionTest(ZulipTestCase):
req = dict(is_admin=ujson.dumps(False))
events = []
with tornado_redirected_to_list(events):
result = self.client_patch('/json/users/othello@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user("othello").id), req)
self.assert_json_success(result)
admin_users = realm.get_admin_users()
self.assertFalse(user in admin_users)
@@ -137,7 +138,7 @@ class PermissionTest(ZulipTestCase):
req = dict(is_admin=ujson.dumps(False))
events = []
with tornado_redirected_to_list(events):
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user("hamlet").id), req)
self.assert_json_success(result)
admin_users = realm.get_admin_users()
self.assertFalse(admin in admin_users)
@@ -145,25 +146,26 @@ class PermissionTest(ZulipTestCase):
self.assertEqual(person['email'], self.example_email("hamlet"))
self.assertEqual(person['is_admin'], False)
with tornado_redirected_to_list([]):
result = self.client_patch('/json/users/iago@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user("iago").id), req)
self.assert_json_error(result, 'Cannot remove the only organization administrator')
# Make sure only admins can patch other user's info.
self.login(self.example_email("othello"))
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user("hamlet").id), req)
self.assert_json_error(result, 'Insufficient permission')
def test_user_cannot_promote_to_admin(self) -> None:
self.login(self.example_email("hamlet"))
req = dict(is_admin=ujson.dumps(True))
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user('hamlet').id), req)
self.assert_json_error(result, 'Insufficient permission')
def test_admin_user_can_change_full_name(self) -> None:
new_name = 'new name'
self.login(self.example_email("iago"))
hamlet = self.example_user('hamlet')
req = dict(full_name=ujson.dumps(new_name))
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
self.assertTrue(result.status_code == 200)
hamlet = self.example_user('hamlet')
self.assertEqual(hamlet.full_name, new_name)
@@ -171,28 +173,28 @@ class PermissionTest(ZulipTestCase):
def test_non_admin_cannot_change_full_name(self) -> None:
self.login(self.example_email("hamlet"))
req = dict(full_name=ujson.dumps('new name'))
result = self.client_patch('/json/users/othello@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user('othello').id), req)
self.assert_json_error(result, 'Insufficient permission')
def test_admin_cannot_set_long_full_name(self) -> None:
new_name = 'a' * (UserProfile.MAX_NAME_LENGTH + 1)
self.login(self.example_email("iago"))
req = dict(full_name=ujson.dumps(new_name))
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user('hamlet').id), req)
self.assert_json_error(result, 'Name too long!')
def test_admin_cannot_set_short_full_name(self) -> None:
new_name = 'a'
self.login(self.example_email("iago"))
req = dict(full_name=ujson.dumps(new_name))
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user('hamlet').id), req)
self.assert_json_error(result, 'Name too short!')
def test_admin_cannot_set_full_name_with_invalid_characters(self) -> None:
new_name = 'Opheli*'
self.login(self.example_email("iago"))
req = dict(full_name=ujson.dumps(new_name))
result = self.client_patch('/json/users/hamlet@zulip.com', req)
result = self.client_patch('/json/users/{}'.format(self.example_user('hamlet').id), req)
self.assert_json_error(result, 'Invalid characters in name!')
class AdminCreateUserTest(ZulipTestCase):
@@ -343,7 +345,7 @@ class ActivateTest(ZulipTestCase):
user = self.example_user('hamlet')
self.assertTrue(user.is_active)
result = self.client_delete('/json/users/hamlet@zulip.com')
result = self.client_delete('/json/users/{}'.format(user.id))
self.assert_json_success(result)
user = self.example_user('hamlet')
self.assertFalse(user.is_active)
@@ -353,22 +355,6 @@ class ActivateTest(ZulipTestCase):
user = self.example_user('hamlet')
self.assertTrue(user.is_active)
def test_api_me_user(self) -> None:
"""This test helps ensure that our URL patterns for /users/me URLs
handle email addresses starting with "me" correctly."""
self.register(self.nonreg_email('me'), "testpassword")
self.login(self.example_email("iago"))
result = self.client_delete('/json/users/me@zulip.com')
self.assert_json_success(result)
user = self.nonreg_user('me')
self.assertFalse(user.is_active)
result = self.client_post('/json/users/{email}/reactivate'.format(email=self.nonreg_email('me')))
self.assert_json_success(result)
user = self.nonreg_user('me')
self.assertTrue(user.is_active)
def test_api_with_nonexistent_user(self) -> None:
admin = self.example_user('othello')
do_change_is_admin(admin, True)
@@ -379,16 +365,17 @@ class ActivateTest(ZulipTestCase):
self.assert_json_error(result, 'No such bot')
# Cannot deactivate a nonexistent user.
result = self.client_delete('/json/users/nonexistent@zulip.com')
invalid_user_id = 1000
result = self.client_delete('/json/users/{}'.format(invalid_user_id))
self.assert_json_error(result, 'No such user')
result = self.client_delete('/json/users/{}'.format(self.example_email("webhook_bot")))
result = self.client_delete('/json/users/{}'.format(self.example_user("webhook_bot").id))
self.assert_json_error(result, 'No such user')
result = self.client_delete('/json/users/iago@zulip.com')
result = self.client_delete('/json/users/{}'.format(self.example_user("iago").id))
self.assert_json_success(result)
result = self.client_delete('/json/users/othello@zulip.com')
result = self.client_delete('/json/users/{}'.format(admin.id))
self.assert_json_error(result, 'Cannot deactivate the only organization administrator')
# Cannot reactivate a nonexistent user.
@@ -401,7 +388,7 @@ class ActivateTest(ZulipTestCase):
self.login(self.example_email("othello"))
# Cannot deactivate a user with the users api
result = self.client_delete('/json/users/hamlet@zulip.com')
result = self.client_delete('/json/users/{}'.format(self.example_user("hamlet").id))
self.assert_json_error(result, 'Insufficient permission')
# Cannot reactivate a user

View File

@@ -33,14 +33,14 @@ from zerver.lib.users import check_valid_bot_type, check_bot_creation_policy, \
from zerver.lib.utils import generate_random_token
from zerver.models import UserProfile, Stream, Message, email_allowed_for_realm, \
get_user_profile_by_id, get_user, Service, get_user_including_cross_realm, \
DomainNotAllowedForRealmError, DisposableEmailError
DomainNotAllowedForRealmError, DisposableEmailError, get_user_profile_by_id_in_realm
from zerver.lib.create_user import random_api_key
def deactivate_user_backend(request: HttpRequest, user_profile: UserProfile,
email: str) -> HttpResponse:
user_id: int) -> HttpResponse:
try:
target = get_user(email, user_profile.realm)
target = get_user_profile_by_id_in_realm(user_id, user_profile.realm)
except UserProfile.DoesNotExist:
return json_error(_('No such user'))
if target.is_bot:
@@ -92,11 +92,11 @@ def reactivate_user_backend(request: HttpRequest, user_profile: UserProfile,
return json_success()
@has_request_variables
def update_user_backend(request: HttpRequest, user_profile: UserProfile, email: str,
def update_user_backend(request: HttpRequest, user_profile: UserProfile, user_id: int,
full_name: Optional[str]=REQ(default="", validator=check_string),
is_admin: Optional[bool]=REQ(default=None, validator=check_bool)) -> HttpResponse:
try:
target = get_user(email, user_profile.realm)
target = get_user_profile_by_id_in_realm(user_id, user_profile.realm)
except UserProfile.DoesNotExist:
return json_error(_('No such user'))

View File

@@ -130,7 +130,7 @@ v1_api_and_json_patterns = [
{'POST': 'zerver.views.users.reactivate_user_backend'}),
url(r'^users/(?!me/)(?P<email>[^/]*)/presence$', rest_dispatch,
{'GET': 'zerver.views.presence.get_presence_backend'}),
url(r'^users/(?!me$)(?P<email>[^/]*)$', rest_dispatch,
url(r'^users/(?P<user_id>[0-9]+)$', rest_dispatch,
{'PATCH': 'zerver.views.users.update_user_backend',
'DELETE': 'zerver.views.users.deactivate_user_backend'}),
url(r'^bots$', rest_dispatch,