Never add labels not from this repository or organisation and remove org labels on...

* 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

diff --git a/integrations/api_issue_label_test.go b/integrations/api_issue_label_test.go
index ddcfdd6615892c45c25b080bad902a9d415826ef..64c3515dd2bb341693b719c4a15e714fb7d1eef1 100644 (file)
--- a/integrations/api_issue_label_test.go
+++ b/integrations/api_issue_label_test.go
@@ -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) {

diff --git a/models/issue.go b/models/issue.go
index 1b634ed9e8d3cfdfb17529dcacca8a8e97306858..3a7a0cd41ae3c720d256213d38ad8ec9340ddca6 100644 (file)
--- a/models/issue.go
+++ b/models/issue.go
@@ -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)

diff --git a/models/issue_label.go b/models/issue_label.go
index 54b286fe7e02742be3e3bc8c2a2cdf452bbb4027..1b5cfd88d5b9a368a5d2ffe66538fcc693ff52eb 100644 (file)
--- a/models/issue_label.go
+++ b/models/issue_label.go
@@ -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)
                }
        }

diff --git a/models/repo_transfer.go b/models/repo_transfer.go
index 9f8fd649b6f357f1ebcd793cf4274dcfc338e8a2..273dca1c5d1885ddcf8b7727f704b74efa36bd18 100644 (file)
--- a/models/repo_transfer.go
+++ b/models/repo_transfer.go
@@ -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)