diff options
author | 6543 <6543@obermui.de> | 2020-04-07 23:52:01 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-07 22:52:01 +0100 |
commit | 71979d9663d8e43b772c37f2a79af5b8911df661 (patch) | |
tree | bfdd683d60759f9f274144fea1265537d1ea396c | |
parent | d00ebf445bac3ee70616032e3fc89781cac3000c (diff) | |
download | gitea-71979d9663d8e43b772c37f2a79af5b8911df661.tar.gz gitea-71979d9663d8e43b772c37f2a79af5b8911df661.zip |
Automatically remove Watches, Assignments, etc if user loses access due to being removed as collaborator or from a team (#10997)
* remove a user from being assigned to any issue/PR if (s)he is removed as a collaborator
* fix gender specific comment
* do not remove users that still have access to the repo if they are a member of a team that can access the repo
* add context to errors
* updates
* incorporate review fixes
* Update models/repo_collaboration.go
Co-Authored-By: 6543 <6543@obermui.de>
* Update models/repo_collaboration.go
Co-Authored-By: 6543 <6543@obermui.de>
* Fix Rebase Relict
* Fix & Impruve
* use xorm builder
* all in one session
* generalize reconsiderIssueAssignees
* Only Unwatch if have no access anymore
* prepare for reuse
* Same things if remove User from Team
* fix lint
* let mysql take time to react
* add description
* CI.restart()
* CI.restart()
Co-authored-by: Lanre Adelowo <yo@lanre.wtf>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
-rw-r--r-- | integrations/release_test.go | 4 | ||||
-rw-r--r-- | models/org_team.go | 13 | ||||
-rw-r--r-- | models/repo_collaboration.go | 41 | ||||
-rw-r--r-- | models/repo_permission.go | 6 |
4 files changed, 51 insertions, 13 deletions
diff --git a/integrations/release_test.go b/integrations/release_test.go index be0abd5e34..176f83d55a 100644 --- a/integrations/release_test.go +++ b/integrations/release_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "testing" + "time" "code.gitea.io/gitea/modules/test" @@ -63,6 +64,9 @@ func TestViewReleases(t *testing.T) { session := loginUser(t, "user2") req := NewRequest(t, "GET", "/user2/repo1/releases") session.MakeRequest(t, req, http.StatusOK) + + // if CI is to slow this test fail, so lets wait a bit + time.Sleep(time.Millisecond * 100) } func TestViewReleasesNoLogin(t *testing.T) { diff --git a/models/org_team.go b/models/org_team.go index 82c27b2c06..a3f1eb92a2 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -917,19 +917,12 @@ func removeTeamMember(e *xorm.Session, team *Team, userID int64) error { } // Remove watches from now unaccessible - has, err := hasAccess(e, userID, repo) - if err != nil { + if err := repo.reconsiderWatches(e, userID); err != nil { return err - } else if has { - continue } - if err = watchRepo(e, userID, repo.ID, false); err != nil { - return err - } - - // Remove all IssueWatches a user has subscribed to in the repositories - if err := removeIssueWatchersByRepoID(e, userID, repo.ID); err != nil { + // Remove issue assignments from now unaccessible + if err := repo.reconsiderIssueAssignees(e, userID); err != nil { return err } } diff --git a/models/repo_collaboration.go b/models/repo_collaboration.go index 85bc99f320..4bb95cd05c 100644 --- a/models/repo_collaboration.go +++ b/models/repo_collaboration.go @@ -7,6 +7,8 @@ package models import ( "fmt" + + "xorm.io/builder" ) // Collaboration represent the relation between an individual and a repository. @@ -189,14 +191,49 @@ func (repo *Repository) DeleteCollaboration(uid int64) (err error) { return err } - // Remove all IssueWatches a user has subscribed to in the repository - if err := removeIssueWatchersByRepoID(sess, uid, repo.ID); err != nil { + if err = repo.reconsiderWatches(sess, uid); err != nil { + return err + } + + // Unassign a user from any issue (s)he has been assigned to in the repository + if err := repo.reconsiderIssueAssignees(sess, uid); err != nil { return err } return sess.Commit() } +func (repo *Repository) reconsiderIssueAssignees(e Engine, uid int64) error { + user, err := getUserByID(e, uid) + if err != nil { + return err + } + + if canAssigned, err := canBeAssigned(e, user, repo, true); err != nil || canAssigned { + return err + } + + if _, err := e.Where(builder.Eq{"assignee_id": uid}). + In("issue_id", builder.Select("id").From("issue").Where(builder.Eq{"repo_id": repo.ID})). + Delete(&IssueAssignees{}); err != nil { + return fmt.Errorf("Could not delete assignee[%d] %v", uid, err) + } + return nil +} + +func (repo *Repository) reconsiderWatches(e Engine, uid int64) error { + if has, err := hasAccess(e, uid, repo); err != nil || has { + return err + } + + if err := watchRepo(e, uid, repo.ID, false); err != nil { + return err + } + + // Remove all IssueWatches a user has subscribed to in the repository + return removeIssueWatchersByRepoID(e, uid, repo.ID) +} + func (repo *Repository) getRepoTeams(e Engine) (teams []*Team, err error) { return teams, e. Join("INNER", "team_repo", "team_repo.team_id = team.id"). diff --git a/models/repo_permission.go b/models/repo_permission.go index 0b3e5b341a..2061f9770d 100644 --- a/models/repo_permission.go +++ b/models/repo_permission.go @@ -339,10 +339,14 @@ func HasAccessUnit(user *User, repo *Repository, unitType UnitType, testMode Acc // Currently any write access (code, issues or pr's) is assignable, to match assignee list in user interface. // FIXME: user could send PullRequest also could be assigned??? func CanBeAssigned(user *User, repo *Repository, isPull bool) (bool, error) { + return canBeAssigned(x, user, repo, isPull) +} + +func canBeAssigned(e Engine, user *User, repo *Repository, _ bool) (bool, error) { if user.IsOrganization() { return false, fmt.Errorf("Organization can't be added as assignee [user_id: %d, repo_id: %d]", user.ID, repo.ID) } - perm, err := GetUserRepoPermission(repo, user) + perm, err := getUserRepoPermission(e, repo, user) if err != nil { return false, err } |