amadan
console: fix delete-account review findings
Delete's failure redirects (mistyped confirm, ops account, still-has-apps, tombstone/store errors) now land on the settings page, where the form lives, instead of bouncing the error flash onto the apps page. The success redirect (account is gone) is unchanged. The settings display gate reads AppCount, the same primitive delete's enforcement uses, so display and enforcement can't drift. Added a member-view leg to TestAccountSettingsPage asserting the danger zone is invisible to non-owners.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 files changed,
+30
−7
internal/console/ownership.go+5 −5internal/console/server.go+2 −2internal/console/server_test.go+23 −0
diff --git a/internal/console/ownership.go b/internal/console/ownership.go| index 36a2ce876..15fb14578 100644 |
| --- a/internal/console/ownership.go |
| +++ b/internal/console/ownership.go |
| @@ -97,7 +97,7 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) { |
| // cost a passkey touch. |
| if strings.TrimSpace(r.FormValue("confirm")) != acct.Name { |
| s.setFlash(w, "err", "Type the account's name exactly to delete it.") |
| - http.Redirect(w, r, "/a/"+acct.Sqid+"/", http.StatusSeeOther) |
| + http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther) |
| return |
| } |
| if !s.requireFreshAuth(w, r, addr, raw) { |
| @@ -110,7 +110,7 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) { |
| // act on by dropping its registry row. |
| if acct.ID == OpsAccountID { |
| s.setFlash(w, "err", deletionAdvice(fmt.Errorf("console: ops"))) |
| - http.Redirect(w, r, "/a/"+acct.Sqid+"/", http.StatusSeeOther) |
| + http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther) |
| return |
| } |
| if n, err := s.store.AppCount(acct.ID); err != nil || n > 0 { |
| @@ -120,7 +120,7 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) { |
| } else { |
| s.setFlash(w, "err", deletionAdvice(fmt.Errorf("console: still holds %d app(s)", n))) |
| } |
| - http.Redirect(w, r, "/a/"+acct.Sqid+"/", http.StatusSeeOther) |
| + http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther) |
| return |
| } |
| // The bucket first, and the deletion is abandoned if it will not take. |
| @@ -143,13 +143,13 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) { |
| s.setFlash(w, "err", "Could not retire this account in the deployment bucket, so nothing "+ |
| "was deleted — an account deleted only here would come back on the next restart. "+ |
| "Try again once the bucket is reachable.") |
| - http.Redirect(w, r, "/a/"+acct.Sqid+"/", http.StatusSeeOther) |
| + http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther) |
| return |
| } |
| if err := s.store.DeleteAccount(acct.ID, time.Now()); err != nil { |
| log.Printf("console: delete account: %v", err) |
| s.setFlash(w, "err", deletionAdvice(err)) |
| - http.Redirect(w, r, "/a/"+acct.Sqid+"/", http.StatusSeeOther) |
| + http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther) |
| return |
| } |
| s.setFlash(w, "ok", "Deleted “"+acct.Name+"”.") |
diff --git a/internal/console/server.go b/internal/console/server.go| index fed6113c8..70a81f386 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -1199,12 +1199,12 @@ func (s *Server) handleAccountSettings(w http.ResponseWriter, r *http.Request) { |
| p := s.accountPage(w, r, addr, raw, acct, role) |
| p.Title = "Account settings" |
| p.ActiveNav = "settings" |
| - apps, err := s.store.AppsIn(acct.ID) |
| + n, err := s.store.AppCount(acct.ID) |
| if err != nil { |
| s.renderMessage(w, r, http.StatusInternalServerError, "Error", "Could not read your apps. Please try again.") |
| return |
| } |
| - p.HasApps = len(apps) > 0 |
| + p.HasApps = n > 0 |
| s.render(w, http.StatusOK, "account_settings", p) |
| } |
diff --git a/internal/console/server_test.go b/internal/console/server_test.go| index bef7da6e3..ad786d45a 100644 |
| --- a/internal/console/server_test.go |
| +++ b/internal/console/server_test.go |
| @@ -566,6 +566,9 @@ func TestTeamPage(t *testing.T) { |
| func TestAccountSettingsPage(t *testing.T) { |
| fx := newConsoleFixture(t) |
| + if err := fx.st.AddMember(OpsAccountID, "kim@keymail.dev", RoleMember, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| c := signInPaul(t, fx) |
| status, _, body := get(t, c, fx.u("/settings")) |
| if status != 200 { |
| @@ -585,6 +588,26 @@ func TestAccountSettingsPage(t *testing.T) { |
| t.Fatal("missing the why-not-deletable explanation") |
| } |
| assertNonMember404(t, fx, "/settings") |
| + |
| + // A non-owner member sees the Identity section and nothing of the danger |
| + // zone at all — not the heading, not the form, not the explanation. The |
| + // section is owner-only display, same as Team's member-administration |
| + // forms; the account having apps here is incidental, not the reason. |
| + fx.setUser("kim@keymail.dev") |
| + member := newBrowser(t) |
| + signUp(t, fx, member, "code-kim-settings") |
| + mStatus, _, mBody := get(t, member, fx.u("/settings")) |
| + if mStatus != 200 { |
| + t.Fatalf("member GET /settings = %d", mStatus) |
| + } |
| + if !strings.Contains(mBody, "<h2>Identity</h2>") { |
| + t.Fatal("member view missing the Identity section") |
| + } |
| + for _, gone := range []string{"Delete this account", `action="` + fx.path("/delete") + `"`} { |
| + if strings.Contains(mBody, gone) { |
| + t.Fatalf("member view of /settings still shows %q", gone) |
| + } |
| + } |
| } |
| func TestAppNamedNewIsRefused(t *testing.T) { |