From b70196653f9d7d3b9d4e72d114e5cc6f472988c4 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 9 Nov 2024 23:47:39 +0100 Subject: [PATCH] fix: anomynous users code search for private/limited user's repository - Consider private/limited users in the `AccessibleRepositoryCondition` query, previously this only considered private/limited organization. This limits the ability for anomynous users to do code search on private/limited user's repository - Unit test added. --- .../repository.yml | 30 +++++++++++++ models/repo/repo_list.go | 7 +-- models/repo/repo_list_test.go | 45 +++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 models/repo/TestSearchRepositoryIDsByCondition/repository.yml diff --git a/models/repo/TestSearchRepositoryIDsByCondition/repository.yml b/models/repo/TestSearchRepositoryIDsByCondition/repository.yml new file mode 100644 index 0000000000..9ce830783d --- /dev/null +++ b/models/repo/TestSearchRepositoryIDsByCondition/repository.yml @@ -0,0 +1,30 @@ +- + id: 1001 + owner_id: 33 + owner_name: user33 + lower_name: repo1001 + name: repo1001 + default_branch: main + num_watches: 0 + num_stars: 0 + num_forks: 0 + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + num_milestones: 0 + num_closed_milestones: 0 + num_projects: 0 + num_closed_projects: 0 + is_private: false + is_empty: false + is_archived: false + is_mirror: false + status: 0 + is_fork: false + fork_id: 0 + is_template: false + template_id: 0 + size: 0 + is_fsck_enabled: true + close_issues_via_commit_in_any_branch: false diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 162f933fbe..fc51f64f6a 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -641,12 +641,9 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu // 1. Be able to see all non-private repositories that either: cond = cond.Or(builder.And( builder.Eq{"`repository`.is_private": false}, - // 2. Aren't in an private organisation or limited organisation if we're not logged in + // 2. Aren't in an private organisation/user or limited organisation/user if the doer is not logged in. builder.NotIn("`repository`.owner_id", builder.Select("id").From("`user`").Where( - builder.And( - builder.Eq{"type": user_model.UserTypeOrganization}, - builder.In("visibility", orgVisibilityLimit)), - )))) + builder.In("visibility", orgVisibilityLimit))))) } if user != nil { diff --git a/models/repo/repo_list_test.go b/models/repo/repo_list_test.go index b31aa1780f..8c13f387ba 100644 --- a/models/repo/repo_list_test.go +++ b/models/repo/repo_list_test.go @@ -4,13 +4,18 @@ package repo_test import ( + "path/filepath" + "slices" "strings" "testing" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -403,3 +408,43 @@ func TestSearchRepositoryByTopicName(t *testing.T) { }) } } + +func TestSearchRepositoryIDsByCondition(t *testing.T) { + defer unittest.OverrideFixtures( + unittest.FixturesOptions{ + Dir: filepath.Join(setting.AppWorkPath, "models/fixtures/"), + Base: setting.AppWorkPath, + Dirs: []string{"models/repo/TestSearchRepositoryIDsByCondition/"}, + }, + )() + require.NoError(t, unittest.PrepareTestDatabase()) + // Sanity check of the database + limitedUser := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 33, Visibility: structs.VisibleTypeLimited}) + unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1001, OwnerID: limitedUser.ID}) + + testCases := []struct { + user *user.User + repoIDs []int64 + }{ + { + user: nil, + repoIDs: []int64{1, 4, 8, 9, 10, 11, 12, 14, 17, 18, 21, 23, 25, 27, 29, 32, 33, 34, 35, 36, 37, 42, 44, 45, 46, 47, 48, 49, 50, 51, 53, 57, 58, 60, 61, 62, 1059}, + }, + { + user: unittest.AssertExistsAndLoadBean(t, &user.User{ID: 4}), + repoIDs: []int64{1, 3, 4, 8, 9, 10, 11, 12, 14, 17, 18, 21, 23, 25, 27, 29, 32, 33, 34, 35, 36, 37, 38, 40, 42, 44, 45, 46, 47, 48, 49, 50, 51, 53, 57, 58, 60, 61, 62, 1001, 1059}, + }, + { + user: unittest.AssertExistsAndLoadBean(t, &user.User{ID: 5}), + repoIDs: []int64{1, 4, 8, 9, 10, 11, 12, 14, 17, 18, 21, 23, 25, 27, 29, 32, 33, 34, 35, 36, 37, 38, 40, 42, 44, 45, 46, 47, 48, 49, 50, 51, 53, 57, 58, 60, 61, 62, 1001, 1059}, + }, + } + + for _, testCase := range testCases { + repoIDs, err := repo_model.FindUserCodeAccessibleRepoIDs(db.DefaultContext, testCase.user) + require.NoError(t, err) + + slices.Sort(repoIDs) + assert.EqualValues(t, testCase.repoIDs, repoIDs) + } +}