fix(repo): preserve transfer recipient collaboration (#39042)

Remove temporary recipient access after a transfer ends while preserving
existing collaboration.

---------

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
bircni
2026-08-22 21:43:42 +02:00
committed by GitHub
parent 9251eeb66b
commit e6af4c341c
7 changed files with 138 additions and 27 deletions

View File

@@ -424,6 +424,7 @@ func prepareMigrationTasks() []*migration {
newMigration(348, "Recreate email_hash table for SHA256 avatar hashes", v28.RecreateEmailHashTable),
newMigration(349, "Expand action_schedule content column", v28.ExpandActionScheduleContent),
newMigration(350, "Add published_unix column to release", v28.AddPublishedUnixToRelease),
newMigration(351, "Track transfer recipient access grants", v28.AddRecipientAccessGrantedToRepoTransfer),
}
return preparedMigrations
}

View File

@@ -0,0 +1,23 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v28
import (
"context"
"gitea.dev/modelmigration/base"
"xorm.io/xorm"
)
func AddRecipientAccessGrantedToRepoTransfer(_ context.Context, x base.EngineMigration) error {
type RepoTransfer struct {
RecipientAccessGranted bool `xorm:"NOT NULL DEFAULT false"`
}
_, err := x.SyncWithOptions(xorm.SyncOptions{
IgnoreConstrains: true,
IgnoreDropIndices: true,
}, new(RepoTransfer))
return err
}

View File

