gsadmin/libredesk
1
0
Fork 0
mirror of https://github.com/abhinavxd/libredesk.git synced 2026-10-03 20:20:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #460 from abhinavxd/fix/otp-verify-email-binding

Browse Source 
bind OTP verification to the attested email so a sibling conversation can't rebind the contact to an unverified address
This commit is contained in:
Abhinav Raut
2026-08-02 10:25:32 +05:30
committed by GitHub
parent 85ed93a03e 642b5a1942
commit d32cb5a767
6 changed files with 270 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
go.mod
+3
View File
@@ -52,6 +52,8 @@ require (
require (
cloud.google.com/go/compute/metadata v0.3.0 // indirect
github.com/alicebob/gopher-json v0.0.0-20230218143504-906a9b012302 // indirect
github.com/alicebob/miniredis/v2 v2.32.1 // indirect
github.com/andybalholm/brotli v1.1.1 // indirect
github.com/cention-sany/utf7 v0.0.0-20170124080048-26cad61bd60a // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
@@ -78,6 +80,7 @@ require (
github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf // indirect
github.com/stretchr/objx v0.5.2 // indirect
github.com/valyala/bytebufferpool v1.0.0 // indirect
github.com/yuin/gopher-lua v1.1.1 // indirect
golang.org/x/image v0.41.0 // indirect
golang.org/x/sys v0.45.0 // indirect
golang.org/x/text v0.37.0 // indirect
go.sum
+5
View File
@@ -4,6 +4,7 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
github.com/abhinavxd/ssrfguard v0.1.0 h1:Ns/llAQ63uGFehxSvhCd+WGDKmBEEmIH+E1AW1CGgWM=
github.com/abhinavxd/ssrfguard v0.1.0/go.mod h1:eNVubb+m/r3KrKWYdG6hxzeAfj+t2ZmZss4V/x7D6Ws=
github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc=
github.com/alicebob/gopher-json v0.0.0-20230218143504-906a9b012302 h1:uvdUDbHQHO85qeSydJtItA4T55Pw6BtAejd0APRJOCE=
github.com/alicebob/gopher-json v0.0.0-20230218143504-906a9b012302/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc=
github.com/alicebob/miniredis/v2 v2.32.1 h1:Bz7CciDnYSaa0mX5xODh6GUITRSx+cVhjNoOR4JssBo=
@@ -20,6 +21,9 @@ github.com/cention-sany/utf7 v0.0.0-20170124080048-26cad61bd60a h1:MISbI8sU/PSK/
github.com/cention-sany/utf7 v0.0.0-20170124080048-26cad61bd60a/go.mod h1:2GxOXOlEPAMFPfp014mK1SWq8G8BN8o7/dfYqJrVGn8=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
github.com/coreos/go-oidc/v3 v3.11.0 h1:Ia3MxdwpSw702YW0xgfmP1GVCMA9aEFWu12XUZ3/OtI=
github.com/coreos/go-oidc/v3 v3.11.0/go.mod h1:gE3LgjOgFoHi9a4ce4/tJczr0Ai2/BoDhf0r5lltWI0=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -216,6 +220,7 @@ golang.org/x/oauth2 v0.27.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190204203706-41f3e6584952/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
internal/aiagent/otp.go
+24 -8
View File
@@ -27,8 +27,8 @@ const (
otpMaxConvSends = 6
)
// checkOTPScript matches the pending code and sets the verified flag on match, all in one step.
// Returns 1 on match, 0 on miss/expiry, -1 on corrupt data (key cleared).
// checkOTPScript matches the pending code and, on match, stores the attested email as the verified
// value, all in one step. Returns 1 on match, 0 on miss/expiry, -1 on corrupt data (key cleared).
var checkOTPScript = redis.NewScript(`
local raw = redis.call('GET', KEYS[1])
if not raw then
@@ -39,9 +39,13 @@ if not ok or type(p) ~= 'table' then
redis.call('DEL', KEYS[1])
return -1
end
if type(p.email) ~= 'string' or p.email == '' then
redis.call('DEL', KEYS[1])
return 0
end
if p.code == ARGV[1] then
redis.call('DEL', KEYS[1])
redis.call('SET', KEYS[2], '1', 'EX', ARGV[3])
redis.call('SET', KEYS[2], p.email, 'EX', ARGV[3])
return 1
end
p.attempts = (p.attempts or 0) + 1
@@ -65,20 +69,32 @@ return n
// pendingOTP is the JSON stored at otpPendingKeyPrefix while a code awaits entry.
type pendingOTP struct {
Code string `json:"code"`
Email string `json:"email"`
Attempts int `json:"attempts"`
}
func otpPendingKey(convUUID string) string { return otpPendingKeyPrefix + convUUID }
func otpVerifiedKey(convUUID string) string { return otpVerifiedKeyPrefix + convUUID }
func normalizeOTPEmail(email string) string {
return strings.ToLower(strings.TrimSpace(email))
}
// otpSendsKey scopes the send budget to one address so correcting a mistyped email gets a fresh one.
func otpSendsKey(convUUID, email string) string {
return otpSendsKeyPrefix + convUUID + ":" + strings.ToLower(strings.TrimSpace(email))
return otpSendsKeyPrefix + convUUID + ":" + normalizeOTPEmail(email)
}
func otpConvSendsKey(convUUID string) string { return otpSendsKeyPrefix + convUUID }
func (m *Manager) isConversationVerified(convUUID string) bool {
// isConversationVerified holds the invariant "verified == the contact's current email is the one
// proven by OTP": the verified value stores the attested address, and a contact email rebound
// after verification (from this or any sibling conversation) no longer matches.
func (m *Manager) isConversationVerified(convUUID, contactEmail string) bool {
email := normalizeOTPEmail(contactEmail)
if email == "" {
return false
}
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
v, err := m.redis.Get(ctx, otpVerifiedKey(convUUID)).Result()
@@ -88,7 +104,7 @@ func (m *Manager) isConversationVerified(convUUID string) bool {
}
return false
}
return v == "1"
return v == email
}
// clearConversationVerified drops the verified flag and any pending code so a changed email must be
@@ -121,10 +137,10 @@ func (m *Manager) incrOTPSends(convUUID, email string) error {
return incrOTPSendsScript.Run(ctx, m.redis, []string{otpConvSendsKey(convUUID)}, ttl).Err()
}
func (m *Manager) storePendingOTP(convUUID, code string) error {
func (m *Manager) storePendingOTP(convUUID, code, email string) error {
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
b, err := json.Marshal(pendingOTP{Code: code})
b, err := json.Marshal(pendingOTP{Code: code, Email: normalizeOTPEmail(email)})
if err != nil {
return err
}
internal/aiagent/otp_test.go
+236
View File
@@ -0,0 +1,236 @@
package aiagent
import (
"testing"
"time"
"github.com/alicebob/miniredis/v2"
"github.com/redis/go-redis/v9"
"github.com/zerodha/logf"
)
func newOTPTestManager(t *testing.T) (*Manager, *miniredis.Miniredis) {
t.Helper()
mr := miniredis.RunT(t)
lo := logf.New(logf.Opts{})
return &Manager{
lo: &lo,
redis: redis.NewClient(&redis.Options{Addr: mr.Addr()}),
}, mr
}
func TestIsConversationVerified(t *testing.T) {
m, mr := newOTPTestManager(t)
const conv = "conv-a"
if m.isConversationVerified(conv, "john@example.com") {
t.Error("no verified key must mean unverified")
}
mr.Set(otpVerifiedKey(conv), "john@example.com")
if !m.isConversationVerified(conv, "john@example.com") {
t.Error("matching email must be verified")
}
if !m.isConversationVerified(conv, " John@Example.COM ") {
t.Error("email comparison must be case- and whitespace-insensitive")
}
if m.isConversationVerified(conv, "victim@example.com") {
t.Error("a rebound contact email must not stay verified")
}
if m.isConversationVerified(conv, "") {
t.Error("empty contact email must be unverified")
}
if m.isConversationVerified("conv-b", "john@example.com") {
t.Error("another conversation must not inherit the verified flag")
}
// Value written by a build that stored a bare flag instead of the email.
mr.Set(otpVerifiedKey("conv-legacy"), "1")
if m.isConversationVerified("conv-legacy", "john@example.com") {
t.Error("legacy flag value must be unverified")
}
}
func TestCheckPendingOTP(t *testing.T) {
m, mr := newOTPTestManager(t)
const conv, email, code = "conv-a", "john@example.com", "123456"
ok, err := m.checkPendingOTP(conv, code)
if err != nil || ok {
t.Errorf("check without a pending code = (%v, %v), want (false, nil)", ok, err)
}
if err := m.storePendingOTP(conv, code, " John@Example.COM "); err != nil {
t.Fatal(err)
}
ok, err = m.checkPendingOTP(conv, "999999")
if err != nil || ok {
t.Errorf("wrong code = (%v, %v), want (false, nil)", ok, err)
}
ok, err = m.checkPendingOTP(conv, code)
if err != nil || !ok {
t.Fatalf("correct code = (%v, %v), want (true, nil)", ok, err)
}
if got, _ := mr.Get(otpVerifiedKey(conv)); got != email {
t.Errorf("verified value = %q, want the normalized attested email %q", got, email)
}
if mr.Exists(otpPendingKey(conv)) {
t.Error("pending key must be deleted on match")
}
if !m.isConversationVerified(conv, email) {
t.Error("conversation must be verified after a correct code")
}
ok, err = m.checkPendingOTP(conv, code)
if err != nil || ok {
t.Errorf("code replay = (%v, %v), want (false, nil)", ok, err)
}
}
func TestCheckPendingOTPAttemptCap(t *testing.T) {
m, mr := newOTPTestManager(t)
const conv, code = "conv-a", "123456"
if err := m.storePendingOTP(conv, code, "john@example.com"); err != nil {
t.Fatal(err)
}
for i := 0; i < otpMaxAttempts; i++ {
if ok, _ := m.checkPendingOTP(conv, "000000"); ok {
t.Fatal("wrong code must not verify")
}
}
if mr.Exists(otpPendingKey(conv)) {
t.Error("pending key must be deleted at the attempt cap")
}
if ok, _ := m.checkPendingOTP(conv, code); ok {
t.Error("correct code must not verify after the attempt cap")
}
}
func TestCheckPendingOTPLegacyAndCorruptPayloads(t *testing.T) {
m, mr := newOTPTestManager(t)
const conv = "conv-a"
// Pending record from a build that stored no attested email.
mr.Set(otpPendingKey(conv), `{"code":"123456"}`)
ok, err := m.checkPendingOTP(conv, "123456")
if err != nil || ok {
t.Errorf("legacy pending without email = (%v, %v), want (false, nil)", ok, err)
}
if mr.Exists(otpPendingKey(conv)) {
t.Error("legacy pending record must be deleted")
}
if mr.Exists(otpVerifiedKey(conv)) {
t.Error("legacy pending record must never set the verified key")
}
mr.Set(otpPendingKey(conv), "not-json")
if ok, err := m.checkPendingOTP(conv, "123456"); err == nil || ok {
t.Errorf("corrupt pending = (%v, %v), want (false, error)", ok, err)
}
if mr.Exists(otpPendingKey(conv)) {
t.Error("corrupt pending record must be deleted")
}
}
func TestClearConversationVerified(t *testing.T) {
m, mr := newOTPTestManager(t)
const conv, email = "conv-a", "john@example.com"
mr.Set(otpVerifiedKey(conv), email)
if err := m.storePendingOTP(conv, "123456", email); err != nil {
t.Fatal(err)
}
if err := m.clearConversationVerified(conv); err != nil {
t.Fatal(err)
}
if m.isConversationVerified(conv, email) {
t.Error("conversation must be unverified after clear")
}
if mr.Exists(otpPendingKey(conv)) {
t.Error("pending key must be deleted on clear")
}
}
// The cross-conversation rebind: verified on conversation A must not survive the shared
// contact's email being rewritten from a sibling conversation.
func TestVerifiedDoesNotSurviveEmailRebind(t *testing.T) {
m, _ := newOTPTestManager(t)
const convA, attacker, victim = "conv-a", "attacker@evil.com", "victim@example.com"
if err := m.storePendingOTP(convA, "123456", attacker); err != nil {
t.Fatal(err)
}
if ok, _ := m.checkPendingOTP(convA, "123456"); !ok {
t.Fatal("verification must succeed for the attested email")
}
if !m.isConversationVerified(convA, attacker) {
t.Fatal("conversation A must be verified for the attested email")
}
// Sibling conversation B rewrites the shared contact row; A's Redis keys are untouched.
if m.isConversationVerified(convA, victim) {
t.Error("conversation A must not stay verified once the contact email is rebound")
}
}
func TestVerifiedTTL(t *testing.T) {
m, mr := newOTPTestManager(t)
const conv, email = "conv-a", "john@example.com"
if err := m.storePendingOTP(conv, "123456", email); err != nil {
t.Fatal(err)
}
if got := mr.TTL(otpPendingKey(conv)); got != otpPendingTTL {
t.Errorf("pending TTL = %v, want %v", got, otpPendingTTL)
}
if ok, _ := m.checkPendingOTP(conv, "123456"); !ok {
t.Fatal("verification must succeed")
}
if got := mr.TTL(otpVerifiedKey(conv)); got != otpVerifiedTTL {
t.Errorf("verified TTL = %v, want %v", got, otpVerifiedTTL)
}
mr.FastForward(otpVerifiedTTL + time.Second)
if m.isConversationVerified(conv, email) {
t.Error("conversation must be unverified after the verified TTL")
}
}
func TestWrongAttemptKeepsPendingTTL(t *testing.T) {
m, mr := newOTPTestManager(t)
const conv = "conv-a"
if err := m.storePendingOTP(conv, "123456", "john@example.com"); err != nil {
t.Fatal(err)
}
mr.FastForward(otpPendingTTL / 2)
if ok, _ := m.checkPendingOTP(conv, "000000"); ok {
t.Fatal("wrong code must not verify")
}
if got := mr.TTL(otpPendingKey(conv)); got != otpPendingTTL/2 {
t.Errorf("pending TTL after a wrong attempt = %v, want %v", got, otpPendingTTL/2)
}
}
// Guards against the redis client failing open on connection errors.
func TestVerifiedFailsClosedOnRedisError(t *testing.T) {
m, mr := newOTPTestManager(t)
mr.Close()
if m.isConversationVerified("conv-a", "john@example.com") {
t.Error("a redis error must mean unverified")
}
}
func TestNormalizeOTPEmail(t *testing.T) {
cases := map[string]string{
" John@Example.COM ": "john@example.com",
"": "",
"a@b.c": "a@b.c",
}
for in, want := range cases {
if got := normalizeOTPEmail(in); got != want {
t.Errorf("normalizeOTPEmail(%q) = %q, want %q", in, got, want)
}
}
}
internal/aiagent/tools.go
+1 -1
View File
@@ -248,7 +248,7 @@ func (t *sendEmailVerificationTool) Execute(ctx context.Context, args string) (s
if err != nil {
return "", err
}
if err := t.m.storePendingOTP(t.conv.UUID, code); err != nil {
if err := t.m.storePendingOTP(t.conv.UUID, code, email); err != nil {
return "", err
}
body := t.m.i18n.Ts("ai.agent.verificationEmailBody", "code", code)
internal/aiagent/worker.go
+1 -1
View File
@@ -279,7 +279,7 @@ func (m *Manager) handle(ctx context.Context, convID int) {
if conv.InboxChannel != channelEmail && conv.Contact.Type == umodels.UserTypeContact {
return true
}
return m.isConversationVerified(conv.UUID)
return m.isConversationVerified(conv.UUID, conv.Contact.Email.String)
}
// Snapshot for the run-start registration decisions (one Redis read); tctx still gets the live
// closure so mid-turn verification is picked up per tool call.