From 92809b733a9ef238f5921d297bf55d3e71ee26da Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 13 Sep 2021 13:56:44 -0400 Subject: [PATCH] Fix unknown permission issue with credential-store (#1524) This also affected credential libraries and managed groups. It seems that any end-to-end tests of these grants currently had them included in wildcards instead of individually, but these types were never added to the valid list for resources. Additionally, credential store wasn't added as a valid top level type for pinning purposes. --- CHANGELOG.md | 10 ++++- internal/perms/acl.go | 1 + internal/perms/grants.go | 25 ++++--------- internal/perms/grants_test.go | 37 ++++--------------- .../handlers/scopes/scope_service.go | 10 +++-- .../handlers/scopes/scope_service_test.go | 6 +++ internal/types/resource/resource.go | 5 +++ 7 files changed, 42 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20102db7e1..cd2d9a48a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,14 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. ## Next -## 0.6.0 (2021/09/03) +### Bug Fixes +* grants: Fix issue where `credential-store`, `credential-library`, and + `managed-group` would not be accepted as specific `type` values in grant + strings. Also, fix authorized actions not showing `credential-store` values in + project scope output. ([PR](https://github.com/hashicorp/boundary/pull/1524)) + +## 0.6.0 (2021/09/03) ### New and Improved @@ -25,7 +31,7 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. ([PR](https://github.com/hashicorp/boundary/pull/1474)) * targets: Fix panic when using `boundary targets authorize-session` ([issue](https://github.com/hashicorp/boundary/issues/1488), - [PR](https://github.com/hashicorp/boundary/pull/1496)). + [PR](https://github.com/hashicorp/boundary/pull/1496)) ## 0.5.1 (2021/08/16) diff --git a/internal/perms/acl.go b/internal/perms/acl.go index 2b92f141c1..c4e813c1d6 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -180,6 +180,7 @@ func topLevelType(typ resource.Type) bool { switch typ { case resource.AuthMethod, resource.AuthToken, + resource.CredentialStore, resource.Group, resource.HostCatalog, resource.Role, diff --git a/internal/perms/grants.go b/internal/perms/grants.go index bc57757aca..75ea1f3ef9 100644 --- a/internal/perms/grants.go +++ b/internal/perms/grants.go @@ -422,26 +422,17 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { return grant, nil } +// validateType ensures that we are not allowing access to disallowed resource +// types. It does not explicitly check the resource string itself; that's the +// job of the parsing functions to look up the string from the Map and ensure +// it's not unknown. func (g Grant) validateType() error { const op = "perms.(Grant).validateType" switch g.typ { - case resource.Unknown, - resource.All, - resource.Scope, - resource.User, - resource.Group, - resource.Role, - resource.AuthMethod, - resource.Account, - resource.AuthToken, - resource.HostCatalog, - resource.HostSet, - resource.Host, - resource.Target, - resource.Session: - return nil - } - return errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("unknown type specifier %q", g.typ)) + case resource.Controller, resource.Worker: + return errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("unknown type specifier %q", g.typ)) + } + return nil } func (g *Grant) parseAndValidateActions() error { diff --git a/internal/perms/grants_test.go b/internal/perms/grants_test.go index 25d6ba3d82..19f8072804 100644 --- a/internal/perms/grants_test.go +++ b/internal/perms/grants_test.go @@ -107,35 +107,14 @@ func Test_ActionParsingValidation(t *testing.T) { func Test_ValidateType(t *testing.T) { t.Parallel() - - type input struct { - name string - input Grant - errResult string - } - - tests := []input{ - { - name: "no specifier", - }, - { - name: "valid specifier", - input: Grant{ - typ: resource.HostCatalog, - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - err := test.input.validateType() - if test.errResult == "" { - require.NoError(t, err) - } else { - require.Error(t, err) - assert.Equal(t, test.errResult, err.Error()) - } - }) + var g Grant + for i := resource.Unknown; i <= resource.CredentialLibrary; i++ { + g.typ = i + if i == resource.Controller || i == resource.Worker { + assert.Error(t, g.validateType()) + } else { + assert.NoError(t, g.validateType()) + } } } diff --git a/internal/servers/controller/handlers/scopes/scope_service.go b/internal/servers/controller/handlers/scopes/scope_service.go index 82fb184ed7..07fd19ac71 100644 --- a/internal/servers/controller/handlers/scopes/scope_service.go +++ b/internal/servers/controller/handlers/scopes/scope_service.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/boundary/internal/servers/controller/handlers" "github.com/hashicorp/boundary/internal/servers/controller/handlers/authmethods" "github.com/hashicorp/boundary/internal/servers/controller/handlers/authtokens" + "github.com/hashicorp/boundary/internal/servers/controller/handlers/credentialstores" "github.com/hashicorp/boundary/internal/servers/controller/handlers/groups" "github.com/hashicorp/boundary/internal/servers/controller/handlers/host_catalogs" "github.com/hashicorp/boundary/internal/servers/controller/handlers/roles" @@ -76,10 +77,11 @@ var ( }, scope.Project.String(): { - resource.Group: groups.CollectionActions, - resource.HostCatalog: host_catalogs.CollectionActions, - resource.Role: roles.CollectionActions, - resource.Target: targets.CollectionActions, + resource.CredentialStore: credentialstores.CollectionActions, + resource.Group: groups.CollectionActions, + resource.HostCatalog: host_catalogs.CollectionActions, + resource.Role: roles.CollectionActions, + resource.Target: targets.CollectionActions, }, } ) diff --git a/internal/servers/controller/handlers/scopes/scope_service_test.go b/internal/servers/controller/handlers/scopes/scope_service_test.go index 94eac8ccd9..3079633340 100644 --- a/internal/servers/controller/handlers/scopes/scope_service_test.go +++ b/internal/servers/controller/handlers/scopes/scope_service_test.go @@ -146,6 +146,12 @@ var orgAuthorizedCollectionActions = map[string]*structpb.ListValue{ } var projectAuthorizedCollectionActions = map[string]*structpb.ListValue{ + "credential-stores": { + Values: []*structpb.Value{ + structpb.NewStringValue("create"), + structpb.NewStringValue("list"), + }, + }, "groups": { Values: []*structpb.Value{ structpb.NewStringValue("create"), diff --git a/internal/types/resource/resource.go b/internal/types/resource/resource.go index a801cc404d..46c67ef862 100644 --- a/internal/types/resource/resource.go +++ b/internal/types/resource/resource.go @@ -25,6 +25,11 @@ const ( ManagedGroup CredentialStore CredentialLibrary + // NOTE: When adding a new type, be sure to update: + // + // * The Grant.validateType function and test + // * The perms.topLevelTypes function + // * The scopes service collection actions for appropriate scopes ) func (r Type) MarshalJSON() ([]byte, error) {