Claus Lohmar
6f2ea67503
REVIEW-001 (M0) M2s: POST /access was a timing oracle — a hit did two synchronous SMTP sends, a miss one SELECT, so "is this email on record?" leaked (A3). Dispatch the notification through a Deps.Dispatch seam (default `go fn()`), detached from the request context (context.WithoutCancel, so audit values survive) and bounded by a 30s timeout. Tests inject a synchronous runner where they assert no-spam semantics, and a new test asserts the default is asynchronous with a blocking mailer. A panic in the detached goroutine is recovered: net/http no longer guards it, and a notification must never take the process down.
122 lines
3.4 KiB
Go
122 lines
3.4 KiB
Go
package recovery_test
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"path/filepath"
|
|
"strings"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/audit"
|
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/auth"
|
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/httpx"
|
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/mail"
|
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/recovery"
|
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/store"
|
|
"git.lohmar.co.uk/cclohmar/NextMyDay/internal/core/users"
|
|
)
|
|
|
|
// blockingMailer blocks every Send until release is closed, and signals when a
|
|
// send has begun.
|
|
type blockingMailer struct {
|
|
started chan struct{}
|
|
release chan struct{}
|
|
done chan struct{}
|
|
once sync.Once
|
|
}
|
|
|
|
func (m *blockingMailer) Send(_ context.Context, _ mail.Message) error {
|
|
select {
|
|
case m.started <- struct{}{}:
|
|
default:
|
|
}
|
|
<-m.release
|
|
m.once.Do(func() { close(m.done) })
|
|
return nil
|
|
}
|
|
|
|
// TestAccessNotificationIsAsynchronous covers M2s: POST /access must answer
|
|
// uniformly whether or not the email matched, so a HIT's notification is
|
|
// delivered OFF the request path. A synchronous implementation would block the
|
|
// response on the (slow) mailer and leak "is this email on record?".
|
|
func TestAccessNotificationIsAsynchronous(t *testing.T) {
|
|
ctx := context.Background()
|
|
|
|
db, err := store.Open(filepath.Join(t.TempDir(), "test.db"))
|
|
if err != nil {
|
|
t.Fatalf("store.Open: %v", err)
|
|
}
|
|
t.Cleanup(func() { _ = db.Close() })
|
|
if err := db.Migrate(); err != nil {
|
|
t.Fatalf("Migrate: %v", err)
|
|
}
|
|
|
|
aud := audit.New(db.DB)
|
|
us := users.New(db.DB, aud)
|
|
if _, err := us.Create(ctx, 0, "admin@example.com", "A", users.RoleAdmin); err != nil {
|
|
t.Fatalf("create user: %v", err)
|
|
}
|
|
|
|
sealer, err := auth.NewAEADSealer(bytes.Repeat([]byte{0x7a}, auth.KeySize))
|
|
if err != nil {
|
|
t.Fatalf("NewAEADSealer: %v", err)
|
|
}
|
|
core := auth.New(db.DB, aud, auth.WithSealer(sealer))
|
|
pre, err := httpx.NewPreSessionCSRF([]byte("flow-cookie-secret-0123456789abcdef"), false)
|
|
if err != nil {
|
|
t.Fatalf("NewPreSessionCSRF: %v", err)
|
|
}
|
|
|
|
m := &blockingMailer{
|
|
started: make(chan struct{}, 1),
|
|
release: make(chan struct{}),
|
|
done: make(chan struct{}),
|
|
}
|
|
// Dispatch is deliberately NOT injected: the default must be asynchronous.
|
|
a, err := recovery.New(recovery.Deps{
|
|
Requests: recovery.NewStore(db.DB, aud),
|
|
Users: us,
|
|
Identities: core.Identities(),
|
|
Mailer: m,
|
|
CSRF: pre,
|
|
Audit: aud,
|
|
BaseURL: "https://myday.example.com",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("recovery.New: %v", err)
|
|
}
|
|
|
|
form := url.Values{"email": {"admin@example.com"}}.Encode()
|
|
req := httptest.NewRequest("POST", "/access?do=auth&action=recover", strings.NewReader(form))
|
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
|
rec := httptest.NewRecorder()
|
|
|
|
returned := make(chan error, 1)
|
|
go func() { returned <- a.Request(rec, req) }()
|
|
|
|
select {
|
|
case err := <-returned:
|
|
if err != nil {
|
|
t.Fatalf("Request: %v", err)
|
|
}
|
|
case <-time.After(2 * time.Second):
|
|
close(m.release)
|
|
t.Fatal("POST /access blocked on the notification — it is still synchronous (M2s)")
|
|
}
|
|
if rec.Code != http.StatusOK {
|
|
t.Errorf("status = %d, want 200", rec.Code)
|
|
}
|
|
|
|
// The notification did run (off the request path); let it finish.
|
|
close(m.release)
|
|
select {
|
|
case <-m.done:
|
|
case <-time.After(2 * time.Second):
|
|
t.Fatal("the detached notification never ran")
|
|
}
|
|
}
|