feat(signup): always require an unverified email and drop the sign up email setting
7 files changed, +27 −64
- README.md modified +2 −2
- admin.go modified +3 −13
- flow_test.go modified +15 −26
- mail_test.go modified +0 −1
- templates/admin.html modified +1 −10
- templates/register.html modified +2 −2
- web.go modified +4 −10
README.md modified
| @@ -23,12 +23,12 @@ Small and lightweight git server for self hosting. Repositories are served over | ||
| 23 | 23 | - History search on the log page through git's pickaxe, either commits that change how often a string occurs (`-S`) or commits whose diff has a line matching a regex (`-G`), which answers when a line arrived or vanished without cloning |
| 24 | 24 | - Atom feeds of commits per branch, of issues and of pull requests per repository, and of every public commit by a user at `~you/feed`, which stand in for notifications when mail is not configured. Each page names its feeds in a `<link rel="alternate">`, so a reader finds them from the ordinary page URL |
| 25 | 25 | - Rename, transfer to another user and archive from the repository settings. The old path keeps working as a permanent redirect for the web, HTTPS clones and SSH, provided the visitor can read the repository at its new home, and an archived repository refuses pushes, pull requests and comments but stays readable |
| 26 | -- An optional email address on each account, used for password resets and for notifications about pull requests, reviews and issues on your repositories when mail is configured. The sign up form requires one by default, an admin can make it optional or drop the question, and nobody has to confirm an address before using their account | |
| 26 | +- An optional email address on each account, used for password resets and for notifications about pull requests, reviews and issues on your repositories when mail is configured. Every account gives one at sign up, and nobody has to confirm it before using their account | |
| 27 | 27 | - TOTP two factor authentication with a QR code for the authenticator app and eight single use recovery codes, which is what a password alone cannot give you on something that accepts pushes |
| 28 | 28 | - SSH keys and access tokens live in settings. HTTPS auth is plain basic auth with a token or your password |
| 29 | 29 | - Relative links and images in a README resolve against the repository, so a `docs/diagram.png` reference renders rather than breaking |
| 30 | 30 | - Registration is open, invite only or closed, and an administrator switches between the three on the admin page or with `gitpocket registration`. Signing up needs only a name and password, and each address can create a handful of accounts every few minutes |
| 31 | -- `/admin` exists for accounts you mark with `gitpocket admin <name>` and 404s for everyone else. Its overview counts accounts, repositories and disk use, and separate sections handle people (ban, unban and grant admin), sign up (registration mode, the email question and invite links), the front page order and storage with a usage bar per owner | |
| 31 | +- `/admin` exists for accounts you mark with `gitpocket admin <name>` and 404s for everyone else. Its overview counts accounts, repositories and disk use, and separate sections handle people (ban, unban and grant admin), sign up (registration mode and invite links), the front page order and storage with a usage bar per owner | |
| 32 | 32 | |
| 33 | 33 | ## Design |
| 34 | 34 | |
admin.go modified
| @@ -1,7 +1,6 @@ | ||
| 1 | 1 | package main |
| 2 | 2 | |
| 3 | 3 | import ( |
| 4 | - "cmp" | |
| 5 | 4 | "net/http" |
| 6 | 5 | "net/url" |
| 7 | 6 | "os" |
| @@ -129,12 +128,6 @@ var adminSections = []struct{ Key, Title string }{ | ||
| 129 | 128 | {"overview", "overview"}, {"people", "people"}, {"signup", "sign up"}, {"projects", "front page"}, {"storage", "storage"}, |
| 130 | 129 | } |
| 131 | 130 | |
| 132 | -var signupEmailModes = []string{"off", "optional", "required"} | |
| 133 | - | |
| 134 | -func (s *Store) SignupEmail() string { | |
| 135 | - return s.Setting("signupemail", "required") | |
| 136 | -} | |
| 137 | - | |
| 138 | 131 | type siteStats struct { |
| 139 | 132 | Users, Orgs, Admins, Banned, Repos, Private, Unlisted int |
| 140 | 133 | } |
| @@ -174,7 +167,6 @@ func (s *Server) admin(w http.ResponseWriter, r *http.Request) { | ||
| 174 | 167 | data["Stats"] = s.store.Stats() |
| 175 | 168 | data["Recent"] = accounts[:min(5, len(accounts))] |
| 176 | 169 | data["Registration"] = s.store.Registration() |
| 177 | - data["SignupEmail"] = s.store.SignupEmail() | |
| 178 | 170 | data["Mail"] = s.mailer != nil |
| 179 | 171 | data["Used"] = usage(filepath.Join(s.root, "repos")) |
| 180 | 172 | data["Quota"] = readQuota(s.root) |
| @@ -189,7 +181,6 @@ func (s *Server) admin(w http.ResponseWriter, r *http.Request) { | ||
| 189 | 181 | data["Accounts"], data["Query"] = shown, q |
| 190 | 182 | case "signup": |
| 191 | 183 | data["Registration"] = s.store.Registration() |
| 192 | - data["SignupEmail"] = s.store.SignupEmail() | |
| 193 | 184 | data["Invites"] = s.store.Invites() |
| 194 | 185 | data["Mail"] = s.mailer != nil |
| 195 | 186 | case "projects": |
| @@ -240,13 +231,12 @@ func (s *Server) adminAction(w http.ResponseWriter, r *http.Request, u *User) { | ||
| 240 | 231 | name := strings.TrimPrefix(strings.TrimSpace(r.FormValue("name")), "~") |
| 241 | 232 | switch r.FormValue("action") { |
| 242 | 233 | case "signup": |
| 243 | - mode, email := cmp.Or(r.FormValue("mode"), s.store.Registration()), cmp.Or(r.FormValue("email"), s.store.SignupEmail()) | |
| 244 | - if !validRegistration(mode) || !slices.Contains(signupEmailModes, email) { | |
| 245 | - fail("Unknown sign up setting") | |
| 234 | + mode := r.FormValue("mode") | |
| 235 | + if !validRegistration(mode) { | |
| 236 | + fail("Unknown registration mode") | |
| 246 | 237 | return |
| 247 | 238 | } |
| 248 | 239 | s.store.SetSetting("registration", mode) |
| 249 | - s.store.SetSetting("signupemail", email) | |
| 250 | 240 | case "invite": |
| 251 | 241 | if _, err := s.store.CreateInvite(); err != nil { |
| 252 | 242 | fail(err.Error()) |
flow_test.go modified
| @@ -57,20 +57,19 @@ func session(w *httptest.ResponseRecorder) string { | ||
| 57 | 57 | |
| 58 | 58 | func TestRegistrationModes(t *testing.T) { |
| 59 | 59 | s, h := testServer(t) |
| 60 | - s.store.SetSetting("signupemail", "off") | |
| 61 | - if w := post(h, "/register", url.Values{"name": {"alice"}, "password": {"correct horse"}}, ""); w.Code != http.StatusSeeOther || session(w) == "" { | |
| 60 | + if w := post(h, "/register", url.Values{"name": {"alice"}, "password": {"correct horse"}, "email": {"alice@example.org"}}, ""); w.Code != http.StatusSeeOther || session(w) == "" { | |
| 62 | 61 | t.Fatalf("open registration failed: %d", w.Code) |
| 63 | 62 | } |
| 64 | 63 | s.store.SetSetting("registration", "invite") |
| 65 | - if w := post(h, "/register", url.Values{"name": {"bob"}, "password": {"correct horse"}}, ""); w.Code != 400 { | |
| 64 | + if w := post(h, "/register", url.Values{"name": {"bob"}, "password": {"correct horse"}, "email": {"bob@example.org"}}, ""); w.Code != 400 { | |
| 66 | 65 | t.Errorf("invite mode let bob in without a code: %d", w.Code) |
| 67 | 66 | } |
| 68 | 67 | code, _ := s.store.CreateInvite() |
| 69 | - if w := post(h, "/register", url.Values{"name": {"bob"}, "password": {"correct horse"}, "invite": {code}}, ""); w.Code != http.StatusSeeOther { | |
| 68 | + if w := post(h, "/register", url.Values{"name": {"bob"}, "password": {"correct horse"}, "email": {"bob@example.org"}, "invite": {code}}, ""); w.Code != http.StatusSeeOther { | |
| 70 | 69 | t.Errorf("invite mode refused a valid code: %d", w.Code) |
| 71 | 70 | } |
| 72 | 71 | s.store.SetSetting("registration", "closed") |
| 73 | - if w := post(h, "/register", url.Values{"name": {"carol"}, "password": {"correct horse"}}, ""); w.Code != 403 { | |
| 72 | + if w := post(h, "/register", url.Values{"name": {"carol"}, "password": {"correct horse"}, "email": {"carol@example.org"}}, ""); w.Code != 403 { | |
| 74 | 73 | t.Errorf("closed mode allowed registration: %d", w.Code) |
| 75 | 74 | } |
| 76 | 75 | if w := get(h, "/", ""); strings.Contains(w.Body.String(), `href="/register"`) { |
| @@ -84,7 +83,7 @@ func TestLoginAndHeaders(t *testing.T) { | ||
| 84 | 83 | if w := post(h, "/login", url.Values{"name": {"alice"}, "password": {"wrong password"}}, ""); w.Code != 401 { |
| 85 | 84 | t.Errorf("wrong password gave %d", w.Code) |
| 86 | 85 | } |
| 87 | - w := post(h, "/login", url.Values{"name": {"alice"}, "password": {"correct horse"}}, "") | |
| 86 | + w := post(h, "/login", url.Values{"name": {"alice"}, "password": {"correct horse"}, "email": {"alice@example.org"}}, "") | |
| 88 | 87 | if session(w) == "" { |
| 89 | 88 | t.Fatal("login did not start a session") |
| 90 | 89 | } |
| @@ -227,27 +226,17 @@ func TestWorkplaceLink(t *testing.T) { | ||
| 227 | 226 | } |
| 228 | 227 | } |
| 229 | 228 | |
| 230 | -func TestSignupEmailSetting(t *testing.T) { | |
| 229 | +func TestSignupEmail(t *testing.T) { | |
| 231 | 230 | s, h := testServer(t) |
| 232 | - s.store.SetSetting("signupemail", "off") | |
| 233 | - post(h, "/register", url.Values{"name": {"alice"}, "password": {"correct horse"}, "email": {"a@example.org"}}, "") | |
| 234 | - u, _ := s.store.UserByName("alice") | |
| 235 | - if s.store.Account(u.ID).Email != "" { | |
| 236 | - t.Error("email stored while the setting is off") | |
| 237 | - } | |
| 238 | - if strings.Contains(get(h, "/register", "").Body.String(), `name="email"`) { | |
| 239 | - t.Error("email field shown while off") | |
| 240 | - } | |
| 241 | - s.store.db.Exec(`delete from settings where key = 'signupemail'`) | |
| 242 | 231 | if w := post(h, "/register", url.Values{"name": {"bob"}, "password": {"correct horse"}}, ""); w.Code != 400 { |
| 243 | - t.Errorf("required email not enforced: %d", w.Code) | |
| 232 | + t.Errorf("registration without an email allowed: %d", w.Code) | |
| 244 | 233 | } |
| 245 | - if w := post(h, "/register", url.Values{"name": {"bob"}, "password": {"correct horse"}, "email": {"b@example.org"}}, ""); w.Code != http.StatusSeeOther { | |
| 246 | - t.Errorf("registration with required email failed: %d", w.Code) | |
| 234 | + if w := post(h, "/register", url.Values{"name": {"bob"}, "password": {"correct horse"}, "email": {"b@example.org"}}, ""); w.Code != http.StatusSeeOther || session(w) == "" { | |
| 235 | + t.Errorf("registration with an email failed: %d", w.Code) | |
| 247 | 236 | } |
| 248 | 237 | bob, _ := s.store.UserByName("bob") |
| 249 | - if s.store.Account(bob.ID).Email != "b@example.org" { | |
| 250 | - t.Error("required email not stored") | |
| 238 | + if a := s.store.Account(bob.ID); a.Email != "b@example.org" || a.Verified { | |
| 239 | + t.Error("email not stored unverified") | |
| 251 | 240 | } |
| 252 | 241 | } |
| 253 | 242 | |
| @@ -261,11 +250,11 @@ func TestAdminSections(t *testing.T) { | ||
| 261 | 250 | t.Errorf("section %s: %d", sec, w.Code) |
| 262 | 251 | } |
| 263 | 252 | } |
| 264 | - if w := post(h, "/admin", url.Values{"s": {"signup"}, "action": {"signup"}, "mode": {"closed"}, "email": {"required"}}, id); w.Code != http.StatusSeeOther || s.store.Registration() != "closed" || s.store.SignupEmail() != "required" { | |
| 265 | - t.Errorf("signup settings not saved: %d %s %s", w.Code, s.store.Registration(), s.store.SignupEmail()) | |
| 253 | + if w := post(h, "/admin", url.Values{"s": {"signup"}, "action": {"signup"}, "mode": {"closed"}}, id); w.Code != http.StatusSeeOther || s.store.Registration() != "closed" { | |
| 254 | + t.Errorf("registration mode not saved: %d %s", w.Code, s.store.Registration()) | |
| 266 | 255 | } |
| 267 | - if w := post(h, "/admin", url.Values{"s": {"signup"}, "action": {"signup"}, "mode": {"open"}, "email": {"sometimes"}}, id); !strings.Contains(w.Header().Get("Location"), "error") { | |
| 268 | - t.Error("bad email mode accepted") | |
| 256 | + if w := post(h, "/admin", url.Values{"s": {"signup"}, "action": {"signup"}, "mode": {"sometimes"}}, id); !strings.Contains(w.Header().Get("Location"), "error") { | |
| 257 | + t.Error("bad registration mode accepted") | |
| 269 | 258 | } |
| 270 | 259 | } |
| 271 | 260 | |
mail_test.go modified
| @@ -91,7 +91,6 @@ func TestSignupVerifyAndReset(t *testing.T) { | ||
| 91 | 91 | fake, m := withCloudflare(t) |
| 92 | 92 | s, h := testServer(t) |
| 93 | 93 | s.mailer = m |
| 94 | - s.store.SetSetting("signupemail", "optional") | |
| 95 | 94 | link := regexp.MustCompile(`https://example\.org(/[a-z]+/[0-9a-f]+)`) |
| 96 | 95 | |
| 97 | 96 | w := post(h, "/register", url.Values{"name": {"alice"}, "password": {"correct horse"}, "email": {"Alice@Example.org"}}, "") |
templates/admin.html modified
| @@ -10,7 +10,7 @@ | ||
| 10 | 10 | <tr><td>Organisations</td><td>{{.Orgs}}</td></tr> |
| 11 | 11 | <tr><td>Repositories</td><td>{{.Repos}} <span class="muted">({{.Private}} private, {{.Unlisted}} unlisted)</span></td></tr> |
| 12 | 12 | <tr><td>Disk usage</td><td>{{size $d.Used}}{{if $d.Quota}} <span class="muted">(quota {{$d.QuotaText}} per owner)</span>{{end}}</td></tr> |
| 13 | -<tr><td>Registration</td><td>{{if eq $d.Registration "open"}}open to everyone{{else if eq $d.Registration "invite"}}invite only{{else}}closed{{end}}, email {{if eq $d.SignupEmail "off"}}not asked{{else}}{{$d.SignupEmail}}{{end}} <a class="muted" href="/admin?s=signup">change</a></td></tr> | |
| 13 | +<tr><td>Registration</td><td>{{if eq $d.Registration "open"}}open to everyone{{else if eq $d.Registration "invite"}}invite only{{else}}closed{{end}} <a class="muted" href="/admin?s=signup">change</a></td></tr> | |
| 14 | 14 | <tr><td>Mail</td><td>{{if $d.Mail}}configured{{else}}not configured, resets go through <code>gitpocket passwd</code>{{end}}</td></tr> |
| 15 | 15 | </table>{{end}} |
| 16 | 16 | <h3 class="rule">Newest accounts</h3> |
| @@ -38,15 +38,6 @@ | ||
| 38 | 38 | <label class="check"><input type="radio" name="mode" value="closed"{{if eq $d.Registration "closed"}} checked{{end}}> Closed, accounts only come from <code>gitpocket user</code></label> |
| 39 | 39 | <div class="actions left"><button class="btn primary">Save ▸</button></div> |
| 40 | 40 | </form> |
| 41 | -<h3 class="rule">Email on sign up</h3> | |
| 42 | -<form method="post" action="/admin" class="stack"> | |
| 43 | -<input type="hidden" name="action" value="signup"><input type="hidden" name="s" value="signup"> | |
| 44 | -<label class="check"><input type="radio" name="email" value="required"{{if eq $d.SignupEmail "required"}} checked{{end}}> Required</label> | |
| 45 | -<label class="check"><input type="radio" name="email" value="optional"{{if eq $d.SignupEmail "optional"}} checked{{end}}> Optional</label> | |
| 46 | -<label class="check"><input type="radio" name="email" value="off"{{if eq $d.SignupEmail "off"}} checked{{end}}> Not asked</label> | |
| 47 | -<small>Addresses are not verified at sign up.{{if $d.Mail}} People can confirm theirs later from settings.{{end}}</small> | |
| 48 | -<div class="actions left"><button class="btn primary">Save ▸</button></div> | |
| 49 | -</form> | |
| 50 | 41 | <h3 class="rule">Invites</h3> |
| 51 | 42 | {{if $d.Invites}}<table class="keys">{{range $d.Invites}}<tr><td><code>https://{{$.Host}}/register?invite={{.}}</code></td><td><form method="post" action="/admin" class="inline"><input type="hidden" name="s" value="signup"><input type="hidden" name="code" value="{{.}}"><button class="btn small danger" name="action" value="revoke">Revoke</button></form></td></tr>{{end}}</table>{{else}}<p class="muted">No unused invites.</p>{{end}} |
| 52 | 43 | <form method="post" action="/admin" class="inline"><input type="hidden" name="s" value="signup"><button class="btn" name="action" value="invite">New invite ▸</button></form> |
templates/register.html modified
| @@ -1,11 +1,11 @@ | ||
| 1 | 1 | {{define "content"}} |
| 2 | 2 | <div class="auth"> |
| 3 | 3 | <h3 class="rule">Register for {{.Host}} <small>or <a href="/login">log in</a></small></h3> |
| 4 | -{{if eq .Data.Mode "closed"}}<p class="info">Registration is closed on {{.Host}}. Ask an administrator for an account.</p>{{else}}<p>Registration gives you a page at <code>~username</code>, repositories under it, SSH and HTTPS push, and pull requests on anything public. {{if eq .Data.AskEmail "off"}}No email is needed{{else if eq .Data.AskEmail "optional"}}An email address is optional{{else}}An email address is needed{{end}} and you are logged in straight away.</p> | |
| 4 | +{{if eq .Data.Mode "closed"}}<p class="info">Registration is closed on {{.Host}}. Ask an administrator for an account.</p>{{else}}<p>Registration gives you a page at <code>~username</code>, repositories under it, SSH and HTTPS push, and pull requests on anything public. It needs an email address, which is never verified, and you are logged in straight away.</p> | |
| 5 | 5 | <form method="post" action="/register" class="stack centred"> |
| 6 | 6 | <label>Username <input name="name" required autofocus autocomplete="username" pattern="[a-z0-9][a-z0-9_.-]{0,38}" value="{{.Data.Name}}"><small>Lowercase letters, digits, dot, dash or underscore.</small></label> |
| 7 | 7 | <label>Password <input name="password" type="password" required minlength="8" autocomplete="new-password"></label> |
| 8 | -{{if ne .Data.AskEmail "off"}}<label>Email{{if eq .Data.AskEmail "optional"}} <span class="muted">(optional)</span>{{end}} <input name="email" type="email" autocomplete="email" maxlength="200" value="{{.Data.Email}}"{{if eq .Data.AskEmail "required"}} required{{end}}><small>{{if .Data.Mail}}We send one link to confirm it, after which it can reset your password. {{end}}Never shown publicly.</small></label>{{end}} | |
| 8 | +<label>Email <input name="email" type="email" autocomplete="email" maxlength="200" value="{{.Data.Email}}" required><small>Never shown publicly.</small></label> | |
| 9 | 9 | {{if eq .Data.Mode "invite"}}<label>Invite code <input name="invite" required value="{{.Data.Invite}}"></label>{{end}} |
| 10 | 10 | <div class="actions"><button class="btn primary">Register ▸</button></div> |
| 11 | 11 | </form>{{end}} |
web.go modified
| @@ -312,14 +312,14 @@ func (s *Server) logout(w http.ResponseWriter, r *http.Request) { | ||
| 312 | 312 | } |
| 313 | 313 | |
| 314 | 314 | func (s *Server) registerForm(w http.ResponseWriter, r *http.Request) { |
| 315 | - s.render(w, r, "register", page{Title: "Register", Data: map[string]any{"Mode": s.store.Registration(), "Invite": r.URL.Query().Get("invite"), "Mail": s.mailer != nil, "AskEmail": s.store.SignupEmail()}}) | |
| 315 | + s.render(w, r, "register", page{Title: "Register", Data: map[string]any{"Mode": s.store.Registration(), "Invite": r.URL.Query().Get("invite"), "Mail": s.mailer != nil}}) | |
| 316 | 316 | } |
| 317 | 317 | |
| 318 | 318 | func (s *Server) register(w http.ResponseWriter, r *http.Request) { |
| 319 | 319 | invite := strings.TrimSpace(r.FormValue("invite")) |
| 320 | 320 | retry := func(code int, msg string) { |
| 321 | 321 | w.WriteHeader(code) |
| 322 | - s.render(w, r, "register", page{Title: "Register", Error: msg, Data: map[string]any{"Mode": s.store.Registration(), "Invite": invite, "Mail": s.mailer != nil, "AskEmail": s.store.SignupEmail(), "Email": r.FormValue("email"), "Name": r.FormValue("name")}}) | |
| 322 | + s.render(w, r, "register", page{Title: "Register", Error: msg, Data: map[string]any{"Mode": s.store.Registration(), "Invite": invite, "Mail": s.mailer != nil, "Email": r.FormValue("email"), "Name": r.FormValue("name")}}) | |
| 323 | 323 | } |
| 324 | 324 | mode := s.store.Registration() |
| 325 | 325 | if mode == "closed" { |
| @@ -337,14 +337,8 @@ func (s *Server) register(w http.ResponseWriter, r *http.Request) { | ||
| 337 | 337 | return |
| 338 | 338 | } |
| 339 | 339 | email := strings.TrimSpace(strings.ToLower(r.FormValue("email"))) |
| 340 | - switch ask := s.store.SignupEmail(); { | |
| 341 | - case ask == "off": | |
| 342 | - email = "" | |
| 343 | - case ask == "required" && email == "": | |
| 344 | - retry(400, "An email address is required to register here") | |
| 345 | - return | |
| 346 | - case email != "" && !validEmail(email): | |
| 347 | - retry(400, "That does not look like an email address") | |
| 340 | + if !validEmail(email) { | |
| 341 | + retry(400, "A valid email address is needed to register") | |
| 348 | 342 | return |
| 349 | 343 | } |
| 350 | 344 | if mode == "invite" && !s.store.UseInvite(invite) { |