Move permission check to router layer

Author: lunny

routers/api/v1/repo/collaborators.go

@@ -17,6 +17,7 @@ import (
 	"code.gitea.io/gitea/routers/api/v1/utils"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
+	issue_service "code.gitea.io/gitea/services/issue"
 	pull_service "code.gitea.io/gitea/services/pull"
 	repo_service "code.gitea.io/gitea/services/repository"
 )
@@ -321,6 +322,12 @@ func GetReviewers(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
+	canChooseReviewer := issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, ctx.Repo.Repository, 0)
+	if !canChooseReviewer {
+		ctx.Error(http.StatusForbidden, "GetReviewers", errors.New("doer has no permission to get reviewers"))
+		return
+	}
+
 	reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "ListCollaborators", err)

routers/web/repo/issue_page_meta.go

@@ -186,7 +186,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
 		if d.Issue == nil {
 			data.CanChooseReviewer = true
 		} else {
-			data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue)
+			data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue.PosterID)
 		}
 	}
 

services/issue/assignee.go

@@ -119,7 +119,7 @@ func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
 		return err
 	}
 
-	canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue)
+	canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID)
 
 	if isAdd {
 		if !permReviewer.CanAccessAny(perm.AccessModeRead, unit.TypePullRequests) {
@@ -178,7 +178,7 @@ func isValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team,
 		}
 	}
 
-	canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue)
+	canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID)
 
 	if isAdd {
 		if issue.Repo.IsPrivate {
@@ -276,12 +276,12 @@ func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doe
 }
 
 // CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR
-func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue) bool {
+func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, posterID int64) bool {
 	if repo.IsArchived {
 		return false
 	}
 	// The poster of the PR can change the reviewers
-	if doer.ID == issue.PosterID {
+	if doer.ID == posterID {
 		return true
 	}
 

services/pull/reviewer.go

@@ -5,7 +5,6 @@ package pull
 
 import (
 	"context"
-	"fmt"
 
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
@@ -53,14 +52,6 @@ func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post
 			return nil, err
 		}
 		uniqueUserIDs.AddMultiple(additionalUserIDs...)
-
-		if repo.Owner.Visibility.IsLimited() && doerID == 0 {
-			return nil, fmt.Errorf("permission denied")
-		}
-
-		if (repo.IsPrivate || repo.Owner.Visibility.IsPrivate()) && !uniqueUserIDs.Contains(doerID) {
-			return nil, fmt.Errorf("permission denied")
-		}
 	} else {
 		userIDs := make([]int64, 0, 10)
 		if err := e.Table("access").
@@ -70,9 +61,6 @@ func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post
 			return nil, err
 		}
 		uniqueUserIDs.AddMultiple(userIDs...)
-		if repo.IsPrivate && !uniqueUserIDs.Contains(doerID) && doerID != repo.OwnerID {
-			return nil, fmt.Errorf("permission denied")
-		}
 	}
 
 	uniqueUserIDs.Remove(posterID) // posterID should not be in the list of reviewers