Skip to content

LDAP: determine shares of offline users only when needed #27698

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.

Sign up for GitHub

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

Jump to bottom
Merged
blizzz merged 1 commit into from
Jun 28, 2021
Merged

LDAP: determine shares of offline users only when needed #27698

blizzz merged 1 commit into from
Jun 28, 2021

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Jun 28, 2021 •
edited
Loading

The story behind is this:

  1. On a migration call that loops over all users
  2. Also all known LDAP users get into consideration
  3. including those that are not found on LDAP anymore (OfflineUser)
  4. and userExists is run against each user
  5. which also causes an existency check against the LDAP server
  6. for which the DN is required
  7. the getter of the OfflinerUser triggered fetching all details
  8. including checking whether they are a sharee
  9. which depending on the share provider logic might invoke a user existence check

But the only place where we need the information is the ldap:show-remnants command.

fixes a segfault on a migration step of accessibility app on upgrade #25761 (wrong issue)

@blizzz
LDAP: determine shares of offline users only when needed
bbcd8c9 
- determine shares may via Sharing code result in user exists checks
- this may result in an infinite loop when user exists was called before
- the info is really only required at one occ command

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz added this to the Nextcloud 23 milestone Jun 28, 2021
@blizzz
Copy link
Member Author

blizzz commented Jun 28, 2021

/backport to stable22

@blizzz
Copy link
Member Author

blizzz commented Jun 28, 2021

/backport to stable21

artonge
artonge approved these changes Jun 28, 2021
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@Pytal
Copy link
Member

Pytal commented Jun 28, 2021

Drone 🤷‍♂️

@blizzz
Copy link
Member Author

blizzz commented Jun 28, 2021

Drone man_shrugging

unrelated

@blizzz blizzz merged commit 73c5738 into master Jun 28, 2021
@blizzz blizzz deleted the fix/25761/userexistsonldap-loop branch June 28, 2021 17:13
This was referenced Jun 28, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@artonge artonge artonge approved these changes

@Pytal Pytal Pytal approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees

No one assigned

Labels

3. to review Waiting for reviews bug feature: ldap regression

Projects

None yet

Milestone

Nextcloud 23

Development

Successfully merging this pull request may close these issues.

4 participants

@blizzz @Pytal @artonge