From 0629526319183eb7c6317907a3a930b4d43d17a9 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 22 Jan 2021 18:38:19 -0500 Subject: [PATCH] Add read/list/cancel:self actions and support for ACL subactions. (#882) Update perms table in anticipation of session support for these. --- internal/perms/acl.go | 30 ++++++++++++--- internal/perms/acl_test.go | 38 +++++++++++++++++++ internal/perms/grants_test.go | 3 +- internal/types/action/action.go | 9 +++++ internal/types/action/action_test.go | 12 ++++++ internal/website/permstable/permstable.go | 21 ++++++++++ .../docs/concepts/security/permissions.mdx | 18 +++++++++ 7 files changed, 125 insertions(+), 6 deletions(-) diff --git a/internal/perms/acl.go b/internal/perms/acl.go index ea13975b86..da9f74efb8 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -14,6 +14,8 @@ construction is thus synthesizing something reasonable from a set of Grants. */ import ( + "strings" + "github.com/hashicorp/boundary/internal/types/action" "github.com/hashicorp/boundary/internal/types/resource" ) @@ -70,9 +72,25 @@ func (a ACL) Allowed(r Resource, aType action.Type) (results ACLResults) { grants := a.scopeMap[r.ScopeId] results.scopeMap = a.scopeMap + var parentAction action.Type + split := strings.Split(aType.String(), ":") + if len(split) == 2 { + parentAction = action.Map[split[0]] + } // Now, go through and check the cases indicated above for _, grant := range grants { - if !(grant.actions[aType] || grant.actions[action.All]) { + switch { + case grant.actions[aType]: + // We have this action + case grant.actions[parentAction]: + // We don't have this action, but it's a subaction and we have the + // parent action. As an example, if we are looking for "list:self" + // and have "list", this is sufficient. + case grant.actions[action.All]: + // All actions are allowed + default: + // No actions in the grant match what we're looking for, so continue + // with the next grant continue } switch { @@ -85,15 +103,17 @@ func (a ACL) Allowed(r Resource, aType action.Type) (results ACLResults) { results.Allowed = true return - // type=;actions= when action is list or create. - // Must be a top level collection, otherwise must be one of the two - // formats specified below. + // type=;actions= when action is list(:self) or + // create. Must be a top level collection, otherwise must be one of the + // two formats specified below. case grant.id == "" && r.Id == "" && grant.typ == r.Type && grant.typ != resource.Unknown && topLevelType(r.Type) && - (aType == action.List || aType == action.Create): + (aType == action.List || + aType == action.ListSelf || + aType == action.Create): results.Allowed = true return diff --git a/internal/perms/acl_test.go b/internal/perms/acl_test.go index ddd3cc9d98..a416b784b8 100644 --- a/internal/perms/acl_test.go +++ b/internal/perms/acl_test.go @@ -35,6 +35,7 @@ func Test_ACLAllowed(t *testing.T) { scope: "o_a", grants: []string{ "id=a_bar;actions=read,update", + "id=a_baz;actions=read:self,update", "type=host-catalog;actions=create", "type=target;actions=list", "id=*;type=host-set;actions=list,create", @@ -59,6 +60,7 @@ func Test_ACLAllowed(t *testing.T) { scope: "o_d", grants: []string{ "id=*;type=*;actions=create,update", + "type=session;actions=list:self", }, }, } @@ -252,6 +254,42 @@ func Test_ACLAllowed(t *testing.T) { }, userId: "u_abcd1234", }, + { + name: "list self with top level list", + resource: Resource{ScopeId: "o_a", Type: resource.Target}, + scopeGrants: commonGrants, + actionsAllowed: []actionAllowed{ + {action: action.List, allowed: true}, + {action: action.ListSelf, allowed: true}, + }, + }, + { + name: "list self with top level list self", + resource: Resource{ScopeId: "o_d", Type: resource.Session}, + scopeGrants: commonGrants, + actionsAllowed: []actionAllowed{ + {action: action.List}, + {action: action.ListSelf, allowed: true}, + }, + }, + { + name: "read self with top level read", + resource: Resource{ScopeId: "o_a", Id: "a_bar"}, + scopeGrants: commonGrants, + actionsAllowed: []actionAllowed{ + {action: action.Read, allowed: true}, + {action: action.ReadSelf, allowed: true}, + }, + }, + { + name: "read self only", + resource: Resource{ScopeId: "o_a", Id: "a_baz"}, + scopeGrants: commonGrants, + actionsAllowed: []actionAllowed{ + {action: action.Read}, + {action: action.ReadSelf, allowed: true}, + }, + }, } for _, test := range tests { diff --git a/internal/perms/grants_test.go b/internal/perms/grants_test.go index c441d484d9..91e22b91bb 100644 --- a/internal/perms/grants_test.go +++ b/internal/perms/grants_test.go @@ -60,7 +60,7 @@ func Test_ActionParsingValidation(t *testing.T) { { name: "all valid", input: Grant{ - actionsBeingParsed: []string{"list", "create", "update", "read", "delete", "authenticate", "authorize-session"}, + actionsBeingParsed: []string{"list", "create", "update", "list:self", "read", "delete", "authenticate", "authorize-session"}, }, result: Grant{ actions: map[action.Type]bool{ @@ -71,6 +71,7 @@ func Test_ActionParsingValidation(t *testing.T) { action.Delete: true, action.Authenticate: true, action.AuthorizeSession: true, + action.ListSelf: true, }, }, }, diff --git a/internal/types/action/action.go b/internal/types/action/action.go index 327a8d6a48..170a25f41d 100644 --- a/internal/types/action/action.go +++ b/internal/types/action/action.go @@ -37,6 +37,9 @@ const ( AddAccounts Type = 28 SetAccounts Type = 29 RemoveAccounts Type = 30 + ReadSelf Type = 31 + ListSelf Type = 32 + CancelSelf Type = 33 ) var Map = map[string]Type{ @@ -70,6 +73,9 @@ var Map = map[string]Type{ AddAccounts.String(): AddAccounts, SetAccounts.String(): SetAccounts, RemoveAccounts.String(): RemoveAccounts, + ReadSelf.String(): ReadSelf, + ListSelf.String(): ListSelf, + CancelSelf.String(): CancelSelf, } func (a Type) String() string { @@ -105,6 +111,9 @@ func (a Type) String() string { "add-accounts", "set-accounts", "remove-accounts", + "read:self", + "list:self", + "cancel:self", }[a] } diff --git a/internal/types/action/action_test.go b/internal/types/action/action_test.go index 40f5f688ec..bbb8f6fff6 100644 --- a/internal/types/action/action_test.go +++ b/internal/types/action/action_test.go @@ -75,6 +75,18 @@ func TestAction(t *testing.T) { action: Deauthenticate, want: "deauthenticate", }, + { + action: ReadSelf, + want: "read:self", + }, + { + action: CancelSelf, + want: "cancel:self", + }, + { + action: ListSelf, + want: "list:self", + }, } for _, tt := range tests { t.Run(tt.want, func(t *testing.T) { diff --git a/internal/website/permstable/permstable.go b/internal/website/permstable/permstable.go index cea342a676..dfc989cd8c 100644 --- a/internal/website/permstable/permstable.go +++ b/internal/website/permstable/permstable.go @@ -687,6 +687,27 @@ var session = &Resource{ "id=;actions=cancel", }, }, + { + Name: "read:self", + Description: "Read a session, which must belong to the calling user", + Examples: []string{ + "id=*;type=session;actions=read:self", + }, + }, + { + Name: "list:self", + Description: "List sessions that belong to the calling user", + Examples: []string{ + "type=session;actions=list:self", + }, + }, + { + Name: "cancel:self", + Description: "Cancel a session, which must belong to the calling user", + Examples: []string{ + "id=*;type=session;actions=cancel:self", + }, + }, }, }, }, diff --git a/website/content/docs/concepts/security/permissions.mdx b/website/content/docs/concepts/security/permissions.mdx index 1f2abbbdfd..6078edb383 100644 --- a/website/content/docs/concepts/security/permissions.mdx +++ b/website/content/docs/concepts/security/permissions.mdx @@ -1092,6 +1092,24 @@ wildcard or templated grant strings.
  • id=<id>;actions=cancel
+
  • + read:self: Read a session, which must belong to the calling user +
    • +
      +
    • id=*;type=session;actions=read:self
      • +
    +
  • + list:self: List sessions that belong to the calling user +
    • +
      +
    • type=session;actions=list:self
      • +
    +
  • + cancel:self: Cancel a session, which must belong to the calling user +
    • +
      +
    • id=*;type=session;actions=cancel:self
      • +