diff --git a/config.py b/config.py index 2a4d1cceb..79f2bf8a0 100644 --- a/config.py +++ b/config.py @@ -220,6 +220,10 @@ class DefaultConfig(ImmutableConfig): # Feature Flag: Whether users can be created (by non-super users). FEATURE_USER_CREATION = True + # Feature Flag: Whether users being created must be invited by another user. If FEATURE_USER_CREATION is off, + # this flag has no effect. + FEATURE_INVITE_ONLY_USER_CREATION = False + # Feature Flag: Whether users can be renamed FEATURE_USER_RENAME = False diff --git a/data/users/federated.py b/data/users/federated.py index 3c61a5837..f2b98ca82 100644 --- a/data/users/federated.py +++ b/data/users/federated.py @@ -4,6 +4,7 @@ import features from collections import namedtuple from data import model +from data.users.shared import can_create_user from util.validation import generate_valid_usernames logger = logging.getLogger(__name__) @@ -99,7 +100,7 @@ class FederatedUsers(object): db_user = model.user.verify_federated_login(self._federated_service, username) if not db_user: # We must create the user in our db. Check to see if this is allowed. - if not features.USER_CREATION: + if not can_create_user(email): return (None, DISABLED_MESSAGE) valid_username = None diff --git a/data/users/shared.py b/data/users/shared.py new file mode 100644 index 000000000..544e8d6be --- /dev/null +++ b/data/users/shared.py @@ -0,0 +1,18 @@ +import features + +from data import model + +def can_create_user(email_address): + """ Returns true if a user with the specified e-mail address can be created. """ + if not features.USER_CREATION: + return False + + if features.INVITE_ONLY_USER_CREATION: + if not email_address: + return False + + # Check to see that there is an invite for the e-mail address. + return bool(model.team.lookup_team_invites_by_email(email_address)) + + # Otherwise the user can be created (assuming it doesn't already exist, of course) + return True diff --git a/data/users/test/test_shared.py b/data/users/test/test_shared.py new file mode 100644 index 000000000..662a39c60 --- /dev/null +++ b/data/users/test/test_shared.py @@ -0,0 +1,38 @@ +import pytest + +from mock import patch + +from data.database import model +from data.users.shared import can_create_user + +from test.fixtures import * + +@pytest.mark.parametrize('open_creation, invite_only, email, has_invite, can_create', [ + # Open user creation => always allowed. + (True, False, None, False, True), + + # Open user creation => always allowed. + (True, False, 'foo@example.com', False, True), + + # Invite only user creation + no invite => disallowed. + (True, True, None, False, False), + + # Invite only user creation + no invite => disallowed. + (True, True, 'foo@example.com', False, False), + + # Invite only user creation + invite => allowed. + (True, True, 'foo@example.com', True, True), + + # No open creation => Disallowed. + (False, True, 'foo@example.com', False, False), + (False, True, 'foo@example.com', True, False), +]) +def test_can_create_user(open_creation, invite_only, email, has_invite, can_create, app): + if has_invite: + inviter = model.user.get_user('devtable') + team = model.team.get_organization_team('buynlarge', 'owners') + model.team.add_or_invite_to_team(inviter, team, email=email) + + with patch('features.USER_CREATION', open_creation): + with patch('features.INVITE_ONLY_USER_CREATION', invite_only): + assert can_create_user(email) == can_create diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 90ef38603..b051d0594 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -21,6 +21,7 @@ from auth.permissions import (AdministerOrganizationPermission, CreateRepository from data import model from data.billing import get_plan from data.database import Repository as RepositoryTable +from data.users.shared import can_create_user from endpoints.api import (ApiResource, nickname, resource, validate_json_request, request_error, log_action, internal_only, require_user_admin, parse_args, query_param, require_scope, format_date, show_if, @@ -424,9 +425,20 @@ class User(ApiResource): if existing_user: raise request_error(message='The username already exists') + # Ensure an e-mail address was specified if required. if features.MAILING and not user_data.get('email'): raise request_error(message='Email address is required') + # If invite-only user creation is turned on and no invite code was sent, return an error. + # Technically, this is handled by the can_create_user call below as well, but it makes + # a nicer error. + if features.INVITE_ONLY_USER_CREATION and not invite_code: + raise request_error(message='Cannot create non-invited user') + + # Ensure that this user can be created. + if not can_create_user(user_data.get('email')): + raise request_error(message='Creation of a user account for this e-mail is disabled; please contact an administrator') + try: prompts = model.user.get_default_user_prompts(features) new_user = model.user.create_user(user_data['username'], user_data['password'], diff --git a/endpoints/oauth/login.py b/endpoints/oauth/login.py index 0a5e76618..e6cfc0e75 100644 --- a/endpoints/oauth/login.py +++ b/endpoints/oauth/login.py @@ -12,6 +12,7 @@ from app import app, analytics, get_app_url, oauth_login, authentication from auth.auth_context import get_authenticated_user from auth.decorators import require_session_login from data import model +from data.users.shared import can_create_user from endpoints.common import common_login from endpoints.web import index, render_page_template_with_routedata from endpoints.csrf import csrf_protect, OAUTH_CSRF_TOKEN_NAME, generate_csrf_token @@ -86,7 +87,7 @@ def _conduct_oauth_login(auth_system, login_service, lid, lusername, lemail, met return _oauthresult(user_obj=user_obj, service_name=service_name) # Otherwise, we need to create a new user account. - if not features.USER_CREATION: + if not can_create_user(lemail): error_message = 'User creation is disabled. Please contact your administrator' return _oauthresult(service_name=service_name, error_message=error_message) @@ -130,7 +131,8 @@ def _conduct_oauth_login(auth_system, login_service, lid, lusername, lemail, met def _render_ologin_error(service_name, error_message=None, register_redirect=False): """ Returns a Flask response indicating an OAuth error. """ - user_creation = bool(features.USER_CREATION and features.DIRECT_LOGIN) + user_creation = bool(features.USER_CREATION and features.DIRECT_LOGIN and + not features.INVITE_ONLY_USER_CREATION) error_info = { 'reason': 'ologinerror', 'service_name': service_name, diff --git a/endpoints/oauth/test/test_login.py b/endpoints/oauth/test/test_login.py index 327bdeaa8..40a51dc91 100644 --- a/endpoints/oauth/test/test_login.py +++ b/endpoints/oauth/test/test_login.py @@ -1,5 +1,7 @@ import pytest +from mock import patch + from data import model, database from data.users import get_users_handler, DatabaseUsers from endpoints.oauth.login import _conduct_oauth_login @@ -71,6 +73,37 @@ def test_new_account_via_database(login_service): federated_login = model.user.lookup_federated_login(new_user, login_service.service_id()) assert federated_login is not None +@pytest.mark.parametrize('open_creation, invite_only, has_invite, expect_success', [ + # Open creation -> Success! + (True, False, False, True), + + # Open creation + invite only + no invite -> Failure! + (True, True, False, False), + + # Open creation + invite only + invite -> Success! + (True, True, True, True), + + # Close creation -> Failure! + (False, False, False, False), +]) +def test_flagged_user_creation(open_creation, invite_only, has_invite, expect_success, login_service): + login_service_lid = 'someexternaluser' + email = 'some@example.com' + + if has_invite: + inviter = model.user.get_user('devtable') + team = model.team.get_organization_team('buynlarge', 'owners') + model.team.add_or_invite_to_team(inviter, team, email=email) + + internal_auth = DatabaseUsers() + + with patch('features.USER_CREATION', open_creation): + with patch('features.INVITE_ONLY_USER_CREATION', invite_only): + # Conduct login. + result = _conduct_oauth_login(internal_auth, login_service, login_service_lid, login_service_lid, + email) + assert (result.user_obj is not None) == expect_success + assert (result.error_message is None) == expect_success @pytest.mark.parametrize('binding_field, lid, lusername, lemail, expected_error', [ # No binding field + newly seen user -> New unlinked user diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html index d8fc2b800..b1e8450fa 100644 --- a/static/directives/config/config-setup-tool.html +++ b/static/directives/config/config-setup-tool.html @@ -1218,8 +1218,20 @@ Enable Open User Creation
- If enabled, user accounts can be created by anyone. - Users can always be created in the users panel under this superuser view. + If enabled, user accounts can be created by anyone (unless restricted below to invited users). + Users can always be created in the users panel in this superuser tool, even if this feature is disabled. +
+ + + + Invite-only User Creation: + +
+ Enable Invite-only User Creation +
+
+ If enabled, user accounts can only be created when a user has been invited, by e-mail address, to join a team. + Users can always be created in the users panel in this superuser tool, even if this feature is enabled.
diff --git a/static/directives/user-setup.html b/static/directives/user-setup.html index 5df52ab86..1221452cd 100644 --- a/static/directives/user-setup.html +++ b/static/directives/user-setup.html @@ -27,8 +27,10 @@