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

Never add labels not from this repository or organisation and remove org labels on transfer (#14928)

Browse source 
* Never add labels not from this repository or organisation and remove org labels on transfer

Prevent the addition of labels from outside of the repository or
organisation and remove organisation labels on transfer.

Related #14908

* switch to use sql

* subquery alias

* once more around the merry go round

* fix api problem
This commit is contained in:
zeripath 2021-03-12 17:45:49 +00:00 committed by GitHub
parent ccfb205ad1
commit 42b9b46ad2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 65 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
integrations/api_issue_label_test.go
View file

@ -91,22 +91,22 @@ func TestAPIAddIssueLabels(t *testing.T) {
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
issue := models.AssertExistsAndLoadBean(t, &models.Issue{RepoID: repo.ID}).(*models.Issue)
label := models.AssertExistsAndLoadBean(t, &models.Label{RepoID: repo.ID}).(*models.Label)
_ = models.AssertExistsAndLoadBean(t, &models.Label{RepoID: repo.ID, ID: 2}).(*models.Label)
owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)
session := loginUser(t, owner.Name)
token := getTokenForLoggedInUser(t, session)
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/labels?token=%s",
owner.Name, repo.Name, issue.Index, token)
repo.OwnerName, repo.Name, issue.Index, token)
req := NewRequestWithJSON(t, "POST", urlStr, &api.IssueLabelsOption{
Labels: []int64{label.ID},
Labels: []int64{1, 2},
})
resp := session.MakeRequest(t, req, http.StatusOK)
var apiLabels []*api.Label
DecodeJSON(t, resp, &apiLabels)
assert.Len(t, apiLabels, models.GetCount(t, &models.IssueLabel{IssueID: issue.ID}))
models.AssertExistsAndLoadBean(t, &models.IssueLabel{IssueID: issue.ID, LabelID: label.ID})
models.AssertExistsAndLoadBean(t, &models.IssueLabel{IssueID: issue.ID, LabelID: 2})
}
func TestAPIReplaceIssueLabels(t *testing.T) {

14
models/issue.go
View file

@ -513,6 +513,10 @@ func (issue *Issue) ReplaceLabels(labels []*Label, doer *User) (err error) {
return err
}
if err = issue.loadRepo(sess); err != nil {
return err
}
if err = issue.loadLabels(sess); err != nil {
return err
}
@ -527,10 +531,18 @@ func (issue *Issue) ReplaceLabels(labels []*Label, doer *User) (err error) {
addLabel := labels[addIndex]
removeLabel := issue.Labels[removeIndex]
if addLabel.ID == removeLabel.ID {
// Silently drop invalid labels
if removeLabel.RepoID != issue.RepoID && removeLabel.OrgID != issue.Repo.OwnerID {
toRemove = append(toRemove, removeLabel)
}
addIndex++
removeIndex++
} else if addLabel.ID < removeLabel.ID {
toAdd = append(toAdd, addLabel)
// Only add if the label is valid
if addLabel.RepoID == issue.RepoID || addLabel.OrgID == issue.Repo.OwnerID {
toAdd = append(toAdd, addLabel)
}
addIndex++
} else {
toRemove = append(toRemove, removeLabel)

25
models/issue_label.go
View file

@ -321,7 +321,7 @@ func GetLabelsByIDs(labelIDs []int64) ([]*Label, error) {
return labels, x.Table("label").
In("id", labelIDs).
Asc("name").
Cols("id").
Cols("id", "repo_id", "org_id").
Find(&labels)
}
@ -632,6 +632,8 @@ func HasIssueLabel(issueID, labelID int64) bool {
return hasIssueLabel(x, issueID, labelID)
}
// newIssueLabel this function creates a new label it does not check if the label is valid for the issue
// YOU MUST CHECK THIS BEFORE THIS FUNCTION
func newIssueLabel(e *xorm.Session, issue *Issue, label *Label, doer *User) (err error) {
if _, err = e.Insert(&IssueLabel{
IssueID: issue.ID,
@ -671,6 +673,15 @@ func NewIssueLabel(issue *Issue, label *Label, doer *User) (err error) {
return err
}
if err = issue.loadRepo(sess); err != nil {
return err
}
// Do NOT add invalid labels
if issue.RepoID != label.RepoID && issue.Repo.OwnerID != label.OrgID {
return nil
}
if err = newIssueLabel(sess, issue, label, doer); err != nil {
return err
}
@ -683,13 +694,19 @@ func NewIssueLabel(issue *Issue, label *Label, doer *User) (err error) {
return sess.Commit()
}
// newIssueLabels add labels to an issue. It will check if the labels are valid for the issue
func newIssueLabels(e *xorm.Session, issue *Issue, labels []*Label, doer *User) (err error) {
for i := range labels {
if hasIssueLabel(e, issue.ID, labels[i].ID) {
if err = issue.loadRepo(e); err != nil {
return err
}
for _, label := range labels {
// Don't add already present labels and invalid labels
if hasIssueLabel(e, issue.ID, label.ID) ||
(label.RepoID != issue.RepoID && label.OrgID != issue.Repo.OwnerID) {
continue
}
if err = newIssueLabel(e, issue, labels[i], doer); err != nil {
if err = newIssueLabel(e, issue, label, doer); err != nil {
return fmt.Errorf("newIssueLabel: %v", err)
}
}

27
models/repo_transfer.go
View file

@ -325,6 +325,33 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
}
}
// Delete labels that belong to the old organization and comments that added these labels
if oldOwner.IsOrganization() {
if _, err := sess.Exec(`DELETE FROM issue_label WHERE issue_label.id IN (
SELECT il_too.id FROM (
SELECT il_too_too.id
FROM issue_label AS il_too_too
INNER JOIN label ON il_too_too.id = label.id
INNER JOIN issue on issue.id = il_too_too.issue_id
WHERE
issue.repo_id = ? AND (issue.repo_id != label.repo_id OR (label.repo_id = 0 AND label.org_id != ?))
) AS il_too )`, repo.ID, newOwner.ID); err != nil {
return fmt.Errorf("Unable to remove old org labels: %v", err)
}
if _, err := sess.Exec(`DELETE FROM comment WHERE comment.id IN (
SELECT il_too.id FROM (
SELECT com.id
FROM comment AS com
INNER JOIN label ON com.label_id = label.id
INNER JOIN issue on issue.id = com.issue_id
WHERE
com.type = ? AND issue.repo_id = ? AND (issue.repo_id != label.repo_id OR (label.repo_id = 0 AND label.org_id != ?))
) AS il_too)`, CommentTypeLabel, repo.ID, newOwner.ID); err != nil {
return fmt.Errorf("Unable to remove old org label comments: %v", err)
}
}
// Rename remote repository to new path and delete local copy.
dir := UserPath(newOwner.Name)