Account signup user name validation issues

[jeffgca]

We're not catching some invalid username on signup. This was caught investigating fission-suite/auth-lobby#81

Usernames like this:

• jeff--

...get an error like

Sorry, jeff-- is not a valid username. You can use letters, numbers and hyphens in between.

After some testing, it looks like the code involved doesn't catch the '_' and '@' characters, leading to the creation of an invalid personal address for the account.

[expede]

Yes, dashes or underscores at the beginning or end of names are not valid URLs, and thus fail server validation since it gets used as a subdomain. We should definitely make the message clearer about that.

[expede]

it looks like the code involved doesn't catch the '_' and '@' characters

Do you have an example of one with @? I'm not able to reproduce that case:

I wonder if the lobby doesn't wait to check if it actually got created before trying to continue, because the server will reject those with a HTTP 422 (validation code here).

-- Convention: Right for "success", and Left for "error"

> mkUsername "je-ff"
Right (Username "je-ff") -- Valid!

> mkUsername "je_ff"
Right (Username "je_ff") -- Valid!

> mkUsername "hey@there"
Left Invalid -- Fails validation

> mkUsername "jeff--"
Left Invalid -- Fails validation

> mkUsername "_jeff"
Left Invalid -- Fails validation

[jeffgca]

Yeah I can't repro the @ issue on mobile. Underscores are definitely a problem.

I'll check macOS / Firefox again tomorrow

[icidasset]

Moving this to the webnative repo, since the username validation happens there.

[matheus23]

This is the logic in the fission server:

-- | Confirm that a raw is valid
isValid :: Text -> Bool
isValid txt =
  all (== True) preds
  where
    preds :: [Bool]
    preds = [ okChars
            , not blank
            , not startsWithHyphen
            , not endsWithHyphen
            , not startsWithUnderscore
            , not inBlocklist
            ]

    blank = Text.null txt

    inBlocklist = elem txt Security.blocklist
    okChars     = Text.all isURLChar txt

    startsWithHyphen = Text.isPrefixOf "-" txt
    endsWithHyphen   = Text.isSuffixOf "-" txt

    startsWithUnderscore = Text.isPrefixOf "_" txt

isURLChar :: Char -> Bool
isURLChar c =
     Char.isAsciiLower c
  || Char.isDigit      c
  || c == '-'
  || c == '_'

And this is the logic in webnative:

/**
 * Check if a username is valid.
 */
export function isUsernameValid(username: string): boolean {
  return !username.startsWith("-") &&
         !username.endsWith("-") &&
         !username.startsWith("_") &&
         /^[a-zA-Z0-9_-]+$/.test(username) &&
         !USERNAME_BLOCKLIST.includes(username.toLowerCase())
}

Yes, dashes or underscores at the beginning or end of names are not valid URLs

If underscores at the end of names are invalid, then we're not checking that both in the server and webnative. Easy enough to fix though :)

I'll create another issue for the web-api repo

[expede]

If underscores at the end of names are invalid, then we're not checking that both in the server and webnative. Easy enough to fix though :)

I misspoke for underscores. Underscores are actually valid at the beginning and end. We used to disallow beginning and end underscores to avoid confusion around things like _dnslink, which is also on the blocklist, but starting with _ often signifies a special/internal subdomain. We had someone complain that they use a name that ends in an underscore elsewhere on the internet, so we lifted the restriction for the end specifically. No change required on the server unless we want to lift the restriction on the front as well.

[expede]

@therealjeffg just following up; are you able to find a case with the @? I think we've determined that the _ is working as expected, unless we want to change those rules.

[jeffgca]

Sorry for the delay in getting back to this. The thing that needs to be addressed a still is the messages that the user sees:

Sorry, _jg is not a valid username. You can use letters, numbers and hyphens in between.

If underscores are valid, we should change this message ti say something like:

You can use letters, numbers, and underscores, with hyphens in between.

[expede]

Sorry for the delay in getting back to this

No worries!

If underscores are valid, we should change this message ti say something like:

Agreed — better message here for sure. Do you think it's worth using a "proper" parser here to give detailed error messages (e.g. "you can't have an underscore at the front and $ is not an allowed character") or is the single standard message sufficient?

[jeffgca]

Agreed — better message here for sure. Do you think it's worth using a "proper" parser here to give detailed error messages (e.g. "you can't have an underscore at the front and $ is not an allowed character") or is the single standard message sufficient?

I think a single message that completely describes what is always allowed and not allowed is the best experience especially as it reduces frustrations cycles if the user understands what the limits are up-front.

Aside: turns out designing systems that allow people to name things ( eg themselves ) is also hard.

[icidasset]

No issue, need to improve message in lobby: https://github.com/fission-codes/auth-lobby/issues/119