github/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-06-27 16:48:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix team permissions (#34827)

Browse Source 
* Fix #34793
* Fix #33456
This commit is contained in:
wxiaoguang 2025-06-24 21:24:09 +08:00 committed by GitHub
parent a789a8cc7a
commit 6a97ab0af4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 163 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
models/git/protected_branch.go
View File

@ -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)
}

5
models/organization/org.go
View File

@ -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
models/organization/team_repo.go
View File

@ -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.
@ -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.
// FIXME: TEAM-UNIT-PERMISSION this logic is not complete, search the fixme keyword to see more details
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))
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
models/organization/team_repo_test.go
View File

@ -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)

38
models/perm/access/repo_permission.go
View File

@ -267,7 +267,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
perm.units = repo.Units
// anonymous user visit private repo.
// TODO: anonymous user visit public unit of private repo???
if user == nil && repo.IsPrivate {
perm.AccessMode = perm_model.AccessModeNone
return perm, nil
@ -286,7 +285,8 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
}
// Prevent strangers from checking out public repo of private organization/users
// Allow user if they are collaborator of a repo within a private user or a private organization but not a member of the organization itself
// Allow user if they are a collaborator of a repo within a private user or a private organization but not a member of the organization itself
// TODO: rename it to "IsOwnerVisibleToDoer"
if !organization.HasOrgOrUserVisible(ctx, repo.Owner, user) && !isCollaborator {
perm.AccessMode = perm_model.AccessModeNone
return perm, nil
@ -304,7 +304,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
return perm, nil
}
// plain user
// plain user TODO: this check should be replaced, only need to check collaborator access mode
perm.AccessMode, err = accessLevel(ctx, user, repo)
if err != nil {
return perm, err
@ -314,6 +314,19 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
return perm, nil
}
// now: the owner is visible to doer, if the repo is public, then the min access mode is read
minAccessMode := util.Iif(!repo.IsPrivate && !user.IsRestricted, perm_model.AccessModeRead, perm_model.AccessModeNone)
perm.AccessMode = max(perm.AccessMode, minAccessMode)
// get units mode from teams
teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID)
if err != nil {
return perm, err
}
if len(teams) == 0 {
return perm, nil
}
perm.unitsMode = make(map[unit.Type]perm_model.AccessMode)
// Collaborators on organization
@ -323,12 +336,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
}
}
// get units mode from teams
teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID)
if err != nil {
return perm, err
}
// if user in an owner team
for _, team := range teams {
if team.HasAdminAccess() {
@ -339,19 +346,12 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
}
for _, u := range repo.Units {
var found bool
for _, team := range teams {
unitAccessMode := minAccessMode
if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist {
perm.unitsMode[u.Type] = max(perm.unitsMode[u.Type], teamMode)
found = true
}
}
// for a public repo on an organization, a non-restricted user has read permission on non-team defined units.
if !found && !repo.IsPrivate && !user.IsRestricted {
if _, ok := perm.unitsMode[u.Type]; !ok {
perm.unitsMode[u.Type] = perm_model.AccessModeRead
unitAccessMode = max(perm.unitsMode[u.Type], unitAccessMode, teamMode)
}
perm.unitsMode[u.Type] = unitAccessMode
}
}

46
models/perm/access/repo_permission_test.go
View File

@ -6,12 +6,16 @@ package access
import (
"testing"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
perm_model "code.gitea.io/gitea/models/perm"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestHasAnyUnitAccess(t *testing.T) {
@ -152,3 +156,45 @@ func TestUnitAccessMode(t *testing.T) {
}
assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "has unit, and map, use map")
}
func TestGetUserRepoPermission(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
ctx := t.Context()
repo32 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32}) // org public repo
require.NoError(t, repo32.LoadOwner(ctx))
require.True(t, repo32.Owner.IsOrganization())
require.NoError(t, db.TruncateBeans(ctx, &organization.Team{}, &organization.TeamUser{}, &organization.TeamRepo{}, &organization.TeamUnit{}))
org := repo32.Owner
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
team := &organization.Team{OrgID: org.ID, LowerName: "test_team"}
require.NoError(t, db.Insert(ctx, team))
t.Run("DoerInTeamWithNoRepo", func(t *testing.T) {
require.NoError(t, db.Insert(ctx, &organization.TeamUser{OrgID: org.ID, TeamID: team.ID, UID: user.ID}))
perm, err := GetUserRepoPermission(ctx, repo32, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
assert.Nil(t, perm.unitsMode) // doer in the team, but has no access to the repo
})
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: org.ID, TeamID: team.ID, RepoID: repo32.ID}))
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeNone}))
t.Run("DoerWithTeamUnitAccessNone", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo32, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeCode])
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
})
require.NoError(t, db.TruncateBeans(ctx, &organization.TeamUnit{}))
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeWrite}))
t.Run("DoerWithTeamUnitAccessWrite", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo32, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
})
}

1
options/locale/locale_en-US.ini
View File

