Bug: fix groups in children scopes being filtered out by grants

internal/authtoken/testing.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/boundary/internal/db"
 	"github.com/hashicorp/boundary/internal/iam"
 	"github.com/hashicorp/boundary/internal/kms"
+	"github.com/hashicorp/go-uuid"
 	"github.com/stretchr/testify/require"
 )
 
@@ -46,3 +47,38 @@ func TestAuthToken(t testing.TB, conn *db.DB, kms *kms.Kms, scopeId string, opt
 	require.NoError(t, err)
 	return at
 }
+
+// TestRoleGrantsForToken contains information used by TestAuthTokenWithRoles to create
+// roles and their associated grants (with grant scopes)
+type TestRoleGrantsForToken struct {
+	RoleScopeID  string
+	GrantStrings []string
+	GrantScopes  []string
+}
+
+// TestAuthTokenWithRoles creates auth token associated with roles as requested by the caller along
+// with any required resources to achieve said token
+func TestAuthTokenWithRoles(t testing.TB, conn *db.DB, kms *kms.Kms, scopeId string, roles []TestRoleGrantsForToken) *AuthToken {
+	t.Helper()
+	ctx := context.Background()
+	rw := db.New(conn)
+	atRepo, err := NewRepository(ctx, rw, rw, kms)
+	require.NoError(t, err)
+
+	iamRepo, err := iam.NewRepository(ctx, rw, rw, kms)
+	require.NoError(t, err)
+
+	authMethod := password.TestAuthMethods(t, conn, scopeId, 1)[0]
+
+	loginName, err := uuid.GenerateUUID()
+	require.NoError(t, err)
+	acct := password.TestAccount(t, conn, authMethod.GetPublicId(), loginName)
+	user := iam.TestUser(t, iamRepo, scopeId, iam.WithAccountIds(acct.GetPublicId()))
+	for _, r := range roles {
+		role := iam.TestRoleWithGrants(t, conn, r.RoleScopeID, r.GrantScopes, r.GrantStrings)
+		_ = iam.TestUserRole(t, conn, role.PublicId, user.PublicId)
+	}
+	fullGrantToken, err := atRepo.CreateAuthToken(ctx, user, acct.GetPublicId())
+	require.NoError(t, err)
+	return fullGrantToken
+}

internal/daemon/controller/auth/testing.go

@@ -5,11 +5,19 @@ package auth
 
 import (
 	"context"
+	"testing"
 
 	"github.com/hashicorp/boundary/globals"
+	"github.com/hashicorp/boundary/internal/authtoken"
 	"github.com/hashicorp/boundary/internal/daemon/controller/common"
+	"github.com/hashicorp/boundary/internal/db"
 	authpb "github.com/hashicorp/boundary/internal/gen/controller/auth"
+	"github.com/hashicorp/boundary/internal/iam"
+	"github.com/hashicorp/boundary/internal/kms"
 	"github.com/hashicorp/boundary/internal/requests"
+	"github.com/hashicorp/boundary/internal/server"
+	wrapping "github.com/hashicorp/go-kms-wrapping/v2"
+	"github.com/stretchr/testify/require"
 )
 
 // DisabledAuthTestContext is meant for testing, and uses a context that has
@@ -30,3 +38,30 @@ func DisabledAuthTestContext(iamRepoFn common.IamRepoFactory, scopeId string, op
 	requestContext := context.WithValue(context.Background(), requests.ContextRequestInformationKey, &requests.RequestContext{})
 	return NewVerifierContext(requestContext, iamRepoFn, nil, nil, opts.withKms, &reqInfo)
 }
