Passwort-Reset: Review-Fixes
- Token vor bcrypt prüfen, damit beliebige Tokens keine CPU-Last erzeugen - CLI meldet DB-Fehler nicht mehr als "Nutzer nicht gefunden" - Reset in einem Browser mit fremder Session räumt diese mit ab - Konto-Löschen entfernt auch offene Reset-Links Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
81a74375b7
commit
764ad50b8e
@@ -758,6 +758,30 @@ func TestPasswordReset(t *testing.T) {
|
|||||||
t.Fatalf("neues Passwort: erwartet 200, bekam %d", got)
|
t.Fatalf("neues Passwort: erwartet 200, bekam %d", got)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reset in einem Browser, in dem noch bob angemeldet ist: bobs Session
|
||||||
|
// wird mit abgeräumt statt verwaist gültig zu bleiben.
|
||||||
|
bob := registerAndLogin(t, srv, "bob")
|
||||||
|
var bobSession string
|
||||||
|
u, _ := url.Parse(srv.URL)
|
||||||
|
for _, ck := range bob.Jar.Cookies(u) {
|
||||||
|
if ck.Name == "session" {
|
||||||
|
bobSession = ck.Value
|
||||||
|
}
|
||||||
|
}
|
||||||
|
tok, _, _ := createResetToken("alice")
|
||||||
|
resp := postForm(t, bob, srv.URL+"/api/auth/reset", url.Values{
|
||||||
|
"token": {tok}, "pass1": {"drittesgeheim"}, "pass2": {"drittesgeheim"},
|
||||||
|
})
|
||||||
|
resp.Body.Close()
|
||||||
|
if resp.StatusCode != http.StatusOK {
|
||||||
|
t.Fatalf("reset mit fremder Session: erwartet 200, bekam %d", resp.StatusCode)
|
||||||
|
}
|
||||||
|
var n int
|
||||||
|
db.QueryRow(`SELECT COUNT(*) FROM session WHERE value = $1`, bobSession).Scan(&n)
|
||||||
|
if n != 0 {
|
||||||
|
t.Fatalf("bobs Session nach Reset im selben Browser noch gültig")
|
||||||
|
}
|
||||||
|
|
||||||
// Ein neuer Link macht den vorigen ungültig; abgelaufene gelten nicht.
|
// Ein neuer Link macht den vorigen ungültig; abgelaufene gelten nicht.
|
||||||
first, _, _ := createResetToken("alice")
|
first, _, _ := createResetToken("alice")
|
||||||
second, _, _ := createResetToken("alice")
|
second, _, _ := createResetToken("alice")
|
||||||
|
|||||||
@@ -36,8 +36,11 @@ func hashResetToken(token string) string {
|
|||||||
// ungültig, abgelaufene aller Nutzer weggeräumt.
|
// ungültig, abgelaufene aller Nutzer weggeräumt.
|
||||||
func createResetToken(username string) (string, time.Time, error) {
|
func createResetToken(username string) (string, time.Time, error) {
|
||||||
var uid int64
|
var uid int64
|
||||||
if err := db.QueryRow(`SELECT uid FROM account WHERE username = $1`, username).Scan(&uid); err != nil {
|
err := db.QueryRow(`SELECT uid FROM account WHERE username = $1`, username).Scan(&uid)
|
||||||
|
if err == sql.ErrNoRows {
|
||||||
return "", time.Time{}, errUserNotFound
|
return "", time.Time{}, errUserNotFound
|
||||||
|
} else if err != nil {
|
||||||
|
return "", time.Time{}, err
|
||||||
}
|
}
|
||||||
|
|
||||||
token, err := newToken()
|
token, err := newToken()
|
||||||
@@ -68,19 +71,29 @@ func createResetToken(username string) (string, time.Time, error) {
|
|||||||
// (oder gleich "abgelaufen" melden) kann, bevor jemand ein Passwort tippt.
|
// (oder gleich "abgelaufen" melden) kann, bevor jemand ein Passwort tippt.
|
||||||
// POST statt GET, damit das Token nicht in der URL und damit im Log steht.
|
// POST statt GET, damit das Token nicht in der URL und damit im Log steht.
|
||||||
func handleResetCheck(w http.ResponseWriter, r *http.Request) error {
|
func handleResetCheck(w http.ResponseWriter, r *http.Request) error {
|
||||||
|
username, err := resetUsername(r.FormValue("token"))
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
writeJSON(w, http.StatusOK, map[string]string{"username": username})
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// resetUsername liefert den Nutzer zu einem gültigen Link, ohne ihn zu
|
||||||
|
// verbrauchen.
|
||||||
|
func resetUsername(token string) (string, error) {
|
||||||
var username string
|
var username string
|
||||||
err := db.QueryRow(
|
err := db.QueryRow(
|
||||||
`SELECT a.username FROM password_reset p JOIN account a ON a.uid = p.uid
|
`SELECT a.username FROM password_reset p JOIN account a ON a.uid = p.uid
|
||||||
WHERE p.token_hash = $1 AND p.expires >= $2`,
|
WHERE p.token_hash = $1 AND p.expires >= $2`,
|
||||||
hashResetToken(r.FormValue("token")), time.Now().Unix(),
|
hashResetToken(token), time.Now().Unix(),
|
||||||
).Scan(&username)
|
).Scan(&username)
|
||||||
if err == sql.ErrNoRows {
|
if err == sql.ErrNoRows {
|
||||||
return errResetInvalid
|
return "", errResetInvalid
|
||||||
} else if err != nil {
|
} else if err != nil {
|
||||||
return Internal(err)
|
return "", Internal(err)
|
||||||
}
|
}
|
||||||
writeJSON(w, http.StatusOK, map[string]string{"username": username})
|
return username, nil
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// handleReset setzt das neue Passwort (pass1/pass2 wie bei der Registrierung),
|
// handleReset setzt das neue Passwort (pass1/pass2 wie bei der Registrierung),
|
||||||
@@ -97,6 +110,13 @@ func handleReset(w http.ResponseWriter, r *http.Request) error {
|
|||||||
return e
|
return e
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Vorab ohne Verbrauch prüfen: bcrypt kostet spürbar CPU und soll nicht
|
||||||
|
// für jedes beliebige Token anlaufen. Verbindlich ist erst das DELETE unten.
|
||||||
|
token := r.FormValue("token")
|
||||||
|
if _, err := resetUsername(token); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
hash, err := bcrypt.GenerateFromPassword([]byte(pass1), bcrypt.DefaultCost)
|
hash, err := bcrypt.GenerateFromPassword([]byte(pass1), bcrypt.DefaultCost)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return Internal(err)
|
return Internal(err)
|
||||||
@@ -113,7 +133,7 @@ func handleReset(w http.ResponseWriter, r *http.Request) error {
|
|||||||
var uid int64
|
var uid int64
|
||||||
err = tx.QueryRow(
|
err = tx.QueryRow(
|
||||||
`DELETE FROM password_reset WHERE token_hash = $1 AND expires >= $2 RETURNING uid`,
|
`DELETE FROM password_reset WHERE token_hash = $1 AND expires >= $2 RETURNING uid`,
|
||||||
hashResetToken(r.FormValue("token")), time.Now().Unix(),
|
hashResetToken(token), time.Now().Unix(),
|
||||||
).Scan(&uid)
|
).Scan(&uid)
|
||||||
if err == sql.ErrNoRows {
|
if err == sql.ErrNoRows {
|
||||||
return errResetInvalid
|
return errResetInvalid
|
||||||
@@ -139,6 +159,11 @@ func handleReset(w http.ResponseWriter, r *http.Request) error {
|
|||||||
return Internal(err)
|
return Internal(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// War im Browser noch jemand anderes angemeldet, wird dessen Cookie gleich
|
||||||
|
// überschrieben -- die Session dazu soll dann nicht verwaist gültig bleiben.
|
||||||
|
if s, ok := getSession(r); ok {
|
||||||
|
db.Exec(`DELETE FROM session WHERE value = $1`, s.Value)
|
||||||
|
}
|
||||||
if err := startSession(w, r, uid); err != nil {
|
if err := startSession(w, r, uid); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -166,9 +166,10 @@ func handleSetAvatar(w http.ResponseWriter, r *http.Request) error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// handleUserDelete löscht den eingeloggten Account dauerhaft, nach Bestätigung
|
// handleUserDelete löscht den eingeloggten Account dauerhaft, nach Bestätigung
|
||||||
// durch das Passwort (Formularfeld pass1). Sessions und Votes werden in einer
|
// durch das Passwort (Formularfeld pass1). Sessions, Reset-Links und Votes
|
||||||
// Transaktion mitgelöscht; die eigenen Beiträge werden soft-gelöscht (als
|
// werden in einer Transaktion mitgelöscht; die eigenen Beiträge werden
|
||||||
// [deleted]-Platzhalter erhalten), damit fremde Antworten nicht verwaisen.
|
// soft-gelöscht (als [deleted]-Platzhalter erhalten), damit fremde Antworten
|
||||||
|
// nicht verwaisen.
|
||||||
// Zugehörige Mediendateien werden best effort entfernt.
|
// Zugehörige Mediendateien werden best effort entfernt.
|
||||||
func handleUserDelete(w http.ResponseWriter, r *http.Request) error {
|
func handleUserDelete(w http.ResponseWriter, r *http.Request) error {
|
||||||
uid := uidFromContext(r.Context())
|
uid := uidFromContext(r.Context())
|
||||||
@@ -205,6 +206,7 @@ func handleUserDelete(w http.ResponseWriter, r *http.Request) error {
|
|||||||
}
|
}
|
||||||
for _, q := range []string{
|
for _, q := range []string{
|
||||||
`DELETE FROM session WHERE uid = $1`,
|
`DELETE FROM session WHERE uid = $1`,
|
||||||
|
`DELETE FROM password_reset WHERE uid = $1`,
|
||||||
`DELETE FROM vote WHERE uid = $1`,
|
`DELETE FROM vote WHERE uid = $1`,
|
||||||
`UPDATE entry SET deleted = 1, content = '', filepath = '', uid = 0 WHERE uid = $1`,
|
`UPDATE entry SET deleted = 1, content = '', filepath = '', uid = 0 WHERE uid = $1`,
|
||||||
`DELETE FROM account WHERE uid = $1`,
|
`DELETE FROM account WHERE uid = $1`,
|
||||||
|
|||||||
Reference in New Issue
Block a user