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

Password protection for link shares should suggest autogenerated passwords #12171

Open
2 of 4 tasks
jancborchardt opened this issue Oct 31, 2018 · 21 comments
Open
2 of 4 tasks

Comments

@jancborchardt
Copy link
Member

jancborchardt commented Oct 31, 2018

Currently when you want to password-protect a link, you have to think of a password yourself. Which is possibly not secure, or not easy to remember. This is especially useful when password-protection is mandatory cause enforced by the admin.

We should help people by autogenerating a default password whenever you check "Password-protect", until it is changed. It should be passwords which can be easily communicated over other channels, so maybe something made of other words like:

house right carton keyboard

Or something like that – you know what I mean. ;)

What do you think @nextcloud/designers @nextcloud/security?

@jancborchardt jancborchardt added enhancement design Design, UI, UX, etc. labels Oct 31, 2018
@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #10644 (Link sharing broken with enforced password protection), #5658 (share by email - password email not sent - solved), #7469 (Cannot set password for share by email links), #4008 (Disable reset password link), and #7258 (strange problem when shared with password ).

@rullzer
Copy link
Member

rullzer commented Oct 31, 2018

The trouble here is that if the password policy app is enabled generating 'easy to remember' passwords becomes hard to generate.

Like special charachters. numbers, uppercase lowercase etc.

@LukasReschke
Copy link
Member

LukasReschke commented Oct 31, 2018

The trouble here is that if the password policy app is enabled generating 'easy to remember' passwords becomes hard to generate.

https://blog.codinghorror.com/password-rules-are-bullshit/ 🙈

… so for whoever enables password policies: just append some random chars which fulfill the policy? (and let them notice that generic password policies are not the best solution 😄 )

@rullzer
Copy link
Member

rullzer commented Nov 1, 2018

@LukasReschke well fair enough :)

@skjnldsv
Copy link
Member

skjnldsv commented Nov 1, 2018

Of course not, because have you met any users lately?
They consistently ruin every piece of software I've ever written. Yes, yes, I know you, Mr. or Ms. über-geek, know all about the concept of entropy. But expressing your love of entropy as terrible, idiosyncratic password rules …

Oh my! 😁
Thanks @LukasReschke

@nickvergessen
Copy link
Member

Sounds fine to me 👍

@jancborchardt
Copy link
Member Author

Setting to "help wanted" since people agree it’s good but no one volunteered to pick it up yet. ;)

Also cc @nextcloud/android @nextcloud/ios @nextcloud/desktop just FYI. Nothing is being developed on this right now, but if it happens we should also have this in the mobile apps and desktop clients for feature parity.

@tobiasKaminsky
Copy link
Member

Server: nextcloud/password_policy#83

@skjnldsv
Copy link
Member

skjnldsv commented Jul 31, 2019

With password policy & check against haveIBeenPwnd (therefore small loading delay) If password_policy is not enabled then we also ship a very simple password generator
Peek 31-07-2019 16-45 Peek 31-07-2019 16-51

@szaimen
Copy link
Contributor

szaimen commented Jul 31, 2019

Nice to see!

@skjnldsv: Will you be able to copy the link first, paste the link to my interlocutor, open the menu afterwards again and copy and paste the password in the second step? I think it is much more logical to send the link first.

@skjnldsv
Copy link
Member

@szaimen depends if the password is enforced or not.
If this is enforced you cannot create the link without a valid password, so you will know the password BEFORE the link token.

If not enforced, sure you can create a link and add a password afterwards.

@szaimen
Copy link
Contributor

szaimen commented Jul 31, 2019

@skjnldsv okay, and if passwords are enforced, can't I just copy the password afterwards? Why should showing the password afterwards in the password-field in cleartext be a security risk, if I am the only one that has access to the link-configuration?

@skjnldsv
Copy link
Member

@szaimen the security is not here. The security is that we don't save the password in plain text. We encrypt it and we save it in the database. So you cannot get the plain text password back. :)

This is just technically not possible.
This is not an issue of showing it in plain text or with ****. We just don't have it in a usable state ;)

@szaimen
Copy link
Contributor

szaimen commented Jul 31, 2019

@skjnldsv thank you for the clarification!
What about the possibility to suggest a new password after the link-creation?
So e.g. with enforced passwords you take the password that is suggested and when you revisit the password-field afterwards there is a new password suggested that you can accept. Then you could copy and paste the new one (there should probably be a hint that this is a new password and not the old one and has to be accepted by the user to get active). I think that would solve this problem.

BTW: if "Always ask for password" and/or "enforce passwords" ist enabled, I think that clicking anywhere else to close the popup-window should automatically accept the password. So if "Always ask for password" is enabled just unchecking or clearing the password-field would disable the password protection (I think that's the best for security).

@skjnldsv
Copy link
Member

Well, yes you can always enter a new password.
But I don't think what you're requesting is in our development scope. 😕
The current process is simple & secure enough for now. :)

BTW: if "Always ask for password" and/or "enforce passwords" ist enabled, I think that clicking anywhere else to close the popup-window should automatically accept the password. So if "Always ask for password" is enabled just unchecking or clearing the password-field would disable the password protection (I think that's the best for security).

That is how it currently is yes (on the upgraded sharing ui)

@szaimen
Copy link
Contributor

szaimen commented Jul 31, 2019

@skjnldsv thank you for your answer.
I have just one last question for now:
When "always ask for password' is enabled (and enforce passwords is disabled), can I manually disable the password during the link-creation process and get a suggested password afterwards when revisiting the password-field that I can accept? (That would be a workaround for now.)
You probably implied it here:

If not enforced, sure you can create a link and add a password afterwards.

But I wasn't completely sure it will work this way.
Or is the password just once and only suggested during the link-creation process?

@skjnldsv
Copy link
Member

The suggestion is only happening once.
But if you disable and enable it again, you will receive a new suggestion.

@skjnldsv
Copy link
Member

Fixed with #15719

@tobiasKaminsky
Copy link
Member

@skjnldsv for clients: is there a way to get some passwords, as specified by password policy rules?

@skjnldsv
Copy link
Member

skjnldsv commented Oct 30, 2019

Yes, the password policy app offers an api for that.
Otherwise I just generate a strong random password (since there is nothing enforced if disabled)

if (this.config.enableLinkPasswordByDefault) {
shareDefaults.password = await this.generatePassword()
}

async generatePassword() {
// password policy is enabled, let's request a pass
if (this.config.passwordPolicy.api && this.config.passwordPolicy.api.generate) {
try {
const request = await axios.get(this.config.passwordPolicy.api.generate)
if (request.data.ocs.data.password) {
return request.data.ocs.data.password
}
} catch (error) {
console.info('Error generating password from password_policy', error)
}
}
// generate password of 10 length based on passwordSet
return Array(10).fill(0)
.reduce((prev, curr) => {
prev += passwordSet.charAt(Math.floor(Math.random() * passwordSet.length))
return prev
}, '')
},

reopened for client parity

@skjnldsv skjnldsv reopened this Oct 30, 2019
@skjnldsv skjnldsv added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels Oct 30, 2019
@tobiasKaminsky
Copy link
Member

curl --request GET \ --url https://localhost/nc/ocs/v2.php/apps/password_policy/api/v1/generate \ --header 'authorization: Basic dG9iaTpTSGZ6ZC04YkQyNC1xOUtSaS1zU2NRay03ZHh4eA==' \ --header 'ocs-apirequest: true'

As it is via https it is no problem to fetch only one password.

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

No branches or pull requests

9 participants