* Check if project has the same repository id with issue when assign project to issue * Check if issue's repository id match project's repository id * Add more permission checking * Remove invalid argument * Fix errors * Add generic check * Remove duplicated check * Return error + add check for new issues * Apply suggestions from code review Co-authored-by: KN4CK3R <admin@oldschoolhack.me> Co-authored-by: Gusted <williamzijl7@hotmail.com> Co-authored-by: KN4CK3R <admin@oldschoolhack.me> Co-authored-by: 6543 <6543@obermui.de>tags/v1.18.0-rc0
@@ -124,6 +124,17 @@ func ChangeProjectAssign(issue *Issue, doer *user_model.User, newProjectID int64 | |||
func addUpdateIssueProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID int64) error { | |||
oldProjectID := issue.projectID(ctx) | |||
// Only check if we add a new project and not remove it. | |||
if newProjectID > 0 { | |||
newProject, err := project_model.GetProjectByID(ctx, newProjectID) | |||
if err != nil { | |||
return err | |||
} | |||
if newProject.RepoID != issue.RepoID { | |||
return fmt.Errorf("issue's repository is not the same as project's repository") | |||
} | |||
} | |||
if _, err := db.GetEngine(ctx).Where("project_issue.issue_id=?", issue.ID).Delete(&project_model.ProjectIssue{}); err != nil { | |||
return err | |||
} |
@@ -124,6 +124,11 @@ func NewMilestone(m *Milestone) (err error) { | |||
return committer.Commit() | |||
} | |||
// HasMilestoneByRepoID returns if the milestone exists in the repository. | |||
func HasMilestoneByRepoID(ctx context.Context, repoID, id int64) (bool, error) { | |||
return db.GetEngine(ctx).ID(id).Where("repo_id=?", repoID).Exist(new(Milestone)) | |||
} | |||
// GetMilestoneByRepoID returns the milestone in a repository. | |||
func GetMilestoneByRepoID(ctx context.Context, repoID, id int64) (*Milestone, error) { | |||
m := new(Milestone) |
@@ -886,7 +886,7 @@ func dismissReview(ctx *context.APIContext, msg string, isDismiss bool) { | |||
return | |||
} | |||
_, err := pull_service.DismissReview(ctx, review.ID, msg, ctx.Doer, isDismiss) | |||
_, err := pull_service.DismissReview(ctx, review.ID, ctx.Repo.Repository.ID, msg, ctx.Doer, isDismiss) | |||
if err != nil { | |||
ctx.Error(http.StatusInternalServerError, "pull_service.DismissReview", err) | |||
return |
@@ -803,7 +803,8 @@ func NewIssue(ctx *context.Context) { | |||
body := ctx.FormString("body") | |||
ctx.Data["BodyQuery"] = body | |||
ctx.Data["IsProjectsEnabled"] = ctx.Repo.CanRead(unit.TypeProjects) | |||
isProjectsEnabled := ctx.Repo.CanRead(unit.TypeProjects) | |||
ctx.Data["IsProjectsEnabled"] = isProjectsEnabled | |||
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled | |||
upload.AddUploadContext(ctx, "comment") | |||
@@ -819,7 +820,7 @@ func NewIssue(ctx *context.Context) { | |||
} | |||
projectID := ctx.FormInt64("project") | |||
if projectID > 0 { | |||
if projectID > 0 && isProjectsEnabled { | |||
project, err := project_model.GetProjectByID(ctx, projectID) | |||
if err != nil { | |||
log.Error("GetProjectByID: %d: %v", projectID, err) | |||
@@ -1043,6 +1044,11 @@ func NewIssuePost(ctx *context.Context) { | |||
} | |||
if projectID > 0 { | |||
if !ctx.Repo.CanRead(unit.TypeProjects) { | |||
// User must also be able to see the project. | |||
ctx.Error(http.StatusBadRequest, "user hasn't permissions to read projects") | |||
return | |||
} | |||
if err := issues_model.ChangeProjectAssign(issue, ctx.Doer, projectID); err != nil { | |||
ctx.ServerError("ChangeProjectAssign", err) | |||
return | |||
@@ -1783,6 +1789,10 @@ func getActionIssues(ctx *context.Context) []*issues_model.Issue { | |||
issueUnitEnabled := ctx.Repo.CanRead(unit.TypeIssues) | |||
prUnitEnabled := ctx.Repo.CanRead(unit.TypePullRequests) | |||
for _, issue := range issues { | |||
if issue.RepoID != ctx.Repo.Repository.ID { | |||
ctx.NotFound("some issue's RepoID is incorrect", errors.New("some issue's RepoID is incorrect")) | |||
return nil | |||
} | |||
if issue.IsPull && !prUnitEnabled || !issue.IsPull && !issueUnitEnabled { | |||
ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil) | |||
return nil |
@@ -5,6 +5,7 @@ | |||
package repo | |||
import ( | |||
"errors" | |||
"fmt" | |||
"net/http" | |||
"net/url" | |||
@@ -633,10 +634,17 @@ func MoveIssues(ctx *context.Context) { | |||
} | |||
if len(movedIssues) != len(form.Issues) { | |||
ctx.ServerError("IssuesNotFound", err) | |||
ctx.ServerError("some issues do not exist", errors.New("some issues do not exist")) | |||
return | |||
} | |||
for _, issue := range movedIssues { | |||
if issue.RepoID != project.RepoID { | |||
ctx.ServerError("Some issue's repoID is not equal to project's repoID", errors.New("Some issue's repoID is not equal to project's repoID")) | |||
return | |||
} | |||
} | |||
if err = project_model.MoveIssuesOnProjectBoard(board, sortedIssueIDs); err != nil { | |||
ctx.ServerError("MoveIssuesOnProjectBoard", err) | |||
return |
@@ -5,6 +5,7 @@ | |||
package repo | |||
import ( | |||
"errors" | |||
"fmt" | |||
"net/http" | |||
@@ -118,6 +119,11 @@ func UpdateResolveConversation(ctx *context.Context) { | |||
return | |||
} | |||
if comment.Issue.RepoID != ctx.Repo.Repository.ID { | |||
ctx.NotFound("comment's repoID is incorrect", errors.New("comment's repoID is incorrect")) | |||
return | |||
} | |||
var permResult bool | |||
if permResult, err = issues_model.CanMarkConversation(comment.Issue, ctx.Doer); err != nil { | |||
ctx.ServerError("CanMarkConversation", err) | |||
@@ -236,7 +242,7 @@ func SubmitReview(ctx *context.Context) { | |||
// DismissReview dismissing stale review by repo admin | |||
func DismissReview(ctx *context.Context) { | |||
form := web.GetForm(ctx).(*forms.DismissReviewForm) | |||
comm, err := pull_service.DismissReview(ctx, form.ReviewID, form.Message, ctx.Doer, true) | |||
comm, err := pull_service.DismissReview(ctx, form.ReviewID, ctx.Repo.Repository.ID, form.Message, ctx.Doer, true) | |||
if err != nil { | |||
ctx.ServerError("pull_service.DismissReview", err) | |||
return |
@@ -901,7 +901,7 @@ func RegisterRoutes(m *web.Route) { | |||
m.Post("/labels", reqRepoIssuesOrPullsWriter, repo.UpdateIssueLabel) | |||
m.Post("/milestone", reqRepoIssuesOrPullsWriter, repo.UpdateIssueMilestone) | |||
m.Post("/projects", reqRepoIssuesOrPullsWriter, repo.UpdateIssueProject) | |||
m.Post("/projects", reqRepoIssuesOrPullsWriter, reqRepoProjectsReader, repo.UpdateIssueProject) | |||
m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee) | |||
m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest) | |||
m.Post("/dismiss_review", reqRepoAdmin, bindIgnErr(forms.DismissReviewForm{}), repo.DismissReview) |
@@ -15,6 +15,17 @@ import ( | |||
) | |||
func changeMilestoneAssign(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldMilestoneID int64) error { | |||
// Only check if milestone exists if we don't remove it. | |||
if issue.MilestoneID > 0 { | |||
has, err := issues_model.HasMilestoneByRepoID(ctx, issue.RepoID, issue.MilestoneID) | |||
if err != nil { | |||
return fmt.Errorf("HasMilestoneByRepoID: %v", err) | |||
} | |||
if !has { | |||
return fmt.Errorf("HasMilestoneByRepoID: issue doesn't exist") | |||
} | |||
} | |||
if err := issues_model.UpdateIssueCols(ctx, issue, "milestone_id"); err != nil { | |||
return err | |||
} |
@@ -271,7 +271,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos | |||
} | |||
// DismissReview dismissing stale review by repo admin | |||
func DismissReview(ctx context.Context, reviewID int64, message string, doer *user_model.User, isDismiss bool) (comment *issues_model.Comment, err error) { | |||
func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss bool) (comment *issues_model.Comment, err error) { | |||
review, err := issues_model.GetReviewByID(ctx, reviewID) | |||
if err != nil { | |||
return | |||
@@ -281,6 +281,16 @@ func DismissReview(ctx context.Context, reviewID int64, message string, doer *us | |||
return nil, fmt.Errorf("not need to dismiss this review because it's type is not Approve or change request") | |||
} | |||
// load data for notify | |||
if err = review.LoadAttributes(ctx); err != nil { | |||
return nil, err | |||
} | |||
// Check if the review's repoID is the one we're currently expecting. | |||
if review.Issue.RepoID != repoID { | |||
return nil, fmt.Errorf("reviews's repository is not the same as the one we expect") | |||
} | |||
if err = issues_model.DismissReview(review, isDismiss); err != nil { | |||
return | |||
} | |||
@@ -289,10 +299,6 @@ func DismissReview(ctx context.Context, reviewID int64, message string, doer *us | |||
return nil, nil | |||
} | |||
// load data for notify | |||
if err = review.LoadAttributes(ctx); err != nil { | |||
return | |||
} | |||
if err = review.Issue.LoadPullRequest(); err != nil { | |||
return | |||
} |