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

sync of local user deletes the local user #511

Open
phil-davis opened this issue Feb 11, 2020 · 4 comments
Open

sync of local user deletes the local user #511

phil-davis opened this issue Feb 11, 2020 · 4 comments

Comments

@phil-davis
Copy link
Contributor

phil-davis commented Feb 11, 2020

  Scenario: sync a local user
    Given user "local-user" has been created with default attributes in the database user backend
    When the administrator changes the display name of user "local-user" to "Test User" using the occ command
    And LDAP user "local-user" is resynced
    Then the command should have been successful
    And user "local-user" should not exist

This scenario added in PR #506 looks a bit surprising to me. The test does the command:

occ user:sync OCA\User_LDAP\User_Proxy -u local-user -m remove

local-user is in the local user database, not in LDAP. So of course it is not found in LDAP. The admin has specified -m remove so the command removes the user. But the user is (and always was) just in the local database, they never came from LDAP in the first place.

If I was the admin then I would be a bit surprised that the user:sync command could ever delete a "real local user", even when I use -m remove. I feel like I should get an error message whenever the UID specified after -u is a "real local user". I should not be able to accidentally mess up a "real local user" with the user:sync command. I should have to explicitly do a user:delete if I want to delete a "real local user" (e.g. before then doing user:sync to sync the same UID that has just been added to LDAP)

@phil-davis
Copy link
Contributor Author

phil-davis commented Feb 11, 2020

Assigned myself so that I follow-up the answer to this.

@pmaier1 @micbar or anybody - opinion?

@pmaier1
Copy link
Contributor

pmaier1 commented Feb 11, 2020

Yes, I fully agree. Great spot @phil-davis. The command's use case is to clean-up LDAP users which are not available in LDAP any more. The command should not delete users that do not come from LDAP.

@phil-davis phil-davis added the bug label Feb 11, 2020
@phil-davis
Copy link
Contributor Author

Added the "bug" label.
The fix is likely to be in core code (just a guess, because the user:sync command is in core)
@micbar please assign priority... for dev action.

@phil-davis phil-davis removed their assignment Feb 26, 2020
@phil-davis
Copy link
Contributor Author

I took myself off assignment, this now needs to be scheduled for developer work...
CC @micbar

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

2 participants