Commit fc527a7e38

fc527a7e38ef65a529314f354e1b3476278e1033

parent: 3a58c644d0

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-20 10:22 UTC

web: require confirmation to remove a push device

key-remove, pgp-remove and email-remove all guard their destructive
button with the typed confirmfield; device-remove did not. A removed
device does not self-heal — the iOS app only re-registers its token on
sign-in or when the token changes — so an accidental click silently
stops that device's notifications with nothing on screen to explain
why.

Also cover the notify-push and device-remove submit paths directly,
rather than only the page render.

Ref #89
internal/httpd/account.go +8 −1
@@ -39,6 +39,7 @@ type accountDevice struct {
3939 Label string
4040 Token string
4141 LastSeenAt string
42 Confirm string // the truncated token, typed back to confirm removal
4243}
4344
4445// accountForm renders the account's own settings: keys, addresses, and the
@@ -80,7 +81,8 @@ func (s *Server) accountPage(w http.ResponseWriter, r *http.Request, u store.Use
8081 var devices []accountDevice
8182 if list, err := s.st.PushDevices(u.ID); err == nil {
8283 for _, d := range list {
83 devices = append(devices, accountDevice{ID: d.ID, Label: d.Label, Token: prefix8(d.Token), LastSeenAt: d.LastSeenAt})
84 token := prefix8(d.Token)
85 devices = append(devices, accountDevice{ID: d.ID, Label: d.Label, Token: token, LastSeenAt: d.LastSeenAt, Confirm: token})
8486 }
8587 }
8688
@@ -275,6 +277,11 @@ func (s *Server) accountSubmit(w http.ResponseWriter, r *http.Request, u store.U
275277 }
276278 back("", "notification preferences saved")
277279 case "device-remove":
280 want := r.FormValue("tokenprefix")
281 if ok, msg := confirmed(r, want); !ok {
282 back(msg, "")
283 return
284 }
278285 if _, msg, ok := s.runControl(u, []string{"notifications", "device", "remove", r.FormValue("id")}); !ok {
279286 back(msg, "")
280287 return
internal/httpd/account_test.go +87
@@ -1,7 +1,10 @@
11package httpd
22
33import (
4 "net/http"
45 "net/http/httptest"
6 "net/url"
7 "strconv"
58 "strings"
69 "testing"
710
@@ -48,3 +51,87 @@ func TestAccountPagePushToggleAndDevices(t *testing.T) {
4851 t.Fatal("the page printed a device token in full")
4952 }
5053}
54
55// submit posts an account settings form as u and returns the recorder.
56func submitAccountForm(t *testing.T, s *Server, u store.User, form url.Values) *httptest.ResponseRecorder {
57 t.Helper()
58 req := httptest.NewRequest("POST", "/settings", strings.NewReader(form.Encode()))
59 req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
60 rr := httptest.NewRecorder()
61 s.accountSubmit(rr, req, u)
62 return rr
63}
64
65// Posting notify-push dispatches to notifications settings push, the same
66// path the mail and watch toggles already use.
67func TestAccountSubmitNotifyPush(t *testing.T) {
68 st, err := store.Open(":memory:")
69 if err != nil {
70 t.Fatal(err)
71 }
72 defer st.Close()
73 if err := st.MigrateUp(); err != nil {
74 t.Fatal(err)
75 }
76 uid, err := st.CreateUser("alice", false)
77 if err != nil {
78 t.Fatal(err)
79 }
80 u := store.User{ID: uid, Username: "alice"}
81 s := New(config.Default(), st)
82
83 rr := submitAccountForm(t, s, u, url.Values{"field": {"notify-push"}, "push": {"on"}})
84 if rr.Code != http.StatusSeeOther {
85 t.Fatalf("status %d, body %s", rr.Code, rr.Body.String())
86 }
87 if on, err := st.PushEnabled(uid); err != nil || !on {
88 t.Fatalf("PushEnabled after notify-push=on: %v %v", on, err)
89 }
90
91 submitAccountForm(t, s, u, url.Values{"field": {"notify-push"}})
92 if on, err := st.PushEnabled(uid); err != nil || on {
93 t.Fatalf("PushEnabled after notify-push off: %v %v", on, err)
94 }
95}
96
97// Removing a device requires the typed confirmation, the same guard
98// key-remove and pgp-remove carry, and then dispatches to notifications
99// device remove, scoped to the caller's own account.
100func TestAccountSubmitDeviceRemove(t *testing.T) {
101 st, err := store.Open(":memory:")
102 if err != nil {
103 t.Fatal(err)
104 }
105 defer st.Close()
106 if err := st.MigrateUp(); err != nil {
107 t.Fatal(err)
108 }
109 uid, err := st.CreateUser("alice", false)
110 if err != nil {
111 t.Fatal(err)
112 }
113 u := store.User{ID: uid, Username: "alice"}
114 token := strings.Repeat("b", 64)
115 id, err := st.AddPushDevice(uid, token, "iphone")
116 if err != nil {
117 t.Fatal(err)
118 }
119 s := New(config.Default(), st)
120
121 want := prefix8(token)
122 idStr := strconv.FormatInt(id, 10)
123
124 // Without the typed confirmation, the device survives.
125 submitAccountForm(t, s, u, url.Values{"field": {"device-remove"}, "id": {idStr}, "tokenprefix": {want}})
126 if devices, _ := st.PushDevices(uid); len(devices) != 1 {
127 t.Fatalf("device removed without confirmation: %v", devices)
128 }
129
130 rr := submitAccountForm(t, s, u, url.Values{"field": {"device-remove"}, "id": {idStr}, "tokenprefix": {want}, "confirm": {want}})
131 if rr.Code != http.StatusSeeOther {
132 t.Fatalf("status %d, body %s", rr.Code, rr.Body.String())
133 }
134 if devices, _ := st.PushDevices(uid); len(devices) != 0 {
135 t.Fatalf("device not removed: %v", devices)
136 }
137}
internal/web/templates/account.html +1 −1
@@ -158,7 +158,7 @@ account and where notifications go.</p>
158158 <td>{{.Label}}</td>
159159 <td class="mono">{{.Token}}</td>
160160 <td>{{if .LastSeenAt}}{{when .LastSeenAt}}{{else}}never{{end}}</td>
161 <td class="act"><form method="post" action="/settings"><input type="hidden" name="field" value="device-remove"><input type="hidden" name="id" value="{{.ID}}"><button type="submit" class="danger">Remove</button></form></td>
161 <td class="act"><form method="post" action="/settings"><input type="hidden" name="field" value="device-remove"><input type="hidden" name="id" value="{{.ID}}"><input type="hidden" name="tokenprefix" value="{{.Confirm}}">{{template "confirmfield" .Confirm}} <button type="submit" class="danger">Remove</button></form></td>
162162</tr>
163163{{end}}</table></div>
164164{{else}}<p class="none">No registered devices.</p>{{end}}