search

factorio-tools / blueprints-database

[WIP] Factorio Blueprints Database
MIT License
7 stars 0 forks source link

[BUG] Trim whitespace from username during registration #4

Closed richardsondev closed 5 years ago

richardsondev commented 5 years ago

Describe the bug User account names can be registered with leading spaces and trailing spaces.

To Reproduce Steps to reproduce the behavior:

  1. Create a user account with a new similar to "test123"
  2. Logout of the new account
  3. Create a user account with the same name "test123"
  4. Receive a 'username in use' error
  5. Attempt to create a user account with the name "test123 " or " test123" (leading / trailing spaces)
  6. Account is created successfully

Expected behavior For both account names listed in step 5, we should receive a 'username in use' error

Screenshots image

teoxoy commented 5 years ago

We need to constrain the input to alphanumeric + .-_ symbols (i.e. [a-zA-Z0-9._-]) so adding whitespace wouldn't be allowed anyway.

Do you think we should add any other symbols other than those mentioned above?

stuffedmotion commented 5 years ago

I can take care of this On Jul 2, 2019, 5:16 AM -0400, Teodor Tanasoaia notifications@github.com, wrote:

We need to constrain the input to alphanumeric + .- symbols (i.e. [a-zA-Z0-9.-]) so adding whitespace wouldn't be allowed anyway. Do you think we should add any other symbols other than those mentioned above? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

teoxoy commented 5 years ago

It's just a matter of adding this decorator to the username field.

@Matches(/^[a-zA-Z0-9._-]+$/, {
    message: 'username must contain only letters, numbers and the following symbols: ._-'
})
stuffedmotion commented 5 years ago

Great I’ll implement that. We will also want to allow more symbols than that (e.g. @ ) to ensure we accept any typical password. (My password uses a @ ;) ) On Jul 2, 2019, 8:14 AM -0400, Teodor Tanasoaia notifications@github.com, wrote:

It's just a matter of adding this decorator to the username field. @Matches(/^[a-zA-Z0-9.-]+$/, { message: 'username must contain only letters, numbers and the following symbols: .-' }) — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

teoxoy commented 5 years ago

That regex is only for the username not for other fields. We shouldn't constrain passwords in the same fashion. I would like to add that password scoring lib and see how it plays out.

stuffedmotion commented 5 years ago

Ah yes, definitely.

On Tue, Jul 2, 2019 at 9:04 AM Teodor Tanasoaia notifications@github.com wrote:

That regex is only for the username not for other fields. We shouldn't constrain passwords in the same fashion. I would like to add that password scoring lib and see how it plays out.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/factorio-tools/blueprints-database/issues/4?email_source=notifications&email_token=AB5F2BMKNX7R2DPW4BNCQRDP5NG47A5CNFSM4H4YPBXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZBGIVA#issuecomment-507667540, or mute the thread https://github.com/notifications/unsubscribe-auth/AB5F2BICEFRZUXRNMVY32T3P5NG47ANCNFSM4H4YPBXA .