Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust Input Validation Policies #297

Merged
merged 5 commits into from
Jun 19, 2020
Merged

Adjust Input Validation Policies #297

merged 5 commits into from
Jun 19, 2020

Conversation

mattxwang
Copy link
Collaborator

@mattxwang mattxwang commented Jun 6, 2020

This is a PR that follows up #283 in applying policy fixes to our input validations. Here is an exhaustive list of the changes that I've proposed:

  • only allows alphanumeric characters and -_. in the username, which is consistent with the username policies of many other websites. key thing to note here is this now invalidates @ in a username, which previously throws an error (as it can't be used in the LHS of an email address)
  • increases the max character cap for usernames to 32: we don't face any storage issues here, and 20 seems slightly too restrictive (looking at twitter handles and the like)
  • removes character restrictions on passwords. I think this is one of the most significant changes. There is no reason as to why spaces, periods, or a variety of other special characters cannot be used in passwords: they hinder password managers, and password restrictions are shown to make users forget their passwords easily and/or write them down physically. Plus, there is no technical need for the restriction: we just pipe the input into a hash function, and we never display it or query with it.
  • increases the max character cap for passwords to 128: the current number of 20 is much too restrictive, both in terms of human generated passwords (such as the classic correcthorsebatterystaple) to the prevalence of password managers. Again, no technical issues here, as we aren't storing the password itself.
  • allows for -_. (in addition to current policy) for display names.

I am especially curious as to what our thoughts are on the username restriction; totally down to follow in the footsteps of Twitter or GitHub and allow for -, _, ., etc.

Also, this PR removes a duplicate set of tests for CreateUserForm that are already covered in validate.

@mattxwang mattxwang added the feature New feature or request label Jun 6, 2020
@krashanoff
Copy link
Contributor

Hey Matt! Thanks for following up on your other PR. After merging this I'll mark another minor release since we will have changed the login experience in rather considerable scale at that point.


I am especially curious as to what our thoughts are on the username restriction; totally down to follow in the footsteps of Twitter or GitHub and allow for -, _, ., etc.

I think that this could be beneficial. It wouldn't mess with anything on our end as far as I know, and it adds a little more variety that kids might have seen in other social media.

@mattxwang
Copy link
Collaborator Author

Updated the changes!

Copy link
Contributor

@krashanoff krashanoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for the work Matt! LGTM

@krashanoff krashanoff merged commit dab4a97 into master Jun 19, 2020
@krashanoff krashanoff deleted the adjust-input-policies branch June 19, 2020 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants