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

Detect and handle password change for non-local users #3555

Closed
blizzz opened this issue May 31, 2013 · 15 comments
Closed

Detect and handle password change for non-local users #3555

blizzz opened this issue May 31, 2013 · 15 comments

Comments

@blizzz
Copy link
Contributor

blizzz commented May 31, 2013

Scenario:
The password is required to decrypt files. The encryption app listen to post_setPassword Hooks. However, they are not triggered for password changes of external auth backends like but not limited to LDAP. At the moment, the recovery key is necessary to regain the files.

Solution:
Check if the password changes and if it is detected, offer a dialog to enter the old password and act upon it. If the old password is unknown, the recovery possiblity still exists (given it is activated by the user).

cc @FlorinPeter @schiesbn @samtuke

@karlitschek
Copy link
Contributor

Yes. This is an limitation of the architecture. A dialog as you proposed could be a solution. This only works if someone is using the webinterface of course. Mobile and Desktop users would still lose access.
That's tricky.

@blizzz
Copy link
Contributor Author

blizzz commented May 31, 2013

True. But the UI is the only way to get around this and such a dialog better than nothing (or the recovery option which is not enabled by default).

What will actually happen when such a users continues to access ownCloud via webdav? will encrypted files be synced down? or is 4xx thrown?

@FlorinPeter
Copy link
Contributor

@blizzz good point. This check is currently missing. I will do a basic check and create a PR so we can discuss it there.

@schiessle
Copy link
Contributor

I would like to use this issue to discuss the recovery scenario. At the moment the admin can insert his recovery password in the user management settings which will automatically recover the files once a new password is set.

IMHO this is a quite nice and transparent integration of the recovery feature into ownClouds user management. But this doesn't work for user-backends who doesn't support changing the password. Technically we could switch from a post-hook to a pre-hook to also trigger the recovery function if the back-end doesn't change the password. In such a case the user managements password filed would only manage the recovery but not the password change. But I fear that this is quite confusing for the admin.

Maybe our @owncloud/designers have a idea how to distinguish this operations visually? Additionally it is possible to use multiple back-ends so it is possible that for some users this field would change log-in and encryption key password and for other users only the encryption key password

cc @jancborchardt @Raydiation @zimba12 @raghunayyar afaik you are in the designer team, I CC'ed you because it seems like the group mention doesn't work.

@blizzz
Copy link
Contributor Author

blizzz commented Jun 4, 2013

One question: what if the password change fails (for whatever reason) after the pro-hook had run?

Please also have in mind that the common LDAP scenario is that up to tens of thousands of users (e.g. TU Berlin) are using ownCloud. Policies might require (as in force) to change the password regularly (which is done by the user, but of course not withing ownCloud). To require the admin become active on such cases is what shall be tried to avoid, because it may put a lot of work on him, but furthermore introduces a delay (from notifying the admin until he recovers the files).

@blizzz
Copy link
Contributor Author

blizzz commented Jun 4, 2013

Oh, correct me if we're already beyond the latter scenario. Feels so, but the issue is targeted as something else. Confusing, esp. at that time :)

@schiessle
Copy link
Contributor

If the user (or the admin) changes the password and the user still remember his old password, what should be the case in this scenario than this is not a problem. The user will be able to update his private key password in his personal owncloud settings. This is already implemented in the pull request mentioned above.

The interesting part is if the user loses his password and want to make use of the recovery feature.

@schiessle
Copy link
Contributor

One question: what if the password change fails (for whatever reason) after the pre-hook had run?

Technically we could have both, the pre-hook and the post-hook where the pre-hook only gets executed if the back-end doesn't support password change for the given user while in all other cases we execute the post-hook to make sure that the password change was successfully.

But I wonder if this is intuitive, or better how to make it intuitive for the admin? Because the same interface would either execute A and B or only B, depending on the back-end (A=change log-in password; B=recover encryption keys)

@schiessle
Copy link
Contributor

This pull request solves the recovery issue for external user back-ends: #3610

Still, comments from our designer for further visual improvements are always welcome.

@jancborchardt
Copy link
Member

@schiesbn from what I read this seems a bit hard to reproduce / set up and I’m still a bit busy with other things. Can you give me 1–2 screenshots so I can check it out? Thanks!

@schiessle
Copy link
Contributor

@jancborchardt see the screenshot below. If the admin enters a "Admin Recovery Password" and changes the users password the files get recovered, so that afterwards the user can log-in with the new password and access his files again.
If the admin doesn't enter a recovery password but the user account would support the recovery feature he gets a error message (the default orange bar at the top) when he tries to change a password. Same happens if he enters the wrong recovery password.
If the user back-end doesn't support password change but the admin still enters a valid recovery password and changes a user password he gets a message that the files where recovered to the new password but the log-in password was not changed (in this case, the log-in password has to be changed directly at the back-end additionally).

The input filed is only visible if the encryption app is enabled and the recovery feature is activated.

screenshot

@jancborchardt
Copy link
Member

So this is a per-user action, right? Then it should not be in the top bar.

If I got it right, the flow would be that the admin would first change the user’s password, correct? Then when ownCloud notices that this user or instance has encryption enabled, the admin would be prompted with the recovery password input.

That would only account for the first case though, where the backend supports password change. For backends where that’s not supported, maybe instead of the passwords, that column should have a »Recover« action, which when clicked also exposes the Admin recovery password input.

Did I get that right?

@schiessle
Copy link
Contributor

The recovery password is not bound to a special user. It is system-wide and valid for every user who has enabled the recovery feature. That's why I had the idea to put it at the top. This way the admin only have to enter the recovery password at the top and than can manipulate as much users as he wants.

If I got it right, the flow would be that the admin would first change the user’s password, correct?

With the current implementation the password change will only happen if a correct recovery password was added, otherwise the admin will get a error message that he should provide the recovery password first. If both passwords are provided (recovery password and the new user password) the password will be changed and the files recovered in one run.

Then when ownCloud notices that this user or instance has encryption enabled, the admin would be prompted with the recovery password input.

How would you ask for the recovery password? A pop-up dialog? Not sure how this could be integrated nicely.

For backends where that’s not supported, maybe instead of the passwords, that column should have a »Recover« action, which when clicked also exposes the Admin recovery password input.

So you would just add a button/action "Recover" in the password column for users where the back-end doesn't support password change? Wouldn't it be irritating that some entries in the password column would have a password input field and some a recover action? Further I could imagine that is would be quite irritating that there are two different ways to execute one action depending on the combination of two factors (recovery enabled for user and backend-support password change) which are not really transparent to the admin at this point.

@jancborchardt
Copy link
Member

Yes, the recovery password is system-wide – but the recovery process most likely is for one specific person who lost their password, right? It’s not like an admin would on a regular basis recover multiple people in one go. At least that sounds wrong.

Maybe it’s actually best if the interface stays like you did for now, and we talk about this at the dev meetup? I still don’t quite get the flows or use-cases, so we better talk about that in person.

@schiessle
Copy link
Contributor

Good idea, let us discuss it at the dev meetup.

I will close the issue now, since the initial problem is solved.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants