fix(auth): longer IDs and invite codes, register limits, single-use invites (FLAW-32, FLAW-39, FLAW-40)
MindSift CI & Docker Release / Build, Test in Container & Push (push) Successful in 1m2s
MindSift CI & Docker Release / Build, Test in Container & Push (push) Successful in 1m2s
- NewID uses 96 random bits; invite codes get their own 100 bits in Base32. - Registration and forgot-password are limited per address via loginGuard. - The invite is claimed atomically before the account is created and released again when the account cannot be created. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1af3a6e90a
commit
3709282a4f
11 files changed
+220
-15
No files matched your search
@@ -168,6 +168,10 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusBadRequest, "Ungültige Anfrage")
|
||||
return
|
||||
}
|
||||
if wait := s.loginGuard.throttle("register", clientIPFrom(r), limitRegisterPerIP); wait > 0 {
|
||||
writeThrottled(w, wait)
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.Email) == "" {
|
||||
writeError(w, http.StatusBadRequest, "E-Mail-Adresse erforderlich")
|
||||
return
|
||||
@@ -212,15 +216,26 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// The invite is claimed before the account exists, so it cannot be redeemed twice at once
|
||||
var claim string
|
||||
if validInvite != nil {
|
||||
if claim, err = s.store.ClaimUserInvite(validInvite.ID); err != nil {
|
||||
writeError(w, http.StatusForbidden, err.Error())
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
user, err := s.store.CreateUser(req.Email, hash, req.DisplayName, role)
|
||||
if err != nil {
|
||||
if validInvite != nil {
|
||||
s.store.ReleaseUserInvite(validInvite.ID, claim)
|
||||
}
|
||||
writeError(w, http.StatusConflict, "E-Mail-Adresse existiert bereits")
|
||||
return
|
||||
}
|
||||
|
||||
if validInvite != nil {
|
||||
// Otherwise the code could be used again
|
||||
if err := s.store.MarkUserInviteUsed(validInvite.ID, user.ID); err != nil {
|
||||
if err := s.store.FinishUserInvite(validInvite.ID, claim, user.ID); err != nil {
|
||||
slog.Error("Einladungscode konnte nicht als benutzt markiert werden", "invite", validInvite.ID, "error", err)
|
||||
}
|
||||
if validInvite.OrganizationID != "" {
|
||||
|
||||
@@ -135,6 +135,46 @@ func (g *loginGuard) cleanupLocked(now time.Time) {
|
||||
}
|
||||
}
|
||||
|
||||
// Other endpoints that can be abused in bulk are limited per address in the same buckets (FLAW-39).
|
||||
// Every request counts here, not only failures: registration (also guessing invite codes) and
|
||||
// "forgot password" (mail flood to many addresses).
|
||||
const (
|
||||
limitRegisterPerIP = 20
|
||||
limitForgotPerIP = 10
|
||||
)
|
||||
|
||||
// throttle counts one request of this kind from the visitor's address and returns how long further
|
||||
// requests are refused (0 = this one may go ahead). Unknown addresses are not limited.
|
||||
func (g *loginGuard) throttle(kind string, ip clientIPInfo, limit int) time.Duration {
|
||||
if !g.enabled || ip.viaProxyWithoutHeader || ip.ip == "" {
|
||||
return 0
|
||||
}
|
||||
g.mu.Lock()
|
||||
defer g.mu.Unlock()
|
||||
now := g.now()
|
||||
g.cleanupLocked(now)
|
||||
key := kind + ":" + ip.ip
|
||||
b := g.buckets[key]
|
||||
if b != nil && b.lockedUntil.After(now) {
|
||||
return b.lockedUntil.Sub(now)
|
||||
}
|
||||
if b == nil || now.Sub(b.windowStart) > loginLockWindow {
|
||||
b = &loginBucket{windowStart: now}
|
||||
g.buckets[key] = b
|
||||
}
|
||||
b.count++
|
||||
if b.count >= limit {
|
||||
b.lockedUntil = now.Add(loginLockWindow)
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
func writeThrottled(w http.ResponseWriter, wait time.Duration) {
|
||||
w.Header().Set("Retry-After", strconv.Itoa(int(math.Ceil(wait.Seconds()))))
|
||||
writeError(w, http.StatusTooManyRequests,
|
||||
fmt.Sprintf("Zu viele Anfragen von dieser Adresse. Bitte versuche es in %d Minuten erneut.", int(math.Ceil(wait.Minutes()))))
|
||||
}
|
||||
|
||||
func writeLoginLocked(w http.ResponseWriter, wait time.Duration) {
|
||||
minutes := int(math.Ceil(wait.Minutes()))
|
||||
w.Header().Set("Retry-After", strconv.Itoa(int(math.Ceil(wait.Seconds()))))
|
||||
|
||||
@@ -29,6 +29,10 @@ func (s *Server) handleForgotPassword(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusServiceUnavailable, "Passwort-Zurücksetzen ist auf diesem Server nicht eingerichtet. Bitte wende dich an die Administration.")
|
||||
return
|
||||
}
|
||||
if wait := s.loginGuard.throttle("forgot", clientIPFrom(r), limitForgotPerIP); wait > 0 {
|
||||
writeThrottled(w, wait)
|
||||
return
|
||||
}
|
||||
var req struct {
|
||||
Email string `json:"email"`
|
||||
}
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"regexp"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestInviteCodeRedeemedOnlyOnce(t *testing.T) {
|
||||
srv, store, _ := setupTestServer(t)
|
||||
router := srv.Router()
|
||||
inv, err := store.CreateUserInvite("", "", "member", "", 7)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !regexp.MustCompile(`^INV-[A-Z2-7]{5}(-[A-Z2-7]{5}){3}$`).MatchString(inv.Code) {
|
||||
t.Fatalf("unexpected invite code format %q", inv.Code)
|
||||
}
|
||||
|
||||
// Two registrations race for the same code: exactly one account
|
||||
var wg sync.WaitGroup
|
||||
codes := make([]int, 2)
|
||||
for i := range codes {
|
||||
wg.Add(1)
|
||||
go func(i int) {
|
||||
defer wg.Done()
|
||||
body := `{"email":"race` + string(rune('a'+i)) + `@example.com","password":"password123","inviteCode":"` + strings.ToLower(inv.Code) + `"}`
|
||||
codes[i] = sessionCall(router, "POST", "/api/v1/auth/register", "", body, "").Code
|
||||
}(i)
|
||||
}
|
||||
wg.Wait()
|
||||
created := 0
|
||||
for _, c := range codes {
|
||||
if c == http.StatusCreated {
|
||||
created++
|
||||
}
|
||||
}
|
||||
if created != 1 {
|
||||
t.Fatalf("want exactly one account, got codes %v", codes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInviteReleasedWhenAccountFails(t *testing.T) {
|
||||
srv, store, _ := setupTestServer(t)
|
||||
router := srv.Router()
|
||||
inv, _ := store.CreateUserInvite("", "", "member", "", 7)
|
||||
_, _ = store.CreateUser("taken@example.com", "hash", "Taken", "member")
|
||||
body := func(email string) string {
|
||||
return `{"email":"` + email + `","password":"password123","inviteCode":"` + inv.Code + `"}`
|
||||
}
|
||||
if rec := sessionCall(router, "POST", "/api/v1/auth/register", "", body("taken@example.com"), ""); rec.Code != http.StatusConflict {
|
||||
t.Fatalf("existing address: want 409, got %d", rec.Code)
|
||||
}
|
||||
if rec := sessionCall(router, "POST", "/api/v1/auth/register", "", body("free@example.com"), ""); rec.Code != http.StatusCreated {
|
||||
t.Fatalf("code must still work after a failed attempt, got %d %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegisterThrottledPerAddress(t *testing.T) {
|
||||
srv, _, _ := setupTestServer(t)
|
||||
router := srv.Router()
|
||||
for i := 0; i < limitRegisterPerIP; i++ {
|
||||
rec := sessionCall(router, "POST", "/api/v1/auth/register", "", `{"email":"x@example.com","password":"password123","inviteCode":"INV-NOPE"}`, "")
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("attempt %d: want 403, got %d", i+1, rec.Code)
|
||||
}
|
||||
}
|
||||
rec := sessionCall(router, "POST", "/api/v1/auth/register", "", `{"email":"x@example.com","password":"password123","inviteCode":"INV-NOPE"}`, "")
|
||||
if rec.Code != http.StatusTooManyRequests || rec.Header().Get("Retry-After") == "" {
|
||||
t.Fatalf("want 429 after %d attempts, got %d", limitRegisterPerIP, rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestThrottleWindowAndSwitch(t *testing.T) {
|
||||
now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC)
|
||||
g := newLoginGuard(true)
|
||||
g.now = func() time.Time { return now }
|
||||
ip := clientIPInfo{ip: "203.0.113.9"}
|
||||
for i := 0; i < 3; i++ {
|
||||
if g.throttle("forgot", ip, 3) != 0 {
|
||||
t.Fatalf("request %d must pass", i+1)
|
||||
}
|
||||
}
|
||||
if g.throttle("forgot", ip, 3) == 0 {
|
||||
t.Fatal("fourth request must be refused")
|
||||
}
|
||||
if g.throttle("register", ip, 3) != 0 {
|
||||
t.Fatal("other kinds have their own bucket")
|
||||
}
|
||||
if g.throttle("forgot", clientIPInfo{ip: "198.51.100.1"}, 3) != 0 {
|
||||
t.Fatal("other addresses are not affected")
|
||||
}
|
||||
if g.throttle("forgot", clientIPInfo{ip: "172.18.0.2", viaProxyWithoutHeader: true}, 0) != 0 {
|
||||
t.Fatal("unknown visitors are not limited")
|
||||
}
|
||||
now = now.Add(loginLockWindow + time.Minute)
|
||||
if g.throttle("forgot", ip, 3) != 0 {
|
||||
t.Fatal("lock must end after the window")
|
||||
}
|
||||
if off := newLoginGuard(false); off.throttle("forgot", ip, 0) != 0 {
|
||||
t.Fatal("LOGIN_PROTECTION=false switches throttling off")
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user