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

* Fix access issues on milestone and issue overview pages.

* Fix filter algorithm

1 files changed, 35 insertions, 45 deletions

diff --git a/routers/user/home.go b/routers/user/home.go
index f5e74b2406..512c60716d 100644
--- a/routers/user/home.go
+++ b/routers/user/home.go
@@ -188,9 +188,13 @@ func Milestones(ctx *context.Context) {
 			ctx.ServerError("env.RepoIDs", err)
 			return
 		}
+		userRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctx.User, userRepoIDs, models.UnitTypeIssues, models.UnitTypePullRequests)
+		if err != nil {
+			ctx.ServerError("FilterOutRepoIdsWithoutUnitAccess", err)
+			return
+		}
 	} else {
-		unitType := models.UnitTypeIssues
-		userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
+		userRepoIDs, err = ctxUser.GetAccessRepoIDs(models.UnitTypeIssues, models.UnitTypePullRequests)
 		if err != nil {
 			ctx.ServerError("ctxUser.GetAccessRepoIDs", err)
 			return
@@ -201,27 +205,30 @@ func Milestones(ctx *context.Context) {
 	}
 
 	var repoIDs []int64
-	if issueReposQueryPattern.MatchString(reposQuery) {
-		// remove "[" and "]" from string
-		reposQuery = reposQuery[1 : len(reposQuery)-1]
-		//for each ID (delimiter ",") add to int to repoIDs
-		reposSet := false
-		for _, rID := range strings.Split(reposQuery, ",") {
-			// Ensure nonempty string entries
-			if rID != "" && rID != "0" {
-				reposSet = true
-				rIDint64, err := strconv.ParseInt(rID, 10, 64)
-				if err == nil && com.IsSliceContainsInt64(userRepoIDs, rIDint64) {
-					repoIDs = append(repoIDs, rIDint64)
+	if len(reposQuery) != 0 {
+		if issueReposQueryPattern.MatchString(reposQuery) {
+			// remove "[" and "]" from string
+			reposQuery = reposQuery[1 : len(reposQuery)-1]
+			//for each ID (delimiter ",") add to int to repoIDs
+			reposSet := false
+			for _, rID := range strings.Split(reposQuery, ",") {
+				// Ensure nonempty string entries
+				if rID != "" && rID != "0" {
+					reposSet = true
+					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) {
+						repoIDs = append(repoIDs, rIDint64)
+					}
 				}
 			}
+			if reposSet && len(repoIDs) == 0 {
+				// force an empty result
+				repoIDs = []int64{-1}
+			}
+		} else {
+			log.Warn("issueReposQueryPattern not match with query")
 		}
-		if reposSet && len(repoIDs) == 0 {
-			// force an empty result
-			repoIDs = []int64{-1}
-		}
-	} else {
-		log.Error("issueReposQueryPattern not match with query")
 	}
 
 	if len(repoIDs) == 0 {
@@ -256,26 +263,6 @@ func Milestones(ctx *context.Context) {
 			}
 		}
 		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)
@@ -345,9 +332,11 @@ var issueReposQueryPattern = regexp.MustCompile(`^\[\d+(,\d+)*,?\]$`)
 // Issues render the user issues page
 func Issues(ctx *context.Context) {
 	isPullList := ctx.Params(":type") == "pulls"
+	unitType := models.UnitTypeIssues
 	if isPullList {
 		ctx.Data["Title"] = ctx.Tr("pull_requests")
 		ctx.Data["PageIsPulls"] = true
+		unitType = models.UnitTypePullRequests
 	} else {
 		ctx.Data["Title"] = ctx.Tr("issues")
 		ctx.Data["PageIsIssues"] = true
@@ -404,7 +393,7 @@ func Issues(ctx *context.Context) {
 				}
 			}
 		} 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)
 			return
 		}
-	} else {
-		unitType := models.UnitTypeIssues
-		if isPullList {
-			unitType = models.UnitTypePullRequests
+		userRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctx.User, userRepoIDs, unitType)
+		if err != nil {
+			ctx.ServerError("FilterOutRepoIdsWithoutUnitAccess", err)
+			return
 		}
+	} else {
 		userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType)
 		if err != nil {
 			ctx.ServerError("ctxUser.GetAccessRepoIDs", err)