diff --git a/changelog/13661.txt b/changelog/13661.txt new file mode 100644 index 000000000..99ea592df --- /dev/null +++ b/changelog/13661.txt @@ -0,0 +1,4 @@ +```release-note:improvement +auth/token: The `auth/token/revoke-accessor` endpoint is now idempotent and will +not error out if the token has already been revoked. +``` diff --git a/vault/logical_system.go b/vault/logical_system.go index 42e5056e7..783fce4c1 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -670,6 +670,9 @@ func (b *SystemBackend) handleCapabilitiesAccessor(ctx context.Context, req *log if err != nil { return nil, err } + if aEntry == nil { + return nil, &logical.StatusBadRequest{Err: "invalid accessor"} + } d.Raw["token"] = aEntry.TokenID return b.handleCapabilities(ctx, req, d) diff --git a/vault/token_store.go b/vault/token_store.go index e7faa50e2..346efec3c 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -747,6 +747,9 @@ func (ts *TokenStore) tokenStoreAccessorList(ctx context.Context, req *logical.R resp.AddWarning(fmt.Sprintf("Found an accessor entry that could not be successfully decoded; associated error is %q", err.Error())) continue } + if aEntry == nil { + continue + } if aEntry.TokenID == "" { resp.AddWarning(fmt.Sprintf("Found an accessor entry missing a token: %v", aEntry.AccessorID)) @@ -1741,12 +1744,12 @@ func (ts *TokenStore) handleCreateAgainstRole(ctx context.Context, req *logical. return ts.handleCreateCommon(ctx, req, d, false, roleEntry) } -func (ts *TokenStore) lookupByAccessor(ctx context.Context, id string, salted, tainted bool) (accessorEntry, error) { +func (ts *TokenStore) lookupByAccessor(ctx context.Context, id string, salted, tainted bool) (*accessorEntry, error) { var aEntry accessorEntry ns, err := namespace.FromContext(ctx) if err != nil { - return aEntry, err + return nil, err } lookupID := id @@ -1755,7 +1758,7 @@ func (ts *TokenStore) lookupByAccessor(ctx context.Context, id string, salted, t if nsID != "" { accessorNS, err := NamespaceByID(ctx, nsID, ts.core) if err != nil { - return aEntry, err + return nil, err } if accessorNS != nil { if accessorNS.ID != ns.ID { @@ -1773,16 +1776,16 @@ func (ts *TokenStore) lookupByAccessor(ctx context.Context, id string, salted, t lookupID, err = ts.SaltID(ctx, id) if err != nil { - return aEntry, err + return nil, err } } entry, err := ts.accessorView(ns).Get(ctx, lookupID) if err != nil { - return aEntry, fmt.Errorf("failed to read index using accessor: %w", err) + return nil, fmt.Errorf("failed to read index using accessor: %w", err) } if entry == nil { - return aEntry, &logical.StatusBadRequest{Err: "invalid accessor"} + return nil, nil } err = jsonutil.DecodeJSON(entry.Value, &aEntry) @@ -1790,7 +1793,7 @@ func (ts *TokenStore) lookupByAccessor(ctx context.Context, id string, salted, t if err != nil { te, err := ts.lookupInternal(ctx, string(entry.Value), false, tainted) if err != nil { - return accessorEntry{}, fmt.Errorf("failed to look up token using accessor index: %w", err) + return nil, fmt.Errorf("failed to look up token using accessor index: %w", err) } // It's hard to reason about what to do here if te is nil -- it may be // that the token was revoked async, or that it's an old accessor index @@ -1808,7 +1811,7 @@ func (ts *TokenStore) lookupByAccessor(ctx context.Context, id string, salted, t aEntry.NamespaceID = namespace.RootNamespaceID } - return aEntry, nil + return &aEntry, nil } // handleTidy handles the cleaning up of leaked accessor storage entries and @@ -1959,6 +1962,10 @@ func (ts *TokenStore) handleTidy(ctx context.Context, req *logical.Request, data tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to read the accessor index: %w", err)) continue } + if accessorEntry == nil { + tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to read the accessor index: invalid accessor")) + continue + } // A valid accessor storage entry should always have a token ID // in it. If not, it is an invalid accessor entry and needs to @@ -2098,6 +2105,9 @@ func (ts *TokenStore) handleUpdateLookupAccessor(ctx context.Context, req *logic if err != nil { return nil, err } + if aEntry == nil { + return nil, &logical.StatusBadRequest{Err: "invalid accessor"} + } // Prepare the field data required for a lookup call d := &framework.FieldData{ @@ -2140,6 +2150,9 @@ func (ts *TokenStore) handleUpdateRenewAccessor(ctx context.Context, req *logica if err != nil { return nil, err } + if aEntry == nil { + return nil, &logical.StatusBadRequest{Err: "invalid accessor"} + } // Prepare the field data required for a lookup call d := &framework.FieldData{ @@ -2190,6 +2203,11 @@ func (ts *TokenStore) handleUpdateRevokeAccessor(ctx context.Context, req *logic if err != nil { return nil, err } + if aEntry == nil { + resp := &logical.Response{} + resp.AddWarning("No token found with this accessor") + return resp, nil + } te, err := ts.Lookup(ctx, aEntry.TokenID) if err != nil { diff --git a/vault/token_store_test.go b/vault/token_store_test.go index a45b6c37b..8f0d36a0f 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -931,6 +931,16 @@ func TestTokenStore_HandleRequest_Renew_Revoke_Accessor(t *testing.T) { t.Fatalf("err: %s", err) } + // Revoking the token using the accessor should be idempotent since + // auth/token/revoke is + resp, err = ts.HandleRequest(namespace.RootContext(nil), req) + if err != nil { + t.Fatalf("err: %s", err) + } + if len(resp.Warnings) != 1 { + t.Fatalf("Was expecting 1 warning, got %d", len(resp.Warnings)) + } + time.Sleep(200 * time.Millisecond) out, err = ts.Lookup(namespace.RootContext(nil), "tokenid")