git724/forgejo
1
0
Fork 0
Code Issues Pull requests 3 Projects 1 Releases Packages Wiki Activity

fix: remove redundant permission check in RemoveLabel (#7835)

Browse source 
Closes #2415
Permissions checks are already done by the callee, which also do more correct permission checks.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7835
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Maxim Slipenko <maks1ms@altlinux.org>
Co-committed-by: Maxim Slipenko <maks1ms@altlinux.org>
This commit is contained in:
Maxim Slipenko 2025-05-14 16:20:43 +00:00 • committed by Gusted
commit b22bea8b45
2 changed files with 23 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

12
services/issue/label.go
View file

@ -8,7 +8,6 @@ import (
"forgejo.org/models/db"
issues_model "forgejo.org/models/issues"
access_model "forgejo.org/models/perm/access"
user_model "forgejo.org/models/user"
notify_service "forgejo.org/services/notify"
)
@ -56,17 +55,6 @@ func RemoveLabel(ctx context.Context, issue *issues_model.Issue, doer *user_mode
return err
}
perm, err := access_model.GetUserRepoPermission(dbCtx, issue.Repo, doer)
if err != nil {
return err
}
if !perm.CanWriteIssuesOrPulls(issue.IsPull) {
if label.OrgID > 0 {
return issues_model.ErrOrgLabelNotExist{}
}
return issues_model.ErrRepoLabelNotExist{}
}
if err := issues_model.DeleteIssueLabel(dbCtx, issue, label, doer); err != nil {
return err
}

23
tests/integration/api_issue_label_test.go
View file

@ -10,6 +10,7 @@ import (
"testing"
"time"
actions_model "forgejo.org/models/actions"
auth_model "forgejo.org/models/auth"
issues_model "forgejo.org/models/issues"
repo_model "forgejo.org/models/repo"
@ -149,6 +150,28 @@ func TestAPIAddIssueLabelsWithLabelNames(t *testing.T) {
MakeRequest(t, req, http.StatusNoContent)
}
func TestAPIRemoveIssueLabel(t *testing.T) {
defer tests.PrepareTestEnv(t)()
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID})
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
issueLabel := unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID})
task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
task.RepoID = repo.ID
task.OwnerID = repo.OwnerID
require.NoError(t, task.GenerateToken())
actions_model.UpdateTask(t.Context(), task)
deleteURL := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/labels/%d", owner.Name, repo.Name, issue.Index, issueLabel.LabelID)
req := NewRequest(t, "DELETE", deleteURL).
AddTokenAuth(task.Token)
MakeRequest(t, req, http.StatusNoContent)
unittest.AssertCount(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: issueLabel.LabelID}, 0)
}
func TestAPIAddIssueLabelsAutoDate(t *testing.T) {
defer tests.PrepareTestEnv(t)()