From 8f7c155a98a71c43ec2a15504380acb47b3a5fc7 Mon Sep 17 00:00:00 2001 From: Ross Golder Date: Sun, 17 May 2026 13:23:57 +0700 Subject: [PATCH] fix(perm): fix RecalculateUserAccess to not insert empty access records Add test for RecalculateUserAccess to verify no access record is created for users without collaboration/team membership, and collaborator gets correct access mode. Co-Authored-By: Ross Golder --- models/perm/access/access.go | 2 +- models/perm/access/access_test.go | 33 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/models/perm/access/access.go b/models/perm/access/access.go index e9e5a0c026..eb0bd57633 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -267,7 +267,7 @@ func RecalculateUserAccess(ctx context.Context, repo *repo_model.Repository, uid // Delete old user accesses and insert new one for repository. if _, err = e.Delete(&Access{RepoID: repo.ID, UserID: uid}); err != nil { return fmt.Errorf("delete old user accesses: %w", err) - } else if accessMode >= minMode { + } else if accessMode > minMode { if err = db.Insert(ctx, &Access{RepoID: repo.ID, UserID: uid, Mode: accessMode}); err != nil { return fmt.Errorf("insert new user accesses: %w", err) } diff --git a/models/perm/access/access_test.go b/models/perm/access/access_test.go index 148a02efa3..b75506cf4c 100644 --- a/models/perm/access/access_test.go +++ b/models/perm/access/access_test.go @@ -26,6 +26,7 @@ func TestAccess(t *testing.T) { t.Run("RecalculateAccesses2", testRecalculateAccesses2) t.Run("RecalculateAccessesUpdateMode", testRecalculateAccessesUpdateMode) t.Run("RecalculateAccessesRemoveAccess", testRecalculateAccessesRemoveAccess) + t.Run("RecalculateUserAccess", testRecalculateUserAccess) } func testAccessLevel(t *testing.T) { @@ -201,3 +202,35 @@ func testRecalculateAccessesRemoveAccess(t *testing.T) { assert.NoError(t, err) assert.False(t, has, "Access should be deleted after removing collaboration") } + +func testRecalculateUserAccess(t *testing.T) { + t.Run("NoAccessForUserWithoutCollaboration", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + assert.NoError(t, repo.LoadOwner(t.Context())) + + userID := int64(8) + _, _ = db.GetEngine(t.Context()).Where("user_id = ? AND repo_id = ?", userID, repo.ID).Delete(&access_model.Access{}) + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), repo, userID)) + + access := &access_model.Access{UserID: userID, RepoID: repo.ID} + has, err := db.GetEngine(t.Context()).Get(access) + assert.NoError(t, err) + assert.False(t, has, "User without collaboration/team membership should have no access record") + }) + + t.Run("CollaboratorGetsAccess", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + assert.NoError(t, repo.LoadOwner(t.Context())) + + userID := int64(4) + _ = db.Insert(t.Context(), &repo_model.Collaboration{UserID: userID, RepoID: repo.ID, Mode: perm_model.AccessModeWrite}) + + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), repo, userID)) + + access := &access_model.Access{UserID: userID, RepoID: repo.ID} + has, err := db.GetEngine(t.Context()).Get(access) + assert.NoError(t, err) + assert.True(t, has) + assert.Equal(t, perm_model.AccessModeWrite, access.Mode) + }) +}