From 325421544dca31a2e9c331c56e69bcb634e19878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Mon, 31 Oct 2022 15:09:12 +0100 Subject: [PATCH 1/3] Refactor binary attributes and add support for objectsid --- lib/Access.php | 125 ++--------------------- lib/Attributes/ConverterException.php | 23 +++++ lib/Attributes/ConverterHub.php | 138 ++++++++++++++++++++++++++ lib/Attributes/GUIDConverter.php | 108 ++++++++++++++++++++ lib/Attributes/IConverter.php | 49 +++++++++ lib/Attributes/SIDConverter.php | 79 +++++++++++++++ lib/User/UserEntry.php | 7 +- 7 files changed, 410 insertions(+), 119 deletions(-) create mode 100644 lib/Attributes/ConverterException.php create mode 100644 lib/Attributes/ConverterHub.php create mode 100644 lib/Attributes/GUIDConverter.php create mode 100644 lib/Attributes/IConverter.php create mode 100644 lib/Attributes/SIDConverter.php diff --git a/lib/Access.php b/lib/Access.php index fdb415ec..35f46caa 100644 --- a/lib/Access.php +++ b/lib/Access.php @@ -41,6 +41,7 @@ use OCA\User_LDAP\User\IUserTools; use OCA\User_LDAP\User\Manager; use OCA\User_LDAP\Mapping\AbstractMapping; +use OCA\User_LDAP\Attributes\ConverterHub; use OCP\Util; /** @@ -327,11 +328,12 @@ public function extractAttributeValuesFromResult($result, $attribute) { $values = []; if (isset($result[$attribute]) && $result[$attribute]['count'] > 0) { $lowercaseAttribute = \strtolower($attribute); + $converterHub = ConverterHub::getDefaultConverterHub(); for ($i=0; $i<$result[$attribute]['count']; $i++) { if ($this->resemblesDN($attribute)) { $values[] = Helper::normalizeDN($result[$attribute][$i]); - } elseif ($lowercaseAttribute === 'objectguid' || $lowercaseAttribute === 'guid') { - $values[] = self::binGUID2str($result[$attribute][$i]); + } elseif ($converterHub->hasConverter($lowercaseAttribute)) { + $values[] = $converterHub->bin2str($lowercaseAttribute, $result[$attribute][$i]); } else { $values[] = $result[$attribute][$i]; } @@ -1727,8 +1729,10 @@ public function getUserDnByUuid($uuid) { } $uuidAttr = $this->connection->ldapUuidUserAttribute; - if ($uuidAttr === 'guid' || $uuidAttr === 'objectguid') { - $uuid = $this->formatGuid2ForFilterUser($uuid); + $lowercaseUuidAttr = \strtolower($uuidAttr); + $converterHub = ConverterHub::getDefaultConverterHub(); + if ($converterHub->hasConverter($lowercaseUuidAttr)) { + $uuid = $converterHub->str2filter($uuid); } $filter = $uuidAttr . '=' . $uuid; @@ -1828,119 +1832,6 @@ public function getUUID($dn, $isUser = true) { return $uuid; } - /** - * converts a binary GUID into a string representation - * - * TODO use shorter version with pack() - * - * General UUID information: @see http://ldapwiki.com/wiki/Universally%20Unique%20Identifier - * - * ## openldap EntryUUID uses RFC4122 see {@link http://ldapwiki.com/wiki/UUID definition} - * see the {@link http://ldapwiki.com/wiki/EntryUUID ldapwiki EntryUUID definition} - * - * ## Microsoft Active Directory objectGUID is defined as 16 byte octet string - * {@link https://msdn.microsoft.com/en-us/library/ms679021(v=vs.85).aspx official objectGUID definition} - * From the {@link http://ldapwiki.com/wiki/ObjectGUID ldapwiki ObjectGUID definition}: - * ObjectGUID is generally a Universally Unique Identifier other than the - * format differs from the UUID standard only in the byte order of the first 3 fields. - * {@link http://support.microsoft.com/default.aspx?scid=kb%3Ben-us%3B325649 conversion to a string} - * - * ## Novell eDirectory GUID is defined as 16 byte octet string - * From the {@link http://ldapwiki.com/wiki/GUID ldapwiki GUID definition}: - * There are several different methods that are used to display any given GUID - * {@link http://www.novell.com/documentation/developer/ndslib/schm_enu/data/sdk1198.html official GUID definition} - * - * ## 389 Directory Server / Oracle Directory Server Enterprise Edition (ODSEE) is defined as utf string - * {@link https://github.com/leto/389-ds/blob/master/ldap/schema/01core389.ldif#L69 schema definition} - * {@link https://docs.oracle.com/cd/E49437_01/reference.111220/e27801/nsuniqueid-virtual-attribute.html official nsuniqueid definition} - * The nsuniqueid values are generated based on the entryuuid value by moving the "-" to comply with the format of the ODSEE Nsuniqueid Virtual Attribute attribute. - * - * ## RedHat FreeIPA is defined as utf string - * {@link https://github.com/freeipa/freeipa/blob/master/install/share/uuid.ldif ipaUniqueID schema} - * - * This implementation was taken from - * {@link http://www.php.net/manual/en/function.ldap-get-values-len.php#73198 The PHP ldap_get_values_lan doc comments} - * - * @param string $binGuid the ObjectGUID / GUID in it's binary form as retrieved from Microsoft AD / Novell eDirectory - * If you pass an already decoded GUID as string, it will be returned as is. - * @return string - * @throws \OutOfBoundsException - */ - public static function binGUID2str($binGuid) { - $guidLength = \strlen($binGuid); - - // The guid should have 16 byte when binary and 36 byte when string (including '-' characters) - if (($guidLength !== 16) && ($guidLength !== 36)) { - throw new \OutOfBoundsException(\sprintf('Invalid GUID with length %d received: <%X>', $guidLength, $binGuid)); - } - - // If we get a guid in string form we simply return it to prevent double decoding - if ($guidLength === 36) { - return $binGuid; - } - - // V = unsigned long (always 32 bit, little endian byte order) - // v = unsigned short (always 16 bit, little endian byte order) - // n = unsigned short (always 16 bit, big endian byte order) - // N = unsigned long (always 32 bit, big endian byte order) - // TODO treat all warnings es error? see https://stackoverflow.com/a/2071048 - $unpacked = \unpack('Va/v2b/n2c/Nd', $binGuid); // only throws a warning if it could not parse the input - $uuid = \sprintf('%08X-%04X-%04X-%04X-%04X%08X', $unpacked['a'], $unpacked['b1'], $unpacked['b2'], $unpacked['c1'], $unpacked['c2'], $unpacked['d']); - // make sure this is not a bogus UUID - if ($uuid === '00000000-0000-0000-0000-000000000000') { - throw new \OutOfBoundsException(\sprintf('Invalid binary uuid <%X>', $binGuid)); - } - return $uuid; - } - - /** - * the first three blocks of the string-converted GUID happen to be in - * reverse order. In order to use it in a filter, this needs to be - * corrected. Furthermore the dashes need to be replaced and \\ preprended - * to every two hax figures. - * - * If an invalid string is passed, it will be returned without change. - * - * @param string $guid - * @return string - * @throws \InvalidArgumentException - */ - public function formatGuid2ForFilterUser($guid) { - if (!\is_string($guid)) { - throw new \InvalidArgumentException('String expected'); - } - $blocks = \explode('-', $guid); - if (\count($blocks) !== 5) { - /* - * Why not throw an Exception instead? This method is a utility - * called only when trying to figure out whether a "missing" known - * LDAP user was or was not renamed on the LDAP server. And this - * even on the use case that a reverse lookup is needed (UUID known, - * not DN), i.e. when finding users (search dialog, users page, - * login, …) this will not be fired. This occurs only if shares from - * a users are supposed to be mounted who cannot be found. Throwing - * an exception here would kill the experience for a valid, acting - * user. Instead we write a log message. - */ - \OC::$server->getLogger()->info( - 'Passed string does not resemble a valid GUID. Known UUID ' . - '({uuid}) probably does not match UUID configuration.', - [ 'app' => 'user_ldap', 'uuid' => $guid ] - ); - return $guid; - } - for ($i=0; $i < 3; $i++) { - $pairs = \str_split($blocks[$i], 2); - $pairs = \array_reverse($pairs); - $blocks[$i] = \implode('', $pairs); - } - for ($i=0; $i < 5; $i++) { - $pairs = \str_split($blocks[$i], 2); - $blocks[$i] = '\\' . \implode('\\', $pairs); - } - return \implode('', $blocks); - } - /** * gets a SID of the domain of the given dn * diff --git a/lib/Attributes/ConverterException.php b/lib/Attributes/ConverterException.php new file mode 100644 index 00000000..601aaf66 --- /dev/null +++ b/lib/Attributes/ConverterException.php @@ -0,0 +1,23 @@ + + * + */ + +namespace OCA\User_LDAP\Attributes; + +class ConverterException extends \Exception { +} diff --git a/lib/Attributes/ConverterHub.php b/lib/Attributes/ConverterHub.php new file mode 100644 index 00000000..7fd71b68 --- /dev/null +++ b/lib/Attributes/ConverterHub.php @@ -0,0 +1,138 @@ + + * + */ + +namespace OCA\User_LDAP\Attributes; + +class ConverterHub { + /** @var ConverterHub */ + private static $defaultConverterHub; + + /** @var array */ + private $attrConverterMap = []; + + /** + * Get the default converted hub. If no hub has been created yet, + * or the $fresh parameter is true, a new ConverterHub will be + * created and set as default, the `registerDefaultConverters` + * method will be called on it, and finally it will be returned. + * @param bool $fresh whether a new instance should be created and set + * as default + * @return ConverterHub + */ + public static function getDefaultConverterHub($fresh = false) { + if (self::$defaultConverterHub === null || $fresh) { + $converterHub = new ConverterHub(); + $converterHub->registerDefaultConverters(); + self::$defaultConverterHub = $converterHub; + } + return self::$defaultConverterHub; + } + + /** + * Register a new IConverter instance for the chosen attribute. + * This will overwrite any previous IConverter register for that attribute + * @param string $attr the attribute that will be converted. The attribute + * should be lowercase + * @param IConverter $converter the converter to be used for that attribute + */ + public function registerConverter(string $attr, IConverter $converter) { + $this->attrConverterMap[$attr] = $converter; + } + + /** + * Remove all registered converters in this instance + */ + public function clearConverters() { + $this->attrConverterMap = []; + } + + /** + * Register the default converters. This method won't remove any existing + * converter, but it will overwrite them if they overlap + */ + public function registerDefaultConverters() { + $this->attrConverterMap['objectguid'] = new GUIDConverter(); + $this->attrConverterMap['guid'] = new GUIDConverter(); + $this->attrConverterMap['objectsid'] = new SIDConverter(); + } + + /** + * Check if this instance has a converter set for the target attribute + * @param string $attr the attribute to check + * @return bool true if there is a converter for the attribute, false otherwise + */ + public function hasConverter(string $attr): bool { + return isset($this->attrConverterMap[$attr]); + } + + /** + * Convert the binary value to its string representation using the + * converter registered for the target attribute. + * If there is no converter registered for the target attribute, a + * ConverterException will be thrown. + * Additional ConverterException might be thrown from the converters + * if there is a problem dealing with the conversion. + * @param string $attr the attribute name for the value + * @param string $binValue the binary value for the associated attribute + * @return string the string representation of the binary value + * accordingly to the associated converter + * @throws ConverterException if no converter is registered, or the + * associated converter can't deal with the input. + */ + public function bin2str(string $attr, string $binValue) { + if (!isset($this->attrConverterMap[$attr])) { + throw new ConverterException("No converter found for attr {$attr}"); + } + + $converter = $this->attrConverterMap[$attr]; + return $converter->bin2str($binValue); + } + + /** + * Convert the string representation of the attribute into a string + * suitable to be used in a LDAP filter. + * The string representation should come from the `bin2str` method + * of this instance (it might have been stored anywhere). + * Since the filter will likely contain binary data, the converters + * are expected to escape the result accordinly, so results such as + * '\11\f8\30\73\7f\f4\bc\41\a4\ff\e7\92\d0\73\f4\1f' are expected + * to be returned. + * The filter is expected to be built like + * ``` + * $strRepr = 'S-1-5-21-12032599-1214884855-3286145327-1103'; + * $filter = "objectsid=" . $hub->str2filter('objectsid', $strRepr); + * ``` + * A ConverterException will be thrown if there is no converter + * associated to the target attribute + * @param string $attr the attribute associated to the string representation + * @param string $strValue the string representation of the value of + * the attribute + * @param string a string suitable to be used as filter + * @throws ConverterException if no converter is registered, or the + * associated converter can't deal with the input. + */ + public function str2filter(string $attr, string $strValue) { + if (!isset($this->attrConverterMap[$attr])) { + throw new ConverterException("No converter found for attr {$attr}"); + } + + $converter = $this->attrConverterMap[$attr]; + return $converter->str2filter($strValue); + } +} diff --git a/lib/Attributes/GUIDConverter.php b/lib/Attributes/GUIDConverter.php new file mode 100644 index 00000000..9916825b --- /dev/null +++ b/lib/Attributes/GUIDConverter.php @@ -0,0 +1,108 @@ + + * + */ + +namespace OCA\User_LDAP\Attributes; + +class GUIDConverter implements IConverter { + /** + * @inheritdoc + * converts a binary GUID into a string representation + * + * TODO use shorter version with pack() + * + * General UUID information: @see http://ldapwiki.com/wiki/Universally%20Unique%20Identifier + * + * ## openldap EntryUUID uses RFC4122 see {@link http://ldapwiki.com/wiki/UUID definition} + * see the {@link http://ldapwiki.com/wiki/EntryUUID ldapwiki EntryUUID definition} + * + * ## Microsoft Active Directory objectGUID is defined as 16 byte octet string + * {@link https://msdn.microsoft.com/en-us/library/ms679021(v=vs.85).aspx official objectGUID definition} + * From the {@link http://ldapwiki.com/wiki/ObjectGUID ldapwiki ObjectGUID definition}: + * ObjectGUID is generally a Universally Unique Identifier other than the + * format differs from the UUID standard only in the byte order of the first 3 fields. + * {@link http://support.microsoft.com/default.aspx?scid=kb%3Ben-us%3B325649 conversion to a string} + * + * ## Novell eDirectory GUID is defined as 16 byte octet string + * From the {@link http://ldapwiki.com/wiki/GUID ldapwiki GUID definition}: + * There are several different methods that are used to display any given GUID + * {@link http://www.novell.com/documentation/developer/ndslib/schm_enu/data/sdk1198.html official GUID definition} + * + * ## 389 Directory Server / Oracle Directory Server Enterprise Edition (ODSEE) is defined as utf string + * {@link https://github.com/leto/389-ds/blob/master/ldap/schema/01core389.ldif#L69 schema definition} + * {@link https://docs.oracle.com/cd/E49437_01/reference.111220/e27801/nsuniqueid-virtual-attribute.html official nsuniqueid definition} + * The nsuniqueid values are generated based on the entryuuid value by moving the "-" to comply with the format of the ODSEE Nsuniqueid Virtual Attribute attribute. + * + * ## RedHat FreeIPA is defined as utf string + * {@link https://github.com/freeipa/freeipa/blob/master/install/share/uuid.ldif ipaUniqueID schema} + * + * This implementation was taken from + * {@link http://www.php.net/manual/en/function.ldap-get-values-len.php#73198 The PHP ldap_get_values_lan doc comments} + * + * @param string $binGuid the ObjectGUID / GUID in it's binary form as retrieved from Microsoft AD / Novell eDirectory + * If you pass an already decoded GUID as string, it will be returned as is. + * @return string + * @throws \OutOfBoundsException + */ + public function bin2str(string $binGuid): string { + $guidLength = \strlen($binGuid); + + // The guid should have 16 byte when binary and 36 byte when string (including '-' characters) + if (($guidLength !== 16) && ($guidLength !== 36)) { + throw new ConverterException(\sprintf('Invalid GUID with length %d received: <%X>', $guidLength, $binGuid)); + } + + // If we get a guid in string form we simply return it to prevent double decoding + if ($guidLength === 36) { + return $binGuid; + } + + // V = unsigned long (always 32 bit, little endian byte order) + // v = unsigned short (always 16 bit, little endian byte order) + // n = unsigned short (always 16 bit, big endian byte order) + // N = unsigned long (always 32 bit, big endian byte order) + // TODO treat all warnings es error? see https://stackoverflow.com/a/2071048 + $unpacked = \unpack('Va/v2b/n2c/Nd', $binGuid); // only throws a warning if it could not parse the input + $uuid = \sprintf('%08X-%04X-%04X-%04X-%04X%08X', $unpacked['a'], $unpacked['b1'], $unpacked['b2'], $unpacked['c1'], $unpacked['c2'], $unpacked['d']); + // make sure this is not a bogus UUID + if ($uuid === '00000000-0000-0000-0000-000000000000') { + throw new ConverterException(\sprintf('Invalid binary uuid <%X>', $binGuid)); + } + return $uuid; + } + + /** + * @inheritdoc + */ + public function str2filter(string $guid): string { + $blocks = \explode('-', $guid); + if (\count($blocks) !== 5) { + throw new ConverterException("{$guid} doesn't resemble a valid GUID"); + } + for ($i=0; $i < 3; $i++) { + $pairs = \str_split($blocks[$i], 2); + $pairs = \array_reverse($pairs); + $blocks[$i] = \implode('', $pairs); + } + for ($i=0; $i < 5; $i++) { + $pairs = \str_split($blocks[$i], 2); + $blocks[$i] = '\\' . \implode('\\', $pairs); + } + return \implode('', $blocks); + } +} diff --git a/lib/Attributes/IConverter.php b/lib/Attributes/IConverter.php new file mode 100644 index 00000000..7586d8f1 --- /dev/null +++ b/lib/Attributes/IConverter.php @@ -0,0 +1,49 @@ + + * + */ + +namespace OCA\User_LDAP\Attributes; + +interface IConverter { + /** + * Convert an binary attribute into an string representation. + * The basic string representation could be the binary attribute + * encoded in base64. + * Some attributes might be expected in some specific format, so + * the string representation should match that expectation in order + * to make it easier for the user to recognize and match the result + * with other tools. + * @param string $binAttr the binary attribute coming from LDAP + * @return string the string representation for the binary attribute + * @throws ConverterException if an error happens + */ + public function bin2str(string $binAttr): string; + /** + * Transform the string representation of a binary attribute (usually + * coming from the bin2str method) into a string suitable to be used + * in a ldap filter. + * This result could be something like "\11\f8\30\73\7f\f4\bc\41\a4\ff\e7\92\d0\73\f4\1f", + * so the ldap filter could be build like + * "binattr=\11\f8\30\73\7f\f4\bc\41\a4\ff\e7\92\d0\73\f4\1f" + * @param string $strRepr the string representation of the binary attribute + * usually coming from the bin2str method + * @return string a string suitable to be used as LDAP filter + * @throws ConverterException if an error happens + */ + public function str2filter(string $strRepr): string; +} diff --git a/lib/Attributes/SIDConverter.php b/lib/Attributes/SIDConverter.php new file mode 100644 index 00000000..55f18428 --- /dev/null +++ b/lib/Attributes/SIDConverter.php @@ -0,0 +1,79 @@ + + * + */ + +namespace OCA\User_LDAP\Attributes; + +/** + * Handle the objectsid attribute + */ +class SIDConverter implements IConverter { + /** + * @inheritdoc + * Implementation copied from https://stackoverflow.com/a/13143132 + */ + public function bin2str(string $binAttr): string { + $hex_sid = \bin2hex($binAttr); + $rev = \hexdec(\substr($hex_sid, 0, 2)); + $subcount = \hexdec(\substr($hex_sid, 2, 2)); + $auth = \hexdec(\substr($hex_sid, 4, 12)); + $result = "{$rev}-{$auth}"; + + for ($x=0;$x < $subcount; $x++) { + $result .= '-' . \hexdec($this->little_endian(\substr($hex_sid, 16 + ($x * 8), 8))); + } + + // Cheat by tacking on the S- + return 'S-' . $result; + } + + private function little_endian($hex) { + $result = ""; + + for ($x = \strlen($hex) - 2; $x >= 0; $x = $x - 2) { + $result .= \substr($hex, $x, 2); + } + return $result; + } + + /** + * @inheritdoc + * Information extracted from https://devblogs.microsoft.com/oldnewthing/20040315-00/?p=40253 + */ + public function str2filter(string $strRepr): string { + $blocks = \explode('-', $strRepr); + $blockCount = \count($blocks); + $dashesCountMinus2 = \strval(\intval($blockCount) - 3); + // skip the first block because it's expected to be 'S-' and it won't be included + // 2 bytes for the revision + // 2 bytes for the number of dashes minus 2 + // 6 bytes big-endian for the authority + $hexData = \sprintf("%02X%02X%012X", $blocks[1], $dashesCountMinus2, $blocks[2]); + for ($i = 3; $i < $blockCount; $i++) { + // for the rest of the blocks, 4 bytes little-endian + $hexData .= $this->little_endian(\sprintf("%08X", $blocks[$i])); + } + + // escape each byte + $hexBytes = \str_split($hexData, 2); + $escapedHexBytes = \array_map(function ($byte) { + return "\\{$byte}"; + }, $hexBytes); + return \implode('', $escapedHexBytes); + } +} diff --git a/lib/User/UserEntry.php b/lib/User/UserEntry.php index 1bcbdeb7..636c755b 100644 --- a/lib/User/UserEntry.php +++ b/lib/User/UserEntry.php @@ -23,6 +23,7 @@ use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; +use OCA\User_LDAP\Attributes\ConverterHub; use OCP\IConfig; use OCP\ILogger; @@ -153,6 +154,7 @@ public function getUUID() { $uuidAttributes = $this->connection->uuidAttributes; } foreach ($uuidAttributes as $uuidAttribute) { + $lowercaseUuidAttribute = \strtolower($uuidAttribute); // uuid may be binary ... must not be trimmed! $uuid = $this->getAttributeValue($uuidAttribute, null, false); if ($uuid === null) { @@ -163,8 +165,9 @@ public function getUUID() { $this->connection->ldapExpertUUIDUserAttr = $uuidAttribute; $this->connection->saveConfiguration(); // FIXME should not be done here. Move to wizard? } - if ($uuidAttribute === 'objectguid' || $uuidAttribute === 'guid') { - $uuid = Access::binGUID2str($uuid); + $converterHub = ConverterHub::getDefaultConverterHub(); + if ($converterHub->hasConverter($lowercaseUuidAttribute)) { + $uuid = $converterHub->bin2str($lowercaseUuidAttribute, $uuid); } return $uuid; From 7712d7020fc86f0ecd31867f3d61297946e75f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 2 Nov 2022 11:15:51 +0100 Subject: [PATCH 2/3] Fix issues with tests and add new ones --- lib/User/UserEntry.php | 10 +- tests/unit/Attributes/ConverterHubTest.php | 138 ++++++++++++++++++++ tests/unit/Attributes/GUIDConverterTest.php | 106 +++++++++++++++ tests/unit/Attributes/SIDConverterTest.php | 68 ++++++++++ tests/unit/User/UserEntryTest.php | 3 +- 5 files changed, 317 insertions(+), 8 deletions(-) create mode 100644 tests/unit/Attributes/ConverterHubTest.php create mode 100644 tests/unit/Attributes/GUIDConverterTest.php create mode 100644 tests/unit/Attributes/SIDConverterTest.php diff --git a/lib/User/UserEntry.php b/lib/User/UserEntry.php index 636c755b..8698996c 100644 --- a/lib/User/UserEntry.php +++ b/lib/User/UserEntry.php @@ -154,7 +154,6 @@ public function getUUID() { $uuidAttributes = $this->connection->uuidAttributes; } foreach ($uuidAttributes as $uuidAttribute) { - $lowercaseUuidAttribute = \strtolower($uuidAttribute); // uuid may be binary ... must not be trimmed! $uuid = $this->getAttributeValue($uuidAttribute, null, false); if ($uuid === null) { @@ -165,10 +164,6 @@ public function getUUID() { $this->connection->ldapExpertUUIDUserAttr = $uuidAttribute; $this->connection->saveConfiguration(); // FIXME should not be done here. Move to wizard? } - $converterHub = ConverterHub::getDefaultConverterHub(); - if ($converterHub->hasConverter($lowercaseUuidAttribute)) { - $uuid = $converterHub->bin2str($lowercaseUuidAttribute, $uuid); - } return $uuid; } @@ -369,8 +364,9 @@ private function getAttributeValue($attributeName, $default = null, $trim = true $value = \trim($value); } - if ($attributeName === 'objectguid' || $attributeName === 'guid') { - $value = Access::binGUID2str($value); + $converterHub = ConverterHub::getDefaultConverterHub(); + if ($converterHub->hasConverter($attributeName)) { + $value = $converterHub->bin2str($attributeName, $value); } if ($value === '') { diff --git a/tests/unit/Attributes/ConverterHubTest.php b/tests/unit/Attributes/ConverterHubTest.php new file mode 100644 index 00000000..f6c907db --- /dev/null +++ b/tests/unit/Attributes/ConverterHubTest.php @@ -0,0 +1,138 @@ + + * + */ + +namespace OCA\User_LDAP\Tests\Attributes; + +use OCA\User_LDAP\Attributes\ConverterHub; +use OCA\User_LDAP\Attributes\IConverter; +use OCA\User_LDAP\Attributes\ConverterException; + +class ConverterHubTest extends \Test\TestCase { + /** @var ConverterHub */ + private $converterHub; + + protected function setUp(): void { + parent::setUp(); + $this->converterHub = new ConverterHub(); + } + + public function testHasConverterFail() { + $this->assertFalse($this->converterHub->hasConverter('aCustomMissingAttr')); + } + + public function testRegisterAndHasConverter() { + $converter = $this->createMock(IConverter::class); + $this->assertFalse($this->converterHub->hasConverter('customAttr')); + $this->converterHub->registerConverter('customAttr', $converter); + $this->assertTrue($this->converterHub->hasConverter('customAttr')); + } + + public function testRegisterDefaultConverters() { + $this->converterHub->registerDefaultConverters(); + $this->assertTrue($this->converterHub->hasConverter('objectguid')); + $this->assertTrue($this->converterHub->hasConverter('guid')); + $this->assertTrue($this->converterHub->hasConverter('objectsid')); + } + + public function testClearConverters() { + $converter = $this->createMock(IConverter::class); + $converter2 = $this->createMock(IConverter::class); + $this->converterHub->registerConverter('customAttr', $converter); + $this->converterHub->registerConverter('customAttr2', $converter2); + $this->assertTrue($this->converterHub->hasConverter('customAttr')); + $this->assertTrue($this->converterHub->hasConverter('customAttr2')); + $this->converterHub->clearConverters(); + $this->assertFalse($this->converterHub->hasConverter('customAttr')); + $this->assertFalse($this->converterHub->hasConverter('customAttr2')); + } + + public function testBin2strException() { + $this->expectException(ConverterException::class); + $this->converterHub->bin2str('missingAttr', 'binary value'); + } + + public function testBin2strException2() { + $this->expectException(ConverterException::class); + + $testString = 'binary string'; + + $converter = $this->createMock(IConverter::class); + $converter->expects($this->once()) + ->method('bin2str') + ->with($testString) + ->will($this->returnCallback(static function ($value) { + throw new ConverterException('Something happended in the converter'); + })); + + $this->converterHub->registerConverter('myAttr', $converter); + $this->converterHub->bin2str('myAttr', $testString); + } + + public function testBin2str() { + $testString = 'binary string'; + + $converter = $this->createMock(IConverter::class); + $converter->expects($this->once()) + ->method('bin2str') + ->with($testString) + ->will($this->returnCallback(static function ($value) { + return \base64_encode($value); + })); + + $this->converterHub->registerConverter('myAttr', $converter); + $this->assertSame(\base64_encode($testString), $this->converterHub->bin2str('myAttr', $testString)); + } + + public function testStr2filterException() { + $this->expectException(ConverterException::class); + $this->converterHub->str2filter('missingAttr', 'binary value'); + } + + public function testStr2filterException2() { + $this->expectException(ConverterException::class); + + $testString = 'binary string'; + + $converter = $this->createMock(IConverter::class); + $converter->expects($this->once()) + ->method('str2filter') + ->with($testString) + ->will($this->returnCallback(static function ($value) { + throw new ConverterException('Something happended in the converter'); + })); + + $this->converterHub->registerConverter('myAttr', $converter); + $this->converterHub->str2filter('myAttr', $testString); + } + + public function testStr2filter() { + $testString = 'binary string'; + + $converter = $this->createMock(IConverter::class); + $converter->expects($this->once()) + ->method('str2filter') + ->with($testString) + ->will($this->returnCallback(static function ($value) { + return \strrev($value); + })); + + $this->converterHub->registerConverter('myAttr', $converter); + $this->assertSame(\strrev($testString), $this->converterHub->str2filter('myAttr', $testString)); + } +} diff --git a/tests/unit/Attributes/GUIDConverterTest.php b/tests/unit/Attributes/GUIDConverterTest.php new file mode 100644 index 00000000..9b227857 --- /dev/null +++ b/tests/unit/Attributes/GUIDConverterTest.php @@ -0,0 +1,106 @@ + + * + */ + +namespace OCA\User_LDAP\Tests\Attributes; + +use OCA\User_LDAP\Attributes\GUIDConverter; +use OCA\User_LDAP\Attributes\ConverterException; + +class GUIDConverterTest extends \Test\TestCase { + /** @var GUIDConverter */ + private $guidConverter; + + protected function setUp(): void { + parent::setUp(); + $this->guidConverter = new GUIDConverter(); + } + + public function bin2strProvider() { + return [ + // 8A A0 73 BE 45 0A 3C 42 AC CD 33 F3 18 8C 74 6F + ["\x8A\xA0\x73\xBE\x45\x0A\x3C\x42\xAC\xCD\x33\xF3\x18\x8C\x74\x6F", 'BE73A08A-0A45-423C-ACCD-33F3188C746F'], + // BE 7A 3E 3F AC 61 53 48 85 D2 3A 7C 81 36 9B DA + ["\xBE\x7A\x3E\x3F\xAC\x61\x53\x48\x85\xD2\x3A\x7C\x81\x36\x9B\xDA", '3F3E7ABE-61AC-4853-85D2-3A7C81369BDA'], + ]; + } + + /** + * @dataProvider bin2strProvider + */ + public function testBin2str($input, $expected) { + $this->assertSame($expected, $this->guidConverter->bin2str($input)); + } + + public function bin2strExceptionProvider() { + return [ + [""], + ["0"], + ["sdlfkj"], + ["1234567890abcdefghijk"], + ]; + } + + /** + * @dataProvider bin2strExceptionProvider + */ + public function testBin2strException($input) { + $this->expectException(ConverterException::class); + $this->guidConverter->bin2str($input); + } + + public function testBin2strNoConversion() { + $this->assertSame('BE73A08A-0A45-423C-ACCD-33F3188C746F', $this->guidConverter->bin2str('BE73A08A-0A45-423C-ACCD-33F3188C746F')); + $this->assertSame('3F3E7ABE-61AC-4853-85D2-3A7C81369BDA', $this->guidConverter->bin2str('3F3E7ABE-61AC-4853-85D2-3A7C81369BDA')); + } + + public function str2filterProvider() { + return [ + // 8A A0 73 BE 45 0A 3C 42 AC CD 33 F3 18 8C 74 6F + ['BE73A08A-0A45-423C-ACCD-33F3188C746F', '\8A\A0\73\BE\45\0A\3C\42\AC\CD\33\F3\18\8C\74\6F'], + // BE 7A 3E 3F AC 61 53 48 85 D2 3A 7C 81 36 9B DA + ['3F3E7ABE-61AC-4853-85D2-3A7C81369BDA', '\BE\7A\3E\3F\AC\61\53\48\85\D2\3A\7C\81\36\9B\DA'], + ]; + } + + /** + * @dataProvider str2filterProvider + */ + public function testStr2filter($input, $expected) { + $this->assertSame($expected, $this->guidConverter->str2filter($input)); + } + + public function str2filterExceptionProvider() { + return [ + [''], + ['BE73A08A'], + ['BE73A08A-0A45'], + ['BE73A08A-0A45-423C'], + ['BE73A08A-0A45-423C-ACCD'], + ['BE73A08A-0A45-423C-ACCD-33F3188C746F-ACDC'], + ]; + } + + /** + * @dataProvider str2filterExceptionProvider + */ + public function testStr2filterException($input) { + $this->expectException(ConverterException::class); + $this->guidConverter->str2filter($input); + } +} diff --git a/tests/unit/Attributes/SIDConverterTest.php b/tests/unit/Attributes/SIDConverterTest.php new file mode 100644 index 00000000..12796f90 --- /dev/null +++ b/tests/unit/Attributes/SIDConverterTest.php @@ -0,0 +1,68 @@ + + * + */ + +namespace OCA\User_LDAP\Tests\Attributes; + +use OCA\User_LDAP\Attributes\SIDConverter; + +class SIDConverterTest extends \Test\TestCase { + /** @var SIDConverter */ + private $sidConverter; + + protected function setUp(): void { + parent::setUp(); + $this->sidConverter = new SIDConverter(); + } + + public function bin2strProvider() { + return [ + // 01 05 00 00 00 00 00 05 15 00 00 00 57 9A B7 00 F7 AB 69 48 2F 99 DE C3 4F 04 00 00 + ["\x01\x05\x00\x00\x00\x00\x00\x05\x15\x00\x00\x00\x57\x9A\xB7\x00\xF7\xAB\x69\x48\x2F\x99\xDE\xC3\x4F\x04\x00\x00", 'S-1-5-21-12032599-1214884855-3286145327-1103'], + // 01 05 00 00 00 00 00 05 15 00 00 00 57 9A B7 00 F7 AB 69 48 2F 99 DE C3 F4 01 00 00 + ["\x01\x05\x00\x00\x00\x00\x00\x05\x15\x00\x00\x00\x57\x9A\xB7\x00\xF7\xAB\x69\x48\x2F\x99\xDE\xC3\xF4\x01\x00\x00", 'S-1-5-21-12032599-1214884855-3286145327-500'], + // 01 02 00 00 00 00 00 05 20 00 00 00 22 02 00 00 + ["\x01\x02\x00\x00\x00\x00\x00\x05\x20\x00\x00\x00\x22\x02\x00\x00", 'S-1-5-32-546'], + // 01 01 00 00 00 00 00 05 0B 00 00 00 + ["\x01\x01\x00\x00\x00\x00\x00\x05\x0B\x00\x00\x00", 'S-1-5-11'], + ]; + } + + /** + * @dataProvider bin2strProvider + */ + public function testBin2str($input, $expected) { + $this->assertSame($expected, $this->sidConverter->bin2str($input)); + } + + public function str2filterProvider() { + return [ + ['S-1-5-21-12032599-1214884855-3286145327-1103', '\01\05\00\00\00\00\00\05\15\00\00\00\57\9A\B7\00\F7\AB\69\48\2F\99\DE\C3\4F\04\00\00'], + ['S-1-5-21-12032599-1214884855-3286145327-500', '\01\05\00\00\00\00\00\05\15\00\00\00\57\9A\B7\00\F7\AB\69\48\2F\99\DE\C3\F4\01\00\00'], + ['S-1-5-32-546', '\01\02\00\00\00\00\00\05\20\00\00\00\22\02\00\00'], + ['S-1-5-11', '\01\01\00\00\00\00\00\05\0B\00\00\00'], + ]; + } + + /** + * @dataProvider str2filterProvider + */ + public function testStr2filter($input, $expected) { + $this->assertSame($expected, $this->sidConverter->str2filter($input)); + } +} diff --git a/tests/unit/User/UserEntryTest.php b/tests/unit/User/UserEntryTest.php index 826e8d0e..7b3a669f 100644 --- a/tests/unit/User/UserEntryTest.php +++ b/tests/unit/User/UserEntryTest.php @@ -23,6 +23,7 @@ use OCA\User_LDAP\Connection; use OCA\User_LDAP\User\UserEntry; +use OCA\User_LDAP\Attributes\ConverterException; use OCP\IConfig; use OCP\ILogger; @@ -245,7 +246,7 @@ public function testGetUUIDUndetermined() { /** */ public function testGetUUIDInvalidBinaryUUID() { - $this->expectException(\OutOfBoundsException::class); + $this->expectException(ConverterException::class); $this->connection->expects($this->exactly(1)) ->method('__get') From d2c11243dcdef742dedc7fd7a7a05fbd2163a0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 2 Nov 2022 14:50:48 +0100 Subject: [PATCH 3/3] Fix code --- lib/Access.php | 2 +- lib/Attributes/ConverterHub.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Access.php b/lib/Access.php index 35f46caa..f5cd45e8 100644 --- a/lib/Access.php +++ b/lib/Access.php @@ -1732,7 +1732,7 @@ public function getUserDnByUuid($uuid) { $lowercaseUuidAttr = \strtolower($uuidAttr); $converterHub = ConverterHub::getDefaultConverterHub(); if ($converterHub->hasConverter($lowercaseUuidAttr)) { - $uuid = $converterHub->str2filter($uuid); + $uuid = $converterHub->str2filter($lowercaseUuidAttr, $uuid); } $filter = $uuidAttr . '=' . $uuid; diff --git a/lib/Attributes/ConverterHub.php b/lib/Attributes/ConverterHub.php index 7fd71b68..eeec9b3d 100644 --- a/lib/Attributes/ConverterHub.php +++ b/lib/Attributes/ConverterHub.php @@ -123,7 +123,7 @@ public function bin2str(string $attr, string $binValue) { * @param string $attr the attribute associated to the string representation * @param string $strValue the string representation of the value of * the attribute - * @param string a string suitable to be used as filter + * @return string a string suitable to be used as filter * @throws ConverterException if no converter is registered, or the * associated converter can't deal with the input. */