From b3d7577473c79c61681d3272cda91da3688a88b8 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 5 May 2017 13:20:20 -0400 Subject: [PATCH] Disable federated login for new users if user creation is disabled Fixes https://www.pivotaltracker.com/story/show/144821585 --- data/users/federated.py | 7 ++++++- data/users/test/test_users.py | 36 +++++++++++++++++++++++++++++++++++ features/__init__.py | 2 -- 3 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 data/users/test/test_users.py diff --git a/data/users/federated.py b/data/users/federated.py index 074818d34..3c61a5837 100644 --- a/data/users/federated.py +++ b/data/users/federated.py @@ -10,6 +10,8 @@ logger = logging.getLogger(__name__) UserInformation = namedtuple('UserInformation', ['username', 'email', 'id']) +DISABLED_MESSAGE = 'User creation is disabled. Please contact your adminstrator to gain access.' + class FederatedUsers(object): """ Base class for all federated users systems. """ @@ -96,7 +98,10 @@ class FederatedUsers(object): def _get_and_link_federated_user_info(self, username, email): db_user = model.user.verify_federated_login(self._federated_service, username) if not db_user: - # We must create the user in our db + # We must create the user in our db. Check to see if this is allowed. + if not features.USER_CREATION: + return (None, DISABLED_MESSAGE) + valid_username = None for valid_username in generate_valid_usernames(username): if model.user.is_username_unique(valid_username): diff --git a/data/users/test/test_users.py b/data/users/test/test_users.py new file mode 100644 index 000000000..007332880 --- /dev/null +++ b/data/users/test/test_users.py @@ -0,0 +1,36 @@ +import pytest + +from mock import patch + +from data.database import model +from data.users.federated import DISABLED_MESSAGE +from test.test_ldap import mock_ldap +from test.test_keystone_auth import fake_keystone + +from test.fixtures import * + +@pytest.mark.parametrize('auth_system_builder, user1, user2', [ + (mock_ldap, ('someuser', 'somepass'), ('testy', 'password')), + (fake_keystone, ('cool.user', 'password'), ('some.neat.user', 'foobar')), +]) +def test_auth_createuser(auth_system_builder, user1, user2, config, app): + with auth_system_builder() as auth: + # Login as a user and ensure a row in the database is created for them. + user, err = auth.verify_and_link_user(*user1) + assert err is None + assert user + + federated_info = model.user.lookup_federated_login(user, auth.federated_service) + assert federated_info is not None + + # Disable user creation. + with patch('features.USER_CREATION', False): + # Ensure that the existing user can login. + user_again, err = auth.verify_and_link_user(*user1) + assert err is None + assert user_again.id == user.id + + # Ensure that a new user cannot. + new_user, err = auth.verify_and_link_user(*user2) + assert new_user is None + assert err == DISABLED_MESSAGE diff --git a/features/__init__.py b/features/__init__.py index b5822681e..ca8c2880a 100644 --- a/features/__init__.py +++ b/features/__init__.py @@ -27,5 +27,3 @@ class FeatureNameValue(object): def __nonzero__(self): return self.value.__nonzero__() - -