Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix escaping and unhandled param for Auth::getMethodName #18903

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+62 −80
Open

Fix escaping and unhandled param for Auth::getMethodName #18903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +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.

#### Deprecated
- Usage of the `/marketplace` path for plugins URLs. All plugins URLs should now start with `/plugins`.
Expand Down
133 changes: 57 additions & 76 deletions src/Auth.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1268,91 +1268,72 @@ public static function dropdownCasVersion($value = 'CAS_VERSION_2_0', array $par
*
* @param integer $authtype Authentication method
* @param integer $auths_id Authentication method ID
* @param integer $link show links to config page? (default 0)
* @param boolean $link show links to config page? (default false)
* @param string $name override the name if not empty (default '')
*
* @return string
* @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 = 0, $name = '')
public static function getMethodName($authtype, $auths_id, $link = false, $name = '')
{
switch ($authtype) {
case self::LDAP:
$auth = new AuthLDAP();
if ($auth->getFromDB($auths_id)) {
//TRANS: %1$s is the auth method type, %2$s the auth method name or link
return sprintf(__('%1$s: %2$s'), AuthLDAP::getTypeName(1), $auth->getLink());
}
return sprintf(__('%1$s: %2$s'), AuthLDAP::getTypeName(1), $name);

case self::MAIL:
$auth = new AuthMail();
if ($auth->getFromDB($auths_id)) {
//TRANS: %1$s is the auth method type, %2$s the auth method name or link
return sprintf(__('%1$s: %2$s'), AuthMail::getTypeName(1), $auth->getLink());
}
return sprintf(__('%1$s: %2$s'), __('Email server'), $name);

case self::CAS:
if ($auths_id > 0) {
$auth = new AuthLDAP();
if ($auth->getFromDB($auths_id)) {
return sprintf(
__('%1$s: %2$s'),
sprintf(
__('%1$s + %2$s'),
__('CAS'),
AuthLDAP::getTypeName(1)
),
$auth->getLink()
);
}
}
return __('CAS');

case self::X509:
if ($auths_id > 0) {
$auth = new AuthLDAP();
if ($auth->getFromDB($auths_id)) {
return sprintf(
__('%1$s: %2$s'),
sprintf(
__('%1$s + %2$s'),
__('x509 certificate authentication'),
AuthLDAP::getTypeName(1)
),
$auth->getLink()
);
}
}
return __('x509 certificate authentication');
$auth_type_label = match ($authtype) {
self::LDAP => AuthLDAP::getTypeName(1),
self::MAIL => AuthMail::getTypeName(1),
self::CAS => __('CAS'),
self::X509 => __('x509 certificate authentication'),
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) {
self::CAS => sprintf(
__('%1$s + %2$s'),
__('CAS'),
AuthLDAP::getTypeName(1)
),
self::X509 => sprintf(
__('%1$s + %2$s'),
__('x509 certificate authentication'),
AuthLDAP::getTypeName(1)
),
self::EXTERNAL => sprintf(
__('%1$s + %2$s'),
__('Other'),
AuthLDAP::getTypeName(1)
),
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;
}

case self::EXTERNAL:
if ($auths_id > 0) {
$auth = new AuthLDAP();
if ($auth->getFromDB($auths_id)) {
return sprintf(
__('%1$s: %2$s'),
sprintf(
__('%1$s + %2$s'),
__('Other'),
AuthLDAP::getTypeName(1)
),
$auth->getLink()
);
}
}
return __('Other');
$auth = match ($authtype) {
self::LDAP, self::CAS, self::X509, self::EXTERNAL => new AuthLDAP(),
self::MAIL => new AuthMail(),
default => null,
};

case self::DB_GLPI:
return __('GLPI internal database');
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;
}

case self::API:
return __("API");
if ($link) {
$auth_type_label = htmlspecialchars($auth_type_label);
}

case self::NOT_YET_AUTHENTIFIED:
return __('Not yet authenticated');
if ($auth_type_label === '') {
return '';
} else if ($auth_server_name === '') {
return $auth_type_label;
}
return '';
return sprintf(__('%1$s: %2$s'), $auth_type_label, $auth_server_name);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/AuthMail.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class AuthMail extends CommonDBTM

public static function getTypeName($nb = 0)
{
return _n('Mail server', 'Mail servers', $nb);
return _n('Email server', 'Email servers', $nb);
}

public static function getSectorizedDetails(): array
Expand Down
2 changes: 1 addition & 1 deletion src/RuleRight.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public function getAdditionalCriteriaDisplayPattern($ID, $condition, $pattern)
{
$crit = $this->getCriteria($ID);
if (count($crit) && $crit['field'] == 'type') {
return Auth::getMethodName($pattern, 0);
return Auth::getMethodName((int) $pattern, 0);
}
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/User.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2979,7 +2979,7 @@ public function showForm($ID, array $options = [])
if (!empty($ID)) {
if (Session::haveRight(self::$rightname, self::READAUTHENT)) {
echo "<td>" . __s('Authentication') . "</td><td>";
echo htmlescape(Auth::getMethodName($this->fields["authtype"], $this->fields["auths_id"]));
echo Auth::getMethodName($this->fields["authtype"], $this->fields["auths_id"], true);
if (!empty($this->fields["date_sync"])) {
//TRANS: %s is the date of last sync
echo '<br>' . sprintf(
Expand Down Expand Up @@ -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);
return Auth::getMethodName($values[$field], $auths_id, true);
case 'picture':
if (isset($options['html']) && $options['html']) {
return Html::image(
Expand Down
Loading