diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..f5fd6a1 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,81 @@ +# cryden — instructions for Claude Code + +Read this file at the start of every session. It is the whole +protocol. Do not deviate to save credits — deviating IS what wastes +them. + +## Startup — do exactly this, nothing more + +1. Read `docs/development/CURRENT-STATE.md`. +2. Read `docs/development/NEXT.md`. +3. Pick the **first unstarted item** in `NEXT.md`. That is your only + job this session. + +Do not read anything else first. Do not "review the codebase to get +oriented." Do not open other branches to "see what's there." The two +files above ARE your orientation — they exist specifically so you +never have to rebuild it from scratch. If a specific implementation +detail in `docs/development/CRYDEN-REVIEW.md` is genuinely needed for +the item you're building, read that one file for that one section — +not the whole thing, not the whole source tree. + +## Hard rules — no exceptions + +- **Never use the Task tool, subagents, or any background/parallel + worker.** One agent, one thread, one file at a time, foreground + only. If you're about to spin up a helper to "work on this in + parallel," stop — that's exactly the failure mode this file exists + to prevent. +- **Never re-read a file you already read this session**, unless you + just edited it and need to confirm the edit landed correctly. +- **Never re-verify or re-review a feature `NEXT.md`/`CURRENT-STATE.md` + says is already done.** Done means done. Trust the files. +- **Build exactly one item per session, completely, then stop.** Don't + chain into the next item in `NEXT.md` automatically. The human + re-invokes you for the next one — that's the checkpoint, not a + courtesy. +- **One git branch per item**, branched from the current tip of + whatever you're on (check with `git branch --show-current` once, + don't second-guess it after). Name it `feat/` or + `fix/`. +- **Never merge to `main`. Never push, even if you have credentials + configured.** The human reviews and pushes by hand, always. +- **Commit at every real step** (new interface, migration, wiring, + tests, docs, smoke test) — not one giant commit at the end. +- **Commit messages: 5 lines maximum.** One summary line, optionally + 2-4 lines of real "why," nothing more. No essay-length commits. +- **Don't ask the human questions mid-task.** If `NEXT.md`'s spec for + the item is ambiguous on some point, make the most reasonable + engineering decision yourself, write one line about it in + `PROGRESS.md`, and keep going. An unattended terminal run can't wait + on an answer — deciding and noting it is strictly better than + blocking. +- Every feature still gets: a `docs/testing/.md` manual test + guide, and a runnable in-memory smoke test at + `cmd/smoketest//main.go` printing ✓/✗ per step, including + negative cases. This hasn't changed from before. +- `gofmt -l` every changed file before each commit. Run `go build + ./...` and `go test ./...` if your environment has real network/ + toolchain access; if it doesn't, say so plainly in `PROGRESS.md` + rather than claiming untested code compiles. +- Standard placeholder identity in all examples/tests, unchanged: + `raymondproguy@dev.com` / `Tr0ubl3-Fr33!2026`. + +## Before you stop for the session + +1. Update `docs/development/CURRENT-STATE.md` — move the item you + built from "in progress"/"not started" to "done," name the branch. +2. Update `docs/development/NEXT.md` — remove the finished item (or + mark it done, whichever the file's own convention is by then), + leave the queue ready for the next invocation. +3. Append one short entry to `docs/development/PROGRESS.md` — date, + item, branch, one line on what got built, one line on any + assumption you made. +4. Commit those three doc updates together, one small commit, + `docs:` prefix. +5. Print a short summary to the terminal: item built, branch name, + what's next in the queue. Nothing else — no recap of the whole + session, no restated plan. + +That's the whole loop. Read state → build one thing → update state → +stop. diff --git a/auth/anomaly.go b/auth/anomaly.go new file mode 100644 index 0000000..8dfc764 --- /dev/null +++ b/auth/anomaly.go @@ -0,0 +1,188 @@ +package auth + +import ( + "context" + "strconv" + "time" + + "github.com/crydensync/cryden/v2/logger" + "github.com/crydensync/cryden/v2/security" + "github.com/crydensync/cryden/v2/store" +) + +// tokenReuseAuditScanLimit bounds how many of a user's most recent +// audit events are scanned for token-reuse history. Bounded on purpose: +// this runs on every successful login, so it must stay a single small +// indexed read. AuditStore has no by-user-AND-type query (ListByUser is +// per-user, SearchByType is system-wide), so the filtering happens here +// — which means a user with more than this many events since their last +// reuse event will not trip the signal. That's an acceptable miss for a +// report-only annotation, and the reuse event itself is still in the +// audit trail regardless. +const tokenReuseAuditScanLimit = 100 + +// detectLoginAnomalies evaluates one primary-authentication success +// against the account's recent history and records +// store.EventAnomalyDetected if anything looks unusual. +// +// It returns nothing. That is deliberate and not an oversight: this +// feature reports, it never decides. There is no error for a caller to +// branch on, no sentinel for "suspicious," and no way for a failing +// AnomalyStore to stop a legitimate login — every storage error below +// is logged and treated as "no evidence." A detector that can lock +// people out of their own accounts on a false positive (travel, a new +// browser, a shared office IP) is worse than no detector. +// +// Ordering matters: observations are gathered BEFORE this attempt is +// recorded, so the attempt can't appear in its own baseline and quietly +// mark its own IP familiar. +func detectLoginAnomalies( + ctx context.Context, + anomalies store.AnomalyStore, + sessions store.SessionStore, + audit store.AuditStore, + log logger.Logger, + thresholds security.AnomalyThresholds, + user store.User, + callerIP string, + userAgent string, +) { + if anomalies == nil { + return + } + + attempt := security.LoginAttemptContext{IP: callerIP, UserAgent: userAgent} + obs := gatherObservations(ctx, anomalies, sessions, audit, log, thresholds, user.ID, callerIP) + signals := thresholds.Evaluate(attempt, obs) + + if len(signals) > 0 { + metadata := map[string]string{"signals": security.JoinAnomalySignals(signals)} + // Only the counts behind signals that actually fired — a + // metadata blob of mostly-zero fields makes the ones that matter + // harder to spot in whatever the host app pipes this into. + for _, s := range signals { + switch s { + case security.SignalUserFailureVelocity: + metadata["user_failures"] = strconv.Itoa(obs.RecentUserFailures) + case security.SignalIPFailureVelocity: + metadata["ip_failures"] = strconv.Itoa(obs.RecentIPFailures) + case security.SignalTokenReuse: + metadata["token_reuse_events"] = strconv.Itoa(obs.RecentTokenReuseEvents) + case security.SignalConcurrentSessions: + metadata["active_sessions"] = strconv.Itoa(obs.ActiveSessions) + } + } + if err := audit.Record(ctx, store.AuditEvent{ + Type: store.EventAnomalyDetected, + UserID: user.ID, + IP: callerIP, + Metadata: metadata, + }); err != nil { + log.Error("anomaly: audit record failed", map[string]string{"error": err.Error(), "user_id": user.ID}) + } + log.Warn("anomaly: login flagged", map[string]string{ + "user_id": user.ID, + "ip": callerIP, + "signals": metadata["signals"], + }) + } + + RecordLoginAttempt(ctx, anomalies, log, store.LoginAttempt{ + UserID: user.ID, + IP: callerIP, + UserAgent: userAgent, + Outcome: store.OutcomeSuccess, + }) +} + +// gatherObservations turns four storage reads into the plain snapshot +// security.AnomalyThresholds.Evaluate judges. Each read degrades +// independently: a failure leaves that one field zero-valued rather +// than abandoning the whole pass, so a broken AnomalyStore doesn't also +// blind the session-count and token-reuse signals. +func gatherObservations( + ctx context.Context, + anomalies store.AnomalyStore, + sessions store.SessionStore, + audit store.AuditStore, + log logger.Logger, + thresholds security.AnomalyThresholds, + userID string, + callerIP string, +) security.AnomalyObservations { + var obs security.AnomalyObservations + now := time.Now() + + recent, err := anomalies.ListRecentSuccesses(ctx, userID, thresholds.HistorySize) + if err != nil { + log.Error("anomaly: recent-success lookup failed", map[string]string{"error": err.Error(), "user_id": userID}) + } else { + // HasLoginHistory stays false when there's nothing here, which + // suppresses new_ip/new_device for a first-ever login — there is + // no baseline yet to deviate from. It also, deliberately, keeps + // the signals quiet when the read failed above: inventing + // "everything is unfamiliar" out of a storage error would flag + // every login during an outage. + obs.HasLoginHistory = len(recent) > 0 + for _, a := range recent { + if a.IP != "" { + obs.KnownIPs = append(obs.KnownIPs, a.IP) + } + if a.UserAgent != "" { + obs.KnownUserAgents = append(obs.KnownUserAgents, a.UserAgent) + } + } + } + + since := now.Add(-thresholds.Window) + if count, err := anomalies.CountFailuresForUser(ctx, userID, since); err != nil { + log.Error("anomaly: per-user failure count failed", map[string]string{"error": err.Error(), "user_id": userID}) + } else { + obs.RecentUserFailures = count + } + + if count, err := anomalies.CountFailuresForIP(ctx, callerIP, since); err != nil { + log.Error("anomaly: per-IP failure count failed", map[string]string{"error": err.Error(), "ip": callerIP}) + } else { + obs.RecentIPFailures = count + } + + if sessions != nil { + if active, err := sessions.ListByUser(ctx, userID); err != nil { + log.Error("anomaly: active-session count failed", map[string]string{"error": err.Error(), "user_id": userID}) + } else { + // ListByUser already filters out revoked sessions in every + // implementation, so this is the active count, not a total. + obs.ActiveSessions = len(active) + } + } + + if audit != nil && thresholds.TokenReuseLookback > 0 { + events, err := audit.ListByUser(ctx, userID, tokenReuseAuditScanLimit) + if err != nil { + log.Error("anomaly: token-reuse lookup failed", map[string]string{"error": err.Error(), "user_id": userID}) + } else { + cutoff := now.Add(-thresholds.TokenReuseLookback) + for _, e := range events { + if e.Type == store.EventTokenReuseDetected && !e.CreatedAt.Before(cutoff) { + obs.RecentTokenReuseEvents++ + } + } + } + } + + return obs +} + +// RecordLoginAttempt stores one observation, best-effort. Exported so +// every primary-auth path can feed the same history — including the +// failure paths, which are what per-user and per-IP velocity are +// counted from. A nil store is a no-op, so callers never need to check. +func RecordLoginAttempt(ctx context.Context, anomalies store.AnomalyStore, log logger.Logger, attempt store.LoginAttempt) { + if anomalies == nil { + return + } + if err := anomalies.RecordAttempt(ctx, attempt); err != nil { + log.Error("anomaly: attempt record failed", map[string]string{"error": err.Error()}) + } +} diff --git a/auth/anomaly_test.go b/auth/anomaly_test.go new file mode 100644 index 0000000..181129d --- /dev/null +++ b/auth/anomaly_test.go @@ -0,0 +1,531 @@ +package auth + +import ( + "context" + "errors" + "strconv" + "testing" + "time" + + "github.com/crydensync/cryden/v2/security" + "github.com/crydensync/cryden/v2/store" + "github.com/crydensync/cryden/v2/store/memory" + "github.com/crydensync/cryden/v2/token" +) + +// anomalyTestThresholds keeps the numbers small so a test can reach a +// velocity threshold in a few calls instead of twenty. +var anomalyTestThresholds = security.AnomalyThresholds{ + Window: 15 * time.Minute, + HistorySize: 20, + UserFailureVelocity: 3, + IPFailureVelocity: 5, + MaxConcurrentSessions: 50, + TokenReuseLookback: 24 * time.Hour, +} + +type anomalyDeps struct { + users *memory.UserStore + sessions *memory.SessionStore + audit *memory.AuditStore + anomalies *memory.AnomalyStore + hasher security.Hasher + ids security.IDGenerator + refresh token.TokenGenerator + jwt *token.JWTIssuer + limiter security.RateLimiter + log testLogger +} + +func newAnomalyDeps(t *testing.T) *anomalyDeps { + t.Helper() + hasher, _ := security.NewBcryptHasher(4) + refreshGen, _ := token.NewCryptoRandTokenGenerator(32) + jwtIssuer, _ := token.NewJWTIssuer("test-secret", time.Minute) + return &anomalyDeps{ + users: memory.NewUserStore(), + sessions: memory.NewSessionStore(), + audit: memory.NewAuditStore(), + anomalies: memory.NewAnomalyStore(), + hasher: hasher, + ids: security.NewUUIDv7Generator(), + refresh: refreshGen, + jwt: jwtIssuer, + limiter: security.NewInMemoryRateLimiter(1000, time.Minute), + log: testLogger{}, + } +} + +// seedUser creates the standard placeholder identity. +func (d *anomalyDeps) seedUser(ctx context.Context, t *testing.T) store.User { + t.Helper() + hash, err := d.hasher.Hash("Tr0ubl3-Fr33!2026") + if err != nil { + t.Fatalf("hashing failed: %v", err) + } + u := storeUser("user-1", "raymondproguy@dev.com", hash) + if err := d.users.Create(ctx, u); err != nil { + t.Fatalf("seeding the user failed: %v", err) + } + return u +} + +// login runs a password login through the real Login path, with the +// anomaly store wired in unless anomalies is explicitly nil. +func (d *anomalyDeps) login(ctx context.Context, anomalies store.AnomalyStore, password, ip, agent string) (Tokens, error) { + return d.loginWith(ctx, anomalies, anomalyTestThresholds, password, ip, agent) +} + +func (d *anomalyDeps) loginWith(ctx context.Context, anomalies store.AnomalyStore, thresholds security.AnomalyThresholds, password, ip, agent string) (Tokens, error) { + return Login(ctx, d.users, d.sessions, nil, nil, nil, anomalies, + d.hasher, d.ids, d.refresh, d.jwt, nil, d.limiter, d.audit, d.log, + "raymondproguy@dev.com", password, ip, agent, 100, time.Minute, thresholds) +} + +func countEvents(t *testing.T, audit *memory.AuditStore, userID string, typ store.AuditEventType) []store.AuditEvent { + t.Helper() + events, err := audit.ListByUser(context.Background(), userID, 100) + if err != nil { + t.Fatalf("listing audit events failed: %v", err) + } + var out []store.AuditEvent + for _, e := range events { + if e.Type == typ { + out = append(out, e) + } + } + return out +} + +// The feature is off until a store is injected, exactly like TOTP and +// recovery codes. A nil AnomalyStore must change nothing at all. +func TestDetectLoginAnomalies_NilStoreIsANoOp(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + if _, err := d.login(ctx, nil, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("login with detection off failed: %v", err) + } + if _, err := d.login(ctx, nil, "wrong-password", "1.2.3.4", "test-agent"); err != ErrInvalidCredentials { + t.Fatalf("expected ErrInvalidCredentials, got %v", err) + } + + if events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected); len(events) != 0 { + t.Fatalf("detection off must record no anomaly events, got %d", len(events)) + } +} + +// A first login has no baseline, so it must be clean — and it must still +// be recorded, because it is the baseline for the next one. +func TestDetectLoginAnomalies_FirstLoginIsCleanButRecorded(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("first login failed: %v", err) + } + + if events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected); len(events) != 0 { + t.Fatalf("a first-ever login should not be flagged, got %v", events) + } + + recent, err := d.anomalies.ListRecentSuccesses(ctx, "user-1", 20) + if err != nil { + t.Fatalf("ListRecentSuccesses failed: %v", err) + } + if len(recent) != 1 { + t.Fatalf("expected the successful attempt to be recorded, got %d", len(recent)) + } + if recent[0].IP != "1.2.3.4" || recent[0].UserAgent != "test-agent" { + t.Fatalf("recorded attempt lost its context: %+v", recent[0]) + } +} + +// Second login from a different address and browser: both signals, on +// an otherwise entirely successful authentication. +func TestDetectLoginAnomalies_NewIPAndDeviceFlagWithoutBlocking(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("baseline login failed: %v", err) + } + + tokens, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "9.9.9.9", "other-agent") + if err != nil { + t.Fatalf("a flagged login must still succeed, got %v", err) + } + if tokens.AccessToken == "" || tokens.RefreshToken == "" { + t.Fatal("a flagged login must still issue tokens") + } + + events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected) + if len(events) != 1 { + t.Fatalf("expected exactly 1 anomaly event, got %d", len(events)) + } + if got := events[0].Metadata["signals"]; got != "new_ip,new_device" { + t.Fatalf("signals = %q, want %q", got, "new_ip,new_device") + } + if events[0].IP != "9.9.9.9" { + t.Fatalf("the event should carry the attempt's IP, got %q", events[0].IP) + } +} + +// A returning user on their known address and browser stays quiet. This +// is the case that decides whether the feature is usable at all: if a +// routine login flags, every login flags. +func TestDetectLoginAnomalies_FamiliarLoginStaysQuiet(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + for i := 0; i < 4; i++ { + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("login %d failed: %v", i, err) + } + } + + if events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected); len(events) != 0 { + t.Fatalf("repeat logins from a known IP and device must stay quiet, got %v", events) + } +} + +// Wrong-password attempts feed the velocity counts — that is the only +// reason the failure path records anything. +func TestDetectLoginAnomalies_FailuresFeedUserVelocity(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + // Baseline first, so the eventual success is judged against a known + // IP and device and only the velocity signal can fire. + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("baseline login failed: %v", err) + } + + for i := 0; i < anomalyTestThresholds.UserFailureVelocity; i++ { + if _, err := d.login(ctx, d.anomalies, "wrong-password", "1.2.3.4", "test-agent"); err != ErrInvalidCredentials { + t.Fatalf("attempt %d: expected ErrInvalidCredentials, got %v", i, err) + } + } + + count, err := d.anomalies.CountFailuresForUser(ctx, "user-1", time.Now().Add(-time.Minute)) + if err != nil { + t.Fatalf("CountFailuresForUser failed: %v", err) + } + if count != anomalyTestThresholds.UserFailureVelocity { + t.Fatalf("expected %d recorded failures, got %d", anomalyTestThresholds.UserFailureVelocity, count) + } + + // The password is right this time; the burst that preceded it is + // what gets surfaced. + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("the recovering login must still succeed: %v", err) + } + + events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected) + if len(events) != 1 { + t.Fatalf("expected 1 anomaly event, got %d", len(events)) + } + if got := events[0].Metadata["signals"]; got != "user_failure_velocity" { + t.Fatalf("signals = %q, want %q", got, "user_failure_velocity") + } + want := strconv.Itoa(anomalyTestThresholds.UserFailureVelocity) + if got := events[0].Metadata["user_failures"]; got != want { + t.Fatalf("user_failures = %q, want %q", got, want) + } +} + +// The per-IP count spans every account an address touched, including +// attempts against emails that resolve to no account at all — which is +// the shape of a spray, and the reason this signal is separate from the +// per-user one. +func TestDetectLoginAnomalies_FailuresFromOneIPCountAcrossAccounts(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("baseline login failed: %v", err) + } + + // Attempts against addresses with no account behind them. These + // carry no user ID, so they can only be counted per-IP. + for i := 0; i < 5; i++ { + _, err := Login(ctx, d.users, d.sessions, nil, nil, nil, d.anomalies, + d.hasher, d.ids, d.refresh, d.jwt, nil, d.limiter, d.audit, d.log, + "nobody@dev.com", "guess", "1.2.3.4", "test-agent", 100, time.Minute, anomalyTestThresholds) + if err != ErrInvalidCredentials { + t.Fatalf("attempt %d: expected ErrInvalidCredentials, got %v", i, err) + } + } + + since := time.Now().Add(-time.Minute) + if n, _ := d.anomalies.CountFailuresForIP(ctx, "1.2.3.4", since); n != 5 { + t.Fatalf("expected 5 failures from the IP, got %d", n) + } + // None of them are attributable to the real account. + if n, _ := d.anomalies.CountFailuresForUser(ctx, "user-1", since); n != 0 { + t.Fatalf("unknown-email failures must not attach to a real user, got %d", n) + } + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("the legitimate login must still succeed: %v", err) + } + + events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected) + if len(events) != 1 { + t.Fatalf("expected 1 anomaly event, got %d", len(events)) + } + if got := events[0].Metadata["signals"]; got != "ip_failure_velocity" { + t.Fatalf("signals = %q, want %q", got, "ip_failure_velocity") + } + if got := events[0].Metadata["ip_failures"]; got != "5" { + t.Fatalf("ip_failures = %q, want %q", got, "5") + } +} + +func TestDetectLoginAnomalies_ConcurrentSessionsFlagged(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + thresholds := anomalyTestThresholds + thresholds.MaxConcurrentSessions = 2 + + // Observations are read before this attempt's own session exists, so + // login N sees N-1 active sessions. Four logins is the first point + // where the count observed (3) exceeds a limit of 2. + for i := 0; i < 4; i++ { + if _, err := d.loginWith(ctx, d.anomalies, thresholds, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("login %d failed: %v", i, err) + } + } + + events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected) + if len(events) != 1 { + t.Fatalf("expected the fourth login to be the only one flagged, got %d", len(events)) + } + if got := events[0].Metadata["signals"]; got != "concurrent_sessions" { + t.Fatalf("signals = %q, want %q", got, "concurrent_sessions") + } + if got := events[0].Metadata["active_sessions"]; got != "3" { + t.Fatalf("active_sessions = %q, want %q", got, "3") + } +} + +// Refresh-token reuse already revokes the family when it happens. This +// signal makes a login arriving afterward visibly connected to it. +func TestDetectLoginAnomalies_TokenReuseHistoryFlagsLaterLogin(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("baseline login failed: %v", err) + } + if err := d.audit.Record(ctx, store.AuditEvent{ + Type: store.EventTokenReuseDetected, + UserID: "user-1", + IP: "9.9.9.9", + }); err != nil { + t.Fatalf("seeding the reuse event failed: %v", err) + } + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("login after a reuse event must still succeed: %v", err) + } + + events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected) + if len(events) != 1 { + t.Fatalf("expected 1 anomaly event, got %d", len(events)) + } + if got := events[0].Metadata["signals"]; got != "token_reuse" { + t.Fatalf("signals = %q, want %q", got, "token_reuse") + } + if got := events[0].Metadata["token_reuse_events"]; got != "1" { + t.Fatalf("token_reuse_events = %q, want %q", got, "1") + } +} + +// A reuse event older than TokenReuseLookback must stop counting, or one +// incident flags every login the account ever makes again. +func TestDetectLoginAnomalies_TokenReuseLookbackExpires(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("baseline login failed: %v", err) + } + _ = d.audit.Record(ctx, store.AuditEvent{Type: store.EventTokenReuseDetected, UserID: "user-1"}) + + thresholds := anomalyTestThresholds + // A lookback this short means the event just recorded is already + // outside it. + thresholds.TokenReuseLookback = time.Nanosecond + + if _, err := d.loginWith(ctx, d.anomalies, thresholds, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("login failed: %v", err) + } + + if events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected); len(events) != 0 { + t.Fatalf("an expired reuse event must not flag, got %v", events) + } +} + +// Ordering guarantee: if the current attempt were recorded before its +// own observations were gathered, its IP would already look familiar and +// new_ip could never fire. This is that invariant, stated directly. +func TestDetectLoginAnomalies_AttemptIsNotInItsOwnBaseline(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent"); err != nil { + t.Fatalf("baseline login failed: %v", err) + } + // Same device, new address: new_ip must fire even though this very + // attempt is about to be added to the history from that address. + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "9.9.9.9", "test-agent"); err != nil { + t.Fatalf("login failed: %v", err) + } + + events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected) + if len(events) != 1 || events[0].Metadata["signals"] != "new_ip" { + t.Fatalf("expected exactly one new_ip event, got %v", events) + } + + // And now that it is in the history, the same address is familiar. + if _, err := d.login(ctx, d.anomalies, "Tr0ubl3-Fr33!2026", "9.9.9.9", "test-agent"); err != nil { + t.Fatalf("repeat login failed: %v", err) + } + if events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected); len(events) != 1 { + t.Fatalf("the now-known address must not flag again, got %d events", len(events)) + } +} + +// Detection lives in completePrimaryAuth, the one tail every primary +// auth path reaches, so OAuth is covered by the same code as password +// login rather than by a second copy of it. +func TestDetectLoginAnomalies_CoversOAuthPath(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + oauth := memory.NewOAuthStore() + + loginOAuth := func(ip, agent string) { + t.Helper() + if _, err := LoginWithOAuth(ctx, d.users, oauth, d.sessions, nil, nil, nil, d.anomalies, + d.ids, d.refresh, d.jwt, nil, d.audit, d.log, + "google", "google-ext-id-1", "raymondproguy@dev.com", ip, agent, anomalyTestThresholds); err != nil { + t.Fatalf("OAuth login from %s failed: %v", ip, err) + } + } + + loginOAuth("1.2.3.4", "test-agent") + user, err := d.users.GetByEmail(ctx, "raymondproguy@dev.com") + if err != nil { + t.Fatalf("expected the OAuth login to create a user: %v", err) + } + if events := countEvents(t, d.audit, user.ID, store.EventAnomalyDetected); len(events) != 0 { + t.Fatalf("a first OAuth login should not be flagged, got %v", events) + } + + loginOAuth("9.9.9.9", "other-agent") + events := countEvents(t, d.audit, user.ID, store.EventAnomalyDetected) + if len(events) != 1 { + t.Fatalf("expected the second OAuth login to be flagged, got %d", len(events)) + } + if got := events[0].Metadata["signals"]; got != "new_ip,new_device" { + t.Fatalf("signals = %q, want %q", got, "new_ip,new_device") + } +} + +func TestDetectLoginAnomalies_CoversMagicLinkPath(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + verifications := memory.NewVerificationStore() + tokenGen, _ := token.NewCryptoRandTokenGenerator(32) + sender := &captureMagicLinkSender{} + + completeMagicLink := func(ip, agent string) { + t.Helper() + if err := RequestMagicLink(ctx, d.users, verifications, sender, tokenGen, d.ids, d.limiter, d.audit, d.log, + "raymondproguy@dev.com", ip); err != nil { + t.Fatalf("RequestMagicLink failed: %v", err) + } + if _, err := CompleteMagicLink(ctx, d.users, d.sessions, verifications, nil, nil, nil, d.anomalies, + d.ids, d.refresh, d.jwt, nil, d.audit, d.log, + sender.rawToken, ip, agent, anomalyTestThresholds); err != nil { + t.Fatalf("CompleteMagicLink from %s failed: %v", ip, err) + } + } + + completeMagicLink("1.2.3.4", "test-agent") + if events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected); len(events) != 0 { + t.Fatalf("a first magic-link login should not be flagged, got %v", events) + } + + completeMagicLink("9.9.9.9", "test-agent") + events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected) + if len(events) != 1 { + t.Fatalf("expected the second magic-link login to be flagged, got %d", len(events)) + } + if got := events[0].Metadata["signals"]; got != "new_ip" { + t.Fatalf("signals = %q, want %q", got, "new_ip") + } +} + +// failingAnomalyStore fails every operation, standing in for a database +// that has gone away mid-request. +type failingAnomalyStore struct{} + +func (failingAnomalyStore) RecordAttempt(ctx context.Context, attempt store.LoginAttempt) error { + return errors.New("anomaly store unavailable") +} + +func (failingAnomalyStore) ListRecentSuccesses(ctx context.Context, userID string, limit int) ([]store.LoginAttempt, error) { + return nil, errors.New("anomaly store unavailable") +} + +func (failingAnomalyStore) CountFailuresForUser(ctx context.Context, userID string, since time.Time) (int, error) { + return 0, errors.New("anomaly store unavailable") +} + +func (failingAnomalyStore) CountFailuresForIP(ctx context.Context, ip string, since time.Time) (int, error) { + return 0, errors.New("anomaly store unavailable") +} + +// A detector that can lock people out of their own accounts when its +// storage breaks is worse than no detector. Every read degrades to "no +// evidence" and the login proceeds. +func TestDetectLoginAnomalies_StorageFailureDoesNotBlockLogin(t *testing.T) { + ctx := context.Background() + d := newAnomalyDeps(t) + d.seedUser(ctx, t) + + tokens, err := d.login(ctx, failingAnomalyStore{}, "Tr0ubl3-Fr33!2026", "1.2.3.4", "test-agent") + if err != nil { + t.Fatalf("a broken anomaly store must not fail a valid login: %v", err) + } + if tokens.AccessToken == "" || tokens.RefreshToken == "" { + t.Fatal("expected tokens to be issued despite the storage failure") + } + + // And a failed read must not be reported as "everything is + // unfamiliar" — that would flag every login during an outage. + if events := countEvents(t, d.audit, "user-1", store.EventAnomalyDetected); len(events) != 0 { + t.Fatalf("a storage failure must not manufacture signals, got %v", events) + } + + // Wrong credentials still fail for the ordinary reason. + if _, err := d.login(ctx, failingAnomalyStore{}, "wrong-password", "1.2.3.4", "test-agent"); err != ErrInvalidCredentials { + t.Fatalf("expected ErrInvalidCredentials, got %v", err) + } +} + +var _ store.AnomalyStore = failingAnomalyStore{} diff --git a/cmd/smoketest/anomaly-detection/main.go b/cmd/smoketest/anomaly-detection/main.go new file mode 100644 index 0000000..9e674f5 --- /dev/null +++ b/cmd/smoketest/anomaly-detection/main.go @@ -0,0 +1,346 @@ +// Command anomaly-detection is a standalone, no-database smoke test for +// login anomaly detection: new-IP/new-device signals, per-user and +// per-IP failure velocity, token-reuse history, concurrent sessions, +// and — the property the whole feature rests on — that a flagged login +// still succeeds. Run with: +// +// go run ./cmd/smoketest/anomaly-detection +package main + +import ( + "context" + "fmt" + "os" + "time" + + "github.com/crydensync/cryden/v2" + "github.com/crydensync/cryden/v2/security" + "github.com/crydensync/cryden/v2/store" + "github.com/crydensync/cryden/v2/store/memory" +) + +const ( + email = "raymondproguy@dev.com" + password = "Tr0ubl3-Fr33!2026" + + knownIP = "1.2.3.4" + knownAgent = "cryden-smoketest/1.0" + strangeIP = "203.0.113.9" + otherAgent = "unknown-browser/9.9" + wrongPass = "not-the-password" + unknownMail = "nobody@dev.com" +) + +// Small numbers so a burst is a handful of calls, not twenty. +var thresholds = security.AnomalyThresholds{ + Window: 15 * time.Minute, + HistorySize: 20, + UserFailureVelocity: 3, + IPFailureVelocity: 5, + MaxConcurrentSessions: 50, + TokenReuseLookback: 24 * time.Hour, +} + +var failures int + +// rig is one isolated engine plus the two stores the checks read back +// from. Each scenario gets a fresh one so signal counts never bleed +// between them. +type rig struct { + engine *cryden.Engine + audit *memory.AuditStore + anomalies *memory.AnomalyStore + userID string +} + +func newRig(ctx context.Context, detectionOn bool) (*rig, error) { + return newRigWith(ctx, detectionOn, thresholds) +} + +func newRigWith(ctx context.Context, detectionOn bool, th security.AnomalyThresholds) (*rig, error) { + r := &rig{ + audit: memory.NewAuditStore(), + anomalies: memory.NewAnomalyStore(), + } + cfg := cryden.Config{ + JWTSecret: "smoketest-jwt-secret", + Users: memory.NewUserStore(), + Sessions: memory.NewSessionStore(), + Audit: r.audit, + AnomalyThresholds: th, + // High enough that lockout never fires first and masks a + // velocity signal — this smoke test is about detection, and + // lockout is a separate, already-shipped feature. + LockoutThreshold: 100, + } + if detectionOn { + cfg.Anomalies = r.anomalies + } + engine, err := cryden.New(cfg) + if err != nil { + return nil, err + } + r.engine = engine + + user, err := cryden.SignUp(ctx, engine, email, password, knownIP) + if err != nil { + return nil, err + } + r.userID = user.ID + return r, nil +} + +// login runs a real password login and reports whether it succeeded. +func (r *rig) login(ctx context.Context, ip, agent, pass string) error { + _, err := cryden.Login(ctx, r.engine, email, pass, ip, agent) + return err +} + +// signals returns the "signals" metadata of every anomaly event +// recorded so far, oldest first. +func (r *rig) signals(ctx context.Context) []string { + events, err := r.audit.ListByUser(ctx, r.userID, 200) + if err != nil { + fail(fmt.Sprintf("reading audit events: %v", err)) + return nil + } + var out []string + // ListByUser is newest-first; walk backwards for chronological order. + for i := len(events) - 1; i >= 0; i-- { + if events[i].Type == store.EventAnomalyDetected { + out = append(out, events[i].Metadata["signals"]) + } + } + return out +} + +func main() { + ctx := context.Background() + + newIPAndDevice(ctx) + userFailureVelocity(ctx) + ipFailureVelocity(ctx) + tokenReuse(ctx) + concurrentSessions(ctx) + detectionOff(ctx) + + fmt.Println() + if failures == 0 { + fmt.Println("ALL CHECKS PASSED") + return + } + fmt.Printf("%d CHECK(S) FAILED\n", failures) + os.Exit(1) +} + +func newIPAndDevice(ctx context.Context) { + fmt.Println("— new IP and new device") + r, err := newRig(ctx, true) + check("engine constructed with an AnomalyStore", err) + if r == nil { + return + } + + // A first-ever login has no baseline to deviate from. Flagging it + // would mean flagging every new account's first login. + check("first login succeeds", r.login(ctx, knownIP, knownAgent, password)) + expectSignals(ctx, r, "first login is not flagged") + + check("second login from the same IP and device succeeds", r.login(ctx, knownIP, knownAgent, password)) + expectSignals(ctx, r, "a familiar login stays quiet") + + // The load-bearing negative case: flagged, and still logged in. + check("login from an unknown IP and device succeeds anyway", + r.login(ctx, strangeIP, otherAgent, password)) + expectSignals(ctx, r, "unfamiliar IP and device are both flagged", "new_ip,new_device") + + // Known device, new address — travel, not a second party. + check("login from another new IP on the known device succeeds", + r.login(ctx, "198.51.100.7", knownAgent, password)) + expectSignals(ctx, r, "a known device on a new IP flags new_ip only", + "new_ip,new_device", "new_ip") + + // The address from the flagged login is now part of the baseline. + check("repeat login from the previously-unknown IP succeeds", + r.login(ctx, strangeIP, otherAgent, password)) + expectSignals(ctx, r, "a now-familiar IP and device stop flagging", + "new_ip,new_device", "new_ip") +} + +func userFailureVelocity(ctx context.Context) { + fmt.Println("\n— per-user failure velocity") + r, err := newRig(ctx, true) + check("engine constructed", err) + if r == nil { + return + } + + // Baseline first, so only the velocity signal can fire on the + // recovering login. + check("baseline login succeeds", r.login(ctx, knownIP, knownAgent, password)) + + for i := 0; i < thresholds.UserFailureVelocity; i++ { + checkExpectError(fmt.Sprintf("wrong password rejected (%d/%d)", i+1, thresholds.UserFailureVelocity), + r.login(ctx, knownIP, knownAgent, wrongPass)) + } + + count, err := r.anomalies.CountFailuresForUser(ctx, r.userID, time.Now().Add(-thresholds.Window)) + check("failed attempts are recorded for the account", err) + expectCount("recorded failures match the burst", count, thresholds.UserFailureVelocity) + + check("the correct password still works after the burst", + r.login(ctx, knownIP, knownAgent, password)) + expectSignals(ctx, r, "the recovering login is flagged for velocity", "user_failure_velocity") +} + +func ipFailureVelocity(ctx context.Context) { + fmt.Println("\n— per-IP failure velocity, across accounts") + r, err := newRig(ctx, true) + check("engine constructed", err) + if r == nil { + return + } + + check("baseline login succeeds", r.login(ctx, knownIP, knownAgent, password)) + + // Attempts against an address with no account behind it. These carry + // no user ID, so they can only be counted per-IP — which is the + // shape of a spray across many accounts from one source. + for i := 0; i < thresholds.IPFailureVelocity; i++ { + _, err := cryden.Login(ctx, r.engine, unknownMail, wrongPass, knownIP, knownAgent) + checkExpectError(fmt.Sprintf("attempt against a nonexistent account rejected (%d/%d)", + i+1, thresholds.IPFailureVelocity), err) + } + + since := time.Now().Add(-thresholds.Window) + ipCount, err := r.anomalies.CountFailuresForIP(ctx, knownIP, since) + check("failures are counted for the source IP", err) + expectCount("per-IP count spans accounts that do not exist", ipCount, thresholds.IPFailureVelocity) + + userCount, err := r.anomalies.CountFailuresForUser(ctx, r.userID, since) + check("per-user count read back", err) + expectCount("unknown-email failures are not attributed to a real user", userCount, 0) + + check("the real account can still log in", r.login(ctx, knownIP, knownAgent, password)) + expectSignals(ctx, r, "the login from the noisy IP is flagged", "ip_failure_velocity") +} + +func tokenReuse(ctx context.Context) { + fmt.Println("\n— token-reuse history") + r, err := newRig(ctx, true) + check("engine constructed", err) + if r == nil { + return + } + + check("baseline login succeeds", r.login(ctx, knownIP, knownAgent, password)) + + // Refresh-token reuse already revoked the family when it happened. + // This records that it did, the way RefreshToken would have. + err = r.audit.Record(ctx, store.AuditEvent{ + Type: store.EventTokenReuseDetected, + UserID: r.userID, + IP: strangeIP, + }) + check("a prior token-reuse detection is on record", err) + + check("login after a reuse incident still succeeds", r.login(ctx, knownIP, knownAgent, password)) + expectSignals(ctx, r, "the login is visibly connected to the reuse incident", "token_reuse") +} + +func concurrentSessions(ctx context.Context) { + fmt.Println("\n— concurrent sessions") + sessionLimited := thresholds + sessionLimited.MaxConcurrentSessions = 2 + r, err := newRigWith(ctx, true, sessionLimited) + check("engine constructed with a 2-session limit", err) + if r == nil { + return + } + + // Observations are read before this attempt's own session exists, so + // login N sees N-1 active sessions. With a limit of 2, the fourth + // login is the first to observe 3. + for i := 0; i < 3; i++ { + check(fmt.Sprintf("login %d of 3 succeeds", i+1), r.login(ctx, knownIP, knownAgent, password)) + } + expectSignals(ctx, r, "holding up to the session limit is not flagged") + + check("login past the session limit succeeds", r.login(ctx, knownIP, knownAgent, password)) + expectSignals(ctx, r, "exceeding the session limit is flagged", "concurrent_sessions") + + sessions, err := cryden.ListSessions(ctx, r.engine, r.userID) + check("sessions read back", err) + expectCount("no session was revoked by the flag", len(sessions), 4) +} + +// The feature must be entirely absent until a store is injected — same +// contract as TOTP, WebAuthn and recovery codes. +func detectionOff(ctx context.Context) { + fmt.Println("\n— detection off (no AnomalyStore configured)") + r, err := newRig(ctx, false) + check("engine constructed without an AnomalyStore", err) + if r == nil { + return + } + + check("login from a known IP succeeds", r.login(ctx, knownIP, knownAgent, password)) + check("login from an unknown IP and device succeeds", r.login(ctx, strangeIP, otherAgent, password)) + checkExpectError("wrong password still rejected", r.login(ctx, knownIP, knownAgent, wrongPass)) + expectSignals(ctx, r, "nothing is flagged and nothing is recorded") + + count, err := r.anomalies.CountFailuresForUser(ctx, r.userID, time.Now().Add(-thresholds.Window)) + check("the unwired store is readable", err) + expectCount("the unwired store received no attempts", count, 0) +} + +// expectSignals asserts the full chronological list of flagged logins so +// far, which catches a missing signal and an extra one equally. +func expectSignals(ctx context.Context, r *rig, step string, want ...string) { + got := r.signals(ctx) + if len(got) != len(want) { + fail(fmt.Sprintf("%s: expected %d flagged login(s) %v, got %d %v", + step, len(want), want, len(got), got)) + return + } + for i := range want { + if got[i] != want[i] { + fail(fmt.Sprintf("%s: flagged login %d was %q, want %q", step, i+1, got[i], want[i])) + return + } + } + pass(step) +} + +func expectCount(step string, got, want int) { + if got != want { + fail(fmt.Sprintf("%s: got %d, want %d", step, got, want)) + return + } + pass(step) +} + +func check(step string, err error) { + if err != nil { + fail(fmt.Sprintf("%s: unexpected error: %v", step, err)) + return + } + pass(step) +} + +func checkExpectError(step string, err error) { + if err == nil { + fail(fmt.Sprintf("%s: expected an error, got nil", step)) + return + } + pass(fmt.Sprintf("%s (%v)", step, err)) +} + +func pass(step string) { + fmt.Println("✓", step) +} + +func fail(msg string) { + failures++ + fmt.Println("✗", msg) +} diff --git a/docs/development/CRYDEN-REVIEW.md b/docs/development/CRYDEN-REVIEW.md new file mode 100644 index 0000000..178efd8 --- /dev/null +++ b/docs/development/CRYDEN-REVIEW.md @@ -0,0 +1,174 @@ +# cryden — architecture review (read sections as needed, not cover to cover) + +This file exists so you never have to rediscover these conventions by +reading the source tree. If you find yourself about to grep the whole +repo "to understand how X works," check here first — it's probably +already answered. + +## What this project is + +`cryden` (`github.com/crydensync/cryden/v2`) is an embeddable, +framework-agnostic Go authentication engine. No HTTP, no hardcoded +storage backend, zero telemetry — it never leaves the consuming app's +infrastructure on its own initiative, including logs and audit data. +The root `cryden` package is the only public import; `auth`, `token`, +`security`, `store`, `session`, `logger` are internal implementation +detail (the root package's own doc comment says this explicitly). + +## Package layout + +- `cryden.go`, `config.go`, `engine.go`, `errors.go` — the public + facade (root package). `Config` wires an `Engine` via `New(cfg)`. + Every public function takes `(ctx, *Engine, ...)`. +- `auth/` — all business logic. Internal. This is where feature work + actually happens. +- `security/` — pluggable security primitives. Interfaces + one + production implementation each (`Hasher`→bcrypt, `TOTPGenerator`→ + pquerna/otp, `WebAuthnProvider`→go-webauthn, `Encryptor`→AES-256-GCM) + — except integrations requiring an outbound network call + (`BreachedPasswordChecker`), which ship **zero** implementations, + same reasoning as `notify/`. +- `store/` — `interfaces.go` defines every storage contract + shared + data types + `AuditEventType` constants. `store/memory/` = test-only + implementations. `store/postgres/` = production, plus + `store/postgres/migrations/*.sql` (numbered sequentially — check the + highest existing number before adding one; currently `0005`). +- `token/` — JWT issuance (`JWTIssuer`), refresh token generation + (`TokenGenerator`), the second-factor pending token + (`MFAPendingIssuer`). +- `notify/` — external delivery interfaces (`EmailSender`, + `MagicLinkSender`). Zero implementations, by design — host app + supplies one. +- `session/` — session listing/revocation helpers. +- `logger/` — `Logger` interface, one console-JSON implementation. +- `cmd/smoketest//main.go` — one per feature, in-memory, + runnable, no external dependencies. +- `docs/testing/.md` — one per feature, manual verification + steps. + +## Core design rules (apply these to every new feature) + +**Interface-first, one production implementation per interface**, +UNLESS the concern requires an outbound network call the engine +shouldn't make on its own initiative — those get an interface with +**zero** shipped implementations (`EmailSender`, `MagicLinkSender`, +`BreachedPasswordChecker` are the existing examples). When in doubt +which bucket a new integration falls into, ask: "does using this +feature necessarily mean cryden talks to some external service over +the network?" If yes → zero-implementation interface, host supplies +one. If it's local computation/crypto only → ship one real +implementation using a well-vetted library, same as TOTP/WebAuthn. + +**Config fields for optional features are nil-safe.** Unset → the +facade function returns a clear `cryden.ErrXNotConfigured`, never a +panic. The one deliberate exception: `Config.PasswordPolicy` has no +"off" state — leaving it as the literal zero value +(`security.PasswordPolicy{}`, compared as a **whole struct**, not by +checking one field) applies `security.DefaultPasswordPolicy` +automatically. Password strength isn't opt-in the way 2FA methods are. + +**Fail loudly, never silently insecure.** Security-critical +misconfiguration (missing JWT secret, missing required store) is a +hard error from `New()`, not a runtime surprise. + +**Storage pattern for a new pluggable feature:** +1. Type + interface in `store/interfaces.go`. +2. `store/memory/_store.go` — test implementation. +3. `store/postgres/_store.go` — production implementation. If + storing a rich/evolving struct (e.g. a third-party library's own + type), store it as a JSON blob column rather than decomposing every + field — see `webauthn_credentials.credential_data` for the pattern. + **Passing a Go `[]byte` to a `jsonb` column via `lib/pq` sends it as + `bytea` on the wire and fails** — cast to `string(...)` first. +4. `store/postgres/migrations/000N_.up.sql` + `.down.sql`. +5. Wire into `Config`, `Engine`, and the `cryden` facade. + +**Error patterns:** +- Simple binary failure → sentinel `var ErrX = errors.New(...)`. +- Failure that carries data the caller needs → a struct type with an + `Error()` method, retrieved via `errors.As` (see + `ErrSecondFactorRequired{PendingToken, Methods}`, + `ErrPasswordPolicyViolation{Violations []string}`, + `ErrOAuthEmailConflict`). Never encode structured data into an error + *string* for a caller to parse. +- Enumeration-avoidance: when a wrong input and a nonexistent-resource + input would otherwise return different errors or take different + time, they must return the identical error AND the identical + execution path (see `Login`'s nonexistent-email case, which still + pays bcrypt's cost via a dummy hash — a fixed historical bug, worth + reading `auth/login.go`'s comment on it once). + +**Audit logging:** every security-relevant event gets an +`AuditEventType` constant in `store/interfaces.go` and is recorded via +`store.AuditStore`. Routine input-validation failures (a malformed +email, a too-short password) are NOT audited — too noisy, not +security-relevant. A confirmed breach, a failed second-factor attempt, +a completed login, an account lock — those are. + +**Second-factor gate:** every *primary* authentication path (password +`Login`, `CompleteMagicLink`, `LoginWithOAuth`) routes through +`completePrimaryAuth` in `auth/login.go`. It collects confirmed +methods (`totp`, `webauthn`, `recovery_code` — the last **only** +alongside a real factor, never standalone, see the comment there for +why) and either pauses with `*ErrSecondFactorRequired{PendingToken, +Methods}` or calls `finishLogin` to issue tokens. **Any new primary +auth method MUST route through `completePrimaryAuth`, never +reimplement session issuance inline** — `LoginWithOAuth` shipped with +exactly that bug once; it's fixed now, don't reintroduce the pattern. + +**Encryption vs. hashing:** passwords, tokens, recovery codes → +one-way hash (bcrypt for passwords; SHA-256 via `token.HashToken` for +everything else, since those are already high-entropy random values, +not human-guessable secrets — bcrypt's slow-hash property defends +against a different threat than these need). TOTP secrets and WebAuthn +ceremony state → `security.Encryptor` (reversible), because the engine +must recover the original plaintext value later. Never mix these up. + +**Fixed, non-configurable security TTLs:** `mfaPendingTTL` (5 min), +`magicLinkTTL` (15 min) are intentionally hardcoded constants, not +`Config` fields. A tuning knob here just invites a deployment to widen +a narrow security window. Follow this precedent for any new short- +lived credential — don't make it configurable without a real reason. + +## Known platform gotchas (don't rediscover these) + +- No network access to `proxy.golang.org` in some sandboxed tool + environments — `go build`/`go test`/`go mod tidy` may not be + runnable there. Say so plainly; don't claim untested code compiles. +- `lib/pq` fails outright on Termux/Android (`os/user.Current` + unimplemented for `GOOS=android`) — not something to fix, the human + works around it with `proot-distro ubuntu` or a real machine. +- Supabase connection strings must use the **session pooler** (port + 5432), not the transaction pooler (6543) or a direct connection. +- `virtualwebauthn`'s simulated credential starts its signature + counter at 0 and never auto-increments on its own — and 0 is itself + a legitimate, spec-allowed value for a real authenticator too. Don't + assert a counter "must be nonzero" in any WebAuthn test; if you need + to test counter pass-through, set it explicitly before the call. +- **Never mutate the real `crypto/rand.Reader` package-level global in + a test**, on any platform — on at least one real environment, a + failed read through that specific global hits the Go runtime's own + unrecoverable fatal-error path (a process crash, not a normal test + failure), not a catchable error. If you need to test an entropy-read + failure path, inject a fake `io.Reader` into a same-package struct + field instead (see `token.CryptoRandTokenGenerator.randReader` for + the pattern). +- **When you change a shared function's signature, grep the ENTIRE + repo for every call site before committing** — + `grep -rn "FunctionName(ctx" --include="*.go" .` — not just the + files you remember touching. This exact mistake (a test file on an + earlier branch not updated when a later, stacked branch changed a + signature it also called) has shipped twice already in this + project's history and only surfaced when `go test ./...` ran after + merging. + +## What's already shipped (Tier 1, tagged v2.2.0) + +Signup/login/logout, account lockout, email verification/change, +OAuth (Google/GitHub, provider-agnostic design — adding a new provider +string needs zero engine changes), TOTP 2FA, WebAuthn passkeys +(second-factor only, not passwordless-primary yet), magic-link login, +recovery codes, breached-password checking (interface-only), password +policy (secure-by-default). Full detail in each feature's README +section and `docs/testing/*.md`. Don't re-verify any of this is +working — `CURRENT-STATE.md` confirms it's tagged and done. diff --git a/docs/development/CURRENT-STATE.md b/docs/development/CURRENT-STATE.md new file mode 100644 index 0000000..59fcb86 --- /dev/null +++ b/docs/development/CURRENT-STATE.md @@ -0,0 +1,130 @@ +# cryden — current state + +Last updated: 2026-09-04 (by the session that built anomaly +detection). Update this file's date and content every time a session +finishes an item — see `CLAUDE.md`'s end-of-session checklist. + +## Tagged releases + +- **v2.2.0** — Tier 1 complete. Currently the latest tag and the + presumed base for all new work, unless a session's `NEXT.md` says + otherwise. + +## Tier 1 — Auth & Login: DONE (all shipped in v2.2.0) + +TOTP (2FA), WebAuthn passkeys (second factor), magic-link login, more +OAuth providers (confirmed zero engine changes needed — provider is a +plain string), recovery/backup codes, breached-password check, +password policy. Plus a fix: `LoginWithOAuth` used to bypass the +second-factor gate entirely — now routes through the same +`completePrimaryAuth` helper every other primary auth method uses. + +Do not re-verify, re-review, or re-read through this work. It's done. +If you find a real bug in it while working on something else, fix it +on its own small branch and note it in `PROGRESS.md` — don't treat +finding it as license to re-audit the rest. + +## Tier 2 — Security & Monitoring: IN PROGRESS (1 of 4 done) + +### Item 8 — anomaly detection: DONE, branch `feat/anomaly-detection` + +Not merged — the human reviews and pushes. Do not re-verify or +re-review this; see the note at the end of the Tier 1 section, it +applies here too. + +Shipped as: `security/anomaly.go` (pure logic — `AnomalySignal`, +`LoginAttemptContext`, `AnomalyObservations`, `AnomalyThresholds`, +`DefaultAnomalyThresholds`, `Evaluate`, `JoinAnomalySignals`), +`auth/anomaly.go` (the storage-reading pass plus the exported +`RecordLoginAttempt` that failure paths call), `store.AnomalyStore` +with `store/memory` + `store/postgres` implementations, migration +`0006_login_attempts`, the `anomaly_detected` audit event type, and +`Config.Anomalies` / `Config.AnomalyThresholds`. + +Six signals ship, not three: the spec's "new IP/device" and +"token reuse / session anomalies" each split into two, because a known +device on a new IP and a new device on a known IP mean different +things, and so do a replayed refresh token and an unusual live-session +count. Codes are `new_ip`, `new_device`, `user_failure_velocity`, +`ip_failure_velocity`, `token_reuse`, `concurrent_sessions`. + +Detection runs inside `completePrimaryAuth`, so all three primary auth +paths (password, magic-link, OAuth) are covered by one call. It is +report-only, nil-safe, and degrades to "no evidence" on any storage +error. Tests: `security/anomaly_test.go` (12, no store at all), +`auth/anomaly_test.go` (13, through the real flows), +`store/memory/anomaly_store_test.go` (6), plus 4 in `config_test.go`. +Smoke test: `cmd/smoketest/anomaly-detection` (54 checks). Manual +guide: `docs/testing/anomaly-detection.md`. + +### Items 9-11: NOT STARTED + +Detailed specs in `NEXT.md`. The design decision recorded for item 8 +below is kept for reference — it is what the shipped code implements. +**Do not re-ask or re-derive it**: + +- **Signals to evaluate**: new IP/device (vs. recent successful + logins), failed-attempt velocity (per-user and per-IP), and + token-reuse/session anomalies (reusing existing + `token_reuse_detected` events + unusual concurrent-session counts). + Impossible-travel (geo-distance) was explicitly deferred — it needs + an external geo API, which breaks the "engine never calls the + internet itself" rule; if it's ever built, it must be interface- + only, host-supplied, same pattern as `BreachedPasswordChecker`. +- **On a flagged attempt**: record only, never block. Emit a new audit + event with risk info; the host app decides what to do with it. No + new sentinel error, no forced step-up, no hard block — those were + all explicitly considered and rejected (false positives locking out + real users was the deciding factor against hard-blocking; step-up + 2FA was rejected because it silently doesn't work for accounts with + no second factor enrolled). +- **Storage**: a new `store.AnomalyStore` interface (with + `store/memory` + `store/postgres` implementations + a migration), + not a reuse of `AuditStore` alone and not the in-memory rate + limiter — matches how every other Tier 1 feature was built, keeps + queries indexed instead of scanning audit history, and avoids the + in-memory rate limiter's known multi-instance correctness gap. + As built, that interface is `RecordAttempt`, `ListRecentSuccesses`, + `CountFailuresForUser` and `CountFailuresForIP` over one + `login_attempts` table with three partial indexes. + +Items 9, 10, 11 (credential-stuffing detection, named/fingerprinted +sessions, Redis-backed rate limiter) have no prior design decisions +recorded — see `NEXT.md` for the level of detail available, make +reasonable calls on anything unspecified, note them in `PROGRESS.md`. + +Item 9 in particular: `login_attempts` already holds everything +credential stuffing needs (per-IP failures across accounts, with +`user_id` NULL for attempts against nonexistent emails). It needs a +distinct-target-accounts-per-IP query and a +`credential_stuffing_detected` event type, but no second migration. + +## Tier 3 — Infrastructure & Extensibility: NOT STARTED + +Seven items. See `NEXT.md`. + +## Tier 4 — AI-assisted admin features: NOT STARTED + +Four items, all read-only/surface-only by explicit, non-negotiable +requirement — no automatic action, ever. See `NEXT.md`. + +## Tier 5 — do not start without an explicit go-ahead from the project owner + +Organizations/multi-tenancy, SSO via OIDC, SAML, RBAC/permissions, +data export/delete-my-data. If you reach the end of Tier 4 with +nothing left queued, stop and say so — don't proceed into Tier 5 on +your own initiative, this was stated explicitly in the original +project brief. + +## Open branches / in-flight work + +- `feat/anomaly-detection` — item 8, complete, 6 commits, branched from + `main` at `5b6c7f5`. Unmerged and unpushed, awaiting the human's + review. Nothing else in flight. Each new session picks +the top item off `NEXT.md`, creates its own branch, and this section +should be updated to reflect that branch's existence and status before +the session ends. If you start a session and this section already +lists an in-progress branch, that means a previous session didn't +finish cleanly — check that branch's own commits before assuming +anything about its state, and update this file to match reality once +you've looked. diff --git a/docs/development/NEXT.md b/docs/development/NEXT.md new file mode 100644 index 0000000..57a1992 --- /dev/null +++ b/docs/development/NEXT.md @@ -0,0 +1,205 @@ +# cryden — next up + +Ordered queue. Take the **first item**, build it completely, then +stop — per `CLAUDE.md`. Remove an item from this file (or mark it done +— pick whichever this file's state already shows by the time you read +it) once it's finished and reflected in `CURRENT-STATE.md`. + +Specs below are deliberately detailed so you don't need to ask +anything mid-build. Where something is genuinely unspecified, make the +most reasonable call consistent with `CRYDEN-REVIEW.md`'s established +patterns and note the assumption in `PROGRESS.md` — don't block on it. + +--- + +## Tier 2 — Security & Monitoring + +### 1. Credential-stuffing detection (item 9) — overlaps with item 8, don't duplicate + +This is "many accounts failing from one IP" — which is *almost* the +same underlying data as item 8's per-IP failed-attempt velocity +signal. Item 8 is **done** (branch `feat/anomaly-detection`), so the +shared piece already exists: `store.AnomalyStore` over a +`login_attempts` table with `RecordAttempt`, `ListRecentSuccesses`, +`CountFailuresForUser` and `CountFailuresForIP`, already called from +every primary auth path including the failure branches. Attempts +against emails with no account behind them are stored with a NULL +`user_id`, which is exactly the population this item cares about. +**Extend it, do not build a second tracking system.** + +The real incremental work: a distinct-target-accounts-per-IP query on +the existing table (no new migration needed — the +`idx_login_attempts_ip_failures` partial index already covers the +access pattern), a threshold tuned for "one IP, many different target +accounts" (existing per-account lockout already handles "one account, +many attempts" — this is the gap that doesn't cover), and its own +audit event type (`credential_stuffing_detected`) so it's +distinguishable from a single-account anomaly in monitoring. + +Follow item 8's split when you build it: pure threshold arithmetic in +`security/`, the storage reads in `auth/`. Same report-only rule — +never block a login. + +### 2. Named/fingerprinted sessions (item 10) — genuinely underspecified, use judgment + +Current `store.Session` already has `IP` and `UserAgent`. "Named/ +fingerprinted" most likely means: a human-readable label for "your +active sessions" UI (e.g. "Chrome on Windows — San Francisco, CA") +instead of a raw session ID. + +- User-Agent → device/browser string: pure parsing, no network call, + no external API — fine to ship as a real engine-side helper (or a + small interface if you think host apps would want to swap parsing + libraries; use your judgment, this is a minor decision either way). +- IP → location string: this DOES require geolocation data from + somewhere. Follow the established rule — if it needs an outbound + network call, it's a new interface (e.g. `security.IPGeolocator`) + with **zero shipped implementations**, host supplies one, exactly + like `BreachedPasswordChecker`. Do not bake in a call to any + specific geo-IP service directly. +- If geolocation feels like it belongs entirely at the `api`/host-app + layer instead of the engine (since the engine already exposes raw + IP on every session), that's a legitimate alternative — note your + reasoning in `PROGRESS.md` either way, this is the item where the + original backlog line is vaguest and a documented judgment call is + expected. + +### 3. Redis-backed rate limiter (item 11) + +`security.RateLimiter` already exists with one implementation +(in-memory, documented as not safe across multiple instances). This is +a **second real implementation**, not an interface-only integration — +Redis is configured infrastructure the host app wires in explicitly +(a connection string/client), the same category as Postgres, not an +arbitrary third-party internet service like HIBP. Ship a real +`security.RedisRateLimiter` (or wherever you decide it should live — +probably `security/`, matching where the in-memory one lives) using a +real, well-established Go Redis client library. `Config` gets a new +way to select/configure it (follow how `Users`/`Sessions`/etc. stores +are injected as already-constructed instances, not built internally +from a connection string — match that pattern here too). + +--- + +## Tier 3 — Infrastructure & Extensibility + +### 4. Argon2id as an additional trusted hasher (item 12) + +Second implementation of `security.Hasher`, not a replacement for +bcrypt. Real design question: how does the engine know which +algorithm a given stored hash used, for a user base that might have a +mix (e.g. mid-migration)? The common answer is sniffing the hash's own +format prefix (`$argon2id$...` vs bcrypt's `$2a$`/`$2b$`) inside a +dispatching `Compare`, while `Hash` always uses whichever algorithm is +currently configured. Build it this way unless you find a strong +reason not to; note the reasoning either way. + +### 5. Additional storage backend beyond Postgres (item 13) + +Every `store.X` interface already exists — implement all of them +against a second backend (SQLite is the most likely candidate per +earlier project notes, but check `CURRENT-STATE.md`/`PROGRESS.md` for +anything more specific by the time you get here). Watch for Postgres- +specific assumptions baked into existing interface docs/behavior +(`JSONB` columns, `ON CONFLICT ... DO UPDATE`, `RETURNING`) — several +`store/postgres/` implementations lean on these and a different +backend will need different real solutions, not just syntax swaps. + +### 6. Cloud logger integrations (item 14) + +`logger.Logger` already exists with one implementation (console JSON). +Decide interface-only-vs-shipped-implementation the same way as +everything else: does using this necessarily mean an outbound network +call? If yes (calling Datadog's/Better Stack's API directly), lean +toward interface-only, zero shipped implementations — most host apps +already have their own logging pipeline wired at their level, and +console-JSON-to-stdout is already the universal integration point +(any log shipper can tail stdout). Only ship a real implementation if +there's a specific strong reason a direct integration adds real value +over "the host app already captures stdout." + +### 7. Extensible JWT claims (item 15) + +Let host apps attach their own data to access tokens. Read +`token/jwt.go`'s current claims struct and `JWTIssuer.Issue` before +proposing anything — whatever you add must not weaken the existing +algorithm-confusion protections already there (the `alg: none`/ +signing-method check). Likely shape: `Issue` gains an optional +`extraClaims map[string]interface{}` parameter, or a small +`ClaimsProvider` hook — pick whichever fits the existing `Issue` +call sites with the least disruption. + +### 8. API keys / machine-to-machine auth (item 16) + +New concept, not a variant of an existing one — no human to prompt, so +this sits outside the second-factor system entirely (confirm this +assumption is right by checking whether M2M auth appears anywhere else +in the codebase already — it shouldn't). Needs its own storage +(`store.APIKeyStore`), fast-hash lookup like recovery codes (SHA-256 +via `token.HashToken`, not bcrypt — these are high-entropy generated +values, not human passwords), and its own facade functions +(`GenerateAPIKey`, `RevokeAPIKey`, and something that validates a +presented key and returns which user/scope it belongs to). + +### 9. Webhooks (item 17) + +Notify the host app on key events. Same question as everything else +that reaches outward: interface-only, zero shipped implementations +(`notify.WebhookSender` or similar), matching `EmailSender` — the +engine surfaces the event, the host app's implementation does the +actual HTTP call, retries, signing, etc. Decide which existing audit +events should also trigger a webhook call (probably a configurable +subset, not all of them) and wire it in wherever `audit.Record` is +already called for those events — don't build a second parallel event +bus. + +### 10. Custom email templates (item 18) + +Check `notify.EmailSender`/`notify.MagicLinkSender` as they exist +today first — there's a real chance this needs **no engine change at +all**, since the host app's own implementation already owns the +actual email body/template (the engine only ever hands over a raw +token, per `EmailSender`'s own doc comment). If that's true, say so +plainly in `PROGRESS.md` and mark the item done-as-a-non-issue rather +than building something speculative to have built something. + +--- + +## Tier 4 — AI-assisted admin features + +**Non-negotiable for all four:** read-only / surface-only. No +automatic action — no auto-lock, no auto-config-change, nothing. Every +one of these produces information for a human to act on. + +### 11. Weekly digest (item 19) +Reads `AuditStore`, summarizes in plain English, returns text. Nothing +else. + +### 12. Support-ticket assistant (item 20) +Read-only diagnosis ("why can't user X log in") — queries +`AuditStore`/`UserStore`/session state, produces an explanation, never +touches anything. + +### 13. Config tuning advisor (item 21) +Produces a report of suggested config changes. Never applies them. + +### 14. Ask-AI widget (item 22) +The most complex of the four. Needs its own full design pass before +any code — at minimum: an LLM provider interface (zero shipped +implementations, host brings their own key/provider, same pattern as +every other external-network-call integration), a defined read-only +query surface, and an explicit answer to how untrusted end-user input +is kept from doing anything beyond reading data (this is exposed to +the host app's own end users, not just admins — prompt-injection +surface is real here). Write the design into this section of +`NEXT.md` (or a new file it points to) before writing any code, even +though there's no human to approve it mid-session — the design still +needs to exist and be reasoned through in writing, just do it as part +of this same session rather than waiting for a reply. + +--- + +## Tier 5 — do not start + +See `CURRENT-STATE.md`. Stop and say so if you reach here with nothing +else queued. diff --git a/docs/development/PROGRESS.md b/docs/development/PROGRESS.md new file mode 100644 index 0000000..09c4a92 --- /dev/null +++ b/docs/development/PROGRESS.md @@ -0,0 +1,109 @@ +# cryden — progress log + +Append-only. Newest entry at the bottom. One entry per session, added +right before that session stops — see `CLAUDE.md`'s end-of-session +checklist. Keep entries short: what got built, the branch, one line on +any assumption made. This is a log, not a design document — detailed +reasoning belongs in commit messages and code comments, not here. + +--- + +## 2026 — Tier 1 (prior work, summarized retroactively) + +Built across multiple sessions before this file existed: TOTP 2FA, +WebAuthn passkeys (second factor), magic-link login, OAuth provider- +agnostic confirmation (no engine changes needed for new providers), +recovery codes, breached-password checking, password policy. Plus a +fix for `LoginWithOAuth` bypassing the second-factor gate. Tagged as +**v2.2.0**. Full detail in each feature's git history and +`docs/testing/*.md` — not repeated here. + +## 2026-09-03 — Development workflow setup + +Added `CLAUDE.md` + this `docs/development/` structure +(`CURRENT-STATE.md`, `NEXT.md`, `CRYDEN-REVIEW.md`, `PROGRESS.md`) so +future terminal sessions read state from files instead of re-deriving +it through conversation each time. No engine code changed. Branch: +none — these are meta/process files, human will decide where they +land (likely directly reviewed and committed to whatever branch/main +state the human already has locally, since prior sessions' branches +were merged outside of this file-writing session). + +--- + + + +## 2026-09-04 — Anomaly detection (item 8) + +Branch: `feat/anomaly-detection` (6 commits, unmerged, unpushed). + +Built: report-only login anomaly detection. Pure threshold arithmetic +in `security/anomaly.go`, the storage-reading pass in +`auth/anomaly.go`, a `store.AnomalyStore` over a new `login_attempts` +table (memory + postgres + migration `0006`), an `anomaly_detected` +audit event, and `Config.Anomalies` / `Config.AnomalyThresholds`. +Detection is called once from `completePrimaryAuth`, which covers +password, magic-link and OAuth login together. 35 Go tests, a 54-check +smoke test at `cmd/smoketest/anomaly-detection`, and +`docs/testing/anomaly-detection.md`. + +Assumptions made (spec left these open): + +- **Config shape.** `NEXT.md` said "`Config.AnomalyDetector` or + similar — you decide." I split it the way the codebase already + splits this kind of thing rather than inventing a third pattern: an + injected optional store (`Config.Anomalies`, nil ⇒ feature off, like + `TOTP`/`WebAuthn`/`RecoveryCodes`) plus a plain config struct + (`Config.AnomalyThresholds`, whole-struct zero ⇒ defaults, like + `PasswordPolicy`). No detector interface: there is nothing here a + host app would want to swap that the thresholds don't already cover, + and an interface would have dragged `store` into `security`. +- **Six signals, not three.** The spec's "new IP/device" and "token + reuse / session anomalies" each carry two distinct meanings, so each + became two signals. A known device on a new IP is travel; a new + device on a known IP is more often a second party. A replayed + refresh token and an unusual live-session count are equally + unrelated. Merging either pair would have made the metadata + ambiguous for exactly the monitoring it exists to feed. +- **Baseline is successes only.** `AnomalyStore.ListRecentSuccesses` + deliberately ignores failures. If failures fed the known-IP list, an + attacker would establish their own address as familiar just by + failing a few times first. +- **First login is never flagged.** `HasLoginHistory` suppresses + new_ip/new_device when the account has no prior success. Otherwise + every account's first login is an anomaly, which is noise. +- **A separate `TokenReuseLookback` (24h) alongside `Window` (15m).** + Failure velocity is a burst happening now; a stolen refresh token + replayed this morning still matters this afternoon. One duration + could not serve both, and unbounded would flag every login the + account ever makes again. +- **Token-reuse history is a bounded 100-event scan** of the user's + audit log, because `AuditStore` has no by-user-and-type query + (`ListByUser` is per-user, `SearchByType` is system-wide). A user + with more than 100 events since their last reuse event misses the + signal. Acceptable for a report-only annotation; adding an + `AuditStore` method for it would have widened the item. +- **Sequencing.** Observations are read before the current attempt is + recorded, so an attempt can never appear in its own baseline. +- **Storage errors degrade to "no evidence,"** never to "everything is + unfamiliar" — the latter would flag every login during an outage. + +Verification: `go build ./...`, `go vet ./...` and `go test ./...` all +run clean here with `GOTOOLCHAIN=go1.25.11 GOPROXY=off` (the module +cache has every dependency; only the toolchain download fails without +network, and `/usr/bin/go` is 1.22.2 while `go.mod` needs 1.25.0 — +hence the explicit `GOTOOLCHAIN`). The smoke test runs and passes all +54 checks. Postgres was not exercised — no database in this +environment; migration `0006` and `store/postgres/anomaly_store.go` +are reviewed-and-compiled only, and section 9 of the manual test guide +covers what to check against a real one. + +Noted in passing, not fixed: `TestLogin_NonexistentUserTimingMatches` +`WrongPassword` in `auth/` is timing-based and flaked once when the +full suite ran packages in parallel (ratio 0.35), then passed on three +consecutive full runs and five isolated ones. Pre-existing and +unrelated to this item — it compares two bcrypt durations and is +sensitive to CPU contention. Worth its own small branch if it recurs. + +Next in queue: item 9, credential-stuffing detection. `login_attempts` +already holds the data it needs; `NEXT.md` has the updated spec. diff --git a/docs/testing/anomaly-detection.md b/docs/testing/anomaly-detection.md new file mode 100644 index 0000000..1e8897f --- /dev/null +++ b/docs/testing/anomaly-detection.md @@ -0,0 +1,192 @@ +# Manual test guide — login anomaly detection + +Anomaly detection annotates successful logins that look unusual. It +**never blocks a login**, never returns a new error, and never forces +step-up authentication. Everything below is about what gets *recorded*. + +The fastest full check is the smoke test: + +``` +go run ./cmd/smoketest/anomaly-detection +``` + +54 checks over six scenarios, no database required. What follows is the +same ground covered by hand, plus the Postgres path the smoke test does +not touch. + +## Setup + +The feature is off until you inject a store, exactly like TOTP and +recovery codes: + +```go +engine, err := cryden.New(cryden.Config{ + JWTSecret: os.Getenv("CRYDEN_JWT_SECRET"), + Users: users, + Sessions: sessions, + Audit: audit, + + // Omit this and detection is entirely absent. + Anomalies: postgres.NewAnomalyStore(db), + + // Omit this and security.DefaultAnomalyThresholds applies. + AnomalyThresholds: security.AnomalyThresholds{ + Window: 15 * time.Minute, + HistorySize: 20, + UserFailureVelocity: 5, + IPFailureVelocity: 20, + MaxConcurrentSessions: 10, + TokenReuseLookback: 24 * time.Hour, + }, +}) +``` + +For Postgres, apply migration `0006_login_attempts.up.sql` first. + +Test identity used throughout: `raymondproguy@dev.com` / +`Tr0ubl3-Fr33!2026`. + +## Reading the results + +Every flagged login writes one `anomaly_detected` audit event whose +metadata carries a comma-separated `signals` key, plus the count behind +each signal that fired: + +```sql +SELECT created_at, ip, metadata +FROM audit_events +WHERE type = 'anomaly_detected' AND user_id = '' +ORDER BY created_at DESC; +``` + +``` + metadata +------------------------------------------------------------- + {"signals": "new_ip,new_device"} + {"signals": "user_failure_velocity", "user_failures": "6"} +``` + +The raw history it judges against is in `login_attempts`: + +```sql +SELECT created_at, user_id, ip, user_agent, outcome +FROM login_attempts +ORDER BY created_at DESC +LIMIT 20; +``` + +## 1. First login is clean + +Sign up, then log in once. + +- Login succeeds. +- **No** `anomaly_detected` event. A first login has no baseline to + deviate from; flagging it would flag every new account. +- One `login_attempts` row with `outcome = 'success'`. + +## 2. A familiar login stays quiet + +Log in two or three more times from the same IP and User-Agent. + +- No new `anomaly_detected` events. + +This is the case that decides whether the feature is usable at all. If a +routine login flags, every login flags. + +## 3. New IP and new device + +Log in from a different IP with a different User-Agent. + +- **The login succeeds and returns normal tokens.** Verify this + explicitly — it is the whole design. +- One event, `signals = "new_ip,new_device"`. +- The event's `ip` is the new address. + +Then log in from a third IP using the *original* User-Agent: + +- `signals = "new_ip"` only. A known device on a new address is travel; + a new device is a different claim, so the signals stay separate. + +Log in from the second IP again: + +- Nothing new. That address is now part of the baseline, because + observations are gathered before the current attempt is recorded and + the earlier attempt is now history. + +## 4. Per-user failure velocity + +With `UserFailureVelocity: 5` and `LockoutThreshold` raised above it (or +lockout will fire first and mask this), submit five wrong passwords, then +the correct one. + +- The five failures each return `ErrInvalidCredentials`. +- Five `login_attempts` rows with `outcome = 'failure'`. +- The successful login is flagged: `signals = "user_failure_velocity"`, + `user_failures = "5"`. + +## 5. Per-IP failure velocity + +Submit `IPFailureVelocity` failed attempts from one IP against an email +that has **no account** (`nobody@dev.com`), then log in legitimately from +that same IP. + +- The rows land with `user_id IS NULL` — there is no user to attribute + them to, and inventing one would be wrong. +- `CountFailuresForIP` counts them; `CountFailuresForUser` does not. +- The legitimate login is flagged `ip_failure_velocity`. + +The per-IP threshold is deliberately much looser than the per-user one: +one office NAT or carrier gateway legitimately produces many users' +typos from a single address. + +## 6. Token-reuse history + +Trigger refresh-token reuse (send a refresh token twice — the second use +revokes the family and records `token_reuse_detected`), then log in. + +- The login succeeds. +- `signals = "token_reuse"`, `token_reuse_events = "1"`. + +Set `TokenReuseLookback` to something short and log in again: the signal +stops. One incident must not flag every login the account ever makes +again. + +## 7. Concurrent sessions + +With `MaxConcurrentSessions: 2`, log in four times without logging out. + +- Logins 1–3 are quiet. Observations are read before the current + attempt's own session exists, so login N sees N-1 active sessions — + login 3 observes 2, which is at the limit, not over it. +- Login 4 observes 3 and is flagged `concurrent_sessions`, + `active_sessions = "3"`. +- **No session is revoked.** Confirm with `cryden.ListSessions`. + +## 8. Detection off + +Build an engine with `Anomalies` omitted and repeat sections 1–3. + +- Every login behaves exactly as it did before this feature existed. +- No `anomaly_detected` events, and no `login_attempts` rows. + +## 9. Storage failure must not lock anyone out + +Point `Anomalies` at a store whose queries fail (drop the +`login_attempts` table, or revoke access to it). + +- Logins still succeed with valid credentials. +- Errors appear in the log (`anomaly: ...`). +- **No** `anomaly_detected` events. A failed read is treated as "no + evidence," not as "everything is unfamiliar" — otherwise an outage + would flag every login in it. + +## Known limits + +- Token-reuse history is found by scanning the user's 100 most recent + audit events, because `AuditStore` has no by-user-and-type query. A + user with more than 100 events since their last reuse event will not + trip the signal. The reuse event itself is still in the audit trail. +- Impossible-travel and geo-velocity are deliberately out of scope: the + engine never calls the internet, so it has no geo-IP source. +- `memory.AnomalyStore` is for tests and local runs only. Two instances + would each hold half the evidence and neither would see the pattern. diff --git a/security/anomaly.go b/security/anomaly.go new file mode 100644 index 0000000..d63569f --- /dev/null +++ b/security/anomaly.go @@ -0,0 +1,193 @@ +package security + +import ( + "strings" + "time" +) + +// AnomalySignal is one machine-readable reason a login attempt looked +// unusual. Like PasswordPolicy's violation codes, these are stable +// short strings rather than human sentences — the engine doesn't own UI +// copy or localization anywhere else, and these end up in an audit +// event's metadata for a host app's monitoring to match on, not in +// front of a user. +type AnomalySignal string + +const ( + // SignalNewIP fires when the attempt's IP has not appeared in the + // user's recent successful logins. + SignalNewIP AnomalySignal = "new_ip" + // SignalNewDevice fires when the attempt's User-Agent has not + // appeared in the user's recent successful logins. Separate from + // SignalNewIP because they mean genuinely different things — a + // known device on a new IP is travel, a new device on a known IP + // is more often a real second party. + SignalNewDevice AnomalySignal = "new_device" + // SignalUserFailureVelocity fires when this one account has + // accumulated failed attempts faster than AnomalyThresholds allows. + SignalUserFailureVelocity AnomalySignal = "user_failure_velocity" + // SignalIPFailureVelocity fires when this one IP has accumulated + // failed attempts faster than AnomalyThresholds allows, counting + // across every account it targeted. + SignalIPFailureVelocity AnomalySignal = "ip_failure_velocity" + // SignalTokenReuse fires when the user has recent + // store.EventTokenReuseDetected history. Refresh-token reuse + // already revokes the whole session family when it happens; this + // signal exists so a login arriving shortly afterward is visibly + // connected to it instead of looking routine. + SignalTokenReuse AnomalySignal = "token_reuse" + // SignalConcurrentSessions fires when the user holds more active + // sessions than AnomalyThresholds allows. + SignalConcurrentSessions AnomalySignal = "concurrent_sessions" +) + +// LoginAttemptContext is what the detector knows about the attempt +// being evaluated right now. Deliberately not store.Session or +// store.LoginAttempt — Evaluate is pure logic in a package that has no +// storage dependency, so it takes plain values. +type LoginAttemptContext struct { + IP string + UserAgent string +} + +// AnomalyObservations is the history snapshot Evaluate judges an +// attempt against — the whole storage-facing side of detection reduced +// to plain numbers and strings. Whoever gathers this (the detector in +// package auth) owns the queries; Evaluate never reads anything itself, +// which is what makes the thresholds testable without a store at all. +type AnomalyObservations struct { + // KnownIPs and KnownUserAgents come from the user's recent + // SUCCESSFUL logins only. A failed attempt from an IP must never + // teach the baseline that the IP is familiar — otherwise an + // attacker establishes their own trust just by failing a few times + // first. + KnownIPs []string + KnownUserAgents []string + // HasLoginHistory reports whether the user has any prior successful + // login at all. Without it, every account's first-ever login trips + // both new_ip and new_device, which is pure noise — there is no + // baseline yet to deviate from. + HasLoginHistory bool + // RecentUserFailures and RecentIPFailures are counted over + // AnomalyThresholds.Window. + RecentUserFailures int + RecentIPFailures int + // RecentTokenReuseEvents counts store.EventTokenReuseDetected + // events for this user. + RecentTokenReuseEvents int + // ActiveSessions is the user's current non-revoked session count. + ActiveSessions int +} + +// AnomalyThresholds tunes which observations count as anomalous. Plain +// configuration data with a method, same shape as PasswordPolicy — the +// swappable part of this feature is the store the observations come +// from, not the arithmetic here. +type AnomalyThresholds struct { + // Window bounds the failure-velocity counts. Defaults to 15 + // minutes when the whole struct is left zero-valued (see + // Config.applyDefaults). + Window time.Duration + // HistorySize is how many recent successful logins form the + // known-IP/known-device baseline. Too small and normal multi-device + // users trip new_device constantly; too large and a long-abandoned + // device stays trusted forever. + HistorySize int + // UserFailureVelocity is the failed-attempt count for ONE account + // within Window that flags the attempt. Defaults to 5, matching + // Config.LockoutThreshold's default — an account that just came off + // (or is riding the edge of) lockout is exactly the case worth + // surfacing. + UserFailureVelocity int + // IPFailureVelocity is the failed-attempt count from ONE IP within + // Window that flags the attempt, counted across every account that + // IP targeted. Deliberately much higher than + // UserFailureVelocity: one office NAT or mobile carrier gateway + // legitimately produces many users' typos from a single address. + IPFailureVelocity int + // MaxConcurrentSessions is the active-session count a user may hold + // before the next login is flagged. Zero disables the check. + MaxConcurrentSessions int + // TokenReuseLookback bounds how far back a + // store.EventTokenReuseDetected event still counts. Separate from + // Window, and much longer, because the two signals live on + // different time scales: failure velocity is about a burst happening + // right now, while a stolen refresh token replayed this morning is + // still the most relevant thing about a login this afternoon. + // Unbounded would be wrong too — one reuse event would then flag + // every login the account ever makes again. + TokenReuseLookback time.Duration +} + +// DefaultAnomalyThresholds is applied whenever Config.AnomalyThresholds +// is left as the zero value. Unlike DefaultPasswordPolicy this is not a +// security floor — anomaly detection as a whole is off until +// Config.Anomalies is set, and these numbers only decide how chatty it +// is once it's on. +var DefaultAnomalyThresholds = AnomalyThresholds{ + Window: 15 * time.Minute, + HistorySize: 20, + UserFailureVelocity: 5, + IPFailureVelocity: 20, + MaxConcurrentSessions: 10, + TokenReuseLookback: 24 * time.Hour, +} + +// Evaluate returns every signal the attempt trips, in a stable order, +// or nil for a clean attempt. It reports; it never decides. Nothing +// here blocks a login, returns a sentinel error, or forces step-up +// authentication — a flagged attempt is recorded and handed to the host +// app, which owns what to do about it. False positives are expected +// (travel, a new browser, a shared office IP), and locking real users +// out over them would be a worse outcome than the detection is worth. +func (t AnomalyThresholds) Evaluate(attempt LoginAttemptContext, obs AnomalyObservations) []AnomalySignal { + var signals []AnomalySignal + + // An empty IP or User-Agent isn't evidence of anything — the caller + // simply didn't supply one. Silently treating "" as an unknown + // device would flag every such attempt forever. + if obs.HasLoginHistory { + if attempt.IP != "" && !containsString(obs.KnownIPs, attempt.IP) { + signals = append(signals, SignalNewIP) + } + if attempt.UserAgent != "" && !containsString(obs.KnownUserAgents, attempt.UserAgent) { + signals = append(signals, SignalNewDevice) + } + } + + if t.UserFailureVelocity > 0 && obs.RecentUserFailures >= t.UserFailureVelocity { + signals = append(signals, SignalUserFailureVelocity) + } + if t.IPFailureVelocity > 0 && obs.RecentIPFailures >= t.IPFailureVelocity { + signals = append(signals, SignalIPFailureVelocity) + } + if obs.RecentTokenReuseEvents > 0 { + signals = append(signals, SignalTokenReuse) + } + if t.MaxConcurrentSessions > 0 && obs.ActiveSessions > t.MaxConcurrentSessions { + signals = append(signals, SignalConcurrentSessions) + } + + return signals +} + +// JoinAnomalySignals renders signals as one comma-separated string, for +// an audit event's metadata (which is map[string]string — no room for a +// list). Order matches Evaluate's, so the value is stable enough for a +// host app's monitoring to match on. +func JoinAnomalySignals(signals []AnomalySignal) string { + parts := make([]string, len(signals)) + for i, s := range signals { + parts[i] = string(s) + } + return strings.Join(parts, ",") +} + +func containsString(haystack []string, needle string) bool { + for _, v := range haystack { + if v == needle { + return true + } + } + return false +} diff --git a/security/anomaly_test.go b/security/anomaly_test.go new file mode 100644 index 0000000..05e0496 --- /dev/null +++ b/security/anomaly_test.go @@ -0,0 +1,228 @@ +package security + +import ( + "testing" + "time" +) + +// testThresholds is deliberately not DefaultAnomalyThresholds: these +// tests are about the arithmetic, and hardcoding the numbers they +// depend on means changing a default never silently changes what a +// test asserts. +var testThresholds = AnomalyThresholds{ + Window: 15 * time.Minute, + HistorySize: 20, + UserFailureVelocity: 5, + IPFailureVelocity: 20, + MaxConcurrentSessions: 10, + TokenReuseLookback: 24 * time.Hour, +} + +func hasSignal(signals []AnomalySignal, want AnomalySignal) bool { + for _, s := range signals { + if s == want { + return true + } + } + return false +} + +func TestEvaluate_CleanAttemptReturnsNoSignals(t *testing.T) { + attempt := LoginAttemptContext{IP: "1.2.3.4", UserAgent: "test-agent"} + obs := AnomalyObservations{ + KnownIPs: []string{"1.2.3.4"}, + KnownUserAgents: []string{"test-agent"}, + HasLoginHistory: true, + ActiveSessions: 2, + } + + if signals := testThresholds.Evaluate(attempt, obs); len(signals) != 0 { + t.Fatalf("expected no signals for a familiar attempt, got %v", signals) + } +} + +func TestEvaluate_NewIPAndNewDeviceAreIndependent(t *testing.T) { + obs := AnomalyObservations{ + KnownIPs: []string{"1.2.3.4"}, + KnownUserAgents: []string{"test-agent"}, + HasLoginHistory: true, + } + + // Known device, new address — travel. + signals := testThresholds.Evaluate(LoginAttemptContext{IP: "9.9.9.9", UserAgent: "test-agent"}, obs) + if !hasSignal(signals, SignalNewIP) || hasSignal(signals, SignalNewDevice) { + t.Fatalf("known device on a new IP should flag new_ip only, got %v", signals) + } + + // Known address, new device — more often a real second party. + signals = testThresholds.Evaluate(LoginAttemptContext{IP: "1.2.3.4", UserAgent: "other-agent"}, obs) + if !hasSignal(signals, SignalNewDevice) || hasSignal(signals, SignalNewIP) { + t.Fatalf("new device on a known IP should flag new_device only, got %v", signals) + } +} + +// A first-ever login has no baseline to deviate from. Flagging it would +// mean every new account's first login is an anomaly, which is noise, +// not signal. +func TestEvaluate_FirstLoginSuppressesNewIPAndDevice(t *testing.T) { + attempt := LoginAttemptContext{IP: "1.2.3.4", UserAgent: "test-agent"} + obs := AnomalyObservations{HasLoginHistory: false} + + signals := testThresholds.Evaluate(attempt, obs) + if len(signals) != 0 { + t.Fatalf("first-ever login should be clean, got %v", signals) + } +} + +// An absent IP or User-Agent means the caller didn't supply one. Reading +// "" as an unknown device would flag every such attempt forever. +func TestEvaluate_EmptyAttemptFieldsAreNotEvidence(t *testing.T) { + obs := AnomalyObservations{ + KnownIPs: []string{"1.2.3.4"}, + KnownUserAgents: []string{"test-agent"}, + HasLoginHistory: true, + } + + if signals := testThresholds.Evaluate(LoginAttemptContext{}, obs); len(signals) != 0 { + t.Fatalf("empty IP and User-Agent should produce no signals, got %v", signals) + } +} + +func TestEvaluate_FailureVelocityFiresAtThreshold(t *testing.T) { + attempt := LoginAttemptContext{IP: "1.2.3.4", UserAgent: "test-agent"} + base := AnomalyObservations{HasLoginHistory: false} + + cases := []struct { + name string + obs AnomalyObservations + want AnomalySignal + fires bool + }{ + {"user one below", AnomalyObservations{RecentUserFailures: 4}, SignalUserFailureVelocity, false}, + {"user at threshold", AnomalyObservations{RecentUserFailures: 5}, SignalUserFailureVelocity, true}, + {"user above", AnomalyObservations{RecentUserFailures: 50}, SignalUserFailureVelocity, true}, + {"ip one below", AnomalyObservations{RecentIPFailures: 19}, SignalIPFailureVelocity, false}, + {"ip at threshold", AnomalyObservations{RecentIPFailures: 20}, SignalIPFailureVelocity, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + obs := tc.obs + obs.HasLoginHistory = base.HasLoginHistory + got := hasSignal(testThresholds.Evaluate(attempt, obs), tc.want) + if got != tc.fires { + t.Fatalf("%s fired=%v, want %v", tc.want, got, tc.fires) + } + }) + } +} + +// The per-IP threshold has to sit well above the per-user one: one +// office NAT legitimately produces many users' typos from one address. +func TestEvaluate_PerIPThresholdIsLooserThanPerUser(t *testing.T) { + if testThresholds.IPFailureVelocity <= testThresholds.UserFailureVelocity { + t.Fatalf("IP threshold %d must exceed user threshold %d", + testThresholds.IPFailureVelocity, testThresholds.UserFailureVelocity) + } + if DefaultAnomalyThresholds.IPFailureVelocity <= DefaultAnomalyThresholds.UserFailureVelocity { + t.Fatal("DefaultAnomalyThresholds must keep the per-IP threshold looser") + } +} + +func TestEvaluate_TokenReuseFiresOnAnyEvent(t *testing.T) { + attempt := LoginAttemptContext{IP: "1.2.3.4", UserAgent: "test-agent"} + + obs := AnomalyObservations{RecentTokenReuseEvents: 1} + if !hasSignal(testThresholds.Evaluate(attempt, obs), SignalTokenReuse) { + t.Fatal("a single token-reuse event should fire token_reuse") + } + + obs = AnomalyObservations{RecentTokenReuseEvents: 0} + if hasSignal(testThresholds.Evaluate(attempt, obs), SignalTokenReuse) { + t.Fatal("no token-reuse history should not fire token_reuse") + } +} + +func TestEvaluate_ConcurrentSessionsFiresOnlyAboveLimit(t *testing.T) { + attempt := LoginAttemptContext{IP: "1.2.3.4", UserAgent: "test-agent"} + + // At the limit is still allowed — MaxConcurrentSessions is how many + // a user may hold, not the first flagged count. + obs := AnomalyObservations{ActiveSessions: 10} + if hasSignal(testThresholds.Evaluate(attempt, obs), SignalConcurrentSessions) { + t.Fatal("holding exactly MaxConcurrentSessions should not fire") + } + + obs = AnomalyObservations{ActiveSessions: 11} + if !hasSignal(testThresholds.Evaluate(attempt, obs), SignalConcurrentSessions) { + t.Fatal("exceeding MaxConcurrentSessions should fire concurrent_sessions") + } +} + +// Zeroed thresholds are how a host app turns individual signals off. +func TestEvaluate_ZeroThresholdsDisableTheirSignals(t *testing.T) { + off := AnomalyThresholds{Window: 15 * time.Minute, HistorySize: 20} + attempt := LoginAttemptContext{IP: "1.2.3.4", UserAgent: "test-agent"} + obs := AnomalyObservations{ + RecentUserFailures: 500, + RecentIPFailures: 500, + ActiveSessions: 500, + } + + if signals := off.Evaluate(attempt, obs); len(signals) != 0 { + t.Fatalf("zeroed thresholds should disable their signals, got %v", signals) + } +} + +// Signal order is part of the contract: the joined string ends up in an +// audit event's metadata, where a host app's monitoring matches on it. +func TestEvaluate_SignalOrderIsStable(t *testing.T) { + attempt := LoginAttemptContext{IP: "9.9.9.9", UserAgent: "other-agent"} + obs := AnomalyObservations{ + KnownIPs: []string{"1.2.3.4"}, + KnownUserAgents: []string{"test-agent"}, + HasLoginHistory: true, + RecentUserFailures: 5, + RecentIPFailures: 20, + RecentTokenReuseEvents: 1, + ActiveSessions: 11, + } + + want := []AnomalySignal{ + SignalNewIP, + SignalNewDevice, + SignalUserFailureVelocity, + SignalIPFailureVelocity, + SignalTokenReuse, + SignalConcurrentSessions, + } + + got := testThresholds.Evaluate(attempt, obs) + if len(got) != len(want) { + t.Fatalf("expected all %d signals, got %v", len(want), got) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("signal %d = %s, want %s (full: %v)", i, got[i], want[i], got) + } + } + + const expected = "new_ip,new_device,user_failure_velocity,ip_failure_velocity,token_reuse,concurrent_sessions" + if joined := JoinAnomalySignals(got); joined != expected { + t.Fatalf("JoinAnomalySignals = %q, want %q", joined, expected) + } +} + +func TestJoinAnomalySignals_EmptyIsEmptyString(t *testing.T) { + if joined := JoinAnomalySignals(nil); joined != "" { + t.Fatalf("expected empty string for no signals, got %q", joined) + } +} + +// The whole-struct zero comparison in Config.applyDefaults only works +// while AnomalyThresholds stays comparable (no slices or maps). +func TestAnomalyThresholds_IsComparable(t *testing.T) { + if (AnomalyThresholds{}) == DefaultAnomalyThresholds { + t.Fatal("the zero value must differ from the defaults, or defaulting is a no-op") + } +} diff --git a/store/memory/anomaly_store.go b/store/memory/anomaly_store.go new file mode 100644 index 0000000..553a2a1 --- /dev/null +++ b/store/memory/anomaly_store.go @@ -0,0 +1,78 @@ +package memory + +import ( + "context" + "sync" + "time" + + "github.com/crydensync/cryden/v2/store" +) + +// AnomalyStore is an in-memory store.AnomalyStore implementation for +// tests and local experimentation only — not a supported production +// backend. Beyond the usual "it forgets everything on restart," an +// in-memory anomaly history is actively misleading in production: two +// instances would each hold half the evidence and neither would see the +// pattern. The Postgres implementation is authoritative for prod. +type AnomalyStore struct { + mu sync.Mutex + attempts []store.LoginAttempt +} + +func NewAnomalyStore() *AnomalyStore { + return &AnomalyStore{} +} + +func (s *AnomalyStore) RecordAttempt(ctx context.Context, attempt store.LoginAttempt) error { + s.mu.Lock() + defer s.mu.Unlock() + attempt.CreatedAt = time.Now() + s.attempts = append(s.attempts, attempt) + return nil +} + +func (s *AnomalyStore) ListRecentSuccesses(ctx context.Context, userID string, limit int) ([]store.LoginAttempt, error) { + s.mu.Lock() + defer s.mu.Unlock() + var out []store.LoginAttempt + for i := len(s.attempts) - 1; i >= 0 && len(out) < limit; i-- { + a := s.attempts[i] + if a.UserID == userID && a.Outcome == store.OutcomeSuccess { + out = append(out, a) + } + } + return out, nil +} + +func (s *AnomalyStore) CountFailuresForUser(ctx context.Context, userID string, since time.Time) (int, error) { + // An empty userID would otherwise match every unknown-email failure + // ever recorded and report them as one user's history. + if userID == "" { + return 0, nil + } + return s.countFailures(func(a store.LoginAttempt) bool { return a.UserID == userID }, since), nil +} + +func (s *AnomalyStore) CountFailuresForIP(ctx context.Context, ip string, since time.Time) (int, error) { + if ip == "" { + return 0, nil + } + return s.countFailures(func(a store.LoginAttempt) bool { return a.IP == ip }, since), nil +} + +func (s *AnomalyStore) countFailures(match func(store.LoginAttempt) bool, since time.Time) int { + s.mu.Lock() + defer s.mu.Unlock() + count := 0 + for _, a := range s.attempts { + if a.Outcome != store.OutcomeFailure || a.CreatedAt.Before(since) { + continue + } + if match(a) { + count++ + } + } + return count +} + +var _ store.AnomalyStore = (*AnomalyStore)(nil) diff --git a/store/memory/anomaly_store_test.go b/store/memory/anomaly_store_test.go new file mode 100644 index 0000000..1111006 --- /dev/null +++ b/store/memory/anomaly_store_test.go @@ -0,0 +1,154 @@ +package memory + +import ( + "context" + "testing" + "time" + + "github.com/crydensync/cryden/v2/store" +) + +// The other memory stores are covered through the auth-layer tests that +// use them. This one gets its own: the empty-key guards, the window +// filter and the newest-first walk are real logic the detector's +// correctness depends on, and none of it is visible from auth. + +func TestAnomalyStore_ListRecentSuccessesIsNewestFirstAndScoped(t *testing.T) { + ctx := context.Background() + s := NewAnomalyStore() + + for _, ip := range []string{"1.1.1.1", "2.2.2.2", "3.3.3.3"} { + if err := s.RecordAttempt(ctx, store.LoginAttempt{ + UserID: "user-1", IP: ip, UserAgent: "test-agent", Outcome: store.OutcomeSuccess, + }); err != nil { + t.Fatalf("RecordAttempt failed: %v", err) + } + } + // Noise that must not appear: another user's success, and this + // user's failure. + _ = s.RecordAttempt(ctx, store.LoginAttempt{UserID: "user-2", IP: "9.9.9.9", Outcome: store.OutcomeSuccess}) + _ = s.RecordAttempt(ctx, store.LoginAttempt{UserID: "user-1", IP: "8.8.8.8", Outcome: store.OutcomeFailure}) + + got, err := s.ListRecentSuccesses(ctx, "user-1", 10) + if err != nil { + t.Fatalf("ListRecentSuccesses failed: %v", err) + } + if len(got) != 3 { + t.Fatalf("expected 3 successes for user-1, got %d", len(got)) + } + if got[0].IP != "3.3.3.3" { + t.Fatalf("expected newest first, got %s", got[0].IP) + } + if got[0].CreatedAt.IsZero() { + t.Fatal("RecordAttempt should stamp CreatedAt") + } +} + +// A failure must never teach the baseline that an IP is familiar — +// otherwise an attacker self-trusts their own address by failing first. +func TestAnomalyStore_FailuresNeverEnterTheSuccessBaseline(t *testing.T) { + ctx := context.Background() + s := NewAnomalyStore() + + for i := 0; i < 5; i++ { + _ = s.RecordAttempt(ctx, store.LoginAttempt{ + UserID: "user-1", IP: "6.6.6.6", UserAgent: "attacker-agent", Outcome: store.OutcomeFailure, + }) + } + + got, err := s.ListRecentSuccesses(ctx, "user-1", 10) + if err != nil { + t.Fatalf("ListRecentSuccesses failed: %v", err) + } + if len(got) != 0 { + t.Fatalf("failures must not appear as successes, got %d", len(got)) + } +} + +func TestAnomalyStore_ListRecentSuccessesHonoursLimit(t *testing.T) { + ctx := context.Background() + s := NewAnomalyStore() + + for i := 0; i < 10; i++ { + _ = s.RecordAttempt(ctx, store.LoginAttempt{UserID: "user-1", IP: "1.2.3.4", Outcome: store.OutcomeSuccess}) + } + + got, _ := s.ListRecentSuccesses(ctx, "user-1", 3) + if len(got) != 3 { + t.Fatalf("expected the limit to cap results at 3, got %d", len(got)) + } +} + +func TestAnomalyStore_CountFailuresIsScopedAndWindowed(t *testing.T) { + ctx := context.Background() + s := NewAnomalyStore() + + // Two accounts targeted from one shared address, plus one success + // that must not be counted as a failure. + _ = s.RecordAttempt(ctx, store.LoginAttempt{UserID: "user-1", IP: "5.5.5.5", Outcome: store.OutcomeFailure}) + _ = s.RecordAttempt(ctx, store.LoginAttempt{UserID: "user-1", IP: "5.5.5.5", Outcome: store.OutcomeFailure}) + _ = s.RecordAttempt(ctx, store.LoginAttempt{UserID: "user-2", IP: "5.5.5.5", Outcome: store.OutcomeFailure}) + _ = s.RecordAttempt(ctx, store.LoginAttempt{UserID: "user-1", IP: "5.5.5.5", Outcome: store.OutcomeSuccess}) + + since := time.Now().Add(-time.Minute) + + userCount, err := s.CountFailuresForUser(ctx, "user-1", since) + if err != nil { + t.Fatalf("CountFailuresForUser failed: %v", err) + } + if userCount != 2 { + t.Fatalf("expected 2 failures for user-1, got %d", userCount) + } + + // The per-IP count spans every account the address targeted, which + // is the whole point of it being separate from the per-user count. + ipCount, err := s.CountFailuresForIP(ctx, "5.5.5.5", since) + if err != nil { + t.Fatalf("CountFailuresForIP failed: %v", err) + } + if ipCount != 3 { + t.Fatalf("expected 3 failures from 5.5.5.5 across accounts, got %d", ipCount) + } + + // A window that opened after everything was recorded sees nothing. + future := time.Now().Add(time.Minute) + if n, _ := s.CountFailuresForUser(ctx, "user-1", future); n != 0 { + t.Fatalf("expected the window to exclude older attempts, got %d", n) + } + if n, _ := s.CountFailuresForIP(ctx, "5.5.5.5", future); n != 0 { + t.Fatalf("expected the window to exclude older attempts, got %d", n) + } +} + +// Failed logins for an unknown email carry no user ID. Without the +// empty-key guard, an empty userID would match every one of them and +// report the pile as a single account's history. +func TestAnomalyStore_EmptyKeysCountNothing(t *testing.T) { + ctx := context.Background() + s := NewAnomalyStore() + + for i := 0; i < 3; i++ { + _ = s.RecordAttempt(ctx, store.LoginAttempt{IP: "", Outcome: store.OutcomeFailure}) + } + + since := time.Now().Add(-time.Minute) + if n, _ := s.CountFailuresForUser(ctx, "", since); n != 0 { + t.Fatalf("an empty userID must count nothing, got %d", n) + } + if n, _ := s.CountFailuresForIP(ctx, "", since); n != 0 { + t.Fatalf("an empty IP must count nothing, got %d", n) + } +} + +func TestAnomalyStore_UnknownUserIsEmptyNotAnError(t *testing.T) { + ctx := context.Background() + s := NewAnomalyStore() + + got, err := s.ListRecentSuccesses(ctx, "nobody", 20) + if err != nil { + t.Fatalf("an unknown user should not be an error: %v", err) + } + if len(got) != 0 { + t.Fatalf("expected no history for an unknown user, got %d", len(got)) + } +} diff --git a/store/postgres/anomaly_store.go b/store/postgres/anomaly_store.go new file mode 100644 index 0000000..6ca24bb --- /dev/null +++ b/store/postgres/anomaly_store.go @@ -0,0 +1,98 @@ +package postgres + +import ( + "context" + "database/sql" + "time" + + "github.com/crydensync/cryden/v2/store" +) + +// AnomalyStore is the v2 production store.AnomalyStore implementation. +type AnomalyStore struct { + db *sql.DB +} + +func NewAnomalyStore(db *sql.DB) *AnomalyStore { + return &AnomalyStore{db: db} +} + +func (s *AnomalyStore) RecordAttempt(ctx context.Context, attempt store.LoginAttempt) error { + // user_id is nullable in the schema — a failure against an email + // that matches no account has no user to attribute to, and an empty + // string is not a valid UUID. Same reasoning (and same fix) as + // AuditStore.Record. + var userID sql.NullString + if attempt.UserID != "" { + userID = sql.NullString{String: attempt.UserID, Valid: true} + } + + _, err := s.db.ExecContext(ctx, ` + INSERT INTO login_attempts (id, user_id, ip, user_agent, outcome) + VALUES (gen_random_uuid(), $1, $2, $3, $4) + `, userID, attempt.IP, attempt.UserAgent, string(attempt.Outcome)) + return err +} + +func (s *AnomalyStore) ListRecentSuccesses(ctx context.Context, userID string, limit int) ([]store.LoginAttempt, error) { + rows, err := s.db.QueryContext(ctx, ` + SELECT id, user_id, ip, user_agent, outcome, created_at + FROM login_attempts + WHERE user_id = $1 AND outcome = 'success' + ORDER BY created_at DESC + LIMIT $2 + `, userID, limit) + if err != nil { + return nil, err + } + defer rows.Close() + + out := []store.LoginAttempt{} + for rows.Next() { + var ( + a store.LoginAttempt + uid sql.NullString + outcome string + ) + if err := rows.Scan(&a.ID, &uid, &a.IP, &a.UserAgent, &outcome, &a.CreatedAt); err != nil { + return nil, err + } + if uid.Valid { + a.UserID = uid.String + } + a.Outcome = store.LoginAttemptOutcome(outcome) + out = append(out, a) + } + return out, rows.Err() +} + +func (s *AnomalyStore) CountFailuresForUser(ctx context.Context, userID string, since time.Time) (int, error) { + // Guarded rather than passed straight through: user_id IS NULL for + // unknown-email failures, and letting "" reach the query as a + // parameter would either error on the UUID cast or, worse in a + // backend that tolerates it, report every unattributed failure in + // the system as this one user's history. + if userID == "" { + return 0, nil + } + var count int + err := s.db.QueryRowContext(ctx, ` + SELECT COUNT(*) FROM login_attempts + WHERE user_id = $1 AND outcome = 'failure' AND created_at >= $2 + `, userID, since).Scan(&count) + return count, err +} + +func (s *AnomalyStore) CountFailuresForIP(ctx context.Context, ip string, since time.Time) (int, error) { + if ip == "" { + return 0, nil + } + var count int + err := s.db.QueryRowContext(ctx, ` + SELECT COUNT(*) FROM login_attempts + WHERE ip = $1 AND outcome = 'failure' AND created_at >= $2 + `, ip, since).Scan(&count) + return count, err +} + +var _ store.AnomalyStore = (*AnomalyStore)(nil) diff --git a/store/postgres/migrations/0006_login_attempts.down.sql b/store/postgres/migrations/0006_login_attempts.down.sql new file mode 100644 index 0000000..322993e --- /dev/null +++ b/store/postgres/migrations/0006_login_attempts.down.sql @@ -0,0 +1,3 @@ +-- 0006_login_attempts.down.sql + +DROP TABLE login_attempts; diff --git a/store/postgres/migrations/0006_login_attempts.up.sql b/store/postgres/migrations/0006_login_attempts.up.sql new file mode 100644 index 0000000..f0fbcf5 --- /dev/null +++ b/store/postgres/migrations/0006_login_attempts.up.sql @@ -0,0 +1,36 @@ +-- 0006_login_attempts.up.sql + +CREATE TABLE login_attempts ( + id UUID PRIMARY KEY, + -- Nullable, and ON DELETE SET NULL rather than CASCADE: a deleted + -- account's attempt rows still carry real evidence about the IP + -- that targeted it, which is exactly what per-IP velocity needs. + -- Matching audit_events, not sessions/recovery_codes. + user_id UUID REFERENCES users(id) ON DELETE SET NULL, + ip TEXT NOT NULL DEFAULT '', + user_agent TEXT NOT NULL DEFAULT '', + outcome TEXT NOT NULL CHECK (outcome IN ('success', 'failure')), + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- Every read of this table is an aggregate over a time window, never a +-- full scan for a human to page through — that difference from +-- audit_events is the entire reason this table exists separately, so +-- the indexes are what justify it. + +-- Per-user failure velocity (CountFailuresForUser). +CREATE INDEX idx_login_attempts_user_failures + ON login_attempts(user_id, created_at DESC) + WHERE outcome = 'failure'; + +-- Per-IP failure velocity (CountFailuresForIP), counted across every +-- account one IP targeted, including unknown-email attempts where +-- user_id IS NULL. +CREATE INDEX idx_login_attempts_ip_failures + ON login_attempts(ip, created_at DESC) + WHERE outcome = 'failure'; + +-- Known-IP/known-device baseline (ListRecentSuccesses). +CREATE INDEX idx_login_attempts_user_successes + ON login_attempts(user_id, created_at DESC) + WHERE outcome = 'success';