Skip to content

Commit a3b60da

Browse files
committed
fix(verify): one-shot consumption of email confirmation tokens
The email confirmation JWT issued by VerifyHandler.sendConfirmation was only protected by its 30-minute expiry: any party who could read the confirmation link (forwarded email, mail-gateway logs, mailbox archive) could redeem it independently within the window, creating a separate authenticated session for the same address. Add a VerifConfirmationStore interface (MarkUsed key, ttl -> bool) and a default in-memory implementation (sync.Mutex-protected map keyed by SHA-256 of the raw token). When VerifyHandler.ConfirmationStore is non-nil, LoginHandler records the token as consumed on first redemption and rejects replays with 403 "confirmation token already consumed". The store key is the SHA-256 of the raw token rather than a jti claim, so existing token fixtures (which carry no jti) are still de-dup'd correctly without changing the wire format. Wired through Service.AddVerifProvider in both auth.go and v2/auth.go. The store is selected once via sync.Once, with this precedence: 1. Opts.VerifConfirmationStore if non-nil (caller-supplied; required for multi-instance deployments — Redis or any shared KV). 2. provider.NewInMemoryVerifStore() default — fine for single instance, broken for LB-fronted multi-instance deployments where instance A's "used" set is unknown to instance B. Documented in README under "Confirmation token replay protection". Tests: * TestInMemoryVerifStore — store-level coverage (replay, expiry, key independence). * TestVerifyHandler_LoginAcceptConfirm_RejectsReplay — integration: same token redeemed twice -> 200 then 403. * TestService_AddVerifProvider_UsesCustomConfirmationStore — Opts injection works. * TestService_AddVerifProvider_DefaultsToInMemory — default install + sync.Once reuse on subsequent AddVerifProvider calls.
1 parent 32d0c6e commit a3b60da

13 files changed

Lines changed: 520 additions & 30 deletions

File tree

README.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,25 @@ The API for this provider:
296296

297297
The provider acts like any other, i.e. will be registered as `/auth/email/login`.
298298

299+
#### Confirmation token replay protection
300+
301+
Confirmation tokens are one-shot: a token redeemed once cannot be redeemed again within its TTL. This stops anyone with read access to the email link (forwarded mail, mail-gateway logs, mailbox archive) from independently consuming it after the user has.
302+
303+
By default the library installs an in-memory store on first call to `AddVerifProvider`. This is correct for **single-instance** deployments.
304+
305+
**Multi-instance deployments behind a load balancer MUST supply a shared backend.** With the default in-memory store, replay protection works only on the instance that originally consumed the token; an attacker who hits any other instance can still replay within the TTL. **This is silent: the request succeeds and the auth flow completes normally, with no log indicating the protection was bypassed.** Plug in Redis or any shared KV by setting `Opts.VerifConfirmationStore` to a value implementing `provider.VerifConfirmationStore`:
306+
307+
```go
308+
type VerifConfirmationStore interface {
309+
// MarkUsed records key as consumed and returns alreadyUsed=true if it was
310+
// already recorded. err signals a backend failure -- callers fail-closed
311+
// (reject the redemption) on non-nil err to avoid replay during outages.
312+
MarkUsed(key string, ttl time.Duration) (alreadyUsed bool, err error)
313+
}
314+
```
315+
316+
The store key is the SHA-256 of the raw confirmation token, so existing tokens issued before this protection landed are de-dup'd correctly without changing the wire format. Consumption is final: a transient downstream failure after the mark burns the token; the user must request a new confirmation email rather than retry the same link.
317+
299318
#### Email-as-identity caveat
300319

301320
The verify provider returns a local user id of the form `<provider>_<sha1(address)>`. The confirmation round-trip proves current control of the address at the moment of login; it does **not** prove stable+unique identity over time. The owner of an address can change without the address changing:

auth.go

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/url"
88
"regexp"
99
"strings"
10+
"sync"
1011
"time"
1112

