Skip to content

Commit a2e79d8

Browse files
hoitihhoitih
committed
When non-admin users use code search, get code unit accessible repos in one main query
1 parent ad551bf commit a2e79d8

File tree

5 files changed

+109
-108
lines changed

5 files changed

+109
-108
lines changed

‎models/issue.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,7 +1375,7 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati
13751375
cond = cond.And(
13761376
builder.Or(
13771377
userOwnedRepoCond(userID), // owned repos
1378-
userCollaborationRepoCond(repoIDstr, userID), // collaboration repos
1378+
userAccessRepoCond(repoIDstr, userID), // collaboration repos
13791379
userAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible public repos
13801380
userMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible public repos
13811381
userCreateIssueRepoCond(repoIDstr, userID, isPull), // user has created issue/pr accessible public repos
@@ -1444,7 +1444,7 @@ func GetRepoIDsForIssuesOptions(opts *IssuesOptions, user *user_model.User) ([]i
14441444

14451445
opts.setupSessionNoLimit(sess)
14461446

1447-
accessCond := accessibleRepositoryCondition(user)
1447+
accessCond := accessibleRepositoryCondition(user, unit.TypeInvalid)
14481448
if err := sess.Where(accessCond).
14491449
Distinct("issue.repo_id").
14501450
Table("issue").

‎models/lfs.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"code.gitea.io/gitea/models/db"
1313
repo_model "code.gitea.io/gitea/models/repo"
14+
"code.gitea.io/gitea/models/unit"
1415
user_model "code.gitea.io/gitea/models/user"
1516
"code.gitea.io/gitea/modules/lfs"
1617
"code.gitea.io/gitea/modules/log"
@@ -142,7 +143,7 @@ func LFSObjectAccessible(user *user_model.User, oid string) (bool, error) {
142143
count, err := db.GetEngine(db.DefaultContext).Count(&LFSMetaObject{Pointer: lfs.Pointer{Oid: oid}})
143144
return count > 0, err
144145
}
145-
cond := accessibleRepositoryCondition(user)
146+
cond := accessibleRepositoryCondition(user, unit.TypeInvalid)
146147
count, err := db.GetEngine(db.DefaultContext).Where(cond).Join("INNER", "repository", "`lfs_meta_object`.repository_id = `repository`.id").Count(&LFSMetaObject{Pointer: lfs.Pointer{Oid: oid}})
147148
return count > 0, err
148149
}
@@ -173,7 +174,7 @@ func LFSAutoAssociate(metas []*LFSMetaObject, user *user_model.User, repoID int6
173174
newMetas := make([]*LFSMetaObject, 0, len(metas))
174175
cond := builder.In(
175176
"`lfs_meta_object`.repository_id",
176-
builder.Select("`repository`.id").From("repository").Where(accessibleRepositoryCondition(user)),
177+
builder.Select("`repository`.id").From("repository").Where(accessibleRepositoryCondition(user, unit.TypeInvalid)),
177178
)
178179
err = sess.Cols("oid").Where(cond).In("oid", oids...).GroupBy("oid").Find(&newMetas)
179180
if err != nil {

‎models/org.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/models/organization"
1515
access_model "code.gitea.io/gitea/models/perm/access"
1616
repo_model "code.gitea.io/gitea/models/repo"
17+
"code.gitea.io/gitea/models/unit"
1718
user_model "code.gitea.io/gitea/models/user"
1819

1920
"xorm.io/builder"
@@ -54,7 +55,7 @@ func GetUserOrgsList(user *user_model.User) ([]*MinimalOrg, error) {
5455
Join("LEFT", builder.
5556
Select("id as repo_id, owner_id as repo_owner_id").
5657
From("repository").
57-
Where(accessibleRepositoryCondition(user)), "`repository`.repo_owner_id = `team`.org_id").
58+
Where(accessibleRepositoryCondition(user, unit.TypeInvalid)), "`repository`.repo_owner_id = `team`.org_id").
5859
Where("`team_user`.uid = ?", user.ID).
5960
GroupBy(groupByStr)
6061

‎models/repo_list.go

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,8 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ
282282
))
283283
}
284284

285-
// userCollaborationRepoCond returns user as collabrators repositories list
286-
func userCollaborationRepoCond(idStr string, userID int64) builder.Cond {
285+
// userAccessRepoCond returns a condition for selecting all repositories a user has access to
286+
func userAccessRepoCond(idStr string, userID int64) builder.Cond {
287287
return builder.In(idStr, builder.Select("repo_id").
288288
From("`access`").
289289
Where(builder.And(
@@ -293,6 +293,17 @@ func userCollaborationRepoCond(idStr string, userID int64) builder.Cond {
293293
)
294294
}
295295

296+
// userCollaborationRepoCond returns a condition for selecting all repositories a user is collaborator in
297+
func userCollaborationRepoCond(idStr string, userID int64) builder.Cond {
298+
return builder.In(idStr, builder.Select("repo_id").
299+
From("`collaboration`").
300+
Where(builder.And(
301+
builder.Eq{"`collaboration`.user_id": userID},
302+
builder.Gt{"`collaboration`.mode": int(perm.AccessModeNone)},
303+
)),
304+
)
305+
}
306+
296307
// userOrgTeamRepoCond selects repos that the given user has access to through team membership
297308
func userOrgTeamRepoCond(idStr string, userID int64) builder.Cond {
298309
return builder.In(idStr, userOrgTeamRepoBuilder(userID))
@@ -310,7 +321,13 @@ func userOrgTeamRepoBuilder(userID int64) *builder.Builder {
310321
func userOrgTeamUnitRepoBuilder(userID int64, unitType unit.Type) *builder.Builder {
311322
return userOrgTeamRepoBuilder(userID).
312323
Join("INNER", "team_unit", "`team_unit`.team_id = `team_repo`.team_id").
313-
Where(builder.Eq{"`team_unit`.`type`": unitType})
324+
Where(builder.Eq{"`team_unit`.`type`": unitType}).
325+
And(builder.Gt{"`team_unit`.`access_mode`": int(perm.AccessModeNone)})
326+
}
327+
328+
// userOrgTeamUnitRepoCond returns a condition to select repo ids where user's teams can access the special unit.
329+
func userOrgTeamUnitRepoCond(idStr string, userID int64, unitType unit.Type) builder.Cond {
330+
return builder.In(idStr, userOrgTeamUnitRepoBuilder(userID, unitType))
314331
}
315332

316333
// userOrgUnitRepoCond selects repos that the given user has access to through org and the special unit
@@ -363,7 +380,7 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond {
363380
if opts.Private {
364381
if opts.Actor != nil && !opts.Actor.IsAdmin && opts.Actor.ID != opts.OwnerID {
365382
// OK we're in the context of a User
366-
cond = cond.And(accessibleRepositoryCondition(opts.Actor))
383+
cond = cond.And(accessibleRepositoryCondition(opts.Actor, unit.TypeInvalid))
367384
}
368385
} else {
369386
// Not looking at private organisations and users
@@ -409,7 +426,7 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond {
409426
// 2. But we can see because of:
410427
builder.Or(
411428
// A. We have access
412-
userCollaborationRepoCond("`repository`.id", opts.OwnerID),
429+
userAccessRepoCond("`repository`.id", opts.OwnerID),
413430
// B. We are in a team for
414431
userOrgTeamRepoCond("`repository`.id", opts.OwnerID),
415432
// C. Public repositories in organizations that we are member of
@@ -483,7 +500,7 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond {
483500
}
484501

485502
if opts.Actor != nil && opts.Actor.IsRestricted {
486-
cond = cond.And(accessibleRepositoryCondition(opts.Actor))
503+
cond = cond.And(accessibleRepositoryCondition(opts.Actor, unit.TypeInvalid))
487504
}
488505

489506
if opts.Archived != util.OptionalBoolNone {
@@ -570,7 +587,7 @@ func searchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond) (db
570587
}
571588

572589
// accessibleRepositoryCondition takes a user a returns a condition for checking if a repository is accessible
573-
func accessibleRepositoryCondition(user *user_model.User) builder.Cond {
590+
func accessibleRepositoryCondition(user *user_model.User, unitType unit.Type) builder.Cond {
574591
cond := builder.NewCond()
575592

576593
if user == nil || !user.IsRestricted || user.ID <= 0 {
@@ -590,13 +607,24 @@ func accessibleRepositoryCondition(user *user_model.User) builder.Cond {
590607
}
591608

592609
if user != nil {
610+
// 2. Be able to see all repositories that we have access to
611+
// 3. Be able to see all repositories through team membership(s)
612+
if unitType == unit.TypeInvalid {
613+
// Regardless of UnitType
614+
cond = cond.Or(
615+
userAccessRepoCond("`repository`.id", user.ID),
616+
userOrgTeamRepoCond("`repository`.id", user.ID),
617+
)
618+
} else {
619+
// For a specific UnitType
620+
cond = cond.Or(
621+
userAccessRepoCond("`repository`.id", user.ID),
622+
userOrgTeamUnitRepoCond("`repository`.id", user.ID, unitType),
623+
)
624+
}
593625
cond = cond.Or(
594-
// 2. Be able to see all repositories that we have access to
595-
userCollaborationRepoCond("`repository`.id", user.ID),
596-
// 3. Repositories that we directly own
626+
// 4. Repositories that we directly own
597627
builder.Eq{"`repository`.owner_id": user.ID},
598-
// 4. Be able to see all repositories that we are in a team
599-
userOrgTeamRepoCond("`repository`.id", user.ID),
600628
// 5. Be able to see all public repos in private organizations that we are an org_user of
601629
userOrgPublicRepoCond(user.ID),
602630
)
@@ -641,18 +669,18 @@ func SearchRepositoryIDs(opts *SearchRepoOptions) ([]int64, int64, error) {
641669
// AccessibleRepoIDsQuery queries accessible repository ids. Usable as a subquery wherever repo ids need to be filtered.
642670
func AccessibleRepoIDsQuery(user *user_model.User) *builder.Builder {
643671
// NB: Please note this code needs to still work if user is nil
644-
return builder.Select("id").From("repository").Where(accessibleRepositoryCondition(user))
672+
return builder.Select("id").From("repository").Where(accessibleRepositoryCondition(user, unit.TypeInvalid))
645673
}
646674

647-
// FindUserAccessibleRepoIDs find all accessible repositories' ID by user's id
648-
func FindUserAccessibleRepoIDs(user *user_model.User) ([]int64, error) {
675+
// FindUserCodeAccessibleRepoIDs find all accessible repositories' ID by user's id
676+
func FindUserCodeAccessibleRepoIDs(user *user_model.User) ([]int64, error) {
649677
repoIDs := make([]int64, 0, 10)
650678
if err := db.GetEngine(db.DefaultContext).
651679
Table("repository").
652680
Cols("id").
653-
Where(accessibleRepositoryCondition(user)).
681+
Where(accessibleRepositoryCondition(user, unit.TypeCode)).
654682
Find(&repoIDs); err != nil {
655-
return nil, fmt.Errorf("FindUserAccesibleRepoIDs: %v", err)
683+
return nil, fmt.Errorf("FindUserCodeAccesibleRepoIDs: %v", err)
656684
}
657685
return repoIDs, nil
658686
}

‎routers/web/explore/code.go

Lines changed: 57 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"code.gitea.io/gitea/models"
1111
repo_model "code.gitea.io/gitea/models/repo"
12-
"code.gitea.io/gitea/models/unit"
1312
"code.gitea.io/gitea/modules/base"
1413
"code.gitea.io/gitea/modules/context"
1514
code_indexer "code.gitea.io/gitea/modules/indexer/code"
@@ -44,106 +43,78 @@ func Code(ctx *context.Context) {
4443
queryType := ctx.FormTrim("t")
4544
isMatch := queryType == "match"
4645

47-
var (
48-
repoIDs []int64
49-
err error
50-
isAdmin bool
51-
)
52-
if ctx.Doer != nil {
53-
isAdmin = ctx.Doer.IsAdmin
54-
}
55-
56-
// guest user or non-admin user
57-
if ctx.Doer == nil || !isAdmin {
58-
repoIDs, err = models.FindUserAccessibleRepoIDs(ctx.Doer)
59-
if err != nil {
60-
ctx.ServerError("SearchResults", err)
61-
return
62-
}
63-
}
64-
65-
var (
66-
total int
67-
searchResults []*code_indexer.Result
68-
searchResultLanguages []*code_indexer.SearchResultLanguages
69-
)
70-
71-
// if non-admin login user, we need check UnitTypeCode at first
72-
if ctx.Doer != nil && len(repoIDs) > 0 {
73-
repoMaps, err := repo_model.GetRepositoriesMapByIDs(repoIDs)
74-
if err != nil {
75-
ctx.ServerError("SearchResults", err)
76-
return
46+
if keyword != "" {
47+
var (
48+
repoIDs []int64
49+
err error
50+
isAdmin bool
51+
)
52+
if ctx.Doer != nil {
53+
isAdmin = ctx.Doer.IsAdmin
7754
}
7855

79-
rightRepoMap := make(map[int64]*repo_model.Repository, len(repoMaps))
80-
repoIDs = make([]int64, 0, len(repoMaps))
81-
for id, repo := range repoMaps {
82-
if models.CheckRepoUnitUser(repo, ctx.Doer, unit.TypeCode) {
83-
rightRepoMap[id] = repo
84-
repoIDs = append(repoIDs, id)
85-
}
86-
}
87-
88-
ctx.Data["RepoMaps"] = rightRepoMap
89-
90-
total, searchResults, searchResultLanguages, err = code_indexer.PerformSearch(ctx, repoIDs, language, keyword, page, setting.UI.RepoSearchPagingNum, isMatch)
91-
if err != nil {
92-
if code_indexer.IsAvailable() {
56+
// guest user or non-admin user
57+
if ctx.Doer == nil || !isAdmin {
58+
repoIDs, err = models.FindUserCodeAccessibleRepoIDs(ctx.Doer)
59+
if err != nil {
9360
ctx.ServerError("SearchResults", err)
9461
return
9562
}
96-
ctx.Data["CodeIndexerUnavailable"] = true
97-
} else {
98-
ctx.Data["CodeIndexerUnavailable"] = !code_indexer.IsAvailable()
9963
}
100-
// if non-login user or isAdmin, no need to check UnitTypeCode
101-
} else if (ctx.Doer == nil && len(repoIDs) > 0) || isAdmin {
102-
total, searchResults, searchResultLanguages, err = code_indexer.PerformSearch(ctx, repoIDs, language, keyword, page, setting.UI.RepoSearchPagingNum, isMatch)
103-
if err != nil {
104-
if code_indexer.IsAvailable() {
105-
ctx.ServerError("SearchResults", err)
106-
return
64+
65+
var (
66+
total int
67+
searchResults []*code_indexer.Result
68+
searchResultLanguages []*code_indexer.SearchResultLanguages
69+
)
70+
71+
if (len(repoIDs) > 0) || isAdmin {
72+
total, searchResults, searchResultLanguages, err = code_indexer.PerformSearch(ctx, repoIDs, language, keyword, page, setting.UI.RepoSearchPagingNum, isMatch)
73+
if err != nil {
74+
if code_indexer.IsAvailable() {
75+
ctx.ServerError("SearchResults", err)
76+
return
77+
}
78+
ctx.Data["CodeIndexerUnavailable"] = true
79+
} else {
80+
ctx.Data["CodeIndexerUnavailable"] = !code_indexer.IsAvailable()
10781
}
108-
ctx.Data["CodeIndexerUnavailable"] = true
109-
} else {
110-
ctx.Data["CodeIndexerUnavailable"] = !code_indexer.IsAvailable()
111-
}
11282

113-
loadRepoIDs := make([]int64, 0, len(searchResults))
114-
for _, result := range searchResults {
115-
var find bool
116-
for _, id := range loadRepoIDs {
117-
if id == result.RepoID {
118-
find = true
119-
break
83+
loadRepoIDs := make([]int64, 0, len(searchResults))
84+
for _, result := range searchResults {
85+
var find bool
86+
for _, id := range loadRepoIDs {
87+
if id == result.RepoID {
88+
find = true
89+
break
90+
}
91+
}
92+
if !find {
93+
loadRepoIDs = append(loadRepoIDs, result.RepoID)
12094
}
12195
}
122-
if !find {
123-
loadRepoIDs = append(loadRepoIDs, result.RepoID)
96+
97+
repoMaps, err := repo_model.GetRepositoriesMapByIDs(loadRepoIDs)
98+
if err != nil {
99+
ctx.ServerError("SearchResults", err)
100+
return
124101
}
125-
}
126102

127-
repoMaps, err := repo_model.GetRepositoriesMapByIDs(loadRepoIDs)
128-
if err != nil {
129-
ctx.ServerError("SearchResults", err)
130-
return
103+
ctx.Data["RepoMaps"] = repoMaps
131104
}
132105

133-
ctx.Data["RepoMaps"] = repoMaps
106+
ctx.Data["Keyword"] = keyword
107+
ctx.Data["Language"] = language
108+
ctx.Data["queryType"] = queryType
109+
ctx.Data["SearchResults"] = searchResults
110+
ctx.Data["SearchResultLanguages"] = searchResultLanguages
111+
ctx.Data["PageIsViewCode"] = true
112+
113+
pager := context.NewPagination(total, setting.UI.RepoSearchPagingNum, page, 5)
114+
pager.SetDefaultParams(ctx)
115+
pager.AddParam(ctx, "l", "Language")
116+
ctx.Data["Page"] = pager
134117
}
135118

136-
ctx.Data["Keyword"] = keyword
137-
ctx.Data["Language"] = language
138-
ctx.Data["queryType"] = queryType
139-
ctx.Data["SearchResults"] = searchResults
140-
ctx.Data["SearchResultLanguages"] = searchResultLanguages
141-
ctx.Data["PageIsViewCode"] = true
142-
143-
pager := context.NewPagination(total, setting.UI.RepoSearchPagingNum, page, 5)
144-
pager.SetDefaultParams(ctx)
145-
pager.AddParam(ctx, "l", "Language")
146-
ctx.Data["Page"] = pager
147-
148119
ctx.HTML(http.StatusOK, tplExploreCode)
149120
}

0 commit comments

Comments
 (0)