diff --git a/models/issues/comment.go b/models/issues/comment.go index c0a13ef03d..325fcbe30b 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -611,9 +611,13 @@ func (c *Comment) UpdateAttachments(ctx context.Context, uuids []string) error { } defer committer.Close() - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) + if err := c.LoadIssue(ctx); err != nil { + return fmt.Errorf("LoadIssue: %w", err) + } + + attachments, err := repo_model.FindRepoAttachmentsByUUID(ctx, c.Issue.RepoID, uuids, repo_model.FindAttachmentOptions{}) if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) + return fmt.Errorf("FindRepoAttachmentsByUUID[uuids=%q,repoID=%d]: %w", uuids, c.Issue.RepoID, err) } for i := 0; i < len(attachments); i++ { attachments[i].IssueID = c.IssueID @@ -889,7 +893,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment // Check comment type. switch opts.Type { case CommentTypeCode: - if err = updateAttachments(ctx, opts, comment); err != nil { + if err := comment.UpdateAttachments(ctx, opts.Attachments); err != nil { return err } if comment.ReviewID != 0 { @@ -909,7 +913,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } fallthrough case CommentTypeReview: - if err = updateAttachments(ctx, opts, comment); err != nil { + if err := comment.UpdateAttachments(ctx, opts.Attachments); err != nil { return err } case CommentTypeReopen, CommentTypeClose: @@ -921,23 +925,6 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment return UpdateIssueCols(ctx, opts.Issue, "updated_unix") } -func updateAttachments(ctx context.Context, opts *CreateCommentOptions, comment *Comment) error { - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, opts.Attachments) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", 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 = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil { - return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err) - } - } - comment.Attachments = attachments - return nil -} - func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) { var content string var commentType CommentType diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 84a8820cd1..22e6fcb8d4 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -234,18 +234,18 @@ func AddDeletePRBranchComment(ctx context.Context, doer *user_model.User, repo * } // UpdateIssueAttachments update attachments by UUIDs for the issue -func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string) (err error) { +func UpdateIssueAttachments(ctx context.Context, issue *Issue, uuids []string) (err error) { ctx, committer, err := db.TxContext(ctx) if err != nil { return err } defer committer.Close() - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) + attachments, err := repo_model.FindRepoAttachmentsByUUID(ctx, issue.RepoID, uuids, repo_model.FindAttachmentOptions{}) if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) + return fmt.Errorf("FindRepoAttachmentsByUUID[uuids=%q,repoID=%d]: %w", uuids, issue.RepoID, err) } for i := 0; i < len(attachments); i++ { - attachments[i].IssueID = issueID + attachments[i].IssueID = issue.ID if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) } @@ -394,18 +394,8 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue return err } - 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) - } - - for i := 0; i < len(attachments); i++ { - attachments[i].IssueID = opts.Issue.ID - if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { - return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) - } - } + if err := UpdateIssueAttachments(ctx, opts.Issue, opts.Attachments); err != nil { + return fmt.Errorf("UpdateIssueAttachments: %w", err) } if err = opts.Issue.LoadAttributes(ctx); err != nil { return err diff --git a/models/project/project.go b/models/project/project.go index c095087e51..3a36613b49 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -309,6 +309,18 @@ func GetProjectForRepoByID(ctx context.Context, repoID, id int64) (*Project, err return p, nil } +// GetProjectForUserByID returns the project by id that belongs to the specified user. +func GetProjectForUserByID(ctx context.Context, uid, id int64) (*Project, error) { + p := new(Project) + has, err := db.GetEngine(ctx).Where("id=? AND owner_id=?", id, uid).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) { @@ -346,42 +358,26 @@ func updateRepositoryProjectCount(ctx context.Context, repoID int64) error { return nil } -// ChangeProjectStatusByRepoIDAndID toggles a project between opened and closed -func ChangeProjectStatusByRepoIDAndID(ctx context.Context, repoID, projectID int64, isClosed bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - p := new(Project) - - has, err := db.GetEngine(ctx).ID(projectID).Where("repo_id = ?", repoID).Get(p) - if err != nil { - return err - } else if !has { - return ErrProjectNotExist{ID: projectID, RepoID: repoID} - } - - if err := changeProjectStatus(ctx, p, isClosed); err != nil { - return err - } - - return committer.Commit() -} - -func changeProjectStatus(ctx context.Context, p *Project, isClosed bool) error { - p.IsClosed = isClosed - p.ClosedDateUnix = timeutil.TimeStampNow() - count, err := db.GetEngine(ctx).ID(p.ID).Where("repo_id = ? AND is_closed = ?", p.RepoID, !isClosed).Cols("is_closed", "closed_date_unix").Update(p) - if err != nil { - return err - } - if count < 1 { +// ChangeProjectStatus changes the status of the specified project to the state +// specified via the `isClosed` argument. +func ChangeProjectStatus(ctx context.Context, p *Project, isClosed bool) error { + if p.IsClosed == isClosed { return nil } - return updateRepositoryProjectCount(ctx, p.RepoID) + return db.WithTx(ctx, func(ctx context.Context) error { + p.IsClosed = isClosed + p.ClosedDateUnix = timeutil.TimeStampNow() + count, err := db.GetEngine(ctx).ID(p.ID).Cols("is_closed", "closed_date_unix").Update(p) + if err != nil { + return err + } + if count < 1 { + return nil + } + + return updateRepositoryProjectCount(ctx, p.RepoID) + }) } // DeleteProjectByID deletes a project from a repository. if it's not in a database diff --git a/models/project/project_test.go b/models/project/project_test.go index b6f7c7db27..8a50d73ebc 100644 --- a/models/project/project_test.go +++ b/models/project/project_test.go @@ -1,4 +1,5 @@ // Copyright 2020 The Gitea Authors. All rights reserved. +// Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package project @@ -7,8 +8,8 @@ import ( "testing" "forgejo.org/models/db" + repo_model "forgejo.org/models/repo" "forgejo.org/models/unittest" - "forgejo.org/modules/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -48,42 +49,6 @@ func TestGetProjects(t *testing.T) { assert.Len(t, projects, 1) } -func TestProject(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - project := &Project{ - Type: TypeRepository, - TemplateType: TemplateTypeBasicKanban, - CardType: CardTypeTextOnly, - Title: "New Project", - RepoID: 1, - CreatedUnix: timeutil.TimeStampNow(), - CreatorID: 2, - } - - require.NoError(t, NewProject(db.DefaultContext, project)) - - _, err := GetProjectByID(db.DefaultContext, project.ID) - require.NoError(t, err) - - // Update project - project.Title = "Updated title" - require.NoError(t, UpdateProject(db.DefaultContext, project)) - - projectFromDB, err := GetProjectByID(db.DefaultContext, project.ID) - require.NoError(t, err) - - assert.Equal(t, project.Title, projectFromDB.Title) - - require.NoError(t, ChangeProjectStatusByRepoIDAndID(db.DefaultContext, project.RepoID, project.ID, true)) - - // Retrieve from DB afresh to check if it is truly closed - projectFromDB, err = GetProjectByID(db.DefaultContext, project.ID) - require.NoError(t, err) - - assert.True(t, projectFromDB.IsClosed) -} - func TestProjectsSort(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) @@ -122,3 +87,74 @@ func TestProjectsSort(t *testing.T) { } } } + +func TestGetProjectForUserByID(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + found := func(t *testing.T, uid, id int64) { + t.Helper() + + p, err := GetProjectForUserByID(t.Context(), uid, id) + require.NoError(t, err) + if assert.NotNil(t, p) { + assert.Equal(t, id, p.ID) + } + } + + notFound := func(t *testing.T, uid, id int64) { + t.Helper() + + p, err := GetProjectForUserByID(t.Context(), uid, id) + require.ErrorIs(t, err, ErrProjectNotExist{ID: id}) + assert.Nil(t, p) + } + + found(t, 2, 4) + found(t, 2, 5) + found(t, 2, 6) + found(t, 3, 7) + notFound(t, 1, 4) + notFound(t, 1, 5) + notFound(t, 1, 6) + notFound(t, 1, 7) +} + +func TestChangeProjectStatus(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("Unchanged", func(t *testing.T) { + project := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + + require.NoError(t, ChangeProjectStatus(t.Context(), project, project.IsClosed)) + + projectAfter := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + assert.Equal(t, project.IsClosed, projectAfter.IsClosed) + }) + + t.Run("Normal", func(t *testing.T) { + project := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + isClosed := !project.IsClosed + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: project.RepoID}) + + require.NoError(t, ChangeProjectStatus(t.Context(), project, isClosed)) + + projectAfter := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + repoAfter := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: project.RepoID}) + assert.Equal(t, isClosed, projectAfter.IsClosed) + assert.Equal(t, repo.NumProjects, repoAfter.NumProjects) + assert.Equal(t, repo.NumOpenProjects-1, repoAfter.NumOpenProjects) + assert.Equal(t, repo.NumClosedProjects+1, repoAfter.NumClosedProjects) + }) + + t.Run("Invalid ID", func(t *testing.T) { + project := &Project{ID: 1001, RepoID: 1} + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: project.RepoID}) + + require.NoError(t, ChangeProjectStatus(t.Context(), project, true)) + + repoAfter := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: project.RepoID}) + assert.Equal(t, repo.NumProjects, repoAfter.NumProjects) + assert.Equal(t, repo.NumOpenProjects, repoAfter.NumOpenProjects) + assert.Equal(t, repo.NumClosedProjects, repoAfter.NumClosedProjects) + }) +} diff --git a/models/repo/attachment.go b/models/repo/attachment.go index 6d903be5f8..1b6af572dd 100644 --- a/models/repo/attachment.go +++ b/models/repo/attachment.go @@ -1,4 +1,5 @@ // Copyright 2017 The Gitea Authors. All rights reserved. +// Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package repo @@ -16,17 +17,30 @@ import ( "forgejo.org/modules/timeutil" "forgejo.org/modules/util" "forgejo.org/modules/validation" + + "xorm.io/builder" ) // Attachment represent a attachment of issue/comment/release. type Attachment struct { - ID int64 `xorm:"pk autoincr"` - UUID string `xorm:"uuid UNIQUE"` - RepoID int64 `xorm:"INDEX"` // this should not be zero - IssueID int64 `xorm:"INDEX"` // maybe zero when creating - ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating - UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added - CommentID int64 `xorm:"INDEX"` + ID int64 `xorm:"pk autoincr"` + // UUID is the public identifier of the attachment, and is used during HTTP + // requests to refer to a specific attachment. + UUID string `xorm:"uuid UNIQUE"` + // UploaderID is always set and non-zero and refers to the user that has + // uploaded this attachment. + UploaderID int64 `xorm:"INDEX DEFAULT 0"` + // RepoID is always set and non-zero and refers to the repository where this + // attachment was uploaded to. + RepoID int64 `xorm:"INDEX"` + // IssueID, ReleaseID and CommentID have multiple possible states: + // - ReleaseID != 0 && IssueID == 0 && CommentID == 0: attached to release with id `ReleaseID`. + // - ReleaseID == 0 && IssueID != 0 && CommentID == 0: attached to the issue with id `IssueID`. + // - ReleaseID == 0 && IssueID != 0 && CommentID != 0: attached to comment with id `CommentID` that is in issue with id `IssueID`. + // All other states should be considered invalid. + IssueID int64 `xorm:"INDEX"` + ReleaseID int64 `xorm:"INDEX"` + CommentID int64 `xorm:"INDEX"` Name string DownloadCount int64 `xorm:"DEFAULT 0"` Size int64 `xorm:"DEFAULT 0"` @@ -73,6 +87,12 @@ func (a *Attachment) DownloadURL() string { return setting.AppURL + "attachments/" + url.PathEscape(a.UUID) } +// IsAttachedToResource returns true if this attachment is attached to a release, +// issue or comment. +func (a *Attachment) IsAttachedToResource() bool { + return a.ReleaseID != 0 || a.IssueID != 0 || a.CommentID != 0 +} + // ErrAttachmentNotExist represents a "AttachmentNotExist" kind of error. type ErrAttachmentNotExist struct { ID int64 @@ -133,15 +153,37 @@ func GetAttachmentByUUID(ctx context.Context, uuid string) (*Attachment, error) return attach, nil } -// GetAttachmentsByUUIDs returns attachment by given UUID list. -func GetAttachmentsByUUIDs(ctx context.Context, uuids []string) ([]*Attachment, error) { +type FindAttachmentOptions struct { + ReleaseID int64 + IssueID int64 + CommentID int64 +} + +func (opts FindAttachmentOptions) ToConds() builder.Cond { + return builder.Eq{"release_id": opts.ReleaseID, "issue_id": opts.IssueID, "comment_id": opts.CommentID} +} + +// FindRepoAttachmentsByUUID always returns attachment that has a UUID that is +// in the given `uuids` argument and is attached to the repository. +// +// The values in `opts` are always as a condition even if they are zero, this +// allows to search for attachments that are not yet attached to any resource by +// specifying a empty struct. +func FindRepoAttachmentsByUUID(ctx context.Context, repoID int64, uuids []string, opts FindAttachmentOptions) ([]*Attachment, error) { + // Nothing to match anyway. if len(uuids) == 0 { return []*Attachment{}, nil } - // Silently drop invalid uuids. + // At maximum nothing is filtered and we get all attachments via the UUID. attachments := make([]*Attachment, 0, len(uuids)) - return attachments, db.GetEngine(ctx).In("uuid", uuids).Find(&attachments) + + err := db.GetEngine(ctx). + Where("repo_id = ?", repoID). + In("uuid", uuids). + And(opts.ToConds()). + Find(&attachments) + return attachments, err } // ExistAttachmentsByUUID returns true if attachment exists with the given UUID diff --git a/models/repo/attachment_test.go b/models/repo/attachment_test.go index 23f4b3799f..2ee4ee91ed 100644 --- a/models/repo/attachment_test.go +++ b/models/repo/attachment_test.go @@ -1,9 +1,12 @@ // Copyright 2017 The Gitea Authors. All rights reserved. +// Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package repo_test import ( + "cmp" + "slices" "testing" "forgejo.org/models/db" @@ -88,14 +91,67 @@ func TestUpdateAttachment(t *testing.T) { unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{Name: "new_name"}) } -func TestGetAttachmentsByUUIDs(t *testing.T) { +func TestFindRepoAttachmentsByUUID(t *testing.T) { + defer unittest.OverrideFixtures("models/repo/fixtures/TestFindRepoAttachmentsByUUID")() require.NoError(t, unittest.PrepareTestDatabase()) - attachList, err := repo_model.GetAttachmentsByUUIDs(db.DefaultContext, []string{"a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", "not-existing-uuid"}) - require.NoError(t, err) - assert.Len(t, attachList, 2) - assert.Equal(t, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", attachList[0].UUID) - assert.Equal(t, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", attachList[1].UUID) - assert.Equal(t, int64(1), attachList[0].IssueID) - assert.Equal(t, int64(5), attachList[1].IssueID) + sort := func(x []*repo_model.Attachment) { + slices.SortFunc(x, func(a, b *repo_model.Attachment) int { + return cmp.Compare(a.ID, b.ID) + }) + } + + t.Run("Empty UUIDs", func(t *testing.T) { + attachments, err := repo_model.FindRepoAttachmentsByUUID(t.Context(), 1001, []string{}, repo_model.FindAttachmentOptions{}) + require.NoError(t, err) + assert.Empty(t, attachments) + }) + + t.Run("Wrong repository", func(t *testing.T) { + attachments, err := repo_model.FindRepoAttachmentsByUUID(t.Context(), 1002, []string{"31b6f65e-2745-4e87-b02c-e6bb9890d399", "e19fd169-c2d1-4fd0-a6d5-9658fd4affed", "758e41f6-e3b7-4420-b34f-1920da0858aa"}, repo_model.FindAttachmentOptions{}) + require.NoError(t, err) + assert.Empty(t, attachments) + }) + + t.Run("Not attached", func(t *testing.T) { + attachments, err := repo_model.FindRepoAttachmentsByUUID(t.Context(), 1001, []string{"31b6f65e-2745-4e87-b02c-e6bb9890d399", "e19fd169-c2d1-4fd0-a6d5-9658fd4affed", "758e41f6-e3b7-4420-b34f-1920da0858aa"}, repo_model.FindAttachmentOptions{}) + require.NoError(t, err) + if assert.Len(t, attachments, 1) { + assert.Equal(t, "31b6f65e-2745-4e87-b02c-e6bb9890d399", attachments[0].UUID) + } + }) + + t.Run("Issue", func(t *testing.T) { + attachments, err := repo_model.FindRepoAttachmentsByUUID(t.Context(), 1001, []string{"17bcdb6b-dd84-4da1-b37a-671165402d8d", "e19fd169-c2d1-4fd0-a6d5-9658fd4affed", "774f276e-c85d-488e-b735-7bc07860c756"}, repo_model.FindAttachmentOptions{IssueID: 1001}) + require.NoError(t, err) + sort(attachments) + if assert.Len(t, attachments, 2) { + assert.Equal(t, "17bcdb6b-dd84-4da1-b37a-671165402d8d", attachments[0].UUID) + assert.Equal(t, "e19fd169-c2d1-4fd0-a6d5-9658fd4affed", attachments[1].UUID) + } + }) + + t.Run("Comment", func(t *testing.T) { + attachments, err := repo_model.FindRepoAttachmentsByUUID(t.Context(), 1001, []string{"edf0d986-8a12-447a-a4bb-e9aefead251b", "774f276e-c85d-488e-b735-7bc07860c756", "e19fd169-c2d1-4fd0-a6d5-9658fd4affed"}, repo_model.FindAttachmentOptions{IssueID: 1001, CommentID: 1001}) + require.NoError(t, err) + if assert.Len(t, attachments, 1) { + assert.Equal(t, "edf0d986-8a12-447a-a4bb-e9aefead251b", attachments[0].UUID) + } + + attachments, err = repo_model.FindRepoAttachmentsByUUID(t.Context(), 1001, []string{"edf0d986-8a12-447a-a4bb-e9aefead251b", "774f276e-c85d-488e-b735-7bc07860c756", "e19fd169-c2d1-4fd0-a6d5-9658fd4affed"}, repo_model.FindAttachmentOptions{IssueID: 1001, CommentID: 1002}) + require.NoError(t, err) + if assert.Len(t, attachments, 1) { + assert.Equal(t, "774f276e-c85d-488e-b735-7bc07860c756", attachments[0].UUID) + } + }) + + t.Run("Release", func(t *testing.T) { + attachments, err := repo_model.FindRepoAttachmentsByUUID(t.Context(), 1001, []string{"d2570bab-c843-486f-b7b7-23e011c42815", "758e41f6-e3b7-4420-b34f-1920da0858aa", "e19fd169-c2d1-4fd0-a6d5-9658fd4affed"}, repo_model.FindAttachmentOptions{ReleaseID: 1001}) + require.NoError(t, err) + if assert.Len(t, attachments, 2) { + sort(attachments) + assert.Equal(t, "758e41f6-e3b7-4420-b34f-1920da0858aa", attachments[0].UUID) + assert.Equal(t, "d2570bab-c843-486f-b7b7-23e011c42815", attachments[1].UUID) + } + }) } diff --git a/models/repo/fixtures/TestFindRepoAttachmentsByUUID/attachment.yml b/models/repo/fixtures/TestFindRepoAttachmentsByUUID/attachment.yml new file mode 100644 index 0000000000..934f92d264 --- /dev/null +++ b/models/repo/fixtures/TestFindRepoAttachmentsByUUID/attachment.yml @@ -0,0 +1,116 @@ +- + id: 1001 + uuid: 70d3e7b8-5e46-41eb-bd2d-afaba53056bd + repo_id: 0 + issue_id: 0 + release_id: 0 + uploader_id: 0 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300000 + +- + id: 1002 + uuid: 31b6f65e-2745-4e87-b02c-e6bb9890d399 + repo_id: 1001 + issue_id: 0 + release_id: 0 + uploader_id: 1001 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300001 + +- + id: 1003 + uuid: 03158f6c-487c-4bc5-b24b-10f13e21c2e7 + repo_id: 1001 + issue_id: 0 + release_id: 0 + uploader_id: 1002 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300002 + +- + id: 1004 + uuid: 17bcdb6b-dd84-4da1-b37a-671165402d8d + repo_id: 1001 + issue_id: 1001 + release_id: 0 + uploader_id: 1001 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300003 + +- + id: 1005 + uuid: e19fd169-c2d1-4fd0-a6d5-9658fd4affed + repo_id: 1001 + issue_id: 1001 + release_id: 0 + uploader_id: 1002 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300004 + +- + id: 1006 + uuid: 758e41f6-e3b7-4420-b34f-1920da0858aa + repo_id: 1001 + issue_id: 0 + release_id: 1001 + uploader_id: 1001 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300005 + +- + id: 1007 + uuid: d2570bab-c843-486f-b7b7-23e011c42815 + repo_id: 1001 + issue_id: 0 + release_id: 1001 + uploader_id: 1002 + comment_id: 0 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300006 + +- + id: 1008 + uuid: edf0d986-8a12-447a-a4bb-e9aefead251b + repo_id: 1001 + issue_id: 1001 + release_id: 0 + uploader_id: 1001 + comment_id: 1001 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300007 + +- + id: 1009 + uuid: 774f276e-c85d-488e-b735-7bc07860c756 + repo_id: 1001 + issue_id: 1001 + release_id: 0 + uploader_id: 1002 + comment_id: 1002 + name: attach1 + download_count: 0 + size: 0 + created_unix: 1771300008 diff --git a/models/repo/release.go b/models/repo/release.go index 4efbfb352d..edd628fa0f 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -224,18 +224,14 @@ func UpdateRelease(ctx context.Context, rel *Release) error { } // AddReleaseAttachments adds a release attachments -func AddReleaseAttachments(ctx context.Context, releaseID int64, attachmentUUIDs []string) (err error) { - // Check attachments - attachments, err := GetAttachmentsByUUIDs(ctx, attachmentUUIDs) +func AddReleaseAttachments(ctx context.Context, release *Release, attachmentUUIDs []string) (err error) { + attachments, err := FindRepoAttachmentsByUUID(ctx, release.RepoID, attachmentUUIDs, FindAttachmentOptions{}) if err != nil { - return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %w", attachmentUUIDs, err) + return fmt.Errorf("FindRepoAttachmentsByUUID[uuids=%q,repoID=%d]: %w", attachmentUUIDs, release.RepoID, err) } for i := range attachments { - if attachments[i].ReleaseID != 0 { - return util.NewPermissionDeniedErrorf("release permission denied") - } - attachments[i].ReleaseID = releaseID + attachments[i].ReleaseID = release.ID // No assign value could be 0, so ignore AllCols(). 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/httplib/url.go b/modules/httplib/url.go index 32a02e3277..66ea77add7 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -1,20 +1,26 @@ // Copyright 2023 The Gitea Authors. All rights reserved. +// Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package httplib import ( "net/url" + "path" "strings" "forgejo.org/modules/setting" ) +// Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH" +// Therefore we should ignore these redirect locations to prevent open redirects. +func isBrowserRedirect(s string) bool { + return len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\') +} + // IsRiskyRedirectURL returns true if the URL is considered risky for redirects func IsRiskyRedirectURL(s string) bool { - // Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH" - // Therefore we should ignore these redirect locations to prevent open redirects - if len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\') { + if isBrowserRedirect(s) { return true } @@ -23,5 +29,12 @@ func IsRiskyRedirectURL(s string) bool { return true } + // If the path contains `..` then it's still possible this is seen + // as a browser redirect, use `path.Clean` to eliminate each inner `..` + // and then check if that might be a browser redirect. + if strings.Contains(u.Path, "..") { + return isBrowserRedirect(path.Clean(u.Path)) + } + return false } diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index cd2ceac267..0fd59c13a6 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -29,7 +29,6 @@ func TestIsRiskyRedirectURL(t *testing.T) { {"/sub/foo", false}, {"http://localhost:3000/sub/foo", false}, {"http://localhost:3000/sub/test?param=false", false}, - // FIXME: should probably be true (would requires resolving references using setting.appURL.ResolveReference(u)) {"/sub/../", false}, {"http://localhost:3000/sub/../", false}, {"/sUb/", false}, @@ -58,10 +57,12 @@ func TestIsRiskyRedirectURL(t *testing.T) { {"://missing protocol scheme", true}, // FIXME: should probably be false {"//localhost:3000/sub/test?param=false", true}, + {"/a/../\\example.com", true}, + {"/a/%2e%2e/\\example.com", true}, } for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { - assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input)) + assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input), tt.input) }) } } @@ -114,6 +115,8 @@ func TestIsRiskyRedirectURLWithoutSubURL(t *testing.T) { {"https://next.forgejo.org", true}, {"//next.forgejo.org/test?param=false", true}, {"//next.forgejo.org/sub/test?param=false", true}, + {"/a/../\\example.com", true}, + {"/a/%2e%2e/\\example.com", true}, } for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index ff67abe384..f87a3a6fdb 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -61,6 +61,7 @@ "issues.updated": "updated %s", "issues.filters.labels.exclude": "Exclude label", "issues.filters.labels.unexclude": "Clear exclusion", + "repo.pulls.auto_merge.no_permission": "You do not have permission to cancel this pull request's auto merge.", "repo.pulls.poster_manage_approval": "Manage approval", "repo.pulls.poster_requires_approval": "Some workflows are waiting to be reviewed.", "repo.pulls.poster_requires_approval.tooltip": "The author of this pull request is not trusted to run workflows triggered by a pull request created from a forked repository or with AGit. The workflows triggered by a `pull_request` event will not run until they are approved.", diff --git a/release-notes/11514.md b/release-notes/11514.md new file mode 100644 index 0000000000..078e479e6a --- /dev/null +++ b/release-notes/11514.md @@ -0,0 +1,7 @@ +- fix: PKCE challenges to Forgejo's OAuth identity provider were not validated when using the `S256` algorithm +- fix: Forgejo supports using an OAuth Bearer token with HTTP basic authentication, rather than Bearer token authentication, but did not properly apply the limited scopes of the OAuth grant +- fix: missing permission checks in attachment-related web endpoints allowed modifying attachments that a user did not own +- fix: email notifications for new releases could be sent to users that no longer access to the repository, or to inactive users +- fix: missing permission checks in user/org-owned projects would allow modifications of the open/closed state to be made to projects via insecure direct object references +- fix: missing permission checks in a web endpoint allowed cancellation of the automerge of a PR +- fix: prevent additional path-traversals in post-login redirect parameters that allowed for arbitrary redirects diff --git a/release-notes/11515.md b/release-notes/11515.md new file mode 100644 index 0000000000..078e479e6a --- /dev/null +++ b/release-notes/11515.md @@ -0,0 +1,7 @@ +- fix: PKCE challenges to Forgejo's OAuth identity provider were not validated when using the `S256` algorithm +- fix: Forgejo supports using an OAuth Bearer token with HTTP basic authentication, rather than Bearer token authentication, but did not properly apply the limited scopes of the OAuth grant +- fix: missing permission checks in attachment-related web endpoints allowed modifying attachments that a user did not own +- fix: email notifications for new releases could be sent to users that no longer access to the repository, or to inactive users +- fix: missing permission checks in user/org-owned projects would allow modifications of the open/closed state to be made to projects via insecure direct object references +- fix: missing permission checks in a web endpoint allowed cancellation of the automerge of a PR +- fix: prevent additional path-traversals in post-login redirect parameters that allowed for arbitrary redirects diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 2e5babdf31..cae39a1466 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -14,6 +14,7 @@ import ( "forgejo.org/models" activities_model "forgejo.org/models/activities" + "forgejo.org/models/db" git_model "forgejo.org/models/git" issues_model "forgejo.org/models/issues" access_model "forgejo.org/models/perm/access" @@ -1373,33 +1374,18 @@ func CancelScheduledAutoMerge(ctx *context.APIContext) { return } - exist, autoMerge, err := pull_model.GetScheduledMergeByPullID(ctx, pull.ID) - if err != nil { - ctx.InternalServerError(err) - return - } - if !exist { - ctx.NotFound() - return - } - - if ctx.Doer.ID != autoMerge.DoerID { - allowed, err := access_model.IsUserRepoAdmin(ctx, ctx.Repo.Repository, ctx.Doer) - if err != nil { - ctx.InternalServerError(err) - return - } - if !allowed { + if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull, ctx.Repo.Permission); err != nil { + switch { + case errors.Is(err, util.ErrPermissionDenied): ctx.Error(http.StatusForbidden, "No permission to cancel", "user has no permission to cancel the scheduled auto merge") - return + case db.IsErrNotExist(err): + ctx.NotFound() + default: + ctx.InternalServerError(err) } + return } - - if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull); err != nil { - ctx.InternalServerError(err) - } else { - ctx.Status(http.StatusNoContent) - } + ctx.Status(http.StatusNoContent) } // GetPullRequestCommits gets all commits associated with a given PR diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 1fd4a0311b..8e8ede0008 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -480,8 +480,7 @@ func AuthorizeOAuth(ctx *context.Context) { // pkce support switch form.CodeChallengeMethod { - case "S256": - case "plain": + case "S256", "plain": if err := ctx.Session.Set("CodeChallengeMethod", form.CodeChallengeMethod); err != nil { handleAuthorizeError(ctx, AuthorizeError{ ErrorCode: ErrorCodeServerError, diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 66c560f55e..a492d85d84 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -218,8 +218,13 @@ func ChangeProjectStatus(ctx *context.Context) { } id := ctx.ParamsInt64(":id") - if err := project_model.ChangeProjectStatusByRepoIDAndID(ctx, 0, id, toClose); err != nil { - ctx.NotFoundOrServerError("ChangeProjectStatusByRepoIDAndID", project_model.IsErrProjectNotExist, err) + project, err := project_model.GetProjectForUserByID(ctx, ctx.ContextUser.ID, id) + if err != nil { + ctx.NotFoundOrServerError("GetProjectForUserByID", project_model.IsErrProjectNotExist, err) + return + } + if err := project_model.ChangeProjectStatus(ctx, project, toClose); err != nil { + ctx.ServerError("ChangeProjectStatus", err) return } ctx.JSONRedirect(project_model.ProjectLinkForOrg(ctx.ContextUser, id)) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 10d3e6651b..cc09651e97 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -3682,7 +3682,7 @@ func updateAttachments(ctx *context.Context, item any, files []string) error { if len(files) > 0 { switch content := item.(type) { case *issues_model.Issue: - err = issues_model.UpdateIssueAttachments(ctx, content.ID, files) + err = issues_model.UpdateIssueAttachments(ctx, content, files) case *issues_model.Comment: err = content.UpdateAttachments(ctx, files) default: diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index ecc1cc89d9..e3e9ce0eb7 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -192,8 +192,13 @@ func ChangeProjectStatus(ctx *context.Context) { } id := ctx.ParamsInt64(":id") - if err := project_model.ChangeProjectStatusByRepoIDAndID(ctx, ctx.Repo.Repository.ID, id, toClose); err != nil { - ctx.NotFoundOrServerError("ChangeProjectStatusByRepoIDAndID", project_model.IsErrProjectNotExist, err) + project, err := project_model.GetProjectForRepoByID(ctx, ctx.Repo.Repository.ID, id) + if err != nil { + ctx.NotFoundOrServerError("GetProjectForRepoByID", project_model.IsErrProjectNotExist, err) + return + } + if err := project_model.ChangeProjectStatus(ctx, project, toClose); err != nil { + ctx.ServerError("ChangeProjectStatus", err) return } ctx.JSONRedirect(project_model.ProjectLinkForRepo(ctx.Repo.Repository, id)) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 69710824a4..781a001fba 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1538,17 +1538,22 @@ func CancelAutoMergePullRequest(ctx *context.Context) { return } - if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, issue.PullRequest); err != nil { - if db.IsErrNotExist(err) { + if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, issue.PullRequest, ctx.Repo.Permission); err != nil { + switch { + case errors.Is(err, util.ErrPermissionDenied): + ctx.Flash.Error(ctx.Tr("repo.pulls.auto_merge.no_permission")) + ctx.Redirect(issue.HTMLURL()) + case db.IsErrNotExist(err): ctx.Flash.Error(ctx.Tr("repo.pulls.auto_merge_not_scheduled")) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index)) - return + ctx.Redirect(issue.HTMLURL()) + default: + ctx.ServerError("RemoveScheduledAutoMerge", err) } - ctx.ServerError("RemoveScheduledAutoMerge", err) return } + ctx.Flash.Success(ctx.Tr("repo.pulls.auto_merge_canceled_schedule")) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index)) + ctx.Redirect(issue.HTMLURL()) } func stopTimerIfAvailable(ctx *context.Context, user *user_model.User, issue *issues_model.Issue) error { diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index b6f763842b..420e95d1eb 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -24,6 +24,9 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R if attach.RepoID == 0 { return nil, fmt.Errorf("attachment %s should belong to a repository", attach.Name) } + if attach.UploaderID == 0 { + return nil, fmt.Errorf("attachment %s should have a uploader", attach.Name) + } err := db.WithTx(ctx, func(ctx context.Context) error { attach.UUID = uuid.New().String() @@ -48,6 +51,9 @@ func NewExternalAttachment(ctx context.Context, attach *repo_model.Attachment) ( if attach.RepoID == 0 { return nil, fmt.Errorf("attachment %s should belong to a repository", attach.Name) } + if attach.UploaderID == 0 { + return nil, fmt.Errorf("attachment %s should have a uploader", attach.Name) + } if attach.ExternalURL == "" { return nil, fmt.Errorf("attachment %s should have a external url", attach.Name) } diff --git a/services/auth/basic.go b/services/auth/basic.go index 4125c914a9..502d806a12 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -73,7 +73,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore } // check oauth2 token - uid, _ := CheckOAuthAccessToken(req.Context(), authToken) + uid, grantScopes := CheckOAuthAccessToken(req.Context(), authToken) if uid != 0 { log.Trace("Basic Authorization: Valid OAuthAccessToken for user[%d]", uid) @@ -84,6 +84,11 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore } store.GetData()["IsApiToken"] = true + if grantScopes != "" { + store.GetData()["ApiTokenScope"] = auth_model.AccessTokenScope(grantScopes) + } else { + store.GetData()["ApiTokenScope"] = auth_model.AccessTokenScopeAll // fallback to all + } return u, nil } diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index 0cdc113379..099d048927 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -20,6 +20,7 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/process" "forgejo.org/modules/queue" + "forgejo.org/modules/util" notify_service "forgejo.org/services/notify" pull_service "forgejo.org/services/pull" repo_service "forgejo.org/services/repository" @@ -67,7 +68,25 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_ } // RemoveScheduledAutoMerge cancels a previously scheduled pull request -func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest) error { +func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, repoPerms access_model.Permission) error { + exist, autoMerge, err := pull_model.GetScheduledMergeByPullID(ctx, pull.ID) + if err != nil { + return err + } + if !exist { + return db.ErrNotExist{Resource: "auto_merge", ID: pull.ID} + } + + if doer.ID != autoMerge.DoerID { + allowed, err := pull_service.IsUserAllowedToMerge(ctx, pull, repoPerms, doer) + if err != nil { + return err + } + if !allowed { + return util.ErrPermissionDenied + } + } + return db.WithTx(ctx, func(ctx context.Context) error { if err := pull_model.DeleteScheduledAutoMerge(ctx, pull.ID); err != nil { return err diff --git a/services/automerge/automerge_test.go b/services/automerge/automerge_test.go new file mode 100644 index 0000000000..38ed4dcd9f --- /dev/null +++ b/services/automerge/automerge_test.go @@ -0,0 +1,55 @@ +// Copyright 2026 Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package automerge + +import ( + "testing" + + "forgejo.org/models/db" + issues_model "forgejo.org/models/issues" + "forgejo.org/models/perm" + access_model "forgejo.org/models/perm/access" + pull_model "forgejo.org/models/pull" + "forgejo.org/models/unit" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/util" + + "github.com/stretchr/testify/require" +) + +func TestRemoveScheduledAutoMerge(t *testing.T) { + defer unittest.OverrideFixtures("services/automerge/fixtures/TestRemoveScheduledAutoMerge")() + require.NoError(t, unittest.PrepareTestDatabase()) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + pull1 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}) + pull2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + + t.Run("No automerge", func(t *testing.T) { + err := RemoveScheduledAutoMerge(t.Context(), user5, pull2, access_model.Permission{}) + require.ErrorIs(t, err, db.ErrNotExist{Resource: "auto_merge", ID: 2}) + }) + + t.Run("No permission", func(t *testing.T) { + err := RemoveScheduledAutoMerge(t.Context(), user5, pull1, access_model.Permission{}) + require.ErrorIs(t, err, util.ErrPermissionDenied) + + err = RemoveScheduledAutoMerge(t.Context(), user5, pull1, access_model.Permission{UnitsMode: map[unit.Type]perm.AccessMode{ + unit.TypePullRequests: perm.AccessModeRead, + }}) + require.ErrorIs(t, err, util.ErrPermissionDenied) + }) + + t.Run("Normal", func(t *testing.T) { + err := RemoveScheduledAutoMerge(t.Context(), user2, pull1, access_model.Permission{UnitsMode: map[unit.Type]perm.AccessMode{ + unit.TypePullRequests: perm.AccessModeWrite, + }}) + require.NoError(t, err) + + unittest.AssertExistsIf(t, false, &pull_model.AutoMerge{PullID: pull1.ID}) + unittest.AssertExistsIf(t, true, &issues_model.Comment{IssueID: pull1.IssueID, Type: issues_model.CommentTypePRUnScheduledToAutoMerge}) + }) +} diff --git a/services/automerge/fixtures/TestRemoveScheduledAutoMerge/pull_auto_merge.yml b/services/automerge/fixtures/TestRemoveScheduledAutoMerge/pull_auto_merge.yml new file mode 100644 index 0000000000..2cacb611c3 --- /dev/null +++ b/services/automerge/fixtures/TestRemoveScheduledAutoMerge/pull_auto_merge.yml @@ -0,0 +1,8 @@ +- + id: 1001 + pull_id: 1 + doer_id: 2 + merge_style: "merge" + message: "Automatically merged" + delete_branch_after_merge: false + created_unix: 1771800000 diff --git a/services/automerge/main_test.go b/services/automerge/main_test.go new file mode 100644 index 0000000000..8c0e558b17 --- /dev/null +++ b/services/automerge/main_test.go @@ -0,0 +1,14 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package automerge + +import ( + "testing" + + "forgejo.org/models/unittest" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m) +} diff --git a/services/mailer/fixtures/TestMailNewRelease/watch.yml b/services/mailer/fixtures/TestMailNewRelease/watch.yml new file mode 100644 index 0000000000..4c488a6f85 --- /dev/null +++ b/services/mailer/fixtures/TestMailNewRelease/watch.yml @@ -0,0 +1,5 @@ +- + id: 1001 + user_id: 11 + repo_id: 41 + mode: 1 diff --git a/services/mailer/mail_release.go b/services/mailer/mail_release.go index 0f2ef33fe1..2111083bd4 100644 --- a/services/mailer/mail_release.go +++ b/services/mailer/mail_release.go @@ -6,8 +6,11 @@ package mailer import ( "bytes" "context" + "slices" + access_model "forgejo.org/models/perm/access" repo_model "forgejo.org/models/repo" + "forgejo.org/models/unit" user_model "forgejo.org/models/user" "forgejo.org/modules/base" "forgejo.org/modules/log" @@ -40,6 +43,12 @@ func MailNewRelease(ctx context.Context, rel *repo_model.Release) { return } + // Users are not eligible to receive this mail if they are not active or + // they don't have permissions to read releases. + recipients = slices.DeleteFunc(recipients, func(u *user_model.User) bool { + return !u.IsActive || !access_model.CheckRepoUnitUser(ctx, rel.Repo, u, unit.TypeReleases) + }) + langMap := make(map[string][]*user_model.User) for _, user := range recipients { if user.ID != rel.PublisherID { diff --git a/services/mailer/mail_release_test.go b/services/mailer/mail_release_test.go new file mode 100644 index 0000000000..4b37afb8cd --- /dev/null +++ b/services/mailer/mail_release_test.go @@ -0,0 +1,122 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package mailer_test + +import ( + "testing" + + "forgejo.org/models/db" + repo_model "forgejo.org/models/repo" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/gitrepo" + "forgejo.org/services/mailer" + release_service "forgejo.org/services/release" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMailNewRelease(t *testing.T) { + defer unittest.OverrideFixtures("services/mailer/fixtures/TestMailNewRelease")() + defer require.NoError(t, unittest.PrepareTestDatabase()) + + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user11 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 11}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + gitRepo, err := gitrepo.OpenRepository(t.Context(), repo) + require.NoError(t, err) + defer gitRepo.Close() + + t.Run("Normal", func(t *testing.T) { + called := false + + defer mailer.MockMailSettings(func(msgs ...*mailer.Message) { + assert.Len(t, msgs, 2) + if user1.EmailTo() == msgs[0].To { + assert.Equal(t, user11.EmailTo(), msgs[1].To) + } else { + assert.Equal(t, user11.EmailTo(), msgs[0].To) + assert.Equal(t, user1.EmailTo(), msgs[1].To) + } + + assert.Equal(t, "v0.1 in user2/repo1 released", msgs[0].Subject) + called = true + })() + + require.NoError(t, release_service.CreateRelease(gitRepo, &repo_model.Release{ + RepoID: repo.ID, + Repo: repo, + PublisherID: user2.ID, + Publisher: user2, + TagName: "v0.1", + Target: "master", + Title: "v0.1 is released", + Note: "v0.1 is released", + }, "", []*release_service.AttachmentChange{})) + + assert.True(t, called) + }) + + t.Run("Non-active user", func(t *testing.T) { + _, err := db.GetEngine(db.DefaultContext).Exec("UPDATE user SET is_active=false WHERE id=11") + require.NoError(t, err) + + t.Cleanup(func() { + _, err := db.GetEngine(db.DefaultContext).Exec("UPDATE user SET is_active=true WHERE id=11") + require.NoError(t, err) + }) + called := false + + defer mailer.MockMailSettings(func(msgs ...*mailer.Message) { + assert.Len(t, msgs, 1) + assert.Equal(t, user1.EmailTo(), msgs[0].To) + + assert.Equal(t, "v0.2 in user2/repo1 released", msgs[0].Subject) + called = true + })() + + require.NoError(t, release_service.CreateRelease(gitRepo, &repo_model.Release{ + RepoID: repo.ID, + Repo: repo, + PublisherID: user2.ID, + Publisher: user2, + TagName: "v0.2", + Target: "master", + Title: "v0.2 is released", + Note: "v0.2 is released", + }, "", []*release_service.AttachmentChange{})) + + assert.True(t, called) + }) + + t.Run("No permissions for releases", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 41}) + + gitRepo, err := gitrepo.OpenRepository(t.Context(), repo) + require.NoError(t, err) + defer gitRepo.Close() + + called := false + + defer mailer.MockMailSettings(func(msgs ...*mailer.Message) { + called = true + })() + + require.NoError(t, release_service.CreateRelease(gitRepo, &repo_model.Release{ + RepoID: repo.ID, + Repo: repo, + PublisherID: user2.ID, + Publisher: user2, + TagName: "v0.2", + Target: "master", + Title: "v0.2 is released", + Note: "v0.2 is released", + }, "", []*release_service.AttachmentChange{})) + + assert.False(t, called) + }) +} diff --git a/services/release/release.go b/services/release/release.go index 69889a1638..9536ebf6dd 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -192,7 +192,7 @@ func CreateRelease(gitRepo *git.Repository, rel *repo_model.Release, msg string, } } - if err = repo_model.AddReleaseAttachments(gitRepo.Ctx, rel.ID, addAttachmentUUIDs.Values()); err != nil { + if err = repo_model.AddReleaseAttachments(gitRepo.Ctx, rel, addAttachmentUUIDs.Values()); err != nil { return err } @@ -314,44 +314,37 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo } } - if err = repo_model.AddReleaseAttachments(ctx, rel.ID, addAttachmentUUIDs.Values()); err != nil { + if err = repo_model.AddReleaseAttachments(ctx, rel, addAttachmentUUIDs.Values()); err != nil { return fmt.Errorf("AddReleaseAttachments: %w", err) } deletedUUIDs := make(container.Set[string]) if len(delAttachmentUUIDs) > 0 { - // Check attachments - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, delAttachmentUUIDs.Values()) + // Check delAttachments + delAttachments, err := repo_model.FindRepoAttachmentsByUUID(ctx, rel.RepoID, delAttachmentUUIDs.Values(), repo_model.FindAttachmentOptions{ReleaseID: rel.ID}) if err != nil { - return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %w", delAttachmentUUIDs, err) + return fmt.Errorf("FindRepoAttachmentsByUUID[uuids=%q,repoID=%d,releaseID=%d]: %w", delAttachmentUUIDs.Values(), rel.RepoID, rel.ID, err) } - for _, attach := range attachments { - if attach.ReleaseID != rel.ID { - return util.SilentWrap{ - Message: "delete attachment of release permission denied", - Err: util.ErrPermissionDenied, - } - } + for _, attach := range delAttachments { deletedUUIDs.Add(attach.UUID) } - if _, err := repo_model.DeleteAttachments(ctx, attachments, true); err != nil { + if _, err := repo_model.DeleteAttachments(ctx, delAttachments, true); err != nil { return fmt.Errorf("DeleteAttachments [uuids: %v]: %w", delAttachmentUUIDs, err) } } if len(updateAttachmentUUIDs) > 0 { - // Check attachments - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, updateAttachmentUUIDs.Values()) + // Check that attachments actually belong to repository and release. + attachments, err := repo_model.FindRepoAttachmentsByUUID(ctx, rel.RepoID, updateAttachmentUUIDs.Values(), repo_model.FindAttachmentOptions{ReleaseID: rel.ID}) if err != nil { - return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %w", updateAttachmentUUIDs, err) + return fmt.Errorf("FindRepoAttachmentsByUUID[uuids=%q,repoID=%d,releaseID=%d]: %w", updateAttachmentUUIDs.Values(), rel.RepoID, rel.ID, err) } - for _, attach := range attachments { - if attach.ReleaseID != rel.ID { - return util.SilentWrap{ - Message: "update attachment of release permission denied", - Err: util.ErrPermissionDenied, - } + + if len(attachments) != len(updateAttachments) { + return util.SilentWrap{ + Message: "update attachment of release permission denied", + Err: util.ErrPermissionDenied, } } } diff --git a/services/release/release_test.go b/services/release/release_test.go index 2a593862ce..e05efd9658 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -14,6 +14,7 @@ import ( "forgejo.org/modules/git" "forgejo.org/modules/gitrepo" "forgejo.org/modules/test" + "forgejo.org/modules/util" "forgejo.org/services/attachment" "github.com/stretchr/testify/assert" @@ -379,6 +380,39 @@ func TestRelease_Update(t *testing.T) { assert.Equal(t, release.ID, release.Attachments[0].ReleaseID) assert.Equal(t, "test2", release.Attachments[0].Name) assert.Equal(t, "https://about.gitea.com/", release.Attachments[0].ExternalURL) + + // delete the attachment + require.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, false, []*AttachmentChange{ + { + Action: "delete", + UUID: externalAttachmentUUID, + }, + })) + release.Attachments = nil + require.NoError(t, repo_model.GetReleaseAttachments(db.DefaultContext, release)) + assert.Empty(t, release.Attachments) + + t.Run("Permission denied", func(t *testing.T) { + require.NoError(t, UpdateRelease(t.Context(), user, gitRepo, release, false, []*AttachmentChange{ + { + Action: "add", + Type: "attachment", + UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a13", + }, + })) + require.NoError(t, repo_model.GetReleaseAttachments(t.Context(), release)) + assert.Empty(t, release.Attachments) + + require.ErrorIs(t, UpdateRelease(t.Context(), user, gitRepo, release, false, []*AttachmentChange{ + { + Action: "update", + Name: "test2.txt", + UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a13", + }, + }), util.ErrPermissionDenied) + require.NoError(t, repo_model.GetReleaseAttachments(t.Context(), release)) + assert.Empty(t, release.Attachments) + }) } func TestRelease_createTag(t *testing.T) { diff --git a/tests/integration/api_token_test.go b/tests/integration/api_token_test.go index 1b61885a9b..f3efd6fa2c 100644 --- a/tests/integration/api_token_test.go +++ b/tests/integration/api_token_test.go @@ -603,13 +603,29 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model unauthorizedScopes = append(unauthorizedScopes, categoryUnauthorizedScopes...) } - accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, unauthorizedScopes) - defer deleteAPIAccessToken(t, accessToken, user) - // Request the endpoint. Verify that permission is denied. - req := NewRequest(t, testCase.method, testCase.url). - AddTokenAuth(accessToken.Token) - MakeRequest(t, req, http.StatusForbidden) + + t.Run("Bearer", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, unauthorizedScopes) + defer deleteAPIAccessToken(t, accessToken, user) + + req := NewRequest(t, testCase.method, testCase.url). + AddTokenAuth(accessToken.Token) + MakeRequest(t, req, http.StatusForbidden) + }) + + t.Run("Basic", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + oauth2Token := createOAuth2Token(t, loginUser(t, user.Name), unauthorizedScopes) + defer unittest.AssertSuccessfulDelete(t, &auth_model.OAuth2Grant{ApplicationID: 2, UserID: user.ID}) + + req := NewRequest(t, testCase.method, testCase.url) + req.SetBasicAuth("x-oauth-basic", oauth2Token) + MakeRequest(t, req, http.StatusForbidden) + }) }) } @@ -647,6 +663,51 @@ func deleteAPIAccessToken(t *testing.T, accessToken api.AccessToken, user *user_ unittest.AssertNotExistsBean(t, &auth_model.AccessToken{ID: accessToken.ID}) } +func createOAuth2Token(t *testing.T, session *TestSession, scopes []auth_model.AccessTokenScope) string { + // Make a call to `/login/oauth/authorize` to get some session data. + session.MakeRequest(t, NewRequest(t, "GET", "/login/oauth/authorize?client_id=ce5a1322-42a7-11ed-b878-0242ac120002&redirect_uri=b&response_type=code&code_challenge_method=plain&code_challenge=CODE&state=thestate"), http.StatusOK) + + var b strings.Builder + switch len(scopes) { + case 0: + break + case 1: + b.WriteString(string(scopes[0])) + default: + b.WriteString(string(scopes[0])) + for _, s := range scopes[1:] { + b.WriteString(" ") + b.WriteString(string(s)) + } + } + + req := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "redirect_uri": "b", + "state": "thestate", + "granted": "true", + "scope": b.String(), + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + + u, err := url.Parse(test.RedirectURL(resp)) + require.NoError(t, err) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "code": u.Query().Get("code"), + "code_verifier": "CODE", + "grant_type": "authorization_code", + "redirect_uri": "b", + }) + resp = MakeRequest(t, req, http.StatusOK) + + var respBody map[string]any + DecodeJSON(t, resp, &respBody) + + return respBody["access_token"].(string) +} + func TestAPITokenCreation(t *testing.T) { defer tests.PrepareTestEnv(t)() @@ -673,39 +734,15 @@ func TestAPITokenCreation(t *testing.T) { t.Run("Via OAuth2", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - // Make a call to `/login/oauth/authorize` to get some session data. - session.MakeRequest(t, NewRequest(t, "GET", "/login/oauth/authorize?client_id=ce5a1322-42a7-11ed-b878-0242ac120002&redirect_uri=b&response_type=code&code_challenge_method=plain&code_challenge=CODE&state=thestate"), http.StatusOK) + accessToken := createOAuth2Token(t, session, nil) - req := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", - "redirect_uri": "b", - "state": "thestate", - "granted": "true", - }) - resp := session.MakeRequest(t, req, http.StatusSeeOther) - - u, err := url.Parse(test.RedirectURL(resp)) - require.NoError(t, err) - - req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", - "code": u.Query().Get("code"), - "code_verifier": "CODE", - "grant_type": "authorization_code", - "redirect_uri": "b", - }) - resp = MakeRequest(t, req, http.StatusOK) - - var respBody map[string]any - DecodeJSON(t, resp, &respBody) - - req = NewRequestWithJSON(t, "POST", "/api/v1/users/user4/tokens", map[string]any{ + req := NewRequestWithJSON(t, "POST", "/api/v1/users/user4/tokens", map[string]any{ "name": "new-new-token", "scopes": []auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteUser}, }) - req.Request.Header.Set("Authorization", "basic "+base64.StdEncoding.EncodeToString([]byte("user4:"+respBody["access_token"].(string)))) + req.SetBasicAuth("user4", accessToken) - resp = MakeRequest(t, req, http.StatusUnauthorized) + resp := MakeRequest(t, req, http.StatusUnauthorized) respMsg := map[string]any{} DecodeJSON(t, resp, &respMsg) diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 9220f03ccd..21a1ff561f 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -1535,62 +1535,127 @@ func TestSignUpViaOAuth2FA(t *testing.T) { func TestAccessTokenWithPKCE(t *testing.T) { defer tests.PrepareTestEnv(t)() - var u *url.URL - t.Run("Grant", func(t *testing.T) { - session := loginUser(t, "user4") + session := loginUser(t, "user4") - session.MakeRequest(t, NewRequest(t, "GET", "/login/oauth/authorize?client_id=ce5a1322-42a7-11ed-b878-0242ac120002&redirect_uri=b&response_type=code&code_challenge_method=plain&code_challenge=CODE&state=thestate"), http.StatusOK) - req := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", - "redirect_uri": "b", - "state": "thestate", - "granted": "true", + t.Run("Plain method", func(t *testing.T) { + defer unittest.AssertSuccessfulDelete(t, &auth_model.OAuth2Grant{UserID: 4, ApplicationID: 2}) + + var u *url.URL + t.Run("Grant", func(t *testing.T) { + session.MakeRequest(t, NewRequest(t, "GET", "/login/oauth/authorize?client_id=ce5a1322-42a7-11ed-b878-0242ac120002&redirect_uri=b&response_type=code&code_challenge_method=plain&code_challenge=CODE&state=thestate"), http.StatusOK) + req := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "redirect_uri": "b", + "state": "thestate", + "granted": "true", + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + + var err error + u, err = url.Parse(test.RedirectURL(resp)) + require.NoError(t, err) }) - resp := session.MakeRequest(t, req, http.StatusSeeOther) - var err error - u, err = url.Parse(test.RedirectURL(resp)) - require.NoError(t, err) + t.Run("Incorrect code verifier", func(t *testing.T) { + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "code": u.Query().Get("code"), + "code_verifier": "just a guess", + "grant_type": "authorization_code", + "redirect_uri": "b", + }) + resp := MakeRequest(t, req, http.StatusBadRequest) + + var respBody map[string]any + DecodeJSON(t, resp, &respBody) + + if assert.Len(t, respBody, 2) { + assert.Equal(t, "unauthorized_client", respBody["error"]) + assert.Equal(t, "failed PKCE code challenge", respBody["error_description"]) + } + }) + + t.Run("Get access token", func(t *testing.T) { + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "code": u.Query().Get("code"), + "code_verifier": "CODE", + "grant_type": "authorization_code", + "redirect_uri": "b", + }) + resp := MakeRequest(t, req, http.StatusOK) + + var respBody map[string]any + DecodeJSON(t, resp, &respBody) + + if assert.Len(t, respBody, 4) { + assert.NotEmpty(t, respBody["access_token"]) + assert.NotEmpty(t, respBody["token_type"]) + assert.NotEmpty(t, respBody["expires_in"]) + assert.NotEmpty(t, respBody["refresh_token"]) + } + }) }) - t.Run("Incorrect code verifier", func(t *testing.T) { - req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", - "code": u.Query().Get("code"), - "code_verifier": "just a guess", - "grant_type": "authorization_code", - "redirect_uri": "b", + t.Run("S256 method", func(t *testing.T) { + var u *url.URL + t.Run("Grant", func(t *testing.T) { + h := sha256.Sum256([]byte("CODE")) + hashedVerifier := base64.RawURLEncoding.EncodeToString(h[:]) + + session.MakeRequest(t, NewRequest(t, "GET", "/login/oauth/authorize?client_id=ce5a1322-42a7-11ed-b878-0242ac120002&redirect_uri=b&response_type=code&code_challenge_method=S256&code_challenge="+hashedVerifier+"&state=thestate"), http.StatusOK) + req := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "redirect_uri": "b", + "state": "thestate", + "granted": "true", + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + + var err error + u, err = url.Parse(test.RedirectURL(resp)) + require.NoError(t, err) }) - resp := MakeRequest(t, req, http.StatusBadRequest) - var respBody map[string]any - DecodeJSON(t, resp, &respBody) + t.Run("Incorrect code verifier", func(t *testing.T) { + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "code": u.Query().Get("code"), + "code_verifier": "just a guess", + "grant_type": "authorization_code", + "redirect_uri": "b", + }) + resp := MakeRequest(t, req, http.StatusBadRequest) - if assert.Len(t, respBody, 2) { - assert.Equal(t, "unauthorized_client", respBody["error"]) - assert.Equal(t, "failed PKCE code challenge", respBody["error_description"]) - } - }) + var respBody map[string]any + DecodeJSON(t, resp, &respBody) - t.Run("Get access token", func(t *testing.T) { - req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", - "code": u.Query().Get("code"), - "code_verifier": "CODE", - "grant_type": "authorization_code", - "redirect_uri": "b", + if assert.Len(t, respBody, 2) { + assert.Equal(t, "unauthorized_client", respBody["error"]) + assert.Equal(t, "failed PKCE code challenge", respBody["error_description"]) + } }) - resp := MakeRequest(t, req, http.StatusOK) - var respBody map[string]any - DecodeJSON(t, resp, &respBody) + t.Run("Get access token", func(t *testing.T) { + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "code": u.Query().Get("code"), + "code_verifier": "CODE", + "grant_type": "authorization_code", + "redirect_uri": "b", + }) + resp := MakeRequest(t, req, http.StatusOK) - if assert.Len(t, respBody, 4) { - assert.NotEmpty(t, respBody["access_token"]) - assert.NotEmpty(t, respBody["token_type"]) - assert.NotEmpty(t, respBody["expires_in"]) - assert.NotEmpty(t, respBody["refresh_token"]) - } + var respBody map[string]any + DecodeJSON(t, resp, &respBody) + + if assert.Len(t, respBody, 4) { + assert.NotEmpty(t, respBody["access_token"]) + assert.NotEmpty(t, respBody["token_type"]) + assert.NotEmpty(t, respBody["expires_in"]) + assert.NotEmpty(t, respBody["refresh_token"]) + } + }) }) } diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index b8fcafab7a..955caaf6f7 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -1,4 +1,5 @@ // Copyright 2023 The Gitea Authors. All rights reserved. +// Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package integration @@ -78,3 +79,84 @@ func TestMoveRepoProjectColumns(t *testing.T) { require.NoError(t, project_model.DeleteProjectByID(db.DefaultContext, project1.ID)) } + +func TestChangeStatusProject(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user5 := loginUser(t, "user5") + user2 := loginUser(t, "user2") + + t.Run("User", func(t *testing.T) { + project4CloseURL := "/user2/-/projects/4/close" + + t.Run("Doer is not context user", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user5.MakeRequest(t, NewRequest(t, "POST", project4CloseURL), http.StatusNotFound) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 4}, "is_closed = false") + }) + + t.Run("Wrong ID", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user5.MakeRequest(t, NewRequest(t, "POST", "/user5/-/projects/4/close"), http.StatusNotFound) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 4}, "is_closed = false") + + user5.MakeRequest(t, NewRequest(t, "POST", "/user5/-/projects/1/close"), http.StatusNotFound) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 1}, "is_closed = false") + + user5.MakeRequest(t, NewRequest(t, "POST", "/user5/-/projects/7/close"), http.StatusNotFound) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 7}, "is_closed = false") + }) + + t.Run("Normal", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user2.MakeRequest(t, NewRequest(t, "POST", project4CloseURL), http.StatusOK) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 4}, "is_closed = true") + }) + }) + + t.Run("Organization", func(t *testing.T) { + project7CloseURL := "/org3/-/projects/7/close" + + t.Run("Doer does not have permission", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user5.MakeRequest(t, NewRequest(t, "POST", project7CloseURL), http.StatusNotFound) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 7}, "is_closed = false") + }) + + t.Run("Normal", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user2.MakeRequest(t, NewRequest(t, "POST", project7CloseURL), http.StatusOK) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 7}, "is_closed = true") + }) + }) + + t.Run("Repository", func(t *testing.T) { + project1CloseURL := "/user2/repo1/projects/1/close" + + t.Run("Doer does not have permission", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user5.MakeRequest(t, NewRequest(t, "POST", project1CloseURL), http.StatusNotFound) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 1}, "is_closed = false") + }) + + t.Run("Wrong ID", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user5.MakeRequest(t, NewRequest(t, "POST", "/user5/repo4/projects/1/close"), http.StatusNotFound) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 1}, "is_closed = false") + }) + + t.Run("Normal", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user2.MakeRequest(t, NewRequest(t, "POST", project1CloseURL), http.StatusOK) + unittest.AssertExistsIf(t, true, &project_model.Project{ID: 1}, "is_closed = true") + }) + }) +} diff --git a/tests/integration/pull_auto_merge_test.go b/tests/integration/pull_auto_merge_test.go new file mode 100644 index 0000000000..e9a428d4e7 --- /dev/null +++ b/tests/integration/pull_auto_merge_test.go @@ -0,0 +1,106 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package integration + +import ( + "fmt" + "net/http" + "net/url" + "os" + "path" + "strings" + "testing" + "time" + + unit_model "forgejo.org/models/unit" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/git" + app_context "forgejo.org/services/context" + files_service "forgejo.org/services/repository/files" + "forgejo.org/tests" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPullRemoveAutomerge(t *testing.T) { + onApplicationRun(t, func(t *testing.T, u *url.URL) { + user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + user5Session := loginUser(t, user5.Name) + user2Session := loginUser(t, "user2") + + repo, _, f := tests.CreateDeclarativeRepo(t, user5, "", + []unit_model.Type{unit_model.TypeCode, unit_model.TypePullRequests}, nil, + []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "FUNFACT", + ContentReader: strings.NewReader( + "The Netherlands got its first openly gay prime minister today."), + }, + }, + ) + defer f() + + dstPath := t.TempDir() + cloneURL, _ := url.Parse(fmt.Sprintf("%suser5/%s.git", u.String(), repo.Name)) + cloneURL.User = url.UserPassword("user5", userPassword) + require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) + doGitSetRemoteURL(dstPath, "origin", cloneURL)(t) + + require.NoError(t, git.NewCommand(t.Context(), "switch", "-c", "new-fun-fact").Run(&git.RunOpts{Dir: dstPath})) + + require.NoError(t, os.WriteFile(path.Join(dstPath, "README.md"), []byte("The house of representative already had that in 1937."), 0o600)) + require.NoError(t, git.AddChanges(dstPath, true)) + require.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Author: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Message: "Update funfact.", + })) + + require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=new-fun-fact").Run(&git.RunOpts{Dir: dstPath})) + + // Create a protected branch rule for automerge. + user5Session.MakeRequest(t, NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/settings/branches/edit", repo.FullName()), map[string]string{ + "rule_name": "main", + "required_approvals": "1", + }), http.StatusSeeOther) + + // Start a automerge for new pull request. + user5Session.MakeRequest(t, NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/pulls/1/merge", repo.FullName()), map[string]string{ + "merge_message_field": "I love automation when it works", + "do": "merge", + "merge_when_checks_succeed": "true", + }), http.StatusOK) + + t.Run("No permission", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user2Session.MakeRequest(t, NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/pulls/1/cancel_auto_merge", repo.FullName()), nil), http.StatusSeeOther) + + flashCookie := user2Session.GetCookie(app_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Equal(t, "error%3DYou%2Bdo%2Bnot%2Bhave%2Bpermission%2Bto%2Bcancel%2Bthis%2Bpull%2Brequest%2527s%2Bauto%2Bmerge.", flashCookie.Value) + }) + + t.Run("Normal", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + user5Session.MakeRequest(t, NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/pulls/1/cancel_auto_merge", repo.FullName()), nil), http.StatusSeeOther) + + flashCookie := user5Session.GetCookie(app_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Equal(t, "success%3DThe%2Bauto%2Bmerge%2Bwas%2Bcanceled%2Bfor%2Bthis%2Bpull%2Brequest.", flashCookie.Value) + }) + }) +}