From d6b2a07961b25223ba85929f1f1da43d782bdd0c Mon Sep 17 00:00:00 2001
From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com>
Date: Thu, 8 Dec 2022 12:00:01 +0100
Subject: [PATCH 1/2] acl: canonicalize ACL Auth Method object

---
 nomad/acl_endpoint.go              |  3 ++-
 nomad/mock/acl.go                  |  1 +
 nomad/state/state_store_acl_sso.go |  1 +
 nomad/structs/acl.go               | 10 +++++++++
 nomad/structs/acl_test.go          | 33 ++++++++++++++++++++++++++++++
 5 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go
index f2de31bcf31..e06b414febd 100644
--- a/nomad/acl_endpoint.go
+++ b/nomad/acl_endpoint.go
@@ -1709,13 +1709,14 @@ func (a *ACL) UpsertAuthMethods(
 		return structs.NewErrRPCCoded(http.StatusBadRequest, "must specify as least one auth method")
 	}
 
-	// Validate each auth method, compute hash
+	// Validate each auth method, canonicalize, and compute hash
 	for idx, authMethod := range args.AuthMethods {
 		if err := authMethod.Validate(
 			a.srv.config.ACLTokenMinExpirationTTL,
 			a.srv.config.ACLTokenMaxExpirationTTL); err != nil {
 			return structs.NewErrRPCCodedf(http.StatusBadRequest, "auth method %d invalid: %v", idx, err)
 		}
+		authMethod.Canonicalize()
 		authMethod.SetHash()
 	}
 
diff --git a/nomad/mock/acl.go b/nomad/mock/acl.go
index 4810affec26..5f34f5c8652 100644
--- a/nomad/mock/acl.go
+++ b/nomad/mock/acl.go
@@ -243,5 +243,6 @@ func ACLAuthMethod() *structs.ACLAuthMethod {
 		ModifyIndex: 10,
 	}
 	method.SetHash()
+	method.Canonicalize()
 	return &method
 }
diff --git a/nomad/state/state_store_acl_sso.go b/nomad/state/state_store_acl_sso.go
index 302dd73f157..8c8016c3e5d 100644
--- a/nomad/state/state_store_acl_sso.go
+++ b/nomad/state/state_store_acl_sso.go
@@ -87,6 +87,7 @@ func (s *StateStore) upsertACLAuthMethodTxn(index uint64, txn *txn, method *stru
 		}
 
 		method.CreateIndex = existing.CreateIndex
+		method.CreateTime = existing.CreateTime
 		method.ModifyIndex = index
 	} else {
 		method.CreateIndex = index
diff --git a/nomad/structs/acl.go b/nomad/structs/acl.go
index cb194cfb7ac..23026941feb 100644
--- a/nomad/structs/acl.go
+++ b/nomad/structs/acl.go
@@ -710,6 +710,16 @@ func (a *ACLAuthMethod) Copy() *ACLAuthMethod {
 	return c
 }
 
+// Canonicalize performs basic canonicalization on the ACL auth method object.
+func (a *ACLAuthMethod) Canonicalize() {
+	t := time.Now().UTC()
+
+	if a.CreateTime.IsZero() {
+		a.CreateTime = t
+		a.ModifyTime = t
+	}
+}
+
 // Validate returns an error is the ACLAuthMethod is invalid.
 //
 // TODO revisit possible other validity conditions in the future
diff --git a/nomad/structs/acl_test.go b/nomad/structs/acl_test.go
index fee3043613e..fddc4b6dce7 100644
--- a/nomad/structs/acl_test.go
+++ b/nomad/structs/acl_test.go
@@ -1061,3 +1061,36 @@ func TestACLAuthMethodConfig_Copy(t *testing.T) {
 	amc3.AllowedRedirectURIs = []string{"new", "urls"}
 	must.NotEq(t, amc1, amc3)
 }
+
+func TestACLAuthMethod_Canonicalize(t *testing.T) {
+	now := time.Now().UTC()
+	tests := []struct {
+		name        string
+		inputMethod *ACLAuthMethod
+	}{
+		{
+			"no create time or modify time set",
+			&ACLAuthMethod{},
+		},
+		{
+			"create time set to now, modify time not set",
+			&ACLAuthMethod{CreateTime: now},
+		},
+		{
+			"both create time and modify time set",
+			&ACLAuthMethod{CreateTime: now, ModifyTime: now.Add(time.Hour)},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			existing := tt.inputMethod.Copy()
+			tt.inputMethod.Canonicalize()
+			if existing.CreateTime.IsZero() {
+				must.NotEq(t, time.Time{}, tt.inputMethod.CreateTime)
+			} else {
+				must.Eq(t, existing.CreateTime, tt.inputMethod.CreateTime)
+				must.Eq(t, existing.ModifyTime, tt.inputMethod.ModifyTime)
+			}
+		})
+	}
+}

From 3114bb5dead8b8434c5f04aeb7932162a0e096f6 Mon Sep 17 00:00:00 2001
From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com>
Date: Thu, 8 Dec 2022 14:03:20 +0100
Subject: [PATCH 2/2] James is right

---
 nomad/structs/acl.go      | 2 +-
 nomad/structs/acl_test.go | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/nomad/structs/acl.go b/nomad/structs/acl.go
index 23026941feb..ca5dbd09711 100644
--- a/nomad/structs/acl.go
+++ b/nomad/structs/acl.go
@@ -716,8 +716,8 @@ func (a *ACLAuthMethod) Canonicalize() {
 
 	if a.CreateTime.IsZero() {
 		a.CreateTime = t
-		a.ModifyTime = t
 	}
+	a.ModifyTime = t
 }
 
 // Validate returns an error is the ACLAuthMethod is invalid.
diff --git a/nomad/structs/acl_test.go b/nomad/structs/acl_test.go
index fddc4b6dce7..dc7a12517b1 100644
--- a/nomad/structs/acl_test.go
+++ b/nomad/structs/acl_test.go
@@ -1089,7 +1089,9 @@ func TestACLAuthMethod_Canonicalize(t *testing.T) {
 				must.NotEq(t, time.Time{}, tt.inputMethod.CreateTime)
 			} else {
 				must.Eq(t, existing.CreateTime, tt.inputMethod.CreateTime)
-				must.Eq(t, existing.ModifyTime, tt.inputMethod.ModifyTime)
+			}
+			if existing.ModifyTime.IsZero() {
+				must.NotEq(t, time.Time{}, tt.inputMethod.ModifyTime)
 			}
 		})
 	}