diff --git a/models/issues/assignees.go b/models/issues/assignees.go index 994e9f0ce91..6324c9c8ffb 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -9,7 +9,6 @@ import ( "gitea.dev/models/db" user_model "gitea.dev/models/user" - "gitea.dev/modules/util" "xorm.io/builder" ) @@ -172,21 +171,3 @@ func toggleUserAssignee(ctx context.Context, issue *Issue, assigneeID int64) (re return found, nil } - -// MakeIDsFromAPIAssigneesToAdd returns an array with all assignee IDs -func MakeIDsFromAPIAssigneesToAdd(ctx context.Context, oneAssignee string, multipleAssignees []string) ([]int64, error) { - var requestAssignees []string - - // Keeping the old assigning method for compatibility reasons - if oneAssignee != "" && !util.SliceContainsString(multipleAssignees, oneAssignee) { - requestAssignees = append(requestAssignees, oneAssignee) - } - - // Prevent empty assignees - if len(multipleAssignees) > 0 && multipleAssignees[0] != "" { - requestAssignees = append(requestAssignees, multipleAssignees...) - } - - // Get the IDs of all assignees - return user_model.GetDoerUserIDsByNames(ctx, requestAssignees) -} diff --git a/models/issues/assignees_test.go b/models/issues/assignees_test.go index 57c52626071..460688be3b6 100644 --- a/models/issues/assignees_test.go +++ b/models/issues/assignees_test.go @@ -65,29 +65,3 @@ func TestUpdateAssignee(t *testing.T) { assert.NoError(t, err) assert.False(t, isAssigned) } - -func TestMakeIDsFromAPIAssigneesToAdd(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - _ = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - _ = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - IDs, err := issues_model.MakeIDsFromAPIAssigneesToAdd(t.Context(), "", []string{""}) - assert.NoError(t, err) - assert.Equal(t, []int64{}, IDs) - - _, err = issues_model.MakeIDsFromAPIAssigneesToAdd(t.Context(), "", []string{"none_existing_user"}) - assert.Error(t, err) - - IDs, err = issues_model.MakeIDsFromAPIAssigneesToAdd(t.Context(), "user1", []string{"user1"}) - assert.NoError(t, err) - assert.Equal(t, []int64{1}, IDs) - - IDs, err = issues_model.MakeIDsFromAPIAssigneesToAdd(t.Context(), "user2", []string{""}) - assert.NoError(t, err) - assert.Equal(t, []int64{2}, IDs) - - IDs, err = issues_model.MakeIDsFromAPIAssigneesToAdd(t.Context(), "", []string{"user1", "user2"}) - assert.NoError(t, err) - assert.Equal(t, []int64{1, 2}, IDs) -} diff --git a/models/user/user.go b/models/user/user.go index 8ba76136005..7c4404d376d 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1180,6 +1180,15 @@ func GetMailableUsersByIDs(ctx context.Context, ids []int64, isMention bool) ([] } func getDoerUserIDsByNames(ctx context.Context, names []string, allowSystemDoer bool) ([]int64, error) { + users, err := getDoerUsersByNames(ctx, names, allowSystemDoer) + ids := make([]int64, 0, len(users)) + for _, u := range users { + ids = append(ids, u.ID) + } + return ids, err +} + +func getDoerUsersByNames(ctx context.Context, names []string, allowSystemDoer bool) ([]*User, error) { namesLower := make([]string, 0, len(names)) namesMap := make(map[string]*User, len(names)) for _, name := range names { @@ -1193,16 +1202,16 @@ func getDoerUserIDsByNames(ctx context.Context, names []string, allowSystemDoer } } - var users []*User - err := db.GetEngine(ctx).In("lower_name", namesLower).Where("`type` IN (?,?)", UserTypeIndividual, UserTypeBot).Find(&users) + var dbUsers []*User + err := db.GetEngine(ctx).In("lower_name", namesLower).Where("`type` IN (?,?)", UserTypeIndividual, UserTypeBot).Find(&dbUsers) if err != nil { return nil, err } - for _, user := range users { + for _, user := range dbUsers { namesMap[user.LowerName] = user } - ids := make([]int64, 0, len(names)) + users := make([]*User, 0, len(names)) var errFinal error for _, nameLower := range namesLower { u := namesMap[nameLower] @@ -1210,9 +1219,13 @@ func getDoerUserIDsByNames(ctx context.Context, names []string, allowSystemDoer errFinal = ErrUserNotExist{Name: nameLower} continue } - ids = append(ids, u.ID) + users = append(users, u) } - return ids, errFinal + return users, errFinal +} + +func GetDoerUsersByNames(ctx context.Context, names []string) ([]*User, error) { + return getDoerUsersByNames(ctx, names, false) } func GetDoerUserIDsByNamesWithSystem(ctx context.Context, names []string) ([]int64, error) { diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 6a20b0ff3c6..565b0c092b2 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -14,9 +14,9 @@ import ( "gitea.dev/models/db" issues_model "gitea.dev/models/issues" access_model "gitea.dev/models/perm/access" - repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" user_model "gitea.dev/models/user" + "gitea.dev/modules/container" issue_indexer "gitea.dev/modules/indexer/issues" "gitea.dev/modules/optional" "gitea.dev/modules/setting" @@ -526,6 +526,39 @@ func GetIssue(ctx *context.APIContext) { ctx.JSON(http.StatusOK, convert.ToAPIIssue(ctx, ctx.Doer, issue)) } +func loadAssigneeUserIDs(ctx *context.APIContext, oneAssignee string, multipleAssignees []string) []int64 { + assigneeNames := container.Set[string]{} + assigneeNames.Add(strings.ToLower(oneAssignee)) + for _, assignee := range multipleAssignees { + assigneeNames.Add(strings.ToLower(assignee)) + } + assigneeNames.Remove("") + assignees, err := user_model.GetDoerUsersByNames(ctx, assigneeNames.Values()) + if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("Assignee does not exist: %s", err)) + } else { + ctx.APIErrorInternal(err) + } + return nil + } + + // Check if the passed assignees is assignable + assigneeIDs := make([]int64, 0, len(assignees)) + for _, assignee := range assignees { + valid, err := access_model.CanBeAssigned(ctx, assignee, ctx.Repo.Repository) + if err != nil { + ctx.APIErrorAuto(err) + return nil + } else if !valid { + ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("User %s cannot be assigned to this issue", assignee.Name)) + return nil + } + assigneeIDs = append(assigneeIDs, assignee.ID) + } + return assigneeIDs +} + // CreateIssue create an issue of a repository func CreateIssue(ctx *context.APIContext) { // swagger:operation POST /repos/{owner}/{repo}/issues issue issueCreateIssue @@ -583,34 +616,10 @@ func CreateIssue(ctx *context.APIContext) { return } issue.MilestoneID = form.Milestone - assigneeIDs, err = issues_model.MakeIDsFromAPIAssigneesToAdd(ctx, form.Assignee, form.Assignees) - if err != nil { - if errors.Is(err, util.ErrNotExist) { - ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("Assignee does not exist: %s", err)) - } else { - ctx.APIErrorInternal(err) - } + assigneeIDs = loadAssigneeUserIDs(ctx, form.Assignee, form.Assignees) + if ctx.Written() { return } - - // Check if the passed assignees is assignable - for _, aID := range assigneeIDs { - assignee, err := user_model.GetUserByID(ctx, aID) - if err != nil { - ctx.APIErrorInternal(err) - return - } - - valid, err := access_model.CanBeAssigned(ctx, assignee, ctx.Repo.Repository) - if err != nil { - ctx.APIErrorInternal(err) - return - } - if !valid { - ctx.APIError(http.StatusUnprocessableEntity, repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: ctx.Repo.Repository.Name}.Error()) - return - } - } } else { // setting labels is not allowed if user is not a writer form.Labels = make([]int64, 0) diff --git a/routers/api/v1/repo/issue_test.go b/routers/api/v1/repo/issue_test.go new file mode 100644 index 00000000000..f07503acde8 --- /dev/null +++ b/routers/api/v1/repo/issue_test.go @@ -0,0 +1,40 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "net/http" + "testing" + + "gitea.dev/models/unittest" + "gitea.dev/services/contexttest" + + "github.com/stretchr/testify/assert" +) + +func TestLoadAssigneeUserIDs(t *testing.T) { + unittest.PrepareTestEnv(t) + + for _, test := range []struct { + name string + oneAssignee string + multipleAssignees []string + wantIDs []int64 + wantStatus int + }{ + {name: "empty", wantIDs: []int64{}, wantStatus: http.StatusOK}, + {name: "combined", oneAssignee: "User2", multipleAssignees: []string{"user2", "USER2", "user1", ""}, wantIDs: []int64{1, 2}, wantStatus: http.StatusOK}, + {name: "unknown user", oneAssignee: "does-not-exist", wantStatus: http.StatusUnprocessableEntity}, + {name: "no access", oneAssignee: "user4", wantStatus: http.StatusUnprocessableEntity}, + } { + t.Run(test.name, func(t *testing.T) { + ctx, resp := contexttest.MockAPIContext(t, "user2/repo2") + contexttest.LoadRepo(t, ctx, 2) + + ids := loadAssigneeUserIDs(ctx, test.oneAssignee, test.multipleAssignees) + assert.Equal(t, test.wantStatus, resp.Code) + assert.ElementsMatch(t, test.wantIDs, ids) + }) + } +} diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 6719d758631..f7a36da2d35 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -524,33 +524,10 @@ func CreatePullRequest(ctx *context.APIContext) { pr.AllowMaintainerEdit = optional.FromPtr(form.AllowMaintainerEdit).ValueOrDefault(unitPullRequest.PullRequestsConfig().DefaultAllowMaintainerEdit) // Get all assignee IDs - assigneeIDs, err := issues_model.MakeIDsFromAPIAssigneesToAdd(ctx, form.Assignee, form.Assignees) - if err != nil { - if errors.Is(err, util.ErrNotExist) { - ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("Assignee does not exist: [name: %s]", err)) - } else { - ctx.APIErrorInternal(err) - } + assigneeIDs := loadAssigneeUserIDs(ctx, form.Assignee, form.Assignees) + if ctx.Written() { return } - // Check if the passed assignees is assignable - for _, aID := range assigneeIDs { - assignee, err := user_model.GetUserByID(ctx, aID) - if err != nil { - ctx.APIErrorInternal(err) - return - } - - valid, err := access_model.CanBeAssigned(ctx, assignee, repo) - if err != nil { - ctx.APIErrorInternal(err) - return - } - if !valid { - ctx.APIError(http.StatusUnprocessableEntity, repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: repo.Name}.Error()) - return - } - } prOpts := &pull_service.NewPullRequestOptions{ Repo: repo, diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 33f16863166..d00f5d00e0a 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -462,7 +462,7 @@ func UpdateIssueAssignee(ctx *context.Context) { return } if !valid { - ctx.ServerError("canBeAssigned", repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: assigneeID, RepoName: issue.Repo.Name}) + ctx.HTTPError(http.StatusForbidden, fmt.Sprintf("User %d cannot be assigned to this issue", assigneeID)) return }