Skip to content

Commit 4734279

Browse files
committed
fix maximum permission granted to doer
1 parent 350d70e commit 4734279

File tree

3 files changed

+65
-20
lines changed

3 files changed

+65
-20
lines changed

models/organization/team_repo.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, m
6161
And(builder.In("team_unit.type", append([]unit.Type{unitType}, unitTypesMore...))).
6262
And(builder.Expr("team_unit.access_mode >= ?", mode))
6363

64-
// FIXME: maybe it should also check "team.includes_all_repositories = true" in the future
6564
err := db.GetEngine(ctx).
6665
Join("INNER", "team_repo", "team_repo.team_id = team.id").
6766
And("team_repo.org_id = ?", orgID).

models/perm/access/repo_permission.go

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
267267
perm.units = repo.Units
268268

269269
// anonymous user visit private repo.
270-
// TODO: anonymous user visit public unit of private repo???
271270
if user == nil && repo.IsPrivate {
272271
perm.AccessMode = perm_model.AccessModeNone
273272
return perm, nil
@@ -286,7 +285,8 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
286285
}
287286

288287
// Prevent strangers from checking out public repo of private organization/users
289-
// 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
288+
// 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
289+
// TODO: rename it to "IsOwnerVisibleToDoer"
290290
if !organization.HasOrgOrUserVisible(ctx, repo.Owner, user) && !isCollaborator {
291291
perm.AccessMode = perm_model.AccessModeNone
292292
return perm, nil
@@ -304,7 +304,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
304304
return perm, nil
305305
}
306306

307-
// plain user
307+
// plain user TODO: this check should be replaced, only need to check collaborator access mode
308308
perm.AccessMode, err = accessLevel(ctx, user, repo)
309309
if err != nil {
310310
return perm, err
@@ -314,6 +314,19 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
314314
return perm, nil
315315
}
316316

317+
// now: the owner is visible to doer, if the repo is public, then the min access mode is read
318+
minAccessMode := util.Iif(!repo.IsPrivate && !user.IsRestricted, perm_model.AccessModeRead, perm_model.AccessModeNone)
319+
perm.AccessMode = max(perm.AccessMode, minAccessMode)
320+
321+
// get units mode from teams
322+
teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID)
323+
if err != nil {
324+
return perm, err
325+
}
326+
if len(teams) == 0 {
327+
return perm, nil
328+
}
329+
317330
perm.unitsMode = make(map[unit.Type]perm_model.AccessMode)
318331

319332
// Collaborators on organization
@@ -323,12 +336,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
323336
}
324337
}
325338

326-
// get units mode from teams
327-
teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID)
328-
if err != nil {
329-
return perm, err
330-
}
331-
332339
// if user in an owner team
333340
for _, team := range teams {
334341
if team.HasAdminAccess() {
@@ -339,19 +346,12 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
339346
}
340347

341348
for _, u := range repo.Units {
342-
var found bool
343349
for _, team := range teams {
350+
unitAccessMode := minAccessMode
344351
if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist {
345-
perm.unitsMode[u.Type] = max(perm.unitsMode[u.Type], teamMode)
346-
found = true
347-
}
348-
}
349-
350-
// for a public repo on an organization, a non-restricted user has read permission on non-team defined units.
351-
if !found && !repo.IsPrivate && !user.IsRestricted {
352-
if _, ok := perm.unitsMode[u.Type]; !ok {
353-
perm.unitsMode[u.Type] = perm_model.AccessModeRead
352+
unitAccessMode = max(perm.unitsMode[u.Type], unitAccessMode, teamMode)
354353
}
354+
perm.unitsMode[u.Type] = unitAccessMode
355355
}
356356
}
357357

models/perm/access/repo_permission_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@ package access
66
import (
77
"testing"
88

9+
"code.gitea.io/gitea/models/db"
10+
"code.gitea.io/gitea/models/organization"
911
perm_model "code.gitea.io/gitea/models/perm"
1012
repo_model "code.gitea.io/gitea/models/repo"
1113
"code.gitea.io/gitea/models/unit"
14+
"code.gitea.io/gitea/models/unittest"
1215
user_model "code.gitea.io/gitea/models/user"
1316

1417
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
1519
)
1620

1721
func TestHasAnyUnitAccess(t *testing.T) {
@@ -152,3 +156,45 @@ func TestUnitAccessMode(t *testing.T) {
152156
}
153157
assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "has unit, and map, use map")
154158
}
159+
160+
func TestGetUserRepoPermission(t *testing.T) {
161+
assert.NoError(t, unittest.PrepareTestDatabase())
162+
ctx := t.Context()
163+
repo32 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32}) // org public repo
164+
require.NoError(t, repo32.LoadOwner(ctx))
165+
require.True(t, repo32.Owner.IsOrganization())
166+
167+
require.NoError(t, db.TruncateBeans(ctx, &organization.Team{}, &organization.TeamUser{}, &organization.TeamRepo{}, &organization.TeamUnit{}))
168+
org := repo32.Owner
169+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
170+
team := &organization.Team{OrgID: org.ID, LowerName: "test_team"}
171+
require.NoError(t, db.Insert(ctx, team))
172+
173+
t.Run("DoerInTeamWithNoRepo", func(t *testing.T) {
174+
require.NoError(t, db.Insert(ctx, &organization.TeamUser{OrgID: org.ID, TeamID: team.ID, UID: user.ID}))
175+
perm, err := GetUserRepoPermission(ctx, repo32, user)
176+
require.NoError(t, err)
177+
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
178+
assert.Nil(t, perm.unitsMode) // doer in the team, but has no access to the repo
179+
})
180+
181+
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: org.ID, TeamID: team.ID, RepoID: repo32.ID}))
182+
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeNone}))
183+
t.Run("DoerWithTeamUnitAccessNone", func(t *testing.T) {
184+
perm, err := GetUserRepoPermission(ctx, repo32, user)
185+
require.NoError(t, err)
186+
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
187+
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeCode])
188+
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
189+
})
190+
191+
require.NoError(t, db.TruncateBeans(ctx, &organization.TeamUnit{}))
192+
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeWrite}))
193+
t.Run("DoerWithTeamUnitAccessWrite", func(t *testing.T) {
194+
perm, err := GetUserRepoPermission(ctx, repo32, user)
195+
require.NoError(t, err)
196+
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
197+
assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
198+
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
199+
})
200+
}

0 commit comments

Comments
 (0)