From 4c924bf43cdc944c6ce34cce54f216c455a346a2 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 12 Nov 2024 04:44:24 +0100 Subject: [PATCH] Limit org member view of restricted users (#32211) currently restricted users can only see the repos of teams in orgs they are part at. they also should only see the users that are also part at the same team. --- *Sponsored by Kithara Software GmbH* --- models/fixtures/org_user.yml | 6 +++ models/fixtures/user.yml | 2 +- models/organization/org.go | 33 +++++++++++++++- models/organization/org_test.go | 70 +++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 3 deletions(-) diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index cf21b84aa9f..73a3e9dba9b 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -129,3 +129,9 @@ uid: 2 org_id: 35 is_public: true + +- + id: 23 + uid: 20 + org_id: 17 + is_public: false diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index c0296deec55..1044e487f81 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -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: "" diff --git a/models/organization/org.go b/models/organization/org.go index 28a46ec8f50..6231f1eeedf 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/util" "xorm.io/builder" + "xorm.io/xorm" ) // ________ .__ __ .__ @@ -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)) @@ -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 { @@ -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) diff --git a/models/organization/org_test.go b/models/organization/org_test.go index 5442c37ccc9..c614aaacf56 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -4,6 +4,7 @@ package organization_test import ( + "slices" "sort" "testing" @@ -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())