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

Previous passwords are remembered after user is deleted #28

Closed
phil-davis opened this issue Jul 10, 2018 · 6 comments
Closed

Previous passwords are remembered after user is deleted #28

phil-davis opened this issue Jul 10, 2018 · 6 comments
Labels
bug Something isn't working p4-low
Milestone

Comments

@phil-davis
Copy link
Contributor

Steps to reproduce:

  1. Have password_policy enabled with "different than last passwords" checked and some number of "last passwords" specified (e.g. 3).
  2. As an admin, create user ```test1with passwordTest1111##``
  3. Login to ```test1, change your password to Test1234##`` and logout.
  4. As an admin, delete test1
  5. As an admin, create user ```test1with passwordTest1111##`` or ``Test1234##``

The admin is told that the password must be different to the last 3 passwords.

@phil-davis
Copy link
Contributor Author

I noticed this because I saw that:

mysql> select * from oc_user_password_history;

was growing with a list of password hashes for users that I had deleted.

When a user is deleted, should password_policy be listening to a hook from that, and delete the password history also?

If so, then what should happen when password_policy is disabled. Users will be deleted but their password history left behind in oc_user_password_history

@phil-davis phil-davis changed the title Prrevious passwords are remembered after user is deleted Previous passwords are remembered after user is deleted Jul 10, 2018
@PVince81 PVince81 added the bug Something isn't working label Jul 10, 2018
@PVince81
Copy link
Contributor

hmmm... does an admin ever reuse user names when dealing with different people ?
I could imagine an admin recreating a user that was deleted by mistake for the same person.

In any case, it sounds reasonable to clear the history as we also delete many other things when a user is deleted.

Technically we should listen to the "user delete" hook to trigger this action.

Hmmm, seems we're missing a symfony event for user deletion ? https://github.com/owncloud/core/blob/v10.0.9RC3/lib/private/User/User.php#L211 @sharidas

@phil-davis
Copy link
Contributor Author

I think this is low priority. It will not hurt anybody who happens to re-use a uid and happens to try to re-use a previous password - whether or not the real user is the same person. They can just choose another password. So it could be fixed whenever the needed hook(s) appear in core.

I raised the issue more because it is nice to keep databases tidy and not have old "unlinked" data hanging around.

@sharidas
Copy link
Contributor

@PVince81 The symfony event we have is here -> https://github.com/owncloud/core/blob/master/lib/private/Server.php#L340-L346
And this is for post delete. For pre-delete yes we are missing event at https://github.com/owncloud/core/blob/v10.0.9RC3/lib/private/User/User.php#L211

@PVince81
Copy link
Contributor

similar to #53

I think both together should be around 0.5md

@PVince81
Copy link
Contributor

put it together in a single ticket #69

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p4-low
Projects
None yet
Development

No branches or pull requests

3 participants