1213
"github.com/go-pkgz/rest"
@@ -26,14 +27,16 @@ type Client struct {
2627

2728
// Service provides higher level wrapper allowing to construct everything and get back token middleware
2829
type Service struct {
29-
logger logger.L
30-
opts Opts
31-
jwtService *token.Service
32-
providers []provider.Service
33-
authMiddleware middleware.Authenticator
34-
avatarProxy *avatar.Proxy
35-
issuer string
36-
useGravatar bool
30+
logger logger.L
31+
opts Opts
32+
jwtService *token.Service
33+
providers []provider.Service
34+
authMiddleware middleware.Authenticator
35+
avatarProxy *avatar.Proxy
36+
issuer string
37+
useGravatar bool
38+
verifConfirmStore provider.VerifConfirmationStore
39+
verifConfirmStoreO sync.Once
3740
}
3841

3942
// Opts is a full set of all parameters to initialize Service
@@ -74,6 +77,15 @@ type Opts struct {
7477
AudSecrets bool // allow multiple secrets (secret per aud)
7578
Logger logger.L // logger interface, default is no logging at all
7679
RefreshCache middleware.RefreshCache // optional cache to keep refreshed tokens
80+
81+
// VerifConfirmationStore enforces one-shot consumption of email
82+
// confirmation tokens issued by the verify provider. The default
83+
// (nil) installs an in-memory store on first use of AddVerifProvider —
84+
// fine for single-instance deployments. Multi-instance deployments
85+
// MUST supply a shared backend (e.g. Redis) implementing
86+
// provider.VerifConfirmationStore, otherwise replay rejection works
87+
// only on the instance that consumed the token.
88+
VerifConfirmationStore provider.VerifConfirmationStore
7789
}
7890

7991
// NewService initializes everything
@@ -419,15 +431,23 @@ func (s *Service) AddDirectProviderWithUserIDFunc(name string, credChecker provi
419431

420432
// AddVerifProvider adds provider user's verification sent by sender
421433
func (s *Service) AddVerifProvider(name, msgTmpl string, sender provider.Sender) {
434+
s.verifConfirmStoreO.Do(func() {
435+
if s.opts.VerifConfirmationStore != nil {
436+
s.verifConfirmStore = s.opts.VerifConfirmationStore
437+
return
438+
}
439+
s.verifConfirmStore = provider.NewInMemoryVerifStore()
440+
})
422441
dh := provider.VerifyHandler{
423-
L: s.logger,
424-
ProviderName: name,
425-
Issuer: s.issuer,
426-
TokenService: s.jwtService,
427-
AvatarSaver: s.avatarProxy,
428-
Sender: sender,
429-
Template: msgTmpl,
430-
UseGravatar: s.useGravatar,
442+
L: s.logger,
443+
ProviderName: name,
444+
Issuer: s.issuer,
445+
TokenService: s.jwtService,
446+
AvatarSaver: s.avatarProxy,
447+
Sender: sender,
448+
Template: msgTmpl,
449+
UseGravatar: s.useGravatar,
450+
ConfirmationStore: s.verifConfirmStore,
431451
}
432452
s.addProvider(dh)
433453
}

auth_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,39 @@ func TestService_AddMicrosoftProvider(t *testing.T) {
117117
})
118118
}
119119

120+
func TestService_AddVerifProvider_UsesCustomConfirmationStore(t *testing.T) {
121+
custom := &countingVerifStore{}
122+
svc := NewService(Opts{
123+
SecretReader: token.SecretFunc(func(string) (string, error) { return "secret", nil }),
124+
URL: "http://127.0.0.1",
125+
Logger: logger.Std,
126+
VerifConfirmationStore: custom,
127+
})
128+
svc.AddVerifProvider("email", "{{.Token}}", provider.SenderFunc(func(string, string) error { return nil }))
129+
assert.Same(t, custom, svc.verifConfirmStore, "custom store from Opts must be used, not the in-memory default")
130+
}
131+
132+
func TestService_AddVerifProvider_DefaultsToInMemory(t *testing.T) {
133+
svc := NewService(Opts{
134+
SecretReader: token.SecretFunc(func(string) (string, error) { return "secret", nil }),
135+
URL: "http://127.0.0.1",
136+
Logger: logger.Std,
137+
})
138+
svc.AddVerifProvider("email", "{{.Token}}", provider.SenderFunc(func(string, string) error { return nil }))
139+
require.NotNil(t, svc.verifConfirmStore, "default store must be installed when Opts.VerifConfirmationStore is nil")
140+
// second call must reuse the same store (sync.Once guards against re-init)
141+
first := svc.verifConfirmStore
142+
svc.AddVerifProvider("email2", "{{.Token}}", provider.SenderFunc(func(string, string) error { return nil }))
143+
assert.Same(t, first, svc.verifConfirmStore, "subsequent AddVerifProvider calls must reuse the same store")
144+
}
145+
146+
type countingVerifStore struct{ calls int }
147+
148+
func (s *countingVerifStore) MarkUsed(string, time.Duration) (bool, error) {
149+
s.calls++
150+
return false, nil
151+
}
152+
120153
func TestService_AddAppleProvider(t *testing.T) {
121154

122155
options := Opts{

middleware/auth_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ func TestBasicAdminUserDoesNotLogPassword(t *testing.T) {
340340
const secret = "super-secret-attempted-passwd"
341341
var buf strings.Builder
342342
a := makeTestAuth(t)
343-
a.L = logger.Func(func(format string, args ...interface{}) {
343+
a.L = logger.Func(func(format string, args ...any) {
344344
fmt.Fprintf(&buf, format, args...)
345345
})
346346

provider/sender/email_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func TestEmail_New(t *testing.T) {
5959
func TestEmail_SendDoesNotLogBody(t *testing.T) {
6060
const secretBody = "Confirmation token: super-secret-magic-link-XYZ"
6161
var buf strings.Builder
62-
capturing := logger.Func(func(format string, args ...interface{}) {
62+
capturing := logger.Func(func(format string, args ...any) {
6363
fmt.Fprintf(&buf, format, args...)
6464
})
6565
p := EmailParams{Host: "127.0.0.2", Port: 25, From: "from@example.com",

provider/verify.go

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ package provider
33
import (
44
"bytes"
55
"crypto/sha1"
6+
"crypto/sha256"
7+
"encoding/hex"
68
"fmt"
79
"html/template"
810
"net/http"
911
"strings"
12+
"sync"
1013
"time"
1114

1215
"github.com/go-pkgz/rest"
@@ -40,6 +43,77 @@ type VerifyHandler struct {
4043
Sender Sender
4144
Template string
4245
UseGravatar bool
46+
47+
// ConfirmationStore enforces one-shot consumption of confirmation tokens.
48+
// When non-nil, a token cannot be redeemed twice within its TTL window.
49+
// Leave nil to keep the legacy behavior (token replayable until expiry).
50+
ConfirmationStore VerifConfirmationStore
51+
}
52+
53+
// VerifConfirmationStore tracks consumed confirmation tokens to prevent replay.
54+
// Implementations must be safe for concurrent use.
55+
type VerifConfirmationStore interface {
56+
// MarkUsed records key as consumed and returns alreadyUsed=true if it was
57+
// already recorded. ttl is an upper bound on how long the entry needs to
58+
// live; the implementation may evict earlier under memory pressure. err
59+
// signals a backend failure (network, disk, etc.) -- callers MUST treat a
60+
// non-nil err as fail-closed (reject the redemption) to avoid replay
61+
// during outages.
62+
MarkUsed(key string, ttl time.Duration) (alreadyUsed bool, err error)
63+
}
64+
65+
// NewInMemoryVerifStore returns a process-local default VerifConfirmationStore.
66+
// Suitable for single-instance deployments. Multi-instance deployments behind
67+
// a load balancer MUST supply a shared backend (e.g. Redis) -- otherwise an
68+
// attacker who lands on a different instance from the legitimate user can
69+
// replay the token there. The default's failure is silent: the request
70+
// completes normally and no log indicates the protection was bypassed.
71+
func NewInMemoryVerifStore() VerifConfirmationStore {
72+
return &inMemoryVerifStore{used: make(map[string]time.Time)}
73+
}
74+
75+
type inMemoryVerifStore struct {
76+
mu sync.Mutex
77+
used map[string]time.Time // key -> expiry
78+
insertCount int
79+
}
80+
81+
// inMemoryVerifStoreSweepEvery is the in-memory store's amortization cadence.
82+
// Walking the whole map on every redemption is O(n) under a single mutex,
83+
// which serializes the hot path. Sweeping every N inserts keeps the map size
84+
// bounded by ~N + (concurrent redemptions during the gap) without holding
85+
// the lock through a full walk on most calls.
86+
const inMemoryVerifStoreSweepEvery = 256
87+
88+
func (s *inMemoryVerifStore) MarkUsed(key string, ttl time.Duration) (bool, error) {
89+
s.mu.Lock()
90+
defer s.mu.Unlock()
91+
now := time.Now()
92+
if exp, ok := s.used[key]; ok && exp.After(now) {
93+
return true, nil
94+
}
95+
// amortized eviction: walk the map only every Nth insert, not on every
96+
// hot-path call. The lookup above already rejects unexpired duplicates,
97+
// so worst-case staleness is bounded by N inserts between sweeps.
98+
s.insertCount++
99+
if s.insertCount >= inMemoryVerifStoreSweepEvery {
100+
s.insertCount = 0
101+
for k, exp := range s.used {
102+
if !exp.After(now) {
103+
delete(s.used, k)
104+
}
105+
}
106+
}
107+
s.used[key] = now.Add(ttl)
108+
return false, nil
109+
}
110+
111+
// confirmationKey hashes the raw token so the store key length is bounded
112+
// regardless of token size, and so the in-memory map doesn't retain the
113+
// signed token itself.
114+
func confirmationKey(rawToken string) string {
115+
sum := sha256.Sum256([]byte(rawToken))
116+
return hex.EncodeToString(sum[:])
43117
}
44118

45119
// Sender defines interface to send emails
@@ -68,7 +142,13 @@ type VerifTokenService interface {
68142
func (e VerifyHandler) Name() string { return e.ProviderName }
69143

70144
// LoginHandler gets name and address from query, makes confirmation token and sends it to user.
71-
// In case if confirmation token presented in the query uses it to create auth token
145+
// In case if confirmation token presented in the query uses it to create auth token.
146+
//
147+
// Consumption is final when ConfirmationStore is configured: the token is
148+
// marked used before any further side effects (avatar fetch, token issuance),
149+
// so a transient downstream failure burns the token and the user must request
150+
// a new confirmation email rather than retry the same link. This trade-off
151+
// keeps the replay check atomic with the security boundary.
72152
func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
73153

74154
// GET /login?site=site&user=name&address=someone@example.com
@@ -91,6 +171,24 @@ func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
91171
return
92172
}
93173

174+
if e.ConfirmationStore != nil {
175+
ttl := time.Until(time.Unix(confClaims.ExpiresAt, 0))
176+
if ttl <= 0 {
177+
ttl = time.Minute
178+
}
179+
alreadyUsed, markErr := e.ConfirmationStore.MarkUsed(confirmationKey(tkn), ttl)
180+
if markErr != nil {
181+
// fail-closed: a backend outage must not let attackers replay
182+
// tokens. Log and reject; the user can request a fresh email.
183+
rest.SendErrorJSON(w, r, e.L, http.StatusForbidden, markErr, "confirmation token store unavailable")
184+
return
185+
}
186+
if alreadyUsed {
187+
rest.SendErrorJSON(w, r, e.L, http.StatusForbidden, fmt.Errorf("token already used"), "confirmation token already consumed")
188+
return
189+
}
190+
}
191+
94192
elems := strings.Split(confClaims.Handshake.ID, "::")
95193
if len(elems) != 2 {
96194
rest.SendErrorJSON(w, r, e.L, http.StatusBadRequest, fmt.Errorf("%s", confClaims.Handshake.ID), "invalid handshake token")

provider/verify_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"net/http/httptest"
77
"net/url"
88
"strings"
9+
"sync"
10+
"sync/atomic"
911
"testing"
1012
"time"
1113

@@ -132,6 +134,88 @@ func TestVerifyHandler_LoginAcceptConfirm(t *testing.T) {
132134
assert.Equal(t, true, claims.SessionOnly)
133135
}
134136

137+
func TestInMemoryVerifStore(t *testing.T) {
138+
mark := func(s VerifConfirmationStore, k string, ttl time.Duration) bool {
139+
used, err := s.MarkUsed(k, ttl)
140+
require.NoError(t, err)
141+
return used
142+
}
143+
144+
t.Run("first MarkUsed returns false, second returns true", func(t *testing.T) {
145+
s := NewInMemoryVerifStore()
146+
assert.False(t, mark(s, "k1", time.Hour), "first call must mark and return not-used")
147+
assert.True(t, mark(s, "k1", time.Hour), "second call must report already-used")
148+
})
149+
t.Run("expired entry is not considered used", func(t *testing.T) {
150+
s := NewInMemoryVerifStore()
151+
assert.False(t, mark(s, "k1", time.Nanosecond))
152+
time.Sleep(2 * time.Millisecond)
153+
assert.False(t, mark(s, "k1", time.Hour), "expired entry should be reusable")
154+
})
155+
t.Run("distinct keys are independent", func(t *testing.T) {
156+
s := NewInMemoryVerifStore()
157+
assert.False(t, mark(s, "k1", time.Hour))
158+
assert.False(t, mark(s, "k2", time.Hour))
159+
assert.True(t, mark(s, "k1", time.Hour))
160+
assert.True(t, mark(s, "k2", time.Hour))
161+
})
162+
t.Run("concurrent same-key redemption: exactly one succeeds", func(t *testing.T) {
163+
s := NewInMemoryVerifStore()
164+
const goroutines = 50
165+
var wg sync.WaitGroup
166+
successes := int32(0)
167+
errs := int32(0)
168+
wg.Add(goroutines)
169+
for i := 0; i < goroutines; i++ {
170+
go func() {
171+
defer wg.Done()
172+
used, err := s.MarkUsed("hot-key", time.Hour)
173+
if err != nil {
174+
atomic.AddInt32(&errs, 1)
175+
return
176+
}
177+
if !used {
178+
atomic.AddInt32(&successes, 1)
179+
}
180+
}()
181+
}
182+
wg.Wait()
183+
assert.EqualValues(t, 0, errs)
184+
assert.EqualValues(t, 1, successes, "exactly one redemption must observe alreadyUsed=false")
185+
})
186+
}
187+
188+
func TestVerifyHandler_LoginAcceptConfirm_RejectsReplay(t *testing.T) {
189+
e := VerifyHandler{
190+
ProviderName: "test",
191+
TokenService: token.NewService(token.Opts{
192+
SecretReader: token.SecretFunc(func(string) (string, error) { return "secret", nil }),
193+
TokenDuration: time.Hour,
194+
CookieDuration: time.Hour * 24 * 31,
195+
}),
196+
Issuer: "iss-test",
197+
L: logger.Std,
198+
ConfirmationStore: NewInMemoryVerifStore(),
199+
}
200+
201+
handler := http.HandlerFunc(e.LoginHandler)
202+
203+
// first use: success
204+
rr1 := httptest.NewRecorder()
205+
req1, err := http.NewRequest("GET", "/login?token="+testConfirmedToken, http.NoBody)
206+
require.NoError(t, err)
207+
handler.ServeHTTP(rr1, req1)
208+
require.Equal(t, 200, rr1.Code, "first consumption must succeed")
209+
210+
// second use: must be rejected
211+
rr2 := httptest.NewRecorder()
212+
req2, err := http.NewRequest("GET", "/login?token="+testConfirmedToken, http.NoBody)
213+
require.NoError(t, err)
214+
handler.ServeHTTP(rr2, req2)
215+
assert.Equal(t, 403, rr2.Code, "replay must be rejected")
216+
assert.Contains(t, rr2.Body.String(), "already")
217+
}
218+
135219
func TestVerifyHandler_LoginAcceptConfirmWithAvatar(t *testing.T) {
136220
e := VerifyHandler{
137221
ProviderName: "test",

0 commit comments

Comments
 (0)