Skip to content

Commit

Permalink
Add team option to grant rights for all organization repositories (#8688
Browse files Browse the repository at this point in the history
)

* 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 <[email protected]>

* fmt after merge

* Change code in API EditTeam similar to EditTeamPost web interface

Signed-off-by: David Svantesson <[email protected]>

* Clarify that all repositories will be added

Signed-off-by: David Svantesson <[email protected]>

* 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 753b7d2.

* Clarify help text what options do.
  • Loading branch information
davidsvantesson authored and lafriks committed Nov 6, 2019
1 parent 0109229 commit 72aa5a2
Show file tree
Hide file tree
Showing 17 changed files with 382 additions and 75 deletions.
40 changes: 24 additions & 16 deletions integrations/api_team_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,57 +55,65 @@ func TestAPITeam(t *testing.T) {

// Create team.
teamToCreate := &api.CreateTeamOption{
Name: "team1",
Description: "team one",
Permission: "write",
Units: []string{"repo.code", "repo.issues"},
Name: "team1",
Description: "team one",
IncludesAllRepositories: true,
Permission: "write",
Units: []string{"repo.code", "repo.issues"},
}
req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams?token=%s", org.Name, token), teamToCreate)
resp = session.MakeRequest(t, req, http.StatusCreated)
DecodeJSON(t, resp, &apiTeam)
checkTeamResponse(t, &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.Permission, teamToCreate.Units)
checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.Permission, teamToCreate.Units)
checkTeamResponse(t, &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
teamToCreate.Permission, teamToCreate.Units)
checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
teamToCreate.Permission, teamToCreate.Units)
teamID := apiTeam.ID

// Edit team.
teamToEdit := &api.EditTeamOption{
Name: "teamone",
Description: "team 1",
Permission: "admin",
Units: []string{"repo.code", "repo.pulls", "repo.releases"},
Name: "teamone",
Description: "team 1",
IncludesAllRepositories: false,
Permission: "admin",
Units: []string{"repo.code", "repo.pulls", "repo.releases"},
}
req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d?token=%s", teamID, token), teamToEdit)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiTeam)
checkTeamResponse(t, &apiTeam, teamToEdit.Name, teamToEdit.Description, teamToEdit.Permission, teamToEdit.Units)
checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEdit.Description, teamToEdit.Permission, teamToEdit.Units)
checkTeamResponse(t, &apiTeam, teamToEdit.Name, teamToEdit.Description, teamToEdit.IncludesAllRepositories,
teamToEdit.Permission, teamToEdit.Units)
checkTeamBean(t, apiTeam.ID, teamToEdit.Name, teamToEdit.Description, teamToEdit.IncludesAllRepositories,
teamToEdit.Permission, teamToEdit.Units)

// Read team.
teamRead := models.AssertExistsAndLoadBean(t, &models.Team{ID: teamID}).(*models.Team)
req = NewRequestf(t, "GET", "/api/v1/teams/%d?token="+token, teamID)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiTeam)
checkTeamResponse(t, &apiTeam, teamRead.Name, teamRead.Description, teamRead.Authorize.String(), teamRead.GetUnitNames())
checkTeamResponse(t, &apiTeam, teamRead.Name, teamRead.Description, teamRead.IncludesAllRepositories,
teamRead.Authorize.String(), teamRead.GetUnitNames())

// Delete team.
req = NewRequestf(t, "DELETE", "/api/v1/teams/%d?token="+token, teamID)
session.MakeRequest(t, req, http.StatusNoContent)
models.AssertNotExistsBean(t, &models.Team{ID: teamID})
}

func checkTeamResponse(t *testing.T, apiTeam *api.Team, name, description string, permission string, units []string) {
func checkTeamResponse(t *testing.T, apiTeam *api.Team, name, description string, includesAllRepositories bool, permission string, units []string) {
assert.Equal(t, name, apiTeam.Name, "name")
assert.Equal(t, description, apiTeam.Description, "description")
assert.Equal(t, includesAllRepositories, apiTeam.IncludesAllRepositories, "includesAllRepositories")
assert.Equal(t, permission, apiTeam.Permission, "permission")
sort.StringSlice(units).Sort()
sort.StringSlice(apiTeam.Units).Sort()
assert.EqualValues(t, units, apiTeam.Units, "units")
}

func checkTeamBean(t *testing.T, id int64, name, description string, permission string, units []string) {
func checkTeamBean(t *testing.T, id int64, name, description string, includesAllRepositories bool, permission string, units []string) {
team := models.AssertExistsAndLoadBean(t, &models.Team{ID: id}).(*models.Team)
assert.NoError(t, team.GetUnits(), "GetUnits")
checkTeamResponse(t, convert.ToTeam(team), name, description, permission, units)
checkTeamResponse(t, convert.ToTeam(team), name, description, includesAllRepositories, permission, units)
}

type TeamSearchResults struct {
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions models/migrations/v105.go
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 9 additions & 5 deletions models/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down Expand Up @@ -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)
Expand Down
70 changes: 58 additions & 12 deletions models/org_team.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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").
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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()
}

Expand Down
Loading

0 comments on commit 72aa5a2

Please sign in to comment.