Get user home folder before deletion #26848

Merged
merged 1 commit into from Dec 23, 2016

Conversation

Projects
None yet
4 participants
@PVince81
Member

PVince81 commented Dec 20, 2016

Description

After the deletion getHome() will fail because the user doesn't exist
any more, so we need to fetch that value earlier.

Related Issue

Discovered while looking into #26846

Motivation and Context

How Has This Been Tested?

Fix was tested by applying it on top of #26846.
To test manually, use the MD5 getHome hack from 67dbcdc, then delete a user who has already logged in once.
Before the fix: home folder (md5) is not correctly deleted.
After the fix: home folder (md5) is still deleted.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Please review @owncloud/qa @butonic @DeepDiver1975 @VicDeo

Get user home folder before deletion
After the deletion getHome() will fail because the user doesn't exist
any more, so we need to fetch that value earlier.
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Dec 20, 2016

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @butonic, @DeepDiver1975 and @bartv2 to be potential reviewers.

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @butonic, @DeepDiver1975 and @bartv2 to be potential reviewers.

@PVince81 PVince81 added this to the 10.0 milestone Dec 20, 2016

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Dec 20, 2016

Member

Note that this might not be reproducible in LDAP cases because for LDAP the user is rather deleted from within LDAP. It could happen however than after the LDAP user becomes a "remnant", deleting using occ user:delete will not delete the home folder when LDAP uses the "home directory rule" approach.

Would be good to test that manually too @davitol @SergioBertolinSG

Member

PVince81 commented Dec 20, 2016

Note that this might not be reproducible in LDAP cases because for LDAP the user is rather deleted from within LDAP. It could happen however than after the LDAP user becomes a "remnant", deleting using occ user:delete will not delete the home folder when LDAP uses the "home directory rule" approach.

Would be good to test that manually too @davitol @SergioBertolinSG

@phisch

This comment has been minimized.

Show comment
Hide comment
@phisch

phisch Dec 23, 2016

Contributor

Besides what @PVince81 mentioned about possible LDAP problems: 👍

Contributor

phisch commented Dec 23, 2016

Besides what @PVince81 mentioned about possible LDAP problems: 👍

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
Member

DeepDiver1975 commented Dec 23, 2016

👍

@DeepDiver1975 DeepDiver1975 merged commit 492320f into master Dec 23, 2016

4 checks passed

Scrutinizer 2 new issues
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@DeepDiver1975 DeepDiver1975 deleted the deleteuser-gethomeearly branch Dec 23, 2016

@LukasReschke LukasReschke referenced this pull request in nextcloud/server Dec 23, 2016

Merged

Get user home folder before deletion #2845

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 24, 2017

Member

stable9.1: #27492

Member

PVince81 commented Mar 24, 2017

stable9.1: #27492

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