From 62ee91c9bf93bddc27c01cf19abd44fcd92e1b17 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Fri, 18 Oct 2024 08:16:47 +0000
Subject: [PATCH] always allow accessing course teachers and tutors and update
 visibility check in user model, fixes #4714

Closes #4714

Merge request studip/studip!3506
---
 .../JsonApi/Routes/Courses/Authority.php      | 21 +++++-----
 .../Courses/CoursesMembershipsIndex.php       | 39 +++++++++++++------
 lib/models/User.php                           |  8 ++--
 3 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/lib/classes/JsonApi/Routes/Courses/Authority.php b/lib/classes/JsonApi/Routes/Courses/Authority.php
index db5a1d60f43..197bb48240a 100644
--- a/lib/classes/JsonApi/Routes/Courses/Authority.php
+++ b/lib/classes/JsonApi/Routes/Courses/Authority.php
@@ -16,15 +16,16 @@ class Authority
     public static function canShowCourse(User $user, Course $course, $scope)
     {
         switch ($scope) {
-        case self::SCOPE_BASIC:
-            return
-                // visible
-                ((int) $course->visible) || $GLOBALS['perm']->have_perm(\Config::get()->SEM_VISIBILITY_PERM)
-                // member
-                || $GLOBALS['perm']->have_studip_perm('user', $course->id, $user->id);
-
-        case self::SCOPE_EXTENDED:
-            return $GLOBALS['perm']->have_studip_perm('user', $course->id, $user->id);
+            case self::SCOPE_BASIC:
+                return
+                    // visible
+                    $course->visible
+                    || $GLOBALS['perm']->have_perm(\Config::get()->SEM_VISIBILITY_PERM)
+                    // member
+                    || $GLOBALS['perm']->have_studip_perm('user', $course->id, $user->id);
+
+            case self::SCOPE_EXTENDED:
+                return $GLOBALS['perm']->have_studip_perm('user', $course->id, $user->id);
         }
 
         return false;
@@ -48,7 +49,7 @@ class Authority
 
     public static function canIndexMemberships(User $user, Course $course)
     {
-        return self::canShowCourse($user, $course, self::SCOPE_EXTENDED);
+        return self::canShowCourse($user, $course, self::SCOPE_BASIC);
     }
 
     public static function canIndexMembershipsOfUser(User $observer, User $user)
diff --git a/lib/classes/JsonApi/Routes/Courses/CoursesMembershipsIndex.php b/lib/classes/JsonApi/Routes/Courses/CoursesMembershipsIndex.php
index d4877bbe969..ceb2c8c5e67 100644
--- a/lib/classes/JsonApi/Routes/Courses/CoursesMembershipsIndex.php
+++ b/lib/classes/JsonApi/Routes/Courses/CoursesMembershipsIndex.php
@@ -45,19 +45,36 @@ class CoursesMembershipsIndex extends JsonApiController
     {
         $memberships = $course->members;
 
-        $visibleMemberships = Authority::canEditCourse($user, $course)
-            ? $memberships
-            : $memberships->filter(function ($membership) use ($user) {
-                return $membership['user_id'] == $user->id ||
-                    !in_array($membership['status'], ['autor', 'user']) ||
-                    'no' != $membership['visible'];
+        // Filter by permission?
+        if (isset($filters['permission'])) {
+            $memberships = $memberships->filter(function (\CourseMember $membership) use ($filters) {
+                return $membership->status === $filters['permission'];
             });
+        }
+
+        // Filter out invisible members if not teacher
+        if (!Authority::canEditCourse($user, $course)) {
+            $memberships = $memberships->filter(function (\CourseMember $membership) use ($user) {
+                return $membership->user->isAccessibleToUser($user->id)
+                    && (
+                        $membership->user_id === $user->id
+                        || $membership->visible !== 'no'
+                    );
+            });
+        }
+
+        // Filter out students if not in course
+        if (!Authority::canShowCourse($user, $course, Authority::SCOPE_EXTENDED)) {
+            $memberships = $memberships->filter(function (\CourseMember $membership) use ($user) {
+                return $membership->user->isAccessibleToUser($user->id)
+                    && (
+                        $membership->user_id === $user->id
+                        || !in_array($membership->status, ['autor', 'user'])
+                    );
+            });
+        }
 
-        return isset($filters['permission'])
-            ? $visibleMemberships->filter(function ($membership) use ($filters) {
-                return $membership['status'] === $filters['permission'];
-            })
-            : $visibleMemberships;
+        return $memberships;
     }
 
     private function validateFilters()
diff --git a/lib/models/User.php b/lib/models/User.php
index a7c8b6ef8b6..190df2b3946 100644
--- a/lib/models/User.php
+++ b/lib/models/User.php
@@ -1507,13 +1507,15 @@ class User extends AuthUserMd5 implements Range, PrivacyObject, Studip\Calendar\
      */
     public function isAccessibleToUser($user_id = null)
     {
-        // TODO: Visibility checks
         if ($user_id === null) {
-            $user_id = $GLOBALS['user']->id;
+            $user_id = self::findCurrent()->id;
         }
+
         return $user_id === $this->user_id
             || static::find($user_id)->perms === 'root'
-            || !in_array(static::find($this->user_id)->visible, ['no', 'never']);
+            || !in_array($this->visible, ['no', 'never'])
+            || (Config::get()->getValue('USER_VISIBILITY_UNKNOWN') && $this->visible === 'unknown')
+            || ($this->perms === 'dozent' && Config::get()->getValue('DOZENT_ALWAYS_VISIBLE'));
     }
 
     /**
-- 
GitLab