problem with custom roles

problem with custom roles

by Laurent Domenech -
Number of replies: 4
Hi there,

When investigating with my newly defined capability (for the database module and MDL-6821), I came across something that may be a bug with the new roles. However, before submitting a tracker issue, I want to make sure I'm right.

The situation:
- On my dev site, I have created a new system-wide role : 'Editing Student', that inherits everything except it has the 'Write Protected Fields' (mod/data) set to Allow.
- Students and Guest have this 'Write Protected Fields' capability set to Prevent (since by default we don't want them to have it).

The problem(s):
1- If I give a student the 'Editing Student' role in my test course, (in addition to his Student role), he still doesn't have the 'Write Protected Fields' capability because the Student role has this capability set to Prevent (-1) and the sum with the same capability from 'Editing Student' is 0. (ref: accesslib.php line 584)

In other words, Allow + Prevent = Prevent.

2- If I just give the 'Editing Student' role to S1, he cannot access the course. He's given the option to enroll, which has the immediate effect to give him the Student role (back to 1).

My opinion on this is that Allow (local) + Prevent (system) should be equal to Allow (locally of course).

If anything is unclear, please let me know.

Regards,
Laurent

Average of ratings: -
In reply to Laurent Domenech

Re: problem with custom roles

by Yu Zhang -
Hi Laurent,

You can set the default of Student Role to inherit instead (0). Since there's nothing to Inherit by itself it will just work like a "prevent". Then the sum will be a 1 and the right capability will be set when the capabilities add up.

Cheers,

Yu
In reply to Yu Zhang

Re: problem with custom roles

by Laurent Domenech -
Thanks for your message.

Yes, this workaround works but IMHO it's not satisfying because it means you cannot use the ability to give users several roles without changing the default roles first. This pretty much makes it unusable.

Plus if I want to give two roles + student to a user, I would have to make sure they don't collide? I believe the intended behavior is to have specific capabilities override general capabilities, otherwise why would there be a Prohibit vs Prevent options? IMHO (again): Prohibit > Allow > Prevent.

Unfortunately I think that adding up the capability values to determine whether to give access or not isn't the best way to do it.
 
Regards,
Laurent
In reply to Laurent Domenech

Re: problem with custom roles

by sam marshall -
Picture of Core developers Picture of Peer reviewers Picture of Plugin developers
Yu's answer is of course correct and you should only use 'Prevent' in order to stop something that another role might have granted. (For example if a forum doesn't allow student contributions then you need to set some things to Prevent, because students normally are Allowed that.)

You can think of the options as numbers: Allow = +1, Inherit = 0, Prevent = -1, Prohibit = -999. (With a total score of '1' required to be allowed to do something.) It's not quite that simple because roles in different contexts have different priorities.

I should say this is partly my fault; moodle.com initially wanted to make Allow 'win'. But there are various conceptual reasons why it shouldn't; in general, defaulting to 'allow' is not a good security model in most systems. For example, let's suppose you want to make the following two roles:

Student [Allow forum:post, lots of other capabilities]
Visitor [Prevent forum:post]

Visitor is an 'extra' role; you can be a Student as well as a Visitor (saves defining every capability in Visitor and allows you to apply Visitor to different roles too).

This wouldn't work if Allow won out over Prevent. Now, you could use Prohibit, because that wins over everything always (it is really intended to be used only for 'banning' people for whatever reason)... but then what if you want to make a forum specifically for visitors, which sets forum:post to Allow again? It wouldn't work, whereas it will under the current system.


I think this type of discussion reveals that the roles system is a bit difficult to understand, which isn't a big surprise to anyone concerned, but still. smile Obviously the system has been implemented in such a way that you mostly don't need to care about it if you only want to use the existing roles...

It might be a good idea if the role definition page defaulted to a 'simple' mode that only has one column with checkboxes (on = Allow, off = Inherit), with the existing 'advanced' mode revealable via JS, or visible by default if any of the options are set to something other than those two.

That would encourage people not to click 'Prevent' when they don't need to. In most circumstances you only want Prevent in role overrides at the module level (i.e. when you're saying that students can't post to news forums, that's a Prevent), so the override screens should reveal Prevent (and maybe not Prohibit since it isn't needed at that level).

--sam
In reply to sam marshall

Re: problem with custom roles

by Laurent Domenech -
I can understand that the developers didn't want to implement a full classic ACL system but what they did is quite close and has misleading characteristics.

I agree with you on the fact that most of the times 'allow' should not win but that's why the contexts have been created, no?

Your example with visitors and students is interesting but it also shows that it could be interpreted several ways. To me you're either a visitor or a student. However, a visitor can have different rights depending on where he is. I believe there's a confusion between the concepts of 'role' and 'authorisation' but that's another story.

Anyway, if the local context was overriding the system context, there would be no problem. It's just my 2 cents and I'll adapt to whatever is there eventually... wink