Merge 8a8e4ebcf433ef91a25fe4afbeeaacc75ad672a0 into ce089f498bce32305b2d9e8c6adfd8cb7c82f88f

services/auth/source/source_group_sync.go

@@ -60,6 +60,28 @@ 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, removeTeams := range membershipsToRemove {
+		addTeams, ok := membershipsToAdd[org]
+		if !ok {
+			continue
+		}
+		addSet := container.SetOf(addTeams...)
+		filtered := make([]string, 0, len(removeTeams))
+		for _, team := range removeTeams {
+			if !addSet.Contains(team) {
+				filtered = append(filtered, team)
+			}
+		}
+		if len(filtered) > 0 {
+			membershipsToRemove[org] = filtered
+		} else {
+			delete(membershipsToRemove, org)
+		}
+	}
+
 	return membershipsToAdd, membershipsToRemove
 }
 
@@ -106,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
 				}

services/auth/source/source_group_sync_test.go

@@ -0,0 +1,95 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package source
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/organization"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/container"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMain(m *testing.M) {
+	unittest.MainTest(m, &unittest.TestOptions{})
+}
+
+func TestSyncGroupsToTeams_LastOwnerNotRemoved(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	// Fixtures: in org3, user2 is the only member of the "Owners" team, and
+	// team "team1" (a non-owner team) has user2 and user4 as members.
+	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)
+
+	sourceUserGroups := container.SetOf("groupA")
+	sourceGroupTeamMapping := map[string]map[string][]string{
+		"groupA": {"org3": {"Owners"}},
+		"groupB": {"org3": {"Owners", "team1"}},
+	}
+
+	// Deduplication: "Owners" must not be in the remove list when groupA grants it,
+	// while "team1" (only mapped by groupB, which the user is not in) must remain.
+	membershipsToAdd, membershipsToRemove := resolveMappedMemberships(sourceUserGroups, sourceGroupTeamMapping)
+	assert.Contains(t, membershipsToAdd["org3"], "Owners")
+	assert.NotContains(t, membershipsToRemove["org3"], "Owners")
+	assert.Contains(t, membershipsToRemove["org3"], "team1")
+
+	// 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) {
+	sourceUserGroups := container.SetOf("in1", "in2")
+	sourceGroupTeamMapping := map[string]map[string][]string{
+		"in1":  {"orgA": {"Owners"}, "orgB": {"team1"}},
+		"in2":  {"orgB": {"team1"}},                     // duplicate add, no effect on remove
+		"out1": {"orgA": {"Owners"}, "orgB": {"team2"}}, // orgA fully overlaps add; orgB team2 has no add
+		"out2": {"orgC": {"team3"}},                     // no add for orgC
+	}
+
+	membershipsToAdd, membershipsToRemove := resolveMappedMemberships(sourceUserGroups, sourceGroupTeamMapping)
+
+	assert.ElementsMatch(t, []string{"Owners"}, membershipsToAdd["orgA"])
+	assert.ElementsMatch(t, []string{"team1", "team1"}, membershipsToAdd["orgB"])
+	_, hasOrgA := membershipsToRemove["orgA"]
+	assert.False(t, hasOrgA, "orgA must be deleted from remove map when all removals overlap adds")
+	assert.ElementsMatch(t, []string{"team2"}, membershipsToRemove["orgB"])
+	assert.ElementsMatch(t, []string{"team3"}, membershipsToRemove["orgC"])
+}