Moodle

New capability for "safe" overriding

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.7
  • Fix Version/s: 1.9.3
  • Component/s: Roles / Access
  • Labels:
    None
  • Affected Branches:
    MOODLE_17_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

At present only admins can use moodle/role:override by default - the reason is security.

We could create a new capability moodle/role:safeoverride - and enable it for normal teachers, it would allow them to override only those capabilities without RISK_PERSONAL | RISK_XSS | RISK_CONFIG in roles they can assign

we could then move some more module and course settings into roles.

time needed for implementation and testing: cca 1 day

Issue Links

This issue has a non-specific relationship to:
MDL-15841 moodle/role:safeoverride should be Not set by default in the Teacher role. Major Closed
This issue has been marked as being related by:
MDL-15737 skodak's list of bugs - work for tul Minor Closed
MDL-15805 document safeoverrides in docs wiki Minor Closed
MDL-8524 Roles administration improvements - META Minor Closed
MDL-7859 Capability definition should include list of contexts where usable Minor Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Martin Dougiamas added a comment -

Can you hold off on this for a while ... I'm a little concerned about the added complexity for GUI and code and documentation and training ...

Show
Martin Dougiamas added a comment - Can you hold off on this for a while ... I'm a little concerned about the added complexity for GUI and code and documentation and training ...
Hide
Permalink
Petr Škoda (skodak) added a comment -

As you wish

  • code - new much shorter patch attached, only minimal relevant changes in code; code is fully tested and working fine for me
  • gui is the same
  • documentation & training- now disabled by default in patch; would be much easier if we fix MDL-7859
Show
Petr Škoda (skodak) added a comment - As you wish
  • code - new much shorter patch attached, only minimal relevant changes in code; code is fully tested and working fine for me
  • gui is the same
  • documentation & training- now disabled by default in patch; would be much easier if we fix MDL-7859
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Sounds interesting (+1) , although I've some "use-related" things to do.

If I would be a "normal" Moodle user, I Think I'll found quite frustrating to go to the module->update page to configure somethings about the module and then, go to the roles/caps page of the module to adjust others.

So I would propose to have ALL the settings in the module page, no matter if they are going to be module settings or capability settings.

Then, both add and update will perform the required tasks, either in the module or in capabilities. I really think we should follow this behaviour. It will allow teachers to ignore all the underlying structure and will make things consistent, instead of having to know WHERE is every setting and what it means.

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Sounds interesting (+1) , although I've some "use-related" things to do. If I would be a "normal" Moodle user, I Think I'll found quite frustrating to go to the module->update page to configure somethings about the module and then, go to the roles/caps page of the module to adjust others. So I would propose to have ALL the settings in the module page, no matter if they are going to be module settings or capability settings. Then, both add and update will perform the required tasks, either in the module or in capabilities. I really think we should follow this behaviour. It will allow teachers to ignore all the underlying structure and will make things consistent, instead of having to know WHERE is every setting and what it means. Ciao
Hide
Permalink
Petr Škoda (skodak) added a comment -

sending latest patch, should be production quality:

1/ the safe overrides now share the same configuration matrix with normal overrides + improved description text above
2/ added safe override notice explaining the locked caps - bellow the capabilities table
3/ improved performance for both assignable and overridable roles

Show
Petr Škoda (skodak) added a comment - sending latest patch, should be production quality: 1/ the safe overrides now share the same configuration matrix with normal overrides + improved description text above 2/ added safe override notice explaining the locked caps - bellow the capabilities table 3/ improved performance for both assignable and overridable roles
Hide
Permalink
Martin Dougiamas added a comment - - edited

I tried the patch and it is really nice to use from the overrides screen. I think it's a great addition. +1 for 1.9.3

Two questions about the admin setup side of it though:

1) Query: If you have override and safeoverride is it exactly the same as having override alone? I assume so.

2) Minor Gui thing: The override and safeoverride are not next to each in the list (sort order?)

