From a261afe07a5d8363c5753f82883c80a91d9eb14e Mon Sep 17 00:00:00 2001 From: Curtis Conard Date: Tue, 11 Feb 2025 15:28:28 -0500 Subject: [PATCH] split method --- CHANGELOG.md | 3 +- phpunit/functional/AuthTest.php | 34 ++++++++++++++ src/Auth.php | 83 +++++++++++++++++++++++---------- src/User.php | 4 +- 4 files changed, 96 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9085de295b..bb71ce5e4c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -228,7 +228,7 @@ The present file will list all changes made to the project; according to the - The `$target` parameter has been removed from the `AuthLDAP::showLdapGroups()` method. - The `$target` parameter has been removed from the `Rule::showRulePreviewCriteriasForm()`, `Rule::showRulePreviewResultsForm()`, `RuleCollection::showRulesEnginePreviewCriteriasForm()`, and `RuleCollection::showRulesEnginePreviewResultsForm()` methods signature. - `Hooks::SHOW_IN_TIMELINE`/`show_in_timeline` plugin hook has been renamed to `Hooks::TIMELINE_ITEMS`/`timeline_items`. -- `link` parameter for `Auth::getMethodName()` is now properly used instead of always showing the links. The default value remains false, so if you want to keep the previous behavior, you need to explicitly set it to false. +- `Auth::getMethodName()` now only returns the name without a link. Use `Auth::getMethodLink()` to get a HTML-safe link. #### Deprecated - Usage of the `/marketplace` path for plugins URLs. All plugins URLs should now start with `/plugins`. @@ -588,6 +588,7 @@ The present file will list all changes made to the project; according to the - `ajax/ticketsatisfaction.php` and `ajax/changesatisfaction.php` scripts. Access `ajax/commonitilsatisfaction.php` directly instead. - Usage of the `$cut` parameter in `formatUserName()` and `DbUtils::formatUserName()`. - Handling of the `delegate` right in `User::getSqlSearchResult()`. +- Usage of the `$link` and `$name` parameters in `Auth::getMethodName()`. ## [10.0.18] unreleased diff --git a/phpunit/functional/AuthTest.php b/phpunit/functional/AuthTest.php index 5660d6cad8d..a527f348f2d 100644 --- a/phpunit/functional/AuthTest.php +++ b/phpunit/functional/AuthTest.php @@ -35,6 +35,9 @@ namespace tests\units; +use Auth; +use AuthLDAP; +use AuthMail; use DbTestCase; use PHPUnit\Framework\Attributes\DataProvider; @@ -191,4 +194,35 @@ public function testValidateLogin(string $login, string $password, bool $noauto, $auth = new \Auth(); $this->assertSame($expected, $auth->validateLogin($login, $password, $noauto, $login_auth)); } + + public function testGetMethodName() + { + $this->assertSame(AuthLDAP::getTypeName(1), Auth::getMethodName(Auth::LDAP, 0)); + $this->assertSame(AuthMail::getTypeName(1), Auth::getMethodName(Auth::MAIL, 0)); + $this->assertSame('CAS', Auth::getMethodName(Auth::CAS, 0)); + $this->assertSame('x509 certificate authentication', Auth::getMethodName(Auth::X509, 0)); + $this->assertSame('Other', Auth::getMethodName(Auth::EXTERNAL, 0)); + $this->assertSame('GLPI internal database', Auth::getMethodName(Auth::DB_GLPI, 0)); + $this->assertSame('API', Auth::getMethodName(Auth::API, 0)); + + $this->assertSame('LDAP directory: _local_ldap', Auth::getMethodLink(Auth::LDAP, getItemByTypeName(AuthLDAP::class, '_local_ldap', true))); + } + + public function testGetMethodLink() + { + $this->login(); + + $this->assertSame(AuthLDAP::getTypeName(1), Auth::getMethodLink(Auth::LDAP, 0)); + $this->assertSame(AuthMail::getTypeName(1), Auth::getMethodLink(Auth::MAIL, 0)); + $this->assertSame('CAS', Auth::getMethodLink(Auth::CAS, 0)); + $this->assertSame('x509 certificate authentication', Auth::getMethodLink(Auth::X509, 0)); + $this->assertSame('Other', Auth::getMethodLink(Auth::EXTERNAL, 0)); + $this->assertSame('GLPI internal database', Auth::getMethodLink(Auth::DB_GLPI, 0)); + $this->assertSame('API', Auth::getMethodLink(Auth::API, 0)); + + $this->assertSame( + 'LDAP directory: _local_ldap', + Auth::getMethodLink(Auth::LDAP, getItemByTypeName(AuthLDAP::class, '_local_ldap', true)) + ); + } } diff --git a/src/Auth.php b/src/Auth.php index d63cb68545d..f932964734b 100644 --- a/src/Auth.php +++ b/src/Auth.php @@ -1263,20 +1263,9 @@ public static function dropdownCasVersion($value = 'CAS_VERSION_2_0', array $par return Dropdown::showFromArray('cas_version', $options, $params); } - /** - * Get name of an authentication method - * - * @param integer $authtype Authentication method - * @param integer $auths_id Authentication method ID - * @param boolean $link show links to config page? (default false) - * @param string $name override the name if not empty (default '') - * - * @return string If links are requested, the resulting string will be sanitized for correct use in HTML. - * If you don't request links and are displaying the result in HTML, you should sanitize the result yourself. - */ - public static function getMethodName($authtype, $auths_id, $link = false, $name = '') + private static function getMethodTypeLabel(int $auth_type, AuthLDAP|AuthMail|null $auth): string { - $auth_type_label = match ($authtype) { + $auth_type_label = match ($auth_type) { self::LDAP => AuthLDAP::getTypeName(1), self::MAIL => AuthMail::getTypeName(1), self::CAS => __('CAS'), @@ -1284,11 +1273,10 @@ public static function getMethodName($authtype, $auths_id, $link = false, $name self::EXTERNAL => __('Other'), self::DB_GLPI => __('GLPI internal database'), self::API => __("API"), - self::NOT_YET_AUTHENTIFIED => __('Not yet authenticated'), default => '', }; // Label used for special auth types when there is also a valid LDAP connection - $auth_type_label_ldap = match ($authtype) { + $auth_type_label_ldap = match ($auth_type) { self::CAS => sprintf( __('%1$s + %2$s'), __('CAS'), @@ -1306,27 +1294,38 @@ public static function getMethodName($authtype, $auths_id, $link = false, $name ), default => '', }; - if (in_array($authtype, [self::DB_GLPI, self::API, self::NOT_YET_AUTHENTIFIED])) { - // escaping when link requested even if no link to be consistent - return $link ? htmlspecialchars($auth_type_label) : $auth_type_label; + + if ($auth === null || $auth->isNewItem()) { + return $auth_type_label; } + return $auth_type_label_ldap ?: $auth_type_label; + } + /** + * Get name of an authentication method + * + * @param integer $authtype Authentication method + * @param integer $auths_id Authentication method ID + * + * @return string + */ + public static function getMethodName($authtype, $auths_id) + { $auth = match ($authtype) { self::LDAP, self::CAS, self::X509, self::EXTERNAL => new AuthLDAP(), self::MAIL => new AuthMail(), default => null, }; - if (!$auth || !$auth->getFromDB($auths_id)) { - $auth_server_name = htmlspecialchars($name); - } else { - $auth_server_name = $link ? $auth->getLink() : $auth->getName(); - $auth_type_label = $auth_type_label_ldap ?: $auth_type_label; + if (in_array($authtype, [self::DB_GLPI, self::API, self::NOT_YET_AUTHENTIFIED])) { + return self::getMethodTypeLabel($authtype, $auth); } - if ($link) { - $auth_type_label = htmlspecialchars($auth_type_label); + $auth_server_name = ''; + if ($auth && $auth->getFromDB($auths_id)) { + $auth_server_name = $auth->getName(); } + $auth_type_label = self::getMethodTypeLabel($authtype, $auth); if ($auth_type_label === '') { return ''; @@ -1336,6 +1335,40 @@ public static function getMethodName($authtype, $auths_id, $link = false, $name return sprintf(__('%1$s: %2$s'), $auth_type_label, $auth_server_name); } + /** + * Get link of an authentication method + * + * @param integer $authtype Authentication method + * @param integer $auths_id Authentication method ID + * + * @return string + */ + public static function getMethodLink(int $authtype, int $auths_id): string + { + $auth = match ($authtype) { + self::LDAP, self::CAS, self::X509, self::EXTERNAL => new AuthLDAP(), + self::MAIL => new AuthMail(), + default => null, + }; + + if (in_array($authtype, [self::DB_GLPI, self::API, self::NOT_YET_AUTHENTIFIED])) { + return self::getMethodTypeLabel($authtype, $auth); + } + + $auth_server_name = ''; + if ($auth && $auth->getFromDB($auths_id)) { + $auth_server_name = $auth->getLink(); + } + $auth_type_label = self::getMethodTypeLabel($authtype, $auth); + + if ($auth_type_label === '') { + return ''; + } else if ($auth_server_name === '') { + return $auth_type_label; + } + return sprintf(__s('%1$s: %2$s'), $auth_type_label, $auth_server_name); + } + /** * Get all the authentication methods parameters for a specific authtype * and auths_id and return it as an array diff --git a/src/User.php b/src/User.php index 66582848074..b83f1c5d652 100644 --- a/src/User.php +++ b/src/User.php @@ -2979,7 +2979,7 @@ public function showForm($ID, array $options = []) if (!empty($ID)) { if (Session::haveRight(self::$rightname, self::READAUTHENT)) { echo "" . __s('Authentication') . ""; - echo Auth::getMethodName($this->fields["authtype"], $this->fields["auths_id"], true); + echo Auth::getMethodLink($this->fields["authtype"], $this->fields["auths_id"]); if (!empty($this->fields["date_sync"])) { //TRANS: %s is the date of last sync echo '
' . sprintf( @@ -4356,7 +4356,7 @@ public static function getSpecificValueToDisplay($field, $values, array $options if (isset($values['auths_id']) && !empty($values['auths_id'])) { $auths_id = $values['auths_id']; } - return Auth::getMethodName($values[$field], $auths_id, true); + return Auth::getMethodLink($values[$field], $auths_id); case 'picture': if (isset($options['html']) && $options['html']) { return Html::image(