diff options
author | David Svantesson <davidsvantesson@gmail.com> | 2019-11-06 10:37:14 +0100 |
---|---|---|
committer | Lauris BH <lauris@nix.lv> | 2019-11-06 11:37:14 +0200 |
commit | 72aa5a20ecf8aa3f7c110fd51c37994d950e0ba8 (patch) | |
tree | 3ac89c3c7b28572737b20dfe8cbeed259184d91b /models | |
parent | 0109229928d8603ebedc2364943538f788635370 (diff) | |
download | gitea-72aa5a20ecf8aa3f7c110fd51c37994d950e0ba8.tar.gz gitea-72aa5a20ecf8aa3f7c110fd51c37994d950e0ba8.zip |
Add team option to grant rights for all organization repositories (#8688)
* Add field IsAllRepositories to team
* Add AllRepositories to team UI
* Manage team with access to all repositories
* Add field IsAllRepositories to team API
* put backticks around table/column names
* rename IsAllRepositories to IncludesAllRepositories
* do not reload slice if already loaded
* add repo to teams with access to all repositories when changing repo owner
* improve tests for teams with access to all repositories
* Merge branch 'master'
* Change code for adding all repositories
Signed-off-by: David Svantesson <davidsvantesson@gmail.com>
* fmt after merge
* Change code in API EditTeam similar to EditTeamPost web interface
Signed-off-by: David Svantesson <davidsvantesson@gmail.com>
* Clarify that all repositories will be added
Signed-off-by: David Svantesson <davidsvantesson@gmail.com>
* All repositories option under Permissions headline
* New setting group 'Repository access'
* Move check IncludeAllRepositories to removeRepository.
* Revert "Move check IncludeAllRepositories to removeRepository." and add comment instead.
This reverts commit 753b7d205be260b8be465b5291a02975a81f3093.
* Clarify help text what options do.
Diffstat (limited to 'models')
-rw-r--r-- | models/migrations/migrations.go | 2 | ||||
-rw-r--r-- | models/migrations/v105.go | 25 | ||||
-rw-r--r-- | models/org.go | 14 | ||||
-rw-r--r-- | models/org_team.go | 70 | ||||
-rw-r--r-- | models/org_team_test.go | 137 | ||||
-rw-r--r-- | models/repo.go | 29 |
6 files changed, 247 insertions, 30 deletions
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index e214f16a2a..5ed70dc4f5 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -264,6 +264,8 @@ var migrations = []Migration{ NewMigration("Add WhitelistDeployKeys to protected branch", addWhitelistDeployKeysToBranches), // v104 -> v105 NewMigration("remove unnecessary columns from label", removeLabelUneededCols), + // v105 -> v106 + NewMigration("add includes_all_repositories to teams", addTeamIncludesAllRepositories), } // Migrate database to current version diff --git a/models/migrations/v105.go b/models/migrations/v105.go new file mode 100644 index 0000000000..6c9a5817af --- /dev/null +++ b/models/migrations/v105.go @@ -0,0 +1,25 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "xorm.io/xorm" +) + +func addTeamIncludesAllRepositories(x *xorm.Engine) error { + + type Team struct { + ID int64 `xorm:"pk autoincr"` + IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` + } + + if err := x.Sync2(new(Team)); err != nil { + return err + } + + _, err := x.Exec("UPDATE `team` SET `includes_all_repositories` = ? WHERE `name`=?", + true, "Owners") + return err +} diff --git a/models/org.go b/models/org.go index 2cc302dac6..78b035b101 100644 --- a/models/org.go +++ b/models/org.go @@ -48,6 +48,9 @@ func (org *User) GetOwnerTeam() (*Team, error) { } func (org *User) getTeams(e Engine) error { + if org.Teams != nil { + return nil + } return e. Where("org_id=?", org.ID). OrderBy("CASE WHEN name LIKE '" + ownerTeamName + "' THEN '' ELSE name END"). @@ -149,11 +152,12 @@ func CreateOrganization(org, owner *User) (err error) { // Create default owner team. t := &Team{ - OrgID: org.ID, - LowerName: strings.ToLower(ownerTeamName), - Name: ownerTeamName, - Authorize: AccessModeOwner, - NumMembers: 1, + OrgID: org.ID, + LowerName: strings.ToLower(ownerTeamName), + Name: ownerTeamName, + Authorize: AccessModeOwner, + NumMembers: 1, + IncludesAllRepositories: true, } if _, err = sess.Insert(t); err != nil { return fmt.Errorf("insert owner team: %v", err) diff --git a/models/org_team.go b/models/org_team.go index a7a179f104..d740e1c240 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -22,17 +22,18 @@ const ownerTeamName = "Owners" // Team represents a organization team. type Team struct { - ID int64 `xorm:"pk autoincr"` - OrgID int64 `xorm:"INDEX"` - LowerName string - Name string - Description string - Authorize AccessMode - Repos []*Repository `xorm:"-"` - Members []*User `xorm:"-"` - NumRepos int - NumMembers int - Units []*TeamUnit `xorm:"-"` + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + LowerName string + Name string + Description string + Authorize AccessMode + Repos []*Repository `xorm:"-"` + Members []*User `xorm:"-"` + NumRepos int + NumMembers int + Units []*TeamUnit `xorm:"-"` + IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` } // SearchTeamOptions holds the search options @@ -149,6 +150,9 @@ func (t *Team) IsMember(userID int64) bool { } func (t *Team) getRepositories(e Engine) error { + if t.Repos != nil { + return nil + } return e.Join("INNER", "team_repo", "repository.id = team_repo.repo_id"). Where("team_repo.team_id=?", t.ID). OrderBy("repository.name"). @@ -220,6 +224,25 @@ func (t *Team) addRepository(e Engine, repo *Repository) (err error) { return nil } +// addAllRepositories adds all repositories to the team. +// If the team already has some repositories they will be left unchanged. +func (t *Team) addAllRepositories(e Engine) error { + var orgRepos []Repository + if err := e.Where("owner_id = ?", t.OrgID).Find(&orgRepos); err != nil { + return fmt.Errorf("get org repos: %v", err) + } + + for _, repo := range orgRepos { + if !t.hasRepository(e, repo.ID) { + if err := t.addRepository(e, &repo); err != nil { + return fmt.Errorf("addRepository: %v", err) + } + } + } + + return nil +} + // AddRepository adds new repository to team of organization. func (t *Team) AddRepository(repo *Repository) (err error) { if repo.OwnerID != t.OrgID { @@ -241,6 +264,8 @@ func (t *Team) AddRepository(repo *Repository) (err error) { return sess.Commit() } +// removeRepository removes a repository from a team and recalculates access +// Note: Repository shall not be removed from team if it includes all repositories (unless the repository is deleted) func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (err error) { if err = removeTeamRepo(e, t.ID, repo.ID); err != nil { return err @@ -284,11 +309,16 @@ func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (e } // RemoveRepository removes repository from team of organization. +// If the team shall include all repositories the request is ignored. func (t *Team) RemoveRepository(repoID int64) error { if !t.HasRepository(repoID) { return nil } + if t.IncludesAllRepositories { + return nil + } + repo, err := GetRepositoryByID(repoID) if err != nil { return err @@ -394,6 +424,14 @@ func NewTeam(t *Team) (err error) { } } + // Add all repositories to the team if it has access to all of them. + if t.IncludesAllRepositories { + err = t.addAllRepositories(sess) + if err != nil { + return fmt.Errorf("addAllRepositories: %v", err) + } + } + // Update organization number of teams. if _, err = sess.Exec("UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID); err != nil { errRollback := sess.Rollback() @@ -446,7 +484,7 @@ func GetTeamByID(teamID int64) (*Team, error) { } // UpdateTeam updates information of team. -func UpdateTeam(t *Team, authChanged bool) (err error) { +func UpdateTeam(t *Team, authChanged bool, includeAllChanged bool) (err error) { if len(t.Name) == 0 { return errors.New("empty team name") } @@ -511,6 +549,14 @@ func UpdateTeam(t *Team, authChanged bool) (err error) { } } + // Add all repositories to the team if it has access to all of them. + if includeAllChanged && t.IncludesAllRepositories { + err = t.addAllRepositories(sess) + if err != nil { + return fmt.Errorf("addAllRepositories: %v", err) + } + } + return sess.Commit() } diff --git a/models/org_team_test.go b/models/org_team_test.go index 06ab4637d8..b7e2ef113d 100644 --- a/models/org_team_test.go +++ b/models/org_team_test.go @@ -5,9 +5,12 @@ package models import ( + "fmt" "strings" "testing" + "code.gitea.io/gitea/modules/structs" + "github.com/stretchr/testify/assert" ) @@ -206,7 +209,7 @@ func TestUpdateTeam(t *testing.T) { team.Name = "newName" team.Description = strings.Repeat("A long description!", 100) team.Authorize = AccessModeAdmin - assert.NoError(t, UpdateTeam(team, true)) + assert.NoError(t, UpdateTeam(team, true, false)) team = AssertExistsAndLoadBean(t, &Team{Name: "newName"}).(*Team) assert.True(t, strings.HasPrefix(team.Description, "A long description!")) @@ -225,7 +228,7 @@ func TestUpdateTeam2(t *testing.T) { team.LowerName = "owners" team.Name = "Owners" team.Description = strings.Repeat("A long description!", 100) - err := UpdateTeam(team, true) + err := UpdateTeam(team, true, false) assert.True(t, IsErrTeamAlreadyExist(err)) CheckConsistencyFor(t, &Team{ID: team.ID}) @@ -374,3 +377,133 @@ func TestUsersInTeamsCount(t *testing.T) { test([]int64{1, 2, 3, 4, 5}, []int64{2, 5}, 2) // userid 2,4 test([]int64{1, 2, 3, 4, 5}, []int64{2, 3, 5}, 3) // userid 2,4,5 } + +func TestIncludesAllRepositoriesTeams(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + testTeamRepositories := func(teamID int64, repoIds []int64) { + team := AssertExistsAndLoadBean(t, &Team{ID: teamID}).(*Team) + assert.NoError(t, team.GetRepositories(), "%s: GetRepositories", team.Name) + assert.Len(t, team.Repos, team.NumRepos, "%s: len repo", team.Name) + assert.Equal(t, len(repoIds), len(team.Repos), "%s: repo count", team.Name) + for i, rid := range repoIds { + if rid > 0 { + assert.True(t, team.HasRepository(rid), "%s: HasRepository(%d) %d", rid, i) + } + } + } + + // Get an admin user. + user, err := GetUserByID(1) + assert.NoError(t, err, "GetUserByID") + + // Create org. + org := &User{ + Name: "All repo", + IsActive: true, + Type: UserTypeOrganization, + Visibility: structs.VisibleTypePublic, + } + assert.NoError(t, CreateOrganization(org, user), "CreateOrganization") + + // Check Owner team. + ownerTeam, err := org.GetOwnerTeam() + assert.NoError(t, err, "GetOwnerTeam") + assert.True(t, ownerTeam.IncludesAllRepositories, "Owner team includes all repositories") + + // Create repos. + repoIds := make([]int64, 0) + for i := 0; i < 3; i++ { + r, err := CreateRepository(user, org, CreateRepoOptions{Name: fmt.Sprintf("repo-%d", i)}) + assert.NoError(t, err, "CreateRepository %d", i) + if r != nil { + repoIds = append(repoIds, r.ID) + } + } + // Get fresh copy of Owner team after creating repos. + ownerTeam, err = org.GetOwnerTeam() + assert.NoError(t, err, "GetOwnerTeam") + + // Create teams and check repositories. + teams := []*Team{ + ownerTeam, + { + OrgID: org.ID, + Name: "team one", + Authorize: AccessModeRead, + IncludesAllRepositories: true, + }, + { + OrgID: org.ID, + Name: "team 2", + Authorize: AccessModeRead, + IncludesAllRepositories: false, + }, + { + OrgID: org.ID, + Name: "team three", + Authorize: AccessModeWrite, + IncludesAllRepositories: true, + }, + { + OrgID: org.ID, + Name: "team 4", + Authorize: AccessModeWrite, + IncludesAllRepositories: false, + }, + } + teamRepos := [][]int64{ + repoIds, + repoIds, + {}, + repoIds, + {}, + } + for i, team := range teams { + if i > 0 { // first team is Owner. + assert.NoError(t, NewTeam(team), "%s: NewTeam", team.Name) + } + testTeamRepositories(team.ID, teamRepos[i]) + } + + // Update teams and check repositories. + teams[3].IncludesAllRepositories = false + teams[4].IncludesAllRepositories = true + teamRepos[4] = repoIds + for i, team := range teams { + assert.NoError(t, UpdateTeam(team, false, true), "%s: UpdateTeam", team.Name) + testTeamRepositories(team.ID, teamRepos[i]) + } + + // Create repo and check teams repositories. + org.Teams = nil // Reset teams to allow their reloading. + r, err := CreateRepository(user, org, CreateRepoOptions{Name: "repo-last"}) + assert.NoError(t, err, "CreateRepository last") + if r != nil { + repoIds = append(repoIds, r.ID) + } + teamRepos[0] = repoIds + teamRepos[1] = repoIds + teamRepos[4] = repoIds + for i, team := range teams { + testTeamRepositories(team.ID, teamRepos[i]) + } + + // Remove repo and check teams repositories. + assert.NoError(t, DeleteRepository(user, org.ID, repoIds[0]), "DeleteRepository") + teamRepos[0] = repoIds[1:] + teamRepos[1] = repoIds[1:] + teamRepos[3] = repoIds[1:3] + teamRepos[4] = repoIds[1:] + for i, team := range teams { + testTeamRepositories(team.ID, teamRepos[i]) + } + + // Wipe created items. + for i, rid := range repoIds { + if i > 0 { // first repo already deleted. + assert.NoError(t, DeleteRepository(user, org.ID, rid), "DeleteRepository %d", i) + } + } + assert.NoError(t, DeleteOrganization(org), "DeleteOrganization") +} diff --git a/models/repo.go b/models/repo.go index 7945cb309d..89e579d1ec 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1447,14 +1447,17 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err } u.NumRepos++ - // Give access to all members in owner team. + // Give access to all members in teams with access to all repositories. if u.IsOrganization() { - t, err := u.getOwnerTeam(e) - if err != nil { - return fmt.Errorf("getOwnerTeam: %v", err) + if err := u.GetTeams(); err != nil { + return fmt.Errorf("GetTeams: %v", err) } - if err = t.addRepository(e, repo); err != nil { - return fmt.Errorf("addRepository: %v", err) + for _, t := range u.Teams { + if t.IncludesAllRepositories { + if err := t.addRepository(e, repo); err != nil { + return fmt.Errorf("addRepository: %v", err) + } + } } } else if err = repo.recalculateAccesses(e); err != nil { // Organization automatically called this in addRepository method. @@ -1641,11 +1644,15 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error } if newOwner.IsOrganization() { - t, err := newOwner.getOwnerTeam(sess) - if err != nil { - return fmt.Errorf("getOwnerTeam: %v", err) - } else if err = t.addRepository(sess, repo); err != nil { - return fmt.Errorf("add to owner team: %v", err) + if err := newOwner.GetTeams(); err != nil { + return fmt.Errorf("GetTeams: %v", err) + } + for _, t := range newOwner.Teams { + if t.IncludesAllRepositories { + if err := t.addRepository(sess, repo); err != nil { + return fmt.Errorf("addRepository: %v", err) + } + } } } else if err = repo.recalculateAccesses(sess); err != nil { // Organization called this in addRepository method. |