diff --git a/services/auth/source/source_group_sync.go b/services/auth/source/source_group_sync.go index aa2dd812ea..5317b4b7ff 100644 --- a/services/auth/source/source_group_sync.go +++ b/services/auth/source/source_group_sync.go @@ -63,8 +63,8 @@ func resolveMappedMemberships(sourceUserGroups container.Set[string], sourceGrou // If another group grants the same team, don't remove it — for the Owners // team this would fail with ErrLastOrgOwner before the add runs. - for org, addTeams := range membershipsToAdd { - removeTeams, ok := membershipsToRemove[org] + for org, removeTeams := range membershipsToRemove { + addTeams, ok := membershipsToAdd[org] if !ok { continue } @@ -128,6 +128,11 @@ func syncGroupsToTeamsCached(ctx context.Context, user *user_model.User, orgTeam } } else if action == syncRemove && isMember { if err := org_service.RemoveTeamMember(ctx, team, user); err != nil { + // Skip sole-owner removal so OAuth login still succeeds. + if organization.IsErrLastOrgOwner(err) { + log.Warn("group sync: Skipping removal of last owner in org %s for user %s: %v", org.Name, user.Name, err) + continue + } log.Error("group sync: Could not remove user from team: %v", err) return err } diff --git a/services/auth/source/source_group_sync_test.go b/services/auth/source/source_group_sync_test.go index f7c7e54d6e..b003eb0488 100644 --- a/services/auth/source/source_group_sync_test.go +++ b/services/auth/source/source_group_sync_test.go @@ -45,6 +45,34 @@ func TestSyncGroupsToTeams_LastOwnerNotRemoved(t *testing.T) { // End-to-end: sync must not fail with ErrLastOrgOwner. require.NoError(t, SyncGroupsToTeams(t.Context(), user2, sourceUserGroups, sourceGroupTeamMapping, true)) + + // User2 must still be a member of the Owners team. + stillOwner, err := organization.IsTeamMember(t.Context(), org3.ID, ownersTeam.ID, user2.ID) + require.NoError(t, err) + assert.True(t, stillOwner) +} + +func TestSyncGroupsToTeams_LastOwnerRemovalSkippedWhenNotGranted(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + org3 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) + ownersTeam, err := org3.GetOwnerTeam(t.Context()) + require.NoError(t, err) + require.Equal(t, 1, ownersTeam.NumMembers) + + // User is in no mapped group, so Owners is scheduled for removal with no + // offsetting add. The remove must be attempted and gracefully skipped. + sourceUserGroups := container.SetOf[string]() + sourceGroupTeamMapping := map[string]map[string][]string{ + "groupA": {"org3": {"Owners"}}, + } + + require.NoError(t, SyncGroupsToTeams(t.Context(), user2, sourceUserGroups, sourceGroupTeamMapping, true)) + + stillOwner, err := organization.IsTeamMember(t.Context(), org3.ID, ownersTeam.ID, user2.ID) + require.NoError(t, err) + assert.True(t, stillOwner, "sole owner must remain after sync that couldn't remove them") } func TestResolveMappedMemberships_Dedup(t *testing.T) {