From cfa2cb140e93b272fb3783ae42c991535399f356 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Mon, 18 Jul 2022 14:19:29 -0400
Subject: [PATCH] fsm: one-time token expiration should be deterministic
 (#13737)

When applying a raft log to expire ACL tokens, we need to use a
timestamp provided by the leader so that the result is deterministic
across servers. Use leader's timestamp from RPC call
---
 .changelog/13737.txt            |  3 +++
 nomad/acl_endpoint.go           |  2 ++
 nomad/fsm.go                    |  2 +-
 nomad/state/state_store.go      |  8 ++++----
 nomad/state/state_store_test.go | 12 ++++++------
 nomad/structs/structs.go        |  1 +
 6 files changed, 17 insertions(+), 11 deletions(-)
 create mode 100644 .changelog/13737.txt

diff --git a/.changelog/13737.txt b/.changelog/13737.txt
new file mode 100644
index 000000000..3130f221c
--- /dev/null
+++ b/.changelog/13737.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+acl: Fixed a bug where the timestamp for expiring one-time tokens was not deterministic between servers
+```
diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go
index 23a9e9913..6c61ce46d 100644
--- a/nomad/acl_endpoint.go
+++ b/nomad/acl_endpoint.go
@@ -1013,6 +1013,8 @@ func (a *ACL) ExpireOneTimeTokens(args *structs.OneTimeTokenExpireRequest, reply
 		}
 	}
 
+	args.Timestamp = time.Now() // use the leader's timestamp
+
 	// Expire token via raft; because this is the only write in the RPC the
 	// caller can safely retry with the same token if the raft write fails
 	_, index, err := a.srv.raftApply(structs.OneTimeTokenExpireRequestType, args)
diff --git a/nomad/fsm.go b/nomad/fsm.go
index 0b4cc1305..4339dcf72 100644
--- a/nomad/fsm.go
+++ b/nomad/fsm.go
@@ -1229,7 +1229,7 @@ func (n *nomadFSM) applyOneTimeTokenExpire(msgType structs.MessageType, buf []by
 		panic(fmt.Errorf("failed to decode request: %v", err))
 	}
 
-	if err := n.state.ExpireOneTimeTokens(msgType, index); err != nil {
+	if err := n.state.ExpireOneTimeTokens(msgType, index, req.Timestamp); err != nil {
 		n.logger.Error("ExpireOneTimeTokens failed", "error", err)
 		return err
 	}
diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go
index 430bd6081..7069f66f4 100644
--- a/nomad/state/state_store.go
+++ b/nomad/state/state_store.go
@@ -5852,11 +5852,11 @@ func (s *StateStore) DeleteOneTimeTokens(msgType structs.MessageType, index uint
 }
 
 // ExpireOneTimeTokens deletes tokens that have expired
-func (s *StateStore) ExpireOneTimeTokens(msgType structs.MessageType, index uint64) error {
+func (s *StateStore) ExpireOneTimeTokens(msgType structs.MessageType, index uint64, timestamp time.Time) error {
 	txn := s.db.WriteTxnMsgT(msgType, index)
 	defer txn.Abort()
 
-	iter, err := s.oneTimeTokensExpiredTxn(txn, nil)
+	iter, err := s.oneTimeTokensExpiredTxn(txn, nil, timestamp)
 	if err != nil {
 		return err
 	}
@@ -5887,14 +5887,14 @@ func (s *StateStore) ExpireOneTimeTokens(msgType structs.MessageType, index uint
 }
 
 // oneTimeTokensExpiredTxn returns an iterator over all expired one-time tokens
-func (s *StateStore) oneTimeTokensExpiredTxn(txn *txn, ws memdb.WatchSet) (memdb.ResultIterator, error) {
+func (s *StateStore) oneTimeTokensExpiredTxn(txn *txn, ws memdb.WatchSet, timestamp time.Time) (memdb.ResultIterator, error) {
 	iter, err := txn.Get("one_time_token", "id")
 	if err != nil {
 		return nil, fmt.Errorf("one-time token lookup failed: %v", err)
 	}
 
 	ws.Add(iter.WatchCh())
-	iter = memdb.NewFilterIterator(iter, expiredOneTimeTokenFilter(time.Now()))
+	iter = memdb.NewFilterIterator(iter, expiredOneTimeTokenFilter(timestamp))
 	return iter, nil
 }
 
diff --git a/nomad/state/state_store_test.go b/nomad/state/state_store_test.go
index 5f3c9f4e1..6e37c4b55 100644
--- a/nomad/state/state_store_test.go
+++ b/nomad/state/state_store_test.go
@@ -8912,9 +8912,9 @@ func TestStateStore_OneTimeTokens(t *testing.T) {
 
 	// now verify expiration
 
-	getExpiredTokens := func() []*structs.OneTimeToken {
+	getExpiredTokens := func(now time.Time) []*structs.OneTimeToken {
 		txn := state.db.ReadTxn()
-		iter, err := state.oneTimeTokensExpiredTxn(txn, nil)
+		iter, err := state.oneTimeTokensExpiredTxn(txn, nil, now)
 		require.NoError(t, err)
 
 		results := []*structs.OneTimeToken{}
@@ -8930,7 +8930,7 @@ func TestStateStore_OneTimeTokens(t *testing.T) {
 		return results
 	}
 
-	results = getExpiredTokens()
+	results = getExpiredTokens(time.Now())
 	require.Len(t, results, 2)
 
 	// results aren't ordered
@@ -8942,10 +8942,10 @@ func TestStateStore_OneTimeTokens(t *testing.T) {
 
 	// clear the expired tokens and verify they're gone
 	index++
-	require.NoError(t,
-		state.ExpireOneTimeTokens(structs.MsgTypeTestSetup, index))
+	require.NoError(t, state.ExpireOneTimeTokens(
+		structs.MsgTypeTestSetup, index, time.Now()))
 
-	results = getExpiredTokens()
+	results = getExpiredTokens(time.Now())
 	require.Len(t, results, 0)
 
 	// query the unexpired token
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
index 800531e1b..4d6e9bc38 100644
--- a/nomad/structs/structs.go
+++ b/nomad/structs/structs.go
@@ -12077,6 +12077,7 @@ type OneTimeTokenDeleteRequest struct {
 
 // OneTimeTokenExpireRequest is a request to delete all expired one-time tokens
 type OneTimeTokenExpireRequest struct {
+	Timestamp time.Time
 	WriteRequest
 }