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

implement trim() to password input field #7661

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

redoo-networks
Copy link

We had a long discussion within our team, if we should trim a password and remove leading and trailing spaces within an existing environment.

We come to the conclusion, that it will provide more stability and default users do not use leading and trailing spaces.
We save more time by get rid of Copy & Paste errors in support requests, then it will cost one time to help users fix their password.

Because we want to push this into main repo, we implement a configuration option to enable this.
Per default this feature is disabled and not used.
We also implement this into the "passwords" plugin to transparent remove leading/trailing spaces.

Because this is our first contribution to roundcube, please let me know when some formatting is not correct or the feature is not wanted.

Regards,
Stefan

@alecpl
Copy link
Member

alecpl commented Nov 23, 2020

I don't like an extra option for this. I guess, a more appropriate way would be:

  1. Always trim passwords in password plugin, or forbid entering them with an error to the user.
  2. When logging in:
  • Try to log in with the supplied password
  • If it fails - check if the typed password had any spaces before or after
  • Try again with leading and trailing spaces stripped away

@philliptaylorpro
Copy link

philliptaylorpro commented Jan 11, 2021

I don't really like this idea. You may be the only company in the world to trim passwords like this that I'm aware, so it seems like perhaps this is one or two people using bad practice and lazy copy/pasting than actually something roundcube should accommodate. Especially since no one else does this. Given the password goes upstream to the mailserver I think this is asking for trouble. Then there's the added in fact that trimming people's passwords will make them less secure than the users think they are, causing possible confusion and reducing the effort for brute force attacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants