Validate and uniqueness-check username on user update (fixes #13) #17

wtclaude · 2026-07-03T02:38:48Z

wtclaude commented

2026-07-03 02:38:48 +00:00

Summary

Fixes #13. PUT /admin/users/:id validated password and role but not username, even though updateUser writes req.body.username to the DB. Two consequences:

a blank or too-short username could be saved;
a duplicate username was not pre-checked (unlike createUser, which does), so it hit the DB unique constraint and surfaced as an opaque 500 Internal Server Error instead of a meaningful 409.

What changed

server/src/router/v1/admin/admin.routes.js — added the same validator used on create, made optional so an update that omits username still works:
```
body('username').optional().isString().trim().isLength({ min: 3, max: 32 })
```
The .trim() sanitizer also mutates req.body.username, so whitespace-only input collapses and then fails the min: 3 check (400), and the value the controller writes is already trimmed — consistent with create.

server/src/router/v1/admin/admin.controller.js — in updateUser, when the username is actually changing, pre-check for an existing user with that name and return 409 Username already taken:

if (req.body.username && req.body.username !== target.username) {
  const clash = await users.getRawByUsername(req.body.username)
  if (clash) return res.status(409).json({ message: 'Username already taken' })
}

Why this approach

Mirror createUser — the create path already validates the username and pre-checks uniqueness with getRawByUsername, returning 409. Reusing the exact same validator and helper keeps the two paths consistent and avoids inventing new behavior.
.optional() — unlike create, an update may legitimately change only the password or role. Making the validator optional preserves that; the field is only validated when present.
Guard req.body.username !== target.username — this skips the uniqueness query when the name is unchanged, and guarantees any row returned by getRawByUsername belongs to a different user, so we never 409 a user against itself.
Explicit 409 over relying on the DB — turning the unique-constraint violation into an intentional 409 gives the client an actionable, non-500 response and matches how create already reports the same conflict.

Testing

node -c syntax check on both changed files.
Traced the cases: omitted username -> unchanged; blank/short -> 400; duplicate -> 409; same-name no-op -> allowed; valid rename -> 200.

Notes

Backend only, no schema or client changes. Branched from current main (which already includes the #10 and #12 fixes).

## Summary Fixes #13. `PUT /admin/users/:id` validated `password` and `role` but **not** `username`, even though `updateUser` writes `req.body.username` to the DB. Two consequences: - a blank or too-short username could be saved; - a duplicate username was not pre-checked (unlike `createUser`, which does), so it hit the DB unique constraint and surfaced as an opaque `500 Internal Server Error` instead of a meaningful `409`. ## What changed - **`server/src/router/v1/admin/admin.routes.js`** — added the same validator used on create, made optional so an update that omits `username` still works: ```js body('username').optional().isString().trim().isLength({ min: 3, max: 32 }) ``` The `.trim()` sanitizer also mutates `req.body.username`, so whitespace-only input collapses and then fails the `min: 3` check (400), and the value the controller writes is already trimmed — consistent with create. - **`server/src/router/v1/admin/admin.controller.js`** — in `updateUser`, when the username is actually changing, pre-check for an existing user with that name and return `409 Username already taken`: ```js if (req.body.username && req.body.username !== target.username) { const clash = await users.getRawByUsername(req.body.username) if (clash) return res.status(409).json({ message: 'Username already taken' }) } ``` ## Why this approach - **Mirror `createUser`** — the create path already validates the username and pre-checks uniqueness with `getRawByUsername`, returning `409`. Reusing the exact same validator and helper keeps the two paths consistent and avoids inventing new behavior. - **`.optional()`** — unlike create, an update may legitimately change only the password or role. Making the validator optional preserves that; the field is only validated when present. - **Guard `req.body.username !== target.username`** — this skips the uniqueness query when the name is unchanged, and guarantees any row returned by `getRawByUsername` belongs to a *different* user, so we never 409 a user against itself. - **Explicit 409 over relying on the DB** — turning the unique-constraint violation into an intentional `409` gives the client an actionable, non-500 response and matches how create already reports the same conflict. ## Testing - `node -c` syntax check on both changed files. - Traced the cases: omitted username -> unchanged; blank/short -> 400; duplicate -> 409; same-name no-op -> allowed; valid rename -> 200. ## Notes Backend only, no schema or client changes. Branched from current `main` (which already includes the #10 and #12 fixes).

wtclaude added 1 commit 2026-07-03 02:38:51 +00:00

Validate and uniqueness-check username on user update (#13 ) 43db509293

PUT /admin/users/:id validated password and role but not username, even
though updateUser writes req.body.username. A blank/too-short username
could be saved, and a duplicate hit the DB unique constraint and
surfaced as an opaque 500.

- Route: add the same validator used on create,
  body('username').optional().isString().trim().isLength({min:3,max:32}).
  The trim sanitizer also collapses whitespace-only input so it fails
  the min-length check.
- Controller: when the username is changing, pre-check for another user
  with that name and return 409 instead of letting the DB throw a 500.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

whitlocktech approved these changes 2026-07-03 02:39:38 +00:00

whitlocktech merged commit d89cc7e691 into main

2026-07-03 02:39:44 +00:00

whitlocktech referenced this issue from a commit

2026-07-03 02:39:46 +00:00

Merge pull request 'Validate and uniqueness-check username on user update (fixes #13)' (#17) from fix/username-validation-update into main

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/website#17