From f9595c302e2db1425ad6398de832f098563c67a2 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Sun, 7 Feb 2021 18:55:30 +0100 Subject: [PATCH 01/43] [API] Add issue and comment attachments --- modules/convert/issue.go | 47 +- modules/convert/issue_comment.go | 34 +- modules/structs/issue.go | 25 +- modules/structs/issue_comment.go | 17 +- routers/api/v1/api.go | 16 + routers/api/v1/repo/issue_attachment.go | 307 ++++++++++++ routers/api/v1/repo/issue_comment.go | 9 + .../api/v1/repo/issue_comment_attachment.go | 303 +++++++++++ templates/swagger/v1_json.tmpl | 472 +++++++++++++++++- 9 files changed, 1187 insertions(+), 43 deletions(-) create mode 100644 routers/api/v1/repo/issue_attachment.go create mode 100644 routers/api/v1/repo/issue_comment_attachment.go diff --git a/modules/convert/issue.go b/modules/convert/issue.go index b773e78a6b5c..e83ee417c01a 100644 --- a/modules/convert/issue.go +++ b/modules/convert/issue.go @@ -26,21 +26,27 @@ func ToAPIIssue(issue *models.Issue) *api.Issue { return &api.Issue{} } + assets := make([]*api.Attachment, 0) + for _, att := range issue.Attachments { + assets = append(assets, ToIssueAttachment(att)) + } + apiIssue := &api.Issue{ - ID: issue.ID, - URL: issue.APIURL(), - HTMLURL: issue.HTMLURL(), - Index: issue.Index, - Poster: ToUser(issue.Poster, false, false), - Title: issue.Title, - Body: issue.Content, - Ref: issue.Ref, - Labels: ToLabelList(issue.Labels), - State: issue.State(), - IsLocked: issue.IsLocked, - Comments: issue.NumComments, - Created: issue.CreatedUnix.AsTime(), - Updated: issue.UpdatedUnix.AsTime(), + ID: issue.ID, + URL: issue.APIURL(), + HTMLURL: issue.HTMLURL(), + Index: issue.Index, + Poster: ToUser(issue.Poster, false, false), + Title: issue.Title, + Body: issue.Content, + Attachments: assets, + Ref: issue.Ref, + Labels: ToLabelList(issue.Labels), + State: issue.State(), + IsLocked: issue.IsLocked, + Comments: issue.NumComments, + Created: issue.CreatedUnix.AsTime(), + Updated: issue.UpdatedUnix.AsTime(), } apiIssue.Repo = &api.RepositoryMeta{ @@ -206,3 +212,16 @@ func ToAPIMilestone(m *models.Milestone) *api.Milestone { } return apiMilestone } + +// ToIssueAttachment converts models.Attachment to api.Attachment +func ToIssueAttachment(a *models.Attachment) *api.Attachment { + return &api.Attachment{ + ID: a.ID, + Name: a.Name, + Created: a.CreatedUnix.AsTime(), + DownloadCount: a.DownloadCount, + Size: a.Size, + UUID: a.UUID, + DownloadURL: a.DownloadURL(), + } +} diff --git a/modules/convert/issue_comment.go b/modules/convert/issue_comment.go index cf65c876d3d0..8b6b50cb3d0f 100644 --- a/modules/convert/issue_comment.go +++ b/modules/convert/issue_comment.go @@ -11,14 +11,32 @@ import ( // ToComment converts a models.Comment to the api.Comment format func ToComment(c *models.Comment) *api.Comment { + assets := make([]*api.Attachment, 0) + for _, att := range c.Attachments { + assets = append(assets, ToCommentAttachment(att)) + } return &api.Comment{ - ID: c.ID, - Poster: ToUser(c.Poster, false, false), - HTMLURL: c.HTMLURL(), - IssueURL: c.IssueURL(), - PRURL: c.PRURL(), - Body: c.Content, - Created: c.CreatedUnix.AsTime(), - Updated: c.UpdatedUnix.AsTime(), + ID: c.ID, + Poster: ToUser(c.Poster, false, false), + HTMLURL: c.HTMLURL(), + IssueURL: c.IssueURL(), + PRURL: c.PRURL(), + Body: c.Content, + Attachments: assets, + Created: c.CreatedUnix.AsTime(), + Updated: c.UpdatedUnix.AsTime(), + } +} + +// ToCommentAttachment converts models.Attachment to api.Attachment +func ToCommentAttachment(a *models.Attachment) *api.Attachment { + return &api.Attachment{ + ID: a.ID, + Name: a.Name, + Created: a.CreatedUnix.AsTime(), + DownloadCount: a.DownloadCount, + Size: a.Size, + UUID: a.UUID, + DownloadURL: a.DownloadURL(), } } diff --git a/modules/structs/issue.go b/modules/structs/issue.go index d990af63b45a..879db345d63e 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -38,18 +38,19 @@ type RepositoryMeta struct { // Issue represents an issue in a repository // swagger:model type Issue struct { - ID int64 `json:"id"` - URL string `json:"url"` - HTMLURL string `json:"html_url"` - Index int64 `json:"number"` - Poster *User `json:"user"` - OriginalAuthor string `json:"original_author"` - OriginalAuthorID int64 `json:"original_author_id"` - Title string `json:"title"` - Body string `json:"body"` - Ref string `json:"ref"` - Labels []*Label `json:"labels"` - Milestone *Milestone `json:"milestone"` + ID int64 `json:"id"` + URL string `json:"url"` + HTMLURL string `json:"html_url"` + Index int64 `json:"number"` + Poster *User `json:"user"` + OriginalAuthor string `json:"original_author"` + OriginalAuthorID int64 `json:"original_author_id"` + Title string `json:"title"` + Body string `json:"body"` + Ref string `json:"ref"` + Attachments []*Attachment `json:"assets"` + Labels []*Label `json:"labels"` + Milestone *Milestone `json:"milestone"` // deprecated Assignee *User `json:"assignee"` Assignees []*User `json:"assignees"` diff --git a/modules/structs/issue_comment.go b/modules/structs/issue_comment.go index 0c8ac20017fb..88588cc0c815 100644 --- a/modules/structs/issue_comment.go +++ b/modules/structs/issue_comment.go @@ -10,14 +10,15 @@ import ( // Comment represents a comment on a commit or issue type Comment struct { - ID int64 `json:"id"` - HTMLURL string `json:"html_url"` - PRURL string `json:"pull_request_url"` - IssueURL string `json:"issue_url"` - Poster *User `json:"user"` - OriginalAuthor string `json:"original_author"` - OriginalAuthorID int64 `json:"original_author_id"` - Body string `json:"body"` + ID int64 `json:"id"` + HTMLURL string `json:"html_url"` + PRURL string `json:"pull_request_url"` + IssueURL string `json:"issue_url"` + Poster *User `json:"user"` + OriginalAuthor string `json:"original_author"` + OriginalAuthorID int64 `json:"original_author_id"` + Body string `json:"body"` + Attachments []*Attachment `json:"assets"` // swagger:strfmt date-time Created time.Time `json:"created_at"` // swagger:strfmt date-time diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 42b52db93657..b4478396d39b 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -779,6 +779,13 @@ func Routes() *web.Route { Get(repo.GetIssueCommentReactions). Post(reqToken(), bind(api.EditReactionOption{}), repo.PostIssueCommentReaction). Delete(reqToken(), bind(api.EditReactionOption{}), repo.DeleteIssueCommentReaction) + m.Combo("/assets").Get(repo.ListIssueCommentAttachments). + Post(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.CreateIssueCommentAttachment) + }) + m.Group("/assets", func() { + m.Combo("/{asset}").Get(repo.GetIssueCommentAttachment). + Patch(reqToken(), reqRepoWriter(models.UnitTypeIssues), bind(api.EditAttachmentOptions{}), repo.EditIssueCommentAttachment). + Delete(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.DeleteIssueCommentAttachment) }) }) m.Group("/{index}", func() { @@ -820,6 +827,15 @@ func Routes() *web.Route { Get(repo.GetIssueReactions). Post(reqToken(), bind(api.EditReactionOption{}), repo.PostIssueReaction). Delete(reqToken(), bind(api.EditReactionOption{}), repo.DeleteIssueReaction) + m.Group("/assets", func() { + m.Combo("").Get(repo.ListIssueAttachments). + Post(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.CreateIssueAttachment) + }) + }) + m.Group("/assets", func() { + m.Combo("/{asset}").Get(repo.GetIssueAttachment). + Patch(reqToken(), reqRepoWriter(models.UnitTypeIssues), bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). + Delete(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.DeleteIssueAttachment) }) }, mustEnableIssuesOrPulls) m.Group("/labels", func() { diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go new file mode 100644 index 000000000000..0cff6efa1624 --- /dev/null +++ b/routers/api/v1/repo/issue_attachment.go @@ -0,0 +1,307 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "net/http" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/convert" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/upload" + "code.gitea.io/gitea/modules/web" +) + +// GetIssueAttachment gets a single attachment of the issue +func GetIssueAttachment(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueGetIssueAttachment + // --- + // summary: Get a issue attachment + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: attachment_id + // in: path + // description: id of the attachment to get + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/Attachment" + + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + return + } + + if attach.IssueID == 0 { + log.Info("User requested attachment is not in issue, attachment_id: %v", attachID) + ctx.NotFound() + return + } + + ctx.JSON(http.StatusOK, convert.ToIssueAttachment(attach)) +} + +// ListIssueAttachments lists all attachments of the issue +func ListIssueAttachments(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/{index}/assets issue issueListIssueAttachments + // --- + // summary: List issue's attachments + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: index + // in: path + // description: id of the issue + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/AttachmentList" + + issueID := ctx.ParamsInt64(":index") + issue, err := models.GetIssueByID(issueID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) + return + } + if issue.RepoID != ctx.Repo.Repository.ID { + ctx.NotFound() + return + } + if err := issue.LoadAttributes(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) + return + } + ctx.JSON(http.StatusOK, convert.ToAPIIssue(issue).Attachments) +} + +// CreateIssueAttachment creates an attachment and saves the given file +func CreateIssueAttachment(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/issues/{index}/assets issue issueCreateIssueAttachment + // --- + // summary: Create a issue attachment + // produces: + // - application/json + // consumes: + // - multipart/form-data + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: index + // in: path + // description: id of the issue + // type: integer + // format: int64 + // required: true + // - name: name + // in: query + // description: name of the attachment + // type: string + // required: false + // - name: attachment + // in: formData + // description: attachment to upload + // type: file + // required: true + // responses: + // "201": + // "$ref": "#/responses/Attachment" + // "400": + // "$ref": "#/responses/error" + + // Check if attachments are enabled + if !setting.Attachment.Enabled { + ctx.NotFound("Attachment is not enabled") + return + } + + // Check if issue exists an load issue + issueID := ctx.ParamsInt64(":index") + issue, err := models.GetIssueByID(issueID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) + return + } + + // Get uploaded file from request + file, header, err := ctx.Req.FormFile("attachment") + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetFile", err) + return + } + defer file.Close() + + buf := make([]byte, 1024) + n, _ := file.Read(buf) + if n > 0 { + buf = buf[:n] + } + + // Check if the filetype is allowed by the settings + err = upload.Verify(buf, header.Filename, setting.Attachment.AllowedTypes) + if err != nil { + ctx.Error(http.StatusBadRequest, "DetectContentType", err) + return + } + + var filename = header.Filename + if query := ctx.Query("name"); query != "" { + filename = query + } + + // Create a new attachment and save the file + attach, err := models.NewAttachment(&models.Attachment{ + UploaderID: ctx.User.ID, + Name: filename, + IssueID: issue.ID, + }, buf, file) + if err != nil { + ctx.Error(http.StatusInternalServerError, "NewAttachment", err) + return + } + + ctx.JSON(http.StatusCreated, convert.ToIssueAttachment(attach)) +} + +// EditIssueAttachment updates the given attachment +func EditIssueAttachment(ctx *context.APIContext) { + // swagger:operation PATCH /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueEditIssueAttachment + // --- + // summary: Edit a issue attachment + // produces: + // - application/json + // consumes: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: attachment_id + // in: path + // description: id of the attachment to edit + // type: integer + // format: int64 + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/EditAttachmentOptions" + // responses: + // "201": + // "$ref": "#/responses/Attachment" + + form := web.GetForm(ctx).(*api.EditAttachmentOptions) + + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + return + } + if attach.IssueID == 0 { + log.Info("User requested attachment is not in issue, attachment_id: %v", attachID) + ctx.NotFound() + return + } + if form.Name != "" { + attach.Name = form.Name + } + + if err := models.UpdateAttachment(attach); err != nil { + ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) + } + ctx.JSON(http.StatusCreated, convert.ToIssueAttachment(attach)) +} + +// DeleteIssueAttachment delete a given attachment +func DeleteIssueAttachment(ctx *context.APIContext) { + // swagger:operation DELETE /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueDeleteIssueAttachment + // --- + // summary: Delete a issue attachment + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: attachment_id + // in: path + // description: id of the attachment to delete + // type: integer + // format: int64 + // required: true + // responses: + // "204": + // "$ref": "#/responses/empty" + + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + return + } + if attach.IssueID == 0 { + log.Info("User requested attachment is not in issue, attachment_id: %v", attachID) + ctx.NotFound() + return + } + + if err := models.DeleteAttachment(attach, true); err != nil { + ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err) + return + } + ctx.Status(http.StatusNoContent) +} diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index d62ca813149a..a6d961543e8a 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -84,6 +84,11 @@ func ListIssueComments(ctx *context.APIContext) { return } + if err := models.CommentList(comments).LoadAttachments(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttachments", err) + return + } + apiComments := make([]*api.Comment, len(comments)) for i, comment := range comments { comment.Issue = issue @@ -164,6 +169,10 @@ func ListRepoIssueComments(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "LoadPosters", err) return } + if err := models.CommentList(comments).LoadAttachments(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttachments", err) + return + } if _, err := models.CommentList(comments).Issues().LoadRepositories(); err != nil { ctx.Error(http.StatusInternalServerError, "LoadRepositories", err) return diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go new file mode 100644 index 000000000000..565c155f13dd --- /dev/null +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -0,0 +1,303 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "net/http" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/convert" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/upload" + "code.gitea.io/gitea/modules/web" +) + +// GetIssueCommentAttachment gets a single attachment of the comment +func GetIssueCommentAttachment(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/comments/assets/{attachment_id} issue issueGetIssueCommentAttachment + // --- + // summary: Get a comment attachment + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: attachment_id + // in: path + // description: id of the attachment to get + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/Attachment" + + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + return + } + if attach.CommentID == 0 { + log.Info("User requested attachment is not in comment, attachment_id: %v", attachID) + ctx.NotFound() + return + } + + ctx.JSON(http.StatusOK, convert.ToCommentAttachment(attach)) +} + +// ListIssueCommentAttachments lists all attachments of the comment +func ListIssueCommentAttachments(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/comments/{id}/assets issue issueListIssueCommentAttachments + // --- + // summary: List comment's attachments + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: id of the comment + // type: integer + // format: int64 + // required: true + // responses: + // "200": + // "$ref": "#/responses/AttachmentList" + + commentID := ctx.ParamsInt64(":id") + comment, err := models.GetCommentByID(commentID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetCommentByID", err) + return + } + + if err := comment.LoadAttachments(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttachments", err) + return + } + + ctx.JSON(http.StatusOK, convert.ToComment(comment).Attachments) +} + +// CreateIssueCommentAttachment creates an attachment and saves the given file +func CreateIssueCommentAttachment(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/issues/comments/{id}/assets issue issueCreateIssueCommentAttachment + // --- + // summary: Create a comment attachment + // produces: + // - application/json + // consumes: + // - multipart/form-data + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: id + // in: path + // description: id of the comment + // type: integer + // format: int64 + // required: true + // - name: name + // in: query + // description: name of the attachment + // type: string + // required: false + // - name: attachment + // in: formData + // description: attachment to upload + // type: file + // required: true + // responses: + // "201": + // "$ref": "#/responses/Attachment" + // "400": + // "$ref": "#/responses/error" + + // Check if attachments are enabled + if !setting.Attachment.Enabled { + ctx.NotFound("Attachment is not enabled") + return + } + // Check if comment exists and load comment + commentID := ctx.ParamsInt64(":id") + comment, err := models.GetCommentByID(commentID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetCommentByID", err) + return + } + + // Get uploaded file from request + file, header, err := ctx.Req.FormFile("attachment") + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetFile", err) + return + } + defer file.Close() + + buf := make([]byte, 1024) + n, _ := file.Read(buf) + if n > 0 { + buf = buf[:n] + } + + // Check if the filetype is allowed by the settings + err = upload.Verify(buf, header.Filename, setting.Attachment.AllowedTypes) + if err != nil { + ctx.Error(http.StatusBadRequest, "DetectContentType", err) + return + } + + var filename = header.Filename + if query := ctx.Query("name"); query != "" { + filename = query + } + + // Create a new attachment and save the file + attach, err := models.NewAttachment(&models.Attachment{ + UploaderID: ctx.User.ID, + Name: filename, + CommentID: comment.ID, + }, buf, file) + if err != nil { + ctx.Error(http.StatusInternalServerError, "NewAttachment", err) + return + } + + ctx.JSON(http.StatusCreated, convert.ToCommentAttachment(attach)) +} + +// EditIssueCommentAttachment updates the given attachment +func EditIssueCommentAttachment(ctx *context.APIContext) { + // swagger:operation PATCH /repos/{owner}/{repo}/issues/comments/assets/{attachment_id} issue issueEditIssueCommentAttachment + // --- + // summary: Edit a comment attachment + // produces: + // - application/json + // consumes: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: attachment_id + // in: path + // description: id of the attachment to edit + // type: integer + // format: int64 + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/EditAttachmentOptions" + // responses: + // "201": + // "$ref": "#/responses/Attachment" + + form := web.GetForm(ctx).(*api.EditAttachmentOptions) + + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + return + } + if attach.CommentID == 0 { + log.Info("User requested attachment is not in comment, attachment_id: %v", attachID) + ctx.NotFound() + return + } + if form.Name != "" { + attach.Name = form.Name + } + + if err := models.UpdateAttachment(attach); err != nil { + ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) + } + ctx.JSON(http.StatusCreated, convert.ToCommentAttachment(attach)) +} + +// DeleteIssueCommentAttachment delete a given attachment +func DeleteIssueCommentAttachment(ctx *context.APIContext) { + // swagger:operation DELETE /repos/{owner}/{repo}/issues/comments/assets/{attachment_id} issue issueDeleteIssueCommentAttachment + // --- + // summary: Delete a comment attachment + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: attachment_id + // in: path + // description: id of the attachment to delete + // type: integer + // format: int64 + // required: true + // responses: + // "204": + // "$ref": "#/responses/empty" + + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + return + } + if attach.CommentID == 0 { + log.Info("User requested attachment is not in comment, attachment_id: %v", attachID) + ctx.NotFound() + return + } + + if err := models.DeleteAttachment(attach, true); err != nil { + ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err) + return + } + ctx.Status(http.StatusNoContent) +} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 5a3be37b4aec..c9a393016db8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4288,6 +4288,135 @@ } } }, + "/repos/{owner}/{repo}/issues/assets/{attachment_id}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Get a issue attachment", + "operationId": "issueGetIssueAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to get", + "name": "attachment_id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/Attachment" + } + } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Delete a issue attachment", + "operationId": "issueDeleteIssueAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to delete", + "name": "attachment_id", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + } + } + }, + "patch": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Edit a issue attachment", + "operationId": "issueEditIssueAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to edit", + "name": "attachment_id", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/EditAttachmentOptions" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/Attachment" + } + } + } + }, "/repos/{owner}/{repo}/issues/comments": { "get": { "produces": [ @@ -4347,6 +4476,135 @@ } } }, + "/repos/{owner}/{repo}/issues/comments/assets/{attachment_id}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Get a comment attachment", + "operationId": "issueGetIssueCommentAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to get", + "name": "attachment_id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/Attachment" + } + } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Delete a comment attachment", + "operationId": "issueDeleteIssueCommentAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to delete", + "name": "attachment_id", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + } + } + }, + "patch": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Edit a comment attachment", + "operationId": "issueEditIssueCommentAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to edit", + "name": "attachment_id", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/EditAttachmentOptions" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/Attachment" + } + } + } + }, "/repos/{owner}/{repo}/issues/comments/{id}": { "get": { "consumes": [ @@ -4500,6 +4758,105 @@ } } }, + "/repos/{owner}/{repo}/issues/comments/{id}/assets": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "List comment's attachments", + "operationId": "issueListIssueCommentAttachments", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment", + "name": "id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/AttachmentList" + } + } + }, + "post": { + "consumes": [ + "multipart/form-data" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Create a comment attachment", + "operationId": "issueCreateIssueCommentAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment", + "name": "id", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the attachment", + "name": "name", + "in": "query" + }, + { + "type": "file", + "description": "attachment to upload", + "name": "attachment", + "in": "formData", + "required": true + } + ], + "responses": { + "201": { + "$ref": "#/responses/Attachment" + }, + "400": { + "$ref": "#/responses/error" + } + } + } + }, "/repos/{owner}/{repo}/issues/comments/{id}/reactions": { "get": { "consumes": [ @@ -4756,6 +5113,105 @@ } } }, + "/repos/{owner}/{repo}/issues/{index}/assets": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "List issue's attachments", + "operationId": "issueListIssueAttachments", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the issue", + "name": "index", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/AttachmentList" + } + } + }, + "post": { + "consumes": [ + "multipart/form-data" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Create a issue attachment", + "operationId": "issueCreateIssueAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the issue", + "name": "index", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the attachment", + "name": "name", + "in": "query" + }, + { + "type": "file", + "description": "attachment to upload", + "name": "attachment", + "in": "formData", + "required": true + } + ], + "responses": { + "201": { + "$ref": "#/responses/Attachment" + }, + "400": { + "$ref": "#/responses/error" + } + } + } + }, "/repos/{owner}/{repo}/issues/{index}/comments": { "get": { "produces": [ @@ -11757,6 +12213,13 @@ "description": "Comment represents a comment on a commit or issue", "type": "object", "properties": { + "assets": { + "type": "array", + "items": { + "$ref": "#/definitions/Attachment" + }, + "x-go-name": "Attachments" + }, "body": { "type": "string", "x-go-name": "Body" @@ -14174,6 +14637,13 @@ "description": "Issue represents an issue in a repository", "type": "object", "properties": { + "assets": { + "type": "array", + "items": { + "$ref": "#/definitions/Attachment" + }, + "x-go-name": "Attachments" + }, "assignee": { "$ref": "#/definitions/User" }, @@ -16889,4 +17359,4 @@ "TOTPHeader": [] } ] -} +} \ No newline at end of file From 04abe85bda14dc92d1256c1553bfa82a9e70a6cb Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Mon, 8 Feb 2021 23:59:26 +0100 Subject: [PATCH 02/43] Specify capacity for assets slice --- modules/convert/issue.go | 2 +- modules/convert/issue_comment.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/convert/issue.go b/modules/convert/issue.go index e83ee417c01a..0f9cc5b4eed3 100644 --- a/modules/convert/issue.go +++ b/modules/convert/issue.go @@ -26,7 +26,7 @@ func ToAPIIssue(issue *models.Issue) *api.Issue { return &api.Issue{} } - assets := make([]*api.Attachment, 0) + assets := make([]*api.Attachment, 0, len(issue.Attachments)) for _, att := range issue.Attachments { assets = append(assets, ToIssueAttachment(att)) } diff --git a/modules/convert/issue_comment.go b/modules/convert/issue_comment.go index 8b6b50cb3d0f..503770de0386 100644 --- a/modules/convert/issue_comment.go +++ b/modules/convert/issue_comment.go @@ -11,7 +11,7 @@ import ( // ToComment converts a models.Comment to the api.Comment format func ToComment(c *models.Comment) *api.Comment { - assets := make([]*api.Attachment, 0) + assets := make([]*api.Attachment, 0, len(c.Attachments)) for _, att := range c.Attachments { assets = append(assets, ToCommentAttachment(att)) } From bef0c929a7dba4e1f0f9fc7b9bce708e7ab14e0e Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Tue, 2 Mar 2021 22:54:14 +0100 Subject: [PATCH 03/43] Make comment attachments accessible --- models/issue_comment.go | 23 +++++++++++-------- .../api/v1/repo/issue_comment_attachment.go | 1 + 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index d8f4f0537ac9..31fac09b229b 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -753,18 +753,21 @@ func updateCommentInfos(e *xorm.Session, opts *CreateCommentOptions, comment *Co } // Check attachments - attachments, err := getAttachmentsByUUIDs(e, opts.Attachments) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %v", opts.Attachments, err) - } + if len(opts.Attachments) > 0 { + attachments, err := getAttachmentsByUUIDs(e, opts.Attachments) + if err != nil { + return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %v", opts.Attachments, err) + } - for i := range attachments { - attachments[i].IssueID = opts.Issue.ID - attachments[i].CommentID = comment.ID - // No assign value could be 0, so ignore AllCols(). - if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { - return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err) + for i := range attachments { + attachments[i].IssueID = opts.Issue.ID + attachments[i].CommentID = comment.ID + // No assign value could be 0, so ignore AllCols(). + if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { + return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err) + } } + comment.Attachments = attachments } case CommentTypeReopen, CommentTypeClose: if err = opts.Issue.updateClosedNum(e); err != nil { diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 565c155f13dd..2ccc1095dd7e 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -189,6 +189,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { UploaderID: ctx.User.ID, Name: filename, CommentID: comment.ID, + IssueID: comment.IssueID, }, buf, file) if err != nil { ctx.Error(http.StatusInternalServerError, "NewAttachment", err) From 10d7f9099fea6a07c65164ecaf07918aad4fd9e0 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Wed, 3 Mar 2021 00:45:12 +0100 Subject: [PATCH 04/43] Update attachments in correct order --- routers/repo/issue.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 3bc20839aa44..f73adafe42fd 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1673,17 +1673,17 @@ func UpdateIssueContent(ctx *context.Context) { return } + files := ctx.QueryStrings("files[]") + if err := updateAttachments(issue, files); err != nil { + ctx.ServerError("UpdateAttachments", err) + } + content := ctx.Query("content") if err := issue_service.ChangeContent(issue, ctx.User, content); err != nil { ctx.ServerError("ChangeContent", err) return } - files := ctx.QueryStrings("files[]") - if err := updateAttachments(issue, files); err != nil { - ctx.ServerError("UpdateAttachments", err) - } - ctx.JSON(200, map[string]interface{}{ "content": string(markdown.Render([]byte(issue.Content), ctx.Query("context"), ctx.Repo.Repository.ComposeMetas())), "attachments": attachmentsHTML(ctx, issue.Attachments, issue.Content), @@ -2091,16 +2091,17 @@ func UpdateCommentContent(ctx *context.Context) { }) return } - if err = comment_service.UpdateComment(comment, ctx.User, oldContent); err != nil { - ctx.ServerError("UpdateComment", err) - return - } files := ctx.QueryStrings("files[]") if err := updateAttachments(comment, files); err != nil { ctx.ServerError("UpdateAttachments", err) } + if err = comment_service.UpdateComment(comment, ctx.User, oldContent); err != nil { + ctx.ServerError("UpdateComment", err) + return + } + ctx.JSON(200, map[string]interface{}{ "content": string(markdown.Render([]byte(comment.Content), ctx.Query("context"), ctx.Repo.Repository.ComposeMetas())), "attachments": attachmentsHTML(ctx, comment.Attachments, comment.Content), From 806143c47aaf1a6786dda29bb5dc6785fac81ee9 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Wed, 3 Mar 2021 02:52:23 +0100 Subject: [PATCH 05/43] Fix: Notifications for API attachments --- routers/api/v1/repo/issue_attachment.go | 9 ++++++++- routers/api/v1/repo/issue_comment_attachment.go | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 0cff6efa1624..6be0e3d7fbfc 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -5,6 +5,7 @@ package repo import ( + issue_service "code.gitea.io/gitea/services/issue" "net/http" "code.gitea.io/gitea/models" @@ -156,7 +157,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { // Check if issue exists an load issue issueID := ctx.ParamsInt64(":index") - issue, err := models.GetIssueByID(issueID) + issue, err := models.GetIssueWithAttrsByID(issueID) if err != nil { ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) return @@ -198,6 +199,12 @@ func CreateIssueAttachment(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "NewAttachment", err) return } + issue.Attachments = append(issue.Attachments, attach) + + if err := issue_service.ChangeContent(issue, ctx.User, issue.Content); err != nil { + ctx.ServerError("ChangeContent", err) + return + } ctx.JSON(http.StatusCreated, convert.ToIssueAttachment(attach)) } diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 2ccc1095dd7e..6f87f084ade1 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -5,6 +5,7 @@ package repo import ( + comment_service "code.gitea.io/gitea/services/comments" "net/http" "code.gitea.io/gitea/models" @@ -195,6 +196,15 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "NewAttachment", err) return } + if err := comment.LoadAttachments(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadAttachments", err) + return + } + + if err = comment_service.UpdateComment(comment, ctx.User, comment.Content); err != nil { + ctx.ServerError("UpdateComment", err) + return + } ctx.JSON(http.StatusCreated, convert.ToCommentAttachment(attach)) } From dc498729df3674c79dcbc37704aad57c1bc49218 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Sat, 13 Mar 2021 02:27:58 +0100 Subject: [PATCH 06/43] Fix: new line in v1_json.tmpl --- templates/swagger/v1_json.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index c9a393016db8..5d9731de1176 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -17359,4 +17359,4 @@ "TOTPHeader": [] } ] -} \ No newline at end of file +} From 7393e67a5180709e562514ff7810bcc922c7fe97 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Sat, 13 Mar 2021 02:44:19 +0100 Subject: [PATCH 07/43] Fix import order --- routers/api/v1/repo/issue_attachment.go | 2 +- routers/api/v1/repo/issue_comment_attachment.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 6be0e3d7fbfc..ca7b16c98e2c 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -5,7 +5,6 @@ package repo import ( - issue_service "code.gitea.io/gitea/services/issue" "net/http" "code.gitea.io/gitea/models" @@ -16,6 +15,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/upload" "code.gitea.io/gitea/modules/web" + issue_service "code.gitea.io/gitea/services/issue" ) // GetIssueAttachment gets a single attachment of the issue diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 6f87f084ade1..910965922086 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -5,7 +5,6 @@ package repo import ( - comment_service "code.gitea.io/gitea/services/comments" "net/http" "code.gitea.io/gitea/models" @@ -16,6 +15,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/upload" "code.gitea.io/gitea/modules/web" + comment_service "code.gitea.io/gitea/services/comments" ) // GetIssueCommentAttachment gets a single attachment of the comment From 488a07220cc30c869eec3ff42c69e8e6a433457f Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Sat, 13 Mar 2021 03:09:46 +0100 Subject: [PATCH 08/43] Fix missed return --- routers/repo/issue.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index f73adafe42fd..03e3c77f9c1f 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -1676,6 +1676,7 @@ func UpdateIssueContent(ctx *context.Context) { files := ctx.QueryStrings("files[]") if err := updateAttachments(issue, files); err != nil { ctx.ServerError("UpdateAttachments", err) + return } content := ctx.Query("content") From 75785e18200848d9cdf33ea918ed3718869f2edd Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 29 Jun 2021 21:54:23 +0100 Subject: [PATCH 09/43] partial correction of permissions checks Signed-off-by: Andrew Thornton --- routers/api/v1/api.go | 4 +- routers/api/v1/repo/issue_attachment.go | 74 ++++++++++++++++++------- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 58e933d84acd..6056c0a5f897 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -862,8 +862,8 @@ func Routes() *web.Route { }) m.Group("/assets", func() { m.Combo("/{asset}").Get(repo.GetIssueAttachment). - Patch(reqToken(), reqRepoWriter(models.UnitTypeIssues), bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). - Delete(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.DeleteIssueAttachment) + Patch(reqToken(), bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). + Delete(reqToken(), repo.DeleteIssueAttachment) }) }, mustEnableIssuesOrPulls) m.Group("/labels", func() { diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index ca7b16c98e2c..b5f7eaae1ade 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -52,12 +52,29 @@ func GetIssueAttachment(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) return } - if attach.IssueID == 0 { - log.Info("User requested attachment is not in issue, attachment_id: %v", attachID) + log.Debug("Requested attachment[%d] is not in an issue.", attachID) ctx.NotFound() return } + issue, err := models.GetIssueByID(attach.IssueID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) + return + } + if issue.RepoID != ctx.Repo.Repository.ID { + log.Debug("Requested attachment[%d] belongs to issue[%d, #%d] which is not in Repo: %-v.", attachID, issue.ID, issue.Index, ctx.Repo.Repository) + ctx.NotFound() + return + } + unitType := models.UnitTypeIssues + if issue.IsPull { + unitType = models.UnitTypePullRequests + } + if !ctx.IsUserRepoReaderSpecific(unitType) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { + ctx.Error(http.StatusForbidden, "reqRepoReader", "user should have a permission to read repo") + return + } ctx.JSON(http.StatusOK, convert.ToIssueAttachment(attach)) } @@ -209,6 +226,38 @@ func CreateIssueAttachment(ctx *context.APIContext) { ctx.JSON(http.StatusCreated, convert.ToIssueAttachment(attach)) } +func reqIssueAttachment(ctx *context.APIContext, attachID int64) *models.Attachment { + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + return nil + } + if attach.IssueID == 0 { + log.Debug("Requested attachment[%d] is not in an issue.", attachID) + ctx.NotFound() + return nil + } + issue, err := models.GetIssueByID(attach.IssueID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) + return nil + } + if issue.RepoID != ctx.Repo.Repository.ID { + log.Debug("Requested attachment[%d] belongs to issue[%d, #%d] which is not in Repo: %-v.", attachID, issue.ID, issue.Index, ctx.Repo.Repository) + ctx.NotFound() + return nil + } + unitType := models.UnitTypeIssues + if issue.IsPull { + unitType = models.UnitTypePullRequests + } + if !ctx.IsUserRepoWriter([]models.UnitType{unitType}) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { + ctx.Error(http.StatusForbidden, "reqRepoWriter", "user should have a permission to write to a repo") + return nil + } + return attach +} + // EditIssueAttachment updates the given attachment func EditIssueAttachment(ctx *context.APIContext) { // swagger:operation PATCH /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueEditIssueAttachment @@ -246,14 +295,8 @@ func EditIssueAttachment(ctx *context.APIContext) { form := web.GetForm(ctx).(*api.EditAttachmentOptions) attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) - return - } - if attach.IssueID == 0 { - log.Info("User requested attachment is not in issue, attachment_id: %v", attachID) - ctx.NotFound() + attach := reqIssueAttachment(ctx, attachID) + if attach == nil { return } if form.Name != "" { @@ -295,17 +338,10 @@ func DeleteIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/empty" attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + attach := reqIssueAttachment(ctx, attachID) + if attach == nil { return } - if attach.IssueID == 0 { - log.Info("User requested attachment is not in issue, attachment_id: %v", attachID) - ctx.NotFound() - return - } - if err := models.DeleteAttachment(attach, true); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err) return From 202ece1c0d320358663265094e2eeff0a5495f19 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Sat, 2 Oct 2021 16:57:40 +0200 Subject: [PATCH 10/43] Fix title of error Co-authored-by: Norwin --- routers/api/v1/repo/issue_attachment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index b5f7eaae1ade..0dd16aeef5a0 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -252,7 +252,7 @@ func reqIssueAttachment(ctx *context.APIContext, attachID int64) *models.Attachm unitType = models.UnitTypePullRequests } if !ctx.IsUserRepoWriter([]models.UnitType{unitType}) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { - ctx.Error(http.StatusForbidden, "reqRepoWriter", "user should have a permission to write to a repo") + ctx.Error(http.StatusForbidden, "reqIssueAttachment", "user should have a permission to write to a repo") return nil } return attach From 587605e0f4d6485a3ef1f6c104774834ea451785 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Sat, 2 Oct 2021 17:32:13 +0200 Subject: [PATCH 11/43] Merge functions ToIssueAttachment and ToCommentAttachment --- modules/convert/convert.go | 13 +++++++++++++ modules/convert/issue.go | 15 +-------------- modules/convert/issue_comment.go | 15 +-------------- routers/api/v1/repo/issue_attachment.go | 6 +++--- routers/api/v1/repo/issue_comment_attachment.go | 6 +++--- 5 files changed, 21 insertions(+), 34 deletions(-) diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 0b2135c5803d..ed93b13ae0e2 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -20,6 +20,19 @@ import ( "code.gitea.io/gitea/services/webhook" ) +// ToAttachment converts models.Attachment to api.Attachment +func ToAttachment(a *models.Attachment) *api.Attachment { + return &api.Attachment{ + ID: a.ID, + Name: a.Name, + Created: a.CreatedUnix.AsTime(), + DownloadCount: a.DownloadCount, + Size: a.Size, + UUID: a.UUID, + DownloadURL: a.DownloadURL(), + } +} + // ToEmail convert models.EmailAddress to api.Email func ToEmail(email *models.EmailAddress) *api.Email { return &api.Email{ diff --git a/modules/convert/issue.go b/modules/convert/issue.go index d9ad91b2828f..a6d750e4cab4 100644 --- a/modules/convert/issue.go +++ b/modules/convert/issue.go @@ -28,7 +28,7 @@ func ToAPIIssue(issue *models.Issue) *api.Issue { assets := make([]*api.Attachment, 0, len(issue.Attachments)) for _, att := range issue.Attachments { - assets = append(assets, ToIssueAttachment(att)) + assets = append(assets, ToAttachment(att)) } apiIssue := &api.Issue{ @@ -212,16 +212,3 @@ func ToAPIMilestone(m *models.Milestone) *api.Milestone { } return apiMilestone } - -// ToIssueAttachment converts models.Attachment to api.Attachment -func ToIssueAttachment(a *models.Attachment) *api.Attachment { - return &api.Attachment{ - ID: a.ID, - Name: a.Name, - Created: a.CreatedUnix.AsTime(), - DownloadCount: a.DownloadCount, - Size: a.Size, - UUID: a.UUID, - DownloadURL: a.DownloadURL(), - } -} diff --git a/modules/convert/issue_comment.go b/modules/convert/issue_comment.go index 62b910227529..cdd37d72e046 100644 --- a/modules/convert/issue_comment.go +++ b/modules/convert/issue_comment.go @@ -13,7 +13,7 @@ import ( func ToComment(c *models.Comment) *api.Comment { assets := make([]*api.Attachment, 0, len(c.Attachments)) for _, att := range c.Attachments { - assets = append(assets, ToCommentAttachment(att)) + assets = append(assets, ToAttachment(att)) } return &api.Comment{ ID: c.ID, @@ -27,16 +27,3 @@ func ToComment(c *models.Comment) *api.Comment { Updated: c.UpdatedUnix.AsTime(), } } - -// ToCommentAttachment converts models.Attachment to api.Attachment -func ToCommentAttachment(a *models.Attachment) *api.Attachment { - return &api.Attachment{ - ID: a.ID, - Name: a.Name, - Created: a.CreatedUnix.AsTime(), - DownloadCount: a.DownloadCount, - Size: a.Size, - UUID: a.UUID, - DownloadURL: a.DownloadURL(), - } -} diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 0dd16aeef5a0..2a10e1b16b04 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -76,7 +76,7 @@ func GetIssueAttachment(ctx *context.APIContext) { return } - ctx.JSON(http.StatusOK, convert.ToIssueAttachment(attach)) + ctx.JSON(http.StatusOK, convert.ToAttachment(attach)) } // ListIssueAttachments lists all attachments of the issue @@ -223,7 +223,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { return } - ctx.JSON(http.StatusCreated, convert.ToIssueAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } func reqIssueAttachment(ctx *context.APIContext, attachID int64) *models.Attachment { @@ -306,7 +306,7 @@ func EditIssueAttachment(ctx *context.APIContext) { if err := models.UpdateAttachment(attach); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) } - ctx.JSON(http.StatusCreated, convert.ToIssueAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } // DeleteIssueAttachment delete a given attachment diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 910965922086..4e078c98443b 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -58,7 +58,7 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { return } - ctx.JSON(http.StatusOK, convert.ToCommentAttachment(attach)) + ctx.JSON(http.StatusOK, convert.ToAttachment(attach)) } // ListIssueCommentAttachments lists all attachments of the comment @@ -206,7 +206,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { return } - ctx.JSON(http.StatusCreated, convert.ToCommentAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } // EditIssueCommentAttachment updates the given attachment @@ -263,7 +263,7 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { if err := models.UpdateAttachment(attach); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) } - ctx.JSON(http.StatusCreated, convert.ToCommentAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } // DeleteIssueCommentAttachment delete a given attachment From 036dd17949d99deb6c557318eac8e8e33735bd25 Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 1 Oct 2021 00:31:47 +0200 Subject: [PATCH 12/43] deduplicate convert.ToAttachment() --- modules/convert/attachment.go | 23 +++++++++++++++++++++++ modules/convert/release.go | 15 +-------------- routers/api/v1/repo/release_attachment.go | 6 +++--- routers/web/repo/issue.go | 4 ++-- 4 files changed, 29 insertions(+), 19 deletions(-) create mode 100644 modules/convert/attachment.go diff --git a/modules/convert/attachment.go b/modules/convert/attachment.go new file mode 100644 index 000000000000..f6e0e66e8c9f --- /dev/null +++ b/modules/convert/attachment.go @@ -0,0 +1,23 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package convert + +import ( + "code.gitea.io/gitea/models" + api "code.gitea.io/gitea/modules/structs" +) + +// ToAttachment converts models.Attachment to api.Attachment +func ToAttachment(a *models.Attachment) *api.Attachment { + return &api.Attachment{ + ID: a.ID, + Name: a.Name, + Created: a.CreatedUnix.AsTime(), + DownloadCount: a.DownloadCount, + Size: a.Size, + UUID: a.UUID, + DownloadURL: a.DownloadURL(), + } +} diff --git a/modules/convert/release.go b/modules/convert/release.go index 70f0d6e76405..3f997e62fbd3 100644 --- a/modules/convert/release.go +++ b/modules/convert/release.go @@ -13,7 +13,7 @@ import ( func ToRelease(r *models.Release) *api.Release { assets := make([]*api.Attachment, 0) for _, att := range r.Attachments { - assets = append(assets, ToReleaseAttachment(att)) + assets = append(assets, ToAttachment(att)) } return &api.Release{ ID: r.ID, @@ -33,16 +33,3 @@ func ToRelease(r *models.Release) *api.Release { Attachments: assets, } } - -// ToReleaseAttachment converts models.Attachment to api.Attachment -func ToReleaseAttachment(a *models.Attachment) *api.Attachment { - return &api.Attachment{ - ID: a.ID, - Name: a.Name, - Created: a.CreatedUnix.AsTime(), - DownloadCount: a.DownloadCount, - Size: a.Size, - UUID: a.UUID, - DownloadURL: a.DownloadURL(), - } -} diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index 0a6425cddc53..82fdd76aeb68 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -64,7 +64,7 @@ func GetReleaseAttachment(ctx *context.APIContext) { return } // FIXME Should prove the existence of the given repo, but results in unnecessary database requests - ctx.JSON(http.StatusOK, convert.ToReleaseAttachment(attach)) + ctx.JSON(http.StatusOK, convert.ToAttachment(attach)) } // ListReleaseAttachments lists all attachments of the release @@ -205,7 +205,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) { return } - ctx.JSON(http.StatusCreated, convert.ToReleaseAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } // EditReleaseAttachment updates the given attachment @@ -271,7 +271,7 @@ func EditReleaseAttachment(ctx *context.APIContext) { if err := models.UpdateAttachment(attach); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) } - ctx.JSON(http.StatusCreated, convert.ToReleaseAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } // DeleteReleaseAttachment delete a given attachment diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 24c6c628f50e..12bde4ed5499 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2443,7 +2443,7 @@ func GetIssueAttachments(ctx *context.Context) { issue := GetActionIssue(ctx) var attachments = make([]*api.Attachment, len(issue.Attachments)) for i := 0; i < len(issue.Attachments); i++ { - attachments[i] = convert.ToReleaseAttachment(issue.Attachments[i]) + attachments[i] = convert.ToAttachment(issue.Attachments[i]) } ctx.JSON(http.StatusOK, attachments) } @@ -2462,7 +2462,7 @@ func GetCommentAttachments(ctx *context.Context) { return } for i := 0; i < len(comment.Attachments); i++ { - attachments = append(attachments, convert.ToReleaseAttachment(comment.Attachments[i])) + attachments = append(attachments, convert.ToAttachment(comment.Attachments[i])) } } ctx.JSON(http.StatusOK, attachments) From 9c433e57b9c6eb609dc8babeccf495528cc7f2ac Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 1 Oct 2021 01:07:29 +0200 Subject: [PATCH 13/43] refer to issue by index, not by ID.. --- routers/api/v1/repo/issue_attachment.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 2a10e1b16b04..4770268e6f42 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -146,7 +146,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { // required: true // - name: index // in: path - // description: id of the issue + // description: index of the issue // type: integer // format: int64 // required: true @@ -172,9 +172,10 @@ func CreateIssueAttachment(ctx *context.APIContext) { return } + repoID := ctx.Repo.Repository.ID + // Check if issue exists an load issue - issueID := ctx.ParamsInt64(":index") - issue, err := models.GetIssueWithAttrsByID(issueID) + issue, err := models.GetIssueByIndex(repoID, ctx.ParamsInt64(":index")) if err != nil { ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) return From d883f69082bd1cd88fcae162f78266e3a52953b3 Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 1 Oct 2021 01:08:23 +0200 Subject: [PATCH 14/43] use attachment_service.UploadAttachment() DRY, but need to refactor to specify comment + issue ID --- routers/api/v1/repo/issue_attachment.go | 30 +++++++------------ .../api/v1/repo/issue_comment_attachment.go | 29 +++++------------- routers/api/v1/repo/release_attachment.go | 7 ++++- routers/web/repo/attachment.go | 6 +++- services/attachment/attachment.go | 11 ++----- 5 files changed, 33 insertions(+), 50 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 4770268e6f42..b9ee0cda3ce2 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -13,8 +13,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/upload" "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/services/attachment" issue_service "code.gitea.io/gitea/services/issue" ) @@ -189,30 +189,22 @@ func CreateIssueAttachment(ctx *context.APIContext) { } defer file.Close() - buf := make([]byte, 1024) - n, _ := file.Read(buf) - if n > 0 { - buf = buf[:n] - } - - // Check if the filetype is allowed by the settings - err = upload.Verify(buf, header.Filename, setting.Attachment.AllowedTypes) - if err != nil { - ctx.Error(http.StatusBadRequest, "DetectContentType", err) - return - } - var filename = header.Filename - if query := ctx.Query("name"); query != "" { + if query := ctx.FormString("name"); query != "" { filename = query } - // Create a new attachment and save the file - attach, err := models.NewAttachment(&models.Attachment{ - UploaderID: ctx.User.ID, + attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &models.Attachment{ Name: filename, + UploaderID: ctx.User.ID, + RepoID: ctx.Repo.Repository.ID, IssueID: issue.ID, - }, buf, file) + }) + if err != nil { + ctx.Error(http.StatusInternalServerError, "UploadAttachment", err) + return + } + if err != nil { ctx.Error(http.StatusInternalServerError, "NewAttachment", err) return diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 4e078c98443b..d5eab94d21ec 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -13,8 +13,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/upload" "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/services/attachment" comment_service "code.gitea.io/gitea/services/comments" ) @@ -167,33 +167,20 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { } defer file.Close() - buf := make([]byte, 1024) - n, _ := file.Read(buf) - if n > 0 { - buf = buf[:n] - } - - // Check if the filetype is allowed by the settings - err = upload.Verify(buf, header.Filename, setting.Attachment.AllowedTypes) - if err != nil { - ctx.Error(http.StatusBadRequest, "DetectContentType", err) - return - } - var filename = header.Filename - if query := ctx.Query("name"); query != "" { + if query := ctx.FormString("name"); query != "" { filename = query } - // Create a new attachment and save the file - attach, err := models.NewAttachment(&models.Attachment{ - UploaderID: ctx.User.ID, + attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &models.Attachment{ Name: filename, - CommentID: comment.ID, + UploaderID: ctx.User.ID, + RepoID: ctx.Repo.Repository.ID, IssueID: comment.IssueID, - }, buf, file) + CommentID: comment.ID, + }) if err != nil { - ctx.Error(http.StatusInternalServerError, "NewAttachment", err) + ctx.Error(http.StatusInternalServerError, "UploadAttachment", err) return } if err := comment.LoadAttachments(); err != nil { diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index b5b63291174b..0bd397596c37 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -183,7 +183,12 @@ func CreateReleaseAttachment(ctx *context.APIContext) { } // Create a new attachment and save the file - attach, err := attachment.UploadAttachment(file, ctx.User.ID, release.RepoID, releaseID, filename, setting.Repository.Release.AllowedTypes) + attach, err := attachment.UploadAttachment(file, setting.Repository.Release.AllowedTypes, &models.Attachment{ + Name: filename, + UploaderID: ctx.User.ID, + RepoID: release.RepoID, + ReleaseID: releaseID, + }) if err != nil { if upload.IsErrFileTypeForbidden(err) { ctx.Error(http.StatusBadRequest, "DetectContentType", err) diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index 3968d2765266..cd35d5d8ee48 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -43,7 +43,11 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { } defer file.Close() - attach, err := attachment.UploadAttachment(file, ctx.User.ID, repoID, 0, header.Filename, allowedTypes) + attach, err := attachment.UploadAttachment(file, allowedTypes, &models.Attachment{ + Name: header.Filename, + UploaderID: ctx.User.ID, + RepoID: repoID, + }) if err != nil { if upload.IsErrFileTypeForbidden(err) { ctx.Error(http.StatusBadRequest, err.Error()) diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 7500a8ac3a65..c31e35784f51 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -39,21 +39,16 @@ func NewAttachment(attach *models.Attachment, file io.Reader) (*models.Attachmen } // UploadAttachment upload new attachment into storage and update database -func UploadAttachment(file io.Reader, actorID, repoID, releaseID int64, fileName string, allowedTypes string) (*models.Attachment, error) { +func UploadAttachment(file io.Reader, allowedTypes string, opts *models.Attachment) (*models.Attachment, error) { buf := make([]byte, 1024) n, _ := file.Read(buf) if n > 0 { buf = buf[:n] } - if err := upload.Verify(buf, fileName, allowedTypes); err != nil { + if err := upload.Verify(buf, opts.Name, allowedTypes); err != nil { return nil, err } - return NewAttachment(&models.Attachment{ - RepoID: repoID, - UploaderID: actorID, - ReleaseID: releaseID, - Name: fileName, - }, io.MultiReader(bytes.NewReader(buf), file)) + return NewAttachment(opts, io.MultiReader(bytes.NewReader(buf), file)) } From e98bf3eba9dba1aeb13639492f94039ed377d727 Mon Sep 17 00:00:00 2001 From: Norwin Date: Sat, 2 Oct 2021 22:31:03 +0200 Subject: [PATCH 15/43] fix permissions of *IssueAttachment() --- routers/api/v1/api.go | 27 ++-- routers/api/v1/repo/issue_attachment.go | 171 ++++++++++++------------ 2 files changed, 106 insertions(+), 92 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index ad04604b4250..0fd3bb322d65 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -528,6 +528,13 @@ func mustNotBeArchived(ctx *context.APIContext) { } } +func mustEnableAttachments(ctx *context.APIContext) { + if !setting.Attachment.Enabled { + ctx.NotFound() + return + } +} + // bind binding an obj to a func(ctx *context.APIContext) func bind(obj interface{}) http.HandlerFunc { var tp = reflect.TypeOf(obj) @@ -806,13 +813,14 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route { Post(reqToken(), bind(api.EditReactionOption{}), repo.PostIssueCommentReaction). Delete(reqToken(), bind(api.EditReactionOption{}), repo.DeleteIssueCommentReaction) m.Combo("/assets").Get(repo.ListIssueCommentAttachments). - Post(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.CreateIssueCommentAttachment) + Post(reqToken(), mustNotBeArchived, repo.CreateIssueCommentAttachment) }) + // FIXME: why is this a separate group? m.Group("/assets", func() { m.Combo("/{asset}").Get(repo.GetIssueCommentAttachment). - Patch(reqToken(), reqRepoWriter(models.UnitTypeIssues), bind(api.EditAttachmentOptions{}), repo.EditIssueCommentAttachment). - Delete(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.DeleteIssueCommentAttachment) - }) + Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueCommentAttachment). + Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueCommentAttachment) + }, mustEnableAttachments) }) m.Group("/{index}", func() { m.Combo("").Get(repo.GetIssue). @@ -855,14 +863,15 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route { Delete(reqToken(), bind(api.EditReactionOption{}), repo.DeleteIssueReaction) m.Group("/assets", func() { m.Combo("").Get(repo.ListIssueAttachments). - Post(reqToken(), reqRepoWriter(models.UnitTypeIssues), repo.CreateIssueAttachment) - }) + Post(reqToken(), mustNotBeArchived, repo.CreateIssueAttachment) + }, mustEnableAttachments) }) + // FIXME: why are these on a different group? this is odd. m.Group("/assets", func() { m.Combo("/{asset}").Get(repo.GetIssueAttachment). - Patch(reqToken(), bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). - Delete(reqToken(), repo.DeleteIssueAttachment) - }) + Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). + Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueAttachment) + }, mustEnableAttachments) }, mustEnableIssuesOrPulls) m.Group("/labels", func() { m.Combo("").Get(repo.ListLabels). diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index b9ee0cda3ce2..2d0a2322ad69 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -18,6 +18,17 @@ import ( issue_service "code.gitea.io/gitea/services/issue" ) +/** + * NOTE about permissions: + * - repo access is already checked via middleware on the /repos/{owner}/{name} group + * - issue/pull *read* access is checked on the ../issues group middleware + * ("read" access allows posting issues, so posting attachments is fine too!) + * - setting.Attachment.Enabled is checked on ../assets group middleware + * All that is left to be checked is + * - canUserWriteIssueAttachment() + * - attachmentBelongsToIssue() + */ + // GetIssueAttachment gets a single attachment of the issue func GetIssueAttachment(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueGetIssueAttachment @@ -46,36 +57,10 @@ func GetIssueAttachment(ctx *context.APIContext) { // "200": // "$ref": "#/responses/Attachment" - attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) - return - } - if attach.IssueID == 0 { - log.Debug("Requested attachment[%d] is not in an issue.", attachID) - ctx.NotFound() - return - } - issue, err := models.GetIssueByID(attach.IssueID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) - return - } - if issue.RepoID != ctx.Repo.Repository.ID { - log.Debug("Requested attachment[%d] belongs to issue[%d, #%d] which is not in Repo: %-v.", attachID, issue.ID, issue.Index, ctx.Repo.Repository) - ctx.NotFound() - return - } - unitType := models.UnitTypeIssues - if issue.IsPull { - unitType = models.UnitTypePullRequests - } - if !ctx.IsUserRepoReaderSpecific(unitType) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { - ctx.Error(http.StatusForbidden, "reqRepoReader", "user should have a permission to read repo") + attach := getIssueAttachmentSafeRead(ctx) + if attach == nil { return } - ctx.JSON(http.StatusOK, convert.ToAttachment(attach)) } @@ -107,14 +92,9 @@ func ListIssueAttachments(ctx *context.APIContext) { // "200": // "$ref": "#/responses/AttachmentList" - issueID := ctx.ParamsInt64(":index") - issue, err := models.GetIssueByID(issueID) + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) - return - } - if issue.RepoID != ctx.Repo.Repository.ID { - ctx.NotFound() + ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) return } if err := issue.LoadAttributes(); err != nil { @@ -166,18 +146,14 @@ func CreateIssueAttachment(ctx *context.APIContext) { // "400": // "$ref": "#/responses/error" - // Check if attachments are enabled - if !setting.Attachment.Enabled { - ctx.NotFound("Attachment is not enabled") + // Check if issue exists and load issue + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err) return } - repoID := ctx.Repo.Repository.ID - - // Check if issue exists an load issue - issue, err := models.GetIssueByIndex(repoID, ctx.ParamsInt64(":index")) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) + if !canUserWriteIssueAttachment(ctx, issue) { return } @@ -219,38 +195,6 @@ func CreateIssueAttachment(ctx *context.APIContext) { ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } -func reqIssueAttachment(ctx *context.APIContext, attachID int64) *models.Attachment { - attach, err := models.GetAttachmentByID(attachID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) - return nil - } - if attach.IssueID == 0 { - log.Debug("Requested attachment[%d] is not in an issue.", attachID) - ctx.NotFound() - return nil - } - issue, err := models.GetIssueByID(attach.IssueID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetIssueByID", err) - return nil - } - if issue.RepoID != ctx.Repo.Repository.ID { - log.Debug("Requested attachment[%d] belongs to issue[%d, #%d] which is not in Repo: %-v.", attachID, issue.ID, issue.Index, ctx.Repo.Repository) - ctx.NotFound() - return nil - } - unitType := models.UnitTypeIssues - if issue.IsPull { - unitType = models.UnitTypePullRequests - } - if !ctx.IsUserRepoWriter([]models.UnitType{unitType}) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { - ctx.Error(http.StatusForbidden, "reqIssueAttachment", "user should have a permission to write to a repo") - return nil - } - return attach -} - // EditIssueAttachment updates the given attachment func EditIssueAttachment(ctx *context.APIContext) { // swagger:operation PATCH /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueEditIssueAttachment @@ -285,17 +229,16 @@ func EditIssueAttachment(ctx *context.APIContext) { // "201": // "$ref": "#/responses/Attachment" - form := web.GetForm(ctx).(*api.EditAttachmentOptions) - - attachID := ctx.ParamsInt64(":asset") - attach := reqIssueAttachment(ctx, attachID) + // get attachment and check permissions + attach := getIssueAttachmentSafeWrite(ctx) if attach == nil { return } + // do changes to attachment. only meaningful change is name. + form := web.GetForm(ctx).(*api.EditAttachmentOptions) if form.Name != "" { attach.Name = form.Name } - if err := models.UpdateAttachment(attach); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) } @@ -330,8 +273,7 @@ func DeleteIssueAttachment(ctx *context.APIContext) { // "204": // "$ref": "#/responses/empty" - attachID := ctx.ParamsInt64(":asset") - attach := reqIssueAttachment(ctx, attachID) + attach := getIssueAttachmentSafeWrite(ctx) if attach == nil { return } @@ -341,3 +283,66 @@ func DeleteIssueAttachment(ctx *context.APIContext) { } ctx.Status(http.StatusNoContent) } + +func getIssueAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { + attach := getIssueAttachmentSafeRead(ctx) + if attach == nil { + return nil + } + issue, err := models.GetIssueByID(attach.IssueID) + if err != nil { + ctx.NotFoundOrServerError("GetIssueByID", models.IsErrIssueNotExist, err) + return nil + } + if !canUserWriteIssueAttachment(ctx, issue) { + return nil + } + return attach +} + +func getIssueAttachmentSafeRead(ctx *context.APIContext) *models.Attachment { + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.NotFoundOrServerError("GetAttachmentByID", models.IsErrAttachmentNotExist, err) + return nil + } + if !attachmentBelongsToRepoOrIssue(ctx, attach, nil, nil) { + return nil + } + return attach +} + +func canUserWriteIssueAttachment(ctx *context.APIContext, i *models.Issue) (success bool) { + canEditIssue := ctx.User.ID == i.PosterID && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() + if !canEditIssue { + ctx.Error(http.StatusForbidden, "IssueEditPerm", "user should have a permission to editIssue") + return + } + + return true +} + +func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, a *models.Attachment, issue *models.Issue, comment *models.Comment) (success bool) { + if a.RepoID != ctx.Repo.Repository.ID { + log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) + ctx.NotFound() + return + } + if a.IssueID == 0 { + // catch people trying to get release assets ;) + log.Debug("Requested attachment[%d] is not in an issue.", a.ID) + ctx.NotFound() + return + } else if issue != nil && a.IssueID != issue.ID { + log.Debug("Requested attachment[%d] does not belong to issue[%d, #%d].", a.ID, issue.ID, issue.Index) + ctx.NotFound() + return + } + if comment != nil && a.CommentID != comment.ID { + log.Debug("Requested attachment[%d] does not belong to comment[%d].", a.ID, comment.ID) + ctx.NotFound() + return + } + return true +} From f2a05e77c2f155686293bf8e48ce483263b8f9d3 Mon Sep 17 00:00:00 2001 From: Norwin Date: Sat, 2 Oct 2021 22:31:19 +0200 Subject: [PATCH 16/43] WIP: fix permissions of *IssueCommentAttachment() --- .../api/v1/repo/issue_comment_attachment.go | 52 +++++++++++++++---- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index d5eab94d21ec..cd4e47199728 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -46,14 +46,14 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { // "200": // "$ref": "#/responses/Attachment" - attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) + attach := getIssueAttachmentSafeRead(ctx) + if attach == nil { return } + // FIXME: if we had the commentID here (which we should..), we could pass the comment in, + // and validate the exact comment.. if attach.CommentID == 0 { - log.Info("User requested attachment is not in comment, attachment_id: %v", attachID) + log.Debug("User requested attachment[%d] is not in comment.", attach.ID) ctx.NotFound() return } @@ -146,11 +146,6 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { // "400": // "$ref": "#/responses/error" - // Check if attachments are enabled - if !setting.Attachment.Enabled { - ctx.NotFound("Attachment is not enabled") - return - } // Check if comment exists and load comment commentID := ctx.ParamsInt64(":id") comment, err := models.GetCommentByID(commentID) @@ -299,3 +294,40 @@ func DeleteIssueCommentAttachment(ctx *context.APIContext) { } ctx.Status(http.StatusNoContent) } + +func getIssueCommentAttachmentSafeRead(ctx *context.APIContext) *models.Attachment { + attachID := ctx.ParamsInt64(":asset") + attach, err := models.GetAttachmentByID(attachID) + if err != nil { + ctx.NotFoundOrServerError("GetAttachmentByID", models.IsErrAttachmentNotExist, err) + return nil + } + if !attachmentBelongsToRepoOrIssue(ctx, attach, nil, nil) { + return nil + } + return attach +} + +func attachmentBelongsToRepoOrComment(ctx *context.APIContext, a *models.Attachment, issue *models.Issue, comment *models.Comment) (success bool) { + if a.RepoID != ctx.Repo.Repository.ID { + log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) + ctx.NotFound() + return + } + if a.IssueID == 0 { + // catch people trying to get release assets ;) + log.Debug("Requested attachment[%d] is not in an issue.", a.ID) + ctx.NotFound() + return + } else if issue != nil && a.IssueID != issue.ID { + log.Debug("Requested attachment[%d] does not belong to issue[%d, #%d].", a.ID, issue.ID, issue.Index) + ctx.NotFound() + return + } + if comment != nil && a.CommentID != comment.ID { + log.Debug("Requested attachment[%d] does not belong to comment[%d].", a.ID, comment.ID) + ctx.NotFound() + return + } + return true +} From ad9a947ef2687b301883e9098a32ccdb4abf1b7f Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 3 Oct 2021 13:55:44 +0200 Subject: [PATCH 17/43] rm dublicate func --- modules/convert/convert.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 2d7f496dd466..3c309a82f861 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -21,19 +21,6 @@ import ( "code.gitea.io/gitea/services/webhook" ) -// ToAttachment converts models.Attachment to api.Attachment -func ToAttachment(a *models.Attachment) *api.Attachment { - return &api.Attachment{ - ID: a.ID, - Name: a.Name, - Created: a.CreatedUnix.AsTime(), - DownloadCount: a.DownloadCount, - Size: a.Size, - UUID: a.UUID, - DownloadURL: a.DownloadURL(), - } -} - // ToEmail convert models.EmailAddress to api.Email func ToEmail(email *models.EmailAddress) *api.Email { return &api.Email{ From f99ee824030ad336732f8c6fce9566d49f44efe2 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 3 Oct 2021 14:03:33 +0200 Subject: [PATCH 18/43] update swagger --- templates/swagger/v1_json.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 247ce5678e11..0823473d6350 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -5399,7 +5399,7 @@ { "type": "integer", "format": "int64", - "description": "id of the issue", + "description": "index of the issue", "name": "index", "in": "path", "required": true From 5134ff6825801191cac3bf07583909658a8170ad Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 8 Oct 2021 19:11:47 +0200 Subject: [PATCH 19/43] fixup! fix permissions of *IssueAttachment() --- routers/api/v1/repo/issue_attachment.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 2d0a2322ad69..dae1aca610f0 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -94,7 +94,7 @@ func ListIssueAttachments(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) + ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err) return } if err := issue.LoadAttributes(); err != nil { @@ -181,10 +181,6 @@ func CreateIssueAttachment(ctx *context.APIContext) { return } - if err != nil { - ctx.Error(http.StatusInternalServerError, "NewAttachment", err) - return - } issue.Attachments = append(issue.Attachments, attach) if err := issue_service.ChangeContent(issue, ctx.User, issue.Content); err != nil { @@ -307,23 +303,23 @@ func getIssueAttachmentSafeRead(ctx *context.APIContext) *models.Attachment { ctx.NotFoundOrServerError("GetAttachmentByID", models.IsErrAttachmentNotExist, err) return nil } - if !attachmentBelongsToRepoOrIssue(ctx, attach, nil, nil) { + if !attachmentBelongsToRepoOrIssue(ctx, attach, issue) { return nil } return attach } func canUserWriteIssueAttachment(ctx *context.APIContext, i *models.Issue) (success bool) { - canEditIssue := ctx.User.ID == i.PosterID && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() + canEditIssue := ctx.User.ID == i.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() if !canEditIssue { - ctx.Error(http.StatusForbidden, "IssueEditPerm", "user should have a permission to editIssue") + ctx.Error(http.StatusForbidden, "IssueEditPerm", "user should have permission to edit issue") return } return true } -func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, a *models.Attachment, issue *models.Issue, comment *models.Comment) (success bool) { +func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, a *models.Attachment, issue *models.Issue) (success bool) { if a.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) ctx.NotFound() @@ -339,10 +335,5 @@ func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, a *models.Attachmen ctx.NotFound() return } - if comment != nil && a.CommentID != comment.ID { - log.Debug("Requested attachment[%d] does not belong to comment[%d].", a.ID, comment.ID) - ctx.NotFound() - return - } return true } From 7557df156a2ccdbd9813bb20990daea8b3304ae6 Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 8 Oct 2021 19:12:15 +0200 Subject: [PATCH 20/43] fix permissions of *IssueCommentAttachment() (cont) --- .../api/v1/repo/issue_comment_attachment.go | 87 ++++++++++++------- 1 file changed, 56 insertions(+), 31 deletions(-) diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index cd4e47199728..0bb72f79cd02 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -46,7 +46,7 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { // "200": // "$ref": "#/responses/Attachment" - attach := getIssueAttachmentSafeRead(ctx) + attach := getIssueCommentAttachmentSafeRead(ctx) if attach == nil { return } @@ -88,11 +88,8 @@ func ListIssueCommentAttachments(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/AttachmentList" - - commentID := ctx.ParamsInt64(":id") - comment, err := models.GetCommentByID(commentID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetCommentByID", err) + comment := getIssueCommentSafe(ctx) + if comment == nil { return } @@ -147,10 +144,12 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { // "$ref": "#/responses/error" // Check if comment exists and load comment - commentID := ctx.ParamsInt64(":id") - comment, err := models.GetCommentByID(commentID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetCommentByID", err) + comment := getIssueCommentSafe(ctx) + if comment == nil { + return + } + + if !canUserWriteIssueCommentAttachment(ctx, comment) { return } @@ -225,19 +224,12 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { // "201": // "$ref": "#/responses/Attachment" - form := web.GetForm(ctx).(*api.EditAttachmentOptions) - - attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) - return - } - if attach.CommentID == 0 { - log.Info("User requested attachment is not in comment, attachment_id: %v", attachID) - ctx.NotFound() + attach := getIssueCommentAttachmentSafeWrite(ctx) + if attach == nil { return } + + form := web.GetForm(ctx).(*api.EditAttachmentOptions) if form.Name != "" { attach.Name = form.Name } @@ -276,15 +268,8 @@ func DeleteIssueCommentAttachment(ctx *context.APIContext) { // "204": // "$ref": "#/responses/empty" - attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetAttachmentByID", err) - return - } - if attach.CommentID == 0 { - log.Info("User requested attachment is not in comment, attachment_id: %v", attachID) - ctx.NotFound() + attach := getIssueCommentAttachmentSafeWrite(ctx) + if attach == nil { return } @@ -295,6 +280,36 @@ func DeleteIssueCommentAttachment(ctx *context.APIContext) { ctx.Status(http.StatusNoContent) } +func getIssueCommentSafe(ctx *context.APIContext) *models.Comment { + comment, err := models.GetCommentByID(ctx.ParamsInt64(":id")) + if err != nil { + ctx.NotFoundOrServerError("GetCommentByID", models.IsErrCommentNotExist, err) + return nil + } + // deny accessing arbitrary comments via this API + // TODO: if issue ID were available, we could check that too. + if comment.Issue == nil || comment.Issue.RepoID != ctx.Repo.Repository.RepoID { + ctx.Error(http.StatusNotFound, "", err) + return nil + } + return comment +} + +func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { + attach := getIssueCommentAttachmentSafeRead(ctx) + if attach == nil { + return nil + } + comment := getIssueCommentSafe(ctx) + if comment == nil { + return nil + } + if !canUserWriteIssueCommentAttachment(ctx, comment) { + return nil + } + return attach +} + func getIssueCommentAttachmentSafeRead(ctx *context.APIContext) *models.Attachment { attachID := ctx.ParamsInt64(":asset") attach, err := models.GetAttachmentByID(attachID) @@ -302,12 +317,22 @@ func getIssueCommentAttachmentSafeRead(ctx *context.APIContext) *models.Attachme ctx.NotFoundOrServerError("GetAttachmentByID", models.IsErrAttachmentNotExist, err) return nil } - if !attachmentBelongsToRepoOrIssue(ctx, attach, nil, nil) { + if !attachmentBelongsToRepoOrComment(ctx, attach, nil, nil) { return nil } return attach } +func canUserWriteIssueCommentAttachment(ctx *context.APIContext, c *models.Comment) (success bool) { + canEditComment := ctx.User.ID == c.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() + if !canEditComment { + ctx.Error(http.StatusForbidden, "CommentEditPerm", "user should have permission to edit comment") + return + } + + return true +} + func attachmentBelongsToRepoOrComment(ctx *context.APIContext, a *models.Attachment, issue *models.Issue, comment *models.Comment) (success bool) { if a.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) From 170134a28b57fd1907626cba5caae0d6d9e2ee8a Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 8 Oct 2021 20:35:26 +0200 Subject: [PATCH 21/43] move all asset APIs into issue / comment namespace --- routers/api/v1/api.go | 30 +- routers/api/v1/repo/issue_attachment.go | 28 +- .../api/v1/repo/issue_comment_attachment.go | 52 +- templates/swagger/v1_json.tmpl | 464 +++++++++--------- 4 files changed, 292 insertions(+), 282 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 0fd3bb322d65..e3cd36c4b3b7 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -812,15 +812,16 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route { Get(repo.GetIssueCommentReactions). Post(reqToken(), bind(api.EditReactionOption{}), repo.PostIssueCommentReaction). Delete(reqToken(), bind(api.EditReactionOption{}), repo.DeleteIssueCommentReaction) - m.Combo("/assets").Get(repo.ListIssueCommentAttachments). - Post(reqToken(), mustNotBeArchived, repo.CreateIssueCommentAttachment) + m.Group("/assets", func() { + m.Combo(""). + Get(repo.ListIssueCommentAttachments). + Post(reqToken(), mustNotBeArchived, repo.CreateIssueCommentAttachment) + m.Combo("/{asset}"). + Get(repo.GetIssueCommentAttachment). + Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueCommentAttachment). + Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueCommentAttachment) + }, mustEnableAttachments) }) - // FIXME: why is this a separate group? - m.Group("/assets", func() { - m.Combo("/{asset}").Get(repo.GetIssueCommentAttachment). - Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueCommentAttachment). - Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueCommentAttachment) - }, mustEnableAttachments) }) m.Group("/{index}", func() { m.Combo("").Get(repo.GetIssue). @@ -862,16 +863,15 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route { Post(reqToken(), bind(api.EditReactionOption{}), repo.PostIssueReaction). Delete(reqToken(), bind(api.EditReactionOption{}), repo.DeleteIssueReaction) m.Group("/assets", func() { - m.Combo("").Get(repo.ListIssueAttachments). + m.Combo(""). + Get(repo.ListIssueAttachments). Post(reqToken(), mustNotBeArchived, repo.CreateIssueAttachment) + m.Combo("/{asset}"). + Get(repo.GetIssueAttachment). + Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). + Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueAttachment) }, mustEnableAttachments) }) - // FIXME: why are these on a different group? this is odd. - m.Group("/assets", func() { - m.Combo("/{asset}").Get(repo.GetIssueAttachment). - Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). - Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueAttachment) - }, mustEnableAttachments) }, mustEnableIssuesOrPulls) m.Group("/labels", func() { m.Combo("").Get(repo.ListLabels). diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index dae1aca610f0..0c295aa8de69 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -31,7 +31,7 @@ import ( // GetIssueAttachment gets a single attachment of the issue func GetIssueAttachment(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueGetIssueAttachment + // swagger:operation GET /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueGetIssueAttachment // --- // summary: Get a issue attachment // produces: @@ -56,8 +56,14 @@ func GetIssueAttachment(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/Attachment" + issueIndex := ctx.ParamsInt64(":index") + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.RepoID, issueIndex) + if err != nil { + ctx.NotFoundOrServerError("GetIssueByID", models.IsErrIssueNotExist, err) + return + } - attach := getIssueAttachmentSafeRead(ctx) + attach := getIssueAttachmentSafeRead(ctx, issue) if attach == nil { return } @@ -193,7 +199,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { // EditIssueAttachment updates the given attachment func EditIssueAttachment(ctx *context.APIContext) { - // swagger:operation PATCH /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueEditIssueAttachment + // swagger:operation PATCH /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueEditIssueAttachment // --- // summary: Edit a issue attachment // produces: @@ -243,7 +249,7 @@ func EditIssueAttachment(ctx *context.APIContext) { // DeleteIssueAttachment delete a given attachment func DeleteIssueAttachment(ctx *context.APIContext) { - // swagger:operation DELETE /repos/{owner}/{repo}/issues/assets/{attachment_id} issue issueDeleteIssueAttachment + // swagger:operation DELETE /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueDeleteIssueAttachment // --- // summary: Delete a issue attachment // produces: @@ -281,11 +287,8 @@ func DeleteIssueAttachment(ctx *context.APIContext) { } func getIssueAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { - attach := getIssueAttachmentSafeRead(ctx) - if attach == nil { - return nil - } - issue, err := models.GetIssueByID(attach.IssueID) + issueIndex := ctx.ParamsInt64(":index") + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) if err != nil { ctx.NotFoundOrServerError("GetIssueByID", models.IsErrIssueNotExist, err) return nil @@ -293,10 +296,15 @@ func getIssueAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { if !canUserWriteIssueAttachment(ctx, issue) { return nil } + + attach := getIssueAttachmentSafeRead(ctx, issue) + if attach == nil { + return nil + } return attach } -func getIssueAttachmentSafeRead(ctx *context.APIContext) *models.Attachment { +func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *models.Issue) *models.Attachment { attachID := ctx.ParamsInt64(":asset") attach, err := models.GetAttachmentByID(attachID) if err != nil { diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 0bb72f79cd02..4110131c006c 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -20,7 +20,7 @@ import ( // GetIssueCommentAttachment gets a single attachment of the comment func GetIssueCommentAttachment(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/issues/comments/assets/{attachment_id} issue issueGetIssueCommentAttachment + // swagger:operation GET /repos/{owner}/{repo}/issues/comments/{id}/assets/{attachment_id} issue issueGetIssueCommentAttachment // --- // summary: Get a comment attachment // produces: @@ -46,15 +46,17 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { // "200": // "$ref": "#/responses/Attachment" - attach := getIssueCommentAttachmentSafeRead(ctx) + comment := getIssueCommentSafe(ctx) + if comment == nil { + return + } + attach := getIssueCommentAttachmentSafeRead(ctx, comment) if attach == nil { return } - // FIXME: if we had the commentID here (which we should..), we could pass the comment in, - // and validate the exact comment.. - if attach.CommentID == 0 { - log.Debug("User requested attachment[%d] is not in comment.", attach.ID) - ctx.NotFound() + if attach.CommentID != comment.ID { + log.Debug("User requested attachment[%d] is not in comment[%d].", attach.ID, comment.ID) + ctx.NotFound("attachment not in comment") return } @@ -192,7 +194,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { // EditIssueCommentAttachment updates the given attachment func EditIssueCommentAttachment(ctx *context.APIContext) { - // swagger:operation PATCH /repos/{owner}/{repo}/issues/comments/assets/{attachment_id} issue issueEditIssueCommentAttachment + // swagger:operation PATCH /repos/{owner}/{repo}/issues/comments/{id}/assets/{attachment_id} issue issueEditIssueCommentAttachment // --- // summary: Edit a comment attachment // produces: @@ -242,7 +244,7 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { // DeleteIssueCommentAttachment delete a given attachment func DeleteIssueCommentAttachment(ctx *context.APIContext) { - // swagger:operation DELETE /repos/{owner}/{repo}/issues/comments/assets/{attachment_id} issue issueDeleteIssueCommentAttachment + // swagger:operation DELETE /repos/{owner}/{repo}/issues/comments/{id}/assets/{attachment_id} issue issueDeleteIssueCommentAttachment // --- // summary: Delete a comment attachment // produces: @@ -287,19 +289,19 @@ func getIssueCommentSafe(ctx *context.APIContext) *models.Comment { return nil } // deny accessing arbitrary comments via this API - // TODO: if issue ID were available, we could check that too. - if comment.Issue == nil || comment.Issue.RepoID != ctx.Repo.Repository.RepoID { - ctx.Error(http.StatusNotFound, "", err) + // TODO: if issue ID were available on context, we could check that too. + if err := comment.LoadIssue(); err != nil { + ctx.Error(http.StatusInternalServerError, "comment.LoadIssue", err) + return nil + } + if comment.Issue == nil || comment.Issue.RepoID != ctx.Repo.Repository.ID { + ctx.Error(http.StatusNotFound, "no matching issue comment found", err) return nil } return comment } func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { - attach := getIssueCommentAttachmentSafeRead(ctx) - if attach == nil { - return nil - } comment := getIssueCommentSafe(ctx) if comment == nil { return nil @@ -307,17 +309,21 @@ func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *models.Attachm if !canUserWriteIssueCommentAttachment(ctx, comment) { return nil } + attach := getIssueCommentAttachmentSafeRead(ctx, comment) + if attach == nil { + return nil + } return attach } -func getIssueCommentAttachmentSafeRead(ctx *context.APIContext) *models.Attachment { +func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *models.Comment) *models.Attachment { attachID := ctx.ParamsInt64(":asset") attach, err := models.GetAttachmentByID(attachID) if err != nil { ctx.NotFoundOrServerError("GetAttachmentByID", models.IsErrAttachmentNotExist, err) return nil } - if !attachmentBelongsToRepoOrComment(ctx, attach, nil, nil) { + if !attachmentBelongsToRepoOrComment(ctx, attach, comment) { return nil } return attach @@ -333,19 +339,15 @@ func canUserWriteIssueCommentAttachment(ctx *context.APIContext, c *models.Comme return true } -func attachmentBelongsToRepoOrComment(ctx *context.APIContext, a *models.Attachment, issue *models.Issue, comment *models.Comment) (success bool) { +func attachmentBelongsToRepoOrComment(ctx *context.APIContext, a *models.Attachment, comment *models.Comment) (success bool) { if a.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) ctx.NotFound() return } - if a.IssueID == 0 { + if a.IssueID == 0 || a.CommentID == 0 { // catch people trying to get release assets ;) - log.Debug("Requested attachment[%d] is not in an issue.", a.ID) - ctx.NotFound() - return - } else if issue != nil && a.IssueID != issue.ID { - log.Debug("Requested attachment[%d] does not belong to issue[%d, #%d].", a.ID, issue.ID, issue.Index) + log.Debug("Requested attachment[%d] is not in a comment.", a.ID) ctx.NotFound() return } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 0823473d6350..734963716dd8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4504,135 +4504,6 @@ } } }, - "/repos/{owner}/{repo}/issues/assets/{attachment_id}": { - "get": { - "produces": [ - "application/json" - ], - "tags": [ - "issue" - ], - "summary": "Get a issue attachment", - "operationId": "issueGetIssueAttachment", - "parameters": [ - { - "type": "string", - "description": "owner of the repo", - "name": "owner", - "in": "path", - "required": true - }, - { - "type": "string", - "description": "name of the repo", - "name": "repo", - "in": "path", - "required": true - }, - { - "type": "integer", - "format": "int64", - "description": "id of the attachment to get", - "name": "attachment_id", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "$ref": "#/responses/Attachment" - } - } - }, - "delete": { - "produces": [ - "application/json" - ], - "tags": [ - "issue" - ], - "summary": "Delete a issue attachment", - "operationId": "issueDeleteIssueAttachment", - "parameters": [ - { - "type": "string", - "description": "owner of the repo", - "name": "owner", - "in": "path", - "required": true - }, - { - "type": "string", - "description": "name of the repo", - "name": "repo", - "in": "path", - "required": true - }, - { - "type": "integer", - "format": "int64", - "description": "id of the attachment to delete", - "name": "attachment_id", - "in": "path", - "required": true - } - ], - "responses": { - "204": { - "$ref": "#/responses/empty" - } - } - }, - "patch": { - "consumes": [ - "application/json" - ], - "produces": [ - "application/json" - ], - "tags": [ - "issue" - ], - "summary": "Edit a issue attachment", - "operationId": "issueEditIssueAttachment", - "parameters": [ - { - "type": "string", - "description": "owner of the repo", - "name": "owner", - "in": "path", - "required": true - }, - { - "type": "string", - "description": "name of the repo", - "name": "repo", - "in": "path", - "required": true - }, - { - "type": "integer", - "format": "int64", - "description": "id of the attachment to edit", - "name": "attachment_id", - "in": "path", - "required": true - }, - { - "name": "body", - "in": "body", - "schema": { - "$ref": "#/definitions/EditAttachmentOptions" - } - } - ], - "responses": { - "201": { - "$ref": "#/responses/Attachment" - } - } - } - }, "/repos/{owner}/{repo}/issues/comments": { "get": { "produces": [ @@ -4692,16 +4563,19 @@ } } }, - "/repos/{owner}/{repo}/issues/comments/assets/{attachment_id}": { + "/repos/{owner}/{repo}/issues/comments/{id}": { "get": { + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], "tags": [ "issue" ], - "summary": "Get a comment attachment", - "operationId": "issueGetIssueCommentAttachment", + "summary": "Get a comment", + "operationId": "issueGetComment", "parameters": [ { "type": "string", @@ -4720,27 +4594,33 @@ { "type": "integer", "format": "int64", - "description": "id of the attachment to get", - "name": "attachment_id", + "description": "id of the comment", + "name": "id", "in": "path", "required": true } ], "responses": { "200": { - "$ref": "#/responses/Attachment" + "$ref": "#/responses/Comment" + }, + "204": { + "$ref": "#/responses/empty" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" } } }, "delete": { - "produces": [ - "application/json" - ], "tags": [ "issue" ], - "summary": "Delete a comment attachment", - "operationId": "issueDeleteIssueCommentAttachment", + "summary": "Delete a comment", + "operationId": "issueDeleteComment", "parameters": [ { "type": "string", @@ -4759,8 +4639,8 @@ { "type": "integer", "format": "int64", - "description": "id of the attachment to delete", - "name": "attachment_id", + "description": "id of comment to delete", + "name": "id", "in": "path", "required": true } @@ -4768,6 +4648,12 @@ "responses": { "204": { "$ref": "#/responses/empty" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" } } }, @@ -4781,8 +4667,8 @@ "tags": [ "issue" ], - "summary": "Edit a comment attachment", - "operationId": "issueEditIssueCommentAttachment", + "summary": "Edit a comment", + "operationId": "issueEditComment", "parameters": [ { "type": "string", @@ -4801,8 +4687,8 @@ { "type": "integer", "format": "int64", - "description": "id of the attachment to edit", - "name": "attachment_id", + "description": "id of the comment to edit", + "name": "id", "in": "path", "required": true }, @@ -4810,30 +4696,36 @@ "name": "body", "in": "body", "schema": { - "$ref": "#/definitions/EditAttachmentOptions" + "$ref": "#/definitions/EditIssueCommentOption" } } ], "responses": { - "201": { - "$ref": "#/responses/Attachment" + "200": { + "$ref": "#/responses/Comment" + }, + "204": { + "$ref": "#/responses/empty" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" } } } }, - "/repos/{owner}/{repo}/issues/comments/{id}": { + "/repos/{owner}/{repo}/issues/comments/{id}/assets": { "get": { - "consumes": [ - "application/json" - ], "produces": [ "application/json" ], "tags": [ "issue" ], - "summary": "Get a comment", - "operationId": "issueGetComment", + "summary": "List comment's attachments", + "operationId": "issueListIssueCommentAttachments", "parameters": [ { "type": "string", @@ -4860,25 +4752,22 @@ ], "responses": { "200": { - "$ref": "#/responses/Comment" - }, - "204": { - "$ref": "#/responses/empty" - }, - "403": { - "$ref": "#/responses/forbidden" - }, - "404": { - "$ref": "#/responses/notFound" + "$ref": "#/responses/AttachmentList" } } }, - "delete": { + "post": { + "consumes": [ + "multipart/form-data" + ], + "produces": [ + "application/json" + ], "tags": [ "issue" ], - "summary": "Delete a comment", - "operationId": "issueDeleteComment", + "summary": "Create a comment attachment", + "operationId": "issueCreateIssueCommentAttachment", "parameters": [ { "type": "string", @@ -4897,36 +4786,45 @@ { "type": "integer", "format": "int64", - "description": "id of comment to delete", + "description": "id of the comment", "name": "id", "in": "path", "required": true + }, + { + "type": "string", + "description": "name of the attachment", + "name": "name", + "in": "query" + }, + { + "type": "file", + "description": "attachment to upload", + "name": "attachment", + "in": "formData", + "required": true } ], "responses": { - "204": { - "$ref": "#/responses/empty" - }, - "403": { - "$ref": "#/responses/forbidden" + "201": { + "$ref": "#/responses/Attachment" }, - "404": { - "$ref": "#/responses/notFound" + "400": { + "$ref": "#/responses/error" } } - }, - "patch": { - "consumes": [ - "application/json" - ], + } + }, + "/repos/{owner}/{repo}/issues/comments/{id}/assets/{attachment_id}": { + "get": { "produces": [ "application/json" ], "tags": [ "issue" ], - "summary": "Edit a comment", - "operationId": "issueEditComment", + "summary": "Get a comment attachment", + "operationId": "issueGetIssueCommentAttachment", "parameters": [ { "type": "string", @@ -4945,45 +4843,27 @@ { "type": "integer", "format": "int64", - "description": "id of the comment to edit", - "name": "id", + "description": "id of the attachment to get", + "name": "attachment_id", "in": "path", "required": true - }, - { - "name": "body", - "in": "body", - "schema": { - "$ref": "#/definitions/EditIssueCommentOption" - } } ], "responses": { "200": { - "$ref": "#/responses/Comment" - }, - "204": { - "$ref": "#/responses/empty" - }, - "403": { - "$ref": "#/responses/forbidden" - }, - "404": { - "$ref": "#/responses/notFound" + "$ref": "#/responses/Attachment" } } - } - }, - "/repos/{owner}/{repo}/issues/comments/{id}/assets": { - "get": { + }, + "delete": { "produces": [ "application/json" ], "tags": [ "issue" ], - "summary": "List comment's attachments", - "operationId": "issueListIssueCommentAttachments", + "summary": "Delete a comment attachment", + "operationId": "issueDeleteIssueCommentAttachment", "parameters": [ { "type": "string", @@ -5002,21 +4882,21 @@ { "type": "integer", "format": "int64", - "description": "id of the comment", - "name": "id", + "description": "id of the attachment to delete", + "name": "attachment_id", "in": "path", "required": true } ], "responses": { - "200": { - "$ref": "#/responses/AttachmentList" + "204": { + "$ref": "#/responses/empty" } } }, - "post": { + "patch": { "consumes": [ - "multipart/form-data" + "application/json" ], "produces": [ "application/json" @@ -5024,8 +4904,8 @@ "tags": [ "issue" ], - "summary": "Create a comment attachment", - "operationId": "issueCreateIssueCommentAttachment", + "summary": "Edit a comment attachment", + "operationId": "issueEditIssueCommentAttachment", "parameters": [ { "type": "string", @@ -5044,31 +4924,22 @@ { "type": "integer", "format": "int64", - "description": "id of the comment", - "name": "id", + "description": "id of the attachment to edit", + "name": "attachment_id", "in": "path", "required": true }, { - "type": "string", - "description": "name of the attachment", - "name": "name", - "in": "query" - }, - { - "type": "file", - "description": "attachment to upload", - "name": "attachment", - "in": "formData", - "required": true + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/EditAttachmentOptions" + } } ], "responses": { "201": { "$ref": "#/responses/Attachment" - }, - "400": { - "$ref": "#/responses/error" } } } @@ -5428,6 +5299,135 @@ } } }, + "/repos/{owner}/{repo}/issues/{index}/assets/{attachment_id}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Get a issue attachment", + "operationId": "issueGetIssueAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to get", + "name": "attachment_id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/Attachment" + } + } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Delete a issue attachment", + "operationId": "issueDeleteIssueAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to delete", + "name": "attachment_id", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + } + } + }, + "patch": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Edit a issue attachment", + "operationId": "issueEditIssueAttachment", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "id of the attachment to edit", + "name": "attachment_id", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/EditAttachmentOptions" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/Attachment" + } + } + } + }, "/repos/{owner}/{repo}/issues/{index}/comments": { "get": { "produces": [ From a776dc2078a07056a71696d9163ba920d223a379 Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 8 Oct 2021 21:49:48 +0200 Subject: [PATCH 22/43] fix swagger docs --- routers/api/v1/repo/issue_attachment.go | 31 ++++++- .../api/v1/repo/issue_comment_attachment.go | 28 +++++++ templates/swagger/v1_json.tmpl | 80 ++++++++++++++++++- 3 files changed, 137 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 0c295aa8de69..a0efc19a11d7 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -47,6 +47,12 @@ func GetIssueAttachment(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true // - name: attachment_id // in: path // description: id of the attachment to get @@ -56,6 +62,9 @@ func GetIssueAttachment(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/Attachment" + // "404": + // "$ref": "#/responses/error" + issueIndex := ctx.ParamsInt64(":index") issue, err := models.GetIssueByIndex(ctx.Repo.Repository.RepoID, issueIndex) if err != nil { @@ -90,13 +99,15 @@ func ListIssueAttachments(ctx *context.APIContext) { // required: true // - name: index // in: path - // description: id of the issue + // description: index of the issue // type: integer // format: int64 // required: true // responses: // "200": // "$ref": "#/responses/AttachmentList" + // "404": + // "$ref": "#/responses/error" issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { @@ -151,6 +162,8 @@ func CreateIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/Attachment" // "400": // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/error" // Check if issue exists and load issue issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) @@ -217,6 +230,12 @@ func EditIssueAttachment(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true // - name: attachment_id // in: path // description: id of the attachment to edit @@ -230,6 +249,8 @@ func EditIssueAttachment(ctx *context.APIContext) { // responses: // "201": // "$ref": "#/responses/Attachment" + // "404": + // "$ref": "#/responses/error" // get attachment and check permissions attach := getIssueAttachmentSafeWrite(ctx) @@ -265,6 +286,12 @@ func DeleteIssueAttachment(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true // - name: attachment_id // in: path // description: id of the attachment to delete @@ -274,6 +301,8 @@ func DeleteIssueAttachment(ctx *context.APIContext) { // responses: // "204": // "$ref": "#/responses/empty" + // "404": + // "$ref": "#/responses/error" attach := getIssueAttachmentSafeWrite(ctx) if attach == nil { diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 4110131c006c..7b92eed294f3 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -36,6 +36,12 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: id + // in: path + // description: id of the comment + // type: integer + // format: int64 + // required: true // - name: attachment_id // in: path // description: id of the attachment to get @@ -45,6 +51,8 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/Attachment" + // "404": + // "$ref": "#/responses/error" comment := getIssueCommentSafe(ctx) if comment == nil { @@ -90,6 +98,8 @@ func ListIssueCommentAttachments(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/AttachmentList" + // "404": + // "$ref": "#/responses/error" comment := getIssueCommentSafe(ctx) if comment == nil { return @@ -144,6 +154,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { // "$ref": "#/responses/Attachment" // "400": // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/error" // Check if comment exists and load comment comment := getIssueCommentSafe(ctx) @@ -212,6 +224,12 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: id + // in: path + // description: id of the comment + // type: integer + // format: int64 + // required: true // - name: attachment_id // in: path // description: id of the attachment to edit @@ -225,6 +243,8 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { // responses: // "201": // "$ref": "#/responses/Attachment" + // "404": + // "$ref": "#/responses/error" attach := getIssueCommentAttachmentSafeWrite(ctx) if attach == nil { @@ -260,6 +280,12 @@ func DeleteIssueCommentAttachment(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: id + // in: path + // description: id of the comment + // type: integer + // format: int64 + // required: true // - name: attachment_id // in: path // description: id of the attachment to delete @@ -269,6 +295,8 @@ func DeleteIssueCommentAttachment(ctx *context.APIContext) { // responses: // "204": // "$ref": "#/responses/empty" + // "404": + // "$ref": "#/responses/error" attach := getIssueCommentAttachmentSafeWrite(ctx) if attach == nil { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 734963716dd8..c865f7990ed2 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4753,6 +4753,9 @@ "responses": { "200": { "$ref": "#/responses/AttachmentList" + }, + "404": { + "$ref": "#/responses/error" } } }, @@ -4811,6 +4814,9 @@ }, "400": { "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/error" } } } @@ -4840,6 +4846,14 @@ "in": "path", "required": true }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment", + "name": "id", + "in": "path", + "required": true + }, { "type": "integer", "format": "int64", @@ -4852,6 +4866,9 @@ "responses": { "200": { "$ref": "#/responses/Attachment" + }, + "404": { + "$ref": "#/responses/error" } } }, @@ -4879,6 +4896,14 @@ "in": "path", "required": true }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment", + "name": "id", + "in": "path", + "required": true + }, { "type": "integer", "format": "int64", @@ -4891,6 +4916,9 @@ "responses": { "204": { "$ref": "#/responses/empty" + }, + "404": { + "$ref": "#/responses/error" } } }, @@ -4921,6 +4949,14 @@ "in": "path", "required": true }, + { + "type": "integer", + "format": "int64", + "description": "id of the comment", + "name": "id", + "in": "path", + "required": true + }, { "type": "integer", "format": "int64", @@ -4940,6 +4976,9 @@ "responses": { "201": { "$ref": "#/responses/Attachment" + }, + "404": { + "$ref": "#/responses/error" } } } @@ -5228,7 +5267,7 @@ { "type": "integer", "format": "int64", - "description": "id of the issue", + "description": "index of the issue", "name": "index", "in": "path", "required": true @@ -5237,6 +5276,9 @@ "responses": { "200": { "$ref": "#/responses/AttachmentList" + }, + "404": { + "$ref": "#/responses/error" } } }, @@ -5295,6 +5337,9 @@ }, "400": { "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/error" } } } @@ -5324,6 +5369,14 @@ "in": "path", "required": true }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, { "type": "integer", "format": "int64", @@ -5336,6 +5389,9 @@ "responses": { "200": { "$ref": "#/responses/Attachment" + }, + "404": { + "$ref": "#/responses/error" } } }, @@ -5363,6 +5419,14 @@ "in": "path", "required": true }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, { "type": "integer", "format": "int64", @@ -5375,6 +5439,9 @@ "responses": { "204": { "$ref": "#/responses/empty" + }, + "404": { + "$ref": "#/responses/error" } } }, @@ -5405,6 +5472,14 @@ "in": "path", "required": true }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, { "type": "integer", "format": "int64", @@ -5424,6 +5499,9 @@ "responses": { "201": { "$ref": "#/responses/Attachment" + }, + "404": { + "$ref": "#/responses/error" } } } From bfafd9d9d0b42329bee35cea30a9ca4b238b4b7f Mon Sep 17 00:00:00 2001 From: Norwin Date: Fri, 8 Oct 2021 21:52:34 +0200 Subject: [PATCH 23/43] fix updating issue --- modules/notification/webhook/webhook.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index acdb91efe373..638d3e9d675a 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -310,8 +310,11 @@ func (m *webhookNotifier) NotifyNewPullRequest(pull *models.PullRequest, mention } func (m *webhookNotifier) NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string) { - mode, _ := models.AccessLevel(issue.Poster, issue.Repo) var err error + if err = issue.LoadRepo(); err != nil { + log.Error("NotifyIssueChangeContent: coulnd't load repo", err) + } + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) if issue.IsPull { issue.PullRequest.Issue = issue err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{ From 6b521f2232659592632eef2f910688e3003ac35e Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Fri, 15 Oct 2021 20:10:09 +0200 Subject: [PATCH 24/43] Correct the year of the code contribution --- routers/api/v1/repo/issue_attachment.go | 2 +- routers/api/v1/repo/issue_comment_attachment.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index a0efc19a11d7..b53164727468 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -1,4 +1,4 @@ -// Copyright 2018 The Gitea Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 7b92eed294f3..8135dcf9c9da 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -1,4 +1,4 @@ -// Copyright 2018 The Gitea Authors. All rights reserved. +// Copyright 2021 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. From 32e85083c714b7b650614e4b58ec3aad010d79bc Mon Sep 17 00:00:00 2001 From: Norwin Date: Sat, 9 Oct 2021 12:17:19 +0200 Subject: [PATCH 25/43] fix errors always return json, always return message, DRY refactor --- modules/context/api.go | 20 ++++++++++--------- routers/api/v1/repo/issue_attachment.go | 12 +++++------ .../api/v1/repo/issue_comment_attachment.go | 10 +++++----- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/modules/context/api.go b/modules/context/api.go index e5216d911f8a..6adc6c36b0a7 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -109,17 +109,19 @@ func (ctx *APIContext) Error(status int, title string, obj interface{}) { // InternalServerError responds with an error message to the client with the error as a message // and the file and line of the caller. func (ctx *APIContext) InternalServerError(err error) { - log.ErrorWithSkip(1, "InternalServerError: %v", err) + ctx.Error(http.StatusInternalServerError, "InternalServerError", err) + return +} - var message string - if !setting.IsProd() || (ctx.User != nil && ctx.User.IsAdmin) { - message = err.Error() +// NotFoundOrServerError use error check function to determine if the error +// is about not found. It responds with 404 status code for not found error, +// or error context description for logging purpose of 500 server error. +func (ctx *APIContext) NotFoundOrServerError(title string, errck func(error) bool, err error) { + if errck(err) { + ctx.Error(http.StatusNotFound, title, err) + return } - - ctx.JSON(http.StatusInternalServerError, APIError{ - Message: message, - URL: setting.API.SwaggerURL, - }) + ctx.Error(http.StatusInternalServerError, title, err) } var ( diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index b53164727468..694869dafcc2 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -203,7 +203,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { issue.Attachments = append(issue.Attachments, attach) if err := issue_service.ChangeContent(issue, ctx.User, issue.Content); err != nil { - ctx.ServerError("ChangeContent", err) + ctx.Error(http.StatusInternalServerError, "ChangeContent", err) return } @@ -263,7 +263,7 @@ func EditIssueAttachment(ctx *context.APIContext) { attach.Name = form.Name } if err := models.UpdateAttachment(attach); err != nil { - ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) + ctx.Error(http.StatusInternalServerError, "UpdateAttachment", err) } ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) } @@ -319,7 +319,7 @@ func getIssueAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { issueIndex := ctx.ParamsInt64(":index") issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) if err != nil { - ctx.NotFoundOrServerError("GetIssueByID", models.IsErrIssueNotExist, err) + ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err) return nil } if !canUserWriteIssueAttachment(ctx, issue) { @@ -359,17 +359,17 @@ func canUserWriteIssueAttachment(ctx *context.APIContext, i *models.Issue) (succ func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, a *models.Attachment, issue *models.Issue) (success bool) { if a.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) - ctx.NotFound() + ctx.NotFound("no such attachment in repo") return } if a.IssueID == 0 { // catch people trying to get release assets ;) log.Debug("Requested attachment[%d] is not in an issue.", a.ID) - ctx.NotFound() + ctx.NotFound("no such attachment in issue") return } else if issue != nil && a.IssueID != issue.ID { log.Debug("Requested attachment[%d] does not belong to issue[%d, #%d].", a.ID, issue.ID, issue.Index) - ctx.NotFound() + ctx.NotFound("no such attachment in issue") return } return true diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 8135dcf9c9da..2d3a52bb215c 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -323,7 +323,7 @@ func getIssueCommentSafe(ctx *context.APIContext) *models.Comment { return nil } if comment.Issue == nil || comment.Issue.RepoID != ctx.Repo.Repository.ID { - ctx.Error(http.StatusNotFound, "no matching issue comment found", err) + ctx.Error(http.StatusNotFound, "", "no matching issue comment found") return nil } return comment @@ -360,7 +360,7 @@ func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *models. func canUserWriteIssueCommentAttachment(ctx *context.APIContext, c *models.Comment) (success bool) { canEditComment := ctx.User.ID == c.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() if !canEditComment { - ctx.Error(http.StatusForbidden, "CommentEditPerm", "user should have permission to edit comment") + ctx.Error(http.StatusForbidden, "", "user should have permission to edit comment") return } @@ -370,18 +370,18 @@ func canUserWriteIssueCommentAttachment(ctx *context.APIContext, c *models.Comme func attachmentBelongsToRepoOrComment(ctx *context.APIContext, a *models.Attachment, comment *models.Comment) (success bool) { if a.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) - ctx.NotFound() + ctx.NotFound("no such attachment in repo") return } if a.IssueID == 0 || a.CommentID == 0 { // catch people trying to get release assets ;) log.Debug("Requested attachment[%d] is not in a comment.", a.ID) - ctx.NotFound() + ctx.NotFound("no such attachment in comment") return } if comment != nil && a.CommentID != comment.ID { log.Debug("Requested attachment[%d] does not belong to comment[%d].", a.ID, comment.ID) - ctx.NotFound() + ctx.NotFound("no such attachment in comment") return } return true From d32924d4d2a925716bc1668336bcb23292026d2f Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Thu, 21 Oct 2021 13:05:18 +0200 Subject: [PATCH 26/43] Fix call of GetIssueByIndex and correction of grammar errors --- routers/api/v1/repo/issue_attachment.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 694869dafcc2..8e43a1d58d41 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -33,7 +33,7 @@ import ( func GetIssueAttachment(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueGetIssueAttachment // --- - // summary: Get a issue attachment + // summary: Get an issue attachment // produces: // - application/json // parameters: @@ -66,7 +66,7 @@ func GetIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/error" issueIndex := ctx.ParamsInt64(":index") - issue, err := models.GetIssueByIndex(ctx.Repo.Repository.RepoID, issueIndex) + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) if err != nil { ctx.NotFoundOrServerError("GetIssueByID", models.IsErrIssueNotExist, err) return @@ -125,7 +125,7 @@ func ListIssueAttachments(ctx *context.APIContext) { func CreateIssueAttachment(ctx *context.APIContext) { // swagger:operation POST /repos/{owner}/{repo}/issues/{index}/assets issue issueCreateIssueAttachment // --- - // summary: Create a issue attachment + // summary: Create an issue attachment // produces: // - application/json // consumes: @@ -214,7 +214,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { func EditIssueAttachment(ctx *context.APIContext) { // swagger:operation PATCH /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueEditIssueAttachment // --- - // summary: Edit a issue attachment + // summary: Edit an issue attachment // produces: // - application/json // consumes: @@ -272,7 +272,7 @@ func EditIssueAttachment(ctx *context.APIContext) { func DeleteIssueAttachment(ctx *context.APIContext) { // swagger:operation DELETE /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueDeleteIssueAttachment // --- - // summary: Delete a issue attachment + // summary: Delete an issue attachment // produces: // - application/json // parameters: From 1459011ebeed0573a40ab73942274042bddd045b Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Thu, 21 Oct 2021 15:30:08 +0200 Subject: [PATCH 27/43] Remove redundant `return` statement --- modules/context/api.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/context/api.go b/modules/context/api.go index ec848868dc36..a5b5b32635a2 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -110,7 +110,6 @@ func (ctx *APIContext) Error(status int, title string, obj interface{}) { // and the file and line of the caller. func (ctx *APIContext) InternalServerError(err error) { ctx.Error(http.StatusInternalServerError, "InternalServerError", err) - return } // NotFoundOrServerError use error check function to determine if the error From 6d6b8587b243cdb70fabc94fcb21228401fef5af Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Thu, 21 Oct 2021 15:49:46 +0200 Subject: [PATCH 28/43] make generate-swagger --- templates/swagger/v1_json.tmpl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fa864e14479e..87388195062b 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -5292,7 +5292,7 @@ "tags": [ "issue" ], - "summary": "Create a issue attachment", + "summary": "Create an issue attachment", "operationId": "issueCreateIssueAttachment", "parameters": [ { @@ -5352,7 +5352,7 @@ "tags": [ "issue" ], - "summary": "Get a issue attachment", + "summary": "Get an issue attachment", "operationId": "issueGetIssueAttachment", "parameters": [ { @@ -5402,7 +5402,7 @@ "tags": [ "issue" ], - "summary": "Delete a issue attachment", + "summary": "Delete an issue attachment", "operationId": "issueDeleteIssueAttachment", "parameters": [ { @@ -5455,7 +5455,7 @@ "tags": [ "issue" ], - "summary": "Edit a issue attachment", + "summary": "Edit an issue attachment", "operationId": "issueEditIssueAttachment", "parameters": [ { @@ -18751,4 +18751,4 @@ "TOTPHeader": [] } ] -} +} \ No newline at end of file From 65173025f790acdce6d3ff2ac089bb0e4871689f Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Thu, 21 Oct 2021 15:54:44 +0200 Subject: [PATCH 29/43] Add newline to v1_json.tmpl --- templates/swagger/v1_json.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 87388195062b..c05e9296d338 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -18751,4 +18751,4 @@ "TOTPHeader": [] } ] -} \ No newline at end of file +} From cbd1a7762cc1f76e8da5e2d968642c1f3b9c7bf9 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Sat, 23 Oct 2021 20:18:42 +0200 Subject: [PATCH 30/43] [API] Add integration tests for issue attachments Signed-off-by: Andre Bruch --- integrations/api_issue_attachment_test.go | 138 ++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 integrations/api_issue_attachment_test.go diff --git a/integrations/api_issue_attachment_test.go b/integrations/api_issue_attachment_test.go new file mode 100644 index 000000000000..b81f765b7ef3 --- /dev/null +++ b/integrations/api_issue_attachment_test.go @@ -0,0 +1,138 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "bytes" + "fmt" + "io" + "mime/multipart" + "net/http" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" + api "code.gitea.io/gitea/modules/structs" + "github.com/stretchr/testify/assert" +) + +func TestAPIGetIssueAttachment(t *testing.T) { + defer prepareTestEnv(t)() + + attachment := db.AssertExistsAndLoadBean(t, &models.Attachment{ID: 1}).(*models.Attachment) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: attachment.RepoID}).(*models.Repository) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{RepoID: attachment.IssueID}).(*models.Issue) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets/%d?token=%s", + repoOwner.Name, repo.Name, issue.Index, attachment.ID, token) + + req := NewRequest(t, "GET", urlStr) + resp := session.MakeRequest(t, req, http.StatusOK) + apiAttachment := new(api.Attachment) + DecodeJSON(t, resp, &apiAttachment) + + db.AssertExistsAndLoadBean(t, &models.Attachment{ID: apiAttachment.ID, IssueID: issue.ID}) +} + +func TestAPIListIssueAttachments(t *testing.T) { + defer prepareTestEnv(t)() + + attachment := db.AssertExistsAndLoadBean(t, &models.Attachment{ID: 1}).(*models.Attachment) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: attachment.RepoID}).(*models.Repository) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{RepoID: attachment.IssueID}).(*models.Issue) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets?token=%s", + repoOwner.Name, repo.Name, issue.Index, token) + + req := NewRequest(t, "GET", urlStr) + resp := session.MakeRequest(t, req, http.StatusOK) + apiAttachment := new([]api.Attachment) + DecodeJSON(t, resp, &apiAttachment) + + db.AssertExistsAndLoadBean(t, &models.Attachment{ID: (*apiAttachment)[0].ID, IssueID: issue.ID}) +} + +func TestAPICreateIssueAttachment(t *testing.T) { + defer prepareTestEnv(t)() + + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{RepoID: repo.ID}).(*models.Issue) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets?token=%s", + repoOwner.Name, repo.Name, issue.Index, token) + + filename := "image.png" + buff := generateImg() + body := &bytes.Buffer{} + + //Setup multi-part + writer := multipart.NewWriter(body) + part, err := writer.CreateFormFile("attachment", filename) + assert.NoError(t, err) + _, err = io.Copy(part, &buff) + assert.NoError(t, err) + err = writer.Close() + assert.NoError(t, err) + + req := NewRequestWithBody(t, "POST", urlStr, body) + req.Header.Add("Content-Type", writer.FormDataContentType()) + resp := session.MakeRequest(t, req, http.StatusCreated) + + apiAttachment := new(api.Attachment) + DecodeJSON(t, resp, &apiAttachment) + + db.AssertExistsAndLoadBean(t, &models.Attachment{ID: apiAttachment.ID, IssueID: issue.ID}) +} + +func TestAPIEditIssueAttachment(t *testing.T) { + defer prepareTestEnv(t)() + const newAttachmentName = "newAttachmentName" + + attachment := db.AssertExistsAndLoadBean(t, &models.Attachment{ID: 1}).(*models.Attachment) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: attachment.RepoID}).(*models.Repository) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{RepoID: attachment.IssueID}).(*models.Issue) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets/%d?token=%s", + repoOwner.Name, repo.Name, issue.Index, attachment.ID, token) + req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{ + "name": newAttachmentName, + }) + resp := session.MakeRequest(t, req, http.StatusCreated) + apiAttachment := new(api.Attachment) + DecodeJSON(t, resp, &apiAttachment) + + db.AssertExistsAndLoadBean(t, &models.Attachment{ID: apiAttachment.ID, IssueID: issue.ID, Name: apiAttachment.Name}) +} + +func TestAPIDeleteIssueAttachment(t *testing.T) { + defer prepareTestEnv(t)() + + attachment := db.AssertExistsAndLoadBean(t, &models.Attachment{ID: 1}).(*models.Attachment) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: attachment.RepoID}).(*models.Repository) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{RepoID: attachment.IssueID}).(*models.Issue) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets/%d?token=%s", + repoOwner.Name, repo.Name, issue.Index, attachment.ID, token) + + req := NewRequest(t, "DELETE", urlStr) + session.MakeRequest(t, req, http.StatusNoContent) + + db.AssertNotExistsBean(t, &models.Attachment{ID: attachment.ID, IssueID: issue.ID}) +} From 922707064842cd9c6118724b73bd74ca54836ed6 Mon Sep 17 00:00:00 2001 From: Andre Bruch Date: Fri, 29 Oct 2021 05:16:35 +0200 Subject: [PATCH 31/43] [API] Add integration tests for comment attachments Signed-off-by: Andre Bruch --- integrations/api_comment_attachment_test.go | 148 ++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 integrations/api_comment_attachment_test.go diff --git a/integrations/api_comment_attachment_test.go b/integrations/api_comment_attachment_test.go new file mode 100644 index 000000000000..f5dd234bac45 --- /dev/null +++ b/integrations/api_comment_attachment_test.go @@ -0,0 +1,148 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "bytes" + "fmt" + "io" + "mime/multipart" + "net/http" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/convert" + api "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" +) + +func TestAPIGetCommentAttachment(t *testing.T) { + defer prepareTestEnv(t)() + + comment := db.AssertExistsAndLoadBean(t, &models.Comment{ID: 2}).(*models.Comment) + assert.NoError(t, comment.LoadIssue()) + assert.NoError(t, comment.LoadAttachments()) + attachment := db.AssertExistsAndLoadBean(t, &models.Attachment{ID: comment.Attachments[0].ID}).(*models.Attachment) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: comment.Issue.RepoID}).(*models.Repository) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/comments/%d/assets/%d", repoOwner.Name, repo.Name, comment.ID, attachment.ID) + resp := session.MakeRequest(t, req, http.StatusOK) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/comments/%d/assets/%d?token=%s", repoOwner.Name, repo.Name, comment.ID, attachment.ID, token) + resp = session.MakeRequest(t, req, http.StatusOK) + + var apiAttachment api.Attachment + DecodeJSON(t, resp, &apiAttachment) + + expect := convert.ToAttachment(attachment) + assert.Equal(t, expect.ID, apiAttachment.ID) + assert.Equal(t, expect.Name, apiAttachment.Name) + assert.Equal(t, expect.UUID, apiAttachment.UUID) + assert.Equal(t, expect.Created.Unix(), apiAttachment.Created.Unix()) +} + +func TestAPIListCommentAttachments(t *testing.T) { + defer prepareTestEnv(t)() + + comment := db.AssertExistsAndLoadBean(t, &models.Comment{ID: 2}).(*models.Comment) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{ID: comment.IssueID}).(*models.Issue) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: issue.RepoID}).(*models.Repository) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/comments/%d/assets", + repoOwner.Name, repo.Name, comment.ID) + resp := session.MakeRequest(t, req, http.StatusOK) + + var apiAttachments []*api.Attachment + DecodeJSON(t, resp, &apiAttachments) + expectedCount := db.GetCount(t, &models.Attachment{CommentID: comment.ID}) + assert.EqualValues(t, expectedCount, len(apiAttachments)) + db.AssertExistsAndLoadBean(t, &models.Attachment{ID: apiAttachments[0].ID, CommentID: comment.ID}) +} + +func TestAPICreateCommentAttachment(t *testing.T) { + defer prepareTestEnv(t)() + + comment := db.AssertExistsAndLoadBean(t, &models.Comment{ID: 2}).(*models.Comment) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{ID: comment.IssueID}).(*models.Issue) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: issue.RepoID}).(*models.Repository) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/assets?token=%s", + repoOwner.Name, repo.Name, comment.ID, token) + + filename := "image.png" + buff := generateImg() + body := &bytes.Buffer{} + + //Setup multi-part + writer := multipart.NewWriter(body) + part, err := writer.CreateFormFile("attachment", filename) + assert.NoError(t, err) + _, err = io.Copy(part, &buff) + assert.NoError(t, err) + err = writer.Close() + assert.NoError(t, err) + + req := NewRequestWithBody(t, "POST", urlStr, body) + req.Header.Add("Content-Type", writer.FormDataContentType()) + resp := session.MakeRequest(t, req, http.StatusCreated) + + apiAttachment := new(api.Attachment) + DecodeJSON(t, resp, &apiAttachment) + + db.AssertExistsAndLoadBean(t, &models.Attachment{ID: apiAttachment.ID, CommentID: comment.ID}) +} + +func TestAPIEditCommentAttachment(t *testing.T) { + defer prepareTestEnv(t)() + const newAttachmentName = "newAttachmentName" + + attachment := db.AssertExistsAndLoadBean(t, &models.Attachment{ID: 6}).(*models.Attachment) + comment := db.AssertExistsAndLoadBean(t, &models.Comment{ID: attachment.CommentID}).(*models.Comment) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{ID: comment.IssueID}).(*models.Issue) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: issue.RepoID}).(*models.Repository) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/assets/%d?token=%s", + repoOwner.Name, repo.Name, comment.ID, attachment.ID, token) + req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{ + "name": newAttachmentName, + }) + resp := session.MakeRequest(t, req, http.StatusCreated) + apiAttachment := new(api.Attachment) + DecodeJSON(t, resp, &apiAttachment) + + db.AssertExistsAndLoadBean(t, &models.Attachment{ID: apiAttachment.ID, CommentID: comment.ID, Name: apiAttachment.Name}) +} + +func TestAPIDeleteCommentAttachment(t *testing.T) { + defer prepareTestEnv(t)() + + attachment := db.AssertExistsAndLoadBean(t, &models.Attachment{ID: 6}).(*models.Attachment) + comment := db.AssertExistsAndLoadBean(t, &models.Comment{ID: attachment.CommentID}).(*models.Comment) + issue := db.AssertExistsAndLoadBean(t, &models.Issue{ID: comment.IssueID}).(*models.Issue) + repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: issue.RepoID}).(*models.Repository) + repoOwner := db.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User) + + session := loginUser(t, repoOwner.Name) + token := getTokenForLoggedInUser(t, session) + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/assets/%d?token=%s", + repoOwner.Name, repo.Name, comment.ID, attachment.ID, token) + + req := NewRequestf(t, "DELETE", urlStr) + session.MakeRequest(t, req, http.StatusNoContent) + + db.AssertNotExistsBean(t, &models.Attachment{ID: attachment.ID, CommentID: comment.ID}) +} From fbab44670e9c01d782b6c73113dbc123d5195cc9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 22 Apr 2022 20:45:02 +0200 Subject: [PATCH 32/43] adapt latest refactors --- models/repo/attachment.go | 5 ---- models/repo/attachment_test.go | 2 +- routers/api/v1/repo/issue_attachment.go | 23 ++++++++++--------- .../api/v1/repo/issue_comment_attachment.go | 23 ++++++++++--------- routers/api/v1/repo/release_attachment.go | 2 +- routers/web/repo/issue.go | 2 +- 6 files changed, 27 insertions(+), 30 deletions(-) diff --git a/models/repo/attachment.go b/models/repo/attachment.go index f5351578fbc5..bb844974d6e5 100644 --- a/models/repo/attachment.go +++ b/models/repo/attachment.go @@ -225,11 +225,6 @@ func DeleteAttachmentsByComment(commentID int64, remove bool) (int, error) { return DeleteAttachments(db.DefaultContext, attachments, remove) } -// UpdateAttachment updates the given attachment in database -func UpdateAttachment(atta *Attachment) error { - return UpdateAttachmentCtx(db.DefaultContext, atta) -} - // UpdateAttachmentByUUID Updates attachment via uuid func UpdateAttachmentByUUID(ctx context.Context, attach *Attachment, cols ...string) error { if attach.UUID == "" { diff --git a/models/repo/attachment_test.go b/models/repo/attachment_test.go index 53c28d5324f8..ff011090c092 100644 --- a/models/repo/attachment_test.go +++ b/models/repo/attachment_test.go @@ -86,7 +86,7 @@ func TestUpdateAttachment(t *testing.T) { assert.Equal(t, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", attach.UUID) attach.Name = "new_name" - assert.NoError(t, UpdateAttachment(attach)) + assert.NoError(t, UpdateAttachmentCtx(db.DefaultContext, attach)) unittest.AssertExistsAndLoadBean(t, &Attachment{Name: "new_name"}) } diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 218654c01598..0213f578e67e 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -8,6 +8,7 @@ import ( "net/http" "code.gitea.io/gitea/models" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/modules/log" @@ -189,9 +190,9 @@ func CreateIssueAttachment(ctx *context.APIContext) { filename = query } - attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &models.Attachment{ + attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{ Name: filename, - UploaderID: ctx.User.ID, + UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, IssueID: issue.ID, }) @@ -202,7 +203,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { issue.Attachments = append(issue.Attachments, attach) - if err := issue_service.ChangeContent(issue, ctx.User, issue.Content); err != nil { + if err := issue_service.ChangeContent(issue, ctx.Doer, issue.Content); err != nil { ctx.Error(http.StatusInternalServerError, "ChangeContent", err) return } @@ -262,7 +263,7 @@ func EditIssueAttachment(ctx *context.APIContext) { if form.Name != "" { attach.Name = form.Name } - if err := models.UpdateAttachment(attach); err != nil { + if err := repo_model.UpdateAttachmentCtx(ctx, attach); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", err) } ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) @@ -308,14 +309,14 @@ func DeleteIssueAttachment(ctx *context.APIContext) { if attach == nil { return } - if err := models.DeleteAttachment(attach, true); err != nil { + if err := repo_model.DeleteAttachment(attach, true); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err) return } ctx.Status(http.StatusNoContent) } -func getIssueAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { +func getIssueAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Attachment { issueIndex := ctx.ParamsInt64(":index") issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) if err != nil { @@ -333,11 +334,11 @@ func getIssueAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { return attach } -func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *models.Issue) *models.Attachment { +func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *models.Issue) *repo_model.Attachment { attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) + attach, err := repo_model.GetAttachmentByID(attachID) if err != nil { - ctx.NotFoundOrServerError("GetAttachmentByID", models.IsErrAttachmentNotExist, err) + ctx.NotFoundOrServerError("GetAttachmentByID", repo_model.IsErrAttachmentNotExist, err) return nil } if !attachmentBelongsToRepoOrIssue(ctx, attach, issue) { @@ -347,7 +348,7 @@ func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *models.Issue) *m } func canUserWriteIssueAttachment(ctx *context.APIContext, i *models.Issue) (success bool) { - canEditIssue := ctx.User.ID == i.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() + canEditIssue := ctx.Doer.ID == i.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() if !canEditIssue { ctx.Error(http.StatusForbidden, "IssueEditPerm", "user should have permission to edit issue") return @@ -356,7 +357,7 @@ func canUserWriteIssueAttachment(ctx *context.APIContext, i *models.Issue) (succ return true } -func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, a *models.Attachment, issue *models.Issue) (success bool) { +func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, a *repo_model.Attachment, issue *models.Issue) (success bool) { if a.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) ctx.NotFound("no such attachment in repo") diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index f576a06ca59a..f5e3d0aa44bc 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -8,6 +8,7 @@ import ( "net/http" "code.gitea.io/gitea/models" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/modules/log" @@ -180,9 +181,9 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { filename = query } - attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &models.Attachment{ + attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{ Name: filename, - UploaderID: ctx.User.ID, + UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, IssueID: comment.IssueID, CommentID: comment.ID, @@ -196,7 +197,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { return } - if err = comment_service.UpdateComment(comment, ctx.User, comment.Content); err != nil { + if err = comment_service.UpdateComment(comment, ctx.Doer, comment.Content); err != nil { ctx.ServerError("UpdateComment", err) return } @@ -256,7 +257,7 @@ func EditIssueCommentAttachment(ctx *context.APIContext) { attach.Name = form.Name } - if err := models.UpdateAttachment(attach); err != nil { + if err := repo_model.UpdateAttachmentCtx(ctx, attach); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) } ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) @@ -303,7 +304,7 @@ func DeleteIssueCommentAttachment(ctx *context.APIContext) { return } - if err := models.DeleteAttachment(attach, true); err != nil { + if err := repo_model.DeleteAttachment(attach, true); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err) return } @@ -329,7 +330,7 @@ func getIssueCommentSafe(ctx *context.APIContext) *models.Comment { return comment } -func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *models.Attachment { +func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Attachment { comment := getIssueCommentSafe(ctx) if comment == nil { return nil @@ -344,11 +345,11 @@ func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *models.Attachm return attach } -func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *models.Comment) *models.Attachment { +func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *models.Comment) *repo_model.Attachment { attachID := ctx.ParamsInt64(":asset") - attach, err := models.GetAttachmentByID(attachID) + attach, err := repo_model.GetAttachmentByID(attachID) if err != nil { - ctx.NotFoundOrServerError("GetAttachmentByID", models.IsErrAttachmentNotExist, err) + ctx.NotFoundOrServerError("GetAttachmentByID", repo_model.IsErrAttachmentNotExist, err) return nil } if !attachmentBelongsToRepoOrComment(ctx, attach, comment) { @@ -358,7 +359,7 @@ func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *models. } func canUserWriteIssueCommentAttachment(ctx *context.APIContext, c *models.Comment) (success bool) { - canEditComment := ctx.User.ID == c.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() + canEditComment := ctx.Doer.ID == c.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() if !canEditComment { ctx.Error(http.StatusForbidden, "", "user should have permission to edit comment") return @@ -367,7 +368,7 @@ func canUserWriteIssueCommentAttachment(ctx *context.APIContext, c *models.Comme return true } -func attachmentBelongsToRepoOrComment(ctx *context.APIContext, a *models.Attachment, comment *models.Comment) (success bool) { +func attachmentBelongsToRepoOrComment(ctx *context.APIContext, a *repo_model.Attachment, comment *models.Comment) (success bool) { if a.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", a.ID, ctx.Repo.Repository) ctx.NotFound("no such attachment in repo") diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index c0e29f34dad5..c19d30744154 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -262,7 +262,7 @@ func EditReleaseAttachment(ctx *context.APIContext) { attach.Name = form.Name } - if err := repo_model.UpdateAttachment(attach); err != nil { + if err := repo_model.UpdateAttachmentCtx(ctx, attach); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach) } ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 9774b2718dc7..98b35c0cd1fc 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1857,7 +1857,7 @@ func UpdateIssueContent(ctx *context.Context) { } content := ctx.FormString("content") - if err := issue_service.ChangeContent(issue, ctx.User, content); err != nil { + if err := issue_service.ChangeContent(issue, ctx.Doer, content); err != nil { ctx.ServerError("ChangeContent", err) return } From c8cafc0b0b32fc1e268c0fc725229a3a841cb7ce Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 22 Apr 2022 20:48:25 +0200 Subject: [PATCH 33/43] rm merge conflict resolve --- models/issue_comment.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index d0b5cdc6a443..d51a645a79fd 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -858,17 +858,8 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err) } - - for i := range attachments { - attachments[i].IssueID = opts.Issue.ID - attachments[i].CommentID = comment.ID - // No assign value could be 0, so ignore AllCols(). - if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { - return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err) - } - } - comment.Attachments = attachments } + comment.Attachments = attachments } case CommentTypeReopen, CommentTypeClose: if err = updateIssueClosedNum(ctx, opts.Issue); err != nil { From 5bd533cb6355a24c2c4bfb0d1c5c04f0eff8e4b7 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 22 Apr 2022 20:55:44 +0200 Subject: [PATCH 34/43] format & fix --- modules/notification/webhook/webhook.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index a01d6bb2d395..28c69a177be3 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -337,11 +337,13 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *user_model.User, issue ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("webhook.NotifyIssueChangeContent User: %s[%d] Issue[%d] #%d in [%d]", doer.Name, doer.ID, issue.ID, issue.Index, issue.RepoID)) defer finished() - var err error - if err = issue.LoadRepo(ctx); err != nil { + if err := issue.LoadRepo(ctx); err != nil { log.Error("NotifyIssueChangeContent: coulnd't load repo", err) + return } + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) + var err error if issue.IsPull { issue.PullRequest.Issue = issue err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequest, &api.PullRequestPayload{ From 91c93edff0ddd1cdddb460d8e6183c7561f49c40 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 12 Nov 2022 09:54:14 +0000 Subject: [PATCH 35/43] Add issue/pull permission check. --- routers/api/v1/repo/issue_attachment.go | 20 +++++------ .../api/v1/repo/issue_comment_attachment.go | 34 +++++++++---------- routers/web/repo/issue.go | 6 ---- .../api_comment_attachment_test.go | 4 +-- 4 files changed, 28 insertions(+), 36 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 9dff820e0c14..12a2e11959d3 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -189,7 +189,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { filename = query } - attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{ + attachment, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -207,7 +207,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { return } - ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attachment)) } // EditIssueAttachment updates the given attachment @@ -253,8 +253,8 @@ func EditIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/error" // get attachment and check permissions - attach := getIssueAttachmentSafeWrite(ctx) - if attach == nil { + attachment := getIssueAttachmentSafeWrite(ctx) + if attachment == nil { return } // do changes to attachment. only meaningful change is name. @@ -262,10 +262,10 @@ func EditIssueAttachment(ctx *context.APIContext) { if form.Name != "" { attach.Name = form.Name } - if err := repo_model.UpdateAttachment(ctx, attach); err != nil { + if err := repo_model.UpdateAttachment(ctx, attachment); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", err) } - ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attachment)) } // DeleteIssueAttachment delete a given attachment @@ -304,11 +304,11 @@ func DeleteIssueAttachment(ctx *context.APIContext) { // "404": // "$ref": "#/responses/error" - attach := getIssueAttachmentSafeWrite(ctx) - if attach == nil { + attachment := getIssueAttachmentSafeWrite(ctx) + if attachment == nil { return } - if err := repo_model.DeleteAttachment(attach, true); err != nil { + if err := repo_model.DeleteAttachment(attachment, true); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err) return } @@ -341,7 +341,7 @@ func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *issues_model.Iss } func canUserWriteIssueAttachment(ctx *context.APIContext, issue *issues_model.Issue) (success bool) { - canEditIssue := ctx.Doer.ID == issue.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() + canEditIssue := ctx.IsSigned && (ctx.Doer.ID == issue.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin()) && ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) if !canEditIssue { ctx.Error(http.StatusForbidden, "IssueEditPerm", "user should have permission to edit issue") return diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 013cd0bb09d5..51c2ded0e6fb 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -59,17 +59,17 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { if comment == nil { return } - attach := getIssueCommentAttachmentSafeRead(ctx, comment) + attachment := getIssueCommentAttachmentSafeRead(ctx, comment) if attach == nil { return } - if attach.CommentID != comment.ID { - log.Debug("User requested attachment[%d] is not in comment[%d].", attach.ID, comment.ID) + if attachment.CommentID != comment.ID { + log.Debug("User requested attachment[%d] is not in comment[%d].", attachment.ID, comment.ID) ctx.NotFound("attachment not in comment") return } - ctx.JSON(http.StatusOK, convert.ToAttachment(attach)) + ctx.JSON(http.StatusOK, convert.ToAttachment(attachment)) } // ListIssueCommentAttachments lists all attachments of the comment @@ -181,7 +181,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { filename = query } - attach, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{ + attachment, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -202,7 +202,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { return } - ctx.JSON(http.StatusCreated, convert.ToAttachment(attach)) + ctx.JSON(http.StatusCreated, convert.ToAttachment(attachment)) } // EditIssueCommentAttachment updates the given attachment @@ -317,8 +317,6 @@ func getIssueCommentSafe(ctx *context.APIContext) *issues_model.Comment { ctx.NotFoundOrServerError("GetCommentByID", issues_model.IsErrCommentNotExist, err) return nil } - // deny accessing arbitrary comments via this API - // TODO: if issue ID were available on context, we could check that too. if err := comment.LoadIssue(); err != nil { ctx.Error(http.StatusInternalServerError, "comment.LoadIssue", err) return nil @@ -341,6 +339,16 @@ func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Att return getIssueCommentAttachmentSafeRead(ctx, comment) } +func canUserWriteIssueCommentAttachment(ctx *context.APIContext, comment *issues_model.Comment) (success bool) { + canEditComment := ctx.IsSigned && (ctx.Doer.ID == comment.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin()) && ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull) + if !canEditComment { + ctx.Error(http.StatusForbidden, "", "user should have permission to edit comment") + return + } + + return true +} + func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *issues_model.Comment) *repo_model.Attachment { attachment, err := repo_model.GetAttachmentByID(ctx, ctx.ParamsInt64("asset")) if err != nil { @@ -353,16 +361,6 @@ func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *issues_ return attachment } -func canUserWriteIssueCommentAttachment(ctx *context.APIContext, comment *issues_model.Comment) (success bool) { - canEditComment := ctx.Doer.ID == comment.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() - if !canEditComment { - ctx.Error(http.StatusForbidden, "", "user should have permission to edit comment") - return - } - - return true -} - func attachmentBelongsToRepoOrComment(ctx *context.APIContext, attachment *repo_model.Attachment, comment *issues_model.Comment) (success bool) { if attachment.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", attachment.ID, ctx.Repo.Repository) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 005c381a0671..ce2371ce5d91 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1965,12 +1965,6 @@ func UpdateIssueContent(ctx *context.Context) { } } - content := ctx.FormString("content") - if err := issue_service.ChangeContent(issue, ctx.Doer, content); err != nil { - ctx.ServerError("ChangeContent", err) - return - } - content, err := markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.FormString("context"), // FIXME: <- IS THIS SAFE ? Metas: ctx.Repo.Repository.ComposeMetas(), diff --git a/tests/integration/api_comment_attachment_test.go b/tests/integration/api_comment_attachment_test.go index 1a3c85371c70..8baaba3645d9 100644 --- a/tests/integration/api_comment_attachment_test.go +++ b/tests/integration/api_comment_attachment_test.go @@ -36,9 +36,9 @@ func TestAPIGetCommentAttachment(t *testing.T) { session := loginUser(t, repoOwner.Name) token := getTokenForLoggedInUser(t, session) req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/comments/%d/assets/%d", repoOwner.Name, repo.Name, comment.ID, attachment.ID) - resp := session.MakeRequest(t, req, http.StatusOK) + session.MakeRequest(t, req, http.StatusOK) req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/issues/comments/%d/assets/%d?token=%s", repoOwner.Name, repo.Name, comment.ID, attachment.ID, token) - resp = session.MakeRequest(t, req, http.StatusOK) + resp := session.MakeRequest(t, req, http.StatusOK) var apiAttachment api.Attachment DecodeJSON(t, resp, &apiAttachment) From b0461f4b6f56bc9a6e62322eb671b49a3af17d43 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 12 Nov 2022 10:31:20 +0000 Subject: [PATCH 36/43] Add issue/comment read access check. --- models/issues/comment.go | 25 ++++---- routers/api/v1/repo/issue_attachment.go | 63 ++++++++++++------- .../api/v1/repo/issue_comment_attachment.go | 18 +++--- routers/web/repo/issue.go | 10 +-- 4 files changed, 69 insertions(+), 47 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 41603451ea29..c03af1a1144c 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -872,22 +872,21 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment fallthrough case CommentTypeReview: // Check attachments - if len(opts.Attachments) > 0 { - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err) - } + attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments) + if err != nil { + return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err) + } - for _, attachment := range attachments { - attachment.IssueID = opts.Issue.ID - attachment.CommentID = comment.ID - // No assign value could be 0, so ignore AllCols(). - if _, err = db.GetEngine(ctx).ID(attachment.ID).Update(attachment); err != nil { - return fmt.Errorf("update attachment [%d]: %v", attachment.ID, err) - } + for _, attachment := range attachments { + attachment.IssueID = opts.Issue.ID + attachment.CommentID = comment.ID + // No assign value could be 0, so ignore AllCols(). + if _, err = db.GetEngine(ctx).ID(attachment.ID).Update(attachment); err != nil { + return fmt.Errorf("update attachment [%d]: %v", attachment.ID, err) } - comment.Attachments = attachments } + + comment.Attachments = attachments case CommentTypeReopen, CommentTypeClose: if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil { return err diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 12a2e11959d3..de2e029f69c4 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -66,9 +66,8 @@ func GetIssueAttachment(ctx *context.APIContext) { // "404": // "$ref": "#/responses/error" - issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64("index")) - if err != nil { - ctx.NotFoundOrServerError("GetIssueByIndex", issues_model.IsErrIssueNotExist, err) + issue := getIssueFromContext(ctx) + if issue == nil { return } @@ -76,6 +75,7 @@ func GetIssueAttachment(ctx *context.APIContext) { if attach == nil { return } + ctx.JSON(http.StatusOK, convert.ToAttachment(attach)) } @@ -109,15 +109,16 @@ func ListIssueAttachments(ctx *context.APIContext) { // "404": // "$ref": "#/responses/error" - issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64("index")) - if err != nil { - ctx.NotFoundOrServerError("GetIssueByIndex", issues_model.IsErrIssueNotExist, err) + issue := getIssueFromContext(ctx) + if issue == nil { return } + if err := issue.LoadAttributes(ctx); err != nil { ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return } + ctx.JSON(http.StatusOK, convert.ToAPIIssue(issue).Attachments) } @@ -165,10 +166,8 @@ func CreateIssueAttachment(ctx *context.APIContext) { // "404": // "$ref": "#/responses/error" - // Check if issue exists and load issue - issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64("index")) - if err != nil { - ctx.NotFoundOrServerError("GetIssueByIndex", issues_model.IsErrIssueNotExist, err) + issue := getIssueFromContext(ctx) + if issue == nil { return } @@ -200,7 +199,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { return } - issue.Attachments = append(issue.Attachments, attach) + issue.Attachments = append(issue.Attachments, attachment) if err := issue_service.ChangeContent(issue, ctx.Doer, issue.Content); err != nil { ctx.Error(http.StatusInternalServerError, "ChangeContent", err) @@ -252,19 +251,21 @@ func EditIssueAttachment(ctx *context.APIContext) { // "404": // "$ref": "#/responses/error" - // get attachment and check permissions attachment := getIssueAttachmentSafeWrite(ctx) if attachment == nil { return } + // do changes to attachment. only meaningful change is name. form := web.GetForm(ctx).(*api.EditAttachmentOptions) if form.Name != "" { - attach.Name = form.Name + attachment.Name = form.Name } + if err := repo_model.UpdateAttachment(ctx, attachment); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateAttachment", err) } + ctx.JSON(http.StatusCreated, convert.ToAttachment(attachment)) } @@ -308,19 +309,38 @@ func DeleteIssueAttachment(ctx *context.APIContext) { if attachment == nil { return } + if err := repo_model.DeleteAttachment(attachment, true); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err) return } + ctx.Status(http.StatusNoContent) } -func getIssueAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Attachment { +func getIssueFromContext(ctx *context.APIContext) *issues_model.Issue { issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64("index")) if err != nil { ctx.NotFoundOrServerError("GetIssueByIndex", issues_model.IsErrIssueNotExist, err) return nil } + + issue.Repo = ctx.Repo.Repository + + if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) { + ctx.Error(http.StatusForbidden, "", "user should have permission to read issue") + return nil + } + + return issue +} + +func getIssueAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Attachment { + issue := getIssueFromContext(ctx) + if issue == nil { + return nil + } + if !canUserWriteIssueAttachment(ctx, issue) { return nil } @@ -340,31 +360,30 @@ func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *issues_model.Iss return attachment } -func canUserWriteIssueAttachment(ctx *context.APIContext, issue *issues_model.Issue) (success bool) { +func canUserWriteIssueAttachment(ctx *context.APIContext, issue *issues_model.Issue) bool { canEditIssue := ctx.IsSigned && (ctx.Doer.ID == issue.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin()) && ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) if !canEditIssue { - ctx.Error(http.StatusForbidden, "IssueEditPerm", "user should have permission to edit issue") - return + ctx.Error(http.StatusForbidden, "", "user should have permission to write issue") + return false } return true } -func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, attachement *repo_model.Attachment, issue *issues_model.Issue) (success bool) { +func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, attachement *repo_model.Attachment, issue *issues_model.Issue) bool { if attachement.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", attachement.ID, ctx.Repo.Repository) ctx.NotFound("no such attachment in repo") - return + return false } if attachement.IssueID == 0 { - // catch people trying to get release assets ;) log.Debug("Requested attachment[%d] is not in an issue.", attachement.ID) ctx.NotFound("no such attachment in issue") - return + return false } else if issue != nil && attachement.IssueID != issue.ID { log.Debug("Requested attachment[%d] does not belong to issue[%d, #%d].", attachement.ID, issue.ID, issue.Index) ctx.NotFound("no such attachment in issue") - return + return false } return true } diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 51c2ded0e6fb..b2a796e504d5 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -60,7 +60,7 @@ func GetIssueCommentAttachment(ctx *context.APIContext) { return } attachment := getIssueCommentAttachmentSafeRead(ctx, comment) - if attach == nil { + if attachment == nil { return } if attachment.CommentID != comment.ID { @@ -325,6 +325,10 @@ func getIssueCommentSafe(ctx *context.APIContext) *issues_model.Comment { ctx.Error(http.StatusNotFound, "", "no matching issue comment found") return nil } + if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) { + ctx.Error(http.StatusForbidden, "", "user should have permission to read issue") + return nil + } return comment } @@ -339,11 +343,11 @@ func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Att return getIssueCommentAttachmentSafeRead(ctx, comment) } -func canUserWriteIssueCommentAttachment(ctx *context.APIContext, comment *issues_model.Comment) (success bool) { +func canUserWriteIssueCommentAttachment(ctx *context.APIContext, comment *issues_model.Comment) bool { canEditComment := ctx.IsSigned && (ctx.Doer.ID == comment.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin()) && ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull) if !canEditComment { ctx.Error(http.StatusForbidden, "", "user should have permission to edit comment") - return + return false } return true @@ -361,21 +365,21 @@ func getIssueCommentAttachmentSafeRead(ctx *context.APIContext, comment *issues_ return attachment } -func attachmentBelongsToRepoOrComment(ctx *context.APIContext, attachment *repo_model.Attachment, comment *issues_model.Comment) (success bool) { +func attachmentBelongsToRepoOrComment(ctx *context.APIContext, attachment *repo_model.Attachment, comment *issues_model.Comment) bool { if attachment.RepoID != ctx.Repo.Repository.ID { log.Debug("Requested attachment[%d] does not belong to repo[%-v].", attachment.ID, ctx.Repo.Repository) ctx.NotFound("no such attachment in repo") - return + return false } if attachment.IssueID == 0 || attachment.CommentID == 0 { log.Debug("Requested attachment[%d] is not in a comment.", attachment.ID) ctx.NotFound("no such attachment in comment") - return + return false } if comment != nil && attachment.CommentID != comment.ID { log.Debug("Requested attachment[%d] does not belong to comment[%d].", attachment.ID, comment.ID) ctx.NotFound("no such attachment in comment") - return + return false } return true } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index ce2371ce5d91..4ecbb6e4f66d 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2747,6 +2747,11 @@ func UpdateCommentContent(ctx *context.Context) { return } + if err = comment_service.UpdateComment(comment, ctx.Doer, oldContent); err != nil { + ctx.ServerError("UpdateComment", err) + return + } + if err := comment.LoadAttachments(); err != nil { ctx.ServerError("LoadAttachments", err) return @@ -2760,11 +2765,6 @@ func UpdateCommentContent(ctx *context.Context) { } } - if err = comment_service.UpdateComment(comment, ctx.Doer, oldContent); err != nil { - ctx.ServerError("UpdateComment", err) - return - } - content, err := markdown.RenderString(&markup.RenderContext{ URLPrefix: ctx.FormString("context"), // FIXME: <- IS THIS SAFE ? Metas: ctx.Repo.Repository.ComposeMetas(), From 27e324af348d7a93e79e1d77d77461f607256987 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 12 Nov 2022 10:34:23 +0000 Subject: [PATCH 37/43] Fix error log. --- models/issues/comment.go | 10 +++++----- modules/notification/webhook/webhook.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index c03af1a1144c..390eaa5c2596 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -877,12 +877,12 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", opts.Attachments, err) } - for _, attachment := range attachments { - attachment.IssueID = opts.Issue.ID - attachment.CommentID = comment.ID + for i := range attachments { + attachments[i].IssueID = opts.Issue.ID + attachments[i].CommentID = comment.ID // No assign value could be 0, so ignore AllCols(). - if _, err = db.GetEngine(ctx).ID(attachment.ID).Update(attachment); err != nil { - return fmt.Errorf("update attachment [%d]: %v", attachment.ID, err) + if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil { + return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err) } } diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 76b0727b6fca..7331d4ea2111 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -339,7 +339,7 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *user_model.User, issue defer finished() if err := issue.LoadRepo(ctx); err != nil { - log.Error("NotifyIssueChangeContent: coulnd't load repo", err) + log.Error("LoadRepo: %v", err) return } From beee20bb06bdf90db9654123831792b8e760148a Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 12 Nov 2022 12:57:13 +0000 Subject: [PATCH 38/43] lint --- routers/api/v1/repo/issue_attachment.go | 16 ---------------- routers/api/v1/repo/issue_comment_attachment.go | 7 +++---- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index de2e029f69c4..ce3881fea667 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -19,17 +19,6 @@ import ( issue_service "code.gitea.io/gitea/services/issue" ) -/** - * NOTE about permissions: - * - repo access is already checked via middleware on the /repos/{owner}/{name} group - * - issue/pull *read* access is checked on the ../issues group middleware - * ("read" access allows posting issues, so posting attachments is fine too!) - * - setting.Attachment.Enabled is checked on ../assets group middleware - * All that is left to be checked is - * - canUserWriteIssueAttachment() - * - attachmentBelongsToIssue() - */ - // GetIssueAttachment gets a single attachment of the issue func GetIssueAttachment(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueGetIssueAttachment @@ -327,11 +316,6 @@ func getIssueFromContext(ctx *context.APIContext) *issues_model.Issue { issue.Repo = ctx.Repo.Repository - if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) { - ctx.Error(http.StatusForbidden, "", "user should have permission to read issue") - return nil - } - return issue } diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index b2a796e504d5..b30b8f689316 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -325,10 +325,9 @@ func getIssueCommentSafe(ctx *context.APIContext) *issues_model.Comment { ctx.Error(http.StatusNotFound, "", "no matching issue comment found") return nil } - if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) { - ctx.Error(http.StatusForbidden, "", "user should have permission to read issue") - return nil - } + + comment.Issue.Repo = ctx.Repo.Repository + return comment } From cf93210bc429e838014154b46cb8892b4eff8451 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Dec 2022 09:29:28 +0000 Subject: [PATCH 39/43] placate spdx Signed-off-by: Andrew Thornton --- modules/convert/attachment.go | 3 +-- routers/api/v1/repo/issue_comment_attachment.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/convert/attachment.go b/modules/convert/attachment.go index 9b82e9da8fde..3bf67549c4db 100644 --- a/modules/convert/attachment.go +++ b/modules/convert/attachment.go @@ -1,6 +1,5 @@ // Copyright 2021 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. +// SPDX-License-Identifier: MIT package convert diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 81fa123a4753..a39eff4cb01d 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -1,6 +1,5 @@ // Copyright 2021 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. +// SPDX-License-Identifier: MIT package repo From 5687791ab6f4e5555bb3631b109a31189b0fd21a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Dec 2022 09:37:18 +0000 Subject: [PATCH 40/43] reduce duplication and remove old ToIssueAttachment Signed-off-by: Andrew Thornton --- modules/convert/attachment.go | 8 ++++++++ modules/convert/issue.go | 20 +------------------- modules/convert/issue_comment.go | 6 +----- modules/convert/release.go | 6 +----- 4 files changed, 11 insertions(+), 29 deletions(-) diff --git a/modules/convert/attachment.go b/modules/convert/attachment.go index 3bf67549c4db..ddba181a1204 100644 --- a/modules/convert/attachment.go +++ b/modules/convert/attachment.go @@ -20,3 +20,11 @@ func ToAttachment(a *repo_model.Attachment) *api.Attachment { DownloadURL: a.DownloadURL(), } } + +func ToAttachments(attachments []*repo_model.Attachment) []*api.Attachment { + converted := make([]*api.Attachment, 0, len(attachments)) + for _, attachment := range attachments { + converted = append(converted, ToAttachment(attachment)) + } + return converted +} diff --git a/modules/convert/issue.go b/modules/convert/issue.go index 5b51e8b03a23..45323b4ce44d 100644 --- a/modules/convert/issue.go +++ b/modules/convert/issue.go @@ -36,11 +36,6 @@ func ToAPIIssue(ctx context.Context, issue *issues_model.Issue) *api.Issue { return &api.Issue{} } - attachments := make([]*api.Attachment, 0, len(issue.Attachments)) - for _, att := range issue.Attachments { - attachments = append(attachments, ToIssueAttachment(att)) - } - apiIssue := &api.Issue{ ID: issue.ID, URL: issue.APIURL(), @@ -49,7 +44,7 @@ func ToAPIIssue(ctx context.Context, issue *issues_model.Issue) *api.Issue { Poster: ToUser(issue.Poster, nil), Title: issue.Title, Body: issue.Content, - Attachments: attachments, + Attachments: ToAttachments(issue.Attachments), Ref: issue.Ref, Labels: ToLabelList(issue.Labels, issue.Repo, issue.Repo.Owner), State: issue.State(), @@ -239,16 +234,3 @@ func ToAPIMilestone(m *issues_model.Milestone) *api.Milestone { } return apiMilestone } - -// ToIssueAttachment converts issues_model.Attachment to api.Attachment -func ToIssueAttachment(a *repo_model.Attachment) *api.Attachment { - return &api.Attachment{ - ID: a.ID, - Name: a.Name, - Created: a.CreatedUnix.AsTime(), - DownloadCount: a.DownloadCount, - Size: a.Size, - UUID: a.UUID, - DownloadURL: a.DownloadURL(), - } -} diff --git a/modules/convert/issue_comment.go b/modules/convert/issue_comment.go index 3f8c48376222..983354438ae4 100644 --- a/modules/convert/issue_comment.go +++ b/modules/convert/issue_comment.go @@ -15,10 +15,6 @@ import ( // ToComment converts a issues_model.Comment to the api.Comment format func ToComment(c *issues_model.Comment) *api.Comment { - attachments := make([]*api.Attachment, 0, len(c.Attachments)) - for _, attachment := range c.Attachments { - attachments = append(attachments, ToAttachment(attachment)) - } return &api.Comment{ ID: c.ID, Poster: ToUser(c.Poster, nil), @@ -26,7 +22,7 @@ func ToComment(c *issues_model.Comment) *api.Comment { IssueURL: c.IssueURL(), PRURL: c.PRURL(), Body: c.Content, - Attachments: attachments, + Attachments: ToAttachments(c.Attachments), Created: c.CreatedUnix.AsTime(), Updated: c.UpdatedUnix.AsTime(), } diff --git a/modules/convert/release.go b/modules/convert/release.go index 2ee49bac3216..3afa53c03f5f 100644 --- a/modules/convert/release.go +++ b/modules/convert/release.go @@ -10,10 +10,6 @@ import ( // ToRelease convert a repo_model.Release to api.Release func ToRelease(r *repo_model.Release) *api.Release { - attachments := make([]*api.Attachment, 0, len(r.Attachments)) - for _, attachment := range r.Attachments { - attachments = append(attachments, ToAttachment(attachment)) - } return &api.Release{ ID: r.ID, TagName: r.TagName, @@ -29,6 +25,6 @@ func ToRelease(r *repo_model.Release) *api.Release { CreatedAt: r.CreatedUnix.AsTime(), PublishedAt: r.CreatedUnix.AsTime(), Publisher: ToUser(r.Publisher, nil), - Attachments: attachments, + Attachments: ToAttachments(r.Attachments), } } From 8848e0b13bd1abab2c50dedc7b66dc3fb6a386df Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Dec 2022 09:45:30 +0000 Subject: [PATCH 41/43] attachment not attachEment Signed-off-by: Andrew Thornton --- models/migrations/v1_10/v96.go | 12 ++++++------ routers/api/v1/repo/issue_attachment.go | 17 ++++++++--------- routers/web/repo/attachment.go | 2 +- services/release/release.go | 4 ++-- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/models/migrations/v1_10/v96.go b/models/migrations/v1_10/v96.go index 2abd260be41a..422defe8387a 100644 --- a/models/migrations/v1_10/v96.go +++ b/models/migrations/v1_10/v96.go @@ -30,19 +30,19 @@ func DeleteOrphanedAttachments(x *xorm.Engine) error { } for { - attachements := make([]Attachment, 0, limit) + attachments := make([]Attachment, 0, limit) if err := sess.Where("`issue_id` = 0 and (`release_id` = 0 or `release_id` not in (select `id` from `release`))"). Cols("id, uuid").Limit(limit). Asc("id"). - Find(&attachements); err != nil { + Find(&attachments); err != nil { return err } - if len(attachements) == 0 { + if len(attachments) == 0 { return nil } ids := make([]int64, 0, limit) - for _, attachment := range attachements { + for _, attachment := range attachments { ids = append(ids, attachment.ID) } if len(ids) > 0 { @@ -51,13 +51,13 @@ func DeleteOrphanedAttachments(x *xorm.Engine) error { } } - for _, attachment := range attachements { + for _, attachment := range attachments { uuid := attachment.UUID if err := util.RemoveAll(filepath.Join(setting.Attachment.Path, uuid[0:1], uuid[1:2], uuid)); err != nil { return err } } - if len(attachements) < limit { + if len(attachments) < limit { return nil } } diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index d80c31cb0b68..4cf108b41378 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -1,6 +1,5 @@ // Copyright 2021 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. +// SPDX-License-Identifier: MIT package repo @@ -354,18 +353,18 @@ func canUserWriteIssueAttachment(ctx *context.APIContext, issue *issues_model.Is return true } -func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, attachement *repo_model.Attachment, issue *issues_model.Issue) bool { - if attachement.RepoID != ctx.Repo.Repository.ID { - log.Debug("Requested attachment[%d] does not belong to repo[%-v].", attachement.ID, ctx.Repo.Repository) +func attachmentBelongsToRepoOrIssue(ctx *context.APIContext, attachment *repo_model.Attachment, issue *issues_model.Issue) bool { + if attachment.RepoID != ctx.Repo.Repository.ID { + log.Debug("Requested attachment[%d] does not belong to repo[%-v].", attachment.ID, ctx.Repo.Repository) ctx.NotFound("no such attachment in repo") return false } - if attachement.IssueID == 0 { - log.Debug("Requested attachment[%d] is not in an issue.", attachement.ID) + if attachment.IssueID == 0 { + log.Debug("Requested attachment[%d] is not in an issue.", attachment.ID) ctx.NotFound("no such attachment in issue") return false - } else if issue != nil && attachement.IssueID != issue.ID { - log.Debug("Requested attachment[%d] does not belong to issue[%d, #%d].", attachement.ID, issue.ID, issue.Index) + } else if issue != nil && attachment.IssueID != issue.ID { + log.Debug("Requested attachment[%d] does not belong to issue[%d, #%d].", attachment.ID, issue.ID, issue.Index) ctx.NotFound("no such attachment in issue") return false } diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index fb498e89b2a1..589632ad6e10 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -86,7 +86,7 @@ func DeleteAttachment(ctx *context.Context) { }) } -// GetAttachment serve attachements +// GetAttachment serve attachments func GetAttachment(ctx *context.Context) { attach, err := repo_model.GetAttachmentByUUID(ctx, ctx.Params(":uuid")) if err != nil { diff --git a/services/release/release.go b/services/release/release.go index 1d599fcda1d5..dfce97776bf8 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -218,7 +218,7 @@ func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *repo_mod } for _, attach := range attachments { if attach.ReleaseID != rel.ID { - return errors.New("delete attachement of release permission denied") + return errors.New("delete attachment of release permission denied") } deletedUUIDs.Add(attach.UUID) } @@ -240,7 +240,7 @@ func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *repo_mod } for _, attach := range attachments { if attach.ReleaseID != rel.ID { - return errors.New("update attachement of release permission denied") + return errors.New("update attachment of release permission denied") } } From 5438207e4eac82722d40a4665bc2960f30597bd9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Dec 2022 10:20:27 +0000 Subject: [PATCH 42/43] use ToAttachments instead Signed-off-by: Andrew Thornton --- routers/api/v1/repo/issue_comment_attachment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index a39eff4cb01d..60ea8d1b832e 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -110,7 +110,7 @@ func ListIssueCommentAttachments(ctx *context.APIContext) { return } - ctx.JSON(http.StatusOK, convert.ToComment(comment).Attachments) + ctx.JSON(http.StatusOK, convert.ToAttachments(comment.Attachments)) } // CreateIssueCommentAttachment creates an attachment and saves the given file From 4f5e61c0145fca9ef62a9e641ede7ae0bcd370bd Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Dec 2022 10:24:32 +0000 Subject: [PATCH 43/43] wrap errors too Signed-off-by: Andrew Thornton --- services/release/release.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/services/release/release.go b/services/release/release.go index dfce97776bf8..13042cd3ac2e 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" ) func createTag(ctx context.Context, gitRepo *git.Repository, rel *repo_model.Release, msg string) (bool, error) { @@ -218,7 +219,10 @@ func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *repo_mod } for _, attach := range attachments { if attach.ReleaseID != rel.ID { - return errors.New("delete attachment of release permission denied") + return util.SilentWrap{ + Message: "delete attachment of release permission denied", + Err: util.ErrPermissionDenied, + } } deletedUUIDs.Add(attach.UUID) } @@ -240,7 +244,10 @@ func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *repo_mod } for _, attach := range attachments { if attach.ReleaseID != rel.ID { - return errors.New("update attachment of release permission denied") + return util.SilentWrap{ + Message: "update attachment of release permission denied", + Err: util.ErrPermissionDenied, + } } }