[UX-902] fix: create user client-side validation

[eblairmckee]

UX-902

Problem: Users got an obtuse server error when clicking "Create" with a username exceeding 128 characters — no client-side validation told them what was wrong, so they had to guess the limit by trial and error.

Fix: Added client-side validation with clear error messages for username/password length limits, derived directly from protobuf field constraints so they can't drift from the server.

Changes:

• proto-constraints.ts (new) — utility that extracts buf.validate annotations from proto schemas and builds Zod validators automatically
• user-create-form-schema.ts (new) — Zod schema using proto-derived constraints for username (max 128) and password (min 3, max 128) with real-time error messages
• user-create.tsx — migrated from legacy patterns to react-hook-form + Zod + Registry components; validation errors now show inline before submission
• user.ts — constants like USERNAME_MAX_LENGTH now derived from proto at runtime instead of hardcoded
• user-create.test.tsx (new) — 9 integration tests covering character limit validation, whitespace, duplicate users, and form submission
• proto-constraints.test.ts (new) — unit tests for the proto-to-Zod utility
• Minor fixes in copy-button.tsx, input.tsx, user-edit-modals.tsx, add-user-step.tsx

After

Before

[malinskibeniamin]

I think we should be using bcrypt or similar to generate the hash, crypto is getting better in native nodejs but nonetheless I would feel safer if we use bcrypt

[malinskibeniamin]

Is there any way we can get it working without casting such as as z.ZodString?

Ideally I would like to rely as much as possible on zod and protovaildate internals.

That being said, we need to discuss whether we go with:
a) merging the zod and protovalidate validation step and present the user with errors accordingly
b) 1st rely on the z.infer() handler where we get all the zod powered schema validation on every form change and then do a 2nd pass on protovalidate on submission? Ideally the 2 schemas should not be very far away from each other
c) have a dedicated package which would have the power to convert all protovalidate schemas to zod, but then we would lose on the protovalidate descriptive errors, so perhaps we may want to consolidate them. Just talking out loud

Personally I would be leaning towards the approach in Gateway where you have protovalidateResolver, but with the additional zod resolver.

[malinskibeniamin]

did you mean to interpolate the string there

[malinskibeniamin]

before we continue with this approach, let's consider creating protovalidate resolver like disccused in this thread https://github.com/redpanda-data/console/pull/2233/changes#r2831228170

[malinskibeniamin]

left more comments

[datamali]

This is a visual design comment and suggestion.
Inline error patterns: the input either turns red with a light red background, or the content stays dark, with the border and the error red.

[weeco]

@claude Review the PR

[claude]

Claude finished @weeco's task in 2m 56s —— View job

---

PR Review: UX-902 fix: create user client-side validation

Tasks

• Gather context & read changed files
• Review proto-constraints.ts utility
• Review user-create-form-schema.ts
• Review user-create.tsx migration
• Review user.ts changes
• Review tests
• Review minor fixes (copy-button, input, user-edit-modals, add-user-step)
• Post comprehensive review

---

The overall direction of this PR is solid — deriving validation constraints directly from proto annotations is the right approach to prevent client/server drift. Here are my findings:

---

Critical Issues

1. Unsafe as z.ZodString casts — runtime error risk

user-create-form-schema.ts:28,32:

username: (protoSchema.shape.name as z.ZodString)
  .regex(USERNAME_REGEX, USERNAME_ERROR_MESSAGE)
  ...
password: protoSchema.shape.password as z.ZodString,

If the proto field ever changes type (or protoToZodSchema returns something other than ZodString for these fields), the cast succeeds but the chained .regex() call will throw at runtime. This is exactly what @malinskibeniamin flagged. A safer pattern would be z.ZodString.safeParse to check the type, or using discriminated unions rather than casts. Fix this →

2. user-edit-modals.tsx uses stale hardcoded constraints inconsistent with this PR's goals

user-edit-modals.tsx:48:

const isValidPassword = password && password.length >= 4 && password.length <= 64;

The change password modal validates min=4, max=64, but the proto constraints are min=3, max=128. This modal was not migrated to use proto-derived constraints, so it's now more out-of-sync than before. This file was touched in this PR but only to import StateRoleSelector — the password constraints should be updated here too. Fix this →

3. Zod schema not memoized — recreated on every render

user-create.tsx:74-81:

const form = useForm<UserCreateFormValues>({
  resolver: zodResolver(createUserFormSchema(existingUsers)),
  ...
});

existingUsers changes identity on every re-render (from useMemo mapping, which still produces a new array when usersData refetches). This means a new Zod schema and resolver is created on each render cycle. The form resolver should be stable:

const schema = useMemo(() => createUserFormSchema(existingUsers), [existingUsers]);
const form = useForm({ resolver: zodResolver(schema), ... });

Fix this →

---

Medium Issues

4. Modulo bias in generatePassword

user.ts:60-65:

const randomValues = new Uint32Array(length);
crypto.getRandomValues(randomValues);
for (const n of randomValues) {
  const index = n % alphabet.length;

crypto.getRandomValues produces values in [0, 2^32). When that range isn't evenly divisible by alphabet.length, characters at lower indices are slightly more probable. For a security-sensitive function like password generation, rejection sampling should be used. The reviewer's suggestion of bcrypt is for hashing, not generation, so crypto.getRandomValues is the right API — just the modulo needs fixing.

5. Missing constraint mappings in proto-constraints.ts

• hostname is extracted (constraints.hostname = true at line 106) but stringConstraintsToZod has no Zod equivalent — it's silently ignored.
• const is extracted (line 92) but never applied in stringConstraintsToZod — silently ignored.

6. Magic numbers for proto scalar types

proto-constraints.ts:213-218:

case 9: // STRING
case 5: // INT32
case 13: // UINT32
case 17: // SINT32
case 8: // BOOL

These should reference the ScalarType enum from @bufbuild/protobuf for readability and resilience:

import { ScalarType } from '@bufbuild/protobuf';
case ScalarType.STRING:

7. Role assignment failures are silently ignored

user-create.tsx:109:

await Promise.allSettled(rolePromises);

Promise.allSettled never rejects, so if any role assignment fails, the user sees the success confirmation screen with no indication that roles weren't applied. At minimum, check the settled results and show a warning toast. Fix this →

8. Username uniqueness is case-sensitive

user-create-form-schema.ts:30:

.refine((val) => !existingUsers.includes(val), 'User already exists')

If the server treats usernames case-insensitively, a user could type "Admin" when "admin" exists and still get a server error. Consider !existingUsers.some((u) => u.toLowerCase() === val.toLowerCase()).

---

Minor Issues

9. Boolean(fieldState.invalid) is redundant

user-create.tsx:158,214:

{Boolean(fieldState.invalid) && <FieldError ... />}

fieldState.invalid is already a boolean. Just use {fieldState.invalid && ...}.

10. Inconsistent extractTypedConstraints casting

proto-constraints.ts:186:

return extractStringConstraints(rules.type.value as StringRules, required);

The rules.type.value as T pattern is repeated across all cases. These casts bypass type safety. Consider using a discriminated union narrowing approach or at minimum document why the runtime type is guaranteed.

11. Fallback with no diagnostic

user.ts:31-33:

export const USERNAME_MAX_LENGTH = nameConstraints?.maxLen ?? 128;

If getStringFieldConstraints returns null (e.g., field name changed in proto), the fallback silently uses 128. A console.warn or a more visible assertion would help catch proto schema drift.

---

Addressing Existing Review Comments

• @malinskibeniamin re: bcrypt (user.ts:70): crypto.getRandomValues is the correct API for random value generation on the frontend — bcrypt is for hashing. The actual issue is modulo bias (point 4 above), not the choice of API.

• @malinskibeniamin re: as z.ZodString and protovalidate resolver approach: The concern is valid. The current implementation is essentially a one-directional extraction (proto → Zod at build/import time). A protovalidateResolver that runs at submission time would give you proto validation errors with their original messages, as a second pass after Zod. The current approach works for the simple field-level constraints (min/max length), but the cast problem is real and would benefit from the instanceof guard.

• @malinskibeniamin re: string interpolation at line 156: In the current code, {USERNAME_MAX_LENGTH} at user-create.tsx:155-156 is correctly interpolated in JSX — this appears to have been addressed or was a confusion with an earlier revision.

• @datamali re: visual error pattern inconsistency: The input's error styling appears to come from the Field component's data-invalid attribute and CSS. This is a UI component library concern — the Input component uses aria-invalid for styling, but whether the container turns red vs. just the border depends on how Field/Input styles are applied. Worth aligning with the design system's defined error pattern in a follow-up.

---

Tests

The integration tests in user-create.test.tsx are well-structured and cover the key scenarios. The proto-constraints.test.ts unit tests directly validate the constraint extraction logic against the real proto schema, which is the right approach.

One gap: there's no test for the Promise.allSettled role assignment failure path. And the as any casts in the test mocks (user-create.test.tsx:88,93,98,102) reduce type safety — typed mock helpers would be better.

---

Summary: The approach is sound but I'd address the user-edit-modals.tsx constraint drift, the schema memoization, and the unsafe ZodString casts before merging. The broader architectural question about using a protovalidateResolver (as discussed in the PR thread) is worth deciding on — but doesn't block this PR if the current scope is intentionally limited to field-length validation.