Merge pull request #2580 from coreos-inc/team-sync-email-fix

Fix handling of team sync when a user already exists with the email address

data/users/federated.py

@@ -107,11 +107,15 @@ class FederatedUsers(object):
         return (None, 'Unable to pick a username. Please report this to your administrator.')
 
       prompts = model.user.get_default_user_prompts(features)
-      db_user = model.user.create_federated_user(valid_username, email, self._federated_service,
+      try:
-                                                 username,
+        db_user = model.user.create_federated_user(valid_username, email, self._federated_service,
-                                                 set_password_notification=False,
+                                                   username,
-                                                 email_required=self._requires_email,
+                                                   set_password_notification=False,
-                                                 prompts=prompts)
+                                                   email_required=self._requires_email,
+                                                   prompts=prompts)
+      except model.InvalidEmailAddressException as iae:
+        return (None, iae.message)
+
     else:
       # Update the db attributes from the federated service.
       if email and db_user.email != email:

data/users/test/test_teamsync.py

@@ -257,4 +257,47 @@ def test_teamsync_end_to_end(auth_system_builder, config, app):
     # Ensure we now have members.
     msg = 'Auth system: %s' % auth.federated_service
     sync_team_info = model.team.get_team_sync_information('buynlarge', 'synced2')
-    assert len(list(model.team.list_team_users(sync_team_info.team))) > 0, msg
+    team_members = list(model.team.list_team_users(sync_team_info.team))
+    assert len(team_members) > 1, msg
+
+    it, _ = auth.iterate_group_members(config)
+    assert len(team_members) == len(list(it)), msg
+
+    sync_team_info.last_updated = datetime.now() - timedelta(hours=6)
+    sync_team_info.save()
+
+    # Remove one of the members and force a sync again to ensure we re-link the correct users.
+    first_member = team_members[0]
+    model.team.remove_user_from_team('buynlarge', 'synced2', first_member.username, 'devtable')
+
+    team_members2 = list(model.team.list_team_users(sync_team_info.team))
+    assert len(team_members2) == 1, msg
+    assert sync_team(auth, sync_team_info)
+
+    team_members3 = list(model.team.list_team_users(sync_team_info.team))
+    assert len(team_members3) > 1, msg
+    assert set([m.id for m in team_members]) == set([m.id for m in team_members3])
+
+
+@pytest.mark.parametrize('auth_system_builder,config', [
+  (mock_ldap, {'group_dn': 'cn=AwesomeFolk'}),
+  (fake_keystone, {'group_id': 'somegroupid'}),
+])
+def test_teamsync_existing_email(auth_system_builder, config, app):
+  with auth_system_builder() as auth:
+    # Create an new team to sync.
+    org = model.organization.get_organization('buynlarge')
+    new_synced_team = model.team.create_team('synced2', org, 'member', 'Some synced team.')
+    sync_team_info = model.team.set_team_syncing(new_synced_team, auth.federated_service, config)
+
+    # Add a new *unlinked* user with the same email address as one of the team members.
+    it, _ = auth.iterate_group_members(config)
+    members = list(it)
+    model.user.create_user_noverify('someusername', members[0][0].email)
+
+    # Sync the team and ensure it doesn't fail.
+    assert sync_team(auth, sync_team_info)
+
+    team_members = list(model.team.list_team_users(sync_team_info.team))
+    assert len(team_members) > 0
+

test/fixtures.py

@@ -56,6 +56,9 @@ def init_db_path(tmpdir_factory):
   application.config.update(conf)
   application.config.update({"DB_URI": sqlitedb})
   initialize_database()
+
+  db.obj.execute_sql('PRAGMA foreign_keys = ON;')
+
   populate_database()
   close_db_filter(None)
   return str(sqlitedb_file)