From 7e33191538e27d6c68c89e340640f305e23e95c3 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 23 Mar 2018 17:50:06 -0400 Subject: [PATCH 01/11] Handle removal of parent index on revoke-orphan and tidy operations --- vault/token_store.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/vault/token_store.go b/vault/token_store.go index 69a6b288b3b6..68df6c1ea66c 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1169,6 +1169,12 @@ func (ts *TokenStore) revokeSalted(ctx context.Context, saltedId string) (ret er } } + // Clear the parent index + parentPath := parentPrefix + saltedId + "/" + if err = logical.ClearView(ctx, ts.view.SubView(parentPath)); err != nil { + return fmt.Errorf("failed to delete entry: %v", err) + } + // Now that the entry is not usable for any revocation tasks, nuke it path := lookupPrefix + saltedId if err = ts.view.Delete(ctx, path); err != nil { @@ -1322,17 +1328,34 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data return nil, fmt.Errorf("failed to fetch secondary index entries: %v", err) } - var countParentList, deletedCountParentList int64 + var countParentEntries, deletedCountParentEntries, countParentList, deletedCountParentList int64 // Scan through the secondary index entries; if there is an entry // with the token's salt ID at the end, remove it for _, parent := range parentList { + // Get the children children, err := ts.view.List(ctx, parentPrefix+parent) if err != nil { tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to read secondary index: %v", err)) continue } + // First check if the salt ID of the parent exists, and if not delete this + // entry and continue to the next one. + countParentEntries++ + if exists, _ := ts.lookupSalted(ctx, strings.TrimSuffix(parent, "/"), true); exists == nil { + index := parentPrefix + parent + ts.logger.Trace("token: deleting invalid parent index", "index", index) + if err = logical.ClearView(ctx, ts.view.SubView(index)); err != nil { + tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to delete parent index: %v", err)) + continue + } + deletedCountParentEntries++ + // Include the number of deleted children + deletedCountParentList += int64(len(children)) + continue + } + for _, child := range children { countParentList++ if countParentList%500 == 0 { @@ -1349,6 +1372,7 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data err = ts.view.Delete(ctx, index) if err != nil { tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to delete secondary index: %v", err)) + continue } deletedCountParentList++ } @@ -1447,6 +1471,8 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data } } + ts.logger.Debug("token: number of entries scanned in parent index", "count", countParentEntries) + ts.logger.Debug("token: number of entries deleted in parent index", "count", deletedCountParentEntries) ts.logger.Debug("token: number of tokens scanned in parent index list", "count", countParentList) ts.logger.Debug("token: number of tokens revoked in parent index list", "count", deletedCountParentList) ts.logger.Debug("token: number of accessors scanned", "count", countAccessorList) From 420373d30e80ebf8be75a87c6ae6179934222b7b Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 23 Mar 2018 18:25:52 -0400 Subject: [PATCH 02/11] Refactor handleTidy to use same for loop children deletion of invalid parent entry --- vault/token_store.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 68df6c1ea66c..d7331f22e447 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1333,6 +1333,8 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data // Scan through the secondary index entries; if there is an entry // with the token's salt ID at the end, remove it for _, parent := range parentList { + countParentEntries++ + // Get the children children, err := ts.view.List(ctx, parentPrefix+parent) if err != nil { @@ -1342,20 +1344,13 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data // First check if the salt ID of the parent exists, and if not delete this // entry and continue to the next one. - countParentEntries++ - if exists, _ := ts.lookupSalted(ctx, strings.TrimSuffix(parent, "/"), true); exists == nil { - index := parentPrefix + parent - ts.logger.Trace("token: deleting invalid parent index", "index", index) - if err = logical.ClearView(ctx, ts.view.SubView(index)); err != nil { - tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to delete parent index: %v", err)) - continue - } - deletedCountParentEntries++ - // Include the number of deleted children - deletedCountParentList += int64(len(children)) - continue + originalChildrenCount := int64(len(children)) + exists, _ := ts.lookupSalted(ctx, strings.TrimSuffix(parent, "/"), true) + if exists == nil { + ts.logger.Trace("token: deleting invalid parent entry", "index", parentPrefix+parent) } + var deletedChildrenCount int64 for _, child := range children { countParentList++ if countParentList%500 == 0 { @@ -1365,8 +1360,9 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data // Look up tainted entries so we can be sure that if this isn't // found, it doesn't exist. Doing the following without locking // since appropriate locks cannot be held with salted token IDs. + // Also perform deletion if the parent doesn't exist any more. te, _ := ts.lookupSalted(ctx, child, true) - if te == nil { + if te == nil || exists == nil { index := parentPrefix + parent + child ts.logger.Trace("token: deleting invalid secondary index", "index", index) err = ts.view.Delete(ctx, index) @@ -1374,9 +1370,15 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to delete secondary index: %v", err)) continue } - deletedCountParentList++ + deletedChildrenCount++ } } + // Add current children deleted count to the total count + deletedCountParentList += deletedChildrenCount + // If we deleted all the children, then add that to our deleted parent entries count + if originalChildrenCount == deletedChildrenCount { + deletedCountParentEntries++ + } } var countAccessorList, From 2533791a7047dce491e17999f131f75202de2dbd Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 23 Mar 2018 18:29:39 -0400 Subject: [PATCH 03/11] Update comments --- vault/token_store.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index d7331f22e447..e40b1e036917 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1169,7 +1169,7 @@ func (ts *TokenStore) revokeSalted(ctx context.Context, saltedId string) (ret er } } - // Clear the parent index + // Clear the parent entry parentPath := parentPrefix + saltedId + "/" if err = logical.ClearView(ctx, ts.view.SubView(parentPath)); err != nil { return fmt.Errorf("failed to delete entry: %v", err) @@ -1342,8 +1342,9 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data continue } - // First check if the salt ID of the parent exists, and if not delete this - // entry and continue to the next one. + // First check if the salt ID of the parent exists, and if not mark this so + // that deletion of children later with this loop below applies to all + // children originalChildrenCount := int64(len(children)) exists, _ := ts.lookupSalted(ctx, strings.TrimSuffix(parent, "/"), true) if exists == nil { @@ -1473,8 +1474,8 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data } } - ts.logger.Debug("token: number of entries scanned in parent index", "count", countParentEntries) - ts.logger.Debug("token: number of entries deleted in parent index", "count", deletedCountParentEntries) + ts.logger.Debug("token: number of entries scanned in parent prefix", "count", countParentEntries) + ts.logger.Debug("token: number of entries deleted in parent prefix", "count", deletedCountParentEntries) ts.logger.Debug("token: number of tokens scanned in parent index list", "count", countParentList) ts.logger.Debug("token: number of tokens revoked in parent index list", "count", deletedCountParentList) ts.logger.Debug("token: number of accessors scanned", "count", countAccessorList) From b027c2f19f5b9c919a4f6d2c01610582066bcae1 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 23 Mar 2018 19:40:06 -0400 Subject: [PATCH 04/11] Add logic for revoke-orphan and tidy to turn no-parent tokens into orphans --- vault/token_store.go | 39 +++++++++- vault/token_store_test.go | 150 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 1 deletion(-) diff --git a/vault/token_store.go b/vault/token_store.go index e40b1e036917..f094a28a3f52 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1169,8 +1169,29 @@ func (ts *TokenStore) revokeSalted(ctx context.Context, saltedId string) (ret er } } - // Clear the parent entry + // Mark all children token as orphan by removing + // their parent index, and clear the parent entry parentPath := parentPrefix + saltedId + "/" + children, err := ts.view.List(ctx, parentPath) + if err != nil { + return fmt.Errorf("failed to scan for children: %v", err) + } + for _, child := range children { + entry, err := ts.lookupSalted(ctx, child, true) + if err != nil { + return fmt.Errorf("failed to get child token: %v", err) + } + lock := locksutil.LockForKey(ts.tokenLocks, entry.ID) + lock.Lock() + + entry.Parent = "" + err = ts.store(ctx, entry) + if err != nil { + lock.Unlock() + return fmt.Errorf("failed to update child token: %v", err) + } + lock.Unlock() + } if err = logical.ClearView(ctx, ts.view.SubView(parentPath)); err != nil { return fmt.Errorf("failed to delete entry: %v", err) } @@ -1363,6 +1384,22 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data // since appropriate locks cannot be held with salted token IDs. // Also perform deletion if the parent doesn't exist any more. te, _ := ts.lookupSalted(ctx, child, true) + // If the child entry is not nil, but the parent doesn't exist, then turn + // that child token into an orphan token. Theres no deletion in this case. + if te != nil && exists == nil { + lock := locksutil.LockForKey(ts.tokenLocks, te.ID) + lock.Lock() + + te.Parent = "" + err = ts.store(ctx, te) + if err != nil { + tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to convert child token into an orphan token: %v", err)) + } + lock.Unlock() + continue + } + // Otherwise, if the entry doesn't exist, or if the parent doesn't exit go + // on with the delete onthe secondary index if te == nil || exists == nil { index := parentPrefix + parent + child ts.logger.Trace("token: deleting invalid secondary index", "index", index) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 57444dfd94ce..1ea63260c60f 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -1417,6 +1417,19 @@ func TestTokenStore_HandleRequest_RevokeOrphan(t *testing.T) { t.Fatalf("bad: %v", out) } + // Check that the parent entry is properly cleaned up + saltedID, err := ts.SaltID(context.Background(), "child") + if err != nil { + t.Fatal(err) + } + children, err := ts.view.List(context.Background(), parentPrefix+saltedID+"/") + if err != nil { + t.Fatalf("err: %v", err) + } + if len(children) != 0 { + t.Fatalf("bad: %v", children) + } + // Sub-child should exist! out, err = ts.Lookup(context.Background(), "sub-child") if err != nil { @@ -3514,6 +3527,143 @@ func TestTokenStore_HandleTidyCase1(t *testing.T) { } } +// Create a token, delete the token entry while leaking accessors, invoke tidy +// and check if the dangling accessor entry is getting removed +func TestTokenStore_HandleTidy_parentCleanup(t *testing.T) { + var resp *logical.Response + var err error + + _, ts, _, root := TestCoreWithTokenStore(t) + + // List the number of accessors. Since there is only root token + // present, the list operation should return only one key. + accessorListReq := &logical.Request{ + Operation: logical.ListOperation, + Path: "accessors", + ClientToken: root, + } + resp, err = ts.HandleRequest(context.Background(), accessorListReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + numberOfAccessors := len(resp.Data["keys"].([]string)) + if numberOfAccessors != 1 { + t.Fatalf("bad: number of accessors. Expected: 1, Actual: %d", numberOfAccessors) + } + + for i := 1; i <= 100; i++ { + // Create a token + tokenReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "create", + ClientToken: root, + Data: map[string]interface{}{ + "policies": []string{"policy1"}, + }, + } + resp, err = ts.HandleRequest(context.Background(), tokenReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + tut := resp.Auth.ClientToken + + // Create a child token + tokenReq = &logical.Request{ + Operation: logical.UpdateOperation, + Path: "create", + ClientToken: tut, + Data: map[string]interface{}{ + "policies": []string{"policy1"}, + }, + } + resp, err = ts.HandleRequest(context.Background(), tokenReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + // Creation of another token should end up with incrementing the number of + // accessors the storage + resp, err = ts.HandleRequest(context.Background(), accessorListReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + numberOfAccessors = len(resp.Data["keys"].([]string)) + if numberOfAccessors != (i*2)+1 { + t.Fatalf("bad: number of accessors. Expected: %d, Actual: %d", i+1, numberOfAccessors) + } + + // Revoke the token while leaking other items associated with the + // token. Do this by doing what revokeSalted used to do before it was + // fixed, i.e., by deleting the storage entry for token and its + // cubbyhole and by not deleting its secondary index, its accessor and + // associated leases. + + saltedTut, err := ts.SaltID(context.Background(), tut) + if err != nil { + t.Fatal(err) + } + _, err = ts.lookupSalted(context.Background(), saltedTut, true) + if err != nil { + t.Fatalf("failed to lookup token: %v", err) + } + + // Destroy the token index + path := lookupPrefix + saltedTut + if ts.view.Delete(context.Background(), path); err != nil { + t.Fatalf("failed to delete token entry: %v", err) + } + + // Destroy the cubby space + err = ts.destroyCubbyhole(context.Background(), saltedTut) + if err != nil { + t.Fatalf("failed to destroyCubbyhole: %v", err) + } + + // Leaking of accessor should have resulted in no change to the number + // of accessors + resp, err = ts.HandleRequest(context.Background(), accessorListReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + numberOfAccessors = len(resp.Data["keys"].([]string)) + if numberOfAccessors != (i*2)+1 { + t.Fatalf("bad: number of accessors. Expected: %d, Actual: %d", (i*2)+1, numberOfAccessors) + } + } + + tidyReq := &logical.Request{ + Path: "tidy", + Operation: logical.UpdateOperation, + ClientToken: root, + } + resp, err = ts.HandleRequest(context.Background(), tidyReq) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("resp: %#v", resp) + } + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + // Tidy should have removed all the dangling accessor entries + resp, err = ts.HandleRequest(context.Background(), accessorListReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%v", err, resp) + } + + // The number of accessors should be equal to number of valid child tokens + // (100) + the root token (1) + numberOfAccessors = len(resp.Data["keys"].([]string)) + if numberOfAccessors != 101 { + t.Fatalf("bad: number of accessors. Expected: 1, Actual: %d", numberOfAccessors) + } +} + func TestTokenStore_TidyLeaseRevocation(t *testing.T) { exp := mockExpiration(t) ts := exp.tokenStore From 73f804075eb601cb2b2b8c089941178a7cda1d40 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 23 Mar 2018 19:50:32 -0400 Subject: [PATCH 05/11] Add orphan check to test --- vault/token_store_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 1ea63260c60f..a4d5a9723270 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -3658,10 +3658,31 @@ func TestTokenStore_HandleTidy_parentCleanup(t *testing.T) { // The number of accessors should be equal to number of valid child tokens // (100) + the root token (1) - numberOfAccessors = len(resp.Data["keys"].([]string)) + keys := resp.Data["keys"].([]string) + numberOfAccessors = len(keys) if numberOfAccessors != 101 { t.Fatalf("bad: number of accessors. Expected: 1, Actual: %d", numberOfAccessors) } + + req := logical.TestRequest(t, logical.UpdateOperation, "lookup-accessor") + + for _, accessor := range keys { + req.Data = map[string]interface{}{ + "accessor": accessor, + } + + resp, err := ts.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("err: %s", err) + } + if resp.Data == nil { + t.Fatalf("response should contain data") + } + // These tokens should now be orphaned + if resp.Data["orphan"] != true { + t.Fatalf("token should be orphan") + } + } } func TestTokenStore_TidyLeaseRevocation(t *testing.T) { From d531227ec131a5fd679c05e7847e052e927f56c8 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 23 Mar 2018 20:00:27 -0400 Subject: [PATCH 06/11] Update test comments --- vault/token_store_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index a4d5a9723270..04be4c75bed1 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -3527,8 +3527,10 @@ func TestTokenStore_HandleTidyCase1(t *testing.T) { } } -// Create a token, delete the token entry while leaking accessors, invoke tidy -// and check if the dangling accessor entry is getting removed +// Create a set of tokens along with a child token for each of them, delete the +// token entry while leaking accessors, invoke tidy and check if the dangling +// accessor entry is getting removed and check if child tokens are still present +// and turned into orphan tokens. func TestTokenStore_HandleTidy_parentCleanup(t *testing.T) { var resp *logical.Response var err error From 6f2baf584279f5bb4c6696b7266c8333b1ddd6ec Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 26 Mar 2018 13:12:21 -0400 Subject: [PATCH 07/11] Fix TestTokenStore_Revoke_Orphan test --- vault/token_store_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 04be4c75bed1..75b840a6bbb5 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -758,6 +758,10 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + + // Unset the expected token parent's ID + ent2.Parent = "" + if !reflect.DeepEqual(out, ent2) { t.Fatalf("bad: expected:%#v\nactual:%#v", ent2, out) } From 6e60ccba0213280fee508ac31e1f443b555849ea Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 26 Mar 2018 14:44:54 -0400 Subject: [PATCH 08/11] Address feedback, add explicit delete when parent prefix is empty --- vault/token_store.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index f094a28a3f52..977dd5ab414c 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1369,7 +1369,7 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data originalChildrenCount := int64(len(children)) exists, _ := ts.lookupSalted(ctx, strings.TrimSuffix(parent, "/"), true) if exists == nil { - ts.logger.Trace("token: deleting invalid parent entry", "index", parentPrefix+parent) + ts.logger.Trace("token: deleting invalid parent prefix entry", "index", parentPrefix+parent) } var deletedChildrenCount int64 @@ -1398,8 +1398,8 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data lock.Unlock() continue } - // Otherwise, if the entry doesn't exist, or if the parent doesn't exit go - // on with the delete onthe secondary index + // Otherwise, if the entry doesn't exist, or if the parent doesn't exist go + // on with the delete on the secondary index if te == nil || exists == nil { index := parentPrefix + parent + child ts.logger.Trace("token: deleting invalid secondary index", "index", index) @@ -1415,6 +1415,15 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data deletedCountParentList += deletedChildrenCount // If we deleted all the children, then add that to our deleted parent entries count if originalChildrenCount == deletedChildrenCount { + // Make sure that we only delete if parent doesn't exist. + // N.B.: This is a no-op for the consul storage backend since the delete doesn't support + // recursive deletes, and delete on a path is a no-op. + if exists == nil { + err := ts.view.Delete(ctx, parentPrefix+parent) + if err != nil { + tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to delete parent prefix entry: %v", err)) + } + } deletedCountParentEntries++ } } From 5648624d0e8fd50414e0feddef6d3ab8192c6e98 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 26 Mar 2018 15:25:32 -0400 Subject: [PATCH 09/11] Revert explicit delete, add comment on why it's not done --- vault/token_store.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 977dd5ab414c..d80b7db4f33b 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1413,17 +1413,10 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data } // Add current children deleted count to the total count deletedCountParentList += deletedChildrenCount - // If we deleted all the children, then add that to our deleted parent entries count + // N.B.: We don't call delete on the parent prefix since physical.Backend.Delete + // implementations should be in charge of deleting empty prefixes. + // If we deleted all the children, then add that to our deleted parent entries count. if originalChildrenCount == deletedChildrenCount { - // Make sure that we only delete if parent doesn't exist. - // N.B.: This is a no-op for the consul storage backend since the delete doesn't support - // recursive deletes, and delete on a path is a no-op. - if exists == nil { - err := ts.view.Delete(ctx, parentPrefix+parent) - if err != nil { - tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to delete parent prefix entry: %v", err)) - } - } deletedCountParentEntries++ } } From 564de0dcda5e88ea82b2cf654b987d42e3a799db Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 26 Mar 2018 16:28:35 -0400 Subject: [PATCH 10/11] Update comment to indicate ok on marking token as orphan --- vault/token_store.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/vault/token_store.go b/vault/token_store.go index d80b7db4f33b..a047ba8e7c9e 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1170,7 +1170,13 @@ func (ts *TokenStore) revokeSalted(ctx context.Context, saltedId string) (ret er } // Mark all children token as orphan by removing - // their parent index, and clear the parent entry + // their parent index, and clear the parent entry. + // + // Marking the token as orphan is the correct behavior in here since + // revokeTreeSalted will ensure that they are deleted anyways if it's not an + // explicit call to orphan the child tokens (the delete occurs at the leaf + // node and uses parent prefix, not entry.Parent, to build the tree for + // traversal). parentPath := parentPrefix + saltedId + "/" children, err := ts.view.List(ctx, parentPath) if err != nil { From fbba0c387eebe464d6fe6273d8d7f167e8977602 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Mon, 26 Mar 2018 18:02:21 -0400 Subject: [PATCH 11/11] Fix test --- vault/token_store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 82ab1d5f719e..dee26268f619 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -3545,7 +3545,7 @@ func TestTokenStore_HandleTidy_parentCleanup(t *testing.T) { // present, the list operation should return only one key. accessorListReq := &logical.Request{ Operation: logical.ListOperation, - Path: "accessors", + Path: "accessors/", ClientToken: root, } resp, err = ts.HandleRequest(context.Background(), accessorListReq)