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

[stable30] fix(user_ldap): Avoid extra LDAP request when mapping a user for the first time #50778

Draft
wants to merge 6 commits into
base: stable30
Choose a base branch
from

Conversation

backportbot[bot]
Copy link

@backportbot backportbot bot commented Feb 12, 2025

Backport of #46114

Warning, This backport's changes differ from the original and might be incomplete ⚠️

Todo

  • Review and resolve any conflicts
  • Amend HEAD commit to remove the line stating to skip CI

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

…first time

Avoids using several LDAP searches to get UUID, display name and
 internal name, now gets all attributes at the same time.
Also avoids extra request to build an unused user object in userExists.

Signed-off-by: Côme Chilliet <[email protected]>
Avoid surprises by making sure these are lowercased apart from
 documented special case user displayname.

Signed-off-by: Côme Chilliet <[email protected]>
Signed-off-by: Côme Chilliet <[email protected]>

[skip ci]
@backportbot backportbot bot requested review from artonge, blizzz and come-nc February 12, 2025 12:16
@backportbot backportbot bot added 3. to review Waiting for reviews feature: ldap labels Feb 12, 2025
@backportbot backportbot bot added this to the Nextcloud 30.0.6 milestone Feb 12, 2025
} else {
$mapper = $this->getGroupMapper();
$nameAttribute = $this->connection->ldapGroupDisplayName;
$filter = $this->connection->ldapGroupFilter;
}

//let's try to retrieve the Nextcloud name from the mappings table
Copy link
Contributor

@max-nextcloud max-nextcloud Feb 12, 2025

Choose a reason for hiding this comment

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

A few lines below...

		if (!$autoMapping) {
			/* If no auto mapping, stop there */
			return false;
		}

Did not exist in the stable31 version instead a long code block was added:
https://github.com/nextcloud/server/pull/46114/files#diff-90df871fbdee47e27be0de35ab5a48b6577b267c7f4b9d9d3dddb3b8cef6e80dR504

Copy link
Contributor

Choose a reason for hiding this comment

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

The rest seems to be the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants