relax strict getHome behaviour for LDAP users in a shadow state #17717

blizzz · 2019-10-28T14:13:06Z

simplifies deletion process
less strange behaviour when looking up home storage (as long as it is local)
enables transfer ownerships after user went invisible on ldap (* not sure about behviour with external storages requiring auth)
storages of deleted ldap users can still be accessed, apart form potential external storages that require auth
decouples userExists from an LDAP check, allows for setting deleted state directly, solves some edge cases
adapt tests

mwuttke · 2019-12-02T13:49:49Z

@blizzz: Can you backport this patch for nc 16 and/or nc 17?

Thanks & Greetings,
Michael form Berlin

blizzz · 2019-12-02T17:15:25Z

@moodlebeuth Heyo Michael :) As it contains elementary and behavorial changes, I hesitate to pour this into a stable release. On the other hand, the changes made are done with patching older versions in mind, in favour of "better" changes (which still might come in as an additional, explicit commit). So at least it would be easy to apply it to an older series. It's not a definite no however.

Did you try it on a test system yet?

blizzz · 2019-12-06T00:28:15Z

Some tests are failing also because of #18254, but another LDAP integration test, too, which does not seem related however. Let's rebase once #18254 is merged.

blizzz · 2020-01-08T08:33:19Z

@rullzer np, probably gonna backport it anyway

juliushaertl · 2020-01-08T13:05:02Z

Still a failure:


--- Failed scenarios:
--
260 |  
261 | /drone/src/build/integration/ldap_features/openldap-numerical-id.feature:50

blizzz · 2020-01-08T13:18:08Z

yep

juliushaertl

Looks good otherwise 👍

blizzz · 2020-01-10T16:20:38Z

okay, I know the reason, but have no good way around. yet. We know the user from the previous test scenario (mapped in DB), but the filter has changed. So we would need to check LDAP to see that the user has gone. Previously it would happen through userExists since it did a lookup in LDAP. Now we only report the local state (and a true is necessary to be able to delete a user locally...). Tricky. To be continued.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

blizzz · 2020-01-14T08:41:31Z

Tests are passing now. When reading members of a group, and those were already mapped in Nextcloud, but now excluded by a filter, they would pass if not ensured that they are available for Nc.

blizzz · 2020-01-14T08:56:57Z

/backport to stable18

blizzz · 2020-01-14T08:57:03Z

/backport to stable17

blizzz · 2020-01-14T08:57:09Z

/backport to stable16

backportbot-nextcloud · 2020-01-14T08:58:26Z

backport to stable18 in #18882

backportbot-nextcloud · 2020-01-14T08:59:13Z

The backport to stable17 failed. Please do this backport manually.

backportbot-nextcloud · 2020-01-14T08:59:54Z

The backport to stable16 failed. Please do this backport manually.

* simplifies deletion process * less strange behaviour when looking up home storage (as long as it is local) * thus could enable transfer ownerships after user went invisible on ldap backport of #17717 Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> decouple userExists from userExistsOnLDAP check allows to mark users as offline right away, avoids a gap of being not a user and causing weird side effects Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> adjust tests Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> remove superfluous tests - user_ldap is not exposed to public api, it is always behind ldap_proxy - this is too much for a unit test - integration tests cover userExists implicitly Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> ensure that only valid group members are returned Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

* simplifies deletion process * less strange behaviour when looking up home storage (as long as it is local) * thus could enable transfer ownerships after user went invisible on ldap backport of #17717 Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> decouple userExists from userExistsOnLDAP check allows to mark users as offline right away, avoids a gap of being not a user and causing weird side effects Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> adjust tests Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> remove superfluous tests - user_ldap is not exposed to public api, it is always behind ldap_proxy - this is too much for a unit test - integration tests cover userExists implicitly Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> ensure that only valid group members are returned Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> fixup! relax strict getHome behaviour for LDAP users in a shadow state fixup! relax strict getHome behaviour for LDAP users in a shadow state

* simplifies deletion process * less strange behaviour when looking up home storage (as long as it is local) * thus could enable transfer ownerships after user went invisible on ldap backport of #17717 Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> decouple userExists from userExistsOnLDAP check allows to mark users as offline right away, avoids a gap of being not a user and causing weird side effects Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> adjust tests Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> remove superfluous tests - user_ldap is not exposed to public api, it is always behind ldap_proxy - this is too much for a unit test - integration tests cover userExists implicitly Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> ensure that only valid group members are returned Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

blizzz added bug enhancement 2. developing Work in progress feature: ldap labels Oct 28, 2019

blizzz added this to the Nextcloud 18 milestone Oct 28, 2019

blizzz mentioned this pull request Oct 28, 2019

Unable to transfer file ownership away from deleted ldap user #15536

Closed

blizzz force-pushed the fix/noid/ldap-relax-getHome branch 2 times, most recently from dc9ca91 to 84ed1ed Compare November 18, 2019 12:13

blizzz added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 18, 2019

blizzz requested review from juliushaertl, kesselb, skjnldsv, rullzer and icewind1991 November 18, 2019 14:38

blizzz mentioned this pull request Nov 19, 2019

Files not accessible after sharing LDAP user was deleted #11551

Closed

blizzz force-pushed the fix/noid/ldap-relax-getHome branch 2 times, most recently from ca5d9ac to 081ffab Compare November 29, 2019 14:27

blizzz added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 29, 2019

skjnldsv approved these changes Nov 29, 2019

View reviewed changes

blizzz force-pushed the fix/noid/ldap-relax-getHome branch from 081ffab to d3fd735 Compare December 5, 2019 22:40

blizzz added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 6, 2019

This was referenced Dec 11, 2019

18 Beta 2 #18343

Merged

18 beta 3 #18370

Merged

rullzer mentioned this pull request Dec 19, 2019

18 beta 4 #18480

Merged

18 tasks

juliushaertl approved these changes Jan 8, 2020

View reviewed changes

blizzz force-pushed the fix/noid/ldap-relax-getHome branch from d67ef95 to 2411b62 Compare January 13, 2020 16:11

ensure that only valid group members are returned

489ed87

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

blizzz force-pushed the fix/noid/ldap-relax-getHome branch from 2411b62 to 489ed87 Compare January 13, 2020 16:13

backportbot-nextcloud bot added the backport-request label Jan 14, 2020

blizzz merged commit 950856d into master Jan 14, 2020

blizzz deleted the fix/noid/ldap-relax-getHome branch January 14, 2020 08:57

backportbot-nextcloud bot mentioned this pull request Jan 14, 2020

[stable18] relax strict getHome behaviour for LDAP users in a shadow state #18882

Merged

blizzz mentioned this pull request Jan 14, 2020

[stable17] relax strict getHome behaviour for LDAP users in a shadow state #18884

Merged

blizzz mentioned this pull request Jan 14, 2020

[stable16] relax strict getHome behaviour for LDAP users in a shadow state #18885

Merged

blizzz removed the backport-request label Jan 14, 2020

deajan mentioned this pull request Jan 23, 2020

NC15 bug, client does not work for users which have shared files from deleted LDAP users #15161

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

relax strict getHome behaviour for LDAP users in a shadow state #17717

relax strict getHome behaviour for LDAP users in a shadow state #17717

blizzz commented Oct 28, 2019 •

edited

mwuttke commented Dec 2, 2019

blizzz commented Dec 2, 2019

blizzz commented Dec 6, 2019

blizzz commented Jan 8, 2020

juliushaertl commented Jan 8, 2020

blizzz commented Jan 8, 2020

juliushaertl left a comment

blizzz commented Jan 10, 2020

blizzz commented Jan 14, 2020

blizzz commented Jan 14, 2020

blizzz commented Jan 14, 2020

blizzz commented Jan 14, 2020

backportbot-nextcloud bot commented Jan 14, 2020

backportbot-nextcloud bot commented Jan 14, 2020

backportbot-nextcloud bot commented Jan 14, 2020

relax strict getHome behaviour for LDAP users in a shadow state #17717

relax strict getHome behaviour for LDAP users in a shadow state #17717

Conversation

blizzz commented Oct 28, 2019 • edited

mwuttke commented Dec 2, 2019

blizzz commented Dec 2, 2019

blizzz commented Dec 6, 2019

blizzz commented Jan 8, 2020

juliushaertl commented Jan 8, 2020

blizzz commented Jan 8, 2020

juliushaertl left a comment

Choose a reason for hiding this comment

blizzz commented Jan 10, 2020

blizzz commented Jan 14, 2020

blizzz commented Jan 14, 2020

blizzz commented Jan 14, 2020

blizzz commented Jan 14, 2020

backportbot-nextcloud bot commented Jan 14, 2020

backportbot-nextcloud bot commented Jan 14, 2020

backportbot-nextcloud bot commented Jan 14, 2020

blizzz commented Oct 28, 2019 •

edited