0
0
mirror of https://github.com/go-gitea/gitea.git synced 2026-05-12 00:22:59 +02:00

Fix OAuth2 group sync removing sole org owner when multiple groups map same team

When multiple OAuth2 groups map to the same org team (e.g. Owners) and
the user is in only one of those groups, `resolveMappedMemberships` put
the team in both the add and remove lists. Since removal runs before
addition, removing the sole owner failed with `ErrLastOrgOwner`, which
the OAuth handler surfaced as HTTP 500, locking the user out on login.

Fix by deduplicating: after building both lists, exclude any team from
the remove list that is also in the add list for that org.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
This commit is contained in:
silverwind 2026-04-09 20:34:10 +02:00
parent 980a8995bc
commit 5109687831
No known key found for this signature in database
GPG Key ID: 2E62B41C93869443
2 changed files with 59 additions and 0 deletions

View File

@ -60,6 +60,24 @@ func resolveMappedMemberships(sourceUserGroups container.Set[string], sourceGrou
}
}
}
// If any group grants a team membership, do not remove it even if another
// group the user is not in also maps to that team.
for org, addTeams := range membershipsToAdd {
removeTeams, ok := membershipsToRemove[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)
}
}
membershipsToRemove[org] = filtered
}
return membershipsToAdd, membershipsToRemove
}

View File

@ -0,0 +1,41 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package source
import (
"testing"
"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{})
}
// Fixture: team 1 = org3's "Owners" team, num_members=1, sole member is user2.
func TestSyncGroupsToTeams_LastOwnerNotRemoved(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
sourceUserGroups := container.SetOf("groupA")
sourceGroupTeamMapping := map[string]map[string][]string{
"groupA": {"org3": {"Owners"}},
"groupB": {"org3": {"Owners", "Developers"}},
}
// Verify deduplication: Owners must not be in remove list when groupA grants it.
membershipsToAdd, membershipsToRemove := resolveMappedMemberships(sourceUserGroups, sourceGroupTeamMapping)
assert.Contains(t, membershipsToAdd["org3"], "Owners")
assert.NotContains(t, membershipsToRemove["org3"], "Owners")
assert.Contains(t, membershipsToRemove["org3"], "Developers")
// End-to-end: sync must not fail with ErrLastOrgOwner.
err := SyncGroupsToTeams(t.Context(), user2, sourceUserGroups, sourceGroupTeamMapping, true)
require.NoError(t, err)
}