Improve grant validation (#3081)

This adds several relationships into the resource package that are then
made use of to improve the grant validation that runs during Parse. By
understanding mappings between ID and resource type the validation
checks can far better align with actual allowed grant formats so we can
find several common misunderstandings before they simply result in an
authorization failure.

In the future some of these functions could be used in the logic in the
actual ACL checks but it's not currently necessary and I don't want to
lump that change in with this.

Note that the tests have not meaningfully changed, but they _have_ had
to be updated because the stricter validation is actually catching
issues.
pull/3094/head
Jeff Mitchell 3 years ago committed by GitHub
parent ef37369c2b
commit fc664eb35f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -3,7 +3,11 @@
package globals
import "strings"
import (
"strings"
"github.com/hashicorp/boundary/internal/types/resource"
)
// This set of consts is intended to be a place to collect commonly-used
// prefixes. This can avoid some cross-package dependency issues. Unlike the
@ -109,33 +113,48 @@ const (
WorkerPrefix = "w"
)
var topLevelPrefixes = map[string]bool{
AuthTokenPrefix: true,
PasswordAuthMethodPrefix: true,
OidcAuthMethodPrefix: true,
ProjectPrefix: true,
OrgPrefix: true,
UserPrefix: true,
GroupPrefix: true,
RolePrefix: true,
StaticCredentialStorePrefix: true,
StaticCredentialStorePreviousPrefix: true,
VaultCredentialStorePrefix: true,
StaticHostCatalogPrefix: true,
PluginHostCatalogPrefix: true,
PluginHostCatalogPreviousPrefix: true,
SessionPrefix: true,
TcpTargetPrefix: true,
SshTargetPrefix: true,
WorkerPrefix: true,
var prefixToResourceType = map[string]resource.Type{
AuthTokenPrefix: resource.AuthToken,
PasswordAuthMethodPrefix: resource.AuthMethod,
PasswordAccountPrefix: resource.Account,
PasswordAccountPreviousPrefix: resource.Account,
OidcAuthMethodPrefix: resource.AuthMethod,
OidcAccountPrefix: resource.Account,
OidcManagedGroupPrefix: resource.ManagedGroup,
GlobalPrefix: resource.Scope,
ProjectPrefix: resource.Scope,
OrgPrefix: resource.Scope,
UserPrefix: resource.User,
GroupPrefix: resource.Group,
RolePrefix: resource.Role,
StaticCredentialStorePrefix: resource.CredentialStore,
StaticCredentialStorePreviousPrefix: resource.CredentialStore,
VaultCredentialStorePrefix: resource.CredentialStore,
VaultCredentialLibraryPrefix: resource.CredentialLibrary,
VaultSshCertificateCredentialLibraryPrefix: resource.CredentialLibrary,
UsernamePasswordCredentialPrefix: resource.Credential,
UsernamePasswordCredentialPreviousPrefix: resource.Credential,
SshPrivateKeyCredentialPrefix: resource.Credential,
JsonCredentialPrefix: resource.Credential,
StaticHostCatalogPrefix: resource.HostCatalog,
StaticHostSetPrefix: resource.HostSet,
StaticHostPrefix: resource.Host,
PluginHostCatalogPrefix: resource.HostCatalog,
PluginHostCatalogPreviousPrefix: resource.HostCatalog,
PluginHostSetPrefix: resource.HostSet,
PluginHostSetPreviousPrefix: resource.HostSet,
PluginHostPrefix: resource.Host,
PluginHostPreviousPrefix: resource.Host,
SessionPrefix: resource.Session,
TcpTargetPrefix: resource.Target,
SshTargetPrefix: resource.Target,
WorkerPrefix: resource.Worker,
}
// IsTopLevelResourcePrefix takes in a resource ID (or a prefix) and indicates
// via the result whether the ID/prefix corresponds to a top-level resource;
// that is, one (like auth method) that does not have a parent (like account or
// managed group)
func IsTopLevelResourcePrefix(in string) bool {
// ResourceTypeFromPrefix takes in a resource ID (or a prefix) and returns the
// corresponding resource typ
func ResourceTypeFromPrefix(in string) resource.Type {
// If full ID, trim to just prefix
in, _, _ = strings.Cut(in, "_")
return topLevelPrefixes[in]
return prefixToResourceType[in]
}

@ -7,22 +7,22 @@ import (
"fmt"
"testing"
"github.com/hashicorp/boundary/internal/types/resource"
"github.com/stretchr/testify/assert"
)
func TestIsTopLevelResourcePrefix(t *testing.T) {
func TestResourceTypeFromPrefix(t *testing.T) {
// Test a random sampling
topLevel := []string{VaultCredentialStorePrefix, StaticCredentialStorePrefix, GroupPrefix, TcpTargetPrefix}
child := []string{VaultCredentialLibraryPrefix, OidcManagedGroupPrefix, StaticHostSetPrefix, JsonCredentialPrefix}
for _, prefix := range topLevel {
assert.True(t, IsTopLevelResourcePrefix(prefix))
assert.True(t, IsTopLevelResourcePrefix(fmt.Sprintf("%s_foobar", prefix)))
assert.False(t, IsTopLevelResourcePrefix(fmt.Sprintf("%sfoobar", prefix)))
vals := map[string]resource.Type{
VaultCredentialLibraryPrefix: resource.CredentialLibrary,
OidcManagedGroupPrefix: resource.ManagedGroup,
StaticHostSetPrefix: resource.HostSet,
JsonCredentialPrefix: resource.Credential,
}
for _, prefix := range child {
assert.False(t, IsTopLevelResourcePrefix(prefix))
assert.False(t, IsTopLevelResourcePrefix(fmt.Sprintf("%s_foobar", prefix)))
assert.False(t, IsTopLevelResourcePrefix(fmt.Sprintf("%sfoobar", prefix)))
for prefix, typ := range vals {
assert.Equal(t, typ, ResourceTypeFromPrefix(prefix))
assert.Equal(t, typ, ResourceTypeFromPrefix(fmt.Sprintf("%s_foobar", prefix)))
assert.Equal(t, resource.Unknown, ResourceTypeFromPrefix(fmt.Sprintf("%sfoobar", prefix)))
}
}

@ -50,8 +50,8 @@ func TestFetchActionSetForId(t *testing.T) {
orgRole := iam.TestRole(t, conn, org.GetPublicId())
iam.TestUserRole(t, conn, orgRole.PublicId, token.UserId)
iam.TestRoleGrant(t, conn, orgRole.PublicId, "id=foo;actions=read,update")
iam.TestRoleGrant(t, conn, orgRole.PublicId, "id=bar;actions=read,update,delete,authorize-session")
iam.TestRoleGrant(t, conn, orgRole.PublicId, "id=ttcp_foo;actions=read,update")
iam.TestRoleGrant(t, conn, orgRole.PublicId, "id=ttcp_bar;actions=read,update,delete,authorize-session")
iam.TestRoleGrant(t, conn, orgRole.PublicId, "id=*;type=role;actions=add-grants,remove-grants")
cases := []struct {
@ -71,7 +71,7 @@ func TestFetchActionSetForId(t *testing.T) {
},
{
name: "disjoint match",
id: "bar",
id: "ttcp_bar",
avail: action.ActionSet{action.Delete, action.AddGrants, action.Read, action.RemoveHostSets},
allowed: action.ActionSet{action.Delete, action.Read},
},
@ -107,7 +107,7 @@ func TestFetchActionSetForId(t *testing.T) {
typ = tt.typeOverride
}
res := auth.Verify(ctx, []auth.Option{
auth.WithId("foo"),
auth.WithId("ttcp_foo"),
auth.WithAction(action.Read),
auth.WithScopeId(org.PublicId),
auth.WithType(typ),

@ -1671,25 +1671,25 @@ func TestAddGrants(t *testing.T) {
},
{
name: "Add grant on role with grant",
existing: []string{"id=1;actions=read"},
existing: []string{"id=u_foo;actions=read"},
add: []string{"id=*;type=*;actions=delete"},
result: []string{"id=1;actions=read", "id=*;type=*;actions=delete"},
result: []string{"id=u_foo;actions=read", "id=*;type=*;actions=delete"},
},
{
name: "Add duplicate grant on role with grant",
existing: []string{"id=aA1;actions=read"},
existing: []string{"id=u_fooaA1;actions=read"},
add: []string{"id=*;type=*;actions=delete", "id=*;type=*;actions=delete"},
result: []string{"id=aA1;actions=read", "id=*;type=*;actions=delete"},
result: []string{"id=u_fooaA1;actions=read", "id=*;type=*;actions=delete"},
},
{
name: "Add grant matching existing grant",
existing: []string{"id=1;actions=read", "id=*;type=*;actions=delete"},
existing: []string{"id=u_foo;actions=read", "id=*;type=*;actions=delete"},
add: []string{"id=*;type=*;actions=delete"},
wantErr: true,
},
{
name: "Check deprecation",
existing: []string{"id=1;actions=read", "id=*;type=*;actions=delete"},
existing: []string{"id=u_foo;actions=read", "id=*;type=*;actions=delete"},
add: []string{"id=*;type=target;actions=add-host-sets"},
wantErr: true,
wantErrContains: "Use \\\"add-host-sources\\\" instead",
@ -1810,31 +1810,31 @@ func TestSetGrants(t *testing.T) {
},
{
name: "Set grant on role with grant",
existing: []string{"id=1;actions=read"},
existing: []string{"id=u_foo;actions=read"},
set: []string{"id=*;type=*;actions=delete"},
result: []string{"id=*;type=*;actions=delete"},
},
{
name: "Set grant matching existing grant",
existing: []string{"id=1;actions=read", "id=*;type=*;actions=delete"},
existing: []string{"id=u_foo;actions=read", "id=*;type=*;actions=delete"},
set: []string{"id=*;type=*;actions=delete"},
result: []string{"id=*;type=*;actions=delete"},
},
{
name: "Set duplicate grant matching existing grant",
existing: []string{"id=1;actions=read", "id=*;type=*;actions=delete"},
existing: []string{"id=u_foo;actions=read", "id=*;type=*;actions=delete"},
set: []string{"id=*;type=*;actions=delete", "id=*;type=*;actions=delete"},
result: []string{"id=*;type=*;actions=delete"},
},
{
name: "Set empty on role",
existing: []string{"id=1;type=*;actions=read", "id=*;type=*;actions=delete"},
existing: []string{"id=hcst_foo;type=*;actions=read", "id=*;type=*;actions=delete"},
set: nil,
result: nil,
},
{
name: "Check deprecation",
existing: []string{"id=1;actions=read", "id=*;type=*;actions=delete"},
existing: []string{"id=u_foo;actions=read", "id=*;type=*;actions=delete"},
set: []string{"id=*;type=target;actions=add-host-sets"},
wantErr: true,
wantErrContains: "Use \\\"add-host-sources\\\" instead",
@ -1940,31 +1940,31 @@ func TestRemoveGrants(t *testing.T) {
}{
{
name: "Remove all",
existing: []string{"id=1;type=*;actions=read"},
remove: []string{"id=1;type=*;actions=read"},
existing: []string{"id=hcst_1;type=*;actions=read"},
remove: []string{"id=hcst_1;type=*;actions=read"},
},
{
name: "Remove partial",
existing: []string{"id=1;type=*;actions=read", "id=2;type=*;actions=delete"},
remove: []string{"id=1;type=*;actions=read"},
result: []string{"id=2;type=*;actions=delete"},
existing: []string{"id=hcst_1;type=*;actions=read", "id=hcst_2;type=*;actions=delete"},
remove: []string{"id=hcst_1;type=*;actions=read"},
result: []string{"id=hcst_2;type=*;actions=delete"},
},
{
name: "Remove duplicate",
existing: []string{"id=1;type=*;actions=read", "id=2;type=*;actions=delete"},
remove: []string{"id=1;type=*;actions=read", "id=1;type=*;actions=read"},
result: []string{"id=2;type=*;actions=delete"},
existing: []string{"id=hcst_1;type=*;actions=read", "id=hcst_2;type=*;actions=delete"},
remove: []string{"id=hcst_1;type=*;actions=read", "id=hcst_1;type=*;actions=read"},
result: []string{"id=hcst_2;type=*;actions=delete"},
},
{
name: "Remove non existant",
existing: []string{"id=2;type=*;actions=delete"},
remove: []string{"id=1;type=*;actions=read"},
result: []string{"id=2;type=*;actions=delete"},
existing: []string{"id=hcst_2;type=*;actions=delete"},
remove: []string{"id=hcst_1;type=*;actions=read"},
result: []string{"id=hcst_2;type=*;actions=delete"},
},
{
name: "Remove from empty role",
existing: []string{},
remove: []string{"id=1;type=*;actions=read"},
remove: []string{"id=hcst_1;type=*;actions=read"},
result: nil,
},
}
@ -2012,7 +2012,7 @@ func TestRemoveGrants(t *testing.T) {
name: "Bad Version",
req: &pbs.RemoveRoleGrantsRequest{
Id: role.GetPublicId(),
GrantStrings: []string{"id=2;type=*;actions=create"},
GrantStrings: []string{"id=hcst_2;type=*;actions=create"},
Version: role.GetVersion() + 2,
},
err: handlers.ApiErrorWithCode(codes.Internal),

@ -179,14 +179,14 @@ func TestRoleGrant_Delete(t *testing.T) {
name: "invalid",
args: args{
roleId: projRole.PublicId,
grant: "id=a_edcb;actions=read,update",
grant: "id=u_edcb;actions=read,update",
},
},
{
name: "valid",
args: args{
roleId: projRole.PublicId,
grant: "id=a_bcde;actions=read,update",
grant: "id=u_bcde;actions=read,update",
},
deletedRows: 1,
},
@ -196,10 +196,10 @@ func TestRoleGrant_Delete(t *testing.T) {
assert, require := assert.New(t), require.New(t)
r := allocRoleGrant()
db.TestDeleteWhere(t, conn, &r, "1=1")
rg, err := NewRoleGrant(projRole.PublicId, "id=a_bcde;actions=read,update")
rg, err := NewRoleGrant(projRole.PublicId, "id=u_bcde;actions=read,update")
require.NoError(err)
require.NoError(rw.Create(context.Background(), rg))
rg, err = NewRoleGrant(projRole.PublicId, "id=a_cdef;actions=read,update")
rg, err = NewRoleGrant(projRole.PublicId, "id=u_cdef;actions=read,update")
require.NoError(err)
require.NoError(rw.Create(context.Background(), rg))
@ -242,21 +242,21 @@ func TestRoleGrant_Clone(t *testing.T) {
assert.True(proto.Equal(cp.(*RoleGrant).RoleGrant, g.RoleGrant))
})
t.Run("not-equal", func(t *testing.T) {
assert := assert.New(t)
require, assert := require.New(t), assert.New(t)
s := testOrg(t, repo, "", "")
role := TestRole(t, conn, s.PublicId)
g, err := NewRoleGrant(role.PublicId, "id=*;type=*;actions=*")
assert.NoError(err)
assert.NotNil(g)
require.NotNil(g)
assert.Equal(g.RoleId, role.PublicId)
assert.Equal(g.RawGrant, "id=*;type=*;actions=*")
g2, err := NewRoleGrant(role.PublicId, "id=foo;actions=read")
g2, err := NewRoleGrant(role.PublicId, "id=u_foo;actions=read")
assert.NoError(err)
assert.NotNil(g2)
require.NotNil(g2)
assert.Equal(g2.RoleId, role.PublicId)
assert.Equal(g2.RawGrant, "id=foo;actions=read")
assert.Equal(g2.RawGrant, "id=u_foo;actions=read")
cp := g.Clone()
assert.True(!proto.Equal(cp.(*RoleGrant).RoleGrant, g2.RoleGrant))

@ -170,10 +170,10 @@ func (a ACL) Allowed(r Resource, aType action.Type, userId string, opt ...Option
found = true
// Case 3: type=<resource.type>;actions=<action> when action is list or
// create. Must be a top level collection, otherwise must be one of the
// two formats specified in cases 4 or 5. Or,
// type=resource.type;output_fields=<fields> and no action. This is more
// of a semantic difference compared to 4 more than a security
// create (cannot be a wildcard). Must be a top level collection,
// otherwise must be one of the two formats specified in cases 4 or 5.
// Or, type=resource.type;output_fields=<fields> and no action. This is
// more of a semantic difference compared to 4 more than a security
// difference; this type is for clarity as it ties more closely to the
// concept of create and list as actions on a collection, operating on a
// collection directly. The format in case 4 will still work for
@ -188,7 +188,7 @@ func (a ACL) Allowed(r Resource, aType action.Type, userId string, opt ...Option
r.Id == "" &&
grant.typ == r.Type &&
grant.typ != resource.Unknown &&
topLevelType(r.Type) &&
resource.TopLevelType(r.Type) &&
(action.List.IsActionOrParent(aType) ||
action.Create.IsActionOrParent(aType)):
@ -213,7 +213,7 @@ func (a ACL) Allowed(r Resource, aType action.Type, userId string, opt ...Option
grant.id == r.Pin &&
grant.typ != resource.Unknown &&
(grant.typ == r.Type || grant.typ == resource.All) &&
!topLevelType(r.Type):
!resource.TopLevelType(r.Type):
found = true
}
@ -309,21 +309,3 @@ func (a ACL) ListPermissions(requestedScopes map[string]*scopes.ScopeInfo,
return perms
}
func topLevelType(typ resource.Type) bool {
switch typ {
case resource.AuthMethod,
resource.AuthToken,
resource.CredentialStore,
resource.Group,
resource.HostCatalog,
resource.Role,
resource.Scope,
resource.Session,
resource.Target,
resource.User,
resource.Worker:
return true
}
return false
}

@ -44,8 +44,8 @@ func Test_ACLAllowed(t *testing.T) {
{
scope: "o_a",
grants: []string{
"id=a_bar;actions=read,update",
"id=a_baz;actions=read:self,update",
"id=ampw_bar;actions=read,update",
"id=ampw_baz;actions=read:self,update",
"type=host-catalog;actions=create",
"type=target;actions=list",
"id=*;type=host-set;actions=list,create",
@ -55,18 +55,11 @@ func Test_ACLAllowed(t *testing.T) {
scope: "o_b",
grants: []string{
"id=*;type=host-set;actions=list,create",
"id=mypin;type=host;actions=*;output_fields=name,description",
"id=hcst_mypin;type=host;actions=*;output_fields=name,description",
"id=*;type=*;actions=authenticate",
"id=*;type=*;output_fields=id",
},
},
{
scope: "o_c",
grants: []string{
"id={{user.id }};actions=read,update",
"id={{ account.id}};actions=change-password",
},
},
{
scope: "o_d",
grants: []string{
@ -76,6 +69,15 @@ func Test_ACLAllowed(t *testing.T) {
},
},
}
templateGrants := []scopeGrant{
{
scope: "o_c",
grants: []string{
"id={{user.id }};actions=read,update",
"id={{ account.id}};actions=change-password",
},
},
}
// See acl.go for expected allowed formats. The goal here is to basically
// test those, but also test a whole bunch of nonconforming values.
@ -117,7 +119,7 @@ func Test_ACLAllowed(t *testing.T) {
},
{
name: "matching scope and id and matching action",
resource: Resource{ScopeId: "o_a", Id: "a_bar"},
resource: Resource{ScopeId: "o_a", Id: "ampw_bar"},
scopeGrants: commonGrants,
actionsAuthorized: []actionAuthorized{
{action: action.Read, authorized: true},
@ -127,7 +129,7 @@ func Test_ACLAllowed(t *testing.T) {
},
{
name: "matching scope and type and all action with valid pin",
resource: Resource{ScopeId: "o_b", Pin: "mypin", Type: resource.Host},
resource: Resource{ScopeId: "o_b", Pin: "hcst_mypin", Type: resource.Host},
scopeGrants: commonGrants,
actionsAuthorized: []actionAuthorized{
{action: action.Read, authorized: true, outputFields: []string{"description", "id", "name"}},
@ -186,7 +188,7 @@ func Test_ACLAllowed(t *testing.T) {
},
{
name: "matching scope, type, action, random id and bad pin",
resource: Resource{ScopeId: "o_a", Id: "anything", Type: resource.HostCatalog, Pin: "a_bar"},
resource: Resource{ScopeId: "o_a", Id: "anything", Type: resource.HostCatalog, Pin: "ampw_bar"},
scopeGrants: commonGrants,
actionsAuthorized: []actionAuthorized{
{action: action.Update},
@ -217,7 +219,7 @@ func Test_ACLAllowed(t *testing.T) {
{
name: "bad templated user id",
resource: Resource{ScopeId: "o_c"},
scopeGrants: commonGrants,
scopeGrants: append(commonGrants, templateGrants...),
actionsAuthorized: []actionAuthorized{
{action: action.List},
{action: action.Authenticate},
@ -228,7 +230,7 @@ func Test_ACLAllowed(t *testing.T) {
{
name: "good templated user id",
resource: Resource{ScopeId: "o_c", Id: "u_abcd1234"},
scopeGrants: commonGrants,
scopeGrants: append(commonGrants, templateGrants...),
actionsAuthorized: []actionAuthorized{
{action: action.Read, authorized: true},
{action: action.Update, authorized: true},
@ -238,7 +240,7 @@ func Test_ACLAllowed(t *testing.T) {
{
name: "bad templated old account id",
resource: Resource{ScopeId: "o_c"},
scopeGrants: commonGrants,
scopeGrants: append(commonGrants, templateGrants...),
actionsAuthorized: []actionAuthorized{
{action: action.List},
{action: action.Authenticate},
@ -249,7 +251,7 @@ func Test_ACLAllowed(t *testing.T) {
{
name: "good templated old account id",
resource: Resource{ScopeId: "o_c", Id: fmt.Sprintf("%s_1234567890", globals.PasswordAccountPreviousPrefix)},
scopeGrants: commonGrants,
scopeGrants: append(commonGrants, templateGrants...),
actionsAuthorized: []actionAuthorized{
{action: action.ChangePassword, authorized: true},
{action: action.Update},
@ -259,7 +261,7 @@ func Test_ACLAllowed(t *testing.T) {
{
name: "bad templated new account id",
resource: Resource{ScopeId: "o_c"},
scopeGrants: commonGrants,
scopeGrants: append(commonGrants, templateGrants...),
actionsAuthorized: []actionAuthorized{
{action: action.List},
{action: action.Authenticate},
@ -270,7 +272,7 @@ func Test_ACLAllowed(t *testing.T) {
{
name: "good templated new account id",
resource: Resource{ScopeId: "o_c", Id: fmt.Sprintf("%s_1234567890", globals.PasswordAccountPrefix)},
scopeGrants: commonGrants,
scopeGrants: append(commonGrants, templateGrants...),
actionsAuthorized: []actionAuthorized{
{action: action.ChangePassword, authorized: true},
{action: action.Update},
@ -305,7 +307,7 @@ func Test_ACLAllowed(t *testing.T) {
},
{
name: "read self with top level read",
resource: Resource{ScopeId: "o_a", Id: "a_bar"},
resource: Resource{ScopeId: "o_a", Id: "ampw_bar"},
scopeGrants: commonGrants,
actionsAuthorized: []actionAuthorized{
{action: action.Read, authorized: true},
@ -314,7 +316,7 @@ func Test_ACLAllowed(t *testing.T) {
},
{
name: "read self only",
resource: Resource{ScopeId: "o_a", Id: "a_baz"},
resource: Resource{ScopeId: "o_a", Id: "ampw_baz"},
scopeGrants: commonGrants,
actionsAuthorized: []actionAuthorized{
{action: action.Read},

@ -345,8 +345,11 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) {
opts := getOpts(opt...)
// Check for templated values ID, and substitute in with the authenticated values
// if so
// Check for templated values ID, and substitute in with the authenticated
// values if so. If we are using a dummy user or account ID, store the
// original ID and return it at the end; this is usually the case when
// validating grant formats.
var origId string
if grant.id != "" && strings.HasPrefix(grant.id, "{{") {
id := strings.TrimSuffix(strings.TrimPrefix(grant.id, "{{"), "}}")
id = strings.TrimSpace(id)
@ -355,10 +358,15 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) {
if opts.withUserId != "" {
grant.id = strings.ToValidUTF8(opts.withUserId, string(unicode.ReplacementChar))
}
// Otherwise, substitute in a dummy value
origId = grant.id
grant.id = "u_dummy"
case "account.id", ".Account.Id":
if opts.withAccountId != "" {
grant.id = strings.ToValidUTF8(opts.withAccountId, string(unicode.ReplacementChar))
}
origId = grant.id
grant.id = "acctoidc_dummy"
default:
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("unknown template %q in grant %q value", grant.id, "id"))
}
@ -373,50 +381,83 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) {
}
if !opts.withSkipFinalValidation {
// Filter out some forms that don't make sense
// First up, an ID is given, no type, and actions contains "create" or
// "list". Note wildcard for actions is still okay.
if grant.id != "" && grant.typ == resource.Unknown {
if grant.actions[action.Create] ||
grant.actions[action.List] {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains create or list action in a format that does not allow these")
switch {
case grant.id == "*":
// Matches
// id=*;type=sometype;actions=foo,bar
// or
// id=*;type=*;actions=foo,bar
// This can be a non-unknown type or wildcard
if grant.typ == resource.Unknown {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains wildcard id and no specified type", grant.CanonicalString()))
}
case grant.id != "":
// Non-wildcard but specified ID. This can match
// id=foo_bar;actions=foo,bar
// or
// id=foo_bar;type=sometype;actions=foo,bar
// or
// id=foo_bar;type=*;actions=foo,bar
// but notably the specified types have to actually make sense: in
// the second example the type corresponding to the ID must have the
// specified type as a child type; in the third the ID must be a
// type that has child types.
idType := globals.ResourceTypeFromPrefix(grant.id)
if idType == resource.Unknown {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains an id %q of an unknown resource type", grant.CanonicalString(), grant.id))
}
}
// If no ID is given...
if grant.id == "" {
// Check the type
switch grant.typ {
case resource.Unknown:
// This is fine as-is but we do not support collection actions
// without a type (either directly specified or wildcard) so
// check that
if grant.actions[action.Create] ||
grant.actions[action.List] {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains create or list action in a format that does not allow these", grant.CanonicalString()))
}
case resource.All:
// Verify that the ID is a type that has child types
if !resource.HasChildTypes(idType) {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains an id that does not support child types", grant.CanonicalString()))
}
default:
// Specified resource type, verify it's a child
if resource.Parent(grant.typ) != idType {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains type %s that is not a child type of the type (%s) of the specified id", grant.CanonicalString(), grant.typ.String(), idType.String()))
}
}
default: // no specified id
switch grant.typ {
case resource.Unknown:
// Error -- no ID or type isn't valid (although we should never
// get to this point because original parsing should error)
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains no id or type")
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains no id or type", grant.CanonicalString()))
case resource.All:
// "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")
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains wildcard type with no id value", grant.CanonicalString()))
default:
// Here we have type=something,actions=<something else>. This
// means we're operating on collections. Note that wildcard
// actions are not okay here; that uses the format
// id=*;type=<something>;actions=*
// means we're operating on collections and support only create
// or list. Note that wildcard actions are not okay here; that
// uses the format id=*;type=<something>;actions=*
switch len(grant.actions) {
case 0:
// It's okay to have no actions if only output fields are being defined
if _, hasSetFields := grant.OutputFields.Fields(); !hasSetFields {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains no actions or output fields")
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains no actions or output fields", grant.CanonicalString()))
}
case 1:
if !grant.hasActionOrSubaction(action.Create) &&
!grant.hasActionOrSubaction(action.List) {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains non-create or non-list action in a format that only allows these")
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains non-create or non-list action in a format that only allows these", grant.CanonicalString()))
}
case 2:
if !grant.hasActionOrSubaction(action.Create) || !grant.hasActionOrSubaction(action.List) {
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains non-create or non-list action in a format that only allows these")
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains non-create or non-list action in a format that only allows these", grant.CanonicalString()))
}
default:
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, "parsed grant string contains non-create or non-list action in a format that only allows these")
return Grant{}, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("parsed grant string %q contains non-create or non-list action in a format that only allows these", grant.CanonicalString()))
}
}
}
@ -430,7 +471,7 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) {
Id: grant.id,
Type: grant.typ,
}
if !topLevelType(grant.typ) {
if !resource.TopLevelType(grant.typ) {
r.Pin = grant.id
}
var allowed bool
@ -447,6 +488,11 @@ func Parse(scopeId, grantString string, opt ...Option) (Grant, error) {
}
}
// If we substituted in a dummy value, replace with the original now
if origId != "" {
grant.id = origId
}
return grant, nil
}

@ -386,7 +386,7 @@ func Test_Parse(t *testing.T) {
},
{
name: "bad type",
input: "id=foobar;type=barfoo;actions=create,read",
input: "id=foobar;type=barfoo;actions=read",
err: `perms.Parse: unable to parse grant string: perms.(Grant).unmarshalText: unknown type specifier "barfoo": parameter violation: error #100`,
},
{
@ -395,44 +395,74 @@ func Test_Parse(t *testing.T) {
err: `perms.Parse: perms.(Grant).parseAndValidateActions: unknown action "createread": parameter violation: error #100`,
},
{
name: "bad create action for id",
name: "bad id type",
input: "id=foobar;actions=create",
err: `perms.Parse: parsed grant string contains create or list action in a format that does not allow these: parameter violation: error #100`,
err: `perms.Parse: parsed grant string "id=foobar;actions=create" contains an id "foobar" of an unknown resource type: parameter violation: error #100`,
},
{
name: "bad create action for id",
input: "id=u_foobar;actions=create",
err: `perms.Parse: parsed grant string "id=u_foobar;actions=create" contains create or list action in a format that does not allow these: parameter violation: error #100`,
},
{
name: "bad create action for id with other perms",
input: "id=foobar;actions=read,create",
err: `perms.Parse: parsed grant string contains create or list action in a format that does not allow these: parameter violation: error #100`,
input: "id=u_foobar;actions=read,create",
err: `perms.Parse: parsed grant string "id=u_foobar;actions=create,read" contains create or list action in a format that does not allow these: parameter violation: error #100`,
},
{
name: "bad list action for id",
input: "id=foobar;actions=list",
err: `perms.Parse: parsed grant string contains create or list action in a format that does not allow these: parameter violation: error #100`,
input: "id=u_foobar;actions=list",
err: `perms.Parse: parsed grant string "id=u_foobar;actions=list" contains create or list action in a format that does not allow these: parameter violation: error #100`,
},
{
name: "bad list action for id with other perms",
input: "type=host-catalog;actions=list,read",
err: `perms.Parse: parsed grant string contains non-create or non-list action in a format that only allows these: parameter violation: error #100`,
err: `perms.Parse: parsed grant string "type=host-catalog;actions=list,read" contains non-create or non-list action in a format that only allows these: parameter violation: error #100`,
},
{
name: "wildcard id and actions without collection",
input: "id=*;actions=read",
err: `perms.Parse: parsed grant string would not result in any action being authorized: parameter violation: error #100`,
err: `perms.Parse: parsed grant string "id=*;actions=read" contains wildcard id and no specified type: parameter violation: error #100`,
},
{
name: "wildcard id and actions with list",
input: "id=*;actions=read,list",
err: `perms.Parse: parsed grant string contains create or list action in a format that does not allow these: parameter violation: error #100`,
err: `perms.Parse: parsed grant string "id=*;actions=list,read" contains wildcard id and no specified type: parameter violation: error #100`,
},
{
name: "wildcard type with no id",
input: "type=*;actions=read,list",
err: `perms.Parse: parsed grant string contains wildcard type with no id value: parameter violation: error #100`,
err: `perms.Parse: parsed grant string "type=*;actions=list,read" contains wildcard type with no id value: parameter violation: error #100`,
},
{
name: "empty id and type",
input: "actions=create",
err: `perms.Parse: parsed grant string contains no id or type: parameter violation: error #100`,
err: `perms.Parse: parsed grant string "actions=create" contains no id or type: parameter violation: error #100`,
},
{
name: "wildcard type non child id",
input: "id=ttcp_1234567890;type=*;actions=create",
err: `perms.Parse: parsed grant string "id=ttcp_1234567890;type=*;actions=create" contains an id that does not support child types: parameter violation: error #100`,
},
{
name: "specified resource type non child",
input: "id=hcst_1234567890;type=account;actions=read",
err: `perms.Parse: parsed grant string "id=hcst_1234567890;type=account;actions=read" contains type account that is not a child type of the type (host-catalog) of the specified id: parameter violation: error #100`,
},
{
name: "no id with one bad action",
input: "type=host-set;actions=read",
err: `perms.Parse: parsed grant string "type=host-set;actions=read" contains non-create or non-list action in a format that only allows these: parameter violation: error #100`,
},
{
name: "no id with two bad action",
input: "type=host-set;actions=read,create",
err: `perms.Parse: parsed grant string "type=host-set;actions=create,read" contains non-create or non-list action in a format that only allows these: parameter violation: error #100`,
},
{
name: "no id with three bad action",
input: "type=host-set;actions=list,read,create",
err: `perms.Parse: parsed grant string "type=host-set;actions=create,list,read" contains non-create or non-list action in a format that only allows these: parameter violation: error #100`,
},
{
name: "empty output fields",
@ -504,13 +534,13 @@ func Test_Parse(t *testing.T) {
},
{
name: "good json id",
input: `{"id":"foobar","actions":["read"]}`,
input: `{"id":"u_foobar","actions":["read"]}`,
expected: Grant{
scope: Scope{
Id: "o_scope",
Type: scope.Org,
},
id: "foobar",
id: "u_foobar",
typ: resource.Unknown,
actions: map[action.Type]bool{
action.Read: true,
@ -519,13 +549,13 @@ func Test_Parse(t *testing.T) {
},
{
name: "good json output fields",
input: `{"id":"foobar","actions":["read"],"output_fields":["version","id","name"]}`,
input: `{"id":"u_foobar","actions":["read"],"output_fields":["version","id","name"]}`,
expected: Grant{
scope: Scope{
Id: "o_scope",
Type: scope.Org,
},
id: "foobar",
id: "u_foobar",
typ: resource.Unknown,
actions: map[action.Type]bool{
action.Read: true,
@ -541,13 +571,13 @@ func Test_Parse(t *testing.T) {
},
{
name: "good json output fields no action",
input: `{"id":"foobar","output_fields":["version","id","name"]}`,
input: `{"id":"u_foobar","output_fields":["version","id","name"]}`,
expected: Grant{
scope: Scope{
Id: "o_scope",
Type: scope.Org,
},
id: "foobar",
id: "u_foobar",
typ: resource.Unknown,
OutputFields: &OutputFields{
fields: map[string]bool{
@ -574,13 +604,13 @@ func Test_Parse(t *testing.T) {
},
{
name: "good text id",
input: `id=foobar;actions=read`,
input: `id=u_foobar;actions=read`,
expected: Grant{
scope: Scope{
Id: "o_scope",
Type: scope.Org,
},
id: "foobar",
id: "u_foobar",
typ: resource.Unknown,
actions: map[action.Type]bool{
action.Read: true,
@ -589,13 +619,13 @@ func Test_Parse(t *testing.T) {
},
{
name: "good output fields",
input: `id=foobar;actions=read;output_fields=version,id,name`,
input: `id=u_foobar;actions=read;output_fields=version,id,name`,
expected: Grant{
scope: Scope{
Id: "o_scope",
Type: scope.Org,
},
id: "foobar",
id: "u_foobar",
typ: resource.Unknown,
actions: map[action.Type]bool{
action.Read: true,
@ -611,14 +641,14 @@ func Test_Parse(t *testing.T) {
},
{
name: "default project scope",
input: `id=foobar;actions=read`,
input: `id=hcst_foobar;actions=read`,
scopeOverride: "p_1234",
expected: Grant{
scope: Scope{
Id: "p_1234",
Type: scope.Project,
},
id: "foobar",
id: "hcst_foobar",
typ: resource.Unknown,
actions: map[action.Type]bool{
action.Read: true,
@ -627,14 +657,14 @@ func Test_Parse(t *testing.T) {
},
{
name: "default org scope",
input: `id=foobar;actions=read`,
input: `id=acctpw_foobar;actions=read`,
scopeOverride: "o_1234",
expected: Grant{
scope: Scope{
Id: "o_1234",
Type: scope.Org,
},
id: "foobar",
id: "acctpw_foobar",
typ: resource.Unknown,
actions: map[action.Type]bool{
action.Read: true,
@ -643,14 +673,14 @@ func Test_Parse(t *testing.T) {
},
{
name: "default global scope",
input: `id=foobar;actions=read`,
input: `id=acctpw_foobar;actions=read`,
scopeOverride: "global",
expected: Grant{
scope: Scope{
Id: "global",
Type: scope.Global,
},
id: "foobar",
id: "acctpw_foobar",
typ: resource.Unknown,
actions: map[action.Type]bool{
action.Read: true,

@ -146,24 +146,24 @@ func Test_ACLOutputFields(t *testing.T) {
tests := []input{
{
name: "default",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
action: action.Read,
grants: []string{"id=bar;actions=read,update"},
grants: []string{"id=u_bar;actions=read,update"},
authorized: true,
},
{
name: "single value",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
grants: []string{"id=bar;actions=read,update;output_fields=id"},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{"id=u_bar;actions=read,update;output_fields=id"},
action: action.Read,
fields: []string{"id"},
authorized: true,
},
{
name: "compound no overlap",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;actions=read,update;output_fields=id",
"id=u_bar;actions=read,update;output_fields=id",
"id=*;type=host-catalog;actions=read,update;output_fields=version",
},
action: action.Read,
@ -172,9 +172,9 @@ func Test_ACLOutputFields(t *testing.T) {
},
{
name: "compound",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;actions=read,update;output_fields=id",
"id=u_bar;actions=read,update;output_fields=id",
"id=*;type=role;output_fields=version",
},
action: action.Read,
@ -183,9 +183,9 @@ func Test_ACLOutputFields(t *testing.T) {
},
{
name: "wildcard with type",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;actions=read,update;output_fields=read",
"id=u_bar;actions=read,update;output_fields=read",
"id=*;type=role;output_fields=*",
},
action: action.Read,
@ -194,9 +194,9 @@ func Test_ACLOutputFields(t *testing.T) {
},
{
name: "wildcard with wildcard type",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;actions=read,update;output_fields=read",
"id=u_bar;actions=read,update;output_fields=read",
"id=*;type=*;output_fields=*",
},
action: action.Read,
@ -205,9 +205,9 @@ func Test_ACLOutputFields(t *testing.T) {
},
{
name: "subaction exact",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;actions=read:self,update;output_fields=version",
"id=u_bar;actions=read:self,update;output_fields=version",
},
action: action.ReadSelf,
fields: []string{"version"},
@ -217,10 +217,10 @@ func Test_ACLOutputFields(t *testing.T) {
// If the action is a subaction, parent output fields will apply, in
// addition to subaction. This matches authorization.
name: "subaction parent action",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;actions=read,update;output_fields=version",
"id=bar;actions=read:self;output_fields=id",
"id=u_bar;actions=read,update;output_fields=version",
"id=u_bar;actions=read:self;output_fields=id",
},
action: action.ReadSelf,
fields: []string{"id", "version"},
@ -232,10 +232,10 @@ func Test_ACLOutputFields(t *testing.T) {
// non-self actions. This is useful to allow more visibility to self
// actions and less in the general case.
name: "subaction child action",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;actions=read:self,update;output_fields=version",
"id=bar;actions=read;output_fields=id",
"id=u_bar;actions=read:self,update;output_fields=version",
"id=u_bar;actions=read;output_fields=id",
},
action: action.Read,
fields: []string{"id"},
@ -243,10 +243,10 @@ func Test_ACLOutputFields(t *testing.T) {
},
{
name: "initial grant unauthorized with star",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;output_fields=*",
"id=bar;actions=delete;output_fields=id",
"id=u_bar;output_fields=*",
"id=u_bar;actions=delete;output_fields=id",
},
action: action.Delete,
fields: []string{"*"},
@ -254,9 +254,9 @@ func Test_ACLOutputFields(t *testing.T) {
},
{
name: "unauthorized id only",
resource: Resource{ScopeId: "o_myorg", Id: "bar", Type: resource.Role},
resource: Resource{ScopeId: "o_myorg", Id: "u_bar", Type: resource.Role},
grants: []string{
"id=bar;output_fields=name",
"id=u_bar;output_fields=name",
},
action: action.Delete,
fields: []string{"name"},

@ -36,7 +36,7 @@ const (
// * The scopes service collection actions for appropriate scopes
// * The Test_AnonRestrictions test: update the following line to include the last resource:
// for i := resource.Type(1); i <= resource.<Resource>; i++ {
// * The prefixes and top level prefixes in globals/prefixes.go
// * The prefixes and mappings in globals/prefixes.go
)
func (r Type) MarshalJSON() ([]byte, error) {
@ -99,3 +99,47 @@ var Map = map[string]Type{
CredentialLibrary.String(): CredentialLibrary,
Credential.String(): Credential,
}
// Parent returns the parent type for a given type; if there is no parent, it
// returns the incoming type
func Parent(in Type) Type {
switch in {
case Account, ManagedGroup:
return AuthMethod
case HostSet, Host:
return HostCatalog
case CredentialLibrary, Credential:
return CredentialStore
}
return in
}
// HasChildTypes indicates whether this is a type that has child resource types;
// it's essentially the inverse of Parent
func HasChildTypes(in Type) bool {
switch in {
case AuthMethod, HostCatalog, CredentialStore:
return true
}
return false
}
// TopLevelType indicates whether this is a type that supports collection
// actions, e.g. Create/List
func TopLevelType(typ Type) bool {
switch typ {
case AuthMethod,
AuthToken,
CredentialStore,
Group,
HostCatalog,
Role,
Scope,
Session,
Target,
User,
Worker:
return true
}
return false
}

@ -11,90 +11,125 @@ import (
func Test_Resource(t *testing.T) {
tests := []struct {
typeString string
want Type
typeString string
want Type
topLevelType bool
hasChildTypes bool
parent Type
}{
{
typeString: "unknown",
want: Unknown,
},
{
typeString: "scope",
want: Scope,
typeString: "scope",
want: Scope,
topLevelType: true,
},
{
typeString: "user",
want: User,
typeString: "user",
want: User,
topLevelType: true,
},
{
typeString: "group",
want: Group,
typeString: "group",
want: Group,
topLevelType: true,
},
{
typeString: "role",
want: Role,
typeString: "role",
want: Role,
topLevelType: true,
},
{
typeString: "auth-method",
want: AuthMethod,
typeString: "auth-method",
want: AuthMethod,
topLevelType: true,
hasChildTypes: true,
},
{
typeString: "account",
want: Account,
parent: AuthMethod,
},
{
typeString: "auth-token",
want: AuthToken,
typeString: "auth-token",
want: AuthToken,
topLevelType: true,
},
{
typeString: "*",
want: All,
},
{
typeString: "host-catalog",
want: HostCatalog,
typeString: "host-catalog",
want: HostCatalog,
topLevelType: true,
hasChildTypes: true,
},
{
typeString: "host-set",
want: HostSet,
parent: HostCatalog,
},
{
typeString: "host",
want: Host,
parent: HostCatalog,
},
{
typeString: "target",
want: Target,
typeString: "target",
want: Target,
topLevelType: true,
},
{
typeString: "controller",
want: Controller,
},
{
typeString: "worker",
want: Worker,
typeString: "worker",
want: Worker,
topLevelType: true,
},
{
typeString: "session",
want: Session,
typeString: "session",
want: Session,
topLevelType: true,
},
{
typeString: "managed-group",
want: ManagedGroup,
parent: AuthMethod,
},
{
typeString: "credential-store",
want: CredentialStore,
typeString: "credential-store",
want: CredentialStore,
topLevelType: true,
hasChildTypes: true,
},
{
typeString: "credential-library",
want: CredentialLibrary,
parent: CredentialStore,
},
{
typeString: "credential",
want: Credential,
parent: CredentialStore,
},
}
for _, tt := range tests {
t.Run(tt.typeString, func(t *testing.T) {
assert.Equalf(t, tt.want, Map[tt.typeString], "unexpected type for %s", tt.typeString)
assert.Equalf(t, tt.typeString, tt.want.String(), "unexpected string for %s", tt.typeString)
assert.Equalf(t, tt.topLevelType, TopLevelType(tt.want), "unexpected top level type types for %s", tt.typeString)
assert.Equalf(t, tt.hasChildTypes, HasChildTypes(tt.want), "unexpected has child types for %s", tt.typeString)
parent := Parent(tt.want)
if tt.parent == Unknown {
assert.Equal(t, tt.want, parent)
} else {
assert.Equal(t, tt.parent, parent)
}
})
}
}

Loading…
Cancel
Save