diff --git a/cmd/archivmail/main.go b/cmd/archivmail/main.go index 3b98293..943bdad 100644 --- a/cmd/archivmail/main.go +++ b/cmd/archivmail/main.go @@ -161,8 +161,8 @@ func main() { } defer ldapSt.Close() - // Auth manager (with LDAP fallback) - authMgr := auth.New(users, ldapSt, jwtSecret) + // Auth manager (with LDAP fallback + TOTP AES key) + authMgr := auth.New(users, ldapSt, jwtSecret, aesKey) // API server apiCfg := config.APIConfig{ diff --git a/docs/security-audit-2026-03-18.md b/docs/security-audit-2026-03-18.md new file mode 100644 index 0000000..277d677 --- /dev/null +++ b/docs/security-audit-2026-03-18.md @@ -0,0 +1,190 @@ +## Security-Audit Bericht -- archivmail (2026-03-18) + +Auditor: Security Engineer (Code-Review) +Scope: Juengste Commits -- HKDF, Rollenhierarchie, Tenant-LDAP, Mail-ID-Validation, SMTP, Export + +--- + +### Zusammenfassung + +PASS: 27 | FAIL: 2 | WARN: 5 + +--- + +### Ergebnisse + +| # | Bereich | Pruefpunkt | Ergebnis | Details | +|---|---------|-----------|----------|---------| +| 1 | SEC-08 HKDF | Zwei verschiedene Salts fuer JWT und AES | PASS | `archivmail-jwt-v1` und `archivmail-aes-v1` korrekt in `cmd/archivmail/main.go:82,87` | +| 2 | SEC-08 HKDF | jwtSecret wird fuer auth.New() verwendet | PASS | `main.go:165`: `auth.New(users, ldapSt, jwtSecret)` | +| 3 | SEC-08 HKDF | aesKey wird fuer ldapcfg, imap, pop3 verwendet | PASS | `main.go:157,243,261`: alle drei Stores erhalten `aesKey` | +| 4 | SEC-08 HKDF | apiCfg.Secret = jwtSecret (nicht cfg.API.Secret) | PASS | `main.go:170`: `Secret: jwtSecret` -- korrekt abgeleitet | +| 5 | SEC-08 HKDF | Keine direkte Nutzung von cfg.API.Secret als Schluessel | PASS | Grep zeigt nur `main.go:80` (masterKey Zuweisung) und Kommentar -- kein Durchreichen | +| 6 | SEC-01 Rollen | roleLevel() Hierarchie korrekt | PASS | `server.go:46-55`: superadmin=5, admin=4, domain_admin=3, auditor=2, user=1. Konsistent mit `auth.go:284-293` | +| 7 | SEC-01 Rollen | handleCreateUser prueft roleLevel | PASS | `server.go:387`: `roleLevel(req.Role) >= roleLevel(sess.Role)` -- domain_admin (3) kann keinen superadmin (5) anlegen | +| 8 | SEC-01 Rollen | handleUpdateUser prueft roleLevel fuer Ziel UND neue Rolle | PASS | `server.go:464,468`: Beide Checks vorhanden (target.Role und req.Role) | +| 9 | SEC-01 Rollen | handleDeleteUser prueft roleLevel | PASS | `server.go:525`: `roleLevel(target.Role) >= roleLevel(sess.Role)` | +| 10 | SEC-01 Rollen | domain_admin kann sich nicht zu superadmin befoerdern | PASS | handleUpdateUser Zeile 468: Selbst-Update wird durch `roleLevel(target.Role) >= roleLevel(sess.Role)` in Zeile 464 blockiert (eigene Rolle = domain_admin (3) >= domain_admin (3) = true -> 403) | +| 11 | BUG-1 Fix | handleSaveTenantLDAP Allowlist | PASS | `ldap_tenants.go:452`: Allowlist `{"user": true, "auditor": true}` korrekt | +| 12 | BUG-1 Fix | handleSaveTenantLDAP prueft group_mappings | PASS | `ldap_tenants.go:457-461`: Jedes GroupMapping wird gegen Allowlist geprueft | +| 13 | BUG-1 Fix | handleAdminSaveTenantLDAP Allowlist | PASS | `ldap_tenants.go:593`: Allowlist `{"user": true, "auditor": true, "domain_admin": true}` -- korrekt, superadmin ausgeschlossen | +| 14 | BUG-1 Fix | handleAdminSaveTenantLDAP prueft group_mappings | PASS | `ldap_tenants.go:598-602`: Jedes GroupMapping wird geprueft | +| 15 | BUG-1 Fix | Leerer default_role erlaubt | PASS | `ldap_tenants.go:453`: `cfg.DefaultRole != ""` -- leerer String wird nicht geprueft, also erlaubt | +| 16 | Login LDAP | Reihenfolge: lokal -> tenant LDAP -> global LDAP | PASS | `auth.go:66-155`: (1) local VerifyPassword (2) tenant LDAP ab Zeile 74 (3) global LDAP ab Zeile 119 | +| 17 | Login LDAP | Globales LDAP kann Tenant-LDAP nicht ueberschreiben | PASS | Tenant-LDAP (Step 2) wird vor globalem LDAP (Step 3) geprueft. Bei Erfolg in Step 2 wird sofort returned | +| 18 | SEC-22 | handleGetMail: isValidMailID vor Store-Zugriff | PASS | `server.go:908-912` | +| 19 | SEC-22 | handleGetAttachment: isValidMailID vor Store-Zugriff | PASS | `server.go:997-1001` | +| 20 | SEC-22 | handleGetRaw: isValidMailID vor Store-Zugriff | PASS | `server.go:1060-1064` | +| 21 | SEC-22 | handleExportPDF: isValidMailID vor Store-Zugriff | PASS | `export.go:331-334` | +| 22 | SEC-22 | handleExportZIP: isValidMailID fuer alle IDs | PASS | `export.go:413-418` | +| 23 | SEC-22 | Regex als Paket-Variable kompiliert | PASS | `server.go:37`: `var mailIDRegex = regexp.MustCompile(...)` -- einmalig kompiliert | +| 24 | SEC-26 | isAllowed: leere AllowedIPs -> false | PASS | `smtpd.go:317-318`: `len(d.cfg.AllowedIPs) == 0` returns false (fail-closed) | +| 25 | SEC-26 | Kommentar in config.go vorhanden | PASS | `config.go:59-61`: Erklaert `allowed_ips: ["0.0.0.0/0", "::/0"]` | +| 26 | SEC-05 | handleExportPDF: Tenant-Isolation | PASS | `export.go:351-357`: Prueft `sess.TenantID != nil` und vergleicht mit Mail-Tenant | +| 27 | SEC-05 | handleExportZIP: Tenant-Isolation | PASS | `export.go:423-439`: Laedt alle IDs via GetAllIDsByTenant und prueft jede angeforderte ID | +| 28 | SEC-05 | superadmin (TenantID==nil) sieht alle Mails | PASS | Alle Tenant-Checks starten mit `if sess.TenantID != nil` -- superadmin hat nil -> kein Check -> Vollzugriff | +| 29 | Tenant-LDAP Auth | /api/tenant/ldap ohne Session -> 401 | PASS | `ldap_tenants.go:396`: `s.authAdmin(...)` = `authMiddleware(tenantMiddleware(requireRole(RoleDomainAdmin, ...)))` -- authMiddleware blockt ohne Token | +| 30 | Tenant-LDAP Auth | user-Rolle kann /api/tenant/ldap nicht aufrufen | PASS | requireRole(RoleDomainAdmin, ...) -> HasRole("user", "domain_admin") = false (1 < 3) -> HTTP 403 | +| 31 | Tenant-LDAP Auth | GET /api/tenant/ldap gibt bind_password NICHT im Klartext zurueck | PASS | `handleGetTenantLDAP` ruft `tenantLdapStore.Get()` auf -> `tenant_store.go:77-78`: Password wird zu "..." maskiert | +| 32 | SEC-01 Hierarchie | Hierarchie-Diskrepanz Spec vs. Code | WARN | Audit-Aufgabe nennt `superadmin=5 > domain_admin=4 > admin=3`, Code hat `admin=4 > domain_admin=3`. Code-Hierarchie ist logisch korrekt (globaler admin > tenant domain_admin). Spec-Text im Audit-Auftrag weicht ab. | +| 33 | Allgemein | SQL Injection | PASS | Keine String-Konkatenation in SQL-Queries gefunden. Alle Stores verwenden pgx parameterized queries | +| 34 | Allgemein | Fehlerausgaben mit err.Error() | FAIL | Siehe Finding F-01 | +| 35 | Allgemein | Audit-Log fuer sicherheitsrelevante Aktionen | WARN | Siehe Finding W-01 | + +--- + +### Kritische Findings (FAIL) + +#### F-01: Interne Fehlerdetails an Client (Severity: MEDIUM, Priority: P2) + +**Datei:** `internal/api/server.go` +**Zeilen:** 407, 480, 535, 1541, 1917 + +Mehrere Handler geben `err.Error()` direkt an den HTTP-Client weiter: + +- Zeile 407 (`handleCreateUser`): `writeError(w, http.StatusBadRequest, err.Error())` +- Zeile 480 (`handleUpdateUser`): `writeError(w, http.StatusBadRequest, err.Error())` +- Zeile 535 (`handleDeleteUser`): `writeError(w, http.StatusInternalServerError, err.Error())` +- Zeile 1541: `writeError(w, http.StatusConflict, err.Error())` +- Zeile 1917: `"could not write jail.local: "+err.Error()` + +Ebenso in `internal/api/upload.go:56`: `"multipart parse failed: "+err.Error()` + +**Risiko:** Interne Implementierungsdetails, Datenbank-Fehlermeldungen oder Dateipfade koennten an Angreifer weitergegeben werden. Dies erleichtert Reconnaissance. + +**Fix-Vorschlag:** Generische Fehlermeldungen an den Client senden, Details nur ins Server-Log schreiben: +```go +s.logger.Error("create user failed", "err", err) +writeError(w, http.StatusBadRequest, "user creation failed") +``` + +--- + +#### F-02: Login-Handler gibt err.Error() im Audit-Detail weiter (Severity: LOW, Priority: P3) + +**Datei:** `internal/api/server.go` +**Zeile:** 247 + +```go +Detail: err.Error(), +``` + +Der Audit-Log-Eintrag bei fehlgeschlagenem Login enthaelt den vollstaendigen Fehlertext. Dies ist fuer das Audit-Log akzeptabel (intern), aber der gleiche `err.Error()` koennte bei unsachgemaesser Audit-Log-Exposition Details verraten. In Kombination mit dem Audit-API-Endpoint (`GET /api/audit`, zugaenglich fuer `auditor`-Rolle) koennten LDAP-Fehlermeldungen mit internen Hostnamen/Ports sichtbar werden. + +**Fix-Vorschlag:** Fehlerdetails im Audit-Log auf vordefinierte Kategorien beschraenken: `"invalid_password"`, `"user_not_found"`, `"ldap_error"`. + +--- + +### Warnungen (WARN) + +#### W-01: Fehlende Audit-Logs bei einigen sicherheitsrelevanten Aktionen + +**Betroffene Handler:** +- `handleUpdateTenant` (`ldap_tenants.go:247-289`): Kein Audit-Log-Eintrag bei Tenant-Update. +- `handleAddTenantDomain` / `handleRemoveTenantDomain`: Kein Audit-Log-Eintrag. +- `handleGetAttachment`, `handleGetRaw`: Kein Audit-Log fuer einzelne Zugriffe (nur Export wird geloggt). + +**Empfehlung:** Audit-Log-Eintraege fuer Tenant-Aenderungen und Domain-Zuweisungen hinzufuegen. Mail-Lesezugriffe sind optional, aber fuer GoBD-Compliance empfehlenswert. + +--- + +#### W-02: Cookie Secure-Flag nicht gesetzt + +**Datei:** `internal/api/server.go` +**Zeile:** 269 + +```go +// Secure: true -- enable when TLS is terminated at this server +``` + +Das `Secure`-Flag ist auskommentiert. Im On-Premise-Deployment ohne TLS-Terminierung am Go-Server ist dies verstaendlich, aber wenn ein Reverse-Proxy (nginx/caddy) TLS terminiert, koennte das Cookie ueber eine unverschluesselte Verbindung gesendet werden. + +**Empfehlung:** Konfigurierbar machen oder automatisch setzen wenn hinter TLS-Proxy (`X-Forwarded-Proto: https`). + +--- + +#### W-03: X-Forwarded-For Header nicht validiert + +**Datei:** `internal/api/server.go` +**Zeile:** 880-884 + +```go +func remoteIP(r *http.Request) string { + if fwd := r.Header.Get("X-Forwarded-For"); fwd != "" { + return strings.Split(fwd, ",")[0] + } + return r.RemoteAddr +} +``` + +Ein Angreifer kann den `X-Forwarded-For`-Header faelschen, um eine andere IP in Audit-Logs und Rate-Limiting zu injizieren. Dies ermoeglicht: +1. Rate-Limit-Umgehung beim Login (verschiedene gefaelschte IPs) +2. Falsche IP-Adressen in Audit-Logs + +**Empfehlung:** Nur dem aeussersten Proxy-Hop vertrauen oder die Anzahl vertrauenswuerdiger Proxies konfigurierbar machen. Alternativ `r.RemoteAddr` verwenden wenn kein Proxy konfiguriert ist. + +--- + +#### W-04: Attachment-Filename ohne Sanitierung im Content-Disposition + +**Datei:** `internal/api/server.go` +**Zeile:** 1052 + +```go +w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, filename)) +``` + +Der Attachment-Filename kommt direkt aus der geparsten E-Mail. Ein boesartiger Absender koennte einen Dateinamen mit `"` oder Newlines senden, was zu Header-Injection fuehren koennte. + +Ebenso in `export.go:377` (dort ist die ID gekuerzt -- sicher) und `server.go:1098` (ID-basiert -- sicher). + +**Empfehlung:** Dateinamen sanitieren: Nur alphanumerische Zeichen, Punkt, Bindestrich und Unterstrich erlauben. Alternativ RFC 6266 `filename*` mit Encoding verwenden. + +--- + +#### W-05: HKDF ohne Info-Parameter + +**Datei:** `cmd/archivmail/main.go` +**Zeilen:** 82, 87 + +```go +hkdf.New(sha256.New, masterKey, []byte("archivmail-jwt-v1"), nil) +``` + +Der HKDF-Aufruf nutzt den Salt-Parameter zur Schluesseltrennung, aber der `info`-Parameter ist `nil`. Gemaess RFC 5869 ist der `info`-Parameter fuer die Kontextbindung vorgesehen, waehrend `salt` fuer die Randomisierung dient. Funktional ist dies korrekt und sicher, da die verschiedenen Salt-Werte eine wirksame Schluesseltrennung gewaehrleisten. Es waere jedoch konzeptionell sauberer, den Kontext (`archivmail-jwt-v1`) als `info` und einen festen Salt zu verwenden. + +**Empfehlung:** Kein sofortiger Handlungsbedarf -- die aktuelle Implementierung ist kryptographisch sicher. Bei einem zukuenftigen Refactoring koennte der Kontext in den `info`-Parameter verschoben werden. + +--- + +### Geprueft und fuer gut befunden + +1. **HKDF-Schluessel-Trennung (SEC-08):** Korrekt implementiert mit zwei unabhaengigen Ableitungen. +2. **Rollenhierarchie (SEC-01):** Konsistent in server.go und auth.go. Privilege-Escalation-Checks in allen drei User-Management-Handlern vorhanden. +3. **BUG-1 Fix (Tenant-LDAP Allowlists):** Beide Endpunkte (domain_admin und superadmin) pruefen sowohl `default_role` als auch `group_mappings[].role`. +4. **Login-Reihenfolge:** Korrekt: lokal -> tenant LDAP -> global LDAP. +5. **Mail-ID-Validierung (SEC-22):** Alle 5 Handler pruefen vor dem Store-Zugriff. Regex einmalig kompiliert. +6. **SMTP Fail-Closed (SEC-26):** Leere Allowlist blockiert alle Verbindungen. +7. **Export Tenant-Isolation (SEC-05):** PDF und ZIP Export pruefen Tenant-Zugehoerigkeit. Superadmin hat Vollzugriff. +8. **Tenant-LDAP Endpunkte:** Authentifizierung und Rollenpruefung korrekt. Passwort wird maskiert. +9. **SQL-Injection:** Keine String-Konkatenation in Queries gefunden. diff --git a/features/INDEX.md b/features/INDEX.md index 79bbb02..93ad804 100644 --- a/features/INDEX.md +++ b/features/INDEX.md @@ -27,7 +27,7 @@ | PROJ-13 | REST API für externe CRM-Anbindung | In Progress | [PROJ-13](PROJ-13-rest-api-crm.md) | 2026-03-13 | | PROJ-14 | E-Mail-Import: POP3-Verbindung | Deployed | [PROJ-14](PROJ-14-import-pop3.md) | 2026-03-13 | | PROJ-15 | CLI Import & Export (archivmail-User) | Deployed | [PROJ-15](PROJ-15-cli-import-export.md) | 2026-03-13 | -| PROJ-16 | LDAP / Active Directory Anbindung | In Progress | [PROJ-16](PROJ-16-ldap-active-directory.md) | 2026-03-13 | +| PROJ-16 | LDAP / Active Directory Anbindung | Deployed | [PROJ-16](PROJ-16-ldap-active-directory.md) | 2026-03-13 | | PROJ-17 | Admin Dashboard – Systemauslastung & Archiv-Übersicht | Deployed | [PROJ-17](PROJ-17-system-dashboard.md) | 2026-03-14 | | PROJ-18 | E-Mail Integritätsprüfung | Deployed | [PROJ-18](PROJ-18-integritaetspruefung.md) | 2026-03-14 | @@ -35,8 +35,9 @@ | PROJ-20 | Nutzer-Löschung & E-Mail-Verbleib (GoBD-konform) | Deployed | [PROJ-20](PROJ-20-nutzer-loeschung.md) | 2026-03-17 | | PROJ-21 | Multi-Mandanten-Fähigkeit (Multi-Tenancy) | In Progress | [PROJ-21](PROJ-21-multi-tenancy.md) | 2026-03-17 | | PROJ-22 | LDAP / AD – Web-GUI Konfiguration & Test | Deployed | [PROJ-22](PROJ-22-ldap-webgui.md) | 2026-03-17 | -| PROJ-23 | Pro-Mandant LDAP / Active Directory (Multi-Tenant Phase B) | Planned | [PROJ-23](PROJ-23-tenant-ldap-pro-mandant.md) | 2026-03-17 | +| PROJ-23 | Pro-Mandant LDAP / Active Directory (Multi-Tenant Phase B) | In Progress | [PROJ-23](PROJ-23-tenant-ldap-pro-mandant.md) | 2026-03-17 | +| PROJ-24 | TOTP Zwei-Faktor-Authentifizierung (2FA) | Planned | [PROJ-24](PROJ-24-totp-zwei-faktor.md) | 2026-03-18 | -## Next Available ID: PROJ-24 +## Next Available ID: PROJ-25 diff --git a/features/PROJ-16-ldap-active-directory.md b/features/PROJ-16-ldap-active-directory.md index 30067ba..d91edc5 100644 --- a/features/PROJ-16-ldap-active-directory.md +++ b/features/PROJ-16-ldap-active-directory.md @@ -1,7 +1,7 @@ --- id: PROJ-16 title: LDAP / Active Directory Anbindung -status: In Progress +status: Deployed priority: P1 created: 2026-03-13 --- diff --git a/features/PROJ-21-multi-tenancy.md b/features/PROJ-21-multi-tenancy.md index d9bfcb5..cc232a2 100644 --- a/features/PROJ-21-multi-tenancy.md +++ b/features/PROJ-21-multi-tenancy.md @@ -41,6 +41,20 @@ created: 2026-03-17 **Offene Phasen:** Phase 4 (Xapian per-tenant Index-Filter) +## Phase 4 -- QA-Bericht (2026-03-18, Code-Review) + +### Gesamtbewertung: APPROVED -- alle 5 Pruefpunkte bestanden + +| # | Pruefpunkt | Ergebnis | Details | +|---|-----------|----------|---------| +| X1 | Pro-Mandant-Verzeichnis tenant-/ | PASS | `tenant_manager.go:69` | +| X2 | ForTenant(nil) nil-safe | PASS | `tenant_manager.go:47` | +| X3 | handleSearch nutzt tenant_id aus Session | PASS | `server.go:627-633` | +| X4 | Thread-Safety (RWMutex + Channel) | PASS | `tenant_manager.go:16,53-62` / `tenant_worker.go:25` | +| X5 | Close() auf allen Indexen | PASS | `tenant_manager.go:91-110`, `main.go:121` | + +Dateien: `internal/index/tenant_manager.go`, `internal/index/tenant_worker.go`, `internal/api/server.go`, `cmd/archivmail/main.go` + ## Ziel Das System soll mehrere Kunden (Mandanten/Tenants) auf einer einzigen Instanz betreiben können. Jeder Mandant verwaltet seine eigene Domain, seine eigenen Nutzer und sieht ausschließlich seine eigenen E-Mails. Kunden-Admins können ihre Domain selbst verwalten und weitere Domain-Admins ernennen. diff --git a/go.mod b/go.mod index fd298f8..47ba3d4 100644 --- a/go.mod +++ b/go.mod @@ -9,12 +9,14 @@ require ( github.com/emersion/go-smtp v0.24.0 github.com/golang-jwt/jwt/v5 v5.2.1 github.com/jackc/pgx/v5 v5.6.0 + github.com/pquerna/otp v1.4.0 golang.org/x/crypto v0.23.0 gopkg.in/yaml.v3 v3.0.1 ) require ( github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 // indirect + github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc // indirect github.com/emersion/go-message v0.18.2 // indirect github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 // indirect github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 0dbc2af..576aad1 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -91,7 +91,7 @@ func newTestEnv(t *testing.T) *testEnv { users.Create(userstore.CreateUserRequest{Username: "auditor", Email: "auditor@x.com", Password: "auditorpass", Role: userstore.RoleAuditor}) users.Create(userstore.CreateUserRequest{Username: "user1", Email: "user1@x.com", Password: "userpass", Role: userstore.RoleUser}) - authMgr := auth.New(users, nil, "test-secret-must-be-long-enough-32") + authMgr := auth.New(users, nil, "test-secret-must-be-long-enough-32", "0000000000000000000000000000000000000000000000000000000000000000") cfg := config.APIConfig{Bind: ":18080", Secret: "test-secret-must-be-long-enough-32"} srv := api.New(cfg, store, idx, authMgr, users, audlog, logger) diff --git a/internal/api/server.go b/internal/api/server.go index 1d91d45..22e9c55 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -194,6 +194,13 @@ func (s *Server) routes() { s.mux.HandleFunc("POST /api/pop3/test", s.auth(s.handleTestPop3)) s.mux.HandleFunc("POST /api/pop3/{id}/import", s.auth(s.handleStartPop3Import)) s.mux.HandleFunc("GET /api/pop3/{id}/progress", s.auth(s.handlePop3Progress)) + + // PROJ-24: TOTP 2FA routes + s.mux.HandleFunc("GET /api/auth/totp/setup", s.auth(s.handleTOTPSetupGet)) + s.mux.HandleFunc("POST /api/auth/totp/setup", s.auth(s.handleTOTPSetupPost)) + s.mux.HandleFunc("DELETE /api/auth/totp", s.auth(s.handleTOTPDisable)) + s.mux.HandleFunc("POST /api/auth/totp", s.handleTOTPLogin) // no auth middleware — uses pending token + s.mux.HandleFunc("POST /api/admin/users/{id}/totp/reset", s.authAdmin(s.handleTOTPReset)) } // ServeHTTP implements http.Handler. @@ -236,7 +243,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { return } - token, user, err := s.authMgr.Login(req.Username, req.Password) + token, user, totpRequired, err := s.authMgr.Login(req.Username, req.Password) if err != nil { _ = s.users.RecordLoginAttempt(req.Username, remoteIP(r)) s.audlog.Log(audit.Entry{ @@ -244,12 +251,28 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { Username: req.Username, IPAddress: remoteIP(r), Success: false, - Detail: err.Error(), + Detail: classifyLoginError(err), }) writeError(w, http.StatusUnauthorized, "invalid credentials") return } + // PROJ-24: If TOTP is enabled, return a pending token instead of a full session. + if totpRequired { + s.audlog.Log(audit.Entry{ + EventType: audit.EventLogin, + Username: user.Username, + IPAddress: remoteIP(r), + Success: true, + Detail: "totp_pending", + }) + writeJSON(w, http.StatusAccepted, map[string]interface{}{ + "totp_required": true, + "session_token": token, + }) + return + } + _ = s.users.UpdateLastLogin(user.ID) s.audlog.Log(audit.Entry{ @@ -404,7 +427,8 @@ func (s *Server) handleCreateUser(w http.ResponseWriter, r *http.Request) { TenantID: tenantID, }) if err != nil { - writeError(w, http.StatusBadRequest, err.Error()) + s.logger.Error("create user failed", "err", err) + writeError(w, http.StatusBadRequest, "user creation failed") return } @@ -477,7 +501,8 @@ func (s *Server) handleUpdateUser(w http.ResponseWriter, r *http.Request) { Password: req.Password, }) if err != nil { - writeError(w, http.StatusBadRequest, err.Error()) + s.logger.Error("update user failed", "err", err) + writeError(w, http.StatusBadRequest, "user update failed") return } @@ -532,7 +557,8 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusConflict, "cannot delete the last active admin") return } - writeError(w, http.StatusInternalServerError, err.Error()) + s.logger.Error("delete user failed", "err", err) + writeError(w, http.StatusInternalServerError, "user deletion failed") return } @@ -876,9 +902,45 @@ func tenantFromCtx(ctx context.Context) *int64 { return v } +// sanitizeFilename strips characters that could be used for HTTP header injection +// (quotes, newlines, control chars) from attachment filenames coming from parsed +// e-mails. Only alphanumerics, spaces, dots, hyphens, and underscores are kept. +func sanitizeFilename(name string) string { + var b strings.Builder + for _, r := range name { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || + r == '.' || r == '-' || r == '_' || r == ' ' { + b.WriteRune(r) + } + } + return b.String() +} + +// classifyLoginError maps internal login errors to safe audit-log categories. +// Raw error messages must not be stored in audit logs since auditor-role +// users can read them via GET /api/audit and internal details (LDAP hostnames, +// port numbers, etc.) would be exposed. +func classifyLoginError(err error) string { + if err == nil { + return "" + } + msg := err.Error() + switch { + case strings.Contains(msg, "not found"), strings.Contains(msg, "invalid password"), + strings.Contains(msg, "invalid credentials"): + return "invalid_password" + case strings.Contains(msg, "ldap"), strings.Contains(msg, "LDAP"): + return "ldap_error" + case strings.Contains(msg, "disabled"), strings.Contains(msg, "inactive"): + return "account_disabled" + default: + return "unknown" + } +} + func remoteIP(r *http.Request) string { if fwd := r.Header.Get("X-Forwarded-For"); fwd != "" { - return strings.Split(fwd, ",")[0] + return strings.TrimSpace(strings.Split(fwd, ",")[0]) } return r.RemoteAddr } @@ -1043,7 +1105,7 @@ func (s *Server) handleGetAttachment(w http.ResponseWriter, r *http.Request) { } a := pm.Attachments[idx] - filename := a.Filename + filename := sanitizeFilename(a.Filename) if filename == "" { filename = fmt.Sprintf("attachment-%d", idx) } @@ -1538,7 +1600,8 @@ func (s *Server) handleSyncNow(w http.ResponseWriter, r *http.Request) { } if err := s.imapScheduler.TriggerSync(r.Context(), id); err != nil { - writeError(w, http.StatusConflict, err.Error()) + s.logger.Error("trigger sync failed", "err", err) + writeError(w, http.StatusConflict, "sync already running or failed to start") return } @@ -1914,7 +1977,8 @@ func (s *Server) handleSecurityFix(w http.ResponseWriter, r *http.Request) { if _, err := os.Stat(jailPath); os.IsNotExist(err) { jailConf := "[sshd]\nenabled = true\nmaxretry = 5\nbantime = 3600\nfindtime = 600\n" if err := os.WriteFile(jailPath, []byte(jailConf), 0644); err != nil { - writeError(w, http.StatusInternalServerError, "could not write jail.local: "+err.Error()) + s.logger.Error("could not write jail.local", "err", err) + writeError(w, http.StatusInternalServerError, "security config update failed") return } } diff --git a/internal/api/totp_handlers.go b/internal/api/totp_handlers.go new file mode 100644 index 0000000..4022849 --- /dev/null +++ b/internal/api/totp_handlers.go @@ -0,0 +1,262 @@ +package api + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "strconv" + + "github.com/archivmail/internal/audit" + "github.com/archivmail/internal/auth" +) + +// ── PROJ-24: TOTP 2FA Handlers ─────────────────────────────────────────── + +// handleTOTPSetupGet generates a new TOTP secret and QR code for the current user. +// GET /api/auth/totp/setup +func (s *Server) handleTOTPSetupGet(w http.ResponseWriter, r *http.Request) { + sess := sessionFromCtx(r.Context()) + if sess.UserID == 0 { + writeError(w, http.StatusUnauthorized, "not authenticated") + return + } + + secret, otpauthURL, qrPNG, err := auth.GenerateSecret(sess.Username, "archivmail") + if err != nil { + s.logger.Error("totp setup: generate secret failed", "err", err) + writeError(w, http.StatusInternalServerError, "failed to generate TOTP secret") + return + } + + // Encrypt the secret with AES-256-GCM before storing + encryptedSecret, err := s.authMgr.EncryptAES([]byte(secret)) + if err != nil { + s.logger.Error("totp setup: encrypt secret failed", "err", err) + writeError(w, http.StatusInternalServerError, "failed to encrypt TOTP secret") + return + } + + // Store encrypted secret in DB (not yet activated) + if err := s.users.SetTOTPSecret(r.Context(), sess.UserID, encryptedSecret); err != nil { + s.logger.Error("totp setup: store secret failed", "err", err) + writeError(w, http.StatusInternalServerError, "failed to store TOTP secret") + return + } + + resp := map[string]interface{}{ + "secret": secret, + "otpauth_url": otpauthURL, + } + if len(qrPNG) > 0 { + resp["qr_code"] = base64.StdEncoding.EncodeToString(qrPNG) + } + + writeJSON(w, http.StatusOK, resp) +} + +// handleTOTPSetupPost confirms TOTP setup by verifying a code, then activates TOTP. +// POST /api/auth/totp/setup { "code": "123456" } +func (s *Server) handleTOTPSetupPost(w http.ResponseWriter, r *http.Request) { + sess := sessionFromCtx(r.Context()) + if sess.UserID == 0 { + writeError(w, http.StatusUnauthorized, "not authenticated") + return + } + + var req struct { + Code string `json:"code"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.Code == "" { + writeError(w, http.StatusBadRequest, "missing or invalid code") + return + } + + // Load encrypted secret from DB + encSecret, _, err := s.users.GetTOTPSecret(r.Context(), sess.UserID) + if err != nil || len(encSecret) == 0 { + writeError(w, http.StatusBadRequest, "no TOTP secret found, run setup first") + return + } + + // Decrypt + plainSecret, err := s.authMgr.DecryptAESForHandler(encSecret) + if err != nil { + s.logger.Error("totp setup confirm: decrypt failed", "err", err) + writeError(w, http.StatusInternalServerError, "failed to decrypt TOTP secret") + return + } + + // Validate code + if !auth.ValidateTOTP(string(plainSecret), req.Code) { + writeError(w, http.StatusBadRequest, "invalid TOTP code") + return + } + + // Activate TOTP + if err := s.users.EnableTOTP(r.Context(), sess.UserID); err != nil { + s.logger.Error("totp setup confirm: enable failed", "err", err) + writeError(w, http.StatusInternalServerError, "failed to enable TOTP") + return + } + + s.audlog.Log(audit.Entry{ + EventType: audit.EventUserMgmt, + Username: sess.Username, + IPAddress: remoteIP(r), + Detail: "totp_enabled", + Success: true, + }) + + writeJSON(w, http.StatusOK, map[string]bool{"ok": true}) +} + +// handleTOTPDisable deactivates TOTP for the current user (requires a valid code). +// DELETE /api/auth/totp { "code": "123456" } +func (s *Server) handleTOTPDisable(w http.ResponseWriter, r *http.Request) { + sess := sessionFromCtx(r.Context()) + if sess.UserID == 0 { + writeError(w, http.StatusUnauthorized, "not authenticated") + return + } + + var req struct { + Code string `json:"code"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.Code == "" { + writeError(w, http.StatusBadRequest, "missing or invalid code") + return + } + + // Load and decrypt secret + encSecret, enabled, err := s.users.GetTOTPSecret(r.Context(), sess.UserID) + if err != nil || !enabled || len(encSecret) == 0 { + writeError(w, http.StatusBadRequest, "TOTP is not enabled") + return + } + + plainSecret, err := s.authMgr.DecryptAESForHandler(encSecret) + if err != nil { + s.logger.Error("totp disable: decrypt failed", "err", err) + writeError(w, http.StatusInternalServerError, "failed to decrypt TOTP secret") + return + } + + if !auth.ValidateTOTP(string(plainSecret), req.Code) { + writeError(w, http.StatusBadRequest, "invalid TOTP code") + return + } + + if err := s.users.DisableTOTP(r.Context(), sess.UserID); err != nil { + s.logger.Error("totp disable: failed", "err", err) + writeError(w, http.StatusInternalServerError, "failed to disable TOTP") + return + } + + s.audlog.Log(audit.Entry{ + EventType: audit.EventUserMgmt, + Username: sess.Username, + IPAddress: remoteIP(r), + Detail: "totp_disabled", + Success: true, + }) + + writeJSON(w, http.StatusOK, map[string]bool{"ok": true}) +} + +// handleTOTPLogin completes a TOTP-pending login by validating the code. +// POST /api/auth/totp { "session_token": "...", "code": "123456" } +func (s *Server) handleTOTPLogin(w http.ResponseWriter, r *http.Request) { + var req struct { + SessionToken string `json:"session_token"` + Code string `json:"code"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil || req.SessionToken == "" || req.Code == "" { + writeError(w, http.StatusBadRequest, "missing session_token or code") + return + } + + token, user, err := s.authMgr.ValidateTOTPLogin(req.SessionToken, req.Code) + if err != nil { + s.audlog.Log(audit.Entry{ + EventType: audit.EventLogin, + IPAddress: remoteIP(r), + Success: false, + Detail: "totp_login_failed: " + err.Error(), + }) + writeError(w, http.StatusUnauthorized, "invalid TOTP code or expired session") + return + } + + _ = s.users.UpdateLastLogin(user.ID) + + s.audlog.Log(audit.Entry{ + EventType: audit.EventLogin, + Username: user.Username, + IPAddress: remoteIP(r), + Success: true, + Detail: "totp_login_completed", + }) + + http.SetCookie(w, &http.Cookie{ + Name: sessionCookieName, + Value: token, + Path: "/", + MaxAge: 8 * 3600, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + }) + + writeJSON(w, http.StatusOK, map[string]interface{}{ + "user": map[string]interface{}{ + "id": user.ID, + "username": user.Username, + "email": user.Email, + "role": user.Role, + }, + }) +} + +// handleTOTPReset allows an admin to reset TOTP for a user. +// POST /api/admin/users/{id}/totp/reset +func (s *Server) handleTOTPReset(w http.ResponseWriter, r *http.Request) { + id, err := strconv.ParseInt(r.PathValue("id"), 10, 64) + if err != nil { + writeError(w, http.StatusBadRequest, "invalid user id") + return + } + + sess := sessionFromCtx(r.Context()) + + // Fetch target user + target, err := s.users.GetByID(id) + if err != nil { + writeError(w, http.StatusNotFound, "user not found") + return + } + + // SEC-02: Tenant isolation — domain_admin can only reset TOTP for users in their own tenant. + if sess.TenantID != nil { + if target.TenantID == nil || *target.TenantID != *sess.TenantID { + writeError(w, http.StatusForbidden, "access denied") + return + } + } + + // Reset TOTP + if err := s.users.ResetTOTP(r.Context(), id, sess.Username); err != nil { + s.logger.Error("totp reset: failed", "err", err, "target_user", id, "admin", sess.Username) + writeError(w, http.StatusInternalServerError, "failed to reset TOTP") + return + } + + s.audlog.Log(audit.Entry{ + EventType: audit.EventUserMgmt, + Username: sess.Username, + IPAddress: remoteIP(r), + Detail: fmt.Sprintf("totp_reset_by_admin: TOTP reset by %s for user %s (id=%d)", sess.Username, target.Username, id), + Success: true, + }) + + writeJSON(w, http.StatusOK, map[string]bool{"ok": true}) +} diff --git a/internal/api/upload.go b/internal/api/upload.go index ed3eb63..c5b9939 100644 --- a/internal/api/upload.go +++ b/internal/api/upload.go @@ -53,7 +53,8 @@ type uploadJobSnapshot struct { func (s *Server) handleUpload(w http.ResponseWriter, r *http.Request) { // 512 MB max total upload if err := r.ParseMultipartForm(512 << 20); err != nil { - writeError(w, http.StatusBadRequest, "multipart parse failed: "+err.Error()) + s.logger.Error("multipart parse failed", "err", err) + writeError(w, http.StatusBadRequest, "multipart parse failed") return } diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 1573c65..a7f9d1b 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -2,10 +2,13 @@ package auth import ( "context" + "crypto/aes" + "crypto/cipher" "crypto/rand" "encoding/hex" "errors" "fmt" + "io" "strings" "time" @@ -37,17 +40,21 @@ type Manager struct { store *userstore.Store ldapStore *ldapcfg.Store jwtSecret []byte + aesKey []byte // PROJ-24: AES-256-GCM key for TOTP secret encryption tenantLdapStore *ldapcfg.TenantStore // PROJ-23: per-tenant LDAP config tenantLookup TenantDomainLookup // PROJ-23: domain -> tenant_id resolution } // New creates a new auth Manager. // ldapStore may be nil; in that case LDAP fallback is disabled. -func New(store *userstore.Store, ldapStore *ldapcfg.Store, jwtSecret string) *Manager { +// aesKey is the hex-encoded AES-256 key for encrypting TOTP secrets. +func New(store *userstore.Store, ldapStore *ldapcfg.Store, jwtSecret string, aesKey string) *Manager { + aesKeyBytes, _ := hex.DecodeString(aesKey) return &Manager{ store: store, ldapStore: ldapStore, jwtSecret: []byte(jwtSecret), + aesKey: aesKeyBytes, } } @@ -61,11 +68,18 @@ func (m *Manager) SetTenantLDAP(tenantLdapStore *ldapcfg.TenantStore, tenantLook // Login verifies credentials and returns a signed JWT token. // It first attempts a local password check. If that fails and LDAP is // configured and enabled, it falls back to LDAP authentication. -func (m *Manager) Login(username, password string) (string, *userstore.User, error) { +// If the user has TOTP enabled, totpRequired is true and the token is a +// short-lived pending token that can only be used with ValidateTOTPLogin. +func (m *Manager) Login(username, password string) (token string, user *userstore.User, totpRequired bool, err error) { // 1. Try local authentication first. - user, err := m.store.VerifyPassword(username, password) + user, err = m.store.VerifyPassword(username, password) if err == nil { - return m.issueToken(user) + if user.TOTPEnabled { + t, e := m.issuePendingTOTPToken(user) + return t, user, true, e + } + t, u, e := m.issueToken(user) + return t, u, false, e } // 2. PROJ-23: Per-tenant LDAP — checked first so tenant config takes priority @@ -107,7 +121,12 @@ func (m *Manager) Login(username, password string) (string, *userstore.User, err } ldapUser, upsertErr := m.store.UpsertLDAPUser(username, email, role, tenantID) if upsertErr == nil { - return m.issueToken(ldapUser) + if ldapUser.TOTPEnabled { + t, e := m.issuePendingTOTPToken(ldapUser) + return t, ldapUser, true, e + } + t, u, e := m.issueToken(ldapUser) + return t, u, false, e } } } @@ -148,13 +167,18 @@ func (m *Manager) Login(username, password string) (string, *userstore.User, err } ldapUser, upsertErr := m.store.UpsertLDAPUser(username, email, role, nil) if upsertErr == nil { - return m.issueToken(ldapUser) + if ldapUser.TOTPEnabled { + t, e := m.issuePendingTOTPToken(ldapUser) + return t, ldapUser, true, e + } + t, u, e := m.issueToken(ldapUser) + return t, u, false, e } } } } - return "", nil, fmt.Errorf("auth: login: invalid credentials") + return "", nil, false, fmt.Errorf("auth: login: invalid credentials") } // issueToken signs a JWT for the given user and returns the token string. @@ -186,6 +210,99 @@ func (m *Manager) issueToken(user *userstore.User) (string, *userstore.User, err return signed, user, nil } +// issuePendingTOTPToken issues a short-lived JWT (5 min) that signals TOTP is required. +// This token MUST NOT be accepted as a full auth token — only for /api/auth/totp. +func (m *Manager) issuePendingTOTPToken(user *userstore.User) (string, error) { + jti := generateJTI() + now := time.Now() + + claims := jwt.MapClaims{ + "sub": user.Username, + "uid": user.ID, + "jti": jti, + "iat": now.Unix(), + "exp": now.Add(5 * time.Minute).Unix(), + "totp_pending": true, + } + + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + signed, err := token.SignedString(m.jwtSecret) + if err != nil { + return "", fmt.Errorf("auth: sign pending totp token: %w", err) + } + return signed, nil +} + +// ValidateTOTPLogin validates a pending TOTP token and TOTP code, then issues a full JWT. +func (m *Manager) ValidateTOTPLogin(pendingToken, code string) (string, *userstore.User, error) { + // Parse the pending token + token, err := jwt.Parse(pendingToken, func(t *jwt.Token) (interface{}, error) { + if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("auth: unexpected signing method: %v", t.Header["alg"]) + } + return m.jwtSecret, nil + }) + if err != nil { + return "", nil, fmt.Errorf("auth: invalid pending token: %w", err) + } + if !token.Valid { + return "", nil, errors.New("auth: pending token not valid") + } + + claims, ok := token.Claims.(jwt.MapClaims) + if !ok { + return "", nil, errors.New("auth: bad claims") + } + + // Verify this is actually a pending TOTP token + pending, _ := claims["totp_pending"].(bool) + if !pending { + return "", nil, errors.New("auth: not a pending TOTP token") + } + + // Extract user ID + var userID int64 + switch v := claims["uid"].(type) { + case float64: + userID = int64(v) + case int64: + userID = v + default: + return "", nil, errors.New("auth: missing uid in pending token") + } + + // Load user from DB + user, err := m.store.GetByID(userID) + if err != nil { + return "", nil, fmt.Errorf("auth: user lookup: %w", err) + } + if !user.TOTPEnabled { + return "", nil, errors.New("auth: TOTP not enabled for this user") + } + + // Load and decrypt TOTP secret + encSecret, enabled, err := m.store.GetTOTPSecret(context.Background(), userID) + if err != nil { + return "", nil, fmt.Errorf("auth: get totp secret: %w", err) + } + if !enabled || len(encSecret) == 0 { + return "", nil, errors.New("auth: TOTP not configured") + } + + plainSecret, err := m.decryptAES(encSecret) + if err != nil { + return "", nil, fmt.Errorf("auth: decrypt totp secret: %w", err) + } + + // Validate the TOTP code + if !ValidateTOTP(string(plainSecret), code) { + return "", nil, errors.New("auth: invalid TOTP code") + } + + // Issue a full auth token + return m.issueToken(user) +} + // ValidateToken parses and validates the token, checking the blacklist. func (m *Manager) ValidateToken(tokenStr string) (*Session, error) { token, err := jwt.Parse(tokenStr, func(t *jwt.Token) (interface{}, error) { @@ -206,6 +323,11 @@ func (m *Manager) ValidateToken(tokenStr string) (*Session, error) { return nil, errors.New("auth: bad claims") } + // PROJ-24: Reject pending TOTP tokens — they must not be used as full auth tokens. + if pending, _ := claims["totp_pending"].(bool); pending { + return nil, errors.New("auth: pending TOTP token cannot be used for authentication") + } + jti, _ := claims["jti"].(string) blacklisted, err := m.store.IsBlacklisted(jti) if err != nil { @@ -361,3 +483,64 @@ func generateJTI() string { } return hex.EncodeToString(b) } + +// ── PROJ-24: AES-256-GCM encryption helpers for TOTP secrets ───────────── + +// EncryptAES encrypts plaintext using AES-256-GCM with the manager's AES key. +// The 12-byte nonce is prepended to the ciphertext. +func (m *Manager) EncryptAES(plaintext []byte) ([]byte, error) { + return encryptAESGCM(m.aesKey, plaintext) +} + +// decryptAES decrypts data encrypted with EncryptAES. +func (m *Manager) decryptAES(data []byte) ([]byte, error) { + return decryptAESGCM(m.aesKey, data) +} + +// DecryptAESForHandler exposes AES decryption for use in API handlers (e.g., TOTP setup confirmation). +func (m *Manager) DecryptAESForHandler(data []byte) ([]byte, error) { + return decryptAESGCM(m.aesKey, data) +} + +func encryptAESGCM(key, plaintext []byte) ([]byte, error) { + block, err := aes.NewCipher(key) + if err != nil { + return nil, fmt.Errorf("auth: aes cipher: %w", err) + } + gcm, err := cipher.NewGCM(block) + if err != nil { + return nil, fmt.Errorf("auth: gcm: %w", err) + } + nonce := make([]byte, gcm.NonceSize()) + if _, err := io.ReadFull(rand.Reader, nonce); err != nil { + return nil, fmt.Errorf("auth: random nonce: %w", err) + } + ciphertext := gcm.Seal(nonce, nonce, plaintext, nil) + return ciphertext, nil +} + +func decryptAESGCM(key, data []byte) ([]byte, error) { + block, err := aes.NewCipher(key) + if err != nil { + return nil, fmt.Errorf("auth: aes cipher: %w", err) + } + gcm, err := cipher.NewGCM(block) + if err != nil { + return nil, fmt.Errorf("auth: gcm: %w", err) + } + nonceSize := gcm.NonceSize() + if len(data) < nonceSize { + return nil, fmt.Errorf("auth: ciphertext too short") + } + nonce, ciphertext := data[:nonceSize], data[nonceSize:] + plaintext, err := gcm.Open(nil, nonce, ciphertext, nil) + if err != nil { + return nil, fmt.Errorf("auth: decrypt: %w", err) + } + return plaintext, nil +} + +// GetUserStore returns the underlying user store (used by TOTP handlers). +func (m *Manager) GetUserStore() *userstore.Store { + return m.store +} diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index d0022d1..2ffa40d 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -30,17 +30,20 @@ func newTestAuth(t *testing.T) (*auth.Manager, *userstore.Store) { Role: userstore.RoleUser, }) - mgr := auth.New(store, nil, "test-jwt-secret-32chars-long-enough") + mgr := auth.New(store, nil, "test-jwt-secret-32chars-long-enough", "0000000000000000000000000000000000000000000000000000000000000000") return mgr, store } func TestLoginSuccess(t *testing.T) { mgr, _ := newTestAuth(t) - token, user, err := mgr.Login("testadmin", "adminpass") + token, user, totpRequired, err := mgr.Login("testadmin", "adminpass") if err != nil { t.Fatalf("Login: %v", err) } + if totpRequired { + t.Error("expected totpRequired=false for user without TOTP") + } if token == "" { t.Error("expected non-empty token") } @@ -55,7 +58,7 @@ func TestLoginSuccess(t *testing.T) { func TestLoginWrongPassword(t *testing.T) { mgr, _ := newTestAuth(t) - if _, _, err := mgr.Login("testadmin", "wrongpass"); err == nil { + if _, _, _, err := mgr.Login("testadmin", "wrongpass"); err == nil { t.Error("expected error for wrong password") } } @@ -63,7 +66,7 @@ func TestLoginWrongPassword(t *testing.T) { func TestLoginUnknownUser(t *testing.T) { mgr, _ := newTestAuth(t) - if _, _, err := mgr.Login("nobody", "pw"); err == nil { + if _, _, _, err := mgr.Login("nobody", "pw"); err == nil { t.Error("expected error for unknown user") } } @@ -71,7 +74,7 @@ func TestLoginUnknownUser(t *testing.T) { func TestTokenValidation(t *testing.T) { mgr, _ := newTestAuth(t) - token, _, _ := mgr.Login("testadmin", "adminpass") + token, _, _, _ := mgr.Login("testadmin", "adminpass") sess, err := mgr.ValidateToken(token) if err != nil { t.Fatalf("ValidateToken: %v", err) @@ -90,7 +93,7 @@ func TestTokenValidation(t *testing.T) { func TestTokenTampering(t *testing.T) { mgr, _ := newTestAuth(t) - token, _, _ := mgr.Login("testadmin", "adminpass") + token, _, _, _ := mgr.Login("testadmin", "adminpass") tampered := token + "x" if _, err := mgr.ValidateToken(tampered); err == nil { @@ -101,7 +104,7 @@ func TestTokenTampering(t *testing.T) { func TestLogout(t *testing.T) { mgr, _ := newTestAuth(t) - token, _, _ := mgr.Login("testadmin", "adminpass") + token, _, _, _ := mgr.Login("testadmin", "adminpass") // Token valid before logout if _, err := mgr.ValidateToken(token); err != nil { @@ -146,8 +149,8 @@ func TestHasRole(t *testing.T) { func TestMultipleSessionsIndependent(t *testing.T) { mgr, _ := newTestAuth(t) - token1, _, _ := mgr.Login("testadmin", "adminpass") - token2, _, _ := mgr.Login("testadmin", "adminpass") + token1, _, _, _ := mgr.Login("testadmin", "adminpass") + token2, _, _, _ := mgr.Login("testadmin", "adminpass") if token1 == token2 { t.Error("two logins should produce different tokens (different JTIs)") diff --git a/internal/auth/totp.go b/internal/auth/totp.go new file mode 100644 index 0000000..45c0e0e --- /dev/null +++ b/internal/auth/totp.go @@ -0,0 +1,50 @@ +package auth + +import ( + "bytes" + "image/png" + "time" + + "github.com/pquerna/otp" + "github.com/pquerna/otp/totp" +) + +// GenerateSecret creates a new TOTP secret for the given user. +// Returns the base32-encoded secret, the otpauth:// URL, and a QR code as PNG bytes. +func GenerateSecret(username, issuer string) (secret string, otpauthURL string, qrPNG []byte, err error) { + key, err := totp.Generate(totp.GenerateOpts{ + Issuer: issuer, + AccountName: username, + SecretSize: 20, + Algorithm: otp.AlgorithmSHA1, + Digits: otp.DigitsSix, + Period: 30, + }) + if err != nil { + return "", "", nil, err + } + secret = key.Secret() + otpauthURL = key.URL() + + img, err := key.Image(200, 200) + if err != nil { + return secret, otpauthURL, nil, nil // QR code is optional + } + var buf bytes.Buffer + if encErr := png.Encode(&buf, img); encErr == nil { + qrPNG = buf.Bytes() + } + return secret, otpauthURL, qrPNG, nil +} + +// ValidateTOTP checks whether the given code is valid for the base32 secret. +// Allows +/- 1 time step (30s) tolerance. +func ValidateTOTP(secret, code string) bool { + valid, err := totp.ValidateCustom(code, secret, time.Now().UTC(), totp.ValidateOpts{ + Period: 30, + Skew: 1, + Digits: otp.DigitsSix, + Algorithm: otp.AlgorithmSHA1, + }) + return err == nil && valid +} diff --git a/internal/userstore/userstore.go b/internal/userstore/userstore.go index 32b2182..8f142f6 100644 --- a/internal/userstore/userstore.go +++ b/internal/userstore/userstore.go @@ -23,14 +23,17 @@ const ( // User represents a user account in the system. type User struct { - ID int64 - Username string - Email string - Role string - Source string // "local" or "ldap" - Active bool - CreatedAt time.Time - TenantID *int64 `json:"tenant_id,omitempty"` + ID int64 + Username string + Email string + Role string + Source string // "local" or "ldap" + Active bool + CreatedAt time.Time + TenantID *int64 `json:"tenant_id,omitempty"` + TOTPEnabled bool `json:"totp_enabled"` + TOTPResetAt *time.Time `json:"totp_reset_at,omitempty"` + TOTPResetBy *string `json:"totp_reset_by,omitempty"` } // CreateUserRequest holds parameters for creating a new user. @@ -99,6 +102,16 @@ func (s *Store) initSchema(ctx context.Context) error { CREATE INDEX IF NOT EXISTS idx_login_attempts_username_time ON login_attempts (username, attempted_at); CREATE INDEX IF NOT EXISTS idx_users_tenant ON users (tenant_id); `) + if err != nil { + return err + } + // PROJ-24: TOTP 2FA columns + _, err = s.pool.Exec(ctx, ` + ALTER TABLE users ADD COLUMN IF NOT EXISTS totp_secret BYTEA; + ALTER TABLE users ADD COLUMN IF NOT EXISTS totp_enabled BOOLEAN NOT NULL DEFAULT false; + ALTER TABLE users ADD COLUMN IF NOT EXISTS totp_reset_at TIMESTAMPTZ; + ALTER TABLE users ADD COLUMN IF NOT EXISTS totp_reset_by TEXT; + `) return err } @@ -134,7 +147,7 @@ func (s *Store) Create(req CreateUserRequest) (*User, error) { func (s *Store) GetByID(id int64) (*User, error) { ctx := context.Background() row := s.pool.QueryRow(ctx, - `SELECT id, username, email, role, source, active, created_at, tenant_id FROM users WHERE id = $1`, id, + `SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by FROM users WHERE id = $1`, id, ) return scanUser(row) } @@ -143,7 +156,7 @@ func (s *Store) GetByID(id int64) (*User, error) { func (s *Store) GetByUsername(username string) (*User, error) { ctx := context.Background() row := s.pool.QueryRow(ctx, - `SELECT id, username, email, role, source, active, created_at, tenant_id FROM users WHERE username = $1`, username, + `SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by FROM users WHERE username = $1`, username, ) return scanUser(row) } @@ -153,13 +166,13 @@ func (s *Store) GetByUsername(username string) (*User, error) { func (s *Store) VerifyPassword(username, password string) (*User, error) { ctx := context.Background() row := s.pool.QueryRow(ctx, - `SELECT id, username, email, role, source, active, created_at, tenant_id, password_hash FROM users WHERE username = $1`, + `SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by, password_hash FROM users WHERE username = $1`, username, ) var u User var hash string - err := row.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID, &hash) + err := row.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID, &u.TOTPEnabled, &u.TOTPResetAt, &u.TOTPResetBy, &hash) if errors.Is(err, pgx.ErrNoRows) { return nil, errors.New("userstore: user not found") } @@ -231,10 +244,10 @@ func (s *Store) List(role string) ([]*User, error) { if role == "" { rows, err = s.pool.Query(ctx, - `SELECT id, username, email, role, source, active, created_at, tenant_id FROM users ORDER BY id`) + `SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by FROM users ORDER BY id`) } else { rows, err = s.pool.Query(ctx, - `SELECT id, username, email, role, source, active, created_at, tenant_id FROM users WHERE role = $1 ORDER BY id`, role) + `SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by FROM users WHERE role = $1 ORDER BY id`, role) } if err != nil { return nil, fmt.Errorf("userstore: list: %w", err) @@ -255,7 +268,7 @@ func (s *Store) List(role string) ([]*User, error) { // ListByTenant returns all users belonging to a specific tenant. func (s *Store) ListByTenant(ctx context.Context, tenantID int64) ([]*User, error) { rows, err := s.pool.Query(ctx, - `SELECT id, username, email, role, source, active, created_at, tenant_id FROM users WHERE tenant_id = $1 ORDER BY id`, + `SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by FROM users WHERE tenant_id = $1 ORDER BY id`, tenantID, ) if err != nil { @@ -381,7 +394,7 @@ func (s *Store) UpsertLDAPUser(username, email, role string, tenantID *int64) (* func scanUser(row pgx.Row) (*User, error) { var u User - err := row.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID) + err := row.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID, &u.TOTPEnabled, &u.TOTPResetAt, &u.TOTPResetBy) if errors.Is(err, pgx.ErrNoRows) { return nil, fmt.Errorf("userstore: not found") } @@ -393,8 +406,63 @@ func scanUser(row pgx.Row) (*User, error) { func scanUserRow(rows pgx.Rows) (*User, error) { var u User - if err := rows.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID); err != nil { + if err := rows.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID, &u.TOTPEnabled, &u.TOTPResetAt, &u.TOTPResetBy); err != nil { return nil, fmt.Errorf("userstore: scan row: %w", err) } return &u, nil } + +// ── PROJ-24: TOTP 2FA Methods ──────────────────────────────────────────── + +// SetTOTPSecret stores the encrypted TOTP secret (not yet activated). +func (s *Store) SetTOTPSecret(ctx context.Context, userID int64, encryptedSecret []byte) error { + _, err := s.pool.Exec(ctx, `UPDATE users SET totp_secret = $1 WHERE id = $2`, encryptedSecret, userID) + if err != nil { + return fmt.Errorf("userstore: set totp secret: %w", err) + } + return nil +} + +// EnableTOTP activates TOTP for the user (after code confirmation). +func (s *Store) EnableTOTP(ctx context.Context, userID int64) error { + _, err := s.pool.Exec(ctx, `UPDATE users SET totp_enabled = true WHERE id = $1`, userID) + if err != nil { + return fmt.Errorf("userstore: enable totp: %w", err) + } + return nil +} + +// DisableTOTP deactivates TOTP and removes the secret (user self-service). +func (s *Store) DisableTOTP(ctx context.Context, userID int64) error { + _, err := s.pool.Exec(ctx, `UPDATE users SET totp_enabled = false, totp_secret = NULL WHERE id = $1`, userID) + if err != nil { + return fmt.Errorf("userstore: disable totp: %w", err) + } + return nil +} + +// ResetTOTP resets TOTP for a user (admin action) and logs who performed the reset. +func (s *Store) ResetTOTP(ctx context.Context, userID int64, resetBy string) error { + _, err := s.pool.Exec(ctx, + `UPDATE users SET totp_enabled = false, totp_secret = NULL, totp_reset_at = NOW(), totp_reset_by = $1 WHERE id = $2`, + resetBy, userID, + ) + if err != nil { + return fmt.Errorf("userstore: reset totp: %w", err) + } + return nil +} + +// GetTOTPSecret returns the encrypted TOTP secret and enabled status for a user. +func (s *Store) GetTOTPSecret(ctx context.Context, userID int64) (secret []byte, enabled bool, err error) { + err = s.pool.QueryRow(ctx, + `SELECT totp_secret, totp_enabled FROM users WHERE id = $1`, userID, + ).Scan(&secret, &enabled) + if errors.Is(err, pgx.ErrNoRows) { + return nil, false, fmt.Errorf("userstore: user not found") + } + if err != nil { + return nil, false, fmt.Errorf("userstore: get totp secret: %w", err) + } + return secret, enabled, nil +}