Make auth/token/revoke-accessor idempotent

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.
+```

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)

vault/token_store.go

@@ -747,6 +747,10 @@ 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 {
+			resp.AddWarning("Found an accessor entry that could not be successfully decoded: invalid accessor")
+			continue
+		}
 
 		if aEntry.TokenID == "" {
 			resp.AddWarning(fmt.Sprintf("Found an accessor entry missing a token: %v", aEntry.AccessorID))
@@ -1741,12 +1745,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 +1759,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,24 +1777,24 @@ 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)
 	// If we hit an error, assume it's a pre-struct straight token ID
 	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 +1812,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 +1963,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 +2106,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 +2151,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 +2204,9 @@ func (ts *TokenStore) handleUpdateRevokeAccessor(ctx context.Context, req *logic
 	if err != nil {
 		return nil, err
 	}
+	if aEntry == nil {
+		return nil, nil
+	}
 
 	te, err := ts.Lookup(ctx, aEntry.TokenID)
 	if err != nil {

vault/token_store_test.go

@@ -931,6 +931,13 @@ 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
+	_, err = ts.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
 	time.Sleep(200 * time.Millisecond)
 
 	out, err = ts.Lookup(namespace.RootContext(nil), "tokenid")