diff --git a/data/users/__init__.py b/data/users/__init__.py index a18ef6ba9..18ebc9bff 100644 --- a/data/users/__init__.py +++ b/data/users/__init__.py @@ -211,11 +211,14 @@ class UserAuthentication(object): """ return self.state.link_user(username_or_email) - def get_and_link_federated_user_info(self, user_info): + def get_and_link_federated_user_info(self, user_info, internal_create=False): """ Returns a tuple containing the database user record linked to the given UserInformation pair and any error that occurred when trying to link the user. + + If `internal_create` is True, the caller is an internal user creation process (such + as team syncing), and the "can a user be created" check will be bypassed. """ - return self.state.get_and_link_federated_user_info(user_info) + return self.state.get_and_link_federated_user_info(user_info, internal_create=internal_create) def confirm_existing_user(self, username, password): """ Verifies that the given password matches to the given DB username. Unlike diff --git a/data/users/database.py b/data/users/database.py index a71669c44..9100b91e4 100644 --- a/data/users/database.py +++ b/data/users/database.py @@ -41,7 +41,7 @@ class DatabaseUsers(object): """ Never used since all users being added are already, by definition, in the database. """ return (None, 'Unsupported for this authentication system') - def get_and_link_federated_user_info(self, user_info): + def get_and_link_federated_user_info(self, user_info, internal_create=False): """ Never used since all users being added are already, by definition, in the database. """ return (None, 'Unsupported for this authentication system') diff --git a/data/users/federated.py b/data/users/federated.py index 424779eee..1ce4fcd88 100644 --- a/data/users/federated.py +++ b/data/users/federated.py @@ -60,8 +60,9 @@ class FederatedUsers(object): return self.get_and_link_federated_user_info(user_info) - def get_and_link_federated_user_info(self, user_info): - return self._get_and_link_federated_user_info(user_info.username, user_info.email) + def get_and_link_federated_user_info(self, user_info, internal_create=False): + return self._get_and_link_federated_user_info(user_info.username, user_info.email, + internal_create=internal_create) def verify_and_link_user(self, username_or_email, password): """ Verifies the given credentials and, if valid, creates/links a database user to the @@ -109,11 +110,12 @@ class FederatedUsers(object): """ return (None, 'Not supported') - def _get_and_link_federated_user_info(self, username, email): + def _get_and_link_federated_user_info(self, username, email, internal_create=False): 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 can_create_user(email): + # We must create the user in our db. Check to see if this is allowed (except for internal + # creation, which is always allowed). + if not internal_create and not can_create_user(email): return (None, DISABLED_MESSAGE) valid_username = None diff --git a/data/users/teamsync.py b/data/users/teamsync.py index 44dc52032..2ab0fea10 100644 --- a/data/users/teamsync.py +++ b/data/users/teamsync.py @@ -82,7 +82,8 @@ def sync_team(authentication, stale_team_sync): continue # Retrieve the Quay user associated with the member info. - (quay_user, err) = authentication.get_and_link_federated_user_info(member_info) + (quay_user, err) = authentication.get_and_link_federated_user_info(member_info, + internal_create=True) if err is not None: logger.error('Could not link external user %s to an internal user: %s', member_info.username, err, diff --git a/data/users/test/test_teamsync.py b/data/users/test/test_teamsync.py index f62f6e3bb..d7c839bef 100644 --- a/data/users/test/test_teamsync.py +++ b/data/users/test/test_teamsync.py @@ -1,8 +1,11 @@ +import os + from datetime import datetime, timedelta -import os import pytest +from mock import patch + from data import model, database from data.users.federated import FederatedUsers, UserInformation from data.users.teamsync import sync_team, sync_teams_to_groups @@ -23,6 +26,18 @@ class FakeUsers(FederatedUsers): return (self.group_tuples, None) +@pytest.fixture(params=[True, False]) +def user_creation(request): + with patch('features.USER_CREATION', request.param): + yield + + +@pytest.fixture(params=[True, False]) +def invite_only_user_creation(request): + with patch('features.INVITE_ONLY_USER_CREATION', request.param): + yield + + @pytest.mark.skipif(os.environ.get('TEST_DATABASE_URI', '').find('postgres') >= 0, reason="Postgres fails when existing members are added under the savepoint") @pytest.mark.parametrize('starting_membership,group_membership,expected_membership', [ @@ -144,7 +159,8 @@ class FakeUsers(FederatedUsers): ], ['anotheruser', 'someuser']), ]) -def test_syncing(starting_membership, group_membership, expected_membership, app): +def test_syncing(user_creation, invite_only_user_creation, starting_membership, group_membership, + expected_membership, app): org = model.organization.get_organization('buynlarge') # Necessary for the fake auth entries to be created in FederatedLogin. @@ -169,7 +185,7 @@ def test_syncing(starting_membership, group_membership, expected_membership, app quay_user = model.user.create_user_noverify(quay_username, email) else: quay_user = model.user.create_federated_user(quay_username, email, _FAKE_AUTH, - fakeauth_username, False) + fakeauth_username, False) model.team.add_user_to_team(quay_user, sync_team_info.team) @@ -187,7 +203,8 @@ def test_syncing(starting_membership, group_membership, expected_membership, app assert len(users_expected) + len(robots_expected) == len(expected_membership) # Check that the team's users match those expected. - service_user_map = model.team.get_federated_team_member_mapping(sync_team_info.team, _FAKE_AUTH) + service_user_map = model.team.get_federated_team_member_mapping(sync_team_info.team, + _FAKE_AUTH) assert set(service_user_map.keys()) == users_expected quay_users = model.team.list_team_users(sync_team_info.team) @@ -204,7 +221,7 @@ def test_syncing(starting_membership, group_membership, expected_membership, app assert robots_expected == robots_found -def test_sync_teams_to_groups(app): +def test_sync_teams_to_groups(user_creation, invite_only_user_creation, app): # Necessary for the fake auth entries to be created in FederatedLogin. database.LoginService.create(name=_FAKE_AUTH) @@ -247,7 +264,8 @@ def test_sync_teams_to_groups(app): (mock_ldap, {'group_dn': 'cn=AwesomeFolk'}), (fake_keystone, {'group_id': 'somegroupid'}), ]) -def test_teamsync_end_to_end(auth_system_builder, config, app): +def test_teamsync_end_to_end(user_creation, invite_only_user_creation, auth_system_builder, config, + app): with auth_system_builder() as auth: # Create an new team to sync. org = model.organization.get_organization('buynlarge') @@ -286,7 +304,8 @@ def test_teamsync_end_to_end(auth_system_builder, config, app): (mock_ldap, {'group_dn': 'cn=AwesomeFolk'}), (fake_keystone, {'group_id': 'somegroupid'}), ]) -def test_teamsync_existing_email(auth_system_builder, config, app): +def test_teamsync_existing_email(user_creation, invite_only_user_creation, auth_system_builder, + config, app): with auth_system_builder() as auth: # Create an new team to sync. org = model.organization.get_organization('buynlarge') @@ -303,4 +322,3 @@ def test_teamsync_existing_email(auth_system_builder, config, app): team_members = list(model.team.list_team_users(sync_team_info.team)) assert len(team_members) > 0 -