Skip to content

Commit be1d381

Browse files
committed
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 <carlschwan@kde.org>
1 parent 733e371 commit be1d381

6 files changed

Lines changed: 64 additions & 8 deletions

File tree

lib/Controller/SAMLController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,8 @@ public function assertionConsumerService(): Http\RedirectResponse {
392392
if ($firstLogin) {
393393
$this->userBackend->initializeHomeDir($user->getUID());
394394
}
395-
} catch (NoUserFoundException) {
396-
throw new \InvalidArgumentException('User "' . $this->userBackend->getCurrentUserId() . '" is not valid');
395+
} catch (NoUserFoundException $e) {
396+
throw new \InvalidArgumentException('User "' . $this->userBackend->getCurrentUserId() . '" is not valid.', previous: $e);
397397
} catch (Exception $e) {
398398
$this->logger->critical($e->getMessage(), ['exception' => $e, 'app' => $this->appName]);
399399
$response = new Http\RedirectResponse($this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.notProvisioned'));

lib/SAMLSettings.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ class SAMLSettings {
5959
'saml-attribute-mapping-home_mapping',
6060
'saml-attribute-mapping-quota_mapping',
6161
'saml-attribute-mapping-mfa_mapping',
62+
'saml-attribute-mapping-user_id_ldap_mapping',
6263
'saml-attribute-mapping-group_mapping_prefix',
6364
'saml-user-filter-reject_groups',
6465
'saml-user-filter-require_groups',

lib/Settings/Admin.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public function __construct(
2424
private readonly IL10N $l10n,
2525
private readonly Defaults $defaults,
2626
private readonly IAppConfig $appConfig,
27+
private readonly IConfig $config,
2728
private readonly SAMLSettings $samlSettings,
2829
private readonly IInitialState $initialState,
2930
) {
@@ -148,13 +149,22 @@ public function getForm(): TemplateResponse {
148149
'type' => 'line',
149150
'required' => false,
150151
],
152+
'user_id_ldap_mapping' => [
153+
'text' => $this->l10n->t('Attribute to map the users to an existing LDAP user'),
154+
'type' => 'line',
155+
'required' => false,
156+
],
151157
'group_mapping_prefix' => [
152158
'text' => $this->l10n->t('Group Mapping Prefix, default: %s', [SAMLSettings::DEFAULT_GROUP_PREFIX]),
153159
'type' => 'line',
154160
'required' => false,
155161
],
156162
];
157163

164+
if (version_compare($this->config->getSystemValueString('version', '0.0.0'), '34.0.0', '<')) {
165+
unset($attributeMappingSettings['user_id_ldap_mapping']);
166+
}
167+
158168
$userFilterSettings = [
159169
'reject_groups' => [
160170
'text' => $this->l10n->t('Reject members of these groups. This setting has precedence over required memberships.'),

lib/UserData.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function getEffectiveUid(): string {
4949
try {
5050
$providedUid = $this->extractSamlUserId();
5151
$uid = $this->testEncodedObjectGUID($providedUid);
52-
$uid = $this->userResolver->findExistingUserId($uid, true, $providedUid !== $uid);
52+
$uid = $this->userResolver->findExistingUserId($uid, $this->getProviderSettings(), true, $providedUid !== $uid);
5353
$this->uid = $uid;
5454
} catch (NoUserFoundException) {
5555
return '';

lib/UserResolver.php

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,52 @@
99
namespace OCA\User_SAML;
1010

1111
use OCA\User_SAML\Exceptions\NoUserFoundException;
12+
use OCP\IConfig;
1213
use OCP\IUser;
1314
use OCP\IUserManager;
15+
use OCP\LDAP\Exceptions\MultipleUsersReturnedException;
16+
use OCP\LDAP\ILDAPProviderFactory;
17+
use OCP\Server;
1418

1519
class UserResolver {
1620
public function __construct(
17-
private IUserManager $userManager,
21+
private readonly IUserManager $userManager,
22+
private readonly ILDAPProviderFactory $ldapProviderFactory,
23+
private readonly IConfig $config,
1824
) {
1925
}
2026

2127
/**
2228
* @throws NoUserFoundException
2329
*/
24-
public function findExistingUserId(string $rawUidCandidate, bool $force = false, bool $isActiveDirectory = false): string {
30+
public function findExistingUserId(string $rawUidCandidate, ?array $idpSettings = null, bool $force = false, bool $isActiveDirectory = false): string {
31+
// If configured, find the user based on a different LDAP attribute.
32+
if ($idpSettings !== null
33+
&& version_compare($this->config->getSystemValueString('version', '0.0.0'), '34.0.0', '>=')
34+
&& $this->ldapProviderFactory->isAvailable()
35+
&& isset($idpSettings['saml-attribute-mapping-user_id_ldap_mapping'])
36+
&& $idpSettings['saml-attribute-mapping-user_id_ldap_mapping'] !== null
37+
&& $idpSettings['saml-attribute-mapping-user_id_ldap_mapping'] !== '') {
38+
$userIdLdapMapping = $idpSettings['saml-attribute-mapping-user_id_ldap_mapping'];
39+
try {
40+
if ($isActiveDirectory) {
41+
/** @psalm-suppress UndefinedInterfaceMethod only in NC 34 or above */
42+
$user = $this->ldapProviderFactory->getLDAPProvider()->findOneUserByAttributeValue($userIdLdapMapping, $this->formatGuid2ForFilterUser($rawUidCandidate));
43+
} else {
44+
/** @psalm-suppress UndefinedInterfaceMethod only in NC 34 or above */
45+
$user = $this->ldapProviderFactory->getLDAPProvider()->findOneUserByAttributeValue($userIdLdapMapping, $rawUidCandidate);
46+
}
47+
/** @psalm-suppress UndefinedClass only in NC 34 or above */
48+
} catch (MultipleUsersReturnedException $e) {
49+
return '';
50+
}
51+
if ($user !== null) {
52+
return $user->getUID();
53+
}
54+
55+
// continue normal workflow
56+
}
57+
2558
if ($force) {
2659
if ($isActiveDirectory) {
2760
$this->ensureUser($this->formatGuid2ForFilterUser($rawUidCandidate));
@@ -86,14 +119,14 @@ public function findExistingUser(string $rawUidCandidate): IUser {
86119
$uid = $this->findExistingUserId($rawUidCandidate);
87120
$user = $this->userManager->get($uid);
88121
if ($user === null) {
89-
throw new NoUserFoundException('User' . $rawUidCandidate . ' not valid or not found');
122+
throw new NoUserFoundException('User ' . $rawUidCandidate . ' not valid or not found.');
90123
}
91124
return $user;
92125
}
93126

94-
public function userExists(string $uid, bool $force = false): bool {
127+
public function userExists(string $uid, array $idpSettings, bool $force = false): bool {
95128
try {
96-
$this->findExistingUserId($uid, $force);
129+
$this->findExistingUserId($uid, $idpSettings, $force);
97130
return true;
98131
} catch (NoUserFoundException) {
99132
return false;

tests/unit/Settings/AdminTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCP\AppFramework\Services\IAppConfig;
1616
use OCP\AppFramework\Services\IInitialState;
1717
use OCP\Defaults;
18+
use OCP\IConfig;
1819
use OCP\IL10N;
1920
use OneLogin\Saml2\Constants;
2021
use Override;
@@ -26,20 +27,23 @@ class AdminTest extends \Test\TestCase {
2627
private IL10N&MockObject $l10n;
2728
private Defaults&MockObject $defaults;
2829
private IAppConfig&MockObject $appConfig;
30+
private IConfig&MockObject $config;
2931
private IInitialState&MockObject $initialState;
3032

3133
#[Override]
3234
protected function setUp(): void {
3335
$this->l10n = $this->createMock(IL10N::class);
3436
$this->defaults = $this->createMock(Defaults::class);
3537
$this->appConfig = $this->createMock(IAppConfig::class);
38+
$this->config = $this->createMock(IConfig::class);
3639
$this->settings = $this->createMock(SAMLSettings::class);
3740
$this->initialState = $this->createMock(IInitialState::class);
3841

3942
$this->admin = new Admin(
4043
$this->l10n,
4144
$this->defaults,
4245
$this->appConfig,
46+
$this->config,
4347
$this->settings,
4448
$this->initialState,
4549
);
@@ -48,6 +52,9 @@ protected function setUp(): void {
4852
}
4953

5054
public function formDataProvider(): array {
55+
$this->config->method('getSystemValueString')->with('version', '0.0.0')
56+
->willReturn('34.0.0');
57+
5158
$this->l10n
5259
->expects($this->any())
5360
->method('t')
@@ -161,6 +168,11 @@ public function formDataProvider(): array {
161168
'type' => 'line',
162169
'required' => false,
163170
],
171+
'user_id_ldap_mapping' => [
172+
'text' => $this->l10n->t('Attribute to map the users to an existing LDAP user'),
173+
'type' => 'line',
174+
'required' => false,
175+
],
164176
'group_mapping_prefix' => [
165177
'text' => $this->l10n->t('Group Mapping Prefix, default: SAML_'),
166178
'type' => 'line',

0 commit comments

Comments
 (0)