From b80aaf2732526d8904398f13412f276c9232ef90 Mon Sep 17 00:00:00 2001 From: jr-ni Date: Wed, 29 Apr 2026 01:38:41 -0700 Subject: [PATCH 1/2] feat(verification): back email/reset codes with Redis when available The verification helpers stored codes in an in-process map keyed by purpose+key, which breaks horizontal scaling: a code registered on one pod is invisible to another, so signup/password-reset round-trips that hit different replicas always fail. When RedisEnabled, store codes at `verify_code::` with the existing `VerificationValidMinutes` TTL so Redis ages them out automatically. Verification fails closed on Redis errors. The in-memory path is preserved for single-pod deployments and is unchanged. Adds a unit test covering the in-memory contract (registration, wrong code, purpose isolation, deletion). The Redis branch was validated live in a multi-pod deployment. Co-Authored-By: Claude Opus 4.7 (1M context) --- common/verification.go | 31 ++++++++++++++++++++++++++++++ common/verification_test.go | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 common/verification_test.go diff --git a/common/verification.go b/common/verification.go index 41fd3c943e7..aa970629e3c 100644 --- a/common/verification.go +++ b/common/verification.go @@ -1,10 +1,12 @@ package common import ( + "context" "strings" "sync" "time" + "github.com/go-redis/redis/v8" "github.com/google/uuid" ) @@ -18,6 +20,8 @@ const ( PasswordResetPurpose = "r" ) +const verificationRedisPrefix = "verify_code:" + var verificationMutex sync.Mutex var verificationMap map[string]verificationValue var verificationMapMaxSize = 10 @@ -32,7 +36,18 @@ func GenerateVerificationCode(length int) string { return code[:length] } +func verificationRedisKey(key, purpose string) string { + return verificationRedisPrefix + purpose + ":" + key +} + func RegisterVerificationCodeWithKey(key string, code string, purpose string) { + if RedisEnabled { + ttl := time.Duration(VerificationValidMinutes) * time.Minute + if err := RedisSet(verificationRedisKey(key, purpose), code, ttl); err != nil { + SysError("RegisterVerificationCodeWithKey: redis set failed: " + err.Error()) + } + return + } verificationMutex.Lock() defer verificationMutex.Unlock() verificationMap[purpose+key] = verificationValue{ @@ -45,6 +60,16 @@ func RegisterVerificationCodeWithKey(key string, code string, purpose string) { } func VerifyCodeWithKey(key string, code string, purpose string) bool { + if RedisEnabled { + stored, err := RedisGet(verificationRedisKey(key, purpose)) + if err != nil { + if err != redis.Nil { + SysError("VerifyCodeWithKey: redis get failed: " + err.Error()) + } + return false + } + return stored == code + } verificationMutex.Lock() defer verificationMutex.Unlock() value, okay := verificationMap[purpose+key] @@ -56,6 +81,12 @@ func VerifyCodeWithKey(key string, code string, purpose string) bool { } func DeleteKey(key string, purpose string) { + if RedisEnabled { + if err := RDB.Del(context.Background(), verificationRedisKey(key, purpose)).Err(); err != nil { + SysError("DeleteKey: redis del failed: " + err.Error()) + } + return + } verificationMutex.Lock() defer verificationMutex.Unlock() delete(verificationMap, purpose+key) diff --git a/common/verification_test.go b/common/verification_test.go new file mode 100644 index 00000000000..a423c261de3 --- /dev/null +++ b/common/verification_test.go @@ -0,0 +1,38 @@ +package common + +import "testing" + +// Exercise the in-process map fallback path that's used when Redis is +// disabled. The Redis path is covered by live testing. +func TestVerifyCodeWithKey_MemoryFallback(t *testing.T) { + prevRedis := RedisEnabled + RedisEnabled = false + t.Cleanup(func() { RedisEnabled = prevRedis }) + + const key = "user@example.com" + const code = "abc123" + + if VerifyCodeWithKey(key, code, EmailVerificationPurpose) { + t.Fatalf("expected false before code is registered") + } + + RegisterVerificationCodeWithKey(key, code, EmailVerificationPurpose) + + if !VerifyCodeWithKey(key, code, EmailVerificationPurpose) { + t.Fatalf("expected true after registration") + } + + if VerifyCodeWithKey(key, "wrong", EmailVerificationPurpose) { + t.Fatalf("expected false for wrong code") + } + + // Different purpose namespace must not collide. + if VerifyCodeWithKey(key, code, PasswordResetPurpose) { + t.Fatalf("purposes must be isolated") + } + + DeleteKey(key, EmailVerificationPurpose) + if VerifyCodeWithKey(key, code, EmailVerificationPurpose) { + t.Fatalf("expected false after DeleteKey") + } +} From 1b28bd96d82baf1f7d0002234540f90ef3fc6e9d Mon Sep 17 00:00:00 2001 From: jr-ni Date: Wed, 29 Apr 2026 01:57:12 -0700 Subject: [PATCH 2/2] review: address CodeRabbit feedback - Use crypto/subtle.ConstantTimeCompare for code equality check to avoid timing-attack surface, even though codes are random UUIDs. - Use the existing common.RedisDelKey wrapper in DeleteKey instead of calling RDB.Del directly, matching the rest of the file. Co-Authored-By: Claude Opus 4.7 (1M context) --- common/verification.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/verification.go b/common/verification.go index aa970629e3c..9424f239316 100644 --- a/common/verification.go +++ b/common/verification.go @@ -1,7 +1,7 @@ package common import ( - "context" + "crypto/subtle" "strings" "sync" "time" @@ -68,7 +68,7 @@ func VerifyCodeWithKey(key string, code string, purpose string) bool { } return false } - return stored == code + return subtle.ConstantTimeCompare([]byte(stored), []byte(code)) == 1 } verificationMutex.Lock() defer verificationMutex.Unlock() @@ -82,7 +82,7 @@ func VerifyCodeWithKey(key string, code string, purpose string) bool { func DeleteKey(key string, purpose string) { if RedisEnabled { - if err := RDB.Del(context.Background(), verificationRedisKey(key, purpose)).Err(); err != nil { + if err := RedisDelKey(verificationRedisKey(key, purpose)); err != nil { SysError("DeleteKey: redis del failed: " + err.Error()) } return