From 36eb5e72e9d984358c4b37e43efc54a5cbcfa4ce Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Mon, 21 Sep 2026 03:23:14 -0700 Subject: [PATCH 1/3] feat: stamp users.password_changed_at on every password write and refuse a password reset link issued at or before it, so a link requested earlier dies when the password is changed from settings, by another reset link or by warmblyctl, with the rule documented on the reset endpoint and the security guide --- .../docs/api/reference/account-org.mdx | 2 +- docs/content/docs/guides/security.mdx | 2 +- internal/app/auth/reset_password.go | 27 +++++++++++++ internal/app/auth/reset_password_test.go | 39 +++++++++++++++++++ .../000195_users_password_changed_at.down.sql | 1 + .../000195_users_password_changed_at.up.sql | 4 ++ internal/repository/pg_auth.go | 23 ++++++++++- 7 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 internal/app/auth/reset_password_test.go create mode 100644 internal/infrastructure/db/migrations/000195_users_password_changed_at.down.sql create mode 100644 internal/infrastructure/db/migrations/000195_users_password_changed_at.up.sql diff --git a/docs/content/docs/api/reference/account-org.mdx b/docs/content/docs/api/reference/account-org.mdx index 5a8c91979..3885e7510 100644 --- a/docs/content/docs/api/reference/account-org.mdx +++ b/docs/content/docs/api/reference/account-org.mdx @@ -191,7 +191,7 @@ Auth: public (not available to API keys). `POST /auth/reset-password/confirm` -Sets a new password using the emailed reset code (carried inside the signed `session` token). +Sets a new password using the emailed reset code (carried inside the signed `session` token). A reset token is single-use and bound to the password it was requested against: once the password is written by any path (this endpoint, the signed-in change, or an operator reset), every reset token issued before that write answers `401` however long its own expiry has left. Auth: public (not available to API keys). diff --git a/docs/content/docs/guides/security.mdx b/docs/content/docs/guides/security.mdx index f15bc0289..141572f81 100644 --- a/docs/content/docs/guides/security.mdx +++ b/docs/content/docs/guides/security.mdx @@ -15,7 +15,7 @@ Everything here lives under **Settings > Security** and concerns signing in to W A first sign-in with a new address via Google or Apple creates your account, workspace, and free trial automatically. -Accounts created through an external sign-in provider have no password initially. Trying password sign-in returns the same invalid-credentials response as an incorrect password. Continue with your provider, or use **Forgot password** to set a password before signing in with it. +Accounts created through an external sign-in provider have no password initially. Trying password sign-in returns the same invalid-credentials response as an incorrect password. Continue with your provider, or use **Forgot password** to set a password before signing in with it. A reset link works once and only for the password it was requested against: changing your password by any route, including using another reset link, makes every earlier link invalid before it expires. A passkey is tied to your device and unlocked with biometrics or a PIN, so it already proves both something you have and something you are or know. That is why it skips the emailed code. diff --git a/internal/app/auth/reset_password.go b/internal/app/auth/reset_password.go index f8b9da259..c37a309ee 100644 --- a/internal/app/auth/reset_password.go +++ b/internal/app/auth/reset_password.go @@ -5,6 +5,7 @@ import ( "errors" "time" + "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" "github.com/rs/zerolog/log" tokenpkg "github.com/warmbly/warmbly/internal/app/token" @@ -158,6 +159,18 @@ func (s *authService) ResetPasswordConfirm(ctx context.Context, data *ResetPassw return errx.ErrToken } + // A link is only good for the password it was requested against. Once the + // password has been written by any path (this flow, the signed-in change, + // the operator CLI), every link issued before that write is dead, however + // long its own expiry has left. + changedAt, err := s.authRepository.PasswordChangedAt(ctx, sess.UserID) + if err != nil { + return err + } + if resetLinkPredatesPassword(sess.IssuedAt, changedAt) { + return errx.ErrToken + } + if err := s.deletePasswordResetSession(ctx, sess.SessionID); err != nil { return err } @@ -194,6 +207,20 @@ func (s *authService) ResetPasswordConfirm(ctx context.Context, data *ResetPassw return nil } +// resetLinkPredatesPassword reports whether a reset token was issued no later +// than the last password write. JWT iat is whole seconds and the write is +// stamped by Postgres at microseconds, so a token minted in the same second as +// the change is refused too: fail closed, the person asks for a new link. +func resetLinkPredatesPassword(issuedAt *jwt.NumericDate, changedAt *time.Time) bool { + if changedAt == nil { + return false + } + if issuedAt == nil { + return true + } + return !issuedAt.Time.After(*changedAt) +} + // ChangePassword updates a logged-in user's password. It verifies the current // password first (so a hijacked but unattended session can't silently change // it), rejects OAuth-only accounts, and enforces the password policy. diff --git a/internal/app/auth/reset_password_test.go b/internal/app/auth/reset_password_test.go new file mode 100644 index 000000000..1908ab6a8 --- /dev/null +++ b/internal/app/auth/reset_password_test.go @@ -0,0 +1,39 @@ +package auth + +import ( + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" +) + +// A reset link is bound to the password it was requested against: once any +// path writes a new one, every link issued at or before that write is dead, +// whatever its own expiry says. +func TestResetLinkPredatesPassword(t *testing.T) { + changed := time.Date(2026, 9, 21, 12, 0, 0, 500_000_000, time.UTC) + at := func(t time.Time) *jwt.NumericDate { return jwt.NewNumericDate(t) } + + cases := []struct { + name string + issuedAt *jwt.NumericDate + changedAt *time.Time + stale bool + }{ + {"never changed", at(changed.Add(-time.Hour)), nil, false}, + {"issued before the change", at(changed.Add(-time.Hour)), &changed, true}, + {"issued after the change", at(changed.Add(time.Hour)), &changed, false}, + // iat is whole seconds, so a token minted in the same second as the + // change cannot prove it came after it. + {"issued in the same second", at(changed), &changed, true}, + {"issued the next second", at(changed.Add(time.Second)), &changed, false}, + {"no issue time at all", nil, &changed, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := resetLinkPredatesPassword(tc.issuedAt, tc.changedAt); got != tc.stale { + t.Fatalf("stale = %v, want %v", got, tc.stale) + } + }) + } +} diff --git a/internal/infrastructure/db/migrations/000195_users_password_changed_at.down.sql b/internal/infrastructure/db/migrations/000195_users_password_changed_at.down.sql new file mode 100644 index 000000000..e58b3acea --- /dev/null +++ b/internal/infrastructure/db/migrations/000195_users_password_changed_at.down.sql @@ -0,0 +1 @@ +ALTER TABLE public.users DROP COLUMN IF EXISTS password_changed_at; diff --git a/internal/infrastructure/db/migrations/000195_users_password_changed_at.up.sql b/internal/infrastructure/db/migrations/000195_users_password_changed_at.up.sql new file mode 100644 index 000000000..f3782ead0 --- /dev/null +++ b/internal/infrastructure/db/migrations/000195_users_password_changed_at.up.sql @@ -0,0 +1,4 @@ +-- Every password write stamps this, and a reset link issued before it is +-- refused. NULL is "never changed since this column existed", so links that +-- are outstanding at deploy time keep working. +ALTER TABLE public.users ADD COLUMN IF NOT EXISTS password_changed_at timestamptz; diff --git a/internal/repository/pg_auth.go b/internal/repository/pg_auth.go index 76fab6411..5f4ad6bc8 100644 --- a/internal/repository/pg_auth.go +++ b/internal/repository/pg_auth.go @@ -20,6 +20,9 @@ type AuthRepository interface { ExternalLogin(ctx context.Context, email string) (*models.User, *errx.Error) ResetPassword(ctx context.Context, userID uuid.UUID, password string) *errx.Error GetPasswordHash(ctx context.Context, userID uuid.UUID) (string, *errx.Error) + // PasswordChangedAt is when the password was last written, nil when it + // has not been since the column existed. + PasswordChangedAt(ctx context.Context, userID uuid.UUID) (*time.Time, *errx.Error) } type authRepository struct { @@ -142,10 +145,28 @@ func (r *authRepository) GetPasswordHash(ctx context.Context, userID uuid.UUID) return *hash, nil } +// PasswordChangedAt returns the last password write, which is the floor a +// reset link's issue time must clear. +func (r *authRepository) PasswordChangedAt(ctx context.Context, userID uuid.UUID) (*time.Time, *errx.Error) { + var at *time.Time + err := r.DB.QueryRow(ctx, `SELECT password_changed_at FROM users WHERE id = $1`, userID).Scan(&at) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, errx.ErrNotFound + } + db.CaptureError(err, "get password changed at", []any{userID}, "queryrow") + return nil, errx.InternalError() + } + return at, nil +} + +// ResetPassword is the one write of a password hash, so it is also the one +// place the change is stamped: every reset link issued before this instant is +// refused from here on, whichever path (reset, change, operator CLI) wrote it. func (r *authRepository) ResetPassword(ctx context.Context, userID uuid.UUID, passwordHash string) *errx.Error { query := ` UPDATE users - SET password_hash = $1, updated_at = now() + SET password_hash = $1, password_changed_at = now(), updated_at = now() WHERE id = $2 ` params := []any{ From 9426c0da512a702614ce9f5e927b28b031b96a30 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Mon, 21 Sep 2026 03:34:03 -0700 Subject: [PATCH 2/3] feat: validate every person, workspace and company name through internal/pkg/displayname on each write path (profile, onboarding, setup, IdP sign-in, org create and rename, org import, enterprise inquiry, admin testers, warmblyctl) with a 400 invalid_name code, render stored names in platform email through the same rules, mirror them in the web forms, check the org slug format, and document the rules in error-codes, security and AGENTS.md --- AGENTS.md | 10 + admin/src/app/dashboard/TestersPage.tsx | 1 + cmd/warmblyctl/user.go | 17 +- docs/content/docs/api/error-codes.mdx | 25 ++ .../docs/development/configuration.mdx | 2 +- docs/content/docs/development/first-run.mdx | 2 +- docs/content/docs/guides/security.mdx | 4 + .../docs/guides/workspace-export-import.mdx | 1 + docs/public/openapi.json | 14 +- go.mod | 2 +- internal/api/handler/admin_tester.go | 6 + internal/api/handler/onboarding.go | 36 ++- internal/api/handler/organization.go | 18 +- internal/api/handler/subscription.go | 23 +- internal/app/auth/external_idtoken.go | 9 +- internal/app/auth/provision.go | 8 +- internal/app/bootstrap/bootstrap.go | 32 ++- internal/app/dangerzone/service.go | 26 +- internal/app/organization/service.go | 18 +- internal/app/orgtransfer/import.go | 17 ++ internal/models/organization.go | 2 +- internal/pkg/displayname/displayname.go | 229 ++++++++++++++++++ internal/pkg/displayname/displayname_test.go | 74 ++++++ internal/repository/pg_user.go | 16 +- web/src/app/app/settings/profile/page.tsx | 36 ++- web/src/app/app/settings/workspace/page.tsx | 25 +- web/src/app/onboarding/page.tsx | 28 ++- web/src/app/setup/page.tsx | 13 +- .../app/billing/EnterpriseInquiryDialog.tsx | 32 ++- .../app/organizations/NewWorkspaceDialog.tsx | 16 +- web/src/components/ui/field.tsx | 10 + web/src/lib/displayName.test.ts | 40 +++ web/src/lib/displayName.ts | 47 ++++ 33 files changed, 718 insertions(+), 121 deletions(-) create mode 100644 internal/pkg/displayname/displayname.go create mode 100644 internal/pkg/displayname/displayname_test.go create mode 100644 web/src/lib/displayName.test.ts create mode 100644 web/src/lib/displayName.ts diff --git a/AGENTS.md b/AGENTS.md index affbade07..de7bbb966 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -230,6 +230,16 @@ Everything else in section 3 of the evidence pack rests on this: no identifier f - the realtime websocket checks the browser's `Origin` against `CHECK_ORIGIN_HOSTS`. Non-browser clients send no origin and are unaffected. Adding a first-party origin means adding it to that list in every environment - webhook targets stay HTTPS and HMAC-signed, and SSRF-prone destinations are refused. Only a self-hosted or development instance may opt out +### Input that other people see + +Anything one person types that Warmbly later shows to someone else is content injection waiting to happen, and platform email is the worst case: a mail client turns anything shaped like an address into a live link, sent under Warmbly's own domain. `html/template` escaping stops markup, not that. So: + +- **every name a person chooses goes through `internal/pkg/displayname`**: first and last names, workspace names, and any new name-like field that can reach another person. It refuses links, web addresses, email addresses, hostnames and IPs (after folding full-width and ideographic dots), control, invisible and bidi characters, markup characters and stacked combining marks, and it bounds length by `Kind`. The refusal is `400 invalid_name`, documented in `api/error-codes.mdx` +- **the server is the authority and the check sits at every write**, not only the one the dashboard uses: the handler or service behind registration, setup, onboarding, profile, org create and rename, the admin panel, `warmblyctl` and an org-transfer import. A new path that writes one of these fields calls the same package. `web/src/lib/displayName.ts` mirrors the rules so a form can explain a refusal before the request, and it is never the only check +- **a value nobody can be asked to correct is cleaned, not refused**: a name from an identity provider or an email local part goes through `displayname.Clean`/`FromEmail`, which drops what fails, so a hostile IdP claim costs the user a name, not a sign-in +- **a stored value is untrusted at render time too.** Rows written before a rule existed are still in the database, so anything interpolated into an email body or subject goes through `displayname.Displayable` (or `FullName`) with a neutral fallback ("A team member", "Your workspace") +- **tighten a rule in both places and in the docs together**: Go package, `displayName.ts`, their tests, and the `invalid_name` section of `api/error-codes.mdx` + ### Errors, logging and data exposure - a server-class (`Internal`) error answers the caller with one fixed sentence and a request id. The real message is logged against that id. `errx.NewPublic` is the narrow exception, for a message an operator can act on, and never for one built from an underlying error diff --git a/admin/src/app/dashboard/TestersPage.tsx b/admin/src/app/dashboard/TestersPage.tsx index df3632e97..0586b4ec8 100644 --- a/admin/src/app/dashboard/TestersPage.tsx +++ b/admin/src/app/dashboard/TestersPage.tsx @@ -208,6 +208,7 @@ export default function TestersPage() { setOrgName(e.target.value)} /> diff --git a/cmd/warmblyctl/user.go b/cmd/warmblyctl/user.go index 18b8737f8..430d498cf 100644 --- a/cmd/warmblyctl/user.go +++ b/cmd/warmblyctl/user.go @@ -17,6 +17,7 @@ import ( "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/pkg/argon2" "github.com/warmbly/warmbly/internal/pkg/crypt" + "github.com/warmbly/warmbly/internal/pkg/displayname" ) // resetSessionKeyPrefix must match getResetPasswordSessionKey in @@ -94,6 +95,9 @@ func runUserCreate(ctx context.Context, args []string) error { if *noOrg && strings.TrimSpace(*orgName) != "" { return errors.New("--org and --no-org contradict each other. Pass one or neither.") } + if _, nerr := displayname.Validate("--org", *orgName, displayname.Workspace, true); nerr != nil { + return errors.New(nerr.Message) + } c, err := connect(ctx) if err != nil { @@ -135,9 +139,9 @@ func runUserCreate(ctx context.Context, args []string) error { changed := []string{fmt.Sprintf("Created account %s (id %s)", created.Email, created.ID)} if !*noOrg { - name := strings.TrimSpace(*orgName) + name := displayname.Normalize(*orgName) if name == "" { - name = defaultOrgName(created.FirstName) + name = displayname.DefaultWorkspace(created.FirstName) } org, oerr := c.orgService().Create(ctx, created.ID, name) if oerr != nil { @@ -520,15 +524,6 @@ func lookupUser(ctx context.Context, c *conn, address string) (*models.User, err return u, nil } -// defaultOrgName mirrors the bootstrap owner's naming so an account created -// here is indistinguishable from one claimed through the setup link. -func defaultOrgName(firstName string) string { - if firstName == "" { - return "My Organization" - } - return firstName + "'s Organization" -} - func adminRoleNames() []string { return []string{ string(models.AdminRoleSuper), diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index d7c890749..7ce1edccb 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/docs/content/docs/api/error-codes.mdx @@ -93,6 +93,7 @@ Returned when the request cannot be processed due to invalid syntax. | `selection_too_large` | A `"all": true` bulk selection resolved to more than `50,000` rows. Narrow the filter and run it in parts; nothing was changed | | `invalid_filter` | A task filter carried an id that is not one: `assigned_to`, `contact_id` and `deal_id` name records, and are matched against id columns. Sent by `POST /crm/tasks/search`, `POST /crm/tasks/summary`, and the `filters` of a `"all": true` bulk selection | | `invalid_setting` | `PATCH /outreach/settings` (or a campaign's advanced settings) carried a value outside the documented vocabulary, for example a `reply_intent.crm_task_intents` entry that is not a reply intent | +| `invalid_slug` | `PATCH /organizations/current` was given a `slug` that is not 2 to 80 lowercase letters, numbers or dashes starting and ending with a letter or number | | `no_organization` | The request needs a workspace and the caller has none selected. Every entitlement, limit and suppression rule is scoped to a workspace, so a write that would run unscoped is refused rather than run without those checks. API keys always carry their workspace; a dashboard session picks one at sign-in, so this normally means the session predates the workspace being chosen. Select a workspace and retry | #### Password refusals @@ -112,6 +113,30 @@ A password must be 8 to 128 characters. There is no composition rule, but it is } ``` +#### Name refusals + +| `code` | Status | Meaning | +|--------|--------|---------| +| `invalid_name` | 400 | A first name, last name, workspace name or company name broke the naming rules. The `message` names the field and the rule | + +Names are shown to other people, including in invitation and notification emails, so they carry plain text only. The same rules apply to the dashboard, the API, the setup page and `warmblyctl`: + +- a first or last name is at most 50 characters and contains a letter; a workspace or company name is at most 64 characters and contains a letter or a number +- no links, web addresses or email addresses: nothing with `@`, `//`, `www.` or a scheme such as `https:`, no hostname such as `example.com`, and no IP address. Full-width and ideographic dots count as dots +- no control characters, invisible formatting characters (zero-width and bidirectional overrides), `<`, `>`, `` ` `` or `\`, and no more than three stacked combining marks +- leading and trailing spaces are trimmed and runs of spaces become one, and that is the form stored + +```json +{ + "error": "Bad Request", + "message": "First name cannot contain a link, web address or email address.", + "code": "invalid_name", + "request_id": "4bbbd1b2-8f86-47dd-8a7f-9476501ad20e" +} +``` + +A name from Google, Apple or a single sign-on provider that breaks these rules is dropped rather than refusing the sign-in, and you are asked for it during onboarding. + ### 401 Unauthorized Returned when authentication fails. diff --git a/docs/content/docs/development/configuration.mdx b/docs/content/docs/development/configuration.mdx index 16ca8d887..1c64a54bb 100644 --- a/docs/content/docs/development/configuration.mdx +++ b/docs/content/docs/development/configuration.mdx @@ -148,7 +148,7 @@ TRUSTED_PROXIES=10.0.0.0/8,172.16.0.0/12 | `WARMBLY_BOOTSTRAP_EMAIL` | First owner's address, read only while the users table is empty | unset | yes | | `WARMBLY_BOOTSTRAP_PASSWORD_HASH` | Argon2 PHC string for that owner. Preferred over the plaintext form | unset | yes | | `WARMBLY_BOOTSTRAP_PASSWORD` | Plaintext convenience form. Warns at boot, and leaves a password in your process environment | unset | yes | -| `WARMBLY_BOOTSTRAP_ORG` | Name of the organization created with that owner | derived from the name | yes | +| `WARMBLY_BOOTSTRAP_ORG` | Name of the organization created with that owner. A value that breaks the [naming rules](/api/error-codes/#name-refusals) is ignored with a warning | derived from the name | yes | | `TWOFA_SECRET` | Key that encrypts stored TOTP secrets. Falls back to `AUTH_SECRET`, so existing deployments keep working; rotating it invalidates every enrolled TOTP secret | `AUTH_SECRET` | yes | | `WEBAUTHN_RP_ID` | Passkey relying party id. Derived from `APP_URL` when unset. Changing it invalidates every enrolled passkey | derived | yes | | `WEBAUTHN_RP_ORIGINS` | Origins accepted for passkey ceremonies | derived from `APP_URL` | yes | diff --git a/docs/content/docs/development/first-run.mdx b/docs/content/docs/development/first-run.mdx index 7b1db1c75..3ff2cc673 100644 --- a/docs/content/docs/development/first-run.mdx +++ b/docs/content/docs/development/first-run.mdx @@ -128,7 +128,7 @@ All of these are read **only while the users table is empty**, so they are a no- | It gets | Detail | |---|---| | An owner account | Owner of a new organization, which is a membership status rather than a role | -| An organization | Named from `WARMBLY_BOOTSTRAP_ORG`, or derived from the owner's name | +| An organization | Named from `WARMBLY_BOOTSTRAP_ORG` when it is a valid workspace name, otherwise derived from the owner's name | | A trial | Irrelevant when `BILLING_PROVIDER=none`, which is the self-host default and unlocks everything. The dashboard header shows a `Self-hosted` badge instead of a plan or trial, and the billing and referral settings pages are hidden | | Every platform admin bit | So the same account signs in to the dashboard on `:5173` and the admin panel on `:5174` | diff --git a/docs/content/docs/guides/security.mdx b/docs/content/docs/guides/security.mdx index f15bc0289..4b20ad3b0 100644 --- a/docs/content/docs/guides/security.mdx +++ b/docs/content/docs/guides/security.mdx @@ -88,6 +88,10 @@ There is no rule about mixing upper case, digits and symbols. A long passphrase Repeated wrong passwords are counted per account, not just per device, so guessing one account from many addresses does not buy an attacker more attempts. After ten failures the account stops accepting password attempts for an hour. Signing in correctly clears the count. +## Names + +Your name and your workspace's name appear in emails Warmbly sends to other people, such as invitations. They are plain text only: a name that contains a link, a web address, an email address or invisible formatting characters is refused, and the field tells you why. The full rules are in [Name refusals](/api/error-codes/#name-refusals). + ## Password and alerts Change your password under **Password**: current password, then a new one. **Changing it signs out every other device automatically**, while the device you change it on stays in. Accounts that only use Google, Apple, or a passkey have no password to change. diff --git a/docs/content/docs/guides/workspace-export-import.mdx b/docs/content/docs/guides/workspace-export-import.mdx index cbe4224c1..a3c1cc6ac 100644 --- a/docs/content/docs/guides/workspace-export-import.mdx +++ b/docs/content/docs/guides/workspace-export-import.mdx @@ -99,6 +99,7 @@ Some things belong to an instance rather than to a workspace, so they are not ap | Scheduled deletions | A pending deletion from the source must never follow the workspace to its new home | | Failure and delivery counters | A webhook endpoint's failure streak and auto-disable state, and whether a notification's email already went out, describe what happened on the source. They start fresh, so an endpoint is not pre-disabled on the new instance and a notification is not re-sent | | Sends still in flight | A campaign step handed to a worker on the source has no worker on the destination to report back, so it arrives queued and is sent there instead of waiting forever. Steps already sent keep their history | +| An invalid workspace name | An archive's workspace name is applied only when it passes the same [naming rules](/api/error-codes/#name-refusals) as a rename. Otherwise the destination keeps its own | | Personal list layouts | Which columns each member shows on the contacts list and how they sort it belongs to the person, not the workspace. Everyone starts from the default view on the new instance and picks their columns again | An import runs as one transaction. If anything fails, nothing lands and the workspace is untouched. diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 7026eec32..92873cc22 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -20831,13 +20831,21 @@ }, "UpdateProfileRequest": { "type": "object", - "description": "Editable profile fields for the authenticated user.", + "description": "Editable profile fields for the authenticated user. Both names are plain text: no links, web addresses, email addresses or invisible formatting characters. A refused name answers 400 with code `invalid_name`.", + "required": [ + "first_name", + "last_name" + ], "properties": { "first_name": { - "type": "string" + "type": "string", + "minLength": 1, + "maxLength": 50 }, "last_name": { - "type": "string" + "type": "string", + "minLength": 1, + "maxLength": 50 } } }, diff --git a/go.mod b/go.mod index dccedd3b3..ee441e77d 100644 --- a/go.mod +++ b/go.mod @@ -58,6 +58,7 @@ require ( golang.org/x/net v0.59.0 golang.org/x/oauth2 v0.36.0 golang.org/x/term v0.46.0 + golang.org/x/text v0.42.0 google.golang.org/api v0.264.0 google.golang.org/grpc v1.83.2 google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.6.1 @@ -329,7 +330,6 @@ require ( golang.org/x/mod v0.41.0 // indirect golang.org/x/sync v0.23.0 // indirect golang.org/x/sys v0.48.0 // indirect - golang.org/x/text v0.42.0 // indirect golang.org/x/time v0.15.0 // indirect golang.org/x/tools v0.49.0 // indirect golang.org/x/tools/go/expect v0.1.1-deprecated // indirect diff --git a/internal/api/handler/admin_tester.go b/internal/api/handler/admin_tester.go index 4f9218b3c..f0bbc1c8b 100644 --- a/internal/api/handler/admin_tester.go +++ b/internal/api/handler/admin_tester.go @@ -12,6 +12,7 @@ import ( "github.com/warmbly/warmbly/internal/api/middleware" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/pkg/argon2" + "github.com/warmbly/warmbly/internal/pkg/displayname" ) // A tester account is one an operator hands to somebody outside the team: a @@ -96,6 +97,11 @@ func (h *Handler) AdminCreateTester(c *gin.Context) { return } + if _, nerr := displayname.Validate("Workspace name", req.OrgName, displayname.Workspace, true); nerr != nil { + errx.JSON(c, nerr) + return + } + if req.OrgID != nil && req.RoleID == nil { errx.JSON(c, errx.New(errx.BadRequest, "joining an existing workspace needs a role, so the access granted is one somebody chose")) return diff --git a/internal/api/handler/onboarding.go b/internal/api/handler/onboarding.go index ff3064080..bfcae38b3 100644 --- a/internal/api/handler/onboarding.go +++ b/internal/api/handler/onboarding.go @@ -9,6 +9,7 @@ import ( "github.com/warmbly/warmbly/internal/api/middleware" "github.com/warmbly/warmbly/internal/config" "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/pkg/displayname" ) type completeOnboardingRequest struct { @@ -53,15 +54,12 @@ func (h *Handler) CompleteOnboarding(c *gin.Context) { return } - if req.FirstName == "" || req.LastName == "" { - errx.Handle(c, errx.New(errx.BadRequest, "First name and last name are required.")) - return - } - - if len(req.FirstName) > 50 || len(req.LastName) > 50 { - errx.Handle(c, errx.New(errx.BadRequest, "Name must be 50 characters or less.")) + first, last, xerr := validatePersonName(req.FirstName, req.LastName) + if xerr != nil { + errx.Handle(c, xerr) return } + req.FirstName, req.LastName = first, last if !validReferralSources[req.ReferralSource] { errx.Handle(c, errx.New(errx.BadRequest, "Invalid referral source.")) @@ -108,14 +106,12 @@ func (h *Handler) UpdateUserProfile(c *gin.Context) { return } - if req.FirstName == "" || req.LastName == "" { - errx.Handle(c, errx.New(errx.BadRequest, "First name and last name are required.")) - return - } - if len(req.FirstName) > 50 || len(req.LastName) > 50 { - errx.Handle(c, errx.New(errx.BadRequest, "Name must be 50 characters or less.")) + first, last, xerr := validatePersonName(req.FirstName, req.LastName) + if xerr != nil { + errx.Handle(c, xerr) return } + req.FirstName, req.LastName = first, last userID := middleware.GetUserID(c) uid, err := uuid.Parse(userID) @@ -167,3 +163,17 @@ func (h *Handler) UpdateSendPreferences(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"undo_send_seconds": req.UndoSendSeconds}) } + +// validatePersonName applies the display-name rules to a first and last name, +// returning their stored forms. +func validatePersonName(first, last string) (string, string, *errx.Error) { + first, xerr := displayname.Validate("First name", first, displayname.Person, false) + if xerr != nil { + return "", "", xerr + } + last, xerr = displayname.Validate("Last name", last, displayname.Person, false) + if xerr != nil { + return "", "", xerr + } + return first, last, nil +} diff --git a/internal/api/handler/organization.go b/internal/api/handler/organization.go index 575d17f81..0eaeae62c 100644 --- a/internal/api/handler/organization.go +++ b/internal/api/handler/organization.go @@ -4,7 +4,7 @@ import ( "context" "fmt" "net/http" - "strings" + "time" "github.com/gin-gonic/gin" @@ -14,6 +14,7 @@ import ( "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/notify/templates" + "github.com/warmbly/warmbly/internal/pkg/displayname" ) // CreateOrganization creates a new organization @@ -209,17 +210,16 @@ func (h *Handler) InviteMember(c *gin.Context) { // Get organization name for email org, _ := h.OrganizationService.Get(c.Request.Context(), *orgID) orgName := "your organization" - if org != nil { - orgName = org.Name + if org != nil && displayname.Displayable(org.Name) != "" { + orgName = displayname.Displayable(org.Name) } - // Get inviter name + // Names predating the display-name rules fall back rather than render. inviter, _ := h.UserService.GetUser(c.Request.Context(), userID) inviterName := "A team member" - if inviter != nil && inviter.FirstName != "" { - inviterName = inviter.FirstName - if inviter.LastName != "" { - inviterName += " " + inviter.LastName + if inviter != nil { + if name := displayname.FullName(inviter.FirstName, inviter.LastName); name != "" { + inviterName = name } } @@ -456,7 +456,7 @@ func (h *Handler) AcceptInvitation(c *gin.Context) { // joiner). Detached + best-effort: a notification hiccup must not fail // the accept. if h.NotificationService != nil { - joiner := strings.TrimSpace(strings.TrimSpace(user.FirstName) + " " + strings.TrimSpace(user.LastName)) + joiner := displayname.FullName(user.FirstName, user.LastName) if joiner == "" { joiner = user.Email } diff --git a/internal/api/handler/subscription.go b/internal/api/handler/subscription.go index 23e17c395..2f7ffedaa 100644 --- a/internal/api/handler/subscription.go +++ b/internal/api/handler/subscription.go @@ -1,6 +1,7 @@ package handler import ( + "fmt" "io" "net/http" "strconv" @@ -10,6 +11,7 @@ import ( "github.com/warmbly/warmbly/internal/api/middleware" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/displayname" ) // GetSubscription returns the current organization's subscription @@ -346,6 +348,8 @@ type EnterpriseInquiryRequest struct { Notes string `json:"notes,omitempty"` } +const enterpriseNotesMaxLength = 2000 + // SubmitEnterpriseInquiry submits an enterprise pricing inquiry func (h *Handler) SubmitEnterpriseInquiry(c *gin.Context) { var req EnterpriseInquiryRequest @@ -354,9 +358,24 @@ func (h *Handler) SubmitEnterpriseInquiry(c *gin.Context) { return } + company, xerr := displayname.Validate("Company name", req.CompanyName, displayname.Workspace, false) + if xerr != nil { + errx.JSON(c, xerr) + return + } + contact, xerr := displayname.Validate("Contact name", req.ContactName, displayname.Person, false) + if xerr != nil { + errx.JSON(c, xerr) + return + } + if len([]rune(req.Notes)) > enterpriseNotesMaxLength { + errx.JSON(c, errx.New(errx.BadRequest, fmt.Sprintf("Notes must be %d characters or less.", enterpriseNotesMaxLength))) + return + } + inquiry := &models.EnterpriseInquiry{ - CompanyName: req.CompanyName, - ContactName: req.ContactName, + CompanyName: company, + ContactName: contact, ContactEmail: req.ContactEmail, EstimatedVolume: req.EstimatedVolume, TeamSize: req.TeamSize, diff --git a/internal/app/auth/external_idtoken.go b/internal/app/auth/external_idtoken.go index 9a1cfb445..86a13ef24 100644 --- a/internal/app/auth/external_idtoken.go +++ b/internal/app/auth/external_idtoken.go @@ -10,6 +10,7 @@ import ( "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/observability/errs" + "github.com/warmbly/warmbly/internal/pkg/displayname" "github.com/warmbly/warmbly/internal/pkg/idtoken" ) @@ -160,6 +161,9 @@ func (s *authService) createExternalUser(ctx context.Context, email *mail.Addres return nil, err } + // A provider-asserted name the rules refuse is dropped, never a failed sign-in. + firstName = displayname.Clean(firstName, displayname.Person) + lastName = displayname.Clean(lastName, displayname.Person) if firstName != "" { // Provider-asserted name beats CreateUser's email local-part default. if perr := s.userRepository.UpdateProfile(ctx, u.ID, firstName, lastName); perr == nil { @@ -180,10 +184,7 @@ func (s *authService) createExternalUser(ctx context.Context, email *mail.Addres var org *models.Organization if s.organizationService != nil { - orgName := u.FirstName + "'s Organization" - if u.FirstName == "" { - orgName = "My Organization" - } + orgName := displayname.DefaultWorkspace(u.FirstName) var orgErr *errx.Error org, orgErr = s.organizationService.Create(ctx, u.ID, orgName) if orgErr != nil { diff --git a/internal/app/auth/provision.go b/internal/app/auth/provision.go index af00a8319..3f5938431 100644 --- a/internal/app/auth/provision.go +++ b/internal/app/auth/provision.go @@ -14,6 +14,7 @@ import ( "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/observability/analytics" + "github.com/warmbly/warmbly/internal/pkg/displayname" "github.com/warmbly/warmbly/internal/pkg/signuprisk" ) @@ -101,10 +102,7 @@ func (s *authService) createAccount(ctx context.Context, address, passwordHash s // Auto-create organization for new user var org *models.Organization if s.organizationService != nil { - orgName := u.FirstName + "'s Organization" - if u.FirstName == "" { - orgName = "My Organization" - } + orgName := displayname.DefaultWorkspace(u.FirstName) var orgErr *errx.Error org, orgErr = s.organizationService.Create(ctx, u.ID, orgName) if orgErr != nil { @@ -238,7 +236,7 @@ func (s *authService) notifyOperatorSignup(u *models.User, workspace string) { "A new account finished signing up.", map[string]string{ "Email": u.Email, - "Name": strings.TrimSpace(u.FirstName + " " + u.LastName), + "Name": displayname.FullName(u.FirstName, u.LastName), "Workspace": workspace, }, ) diff --git a/internal/app/bootstrap/bootstrap.go b/internal/app/bootstrap/bootstrap.go index f3d07f9bc..7d09775ad 100644 --- a/internal/app/bootstrap/bootstrap.go +++ b/internal/app/bootstrap/bootstrap.go @@ -36,6 +36,7 @@ import ( "github.com/warmbly/warmbly/internal/observability/errs" "github.com/warmbly/warmbly/internal/pkg/argon2" "github.com/warmbly/warmbly/internal/pkg/crypt" + "github.com/warmbly/warmbly/internal/pkg/displayname" "github.com/warmbly/warmbly/internal/repository" ) @@ -124,10 +125,7 @@ func (s *Service) createOwner(ctx context.Context, address string) error { return fmt.Errorf("bootstrap: saving the owner: %w", err) } - orgName := strings.TrimSpace(os.Getenv("WARMBLY_BOOTSTRAP_ORG")) - if orgName == "" { - orgName = defaultOrgName(u.FirstName) - } + orgName := bootstrapOrgName(u.FirstName) org, orgErr := s.orgSvc.Create(ctx, u.ID, orgName) if orgErr != nil { return fmt.Errorf("bootstrap: creating the organization: %w", orgErr) @@ -264,6 +262,14 @@ func (s *Service) Claim(ctx context.Context, token, address, password, firstName if perr := crypt.PasswordError(password); perr != nil { return nil, perr } + firstName, nerr := displayname.Validate("Name", firstName, displayname.Person, true) + if nerr != nil { + return nil, nerr + } + lastName, nerr = displayname.Validate("Last name", lastName, displayname.Person, true) + if nerr != nil { + return nil, nerr + } // Refuse on an instance that already has accounts, even with a valid // token: a stale link out of an old log must never mint a second owner. @@ -313,10 +319,7 @@ func (s *Service) Claim(ctx context.Context, token, address, password, firstName return nil, err } - orgName := strings.TrimSpace(os.Getenv("WARMBLY_BOOTSTRAP_ORG")) - if orgName == "" { - orgName = defaultOrgName(u.FirstName) - } + orgName := bootstrapOrgName(u.FirstName) org, orgErr := s.orgSvc.Create(ctx, u.ID, orgName) if orgErr != nil { errs.CaptureException(orgErr) @@ -335,11 +338,16 @@ func (s *Service) Claim(ctx context.Context, token, address, password, firstName return u, nil } -func defaultOrgName(firstName string) string { - if firstName == "" { - return "My Organization" +// bootstrapOrgName is WARMBLY_BOOTSTRAP_ORG when it is a valid workspace name. +func bootstrapOrgName(firstName string) string { + env := os.Getenv("WARMBLY_BOOTSTRAP_ORG") + if name := displayname.Clean(env, displayname.Workspace); name != "" { + return name } - return firstName + "'s Organization" + if strings.TrimSpace(env) != "" { + log.Printf("Warning: WARMBLY_BOOTSTRAP_ORG is not a valid workspace name; using the default.") + } + return displayname.DefaultWorkspace(firstName) } func hashToken(token string) string { diff --git a/internal/app/dangerzone/service.go b/internal/app/dangerzone/service.go index aa214574f..0703d9226 100644 --- a/internal/app/dangerzone/service.go +++ b/internal/app/dangerzone/service.go @@ -25,6 +25,7 @@ import ( "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/notify" "github.com/warmbly/warmbly/internal/notify/templates" + "github.com/warmbly/warmbly/internal/pkg/displayname" "github.com/warmbly/warmbly/internal/repository" ) @@ -458,8 +459,8 @@ func (s *service) sendOrgScheduledEmail(ctx context.Context, org *models.Organiz if len(recipients) == 0 { return } - subject := fmt.Sprintf("%s scheduled for deletion", org.Name) - body, err := templates.GenerateOrgDeletionScheduledHTML(org.Name, d.ExecuteAfter, d.GraceDays, s.frontendBaseURL+orgDangerZonePath) + subject := fmt.Sprintf("%s scheduled for deletion", emailOrgName(org)) + body, err := templates.GenerateOrgDeletionScheduledHTML(emailOrgName(org), d.ExecuteAfter, d.GraceDays, s.frontendBaseURL+orgDangerZonePath) if err != nil { return } @@ -474,8 +475,8 @@ func (s *service) sendOrgCancelledEmail(ctx context.Context, org *models.Organiz if len(recipients) == 0 { return } - subject := fmt.Sprintf("Deletion cancelled for %s", org.Name) - body, err := templates.GenerateOrgDeletionCancelledHTML(org.Name, d.ExecuteAfter) + subject := fmt.Sprintf("Deletion cancelled for %s", emailOrgName(org)) + body, err := templates.GenerateOrgDeletionCancelledHTML(emailOrgName(org), d.ExecuteAfter) if err != nil { return } @@ -531,7 +532,7 @@ func (s *service) buildReminder(ctx context.Context, d *models.ScheduledDeletion return nil, "", "" } recipients = s.orgRecipients(ctx, org) - resourceName = org.Name + resourceName = emailOrgName(org) case models.DeletionResourceUser: user, _ := s.userRepo.GetUser(ctx, d.ResourceID) if user == nil { @@ -613,8 +614,7 @@ func nilIfEmpty(s string) *string { } func displayName(u *models.User) string { - name := strings.TrimSpace(strings.TrimSpace(u.FirstName) + " " + strings.TrimSpace(u.LastName)) - if name != "" { + if name := displayname.FullName(u.FirstName, u.LastName); name != "" { return name } return u.Email @@ -623,12 +623,20 @@ func displayName(u *models.User) string { // firstNameOrEmail is the friendly greeting name for deletion emails: // the user's first name when set, otherwise their email address. func firstNameOrEmail(u *models.User) string { - if strings.TrimSpace(u.FirstName) != "" { - return u.FirstName + if name := displayname.Displayable(u.FirstName); name != "" { + return name } return u.Email } +// emailOrgName is the workspace name as deletion emails show it. +func emailOrgName(org *models.Organization) string { + if name := displayname.Displayable(org.Name); name != "" { + return name + } + return "Your workspace" +} + // orgRecipients returns every member email for an org, owner first. // Owner is always included even if GetMembers somehow fails to load // them (defensive — the owner is the only person who can actually diff --git a/internal/app/organization/service.go b/internal/app/organization/service.go index fec0c58ca..568489e52 100644 --- a/internal/app/organization/service.go +++ b/internal/app/organization/service.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "encoding/hex" + "regexp" "strconv" "strings" "time" @@ -15,6 +16,7 @@ import ( "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/observability/errs" "github.com/warmbly/warmbly/internal/pkg/crypt" + "github.com/warmbly/warmbly/internal/pkg/displayname" "github.com/warmbly/warmbly/internal/repository" ) @@ -254,6 +256,11 @@ func NewService( // Create creates a new organization and adds the user as owner func (s *organizationService) Create(ctx context.Context, userID uuid.UUID, name string) (*models.Organization, *errx.Error) { + name, nerr := displayname.Validate("Workspace name", name, displayname.Workspace, false) + if nerr != nil { + return nil, nerr + } + // Ban-scope enforcement (migration 000045). Block new workspace // creation when the admin's set the BanScopeOrgCreate bit, even // if the user can otherwise log in. @@ -398,9 +405,16 @@ func (s *organizationService) Update(ctx context.Context, orgID uuid.UUID, req * } if req.Name != nil { - org.Name = *req.Name + name, nerr := displayname.Validate("Workspace name", *req.Name, displayname.Workspace, false) + if nerr != nil { + return nil, nerr + } + org.Name = name } if req.Slug != nil { + if !slugPattern.MatchString(*req.Slug) { + return nil, errx.NewWithIdentifier(errx.BadRequest, "invalid_slug", "Slug must be 2 to 80 lowercase letters, numbers or dashes, starting and ending with a letter or number.") + } // Validate slug uniqueness existing, _ := s.orgRepo.GetBySlug(ctx, *req.Slug) if existing != nil && existing.ID != orgID { @@ -1246,6 +1260,8 @@ func (s *organizationService) CreateEnterpriseInquiry(ctx context.Context, inqui // Helper functions +var slugPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,78}[a-z0-9]$`) + func generateSlug(name string) string { // Simple slug generation - lowercase, replace spaces with dashes slug := strings.ToLower(name) diff --git a/internal/app/orgtransfer/import.go b/internal/app/orgtransfer/import.go index 72c853055..191f6b3a1 100644 --- a/internal/app/orgtransfer/import.go +++ b/internal/app/orgtransfer/import.go @@ -16,6 +16,7 @@ import ( "github.com/warmbly/warmbly/internal/app/cipher" "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/displayname" "github.com/warmbly/warmbly/internal/repository" ) @@ -529,6 +530,7 @@ func (s *service) mergeOrganization(ctx context.Context, tx pgx.Tx, orgID uuid.U if len(m.Organization) == 0 { return nil } + cleanArchiveOrgName(m.Organization) destCols, err := s.repo.TableColumns(ctx, "organizations") if err != nil { return err @@ -575,6 +577,21 @@ func (s *service) mergeOrganization(ctx context.Context, tx pgx.Tx, orgID uuid.U return nil } +// cleanArchiveOrgName keeps the destination's name unless the archive's passes +// the same rules as a rename. +func cleanArchiveOrgName(org map[string]any) { + raw, ok := org["name"] + if !ok { + return + } + name, _ := raw.(string) + if clean := displayname.Clean(name, displayname.Workspace); clean != "" { + org["name"] = clean + return + } + delete(org, "name") +} + // orgMergeExcluded are the organization columns an archive may never set. var orgMergeExcluded = func() map[string]bool { out := map[string]bool{ diff --git a/internal/models/organization.go b/internal/models/organization.go index 22da0b08e..e8550107a 100644 --- a/internal/models/organization.go +++ b/internal/models/organization.go @@ -153,7 +153,7 @@ const ( // CreateOrganizationRequest represents the request to create a new organization type CreateOrganizationRequest struct { - Name string `json:"name" binding:"required,min=1,max=255"` + Name string `json:"name" binding:"required"` } // UpdateOrganizationRequest represents the request to update an organization diff --git a/internal/pkg/displayname/displayname.go b/internal/pkg/displayname/displayname.go new file mode 100644 index 000000000..6cacf61c8 --- /dev/null +++ b/internal/pkg/displayname/displayname.go @@ -0,0 +1,229 @@ +// Package displayname is the one set of rules for a name a person chooses for +// themselves or their workspace. Those names are shown to other people, +// including in platform email, where a client turns anything that looks like +// an address into a live link under Warmbly's sender. So a name carries text +// and nothing a mail client, browser or terminal would act on. +package displayname + +import ( + "fmt" + "regexp" + "strings" + "unicode" + "unicode/utf8" + + "github.com/warmbly/warmbly/internal/errx" + "golang.org/x/text/unicode/norm" +) + +// Kind selects the length bound and the minimum content of a name. +type Kind int + +const ( + // Person is a first or last name. + Person Kind = iota + // Workspace is an organization name. + Workspace +) + +const ( + PersonMaxLength = 50 + WorkspaceMaxLength = 64 + maxCombiningRun = 3 +) + +// Rejection says why a name was refused. +type Rejection int + +const ( + OK Rejection = iota + Empty + TooLong + Link + Characters + NoLetter +) + +// ErrorCode is the response `code` for every refused name. +const ErrorCode = "invalid_name" + +var ( + schemeRe = regexp.MustCompile(`(?i)(?:^|[^\p{L}\p{N}])(?:https?|ftps?|mailto|tel|sms|callto|skype|javascript|data|file|news|irc|xmpp|ssh|git|ws|wss)\s*:`) + // A label, a dot and a letter run: what linkifiers read as a hostname. + domainRe = regexp.MustCompile(`[\p{L}\p{N}-]\.(?:[\p{L}]{2,}|xn--)`) + ipv4Re = regexp.MustCompile(`\d{1,3}(?:\.\d{1,3}){3}`) +) + +// Normalize trims the name and collapses every run of spaces to one ASCII +// space. It is the stored form of a name that passes Check. +func Normalize(s string) string { + s = norm.NFC.String(strings.TrimSpace(s)) + var b strings.Builder + b.Grow(len(s)) + space := false + for _, r := range s { + if unicode.Is(unicode.Zs, r) || r == ' ' { + space = true + continue + } + if space && b.Len() > 0 { + b.WriteByte(' ') + } + space = false + b.WriteRune(r) + } + return b.String() +} + +// Check normalizes s and applies every rule for kind. +func Check(s string, kind Kind) (string, Rejection) { + s = Normalize(s) + if s == "" { + return "", Empty + } + if utf8.RuneCountInString(s) > maxLength(kind) { + return s, TooLong + } + if r := content(s); r != OK { + return s, r + } + if !hasLetter(s, kind) { + return s, NoLetter + } + return s, OK +} + +// Clean returns the normalized name when it passes Check, otherwise "". Use it +// for names that arrive from somewhere nobody can be asked to correct, such as +// an identity provider or an email address. +func Clean(s string, kind Kind) string { + if out, r := Check(s, kind); r == OK { + return out + } + return "" +} + +// Displayable returns the normalized name when it is safe to show another +// person, otherwise "". It skips the length bound so a stored name that +// predates it still renders; the caller supplies the fallback. +func Displayable(s string) string { + s = Normalize(s) + if s == "" || content(s) != OK { + return "" + } + return s +} + +// FromEmail derives a first name from an address's local part, with the +// separators people use between names turned into spaces. +func FromEmail(address string) string { + local, _, ok := strings.Cut(address, "@") + if !ok { + return "" + } + local, _, _ = strings.Cut(local, "+") + local = strings.NewReplacer(".", " ", "_", " ", "-", " ").Replace(local) + if n := []rune(Normalize(local)); len(n) > PersonMaxLength { + local = string(n[:PersonMaxLength]) + } + return Clean(local, Person) +} + +// Validate is Check answered as an API error naming the field. An empty value +// passes when optional is true. +func Validate(label, s string, kind Kind, optional bool) (string, *errx.Error) { + out, r := Check(s, kind) + if r == Empty && optional { + return "", nil + } + if r == OK { + return out, nil + } + return "", Error(label, kind, r) +} + +// Error is the API error for a refused name. +func Error(label string, kind Kind, r Rejection) *errx.Error { + var msg string + switch r { + case Empty: + msg = label + " is required." + case TooLong: + msg = fmt.Sprintf("%s must be %d characters or less.", label, maxLength(kind)) + case Link: + msg = label + " cannot contain a link, web address or email address." + case NoLetter: + if kind == Workspace { + msg = label + " must contain a letter or a number." + } else { + msg = label + " must contain a letter." + } + default: + msg = label + " contains characters that are not allowed." + } + return errx.NewWithIdentifier(errx.BadRequest, ErrorCode, msg) +} + +func maxLength(kind Kind) int { + if kind == Workspace { + return WorkspaceMaxLength + } + return PersonMaxLength +} + +func content(s string) Rejection { + combining := 0 + for _, r := range s { + switch { + case r == utf8.RuneError, + unicode.IsControl(r), + unicode.In(r, unicode.Cf, unicode.Co, unicode.Cs, unicode.Zl, unicode.Zp), + strings.ContainsRune("<>`\\", r): + return Characters + case unicode.Is(unicode.Mn, r) || unicode.Is(unicode.Me, r): + combining++ + if combining > maxCombiningRun { + return Characters + } + default: + combining = 0 + } + } + if looksLikeLink(s) { + return Link + } + return OK +} + +// looksLikeLink reads the name the way a linkifier would, after folding the +// full-width and ideographic forms that resolve to the same address. +func looksLikeLink(s string) bool { + f := strings.ToLower(norm.NFKC.String(s)) + f = strings.NewReplacer("\u3002", ".", "\uff61", ".").Replace(f) + if strings.Contains(f, "@") || strings.Contains(f, "//") || strings.Contains(f, "www.") { + return true + } + return schemeRe.MatchString(f) || domainRe.MatchString(f) || ipv4Re.MatchString(f) +} + +func hasLetter(s string, kind Kind) bool { + for _, r := range s { + if unicode.IsLetter(r) || (kind == Workspace && unicode.IsDigit(r)) { + return true + } + } + return false +} + +// DefaultWorkspace names the workspace created for a new account. +func DefaultWorkspace(firstName string) string { + if name := Clean(firstName+"'s Organization", Workspace); name != "" { + return name + } + return "My Organization" +} + +// FullName joins the displayable parts of a person's name, "" when neither is. +func FullName(first, last string) string { + return strings.TrimSpace(Displayable(first) + " " + Displayable(last)) +} diff --git a/internal/pkg/displayname/displayname_test.go b/internal/pkg/displayname/displayname_test.go new file mode 100644 index 000000000..8d38fefc5 --- /dev/null +++ b/internal/pkg/displayname/displayname_test.go @@ -0,0 +1,74 @@ +package displayname + +import "testing" + +func TestCheck(t *testing.T) { + cases := []struct { + in string + kind Kind + want Rejection + }{ + {"Ada", Person, OK}, + {" Mary Ann ", Person, OK}, + {"O'Brien-Smith", Person, OK}, + {"J.R.R. Tolkien", Person, OK}, + {"St. John", Person, OK}, + {"Zo\u00eb", Person, OK}, + {"\u674e\u5c0f\u9f99", Person, OK}, + {"Acme 2.0", Workspace, OK}, + {"3M", Workspace, OK}, + {"", Person, Empty}, + {" ", Person, Empty}, + {"https://www.google.com", Person, Link}, + {"www.example", Person, Link}, + {"google.com", Person, Link}, + {"Visit evil.co now", Workspace, Link}, + {"\uff47\uff4f\uff4f\uff47\uff4c\uff45\uff0e\uff43\uff4f\uff4d", Person, Link}, + {"google\u3002com", Person, Link}, + {"a@b", Person, Link}, + {"javascript:alert(1)", Person, Link}, + {"mailto: x", Person, Link}, + {"//evil", Person, Link}, + {"10.0.0.1", Workspace, Link}, + {"xn--80ak6aa92e.xn--p1ai", Workspace, Link}, + {"Ada\u202eevil", Person, Characters}, + {"Ada\u200b", Person, Characters}, + {"Ada\nLovelace", Person, Characters}, + {"Ada", Person, Characters}, + {"Z\u0301\u0301\u0301\u0301\u0301", Person, Characters}, + {"1234", Person, NoLetter}, + {"!!!", Workspace, NoLetter}, + {"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Person, TooLong}, + } + for _, c := range cases { + if _, got := Check(c.in, c.kind); got != c.want { + t.Errorf("Check(%q) = %d, want %d", c.in, got, c.want) + } + } +} + +func TestFromEmail(t *testing.T) { + cases := map[string]string{ + "john.smith@example.com": "john smith", + "ada+tag@example.com": "ada", + "google.com@example.com": "google com", + "www.evil.com@x.io": "www evil com", + "12345@example.com": "", + "no-at-sign": "", + } + for in, want := range cases { + if got := FromEmail(in); got != want { + t.Errorf("FromEmail(%q) = %q, want %q", in, got, want) + } + } +} + +func TestDisplayable(t *testing.T) { + if got := Displayable("https://evil.example"); got != "" { + t.Errorf("Displayable kept a link: %q", got) + } + long := "A very long workspace name that predates the sixty four character bound" + if got := Displayable(long); got != long { + t.Errorf("Displayable dropped a long legacy name: %q", got) + } +} diff --git a/internal/repository/pg_user.go b/internal/repository/pg_user.go index 752c1890b..8505c4029 100644 --- a/internal/repository/pg_user.go +++ b/internal/repository/pg_user.go @@ -15,6 +15,7 @@ import ( "github.com/warmbly/warmbly/internal/infrastructure/db" "github.com/warmbly/warmbly/internal/infrastructure/kms" "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/displayname" ) type UserRepository interface { @@ -93,14 +94,8 @@ func (r *userRepository) CreateUser(ctx context.Context, email *mail.Address, pa // case someone typed is a row no sign-in and no reset can find. address := normalizeUserEmail(email.Address) - var firstName string - - nameSplit := strings.SplitN(address, "@", 2) - if len(nameSplit) < 2 { - firstName = "Unknown" - } else { - firstName = nameSplit[0] - } + // Only a local part that passes the display-name rules becomes a name. + firstName := displayname.FromEmail(address) var lastName string now := time.Now() @@ -369,10 +364,7 @@ func (r *userRepository) CreateExemptUser(ctx context.Context, email *mail.Addre id := uuid.New() address := normalizeUserEmail(email.Address) - firstName := "Unknown" - if parts := strings.SplitN(address, "@", 2); len(parts) == 2 { - firstName = parts[0] - } + firstName := displayname.FromEmail(address) now := time.Now() if _, ierr := tx.Exec(ctx, ` INSERT INTO users (id, email, password_hash, first_name, last_name, created_at, updated_at) diff --git a/web/src/app/app/settings/profile/page.tsx b/web/src/app/app/settings/profile/page.tsx index 779a9fb5a..0f69d7346 100644 --- a/web/src/app/app/settings/profile/page.tsx +++ b/web/src/app/app/settings/profile/page.tsx @@ -1,6 +1,6 @@ import React from "react"; import { useUserProfile } from "@/hooks/context/user"; -import { NumberInput, TextInput } from "@/components/ui/field"; +import { FieldError, NumberInput, TextInput } from "@/components/ui/field"; import useUpdateProfile from "@/lib/api/hooks/auth/useUpdateProfile"; import useUpdateSendPreferences from "@/lib/api/hooks/auth/useUpdateSendPreferences"; import { AvatarUploader } from "@/components/app/avatar/AvatarUploader"; @@ -12,6 +12,7 @@ import { Row, Section, SectionShell, initials } from "../_components/SectionShel import SaveStatus from "../_components/SaveStatus"; import { useAutosave, type AutosaveStatus } from "@/hooks/useAutosave"; import { useRegisterUnsaved } from "@/hooks/context/unsaved"; +import { PERSON_NAME_MAX, nameError, normalizeName } from "@/lib/displayName"; // Header indicator priority when two autosaves share one SaveStatus. function combineStatus(a: AutosaveStatus, b: AutosaveStatus): AutosaveStatus { @@ -30,18 +31,21 @@ export default function ProfileSettingsPage() { const removeAvatar = useDeleteUserAvatar(); const updateProfile = useUpdateProfile(); - // Auto-save names ~700ms after the user stops typing. Empty names are not - // persisted (the server requires both), so the field just stays unsaved. + // Auto-save names ~700ms after the user stops typing. A name the server + // would refuse is not sent; the field shows why and stays unsaved. // Memoized so the debounce timer only resets on an actual name change. const value = React.useMemo( - () => ({ firstName: firstName.trim(), lastName: lastName.trim() }), + () => ({ firstName: normalizeName(firstName), lastName: normalizeName(lastName) }), [firstName, lastName], ); + const firstError = nameError("First name", firstName, "person"); + const lastError = nameError("Last name", lastName, "person"); const autosave = useAutosave({ value, debounceMs: 700, save: async (v) => { - if (!v.firstName || !v.lastName) throw new Error("name required"); + const invalid = nameError("First name", v.firstName, "person") ?? nameError("Last name", v.lastName, "person"); + if (invalid) throw new Error(invalid); await updateProfile.mutateAsync({ first_name: v.firstName, last_name: v.lastName }); }, }); @@ -87,11 +91,29 @@ export default function ProfileSettingsPage() {
First name - + +
Last name - + +
{ - if (!v) throw new Error("name required"); + const invalid = nameError("Workspace name", v, "workspace"); + if (invalid) throw new Error(invalid); await saveToThisWorkspace({ name: v }); }, }); @@ -158,7 +161,17 @@ function WorkspaceSettings({ org: currentOrg }: { org: StoreOrganization | null /> - +
+ + +
+ z.string().superRefine((value, ctx) => { + const message = nameError(label, value, kind); + if (message) ctx.addIssue({ code: "custom", message }); + }); + const schema = z.object({ - first_name: z.string().min(1, "First name is required").max(50, "50 characters max"), - last_name: z.string().min(1, "Last name is required").max(50, "50 characters max"), - workspace: z.string().min(1, "Workspace name is required").max(60, "60 characters max"), + first_name: displayName("First name", "person"), + last_name: displayName("Last name", "person"), + workspace: displayName("Workspace name", "workspace"), role: z.enum(["founder", "sales", "marketing", "agency", "recruiter", "other"], { error: "Pick the closest one", }), @@ -186,16 +193,17 @@ export default function OnboardingPage() { try { // Rename the auto-created workspace if the user changed it. Best // effort: a rename hiccup must never block completing onboarding. - if (org?.name && org.name !== data.workspace) { + const workspace = normalizeName(data.workspace); + if (org?.name && org.name !== workspace) { try { - await updateOrganization.mutateAsync({ name: data.workspace }); + await updateOrganization.mutateAsync({ name: workspace }); } catch { /* keep going — onboarding completion matters more */ } } await completeOnboarding.mutateAsync({ - first_name: data.first_name, - last_name: data.last_name, + first_name: normalizeName(data.first_name), + last_name: normalizeName(data.last_name), referral_source: data.referral_source, role: data.role, team_size: data.team_size, @@ -273,12 +281,12 @@ export default function OnboardingPage() {
First name - +
Last name - +
@@ -287,7 +295,7 @@ export default function OnboardingPage() { {step === 1 && (
Workspace name - +
)} diff --git a/web/src/app/setup/page.tsx b/web/src/app/setup/page.tsx index 9106a723b..caf3862e1 100644 --- a/web/src/app/setup/page.tsx +++ b/web/src/app/setup/page.tsx @@ -30,7 +30,8 @@ import getUser from "@/lib/api/client/auth/getUser"; import useAuthConfig from "@/lib/api/hooks/auth/useAuthConfig"; import { usePasswordStrength } from "@/hooks/usePasswordStrength"; import { saveTokens } from "@/lib/auth"; -import { Label, TextInput } from "@/components/ui/field"; +import { FieldError, Label, TextInput } from "@/components/ui/field"; +import { PERSON_NAME_MAX, nameError, normalizeName } from "@/lib/displayName"; import type { AppError } from "@/lib/api/client/normalizeError"; import type AuthConfig from "@/lib/api/models/auth/AuthConfig"; @@ -81,6 +82,11 @@ export default function SetupPage() { toast.error("Enter the email address for the owner account."); return; } + const invalidName = nameError("Name", firstName, "person", true); + if (invalidName) { + toast.error(invalidName); + return; + } if (password.length < 8) { toast.error("Password must be at least 8 characters long."); return; @@ -102,7 +108,7 @@ export default function SetupPage() { token, email, password, - first_name: firstName || undefined, + first_name: normalizeName(firstName) || undefined, }); saveTokens(session as unknown as Record); // Prime the profile with the new token before entering the gated @@ -198,8 +204,11 @@ export default function SetupPage() { onChange={setFirstName} autoComplete="given-name" placeholder="Alex" + invalid={!!nameError("Name", firstName, "person", true)} + maxLength={PERSON_NAME_MAX} className={FIELD} /> +
diff --git a/web/src/components/app/billing/EnterpriseInquiryDialog.tsx b/web/src/components/app/billing/EnterpriseInquiryDialog.tsx index 6fa9d1df5..0c2aaf822 100644 --- a/web/src/components/app/billing/EnterpriseInquiryDialog.tsx +++ b/web/src/components/app/billing/EnterpriseInquiryDialog.tsx @@ -12,7 +12,10 @@ import { useAppStore } from "@/stores"; import useEnterpriseInquiry from "@/lib/api/hooks/app/subscription/useEnterpriseInquiry"; import type { AppError } from "@/lib/api/client/normalizeError"; import buildError from "@/lib/helper/buildError"; -import { Label, NumberInput, TextInput } from "@/components/ui/field"; +import { FieldError, Label, NumberInput, TextInput } from "@/components/ui/field"; +import { PERSON_NAME_MAX, WORKSPACE_NAME_MAX, nameError, normalizeName } from "@/lib/displayName"; + +const NOTES_MAX = 2000; export default function EnterpriseInquiryDialog({ open, @@ -85,14 +88,16 @@ export default function EnterpriseInquiryDialog({ }; }, [open, inquiry.isPending, onClose]); - const valid = company.trim() && name.trim() && /.+@.+\..+/.test(email.trim()); + const companyError = company.trim() ? nameError("Company name", company, "workspace") : null; + const nameInvalid = name.trim() ? nameError("Contact name", name, "person") : null; + const valid = company.trim() && name.trim() && !companyError && !nameInvalid && /.+@.+\..+/.test(email.trim()); async function submit() { if (!valid || inquiry.isPending) return; try { const res = await inquiry.mutateAsync({ - company_name: company.trim(), - contact_name: name.trim(), + company_name: normalizeName(company), + contact_name: normalizeName(name), contact_email: email.trim(), estimated_volume: Number.isFinite(volume) ? volume : undefined, team_size: Number.isFinite(teamSize) ? teamSize : undefined, @@ -159,12 +164,26 @@ export default function EnterpriseInquiryDialog({

- + +
- + +
@@ -191,6 +210,7 @@ export default function EnterpriseInquiryDialog({