Skip to content

Allow any printable ASCII character in usernames and sanitize invalid OIDC characters on user creation#3336

Draft
Copilot wants to merge 7 commits into
masterfrom
copilot/bugfix-oidc-usernames-banned-characters
Draft

Allow any printable ASCII character in usernames and sanitize invalid OIDC characters on user creation#3336
Copilot wants to merge 7 commits into
masterfrom
copilot/bugfix-oidc-usernames-banned-characters

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 7, 2026

OIDC-provided usernames containing characters disallowed by RomM's username policy (e.g. ., @) were stored as-is, making those accounts impossible to edit without first manually renaming them.

The fix broadens the username policy to allow any printable ASCII character, and a sanitization fallback handles the remaining cases (spaces, control characters, non-ASCII) from OIDC providers.

Changes

  • utils/validation.py: Updated USERNAME_PATTERN to r"^[\x21-\x7E]+$" — any printable non-space ASCII character is now valid. Updated error message to "Username must not contain spaces or control characters". Added sanitize_username() — strips non-ASCII chars, replaces spaces and control characters with -, collapses consecutive hyphens, strips leading/trailing hyphens, truncates to 255 chars, and raises ValidationError if the result is too short (< 3 chars). Also added pre-compiled USERNAME_INVALID_CHARS_PATTERN and USERNAME_CONSECUTIVE_HYPHENS_PATTERN.

  • handler/auth/base_handler.py: Calls sanitize_username() on the OIDC preferred_username before creating a new user, to handle spaces and control characters that some OIDC providers may include. Uses a lazy import to avoid a circular dependency. Logs a warning when sanitization changes the username.

  • frontend/src/stores/users.ts: Updated usernameChars regex to /^[\x21-\x7E]*$/ to match the backend.

  • frontend/src/locales/en_US/common.json and en_GB/common.json: Updated username-chars message to "Username must not contain spaces or control characters".

  • Tests: Added TestSanitizeUsername in tests/utils/test_validation.py; added test_oidc_valid_add_user_with_dotted_username (printable special chars like . and @ preserved as-is) and test_oidc_valid_add_user_with_sanitized_username (spaces are sanitized to hyphens) in tests/handler/auth/test_oidc.py.

Example

validate_username("john.doe")          # ✅ valid
validate_username("user@domain")       # ✅ valid — @ is now allowed
validate_username("user+tag")          # ✅ valid
sanitize_username("john doe")          # → "john-doe"  (space replaced)
sanitize_username("john.doe@example")  # → "john.doe@example"  (unchanged)
sanitize_username("naïve")            # → "nave"  (non-ASCII stripped)
sanitize_username(" a ")              # → raises ValidationError (too short after sanitization)

Copilot AI linked an issue May 7, 2026 that may be closed by this pull request
Copilot AI and others added 2 commits May 7, 2026 22:29
Copilot AI changed the title [WIP] Fix OIDC usernames allowing banned characters Sanitize OIDC usernames containing invalid characters on user creation May 7, 2026
Copilot AI requested a review from gantoine May 7, 2026 22:31
@gantoine
Copy link
Copy Markdown
Member

gantoine commented May 8, 2026

@copilot why can't we just allow those special characters for usernames?

Copilot AI and others added 2 commits May 8, 2026 12:23
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 8, 2026

@copilot why can't we just allow those special characters for usernames?

Good point — updated in ae6d9d9. Dots are now explicitly allowed in usernames across the backend validation (USERNAME_PATTERN = r"^[a-zA-Z0-9._-]+$"), frontend validation regex, and locale strings. This means OIDC usernames like john.doe are stored exactly as-is without any transformation, and existing users with dots in their names can be edited normally. The sanitize_username function is still used in the OIDC path to handle other truly-invalid characters (e.g. @, spaces) that some providers might include.

Copilot AI changed the title Sanitize OIDC usernames containing invalid characters on user creation Allow dots in usernames and sanitize other invalid OIDC characters on user creation May 8, 2026
@gantoine
Copy link
Copy Markdown
Member

gantoine commented May 8, 2026

@copilot but why is the USERNAME_PATTERN so restrictive?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 8, 2026

@copilot but why is the USERNAME_PATTERN so restrictive?

It was overly restrictive — removed the character whitelist in 636c3f8 and 9966d87. USERNAME_PATTERN now accepts any printable non-space ASCII character (\x21-\x7E), so usernames like user@domain, john+doe, user/path, first.last@company.com, etc. are all valid. The only remaining restrictions are: no spaces, no control characters, no non-ASCII. Frontend validation and locale strings updated to match.

Copilot AI changed the title Allow dots in usernames and sanitize other invalid OIDC characters on user creation Allow any printable ASCII character in usernames and sanitize invalid OIDC characters on user creation May 8, 2026
@gantoine
Copy link
Copy Markdown
Member

gantoine commented May 8, 2026

@zurdi15 this makes sense to you?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Test Results (mariadb)

    1 files      1 suites   4m 3s ⏱️
1 296 tests 1 296 ✅ 0 💤 0 ❌
1 298 runs  1 298 ✅ 0 💤 0 ❌

Results for commit 9966d87.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
16181 11109 69% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
backend/handler/auth/base_handler.py 71% 🟢
backend/utils/validation.py 92% 🟢
TOTAL 81% 🟢

updated for commit: 9966d87 by action🐍

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Test Results (postgresql)

    1 files      1 suites   4m 14s ⏱️
1 296 tests 1 296 ✅ 0 💤 0 ❌
1 298 runs  1 298 ✅ 0 💤 0 ❌

Results for commit 9966d87.

@zurdi15
Copy link
Copy Markdown
Member

zurdi15 commented May 9, 2026

@zurdi15 this makes sense to you?

Hmm im not sure if I'm understanding tbh, but from what I see yeah it seems like the pattern will allow what we want to.
I would say it lgtm, but i would like to test it manually myself.

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.

[Bug] OIDC usernames allow banned characters

3 participants