Add API management for issue/pull and comment attachments (#21783)

Close #14601
Fix #3690

Revive of #14601.
Updated to current code, cleanup and added more read/write checks.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andre Bruch <ab@andrebruch.com>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: Norwin <git@nroo.de>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

routers/api/v1/api.go

@@ -567,6 +567,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 {
 	tp := reflect.TypeOf(obj)
@@ -892,6 +899,15 @@ func Routes(ctx gocontext.Context) *web.Route {
 								Get(repo.GetIssueCommentReactions).
 								Post(reqToken(), bind(api.EditReactionOption{}), repo.PostIssueCommentReaction).
 								Delete(reqToken(), bind(api.EditReactionOption{}), repo.DeleteIssueCommentReaction)
+							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)
 						})
 					})
 					m.Group("/{index}", func() {
@@ -935,6 +951,15 @@ func Routes(ctx gocontext.Context) *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(), mustNotBeArchived, repo.CreateIssueAttachment)
+							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() {

routers/api/v1/repo/issue_attachment.go

@@ -0,0 +1,372 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package repo
+
+import (
+	"net/http"
+
+	issues_model "code.gitea.io/gitea/models/issues"
+	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"
+	"code.gitea.io/gitea/modules/setting"
+	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/services/attachment"
+	issue_service "code.gitea.io/gitea/services/issue"
+)
+
+// 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
+	// ---
+	// summary: Get an 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: 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
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// responses:
+	//   "200":
+	//     "$ref": "#/responses/Attachment"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	issue := getIssueFromContext(ctx)
+	if issue == nil {
+		return
+	}
+
+	attach := getIssueAttachmentSafeRead(ctx, issue)
+	if attach == nil {
+		return
+	}
+
+	ctx.JSON(http.StatusOK, convert.ToAttachment(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: index of the issue
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// responses:
+	//   "200":
+	//     "$ref": "#/responses/AttachmentList"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	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(ctx, 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 an 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: index 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"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	issue := getIssueFromContext(ctx)
+	if issue == nil {
+		return
+	}
+
+	if !canUserWriteIssueAttachment(ctx, issue) {
+		return
+	}
+
+	// Get uploaded file from request
+	file, header, err := ctx.Req.FormFile("attachment")
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "FormFile", err)
+		return
+	}
+	defer file.Close()
+
+	filename := header.Filename
+	if query := ctx.FormString("name"); query != "" {
+		filename = query
+	}
+
+	attachment, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{
+		Name:       filename,
+		UploaderID: ctx.Doer.ID,
+		RepoID:     ctx.Repo.Repository.ID,
+		IssueID:    issue.ID,
+	})
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "UploadAttachment", err)
+		return
+	}
+
+	issue.Attachments = append(issue.Attachments, attachment)
+
+	if err := issue_service.ChangeContent(issue, ctx.Doer, issue.Content); err != nil {
+		ctx.Error(http.StatusInternalServerError, "ChangeContent", err)
+		return
+	}
+
+	ctx.JSON(http.StatusCreated, convert.ToAttachment(attachment))
+}
+
+// EditIssueAttachment updates the given attachment
+func EditIssueAttachment(ctx *context.APIContext) {
+	// swagger:operation PATCH /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueEditIssueAttachment
+	// ---
+	// summary: Edit an 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: 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
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// - name: body
+	//   in: body
+	//   schema:
+	//     "$ref": "#/definitions/EditAttachmentOptions"
+	// responses:
+	//   "201":
+	//     "$ref": "#/responses/Attachment"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	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 != "" {
+		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))
+}
+
+// DeleteIssueAttachment delete a given attachment
+func DeleteIssueAttachment(ctx *context.APIContext) {
+	// swagger:operation DELETE /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} issue issueDeleteIssueAttachment
+	// ---
+	// summary: Delete an 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: 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
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// responses:
+	//   "204":
+	//     "$ref": "#/responses/empty"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	attachment := getIssueAttachmentSafeWrite(ctx)
+	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 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
+
+	return issue
+}
+
+func getIssueAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Attachment {
+	issue := getIssueFromContext(ctx)
+	if issue == nil {
+		return nil
+	}
+
+	if !canUserWriteIssueAttachment(ctx, issue) {
+		return nil
+	}
+
+	return getIssueAttachmentSafeRead(ctx, issue)
+}
+
+func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *issues_model.Issue) *repo_model.Attachment {
+	attachment, err := repo_model.GetAttachmentByID(ctx, ctx.ParamsInt64("asset"))
+	if err != nil {
+		ctx.NotFoundOrServerError("GetAttachmentByID", repo_model.IsErrAttachmentNotExist, err)
+		return nil
+	}
+	if !attachmentBelongsToRepoOrIssue(ctx, attachment, issue) {
+		return nil
+	}
+	return attachment
+}
+
+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, "", "user should have permission to write issue")
+		return false
+	}
+
+	return true
+}
+
+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 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 && 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
+	}
+	return true
+}