+
+// TestAuthContextFromToken creates an auth context with provided token
+// This is used in conjunction with TestAuthTokenWithRoles which creates a test token
+func TestAuthContextFromToken(t *testing.T, conn *db.DB, wrap wrapping.Wrapper, token *authtoken.AuthToken, iamRepo *iam.Repository) context.Context {
+	t.Helper()
+	ctx := context.Background()
+	rw := db.New(conn)
+	kmsCache := kms.TestKms(t, conn, wrap)
+	atRepo, err := authtoken.NewRepository(ctx, rw, rw, kmsCache)
+	require.NoError(t, err)
+	serversRepoFn := func() (*server.Repository, error) {
+		return server.NewRepository(ctx, rw, rw, kmsCache)
+	}
+	iamRepoFn := func() (*iam.Repository, error) {
+		return iamRepo, nil
+	}
+	atRepoFn := func() (*authtoken.Repository, error) {
+		return atRepo, nil
+	}
+	fullGrantAuthCtx := NewVerifierContext(requests.NewRequestContext(ctx, requests.WithUserId(token.GetIamUserId())),
+		iamRepoFn, atRepoFn, serversRepoFn, kmsCache, &authpb.RequestInfo{
+			PublicId:    token.PublicId,
+			Token:       token.GetToken(),
+			TokenFormat: uint32(AuthTokenTypeBearer),
+		})
+	return fullGrantAuthCtx
+}

internal/daemon/controller/handlers/groups/grants_test.go

@@ -0,0 +1,121 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package groups_test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/hashicorp/boundary/globals"
+	"github.com/hashicorp/boundary/internal/authtoken"
+	"github.com/hashicorp/boundary/internal/daemon/controller/auth"
+	"github.com/hashicorp/boundary/internal/daemon/controller/handlers/groups"
+	"github.com/hashicorp/boundary/internal/db"
+	pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services"
+	"github.com/hashicorp/boundary/internal/iam"
+	"github.com/hashicorp/boundary/internal/kms"
+	"github.com/stretchr/testify/require"
+)
+
+// TestGrants_ReadActions tests read actions to assert that grants are being applied properly
+//
+//	 Role - which scope the role is created in
+//			- global level
+//			- org level
+//			- project level
+//		Grant - what IAM grant scope is set for the permission
+//			- global: descendant
+//			- org: children
+//			- project
+//		Scopes [resource]:
+//			- global [globalGroup]
+//				- org1 [org1Group]
+//					- proj1 [proj1Group]
+//				- org2 [org2Group]
+//					- proj2 [proj2Group]
+//					- proj3 [proj3Group]
+func TestGrants_ReadActions(t *testing.T) {
+	ctx := context.Background()
+	conn, _ := db.TestSetup(t, "postgres")
+	wrap := db.TestWrapper(t)
+	iamRepo := iam.TestRepo(t, conn, wrap)
+	repoFn := func() (*iam.Repository, error) {
+		return iamRepo, nil
+	}
+	kmsCache := kms.TestKms(t, conn, wrap)
+	s, err := groups.NewService(ctx, repoFn, 1000)
+	require.NoError(t, err)
+	org1, _ := iam.TestScopes(t, iamRepo)
+	org2, proj2 := iam.TestScopes(t, iamRepo)
+	proj3 := iam.TestProject(t, iamRepo, org2.PublicId)
+	globalGroup := iam.TestGroup(t, conn, globals.GlobalPrefix, iam.WithDescription("global"), iam.WithName("global"))
+	org1Group := iam.TestGroup(t, conn, org1.GetPublicId(), iam.WithDescription("org1"), iam.WithName("org1"))
+	org2Group := iam.TestGroup(t, conn, org2.GetPublicId(), iam.WithDescription("org2"), iam.WithName("org2"))
+
+	proj2Group := iam.TestGroup(t, conn, proj2.GetPublicId(), iam.WithDescription("proj2"), iam.WithName("proj2"))
+	proj3Group := iam.TestGroup(t, conn, proj3.GetPublicId(), iam.WithDescription("proj3"), iam.WithName("proj3"))
+
+	t.Run("List", func(t *testing.T) {
+		testcases := []struct {
+			name          string
+			input         *pbs.ListGroupsRequest
+			rolesToCreate []authtoken.TestRoleGrantsForToken
+			wantErr       error
+			wantIDs       []string
+		}{
+			{
+				name: "global role grant this and children returns global and org groups",
+				input: &pbs.ListGroupsRequest{
+					ScopeId:   globals.GlobalPrefix,
+					Recursive: true,
+				},
+				rolesToCreate: []authtoken.TestRoleGrantsForToken{
+					{
+						RoleScopeID:  globals.GlobalPrefix,
+						GrantStrings: []string{"ids=*;type=group;actions=list,read"},
+						GrantScopes:  []string{globals.GrantScopeThis, globals.GrantScopeChildren},
+					},
+				},
+				wantErr: nil,
+				wantIDs: []string{globalGroup.PublicId, org1Group.PublicId, org2Group.PublicId},
+			},
+			{
+				name: "org role grant this and children returns org and project groups",
+				input: &pbs.ListGroupsRequest{
+					ScopeId:   org2.PublicId,
+					Recursive: true,
+				},
+				rolesToCreate: []authtoken.TestRoleGrantsForToken{
+					{
+						RoleScopeID:  org2.PublicId,
+						GrantStrings: []string{"ids=*;type=group;actions=list,read"},
+						GrantScopes:  []string{globals.GrantScopeThis, globals.GrantScopeChildren},
+					},
+				},
+				wantErr: nil,
+				wantIDs: []string{org2Group.PublicId, proj2Group.PublicId, proj3Group.PublicId},
+			},
+		}
+
+		for _, tc := range testcases {
+			t.Run(tc.name, func(t *testing.T) {
+				tok := authtoken.TestAuthTokenWithRoles(t, conn, kmsCache, globals.GlobalPrefix, tc.rolesToCreate)
+				fullGrantAuthCtx := auth.TestAuthContextFromToken(t, conn, wrap, tok, iamRepo)
+				got, finalErr := s.ListGroups(fullGrantAuthCtx, tc.input)
+				if tc.wantErr != nil {
+					require.ErrorIs(t, finalErr, tc.wantErr)
+					return
+				}
+				require.NoError(t, finalErr)
+				var gotIDs []string
+				for _, g := range got.Items {
+					fmt.Println(g.GetName())
+					gotIDs = append(gotIDs, g.GetId())
+				}
+				require.ElementsMatch(t, tc.wantIDs, gotIDs)
+			})
+		}
+	})
+}

