From 61c90c562398ae0220d84ce55192b765df19a089 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Fri, 6 Jan 2023 17:27:19 -0600 Subject: [PATCH] Add grant parsing fuzz test (#2534) * testing(perms): Add Parse fuzz test I ran this for a few minutes and it didn't discover anything catastrophic. * Add roundtrip tests * Fix invalid unicode parsing, empty output fields * Do the unicode conversion once on input * Remove incorrect fix for empty output fields * Allow empty output_fields This is the same behavior as output_fields=, * Add tests back, make JSON behaviour consistent * Rework the output fields mechanism to fix some bugs such as properly handling no fields * Change signature of Fields Co-authored-by: Jeff Mitchell --- internal/daemon/controller/auth/auth.go | 9 +- .../handlers/accounts/account_service.go | 12 +- .../authmethods/authmethod_service.go | 10 +- .../handlers/authtokens/authtoken_service.go | 6 +- .../credentiallibrary_service.go | 8 +- .../credentials/credential_service.go | 8 +- .../credentialstore_service.go | 8 +- .../handlers/groups/group_service.go | 14 +- .../host_catalogs/host_catalog_service.go | 8 +- .../handlers/host_sets/host_set_service.go | 14 +- .../controller/handlers/hosts/host_service.go | 8 +- .../managed_groups/managed_group_service.go | 8 +- .../daemon/controller/handlers/option_test.go | 6 +- .../daemon/controller/handlers/options.go | 4 +- .../controller/handlers/roles/role_service.go | 20 +- .../handlers/scopes/scope_service.go | 12 +- .../handlers/scopes/scope_service_test.go | 6 +- .../handlers/sessions/session_service.go | 10 +- .../handlers/targets/target_service.go | 20 +- .../controller/handlers/users/user_service.go | 14 +- .../handlers/workers/worker_service.go | 14 +- internal/perms/acl.go | 8 +- internal/perms/acl_test.go | 3 +- internal/perms/grants.go | 57 +++--- internal/perms/grants_test.go | 139 +++++++++++--- internal/perms/output_fields.go | 135 +++++++++----- internal/perms/output_fields_test.go | 172 +++++++++++++----- ...0eb5c61865801c150feb07639363c7123ce92d5d83 | 2 + ...031a98efb46f623f95a9b8d3038982bd4cd36298b1 | 2 + internal/requests/requests.go | 4 +- 30 files changed, 476 insertions(+), 265 deletions(-) create mode 100644 internal/perms/testdata/fuzz/FuzzParse/1686a11fab3efeb7d05bf50eb5c61865801c150feb07639363c7123ce92d5d83 create mode 100644 internal/perms/testdata/fuzz/FuzzParse/3c6b045265fd967a6add10031a98efb46f623f95a9b8d3038982bd4cd36298b1 diff --git a/internal/daemon/controller/auth/auth.go b/internal/daemon/controller/auth/auth.go index 89f53e5d03..df55d792e3 100644 --- a/internal/daemon/controller/auth/auth.go +++ b/internal/daemon/controller/auth/auth.go @@ -720,17 +720,18 @@ func (r *VerifyResults) fetchActions(id string, typ resource.Type, availableActi return ret } -func (r *VerifyResults) FetchOutputFields(res perms.Resource, act action.Type) perms.OutputFieldsMap { +func (r *VerifyResults) FetchOutputFields(res perms.Resource, act action.Type) *perms.OutputFields { + var ret *perms.OutputFields switch { case r.v.requestInfo.TokenFormat == uint32(AuthTokenTypeRecoveryKms): - return perms.OutputFieldsMap{"*": true} + return ret.AddFields([]string{"*"}) case r.v.requestInfo.DisableAuthEntirely: - return nil + return ret case r.UserData.User.Id == nil: // If there is no user ID set by definition there are no actions to fetch. // This shouldn't happen because we should always fall back to at least the // anonymous user so it's defense in depth. - return nil + return ret } return r.v.acl.Allowed(res, act, *r.UserData.User.Id).OutputFields diff --git a/internal/daemon/controller/handlers/accounts/account_service.go b/internal/daemon/controller/handlers/accounts/account_service.go index 12af47fb2a..e20dd4713b 100644 --- a/internal/daemon/controller/handlers/accounts/account_service.go +++ b/internal/daemon/controller/handlers/accounts/account_service.go @@ -153,7 +153,7 @@ func (s Service) ListAccounts(ctx context.Context, req *pbs.ListAccountsRequest) outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -202,7 +202,7 @@ func (s Service) GetAccount(ctx context.Context, req *pbs.GetAccountRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -244,7 +244,7 @@ func (s Service) CreateAccount(ctx context.Context, req *pbs.CreateAccountReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -283,7 +283,7 @@ func (s Service) UpdateAccount(ctx context.Context, req *pbs.UpdateAccountReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -338,7 +338,7 @@ func (s Service) ChangePassword(ctx context.Context, req *pbs.ChangePasswordRequ } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -377,7 +377,7 @@ func (s Service) SetPassword(ctx context.Context, req *pbs.SetPasswordRequest) ( } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/authmethods/authmethod_service.go b/internal/daemon/controller/handlers/authmethods/authmethod_service.go index 127c84abdf..bc34cbc972 100644 --- a/internal/daemon/controller/handlers/authmethods/authmethod_service.go +++ b/internal/daemon/controller/handlers/authmethods/authmethod_service.go @@ -165,7 +165,7 @@ func (s Service) ListAuthMethods(ctx context.Context, req *pbs.ListAuthMethodsRe outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[am.GetScopeId()])) } @@ -220,7 +220,7 @@ func (s Service) GetAuthMethod(ctx context.Context, req *pbs.GetAuthMethodReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -265,7 +265,7 @@ func (s Service) CreateAuthMethod(ctx context.Context, req *pbs.CreateAuthMethod } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -315,7 +315,7 @@ func (s Service) UpdateAuthMethod(ctx context.Context, req *pbs.UpdateAuthMethod } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -369,7 +369,7 @@ func (s Service) ChangeState(ctx context.Context, req *pbs.ChangeStateRequest) ( } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/authtokens/authtoken_service.go b/internal/daemon/controller/handlers/authtokens/authtoken_service.go index be3450e910..377b589512 100644 --- a/internal/daemon/controller/handlers/authtokens/authtoken_service.go +++ b/internal/daemon/controller/handlers/authtokens/authtoken_service.go @@ -120,7 +120,7 @@ func (s Service) ListAuthTokens(ctx context.Context, req *pbs.ListAuthTokensRequ outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[at.GetScopeId()])) } @@ -157,7 +157,7 @@ func (s Service) GetAuthToken(ctx context.Context, req *pbs.GetAuthTokenRequest) return nil, err } - var outputFields perms.OutputFieldsMap + var outputFields *perms.OutputFields authorizedActions := authResults.FetchActionSetForId(ctx, at.GetPublicId(), IdActions) // Check to see if we need to verify Read vs. just ReadSelf @@ -179,7 +179,7 @@ func (s Service) GetAuthToken(ctx context.Context, req *pbs.GetAuthTokenRequest) } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go b/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go index c42ec1d70b..c8c3d03e15 100644 --- a/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go +++ b/internal/daemon/controller/handlers/credentiallibraries/credentiallibrary_service.go @@ -131,7 +131,7 @@ func (s Service) ListCredentialLibraries(ctx context.Context, req *pbs.ListCrede outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -177,7 +177,7 @@ func (s Service) GetCredentialLibrary(ctx context.Context, req *pbs.GetCredentia } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -215,7 +215,7 @@ func (s Service) CreateCredentialLibrary(ctx context.Context, req *pbs.CreateCre } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -266,7 +266,7 @@ func (s Service) UpdateCredentialLibrary(ctx context.Context, req *pbs.UpdateCre } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/credentials/credential_service.go b/internal/daemon/controller/handlers/credentials/credential_service.go index 558df4e8c6..f57826c082 100644 --- a/internal/daemon/controller/handlers/credentials/credential_service.go +++ b/internal/daemon/controller/handlers/credentials/credential_service.go @@ -135,7 +135,7 @@ func (s Service) ListCredentials(ctx context.Context, req *pbs.ListCredentialsRe outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -181,7 +181,7 @@ func (s Service) GetCredential(ctx context.Context, req *pbs.GetCredentialReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -219,7 +219,7 @@ func (s Service) CreateCredential(ctx context.Context, req *pbs.CreateCredential } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -266,7 +266,7 @@ func (s Service) UpdateCredential(ctx context.Context, req *pbs.UpdateCredential } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go b/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go index d97e8c3494..6b1c1f7c55 100644 --- a/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go +++ b/internal/daemon/controller/handlers/credentialstores/credentialstore_service.go @@ -169,7 +169,7 @@ func (s Service) ListCredentialStores(ctx context.Context, req *pbs.ListCredenti outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetProjectId()])) } @@ -222,7 +222,7 @@ func (s Service) GetCredentialStore(ctx context.Context, req *pbs.GetCredentialS } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -267,7 +267,7 @@ func (s Service) CreateCredentialStore(ctx context.Context, req *pbs.CreateCrede } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -315,7 +315,7 @@ func (s Service) UpdateCredentialStore(ctx context.Context, req *pbs.UpdateCrede } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/groups/group_service.go b/internal/daemon/controller/handlers/groups/group_service.go index c11b5bf938..f820ed5882 100644 --- a/internal/daemon/controller/handlers/groups/group_service.go +++ b/internal/daemon/controller/handlers/groups/group_service.go @@ -127,7 +127,7 @@ func (s Service) ListGroups(ctx context.Context, req *pbs.ListGroupsRequest) (*p outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetScopeId()])) } @@ -169,7 +169,7 @@ func (s Service) GetGroup(ctx context.Context, req *pbs.GetGroupRequest) (*pbs.G } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -207,7 +207,7 @@ func (s Service) CreateGroup(ctx context.Context, req *pbs.CreateGroupRequest) ( } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -245,7 +245,7 @@ func (s Service) UpdateGroup(ctx context.Context, req *pbs.UpdateGroupRequest) ( } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -299,7 +299,7 @@ func (s Service) AddGroupMembers(ctx context.Context, req *pbs.AddGroupMembersRe } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -337,7 +337,7 @@ func (s Service) SetGroupMembers(ctx context.Context, req *pbs.SetGroupMembersRe } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -375,7 +375,7 @@ func (s Service) RemoveGroupMembers(ctx context.Context, req *pbs.RemoveGroupMem } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go b/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go index 840fcde1c7..bab1a9341e 100644 --- a/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/daemon/controller/handlers/host_catalogs/host_catalog_service.go @@ -164,7 +164,7 @@ func (s Service) ListHostCatalogs(ctx context.Context, req *pbs.ListHostCatalogs outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetProjectId()])) } @@ -234,7 +234,7 @@ func (s Service) GetHostCatalog(ctx context.Context, req *pbs.GetHostCatalogRequ } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -291,7 +291,7 @@ func (s Service) CreateHostCatalog(ctx context.Context, req *pbs.CreateHostCatal } outputOpts := make([]handlers.Option, 0, 4) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -351,7 +351,7 @@ func (s Service) UpdateHostCatalog(ctx context.Context, req *pbs.UpdateHostCatal } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if plg != nil { outputOpts = append(outputOpts, handlers.WithPlugin(plg)) } diff --git a/internal/daemon/controller/handlers/host_sets/host_set_service.go b/internal/daemon/controller/handlers/host_sets/host_set_service.go index b3235c7cab..95c2dd2df8 100644 --- a/internal/daemon/controller/handlers/host_sets/host_set_service.go +++ b/internal/daemon/controller/handlers/host_sets/host_set_service.go @@ -137,7 +137,7 @@ func (s Service) ListHostSetsWithOptions(ctx context.Context, req *pbs.ListHostS outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -188,7 +188,7 @@ func (s Service) GetHostSet(ctx context.Context, req *pbs.GetHostSetRequest) (*p } outputOpts := make([]handlers.Option, 0, 4) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -230,7 +230,7 @@ func (s Service) CreateHostSet(ctx context.Context, req *pbs.CreateHostSetReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -275,7 +275,7 @@ func (s Service) UpdateHostSet(ctx context.Context, req *pbs.UpdateHostSetReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if plg != nil { outputOpts = append(outputOpts, handlers.WithPlugin(plg)) } @@ -333,7 +333,7 @@ func (s Service) AddHostSetHosts(ctx context.Context, req *pbs.AddHostSetHostsRe } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -371,7 +371,7 @@ func (s Service) SetHostSetHosts(ctx context.Context, req *pbs.SetHostSetHostsRe } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -410,7 +410,7 @@ func (s Service) RemoveHostSetHosts(ctx context.Context, req *pbs.RemoveHostSetH } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/hosts/host_service.go b/internal/daemon/controller/handlers/hosts/host_service.go index f1799ac032..1a5bf70f54 100644 --- a/internal/daemon/controller/handlers/hosts/host_service.go +++ b/internal/daemon/controller/handlers/hosts/host_service.go @@ -122,7 +122,7 @@ func (s Service) ListHosts(ctx context.Context, req *pbs.ListHostsRequest) (*pbs outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if plg != nil { outputOpts = append(outputOpts, handlers.WithPlugin(plg)) } @@ -173,7 +173,7 @@ func (s Service) GetHost(ctx context.Context, req *pbs.GetHostRequest) (*pbs.Get } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if plg != nil { outputOpts = append(outputOpts, handlers.WithPlugin(plg)) } @@ -215,7 +215,7 @@ func (s Service) CreateHost(ctx context.Context, req *pbs.CreateHostRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -257,7 +257,7 @@ func (s Service) UpdateHost(ctx context.Context, req *pbs.UpdateHostRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/managed_groups/managed_group_service.go b/internal/daemon/controller/handlers/managed_groups/managed_group_service.go index 0d99124c6b..40dc945d75 100644 --- a/internal/daemon/controller/handlers/managed_groups/managed_group_service.go +++ b/internal/daemon/controller/handlers/managed_groups/managed_group_service.go @@ -116,7 +116,7 @@ func (s Service) ListManagedGroups(ctx context.Context, req *pbs.ListManagedGrou outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -165,7 +165,7 @@ func (s Service) GetManagedGroup(ctx context.Context, req *pbs.GetManagedGroupRe } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -207,7 +207,7 @@ func (s Service) CreateManagedGroup(ctx context.Context, req *pbs.CreateManagedG } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -246,7 +246,7 @@ func (s Service) UpdateManagedGroup(ctx context.Context, req *pbs.UpdateManagedG } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/option_test.go b/internal/daemon/controller/handlers/option_test.go index e8a9bbb1c6..10b6c84ee2 100644 --- a/internal/daemon/controller/handlers/option_test.go +++ b/internal/daemon/controller/handlers/option_test.go @@ -39,16 +39,14 @@ func Test_GetOpts(t *testing.T) { }) t.Run("WithOutputFields", func(t *testing.T) { assert := assert.New(t) - require := require.New(t) opts := GetOpts() assert.Nil(opts.WithOutputFields) - var out perms.OutputFieldsMap + var out perms.OutputFields opts = GetOpts(WithOutputFields(&out)) - require.NotNil(opts.WithOutputFields) - assert.Nil(*opts.WithOutputFields) + assert.NotNil(opts.WithOutputFields) }) t.Run("WithManagedGroupIds", func(t *testing.T) { assert := assert.New(t) diff --git a/internal/daemon/controller/handlers/options.go b/internal/daemon/controller/handlers/options.go index 423a95bcae..7d757da062 100644 --- a/internal/daemon/controller/handlers/options.go +++ b/internal/daemon/controller/handlers/options.go @@ -25,7 +25,7 @@ type Option func(*options) type options struct { withDiscardUnknownFields bool WithUserIsAnonymous bool - WithOutputFields *perms.OutputFieldsMap + WithOutputFields *perms.OutputFields WithScope *scopes.ScopeInfo WithPlugin *plugins.PluginInfo WithAuthorizedActions []string @@ -58,7 +58,7 @@ func WithUserIsAnonymous(anonListing bool) Option { // WithOutputFields provides an option when creating responses to only include // specific fields -func WithOutputFields(fields *perms.OutputFieldsMap) Option { +func WithOutputFields(fields *perms.OutputFields) Option { return func(o *options) { o.WithOutputFields = fields } diff --git a/internal/daemon/controller/handlers/roles/role_service.go b/internal/daemon/controller/handlers/roles/role_service.go index 98d5331862..75de293ccb 100644 --- a/internal/daemon/controller/handlers/roles/role_service.go +++ b/internal/daemon/controller/handlers/roles/role_service.go @@ -131,7 +131,7 @@ func (s Service) ListRoles(ctx context.Context, req *pbs.ListRolesRequest) (*pbs outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetScopeId()])) } @@ -173,7 +173,7 @@ func (s Service) GetRole(ctx context.Context, req *pbs.GetRoleRequest) (*pbs.Get } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -211,7 +211,7 @@ func (s Service) CreateRole(ctx context.Context, req *pbs.CreateRoleRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -249,7 +249,7 @@ func (s Service) UpdateRole(ctx context.Context, req *pbs.UpdateRoleRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -303,7 +303,7 @@ func (s Service) AddRolePrincipals(ctx context.Context, req *pbs.AddRolePrincipa } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -341,7 +341,7 @@ func (s Service) SetRolePrincipals(ctx context.Context, req *pbs.SetRolePrincipa } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -379,7 +379,7 @@ func (s Service) RemoveRolePrincipals(ctx context.Context, req *pbs.RemoveRolePr } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -417,7 +417,7 @@ func (s Service) AddRoleGrants(ctx context.Context, req *pbs.AddRoleGrantsReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -455,7 +455,7 @@ func (s Service) SetRoleGrants(ctx context.Context, req *pbs.SetRoleGrantsReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -493,7 +493,7 @@ func (s Service) RemoveRoleGrants(ctx context.Context, req *pbs.RemoveRoleGrants } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/scopes/scope_service.go b/internal/daemon/controller/handlers/scopes/scope_service.go index dd24ff6718..370e07b7fe 100644 --- a/internal/daemon/controller/handlers/scopes/scope_service.go +++ b/internal/daemon/controller/handlers/scopes/scope_service.go @@ -184,7 +184,7 @@ func (s Service) ListScopes(ctx context.Context, req *pbs.ListScopesRequest) (*p outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetParentId()])) } @@ -234,7 +234,7 @@ func (s Service) GetScope(ctx context.Context, req *pbs.GetScopeRequest) (*pbs.G } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -284,7 +284,7 @@ func (s Service) CreateScope(ctx context.Context, req *pbs.CreateScopeRequest) ( } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -329,7 +329,7 @@ func (s Service) UpdateScope(ctx context.Context, req *pbs.UpdateScopeRequest) ( } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -399,7 +399,7 @@ func (s Service) ListKeys(ctx context.Context, req *pbs.ListKeysRequest) (*pbs.L outputFields := authResults.FetchOutputFields(res, action.ListScopeKeys).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -460,7 +460,7 @@ func (s Service) ListKeyVersionDestructionJobs(ctx context.Context, req *pbs.Lis outputFields := authResults.FetchOutputFields(res, action.ListScopeKeyVersionDestructionJobs).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/scopes/scope_service_test.go b/internal/daemon/controller/handlers/scopes/scope_service_test.go index 110df25caa..44495f727e 100644 --- a/internal/daemon/controller/handlers/scopes/scope_service_test.go +++ b/internal/daemon/controller/handlers/scopes/scope_service_test.go @@ -326,15 +326,15 @@ func TestList(t *testing.T) { _, err = repo.DeleteScope(context.Background(), p2.GetPublicId()) require.NoError(t, err) - outputFields := perms.OutputFieldsMap(nil).SelfOrDefaults(globals.AnyAuthenticatedUserId) + outputFields := new(perms.OutputFields).SelfOrDefaults(globals.AnyAuthenticatedUserId) var initialOrgs []*pb.Scope globalScope := &pb.ScopeInfo{Id: "global", Type: scope.Global.String(), Name: scope.Global.String(), Description: "Global Scope"} - oNoProjectsProto, err := scopes.ToProto(context.Background(), oNoProjects, handlers.WithOutputFields(&outputFields)) + oNoProjectsProto, err := scopes.ToProto(context.Background(), oNoProjects, handlers.WithOutputFields(outputFields)) require.NoError(t, err) oNoProjectsProto.Scope = globalScope oNoProjectsProto.AuthorizedActions = testAuthorizedActions oNoProjectsProto.AuthorizedCollectionActions = orgAuthorizedCollectionActions - oWithProjectsProto, err := scopes.ToProto(context.Background(), oWithProjects, handlers.WithOutputFields(&outputFields)) + oWithProjectsProto, err := scopes.ToProto(context.Background(), oWithProjects, handlers.WithOutputFields(outputFields)) require.NoError(t, err) oWithProjectsProto.Scope = globalScope oWithProjectsProto.AuthorizedActions = testAuthorizedActions diff --git a/internal/daemon/controller/handlers/sessions/session_service.go b/internal/daemon/controller/handlers/sessions/session_service.go index 351374a755..32f6e375c5 100644 --- a/internal/daemon/controller/handlers/sessions/session_service.go +++ b/internal/daemon/controller/handlers/sessions/session_service.go @@ -79,7 +79,7 @@ func (s Service) GetSession(ctx context.Context, req *pbs.GetSessionRequest) (*p return nil, err } - var outputFields perms.OutputFieldsMap + var outputFields *perms.OutputFields authorizedActions := authResults.FetchActionSetForId(ctx, ses.GetPublicId(), IdActions) // Check to see if we need to verify Read vs. just ReadSelf @@ -101,7 +101,7 @@ func (s Service) GetSession(ctx context.Context, req *pbs.GetSessionRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -184,7 +184,7 @@ func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest) outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeIds[item.ProjectId])) } @@ -236,7 +236,7 @@ func (s Service) CancelSession(ctx context.Context, req *pbs.CancelSessionReques return nil, handlers.NotFoundErrorf("Session %q doesn't exist.", req.GetId()) } - var outputFields perms.OutputFieldsMap + var outputFields *perms.OutputFields authorizedActions := authResults.FetchActionSetForId(ctx, ses.GetPublicId(), IdActions) // Check to see if we need to verify Cancel vs. just CancelSelf @@ -274,7 +274,7 @@ func (s Service) CancelSession(ctx context.Context, req *pbs.CancelSessionReques } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index 06fbbd3ba8..17c0500c7f 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -232,7 +232,7 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( outputFields := authResults.FetchOutputFields(pr, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authzScopes[item.GetProjectId()])) @@ -281,7 +281,7 @@ func (s Service) GetTarget(ctx context.Context, req *pbs.GetTargetRequest) (*pbs } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -319,7 +319,7 @@ func (s Service) CreateTarget(ctx context.Context, req *pbs.CreateTargetRequest) } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -357,7 +357,7 @@ func (s Service) UpdateTarget(ctx context.Context, req *pbs.UpdateTargetRequest) } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -411,7 +411,7 @@ func (s Service) AddTargetHostSources(ctx context.Context, req *pbs.AddTargetHos } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -449,7 +449,7 @@ func (s Service) SetTargetHostSources(ctx context.Context, req *pbs.SetTargetHos } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -487,7 +487,7 @@ func (s Service) RemoveTargetHostSources(ctx context.Context, req *pbs.RemoveTar } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -527,7 +527,7 @@ func (s Service) AddTargetCredentialSources(ctx context.Context, req *pbs.AddTar } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -567,7 +567,7 @@ func (s Service) SetTargetCredentialSources(ctx context.Context, req *pbs.SetTar } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -607,7 +607,7 @@ func (s Service) RemoveTargetCredentialSources(ctx context.Context, req *pbs.Rem } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/users/user_service.go b/internal/daemon/controller/handlers/users/user_service.go index 8101e3f5ca..2f6ad5ab6f 100644 --- a/internal/daemon/controller/handlers/users/user_service.go +++ b/internal/daemon/controller/handlers/users/user_service.go @@ -129,7 +129,7 @@ func (s Service) ListUsers(ctx context.Context, req *pbs.ListUsersRequest) (*pbs outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(*authResults.UserData.User.Id) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetScopeId()])) } @@ -171,7 +171,7 @@ func (s Service) GetUser(ctx context.Context, req *pbs.GetUserRequest) (*pbs.Get } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -209,7 +209,7 @@ func (s Service) CreateUser(ctx context.Context, req *pbs.CreateUserRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -246,7 +246,7 @@ func (s Service) UpdateUser(ctx context.Context, req *pbs.UpdateUserRequest) (*p } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -300,7 +300,7 @@ func (s Service) AddUserAccounts(ctx context.Context, req *pbs.AddUserAccountsRe } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -338,7 +338,7 @@ func (s Service) SetUserAccounts(ctx context.Context, req *pbs.SetUserAccountsRe } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -376,7 +376,7 @@ func (s Service) RemoveUserAccounts(ctx context.Context, req *pbs.RemoveUserAcco } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/daemon/controller/handlers/workers/worker_service.go b/internal/daemon/controller/handlers/workers/worker_service.go index dd6d4f94c8..d1365f5a1a 100644 --- a/internal/daemon/controller/handlers/workers/worker_service.go +++ b/internal/daemon/controller/handlers/workers/worker_service.go @@ -153,7 +153,7 @@ func (s Service) ListWorkers(ctx context.Context, req *pbs.ListWorkersRequest) ( outputFields := authResults.FetchOutputFields(res, action.List).SelfOrDefaults(authResults.UserId) outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(scopeInfoMap[item.GetScopeId()])) } @@ -195,7 +195,7 @@ func (s Service) GetWorker(ctx context.Context, req *pbs.GetWorkerRequest) (*pbs } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -280,7 +280,7 @@ func (s Service) createCommon(ctx context.Context, in *pb.Worker, act action.Typ } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -334,7 +334,7 @@ func (s Service) UpdateWorker(ctx context.Context, req *pbs.UpdateWorkerRequest) } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -371,7 +371,7 @@ func (s Service) AddWorkerTags(ctx context.Context, req *pbs.AddWorkerTagsReques return nil, errors.New(ctx, errors.Internal, op, "no request context found") } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -407,7 +407,7 @@ func (s Service) SetWorkerTags(ctx context.Context, req *pbs.SetWorkerTagsReques return nil, errors.New(ctx, errors.Internal, op, "no request context found") } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -443,7 +443,7 @@ func (s Service) RemoveWorkerTags(ctx context.Context, req *pbs.RemoveWorkerTags return nil, errors.New(ctx, errors.Internal, op, "no request context found") } outputOpts := make([]handlers.Option, 0, 3) - outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + outputOpts = append(outputOpts, handlers.WithOutputFields(outputFields)) if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } diff --git a/internal/perms/acl.go b/internal/perms/acl.go index 37316ab4db..5e42ad6acf 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -21,7 +21,7 @@ type ACL struct { type ACLResults struct { AuthenticationFinished bool Authorized bool - OutputFields OutputFieldsMap + OutputFields *OutputFields // This is included but unexported for testing/debugging scopeMap map[string][]Grant @@ -96,7 +96,7 @@ func (a ACL) Allowed(r Resource, aType action.Type, userId string, opt ...Option // Continue with the next grant, unless we have output fields // specified in which case we continue to be able to apply the // output fields depending on ID and type. - if len(grant.OutputFields) > 0 { + if _, hasSetFields := grant.OutputFields.Fields(); hasSetFields { outputFieldsOnly = true } else { continue @@ -219,7 +219,9 @@ func (a ACL) Allowed(r Resource, aType action.Type, userId string, opt ...Option if !outputFieldsOnly { results.Authorized = true } - if results.OutputFields = results.OutputFields.AddFields(grant.OutputFields.Fields()); results.OutputFields.HasAll() && results.Authorized { + fields, _ := grant.OutputFields.Fields() + results.OutputFields = results.OutputFields.AddFields(fields) + if results.OutputFields.Has("*") && results.Authorized { return } } diff --git a/internal/perms/acl_test.go b/internal/perms/acl_test.go index 4a486e8b90..da7e3f7fc4 100644 --- a/internal/perms/acl_test.go +++ b/internal/perms/acl_test.go @@ -369,7 +369,8 @@ func Test_ACLAllowed(t *testing.T) { } result := acl.Allowed(test.resource, aa.action, userId) assert.True(t, result.Authorized == aa.authorized, "action: %s, acl authorized: %t, test action authorized: %t", aa.action, result.Authorized, aa.authorized) - assert.ElementsMatch(t, result.OutputFields.Fields(), aa.outputFields) + fields, _ := result.OutputFields.Fields() + assert.ElementsMatch(t, fields, aa.outputFields) } }) } diff --git a/internal/perms/grants.go b/internal/perms/grants.go index 59e41a53c9..779c20ec49 100644 --- a/internal/perms/grants.go +++ b/internal/perms/grants.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" "strings" + "unicode" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/errors" @@ -46,7 +47,7 @@ type Grant struct { actions map[action.Type]bool // The set of output fields granted - OutputFields OutputFieldsMap + OutputFields *OutputFields // This is used as a temporary staging area before validating permissions to // allow the same validation code across grant string formats @@ -104,11 +105,12 @@ func (g Grant) clone() *Grant { ret.actions[action] = true } } - if g.OutputFields != nil { - ret.OutputFields = make(OutputFieldsMap, len(g.OutputFields)) - for k, v := range g.OutputFields { - ret.OutputFields[k] = v + if outFields, hasSetFields := g.OutputFields.Fields(); hasSetFields { + fieldsToAdd := make([]string, 0, len(outFields)) + for _, v := range outFields { + fieldsToAdd = append(fieldsToAdd, v) } + ret.OutputFields = ret.OutputFields.AddFields(fieldsToAdd) } return ret } @@ -134,8 +136,8 @@ func (g Grant) CanonicalString() string { builder = append(builder, fmt.Sprintf("actions=%s", strings.Join(actions, ","))) } - if len(g.OutputFields) > 0 { - builder = append(builder, fmt.Sprintf("output_fields=%s", strings.Join(g.OutputFields.Fields(), ","))) + if outFields, hasSetFields := g.OutputFields.Fields(); hasSetFields { + builder = append(builder, fmt.Sprintf("output_fields=%s", strings.Join(outFields, ","))) } return strings.Join(builder, ";") @@ -159,8 +161,8 @@ func (g Grant) MarshalJSON() ([]byte, error) { sort.Strings(actions) res["actions"] = actions } - if len(g.OutputFields) > 0 { - res["output_fields"] = g.OutputFields.Fields() + if outFields, hasSetFields := g.OutputFields.Fields(); hasSetFields { + res["output_fields"] = outFields } b, err := json.Marshal(res) if err != nil { @@ -222,17 +224,22 @@ func (g *Grant) unmarshalJSON(data []byte) error { } // We do the make here because we detect later if the field was set but // no values given - g.OutputFields = make(OutputFieldsMap, len(interfaceOutputFields)) - if len(interfaceOutputFields) > 0 { + switch len(interfaceOutputFields) { + case 0: + // JSON was set but no fields defined, add an empty array + g.OutputFields = g.OutputFields.AddFields([]string{}) + default: + fields := make([]string, 0, len(interfaceOutputFields)) for _, v := range interfaceOutputFields { field, ok := v.(string) switch { case !ok: return errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("unable to interpret %v in output_fields array as string", v)) default: - g.OutputFields[field] = true + fields = append(fields, field) } } + g.OutputFields = g.OutputFields.AddFields(fields) } } return nil @@ -250,7 +257,7 @@ func (g *Grant) unmarshalText(grantString string) error { return errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("segment %q not formatted correctly, wrong number of equal signs", segment)) case len(kv[0]) == 0: return errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("segment %q not formatted correctly, missing key", segment)) - case len(kv[1]) == 0: + case len(kv[1]) == 0 && kv[0] != "output_fields": return errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("segment %q not formatted correctly, missing value", segment)) } @@ -278,7 +285,12 @@ func (g *Grant) unmarshalText(grantString string) error { } case "output_fields": - g.OutputFields = g.OutputFields.AddFields(strings.Split(kv[1], ",")) + switch len(kv[1]) { + case 0: + g.OutputFields = g.OutputFields.AddFields([]string{}) + default: + g.OutputFields = g.OutputFields.AddFields(strings.Split(kv[1], ",")) + } } } @@ -300,9 +312,10 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { if scopeId == "" { return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "missing scope id") } + grantString = strings.ToValidUTF8(grantString, string(unicode.ReplacementChar)) grant := Grant{ - scope: Scope{Id: scopeId}, + scope: Scope{Id: strings.ToValidUTF8(scopeId, string(unicode.ReplacementChar))}, } switch { case scopeId == scope.Global.String(): @@ -337,11 +350,11 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { switch id { case "user.id", ".User.Id": if opts.withUserId != "" { - grant.id = opts.withUserId + grant.id = strings.ToValidUTF8(opts.withUserId, string(unicode.ReplacementChar)) } case "account.id", ".Account.Id": if opts.withAccountId != "" { - grant.id = opts.withAccountId + grant.id = strings.ToValidUTF8(opts.withAccountId, string(unicode.ReplacementChar)) } default: return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("unknown template %q in grant %q value", grant.id, "id")) @@ -376,7 +389,7 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { // get to this point because original parsing should error) return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains no id or type") case resource.All: - // "type=*;actions=..." is not supported -- we reqiure you to + // "type=*;actions=..." is not supported -- we require you to // explicitly set a pin or set the ID to * return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains wildcard type with no id value") default: @@ -387,7 +400,7 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { switch len(grant.actions) { case 0: // It's okay to have no actions if only output fields are being defined - if len(grant.OutputFields) == 0 { + if _, hasSetFields := grant.OutputFields.Fields(); !hasSetFields { return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains no actions or output fields") } case 1: @@ -404,10 +417,6 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { } } } - // Set but empty output fields... - if grant.OutputFields != nil && len(grant.OutputFields) == 0 { - return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string has output_fields set but empty") - } // This might be zero if output fields is populated if len(grant.actions) > 0 { // Create a dummy resource and pass it through Allowed and ensure that @@ -457,7 +466,7 @@ func (g *Grant) parseAndValidateActions() error { g.actionsBeingParsed = nil // If there are no actions it's fine if the grant is just used to // specify output fields - if len(g.OutputFields) > 0 { + if _, hasSetFields := g.OutputFields.Fields(); hasSetFields { return nil } return errors.NewDeprecated(errors.InvalidParameter, op, "missing actions") diff --git a/internal/perms/grants_test.go b/internal/perms/grants_test.go index df1a4671bc..5c325770e1 100644 --- a/internal/perms/grants_test.go +++ b/internal/perms/grants_test.go @@ -37,13 +37,17 @@ func Test_ActionParsingValidation(t *testing.T) { { name: "empty action with output fields", input: Grant{ - OutputFields: OutputFieldsMap{ - "id": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "id": true, + }, }, }, result: Grant{ - OutputFields: OutputFieldsMap{ - "id": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "id": true, + }, }, }, }, @@ -159,10 +163,12 @@ func Test_MarshalingAndCloning(t *testing.T) { Type: scope.Project, }, typ: resource.Group, - OutputFields: OutputFieldsMap{ - "name": true, - "version": true, - "id": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "name": true, + "version": true, + "id": true, + }, }, }, jsonOutput: `{"id":"baz","output_fields":["id","name","version"],"type":"group"}`, @@ -181,10 +187,12 @@ func Test_MarshalingAndCloning(t *testing.T) { action.Read: true, }, actionsBeingParsed: []string{"create", "read"}, - OutputFields: OutputFieldsMap{ - "name": true, - "version": true, - "id": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "name": true, + "version": true, + "id": true, + }, }, }, jsonOutput: `{"actions":["create","read"],"id":"baz","output_fields":["id","name","version"],"type":"group"}`, @@ -268,10 +276,12 @@ func Test_Unmarshaling(t *testing.T) { { name: "good output fields", expected: Grant{ - OutputFields: OutputFieldsMap{ - "name": true, - "version": true, - "id": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "name": true, + "version": true, + "id": true, + }, }, }, jsonInput: `{"output_fields":["id","name","version"]}`, @@ -424,12 +434,40 @@ func Test_Parse(t *testing.T) { { name: "empty output fields", input: "id=*;type=*;actions=read,list;output_fields=", - err: `perms.Parse: unable to parse grant string: perms.(Grant).unmarshalText: segment "output_fields=" not formatted correctly, missing value: parameter violation: error #100`, + expected: Grant{ + scope: Scope{ + Id: "o_scope", + Type: scope.Org, + }, + id: "*", + typ: resource.All, + actions: map[action.Type]bool{ + action.Read: true, + action.List: true, + }, + OutputFields: &OutputFields{ + fields: make(map[string]bool), + }, + }, }, { name: "empty output fields json", input: `{"id": "*", "type": "*", "actions": ["read", "list"], "output_fields": []}`, - err: "perms.Parse: parsed grant string has output_fields set but empty: parameter violation: error #100", + expected: Grant{ + scope: Scope{ + Id: "o_scope", + Type: scope.Org, + }, + id: "*", + typ: resource.All, + actions: map[action.Type]bool{ + action.Read: true, + action.List: true, + }, + OutputFields: &OutputFields{ + fields: make(map[string]bool), + }, + }, }, { name: "wildcard id and type and actions with list", @@ -489,10 +527,12 @@ func Test_Parse(t *testing.T) { actions: map[action.Type]bool{ action.Read: true, }, - OutputFields: OutputFieldsMap{ - "version": true, - "id": true, - "name": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "version": true, + "id": true, + "name": true, + }, }, }, }, @@ -506,10 +546,12 @@ func Test_Parse(t *testing.T) { }, id: "foobar", typ: resource.Unknown, - OutputFields: OutputFieldsMap{ - "version": true, - "id": true, - "name": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "version": true, + "id": true, + "name": true, + }, }, }, }, @@ -555,10 +597,12 @@ func Test_Parse(t *testing.T) { actions: map[action.Type]bool{ action.Read: true, }, - OutputFields: OutputFieldsMap{ - "version": true, - "id": true, - "name": true, + OutputFields: &OutputFields{ + fields: map[string]bool{ + "version": true, + "id": true, + "name": true, + }, }, }, }, @@ -754,3 +798,38 @@ func TestHasActionOrSubaction(t *testing.T) { }) } } + +func FuzzParse(f *testing.F) { + f.Add("type=host-catalog;actions=create") + f.Add("type=*;actions=*") + f.Add("id=*;type=*;actions=*") + f.Add("id=*;type=*;actions=read,list") + f.Add("id=foobar;actions=read;output_fields=version,id,name") + f.Add("id={{account.id}};actions=update,read") + f.Add(`{id:"foobar","type":"host-catalog","actions":["create"]}`) + + f.Fuzz(func(t *testing.T, grant string) { + g, err := Parse("global", grant, WithSkipFinalValidation(true)) + if err != nil { + return + } + g2, err := Parse("global", g.CanonicalString(), WithSkipFinalValidation(true)) + if err != nil { + t.Fatal("Failed to parse canonical string:", err) + } + if g.CanonicalString() != g2.CanonicalString() { + t.Errorf("grant roundtrip failed, input %q, output %q", g.CanonicalString(), g2.CanonicalString()) + } + jsonBytes, err := g.MarshalJSON() + if err != nil { + t.Error("Failed to marshal JSON:", err) + } + g3, err := Parse("global", string(jsonBytes), WithSkipFinalValidation(true)) + if err != nil { + t.Fatal("Failed to parse json string:", err) + } + if g.CanonicalString() != g3.CanonicalString() { + t.Errorf("grant JSON roundtrip failed, input %q, output %q", g.CanonicalString(), g3.CanonicalString()) + } + }) +} diff --git a/internal/perms/output_fields.go b/internal/perms/output_fields.go index ed9ca270a6..e3502360b2 100644 --- a/internal/perms/output_fields.go +++ b/internal/perms/output_fields.go @@ -6,68 +6,107 @@ import ( "github.com/hashicorp/boundary/globals" ) -// OutputFieldsMap is used to store information about allowed output fields in +// OutputFields is used to store information about allowed output fields in // grants -type OutputFieldsMap map[string]bool +type OutputFields struct { + fields map[string]bool +} -// AddFields adds the given fields and returns the map. -func (o OutputFieldsMap) AddFields(input []string) (ret OutputFieldsMap) { +// AddFields adds the given fields and returns the interface. It is safe to call +// this on a nil object, which will create a new object and add the fields to +// it; if relying on this make sure to assign to the output, e.g.: +// +// outFields = outFields.AddFields([]string{"foo", "bar"}) +// +// Notes: +// +// - Adding non-nil but empty input will be construed as "no fields" +// +// - Fields compose, they do not overwrite; if you want to start over, create a +// new OutputFields struct +func (o *OutputFields) AddFields(input []string) *OutputFields { + ret := o + if ret == nil { + ret = new(OutputFields) + } switch { + case input == nil: + // Do nothing + case len(input) == 0: - if o == nil { - return o + // Ensure we set to non-nil if it isn't already to capture that input is + // not empty but fields are not being added + if ret.fields == nil { + ret.fields = make(map[string]bool) } - return o - case o == nil: - ret = make(OutputFieldsMap, len(input)) - case len(o) == 1 && o["*"]: - return o + + case len(ret.fields) == 1 && ret.fields["*"]: + // Again do nothing, there's nothing to add + default: - ret = o - } - for _, k := range input { - if k == "*" { - ret = OutputFieldsMap{k: true} - return + // Ensure the map is valid + if ret.fields == nil { + ret.fields = make(map[string]bool, len(input)) + } + + // Go through and add fields + for _, k := range input { + if k == "*" { + ret.fields = map[string]bool{k: true} + return ret + } + ret.fields[k] = true } - ret[k] = true } - return -} -func (o OutputFieldsMap) HasAll() bool { - return o["*"] + return ret } -// Fields returns an alphabetical string slice of the fields in the map -func (o OutputFieldsMap) Fields() (ret []string) { - if o == nil { - return nil +// Fields returns an alphabetical string slice of the fields in the map. The +// return value will be nil with hasSetFields false if fields are unset (e.g. +// we'd use the defaults in SelfOrDefaults), and non-nil (but empty if no fields +// are allowed) with hasSetFields true if fields have been configured. It is +// safe to call this on a nil object; it will return a nil slice and false for +// hasSetFields. +func (o *OutputFields) Fields() (fields []string, hasSetFields bool) { + if o == nil || o.fields == nil { + return nil, false } - if len(o) == 0 { - return []string{} + if len(o.fields) == 0 { + return []string{}, true } - ret = make([]string, 0, len(o)) - for f := range o { + ret := make([]string, 0, len(o.fields)) + for f := range o.fields { ret = append(ret, f) } sort.Strings(ret) - return + return ret, true } -// SelfOrDefaults returns either the fields map itself or the defaults for the -// given user -func (o OutputFieldsMap) SelfOrDefaults(userId string) OutputFieldsMap { +// SelfOrDefaults returns either the output fields itself or the defaults for +// the given user. It is safe to call this on a nil object (it will always +// return defaults for the given user ID); if relying on this make sure to +// assign to the output, e.g.: +// +// outFields = outFields.SelfOrDefaults("foo") +func (o *OutputFields) SelfOrDefaults(userId string) *OutputFields { + ret := o + if ret == nil { + ret = new(OutputFields) + } switch { - case o != nil: + case ret.fields != nil: // We have values set (which may be empty) so use those - return o + return ret + case userId == "": // This shouldn't happen, and if it does, don't allow anything to be - // output - return OutputFieldsMap{} + // output -- keep map empty and set to not-default so we use the empty + // map + ret.fields = make(map[string]bool) + case userId == globals.AnonymousUserId: - return OutputFieldsMap{ + ret.fields = map[string]bool{ globals.IdField: true, globals.ScopeField: true, globals.ScopeIdField: true, @@ -81,19 +120,23 @@ func (o OutputFieldsMap) SelfOrDefaults(userId string) OutputFieldsMap { globals.AuthorizedActionsField: true, globals.AuthorizedCollectionActionsField: true, } + default: - return OutputFieldsMap{ + // Default behavior is to allow all fields + ret.fields = map[string]bool{ "*": true, } } + + return ret } -// Has returns true if the value exists; that is, it is directly in the map, or -// the map contains * -func (o OutputFieldsMap) Has(in string) bool { - // Handle nil or empty case - if len(o) == 0 { +// Has returns true if the field should be allowed; that is, it is explicitly +// allowed, or the fields contains *. It is safe to call this on a nil object +// (it will always return false). +func (o *OutputFields) Has(in string) bool { + if o == nil || o.fields == nil { return false } - return o.HasAll() || o[in] + return o.fields["*"] || o.fields[in] } diff --git a/internal/perms/output_fields_test.go b/internal/perms/output_fields_test.go index 0c2c6a771e..c164edf9a0 100644 --- a/internal/perms/output_fields_test.go +++ b/internal/perms/output_fields_test.go @@ -16,49 +16,106 @@ func Test_OutputFields(t *testing.T) { type input struct { name string fields []string - startMap OutputFieldsMap - resMap OutputFieldsMap + startMap *OutputFields + resMap *OutputFields resStar bool } tests := []input{ { - name: "nil map, add nil", + name: "nil map, add nil", + resMap: &OutputFields{}, + }, + { + name: "nil map, add empty fields", + fields: []string{}, + resMap: &OutputFields{ + fields: map[string]bool{}, + }, }, { name: "nil map, add fields", fields: []string{"id", "version"}, - resMap: OutputFieldsMap{"id": true, "version": true}, + resMap: &OutputFields{ + fields: map[string]bool{ + "id": true, + "version": true, + }, + }, }, { - name: "existing map, add nil", - startMap: OutputFieldsMap{"id": true, "version": true}, - resMap: OutputFieldsMap{"id": true, "version": true}, + name: "existing map, add nil", + startMap: &OutputFields{ + fields: map[string]bool{ + "id": true, + "version": true, + }, + }, + resMap: &OutputFields{ + fields: map[string]bool{ + "id": true, + "version": true, + }, + }, }, { - name: "existing with star, add nil", - startMap: OutputFieldsMap{"*": true}, - resMap: OutputFieldsMap{"*": true}, - resStar: true, + name: "existing with star, add nil", + startMap: &OutputFields{ + fields: map[string]bool{ + "*": true, + }, + }, + resMap: &OutputFields{ + fields: map[string]bool{ + "*": true, + }, + }, + resStar: true, }, { - name: "existing with star, add new", - fields: []string{"id", "version"}, - startMap: OutputFieldsMap{"*": true}, - resMap: OutputFieldsMap{"*": true}, - resStar: true, + name: "existing with star, add new", + fields: []string{"id", "version"}, + startMap: &OutputFields{ + fields: map[string]bool{ + "*": true, + }, + }, + resMap: &OutputFields{ + fields: map[string]bool{ + "*": true, + }, + }, + resStar: true, }, { - name: "existing without star, add new", - fields: []string{"id", "version"}, - startMap: OutputFieldsMap{"name": true}, - resMap: OutputFieldsMap{"id": true, "version": true, "name": true}, + name: "existing without star, add new", + fields: []string{"id", "version"}, + startMap: &OutputFields{ + fields: map[string]bool{ + "name": true, + }, + }, + resMap: &OutputFields{ + fields: map[string]bool{ + "id": true, + "version": true, + "name": true, + }, + }, }, { - name: "existing without star, add star", - fields: []string{"id", "*"}, - startMap: OutputFieldsMap{"name": true}, - resMap: OutputFieldsMap{"*": true}, - resStar: true, + name: "existing without star, add star", + fields: []string{"id", "*"}, + startMap: &OutputFields{ + fields: map[string]bool{ + "name": true, + }, + }, + resMap: &OutputFields{ + fields: map[string]bool{ + "*": true, + }, + }, + resStar: true, }, } @@ -66,7 +123,7 @@ func Test_OutputFields(t *testing.T) { t.Run(test.name, func(t *testing.T) { assert := assert.New(t) out := test.startMap.AddFields(test.fields) - assert.True(out.HasAll() == test.resStar) + assert.True(out.Has("*") == test.resStar) assert.Equal(test.resMap, out) }) } @@ -222,7 +279,8 @@ func Test_ACLOutputFields(t *testing.T) { } acl := NewACL(grants...) results := acl.Allowed(test.resource, test.action, globals.AnonymousUserId, WithSkipAnonymousUserRestrictions(true)) - assert.ElementsMatch(t, results.OutputFields.Fields(), test.fields) + fields, _ := results.OutputFields.Fields() + assert.ElementsMatch(t, fields, test.fields) assert.True(t, test.authorized == results.Authorized) }) } @@ -233,42 +291,58 @@ func Test_ACLSelfOrDefault(t *testing.T) { type input struct { name string - input OutputFieldsMap - output OutputFieldsMap + input *OutputFields + output *OutputFields userId string } tests := []input{ { - name: "nil, no user ID", - output: OutputFieldsMap{}, + name: "nil, no user ID, set but empty", + output: &OutputFields{ + fields: map[string]bool{}, + }, }, { - name: "nil, non anon id", - output: OutputFieldsMap{"*": true}, + name: "nil, non anon id", + output: &OutputFields{ + fields: map[string]bool{ + "*": true, + }, + }, userId: "u_abc123", }, { name: "nil, anon id", - output: OutputFieldsMap{ - globals.IdField: true, - globals.ScopeField: true, - globals.ScopeIdField: true, - globals.PluginField: true, - globals.PluginIdField: true, - globals.NameField: true, - globals.DescriptionField: true, - globals.TypeField: true, - globals.IsPrimaryField: true, - globals.PrimaryAuthMethodIdField: true, - globals.AuthorizedActionsField: true, - globals.AuthorizedCollectionActionsField: true, + output: &OutputFields{ + fields: map[string]bool{ + globals.IdField: true, + globals.ScopeField: true, + globals.ScopeIdField: true, + globals.PluginField: true, + globals.PluginIdField: true, + globals.NameField: true, + globals.DescriptionField: true, + globals.TypeField: true, + globals.IsPrimaryField: true, + globals.PrimaryAuthMethodIdField: true, + globals.AuthorizedActionsField: true, + globals.AuthorizedCollectionActionsField: true, + }, }, userId: globals.AnonymousUserId, }, { - name: "not nil", - input: OutputFieldsMap{"foo": true}, - output: OutputFieldsMap{"foo": true}, + name: "not nil", + input: &OutputFields{ + fields: map[string]bool{ + "foo": true, + }, + }, + output: &OutputFields{ + fields: map[string]bool{ + "foo": true, + }, + }, }, } for _, test := range tests { diff --git a/internal/perms/testdata/fuzz/FuzzParse/1686a11fab3efeb7d05bf50eb5c61865801c150feb07639363c7123ce92d5d83 b/internal/perms/testdata/fuzz/FuzzParse/1686a11fab3efeb7d05bf50eb5c61865801c150feb07639363c7123ce92d5d83 new file mode 100644 index 0000000000..c61747d9a5 --- /dev/null +++ b/internal/perms/testdata/fuzz/FuzzParse/1686a11fab3efeb7d05bf50eb5c61865801c150feb07639363c7123ce92d5d83 @@ -0,0 +1,2 @@ +go test fuzz v1 +string("output_fields=,") diff --git a/internal/perms/testdata/fuzz/FuzzParse/3c6b045265fd967a6add10031a98efb46f623f95a9b8d3038982bd4cd36298b1 b/internal/perms/testdata/fuzz/FuzzParse/3c6b045265fd967a6add10031a98efb46f623f95a9b8d3038982bd4cd36298b1 new file mode 100644 index 0000000000..638b2482ba --- /dev/null +++ b/internal/perms/testdata/fuzz/FuzzParse/3c6b045265fd967a6add10031a98efb46f623f95a9b8d3038982bd4cd36298b1 @@ -0,0 +1,2 @@ +go test fuzz v1 +string("output_fields=\xf1") diff --git a/internal/requests/requests.go b/internal/requests/requests.go index 2c50a3aeaf..b49cc5676f 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -33,7 +33,7 @@ type RequestContext struct { // OutputFields is the set of fields authorized for output for the // authorized action, if not the default - OutputFields perms.OutputFieldsMap + OutputFields *perms.OutputFields } // NewRequestContext returns a derived context with a new RequestContext value @@ -64,7 +64,7 @@ func RequestContextFromCtx(ctx context.Context) (*RequestContext, bool) { // OutputFields returns output fields from the given context and calls // SelfOrDefaults on it. If the context does not contain a RequestContext, // this will return nil, false. -func OutputFields(ctx context.Context) (perms.OutputFieldsMap, bool) { +func OutputFields(ctx context.Context) (*perms.OutputFields, bool) { reqCtx, ok := RequestContextFromCtx(ctx) if !ok { return nil, false