Skip to content

Commit a88e3e6

Browse files
Gustedearl-warren
authored andcommitted
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. (cherry picked from commit b701966)
1 parent 6c75d1a commit a88e3e6

File tree

3 files changed

+77
-5
lines changed

3 files changed

+77
-5
lines changed
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
-
2+
id: 1001
3+
owner_id: 33
4+
owner_name: user33
5+
lower_name: repo1001
6+
name: repo1001
7+
default_branch: main
8+
num_watches: 0
9+
num_stars: 0
10+
num_forks: 0
11+
num_issues: 0
12+
num_closed_issues: 0
13+
num_pulls: 0
14+
num_closed_pulls: 0
15+
num_milestones: 0
16+
num_closed_milestones: 0
17+
num_projects: 0
18+
num_closed_projects: 0
19+
is_private: false
20+
is_empty: false
21+
is_archived: false
22+
is_mirror: false
23+
status: 0
24+
is_fork: false
25+
fork_id: 0
26+
is_template: false
27+
template_id: 0
28+
size: 0
29+
is_fsck_enabled: true
30+
close_issues_via_commit_in_any_branch: false

models/repo/repo_list.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -641,12 +641,9 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu
641641
// 1. Be able to see all non-private repositories that either:
642642
cond = cond.Or(builder.And(
643643
builder.Eq{"`repository`.is_private": false},
644-
// 2. Aren't in an private organisation or limited organisation if we're not logged in
644+
// 2. Aren't in an private organisation/user or limited organisation/user if the doer is not logged in.
645645
builder.NotIn("`repository`.owner_id", builder.Select("id").From("`user`").Where(
646-
builder.And(
647-
builder.Eq{"type": user_model.UserTypeOrganization},
648-
builder.In("visibility", orgVisibilityLimit)),
649-
))))
646+
builder.In("visibility", orgVisibilityLimit)))))
650647
}
651648

652649
if user != nil {

models/repo/repo_list_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@
44
package repo_test
55

66
import (
7+
"path/filepath"
8+
"slices"
79
"strings"
810
"testing"
911

1012
"code.gitea.io/gitea/models/db"
1113
repo_model "code.gitea.io/gitea/models/repo"
1214
"code.gitea.io/gitea/models/unittest"
15+
"code.gitea.io/gitea/models/user"
1316
"code.gitea.io/gitea/modules/optional"
17+
"code.gitea.io/gitea/modules/setting"
18+
"code.gitea.io/gitea/modules/structs"
1419

1520
"github.com/stretchr/testify/assert"
1621
"github.com/stretchr/testify/require"
@@ -403,3 +408,43 @@ func TestSearchRepositoryByTopicName(t *testing.T) {
403408
})
404409
}
405410
}
411+
412+
func TestSearchRepositoryIDsByCondition(t *testing.T) {
413+
defer unittest.OverrideFixtures(
414+
unittest.FixturesOptions{
415+
Dir: filepath.Join(setting.AppWorkPath, "models/fixtures/"),
416+
Base: setting.AppWorkPath,
417+
Dirs: []string{"models/repo/TestSearchRepositoryIDsByCondition/"},
418+
},
419+
)()
420+
require.NoError(t, unittest.PrepareTestDatabase())
421+
// Sanity check of the database
422+
limitedUser := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 33, Visibility: structs.VisibleTypeLimited})
423+
unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1001, OwnerID: limitedUser.ID})
424+
425+
testCases := []struct {
426+
user *user.User
427+
repoIDs []int64
428+
}{
429+
{
430+
user: nil,
431+
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},
432+
},
433+
{
434+
user: unittest.AssertExistsAndLoadBean(t, &user.User{ID: 4}),
435+
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},
436+
},
437+
{
438+
user: unittest.AssertExistsAndLoadBean(t, &user.User{ID: 5}),
439+
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},
440+
},
441+
}
442+
443+
for _, testCase := range testCases {
444+
repoIDs, err := repo_model.FindUserCodeAccessibleRepoIDs(db.DefaultContext, testCase.user)
445+
require.NoError(t, err)
446+
447+
slices.Sort(repoIDs)
448+
assert.EqualValues(t, testCase.repoIDs, repoIDs)
449+
}
450+
}

0 commit comments

Comments
 (0)