tenseleyflow/shithub / ad881a7

Browse files

S10: Account — username change with redirect, 3-per-60d rate limit

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
ad881a7eed2441d0b73e7c2dfc69cc89439ac642
Parents
7e9997f
Tree
fff0a29

5 changed files

StatusFile+-
A internal/web/handlers/auth/account.go 162 0
A internal/web/handlers/auth/account_test.go 162 0
M internal/web/handlers/auth/auth.go 2 0
M internal/web/handlers/auth/auth_test.go 2 0
A internal/web/templates/settings/account.html 33 0
internal/web/handlers/auth/account.goadded
@@ -0,0 +1,162 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package auth
4
+
5
+import (
6
+	"errors"
7
+	"net/http"
8
+	"strings"
9
+	"time"
10
+
11
+	"github.com/jackc/pgx/v5"
12
+	"github.com/jackc/pgx/v5/pgconn"
13
+	"github.com/jackc/pgx/v5/pgtype"
14
+
15
+	authpkg "github.com/tenseleyFlow/shithub/internal/auth"
16
+	"github.com/tenseleyFlow/shithub/internal/auth/audit"
17
+	usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc"
18
+	"github.com/tenseleyFlow/shithub/internal/web/middleware"
19
+)
20
+
21
+// usernameChangeWindow is the rolling rate-limit window for renames.
22
+// Counted against username_redirects.changed_at — the redirect row IS
23
+// the audit trail, so no separate counter table is needed.
24
+const usernameChangeWindow = 60 * 24 * time.Hour
25
+
26
+// usernameChangeMax caps how many renames a user can make in
27
+// usernameChangeWindow. Three is GitHub's posted ceiling.
28
+const usernameChangeMax = 3
29
+
30
+// settingsAccountForm renders GET /settings/account.
31
+func (h *Handlers) settingsAccountForm(w http.ResponseWriter, r *http.Request) {
32
+	h.renderAccountForm(w, r, "", "")
33
+}
34
+
35
+// settingsAccountUsername handles POST /settings/account/username.
36
+func (h *Handlers) settingsAccountUsername(w http.ResponseWriter, r *http.Request) {
37
+	if err := r.ParseForm(); err != nil {
38
+		h.d.Render.HTTPError(w, r, http.StatusBadRequest, "form parse")
39
+		return
40
+	}
41
+	user := middleware.CurrentUserFromContext(r.Context())
42
+	desired := strings.ToLower(strings.TrimSpace(r.PostFormValue("new_username")))
43
+
44
+	// Quick local checks: shape, reservation, no-op.
45
+	if desired == user.Username {
46
+		h.renderAccountForm(w, r, "That's already your username.", "")
47
+		return
48
+	}
49
+	if !usernameRE.MatchString(desired) {
50
+		h.renderAccountForm(w, r, "Username must be 1–39 characters: lowercase letters, digits, and hyphens (no leading/trailing hyphen).", "")
51
+		return
52
+	}
53
+	if authpkg.IsReserved(desired) {
54
+		h.renderAccountForm(w, r, "That username is reserved.", "")
55
+		return
56
+	}
57
+
58
+	// Rate limit.
59
+	count, err := h.q.CountRecentUsernameChanges(r.Context(), h.d.Pool, usersdb.CountRecentUsernameChangesParams{
60
+		UserID:    user.ID,
61
+		ChangedAt: pgtype.Timestamptz{Time: time.Now().Add(-usernameChangeWindow), Valid: true},
62
+	})
63
+	if err != nil {
64
+		h.d.Logger.ErrorContext(r.Context(), "account: count renames", "error", err)
65
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "")
66
+		return
67
+	}
68
+	if count >= usernameChangeMax {
69
+		h.renderAccountForm(w, r, "You've changed your username too many times recently. Try again later.", "")
70
+		return
71
+	}
72
+
73
+	// Tx: redirect-row + rename. The unique constraint on
74
+	// username_redirects.old_username AND users.username (citext) blocks
75
+	// taking a name held by either an active user or a recent redirect.
76
+	tx, err := h.d.Pool.Begin(r.Context())
77
+	if err != nil {
78
+		h.d.Logger.ErrorContext(r.Context(), "account: begin tx", "error", err)
79
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "")
80
+		return
81
+	}
82
+	defer func() { _ = tx.Rollback(r.Context()) }()
83
+
84
+	if err := h.q.InsertUsernameRedirect(r.Context(), tx, usersdb.InsertUsernameRedirectParams{
85
+		OldUsername: user.Username,
86
+		UserID:      user.ID,
87
+	}); err != nil {
88
+		h.d.Logger.ErrorContext(r.Context(), "account: insert redirect", "error", err)
89
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "")
90
+		return
91
+	}
92
+
93
+	if err := h.q.RenameUser(r.Context(), tx, usersdb.RenameUserParams{
94
+		ID:       user.ID,
95
+		Username: desired,
96
+	}); err != nil {
97
+		if isUsernameTaken(err) {
98
+			h.renderAccountForm(w, r, "That username is taken.", "")
99
+			return
100
+		}
101
+		h.d.Logger.ErrorContext(r.Context(), "account: rename", "error", err)
102
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "")
103
+		return
104
+	}
105
+
106
+	if err := tx.Commit(r.Context()); err != nil {
107
+		h.d.Logger.ErrorContext(r.Context(), "account: commit", "error", err)
108
+		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "")
109
+		return
110
+	}
111
+
112
+	if err := h.d.Audit.Record(r.Context(), h.d.Pool, user.ID,
113
+		audit.ActionUsernameChanged, audit.TargetUser, user.ID, map[string]any{
114
+			"from": user.Username,
115
+			"to":   desired,
116
+		}); err != nil {
117
+		h.d.Logger.WarnContext(r.Context(), "account: audit rename", "error", err)
118
+	}
119
+
120
+	h.renderAccountForm(w, r, "", "Username updated to "+desired+".")
121
+}
122
+
123
+// renderAccountForm is the shared render path. The current username is
124
+// pulled out of context so it reflects post-rename state on success.
125
+func (h *Handlers) renderAccountForm(w http.ResponseWriter, r *http.Request, errMsg, successMsg string) {
126
+	user := middleware.CurrentUserFromContext(r.Context())
127
+	// Re-read the canonical username from the DB so the form always
128
+	// reflects post-commit state when called after a successful rename.
129
+	canonical := user.Username
130
+	if row, err := h.q.GetUserByID(r.Context(), h.d.Pool, user.ID); err == nil {
131
+		canonical = row.Username
132
+	}
133
+
134
+	count, _ := h.q.CountRecentUsernameChanges(r.Context(), h.d.Pool, usersdb.CountRecentUsernameChangesParams{
135
+		UserID:    user.ID,
136
+		ChangedAt: pgtype.Timestamptz{Time: time.Now().Add(-usernameChangeWindow), Valid: true},
137
+	})
138
+	h.renderPage(w, r, "settings/account", map[string]any{
139
+		"Title":             "Account",
140
+		"CSRFToken":         middleware.CSRFTokenForRequest(r),
141
+		"SettingsActive":    "account",
142
+		"CurrentUsername":   canonical,
143
+		"RecentRenames":     count,
144
+		"MaxRenames":        usernameChangeMax,
145
+		"WindowDays":        int(usernameChangeWindow / (24 * time.Hour)),
146
+		"RenameRateLimited": count >= usernameChangeMax,
147
+		"Error":             errMsg,
148
+		"Success":           successMsg,
149
+	})
150
+}
151
+
152
+// isUsernameTaken matches the unique-violation surface for username collisions.
153
+// Both users.username (citext, unique) and username_redirects.old_username
154
+// (unique) raise SQLSTATE 23505 here.
155
+func isUsernameTaken(err error) bool {
156
+	var pgErr *pgconn.PgError
157
+	if errors.As(err, &pgErr) {
158
+		return pgErr.Code == "23505"
159
+	}
160
+	// pgx v5 also wraps tx errors; double-check the not-rows path.
161
+	return errors.Is(err, pgx.ErrNoRows)
162
+}
internal/web/handlers/auth/account_test.goadded
@@ -0,0 +1,162 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package auth_test
4
+
5
+import (
6
+	"io"
7
+	"net/http"
8
+	"net/url"
9
+	"strings"
10
+	"testing"
11
+)
12
+
13
+// loginAccountUser is the same login helper as profile_test.go but
14
+// against a different account so tests stay isolated when run in
15
+// parallel.
16
+func loginAccountUser(t *testing.T, name string) *client {
17
+	t.Helper()
18
+	httpsrv, captor := newTestServer(t, false)
19
+	cli := newClient(t, httpsrv)
20
+	mustSignup(t, cli, name, name+"@example.com", "correct horse battery staple")
21
+	tok := extractTokenFromMessage(t, captor.all()[0], "/verify-email")
22
+	_ = cli.get(t, "/verify-email/"+tok).Body.Close()
23
+
24
+	csrf := cli.extractCSRF(t, "/login")
25
+	resp := cli.post(t, "/login", url.Values{
26
+		"csrf_token": {csrf},
27
+		"username":   {name},
28
+		"password":   {"correct horse battery staple"},
29
+	})
30
+	if resp.StatusCode != http.StatusSeeOther {
31
+		t.Fatalf("login: %d", resp.StatusCode)
32
+	}
33
+	_ = resp.Body.Close()
34
+	return cli
35
+}
36
+
37
+func TestAccount_RenameRoundtrip(t *testing.T) {
38
+	t.Parallel()
39
+	cli := loginAccountUser(t, "renamea")
40
+	csrf := cli.extractCSRF(t, "/settings/account")
41
+
42
+	resp := cli.post(t, "/settings/account/username", url.Values{
43
+		"csrf_token":   {csrf},
44
+		"new_username": {"renameb"},
45
+	})
46
+	defer func() { _ = resp.Body.Close() }()
47
+	if resp.StatusCode != http.StatusOK {
48
+		body, _ := io.ReadAll(resp.Body)
49
+		t.Fatalf("status=%d body=%s", resp.StatusCode, body)
50
+	}
51
+	body, _ := io.ReadAll(resp.Body)
52
+	if !strings.Contains(string(body), "Username updated to renameb") {
53
+		t.Errorf("missing success message; got: %s", body)
54
+	}
55
+	if !strings.Contains(string(body), "USERNAME=renameb") {
56
+		t.Errorf("expected USERNAME=renameb in body; got: %s", body)
57
+	}
58
+	if !strings.Contains(string(body), "USED=1/3") {
59
+		t.Errorf("expected USED=1/3 after rename; got: %s", body)
60
+	}
61
+
62
+	// The old name should now redirect to the new profile (/oldname → /newname).
63
+	resp = cli.get(t, "/renamea")
64
+	defer func() { _ = resp.Body.Close() }()
65
+	// /renamea hits the catch-all in this minimal test rig — we don't have
66
+	// the profile handler mounted, so the only thing we can verify here
67
+	// is the DB state.
68
+}
69
+
70
+func TestAccount_RejectsReservedName(t *testing.T) {
71
+	t.Parallel()
72
+	cli := loginAccountUser(t, "reserveda")
73
+	csrf := cli.extractCSRF(t, "/settings/account")
74
+	resp := cli.post(t, "/settings/account/username", url.Values{
75
+		"csrf_token":   {csrf},
76
+		"new_username": {"settings"},
77
+	})
78
+	defer func() { _ = resp.Body.Close() }()
79
+	body, _ := io.ReadAll(resp.Body)
80
+	if !strings.Contains(string(body), "reserved") {
81
+		t.Fatalf("expected reserved error, got: %s", body)
82
+	}
83
+}
84
+
85
+func TestAccount_RejectsInvalidShape(t *testing.T) {
86
+	t.Parallel()
87
+	cli := loginAccountUser(t, "shapea")
88
+	csrf := cli.extractCSRF(t, "/settings/account")
89
+	// UPPER is normalized to lowercase by the handler (user-friendly), so
90
+	// it isn't on this rejection list.
91
+	for _, bad := range []string{"-leading", "trailing-", "with spaces", ""} {
92
+		resp := cli.post(t, "/settings/account/username", url.Values{
93
+			"csrf_token":   {csrf},
94
+			"new_username": {bad},
95
+		})
96
+		body, _ := io.ReadAll(resp.Body)
97
+		_ = resp.Body.Close()
98
+		if !strings.Contains(string(body), "Username must") && !strings.Contains(string(body), "1–39") {
99
+			t.Errorf("input %q: expected validation error, got: %s", bad, body)
100
+		}
101
+	}
102
+}
103
+
104
+func TestAccount_RejectsTaken(t *testing.T) {
105
+	t.Parallel()
106
+	// Two clients on the SAME server so they share the same DB.
107
+	httpsrv, captor := newTestServer(t, false)
108
+	cliA := newClient(t, httpsrv)
109
+	cliB := newClient(t, httpsrv)
110
+
111
+	mustSignup(t, cliA, "takena", "takena@example.com", "correct horse battery staple")
112
+	mustSignup(t, cliB, "takenb", "takenb@example.com", "correct horse battery staple")
113
+	for _, m := range captor.all() {
114
+		// Verify each.
115
+		if tok := extractTokenFromMessage(t, m, "/verify-email"); tok != "" {
116
+			_ = cliA.get(t, "/verify-email/"+tok).Body.Close()
117
+		}
118
+	}
119
+	csrf := cliA.extractCSRF(t, "/login")
120
+	_ = cliA.post(t, "/login", url.Values{
121
+		"csrf_token": {csrf}, "username": {"takena"}, "password": {"correct horse battery staple"},
122
+	}).Body.Close()
123
+
124
+	csrf = cliA.extractCSRF(t, "/settings/account")
125
+	resp := cliA.post(t, "/settings/account/username", url.Values{
126
+		"csrf_token":   {csrf},
127
+		"new_username": {"takenb"}, // already used by cliB
128
+	})
129
+	defer func() { _ = resp.Body.Close() }()
130
+	body, _ := io.ReadAll(resp.Body)
131
+	if !strings.Contains(string(body), "taken") {
132
+		t.Fatalf("expected taken error, got: %s", body)
133
+	}
134
+}
135
+
136
+func TestAccount_RateLimitAtThree(t *testing.T) {
137
+	t.Parallel()
138
+	cli := loginAccountUser(t, "ratea")
139
+
140
+	// Burn three rename slots.
141
+	names := []string{"rateb", "ratec", "rated"}
142
+	for _, n := range names {
143
+		csrf := cli.extractCSRF(t, "/settings/account")
144
+		resp := cli.post(t, "/settings/account/username", url.Values{
145
+			"csrf_token":   {csrf},
146
+			"new_username": {n},
147
+		})
148
+		_ = resp.Body.Close()
149
+	}
150
+
151
+	// Fourth attempt should be blocked.
152
+	csrf := cli.extractCSRF(t, "/settings/account")
153
+	resp := cli.post(t, "/settings/account/username", url.Values{
154
+		"csrf_token":   {csrf},
155
+		"new_username": {"ratee"},
156
+	})
157
+	defer func() { _ = resp.Body.Close() }()
158
+	body, _ := io.ReadAll(resp.Body)
159
+	if !strings.Contains(string(body), "too many") {
160
+		t.Fatalf("expected rate-limit error, got: %s", body)
161
+	}
162
+}
internal/web/handlers/auth/auth.gomodified
@@ -134,6 +134,8 @@ func (h *Handlers) Mount(r chi.Router) {
134134
 				r.Post("/settings/profile/avatar", h.settingsAvatarUpload)
135135
 				r.Post("/settings/profile/avatar/remove", h.settingsAvatarRemove)
136136
 			}
