From a247b959eae4129cf330f801052fec5f98b07ac1 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 17 Jan 2017 21:34:14 -0500 Subject: [PATCH] Don't sanitize disallowed_policies on token role --- helper/strutil/strutil.go | 2 +- vault/token_store.go | 6 +++--- vault/token_store_test.go | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/helper/strutil/strutil.go b/helper/strutil/strutil.go index cccb8a22c4..d079b9cb92 100644 --- a/helper/strutil/strutil.go +++ b/helper/strutil/strutil.go @@ -34,7 +34,7 @@ func StrListSubset(super, sub []string) bool { // empty items. The values will be converted to lower case. func ParseDedupAndSortStrings(input string, sep string) []string { input = strings.TrimSpace(input) - var parsed []string + parsed := []string{} if input == "" { // Don't return nil return parsed diff --git a/vault/token_store.go b/vault/token_store.go index d1b56d9ae7..39e532c793 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1485,7 +1485,7 @@ func (ts *TokenStore) handleCreateCommon( if len(role.DisallowedPolicies) > 0 { // We don't add the default here because we only want to disallow it if it's explicitly set - sanitizedRolePolicies = policyutil.SanitizePolicies(role.DisallowedPolicies, policyutil.DoNotAddDefaultPolicy) + sanitizedRolePolicies = strutil.RemoveDuplicates(role.DisallowedPolicies) for _, finalPolicy := range finalPolicies { if strutil.StrListContains(sanitizedRolePolicies, finalPolicy) { @@ -2218,9 +2218,9 @@ func (ts *TokenStore) tokenStoreRoleCreateUpdate( disallowedPoliciesStr, ok := data.GetOk("disallowed_policies") if ok { - entry.DisallowedPolicies = policyutil.SanitizePolicies(strings.Split(disallowedPoliciesStr.(string), ","), policyutil.DoNotAddDefaultPolicy) + entry.DisallowedPolicies = strutil.ParseDedupAndSortStrings(disallowedPoliciesStr.(string), ",") } else if req.Operation == logical.CreateOperation { - entry.DisallowedPolicies = policyutil.SanitizePolicies(strings.Split(data.Get("disallowed_policies").(string), ","), policyutil.DoNotAddDefaultPolicy) + entry.DisallowedPolicies = strutil.ParseDedupAndSortStrings(data.Get("disallowed_policies").(string), ",") } // Store it diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 7e05ed76db..62ca4a3fc2 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -1792,6 +1792,38 @@ func TestTokenStore_RoleCRUD(t *testing.T) { } } +func TestTokenStore_RoleDisallowedPoliciesWithRoot(t *testing.T) { + var resp *logical.Response + var err error + + _, ts, _, root := TestCoreWithTokenStore(t) + + // Don't set disallowed_policies. Verify that a read on the role does return a non-nil value. + roleReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "roles/role1", + Data: map[string]interface{}{ + "disallowed_policies": "root,testpolicy", + }, + ClientToken: root, + } + resp, err = ts.HandleRequest(roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + roleReq.Operation = logical.ReadOperation + resp, err = ts.HandleRequest(roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + expected := []string{"root", "testpolicy"} + if !reflect.DeepEqual(resp.Data["disallowed_policies"], expected) { + t.Fatalf("bad: expected: %#v, actual: %#v", expected, resp.Data["disallowed_policies"]) + } +} + func TestTokenStore_RoleDisallowedPolicies(t *testing.T) { var req *logical.Request var resp *logical.Response