Allow team syncing if user creation is disabled

Before this change, if user creation was disabled, team sync would fail to sync over users that had not yet been invited/logged in, because their accounts could not be created. Following this change, team syncing of users not yet in the system will create those user accounts, allowing users to be "auto invited" via team sync.

Fixes https://jira.coreos.com/browse/QUAY-910
This commit is contained in:
Joseph Schorr 2018-05-22 14:09:40 -04:00
parent 0c1b13828f
commit 861e81cccd
5 changed files with 41 additions and 17 deletions

View file

@ -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

View file

@ -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')

View file

@ -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

View file

@ -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,

View file

@ -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