diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index f5c45dfc2f..f2b603e83a 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -15,7 +15,6 @@ import ( "github.com/hashicorp/boundary/internal/credential/vault" "github.com/hashicorp/boundary/internal/daemon/controller/auth" "github.com/hashicorp/boundary/internal/daemon/controller/common" - "github.com/hashicorp/boundary/internal/daemon/controller/common/scopeids" "github.com/hashicorp/boundary/internal/daemon/controller/handlers" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" @@ -33,6 +32,7 @@ import ( "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/boundary/internal/types/subtypes" + "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/scopes" pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/targets" "github.com/hashicorp/go-bexpr" "github.com/hashicorp/go-secure-stdlib/strutil" @@ -183,34 +183,24 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( } } - repo, err := s.repoFn() - if err != nil { - return nil, errors.Wrap(ctx, err, op) + var err error + var authzScopes map[string]*scopes.ScopeInfo + if req.GetRecursive() { + authzScopes, err = authResults.ScopesAuthorizedForList(ctx, req.GetScopeId(), resource.Target) + } else { + authzScopes = map[string]*scopes.ScopeInfo{authResults.Scope.Id: authResults.Scope} } - - scopeResourceInfo, err := scopeids.GetListingResourceInformation( - ctx, - scopeids.GetListingResourceInformationInput{ - IamRepoFn: s.iamRepoFn, - AuthResults: authResults, - RootScopeId: req.GetScopeId(), - Type: resource.Target, - Recursive: req.GetRecursive(), - AuthzProtectedEntityProvider: repo, - ActionSet: IdActions, - }, - ) if err != nil { return nil, err } - // If no scopes match, return an empty response - if len(scopeResourceInfo.ScopeIds) == 0 || - len(scopeResourceInfo.ResourceIds) == 0 { + // Get all user permissions for the requested scope(s). + userPerms := authResults.ACL().ListPermissions(authzScopes, resource.Target, IdActions) + if len(userPerms) == 0 { return &pbs.ListTargetsResponse{}, nil } - tl, err := s.listFromRepo(ctx, scopeResourceInfo.ResourceIds) + tl, err := s.listFromRepo(ctx, userPerms) if err != nil { return nil, err } @@ -222,19 +212,21 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( if err != nil { return nil, err } + finalItems := make([]*pb.Target, 0, len(tl)) - res := perms.Resource{ - Type: resource.Target, - } for _, item := range tl { - outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) + pr := perms.Resource{Id: item.GetPublicId(), ScopeId: item.GetProjectId(), Type: resource.Target} + outputFields := authResults.FetchOutputFields(pr, action.List).SelfOrDefaults(authResults.UserId) + outputOpts := make([]handlers.Option, 0, 3) outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + if outputFields.Has(globals.ScopeField) { - outputOpts = append(outputOpts, handlers.WithScope(scopeResourceInfo.ScopeResourceMap[item.GetProjectId()].ScopeInfo)) + outputOpts = append(outputOpts, handlers.WithScope(authzScopes[item.GetProjectId()])) } if outputFields.Has(globals.AuthorizedActionsField) { - outputOpts = append(outputOpts, handlers.WithAuthorizedActions(scopeResourceInfo.ScopeResourceMap[item.GetProjectId()].Resources[item.GetPublicId()].AuthorizedActions.Strings())) + authorizedActions := authResults.FetchActionSetForId(ctx, item.GetPublicId(), IdActions, auth.WithResource(&pr)).Strings() + outputOpts = append(outputOpts, handlers.WithAuthorizedActions(authorizedActions)) } item, err := toProto(ctx, item, nil, nil, outputOpts...) @@ -250,6 +242,7 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( finalItems = append(finalItems, item) } } + return &pbs.ListTargetsResponse{Items: finalItems}, nil } @@ -1233,12 +1226,12 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { return rows > 0, nil } -func (s Service) listFromRepo(ctx context.Context, targetIds []string) ([]target.Target, error) { - repo, err := s.repoFn() +func (s Service) listFromRepo(ctx context.Context, perms []perms.Permission) ([]target.Target, error) { + repo, err := s.repoFn(target.WithPermissions(perms)) if err != nil { return nil, err } - ul, err := repo.ListTargets(ctx, target.WithTargetIds(targetIds)) + ul, err := repo.ListTargets(ctx) if err != nil { return nil, err } diff --git a/internal/target/repository.go b/internal/target/repository.go index 07594e92d3..7ec5d1b2df 100644 --- a/internal/target/repository.go +++ b/internal/target/repository.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/internal/perms" + "github.com/hashicorp/boundary/internal/types/action" "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/go-dbw" @@ -212,59 +213,36 @@ func (r *Repository) FetchAuthzProtectedEntitiesByScope(ctx context.Context, pro return targetsMap, nil } -// ListTargets in targets in a project. Supports the WithProjectId, WithLimit, WithType options. +// ListTargets lists targets in a project based on the data in the WithPermissions option +// provided to the Repository constructor. If no permissions are available, this function +// is a no-op. +// Supports WithLimit which overrides the limit set in the Repository object. func (r *Repository) ListTargets(ctx context.Context, opt ...Option) ([]Target, error) { const op = "target.(Repository).ListTargets" - opts := GetOpts(opt...) - if len(opts.WithProjectIds) == 0 && opts.WithUserId == "" && len(opts.WithTargetIds) == 0 { - return nil, errors.New(ctx, errors.InvalidParameter, op, "must specify either project ids, target ids, or user id") - } - // TODO (jimlambrt 8/2020) - implement WithUserId() optional filtering. - var where []string - var args []interface{} - inClauseCnt := 0 - switch len(opts.WithProjectIds) { - case 0: - case 1: - inClauseCnt += 1 - where, args = append(where, fmt.Sprintf("project_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.WithProjectIds[0])) - default: - idsInClause := make([]string, 0, len(opts.WithProjectIds)) - for _, id := range opts.WithProjectIds { - inClauseCnt += 1 - idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) - } - where = append(where, fmt.Sprintf("project_id in (%s)", strings.Join(idsInClause, ","))) + if len(r.permissions) == 0 { + return []Target{}, nil } - switch len(opts.WithTargetIds) { - case 0: - case 1: - inClauseCnt += 1 - where, args = append(where, fmt.Sprintf("public_id = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.WithTargetIds[0])) - default: - idsInClause := make([]string, 0, len(opts.WithTargetIds)) - for _, id := range opts.WithTargetIds { - inClauseCnt += 1 - idsInClause, args = append(idsInClause, fmt.Sprintf("@%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), id)) - } - where = append(where, fmt.Sprintf("public_id in (%s)", strings.Join(idsInClause, ","))) + where, args := r.listPermissionWhereClauses() + if len(where) == 0 { + return []Target{}, nil } - if opts.WithType != "" { - inClauseCnt += 1 - where, args = append(where, fmt.Sprintf("type = @%d", inClauseCnt)), append(args, sql.Named(fmt.Sprintf("%d", inClauseCnt), opts.WithType.String())) + opts := GetOpts(opt...) + limit := r.defaultLimit + if opts.WithLimit != 0 { + limit = opts.WithLimit } var foundTargets []*targetView - err := r.list(ctx, &foundTargets, strings.Join(where, " and "), args, opt...) + err := r.reader.SearchWhere(ctx, &foundTargets, strings.Join(where, " or "), args, + db.WithLimit(limit)) if err != nil { return nil, errors.Wrap(ctx, err, op) } targets := make([]Target, 0, len(foundTargets)) - for _, t := range foundTargets { subtype, err := t.targetSubtype(ctx) if err != nil { @@ -272,25 +250,34 @@ func (r *Repository) ListTargets(ctx context.Context, opt ...Option) ([]Target, } targets = append(targets, subtype) } + return targets, nil } -// list will return a listing of resources and honor the WithLimit option or the -// repo defaultLimit -func (r *Repository) list(ctx context.Context, resources interface{}, where string, args []interface{}, opt ...Option) error { - const op = "target.(Repository).list" - opts := GetOpts(opt...) - limit := r.defaultLimit - var dbOpts []db.Option - if opts.WithLimit != 0 { - // non-zero signals an override of the default limit for the repo. - limit = opts.WithLimit - } - dbOpts = append(dbOpts, db.WithLimit(limit)) - if err := r.reader.SearchWhere(ctx, resources, where, args, dbOpts...); err != nil { - return errors.Wrap(ctx, err, op) +func (r *Repository) listPermissionWhereClauses() ([]string, []interface{}) { + var where []string + var args []interface{} + + inClauseCnt := 0 + for _, p := range r.permissions { + if p.Action != action.List { + continue + } + inClauseCnt++ + + var clauses []string + clauses = append(clauses, fmt.Sprintf("project_id = @project_id_%d", inClauseCnt)) + args = append(args, sql.Named(fmt.Sprintf("project_id_%d", inClauseCnt), p.ScopeId)) + + if len(p.ResourceIds) > 0 { + clauses = append(clauses, fmt.Sprintf("public_id = any(@public_id_%d)", inClauseCnt)) + args = append(args, sql.Named(fmt.Sprintf("public_id_%d", inClauseCnt), "{"+strings.Join(p.ResourceIds, ",")+"}")) + } + + where = append(where, fmt.Sprintf("(%s)", strings.Join(clauses, " and "))) } - return nil + + return where, args } // DeleteTarget will delete a target from the repository. diff --git a/internal/target/repository_test.go b/internal/target/repository_test.go index e26b06c4f5..8468756f24 100644 --- a/internal/target/repository_test.go +++ b/internal/target/repository_test.go @@ -1,11 +1,13 @@ package target import ( + "database/sql" "testing" "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/perms" + "github.com/hashicorp/boundary/internal/types/action" "github.com/hashicorp/boundary/internal/types/resource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -136,3 +138,173 @@ func TestNewRepository(t *testing.T) { }) } } + +func TestRepositoryListPermissionWhereClauses(t *testing.T) { + tests := []struct { + name string + perms []perms.Permission + expWhere []string + expArgs []any + }{ + { + name: "nilPerms", + perms: nil, + expWhere: []string{}, + expArgs: []any{}, + }, + { + name: "emptyPerms", + perms: []perms.Permission{}, + expWhere: []string{}, + expArgs: []any{}, + }, + { + name: "noListActionPerms", + perms: []perms.Permission{ + { + ScopeId: "scope_a", + Action: action.Create, + }, + { + ScopeId: "scope_b", + Action: action.Read, + }, + { + ScopeId: "scope_c", + Action: action.Delete, + }, + }, + expWhere: []string{}, + expArgs: []any{}, + }, + { + name: "onePermissionAllResources", + perms: []perms.Permission{ + { + ScopeId: "scope_a", + Action: action.List, + }, + }, + expWhere: []string{"(project_id = @project_id_1)"}, + expArgs: []any{sql.Named("project_id_1", "scope_a")}, + }, + { + name: "onePermissionAllResourcesNonListIgnored", + perms: []perms.Permission{ + { + ScopeId: "scope_a", + Action: action.List, + }, + { + ScopeId: "scope_b", + Action: action.Create, + }, + }, + expWhere: []string{"(project_id = @project_id_1)"}, + expArgs: []any{sql.Named("project_id_1", "scope_a")}, + }, + { + name: "onePermissionResourceIds", + perms: []perms.Permission{ + { + ScopeId: "scope_a", + Action: action.List, + ResourceIds: []string{"resourceid1", "resourceid2"}, + }, + }, + expWhere: []string{"(project_id = @project_id_1 and public_id = any(@public_id_1))"}, + expArgs: []any{ + sql.Named("project_id_1", "scope_a"), + sql.Named("public_id_1", "{resourceid1,resourceid2}"), + }, + }, + { + name: "multiplePermissionsAllResources", + perms: []perms.Permission{ + {ScopeId: "scope_a", Action: action.List}, + {ScopeId: "scope_b", Action: action.List}, + {ScopeId: "scope_c", Action: action.List}, + {ScopeId: "scope_d", Action: action.List}, + }, + expWhere: []string{ + "(project_id = @project_id_1)", + "(project_id = @project_id_2)", + "(project_id = @project_id_3)", + "(project_id = @project_id_4)", + }, + expArgs: []any{ + sql.Named("project_id_1", "scope_a"), + sql.Named("project_id_2", "scope_b"), + sql.Named("project_id_3", "scope_c"), + sql.Named("project_id_4", "scope_d"), + }, + }, + { + name: "multiplePermissionsResourceIds", + perms: []perms.Permission{ + { + ScopeId: "scope_a", + Action: action.List, + ResourceIds: []string{"resourceid1", "resourceid2"}, + }, + { + ScopeId: "scope_b", + Action: action.List, + ResourceIds: []string{"resourceid3", "resourceid4"}, + }, + }, + expWhere: []string{ + "(project_id = @project_id_1 and public_id = any(@public_id_1))", + "(project_id = @project_id_2 and public_id = any(@public_id_2))", + }, + expArgs: []any{ + sql.Named("project_id_1", "scope_a"), + sql.Named("project_id_2", "scope_b"), + sql.Named("public_id_1", "{resourceid1,resourceid2}"), + sql.Named("public_id_2", "{resourceid3,resourceid4}"), + }, + }, + { + name: "multiplePermissionsMix", + perms: []perms.Permission{ + { + ScopeId: "scope_a", + Action: action.List, + ResourceIds: []string{"resourceid1", "resourceid2"}, + }, + { + ScopeId: "scope_b", + Action: action.List, + ResourceIds: []string{"resourceid3", "resourceid4"}, + }, + {ScopeId: "scope_c", Action: action.List}, + {ScopeId: "scope_d", Action: action.List}, + }, + expWhere: []string{ + "(project_id = @project_id_1 and public_id = any(@public_id_1))", + "(project_id = @project_id_2 and public_id = any(@public_id_2))", + "(project_id = @project_id_3)", + "(project_id = @project_id_4)", + }, + expArgs: []any{ + sql.Named("project_id_1", "scope_a"), + sql.Named("public_id_1", "{resourceid1,resourceid2}"), + sql.Named("project_id_2", "scope_b"), + sql.Named("public_id_2", "{resourceid3,resourceid4}"), + sql.Named("project_id_3", "scope_c"), + sql.Named("project_id_4", "scope_d"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := Repository{} + repo.permissions = tt.perms + + where, args := repo.listPermissionWhereClauses() + require.ElementsMatch(t, tt.expWhere, where) + require.ElementsMatch(t, tt.expArgs, args) + }) + } +} diff --git a/internal/target/tcp/repository_test.go b/internal/target/tcp/repository_test.go index d5ca11008a..b7c8e3c60f 100644 --- a/internal/target/tcp/repository_test.go +++ b/internal/target/tcp/repository_test.go @@ -3,7 +3,6 @@ package tcp_test import ( "context" "fmt" - "strconv" "testing" "time" @@ -12,8 +11,11 @@ import ( "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/boundary/internal/perms" "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/target/tcp" + "github.com/hashicorp/boundary/internal/types/action" + "github.com/hashicorp/boundary/internal/types/resource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -135,119 +137,15 @@ func TestRepository_LookupTarget(t *testing.T) { } } -func TestRepository_ListTargets(t *testing.T) { - t.Parallel() - conn, _ := db.TestSetup(t, "postgres") - const testLimit = 10 - wrapper := db.TestWrapper(t) - testKms := kms.TestKms(t, conn, wrapper) - iamRepo := iam.TestRepo(t, conn, wrapper) - _, proj := iam.TestScopes(t, iamRepo) - rw := db.New(conn) - repo, err := target.NewRepository(rw, rw, testKms, target.WithLimit(testLimit)) - require.NoError(t, err) - - type args struct { - opt []target.Option - } - tests := []struct { - name string - createCnt int - createProjectId string - createProjectId2 string - grantUserId string - args args - wantCnt int - wantErr bool - }{ - { - name: "tcp-target", - createCnt: 5, - createProjectId: proj.PublicId, - args: args{ - opt: []target.Option{target.WithType(tcp.Subtype), target.WithProjectIds([]string{proj.PublicId})}, - }, - wantCnt: 5, - wantErr: false, - }, - { - name: "no-limit", - createCnt: testLimit + 1, - createProjectId: proj.PublicId, - args: args{ - opt: []target.Option{target.WithLimit(-1), target.WithProjectIds([]string{proj.PublicId})}, - }, - wantCnt: testLimit + 1, - wantErr: false, - }, - { - name: "default-limit", - createCnt: testLimit + 1, - createProjectId: proj.PublicId, - args: args{ - opt: []target.Option{target.WithProjectIds([]string{proj.PublicId})}, - }, - wantCnt: testLimit, - wantErr: false, - }, - { - name: "custom-limit", - createCnt: testLimit + 1, - createProjectId: proj.PublicId, - args: args{ - opt: []target.Option{target.WithLimit(3), target.WithProjectIds([]string{proj.PublicId})}, - }, - wantCnt: 3, - wantErr: false, - }, - { - name: "bad-org", - createCnt: 1, - createProjectId: proj.PublicId, - args: args{ - opt: []target.Option{target.WithProjectIds([]string{"bad-id"})}, - }, - wantCnt: 0, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert, require := assert.New(t), require.New(t) - ctx := context.Background() - db.TestDeleteWhere(t, conn, tcp.NewTestTarget(""), "1=1") - testGroups := []target.Target{} - for i := 0; i < tt.createCnt; i++ { - switch { - case tt.createProjectId2 != "" && i%2 == 0: - testGroups = append(testGroups, tcp.TestTarget(ctx, t, conn, tt.createProjectId2, strconv.Itoa(i))) - default: - testGroups = append(testGroups, tcp.TestTarget(ctx, t, conn, tt.createProjectId, strconv.Itoa(i))) - } - } - assert.Equal(tt.createCnt, len(testGroups)) - got, err := repo.ListTargets(ctx, tt.args.opt...) - if tt.wantErr { - require.Error(err) - return - } - require.NoError(err) - assert.Equal(tt.wantCnt, len(got)) - }) - } -} - func TestRepository_ListRoles_Multiple_Scopes(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") wrapper := db.TestWrapper(t) testKms := kms.TestKms(t, conn, wrapper) iamRepo := iam.TestRepo(t, conn, wrapper) + _, proj1 := iam.TestScopes(t, iamRepo) _, proj2 := iam.TestScopes(t, iamRepo) - rw := db.New(conn) - repo, err := target.NewRepository(rw, rw, testKms) - require.NoError(t, err) db.TestDeleteWhere(t, conn, tcp.NewTestTarget(""), "1=1") @@ -261,7 +159,26 @@ func TestRepository_ListRoles_Multiple_Scopes(t *testing.T) { total++ } - got, err := repo.ListTargets(ctx, target.WithProjectIds([]string{"global", proj1.GetPublicId(), proj2.GetPublicId()})) + rw := db.New(conn) + repo, err := target.NewRepository(rw, rw, testKms, + target.WithPermissions([]perms.Permission{ + { + ScopeId: proj1.PublicId, + Resource: resource.Target, + Action: action.List, + All: true, + }, + { + ScopeId: proj2.PublicId, + Resource: resource.Target, + Action: action.List, + All: true, + }, + }), + ) + require.NoError(t, err) + + got, err := repo.ListTargets(ctx) require.NoError(t, err) assert.Equal(t, total, len(got)) }