routers/api/v1/repo/issue_comment.go

@@ -95,6 +95,11 @@ func ListIssueComments(ctx *context.APIContext) {
 		return
 	}
 
+	if err := issues_model.CommentList(comments).LoadAttachments(ctx); err != nil {
+		ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
+		return
+	}
+
 	apiComments := make([]*api.Comment, len(comments))
 	for i, comment := range comments {
 		comment.Issue = issue
@@ -294,6 +299,10 @@ func ListRepoIssueComments(ctx *context.APIContext) {
 		ctx.Error(http.StatusInternalServerError, "LoadPosters", err)
 		return
 	}
+	if err := issues_model.CommentList(comments).LoadAttachments(ctx); err != nil {
+		ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
+		return
+	}
 	if _, err := issues_model.CommentList(comments).Issues().LoadRepositories(ctx); err != nil {
 		ctx.Error(http.StatusInternalServerError, "LoadRepositories", err)
 		return

routers/api/v1/repo/issue_comment_attachment.go

@@ -0,0 +1,383 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package repo
+
+import (
+	"net/http"
+
+	issues_model "code.gitea.io/gitea/models/issues"
+	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"
+	"code.gitea.io/gitea/modules/setting"
+	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/services/attachment"
+	comment_service "code.gitea.io/gitea/services/comments"
+)
+
+// GetIssueCommentAttachment gets a single attachment of the comment
+func GetIssueCommentAttachment(ctx *context.APIContext) {
+	// swagger:operation GET /repos/{owner}/{repo}/issues/comments/{id}/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: 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
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// responses:
+	//   "200":
+	//     "$ref": "#/responses/Attachment"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	comment := getIssueCommentSafe(ctx)
+	if comment == nil {
+		return
+	}
+	attachment := getIssueCommentAttachmentSafeRead(ctx, comment)
+	if attachment == nil {
+		return
+	}
+	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(attachment))
+}
+
+// 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"
+	//   "404":
+	//     "$ref": "#/responses/error"
+	comment := getIssueCommentSafe(ctx)
+	if comment == nil {
+		return
+	}
+
+	if err := comment.LoadAttachments(ctx); err != nil {
+		ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
+		return
+	}
+
+	ctx.JSON(http.StatusOK, convert.ToAttachments(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"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	// Check if comment exists and load comment
+	comment := getIssueCommentSafe(ctx)
+	if comment == nil {
+		return
+	}
+
+	if !canUserWriteIssueCommentAttachment(ctx, comment) {
+		return
+	}
+
+	// Get uploaded file from request
+	file, header, err := ctx.Req.FormFile("attachment")
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "FormFile", err)
+		return
+	}
+	defer file.Close()
+
+	filename := header.Filename
+	if query := ctx.FormString("name"); query != "" {
+		filename = query
+	}
+
+	attachment, err := attachment.UploadAttachment(file, setting.Attachment.AllowedTypes, &repo_model.Attachment{
+		Name:       filename,
+		UploaderID: ctx.Doer.ID,
+		RepoID:     ctx.Repo.Repository.ID,
+		IssueID:    comment.IssueID,
+		CommentID:  comment.ID,
+	})
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "UploadAttachment", err)
+		return
+	}
+	if err := comment.LoadAttachments(ctx); err != nil {
+		ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
+		return
+	}
+
+	if err = comment_service.UpdateComment(ctx, comment, ctx.Doer, comment.Content); err != nil {
+		ctx.ServerError("UpdateComment", err)
+		return
+	}
+
+	ctx.JSON(http.StatusCreated, convert.ToAttachment(attachment))
+}
+
+// EditIssueCommentAttachment updates the given attachment
+func EditIssueCommentAttachment(ctx *context.APIContext) {
+	// swagger:operation PATCH /repos/{owner}/{repo}/issues/comments/{id}/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: 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
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// - name: body
+	//   in: body
+	//   schema:
+	//     "$ref": "#/definitions/EditAttachmentOptions"
+	// responses:
+	//   "201":
+	//     "$ref": "#/responses/Attachment"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	attach := getIssueCommentAttachmentSafeWrite(ctx)
+	if attach == nil {
+		return
+	}
+
+	form := web.GetForm(ctx).(*api.EditAttachmentOptions)
+	if form.Name != "" {
+		attach.Name = form.Name
+	}
+
+	if err := repo_model.UpdateAttachment(ctx, attach); err != nil {
+		ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach)
+	}
+	ctx.JSON(http.StatusCreated, convert.ToAttachment(attach))
+}
+
+// DeleteIssueCommentAttachment delete a given attachment
+func DeleteIssueCommentAttachment(ctx *context.APIContext) {
+	// swagger:operation DELETE /repos/{owner}/{repo}/issues/comments/{id}/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: 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
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// responses:
+	//   "204":
+	//     "$ref": "#/responses/empty"
+	//   "404":
+	//     "$ref": "#/responses/error"
+
+	attach := getIssueCommentAttachmentSafeWrite(ctx)
+	if attach == nil {
+		return
+	}
+
+	if err := repo_model.DeleteAttachment(attach, true); err != nil {
+		ctx.Error(http.StatusInternalServerError, "DeleteAttachment", err)
+		return
+	}
+	ctx.Status(http.StatusNoContent)
+}
+
+func getIssueCommentSafe(ctx *context.APIContext) *issues_model.Comment {
+	comment, err := issues_model.GetCommentByID(ctx, ctx.ParamsInt64("id"))
+	if err != nil {
+		ctx.NotFoundOrServerError("GetCommentByID", issues_model.IsErrCommentNotExist, err)
+		return nil
+	}
+	if err := comment.LoadIssue(ctx); 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")
+		return nil
+	}
+
+	comment.Issue.Repo = ctx.Repo.Repository
+
+	return comment
+}
+
+func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Attachment {
+	comment := getIssueCommentSafe(ctx)
+	if comment == nil {
+		return nil
+	}
+	if !canUserWriteIssueCommentAttachment(ctx, comment) {
+		return nil
+	}
+	return getIssueCommentAttachmentSafeRead(ctx, comment)
+}
+
+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 false
+	}
+
+	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 {
+		ctx.NotFoundOrServerError("GetAttachmentByID", repo_model.IsErrAttachmentNotExist, err)
+		return nil
+	}
+	if !attachmentBelongsToRepoOrComment(ctx, attachment, comment) {
+		return nil
+	}
+	return attachment
+}
+
+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 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 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 false
+	}
+	return true
+}

