respect global visibility setting 'always' and sanitize associated code, fixes #938

Closes #938

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);
-
     }
 
     /**

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'; ?>>

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]);
+    }
 }
-?>

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;
 }