internal/daemon/controller/handlers/groups/group_service.go

@@ -840,6 +840,7 @@ func newOutputOpts(ctx context.Context, item *iam.Group, scopeInfoMap map[string
 	}
 	res.Id = item.GetPublicId()
 	res.ScopeId = item.GetScopeId()
+	res.ParentScopeId = scopeInfoMap[item.GetScopeId()].GetParentScopeId()
 	authorizedActions := authResults.FetchActionSetForId(ctx, item.GetPublicId(), IdActions, auth.WithResource(&res))
 	if len(authorizedActions) == 0 {
 		return nil, false

internal/iam/testing.go

@@ -223,6 +223,36 @@ func TestRole(t testing.TB, conn *db.DB, scopeId string, opt ...Option) *Role {
 	return role
 }
 
+// TestRoleWithGrants creates a role suitable for testing along with grants
+// Functional options for GrantScopeIDs aren't used to express that
+// this function does not provide any default grant scope unlike TestRole
+func TestRoleWithGrants(t testing.TB, conn *db.DB, scopeId string, grantScopeIDs []string, grants []string) *Role {
+	t.Helper()
+
+	ctx := context.Background()
+	require := require.New(t)
+	rw := db.New(conn)
+
+	role, err := NewRole(ctx, scopeId)
+	require.NoError(err)
+	id, err := newRoleId(ctx)
+	require.NoError(err)
+	role.PublicId = id
+	require.NoError(rw.Create(ctx, role))
+	require.NotEmpty(role.PublicId)
+
+	for _, gsi := range grantScopeIDs {
+		gs, err := NewRoleGrantScope(ctx, id, gsi)
+		require.NoError(err)
+		require.NoError(rw.Create(ctx, gs))
+		role.GrantScopes = append(role.GrantScopes, gs)
+	}
+	for _, g := range grants {
+		_ = TestRoleGrant(t, conn, role.PublicId, g)
+	}
+	return role
+}
+
 func TestRoleGrant(t testing.TB, conn *db.DB, roleId, grant string, opt ...Option) *RoleGrant {
 	t.Helper()
 	require := require.New(t)