From b27d77c6abe52c8b1f8995e56b2216d8ef32968e Mon Sep 17 00:00:00 2001 From: Jan-Hendrik Willms <tleilax+studip@gmail.com> Date: Wed, 20 Apr 2022 16:08:18 +0000 Subject: [PATCH] respect global visibility setting 'always' and sanitize associated code, fixes #938 Closes #938 --- app/controllers/settings/privacy.php | 6 +-- app/views/settings/privacy.php | 16 +++---- lib/classes/Visibility.php | 64 +++++++++++++++++++++++++++- lib/user_visible.inc.php | 56 +++++++++++++----------- 4 files changed, 100 insertions(+), 42 deletions(-) diff --git a/app/controllers/settings/privacy.php b/app/controllers/settings/privacy.php index 1b85abc241e..e61e4be9091 100644 --- a/app/controllers/settings/privacy.php +++ b/app/controllers/settings/privacy.php @@ -48,16 +48,14 @@ class Settings_PrivacyController extends Settings_SettingsController // Get default visibility for homepage elements. $this->default_homepage_visibility = Visibility::get_default_homepage_visibility($this->user->user_id); - $this->NOT_HIDEABLE_FIELDS = $GLOBALS['NOT_HIDEABLE_FIELDS']; - $this->user_perm = $GLOBALS['perm']->get_perm($this->user->user_id); - $this->user_domains = UserDomain::getUserDomains(); + $this->user_perm = $GLOBALS['perm']->get_perm($this->user->user_id); + $this->user_domains = UserDomain::getUserDomains(); // Calculate colWidth and colCount for different visibilities $this->colCount = Visibility::getColCount(); $this->colWidth = 67 / $this->colCount; $this->visibilities = Visibility::getVisibilities(); $this->homepage_elements = Visibility::getHTMLArgs($this->user->user_id); - } /** diff --git a/app/views/settings/privacy.php b/app/views/settings/privacy.php index 64bf97f159e..fc8c868b2a1 100644 --- a/app/views/settings/privacy.php +++ b/app/views/settings/privacy.php @@ -49,37 +49,31 @@ use Studip\Button, Studip\LinkButton; </div> </label> - <? if ((in_array($global_visibility, ['yes', 'global']) || - ($global_visibility === 'unknown' && Config::get()->USER_VISIBILITY_UNKNOWN) || - ($user_perm === 'dozent' && Config::get()->DOZENT_ALWAYS_VISIBLE)) && - (!$NOT_HIDEABLE_FIELDS[$user_perm]['online'] || - !$NOT_HIDEABLE_FIELDS[$user_perm]['search'] || - !$NOT_HIDEABLE_FIELDS[$user_perm]['email']) - ) : ?> + <? if (Visibility::allowExtendedSettings($user)): ?> <div> <?= _('Erweiterte Einstellungen') ?> <?= tooltipIcon( _('Stellen Sie hier ein, in welchen Bereichen des Systems Sie erscheinen wollen.') - . (!$NOT_HIDEABLE_FIELDS[$user_perm]['email'] + . (Visibility::isFieldHideableForUser('email', $user) ? _('Wenn Sie hier Ihre E-Mail-Adresse verstecken, wird stattdessen die E-Mail-Adresse Ihrer (Standard-)Einrichtung angezeigt.') : '') ) ?> - <? if (!$NOT_HIDEABLE_FIELDS[$user_perm]['online']): ?> + <? if (Visibility::isFieldHideableForUser('online', $user)): ?> <label> <input type="checkbox" name="online" value="1" <? if ($online_visibility) echo 'checked'; ?>> <?= _('sichtbar in "Wer ist online"') ?> </label> <? endif; ?> - <? if (!$NOT_HIDEABLE_FIELDS[$user_perm]['search']): ?> + <? if (Visibility::isFieldHideableForUser('search', $user)): ?> <label> <input type="checkbox" name="search" value="1" <? if ($search_visibility) echo 'checked'; ?>> <?= _('auffindbar über die Personensuche') ?> </label> <? endif; ?> - <? if (!$NOT_HIDEABLE_FIELDS[$user_perm]['email']): ?> + <? if (Visibility::isFieldHideableForUser('email', $user)): ?> <label> <input type="checkbox" name="email" value="1" <? if ($email_visibility) echo 'checked'; ?>> diff --git a/lib/classes/Visibility.php b/lib/classes/Visibility.php index 41054ddec99..c4499492089 100644 --- a/lib/classes/Visibility.php +++ b/lib/classes/Visibility.php @@ -644,5 +644,67 @@ class Visibility $stmt->execute(); return $stmt->rowCount(); } + + /** + * Returns whether the user should be allowed to configure the extended + * settings. + * + * @param User|null $user User object (defaults to current user) + * + * @return bool + */ + public static function allowExtendedSettings(\User $user = null): bool + { + $user = $user ?? User::findCurrent(); + + if (!$user) { + return false; + } + + // Check if all fields for the extended settings are not hideable. If + // so, we mustn't show the extended settings. + if ( + !self::isFieldHideableForUser('online', $user) + && !self::isFieldHideableForUser('search', $user) + && !self::isFieldHideableForUser('email', $user) + ) { + return false; + } + + // User chose to be visible and may configure the extended settings. + if (in_array($user->visible, ['yes', 'global'])) { + return true; + } + + // User did not specify a visibility but the global config defines + // visibility, so the user may configure the extended settings. + if ($user->visible === 'unknown' && Config::get()->USER_VISIBILITY_UNKNOWN) { + return true; + } + + // Teachers are always visible by configuration? So they may see the + // extended settings + if ($user->perms === 'dozent' && Config::get()->DOZENT_ALWAYS_VISIBLE) { + return true; + } + + // In all other cases, don't show the extended settings. + return false; + } + + /** + * Returns whether the given field is hideable by configuration for the + * given user permission. + * + * @param string $field Field that should be hidden + * @param User|null $user User object (defaults to current user) + * + * @return bool + */ + public static function isFieldHideableForUser(string $field, User $user = null): bool + { + $user = $user ?? User::findCurrent(); + + return empty($GLOBALS['NOT_HIDEABLE_FIELDS'][$user->perms][$field]); + } } -?> diff --git a/lib/user_visible.inc.php b/lib/user_visible.inc.php index d0b12853eed..cc13047bd2a 100644 --- a/lib/user_visible.inc.php +++ b/lib/user_visible.inc.php @@ -274,15 +274,23 @@ function first_decision($userid) { function get_local_visibility_by_id($user_id, $context, $return_user_perm=false) { global $NOT_HIDEABLE_FIELDS; - $query = "SELECT a.perms, u.`{$context}` - FROM auth_user_md5 AS a - LEFT JOIN user_visibility AS u USING (user_id) - WHERE user_id = ?"; - $statement = DBManager::get()->prepare($query); - $statement->execute([$user_id]); - $data = $statement->fetch(PDO::FETCH_ASSOC); + $user = User::find($user_id); + + if (Visibility::allowExtendedSettings($user)) { + $query = "SELECT u.`{$context}` + FROM auth_user_md5 AS a + LEFT JOIN user_visibility AS u USING (user_id) + WHERE user_id = ?"; + $statement = DBManager::get()->prepare($query); + $statement->execute([$user_id]); + $data = $statement->fetch(PDO::FETCH_ASSOC); + } else { + $data = []; + } if ($context === 'homepage') { + $homepage_settings = []; + $settings = User_Visibility_Settings::findByUser_id($user_id); foreach ($settings as $setting) { if ($setting['category'] == 1) { @@ -296,30 +304,26 @@ function get_local_visibility_by_id($user_id, $context, $return_user_perm=false) } if ($data[$context] === null) { - $user_perm = $data['perm']; - $data['perms'] = $user_perm; - $data[$context] = Config::get()->getValue(mb_strtoupper($context) . '_VISIBILITY_DEFAULT'); } - // Valid context given. - if (isset($data[$context])) { + + if (!isset($data[$context])) { + // No valid context given. + $result = false; + } elseif (!Visibility::isFieldHideableForUser($context, $user)) { // Context may not be hidden per global config setting. - if ($NOT_HIDEABLE_FIELDS[$data['perms']][$context]) { - $result = true; - } else { - // Give also user's permission level. - if ($return_user_perm) { - $result = [ - 'perms' => $data['perms'], - $context => $data[$context] - ]; - } else { - $result = $data[$context]; - } - } + $result = true; + } elseif ($return_user_perm) { + // Give also user's permission level. + $result = [ + 'perms' => $user->perms, + $context => $data[$context] + ]; } else { - $result = false; + // Valid context given. + $result = $data[$context]; } + return $result; } -- GitLab