Skip to content

Fix: OAuth signups crash when generated values exceed User column widths#274

Open
carlosplanchon wants to merge 1 commit into
benavlabs:mainfrom
carlosplanchon:fix/oauth-username-column-width
Open

Fix: OAuth signups crash when generated values exceed User column widths#274
carlosplanchon wants to merge 1 commit into
benavlabs:mainfrom
carlosplanchon:fix/oauth-username-column-width

Conversation

@carlosplanchon

Copy link
Copy Markdown
Collaborator

crudauth caps generated OAuth usernames at 32 chars but User.username is String(20), so providers whose sanitized username exceeds 20 chars (e.g. OIDC preferred_username shaped like user@org.domain) crash the callback with StringDataRightTruncationError. Also bounds the provider's unbounded display name to User.name's String(30). Found live with a self-hosted Zitadel instance; Google never trips it, which is why it went unnoticed. Includes a regression test that fails without the fix. Note for existing DBs: ALTER TABLE "user" ALTER COLUMN username TYPE VARCHAR(32).

crudauth's OAuth provisioning sanitizes the provider username and caps
it at USERNAME_MAX_LENGTH = 32, but User.username was String(20), so any
OAuth signup whose sanitized username exceeded 20 chars crashed the
callback with StringDataRightTruncationError. Found live with a
self-hosted OIDC provider: preferred_username values shaped like
user@org.domain sanitize to 32 chars. Google never trips it (no
preferred_username, falls back to the short given name), which is why it
went unnoticed. The column is now String(32), matching the generator's
cap.

Same class of bug one field over: NewUserContext.suggested_name returns
the provider's full display name unbounded while User.name is
String(30), so new_user_fields now truncates it to the column width.

Adds a regression test exercising a long provider username + display
name against the real DB (fails without the fix, with the exact
truncation error seen in the field).

Note: existing databases need
`ALTER TABLE "user" ALTER COLUMN username TYPE VARCHAR(32);`
since create_all only covers fresh databases.

Signed-off-by: Carlos Andrés Planchón Prestes <carlosandresplanchonprestes@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant