Skip to content

Conversation

@Boy132
Copy link
Member

@Boy132 Boy132 commented Jan 29, 2026

Closes #2138

Before:
Backend silently removed . and - from username, made it lowercase and max length was 64.
Frontend did not check these restrictions.

Now:
Username is only made lowercase. (to allow case insensitive login) . and - are allowed. (will cause no problems with sftp)
Frontend and backend limits for max length match.

@Boy132 Boy132 self-assigned this Jan 29, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

📝 Walkthrough

Walkthrough

The changes modify username normalization behavior by removing the character sanitization chain that stripped special characters during user creation, while simplifying lowercase enforcement to occur uniformly at the model persistence layer for both username and email fields.

Changes

Cohort / File(s) Summary
Model Persistence Layer
app/Models/User.php
Modified the booted saving behavior to lowercase both username and email using Str pipeline during save, consolidating case normalization at persistence time instead of relying solely on the existing email-only lowercasing via mb_strtolower.
User Creation Service
app/Services/Users/UserCreationService.php
Removed the username normalization chain that previously sanitized usernames by replacing periods and hyphens, converting to ASCII, and truncating to 64 characters. The generated default username now bypasses these transformations.

Possibly related PRs

  • #1731: Modifies username normalization in app/Models/User.php by enforcing lowercase/trim on username fields
  • #1702: Removes the Username validation rule and related UI, complementing the shift away from character sanitization enforcement
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: sanitization logic is relocated to the model and constraints are relaxed.
Description check ✅ Passed The description clearly explains the before/after behavior and addresses the issue: less strict sanitization and frontend-backend alignment.
Linked Issues check ✅ Passed The code changes meet issue #2138 requirements by allowing dashes and preserving case via lowercase-only sanitization, directly addressing the reported problems.
Out of Scope Changes check ✅ Passed Both file changes directly address the linked issue: removing strict sanitization from the service and moving lowercase-only normalization to the model layer.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Boy132 Boy132 merged commit a477c89 into main Jan 31, 2026
32 checks passed
@Boy132 Boy132 deleted the boy132/fix-username-sanitization branch January 31, 2026 23:07
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New User Usernames are Unexpectedly Sanitized

3 participants