Fix comment permissions (#28213)

This PR will fix some missed checks for private repositories' data on
web routes and API routes.

diff --git a/models/asymkey/gpg_key.go b/models/asymkey/gpg_key.go
index 1da7c346def58cf18301d8c409ef8cd35c16755e..421f24d4de9394ec56b3b8cc23007023ab35cd9d 100644 (file)
--- a/models/asymkey/gpg_key.go
+++ b/models/asymkey/gpg_key.go
@@ -92,10 +92,9 @@ func CountUserGPGKeys(ctx context.Context, userID int64) (int64, error) {
        return db.GetEngine(ctx).Where("owner_id=? AND primary_key_id=''", userID).Count(&GPGKey{})
 }
 
-// GetGPGKeyByID returns public key by given ID.
-func GetGPGKeyByID(ctx context.Context, keyID int64) (*GPGKey, error) {
+func GetGPGKeyForUserByID(ctx context.Context, ownerID, keyID int64) (*GPGKey, error) {
        key := new(GPGKey)
-       has, err := db.GetEngine(ctx).ID(keyID).Get(key)
+       has, err := db.GetEngine(ctx).Where("id=? AND owner_id=?", keyID, ownerID).Get(key)
        if err != nil {
                return nil, err
        } else if !has {
@@ -225,7 +224,7 @@ func deleteGPGKey(ctx context.Context, keyID string) (int64, error) {
 
 // DeleteGPGKey deletes GPG key information in database.
 func DeleteGPGKey(ctx context.Context, doer *user_model.User, id int64) (err error) {
-       key, err := GetGPGKeyByID(ctx, id)
+       key, err := GetGPGKeyForUserByID(ctx, doer.ID, id)
        if err != nil {
                if IsErrGPGKeyNotExist(err) {
                        return nil
@@ -233,11 +232,6 @@ func DeleteGPGKey(ctx context.Context, doer *user_model.User, id int64) (err err
                return fmt.Errorf("GetPublicKeyByID: %w", err)
        }
 
-       // Check if user has access to delete this key.
-       if !doer.IsAdmin && doer.ID != key.OwnerID {
-               return ErrGPGKeyAccessDenied{doer.ID, key.ID}
-       }
-
        ctx, committer, err := db.TxContext(ctx)
        if err != nil {
                return err

diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml
index bd64680c8c7872800caaa33c1252a4fa9a6cb38e..17586caa2191f96473785d668ff59708f99c92bb 100644 (file)
--- a/models/fixtures/comment.yml
+++ b/models/fixtures/comment.yml
@@ -66,3 +66,12 @@
   tree_path: "README.md"
   created_unix: 946684812
   invalidated: true
+
+-
+  id: 8
+  type: 0 # comment
+  poster_id: 2
+  issue_id: 4 # in repo_id 2
+  content: "comment in private pository"
+  created_unix: 946684811
+  updated_unix: 946684811

diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml
index ccc1fe41fbd9761715c9599b3c15b050060ae77f..0c9b6ff4060fe0dc482bc8707db20ae1f29029fb 100644 (file)
--- a/models/fixtures/issue.yml
+++ b/models/fixtures/issue.yml
@@ -61,7 +61,7 @@
   priority: 0
   is_closed: true
   is_pull: false
-  num_comments: 0
+  num_comments: 1
   created_unix: 946684830
   updated_unix: 978307200
   is_locked: false

diff --git a/models/issues/comment.go b/models/issues/comment.go
index a59fa570af95dda110cfc8ac5f6642c56d0111b2..ba5aed9c652e9e82fe148f7d10055e3e548f9a2b 100644 (file)
--- a/models/issues/comment.go
+++ b/models/issues/comment.go
@@ -1024,6 +1024,7 @@ type FindCommentsOptions struct {
        Type        CommentType
        IssueIDs    []int64
        Invalidated util.OptionalBool
+       IsPull      util.OptionalBool
 }
 
 // ToConds implements FindOptions interface
@@ -1058,6 +1059,9 @@ func (opts FindCommentsOptions) ToConds() builder.Cond {
        if !opts.Invalidated.IsNone() {
                cond = cond.And(builder.Eq{"comment.invalidated": opts.Invalidated.IsTrue()})
        }
+       if opts.IsPull != util.OptionalBoolNone {
+               cond = cond.And(builder.Eq{"issue.is_pull": opts.IsPull.IsTrue()})
+       }
        return cond
 }
 
@@ -1065,7 +1069,7 @@ func (opts FindCommentsOptions) ToConds() builder.Cond {
 func FindComments(ctx context.Context, opts *FindCommentsOptions) (CommentList, error) {
        comments := make([]*Comment, 0, 10)
        sess := db.GetEngine(ctx).Where(opts.ToConds())
-       if opts.RepoID > 0 {
+       if opts.RepoID > 0 || opts.IsPull != util.OptionalBoolNone {
                sess.Join("INNER", "issue", "issue.id = comment.issue_id")
        }
 

diff --git a/models/issues/content_history.go b/models/issues/content_history.go
index cc06b184d78097ec755c66bd5b7afd1b8d835a43..8c333bc6dd37c5e0589a8efee17645030bb85581 100644 (file)
--- a/models/issues/content_history.go
+++ b/models/issues/content_history.go
@@ -218,9 +218,9 @@ func GetIssueContentHistoryByID(dbCtx context.Context, id int64) (*ContentHistor
 }
 
 // GetIssueContentHistoryAndPrev get a history and the previous non-deleted history (to compare)
-func GetIssueContentHistoryAndPrev(dbCtx context.Context, id int64) (history, prevHistory *ContentHistory, err error) {
+func GetIssueContentHistoryAndPrev(dbCtx context.Context, issueID, id int64) (history, prevHistory *ContentHistory, err error) {
        history = &ContentHistory{}
-       has, err := db.GetEngine(dbCtx).ID(id).Get(history)
+       has, err := db.GetEngine(dbCtx).Where("id=? AND issue_id=?", id, issueID).Get(history)
        if err != nil {
                log.Error("failed to get issue content history %v. err=%v", id, err)
                return nil, nil, err

diff --git a/models/issues/content_history_test.go b/models/issues/content_history_test.go
index 53638e967f200750ac5bc1dbf10e00fb40cd7a32..0ea1d0f7b2e2032bbb91f74a37e3201ce46a2890 100644 (file)
--- a/models/issues/content_history_test.go
+++ b/models/issues/content_history_test.go
@@ -58,13 +58,13 @@ func TestContentHistory(t *testing.T) {
        hasHistory2, _ := issues_model.HasIssueContentHistory(dbCtx, 10, 1)
        assert.False(t, hasHistory2)
 
-       h6, h6Prev, _ := issues_model.GetIssueContentHistoryAndPrev(dbCtx, 6)
+       h6, h6Prev, _ := issues_model.GetIssueContentHistoryAndPrev(dbCtx, 10, 6)
        assert.EqualValues(t, 6, h6.ID)
        assert.EqualValues(t, 5, h6Prev.ID)
 
        // soft-delete
        _ = issues_model.SoftDeleteIssueContentHistory(dbCtx, 5)
-       h6, h6Prev, _ = issues_model.GetIssueContentHistoryAndPrev(dbCtx, 6)
+       h6, h6Prev, _ = issues_model.GetIssueContentHistoryAndPrev(dbCtx, 10, 6)
        assert.EqualValues(t, 6, h6.ID)
        assert.EqualValues(t, 4, h6Prev.ID)
 

diff --git a/models/project/project.go b/models/project/project.go
index becfcbea1e687084f5efccbbe3e11f865ac8a39e..d2fca6cdc8a8acafbc1709484d78772068d5ff97 100644 (file)
--- a/models/project/project.go
+++ b/models/project/project.go
@@ -294,6 +294,18 @@ func GetProjectByID(ctx context.Context, id int64) (*Project, error) {
        return p, nil
 }
 
+// GetProjectForRepoByID returns the projects in a repository
+func GetProjectForRepoByID(ctx context.Context, repoID, id int64) (*Project, error) {
+       p := new(Project)
+       has, err := db.GetEngine(ctx).Where("id=? AND repo_id=?", id, repoID).Get(p)
+       if err != nil {
+               return nil, err
+       } else if !has {
+               return nil, ErrProjectNotExist{ID: id}
+       }
+       return p, nil
+}
+
 // UpdateProject updates project properties
 func UpdateProject(ctx context.Context, p *Project) error {
        if !IsCardTypeValid(p.CardType) {

diff --git a/models/repo/release.go b/models/repo/release.go
index ff31ec451025e4649d6e00117ec2001c361bb901..223d3f250192275814f4f74471bda9d6220500b8 100644 (file)
--- a/models/repo/release.go
+++ b/models/repo/release.go
@@ -207,6 +207,21 @@ func GetReleaseByID(ctx context.Context, id int64) (*Release, error) {
        return rel, nil
 }
 
+// GetReleaseForRepoByID returns release with given ID.
+func GetReleaseForRepoByID(ctx context.Context, repoID, id int64) (*Release, error) {
+       rel := new(Release)
+       has, err := db.GetEngine(ctx).
+               Where("id=? AND repo_id=?", id, repoID).
+               Get(rel)
+       if err != nil {
+               return nil, err
+       } else if !has {
+               return nil, ErrReleaseNotExist{id, ""}
+       }
+
+       return rel, nil
+}
+
 // FindReleasesOptions describes the conditions to Find releases
 type FindReleasesOptions struct {
        db.ListOptions

diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go
index 408023507a5f70253ac4b8453910a9d120de029b..a72bd938aacbb8921ee8b939102011d33a9be349 100644 (file)
--- a/models/webhook/webhook.go
+++ b/models/webhook/webhook.go
@@ -392,39 +392,40 @@ func CreateWebhooks(ctx context.Context, ws []*Webhook) error {
        return db.Insert(ctx, ws)
 }
 
-// getWebhook uses argument bean as query condition,
-// ID must be specified and do not assign unnecessary fields.
-func getWebhook(ctx context.Context, bean *Webhook) (*Webhook, error) {
-       has, err := db.GetEngine(ctx).Get(bean)
+// GetWebhookByID returns webhook of repository by given ID.
+func GetWebhookByID(ctx context.Context, id int64) (*Webhook, error) {
+       bean := new(Webhook)
+       has, err := db.GetEngine(ctx).ID(id).Get(bean)
        if err != nil {
                return nil, err
        } else if !has {
-               return nil, ErrWebhookNotExist{ID: bean.ID}
+               return nil, ErrWebhookNotExist{ID: id}
        }
        return bean, nil
 }
 
-// GetWebhookByID returns webhook of repository by given ID.
-func GetWebhookByID(ctx context.Context, id int64) (*Webhook, error) {
-       return getWebhook(ctx, &Webhook{
-               ID: id,
-       })
-}
-
 // GetWebhookByRepoID returns webhook of repository by given ID.
 func GetWebhookByRepoID(ctx context.Context, repoID, id int64) (*Webhook, error) {
-       return getWebhook(ctx, &Webhook{
-               ID:     id,
-               RepoID: repoID,
-       })
+       webhook := new(Webhook)
+       has, err := db.GetEngine(ctx).Where("id=? AND repo_id=?", id, repoID).Get(webhook)
+       if err != nil {
+               return nil, err
+       } else if !has {
+               return nil, ErrWebhookNotExist{ID: id}
+       }
+       return webhook, nil
 }
 
 // GetWebhookByOwnerID returns webhook of a user or organization by given ID.
 func GetWebhookByOwnerID(ctx context.Context, ownerID, id int64) (*Webhook, error) {
-       return getWebhook(ctx, &Webhook{
-               ID:      id,
-               OwnerID: ownerID,
-       })
+       webhook := new(Webhook)
+       has, err := db.GetEngine(ctx).Where("id=? AND owner_id=?", id, ownerID).Get(webhook)
+       if err != nil {
+               return nil, err
+       } else if !has {
+               return nil, ErrWebhookNotExist{ID: id}
+       }
+       return webhook, nil
 }
 
 // ListWebhookOptions are options to filter webhooks on ListWebhooksByOpts
@@ -461,20 +462,20 @@ func UpdateWebhookLastStatus(ctx context.Context, w *Webhook) error {
        return err
 }
 
-// deleteWebhook uses argument bean as query condition,
+// DeleteWebhookByID uses argument bean as query condition,
 // ID must be specified and do not assign unnecessary fields.
-func deleteWebhook(ctx context.Context, bean *Webhook) (err error) {
+func DeleteWebhookByID(ctx context.Context, id int64) (err error) {
        ctx, committer, err := db.TxContext(ctx)
        if err != nil {
                return err
        }
        defer committer.Close()
 
-       if count, err := db.DeleteByBean(ctx, bean); err != nil {
+       if count, err := db.DeleteByID(ctx, id, new(Webhook)); err != nil {
                return err
        } else if count == 0 {
-               return ErrWebhookNotExist{ID: bean.ID}
-       } else if _, err = db.DeleteByBean(ctx, &HookTask{HookID: bean.ID}); err != nil {
+               return ErrWebhookNotExist{ID: id}
+       } else if _, err = db.DeleteByBean(ctx, &HookTask{HookID: id}); err != nil {
                return err
        }
 
@@ -483,16 +484,16 @@ func deleteWebhook(ctx context.Context, bean *Webhook) (err error) {
 
 // DeleteWebhookByRepoID deletes webhook of repository by given ID.
 func DeleteWebhookByRepoID(ctx context.Context, repoID, id int64) error {
-       return deleteWebhook(ctx, &Webhook{
-               ID:     id,
-               RepoID: repoID,
-       })
+       if _, err := GetWebhookByRepoID(ctx, repoID, id); err != nil {
+               return err
+       }
+       return DeleteWebhookByID(ctx, id)
 }
 
 // DeleteWebhookByOwnerID deletes webhook of a user or organization by given ID.
 func DeleteWebhookByOwnerID(ctx context.Context, ownerID, id int64) error {
-       return deleteWebhook(ctx, &Webhook{
-               ID:      id,
-               OwnerID: ownerID,
-       })
+       if _, err := GetWebhookByOwnerID(ctx, ownerID, id); err != nil {
+               return err
+       }
+       return DeleteWebhookByID(ctx, id)
 }

diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index 2a41619c3c772e67609759cebc33a223a613dff3..623c798feea68116cc1c1f6ac54dfae0168de7c2 100644 (file)
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -1259,8 +1259,8 @@ func Routes() *web.Route {
                        m.Group("/{username}/{reponame}", func() {
                                m.Group("/issues", func() {
                                        m.Combo("").Get(repo.ListIssues).
-                                               Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), repo.CreateIssue)
-                                       m.Get("/pinned", repo.ListPinnedIssues)
+                                               Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), reqRepoReader(unit.TypeIssues), repo.CreateIssue)
+                                       m.Get("/pinned", reqRepoReader(unit.TypeIssues), repo.ListPinnedIssues)
                                        m.Group("/comments", func() {
                                                m.Get("", repo.ListRepoIssueComments)
                                                m.Group("/{id}", func() {

diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go
index 74e6361f6c5d1e4ac92a99eee864fa06adc93149..0f76a4b4ff26d84a8b2e75921636ab55009c7a2f 100644 (file)
--- a/routers/api/v1/repo/issue.go
+++ b/routers/api/v1/repo/issue.go
@@ -462,6 +462,24 @@ func ListIssues(ctx *context.APIContext) {
                isPull = util.OptionalBoolNone
        }
 
+       if isPull != util.OptionalBoolNone && !ctx.Repo.CanReadIssuesOrPulls(isPull.IsTrue()) {
+               ctx.NotFound()
+               return
+       }
+
+       if isPull == util.OptionalBoolNone {
+               canReadIssues := ctx.Repo.CanRead(unit.TypeIssues)
+               canReadPulls := ctx.Repo.CanRead(unit.TypePullRequests)
+               if !canReadIssues && !canReadPulls {
+                       ctx.NotFound()
+                       return
+               } else if !canReadIssues {
+                       isPull = util.OptionalBoolTrue
+               } else if !canReadPulls {
+                       isPull = util.OptionalBoolFalse
+               }
+       }
+
        // FIXME: we should be more efficient here
        createdByID := getUserIDForFilter(ctx, "created_by")
        if ctx.Written() {
@@ -593,6 +611,10 @@ func GetIssue(ctx *context.APIContext) {
                }
                return
        }
+       if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
+               ctx.NotFound()
+               return
+       }
        ctx.JSON(http.StatusOK, convert.ToAPIIssue(ctx, issue))
 }
 

diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go
index c718424f7ebc242ce394a067114e1053a42f32b6..4db2c68a7980068b740fb51b17da15fd86d16809 100644 (file)
--- a/routers/api/v1/repo/issue_comment.go
+++ b/routers/api/v1/repo/issue_comment.go
@@ -12,9 +12,11 @@ import (
        issues_model "code.gitea.io/gitea/models/issues"
        access_model "code.gitea.io/gitea/models/perm/access"
        repo_model "code.gitea.io/gitea/models/repo"
+       "code.gitea.io/gitea/models/unit"
        user_model "code.gitea.io/gitea/models/user"
        "code.gitea.io/gitea/modules/context"
        api "code.gitea.io/gitea/modules/structs"
+       "code.gitea.io/gitea/modules/util"
        "code.gitea.io/gitea/modules/web"
        "code.gitea.io/gitea/routers/api/v1/utils"
        "code.gitea.io/gitea/services/convert"
@@ -71,6 +73,11 @@ func ListIssueComments(ctx *context.APIContext) {
                ctx.Error(http.StatusInternalServerError, "GetRawIssueByIndex", err)
                return
        }
+       if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
+               ctx.NotFound()
+               return
+       }
+
        issue.Repo = ctx.Repo.Repository
 
        opts := &issues_model.FindCommentsOptions{
@@ -271,12 +278,27 @@ func ListRepoIssueComments(ctx *context.APIContext) {
                return
        }
 
+       var isPull util.OptionalBool
+       canReadIssue := ctx.Repo.CanRead(unit.TypeIssues)
+       canReadPull := ctx.Repo.CanRead(unit.TypePullRequests)
+       if canReadIssue && canReadPull {
+               isPull = util.OptionalBoolNone
+       } else if canReadIssue {
+               isPull = util.OptionalBoolFalse
+       } else if canReadPull {
+               isPull = util.OptionalBoolTrue
+       } else {
+               ctx.NotFound()
+               return
+       }
+
        opts := &issues_model.FindCommentsOptions{
                ListOptions: utils.GetListOptions(ctx),
                RepoID:      ctx.Repo.Repository.ID,
                Type:        issues_model.CommentTypeComment,
                Since:       since,
                Before:      before,
+               IsPull:      isPull,
        }
 
        comments, err := issues_model.FindComments(ctx, opts)
@@ -367,6 +389,11 @@ func CreateIssueComment(ctx *context.APIContext) {
                return
        }
 
+       if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
+               ctx.NotFound()
+               return
+       }
+
        if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.Doer.IsAdmin {
                ctx.Error(http.StatusForbidden, "CreateIssueComment", errors.New(ctx.Tr("repo.issues.comment_on_locked")))
                return
@@ -436,6 +463,11 @@ func GetIssueComment(ctx *context.APIContext) {
                return
        }
 
+       if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
+               ctx.NotFound()
+               return
+       }
+
        if comment.Type != issues_model.CommentTypeComment {
                ctx.Status(http.StatusNoContent)
                return
@@ -555,7 +587,17 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
                return
        }
 
-       if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.IsAdmin()) {
+       if err := comment.LoadIssue(ctx); err != nil {
+               ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
+               return
+       }
+
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.Status(http.StatusNotFound)
+               return
+       }
+
+       if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
                ctx.Status(http.StatusForbidden)
                return
        }
@@ -658,7 +700,17 @@ func deleteIssueComment(ctx *context.APIContext) {
                return
        }
 
-       if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.IsAdmin()) {
+       if err := comment.LoadIssue(ctx); err != nil {
+               ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
+               return
+       }
+
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.Status(http.StatusNotFound)
+               return
+       }
+
+       if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
                ctx.Status(http.StatusForbidden)
                return
        } else if comment.Type != issues_model.CommentTypeComment {

diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go
index c327c54d101c0fa108226bb8bde74fb5248c1c98..21e2f4dabd6b826482609f35e9dc3bc20cc64330 100644 (file)
--- a/routers/api/v1/repo/issue_comment_attachment.go
+++ b/routers/api/v1/repo/issue_comment_attachment.go
@@ -329,6 +329,10 @@ func getIssueCommentSafe(ctx *context.APIContext) *issues_model.Comment {
                return nil
        }
 
+       if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
+               return nil
+       }
+
        comment.Issue.Repo = ctx.Repo.Repository
 
        return comment

diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go
index 29c99184e7649a95a9719a98ea0673b5c0a8f234..c886bd71b7687087515396126ea484e962291f9e 100644 (file)
--- a/routers/api/v1/repo/issue_reaction.go
+++ b/routers/api/v1/repo/issue_reaction.go
@@ -61,6 +61,12 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
 
        if err := comment.LoadIssue(ctx); err != nil {
                ctx.Error(http.StatusInternalServerError, "comment.LoadIssue", err)
+               return
+       }
+
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound()
+               return
        }
 
        if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
@@ -190,9 +196,19 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp
                return
        }
 
-       err = comment.LoadIssue(ctx)
-       if err != nil {
+       if err = comment.LoadIssue(ctx); err != nil {
                ctx.Error(http.StatusInternalServerError, "comment.LoadIssue() failed", err)
+               return
+       }
+
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound()
+               return
+       }
+
+       if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
+               ctx.NotFound()
+               return
        }
 
        if comment.Issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull) {

diff --git a/routers/api/v1/repo/key.go b/routers/api/v1/repo/key.go
index 3dc5a60d1c65430d709071c141a7a739e3ed8896..af48c40885df515e792285e571420296741f7784 100644 (file)
--- a/routers/api/v1/repo/key.go
+++ b/routers/api/v1/repo/key.go
@@ -153,6 +153,12 @@ func GetDeployKey(ctx *context.APIContext) {
                return
        }
 
+       // this check make it more consistent
+       if key.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound()
+               return
+       }
+
        if err = key.GetContent(ctx); err != nil {
                ctx.Error(http.StatusInternalServerError, "GetContent", err)
                return

diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go
index 61e5bdd67943c230379b90a841a95b87ad83fbc2..6c70bffca33be40f6a65d114f808182f7631173b 100644 (file)
--- a/routers/api/v1/repo/release.go
+++ b/routers/api/v1/repo/release.go
@@ -49,13 +49,12 @@ func GetRelease(ctx *context.APIContext) {
        //     "$ref": "#/responses/notFound"
 
        id := ctx.ParamsInt64(":id")
-       release, err := repo_model.GetReleaseByID(ctx, id)
+       release, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
        if err != nil && !repo_model.IsErrReleaseNotExist(err) {
-               ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+               ctx.Error(http.StatusInternalServerError, "GetReleaseForRepoByID", err)
                return
        }
-       if err != nil && repo_model.IsErrReleaseNotExist(err) ||
-               release.IsTag || release.RepoID != ctx.Repo.Repository.ID {
+       if err != nil && repo_model.IsErrReleaseNotExist(err) || release.IsTag {
                ctx.NotFound()
                return
        }
@@ -315,13 +314,12 @@ func EditRelease(ctx *context.APIContext) {
 
        form := web.GetForm(ctx).(*api.EditReleaseOption)
        id := ctx.ParamsInt64(":id")
-       rel, err := repo_model.GetReleaseByID(ctx, id)
+       rel, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
        if err != nil && !repo_model.IsErrReleaseNotExist(err) {
-               ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+               ctx.Error(http.StatusInternalServerError, "GetReleaseForRepoByID", err)
                return
        }
-       if err != nil && repo_model.IsErrReleaseNotExist(err) ||
-               rel.IsTag || rel.RepoID != ctx.Repo.Repository.ID {
+       if err != nil && repo_model.IsErrReleaseNotExist(err) || rel.IsTag {
                ctx.NotFound()
                return
        }
@@ -393,17 +391,16 @@ func DeleteRelease(ctx *context.APIContext) {
        //     "$ref": "#/responses/empty"
 
        id := ctx.ParamsInt64(":id")
-       rel, err := repo_model.GetReleaseByID(ctx, id)
+       rel, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
        if err != nil && !repo_model.IsErrReleaseNotExist(err) {
-               ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+               ctx.Error(http.StatusInternalServerError, "GetReleaseForRepoByID", err)
                return
        }
-       if err != nil && repo_model.IsErrReleaseNotExist(err) ||
-               rel.IsTag || rel.RepoID != ctx.Repo.Repository.ID {
+       if err != nil && repo_model.IsErrReleaseNotExist(err) || rel.IsTag {
                ctx.NotFound()
                return
        }
-       if err := release_service.DeleteReleaseByID(ctx, id, ctx.Doer, false); err != nil {
+       if err := release_service.DeleteReleaseByID(ctx, ctx.Repo.Repository, rel, ctx.Doer, false); err != nil {
                if models.IsErrProtectedTagName(err) {
                        ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
                        return

diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go
index 168ef550c5ea98d76e4b8813b1ad249c0c12f615..c36bf12e6d330201c2b76d70c157170f0b1ff48a 100644 (file)
--- a/routers/api/v1/repo/release_attachment.go
+++ b/routers/api/v1/repo/release_attachment.go
@@ -17,6 +17,23 @@ import (
        "code.gitea.io/gitea/services/convert"
 )
 
+func checkReleaseMatchRepo(ctx *context.APIContext, releaseID int64) bool {
+       release, err := repo_model.GetReleaseByID(ctx, releaseID)
+       if err != nil {
+               if repo_model.IsErrReleaseNotExist(err) {
+                       ctx.NotFound()
+                       return false
+               }
+               ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+               return false
+       }
+       if release.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound()
+               return false
+       }
+       return true
+}
+
 // GetReleaseAttachment gets a single attachment of the release
 func GetReleaseAttachment(ctx *context.APIContext) {
        // swagger:operation GET /repos/{owner}/{repo}/releases/{id}/assets/{attachment_id} repository repoGetReleaseAttachment
@@ -54,6 +71,10 @@ func GetReleaseAttachment(ctx *context.APIContext) {
        //     "$ref": "#/responses/notFound"
 
        releaseID := ctx.ParamsInt64(":id")
+       if !checkReleaseMatchRepo(ctx, releaseID) {
+               return
+       }
+
        attachID := ctx.ParamsInt64(":attachment_id")
        attach, err := repo_model.GetAttachmentByID(ctx, attachID)
        if err != nil {
@@ -176,13 +197,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 
        // Check if release exists an load release
        releaseID := ctx.ParamsInt64(":id")
-       release, err := repo_model.GetReleaseByID(ctx, releaseID)
-       if err != nil {
-               if repo_model.IsErrReleaseNotExist(err) {
-                       ctx.NotFound()
-                       return
-               }
-               ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+       if !checkReleaseMatchRepo(ctx, releaseID) {
                return
        }
 
@@ -203,7 +218,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
        attach, err := attachment.UploadAttachment(ctx, file, setting.Repository.Release.AllowedTypes, header.Size, &repo_model.Attachment{
                Name:       filename,
                UploaderID: ctx.Doer.ID,
-               RepoID:     release.RepoID,
+               RepoID:     ctx.Repo.Repository.ID,
                ReleaseID:  releaseID,
        })
        if err != nil {
@@ -264,6 +279,10 @@ func EditReleaseAttachment(ctx *context.APIContext) {
 
        // Check if release exists an load release
        releaseID := ctx.ParamsInt64(":id")
+       if !checkReleaseMatchRepo(ctx, releaseID) {
+               return
+       }
+
        attachID := ctx.ParamsInt64(":attachment_id")
        attach, err := repo_model.GetAttachmentByID(ctx, attachID)
        if err != nil {
@@ -328,6 +347,10 @@ func DeleteReleaseAttachment(ctx *context.APIContext) {
 
        // Check if release exists an load release
        releaseID := ctx.ParamsInt64(":id")
+       if !checkReleaseMatchRepo(ctx, releaseID) {
+               return
+       }
+
        attachID := ctx.ParamsInt64(":attachment_id")
        attach, err := repo_model.GetAttachmentByID(ctx, attachID)
        if err != nil {

diff --git a/routers/api/v1/repo/release_tags.go b/routers/api/v1/repo/release_tags.go
index 926a713c9477c01b00f4a68641e9759d8279916b..9f2098df06642026a29b6e9ecc9ebf8f87343e5b 100644 (file)
--- a/routers/api/v1/repo/release_tags.go
+++ b/routers/api/v1/repo/release_tags.go
@@ -112,7 +112,7 @@ func DeleteReleaseByTag(ctx *context.APIContext) {
                return
        }
 
-       if err = releaseservice.DeleteReleaseByID(ctx, release.ID, ctx.Doer, false); err != nil {
+       if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, release, ctx.Doer, false); err != nil {
                if models.IsErrProtectedTagName(err) {
                        ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
                        return

diff --git a/routers/api/v1/repo/tag.go b/routers/api/v1/repo/tag.go
index dbc8df0ef81b10addf05fcb7f27acf18cedf4762..2f19f95e66dec58725e48268146123882e8b41df 100644 (file)
--- a/routers/api/v1/repo/tag.go
+++ b/routers/api/v1/repo/tag.go
@@ -272,7 +272,7 @@ func DeleteTag(ctx *context.APIContext) {
                return
        }
 
-       if err = releaseservice.DeleteReleaseByID(ctx, tag.ID, ctx.Doer, true); err != nil {
+       if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, tag, ctx.Doer, true); err != nil {
                if models.IsErrProtectedTagName(err) {
                        ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
                        return

diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go
index edbc1d17b464016e8677d062eb49dd7e91cb4e1e..f045fb4d5d8c44a18324f690c8af469800a3bc91 100644 (file)
--- a/routers/api/v1/user/app.go
+++ b/routers/api/v1/user/app.go
@@ -342,6 +342,10 @@ func GetOauth2Application(ctx *context.APIContext) {
                }
                return
        }
+       if app.UID != ctx.Doer.ID {
+               ctx.NotFound()
+               return
+       }
 
        app.ClientSecret = ""
 

diff --git a/routers/api/v1/user/gpg_key.go b/routers/api/v1/user/gpg_key.go
index 404b1d221e1d0bfd84a7a18da1a514bc7dfcd91c..4f8bcaca3e6d5a01fa42d1fcc80383408057f283 100644 (file)
--- a/routers/api/v1/user/gpg_key.go
+++ b/routers/api/v1/user/gpg_key.go
@@ -112,7 +112,7 @@ func GetGPGKey(ctx *context.APIContext) {
        //   "404":
        //     "$ref": "#/responses/notFound"
 
-       key, err := asymkey_model.GetGPGKeyByID(ctx, ctx.ParamsInt64(":id"))
+       key, err := asymkey_model.GetGPGKeyForUserByID(ctx, ctx.Doer.ID, ctx.ParamsInt64(":id"))
        if err != nil {
                if asymkey_model.IsErrGPGKeyNotExist(err) {
                        ctx.NotFound()

diff --git a/routers/api/v1/user/hook.go b/routers/api/v1/user/hook.go
index 50be519c815fa73c39409ee181d46a5cfe021a50..e87385e4a2602fb6bf14da187f683d5e176ec7e6 100644 (file)
--- a/routers/api/v1/user/hook.go
+++ b/routers/api/v1/user/hook.go
@@ -62,6 +62,11 @@ func GetHook(ctx *context.APIContext) {
                return
        }
 
+       if !ctx.Doer.IsAdmin && hook.OwnerID != ctx.Doer.ID {
+               ctx.NotFound()
+               return
+       }
+
        apiHook, err := webhook_service.ToHook(ctx.Doer.HomeLink(), hook)
        if err != nil {
                ctx.InternalServerError(err)

diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index fad4a10de814d97e8d7d1a10339980db9355bdf7..3ea40fe8c9a725dff92df4acf4d007874c4589a0 100644 (file)
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -3106,6 +3106,11 @@ func UpdateCommentContent(ctx *context.Context) {
                return
        }
 
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound("CompareRepoID", issues_model.ErrCommentNotExist{})
+               return
+       }
+
        if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
                ctx.Error(http.StatusForbidden)
                return
@@ -3172,6 +3177,11 @@ func DeleteComment(ctx *context.Context) {
                return
        }
 
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound("CompareRepoID", issues_model.ErrCommentNotExist{})
+               return
+       }
+
        if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
                ctx.Error(http.StatusForbidden)
                return
@@ -3298,6 +3308,11 @@ func ChangeCommentReaction(ctx *context.Context) {
                return
        }
 
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound("CompareRepoID", issues_model.ErrCommentNotExist{})
+               return
+       }
+
        if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull)) {
                if log.IsTrace() {
                        if ctx.IsSigned {
@@ -3441,6 +3456,21 @@ func GetCommentAttachments(ctx *context.Context) {
                return
        }
 
+       if err := comment.LoadIssue(ctx); err != nil {
+               ctx.NotFoundOrServerError("LoadIssue", issues_model.IsErrIssueNotExist, err)
+               return
+       }
+
+       if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+               ctx.NotFound("CompareRepoID", issues_model.ErrCommentNotExist{})
+               return
+       }
+
+       if !ctx.Repo.Permission.CanReadIssuesOrPulls(comment.Issue.IsPull) {
+               ctx.NotFound("CanReadIssuesOrPulls", issues_model.ErrCommentNotExist{})
+               return
+       }
+
        if !comment.Type.HasAttachmentSupport() {
                ctx.ServerError("GetCommentAttachments", fmt.Errorf("comment type %v does not support attachments", comment.Type))
                return

diff --git a/routers/web/repo/issue_content_history.go b/routers/web/repo/issue_content_history.go
index 5c378fe9d79dfeaa6a34c2c9ddb6dd5409e7af65..473ab260f3ea05efed59a209b83fad7cd00f7e65 100644 (file)
--- a/routers/web/repo/issue_content_history.go
+++ b/routers/web/repo/issue_content_history.go
@@ -122,7 +122,7 @@ func GetContentHistoryDetail(ctx *context.Context) {
        }
 
        historyID := ctx.FormInt64("history_id")
-       history, prevHistory, err := issues_model.GetIssueContentHistoryAndPrev(ctx, historyID)
+       history, prevHistory, err := issues_model.GetIssueContentHistoryAndPrev(ctx, issue.ID, historyID)
        if err != nil {
                ctx.JSON(http.StatusNotFound, map[string]any{
                        "message": "Can not find the content history",

diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go
index 199a06524546d6d25c4b1fba521b620f3cce2d16..5694575b468de2f24bf5b4ce79d5e00efe89742d 100644 (file)
--- a/routers/web/repo/projects.go
+++ b/routers/web/repo/projects.go
@@ -468,7 +468,7 @@ func AddBoardToProjectPost(ctx *context.Context) {
                return
        }
 
-       project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id"))
+       project, err := project_model.GetProjectForRepoByID(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":id"))
        if err != nil {
                if project_model.IsErrProjectNotExist(err) {
                        ctx.NotFound("", nil)

diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go
index 5cbd6b3d51c3bfcc931be47a129258ab3f41a0d1..761dadd5444c5e7483ade339e28f6c78e48a0f1b 100644 (file)
--- a/routers/web/repo/release.go
+++ b/routers/web/repo/release.go
@@ -616,7 +616,27 @@ func DeleteTag(ctx *context.Context) {
 }
 
 func deleteReleaseOrTag(ctx *context.Context, isDelTag bool) {
-       if err := releaseservice.DeleteReleaseByID(ctx, ctx.FormInt64("id"), ctx.Doer, isDelTag); err != nil {
+       redirect := func() {
+               if isDelTag {
+                       ctx.JSONRedirect(ctx.Repo.RepoLink + "/tags")
+                       return
+               }
+
+               ctx.JSONRedirect(ctx.Repo.RepoLink + "/releases")
+       }
+
+       rel, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, ctx.FormInt64("id"))
+       if err != nil {
+               if repo_model.IsErrReleaseNotExist(err) {
+                       ctx.NotFound("GetReleaseForRepoByID", err)
+               } else {
+                       ctx.Flash.Error("DeleteReleaseByID: " + err.Error())
+                       redirect()
+               }
+               return
+       }
+
+       if err := releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, rel, ctx.Doer, isDelTag); err != nil {
                if models.IsErrProtectedTagName(err) {
                        ctx.Flash.Error(ctx.Tr("repo.release.tag_name_protected"))
                } else {
@@ -630,10 +650,5 @@ func deleteReleaseOrTag(ctx *context.Context, isDelTag bool) {
                }
        }
 
-       if isDelTag {
-               ctx.JSONRedirect(ctx.Repo.RepoLink + "/tags")
-               return
-       }
-
-       ctx.JSONRedirect(ctx.Repo.RepoLink + "/releases")
+       redirect()
 }

diff --git a/services/release/release.go b/services/release/release.go
index e0035d42fc2a74d1732cb71af9f337b89d70f737..3ba2a3f611786ee1d8f4d878324f75f1036978bf 100644 (file)
--- a/services/release/release.go
+++ b/services/release/release.go
@@ -301,17 +301,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo
 }
 
 // DeleteReleaseByID deletes a release and corresponding Git tag by given ID.
-func DeleteReleaseByID(ctx context.Context, id int64, doer *user_model.User, delTag bool) error {
-       rel, err := repo_model.GetReleaseByID(ctx, id)
-       if err != nil {
-               return fmt.Errorf("GetReleaseByID: %w", err)
-       }
-
-       repo, err := repo_model.GetRepositoryByID(ctx, rel.RepoID)
-       if err != nil {
-               return fmt.Errorf("GetRepositoryByID: %w", err)
-       }
-
+func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *repo_model.Release, doer *user_model.User, delTag bool) error {
        if delTag {
                protectedTags, err := git_model.GetProtectedTags(ctx, rel.RepoID)
                if err != nil {
@@ -344,19 +334,19 @@ func DeleteReleaseByID(ctx context.Context, id int64, doer *user_model.User, del
                        }, repository.NewPushCommits())
                notify_service.DeleteRef(ctx, doer, repo, refName)
 
-               if err := repo_model.DeleteReleaseByID(ctx, id); err != nil {
+               if err := repo_model.DeleteReleaseByID(ctx, rel.ID); err != nil {
                        return fmt.Errorf("DeleteReleaseByID: %w", err)
                }
        } else {
                rel.IsTag = true
 
-               if err = repo_model.UpdateRelease(ctx, rel); err != nil {
+               if err := repo_model.UpdateRelease(ctx, rel); err != nil {
                        return fmt.Errorf("Update: %w", err)
                }
        }
 
        rel.Repo = repo
-       if err = rel.LoadAttributes(ctx); err != nil {
+       if err := rel.LoadAttributes(ctx); err != nil {
                return fmt.Errorf("LoadAttributes: %w", err)
        }
 

diff --git a/tests/integration/api_comment_attachment_test.go b/tests/integration/api_comment_attachment_test.go
index e211376c3c8886f78edf3701c5f5c74ce652146e..95a7a81eb4040fd36a4b2c2167a3a07712e310a2 100644 (file)
--- a/tests/integration/api_comment_attachment_test.go
+++ b/tests/integration/api_comment_attachment_test.go
@@ -35,6 +35,14 @@ func TestAPIGetCommentAttachment(t *testing.T) {
        repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: comment.Issue.RepoID})
        repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
 
+       t.Run("UnrelatedCommentID", func(t *testing.T) {
+               repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+               repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+               token := getUserToken(t, repoOwner.Name, auth_model.AccessTokenScopeWriteIssue)
+               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)
+               MakeRequest(t, req, http.StatusNotFound)
+       })
+
        session := loginUser(t, repoOwner.Name)
        token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadIssue)
        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)

diff --git a/tests/integration/api_comment_test.go b/tests/integration/api_comment_test.go
index 0be4896105bf524d1db03e5ccd081447df8c4df1..fe272cf926932fd4da59276965cb7f614e72738b 100644 (file)
--- a/tests/integration/api_comment_test.go
+++ b/tests/integration/api_comment_test.go
@@ -177,12 +177,25 @@ func TestAPIEditComment(t *testing.T) {
        defer tests.PrepareTestEnv(t)()
        const newCommentBody = "This is the new comment body"
 
-       comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{},
+       comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 8},
                unittest.Cond("type = ?", issues_model.CommentTypeComment))
        issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID})
        repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
        repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
 
+       t.Run("UnrelatedCommentID", func(t *testing.T) {
+               // Using the ID of a comment that does not belong to the repository must fail
+               repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+               repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+               token := getUserToken(t, repoOwner.Name, auth_model.AccessTokenScopeWriteIssue)
+               urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d?token=%s",
+                       repoOwner.Name, repo.Name, comment.ID, token)
+               req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{
+                       "body": newCommentBody,
+               })
+               MakeRequest(t, req, http.StatusNotFound)
+       })
+
        token := getUserToken(t, repoOwner.Name, auth_model.AccessTokenScopeWriteIssue)
        urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d?token=%s",
                repoOwner.Name, repo.Name, comment.ID, token)
@@ -201,12 +214,22 @@ func TestAPIEditComment(t *testing.T) {
 func TestAPIDeleteComment(t *testing.T) {
        defer tests.PrepareTestEnv(t)()
 
-       comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{},
+       comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 8},
                unittest.Cond("type = ?", issues_model.CommentTypeComment))
        issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID})
        repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
        repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
 
+       t.Run("UnrelatedCommentID", func(t *testing.T) {
+               // Using the ID of a comment that does not belong to the repository must fail
+               repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+               repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+               token := getUserToken(t, repoOwner.Name, auth_model.AccessTokenScopeWriteIssue)
+               req := NewRequestf(t, "DELETE", "/api/v1/repos/%s/%s/issues/comments/%d?token=%s",
+                       repoOwner.Name, repo.Name, comment.ID, token)
+               MakeRequest(t, req, http.StatusNotFound)
+       })
+
        token := getUserToken(t, repoOwner.Name, auth_model.AccessTokenScopeWriteIssue)
        req := NewRequestf(t, "DELETE", "/api/v1/repos/%s/%s/issues/comments/%d?token=%s",
                repoOwner.Name, repo.Name, comment.ID, token)

diff --git a/tests/integration/api_issue_reaction_test.go b/tests/integration/api_issue_reaction_test.go
index 7d3ee2d15432fae8792ef7485315f5c081560294..124d729353ca7a9a8282cb566f284e1d2f3c7a7e 100644 (file)
--- a/tests/integration/api_issue_reaction_test.go
+++ b/tests/integration/api_issue_reaction_test.go
@@ -12,6 +12,7 @@ import (
        auth_model "code.gitea.io/gitea/models/auth"
        "code.gitea.io/gitea/models/db"
        issues_model "code.gitea.io/gitea/models/issues"
+       repo_model "code.gitea.io/gitea/models/repo"
        "code.gitea.io/gitea/models/unittest"
        user_model "code.gitea.io/gitea/models/user"
        api "code.gitea.io/gitea/modules/structs"
@@ -107,6 +108,26 @@ func TestAPICommentReactions(t *testing.T) {
        })
        MakeRequest(t, req, http.StatusOK)
 
+       t.Run("UnrelatedCommentID", func(t *testing.T) {
+               // Using the ID of a comment that does not belong to the repository must fail
+               repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+               repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+               token := getUserToken(t, repoOwner.Name, auth_model.AccessTokenScopeWriteIssue)
+               urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/reactions?token=%s",
+                       repoOwner.Name, repo.Name, comment.ID, token)
+               req = NewRequestWithJSON(t, "POST", urlStr, &api.EditReactionOption{
+                       Reaction: "+1",
+               })
+               MakeRequest(t, req, http.StatusNotFound)
+               req = NewRequestWithJSON(t, "DELETE", urlStr, &api.EditReactionOption{
+                       Reaction: "+1",
+               })
+               MakeRequest(t, req, http.StatusNotFound)
+
+               req = NewRequestf(t, "GET", urlStr)
+               MakeRequest(t, req, http.StatusNotFound)
+       })
+
        // Add allowed reaction
        req = NewRequestWithJSON(t, "POST", urlStr, &api.EditReactionOption{
                Reaction: "+1",

diff --git a/tests/integration/api_keys_test.go b/tests/integration/api_keys_test.go
index 238c3cb823230a15301dca4baee3aeb2498562f9..03d28c9126656f7ceb8a2775eb911f9b9bc5a7a9 100644 (file)
--- a/tests/integration/api_keys_test.go
+++ b/tests/integration/api_keys_test.go
@@ -72,6 +72,17 @@ func TestCreateReadOnlyDeployKey(t *testing.T) {
                Content: rawKeyBody.Key,
                Mode:    perm.AccessModeRead,
        })
+
+       // Using the ID of a key that does not belong to the repository must fail
+       {
+               req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/keys/%d?token=%s", repoOwner.Name, repo.Name, newDeployKey.ID, token))
+               MakeRequest(t, req, http.StatusOK)
+
+               session5 := loginUser(t, "user5")
+               token5 := getTokenForLoggedInUser(t, session5, auth_model.AccessTokenScopeWriteRepository)
+               req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/user5/repo4/keys/%d?token=%s", newDeployKey.ID, token5))
+               MakeRequest(t, req, http.StatusNotFound)
+       }
 }
 
 func TestCreateReadWriteDeployKey(t *testing.T) {

diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go
index 4cbd25f5deab1db812e0761f56f880afb024e693..fb35d72ac2ffc9493178377b34a9e14f1a771300 100644 (file)
--- a/tests/integration/api_nodeinfo_test.go
+++ b/tests/integration/api_nodeinfo_test.go
@@ -34,6 +34,6 @@ func TestNodeinfo(t *testing.T) {
                assert.Equal(t, "gitea", nodeinfo.Software.Name)
                assert.Equal(t, 25, nodeinfo.Usage.Users.Total)
                assert.Equal(t, 20, nodeinfo.Usage.LocalPosts)
-               assert.Equal(t, 2, nodeinfo.Usage.LocalComments)
+               assert.Equal(t, 3, nodeinfo.Usage.LocalComments)
        })
 }

diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go
index ac06b487db25f2cff12dfee29cf141a97431496b..b1080c998a36158082c75b6c982296207e0ef8aa 100644 (file)
--- a/tests/integration/issue_test.go
+++ b/tests/integration/issue_test.go
@@ -206,6 +206,56 @@ func TestIssueCommentClose(t *testing.T) {
        assert.Equal(t, "Description", val)
 }
 
+func TestIssueCommentDelete(t *testing.T) {
+       defer tests.PrepareTestEnv(t)()
+       session := loginUser(t, "user2")
+       issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description")
+       comment1 := "Test comment 1"
+       commentID := testIssueAddComment(t, session, issueURL, comment1, "")
+       comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: commentID})
+       assert.Equal(t, comment1, comment.Content)
+
+       // Using the ID of a comment that does not belong to the repository must fail
+       req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d/delete", "user5", "repo4", commentID), map[string]string{
+               "_csrf": GetCSRF(t, session, issueURL),
+       })
+       session.MakeRequest(t, req, http.StatusNotFound)
+       req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d/delete", "user2", "repo1", commentID), map[string]string{
+               "_csrf": GetCSRF(t, session, issueURL),
+       })
+       session.MakeRequest(t, req, http.StatusOK)
+       unittest.AssertNotExistsBean(t, &issues_model.Comment{ID: commentID})
+}
+
+func TestIssueCommentUpdate(t *testing.T) {
+       defer tests.PrepareTestEnv(t)()
+       session := loginUser(t, "user2")
+       issueURL := testNewIssue(t, session, "user2", "repo1", "Title", "Description")
+       comment1 := "Test comment 1"
+       commentID := testIssueAddComment(t, session, issueURL, comment1, "")
+
+       comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: commentID})
+       assert.Equal(t, comment1, comment.Content)
+
+       modifiedContent := comment.Content + "MODIFIED"
+
+       // Using the ID of a comment that does not belong to the repository must fail
+       req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user5", "repo4", commentID), map[string]string{
+               "_csrf":   GetCSRF(t, session, issueURL),
+               "content": modifiedContent,
+       })
+       session.MakeRequest(t, req, http.StatusNotFound)
+
+       req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/comments/%d", "user2", "repo1", commentID), map[string]string{
+               "_csrf":   GetCSRF(t, session, issueURL),
+               "content": modifiedContent,
+       })
+       session.MakeRequest(t, req, http.StatusOK)
+
+       comment = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: commentID})
+       assert.Equal(t, modifiedContent, comment.Content)
+}
+
 func TestIssueReaction(t *testing.T) {
        defer tests.PrepareTestEnv(t)()
        session := loginUser(t, "user2")

diff --git a/tests/integration/mirror_pull_test.go b/tests/integration/mirror_pull_test.go
index e1c7c6b1709414e8767edaf8a198fb890c649d4c..c02e16bfc0b676f9ebda2e271b74144d48e20627 100644 (file)
--- a/tests/integration/mirror_pull_test.go
+++ b/tests/integration/mirror_pull_test.go
@@ -88,7 +88,7 @@ func TestMirrorPull(t *testing.T) {
 
        release, err := repo_model.GetRelease(db.DefaultContext, repo.ID, "v0.2")
        assert.NoError(t, err)
-       assert.NoError(t, release_service.DeleteReleaseByID(ctx, release.ID, user, true))
+       assert.NoError(t, release_service.DeleteReleaseByID(ctx, repo, release, user, true))
 
        ok = mirror_service.SyncPullMirror(ctx, mirror.ID)
        assert.True(t, ok)