api: Require can_create_users permission to create users via API.

Allowing any admins to create arbitrary users is not ideal because it
can lead to abuse issues.  We should require something stronger that
requires the server operator's approval and thus we add a new
can_create_users permission.
This commit is contained in:
Mateusz Mandera 2020-12-14 22:02:22 +01:00 committed by Tim Abbott
parent c9b6d8ddad
commit 160cc5120a
11 changed files with 87 additions and 12 deletions

View File

@ -10,6 +10,11 @@ below features are supported.
## Changes in Zulip 4.0
**Feature level 36**
* [`POST /users`](/api/create-user): Restricted access to organization
administrators with the `can_create_users` permission.
**Feature level 35**
* The peer_add and peer_remove subscription events now have plural

View File

@ -0,0 +1,12 @@
!!! warn ""
This endpoint is limited to organizations administrators who
additionally have the `can_create_users` permission for the Zulip organization.
Zulip Cloud users can request the `can_create_users` permission for a bot by contacting
[Zulip Cloud support](/help/contact-support) with an explanation for why it is needed.
Self-hosted installations can toggle `can_create_users` on an account using
the `manage.py change_user_role` command.
**Changes**: Before Zulip 4.0 (feature level 36), this endpoint was
available to all organization administrators.

View File

@ -47,6 +47,10 @@ with test_server_running(force=options.force, external_host='zulipdev.com:9981')
email = 'iago@zulip.com' # Iago is an admin
realm = get_realm('zulip')
user = get_user(email, realm)
# Required to test can_create_users endpoints.
user.can_create_users = True
user.save(update_fields=["can_create_users"])
api_key = get_api_key(user)
site = 'http://zulip.zulipdev.com:9981'
client = Client(

View File

@ -28,7 +28,7 @@ DESKTOP_WARNING_VERSION = "5.2.0"
#
# Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md.
API_FEATURE_LEVEL = 35
API_FEATURE_LEVEL = 36
# 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

View File

@ -3621,6 +3621,10 @@ def do_change_can_forge_sender(user_profile: UserProfile, value: bool) -> None:
user_profile.can_forge_sender = value
user_profile.save(update_fields=["can_forge_sender"])
def do_change_can_create_users(user_profile: UserProfile, value: bool) -> None:
user_profile.can_create_users = value
user_profile.save(update_fields=["can_create_users"])
def do_change_stream_invite_only(stream: Stream, invite_only: bool,
history_public_to_subscribers: Optional[bool]=None) -> None:
history_public_to_subscribers = get_default_value_for_history_public_to_subscribers(

View File

@ -3,7 +3,11 @@ from typing import Any
from django.core.management.base import CommandError
from zerver.lib.actions import do_change_can_forge_sender, do_change_user_role
from zerver.lib.actions import (
do_change_can_create_users,
do_change_can_forge_sender,
do_change_user_role,
)
from zerver.lib.management import ZulipBaseCommand
from zerver.models import UserProfile
@ -18,12 +22,13 @@ ONLY perform this on customer request from an authorized person.
parser.add_argument('email', metavar='<email>',
help="email of user to change role")
parser.add_argument('new_role', metavar='<new_role>',
choices=['owner', 'admin', 'member', 'guest', 'can_forge_sender'],
choices=['owner', 'admin', 'member', 'guest', 'can_forge_sender',
'can_create_users'],
help="new role of the user")
parser.add_argument('--revoke',
dest='grant',
action="store_false",
help='Remove can_forge_sender permission.')
help='Remove can_forge_sender or can_create_users permission.')
self.add_realm_args(parser, True)
def handle(self, *args: Any, **options: Any) -> None:
@ -37,7 +42,7 @@ ONLY perform this on customer request from an authorized person.
'member': UserProfile.ROLE_MEMBER,
'guest': UserProfile.ROLE_GUEST}
if options['new_role'] != 'can_forge_sender':
if options['new_role'] not in ['can_forge_sender', 'can_create_users']:
new_role = user_role_map[options['new_role']]
if not options['grant']:
raise CommandError("Revoke not supported with this permission; please specify new role.")
@ -47,11 +52,20 @@ ONLY perform this on customer request from an authorized person.
do_change_user_role(user, new_role, acting_user=None)
new_role_name = UserProfile.ROLE_ID_TO_NAME_MAP[user.role]
print(f"Role for {user.delivery_email} changed from {old_role_name} to {new_role_name}.")
else:
return
if options['new_role'] == 'can_forge_sender':
if user.can_forge_sender and options['grant']:
raise CommandError("User can already forge messages for this realm.")
elif not user.can_forge_sender and not options['grant']:
raise CommandError("User can't forge messages for this realm.")
do_change_can_forge_sender(user, options['grant'])
granted_text = "have" if options['grant'] else "not have"
print(f"{user.delivery_email} changed to {granted_text} {options['new_role']} permission.")
else:
if user.can_create_users and options['grant']:
raise CommandError("User can already create users for this realm.")
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'])

