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

OAuth 2 system account usage requires one access token per session

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.6
    • Fix Version/s: 3.6
    • Component/s: Authentication, Other
    • Labels:
    • Testing Instructions:
      Hide

      Test should be performed twice, ideally. Once using Google (as a regression test to ensure that we didn't break anything) and once using Nextcloud, to check whether this now works.

      Setup

      1. You will need a Moodle course and two Moodle users with access to it. At least one of the users needs to be able to add an activity to the course (this can be the system admin). Start with the admin. 
      2. In `/admin/tool/oauth2/issuers.php`, create an issuer and connect a system account. Use e.g. Google or Nextcloud for this.
        • Nextcloud: Use demo.nextcloud.com to get a testing instance really quickly, and configure it as described in the Moodle docs. You need two accounts. One can be the admin one, one can be "systemaccount" or something.
        • Google: Follow the instructions from Moodle docs. One Google account will be sufficient.
      3. Connect a system account.
      4. Configure a repository using the previously configured issuer. Types need to match, i.e. use a Nextcloud repository for a Nextcloud issuer and a Google Drive repository for a Google issuer.
      5. (If you are using Nextcloud, log out there! In the next step you will need to log in with a different account.)

      Testing

      Browser A

      1. (If you are using Nextcloud, when prompted to log in to Nextcloud in the following, use any account that is NOT the system account. You can use a single one for all tests, no problem.)
      2. Go to a course and add a resource. Select the repository and choose a PDF file. Upload it as "Access controlled link". Click save and return to course.
      3. Access the file from the course. Check that it is displayed/downloaded.
      4. Check the database and ensure that `mdl_oauth2_access_tokens` has a single entry for the issuer that you are currently testing with.
      5. Leave the current window open and stay logged in!

      Browser B

      1. Open a different browser.
      2. Log in with the other Moodle account.
      3. Enter the course. Access the resource. Check that it is displayed/downloaded.
      4. Check the database again. There must still be only one entry for your issuer. It should be identical to the one above (except for very short-lived access tokens, maybe).

      Browser A

      1. Go back to the first browser. There, access the resource again. Check that it is displayed/downloaded.
      Show
      Test should be performed twice, ideally. Once using Google (as a regression test to ensure that we didn't break anything) and once using Nextcloud, to check whether this now works. Setup You will need a Moodle course and two Moodle users with access to it. At least one of the users needs to be able to add an activity to the course (this can be the system admin). Start with the admin.  In `/admin/tool/oauth2/issuers.php`, create an issuer and connect a system account. Use e.g. Google or Nextcloud for this. Nextcloud: Use demo.nextcloud.com to get a testing instance really quickly, and configure it as described in the Moodle docs . You need two accounts. One can be the admin one, one can be "systemaccount" or something. Google: Follow the instructions from Moodle docs . One Google account will be sufficient. Connect a system account. Configure a repository using the previously configured issuer. Types need to match, i.e. use a Nextcloud repository for a Nextcloud issuer and a Google Drive repository for a Google issuer. (If you are using Nextcloud, log out there! In the next step you will need to log in with a different account .) Testing Browser A (If you are using Nextcloud, when prompted to log in to Nextcloud in the following, use any account that is NOT the system account. You can use a single one for all tests, no problem.) Go to a course and add a resource. Select the repository and choose a PDF file. Upload it as "Access controlled link". Click save and return to course. Access the file from the course. Check that it is displayed/downloaded. Check the database and ensure that `mdl_oauth2_access_tokens` has a single  entry for the issuer that you are currently testing with. Leave the current window open and stay logged in! Browser B Open a different browser. Log in with the other Moodle account. Enter the course. Access the resource. Check that it is displayed/downloaded. Check the database again. There must still be only one entry for your issuer . It should be identical to the one above (except for very short-lived access tokens, maybe). Browser A Go back to the first browser. There, access the resource again. Check that it is displayed/downloaded.
    • Affected Branches:
      MOODLE_36_STABLE
    • Fixed Branches:
      MOODLE_36_STABLE
    • Pull Master Branch:
      MDL-63696-master

      Description

      In the context of MDL-58943 (which heavily uses OAuth 2), Jake Dallimore observed the following:

      Ok, it seems the following steps can reproduce this problem every time. You'll need 2 next cloud users inn addition to the linked system user to replicate this.

      • As a teacher, create the ACL for a file (let's call this one 'file1') in session 1 / browser 1. This should be using the repo of user1 in NC. Make sure display is set to 'download'. This should all work ok.
      • Now, log in as a student in browser 2 /session 2 and try to view the acl file you just created as the teacher. When prompted, sign in as user2 in nextcloud, and confirm you see the download prompt, but cancel this as it doesn't matter whether we download the file or not. Really, we just want to trigger the use of the system account, I think.
      • Now, back in the teacher browser/session try to create another ACL using another file in NC. Let's call this one file2.

      Notice the system account error message as above ("..Folder path /System (master) could not be created in the system account"). Inspection of the web traffic reveals this is due to a 401, unauthorised when trying to create the ACL file. 

      Effectively, Nextcloud prevents the first user session from accessing the system account as soon as the second session requests access to that.

      The root cause is that Moodle caches access tokens in the user session, even for the Moodle system account: https://github.com/moodle/moodle/blob/8675c2c90846a1d1fdc21a57591bb550be0d2eda/lib/oauthlib.php#L703 in combination with https://github.com/moodle/moodle/blob/8675c2c90846a1d1fdc21a57591bb550be0d2eda/lib/classes/oauth2/client.php#L147-L151. As a consequence, when multiple Moodle users access a feature that try to use the system account at the same time, Moodle assumes that several access tokens are valid to use the system account in Nextcloud. However, Nextcloud always accepts the most recent access token only, as using the refresh token invalidates the previous access token: https://github.com/nextcloud/server/blob/2634ceb35b72eac94e6bf4c61640036392c5f97f/apps/oauth2/lib/Controller/OauthApiController.php#L148.

      In the case quoted above, Moodle assumes that the first session is still logged in to Nextcloud, because there is an access token in the session that hasn't reached the expiry date yet. Meanwhile, Nextcloud has dropped that stored token because Moodle requested a second one.

       

      Expected behaviour

      The specs (https://tools.ietf.org/html/rfc6749#section-5.1) don't prescribe anything for this, i.e. they neither mandate that access tokens must be valid until their expiry date is reached, nor that they must be deleted on subsequent refreshes. However, I must say I think that the Nextcloud behaviour is right. After all it is not the (Moodle) user who is granted access to the system account; it is the (Moodle) server. Therefore, the access token (for the system account) should be persisted in the database, not in the users' sessions.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jan.dagefoerde Jan Dageförde
                Reporter:
                jan.dagefoerde Jan Dageförde
                Peer reviewer:
                Víctor Déniz Falcón
                Integrator:
                Damyon Wiese
                Tester:
                Damyon Wiese
                Participants:
                Component watchers:
                Jake Dallimore, Jun Pataleta, Ryan Wyllie, Adrian Greeve, Mihail Geshoski, Peter Dias
              • Votes:
                6 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Dec/18