diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index 8d58169a32f17..9612a653da768 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -99,3 +99,15 @@ uid: 5 org_id: 36 is_public: true + +- + id: 18 + uid: 31 + org_id: 3 + is_public: false + +- + id: 19 + uid: 33 + org_id: 3 + is_public: false diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index 295e51e39ce94..a82ba2eacb9e1 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -71,7 +71,7 @@ name: test_team authorize: 2 # write num_repos: 1 - num_members: 1 + num_members: 3 includes_all_repositories: false can_create_org_repo: false diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index 9142fe609ad76..fc7500252f40b 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -129,3 +129,15 @@ org_id: 17 team_id: 9 uid: 15 + +- + id: 23 + org_id: 3 + team_id: 7 + uid: 31 + +- + id: 24 + org_id: 3 + team_id: 7 + uid: 33 diff --git a/models/organization/org.go b/models/organization/org.go index 23a4e2f96a508..fa0f0c034db06 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -194,17 +194,54 @@ func (org *Organization) CanCreateRepo() bool { // FindOrgMembersOpts represensts find org members conditions type FindOrgMembersOpts struct { db.ListOptions - OrgID int64 - PublicOnly bool + OrgID int64 + DoerID int64 + IsAdmin bool + IsOrgMember bool + IsOrgAdmin bool +} + +// FindOrgMembersOptsCondition creates a query condition according find org members options +func FindOrgMembersOptsCondition(ctx context.Context, opts *FindOrgMembersOpts) (builder.Cond, error) { + cond := builder.And(builder.Eq{"`org_user`.org_id": opts.OrgID}) + + // for no-login user, they can only see public users with public visibility + isPublic := true + userVisibility := structs.VisibleTypePublic + + if opts.DoerID != 0 { + userVisibility = structs.VisibleTypeLimited + + // only org members and site admins can see private users in the org + // users with private visibility can not be seen in this case + isPublic = !opts.IsOrgMember && !opts.IsAdmin + + // only site admin or org admin can see private users with private visibility + if opts.IsAdmin || opts.IsOrgAdmin { + userVisibility = structs.VisibleTypePrivate + } + } + visibilityCond := builder.And(builder.Lte{"`user`.visibility": userVisibility}) + if isPublic { + visibilityCond = visibilityCond.And(builder.Eq{"`org_user`.is_public": isPublic}) + } + + cond = cond.And(builder.Or( + // doers can see themselves + builder.Eq{"`user`.id": opts.DoerID}, + visibilityCond, + )) + return cond, nil } // 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 := db.GetEngine(ctx).Join("INNER", "`user`", "`user`.id == `org_user`.uid") + cond, err := FindOrgMembersOptsCondition(ctx, opts) + if err != nil { + return 0, err } - return sess.Count(new(OrgUser)) + return sess.Where(cond).Count(new(OrgUser)) } // FindOrgMembers loads organization members according conditions @@ -519,10 +556,12 @@ func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organiz // GetOrgUsersByOrgID returns all organization-user relations by organization ID. 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 := db.GetEngine(ctx).Join("INNER", "`user`", "`user`.id == `org_user`.uid") + cond, err := FindOrgMembersOptsCondition(ctx, opts) + if err != nil { + return nil, err } + sess = sess.Where(cond) if opts.ListOptions.PageSize > 0 { sess = db.SetSessionPagination(sess, opts) diff --git a/models/organization/org_test.go b/models/organization/org_test.go index 5e40dd41907bd..156ad18962007 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -210,13 +210,39 @@ func TestFindOrgs(t *testing.T) { func TestGetOrgUsersByOrgID(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + // no-login user can see public user with public visibility orgUsers, err := organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{ ListOptions: db.ListOptions{}, OrgID: 3, - PublicOnly: false, }) assert.NoError(t, err) - if assert.Len(t, orgUsers, 3) { + if assert.Len(t, orgUsers, 2) { + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[0].ID, + OrgID: 3, + UID: 2, + IsPublic: true, + }, *orgUsers[0]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[1].ID, + OrgID: 3, + UID: 28, + IsPublic: true, + }, *orgUsers[1]) + } + + // org member can see private users with public and limited visibility and self + // user 31 has private user visibility + orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{ + ListOptions: db.ListOptions{}, + OrgID: 3, + DoerID: 31, + IsAdmin: false, + IsOrgMember: true, + IsOrgAdmin: false, + }) + assert.NoError(t, err) + if assert.Len(t, orgUsers, 5) { assert.Equal(t, organization.OrgUser{ ID: orgUsers[0].ID, OrgID: 3, @@ -235,12 +261,109 @@ func TestGetOrgUsersByOrgID(t *testing.T) { UID: 28, IsPublic: true, }, *orgUsers[2]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[3].ID, + OrgID: 3, + UID: 31, + IsPublic: false, + }, *orgUsers[3]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[4].ID, + OrgID: 3, + UID: 33, + IsPublic: false, + }, *orgUsers[4]) + } + + // org admin can see user all users + orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{ + ListOptions: db.ListOptions{}, + OrgID: 3, + DoerID: 28, + IsAdmin: false, + IsOrgMember: true, + IsOrgAdmin: true, + }) + assert.NoError(t, err) + if assert.Len(t, orgUsers, 5) { + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[0].ID, + OrgID: 3, + UID: 2, + IsPublic: true, + }, *orgUsers[0]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[1].ID, + OrgID: 3, + UID: 4, + IsPublic: false, + }, *orgUsers[1]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[2].ID, + OrgID: 3, + UID: 28, + IsPublic: true, + }, *orgUsers[2]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[3].ID, + OrgID: 3, + UID: 31, + IsPublic: false, + }, *orgUsers[3]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[4].ID, + OrgID: 3, + UID: 33, + IsPublic: false, + }, *orgUsers[4]) + } + + // admin can see user all users + orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{ + ListOptions: db.ListOptions{}, + OrgID: 3, + DoerID: 1, + IsAdmin: true, + IsOrgMember: false, + IsOrgAdmin: false, + }) + assert.NoError(t, err) + if assert.Len(t, orgUsers, 5) { + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[0].ID, + OrgID: 3, + UID: 2, + IsPublic: true, + }, *orgUsers[0]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[1].ID, + OrgID: 3, + UID: 4, + IsPublic: false, + }, *orgUsers[1]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[2].ID, + OrgID: 3, + UID: 28, + IsPublic: true, + }, *orgUsers[2]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[3].ID, + OrgID: 3, + UID: 31, + IsPublic: false, + }, *orgUsers[3]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[4].ID, + OrgID: 3, + UID: 33, + IsPublic: false, + }, *orgUsers[4]) } orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{ ListOptions: db.ListOptions{}, OrgID: unittest.NonexistentID, - PublicOnly: false, }) assert.NoError(t, err) assert.Len(t, orgUsers, 0) diff --git a/modules/context/org.go b/modules/context/org.go index d068646577ef5..51d81f442f9d5 100644 --- a/modules/context/org.go +++ b/modules/context/org.go @@ -24,7 +24,6 @@ type Organization struct { Organization *organization.Organization OrgLink string CanCreateOrgRepo bool - PublicMemberOnly bool // Only display public members Team *organization.Team Teams []*organization.Team @@ -174,10 +173,14 @@ func HandleOrgAssignment(ctx *Context, args ...bool) { ctx.Data["OrgLink"] = ctx.Org.OrgLink // Member - ctx.Org.PublicMemberOnly = ctx.Doer == nil || !ctx.Org.IsMember && !ctx.Doer.IsAdmin opts := &organization.FindOrgMembersOpts{ - OrgID: org.ID, - PublicOnly: ctx.Org.PublicMemberOnly, + OrgID: org.ID, + } + if ctx.Doer != nil { + opts.DoerID = ctx.Doer.ID + opts.IsAdmin = ctx.Doer.IsAdmin + opts.IsOrgMember = ctx.Org.IsMember + opts.IsOrgAdmin = ctx.Org.IsTeamAdmin } ctx.Data["NumMembers"], err = organization.CountOrgMembers(ctx, opts) if err != nil { diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go index 422b7cecfee14..b2d663e9f8271 100644 --- a/routers/api/v1/org/member.go +++ b/routers/api/v1/org/member.go @@ -21,10 +21,26 @@ import ( func listMembers(ctx *context.APIContext, publicOnly bool) { opts := &organization.FindOrgMembersOpts{ OrgID: ctx.Org.Organization.ID, - PublicOnly: publicOnly, ListOptions: utils.GetListOptions(ctx), } + if ctx.Doer != nil { + opts.DoerID = ctx.Doer.ID + + var err error + opts.IsOrgMember, err = ctx.Org.Organization.IsOrgMember(ctx, ctx.Doer.ID) + if err != nil { + ctx.InternalServerError(err) + return + } + opts.IsOrgAdmin, err = ctx.Org.Organization.IsOrgAdmin(ctx, ctx.Doer.ID) + if err != nil { + ctx.InternalServerError(err) + return + } + + } + count, err := organization.CountOrgMembers(ctx, opts) if err != nil { ctx.InternalServerError(err) diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 8bf02b2c42a74..a1f5de8328eed 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -122,9 +122,14 @@ func Home(ctx *context.Context) { opts := &organization.FindOrgMembersOpts{ OrgID: org.ID, - PublicOnly: ctx.Org.PublicMemberOnly, ListOptions: db.ListOptions{Page: 1, PageSize: 25}, } + if ctx.Doer != nil { + opts.DoerID = ctx.Doer.ID + opts.IsAdmin = ctx.Doer.IsAdmin + opts.IsOrgMember = ctx.Org.IsMember + opts.IsOrgAdmin = ctx.Org.IsTeamAdmin + } members, _, err := organization.FindOrgMembers(ctx, opts) if err != nil { ctx.ServerError("FindOrgMembers", err) diff --git a/routers/web/org/members.go b/routers/web/org/members.go index 15a615c706fe6..ee7ac7d510780 100644 --- a/routers/web/org/members.go +++ b/routers/web/org/members.go @@ -33,19 +33,24 @@ func Members(ctx *context.Context) { } opts := &organization.FindOrgMembersOpts{ - OrgID: org.ID, - PublicOnly: true, + OrgID: org.ID, } if ctx.Doer != nil { - isMember, err := ctx.Org.Organization.IsOrgMember(ctx, ctx.Doer.ID) + opts.DoerID = ctx.Doer.ID + + var err error + opts.IsOrgMember, err = ctx.Org.Organization.IsOrgMember(ctx, ctx.Doer.ID) if err != nil { ctx.Error(http.StatusInternalServerError, "IsOrgMember") return } - opts.PublicOnly = !isMember && !ctx.Doer.IsAdmin + opts.IsOrgAdmin, err = ctx.Org.Organization.IsOrgAdmin(ctx, ctx.Doer.ID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "IsOrgAdmin") + return + } } - ctx.Data["PublicOnly"] = opts.PublicOnly total, err := organization.CountOrgMembers(ctx, opts) if err != nil {