Merge bda7281fcb65fd12359f1234bad6200705fd0108 into ce089f498bce32305b2d9e8c6adfd8cb7c82f88f

services/auth/source/oauth2/source_group_sync_test.go

@@ -0,0 +1,54 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package oauth2
+
+import (
+	"sync"
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	"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"
+	auth_source "code.gitea.io/gitea/services/auth/source"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSyncGroupsToTeamsConcurrentRuns(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	ctx := t.Context()
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+	team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2})
+	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: team.OrgID})
+
+	sourceGroupTeamMapping := map[string]map[string][]string{
+		"ldap-group": {
+			org.Name: []string{team.Name},
+		},
+	}
+
+	var wg sync.WaitGroup
+	for range 10 {
+		wg.Go(func() {
+			assert.NoError(t, auth_source.SyncGroupsToTeams(ctx, user, container.SetOf("ldap-group"), sourceGroupTeamMapping, true))
+		})
+		wg.Go(func() {
+			assert.NoError(t, auth_source.SyncGroupsToTeams(ctx, user, container.Set[string]{}, sourceGroupTeamMapping, true))
+		})
+	}
+	wg.Wait()
+
+	memberCount, err := db.GetEngine(ctx).Count(&organization.TeamUser{OrgID: team.OrgID, TeamID: team.ID, UID: user.ID})
+	assert.NoError(t, err)
+	assert.LessOrEqual(t, memberCount, int64(1))
+
+	team = unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: team.ID})
+	totalCount, err := db.GetEngine(ctx).Count(&organization.TeamUser{OrgID: team.OrgID, TeamID: team.ID})
+	assert.NoError(t, err)
+	assert.EqualValues(t, team.NumMembers, totalCount)
+	assert.GreaterOrEqual(t, team.NumMembers, 0)
+}

services/auth/source/source_group_sync.go

@@ -10,6 +10,7 @@ import (
 	"code.gitea.io/gitea/models/organization"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/container"
+	"code.gitea.io/gitea/modules/globallock"
 	"code.gitea.io/gitea/modules/log"
 	org_service "code.gitea.io/gitea/services/org"
 )
@@ -94,21 +95,26 @@ func syncGroupsToTeamsCached(ctx context.Context, user *user_model.User, orgTeam
 				teamCache[orgName+teamName] = team
 			}
 
-			isMember, err := organization.IsTeamMember(ctx, org.ID, team.ID, user.ID)
+			if err := globallock.LockAndDo(ctx, fmt.Sprintf("group_sync_team_%d", team.ID), func(ctx context.Context) error {
-			if err != nil {
+				isMember, err := organization.IsTeamMember(ctx, org.ID, team.ID, user.ID)
-				return err
+				if err != nil {
-			}
+					return err
+				}
 
-			if action == syncAdd && !isMember {
+				if action == syncAdd && !isMember {
-				if err := org_service.AddTeamMember(ctx, team, user); err != nil {
+					if err := org_service.AddTeamMember(ctx, team, user); err != nil {
-					log.Error("group sync: Could not add user to team: %v", err)
+						log.Error("group sync: Could not add user to team: %v", err)
-					return err
+						return err
-				}
+					}
-			} else if action == syncRemove && isMember {
+				} else if action == syncRemove && isMember {
-				if err := org_service.RemoveTeamMember(ctx, team, user); err != nil {
+					if err := org_service.RemoveTeamMember(ctx, team, user); err != nil {
-					log.Error("group sync: Could not remove user from team: %v", err)
+						log.Error("group sync: Could not remove user from team: %v", err)
-					return err
+						return err
+					}
 				}
+				return nil
+			}); err != nil {
+				return err
 			}
 		}
 	}

services/org/team.go

@@ -233,13 +233,20 @@ func AddTeamMember(ctx context.Context, team *organization.Team, user *user_mode
 
 		sess := db.GetEngine(ctx)
 
-		if err := db.Insert(ctx, &organization.TeamUser{
+		res, err := sess.Exec("INSERT INTO team_user (org_id, team_id, uid) SELECT ?, ?, ? WHERE NOT EXISTS (SELECT 1 FROM team_user WHERE org_id=? AND team_id=? AND uid=?)",
-			UID:    user.ID,
+			team.OrgID, team.ID, user.ID, team.OrgID, team.ID, user.ID)
-			OrgID:  team.OrgID,
+		if err != nil {
-			TeamID: team.ID,
-		}); err != nil {
 			return err
-		} else if _, err := sess.Incr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil {
+		}
+		rowsAffected, err := res.RowsAffected()
+		if err != nil {
+			return err
+		}
+		if rowsAffected == 0 {
+			return nil
+		}
+
+		if _, err := sess.Incr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil {
 			return err
 		}
 
@@ -285,8 +292,6 @@ func removeTeamMember(ctx context.Context, team *organization.Team, user *user_m
 		return organization.ErrLastOrgOwner{UID: user.ID}
 	}
 
-	team.NumMembers--
-
 	repos, err := repo_model.GetTeamRepositories(ctx, &repo_model.SearchTeamRepoOptions{
 		TeamID: team.ID,
 	})
@@ -294,18 +299,24 @@ func removeTeamMember(ctx context.Context, team *organization.Team, user *user_m
 		return err
 	}
 
-	if _, err := e.Delete(&organization.TeamUser{
+	rowsAffected, err := e.Delete(&organization.TeamUser{
 		UID:    user.ID,
 		OrgID:  team.OrgID,
 		TeamID: team.ID,
-	}); err != nil {
+	})
+	if err != nil {
 		return err
-	} else if _, err = e.
+	}
-		ID(team.ID).
+	if rowsAffected == 0 {
-		Cols("num_members").
+		return nil
-		Update(team); err != nil {
+	}
+
+	if _, err = e.Decr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil {
 		return err
 	}
+	if team.NumMembers > 0 {
+		team.NumMembers--
+	}
 
 	// Delete access to team repositories. If any user or repo is missing, we can continue.
 	for _, repo := range repos {

services/org/team_test.go

@@ -6,8 +6,10 @@ package org
 import (
 	"fmt"
 	"strings"
+	"sync"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/perm"
@@ -328,3 +330,39 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) {
 	}
 	assert.NoError(t, DeleteOrganization(t.Context(), org, false), "DeleteOrganization")
 }
+
+func TestTeamMemberConcurrentAddRemoveIdempotent(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	ctx := t.Context()
+	team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+
+	var wg sync.WaitGroup
+	for range 8 {
+		wg.Go(func() {
+			assert.NoError(t, AddTeamMember(ctx, team, user))
+		})
+	}
+	wg.Wait()
+
+	count, err := db.GetEngine(ctx).Count(&organization.TeamUser{OrgID: team.OrgID, TeamID: team.ID, UID: user.ID})
+	assert.NoError(t, err)
+	assert.EqualValues(t, 1, count)
+
+	for range 8 {
+		wg.Go(func() {
+			assert.NoError(t, RemoveTeamMember(ctx, team, user))
+		})
+	}
+	wg.Wait()
+
+	count, err = db.GetEngine(ctx).Count(&organization.TeamUser{OrgID: team.OrgID, TeamID: team.ID, UID: user.ID})
+	assert.NoError(t, err)
+	assert.EqualValues(t, 0, count)
+
+	team = unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: team.ID})
+	memberCount, err := db.GetEngine(ctx).Count(&organization.TeamUser{OrgID: team.OrgID, TeamID: team.ID})
+	assert.NoError(t, err)
+	assert.EqualValues(t, team.NumMembers, memberCount)
+}