[#5964] refresh the token key on email change

CHANGELOG.md

@@ -10,6 +10,8 @@
 
 - Added `mailer.Message.InlineAttachments` field for attaching inline files to an email (_aka. `cid` links_).
 
+- Invalidate all record tokens when the auth record email is changed programmatically or by a superuser ([#5964](https://github.com/pocketbase/pocketbase/issues/5964)).
+
 - ⚠️ Removed the "dry submit" when executing the collections Create API rule
     (you can find more details why this change was introduced and how it could affect your app in https://github.com/pocketbase/pocketbase/discussions/6073).
     For most users it should be non-breaking change, BUT if you have Create API rules that uses self-references or view counters you may have to adjust them manually.

apis/record_auth_email_change_confirm.go

@@ -38,9 +38,8 @@ func recordConfirmEmailChange(e *core.RequestEvent) error {
 	event.NewEmail = newEmail
 
 	return e.App.OnRecordConfirmEmailChangeRequest().Trigger(event, func(e *core.RecordConfirmEmailChangeRequestEvent) error {
-		authRecord.Set(core.FieldNameEmail, e.NewEmail)
-		authRecord.Set(core.FieldNameVerified, true)
-		authRecord.RefreshTokenKey() // invalidate old tokens
+		e.Record.SetEmail(e.NewEmail)
+		e.Record.SetVerified(true)
 
 		if err := e.App.Save(e.Record); err != nil {
 			return firstApiError(err, e.BadRequestError("Failed to confirm email change.", err))

apis/record_auth_password_reset_confirm.go

@@ -47,12 +47,12 @@ func recordConfirmPasswordReset(e *core.RequestEvent) error {
 			}
 		}
 
-		err = form.app.Save(authRecord)
+		err = e.App.Save(authRecord)
 		if err != nil {
 			return firstApiError(err, e.BadRequestError("Failed to set new password.", err))
 		}
 
-		form.app.Store().Remove(getPasswordResetResendKey(authRecord))
+		e.App.Store().Remove(getPasswordResetResendKey(authRecord))
 
 		return e.NoContent(http.StatusNoContent)
 	})

apis/record_auth_password_reset_confirm_test.go

@@ -186,11 +186,20 @@ func TestRecordConfirmPasswordReset(t *testing.T) {
 					t.Fatal("Expected the user to be unverified")
 				}
 
+				oldTokenKey := user.TokenKey()
+
 				// manually change the email to check whether the verified state will be updated
 				user.SetEmail("test_update@example.com")
-				if err := app.Save(user); err != nil {
+				if err = app.Save(user); err != nil {
 					t.Fatalf("Failed to update user test email: %v", err)
 				}
+
+				// resave with the old token key since the email change above
+				// would change it and will make the password token invalid
+				user.SetTokenKey(oldTokenKey)
+				if err = app.Save(user); err != nil {
+					t.Fatalf("Failed to restore original user tokenKey: %v", err)
+				}
 			},
 			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
 				_, err := app.FindAuthRecordByToken(

apis/record_auth_with_oauth2_test.go

@@ -558,12 +558,21 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
 					t.Fatalf("Expected password %q to be valid", "1234567890")
 				}
 
+				oldTokenKey := user.TokenKey()
+
 				// manually unset the user email
 				user.SetEmail("")
-				if err := app.Save(user); err != nil {
+				if err = app.Save(user); err != nil {
 					t.Fatal(err)
 				}
 
+				// resave with the old token key since the email change above
+				// would change it and will make the password token invalid
+				user.SetTokenKey(oldTokenKey)
+				if err = app.Save(user); err != nil {
+					t.Fatalf("Failed to restore original user tokenKey: %v", err)
+				}
+
 				// register the test provider
 				auth.Providers["test"] = func() auth.Provider {
 					return &oauth2MockProvider{

core/record_model.go

@@ -1413,12 +1413,18 @@ func onRecordValidate(e *RecordEvent) error {
 
 func onRecordSaveExecute(e *RecordEvent) error {
 	if e.Record.Collection().IsAuth() {
-		// ensure that the token key is different on password change
-		old := e.Record.Original()
-		if !e.Record.IsNew() &&
-			old.TokenKey() == e.Record.TokenKey() &&
-			old.Get(FieldNamePassword) != e.Record.Get(FieldNamePassword) {
-			e.Record.RefreshTokenKey()
+		// ensure that the token key is regenerated on password change or email change
+		if !e.Record.IsNew() {
+			lastSavedRecord, err := e.App.FindRecordById(e.Record.Collection(), e.Record.Id)
+			if err != nil {
+				return err
+			}
+
+			if lastSavedRecord.TokenKey() == e.Record.TokenKey() &&
+				(lastSavedRecord.Get(FieldNamePassword) != e.Record.Get(FieldNamePassword) ||
+					lastSavedRecord.Email() != e.Record.Email()) {
+				e.Record.RefreshTokenKey()
+			}
 		}
 
 		// cross-check that the auth record id is unique across all auth collections.

core/record_model_test.go

@@ -1822,44 +1822,59 @@ func TestRecordSaveIdUpdateNoValidation(t *testing.T) {
 	}
 }
 
-func TestRecordSaveWithChangedPassword(t *testing.T) {
+func TestRecordSaveWithAutoTokenKeyRefresh(t *testing.T) {
 	t.Parallel()
 
 	app, _ := tests.NewTestApp()
 	defer app.Cleanup()
 
-	record, err := app.FindAuthRecordByEmail("nologin", "test@example.com")
-	if err != nil {
-		t.Fatal(err)
+	scenarios := []struct {
+		name           string
+		payload        map[string]any
+		expectedChange bool
+	}{
+		{
+			"no email or password change",
+			map[string]any{"name": "example"},
+			false,
+		},
+		{
+			"password change",
+			map[string]any{"password": "1234567890"},
+			true,
+		},
+		{
+			"email change",
+			map[string]any{"email": "test_update@example.com"},
+			true,
+		},
 	}
 
-	originalTokenKey := record.TokenKey()
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			record, err := app.FindFirstRecordByFilter("nologin", "1=1")
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	t.Run("no password change shouldn't change the tokenKey", func(t *testing.T) {
-		record.Set("name", "example")
+			originalTokenKey := record.TokenKey()
 
-		if err := app.Save(record); err != nil {
-			t.Fatal(err)
-		}
+			record.Load(s.payload)
 
-		tokenKey := record.TokenKey()
-		if tokenKey == "" || originalTokenKey != tokenKey {
-			t.Fatalf("Expected tokenKey to not change, got %q VS %q", originalTokenKey, tokenKey)
-		}
-	})
+			err = app.Save(record)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	t.Run("password change should change the tokenKey", func(t *testing.T) {
-		record.Set("password", "1234567890")
+			newTokenKey := record.TokenKey()
 
-		if err := app.Save(record); err != nil {
-			t.Fatal(err)
-		}
+			hasChange := originalTokenKey != newTokenKey
 
-		tokenKey := record.TokenKey()
-		if tokenKey == "" || originalTokenKey == tokenKey {
-			t.Fatalf("Expected tokenKey to change, got %q VS %q", originalTokenKey, tokenKey)
-		}
-	})
+			if hasChange != s.expectedChange {
+				t.Fatalf("Expected hasChange %v, got %v", s.expectedChange, hasChange)
+			}
+		})
+	}
 }
 
 func TestRecordDelete(t *testing.T) {