feat(auth): throttle login, register, password reset and Subsonic auth failures (M462 #4976)
test-go / test (push) Successful in 1m54s
test-web / test (push) Successful in 1m34s
test-go / integration (push) Successful in 4m56s
android / Build + lint + test (push) Successful in 5m41s
release / Build signed APK (releases and dev) (push) Successful in 5m52s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
test-go / test (push) Successful in 1m54s
test-web / test (push) Successful in 1m34s
test-go / integration (push) Successful in 4m56s
android / Build + lint + test (push) Successful in 5m41s
release / Build signed APK (releases and dev) (push) Successful in 5m52s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
Every password-shaped check was mounted bare, so guessing was limited only by bcrypt cost. A shared in-memory AttemptLimiter now sits in front of them: - login: 10 failures per account and 50 per address per 15 min, checked before the user lookup and bcrypt; 429 with Retry-After. A success clears the account's count but not the address's. - unknown usernames run a dummy bcrypt compare, so timing no longer says which accounts exist. - register: 10 per address per hour; forgot-password: 5 per address and 3 per email per hour (applied whether or not the email matches); reset: 20 failed tokens per address per 15 min. - Subsonic /rest: same limits as login, counting only wrong credentials, since clients authenticate on every request. Web login, register, reset and forgot-password screens say how long to wait; web and Android carry copy for the rate_limited code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@ package api
|
||||
import (
|
||||
"log/slog"
|
||||
"math/rand"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
@@ -58,6 +59,11 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
|
||||
reacqSettings: reacqSettings,
|
||||
fingerprintSettings: fpSettings,
|
||||
librarySize: recommendation.NewLibrarySize(nil),
|
||||
loginGuard: auth.NewLoginGuard(),
|
||||
registerLimit: auth.NewAttemptLimiter(registerPerAddressMax, time.Hour),
|
||||
forgotAddressLimit: auth.NewAttemptLimiter(forgotPerAddressMax, time.Hour),
|
||||
forgotEmailLimit: auth.NewAttemptLimiter(forgotPerEmailMax, time.Hour),
|
||||
resetLimit: auth.NewAttemptLimiter(resetFailuresPerAddressMax, 15*time.Minute),
|
||||
}
|
||||
|
||||
r.Route("/api", func(api chi.Router) {
|
||||
@@ -313,6 +319,14 @@ type handlers struct {
|
||||
// instance the scanner and the fingerprint workers read, so a save from the
|
||||
// admin card reaches them without a restart. Nil serves the defaults.
|
||||
fingerprintSettings *library.FingerprintSettingsService
|
||||
// loginGuard throttles failed logins per account and per address, and
|
||||
// the limiters below cap the other unauthenticated auth routes. All are
|
||||
// nil-safe, so tests that build handlers directly run unthrottled.
|
||||
loginGuard *auth.LoginGuard
|
||||
registerLimit *auth.AttemptLimiter
|
||||
forgotAddressLimit *auth.AttemptLimiter
|
||||
forgotEmailLimit *auth.AttemptLimiter
|
||||
resetLimit *auth.AttemptLimiter
|
||||
// netSettings caches the trusted reverse-proxy depth read by the auth
|
||||
// middleware on every request and edited from the admin network card.
|
||||
netSettings *netsettings.Service
|
||||
|
||||
+29
-1
@@ -19,6 +19,20 @@ import (
|
||||
// so an abandoned laptop doesn't stay logged in forever.
|
||||
const sessionCookieMaxAge = 30 * 24 * time.Hour
|
||||
|
||||
// Limits on the unauthenticated auth routes other than login (which uses
|
||||
// auth.LoginGuard). Per address, per window as wired in Mount.
|
||||
const (
|
||||
// registerPerAddressMax: 10 registrations an hour. Every attempt counts,
|
||||
// typos included, which is still far above a household's need.
|
||||
registerPerAddressMax = 10
|
||||
// forgotPerAddressMax / forgotPerEmailMax: reset emails an hour. The
|
||||
// per-email cap is what keeps one inbox from being mailbombed.
|
||||
forgotPerAddressMax = 5
|
||||
forgotPerEmailMax = 3
|
||||
// resetFailuresPerAddressMax: wrong or expired reset tokens per 15 min.
|
||||
resetFailuresPerAddressMax = 20
|
||||
)
|
||||
|
||||
func (h *handlers) handleLogout(w http.ResponseWriter, r *http.Request) {
|
||||
// The session token can be on the cookie OR bearer header — RequireUser
|
||||
// accepted either. Re-resolve it here so we can delete the row.
|
||||
@@ -70,10 +84,22 @@ func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Checked before any lookup or bcrypt, so a throttled guess costs the
|
||||
// server nothing and learns nothing.
|
||||
addr := auth.ClientIP(r, h.netSettings.Hops())
|
||||
if blocked, wait := h.loginGuard.Blocked(req.Username, addr); blocked {
|
||||
writeRateLimited(w, wait)
|
||||
return
|
||||
}
|
||||
|
||||
q := dbq.New(h.pool)
|
||||
user, err := q.GetUserByUsername(r.Context(), req.Username)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
// Same bcrypt time as a wrong password, so timing doesn't say
|
||||
// which usernames exist.
|
||||
auth.DummyVerify(req.Password)
|
||||
h.loginGuard.Fail(req.Username, addr)
|
||||
writeErr(w, apierror.Unauthorized("invalid_credentials", "invalid username or password"))
|
||||
return
|
||||
}
|
||||
@@ -82,9 +108,11 @@ func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if !auth.VerifyPassword(user.PasswordHash, req.Password) {
|
||||
h.loginGuard.Fail(req.Username, addr)
|
||||
writeErr(w, apierror.Unauthorized("invalid_credentials", "invalid username or password"))
|
||||
return
|
||||
}
|
||||
h.loginGuard.Succeed(req.Username)
|
||||
|
||||
token, err := auth.MintSessionToken()
|
||||
if err != nil {
|
||||
@@ -100,7 +128,7 @@ func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
// the active-sessions surface: a session that was born somewhere the
|
||||
// user recognises but is being used from somewhere they don't is the
|
||||
// case this whole surface exists to surface.
|
||||
Ip: auth.ClientIP(r, h.netSettings.Hops()),
|
||||
Ip: addr,
|
||||
}); err != nil {
|
||||
h.logger.Error("api: insert session failed", "err", err)
|
||||
writeErr(w, apierror.InternalMsg("insert failed", err))
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/mailer"
|
||||
)
|
||||
@@ -47,6 +48,19 @@ func (h *handlers) handleForgotPassword(w http.ResponseWriter, r *http.Request)
|
||||
}
|
||||
email := strings.ToLower(strings.TrimSpace(req.Email))
|
||||
|
||||
// Throttled per address (a spray) and per email (a mailbomb of one
|
||||
// inbox). Applied whether or not the email matches, so a 429 says
|
||||
// nothing about which addresses are registered.
|
||||
addr := auth.ClientIP(r, h.netSettings.Hops())
|
||||
blockedAddr, waitAddr := h.forgotAddressLimit.Blocked(addr)
|
||||
blockedEmail, waitEmail := h.forgotEmailLimit.Blocked(email)
|
||||
if blockedAddr || blockedEmail {
|
||||
writeRateLimited(w, max(waitAddr, waitEmail))
|
||||
return
|
||||
}
|
||||
h.forgotAddressLimit.Record(addr)
|
||||
h.forgotEmailLimit.Record(email)
|
||||
|
||||
q := dbq.New(h.pool)
|
||||
var matched bool
|
||||
var auditTarget pgtype.UUID
|
||||
|
||||
@@ -55,6 +55,15 @@ type registerReq struct {
|
||||
// - 409 username_taken (PG unique violation on users.username)
|
||||
// - 500 server_error otherwise
|
||||
func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
// Every attempt counts, not only failures: a successful registration is
|
||||
// the thing being sprayed when the mode is open.
|
||||
addr := auth.ClientIP(r, h.netSettings.Hops())
|
||||
if blocked, wait := h.registerLimit.Blocked(addr); blocked {
|
||||
writeRateLimited(w, wait)
|
||||
return
|
||||
}
|
||||
h.registerLimit.Record(addr)
|
||||
|
||||
var req registerReq
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeErr(w, apierror.BadRequest("invalid_body", "invalid JSON body"))
|
||||
@@ -175,7 +184,7 @@ func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
UserID: user.ID,
|
||||
TokenHash: auth.HashSessionToken(sessionToken),
|
||||
UserAgent: r.UserAgent(),
|
||||
Ip: auth.ClientIP(r, h.netSettings.Hops()),
|
||||
Ip: addr,
|
||||
}); err != nil {
|
||||
h.logger.Error("register: insert session failed", "err", err)
|
||||
writeErr(w, apierror.Internal(err))
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
)
|
||||
|
||||
@@ -42,6 +43,15 @@ func (h *handlers) handleResetPassword(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Tokens carry 256 bits, so guessing one is hopeless; the limit is on
|
||||
// failed tries per address so that nobody gets to find that out at our
|
||||
// expense.
|
||||
addr := auth.ClientIP(r, h.netSettings.Hops())
|
||||
if blocked, wait := h.resetLimit.Blocked(addr); blocked {
|
||||
writeRateLimited(w, wait)
|
||||
return
|
||||
}
|
||||
|
||||
q := dbq.New(h.pool)
|
||||
|
||||
// Look up the reset record so we know which user to update before
|
||||
@@ -50,6 +60,7 @@ func (h *handlers) handleResetPassword(w http.ResponseWriter, r *http.Request) {
|
||||
reset, err := q.GetPasswordReset(r.Context(), req.Token)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
h.resetLimit.Record(addr)
|
||||
writeErr(w, apierror.BadRequest("invalid_token", ""))
|
||||
return
|
||||
}
|
||||
@@ -69,6 +80,7 @@ func (h *handlers) handleResetPassword(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if rows == 0 {
|
||||
h.resetLimit.Record(addr)
|
||||
writeErr(w, apierror.BadRequest("invalid_token", ""))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -179,6 +179,36 @@ func TestHandleLogin_UnknownUserReturns401(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A guesser who reaches the account limit is refused with 429 before any
|
||||
// password check, so even the right password is turned away until the window
|
||||
// passes, and the response says when to come back.
|
||||
func TestHandleLogin_ThrottledAfterRepeatedFailures(t *testing.T) {
|
||||
h, pool := testHandlers(t)
|
||||
h.loginGuard = auth.NewLoginGuard()
|
||||
seedUser(t, pool, "alice", "hunter2", false)
|
||||
|
||||
login := func(password string) *httptest.ResponseRecorder {
|
||||
body := strings.NewReader(`{"username":"test-alice","password":"` + password + `"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/auth/login", body)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
h.handleLogin(w, req)
|
||||
return w
|
||||
}
|
||||
for i := 0; i < 10; i++ {
|
||||
if w := login("wrong"); w.Code != http.StatusUnauthorized {
|
||||
t.Fatalf("attempt %d: status = %d, want 401", i+1, w.Code)
|
||||
}
|
||||
}
|
||||
w := login("hunter2")
|
||||
if w.Code != http.StatusTooManyRequests {
|
||||
t.Fatalf("status = %d, want 429 once the account limit is reached", w.Code)
|
||||
}
|
||||
if w.Header().Get("Retry-After") == "" {
|
||||
t.Error("429 without Retry-After")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleLogin_MalformedBodyReturns400(t *testing.T) {
|
||||
h, _ := testHandlers(t)
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/auth/login",
|
||||
|
||||
@@ -3,7 +3,10 @@ package api
|
||||
import (
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
"math"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
)
|
||||
@@ -36,6 +39,17 @@ func writeErr(w http.ResponseWriter, err error) {
|
||||
}})
|
||||
}
|
||||
|
||||
// writeRateLimited answers 429 with Retry-After in whole seconds, rounded
|
||||
// up so a client that honours it never arrives a moment early.
|
||||
func writeRateLimited(w http.ResponseWriter, wait time.Duration) {
|
||||
secs := int(math.Ceil(wait.Seconds()))
|
||||
if secs < 1 {
|
||||
secs = 1
|
||||
}
|
||||
w.Header().Set("Retry-After", strconv.Itoa(secs))
|
||||
writeErr(w, apierror.TooManyRequests("rate_limited", "too many attempts; try again later"))
|
||||
}
|
||||
|
||||
// writeErrWithLog logs the error at Error level and writes the response.
|
||||
// Use for 500-class errors where the operator needs the cause in logs.
|
||||
func writeErrWithLog(w http.ResponseWriter, logger *slog.Logger, msg string, err error) {
|
||||
|
||||
Reference in New Issue
Block a user