Skip to content

Commit 01ec51d

Browse files
committed
fix(LDAP): lookup of AD users through objectGUID
when the objectguid is used in the search, it has to be encoded in a specific way. LDAP does it already, their implemengtation (\OCA\User_LDAP\Access::formatGuid2ForFilterUser) is taken over for now and the generated value passed to search. Signed-off-by: Arthur Schiwon <[email protected]>
1 parent 6c08514 commit 01ec51d

File tree

2 files changed

+45
-5
lines changed

2 files changed

+45
-5
lines changed

lib/UserData.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ public function getEffectiveUid(): string {
4848
}
4949
$this->assertIsInitialized();
5050
try {
51-
$uid = $this->extractSamlUserId();
52-
$uid = $this->testEncodedObjectGUID($uid);
53-
$uid = $this->userResolver->findExistingUserId($uid, true);
51+
$providedUid = $this->extractSamlUserId();
52+
$uid = $this->testEncodedObjectGUID($providedUid);
53+
$uid = $this->userResolver->findExistingUserId($uid, true, $providedUid !== $uid);
5454
$this->uid = $uid;
5555
} catch (NoUserFoundException) {
5656
return '';

lib/UserResolver.php

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ public function __construct(IUserManager $userManager) {
2323
/**
2424
* @throws NoUserFoundException
2525
*/
26-
public function findExistingUserId(string $rawUidCandidate, bool $force = false): string {
26+
public function findExistingUserId(string $rawUidCandidate, bool $force = false, bool $isActiveDirectory = false): string {
2727
if ($force) {
28-
$this->ensureUser($rawUidCandidate);
28+
if ($isActiveDirectory) {
29+
$this->ensureUser($this->formatGuid2ForFilterUser($rawUidCandidate));
30+
} else {
31+
$this->ensureUser($rawUidCandidate);
32+
}
2933
}
3034
if ($this->userManager->userExists($rawUidCandidate)) {
3135
return $rawUidCandidate;
@@ -41,6 +45,42 @@ public function findExistingUserId(string $rawUidCandidate, bool $force = false)
4145
throw new NoUserFoundException('User' . $rawUidCandidate . ' not valid or not found');
4246
}
4347

48+
/**
49+
* @see \OCA\User_LDAP\Access::formatGuid2ForFilterUser
50+
*/
51+
private function formatGuid2ForFilterUser(string $guid): string {
52+
$blocks = explode('-', $guid);
53+
if (count($blocks) !== 5) {
54+
/*
55+
* Why not throw an Exception instead? This method is a utility
56+
* called only when trying to figure out whether a "missing" known
57+
* LDAP user was or was not renamed on the LDAP server. And this
58+
* even on the use case that a reverse lookup is needed (UUID known,
59+
* not DN), i.e. when finding users (search dialog, users page,
60+
* login, …) this will not be fired. This occurs only if shares from
61+
* a users are supposed to be mounted who cannot be found. Throwing
62+
* an exception here would kill the experience for a valid, acting
63+
* user. Instead we write a log message.
64+
*/
65+
$this->logger->info(
66+
'Passed string does not resemble a valid GUID. Known UUID '
67+
. '({uuid}) probably does not match UUID configuration.',
68+
['app' => 'user_saml', 'uuid' => $guid]
69+
);
70+
return $guid;
71+
}
72+
for ($i = 0; $i < 3; $i++) {
73+
$pairs = str_split($blocks[$i], 2);
74+
$pairs = array_reverse($pairs);
75+
$blocks[$i] = implode('', $pairs);
76+
}
77+
for ($i = 0; $i < 5; $i++) {
78+
$pairs = str_split($blocks[$i], 2);
79+
$blocks[$i] = '\\' . implode('\\', $pairs);
80+
}
81+
return implode('', $blocks);
82+
}
83+
4484
/**
4585
* @throws NoUserFoundException
4686
*/

0 commit comments

Comments
 (0)