View File

@ -0,0 +1,18 @@
# Generated by Django 2.2.17 on 2020-12-20 14:18
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('zerver', '0308_remove_reduntant_realm_meta_permissions'),
]
operations = [
migrations.AddField(
model_name='userprofile',
name='can_create_users',
field=models.BooleanField(db_index=True, default=False),
),
]

View File

@ -985,6 +985,8 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
# Users with this flag set are allowed to forge messages as sent by another
# user and to send to private streams; also used for Zephyr/Jabber mirroring.
can_forge_sender: bool = models.BooleanField(default=False, db_index=True)
# Users with this flag set can create other users via API.
can_create_users: bool = models.BooleanField(default=False, db_index=True)
### Notifications settings. ###

View File

@ -4179,7 +4179,7 @@ paths:
operationId: create_user
tags: ["users"]
description: |
{!api-admin-only.md!}
{!can-create-users-only.md!}
Create a new user account via the API.

View File

@ -12,6 +12,7 @@ from django.utils.timezone import now as timezone_now
from zerver.lib.actions import (
create_users,
do_change_can_create_users,
do_change_user_role,
do_create_user,
do_deactivate_user,
@ -840,6 +841,23 @@ class AdminCreateUserTest(ZulipTestCase):
admin = self.example_user('hamlet')
realm = admin.realm
self.login_user(admin)
do_change_user_role(admin, UserProfile.ROLE_REALM_ADMINISTRATOR)
valid_params = dict(
email='romeo@zulip.net',
password='xxxx',
full_name='Romeo Montague',
)
self.assertEqual(admin.can_create_users, False)
result = self.client_post("/json/users", valid_params)
self.assert_json_error(result, "User not authorized for this query")
do_change_can_create_users(admin, True)
# can_create_users is insufficient without being a realm administrator:
do_change_user_role(admin, UserProfile.ROLE_MEMBER)
result = self.client_post("/json/users", valid_params)
self.assert_json_error(result, "Must be an organization administrator")
do_change_user_role(admin, UserProfile.ROLE_REALM_ADMINISTRATOR)
result = self.client_post("/json/users", {})
@ -882,11 +900,6 @@ class AdminCreateUserTest(ZulipTestCase):
"Email 'romeo@not-zulip.com' not allowed in this organization")
RealmDomain.objects.create(realm=get_realm('zulip'), domain='zulip.net')
valid_params = dict(
email='romeo@zulip.net',
password='xxxx',
full_name='Romeo Montague',
)
# Check can't use a bad password with zxcvbn enabled
with self.settings(PASSWORD_MIN_LENGTH=6, PASSWORD_MIN_GUESSES=1000):
result = self.client_post("/json/users", valid_params)

View File

@ -493,6 +493,9 @@ def create_user_backend(
password: str=REQ(),
full_name_raw: str=REQ("full_name"),
) -> HttpResponse:
if not user_profile.can_create_users:
return json_error(_("User not authorized for this query"))
full_name = check_full_name(full_name_raw)
form = CreateUserForm({'full_name': full_name, 'email': email})
if not form.is_valid():