fix: updateRepoPermissions to cleanup old permissions

server/api/login.go

@@ -314,6 +314,8 @@ func updateRepoPermissions(c *gin.Context, user *model.User, _store store.Store,
 		return err
 	}
 
+	var repoIDs []int64
+
 	for _, forgeRepo := range repos {
 		// make sure forgeID is set
 		forgeRepo.ForgeID = forgeID
@@ -339,6 +341,12 @@ func updateRepoPermissions(c *gin.Context, user *model.User, _store store.Store,
 		if err := _store.PermUpsert(perm); err != nil {
 			return err
 		}
+
+		repoIDs = append(repoIDs, dbRepo.ID)
+	}
+
+	if err := _store.PermPrune(user.ID, repoIDs); err != nil {
+		return err
 	}
 
 	return nil

server/api/login_test.go

@@ -164,6 +164,7 @@ func TestHandleAuth(t *testing.T) {
 		_store.On("OrgFindByName", user.Login, user.ForgeID).Return(nil, nil)
 		_store.On("OrgCreate", mock.Anything).Return(nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
+		_store.On("PermPrune", mock.Anything, []int64(nil)).Return(nil)
 		_forge.On("Repos", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
 
 		api.HandleAuth(c)
@@ -196,6 +197,7 @@ func TestHandleAuth(t *testing.T) {
 		_store.On("GetUserByRemoteID", user.ForgeID, user.ForgeRemoteID).Return(user, nil)
 		_store.On("OrgGet", org.ID).Return(org, nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
+		_store.On("PermPrune", mock.Anything, []int64(nil)).Return(nil)
 		_forge.On("Repos", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
 
 		api.HandleAuth(c)
@@ -291,6 +293,7 @@ func TestHandleAuth(t *testing.T) {
 		_store.On("OrgFindByName", user.Login, user.ForgeID).Return(nil, types.RecordNotExist)
 		_store.On("OrgCreate", mock.Anything).Return(nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
+		_store.On("PermPrune", mock.Anything, []int64(nil)).Return(nil)
 		_forge.On("Repos", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
 
 		api.HandleAuth(c)
@@ -325,6 +328,7 @@ func TestHandleAuth(t *testing.T) {
 		_store.On("OrgFindByName", user.Login, user.ForgeID).Return(org, nil)
 		_store.On("OrgUpdate", mock.Anything).Return(nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
+		_store.On("PermPrune", mock.Anything, []int64(nil)).Return(nil)
 		_forge.On("Repos", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
 
 		api.HandleAuth(c)
@@ -359,6 +363,7 @@ func TestHandleAuth(t *testing.T) {
 		_store.On("OrgGet", user.OrgID).Return(org, nil)
 		_store.On("OrgUpdate", mock.Anything).Return(nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
+		_store.On("PermPrune", mock.Anything, []int64(nil)).Return(nil)
 		_forge.On("Repos", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
 
 		api.HandleAuth(c)

server/store/datastore/permission.go

@@ -83,3 +83,18 @@ func userPushOrAdminCondition(userID int64) builder.Cond {
 func userIDAndRepoIDCond(perm *model.Perm) builder.Cond {
 	return builder.Eq{"user_id": perm.UserID, "repo_id": perm.RepoID}
 }
+
+// PermPrune deletes all permission rows for a user
+// where the repo_id is NOT IN the provided keepRepoIDs list. If keepRepoIDs
+// is empty, all permissions for the user are deleted.
+func (s storage) PermPrune(userID int64, keepRepoIDs []int64) error {
+	if len(keepRepoIDs) == 0 {
+		_, err := s.engine.Where(builder.Eq{"user_id": userID}).Delete(new(model.Perm))
+		return err
+	}
+
+	_, err := s.engine.Where(builder.Eq{"user_id": userID}).
+		And(builder.NotIn("repo_id", keepRepoIDs)).
+		Delete(new(model.Perm))
+	return err
+}

server/store/datastore/permission_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"go.woodpecker-ci.org/woodpecker/v3/server/model"
+	"go.woodpecker-ci.org/woodpecker/v3/server/store/types"
 )
 
 func TestPermFind(t *testing.T) {
@@ -110,3 +111,83 @@ func TestPermUpsert(t *testing.T) {
 	assert.True(t, perm.Push)
 	assert.True(t, perm.Admin)
 }
+
+func TestPermPruneDeleteAll(t *testing.T) {
+	store, closer := newTestStore(t, new(model.Repo), new(model.Perm), new(model.User))
+	defer closer()
+
+	user := &model.User{ID: 1}
+
+	repo1 := &model.Repo{
+		UserID:        1,
+		FullName:      "woodpecker-ci/woodpecker1",
+		Owner:         "woodpecker-ci",
+		Name:          "repo1",
+		ForgeRemoteID: "101",
+	}
+	repo2 := &model.Repo{
+		UserID:        1,
+		FullName:      "woodpecker-ci/woodpecker2",
+		Owner:         "woodpecker",
+		Name:          "repo2",
+		ForgeRemoteID: "102",
+	}
+	assert.NoError(t, store.CreateRepo(repo1))
+	assert.NoError(t, store.CreateRepo(repo2))
+
+	assert.NoError(t, store.PermUpsert(&model.Perm{UserID: user.ID, RepoID: repo1.ID, Repo: repo1, Pull: true}))
+	assert.NoError(t, store.PermUpsert(&model.Perm{UserID: user.ID, RepoID: repo2.ID, Repo: repo2, Pull: true}))
+
+	_, err := store.PermFind(user, repo1)
+	assert.NoError(t, err)
+	_, err = store.PermFind(user, repo2)
+	assert.NoError(t, err)
+
+	assert.NoError(t, store.PermPrune(user.ID, []int64{}))
+
+	_, err = store.PermFind(user, repo1)
+	assert.ErrorIs(t, err, types.RecordNotExist)
+	_, err = store.PermFind(user, repo2)
+	assert.ErrorIs(t, err, types.RecordNotExist)
+}
+
+func TestPermPruneKeepOne(t *testing.T) {
+	store, closer := newTestStore(t, new(model.Repo), new(model.Perm), new(model.User))
+	defer closer()
+
+	user := &model.User{ID: 1}
+
+	repo1 := &model.Repo{
+		UserID:        1,
+		FullName:      "woodpecker-ci/woodpecker1",
+		Owner:         "woodpecker-ci",
+		Name:          "repo1",
+		ForgeRemoteID: "101",
+	}
+	repo2 := &model.Repo{
+		UserID:        1,
+		FullName:      "woodpecker-ci/woodpecker2",
+		Owner:         "woodpecker",
+		Name:          "repo2",
+		ForgeRemoteID: "102",
+	}
+	assert.NoError(t, store.CreateRepo(repo1))
+	assert.NoError(t, store.CreateRepo(repo2))
+
+	assert.NoError(t, store.PermUpsert(&model.Perm{UserID: user.ID, RepoID: repo1.ID, Repo: repo1, Pull: true}))
+	assert.NoError(t, store.PermUpsert(&model.Perm{UserID: user.ID, RepoID: repo2.ID, Repo: repo2, Pull: true}))
+
+	_, err := store.PermFind(user, repo1)
+	assert.NoError(t, err)
+	_, err = store.PermFind(user, repo2)
+	assert.NoError(t, err)
+
+	// Prune everything EXCEPT repo2
+	assert.NoError(t, store.PermPrune(user.ID, []int64{repo2.ID}))
+
+	_, err = store.PermFind(user, repo1)
+	assert.ErrorIs(t, err, types.RecordNotExist)
+
+	_, err = store.PermFind(user, repo2)
+	assert.NoError(t, err)
+}

server/store/store.go

@@ -104,6 +104,7 @@ type Store interface {
 	// Permissions
 	PermFind(user *model.User, repo *model.Repo) (*model.Perm, error)
 	PermUpsert(perm *model.Perm) error
+	PermPrune(userID int64, keepRepoIDs []int64) error
 
 	// Configs
 	ConfigsForPipeline(pipelineID int64) ([]*model.Config, error)