Moodle

Should be fixed now (19_STABLE and HEAD).

Please test as admin/coursecreator/teacher both with backup files stored in SITE course (shouldn't show the course selection page) and normal courses and with users having "restore" cap in only one course (shouldn't show the course selection page) or in more.

The text below explains current behaviour:

• A) If user has 'moodle/site:restore' in more than one course and cannot create courses and he isn't restoring from SITEID:
  • 0-Existing course, deleting: Select the destination course and launch the check again, then put $restore->course_id and $restore->deleting (true), create the restore object.
  • 1-Existing course, adding: Select the destination course and launch the check again, then put $restore->course_id and $restore->deleting (false), create the restore object.
• B) Else, if user cannot create courses or he is restoring from within SITEID:
  • 0-Current course, deleting: Put $restore->course_id and $restore->deleting (true), create the restore object
  • 1-Current course, adding: Put $restore->course_id and $restore->deleting (false), create the restore object
• C) If the user is a creator:
  • 0-Existing course, deleting: Select the destination course and launch the check again, then put $restore->course_id and $restore->deleting (true), create the restore object.
  • 1-Existing course, adding: Select the destination course and launch the check again, then put $restore->course_id and $restore->deleting (false), create the restore object.
  • 2-New course: Create the restore object and launch the execute.
• NOTE: SITEID is prevented as a source for restoring because it's public area and we must encourage admins to avoid using it as a "repository" for courses storage.

Resolving as fixed for 1.9.5... testing here a bit more...

Eloy, thanks for fixing this issue.

After testing as an admin and as a teacher in the front page course and in normal courses I can confirm that where appropriate, you are given the option to choose which course to restore to.

The only problem I came across was the following error message displayed when logged in as a teacher and attempting to restore to an existing course (either deleting it or adding data to it):

Notice: Undefined property: stdClass::$shortname in /var/www/19/moodle/backup/restore_check.html on line 22

Notice: Undefined property: stdClass::$fullname in /var/www/19/moodle/backup/restore_check.html on line 23

I've detected one problem in above A, B, C with one case not being covered: "cancreatecourse" users (admins, course creators...) trying to restore in a site with only ONE course. So, I've fixed it now both in 1.9 and HEAD and (hopefully) final algorithm is:

• A) If user has 'moodle/site:restore' in more than one course and cannot create courses and he isn't restoring from SITEID:
  • 0-Existing course, deleting: Select the destination course and launch the check again, then ut $restore->course_id and $restore->deleting (true), create the restore object.
  • 1-Existing course, adding: Select the destination course and launch the check again, then ut $restore->course_id and $restore->deleting (false), create the restore object.
• B) Else, if user has 'moodle/site:restore' in only 1 course or cannot create courses or he is restoring from within SITEID:
  • 0-Current course, deleting: Put $restore->course_id and $restore->deleting (true), create the restore object
  • 1-Current course, adding: Put $restore->course_id and $restore->deleting (false), create the restore object
• C) If the user is a creator:
  • 0-Existing course, deleting: Select the destination course and launch the check again, then put $restore->course_id and $restore->deleting (true), create the restore object.
  • 1-Existing course, adding: Select the destination course and launch the check again, then put $restore->course_id and $restore->deleting (false), create the restore object.
  • 2-New course: Create the restore object and launch the execute.
• NOTE: SITEID is prevented as a source for restoring because it's public area and we must encourage admins to avoid using it as a "repository" for courses storage.

I will fix those notices later today, Helen (they doesn't cause any other problem when restoring to existing course but the notice itself). Thanks a lot for deep testing of combinations! They are a lot!

Ciao

After some discussion in http://moodle.org/mod/forum/discuss.php?d=123737 I'm going to:

1) Add one notice in the last page before restore happens, warning about going to restore over SITE.
2) Document in Docs the SITE behaviour, so it isn't possible anymore to restore courses from SITE files over (to) existing courses.

I'm afraid I neither understand or agree with the logic. I don't see why restoring into an existing course is to be prevented when restoring into a new course is not.

Also....

I'm perfectly capable of deciding if I want backup files in my Site Files or not. IMHO the only thing that is required is a big red notice saying "these files can be accessed by anybody.... don't leave stuff here that's not for public viewing". After that it's (rightly) the admins problem.

Please stop trying to make arbitrary decisions for users. The issue is to do with the functionality of the files area and that will be sorted out in v2.0. The openness of the Site Files is not a backup issue.

Helen, I've already fixed the notices and implemented 1) above. I leave 2) for you as talked, thanks!

Howard, I can agree more or less with your assertion "The openness of the Site Files is not a backup issue", agree.

But, in the other side, one of the reasons (if not the main one) to have out there so many moodle servers having backup files in public area is, exclusively, that up to now, restore allowed to operate with those files. So admins abused that a lot (when the difficult of copying the files to SITEID is exactly the same than to copy them to course XX).

I really think that, by preventing the ability to restore from SITE files will help people to stop abusing that public dir, of course they will continue being free to user such area for other uses (as voluntarily "publish" backup files or whatever). But restore won't be one reason for that anymore. At least that is my POV.

Ciao

Eloy - unless I have mistaken something you are not preventing the ability to restore from SITE files. You are preventing restoring into EXISTING COURSES from SITE files. Restoring to a NEW course from SITE files is permissable. My experience is that restoring to NEW courses is (by a wide margin) the common practice. Therefore, I think that at the same time you have done almost nothing to prevent "abuse" (sic.) of public dir and provided considerable inconvenience and potential confusion by now allowing restore into EXISTING courses.

I just don't understand - it seems completely arbitrary. If you had said "NO restore from SITE files AT ALL" then I might not have liked it but I would have understood

This is not the way to get admins to change poor practice. It will just cause anger and annoyance.

I've commented in the forum Howard... http://moodle.org/mod/forum/discuss.php?d=123737

anyway, just for the records... did you know that the behaviour was introduced more than 13 months ago and nobody has claimed about that? Current changes in restore_check.php only are there to "ensure" that settings passed from previous steps are solid enough.

Ciao

My point exactly... the restriction doesn't do anything. Except on the one day of the year when I need to urgently restore into another course

The more I think, the more I'm towards complete prevention of restore from site files (alternative 1) in the forum:

• admins will need to start copying those files to other courses file areas. Easy workaround without losing functionality.
• site files only will be able to restore over site course (current).
• it will be warned in the early stages of restore.
• it will be warned in release notes

Ciao

+100 to fix the behaviour to the original behaviour and allow backups to be restored normally from site files.

The same argument for backups applies to ANY sensitive information. eg a file of grades or an assignment solution or your credit card records.

We can encourage users to avoid using it as a repository for course storage by:

• improving the warning at the top of the page to make it explicit about backups and the dangers of leaving them there
• adding a check to the security report for .zip files in the site files area, aksing admins to make sure they are not leaving course backups there

Re-resolving this a fixed (notices causing it to be reopened are out, thanks Helen). So no further action is necessary here.

The re-implementation of restore from site files will continue in MDL-19246...

Thanks everybody and ciao

Eloy, thanks for fixing. No more notices, so closing this issue.