krz/gitbay

A CLI-first git forge. cli forge git self-hosted https://gitbay.org

Commit d7a0d77892

d7a0d7789291fb1f35519864bdefb32e95bf764a

parent: 3dd60ce9a5

Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-05 19:37 UTC 

store, control: resolve a login link to any verified address, not just the primary

RequestLoginLink resolved a username through PrimaryVerifiedEmail, which
requires is_primary = 1 AND verified_at IS NOT NULL. An account that
verified a second address but left its primary unverified got nothing
by username, while the same address succeeded when given directly.

Add PreferredVerifiedEmail: the primary if it is verified, otherwise
the account's other verified address that sorts first by address.
PrimaryVerifiedEmail is unchanged — mr.go, commitfile.go,
notifications.go, and deps/worker.go all rely on it meaning exactly
"the primary, and only if verified" for git commit attribution and
notification routing, and none of them should follow a login-link
style fallback.

Closes #158

Layout: unified · split

e2e/emaillogin_test.go +52
@@ -68,6 +68,43 @@ func TestEmailLogin(t *testing.T) {
68 } 68 }
69} 69}
70 70
71// A verified secondary address stands in for an unverified primary:
72// resolution by username must not stop at the primary (#158).
73func TestEmailLoginResolvesVerifiedSecondary(t *testing.T) {
74 smtp := startFakeSMTP(t)
75 inst := startInstanceWith(t, fmt.Sprintf(
76 "[web]\nmode = \"accounts\"\n[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n",
77 smtp.addr))
78
79 key := inst.newKey(t, "gus")
80 inst.admin(t, "admin", "user", "create", "gus", "--key", key+".pub",
81 "--email", "gus@primary.test") // primary added, left unverified
82 if _, errOut, code := inst.ssh(t, key, "", "email", "add", "gus@secondary.test"); code != 0 {
83 t.Fatalf("email add: exit %d %s", code, errOut)
84 }
85 verifyCode := extractCode(t, smtp.waitMail(t, 0))
86 if _, errOut, code := inst.ssh(t, key, "", "email", "verify", verifyCode); code != 0 {
87 t.Fatalf("email verify: exit %d %s", code, errOut)
88 }
89
90 browser := newBrowser(t)
91 status, body := browserPost(t, browser, inst.base()+"/login", url.Values{"identifier": {"gus"}})
92 if status != 200 || !strings.Contains(body, "on its way") {
93 t.Fatalf("POST /login by username with an unverified primary: %d %s", status, body)
94 }
95
96 link := loginLinkIn(smtp.waitFor(t, "gus@secondary.test", "/login?token="))
97 if status, _ := browserGet(t, browser, inst.base()+link); status != 200 {
98 t.Fatalf("following the link: %d", status)
99 }
100 if _, body := browserGet(t, browser, inst.base()+"/settings"); !strings.Contains(body, "gus@secondary.test") {
101 t.Fatal("not logged in via the verified secondary address")
102 }
103 if len(smtp.mailTo("gus@primary.test")) != 0 {
104 t.Error("mailed the unverified primary")
105 }
106}
107
71// The response must not say whether an account exists. A different status, 108// The response must not say whether an account exists. A different status,
72// body, or destination answers "is this person here?" to anyone who asks. 109// body, or destination answers "is this person here?" to anyone who asks.
73func TestEmailLoginDoesNotEnumerate(t *testing.T) { 110func TestEmailLoginDoesNotEnumerate(t *testing.T) {
@@ -80,6 +117,18 @@ func TestEmailLoginDoesNotEnumerate(t *testing.T) {
80 // An account whose address was never verified must look like an absent 117 // An account whose address was never verified must look like an absent
81 // one, or an unverified address becomes an oracle. 118 // one, or an unverified address becomes an oracle.
82 inst.admin(t, "admin", "user", "create", "eve", "--email", "eve@example.test") 119 inst.admin(t, "admin", "user", "create", "eve", "--email", "eve@example.test")
120 // An unverified primary with a verified secondary resolves the same as
121 // a normal hit (#158) — this must be indistinguishable too.
122 frankKey := inst.newKey(t, "frank")
123 inst.admin(t, "admin", "user", "create", "frank", "--key", frankKey+".pub",
124 "--email", "frank@example.test")
125 if _, errOut, code := inst.ssh(t, frankKey, "", "email", "add", "frank2@example.test"); code != 0 {
126 t.Fatalf("email add: exit %d %s", code, errOut)
127 }
128 if _, errOut, code := inst.ssh(t, frankKey, "", "email", "verify",
129 extractCode(t, smtp.waitMail(t, 0))); code != 0 {
130 t.Fatalf("email verify: exit %d %s", code, errOut)
131 }
83 132
84 browser := newBrowser(t) 133 browser := newBrowser(t)
85 real1, bodyReal := browserPost(t, browser, inst.base()+"/login", 134 real1, bodyReal := browserPost(t, browser, inst.base()+"/login",
@@ -97,6 +146,8 @@ func TestEmailLoginDoesNotEnumerate(t *testing.T) {
97 url.Values{"identifier": {""}}) 146 url.Values{"identifier": {""}})
98 absentUser, bodyAbsentUser := browserPost(t, browser, inst.base()+"/login", 147 absentUser, bodyAbsentUser := browserPost(t, browser, inst.base()+"/login",
99 url.Values{"identifier": {"nosuchuser"}}) 148 url.Values{"identifier": {"nosuchuser"}})
149 unverPrimary, bodyUnverPrimary := browserPost(t, browser, inst.base()+"/login",
150 url.Values{"identifier": {"frank"}})
100 151
101 for _, c := range []struct { 152 for _, c := range []struct {
102 name string 153 name string
@@ -107,6 +158,7 @@ func TestEmailLoginDoesNotEnumerate(t *testing.T) {
107 {"unverified", unver, bodyUnver}, 158 {"unverified", unver, bodyUnver},
108 {"empty", empty, bodyEmpty}, 159 {"empty", empty, bodyEmpty},
109 {"absent-username", absentUser, bodyAbsentUser}, 160 {"absent-username", absentUser, bodyAbsentUser},
161 {"unverified-primary-verified-secondary", unverPrimary, bodyUnverPrimary},
110 } { 162 } {
111 if c.status != real1 || c.body != bodyReal { 163 if c.status != real1 || c.body != bodyReal {
112 t.Errorf("%s differs from a real address: status %d vs %d", c.name, c.status, real1) 164 t.Errorf("%s differs from a real address: status %d vs %d", c.name, c.status, real1)
internal/control/loginlink.go +1 −1
@@ -61,7 +61,7 @@ func RequestLoginLink(cfg config.Config, st *store.Store, identifier string) err
61 if err != nil { 61 if err != nil {
62 return nil 62 return nil
63 } 63 }
64 addr, err := st.PrimaryVerifiedEmail(u.ID) 64 addr, err := st.PreferredVerifiedEmail(u.ID)
65 if err != nil || addr == "" { 65 if err != nil || addr == "" {
66 return nil 66 return nil
67 } 67 }
internal/store/mrs.go +17
@@ -447,3 +447,20 @@ func (s *Store) PrimaryVerifiedEmail(userID int64) (string, error) {
447 } 447 }
448 return addr, err 448 return addr, err
449} 449}
450
451// PreferredVerifiedEmail returns the primary address if it is verified,
452// otherwise the account's other verified address that sorts first by
453// address; "" if none is verified. Unlike PrimaryVerifiedEmail, a verified
454// secondary counts: an account that verified one address but not its
455// primary still has somewhere to send a login link.
456func (s *Store) PreferredVerifiedEmail(userID int64) (string, error) {
457 var addr string
458 err := s.DB.QueryRow(
459 `SELECT address FROM emails WHERE user_id = ? AND verified_at IS NOT NULL
460 ORDER BY is_primary DESC, address LIMIT 1`,
461 userID).Scan(&addr)
462 if errors.Is(err, sql.ErrNoRows) {
463 return "", nil
464 }
465 return addr, err
466}
internal/store/mrs_test.go +71
@@ -71,3 +71,74 @@ func TestResolutionStampImported(t *testing.T) {
71 t.Fatalf("imported merge stamp: %+v", mr) 71 t.Fatalf("imported merge stamp: %+v", mr)
72 } 72 }
73} 73}
74
75// PreferredVerifiedEmail falls back to a verified secondary when the
76// primary is not verified, unlike PrimaryVerifiedEmail (#158).
77func TestPreferredVerifiedEmail(t *testing.T) {
78 s := open(t)
79 if err := s.MigrateUp(); err != nil {
80 t.Fatal(err)
81 }
82 uid, err := s.CreateUser("gus", false)
83 if err != nil {
84 t.Fatal(err)
85 }
86
87 if addr, err := s.PreferredVerifiedEmail(uid); err != nil || addr != "" {
88 t.Fatalf("no addresses at all: %q, %v", addr, err)
89 }
90
91 if err := s.AddEmail(uid, "primary@example.test", "", true); err != nil {
92 t.Fatal(err)
93 }
94 if addr, err := s.PreferredVerifiedEmail(uid); err != nil || addr != "" {
95 t.Fatalf("unverified primary only: %q, %v", addr, err)
96 }
97 if addr, err := s.PrimaryVerifiedEmail(uid); err != nil || addr != "" {
98 t.Fatalf("PrimaryVerifiedEmail on an unverified primary: %q, %v", addr, err)
99 }
100
101 if err := s.AddEmail(uid, "secondary@example.test", "smtp", false); err != nil {
102 t.Fatal(err)
103 }
104 if addr, err := s.PreferredVerifiedEmail(uid); err != nil || addr != "secondary@example.test" {
105 t.Fatalf("unverified primary, verified secondary: %q, %v", addr, err)
106 }
107 // PrimaryVerifiedEmail keeps meaning exactly what it says: still "",
108 // because the primary itself is still unverified.
109 if addr, err := s.PrimaryVerifiedEmail(uid); err != nil || addr != "" {
110 t.Fatalf("PrimaryVerifiedEmail with only the secondary verified: %q, %v", addr, err)
111 }
112
113 if err := s.VerifyEmail(uid, "primary@example.test", "admin"); err != nil {
114 t.Fatal(err)
115 }
116 if addr, err := s.PreferredVerifiedEmail(uid); err != nil || addr != "primary@example.test" {
117 t.Fatalf("both verified, primary should win: %q, %v", addr, err)
118 }
119}
120
121// With no verified primary, the choice among verified secondaries must not
122// depend on insertion or row order.
123func TestPreferredVerifiedEmailDeterministicTiebreak(t *testing.T) {
124 s := open(t)
125 if err := s.MigrateUp(); err != nil {
126 t.Fatal(err)
127 }
128 uid, err := s.CreateUser("gus", false)
129 if err != nil {
130 t.Fatal(err)
131 }
132 if err := s.AddEmail(uid, "primary@example.test", "", true); err != nil {
133 t.Fatal(err)
134 }
135 if err := s.AddEmail(uid, "zzz@example.test", "smtp", false); err != nil {
136 t.Fatal(err)
137 }
138 if err := s.AddEmail(uid, "aaa@example.test", "smtp", false); err != nil {
139 t.Fatal(err)
140 }
141 if addr, err := s.PreferredVerifiedEmail(uid); err != nil || addr != "aaa@example.test" {
142 t.Fatalf("tiebreak should be alphabetical: %q, %v", addr, err)
143 }
144}