@@ -72,6 +72,8 @@ type RepoTransfer struct { //nolint:revive // export stutter
TeamIDs []int64
Teams []*organization.Team `xorm:"-"`
RecipientAccessGranted bool `xorm:"NOT NULL DEFAULT false"`
CreatedUnix timeutil.TimeStamp `xorm:"INDEX NOT NULL created"`
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX NOT NULL updated"`
}
@@ -221,7 +223,7 @@ func TestRepositoryReadyForTransfer(status RepositoryStatus) error {
// CreatePendingRepositoryTransfer transfer a repo from one owner to a new one.
// it marks the repository transfer as "pending"
func CreatePendingRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.User, repoID int64, teams []*organization.Team) error {
func CreatePendingRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.User, repoID int64, teams []*organization.Team, recipientAccessGranted bool) error {
return db.WithTx(ctx, func(ctx context.Context) error {
repo, err := GetRepositoryByID(ctx, repoID)
if err != nil {
@@ -270,6 +272,8 @@ func CreatePendingRepositoryTransfer(ctx context.Context, doer, newOwner *user_m
UpdatedUnix: timeutil.TimeStampNow(),
DoerID: doer.ID,
TeamIDs: make([]int64, 0, len(teams)),
RecipientAccessGranted: recipientAccessGranted,
}
for k := range teams {

View File

@@ -67,31 +67,31 @@ func AddOrUpdateCollaborator(ctx context.Context, repo *repo_model.Repository, u
}
// DeleteCollaboration removes collaboration relation between the user and repository.
func DeleteCollaboration(ctx context.Context, repo *repo_model.Repository, collaborator *user_model.User) (err error) {
collaboration := &repo_model.Collaboration{
RepoID: repo.ID,
UserID: collaborator.ID,
}
func DeleteCollaboration(ctx context.Context, repo *repo_model.Repository, collaborator *user_model.User) error {
return deleteCollaboration(ctx, repo, collaborator, &repo_model.Collaboration{RepoID: repo.ID, UserID: collaborator.ID})
}
func deleteCollaborationByMode(ctx context.Context, repo *repo_model.Repository, collaborator *user_model.User, mode perm.AccessMode) error {
return deleteCollaboration(ctx, repo, collaborator, &repo_model.Collaboration{
RepoID: repo.ID, UserID: collaborator.ID, Mode: mode,
})
}
func deleteCollaboration(ctx context.Context, repo *repo_model.Repository, collaborator *user_model.User, collaboration *repo_model.Collaboration) (err error) {
return db.WithTx(ctx, func(ctx context.Context) error {
if has, err := db.GetEngine(ctx).Delete(collaboration); err != nil {
if deleted, err := db.GetEngine(ctx).Delete(collaboration); err != nil {
return err
} else if has == 0 {
} else if deleted == 0 {
return nil
}
if err := repo.LoadOwner(ctx); err != nil {
return err
}
if err = access_model.RecalculateAccesses(ctx, repo); err != nil {
return err
}
if err = repo_model.WatchRepoAuto(ctx, collaborator, repo, false); err != nil {
return err
}
if err = ReconsiderWatches(ctx, repo, collaborator); err != nil {
return err
}
@@ -115,7 +115,8 @@ func ReconsiderRepoIssuesAssignee(ctx context.Context, repo *repo_model.Reposito
}
func ReconsiderWatches(ctx context.Context, repo *repo_model.Repository, user *user_model.User) error {
if has, err := access_model.HasAnyUnitAccess(ctx, user.ID, repo); err != nil || has {
permission, err := access_model.GetIndividualUserRepoPermission(ctx, repo, user)
if err != nil || permission.HasAnyUnitAccessOrPublicAccess() {
return err
}
if err := repo_model.WatchRepoAuto(ctx, user, repo, false); err != nil {

View File

@@ -11,6 +11,7 @@ import (
"gitea.dev/models/perm"
access_model "gitea.dev/models/perm/access"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unit"
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
@@ -98,3 +99,22 @@ func TestRepository_DeleteCollaborationRemovesSubscriptionsAndStopwatches(t *tes
assert.NoError(t, err)
assert.False(t, hasStopwatch)
}
func TestRepository_DeleteCollaborationPreservesWatchWithPublicAccess(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
ctx := t.Context()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
assert.NoError(t, repo_model.UpdateRepoUnitPublicAccess(ctx, &repo_model.RepoUnit{
RepoID: 2, Type: unit.TypeIssues, EveryoneAccessMode: perm.AccessModeRead,
}))
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
assert.NoError(t, repo.LoadOwner(ctx))
assert.NoError(t, AddOrUpdateCollaborator(ctx, repo, user, perm.AccessModeRead))
assert.NoError(t, repo_model.WatchRepoAuto(ctx, user, repo, true))
assert.NoError(t, DeleteCollaboration(ctx, repo, user))
watch, err := repo_model.GetWatch(ctx, user.ID, repo.ID)
assert.NoError(t, err)
assert.True(t, repo_model.IsWatchModeWatching(watch.Mode))
}

View File

@@ -450,7 +450,7 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
return transferOwnership(ctx, doer, newOwner.Name, repo, teams)
}
if user_model.IsUserBlockedBy(ctx, doer, newOwner.ID) {
if user_model.IsUserBlockedBy(ctx, doer, newOwner.ID) || user_model.IsUserBlockedBy(ctx, newOwner, repo.OwnerID) {
return user_model.ErrBlockedUser
}
@@ -471,15 +471,19 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
if err != nil {
return err
}
if !hasAccess {
if err := AddOrUpdateCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil {
grantRecipientTempAccess := !hasAccess
if grantRecipientTempAccess {
if err := db.Insert(ctx, &repo_model.Collaboration{RepoID: repo.ID, UserID: newOwner.ID, Mode: perm.AccessModeRead}); err != nil {
return err
}
if err := access_model.RecalculateUserAccess(ctx, repo, newOwner.ID); err != nil {
return err
}
}
// Make repo as pending for transfer
repo.Status = repo_model.RepositoryPendingTransfer
return repo_model.CreatePendingRepositoryTransfer(ctx, doer, newOwner, repo.ID, teams)
return repo_model.CreatePendingRepositoryTransfer(ctx, doer, newOwner, repo.ID, teams, grantRecipientTempAccess)
}); err != nil {
return err
}
@@ -511,6 +515,9 @@ func RejectRepositoryTransfer(ctx context.Context, repo *repo_model.Repository,
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
return util.ErrPermissionDenied
}
if err := removeTransferRecipientCollaboration(ctx, repoTransfer); err != nil {
return err
}
repo.Status = repo_model.RepositoryReady
if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "status"); err != nil {
@@ -521,6 +528,13 @@ func RejectRepositoryTransfer(ctx context.Context, repo *repo_model.Repository,
})
}
func removeTransferRecipientCollaboration(ctx context.Context, repoTransfer *repo_model.RepoTransfer) error {
if !repoTransfer.RecipientAccessGranted {
return nil
}
return deleteCollaborationByMode(ctx, repoTransfer.Repo, repoTransfer.Recipient, perm.AccessModeRead)
}
func canUserCancelTransfer(ctx context.Context, r *repo_model.RepoTransfer, u *user_model.User) bool {
if u.IsAdmin || u.ID == r.DoerID {
return true
@@ -559,6 +573,9 @@ func CancelRepositoryTransfer(ctx context.Context, repoTransfer *repo_model.Repo
if !canUserCancelTransfer(ctx, repoTransfer, doer) {
return util.ErrPermissionDenied
}
if err := removeTransferRecipientCollaboration(ctx, repoTransfer); err != nil {
return err
}
repoTransfer.Repo.Status = repo_model.RepositoryReady
if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repoTransfer.Repo, "status"); err != nil {

View File

@@ -9,6 +9,7 @@ import (
activities_model "gitea.dev/models/activities"
"gitea.dev/models/organization"
"gitea.dev/models/perm"
access_model "gitea.dev/models/perm/access"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unittest"
@@ -72,16 +73,60 @@ func TestStartRepositoryTransferSetPermission(t *testing.T) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
assert.NoError(t, repo.LoadOwner(t.Context()))
// the recipient doesn't have permission to access the repo
hasAccess, err := access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
assert.NoError(t, err)
assert.False(t, hasAccess)
assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
assert.NoError(t, err)
assert.True(t, hasAccess)
t.Run("RevokeAccessAfterRejection", func(t *testing.T) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
assert.NoError(t, repo.LoadOwner(t.Context()))
assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
assert.NoError(t, err)
assert.True(t, hasAccess)
assert.NoError(t, RejectRepositoryTransfer(t.Context(), repo, recipient))
hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
assert.NoError(t, err)
assert.False(t, hasAccess)
})
t.Run("RevokeAccessAfterCancel", func(t *testing.T) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
assert.NoError(t, repo.LoadOwner(t.Context()))
assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
assert.NoError(t, err)
assert.True(t, hasAccess)
transfer, err := repo_model.GetPendingRepositoryTransfer(t.Context(), repo)
assert.NoError(t, err)
assert.NoError(t, CancelRepositoryTransfer(t.Context(), transfer, doer))
hasAccess, err = access_model.HasAnyUnitAccess(t.Context(), recipient.ID, repo)
assert.NoError(t, err)
assert.False(t, hasAccess)
})
t.Run("KeepOriginAccessOnStop", func(t *testing.T) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
assert.NoError(t, repo.LoadOwner(t.Context()))
assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo, recipient, perm.AccessModeWrite))
assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
assert.NoError(t, RejectRepositoryTransfer(t.Context(), repo, recipient))
collaboration, err := repo_model.GetCollaboration(t.Context(), repo.ID, recipient.ID)
assert.NoError(t, err)
assert.Equal(t, perm.AccessModeWrite, collaboration.Mode)
assert.NoError(t, DeleteCollaboration(t.Context(), repo, recipient))
})
t.Run("KeepNonReadAccessOnStop", func(t *testing.T) {
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
assert.NoError(t, repo.LoadOwner(t.Context()))
assert.NoError(t, StartRepositoryTransfer(t.Context(), doer, recipient, repo, nil))
transfer, err := repo_model.GetPendingRepositoryTransfer(t.Context(), repo)
assert.NoError(t, err)
assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo, recipient, perm.AccessModeWrite))
assert.NoError(t, CancelRepositoryTransfer(t.Context(), transfer, doer))
collaboration, err := repo_model.GetCollaboration(t.Context(), repo.ID, recipient.ID)
assert.NoError(t, err)
assert.Equal(t, perm.AccessModeWrite, collaboration.Mode)
})
unittest.CheckConsistencyFor(t, &repo_model.Repository{}, &user_model.User{}, &organization.Team{})
}
@@ -105,7 +150,7 @@ func TestRepositoryTransfer(t *testing.T) {
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
assert.NoError(t, repo_model.CreatePendingRepositoryTransfer(t.Context(), doer, user2, repo.ID, nil))
assert.NoError(t, repo_model.CreatePendingRepositoryTransfer(t.Context(), doer, user2, repo.ID, nil, false))
transfer, err = repo_model.GetPendingRepositoryTransfer(t.Context(), repo)
assert.NoError(t, err)
@@ -115,13 +160,13 @@ func TestRepositoryTransfer(t *testing.T) {
org6 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
// Only transfer can be started at any given time
err = repo_model.CreatePendingRepositoryTransfer(t.Context(), doer, org6, repo.ID, nil)
err = repo_model.CreatePendingRepositoryTransfer(t.Context(), doer, org6, repo.ID, nil, false)
assert.Error(t, err)
assert.True(t, repo_model.IsErrRepoTransferInProgress(err))
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
// Unknown user, transfer non-existent transfer repo id = 2
err = repo_model.CreatePendingRepositoryTransfer(t.Context(), doer, &user_model.User{ID: 1000, LowerName: "user1000"}, repo2.ID, nil)
err = repo_model.CreatePendingRepositoryTransfer(t.Context(), doer, &user_model.User{ID: 1000, LowerName: "user1000"}, repo2.ID, nil, false)
assert.Error(t, err)
// Reject transfer