diff --git a/endpoints_test.go b/endpoints_test.go index 0cc9502..5578037 100644 --- a/endpoints_test.go +++ b/endpoints_test.go @@ -758,6 +758,30 @@ func TestPasswordReset(t *testing.T) { 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. first, _, _ := createResetToken("alice") second, _, _ := createResetToken("alice") diff --git a/reset.go b/reset.go index a4bf9a1..262cfbc 100644 --- a/reset.go +++ b/reset.go @@ -36,8 +36,11 @@ func hashResetToken(token string) string { // ungültig, abgelaufene aller Nutzer weggeräumt. func createResetToken(username string) (string, time.Time, error) { 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 + } else if err != nil { + return "", time.Time{}, err } 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. // POST statt GET, damit das Token nicht in der URL und damit im Log steht. 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 err := db.QueryRow( `SELECT a.username FROM password_reset p JOIN account a ON a.uid = p.uid WHERE p.token_hash = $1 AND p.expires >= $2`, - hashResetToken(r.FormValue("token")), time.Now().Unix(), + hashResetToken(token), time.Now().Unix(), ).Scan(&username) if err == sql.ErrNoRows { - return errResetInvalid + return "", errResetInvalid } else if err != nil { - return Internal(err) + return "", Internal(err) } - writeJSON(w, http.StatusOK, map[string]string{"username": username}) - return nil + return username, nil } // 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 } + // 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) if err != nil { return Internal(err) @@ -113,7 +133,7 @@ func handleReset(w http.ResponseWriter, r *http.Request) error { var uid int64 err = tx.QueryRow( `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) if err == sql.ErrNoRows { return errResetInvalid @@ -139,6 +159,11 @@ func handleReset(w http.ResponseWriter, r *http.Request) error { 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 { return err } diff --git a/user.go b/user.go index 9f78959..a5cd8dd 100644 --- a/user.go +++ b/user.go @@ -166,9 +166,10 @@ func handleSetAvatar(w http.ResponseWriter, r *http.Request) error { } // handleUserDelete löscht den eingeloggten Account dauerhaft, nach Bestätigung -// durch das Passwort (Formularfeld pass1). Sessions und Votes werden in einer -// Transaktion mitgelöscht; die eigenen Beiträge werden soft-gelöscht (als -// [deleted]-Platzhalter erhalten), damit fremde Antworten nicht verwaisen. +// durch das Passwort (Formularfeld pass1). Sessions, Reset-Links und Votes +// werden in einer Transaktion mitgelöscht; die eigenen Beiträge werden +// soft-gelöscht (als [deleted]-Platzhalter erhalten), damit fremde Antworten +// nicht verwaisen. // Zugehörige Mediendateien werden best effort entfernt. func handleUserDelete(w http.ResponseWriter, r *http.Request) error { uid := uidFromContext(r.Context()) @@ -205,6 +206,7 @@ func handleUserDelete(w http.ResponseWriter, r *http.Request) error { } for _, q := range []string{ `DELETE FROM session WHERE uid = $1`, + `DELETE FROM password_reset WHERE uid = $1`, `DELETE FROM vote WHERE uid = $1`, `UPDATE entry SET deleted = 1, content = '', filepath = '', uid = 0 WHERE uid = $1`, `DELETE FROM account WHERE uid = $1`,