feat: account names are validated at the three doors a human types one through
Wire domains.ValidAccountName into handleNewAccount (console form), handleCLINewAccount (POST /auth/cli/accounts), and runAccountsCreate (carlos accounts create, both the -owner and console-login paths) — the three edges where a human types an account name. mintAccount/auto-mint is deliberately left unvalidated: the personal account is named by keymail address, and a mint that could refuse would strand a signed-in user on landing. Existing accounts with non-slug names stay valid; only creation is gated, so no migration or normalization. Updated tests that created accounts through these three edges with non-slug fixture names (Kass, Woodstar, Orphan, Second) to use valid slugs, and added TestNewAccountNameValidated plus a CLI 400 assertion for a non-slug POST body.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGdTfzwT492ysLPDaWfpvJ
9 files changed,
+82
−35
accounts.go+6 −1auth_e2e_test.go+5 −5e2e_test.go+9 −9internal/console/authcli.go+5 −2internal/console/authcli_test.go+8 −2internal/console/authowner_test.go+1 −1internal/console/server.go+5 −2internal/console/server_account_test.go+36 −8internal/console/templates/newaccount.html+7 −5
diff --git a/accounts.go b/accounts.go| index 2eb55ba8b..43fd3f125 100644 |
| --- a/accounts.go |
| +++ b/accounts.go |
| @@ -79,7 +79,7 @@ func RunAccounts(args []string) { |
| func runAccountsCreate(args []string) { |
| fs := flag.NewFlagSet("accounts create", flag.ExitOnError) |
| regPath := fs.String("registry", defaultRegistry(), "registry database") |
| - name := fs.String("name", "", "account name (free text, need not be unique)") |
| + name := fs.String("name", "", "account name (a-z, 0-9, hyphen; need not be unique)") |
| owner := fs.String("owner", "", "keymail address that owns the account — the box-side/headless path, "+ |
| "and what to use when this terminal is not logged in") |
| console := fs.String("console", "", "which logged-in console to create it through (only needed with several)") |
| @@ -87,6 +87,11 @@ func runAccountsCreate(args []string) { |
| if strings.TrimSpace(*name) == "" { |
| log.Fatal("usage: carlos accounts create -name <name> [-owner <address>]") |
| } |
| + if !domains.ValidAccountName(strings.TrimSpace(*name)) { |
| + log.Fatalf("accounts create: %q is not a usable account name — 1-63 characters of a-z, 0-9\n"+ |
| + "and hyphen, no leading or trailing hyphen. The name appears in console URLs and\n"+ |
| + "resolves -account.", *name) |
| + } |
| // An ownerless account is the failure this whole path exists to prevent: |
| // it materializes into the console with no members, which means nobody can |
diff --git a/auth_e2e_test.go b/auth_e2e_test.go| index bf3bc0d07..936a409ac 100644 |
| --- a/auth_e2e_test.go |
| +++ b/auth_e2e_test.go |
| @@ -117,7 +117,7 @@ func TestAccountsCreateNeedsAnOwner(t *testing.T) { |
| "CARLOS_CREDENTIALS="+filepath.Join(tmp, "credentials"), |
| "CARLOS_DATA_DIR="+tmp, |
| ) |
| - cmd := exec.Command(bin, "accounts", "create", "-name", "Orphan") |
| + cmd := exec.Command(bin, "accounts", "create", "-name", "orphan") |
| cmd.Env = env |
| out, err := cmd.CombinedOutput() |
| if err == nil { |
| @@ -140,7 +140,7 @@ func TestAccountsCreateWithOwnerFlag(t *testing.T) { |
| bin := buildCarlos(t) |
| tmp := t.TempDir() |
| storeDir := filepath.Join(tmp, "store") |
| - cmd := exec.Command(bin, "accounts", "create", "-name", "Kass", "-owner", "paul@keymail.dev") |
| + cmd := exec.Command(bin, "accounts", "create", "-name", "kass", "-owner", "paul@keymail.dev") |
| cmd.Env = append(os.Environ(), |
| "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| "CARLOS_CREDENTIALS="+filepath.Join(tmp, "credentials"), |
| @@ -166,8 +166,8 @@ func TestAccountsCreateWithOwnerFlag(t *testing.T) { |
| if err := json.Unmarshal(body, &m); err != nil { |
| t.Fatal(err) |
| } |
| - if m.Owner != "paul@keymail.dev" || m.Name != "Kass" { |
| - t.Fatalf("manifest = %+v, want Kass owned by paul", m) |
| + if m.Owner != "paul@keymail.dev" || m.Name != "kass" { |
| + t.Fatalf("manifest = %+v, want kass owned by paul", m) |
| } |
| } |
| @@ -189,7 +189,7 @@ func TestAccountsCreateUsesTheLoggedInIdentity(t *testing.T) { |
| if err := os.WriteFile(creds, raw, 0o600); err != nil { |
| t.Fatal(err) |
| } |
| - cmd := exec.Command(bin, "accounts", "create", "-name", "Kass") |
| + cmd := exec.Command(bin, "accounts", "create", "-name", "kass") |
| cmd.Env = append(os.Environ(), |
| "CARLOS_CREDENTIALS="+creds, |
| "CARLOS_DATA_DIR="+tmp, |
diff --git a/e2e_test.go b/e2e_test.go| index a0b65ab73..2b1005feb 100644 |
| --- a/e2e_test.go |
| +++ b/e2e_test.go |
| @@ -725,7 +725,7 @@ func TestAccountsCLI(t *testing.T) { |
| // membership is authorization, so the operator could not open either one in |
| // the console. -owner is required, and its absence has to name the flag |
| // rather than print a shrug. |
| - failed := exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "Kass") |
| + failed := exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "kass") |
| failed.Env = env |
| refusal, err := failed.CombinedOutput() |
| if err == nil { |
| @@ -742,14 +742,14 @@ func TestAccountsCLI(t *testing.T) { |
| // A typo'd address is refused with the same rule the console's sign-in form |
| // uses — an owner keymail could never authenticate is a door that does not |
| // open either. |
| - failed = exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "paul") |
| + failed = exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "paul") |
| failed.Env = env |
| if refusal, err := failed.CombinedOutput(); err == nil { |
| t.Errorf("accounts create accepted the non-address owner %q:\n%s", "paul", refusal) |
| } |
| // create: a minted id, its frozen sqid printed for the operator to use. |
| - out := run("accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "Paul@Keymail.dev") |
| + out := run("accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "Paul@Keymail.dev") |
| m := regexp.MustCompile(`(?m)^\s*sqid:\s*(\S+)`).FindStringSubmatch(out) |
| if m == nil { |
| t.Fatalf("accounts create printed no sqid:\n%s", out) |
| @@ -780,7 +780,7 @@ func TestAccountsCLI(t *testing.T) { |
| } |
| if out := run("accounts", "list", "-registry", regPath); !strings.Contains(out, sqid) || |
| - !strings.Contains(out, "Kass") || !strings.Contains(out, "ops") { |
| + !strings.Contains(out, "kass") || !strings.Contains(out, "ops") { |
| t.Fatalf("accounts list = %q, want ops and the new account", out) |
| } |
| @@ -964,7 +964,7 @@ func TestAccountsMigrateTakesTheLiveObjects(t *testing.T) { |
| } |
| reg.Close() |
| - out, err := run("accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "paul@keymail.dev") |
| + out, err := run("accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "paul@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, out) |
| } |
| @@ -1073,7 +1073,7 @@ func TestAccountsMigrateConvergesADivergedPointer(t *testing.T) { |
| } |
| reg.Close() |
| - out, err := run("accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "paul@keymail.dev") |
| + out, err := run("accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "paul@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, out) |
| } |
| @@ -2098,7 +2098,7 @@ func TestRouteCLI(t *testing.T) { |
| }) |
| t.Run("-app form optionally filtered by -account", func(t *testing.T) { |
| - accOut, err := run("accounts", "create", "-registry", regPath, "-name", "Second", "-owner", "second@keymail.dev") |
| + accOut, err := run("accounts", "create", "-registry", regPath, "-name", "second", "-owner", "second@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, accOut) |
| } |
| @@ -2537,7 +2537,7 @@ func TestRoutePlatformCanaryRefusesMoveBetweenCanaryChannels(t *testing.T) { |
| // all opts into the cross-account sweep Tito's eighteen routes need. |
| func TestRouteAccountDefaultsToOpsAndAllOptsIn(t *testing.T) { |
| f := newRouteTestFixture(t) |
| - accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "Second", "-owner", "second@keymail.dev") |
| + accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "second", "-owner", "second@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, accOut) |
| } |
| @@ -2788,7 +2788,7 @@ func TestRouteChannelAloneNeverTouchesEnvironment(t *testing.T) { |
| // whose bundle genuinely exists. |
| func TestRouteEnvironmentAppBulkAllOrNothingAcrossAccounts(t *testing.T) { |
| f := newRouteTestFixture(t) |
| - accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "Second", "-owner", "second@keymail.dev") |
| + accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "second", "-owner", "second@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, accOut) |
| } |
diff --git a/internal/console/authcli.go b/internal/console/authcli.go| index 122cec00e..60db52c59 100644 |
| --- a/internal/console/authcli.go |
| +++ b/internal/console/authcli.go |
| @@ -33,6 +33,8 @@ import ( |
| "strings" |
| "sync" |
| "time" |
| + |
| + "github.com/carlosframework/platform/internal/domains" |
| ) |
| // Rate limits. Deliberately generous for humans and tight for scripts: the |
| @@ -699,8 +701,9 @@ func (s *Server) handleCLINewAccount(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| name := strings.TrimSpace(req.Name) |
| - if name == "" || len(name) > 64 { |
| - jsonError(w, http.StatusBadRequest, "an account needs a name (up to 64 characters)") |
| + if !domains.ValidAccountName(name) { |
| + jsonError(w, http.StatusBadRequest, |
| + "an account name appears in URLs and -account: 1-63 characters of a-z, 0-9 and hyphen, no leading or trailing hyphen") |
| return |
| } |
| ctx, cancel := context.WithTimeout(r.Context(), 20*time.Second) |
diff --git a/internal/console/authcli_test.go b/internal/console/authcli_test.go| index ee59bc508..b7be257e6 100644 |
| --- a/internal/console/authcli_test.go |
| +++ b/internal/console/authcli_test.go |
| @@ -412,12 +412,12 @@ func TestCLICreatesAccountOwnedByTheToken(t *testing.T) { |
| fx := newConsoleFixture(t) |
| token := cliToken(t, fx, "paul@keymail.dev", "code-acct") |
| - status, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "Kass"}, token) |
| + status, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "kass"}, token) |
| if status != http.StatusOK { |
| t.Fatalf("accounts create = %d %v", status, out) |
| } |
| sqid := str(t, out, "sqid") |
| - if sqid == "" || str(t, out, "name") != "Kass" || str(t, out, "owner") != "paul@keymail.dev" { |
| + if sqid == "" || str(t, out, "name") != "kass" || str(t, out, "owner") != "paul@keymail.dev" { |
| t.Fatalf("accounts create returned %v", out) |
| } |
| acct, err := fx.st.AccountBySqid(sqid) |
| @@ -440,6 +440,12 @@ func TestCLICreatesAccountOwnedByTheToken(t *testing.T) { |
| if status, _ := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": ""}, token); status != http.StatusBadRequest { |
| t.Fatalf("nameless accounts create = %d, want 400", status) |
| } |
| + // A non-slug name is refused the same way: it would appear in URLs and |
| + // -account exactly as typed. |
| + if status, _ := postJSON(t, fx.ts.URL+"/auth/cli/accounts", |
| + map[string]string{"name": "Paul's Stuff!"}, token); status != http.StatusBadRequest { |
| + t.Fatalf("non-slug accounts create = %d, want 400", status) |
| + } |
| } |
| // `carlos auth logout` retires the credential rather than just forgetting it. |
diff --git a/internal/console/authowner_test.go b/internal/console/authowner_test.go| index fc3a489ed..1fa7864ee 100644 |
| --- a/internal/console/authowner_test.go |
| +++ b/internal/console/authowner_test.go |
| @@ -117,7 +117,7 @@ func TestMaterializeRefusesAnUnusableOwner(t *testing.T) { |
| func TestConsoleMintStampsTheOwner(t *testing.T) { |
| fx := newConsoleFixture(t) |
| token := cliToken(t, fx, "paul@keymail.dev", "code-owner") |
| - _, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "Kass"}, token) |
| + _, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "kass"}, token) |
| sqid := str(t, out, "sqid") |
| body, _, err := fx.rel.Get(context.Background(), AccountManifestKey(sqid)) |
| if err != nil { |
diff --git a/internal/console/server.go b/internal/console/server.go| index f3fa77f6c..7f65c1a44 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -512,6 +512,9 @@ func (s *Server) memberAccount(addr string) (*Account, error) { |
| // what stops two boxes minting the same integer at the same moment. A console |
| // row whose manifest never landed would be an account that vanishes on the |
| // next rebuild — worse than a "new account" button that reports the outage. |
| +// |
| +// The name is NOT validated here: auto-mint names personal accounts by |
| +// keymail address, and landing must never refuse to mint (spec §1). |
| func (s *Server) mintAccount(ctx context.Context, name, ownerAddr string) (*Account, error) { |
| now := time.Now() |
| id, sqid, err := domains.MintAccount(ctx, s.cfg.Releases, name, ownerAddr, now) |
| @@ -679,10 +682,10 @@ func (s *Server) handleNewAccount(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| name := strings.TrimSpace(r.FormValue("name")) |
| - if name == "" || len(name) > 64 { |
| + if !domains.ValidAccountName(name) { |
| // Back to the screen the mistake was made on, not off to some other |
| // account: the form is a page now, and a page can show its own error. |
| - s.setFlash(w, "err", "An account needs a name (up to 64 characters).") |
| + s.setFlash(w, "err", "Account names appear in URLs: 1–63 characters of a-z, 0-9 and hyphen, no leading or trailing hyphen.") |
| http.Redirect(w, r, "/accounts/new", http.StatusSeeOther) |
| return |
| } |
diff --git a/internal/console/server_account_test.go b/internal/console/server_account_test.go| index e6d6b2f14..a36eb87ae 100644 |
| --- a/internal/console/server_account_test.go |
| +++ b/internal/console/server_account_test.go |
| @@ -514,26 +514,26 @@ func TestNewAccountScreen(t *testing.T) { |
| // Creating from the screen lands you in the account it made. |
| csrf := sessionToken(t, fx, c) |
| st, loc, _ := postForm(t, c, fx.ts.URL+"/accounts/new", |
| - url.Values{"csrf": {csrf}, "name": {"Woodstar"}}) |
| + url.Values{"csrf": {csrf}, "name": {"woodstar"}}) |
| if st != http.StatusSeeOther { |
| t.Fatalf("create = %d -> %q, want 303", st, loc) |
| } |
| accts, _ := fx.st.AccountsFor("paul@keymail.dev") |
| var made *Account |
| for i := range accts { |
| - if accts[i].Name == "Woodstar" { |
| + if accts[i].Name == "woodstar" { |
| made = &accts[i] |
| } |
| } |
| if made == nil { |
| - t.Fatalf("AccountsFor = %+v, want a Woodstar", accts) |
| + t.Fatalf("AccountsFor = %+v, want a woodstar", accts) |
| } |
| if loc != "/a/"+made.Sqid+"/" { |
| t.Fatalf("landed at %q, want /a/%s/", loc, made.Sqid) |
| } |
| // And the account you landed in is the one the header now names. |
| _, _, body = get(t, c, fx.ts.URL+loc) |
| - if h := header(t, body); !strings.Contains(h, "Woodstar") { |
| + if h := header(t, body); !strings.Contains(h, "woodstar") { |
| t.Errorf("the header does not name the account just created:\n%s", h) |
| } |
| } |
| @@ -549,11 +549,39 @@ func TestNewAccountScreenRejectsAnEmptyNameInPlace(t *testing.T) { |
| if st != http.StatusSeeOther || loc != "/accounts/new" { |
| t.Fatalf("empty name = %d -> %q, want 303 -> /accounts/new", st, loc) |
| } |
| - if _, _, body := get(t, c, fx.ts.URL+"/accounts/new"); !strings.Contains(body, "needs a name") { |
| + if _, _, body := get(t, c, fx.ts.URL+"/accounts/new"); !strings.Contains(body, "Account names appear in URLs") { |
| t.Errorf("the screen does not say why it refused:\n%s", body) |
| } |
| } |
| +// The three edges where a human types a name all refuse a non-slug; the |
| +// auto-mint path (address-named personal accounts) must NOT be validated — |
| +// a mint that can refuse strands a signed-in user. |
| +func TestNewAccountNameValidated(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + csrf := sessionToken(t, fx, c) |
| + before, err := fx.st.AccountsFor("paul@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + st, loc, _ := postForm(t, c, fx.ts.URL+"/accounts/new", url.Values{"csrf": {csrf}, "name": {"Paul's Stuff!"}}) |
| + if st != http.StatusSeeOther || loc != "/accounts/new" { |
| + t.Fatalf("bad name = %d -> %q, want 303 back to the form", st, loc) |
| + } |
| + after, err := fx.st.AccountsFor("paul@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(after) != len(before) { |
| + t.Fatal("a rejected name must not create an account") |
| + } |
| + if st, _, _ := postForm(t, c, fx.ts.URL+"/accounts/new", url.Values{"csrf": {csrf}, "name": {"amadan"}}); st != http.StatusSeeOther { |
| + t.Fatalf("good name = %d, want 303 into the new account", st) |
| + } |
| +} |
| + |
| // Visiting an account page updates the MRU; visiting /accounts/new does not |
| // — it is not an account you are "in", and moving the MRU there would send |
| // the next sign-in somewhere the user never chose. |
| @@ -675,19 +703,19 @@ func TestCreateAccountFromConsole(t *testing.T) { |
| csrf := sessionToken(t, fx, c) |
| st, loc, _ := postForm(t, c, fx.ts.URL+"/accounts/new", |
| - url.Values{"csrf": {csrf}, "name": {"Kass"}}) |
| + url.Values{"csrf": {csrf}, "name": {"kass"}}) |
| if st != http.StatusSeeOther || !strings.HasPrefix(loc, "/a/") { |
| t.Fatalf("create account = %d -> %q", st, loc) |
| } |
| accts, _ := fx.st.AccountsFor("paul@keymail.dev") |
| var made *Account |
| for i := range accts { |
| - if accts[i].Name == "Kass" { |
| + if accts[i].Name == "kass" { |
| made = &accts[i] |
| } |
| } |
| if made == nil || made.Role != RoleOwner { |
| - t.Fatalf("AccountsFor = %+v, want an owned Kass", accts) |
| + t.Fatalf("AccountsFor = %+v, want an owned kass", accts) |
| } |
| if _, _, err := fx.rel.Get(t.Context(), AccountManifestKey(made.Sqid)); err != nil { |
| t.Errorf("no manifest for the new account: %v", err) |
diff --git a/internal/console/templates/newaccount.html b/internal/console/templates/newaccount.html| index 211541b7c..ef42103ca 100644 |
| --- a/internal/console/templates/newaccount.html |
| +++ b/internal/console/templates/newaccount.html |
| @@ -7,12 +7,14 @@ and deploys independently of your other accounts.</p> |
| <form class="signin" method="post" action="/accounts/new"> |
| <input type="hidden" name="csrf" value="{{.CSRF}}"> |
| <label class="visually-hidden" for="account-name">Account name</label> |
| - <input id="account-name" type="text" name="name" placeholder="Kass" autofocus> |
| + <input id="account-name" type="text" name="name" placeholder="kass" autofocus |
| + pattern="[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?"> |
| + <p class="small">a-z, 0-9 and hyphens — it appears in your URLs</p> |
| <button type="submit">Create account</button> |
| </form> |
| -<p class="small">Names are free text, up to 64 characters, and deliberately |
| -not unique — two accounts may both be called “Blog”. Identity is the short |
| -id in the URL, so there is nothing to squat. You will be its first owner, |
| -and you can invite teammates from the account's page.</p> |
| +<p class="small">Names are deliberately not unique — two accounts may both be |
| +called “blog”. Identity is the short id in the URL, so there is nothing to |
| +squat. You will be its first owner, and you can invite teammates from the |
| +account's page.</p> |
| {{if .Account}}<p class="small"><a href="{{.Nav}}/">Back to {{.Account.Name}}</a></p>{{end}} |
| {{template "bottom" .}}{{end}} |