Fix access issues on milestone and issue overview pages. (#9603)

* Fix access issues on milestone and issue overview pages.

* Fix filter algorithm
This commit is contained in:
David Svantesson 2020-01-05 02:23:29 +01:00 committed by techknowlogick
parent 8b24073713
commit 03d59bcd1d
3 changed files with 65 additions and 54 deletions

View file

@ -369,3 +369,23 @@ func hasAccess(e Engine, userID int64, repo *Repository) (bool, error) {
func HasAccess(userID int64, repo *Repository) (bool, error) { func HasAccess(userID int64, repo *Repository) (bool, error) {
return hasAccess(x, userID, repo) return hasAccess(x, userID, repo)
} }
// FilterOutRepoIdsWithoutUnitAccess filter out repos where user has no access to repositories
func FilterOutRepoIdsWithoutUnitAccess(u *User, repoIDs []int64, units ...UnitType) ([]int64, error) {
i := 0
for _, rID := range repoIDs {
repo, err := GetRepositoryByID(rID)
if err != nil {
return nil, err
}
perm, err := GetUserRepoPermission(repo, u)
if err != nil {
return nil, err
}
if perm.CanReadAny(units...) {
repoIDs[i] = rID
i++
}
}
return repoIDs[:i], nil
}

View file

@ -638,19 +638,20 @@ func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) {
func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) { func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) {
var ids []int64 var ids []int64
sess := x.Table("repository"). if err := x.Table("repository").
Cols("repository.id"). Cols("repository.id").
Join("INNER", "team_user", "repository.owner_id = team_user.org_id"). Join("INNER", "team_user", "repository.owner_id = team_user.org_id").
Join("INNER", "team_repo", "repository.is_private != ? OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true) Join("INNER", "team_repo", "repository.is_private != ? OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true).
Where("team_user.uid = ?", u.ID).
if len(units) > 0 { GroupBy("repository.id").Find(&ids); err != nil {
sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id") return nil, err
sess = sess.In("team_unit.type", units)
} }
return ids, sess. if len(units) > 0 {
Where("team_user.uid = ?", u.ID). return FilterOutRepoIdsWithoutUnitAccess(u, ids, units...)
GroupBy("repository.id").Find(&ids) }
return ids, nil
} }
// GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations // GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations

View file

@ -188,9 +188,13 @@ func Milestones(ctx *context.Context) {
ctx.ServerError("env.RepoIDs", err) ctx.ServerError("env.RepoIDs", err)
return return
} }
userRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctx.User, userRepoIDs, models.UnitTypeIssues, models.UnitTypePullRequests)
if err != nil {
ctx.ServerError("FilterOutRepoIdsWithoutUnitAccess", err)
return
}
} else { } else {
unitType := models.UnitTypeIssues userRepoIDs, err = ctxUser.GetAccessRepoIDs(models.UnitTypeIssues, models.UnitTypePullRequests)
userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
if err != nil { if err != nil {
ctx.ServerError("ctxUser.GetAccessRepoIDs", err) ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
return return
@ -201,6 +205,7 @@ func Milestones(ctx *context.Context) {
} }
var repoIDs []int64 var repoIDs []int64
if len(reposQuery) != 0 {
if issueReposQueryPattern.MatchString(reposQuery) { if issueReposQueryPattern.MatchString(reposQuery) {
// remove "[" and "]" from string // remove "[" and "]" from string
reposQuery = reposQuery[1 : len(reposQuery)-1] reposQuery = reposQuery[1 : len(reposQuery)-1]
@ -211,6 +216,7 @@ func Milestones(ctx *context.Context) {
if rID != "" && rID != "0" { if rID != "" && rID != "0" {
reposSet = true reposSet = true
rIDint64, err := strconv.ParseInt(rID, 10, 64) rIDint64, err := strconv.ParseInt(rID, 10, 64)
// If the repo id specified by query is not parseable or not accessible by user, just ignore it.
if err == nil && com.IsSliceContainsInt64(userRepoIDs, rIDint64) { if err == nil && com.IsSliceContainsInt64(userRepoIDs, rIDint64) {
repoIDs = append(repoIDs, rIDint64) repoIDs = append(repoIDs, rIDint64)
} }
@ -221,7 +227,8 @@ func Milestones(ctx *context.Context) {
repoIDs = []int64{-1} repoIDs = []int64{-1}
} }
} else { } else {
log.Error("issueReposQueryPattern not match with query") log.Warn("issueReposQueryPattern not match with query")
}
} }
if len(repoIDs) == 0 { if len(repoIDs) == 0 {
@ -256,26 +263,6 @@ func Milestones(ctx *context.Context) {
} }
} }
showReposMap[rID] = repo showReposMap[rID] = repo
// Check if user has access to given repository.
perm, err := models.GetUserRepoPermission(repo, ctxUser)
if err != nil {
ctx.ServerError("GetUserRepoPermission", fmt.Errorf("[%d]%v", rID, err))
return
}
if !perm.CanRead(models.UnitTypeIssues) {
if log.IsTrace() {
log.Trace("Permission Denied: User %-v cannot read %-v of repo %-v\n"+
"User in repo has Permissions: %-+v",
ctxUser,
models.UnitTypeIssues,
repo,
perm)
}
ctx.Status(404)
return
}
} }
showRepos := models.RepositoryListOfMap(showReposMap) showRepos := models.RepositoryListOfMap(showReposMap)
@ -345,9 +332,11 @@ var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`)
// Issues render the user issues page // Issues render the user issues page
func Issues(ctx *context.Context) { func Issues(ctx *context.Context) {
isPullList := ctx.Params(":type") == "pulls" isPullList := ctx.Params(":type") == "pulls"
unitType := models.UnitTypeIssues
if isPullList { if isPullList {
ctx.Data["Title"] = ctx.Tr("pull_requests") ctx.Data["Title"] = ctx.Tr("pull_requests")
ctx.Data["PageIsPulls"] = true ctx.Data["PageIsPulls"] = true
unitType = models.UnitTypePullRequests
} else { } else {
ctx.Data["Title"] = ctx.Tr("issues") ctx.Data["Title"] = ctx.Tr("issues")
ctx.Data["PageIsIssues"] = true ctx.Data["PageIsIssues"] = true
@ -404,7 +393,7 @@ func Issues(ctx *context.Context) {
} }
} }
} else { } else {
log.Error("issueReposQueryPattern not match with query") log.Warn("issueReposQueryPattern not match with query")
} }
} }
@ -424,11 +413,12 @@ func Issues(ctx *context.Context) {
ctx.ServerError("env.RepoIDs", err) ctx.ServerError("env.RepoIDs", err)
return return
} }
} else { userRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctx.User, userRepoIDs, unitType)
unitType := models.UnitTypeIssues if err != nil {
if isPullList { ctx.ServerError("FilterOutRepoIdsWithoutUnitAccess", err)
unitType = models.UnitTypePullRequests return
} }
} else {
userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType) userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
if err != nil { if err != nil {
ctx.ServerError("ctxUser.GetAccessRepoIDs", err) ctx.ServerError("ctxUser.GetAccessRepoIDs", err)