3) Major GUI thing: The mix of the matrix and the two override caps is a bit confusing ... since the matrix is there as protection can we not just make at least safeoverride as ALLOW by default for all roles? Then we can be more explicit on the matrix screen about what to do if you want to allow people to override unsafe things. Or perhaps better, can we allow setting of the override/safeoverride cap from the matrix screen (checkboxes under role name on left)?

Show
Martin Dougiamas added a comment - - edited I tried the patch and it is really nice to use from the overrides screen. I think it's a great addition. +1 for 1.9.3 Two questions about the admin setup side of it though: 1) Query: If you have override and safeoverride is it exactly the same as having override alone? I assume so. 2) Minor Gui thing: The override and safeoverride are not next to each in the list (sort order?) 3) Major GUI thing: The mix of the matrix and the two override caps is a bit confusing ... since the matrix is there as protection can we not just make at least safeoverride as ALLOW by default for all roles? Then we can be more explicit on the matrix screen about what to do if you want to allow people to override unsafe things. Or perhaps better, can we allow setting of the override/safeoverride cap from the matrix screen (checkboxes under role name on left)?
Hide
Permalink
Petr Škoda (skodak) added a comment -

1/ yes - normal override has higher priority - you need to prohibit both if you want to prevent access completely
2/ going to work on that list ordering too in separate issue

Show
Petr Škoda (skodak) added a comment - 1/ yes - normal override has higher priority - you need to prohibit both if you want to prevent access completely 2/ going to work on that list ordering too in separate issue
Hide
Permalink
Petr Škoda (skodak) added a comment - - edited

1/ fixed risk definitions in access.php's
2/ added allow override editingteacher -> student, guest and non-editing teacher (new install and 1.6 upgrade)
3/ enabled safeoverride by default for editing teachers
4/ hardcoded exception for course and category delete and reset - these do not have risks, but are not "safe"

Expected commit day is Wednesday

Show
Petr Škoda (skodak) added a comment - - edited 1/ fixed risk definitions in access.php's 2/ added allow override editingteacher -> student, guest and non-editing teacher (new install and 1.6 upgrade) 3/ enabled safeoverride by default for editing teachers 4/ hardcoded exception for course and category delete and reset - these do not have risks, but are not "safe" Expected commit day is Wednesday
Hide
Permalink
Petr Škoda (skodak) added a comment -

added new risk RISK_UNSAFEOVERRIDE instead of the hardcoded list

Show
Petr Škoda (skodak) added a comment - added new risk RISK_UNSAFEOVERRIDE instead of the hardcoded list
Hide
Permalink
Petr Škoda (skodak) added a comment -

sending latest patch, the safeoverrides cap is not enable by default for any role, I am bit afraid because in might have some unexpected results on existing sites.

Show
Petr Škoda (skodak) added a comment - sending latest patch, the safeoverrides cap is not enable by default for any role, I am bit afraid because in might have some unexpected results on existing sites.
Hide
Permalink
Petr Škoda (skodak) added a comment -

RISK_DATALOSS is a better name, thanks Martin!

Show
Petr Škoda (skodak) added a comment - RISK_DATALOSS is a better name, thanks Martin!
Hide
Permalink
Petr Škoda (skodak) added a comment -

code in cvs, assigning to Helen, please update docs

Show
Petr Škoda (skodak) added a comment - code in cvs, assigning to Helen, please update docs
Hide
Permalink
Petr Škoda (skodak) added a comment -

I have file a separate issue for docs, closing now
thanks!

Show
Petr Škoda (skodak) added a comment - I have file a separate issue for docs, closing now thanks!
Hide
Permalink
Martin Dougiamas added a comment -

I took the defaults out of 1.9 again : MDL-15841

Show
Martin Dougiamas added a comment - I took the defaults out of 1.9 again : MDL-15841
Hide
Permalink
Helen Foster added a comment -

Petr, thanks a lot for creating the safe overrides capability

Show
Helen Foster added a comment - Petr, thanks a lot for creating the safe overrides capability

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.