Disallowed underscores at the end of usernames aren't detected

[matheus23]

Summary

Usernames like foo_ are invalid (because they can't be used in domains like foo_,files.fission.name), but these cases aren't caught by the code.

Impact

Not sure. Probably a bad error message in the CLI on account creation (with a name that has a trailing underscore).

Solution

Add a check for a trailing underscore to isValid.

Detail

Missing a note endsWithUnderscore in the preds list in isValid:

-- | 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 == '_'

[expede]

@matheus23 I don't think that this is a bug. Here's some (manual) examples with multiple trailing underscores:

❯ nslookup -q=TXT foo_.bar_.runfission.net
Server:		192.168.0.1
Address:	192.168.0.1#53

Non-authoritative answer:
foo_.bar_.runfission.net	text = "Testing by @expede"

❯ nslookup -q=TXT foo_____.runfission.net
Server:		192.168.0.1
Address:	192.168.0.1#53

Non-authoritative answer:
foo_____.runfission.net	text = "Another test by @expede"

We've disallowed underscores at the beginning of usernames, because those are often reserved words in many systems.

[expede]

For context, I believe that this was opened in relation to https://github.com/fission-suite/webnative/issues/259

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.

[matheus23]

I guess we can just close both issues? As far as I understand, there's nothing wrong with username validation, right?

[expede]

On the backend at least, yeah. I don't think I can reproduce the FE issues, but I'll ping Jeff about that on the original issue