krz/gitbay

control, store, web, wiki: email list, remove and primary !328

merged merged by cmc on 2026-09-07 18:32 UTC · krz/gitbay:email-remove into main

10 files changed, +439 −2

Layout: unified · split

.gitbay/wiki/Parity.org +1
@@ -273,6 +273,7 @@ client has no use for one (krz/gitbay#57).
273| SSH keys: list, add, remove | yes | yes | yes | 273| SSH keys: list, add, remove | yes | yes | yes |
274| PGP keys: list, add, remove | yes | yes | yes | 274| PGP keys: list, add, remove | yes | yes | yes |
275| email add and verify | yes | yes | yes | 275| email add and verify | yes | yes | yes |
276| email list, remove, primary | yes | yes | no |
276| dashboard aggregate | yes | yes | yes | 277| dashboard aggregate | yes | yes | yes |
277| notification inbox | yes | yes | yes | 278| notification inbox | yes | yes | yes |
278| API token mint | yes | no | no | 279| API token mint | yes | no | no |
.gitbay/wiki/Users.org +8 −1
@@ -79,7 +79,14 @@ set. No separate registration step.
79 79
80Add and verify additional addresses with =email add <address>= / 80Add and verify additional addresses with =email add <address>= /
81=email verify <code>= (requires the instance to have SMTP; otherwise an 81=email verify <code>= (requires the instance to have SMTP; otherwise an
82admin can assert an address for you). 82admin can assert an address for you). =email list= shows them. =email
83remove <address>= drops one, with two refusals: the primary stays until
84=email primary <address>= names another verified address, and the last
85verified address stays, since activation, login links and commit
86identity all resolve through verified addresses. Removing a verified
87address re-evaluates signature states, so a commit authored from it
88shows as =signed_email_mismatch= afterwards. A removed address is free
89for any account to claim.
83 90
84The states you will see, in decreasing order of trust: =verified=, 91The states you will see, in decreasing order of trust: =verified=,
85=signed_unknown_key= (valid signature, key not registered here — register 92=signed_unknown_key= (valid signature, key not registered here — register
cmd/gitbay/main.go +3
@@ -379,6 +379,9 @@ func authCmd() *cobra.Command {
379 group("email", "manage email addresses", 379 group("email", "manage email addresses",
380 pass("add", "add an address and get a verification code by mail", passOpts{server: []string{"email", "add"}}), 380 pass("add", "add an address and get a verification code by mail", passOpts{server: []string{"email", "add"}}),
381 pass("verify", "confirm a verification code", passOpts{server: []string{"email", "verify"}}), 381 pass("verify", "confirm a verification code", passOpts{server: []string{"email", "verify"}}),
382 pass("list", "list the addresses on your account", passOpts{server: []string{"email", "list"}}),
383 pass("remove", "remove an address; not the primary, nor the last verified one", passOpts{server: []string{"email", "remove"}}),
384 pass("primary", "make a verified address the primary", passOpts{server: []string{"email", "primary"}}),
382 ), 385 ),
383 group("pgp", "manage OpenPGP keys", 386 group("pgp", "manage OpenPGP keys",
384 pass("list", "list registered PGP keys", passOpts{server: []string{"pgp", "list"}}), 387 pass("list", "list registered PGP keys", passOpts{server: []string{"pgp", "list"}}),
e2e/emailremove_test.go added +64
@@ -0,0 +1,64 @@
1package e2e
2
3import (
4 "encoding/json"
5 "fmt"
6 "strings"
7 "testing"
8)
9
10// An address can be removed and the primary moved, within the rules the
11// store holds: the primary stays until another is primary, and the last
12// verified address stays (#181).
13func TestEmailRemoveAndPrimary(t *testing.T) {
14 smtp := startFakeSMTP(t)
15 inst := startInstanceWith(t, fmt.Sprintf(
16 "[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n", smtp.addr))
17 key := inst.newKey(t, "gus")
18 inst.admin(t, "admin", "user", "create", "gus",
19 "--key", key+".pub", "--email", "gus@primary.test", "--verified")
20 for _, a := range []string{"typo@example.test", "gus@next.test"} {
21 if _, errOut, code := inst.ssh(t, key, "", "email", "add", a); code != 0 {
22 t.Fatalf("email add %s: %s", a, errOut)
23 }
24 }
25
26 if _, errOut, code := inst.ssh(t, key, "", "email", "remove", "gus@primary.test"); code != 4 || !strings.Contains(errOut, "primary") {
27 t.Fatalf("removing the primary: exit %d %s", code, errOut)
28 }
29 if _, errOut, code := inst.ssh(t, key, "", "email", "remove", "nobody@example.test"); code != 3 {
30 t.Fatalf("removing an absent address: exit %d %s", code, errOut)
31 }
32 if _, errOut, code := inst.ssh(t, key, "", "email", "primary", "gus@next.test"); code != 4 || !strings.Contains(errOut, "not verified") {
33 t.Fatalf("unverified address as primary: exit %d %s", code, errOut)
34 }
35 if _, errOut, code := inst.ssh(t, key, "", "email", "remove", "typo@example.test"); code != 0 {
36 t.Fatalf("removing an unverified address: exit %d %s", code, errOut)
37 }
38
39 inst.admin(t, "admin", "email", "verify", "gus", "gus@next.test")
40 if _, errOut, code := inst.ssh(t, key, "", "email", "primary", "gus@next.test"); code != 0 {
41 t.Fatalf("email primary: exit %d %s", code, errOut)
42 }
43 if _, errOut, code := inst.ssh(t, key, "", "email", "remove", "gus@primary.test"); code != 0 {
44 t.Fatalf("removing the former primary: exit %d %s", code, errOut)
45 }
46
47 out, errOut, code := inst.ssh(t, key, "", "email", "list", "--json")
48 if code != 0 {
49 t.Fatalf("email list: exit %d %s", code, errOut)
50 }
51 var env struct {
52 Data []struct {
53 Address string `json:"address"`
54 Verified bool `json:"verified"`
55 Primary bool `json:"primary"`
56 } `json:"data"`
57 }
58 if err := json.Unmarshal([]byte(out), &env); err != nil {
59 t.Fatalf("email list --json: %v\n%s", err, out)
60 }
61 if len(env.Data) != 1 || env.Data[0].Address != "gus@next.test" || !env.Data[0].Verified || !env.Data[0].Primary {
62 t.Fatalf("addresses after the changes: %+v", env.Data)
63 }
64}
e2e/readonly_test.go +1
@@ -90,6 +90,7 @@ func TestReadOnlyCommandsWriteNothing(t *testing.T) {
90 "explore": {}, 90 "explore": {},
91 "audit": {}, 91 "audit": {},
92 "keys list": {}, 92 "keys list": {},
93 "email list": {},
93 "pgp list": {}, 94 "pgp list": {},
94 "token list": {}, 95 "token list": {},
95 "web sessions list": {}, 96 "web sessions list": {},
internal/control/register.go +78
@@ -31,6 +31,84 @@ func init() {
31 register(Command{Path: []string{"email", "verify"}, 31 register(Command{Path: []string{"email", "verify"},
32 Summary: "confirm a verification code", 32 Summary: "confirm a verification code",
33 Usage: "email verify <code>", Run: runEmailVerify}) 33 Usage: "email verify <code>", Run: runEmailVerify})
34 register(Command{Path: []string{"email", "list"},
35 Summary: "list the addresses on your account",
36 Usage: "email list",
37 ReadOnly: true, Run: runEmailList})
38 register(Command{Path: []string{"email", "remove"},
39 Summary: "remove an address; not the primary, nor the last verified one",
40 Usage: "email remove <address>", Run: runEmailRemove})
41 register(Command{Path: []string{"email", "primary"},
42 Summary: "make a verified address the primary",
43 Usage: "email primary <address>", Run: runEmailPrimary})
44}
45
46func runEmailList(c *Ctx, args []string) int {
47 if len(args) != 0 {
48 return c.fail(protocol.ExitUsage, "usage: email list [--json]")
49 }
50 emails, err := c.Store.ListEmails(c.User.ID)
51 if err != nil {
52 return c.fail(protocol.ExitFailure, "listing addresses: %v", err)
53 }
54 type out struct {
55 Address string `json:"address"`
56 Verified bool `json:"verified"`
57 VerifiedBy string `json:"verified_by,omitempty"`
58 Primary bool `json:"primary"`
59 }
60 ds := make([]out, 0, len(emails))
61 for _, e := range emails {
62 ds = append(ds, out{e.Address, e.Verified, e.VerifiedBy, e.Primary})
63 }
64 return c.emit(ds, func(w io.Writer) {
65 for _, d := range ds {
66 state := "unverified"
67 if d.Verified {
68 state = "verified"
69 }
70 if d.Primary {
71 state += "\tprimary"
72 }
73 fmt.Fprintf(w, "%s\t%s\n", d.Address, state)
74 }
75 })
76}
77
78// emailErr maps the store's refusals onto exit codes: a missing address is
79// not found, a rule is denied, anything else is a failure.
80func emailErr(c *Ctx, verb string, err error) int {
81 switch {
82 case errors.Is(err, store.ErrNotFound):
83 return c.fail(protocol.ExitNotFound, "no such address on your account")
84 case errors.Is(err, store.ErrPrimaryEmail), errors.Is(err, store.ErrLastVerifiedEmail), errors.Is(err, store.ErrUnverifiedEmail):
85 return c.fail(protocol.ExitDenied, "%v", err)
86 }
87 return c.fail(protocol.ExitFailure, "%s: %v", verb, err)
88}
89
90func runEmailRemove(c *Ctx, args []string) int {
91 if len(args) != 1 {
92 return c.fail(protocol.ExitUsage, "usage: email remove <address>")
93 }
94 if err := c.Store.RemoveEmail(c.User.ID, args[0]); err != nil {
95 return emailErr(c, "removing address", err)
96 }
97 return c.emit(map[string]string{"address": args[0], "status": "removed"}, func(w io.Writer) {
98 fmt.Fprintf(w, "%s removed\n", args[0])
99 })
100}
101
102func runEmailPrimary(c *Ctx, args []string) int {
103 if len(args) != 1 {
104 return c.fail(protocol.ExitUsage, "usage: email primary <address>")
105 }
106 if err := c.Store.SetPrimaryEmail(c.User.ID, args[0]); err != nil {
107 return emailErr(c, "setting primary", err)
108 }
109 return c.emit(map[string]string{"address": args[0], "status": "primary"}, func(w io.Writer) {
110 fmt.Fprintf(w, "%s is now the primary address\n", args[0])
111 })
34} 112}
35 113
36func siteHost(cfg config.Config) string { 114func siteHost(cfg config.Config) string {
internal/httpd/account.go +12
@@ -179,6 +179,18 @@ func (s *Server) accountSubmit(w http.ResponseWriter, r *http.Request, u store.U
179 return 179 return
180 } 180 }
181 back("", "address verified") 181 back("", "address verified")
182 case "email-remove":
183 if _, msg, ok := s.runControl(u, []string{"email", "remove", r.FormValue("address")}); !ok {
184 back(msg, "")
185 return
186 }
187 back("", "address removed")
188 case "email-primary":
189 if _, msg, ok := s.runControl(u, []string{"email", "primary", r.FormValue("address")}); !ok {
190 back(msg, "")
191 return
192 }
193 back("", "primary address changed")
182 case "profile": 194 case "profile":
183 format := r.FormValue("format") 195 format := r.FormValue("format")
184 if format != "org" { 196 if format != "org" {
internal/store/emails_test.go added +187
@@ -0,0 +1,187 @@
1package store
2
3import (
4 "errors"
5 "testing"
6 "time"
7)
8
9func emailFixture(t *testing.T) (*Store, int64) {
10 t.Helper()
11 s := open(t)
12 if err := s.MigrateUp(); err != nil {
13 t.Fatal(err)
14 }
15 uid, err := s.CreateUser("gus", false)
16 if err != nil {
17 t.Fatal(err)
18 }
19 if err := s.AddEmail(uid, "gus@primary.test", "smtp", true); err != nil {
20 t.Fatal(err)
21 }
22 return s, uid
23}
24
25func hasEmail(t *testing.T, s *Store, uid int64, address string) bool {
26 t.Helper()
27 list, err := s.ListEmails(uid)
28 if err != nil {
29 t.Fatal(err)
30 }
31 for _, e := range list {
32 if e.Address == address {
33 return true
34 }
35 }
36 return false
37}
38
39func epoch(t *testing.T, s *Store) int64 {
40 t.Helper()
41 v, err := s.KeyEpoch()
42 if err != nil {
43 t.Fatal(err)
44 }
45 return v
46}
47
48// Removing an unverified address takes its pending verification code
49// with it, and does not touch the key epoch: an unverified address was
50// never a trust input.
51func TestRemoveEmailUnverified(t *testing.T) {
52 s, uid := emailFixture(t)
53 if err := s.AddEmail(uid, "typo@example.test", "", false); err != nil {
54 t.Fatal(err)
55 }
56 if err := s.CreateEmailToken(uid, "typo@example.test", "hash1", time.Hour); err != nil {
57 t.Fatal(err)
58 }
59 before := epoch(t, s)
60 if err := s.RemoveEmail(uid, "typo@example.test"); err != nil {
61 t.Fatal(err)
62 }
63 if hasEmail(t, s, uid, "typo@example.test") {
64 t.Fatal("address still listed after removal")
65 }
66 if _, err := s.ConsumeEmailToken(uid, "hash1"); !errors.Is(err, ErrNotFound) {
67 t.Fatalf("verification code survived the address: %v", err)
68 }
69 if got := epoch(t, s); got != before {
70 t.Fatalf("key epoch moved %d -> %d for an unverified address", before, got)
71 }
72}
73
74// A verified address is a trust input for signature states, so removing
75// one invalidates the cache.
76func TestRemoveEmailVerifiedBumpsKeyEpoch(t *testing.T) {
77 s, uid := emailFixture(t)
78 if err := s.AddEmail(uid, "old@example.test", "smtp", false); err != nil {
79 t.Fatal(err)
80 }
81 before := epoch(t, s)
82 if err := s.RemoveEmail(uid, "old@example.test"); err != nil {
83 t.Fatal(err)
84 }
85 if got := epoch(t, s); got != before+1 {
86 t.Fatalf("key epoch %d -> %d, want +1", before, got)
87 }
88}
89
90func TestRemoveEmailRefusesPrimary(t *testing.T) {
91 s, uid := emailFixture(t)
92 if err := s.AddEmail(uid, "other@example.test", "smtp", false); err != nil {
93 t.Fatal(err)
94 }
95 if err := s.RemoveEmail(uid, "gus@primary.test"); !errors.Is(err, ErrPrimaryEmail) {
96 t.Fatalf("removing the primary: %v", err)
97 }
98 if !hasEmail(t, s, uid, "gus@primary.test") {
99 t.Fatal("primary removed despite refusal")
100 }
101}
102
103// Activation, login links and commit identity all resolve through
104// verified addresses; the last one stays even when it is not primary.
105func TestRemoveEmailRefusesLastVerified(t *testing.T) {
106 s := open(t)
107 if err := s.MigrateUp(); err != nil {
108 t.Fatal(err)
109 }
110 uid, err := s.CreateUser("gus", false)
111 if err != nil {
112 t.Fatal(err)
113 }
114 if err := s.AddEmail(uid, "gus@primary.test", "", true); err != nil {
115 t.Fatal(err)
116 }
117 if err := s.AddEmail(uid, "only@example.test", "smtp", false); err != nil {
118 t.Fatal(err)
119 }
120 if err := s.RemoveEmail(uid, "only@example.test"); !errors.Is(err, ErrLastVerifiedEmail) {
121 t.Fatalf("removing the only verified address: %v", err)
122 }
123 if !hasEmail(t, s, uid, "only@example.test") {
124 t.Fatal("last verified address removed despite refusal")
125 }
126}
127
128// An address on another account, or on none, is not found: the
129// uniqueness of addresses must not let one account act on another's.
130func TestRemoveEmailNotFound(t *testing.T) {
131 s, uid := emailFixture(t)
132 other, err := s.CreateUser("ada", false)
133 if err != nil {
134 t.Fatal(err)
135 }
136 if err := s.AddEmail(other, "ada@example.test", "", true); err != nil {
137 t.Fatal(err)
138 }
139 if err := s.RemoveEmail(uid, "ada@example.test"); !errors.Is(err, ErrNotFound) {
140 t.Fatalf("another account's address: %v", err)
141 }
142 if !hasEmail(t, s, other, "ada@example.test") {
143 t.Fatal("another account's address was removed")
144 }
145 if err := s.RemoveEmail(uid, "nobody@example.test"); !errors.Is(err, ErrNotFound) {
146 t.Fatalf("absent address: %v", err)
147 }
148}
149
150func TestSetPrimaryEmail(t *testing.T) {
151 s, uid := emailFixture(t)
152 if err := s.AddEmail(uid, "new@example.test", "smtp", false); err != nil {
153 t.Fatal(err)
154 }
155 if err := s.AddEmail(uid, "pending@example.test", "", false); err != nil {
156 t.Fatal(err)
157 }
158 if err := s.SetPrimaryEmail(uid, "pending@example.test"); !errors.Is(err, ErrUnverifiedEmail) {
159 t.Fatalf("unverified address as primary: %v", err)
160 }
161 if err := s.SetPrimaryEmail(uid, "nobody@example.test"); !errors.Is(err, ErrNotFound) {
162 t.Fatalf("absent address as primary: %v", err)
163 }
164 if err := s.SetPrimaryEmail(uid, "new@example.test"); err != nil {
165 t.Fatal(err)
166 }
167 list, err := s.ListEmails(uid)
168 if err != nil {
169 t.Fatal(err)
170 }
171 var primaries []string
172 for _, e := range list {
173 if e.Primary {
174 primaries = append(primaries, e.Address)
175 }
176 }
177 if len(primaries) != 1 || primaries[0] != "new@example.test" {
178 t.Fatalf("primaries after change: %v", primaries)
179 }
180 if addr, _ := s.PrimaryVerifiedEmail(uid); addr != "new@example.test" {
181 t.Fatalf("PrimaryVerifiedEmail after change: %q", addr)
182 }
183 // The old primary can go now.
184 if err := s.RemoveEmail(uid, "gus@primary.test"); err != nil {
185 t.Fatalf("removing the former primary: %v", err)
186 }
187}
internal/store/users.go +82
@@ -320,6 +320,88 @@ func (s *Store) AddEmail(userID int64, address, verifiedBy string, primary bool)
320 return tx.Commit() 320 return tx.Commit()
321} 321}
322 322
323var (
324 ErrPrimaryEmail = errors.New("that is the primary address; make another address primary first")
325 ErrLastVerifiedEmail = errors.New("that is the only verified address on the account; verify another first")
326 ErrUnverifiedEmail = errors.New("that address is not verified")
327)
328
329// RemoveEmail drops an address from the account, and any verification
330// code pending for it. The primary and the last verified address stay:
331// activation, login links and commit identity all resolve through
332// verified addresses. Removing a verified address bumps the key epoch,
333// since the signature cache keys on verified addresses too.
334func (s *Store) RemoveEmail(userID int64, address string) error {
335 tx, err := s.DB.Begin()
336 if err != nil {
337 return err
338 }
339 defer tx.Rollback()
340 var primary, verified bool
341 err = tx.QueryRow("SELECT is_primary, verified_at IS NOT NULL FROM emails WHERE user_id = ? AND address = ?",
342 userID, address).Scan(&primary, &verified)
343 if errors.Is(err, sql.ErrNoRows) {
344 return ErrNotFound
345 }
346 if err != nil {
347 return err
348 }
349 if primary {
350 return ErrPrimaryEmail
351 }
352 if verified {
353 var others int
354 if err := tx.QueryRow("SELECT count(*) FROM emails WHERE user_id = ? AND verified_at IS NOT NULL AND address != ?",
355 userID, address).Scan(&others); err != nil {
356 return err
357 }
358 if others == 0 {
359 return ErrLastVerifiedEmail
360 }
361 }
362 if _, err := tx.Exec("DELETE FROM email_tokens WHERE user_id = ? AND address = ?", userID, address); err != nil {
363 return err
364 }
365 if _, err := tx.Exec("DELETE FROM emails WHERE user_id = ? AND address = ?", userID, address); err != nil {
366 return err
367 }
368 if verified {
369 if err := bumpKeyEpoch(tx); err != nil {
370 return err
371 }
372 }
373 return tx.Commit()
374}
375
376// SetPrimaryEmail makes a verified address the account's primary. The
377// verified set is unchanged, so the key epoch is not.
378func (s *Store) SetPrimaryEmail(userID int64, address string) error {
379 tx, err := s.DB.Begin()
380 if err != nil {
381 return err
382 }
383 defer tx.Rollback()
384 var verified bool
385 err = tx.QueryRow("SELECT verified_at IS NOT NULL FROM emails WHERE user_id = ? AND address = ?",
386 userID, address).Scan(&verified)
387 if errors.Is(err, sql.ErrNoRows) {
388 return ErrNotFound
389 }
390 if err != nil {
391 return err
392 }
393 if !verified {
394 return ErrUnverifiedEmail
395 }
396 if _, err := tx.Exec("UPDATE emails SET is_primary = 0 WHERE user_id = ?", userID); err != nil {
397 return err
398 }
399 if _, err := tx.Exec("UPDATE emails SET is_primary = 1 WHERE user_id = ? AND address = ?", userID, address); err != nil {
400 return err
401 }
402 return tx.Commit()
403}
404
323func (s *Store) KeyEpoch() (int64, error) { 405func (s *Store) KeyEpoch() (int64, error) {
324 var v int64 406 var v int64
325 err := s.DB.QueryRow("SELECT value FROM settings WHERE key = 'key_epoch'").Scan(&v) 407 err := s.DB.QueryRow("SELECT value FROM settings WHERE key = 'key_epoch'").Scan(&v)
internal/web/templates/account.html +3 −1
@@ -56,7 +56,9 @@ account, and where notifications go.</p>
56{{range .Emails}}<li>{{.Address}} 56{{range .Emails}}<li>{{.Address}}
57 {{if .Primary}}<span class="chip">primary</span>{{end}} 57 {{if .Primary}}<span class="chip">primary</span>{{end}}
58 {{if .Verified}}<span class="badge badge-verified">verified{{with .VerifiedBy}} · {{.}}{{end}}</span> 58 {{if .Verified}}<span class="badge badge-verified">verified{{with .VerifiedBy}} · {{.}}{{end}}</span>
59 {{else}}<span class="badge badge-unsigned">unverified</span>{{end}}</li> 59 {{else}}<span class="badge badge-unsigned">unverified</span>{{end}}
60 {{if not .Primary}}{{if .Verified}}<form method="post" action="/settings" class="inline"><input type="hidden" name="field" value="email-primary"><input type="hidden" name="address" value="{{.Address}}"><button type="submit" class="linklike">Make primary</button></form>{{end}}
61 <form method="post" action="/settings" class="inline"><input type="hidden" name="field" value="email-remove"><input type="hidden" name="address" value="{{.Address}}"><button type="submit" class="linklike">Remove</button></form>{{end}}</li>
60{{end}}</ul>{{end}} 62{{end}}</ul>{{end}}
61<details class="editbox"> 63<details class="editbox">
62 <summary>Add an address</summary> 64 <summary>Add an address</summary>