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>
This commit is contained in:
		
							parent
							
								
									d00ebf445b
								
							
						
					
					
						commit
						71979d9663
					
				
					 4 changed files with 51 additions and 13 deletions
				
			
		|  | @ -8,6 +8,7 @@ import ( | ||||||
| 	"fmt" | 	"fmt" | ||||||
| 	"net/http" | 	"net/http" | ||||||
| 	"testing" | 	"testing" | ||||||
|  | 	"time" | ||||||
| 
 | 
 | ||||||
| 	"code.gitea.io/gitea/modules/test" | 	"code.gitea.io/gitea/modules/test" | ||||||
| 
 | 
 | ||||||
|  | @ -63,6 +64,9 @@ func TestViewReleases(t *testing.T) { | ||||||
| 	session := loginUser(t, "user2") | 	session := loginUser(t, "user2") | ||||||
| 	req := NewRequest(t, "GET", "/user2/repo1/releases") | 	req := NewRequest(t, "GET", "/user2/repo1/releases") | ||||||
| 	session.MakeRequest(t, req, http.StatusOK) | 	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) { | func TestViewReleasesNoLogin(t *testing.T) { | ||||||
|  |  | ||||||
|  | @ -917,19 +917,12 @@ func removeTeamMember(e *xorm.Session, team *Team, userID int64) error { | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
| 		// Remove watches from now unaccessible
 | 		// Remove watches from now unaccessible
 | ||||||
| 		has, err := hasAccess(e, userID, repo) | 		if err := repo.reconsiderWatches(e, userID); err != nil { | ||||||
| 		if err != nil { |  | ||||||
| 			return err |  | ||||||
| 		} else if has { |  | ||||||
| 			continue |  | ||||||
| 		} |  | ||||||
| 
 |  | ||||||
| 		if err = watchRepo(e, userID, repo.ID, false); err != nil { |  | ||||||
| 			return err | 			return err | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
| 		// Remove all IssueWatches a user has subscribed to in the repositories
 | 		// Remove issue assignments from now unaccessible
 | ||||||
| 		if err := removeIssueWatchersByRepoID(e, userID, repo.ID); err != nil { | 		if err := repo.reconsiderIssueAssignees(e, userID); err != nil { | ||||||
| 			return err | 			return err | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -7,6 +7,8 @@ package models | ||||||
| 
 | 
 | ||||||
| import ( | import ( | ||||||
| 	"fmt" | 	"fmt" | ||||||
|  | 
 | ||||||
|  | 	"xorm.io/builder" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| // Collaboration represent the relation between an individual and a repository.
 | // Collaboration represent the relation between an individual and a repository.
 | ||||||
|  | @ -189,14 +191,49 @@ func (repo *Repository) DeleteCollaboration(uid int64) (err error) { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	// Remove all IssueWatches a user has subscribed to in the repository
 | 	if err = repo.reconsiderWatches(sess, uid); err != nil { | ||||||
| 	if err := removeIssueWatchersByRepoID(sess, uid, repo.ID); 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 err | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	return sess.Commit() | 	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) { | func (repo *Repository) getRepoTeams(e Engine) (teams []*Team, err error) { | ||||||
| 	return teams, e. | 	return teams, e. | ||||||
| 		Join("INNER", "team_repo", "team_repo.team_id = team.id"). | 		Join("INNER", "team_repo", "team_repo.team_id = team.id"). | ||||||
|  |  | ||||||
|  | @ -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.
 | // 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???
 | // FIXME: user could send PullRequest also could be assigned???
 | ||||||
| func CanBeAssigned(user *User, repo *Repository, isPull bool) (bool, error) { | 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() { | 	if user.IsOrganization() { | ||||||
| 		return false, fmt.Errorf("Organization can't be added as assignee [user_id: %d, repo_id: %d]", user.ID, repo.ID) | 		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 { | 	if err != nil { | ||||||
| 		return false, err | 		return false, err | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue