perf(target): Use new Permissions object to determine resource access

Using the new Permissions object, we can build a list of the level of
access a user should have based on the grants they've been given,
meaining we don't need to fetch all resources from the database to then
remove the ones the user shouldn't be able to see.

This commit also removes some unused options in `ListTargets`
for the sake of simplicity.
pull/2342/head
Hugo Vieira 4 years ago committed by Timothy Messier
parent 63196ee4ca
commit bf263fbd7d
No known key found for this signature in database
GPG Key ID: EFD2F184F7600572

@ -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
}

@ -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.

@ -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)
})
}
}

@ -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))
}

Loading…
Cancel
Save