@ -2163,6 +2163,7 @@ settings.collaboration.write = Write
settings.collaboration.read = Read
settings.collaboration.owner = Owner
settings.collaboration.undefined = Undefined
settings.collaboration.per_unit = Unit Permissions
settings.hooks = Webhooks
settings.githooks = Git Hooks
settings.basic_settings = Basic Settings

15
routers/web/org/teams.go
View File

@ -283,11 +283,22 @@ func NewTeam(ctx *context.Context) {
}
// FIXME: TEAM-UNIT-PERMISSION: this design is not right, when a new unit is added in the future,
// admin team won't inherit the correct admin permission for the new unit.
// The existing teams won't inherit the correct admin permission for the new unit.
// The full history is like this:
// 1. There was only "team", no "team unit", so "team.authorize" was used to determine the team permission.
// 2. Later, "team unit" was introduced, then the usage of "team.authorize" became inconsistent, and causes various bugs.
// - Sometimes, "team.authorize" is used to determine the team permission, e.g. admin, owner
// - Sometimes, "team unit" is used not really used and "team unit" is used.
// - Some functions like `GetTeamsWithAccessToAnyRepoUnit` use both.
//
// 3. After introducing "team unit" and more unclear changes, it becomes difficult to maintain team permissions.
// - Org owner need to click the permission for each unit, but can't just set a common "write" permission for all units.
//
// Ideally, "team.authorize=write" should mean the team has write access to all units including newly (future) added ones.
func getUnitPerms(forms url.Values, teamPermission perm.AccessMode) map[unit_model.Type]perm.AccessMode {
unitPerms := make(map[unit_model.Type]perm.AccessMode)
for _, ut := range unit_model.AllRepoUnitTypes {
// Default accessmode is none
// Default access mode is none
unitPerms[ut] = perm.AccessModeNone
v, ok := forms[fmt.Sprintf("unit_%d", ut)]

3
routers/web/repo/setting/protected_branch.go
View File

@ -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"
@ -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

3
routers/web/repo/setting/protected_tag.go
View File

@ -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"
@ -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

4
services/convert/convert.go
View File

@ -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)
}
@ -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)
}

2
services/issue/assignee.go
View File

@ -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

2
services/pull/reviewer.go
View File

@ -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)
}

26
templates/repo/settings/collaboration.tmpl
View File

@ -63,13 +63,33 @@
{{.Name}}
</a>
<div class="flex-item-body flex-text-block">
{{/*FIXME: TEAM-UNIT-PERMISSION this display is not right, search the fixme keyword to see more details */}}
{{svg "octicon-shield-lock"}}
{{if eq .AccessMode 1}}{{ctx.Locale.Tr "repo.settings.collaboration.read"}}{{else if eq .AccessMode 2}}{{ctx.Locale.Tr "repo.settings.collaboration.write"}}{{else if eq .AccessMode 3}}{{ctx.Locale.Tr "repo.settings.collaboration.admin"}}{{else if eq .AccessMode 4}}{{ctx.Locale.Tr "repo.settings.collaboration.owner"}}{{else}}{{ctx.Locale.Tr "repo.settings.collaboration.undefined"}}{{end}}
{{if eq .AccessMode 0}}
{{ctx.Locale.Tr "repo.settings.collaboration.per_unit"}}
{{else if eq .AccessMode 1}}
{{ctx.Locale.Tr "repo.settings.collaboration.read"}}
{{else if eq .AccessMode 2}}
{{ctx.Locale.Tr "repo.settings.collaboration.write"}}
{{else if eq .AccessMode 3}}
{{ctx.Locale.Tr "repo.settings.collaboration.admin"}}
{{else if eq .AccessMode 4}}
{{ctx.Locale.Tr "repo.settings.collaboration.owner"}}
{{else}}
{{ctx.Locale.Tr "repo.settings.collaboration.undefined"}}
{{end}}
</div>
{{if or (eq .AccessMode 1) (eq .AccessMode 2)}}
{{if or (eq .AccessMode 0) (eq .AccessMode 1) (eq .AccessMode 2)}}
{{$first := true}}
<div class="flex-item-body" data-tooltip-content="{{ctx.Locale.Tr "repo.settings.change_team_permission_tip"}}">
Sections: {{range $u, $unit := $.Units}}{{if and ($.Repo.UnitEnabled ctx $unit.Type) ($team.UnitEnabled ctx $unit.Type)}}{{if $first}}{{$first = false}}{{else}}, {{end}}{{ctx.Locale.Tr $unit.NameKey}}{{end}}{{end}} {{if $first}}None{{end}}
Units:
{{range $u, $unit := $.Units}}
{{- if and ($.Repo.UnitEnabled ctx $unit.Type) ($team.UnitEnabled ctx $unit.Type) -}}
{{- Iif $first "" ", "}}{{ctx.Locale.Tr $unit.NameKey -}}
{{- $first = false -}}
{{- end -}}
{{end}}
{{if $first}}None{{end}}
</div>
{{end}}
</div>

33
tests/integration/org_test.go
View File

@ -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) {
@ -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"
})
}