-
Bug
-
Resolution: Fixed
-
Major
-
3.6
-
MOODLE_36_STABLE
-
MOODLE_36_STABLE
-
MDL-63696-master -
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.
- blocks
-
MDLQA-12855 CLONE - A teacher can annotate a Nextcloud doc submitted as a controlled link
- Passed
- caused a regression
-
MDL-64079 OAuth2 access token expiry is optional
- Closed
- has been marked as being related by
-
MDL-58943 Create Nextcloud integration, similar to G-Suite and Office
- Closed
- is duplicated by
-
MDL-63822 System OAuth client conflicts when accessed by different user logins
- Closed
- Testing discovered
-
MDL-63723 Configurable request timeouts in oauth and rest clients (and saner defaults)
- Closed
- will help resolve
-
MDL-63822 System OAuth client conflicts when accessed by different user logins
- Closed