Skip to content
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.

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

Limit org member view of restricted users #32211

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
f0348be
refactor: calculate if only public should be shown in a single place …
6543 Oct 7, 2024
3fd5962
create addTeamMatesOnlyFilter()
6543 Oct 7, 2024
340e8ea
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Oct 10, 2024
4bcec1e
cleanup
6543 Oct 10, 2024
317176e
jup
6543 Oct 10, 2024
fb83464
Merge branch 'refactor-of-32211' into limit-org-member-view-of-restri…
6543 Oct 10, 2024
77f7179
Revert "cleanup"
6543 Oct 10, 2024
db511aa
Merge branch 'main' into refactor-of-32211
6543 Oct 11, 2024
4f38f65
no builder for now
6543 Oct 11, 2024
e79ef77
Revert "no builder for now"
6543 Oct 11, 2024
23f8b3b
fix
6543 Oct 11, 2024
695db86
fix test ...
6543 Oct 11, 2024
26d90b9
Merge branch 'main' into refactor-of-32211
6543 Oct 12, 2024
decaf7f
Merge branch 'main' into refactor-of-32211
6543 Oct 13, 2024
3af5ae7
try with less eslect
6543 Oct 14, 2024
6a26382
rename func to getUserTeamIDsQueryBuilder
6543 Oct 14, 2024
efcb63d
Revert "try with less eslect"
6543 Oct 14, 2024
aed72f4
Merge branch 'main' into refactor-of-32211
6543 Oct 15, 2024
518da00
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Oct 15, 2024
78c0ad7
Merge branch 'refactor-of-32211' into limit-org-member-view-of-restri…
6543 Oct 15, 2024
5e3b1af
adopt changes
6543 Oct 15, 2024
db7a59a
fix lint and better func wording
6543 Oct 15, 2024
5128b0b
fix lint2 err
6543 Oct 15, 2024
679680d
no return
6543 Oct 15, 2024
0baab56
Merge branch 'main' into refactor-of-32211
6543 Oct 16, 2024
88b0add
Merge branch 'main' into refactor-of-32211
6543 Oct 18, 2024
b2923cb
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Oct 18, 2024
4d9df45
Merge branch 'main' into refactor-of-32211
6543 Oct 20, 2024
b5e6a58
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Oct 21, 2024
12bd60b
Merge branch 'main' into refactor-of-32211
6543 Oct 22, 2024
89a3887
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Oct 22, 2024
7bb645e
not needed here and dublicated on related pull
6543 Oct 22, 2024
a993136
Merge branch 'refactor-of-32211' into limit-org-member-view-of-restri…
6543 Oct 22, 2024
8780d12
add dedicated func getUserTeamIDsQueryBuilder(orgID, userID int64) *b…
6543 Oct 22, 2024
389d8d0
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Nov 10, 2024
fbda2fd
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Nov 11, 2024
a5c04af
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Nov 11, 2024
fdc7087
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Nov 11, 2024
80ee5a2
still have public members visible while restricted user is loged in ...
6543 Nov 11, 2024
1ce2980
cover by tests
6543 Nov 11, 2024
2ab1083
fix & enhance test
6543 Nov 11, 2024
60ad5ed
fix fixtures inconsistency
6543 Nov 11, 2024
0b102df
Merge branch 'main' into limit-org-member-view-of-restricted-users
6543 Nov 12, 2024
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
6 changes: 6 additions & 0 deletions models/fixtures/org_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,9 @@
uid: 2
org_id: 35
is_public: true

-
6543 marked this conversation as resolved.
Show resolved Hide resolved
id: 23
uid: 20
org_id: 17
is_public: false
2 changes: 1 addition & 1 deletion models/fixtures/user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@
num_stars: 0
num_repos: 2
num_teams: 3
num_members: 4
num_members: 5
visibility: 0
repo_admin_change_team_access: false
theme: ""
Expand Down
33 changes: 31 additions & 2 deletions models/organization/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
"xorm.io/xorm"
)

// ________ .__ __ .__
Expand Down Expand Up @@ -205,11 +206,28 @@ func (opts FindOrgMembersOpts) PublicOnly() bool {
return opts.Doer == nil || !(opts.IsDoerMember || opts.Doer.IsAdmin)
}

