diff --git a/internal/server/handlers_auth.go b/internal/server/handlers_auth.go index d626694..47cc4f3 100644 --- a/internal/server/handlers_auth.go +++ b/internal/server/handlers_auth.go @@ -193,24 +193,18 @@ func (s *Server) handleReset(w http.ResponseWriter, r *http.Request) { jsonErr(w, 400, err) return } - var userID, expiresAt string - err := s.db.QueryRow("SELECT user_id, expires_at FROM server_email_tokens WHERE token=? AND purpose='reset'", - req.Token).Scan(&userID, &expiresAt) - if err != nil { + _, err := s.resetPasswordWithToken(req.Token, req.NewPassword) + if err == errResetTokenInvalid { jsonErr(w, 400, "invalid or expired token") return } - exp, err := time.Parse(time.RFC3339, expiresAt) - if err != nil || time.Now().After(exp) { + if err == errResetTokenExpired { jsonErr(w, 400, "token expired") return } - hash, err := bcrypt.GenerateFromPassword([]byte(req.NewPassword), bcrypt.DefaultCost) if err != nil { jsonErr(w, 500, "internal error") return } - s.db.Exec("UPDATE server_users SET password_hash=? WHERE id=?", string(hash), userID) - s.db.Exec("DELETE FROM server_email_tokens WHERE token=?", req.Token) jsonOK(w, map[string]string{"status": "password reset"}) } diff --git a/internal/server/handlers_web_user.go b/internal/server/handlers_web_user.go index f900fcc..e94d22f 100644 --- a/internal/server/handlers_web_user.go +++ b/internal/server/handlers_web_user.go @@ -212,15 +212,17 @@ func (s *Server) handleUserWebReset(w http.ResponseWriter, r *http.Request) { w.Write([]byte(errorPageHTML(locale, t(locale, "common.error"), t(locale, "server.passwordsDoNotMatch"), "/reset?token="+token))) return } - var userID string - err := s.db.QueryRow("SELECT user_id FROM server_email_tokens WHERE token=? AND purpose='reset'", token).Scan(&userID) - if err != nil { + userID, err := s.resetPasswordWithToken(token, newPass) + if err == errResetTokenInvalid || err == errResetTokenExpired { http.Redirect(w, r, "/forgot", http.StatusFound) return } - hash, _ := bcrypt.GenerateFromPassword([]byte(newPass), bcrypt.DefaultCost) - s.db.Exec("UPDATE server_users SET password_hash=? WHERE id=?", string(hash), userID) - s.db.Exec("DELETE FROM server_email_tokens WHERE token=?", token) + if err != nil { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(errorPageHTML(locale, t(locale, "common.error"), t(locale, "common.error"), "/forgot"))) + return + } log.Printf("reset: user %s reset password", userID) w.Header().Set("Content-Type", "text/html; charset=utf-8") w.Write([]byte(resetDoneHTML(locale))) diff --git a/internal/server/password_reset.go b/internal/server/password_reset.go new file mode 100644 index 0000000..d9e76dd --- /dev/null +++ b/internal/server/password_reset.go @@ -0,0 +1,69 @@ +package server + +import ( + "database/sql" + "errors" + "time" + + "golang.org/x/crypto/bcrypt" +) + +var ( + errResetTokenInvalid = errors.New("invalid reset token") + errResetTokenExpired = errors.New("expired reset token") +) + +func (s *Server) resetPasswordWithToken(token, newPassword string) (string, error) { + tx, err := s.db.Begin() + if err != nil { + return "", err + } + defer tx.Rollback() + + var userID, expiresAt string + err = tx.QueryRow(`SELECT user_id, expires_at FROM server_email_tokens + WHERE token=? AND purpose='reset'`, token).Scan(&userID, &expiresAt) + if errors.Is(err, sql.ErrNoRows) { + return "", errResetTokenInvalid + } + if err != nil { + return "", err + } + expiresAtTime, err := time.Parse(time.RFC3339, expiresAt) + if err != nil || !time.Now().Before(expiresAtTime) { + return "", errResetTokenExpired + } + + deleted, err := tx.Exec(`DELETE FROM server_email_tokens + WHERE token=? AND purpose='reset' AND expires_at=?`, token, expiresAt) + if err != nil { + return "", err + } + deletedCount, err := deleted.RowsAffected() + if err != nil { + return "", err + } + if deletedCount != 1 { + return "", errResetTokenInvalid + } + + hash, err := bcrypt.GenerateFromPassword([]byte(newPassword), bcrypt.DefaultCost) + if err != nil { + return "", err + } + updated, err := tx.Exec("UPDATE server_users SET password_hash=? WHERE id=?", string(hash), userID) + if err != nil { + return "", err + } + updatedCount, err := updated.RowsAffected() + if err != nil { + return "", err + } + if updatedCount != 1 { + return "", errResetTokenInvalid + } + if err := tx.Commit(); err != nil { + return "", err + } + return userID, nil +} diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 2723af5..d2c7c6c 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -6,8 +6,10 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" + "strings" "testing" "time" @@ -343,6 +345,33 @@ func TestDeviceRegisterRequiresValidVaultID(t *testing.T) { } } +func TestWebResetRejectsExpiredToken(t *testing.T) { + s, _ := newSyncHTTPServer(t) + defer s.Close() + + oldPassword := "correct horse battery staple" + insertPairableUser(t, s, "user-a", "alice", oldPassword) + now := time.Now().UTC().Format(time.RFC3339) + if _, err := s.db.Exec(`INSERT INTO server_email_tokens + (token, user_id, purpose, expires_at, created_at) + VALUES (?, ?, 'reset', ?, ?)`, "expired-reset-token", "user-a", time.Now().Add(-time.Hour).UTC().Format(time.RFC3339), now); err != nil { + t.Fatalf("insert reset token: %v", err) + } + + response := postWebReset(t, s, "expired-reset-token", "new secret password") + if response.Code != http.StatusFound || response.Header().Get("Location") != "/forgot" { + t.Fatalf("expired reset response status=%d location=%q, want 302 /forgot", response.Code, response.Header().Get("Location")) + } + + var passwordHash string + if err := s.db.QueryRow("SELECT password_hash FROM server_users WHERE id=?", "user-a").Scan(&passwordHash); err != nil { + t.Fatalf("read password hash: %v", err) + } + if bcrypt.CompareHashAndPassword([]byte(passwordHash), []byte(oldPassword)) != nil { + t.Fatal("expired reset changed the password") + } +} + func TestNewServerMigratesLegacyOperationScope(t *testing.T) { dir := t.TempDir() dbPath := filepath.Join(dir, "legacy.db") @@ -508,6 +537,20 @@ func pairSyncDevice(t *testing.T, serverURL, username, password, vaultID string) } } +func postWebReset(t *testing.T, s *Server, token, password string) *httptest.ResponseRecorder { + t.Helper() + form := url.Values{ + "token": {token}, + "password": {password}, + "confirm": {password}, + } + request := httptest.NewRequest(http.MethodPost, "/reset", strings.NewReader(form.Encode())) + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + response := httptest.NewRecorder() + s.mux.ServeHTTP(response, request) + return response +} + func postJSON(t *testing.T, url, token string, body interface{}) map[string]interface{} { t.Helper() status, out := postJSONStatus(t, url, token, body)