137
+			r.Get("/settings/account", h.settingsAccountForm)
138
+			r.Post("/settings/account/username", h.settingsAccountUsername)
137139
 			r.Get("/settings/keys", h.sshKeysList)
138140
 			r.Post("/settings/keys", h.sshKeysAdd)
139141
 			r.Post("/settings/keys/{id}/delete", h.sshKeysDelete)
internal/web/handlers/auth/auth_test.gomodified
@@ -163,6 +163,7 @@ func authTemplatesFS() fs.FS {
163163
 	//nolint:gosec // G101 false positive: test fixture, not a hardcoded credential.
164164
 	tokensTpl := `{{ define "page" }}<form>{{ with .CreateError }}<p class=error>{{.}}</p>{{ end }}<input name=csrf_token value="{{.CSRFToken}}">{{ if .JustCreatedRaw }}RAW={{.JustCreatedRaw}}{{ end }}TOKENS={{ range .Tokens }}{{.ID}}:{{.TokenPrefix}}{{ if .RevokedAt.Valid }}:revoked{{ end }};{{ end }}</form>{{ end }}`
165165
 	profileTpl := `{{ define "page" }}<h1>Public profile</h1>{{ with .Error }}<p class=error>{{.}}</p>{{ end }}{{ with .Success }}<p class=notice>{{.}}</p>{{ end }}<form><input name=csrf_token value="{{.CSRFToken}}">DISPLAY={{.Form.DisplayName}};BIO={{.Form.Bio}};LOCATION={{.Form.Location}};WEBSITE={{.Form.Website}};COMPANY={{.Form.Company}};PRONOUNS={{.Form.Pronouns}};</form>{{ if .HasAvatar }}<form action="/settings/profile/avatar/remove" method=POST><input name=csrf_token value="{{.CSRFToken}}"><button>Remove</button></form>{{ end }}{{ end }}`
166
+	accountTpl := `{{ define "page" }}<h1>Account</h1>{{ with .Error }}<p class=error>{{.}}</p>{{ end }}{{ with .Success }}<p class=notice>{{.}}</p>{{ end }}<form action="/settings/account/username" method=POST><input name=csrf_token value="{{.CSRFToken}}">USERNAME={{.CurrentUsername}};USED={{.RecentRenames}}/{{.MaxRenames}};</form>{{ end }}`
166167
 	errorPage := `{{ define "page" }}<h1>{{.Status}} {{.StatusText}}</h1><p>{{.Message}}</p>{{ end }}`
167168
 	return fstest.MapFS{
168169
 		"_layout.html":               {Data: []byte(layout)},
@@ -179,6 +180,7 @@ func authTemplatesFS() fs.FS {
179180
 		"settings/keys.html":         {Data: []byte(keysTpl)},
180181
 		"settings/tokens.html":       {Data: []byte(tokensTpl)},
181182
 		"settings/profile.html":      {Data: []byte(profileTpl)},
183
+		"settings/account.html":      {Data: []byte(accountTpl)},
182184
 		"errors/404.html":            {Data: []byte(errorPage)},
183185
 		"errors/403.html":            {Data: []byte(errorPage)},
184186
 		"errors/429.html":            {Data: []byte(errorPage)},
internal/web/templates/settings/account.htmladded
@@ -0,0 +1,33 @@
1
+{{ define "page" -}}
2
+<div class="shithub-settings-page">
3
+  {{ template "settings-nav" . }}
4
+  <div class="shithub-settings-content">
5
+    <h1>Account</h1>
6
+
7
+    {{ with .Error }}<p class="shithub-flash shithub-flash-error" role="alert">{{ . }}</p>{{ end }}
8
+    {{ with .Success }}<p class="shithub-flash shithub-flash-notice" role="status">{{ . }}</p>{{ end }}
9
+
10
+    <section class="shithub-settings-section">
11
+      <h2>Change username</h2>
12
+      <p>Changing your username can have <a href="https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-personal-account-settings/changing-your-github-username">unintended side effects</a>. Your old name becomes a redirect for 30 days, after which it can be claimed by someone else.</p>
13
+      <p>You've used <strong>{{ .RecentRenames }}/{{ .MaxRenames }}</strong> changes in the last {{ .WindowDays }} days.</p>
14
+
15
+      <form method="POST" action="/settings/account/username" novalidate>
16
+        <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
17
+        <label>
18
+          <span>New username</span>
19
+          <input type="text" name="new_username" required maxlength="39" pattern="^[a-z0-9](?:[a-z0-9-]{0,37}[a-z0-9])?$"
20
+                 placeholder="{{ .CurrentUsername }}" autocomplete="off" spellcheck="false">
21
+          <small>Lowercase letters, digits, and hyphens. 1–39 characters; no leading/trailing hyphen.</small>
22
+        </label>
23
+        <button type="submit" class="shithub-button shithub-button-primary"{{ if .RenameRateLimited }} disabled{{ end }}>Change username</button>
24
+      </form>
25
+    </section>
26
+
27
+    <section class="shithub-settings-section">
28
+      <h2>Export account data</h2>
29
+      <p>Download a portable archive of your repositories, issues, and metadata. Coming soon.</p>
30
+    </section>
31
+  </div>
32
+</div>
33
+{{- end }}