Merge pull request #3089 from quay/joseph.schorr/QUAY-910/team-sync-disabled

Allow team syncing if user creation is disabled

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

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

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):
+  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)
+    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.
+      # We must create the user in our db. Check to see if this is allowed (except for internal
-      if not can_create_user(email):
+      # creation, which is always allowed).
+      if not internal_create and not can_create_user(email):
         return (None, DISABLED_MESSAGE)
 
       valid_username = None

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,

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
-