-
Notifications
You must be signed in to change notification settings - Fork 210
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
Sharing key increase complexity #901
Sharing key increase complexity #901
Conversation
…s setting to generate a more complex sharing key.
name: $localize`Sharing key length`, | ||
priority: ConfigPriority.underTheHood | ||
}, | ||
description: $localize`The longer the keys are, the more secure the share links will be.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather say something like: that longer the key, harder is it to guess it. The current phrasing implies better underlying security with longer keys.
I would also add that using lenght of 8 takes 70000 years to try all permutation (assuming the server can serve a request in 10s), which should be complex enough.
Also i would add a note that changing this number won't invalidate existing sharing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I changed to "The longer the keys are, the harder they are to guess.".
Do you want I add "Changing this number won't invalidate existing sharing" in the setting description ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, should I do a new PR with these changes, or can I update this one ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please add that text too.
Yes you should be able merge the changes here too.
Nice! Thank you! I made a comment on the config description. Otherwise it looks good to me. |
Sorry for the close, I had some troubles with PR. I'm just submitting a new one. Replaced by #912 |
Add a sharing key complexity setting to generate more complex keys.
Keys too long will be displayed with an ellipsis.
(refs #883)
That's my first github PR. I updated my branch twice so there are merge commits. Tell me if this is ok or if you want I make a new more clean branch.
Thanks