Sending for peer review for a sanity check. The repositories got me really confused and I'd like to make sure I'm doing the right thing. Once reviewed and fixed, I'll backport the branches.

Hi Frédéric
Good to see this bug getting fixed.
I suppose you could raise the priority to major.

I updated to 2.4.2 yesterday and am experiencing this same bug.

looks good Fred. We are backporting it to previous versions as well, right?

btw should not we add tests for Course files repository as well?

Yes, I am currently preparing 1.9 to test the course files repository. I will backport the patches down to 2.2 after a successful peer review.

+1, thanks Fred, for asking my concerns!

Thanks for responding quickly on this. Partners are now involved and following our actions on this one.

Sorry for breaking your Moodle guys, I have secured the repositories a bit too much apparently . Anyway, here are patches for 2.2 onwards. That would be great if some of the watchers of this issue could (quickly?) test the patch.

In a nutshell, this is what happens:

When browsing a repository using the File Manager (repository_ajax.php, filepicker.js), the context passed to the repository instance is the current one. This context is used to verify that the user can access its content in repository::check_capability(). To ensure that the user is not trying to pass another $contextid (as a GET parameter) to be granted an access to a repository, I've added some context validation. So we're not using the context of the repository to check the capability, but to validate the context passed. This solves the problem of teachers not having a capability on a system context.

Submitting for integration. Thanks!

Starting review of this for integration out of cycle.

Added an issue to create units tests for this regression.

Thanks Fred,

I have integrated this to 22, 23, 24 and master branches. I set the fix version to the released versions because I think we will do something special to get this patch out in those versions.

I'll test this issue's testing instructions with the upgrades and server files and you (Michael, Adrian and Ankit) can split MDL-36426 and MDL-37852 as you wish, between branches or repository types.

I'll take MDK-37852. Adrian and Ankit, can you please split MDL-36426?

Noticed that webdav repos are not listing in the non-js version , but seems it was the same before these changes were made

I just pushed an additional patch from Fred for the 23 and 24 branches to fix an undefined variable.

I ran through the MDL-36426 testing instructions on 2.2 and 2.3. After a couple of hiccups everything is now working properly.

Added an issue MDL-38500 - but it is an existing bug and should be worked on separately.

MDL-36426 Looks fine to me on 24 and master.
Thanks

I just pushed an additional patch from Fred for the master branch to fix editing a repository instance in a course.

MDL-38474 testing instructions working as expected

MDL-37852 now completes as expected.

Passing it

Closing thanks!

Note: weeklies with this fix have been already rolled to git repositories and http://download.moodle.org . Also new point releases 2.4.3, 2.3.6 and 2.2.9 will happen next Monday 18th March.

Ciao

Thank you very much!!!

---------
Muchas Gracias a todos!!!

I know this has been integrated, don't beat me please, I did mention it to Fred but was not persuasive enough. When can it happen that filepicker is accessed from block context? And if it ever happens, why are you sure that block is on the course page and not on system page? There would be a fatal error when you call $coursecontext->instanceid

That's a good point Marina. But I don't think that would ever cause an issue. If you are accessing the File Manager from the block context, assuming it's not in a course, then the repositories listed should not contain the course instances. If you want to access course instances, you have to be in a course, whether it's from a block, or somewhere else.