Skip to content

Commit e92d7d1

Browse files
fix(users): normalize username in user.deleted publish
Align event payload with username index normalization, document best-effort publish tradeoff, add publishUserDeletedEvent test, and note x/crypto bump. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Andres Tobon <andrest2455@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent e5f41c3 commit e92d7d1

3 files changed

Lines changed: 64 additions & 6 deletions

File tree

cmd/lfx-v1-sync-helper/handlers_users.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,12 @@ var publishUserDeletedEventFn = publishUserDeletedEvent
6262
// key segment safe for NATS KV. Order: TrimSpace → ToLower → NFC → RawURLEncoding.
6363
// NFC unifies decomposed/precomposed Unicode (e.g. n\u0303 ≡ ñ) without semantic
6464
// transposition. RawURLEncoding (no padding) keeps keys opaque and short.
65+
func normalizeKVSegment(s string) string {
66+
return norm.NFC.String(strings.ToLower(strings.TrimSpace(s)))
67+
}
68+
6569
func toKVKey(s string) string {
66-
s = norm.NFC.String(strings.ToLower(strings.TrimSpace(s)))
70+
s = normalizeKVSegment(s)
6771
if s == "" {
6872
return ""
6973
}
@@ -151,8 +155,8 @@ func handleMergedUserDelete(ctx context.Context, key, userSfid string, v1Data ma
151155
// handleAlternateEmailDelete cleans those up individually — but if the user is deleted
152156
// without its alternate emails being deleted first, those entries will be orphaned.
153157

154-
if username != "" {
155-
publishUserDeletedEventFn(ctx, key, username)
158+
if normalizedUsername := normalizeKVSegment(username); normalizedUsername != "" {
159+
publishUserDeletedEventFn(ctx, key, normalizedUsername)
156160
}
157161

158162
return false
@@ -168,15 +172,21 @@ type userDeletedEvent struct {
168172
const v1SyncHelperUserDeletedSubject = "lfx.v1-sync-helper.user.deleted"
169173

170174
// publishUserDeletedEvent publishes a user-deleted NATS event. Best-effort: publish
171-
// errors are logged and do not affect the delete handler's return value.
175+
// errors are logged and do not affect the delete handler's return value (the JetStream
176+
// KV delete is already ACKed). A failed publish can leave username PII in v2 settings
177+
// until a manual re-sync; scrub subscribers treat the event as idempotent.
178+
var natsPublishBytesFn = func(subject string, data []byte) error {
179+
return natsConn.Publish(subject, data)
180+
}
181+
172182
func publishUserDeletedEvent(ctx context.Context, key, username string) {
173183
payload, err := json.Marshal(userDeletedEvent{Username: username})
174184
if err != nil {
175185
logger.With(errKey, err, "key", key).
176186
ErrorContext(ctx, "failed to marshal user-deleted event; committee username scrub skipped")
177187
return
178188
}
179-
if err := natsConn.Publish(v1SyncHelperUserDeletedSubject, payload); err != nil {
189+
if err := natsPublishBytesFn(v1SyncHelperUserDeletedSubject, payload); err != nil {
180190
logger.With(errKey, err, "key", key).
181191
ErrorContext(ctx, "failed to publish user-deleted event; committee username scrub skipped")
182192
return

cmd/lfx-v1-sync-helper/handlers_users_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package main
55

66
import (
77
"context"
8+
"encoding/json"
89
"errors"
910
"fmt"
1011
"io"
@@ -345,6 +346,15 @@ func TestHandleMergedUserDeleteScrub(t *testing.T) {
345346
wantPublished bool
346347
wantUsername string
347348
}{
349+
{
350+
name: "username present → publish normalized event",
351+
v1Data: map[string]any{
352+
"sfid": userSfid,
353+
"username__c": " Alice ",
354+
},
355+
wantPublished: true,
356+
wantUsername: "alice",
357+
},
348358
{
349359
name: "username present → publish event",
350360
v1Data: map[string]any{
@@ -393,6 +403,44 @@ func TestHandleMergedUserDeleteScrub(t *testing.T) {
393403
}
394404
}
395405

406+
func TestPublishUserDeletedEvent(t *testing.T) {
407+
origLogger := logger
408+
origPublish := natsPublishBytesFn
409+
t.Cleanup(func() {
410+
logger = origLogger
411+
natsPublishBytesFn = origPublish
412+
})
413+
logger = slog.New(slog.NewTextHandler(io.Discard, nil))
414+
415+
t.Run("publishes normalized payload on subject", func(t *testing.T) {
416+
var gotSubject string
417+
var gotPayload userDeletedEvent
418+
natsPublishBytesFn = func(subject string, data []byte) error {
419+
gotSubject = subject
420+
if err := json.Unmarshal(data, &gotPayload); err != nil {
421+
t.Fatalf("unmarshal payload: %v", err)
422+
}
423+
return nil
424+
}
425+
426+
publishUserDeletedEvent(context.Background(), "test-key", "alice")
427+
428+
if gotSubject != v1SyncHelperUserDeletedSubject {
429+
t.Fatalf("subject = %q, want %q", gotSubject, v1SyncHelperUserDeletedSubject)
430+
}
431+
if gotPayload.Username != "alice" {
432+
t.Fatalf("username = %q, want alice", gotPayload.Username)
433+
}
434+
})
435+
436+
t.Run("publish error is swallowed", func(_ *testing.T) {
437+
natsPublishBytesFn = func(_ string, _ []byte) error {
438+
return errors.New("nats unavailable")
439+
}
440+
publishUserDeletedEvent(context.Background(), "test-key", "alice")
441+
})
442+
}
443+
396444
// TestExtractEmailIndex covers the field extraction for the alternate_email reindex phase.
397445
func TestExtractEmailIndex(t *testing.T) {
398446
tests := []struct {

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,6 @@ require (
5656
github.com/segmentio/asm v1.2.1 // indirect
5757
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
5858
go.devnw.com/structs v1.0.0 // indirect
59-
golang.org/x/crypto v0.54.0 // indirect
59+
golang.org/x/crypto v0.54.0 // indirect; go mod tidy — GO-2026-5932 has no upstream fix yet
6060
golang.org/x/sys v0.47.0 // indirect
6161
)

0 commit comments

Comments
 (0)