From f6ab441552bc3c266d8f2b5c03878735dfb44dad Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 7 May 2026 11:53:50 +0200 Subject: [PATCH] feat: Flexible matching between LDAP and SAML user ids Test plan: 1. Setup LDAP with an user uid=bender mail=bender@planetexpress.com 2. Setup SAML with an user uid=bender@planetexpress.com and attribute to map user to existing LDAP users = mail 3. Login with SAML, we will end up with the LDAP user Signed-off-by: Carl Schwan --- lib/Controller/SAMLController.php | 4 +-- lib/SAMLSettings.php | 1 + lib/Settings/Admin.php | 11 +++++++ lib/UserData.php | 2 +- lib/UserResolver.php | 48 +++++++++++++++++++++++++++---- tests/unit/Settings/AdminTest.php | 12 ++++++++ 6 files changed, 69 insertions(+), 9 deletions(-) diff --git a/lib/Controller/SAMLController.php b/lib/Controller/SAMLController.php index b4cc693b6..2227a1e4b 100644 --- a/lib/Controller/SAMLController.php +++ b/lib/Controller/SAMLController.php @@ -392,8 +392,8 @@ public function assertionConsumerService(): Http\RedirectResponse { if ($firstLogin) { $this->userBackend->initializeHomeDir($user->getUID()); } - } catch (NoUserFoundException) { - throw new \InvalidArgumentException('User "' . $this->userBackend->getCurrentUserId() . '" is not valid'); + } catch (NoUserFoundException $e) { + throw new \InvalidArgumentException('User "' . $this->userBackend->getCurrentUserId() . '" is not valid.', previous: $e); } catch (Exception $e) { $this->logger->critical($e->getMessage(), ['exception' => $e, 'app' => $this->appName]); $response = new Http\RedirectResponse($this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.notProvisioned')); diff --git a/lib/SAMLSettings.php b/lib/SAMLSettings.php index 269b8dbb1..1877ee4b5 100644 --- a/lib/SAMLSettings.php +++ b/lib/SAMLSettings.php @@ -59,6 +59,7 @@ class SAMLSettings { 'saml-attribute-mapping-home_mapping', 'saml-attribute-mapping-quota_mapping', 'saml-attribute-mapping-mfa_mapping', + 'saml-attribute-mapping-user_id_ldap_mapping', 'saml-attribute-mapping-group_mapping_prefix', 'saml-user-filter-reject_groups', 'saml-user-filter-require_groups', diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 9f89c3b86..7ddd2d18c 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -14,6 +14,7 @@ use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Services\IInitialState; use OCP\Defaults; +use OCP\IConfig; use OCP\IL10N; use OCP\Settings\IDelegatedSettings; use OneLogin\Saml2\Constants; @@ -24,6 +25,7 @@ public function __construct( private readonly IL10N $l10n, private readonly Defaults $defaults, private readonly IAppConfig $appConfig, + private readonly IConfig $config, private readonly SAMLSettings $samlSettings, private readonly IInitialState $initialState, ) { @@ -148,6 +150,11 @@ public function getForm(): TemplateResponse { 'type' => 'line', 'required' => false, ], + 'user_id_ldap_mapping' => [ + 'text' => $this->l10n->t('Attribute to map the users to an existing LDAP user'), + 'type' => 'line', + 'required' => false, + ], 'group_mapping_prefix' => [ 'text' => $this->l10n->t('Group Mapping Prefix, default: %s', [SAMLSettings::DEFAULT_GROUP_PREFIX]), 'type' => 'line', @@ -155,6 +162,10 @@ public function getForm(): TemplateResponse { ], ]; + if (version_compare($this->config->getSystemValueString('version', '0.0.0'), '34.0.0', '<')) { + unset($attributeMappingSettings['user_id_ldap_mapping']); + } + $userFilterSettings = [ 'reject_groups' => [ 'text' => $this->l10n->t('Reject members of these groups. This setting has precedence over required memberships.'), diff --git a/lib/UserData.php b/lib/UserData.php index 098384077..f091aff04 100644 --- a/lib/UserData.php +++ b/lib/UserData.php @@ -49,7 +49,7 @@ public function getEffectiveUid(): string { try { $providedUid = $this->extractSamlUserId(); $uid = $this->testEncodedObjectGUID($providedUid); - $uid = $this->userResolver->findExistingUserId($uid, true, $providedUid !== $uid); + $uid = $this->userResolver->findExistingUserId($uid, $this->getProviderSettings(), true, $providedUid !== $uid); $this->uid = $uid; } catch (NoUserFoundException) { return ''; diff --git a/lib/UserResolver.php b/lib/UserResolver.php index 84b806857..bf328c7cb 100644 --- a/lib/UserResolver.php +++ b/lib/UserResolver.php @@ -9,19 +9,55 @@ namespace OCA\User_SAML; use OCA\User_SAML\Exceptions\NoUserFoundException; +use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; +use OCP\LDAP\Exceptions\MultipleUsersReturnedException; +use OCP\LDAP\ILDAPProviderFactory; +use OCP\Server; class UserResolver { public function __construct( - private IUserManager $userManager, + private readonly IUserManager $userManager, + private readonly ILDAPProviderFactory $ldapProviderFactory, + private readonly IConfig $config, ) { } /** * @throws NoUserFoundException */ - public function findExistingUserId(string $rawUidCandidate, bool $force = false, bool $isActiveDirectory = false): string { + public function findExistingUserId(string $rawUidCandidate, ?array $idpSettings = null, bool $force = false, bool $isActiveDirectory = false): string { + // If configured, find the user based on a different LDAP attribute. + if ($idpSettings !== null + && version_compare($this->config->getSystemValueString('version', '0.0.0'), '34.0.0', '>=') + && $this->ldapProviderFactory->isAvailable() + && isset($idpSettings['saml-attribute-mapping-user_id_ldap_mapping']) + && $idpSettings['saml-attribute-mapping-user_id_ldap_mapping'] !== null + && $idpSettings['saml-attribute-mapping-user_id_ldap_mapping'] !== '') { + $userIdLdapMapping = $idpSettings['saml-attribute-mapping-user_id_ldap_mapping']; + /** @psalm-suppress UndefinedClass only in NC 34 or above */ + try { + if ($isActiveDirectory) { + $rawUidCandidate = $this->formatGuid2ForFilterUser($rawUidCandidate); + } + + if ($rawUidCandidate === '') { + throw new NoUserFoundException('User id is empty'); + } + + /** @psalm-suppress UndefinedInterfaceMethod only in NC 34 or above */ + $user = $this->ldapProviderFactory->getLDAPProvider()->findOneUserByAttributeValue($userIdLdapMapping, $rawUidCandidate); + } catch (MultipleUsersReturnedException $e) { + return ''; + } + if ($user !== null) { + return $user->getUID(); + } + + // continue normal workflow + } + if ($force) { if ($isActiveDirectory) { $this->ensureUser($this->formatGuid2ForFilterUser($rawUidCandidate)); @@ -40,7 +76,7 @@ public function findExistingUserId(string $rawUidCandidate, bool $force = false, if ($this->userManager->userExists($sanitized)) { return $sanitized; } - throw new NoUserFoundException('User' . $rawUidCandidate . ' not valid or not found'); + throw new NoUserFoundException('User ' . $rawUidCandidate . ' not valid or not found'); } /** @@ -86,14 +122,14 @@ public function findExistingUser(string $rawUidCandidate): IUser { $uid = $this->findExistingUserId($rawUidCandidate); $user = $this->userManager->get($uid); if ($user === null) { - throw new NoUserFoundException('User' . $rawUidCandidate . ' not valid or not found'); + throw new NoUserFoundException('User ' . $rawUidCandidate . ' not valid or not found.'); } return $user; } - public function userExists(string $uid, bool $force = false): bool { + public function userExists(string $uid, array $idpSettings, bool $force = false): bool { try { - $this->findExistingUserId($uid, $force); + $this->findExistingUserId($uid, $idpSettings, $force); return true; } catch (NoUserFoundException) { return false; diff --git a/tests/unit/Settings/AdminTest.php b/tests/unit/Settings/AdminTest.php index 3cd153f16..7de3f07a0 100644 --- a/tests/unit/Settings/AdminTest.php +++ b/tests/unit/Settings/AdminTest.php @@ -15,6 +15,7 @@ use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Services\IInitialState; use OCP\Defaults; +use OCP\IConfig; use OCP\IL10N; use OneLogin\Saml2\Constants; use Override; @@ -26,6 +27,7 @@ class AdminTest extends \Test\TestCase { private IL10N&MockObject $l10n; private Defaults&MockObject $defaults; private IAppConfig&MockObject $appConfig; + private IConfig&MockObject $config; private IInitialState&MockObject $initialState; #[Override] @@ -33,6 +35,7 @@ protected function setUp(): void { $this->l10n = $this->createMock(IL10N::class); $this->defaults = $this->createMock(Defaults::class); $this->appConfig = $this->createMock(IAppConfig::class); + $this->config = $this->createMock(IConfig::class); $this->settings = $this->createMock(SAMLSettings::class); $this->initialState = $this->createMock(IInitialState::class); @@ -40,6 +43,7 @@ protected function setUp(): void { $this->l10n, $this->defaults, $this->appConfig, + $this->config, $this->settings, $this->initialState, ); @@ -48,6 +52,9 @@ protected function setUp(): void { } public function formDataProvider(): array { + $this->config->method('getSystemValueString')->with('version', '0.0.0') + ->willReturn('34.0.0'); + $this->l10n ->expects($this->any()) ->method('t') @@ -161,6 +168,11 @@ public function formDataProvider(): array { 'type' => 'line', 'required' => false, ], + 'user_id_ldap_mapping' => [ + 'text' => $this->l10n->t('Attribute to map the users to an existing LDAP user'), + 'type' => 'line', + 'required' => false, + ], 'group_mapping_prefix' => [ 'text' => $this->l10n->t('Group Mapping Prefix, default: SAML_'), 'type' => 'line',