fix(#48): username confusable protection#50
Open
DSanich wants to merge 2 commits into
Open
Conversation
Normalize usernames with NFKC before validation, add namespace-guard skeleton collision checks on public check/reserve/claim and admin assign/reserve paths (including bulk).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
namespace-guard) on registration-related flows.validateUsernameso equivalent Unicode representations collapse before other rules run.getPotentialConfusableUsernamesand utility modulesrc/utils/username-confusable.ts(including sharedgetConfusableCollision).GET /api/username/check/:name,POST /api/username/reserve,POST /api/username/claim— reject with clear messaging; check returnscode: "confusable"when applicable.POST .../reserve,reserve-bulk,assign,assign-bulk— same rules; bulk failures for confusables use a consistentstatus: "confusable"(aligned between reserve-bulk and assign-bulk).username-confusable, and NFKC invalidation; separate commit adjustsnip05-statusexpectations for revoked users when the Fastly key is absent.Motivation
Related Issue
Validation
npm run test:oncenpm run build:adminManual Test Plan / Notes
GET /api/username/check/...for a string that looks like an existing name but uses a different script — expectavailable: false,code: "confusable", HTTP 200.POST /api/username/reserveandPOST /api/username/claimwith such a name — HTTP 409 and an error mentioning visually confusable.reserve-bulk/assign-bulk, failed rows havesuccess: falseandstatus: "confusable".Visuals / API Examples
Example
checkresponse when an existingmattcollides with a visually similar label that punycodes differently:{ "ok": true, "available": false, "name": "…", "canonical": "…", "code": "confusable", "reason": "Username is visually confusable with existing name \"matt\"" }