Skip to content

Fix team permissions #34827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 24, 2025
Merged

Fix team permissions #34827

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f788a8d
fix
wxiaoguang Jun 23, 2025
b997b21
fix team display
wxiaoguang Jun 24, 2025
350d70e
add comments
wxiaoguang Jun 24, 2025
4734279
fix maximum permission granted to doer
wxiaoguang Jun 24, 2025
4340038
Merge branch 'main' into fix-team-access
wxiaoguang Jun 24, 2025
83c59ad
Merge branch 'main' into fix-team-access
wxiaoguang Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion models/git/protected_branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, curre
return currentWhitelist, nil
}

teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead)
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
if err != nil {
return nil, fmt.Errorf("GetTeamsWithAccessToRepo [org_id: %d, repo_id: %d]: %v", repo.OwnerID, repo.ID, err)
}
Expand Down
5 changes: 0 additions & 5 deletions models/organization/org.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,3 @@ func getUserTeamIDsQueryBuilder(orgID, userID int64) *builder.Builder {
"team_user.uid": userID,
})
}

// TeamsWithAccessToRepo returns all teams that have given access level to the repository.
func (org *Organization) TeamsWithAccessToRepo(ctx context.Context, repoID int64, mode perm.AccessMode) ([]*Team, error) {
return GetTeamsWithAccessToRepo(ctx, org.ID, repoID, mode)
}
33 changes: 18 additions & 15 deletions models/organization/team_repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
"code.gitea.io/gitea/models/unit"

"xorm.io/builder"
)

// TeamRepo represents an team-repository relation.
Expand Down Expand Up @@ -48,26 +50,27 @@ func RemoveTeamRepo(ctx context.Context, teamID, repoID int64) error {
return err
}

// GetTeamsWithAccessToRepo returns all teams in an organization that have given access level to the repository.
func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode perm.AccessMode) ([]*Team, error) {
// GetTeamsWithAccessToAnyRepoUnit returns all teams in an organization that have given access level to the repository special unit.
// This function is only used for finding some teams that can be used as branch protection allowlist or reviewers, it isn't really used for access control.
func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) ([]*Team, error) {
teams := make([]*Team, 0, 5)
return teams, db.GetEngine(ctx).Where("team.authorize >= ?", mode).
Join("INNER", "team_repo", "team_repo.team_id = team.id").
And("team_repo.org_id = ?", orgID).
And("team_repo.repo_id = ?", repoID).
OrderBy("name").
Find(&teams)
}

// GetTeamsWithAccessToRepoUnit returns all teams in an organization that have given access level to the repository special unit.
func GetTeamsWithAccessToRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type) ([]*Team, error) {
teams := make([]*Team, 0, 5)
return teams, db.GetEngine(ctx).Where("team_unit.access_mode >= ?", mode).
sub := builder.Select("team_id").From("team_unit").
Where(builder.Expr("team_unit.team_id = team.id")).
And(builder.In("team_unit.type", append([]unit.Type{unitType}, unitTypesMore...))).
And(builder.Expr("team_unit.access_mode >= ?", mode))

// FIXME: maybe it should also check "team.includes_all_repositories = true" in the future
err := db.GetEngine(ctx).
Join("INNER", "team_repo", "team_repo.team_id = team.id").
Join("INNER", "team_unit", "team_unit.team_id = team.id").
And("team_repo.org_id = ?", orgID).
And("team_repo.repo_id = ?", repoID).
And("team_unit.type = ?", unitType).
And(builder.Or(
builder.Expr("team.authorize >= ?", mode),
builder.In("team.id", sub),
)).
OrderBy("name").
Find(&teams)

return teams, err
}
2 changes: 1 addition & 1 deletion models/organization/team_repo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestGetTeamsWithAccessToRepoUnit(t *testing.T) {
org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41})
repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61})

teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
assert.NoError(t, err)
if assert.Len(t, teams, 2) {
assert.EqualValues(t, 21, teams[0].ID)
Expand Down
3 changes: 2 additions & 1 deletion routers/web/repo/setting/protected_branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/web"
Expand Down Expand Up @@ -89,7 +90,7 @@ func SettingsProtectedBranch(c *context.Context) {
c.Data["recent_status_checks"] = contexts

if c.Repo.Owner.IsOrganization() {
teams, err := organization.OrgFromUser(c.Repo.Owner).TeamsWithAccessToRepo(c, c.Repo.Repository.ID, perm.AccessModeRead)
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(c, c.Repo.Owner.ID, c.Repo.Repository.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
if err != nil {
c.ServerError("Repo.Owner.TeamsWithAccessToRepo", err)
return
Expand Down
3 changes: 2 additions & 1 deletion routers/web/repo/setting/protected_tag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
Expand Down Expand Up @@ -156,7 +157,7 @@ func setTagsContext(ctx *context.Context) error {
ctx.Data["Users"] = users

if ctx.Repo.Owner.IsOrganization() {
teams, err := organization.OrgFromUser(ctx.Repo.Owner).TeamsWithAccessToRepo(ctx, ctx.Repo.Repository.ID, perm.AccessModeRead)
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, ctx.Repo.Owner.ID, ctx.Repo.Repository.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
if err != nil {
ctx.ServerError("Repo.Owner.TeamsWithAccessToRepo", err)
return err
Expand Down
4 changes: 2 additions & 2 deletions services/convert/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo
mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs)
approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs)

teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead)
teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
if err != nil {
log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err)
}
Expand Down Expand Up @@ -727,7 +727,7 @@ func ToTagProtection(ctx context.Context, pt *git_model.ProtectedTag, repo *repo

whitelistUsernames := getWhitelistEntities(readers, pt.AllowlistUserIDs)

teamReaders, err := organization.OrgFromUser(repo.Owner).TeamsWithAccessToRepo(ctx, repo.ID, perm.AccessModeRead)
teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests)
if err != nil {
log.Error("Repo.Owner.TeamsWithAccessToRepo: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion services/issue/assignee.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, rep

// If the repo's owner is an organization, members of teams with read permission on pull requests can change reviewers
if repo.Owner.IsOrganization() {
teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead)
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
if err != nil {
log.Error("GetTeamsWithAccessToRepo: %v", err)
return false
Expand Down
2 changes: 1 addition & 1 deletion services/pull/reviewer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,5 @@ func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*orga
return nil, nil
}

return organization.GetTeamsWithAccessToRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
return organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
}
33 changes: 33 additions & 0 deletions tests/integration/org_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,17 @@ import (
"testing"

auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/tests"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestOrgRepos(t *testing.T) {
Expand Down Expand Up @@ -217,4 +222,32 @@ func TestTeamSearch(t *testing.T) {
session = loginUser(t, user5.Name)
req = NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "team")
session.MakeRequest(t, req, http.StatusNotFound)

t.Run("SearchWithPermission", func(t *testing.T) {
ctx := t.Context()
const testOrgID int64 = 500
const testRepoID int64 = 2000
testTeam := &organization.Team{OrgID: testOrgID, LowerName: "test_team", AccessMode: perm.AccessModeNone}
require.NoError(t, db.Insert(ctx, testTeam))
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: testOrgID, TeamID: testTeam.ID, RepoID: testRepoID}))
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeam.ID, Type: unit.TypeCode, AccessMode: perm.AccessModeRead}))
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: testOrgID, TeamID: testTeam.ID, Type: unit.TypeIssues, AccessMode: perm.AccessModeWrite}))

teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeRead, unit.TypeCode, unit.TypeIssues)
require.NoError(t, err)
assert.Len(t, teams, 1) // can read "code" or "issues"

teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeCode)
require.NoError(t, err)
assert.Empty(t, teams) // cannot write "code"

teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeIssues)
require.NoError(t, err)
assert.Len(t, teams, 1) // can write "issues"

_, _ = db.GetEngine(ctx).ID(testTeam.ID).Update(&organization.Team{AccessMode: perm.AccessModeWrite})
teams, err = organization.GetTeamsWithAccessToAnyRepoUnit(ctx, testOrgID, testRepoID, perm.AccessModeWrite, unit.TypeCode)
require.NoError(t, err)
assert.Len(t, teams, 1) // team permission is "write", so can write "code"
})
}