From d1e4068636caf4e4567545875f3205bef373eaa4 Mon Sep 17 00:00:00 2001 From: mirivlad Date: Fri, 17 Jul 2026 07:04:46 +0800 Subject: [PATCH] fix(web): keep generated passwords out of page source --- README.md | 5 +++-- internal/server/routes.go | 1 + internal/server/web/static/app.js | 8 ++++++++ .../web/templates/admin_password_result.html | 2 +- internal/server/web_admin.go | 18 +++++++++++++++--- internal/server/web_locale_test.go | 19 +++++++++++++++---- internal/server/web_render.go | 1 - scripts/smoke-web-browser.mjs | 1 + 8 files changed, 44 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 9a69e60..56b01d6 100644 --- a/README.md +++ b/README.md @@ -240,8 +240,9 @@ users, devices, vaults, storage, audit, SMTP settings, and diagnostics. Lists use bounded server-side search, filters, whitelisted sort order, and pagination. Administrators can create, edit, confirm, block, reset, and delete users; revoke devices; and permanently remove only a previously revoked device. A browser -password reset generates a random password and exposes it once on a `no-store` -page; it is never placed in the URL, audit log, cookies, or database plaintext. +password reset generates a random password and exposes it once through a +CSRF-protected `no-store` POST after the result page loads; it is never placed +in the URL, initial HTML source, audit log, cookies, or database plaintext. Destructive browser actions use the shared local confirmation dialog. Blocking, credential changes, device actions, cleanup, and SMTP changes require the current diff --git a/internal/server/routes.go b/internal/server/routes.go index 0fb962f..29622a9 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -38,6 +38,7 @@ func (s *Server) routes() { s.mux.HandleFunc("/admin/logout", s.handleAdminWebLogout) s.mux.HandleFunc("/admin/action", s.handleAdminWebAction) s.mux.HandleFunc("/admin/password-result", s.handleAdminPasswordResult) + s.mux.HandleFunc("/admin/password-result/secret", s.handleAdminPasswordResultSecret) s.mux.HandleFunc("/admin/dashboard", s.handleAdminWeb) s.mux.HandleFunc("/admin/users", s.handleAdminWeb) s.mux.HandleFunc("/admin/create-user", s.handleAdminCreateUserWeb) diff --git a/internal/server/web/static/app.js b/internal/server/web/static/app.js index 460e5ab..7bbbc48 100644 --- a/internal/server/web/static/app.js +++ b/internal/server/web/static/app.js @@ -36,3 +36,11 @@ document.addEventListener("click", async function (event) { // The downloadable JSON link remains available when clipboard access is unavailable. } }); + +const oneTimeSecret = document.querySelector("[data-one-time-secret-url]"); +if (oneTimeSecret) { + fetch(oneTimeSecret.dataset.oneTimeSecretUrl, { method: "POST", credentials: "same-origin", headers: { "X-CSRF-Token": oneTimeSecret.dataset.csrfToken } }) + .then(async function (response) { if (!response.ok) throw new Error("one-time secret unavailable"); return response.json(); }) + .then(function (data) { oneTimeSecret.querySelector(".one-time-secret").textContent = data.password; }) + .catch(function () { oneTimeSecret.querySelector(".one-time-secret").textContent = "—"; }); +} diff --git a/internal/server/web/templates/admin_password_result.html b/internal/server/web/templates/admin_password_result.html index d41b7f4..cd97e68 100644 --- a/internal/server/web/templates/admin_password_result.html +++ b/internal/server/web/templates/admin_password_result.html @@ -2,7 +2,7 @@ {{define "content"}}
{{template "admin_nav" .}}

{{t .Locale "admin.resultTitle"}}

{{t .Locale "admin.resetPassword"}}

-

{{t .Locale "admin.oneTimePasswordNotice"}}

{{.OneTimeSecret}}

{{t .Locale "admin.oneTimePasswordHint"}}

+

{{t .Locale "admin.oneTimePasswordNotice"}}

{{t .Locale "common.loading"}}

{{t .Locale "admin.oneTimePasswordHint"}}

{{t .Locale "admin.users"}}
{{end}} diff --git a/internal/server/web_admin.go b/internal/server/web_admin.go index 43378f9..8fc4ee3 100644 --- a/internal/server/web_admin.go +++ b/internal/server/web_admin.go @@ -62,18 +62,30 @@ func (s *Server) handleAdminPasswordResult(w http.ResponseWriter, r *http.Reques if !s.requireAdminCookie(w, r) { return } + w.Header().Set("Cache-Control", "no-store, max-age=0") + s.renderPage(w, r, "admin_password_result", webPage{Title: "admin.resetPassword", Admin: true, AdminPage: "users"}) +} + +func (s *Server) handleAdminPasswordResultSecret(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + methodNotAllowed(w, http.MethodPost) + return + } + if !s.requireAdminMutation(w, r) { + return + } cookie, err := r.Cookie("admin_session") if err != nil { - http.Redirect(w, r, "/admin/users", http.StatusSeeOther) + jsonErrCode(w, http.StatusForbidden, "session_invalid", "administrator session is required") return } secret := s.takeAdminOneTimeSecret(cookie.Value) if secret == "" { - http.Redirect(w, r, "/admin/users", http.StatusSeeOther) + jsonErrCode(w, http.StatusGone, "one_time_secret_expired", "one-time password is no longer available") return } w.Header().Set("Cache-Control", "no-store, max-age=0") - s.renderPage(w, r, "admin_password_result", webPage{Title: "admin.resetPassword", Admin: true, AdminPage: "users", OneTimeSecret: secret}) + jsonOK(w, map[string]string{"password": secret}) } func (s *Server) handleAdminVaultDetail(w http.ResponseWriter, r *http.Request) { diff --git a/internal/server/web_locale_test.go b/internal/server/web_locale_test.go index 68f760e..cc7bbfb 100644 --- a/internal/server/web_locale_test.go +++ b/internal/server/web_locale_test.go @@ -650,15 +650,26 @@ func TestAdminPasswordResetShowsGeneratedSecretOnce(t *testing.T) { resultRequest.AddCookie(&http.Cookie{Name: "admin_session", Value: adminToken}) resultResponse := httptest.NewRecorder() s.Handler().ServeHTTP(resultResponse, resultRequest) - if resultResponse.Code != http.StatusOK || !strings.Contains(resultResponse.Header().Get("Cache-Control"), "no-store") || !strings.Contains(resultResponse.Body.String(), "one-time-secret") { + if resultResponse.Code != http.StatusOK || !strings.Contains(resultResponse.Header().Get("Cache-Control"), "no-store") || !strings.Contains(resultResponse.Body.String(), "data-one-time-secret-url") { t.Fatalf("password result=%d headers=%v body=%s", resultResponse.Code, resultResponse.Header(), resultResponse.Body.String()) } - secondRequest := httptest.NewRequest(http.MethodGet, "/admin/password-result", nil) + secretRequest := httptest.NewRequest(http.MethodPost, "/admin/password-result/secret", nil) + secretRequest.Header.Set("X-CSRF-Token", csrf) + secretRequest.AddCookie(&http.Cookie{Name: "admin_session", Value: adminToken}) + secretRequest.AddCookie(&http.Cookie{Name: "csrf_token", Value: csrf}) + secretResponse := httptest.NewRecorder() + s.Handler().ServeHTTP(secretResponse, secretRequest) + if secretResponse.Code != http.StatusOK || !strings.Contains(secretResponse.Header().Get("Cache-Control"), "no-store") || !strings.Contains(secretResponse.Body.String(), `"password"`) { + t.Fatalf("secret response=%d headers=%v body=%s", secretResponse.Code, secretResponse.Header(), secretResponse.Body.String()) + } + secondRequest := httptest.NewRequest(http.MethodPost, "/admin/password-result/secret", nil) + secondRequest.Header.Set("X-CSRF-Token", csrf) secondRequest.AddCookie(&http.Cookie{Name: "admin_session", Value: adminToken}) + secondRequest.AddCookie(&http.Cookie{Name: "csrf_token", Value: csrf}) secondResponse := httptest.NewRecorder() s.Handler().ServeHTTP(secondResponse, secondRequest) - if secondResponse.Code != http.StatusSeeOther || secondResponse.Header().Get("Location") != "/admin/users" { - t.Fatalf("second password result=%d %q", secondResponse.Code, secondResponse.Header().Get("Location")) + if secondResponse.Code != http.StatusGone { + t.Fatalf("second password result=%d: %s", secondResponse.Code, secondResponse.Body.String()) } } diff --git a/internal/server/web_render.go b/internal/server/web_render.go index dc4cd75..e156c00 100644 --- a/internal/server/web_render.go +++ b/internal/server/web_render.go @@ -44,7 +44,6 @@ type webPage struct { FormAction string BackURL string Token string - OneTimeSecret string Admin bool UserName string Email string diff --git a/scripts/smoke-web-browser.mjs b/scripts/smoke-web-browser.mjs index 3cc0d86..f5752fa 100755 --- a/scripts/smoke-web-browser.mjs +++ b/scripts/smoke-web-browser.mjs @@ -125,6 +125,7 @@ await toggleTemporaryUser(); // process so the subsequent user login exercises the generated credential. await evaluate(`(() => { const row = [...document.querySelectorAll('tbody tr')].find((item) => item.textContent.includes('browser-smoke-user')); const details = row.querySelector('details'); details.open = true; const form = [...row.querySelectorAll('form')].find((item) => item.querySelector('[name=action]')?.value === 'reset-user-password'); form.querySelector('[name=password]').value = 'browser-smoke-admin-password'; form.querySelector('button').click(); })()`); await confirmDialog("/admin/password-result"); +await waitFor(() => evaluate("!['Загрузка...', 'Loading...', '—'].includes(document.querySelector('.one-time-secret')?.textContent.trim())"), "generated one-time password"); await screenshot("admin-password-result"); const generatedPassword = await evaluate("document.querySelector('.one-time-secret')?.textContent.trim()"); if (!generatedPassword || (await evaluate("location.href")).includes(generatedPassword)) throw new Error("one-time password result is missing or leaked into the URL");