routers/api/v1/repo/release_attachment.go

@@ -68,7 +68,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
@@ -194,7 +194,12 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 	}
 
 	// Create a new attachment and save the file
-	attach, err := attachment.UploadAttachment(file, ctx.Doer.ID, release.RepoID, releaseID, filename, setting.Repository.Release.AllowedTypes)
+	attach, err := attachment.UploadAttachment(file, setting.Repository.Release.AllowedTypes, &repo_model.Attachment{
+		Name:       filename,
+		UploaderID: ctx.Doer.ID,
+		RepoID:     release.RepoID,
+		ReleaseID:  releaseID,
+	})
 	if err != nil {
 		if upload.IsErrFileTypeForbidden(err) {
 			ctx.Error(http.StatusBadRequest, "DetectContentType", err)
@@ -204,7 +209,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
@@ -274,7 +279,7 @@ func EditReleaseAttachment(ctx *context.APIContext) {
 	if err := repo_model.UpdateAttachment(ctx, 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

routers/web/repo/attachment.go

@@ -44,7 +44,11 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) {
 	}
 	defer file.Close()
 
-	attach, err := attachment.UploadAttachment(file, ctx.Doer.ID, repoID, 0, header.Filename, allowedTypes)
+	attach, err := attachment.UploadAttachment(file, allowedTypes, &repo_model.Attachment{
+		Name:       header.Filename,
+		UploaderID: ctx.Doer.ID,
+		RepoID:     repoID,
+	})
 	if err != nil {
 		if upload.IsErrFileTypeForbidden(err) {
 			ctx.Error(http.StatusBadRequest, err.Error())
@@ -82,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 {

routers/web/repo/issue.go

@@ -2749,6 +2749,7 @@ func UpdateCommentContent(ctx *context.Context) {
 		})
 		return
 	}
+
 	if err = comment_service.UpdateComment(ctx, comment, ctx.Doer, oldContent); err != nil {
 		ctx.ServerError("UpdateComment", err)
 		return
@@ -3050,7 +3051,7 @@ func GetIssueAttachments(ctx *context.Context) {
 	issue := GetActionIssue(ctx)
 	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)
 }
@@ -3069,7 +3070,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)