Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-59825

user_can_view_profile() doesn't check all courses where $user is enrolled

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide
      • Make sure you have just 1 course on the site with only 1 enrolled student, s1.
      • Edit the manager role (define roles) and set 'viewdetails' and 'viewalldetails' to prevent.
      • Assign manager role to a user at system level.
      • Edit permissions in the course and override the manager role's capabilities, setting 'viewdetails' to 'allow'.
      • Log in as the user with the manager's role and view the user's profile within the course and confirm that you see the profile information (user pic might be missing - this is being addressed in MDL-59826)
      • Now, view your own profile from the user menu and change the id to that of s1.
      • Observe that you CAN now view this profile. 
      Show
      Make sure you have just 1 course on the site with only 1 enrolled student, s1. Edit the manager role (define roles) and set 'viewdetails' and 'viewalldetails' to prevent. Assign manager role to a user at system level. Edit permissions in the course and override the manager role's capabilities, setting 'viewdetails' to 'allow'. Log in as the user with the manager's role and view the user's profile within the course and confirm that you see the profile information (user pic might be missing - this is being addressed in  MDL-59826 ) Now, view your own profile from the user menu and change the id to that of s1. Observe that you CAN now view this profile.  
    • Affected Branches:
      MOODLE_32_STABLE, MOODLE_33_STABLE, MOODLE_34_STABLE
    • Fixed Branches:
      MOODLE_32_STABLE, MOODLE_33_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-59825-master
    • Sprint:
      3.4 Sprint 4

      Description

      I saw this while working on MDL-58953. The original function, user_can_view_profile($user, $course) was introduced in ---------MDL-27177---------, and was designed to be used to check whether the current user could view another user's (as specified by $user) course profile.

      It provides 2 modes of checking this, depending on whether the $course param is provided:

      • If provided, only permissions in that specific course are checked.
      • If not provided, then it attempts to check all courses using enrol_get_shared_courses(). This function assumes that both the current user and the other user ($user) are both enrolled in the courses it returns. What this calls fails to capture however, are situations like when a manager has the capability 'user:viewdetails' in a course, but is not necessarily enrolled in said course.

      I think what this function should be doing, when $course is not supplied, is checking all places which the $user is enrolled, not just the shared courses as this missed some courses where the current user may be able to view the profile.

      Example (Replication steps):

      Consider the following example using a manager override:

      • Make sure you have just 1 course on the site with only 1 enrolled student, s1.
      • Edit the manager role (define roles) and set 'viewdetails' and 'viewalldetails' to prevent.
      • Assign manager role to a user at system level.
      • Edit permissions in the course and override the manager role's capabilities, setting 'viewdetails' to 'allow'.
      • View the user's profile within the course and confirm that you see the profile information (user pic might be missing - this is being addressed in MDL-59826)
      • Now, view your own profile from the user menu and change the id to that of s1.
      • Observe that you cannot view this profile, despite being able to view it in the course. This is a bug.

      Given the above, the manager is now NOT enrolled in the course, thus this is not a shared course. They also have no capabilities in the user context (prevented), thus that check fails. This causes the user_can_view_profile() function to return false -  incorrectly, as the current user can view details of users in that course.

       

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jaked Jake Dallimore
                Reporter:
                jaked Jake Dallimore
                Peer reviewer:
                Adrian Greeve
                Integrator:
                Andrew Nicols
                Tester:
                Ryan Wyllie
                Participants:
                Component watchers:
                Amaia Anabitarte, Bas Brands, Carlos Escobedo, Sara Arjona (@sarjona), Víctor Déniz Falcón, Amaia Anabitarte, Bas Brands, Carlos Escobedo, Sara Arjona (@sarjona), Víctor Déniz Falcón
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Sep/17