// applyTeamMatesOnlyFilter make sure restricted users only see public team members and there own team mates
func (opts FindOrgMembersOpts) applyTeamMatesOnlyFilter(sess *xorm.Session) {
if opts.Doer != nil && opts.IsDoerMember && opts.Doer.IsRestricted {
teamMates := builder.Select("DISTINCT team_user.uid").
From("team_user").
Where(builder.In("team_user.team_id", getUserTeamIDsQueryBuilder(opts.OrgID, opts.Doer.ID))).
And(builder.Eq{"team_user.org_id": opts.OrgID})

sess.And(
builder.In("org_user.uid", teamMates).
Or(builder.Eq{"org_user.is_public": true}),
)
}
}

// CountOrgMembers counts the organization's members
func CountOrgMembers(ctx context.Context, opts *FindOrgMembersOpts) (int64, error) {
sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID)
if opts.PublicOnly() {
sess.And("is_public = ?", true)
sess = sess.And("is_public = ?", true)
} else {
opts.applyTeamMatesOnlyFilter(sess)
}

return sess.Count(new(OrgUser))
Expand Down Expand Up @@ -533,7 +551,9 @@ func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organiz
func GetOrgUsersByOrgID(ctx context.Context, opts *FindOrgMembersOpts) ([]*OrgUser, error) {
sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID)
if opts.PublicOnly() {
sess.And("is_public = ?", true)
sess = sess.And("is_public = ?", true)
} else {
opts.applyTeamMatesOnlyFilter(sess)
}

if opts.ListOptions.PageSize > 0 {
Expand Down Expand Up @@ -664,6 +684,15 @@ func (org *Organization) getUserTeamIDs(ctx context.Context, userID int64) ([]in
Find(&teamIDs)
}

func getUserTeamIDsQueryBuilder(orgID, userID int64) *builder.Builder {
return builder.Select("team.id").From("team").
InnerJoin("team_user", "team_user.team_id = team.id").
Where(builder.Eq{
"team_user.org_id": orgID,
"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)
Expand Down
70 changes: 70 additions & 0 deletions models/organization/org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package organization_test

import (
"slices"
"sort"
"testing"

Expand Down Expand Up @@ -181,6 +182,75 @@ func TestIsPublicMembership(t *testing.T) {
test(unittest.NonexistentID, unittest.NonexistentID, false)
}

func TestRestrictedUserOrgMembers(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

restrictedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{
ID: 29,
IsRestricted: true,
})
if !assert.True(t, restrictedUser.IsRestricted) {
return // ensure fixtures return restricted user
}

testCases := []struct {
name string
opts *organization.FindOrgMembersOpts
expectedUIDs []int64
}{
{
name: "restricted user sees public members and teammates",
opts: &organization.FindOrgMembersOpts{
OrgID: 17, // org17 where user29 is in team9
Doer: restrictedUser,
IsDoerMember: true,
},
expectedUIDs: []int64{2, 15, 20, 29}, // Public members (2) + teammates in team9 (15, 20, 29)
},
{
name: "restricted user sees only public members when not member",
opts: &organization.FindOrgMembersOpts{
OrgID: 3, // org3 where user29 is not a member
Doer: restrictedUser,
},
expectedUIDs: []int64{2, 28}, // Only public members
},
{
name: "non logged in only shows public members",
opts: &organization.FindOrgMembersOpts{
OrgID: 3,
},
expectedUIDs: []int64{2, 28}, // Only public members
},
{
name: "non restricted user sees all members",
opts: &organization.FindOrgMembersOpts{
OrgID: 17,
Doer: unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15}),
IsDoerMember: true,
},
expectedUIDs: []int64{2, 15, 18, 20, 29}, // All members
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
count, err := organization.CountOrgMembers(db.DefaultContext, tc.opts)
assert.NoError(t, err)
assert.EqualValues(t, len(tc.expectedUIDs), count)

members, err := organization.GetOrgUsersByOrgID(db.DefaultContext, tc.opts)
assert.NoError(t, err)
memberUIDs := make([]int64, 0, len(members))
for _, member := range members {
memberUIDs = append(memberUIDs, member.UID)
}
slices.Sort(memberUIDs)
assert.EqualValues(t, tc.expectedUIDs, memberUIDs)
})
}
}

func TestFindOrgs(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

Expand Down
Loading