allow non-site admins to list their orgs' members on dotcom (#63934)

Previously, only site admins could see the members of an org, even of
orgs that they were a member of. This restriction does not make any
sense and makes the orgs feature broken on dotcom.

## Test plan

Added test.

Author: sqs

cmd/frontend/graphqlbackend/org.go

@@ -163,9 +163,11 @@ func (o *OrgResolver) Members(ctx context.Context, args struct {
 	Query *string
 },
 ) (*graphqlutil.ConnectionResolver[*UserResolver], error) {
-	// 🚨 SECURITY: Verify listing users is allowed.
-	if err := checkMembersAccess(ctx, o.db); err != nil {
-		return nil, err
+	// 🚨 SECURITY: On dotcom, only an org's members can list its members.
+	if dotcom.SourcegraphDotComMode() {
+		if err := auth.CheckOrgAccess(ctx, o.db, o.org.ID); err != nil {
+			return nil, err
+		}
 	}
 
 	connectionStore := &membersConnectionStore{

cmd/frontend/graphqlbackend/org_test.go

@@ -220,6 +220,123 @@ func TestOrganization(t *testing.T) {
 	})
 }
 
+func TestOrganizationMembers(t *testing.T) {
+	users := dbmocks.NewMockUserStore()
+	users.ListByOrgFunc.SetDefaultReturn([]*types.User{
+		{ID: 1, Username: "alice"},
+		{ID: 2, Username: "bob"},
+	}, nil)
+
+	orgMembers := dbmocks.NewMockOrgMemberStore()
+	orgMembers.GetByOrgIDAndUserIDFunc.SetDefaultHook(func(ctx context.Context, orgID, userID int32) (*types.OrgMembership, error) {
+		if orgID == 1 && userID == 1 {
+			return &types.OrgMembership{OrgID: 1, UserID: 1}, nil
+		}
+		return nil, &database.ErrOrgMemberNotFound{}
+	})
+
+	orgs := dbmocks.NewMockOrgStore()
+	mockedOrg := types.Org{ID: 1, Name: "acme"}
+	orgs.GetByNameFunc.SetDefaultReturn(&mockedOrg, nil)
+
+	db := dbmocks.NewMockDB()
+	db.OrgsFunc.SetDefaultReturn(orgs)
+	db.UsersFunc.SetDefaultReturn(users)
+	db.OrgMembersFunc.SetDefaultReturn(orgMembers)
+
+	t.Run("org members can list members", func(t *testing.T) {
+		users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{Username: "alice", ID: 1}, nil)
+		for _, isDotcom := range []bool{true, false} {
+			t.Run(fmt.Sprintf("dotcom=%v", isDotcom), func(t *testing.T) {
+				dotcom.MockSourcegraphDotComMode(t, isDotcom)
+				RunTests(t, []*Test{
+					{
+						Schema: mustParseGraphQLSchema(t, db),
+						Query: `
+					{
+						organization(name: "acme") {
+							members {
+								nodes { username }
+							}
+						}
+					}
+				`,
+						ExpectedResult: `
+					{
+						"organization": {
+							"members": {
+								"nodes": [{"username": "alice"}, {"username": "bob"}]
+							}
+						}
+					}
+				`,
+					},
+				})
+			})
+		}
+	})
+
+	t.Run("non-members", func(t *testing.T) {
+		users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{Username: "xavier", ID: 10}, nil)
+
+		t.Run("can list members on non-dotcom", func(t *testing.T) {
+			dotcom.MockSourcegraphDotComMode(t, false)
+			RunTests(t, []*Test{
+				{
+					Schema: mustParseGraphQLSchema(t, db),
+					Query: `
+						{
+							organization(name: "acme") {
+								members {
+									nodes { username }
+								}
+							}
+						}
+					`,
+					ExpectedResult: `
+						{
+							"organization": {
+								"members": {
+									"nodes": [{"username": "alice"}, {"username": "bob"}]
+								}
+							}
+						}
+					`,
+				},
+			})
+		})
+
+		t.Run("cannot list members on dotcom", func(t *testing.T) {
+			dotcom.MockSourcegraphDotComMode(t, true)
+			RunTests(t, []*Test{
+				{
+					Schema: mustParseGraphQLSchema(t, db),
+					Query: `
+					{
+						organization(name: "acme") {
+							members {
+								nodes { username }
+							}
+						}
+					}
+				`,
+					ExpectedResult: `
+				{
+					"organization": null
+				}
+				`,
+					ExpectedErrors: []*gqlerrors.QueryError{
+						{
+							Message: "org not found: name acme",
+							Path:    []any{"organization"},
+						},
+					},
+				},
+			})
+		})
+	})
+}
+
 func TestCreateOrganization(t *testing.T) {
 	userID := int32(1)
 

cmd/frontend/graphqlbackend/users.go

@@ -24,9 +24,11 @@ type usersArgs struct {
 }
 
 func (r *schemaResolver) Users(ctx context.Context, args *usersArgs) (*userConnectionResolver, error) {
-	// 🚨 SECURITY: Verify listing users is allowed.
-	if err := checkMembersAccess(ctx, r.db); err != nil {
-		return nil, err
+	// 🚨 SECURITY: Only site admins can list all users on sourcegraph.com.
+	if dotcom.SourcegraphDotComMode() {
+		if err := auth.CheckCurrentUserIsSiteAdmin(ctx, r.db); err != nil {
+			return nil, err
+		}
 	}
 
 	opt := database.UsersListOptions{
@@ -141,13 +143,3 @@ func (r *userConnectionResolver) PageInfo(ctx context.Context) (*graphqlutil.Pag
 	}
 	return graphqlutil.HasNextPage(false), nil
 }
-
-func checkMembersAccess(ctx context.Context, db database.DB) error {
-	// 🚨 SECURITY: Only site admins can list users on sourcegraph.com.
-	if dotcom.SourcegraphDotComMode() {
-		if err := auth.CheckCurrentUserIsSiteAdmin(ctx, db); err != nil {
-			return err
-		}
-	}
-	return nil
-}