Harden auth: setup secret, throttling, session destroy, secure cookies.
Replace username-based admin bootstrap with a one-time setup secret, rate-limit login/register, equalize login bcrypt timing, cap passwords at 72 bytes, destroy sessions on logout, and require Secure cookies when PORT is set.
This commit is contained in:
+7
-4
@@ -5,11 +5,14 @@ LISTEN=:8080
|
||||
DATABASE_URL=postgresql://user:password@host.example.com:5432/postgres?sslmode=verify-full
|
||||
# Required for integration tests (do not point at the runtime DATABASE_URL).
|
||||
# TEST_DATABASE_URL=postgresql://user:password@host.example.com:5432/plumber_test?sslmode=verify-full
|
||||
# Optional: first matching registrant becomes admin only if no admin exists yet.
|
||||
# Later promote/demote via /admin/users (admins only).
|
||||
ADMIN_USERNAME=yourusername
|
||||
# Set to 1 when serving over HTTPS
|
||||
# One-time first-admin bootstrap: registrant must also POST setup_secret matching this value,
|
||||
# and only while no admin exists yet. Leave unset after bootstrap. Prefer a long random string.
|
||||
# ADMIN_SETUP_SECRET=
|
||||
# When PORT is set (App Platform), cookies are Secure by default; SECURE_COOKIE=0 is rejected.
|
||||
# Locally, set to 1 when serving over HTTPS:
|
||||
SECURE_COOKIE=0
|
||||
# Set to 1 only behind a trusted reverse proxy that sets X-Forwarded-For.
|
||||
# TRUST_PROXY=0
|
||||
# DigitalOcean Spaces (profile avatars). Leave unset to disable uploads.
|
||||
# SPACES_KEY=
|
||||
# SPACES_SECRET=
|
||||
|
||||
+16
-2
@@ -54,8 +54,9 @@ func openDB() (*sql.DB, *store.SessionStore) {
|
||||
|
||||
func newHandler(db *sql.DB, sessions *store.SessionStore, uploader blob.Uploader) http.Handler {
|
||||
srv, err := web.New(store.NewPostgres(db), sessions.Store(), plumber.TemplateFS, plumber.StaticFS, web.Config{
|
||||
AdminUsername: os.Getenv("ADMIN_USERNAME"),
|
||||
SecureCookie: os.Getenv("SECURE_COOKIE") == "1",
|
||||
AdminSetupSecret: strings.TrimSpace(os.Getenv("ADMIN_SETUP_SECRET")),
|
||||
SecureCookie: secureCookieFromEnv(),
|
||||
TrustProxy: os.Getenv("TRUST_PROXY") == "1",
|
||||
Blob: uploader,
|
||||
})
|
||||
if err != nil {
|
||||
@@ -64,6 +65,19 @@ func newHandler(db *sql.DB, sessions *store.SessionStore, uploader blob.Uploader
|
||||
return srv.Handler()
|
||||
}
|
||||
|
||||
// secureCookieFromEnv defaults to secure when PORT is set (PaaS/production)
|
||||
// and refuses an explicit disable in that environment.
|
||||
func secureCookieFromEnv() bool {
|
||||
v := strings.TrimSpace(os.Getenv("SECURE_COOKIE"))
|
||||
if strings.TrimSpace(os.Getenv("PORT")) != "" {
|
||||
if v == "0" {
|
||||
log.Fatal("SECURE_COOKIE=0 is not allowed when PORT is set")
|
||||
}
|
||||
return true
|
||||
}
|
||||
return v == "1"
|
||||
}
|
||||
|
||||
func run(httpSrv *http.Server) {
|
||||
errCh := make(chan error, 1)
|
||||
go func() {
|
||||
|
||||
+61
-11
@@ -1,6 +1,7 @@
|
||||
package web
|
||||
|
||||
import (
|
||||
"crypto/subtle"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"regexp"
|
||||
@@ -14,6 +15,23 @@ import (
|
||||
|
||||
var usernameRe = regexp.MustCompile(`^[a-zA-Z0-9_]{3,20}$`)
|
||||
|
||||
const (
|
||||
minPasswordRunes = 8
|
||||
maxPasswordBytes = 72 // bcrypt truncation limit
|
||||
)
|
||||
|
||||
// loginDummyHash is compared when the username is unknown so login timing
|
||||
// does not reveal whether an account exists (same bcrypt cost as real hashes).
|
||||
var loginDummyHash = mustBcrypt("timing-dummy-not-a-real-password")
|
||||
|
||||
func mustBcrypt(s string) []byte {
|
||||
h, err := bcrypt.GenerateFromPassword([]byte(s), bcrypt.DefaultCost)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
return h
|
||||
}
|
||||
|
||||
func safeNext(raw string) string {
|
||||
if raw == "" {
|
||||
return "/"
|
||||
@@ -25,6 +43,16 @@ func safeNext(raw string) string {
|
||||
return u.RequestURI()
|
||||
}
|
||||
|
||||
func passwordValid(password string) (ok bool, msg string) {
|
||||
if utf8.RuneCountInString(password) < minPasswordRunes {
|
||||
return false, "Password must be at least 8 characters."
|
||||
}
|
||||
if len(password) > maxPasswordBytes {
|
||||
return false, "Password must be at most 72 bytes."
|
||||
}
|
||||
return true, ""
|
||||
}
|
||||
|
||||
func (s *Server) handleLoginForm(w http.ResponseWriter, r *http.Request) {
|
||||
if currentUser(r) != nil {
|
||||
http.Redirect(w, r, safeNext(r.URL.Query().Get("next")), http.StatusSeeOther)
|
||||
@@ -43,8 +71,16 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
username := strings.TrimSpace(r.PostFormValue("username"))
|
||||
password := r.PostFormValue("password")
|
||||
next := safeNext(r.PostFormValue("next"))
|
||||
if !s.allowLoginAttempt(w, r, store.NormalizeUsername(username)) {
|
||||
return
|
||||
}
|
||||
|
||||
u, err := s.store.UserByUsername(r.Context(), username)
|
||||
if err != nil || bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte(password)) != nil {
|
||||
hash := loginDummyHash
|
||||
if err == nil {
|
||||
hash = []byte(u.PasswordHash)
|
||||
}
|
||||
if err != nil || bcrypt.CompareHashAndPassword(hash, []byte(password)) != nil {
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
s.exec(w, "login", authPage{
|
||||
page: s.basePage(r, "Sign in"),
|
||||
@@ -74,16 +110,20 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
if !s.requireCSRF(w, r) {
|
||||
return
|
||||
}
|
||||
if !s.allowRegisterAttempt(w, r) {
|
||||
return
|
||||
}
|
||||
username := strings.TrimSpace(r.PostFormValue("username"))
|
||||
password := r.PostFormValue("password")
|
||||
setupSecret := r.PostFormValue("setup_secret")
|
||||
p := authPage{page: s.basePage(r, "Create account"), Username: username}
|
||||
if !usernameRe.MatchString(username) {
|
||||
p.Error = "Username must be 3–20 letters, numbers, or underscores."
|
||||
s.exec(w, "register", p)
|
||||
return
|
||||
}
|
||||
if utf8.RuneCountInString(password) < 8 {
|
||||
p.Error = "Password must be at least 8 characters."
|
||||
if ok, msg := passwordValid(password); !ok {
|
||||
p.Error = msg
|
||||
s.exec(w, "register", p)
|
||||
return
|
||||
}
|
||||
@@ -93,16 +133,9 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
role := store.RoleUser
|
||||
if s.cfg.AdminUsername != "" && store.NormalizeUsername(username) == store.NormalizeUsername(s.cfg.AdminUsername) {
|
||||
n, err := s.store.CountAdmins(r.Context())
|
||||
if err != nil {
|
||||
http.Error(w, "could not create account", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
if n == 0 {
|
||||
if s.consumeAdminSetup(r, setupSecret) {
|
||||
role = store.RoleAdmin
|
||||
}
|
||||
}
|
||||
u := &store.User{
|
||||
Username: username,
|
||||
PasswordHash: string(hash),
|
||||
@@ -121,6 +154,23 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
}
|
||||
|
||||
// consumeAdminSetup grants first-admin when a strong one-time setup secret matches
|
||||
// and no admin exists yet. Username alone is never enough.
|
||||
func (s *Server) consumeAdminSetup(r *http.Request, provided string) bool {
|
||||
want := s.cfg.AdminSetupSecret
|
||||
if want == "" || provided == "" {
|
||||
return false
|
||||
}
|
||||
if subtle.ConstantTimeCompare([]byte(provided), []byte(want)) != 1 {
|
||||
return false
|
||||
}
|
||||
n, err := s.store.CountAdmins(r.Context())
|
||||
if err != nil || n > 0 {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func (s *Server) handleAuthPrompt(w http.ResponseWriter, r *http.Request) {
|
||||
s.exec(w, "signin-prompt", s.basePage(r, ""))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
package web
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestPasswordMaxBytes(t *testing.T) {
|
||||
if ok, _ := passwordValid(strings.Repeat("a", 8)); !ok {
|
||||
t.Fatal("8 ascii runes should pass")
|
||||
}
|
||||
if ok, msg := passwordValid(strings.Repeat("a", 73)); ok || !strings.Contains(msg, "72") {
|
||||
t.Fatalf("73 bytes should fail: ok=%v msg=%q", ok, msg)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLogoutDestroysSession(t *testing.T) {
|
||||
srv, _ := newTestServer(t, Config{})
|
||||
h := srv.Handler()
|
||||
name := uniq("out")
|
||||
cookies := registerUser(t, h, name, "hunter22")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/profile", nil)
|
||||
for _, c := range cookies {
|
||||
req.AddCookie(c)
|
||||
}
|
||||
h.ServeHTTP(rec, req)
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("profile before logout %d", rec.Code)
|
||||
}
|
||||
csrf := csrfFrom(rec.Body.String())
|
||||
form := strings.NewReader("_csrf=" + csrf)
|
||||
req = httptest.NewRequest(http.MethodPost, "/logout", form)
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
for _, c := range cookies {
|
||||
req.AddCookie(c)
|
||||
}
|
||||
rec = httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusSeeOther {
|
||||
t.Fatalf("logout %d", rec.Code)
|
||||
}
|
||||
postLogout := mergeCookies(cookies, rec.Result().Cookies())
|
||||
|
||||
req = httptest.NewRequest(http.MethodGet, "/profile", nil)
|
||||
for _, c := range postLogout {
|
||||
req.AddCookie(c)
|
||||
}
|
||||
rec = httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusSeeOther {
|
||||
t.Fatalf("profile after logout should redirect, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
+17
-2
@@ -24,8 +24,12 @@ import (
|
||||
)
|
||||
|
||||
type Config struct {
|
||||
AdminUsername string
|
||||
// AdminSetupSecret, when set, can promote the first registrant who also
|
||||
// posts the matching setup_secret. It is ignored once any admin exists.
|
||||
AdminSetupSecret string
|
||||
SecureCookie bool
|
||||
// TrustProxy enables X-Forwarded-For / RealIP only behind a known proxy.
|
||||
TrustProxy bool
|
||||
Blob blob.Uploader
|
||||
}
|
||||
|
||||
@@ -35,6 +39,9 @@ type Server struct {
|
||||
tmpl *template.Template
|
||||
cfg Config
|
||||
static http.Handler
|
||||
loginIP *throttle
|
||||
loginUser *throttle
|
||||
registerIP *throttle
|
||||
}
|
||||
|
||||
type page struct {
|
||||
@@ -129,13 +136,18 @@ func New(st store.Store, sessionStore scs.Store, templateFS fs.FS, staticFS fs.F
|
||||
tmpl: tmpl,
|
||||
cfg: cfg,
|
||||
static: http.StripPrefix("/static/", http.FileServer(http.FS(sub))),
|
||||
loginIP: newThrottle(20, 15*time.Minute),
|
||||
loginUser: newThrottle(10, 15*time.Minute),
|
||||
registerIP: newThrottle(10, 15*time.Minute),
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (s *Server) Handler() http.Handler {
|
||||
r := chi.NewRouter()
|
||||
r.Use(middleware.RequestID)
|
||||
if s.cfg.TrustProxy {
|
||||
r.Use(middleware.RealIP)
|
||||
}
|
||||
r.Use(middleware.Logger)
|
||||
r.Use(middleware.Recoverer)
|
||||
r.Use(func(next http.Handler) http.Handler {
|
||||
@@ -507,7 +519,10 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) {
|
||||
if !s.requireCSRF(w, r) {
|
||||
return
|
||||
}
|
||||
s.sessions.Remove(r.Context(), "user_id")
|
||||
if err := s.sessions.Destroy(r.Context()); err != nil {
|
||||
http.Error(w, "could not sign out", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, "/", http.StatusSeeOther)
|
||||
}
|
||||
|
||||
|
||||
+23
-10
@@ -105,15 +105,18 @@ func loginUser(t *testing.T, h http.Handler, username, password string) []*http.
|
||||
return post
|
||||
}
|
||||
|
||||
func registerUser(t *testing.T, h http.Handler, username, password string) []*http.Cookie {
|
||||
func registerUser(t *testing.T, h http.Handler, username, password string, setupSecret ...string) []*http.Cookie {
|
||||
t.Helper()
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/register", nil))
|
||||
pre := rec.Result().Cookies()
|
||||
preToken := sessionValue(pre)
|
||||
csrf := csrfFrom(rec.Body.String())
|
||||
form := strings.NewReader("_csrf=" + csrf + "&username=" + username + "&password=" + password)
|
||||
req := httptest.NewRequest(http.MethodPost, "/register", form)
|
||||
form := "_csrf=" + csrf + "&username=" + username + "&password=" + password
|
||||
if len(setupSecret) > 0 && setupSecret[0] != "" {
|
||||
form += "&setup_secret=" + setupSecret[0]
|
||||
}
|
||||
req := httptest.NewRequest(http.MethodPost, "/register", strings.NewReader(form))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
for _, c := range pre {
|
||||
req.AddCookie(c)
|
||||
@@ -179,25 +182,35 @@ func TestRegisterLoginAsk(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdminSeedOnlyWhenNoAdmins(t *testing.T) {
|
||||
func TestAdminSetupSecretOnlyWhenNoAdmins(t *testing.T) {
|
||||
mem := store.NewMemory()
|
||||
secret := "one-time-admin-setup-secret"
|
||||
adminName := uniq("seed")
|
||||
srv := newTestServerStore(t, mem, Config{AdminUsername: adminName})
|
||||
srv := newTestServerStore(t, mem, Config{AdminSetupSecret: secret})
|
||||
h := srv.Handler()
|
||||
registerUser(t, h, adminName, "hunter22")
|
||||
|
||||
plain := uniq("plain")
|
||||
registerUser(t, h, plain, "hunter22")
|
||||
uPlain, err := mem.UserByUsername(context.Background(), plain)
|
||||
if err != nil || uPlain.Admin() {
|
||||
t.Fatalf("register without setup secret must stay user: %+v %v", uPlain, err)
|
||||
}
|
||||
|
||||
registerUser(t, h, adminName, "hunter22", secret)
|
||||
u, err := mem.UserByUsername(context.Background(), adminName)
|
||||
if err != nil || !u.Admin() {
|
||||
t.Fatalf("first matching registrant should be admin: %+v %v", u, err)
|
||||
t.Fatalf("setup secret registrant should be admin: %+v %v", u, err)
|
||||
}
|
||||
|
||||
later := uniq("later")
|
||||
srv2 := newTestServerStore(t, mem, Config{AdminUsername: later})
|
||||
registerUser(t, srv2.Handler(), later, "hunter22")
|
||||
srv2 := newTestServerStore(t, mem, Config{AdminSetupSecret: secret})
|
||||
registerUser(t, srv2.Handler(), later, "hunter22", secret)
|
||||
u2, err := mem.UserByUsername(context.Background(), later)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if u2.Admin() {
|
||||
t.Fatal("later admin username must stay user when an admin already exists")
|
||||
t.Fatal("setup secret must not grant admin once an admin already exists")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
package web
|
||||
|
||||
import (
|
||||
"net"
|
||||
"net/http"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// throttle is a simple sliding-window rate limiter for auth endpoints.
|
||||
type throttle struct {
|
||||
mu sync.Mutex
|
||||
hits map[string][]time.Time
|
||||
limit int
|
||||
window time.Duration
|
||||
}
|
||||
|
||||
func newThrottle(limit int, window time.Duration) *throttle {
|
||||
return &throttle{
|
||||
hits: map[string][]time.Time{},
|
||||
limit: limit,
|
||||
window: window,
|
||||
}
|
||||
}
|
||||
|
||||
func (t *throttle) allow(key string) bool {
|
||||
if t == nil || key == "" {
|
||||
return true
|
||||
}
|
||||
t.mu.Lock()
|
||||
defer t.mu.Unlock()
|
||||
now := time.Now()
|
||||
cutoff := now.Add(-t.window)
|
||||
xs := t.hits[key]
|
||||
n := 0
|
||||
for _, ts := range xs {
|
||||
if ts.After(cutoff) {
|
||||
xs[n] = ts
|
||||
n++
|
||||
}
|
||||
}
|
||||
xs = xs[:n]
|
||||
if len(xs) >= t.limit {
|
||||
t.hits[key] = xs
|
||||
return false
|
||||
}
|
||||
t.hits[key] = append(xs, now)
|
||||
return true
|
||||
}
|
||||
|
||||
func (s *Server) clientIP(r *http.Request) string {
|
||||
if s.cfg.TrustProxy {
|
||||
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
|
||||
return strings.TrimSpace(strings.Split(xff, ",")[0])
|
||||
}
|
||||
}
|
||||
host, _, err := net.SplitHostPort(r.RemoteAddr)
|
||||
if err != nil {
|
||||
return r.RemoteAddr
|
||||
}
|
||||
return host
|
||||
}
|
||||
|
||||
func authTooMany(w http.ResponseWriter) {
|
||||
http.Error(w, "too many attempts; try again later", http.StatusTooManyRequests)
|
||||
}
|
||||
|
||||
func (s *Server) allowLoginAttempt(w http.ResponseWriter, r *http.Request, usernameKey string) bool {
|
||||
if !s.loginIP.allow(s.clientIP(r)) || !s.loginUser.allow(usernameKey) {
|
||||
authTooMany(w)
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func (s *Server) allowRegisterAttempt(w http.ResponseWriter, r *http.Request) bool {
|
||||
if !s.registerIP.allow(s.clientIP(r)) {
|
||||
authTooMany(w)
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
@@ -10,8 +10,11 @@
|
||||
<input id="username" name="username" type="text" required minlength="3" maxlength="20" pattern="[A-Za-z0-9_]+" autocomplete="username" autocapitalize="off" spellcheck="false" value="{{.Username}}">
|
||||
<p class="hint">3–20 letters, numbers, or underscores.</p>
|
||||
<label for="password">Password</label>
|
||||
<input id="password" name="password" type="password" required minlength="8" autocomplete="new-password">
|
||||
<p class="hint">At least 8 characters.</p>
|
||||
<input id="password" name="password" type="password" required minlength="8" maxlength="72" autocomplete="new-password">
|
||||
<p class="hint">At least 8 characters (max 72 bytes).</p>
|
||||
<label for="setup_secret">Setup secret <span class="hint">(optional, first install only)</span></label>
|
||||
<input id="setup_secret" name="setup_secret" type="password" autocomplete="off">
|
||||
<p class="hint">Only needed once to create the first admin. Leave blank otherwise.</p>
|
||||
<button type="submit" class="btn btn-primary">Create account</button>
|
||||
</form>
|
||||
<p class="switch">Already have an account? <a href="/login">Sign in</a></p>
|
||||
|
||||
@@ -7,11 +7,12 @@ From the project review. Priority order within each section.
|
||||
- [x] **Persist sessions** — Custom sqlc-backed `SessionStore` (scs API kept; no `postgresstore`).
|
||||
- [x] **Drop Dockerfile** — DigitalOcean App Platform buildpack from `go.mod`.
|
||||
- [x] **Rune-safe truncation** — Form fields truncate by runes.
|
||||
- [x] **Admin bootstrap** — `ADMIN_USERNAME` seeds first admin only when none exist; `/admin/users` for promote/demote.
|
||||
- [x] **Admin bootstrap** — One-time `ADMIN_SETUP_SECRET` on register (not username alone); `/admin/users` for promote/demote.
|
||||
- [x] **Graceful shutdown** — Signal-aware `http.Server.Shutdown` with timeouts.
|
||||
- [x] **Handler tests** — Vote HTMX, answer/hide, CSRF, session rotation via in-memory `Store` (no Postgres for web suite).
|
||||
- [x] **App Platform listen port** — Prefers `PORT`, then `LISTEN`, then `:8080`.
|
||||
- [x] **Prod DB = PlanetScale Postgres** — Required `DATABASE_URL`; DSN cleanup for PlanetScale/libpq-only params.
|
||||
- [x] **Auth hardening** — Rate limits, timing-safe login, password ≤72 bytes, logout destroys session, Secure cookies required when `PORT` is set.
|
||||
|
||||
## Docs & ops
|
||||
|
||||
@@ -20,12 +21,10 @@ From the project review. Priority order within each section.
|
||||
|
||||
## Smaller / later
|
||||
|
||||
- [ ] Rate-limit login/register (bcrypt helps; still open to brute-force).
|
||||
- [ ] Cursor pagination UI when hunt/profile lists hit their row limits.
|
||||
- [ ] Optional Postgres integration tests (`TEST_DATABASE_URL`) for sqlc SessionStore / advisory locks.
|
||||
|
||||
## Suggested order of attack
|
||||
|
||||
1. Short README (run, env, admin, App Platform + PlanetScale).
|
||||
1. Short README (run, env, admin setup secret, App Platform + PlanetScale).
|
||||
2. Migrations plan before the next schema change.
|
||||
3. Rate-limit auth endpoints.
|
||||
|
||||
Reference in New Issue
Block a user