fix: allow updating just the verified_at timestamp of addresses (#3880)

ory · Apr 17, 2024 · 696cc1b · 696cc1b
1 parent 31f77b8
commit 696cc1b
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 6 deletions.
diff --git a/identity/handler_test.go b/identity/handler_test.go
@@ -928,6 +928,98 @@ func TestHandler(t *testing.T) {
 		}
 	})
 
+	t.Run("case=PATCH should update verified_at timestamp", func(t *testing.T) {
+		for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} {
+			t.Run("endpoint="+name, func(t *testing.T) {
+				email := x.NewUUID().String() + "@ory.sh"
+				var cr identity.CreateIdentityBody
+				cr.SchemaID = "employee"
+				cr.Traits = []byte(`{"email":"` + email + `"}`)
+				res := send(t, ts, "POST", "/identities", http.StatusCreated, &cr)
+				assert.EqualValues(t, email, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
+				assert.EqualValues(t, email, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
+				assert.Falsef(t, res.Get("verifiable_addresses.0.verified").Bool(), "%s", res.Raw)
+				assert.Falsef(t, res.Get("verifiable_addresses.0.verified_at").Exists(), "%s", res.Raw)
+				identityID := res.Get("id").String()
+
+				// set to verified, should also update verified_at timestamp
+				patch1 := []patch{
+					{
+						"op":    "replace",
+						"path":  "/verifiable_addresses/0/verified",
+						"value": true,
+					},
+				}
+
+				now := time.Now()
+
+				res = send(t, ts, "PATCH", "/identities/"+identityID, http.StatusOK, &patch1)
+				assert.EqualValues(t, email, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
+				assert.EqualValues(t, email, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
+				assert.Truef(t, res.Get("verifiable_addresses.0.verified").Bool(), "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.updated_at").Time(), 5*time.Second, "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.verified_at").Time(), 5*time.Second, "%s", res.Raw)
+
+				res = get(t, ts, "/identities/"+identityID, http.StatusOK)
+				assert.EqualValues(t, email, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
+				assert.EqualValues(t, email, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
+				assert.Truef(t, res.Get("verifiable_addresses.0.verified").Bool(), "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.updated_at").Time(), 5*time.Second, "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.verified_at").Time(), 5*time.Second, "%s", res.Raw)
+
+				// update only verified_at timestamp
+				verifiedAt := time.Date(1999, 1, 7, 8, 23, 19, 0, time.UTC)
+				patch2 := []patch{
+					{
+						"op":    "replace",
+						"path":  "/verifiable_addresses/0/verified_at",
+						"value": verifiedAt.Format(time.RFC3339),
+					},
+				}
+
+				now = time.Now()
+				res = send(t, ts, "PATCH", "/identities/"+identityID, http.StatusOK, &patch2)
+				assert.EqualValues(t, email, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
+				assert.EqualValues(t, email, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
+				assert.Truef(t, res.Get("verifiable_addresses.0.verified").Bool(), "%s", res.Raw)
+				assert.Equalf(t, verifiedAt, res.Get("verifiable_addresses.0.verified_at").Time(), "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.updated_at").Time(), 5*time.Second, "%s", res.Raw)
+
+				res = get(t, ts, "/identities/"+identityID, http.StatusOK)
+				assert.EqualValues(t, email, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
+				assert.EqualValues(t, email, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
+				assert.Truef(t, res.Get("verifiable_addresses.0.verified").Bool(), "%s", res.Raw)
+				assert.Equalf(t, verifiedAt, res.Get("verifiable_addresses.0.verified_at").Time(), "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.updated_at").Time(), 5*time.Second, "%s", res.Raw)
+
+				// remove verified status
+				patch3 := []patch{
+					{
+						"op":    "replace",
+						"path":  "/verifiable_addresses/0/verified",
+						"value": false,
+					},
+				}
+
+				now = time.Now()
+
+				res = send(t, ts, "PATCH", "/identities/"+identityID, http.StatusOK, &patch3)
+				assert.EqualValues(t, email, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
+				assert.EqualValues(t, email, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
+				assert.Falsef(t, res.Get("verifiable_addresses.0.verified").Bool(), "%s", res.Raw)
+				assert.Falsef(t, res.Get("verifiable_addresses.0.verified_at").Exists(), "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.updated_at").Time(), 5*time.Second, "%s", res.Raw)
+
+				res = get(t, ts, "/identities/"+identityID, http.StatusOK)
+				assert.EqualValues(t, email, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
+				assert.EqualValues(t, email, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
+				assert.Falsef(t, res.Get("verifiable_addresses.0.verified").Bool(), "%s", res.Raw)
+				assert.Falsef(t, res.Get("verifiable_addresses.0.verified_at").Exists(), "%s", res.Raw)
+				assert.WithinDurationf(t, now, res.Get("verifiable_addresses.0.updated_at").Time(), 5*time.Second, "%s", res.Raw)
+			})
+		}
+	})
+
 	t.Run("case=PATCH update should not persist if schema id is invalid", func(t *testing.T) {
 		uuid := x.NewUUID().String()
 		i := &identity.Identity{Traits: identity.Traits(fmt.Sprintf(`{"subject":"%s"}`, uuid))}

diff --git a/identity/identity_verification.go b/identity/identity_verification.go
@@ -118,5 +118,5 @@ func (a VerifiableAddress) ValidateNID() error {
 
 // Hash returns a unique string representation for the recovery address.
 func (a VerifiableAddress) Hash() string {
-	return fmt.Sprintf("%v|%v|%v|%v|%v|%v", a.Value, a.Verified, a.Via, a.Status, a.IdentityID, a.NID)
+	return fmt.Sprintf("%v|%v|%v|%v|%v|%v|%v", a.Value, a.Verified, a.Via, a.Status, a.VerifiedAt, a.IdentityID, a.NID)
 }
diff --git a/identity/identity_verification_test.go b/identity/identity_verification_test.go
@@ -34,10 +34,10 @@ func TestNewVerifiableEmailAddress(t *testing.T) {
 }
 
 var tagsIgnoredForHashing = map[string]struct{}{
-	"id":          {},
-	"created_at":  {},
-	"updated_at":  {},
-	"verified_at": {},
+	"id":         {},
+	"created_at": {},
+	"updated_at": {},
+	// "verified_at": {}, // we explicitly want to be able to update just this field and nothing else
 }
 
 func reflectiveHash(record any) string {
@@ -102,5 +102,4 @@ func TestVerifiableAddress_Hash(t *testing.T) {
 			)
 		})
 	}
-
 }
diff --git a/persistence/sql/identity/persister_identity.go b/persistence/sql/identity/persister_identity.go
@@ -477,6 +477,9 @@ func (p *IdentityPersister) normalizeVerifiableAddresses(ctx context.Context, id
 		if v.Verified && (v.VerifiedAt == nil || time.Time(*v.VerifiedAt).IsZero()) {
 			v.VerifiedAt = pointerx.Ptr(sqlxx.NullTime(time.Now()))
 		}
+		if !v.Verified {
+			v.VerifiedAt = nil
+		}
 
 		id.VerifiableAddresses[k] = v
 	}