the curiously strong allow - a roles/overrides use case

the curiously strong allow - a roles/overrides use case

by Martín Langhoff -
Number of replies: 21

Just found myself in an interesting roles/overrides situation. Scenario

  • Administrator whats to show a moodle block on the sitecourse only to staff
  • "Staff" is a role assigned sitewide by the auth plugin
  • My master plan: go to the block's override, set overrides to "prevent" for autenticated user and other interlopers, and "allow" for staff.

Of course, it doesn't work. Why? The staff user is also a logged in user, so the 2 cancel eachother. Even locality rules won't help:

  • sitewide RA as authenticated user with capability moodle/block:view - allow sitewide and prevent override at the block level
  • sitewide staff with capability moodle/block:view - allow sitewide

Moodle will resolve each role independently, and so that boils down to

  • sitewide authenticated user -- capability moodle/block:view - prevent
  • sitewide staff -- capability moodle/block:view - allow

I thought for a while that this had been changed by the locality changes in v1.9 that I made (see http://docs.moodle.org/en/Development:Roles#Capability-locality_changes_in_v1.9 ) but this case always has matching locality on the "authenticated user" role, so deny wins.

This only happens because the capability is normally granted to all users, and we are selectively reversing that and trying to apply an exception to that reversal. The exception has the same weight as the reversal, and it loses.

I am not sure what the right solution is. In the meantime, I've crafted a patch that introduces "Allow+", which has a "permission" value of 2, so it trumps the reversal. It's a curiously strong allow : it can win over a prevents, but a prohibits will still trump it.

Edit: attached the patch

Average of ratings: -
In reply to Martín Langhoff

Re: the curiously strong allow - a roles/overrides use case

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
I think the solution is not not use prevent.

Override the authenticated user role so that it gets capability moodle/block:view - inherit.

Doesn't that work?
Average of ratings: -
In reply to Tim Hunt

Re: the curiously strong allow - a roles/overrides use case

by Séverin Terrier -
Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Testers Picture of Translators
Hi,

I would better say to overide by using Prohibit, than inherit...

Hope this helps
Average of ratings: -
In reply to Tim Hunt

Re: the curiously strong allow - a roles/overrides use case

by Dan Poltawski -
Override the authenticated user role and all other roles which have moodle/block:view - allow to inherit - otherwise it wouldn't work, or you'd get the same problem with a user with two roles?

I thoguht i'd tried this exact scenario and worked in the way Martin described though. Hmm its quite tricky with these permissions which want to be negative..
Average of ratings: -
In reply to Tim Hunt

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -
A "local" inherit for authenticated-user in the specific block instance in the sitecourse will still inherit from authenticated-user systemwide.

Of course, I could say "inherit" to the sitewide definition of authenticated-user. But then I'll have to re-add it in every top-level context (top categories for example) and in every block I want to show in the sitecourse. Which is a lot of long-term pain for a small "exception".

(As part of the accesslib rework I rewrote the implementation of this. So by now I think I understand most of the nuances there...)
Average of ratings: -
In reply to Martín Langhoff

Re: the curiously strong allow - a roles/overrides use case

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
In that case I don't understand why it doesn't work.

If Staff and Authenticated user are both overridden in the block to allow and prevent, so they cancel at that level, why does it not then inherit from the sitewide definition of the two roles, which then gives allow?
Average of ratings: -
In reply to Tim Hunt

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -

As I wrote above "Moodle will resolve each role first". It doesn't do the resolution horizontally first at each level. It first resolves it vertically the capabilities that apply to each role that you have in the context. And then it will look at whether the roles are in conflict.

has_capability_from_accessdata() is the long-named-function that walks the data structure thus. It does this mode of resolution because:

  • It's what 1.7/1.8 did anyway - the change would be significant...
  • It's IMHO saner with all the cases we have to support
  • It's faster (walks the data structure the natural way, and offers cases where you can shortcut)
Average of ratings: -
In reply to Martín Langhoff

Re: the curiously strong allow - a roles/overrides use case

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
So, as I see it, 'Prevent' always trumps 'Allow' if you have rules that provide both? I thought the 1.9 rules were supposed to change that so 'Allow' would override 'Prevent', but not 'Prohibit'?

It makes sense to have the strongest setting be the one that removes access (Prohibit), rather than one that allows (Allow+). I think the correct thing here is to allow 'Allow' to override 'Prevent' when there both are available at the same level.
Average of ratings: -
In reply to Mike Churchward

Re: the curiously strong allow - a roles/overrides use case

by Séverin Terrier -
Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Testers Picture of Translators
I remember (and it's still in the french version) that in old versions of the roles documentation, it was explained that "Allow" and "Prevent" are killing each other if assigned at the same context.

If one role "Allow" (say +1), and another role "Prevent" (say -1), you still have 0 at the end...
if you have a third role, it will be determining (unless "Inherit"), or permissions will be checked at a higher level (if "Inherit" for this third role)...

Hope this helps
Average of ratings: -
In reply to Séverin Terrier

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -
Exactly. This new Allow+ has "+2", so you need 2 prevents to counter it wink
Average of ratings: -
In reply to Mike Churchward

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -
The change in 1.9 has to do with locality, not with the value.

Prevent + Allow cancel eachother and refer to a "higher" (less local) capability assignment to clear up the situation. Still, if you get conflicts and no capability assignment to resolve it, the default is to no allow things. So "no" tends to win.

Allow+ gives the allow a slightly stronger stance. Hence the subject...
Average of ratings: -
In reply to Martín Langhoff

Re: the curiously strong allow - a roles/overrides use case

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
I am 100% against adding a whole new permission like Allow+ (200% if you want a curiously strong vote)... it just complicates everything beyond belief.

Because the SITE/Frontpage is under the SYSTEM level, you can use a PREVENT for all roles at SITE and turn on ALLOW overrides for each block as required.
Average of ratings: -
In reply to Martin Dougiamas

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -

smile

I'm not entirely happy with that patch either. Not 200% against, but bad-taste-in-my-mouth...

Because the SITE/Frontpage is under the SYSTEM level, you can use a PREVENT for all roles at SITE and turn on ALLOW overrides for each block as required.

After a bit of mulling, testing and looking... it does work and I think it's not too burdensome. For anyone trying to do this in the sitecourse (aka "front page"):

  1. As admin, go to admin->front page->front page roles->overrides
  2. Set a "deny" override for the authenticated user, and also for guest.
  3. Note - this depends on having "none" as the "front page default role" in the "front page settings" page. Check it!
  4. Now, all the blocks are invisible to a mere logged in user, and to someone not logged in. The "default stance" for blocks in the sitecourse is hide.
  5. Turn editing on
  6. In each block, add an override with "moodle/block:view=allow" for each role that should see this block.
  7. Highly recommended, have 2 or 3 different webbrowsers logged in as users with different roles. Refresh each of them to check that the appropriate blocks are visible.

For a plain old course, do step 1 & 2 as a course-level override, and skip step 3.

(I had wondered about this strategy yesterday, but thought I'd have to add those overrides in the DB by hand. Finding that there is a Front Page Roles page was a bit of an aha! moment. Thanks!)

Edit: Anyone keen on reformatting this for a "roles tips and tricks" page in the wiki?

Average of ratings: Useful (1)
In reply to Martín Langhoff

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -

After a bit of mulling, testing and looking... it does work

..does it? I just found out that my tests worked because I still had the "2" permission stuck there there.

Actually, once I removed the magical "2" permission... it doesn't work. With the 1.9 code (and stepping through the code), a more local rolecap wins, but only within that role.

So with the plan above, there are 2 roles:

  • Staff that gets a +1 from the very local roledef at the block level
  • Authenticated user, which gets a -1 from the course-level "prevent"

But the code that does the final addition does not consider locality of the roledef, only locality of the role-assignment. Both RAs are at the SYSTEM context, so they are evenly matched.

The analysis in the opening post of this thread is right again. Locality matters in resolving rolecaps per-role.

It's all back to the discussion we had under bug MDL-11218 ... and I guess it was wrong to drop that aspect of role locality. The issue is relatively easy to address inside of has_cap_fad() but makes it pretty near impossible to do work in pure SQL. sad

Average of ratings: -
In reply to Martín Langhoff

Re: the curiously strong allow - a roles/overrides use case

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
Bugger, I forgot that change in 1.9! I don't think rewriting that bit is really an option at this point unless someone suddenly has a massive performance-retaining brainwave.

But you do have some other easy options:
  1. You can assign roles to people directly in the block or course, or
  2. If it's a custom block, you can create a new capability just for that block (in blocks/myblock/db/access.php), and set that capability to ALLOW for your staff role. The block code needs to obviously check for the capability before displaying anything.

Average of ratings: -
In reply to Martin Dougiamas

Re: the curiously strong allow - a roles/overrides use case

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
When I had a need of showing a block only to certain roles, I did 2, and it worked and was easy to understand.

I think that all blocks should have their own view capability for this reason.
Average of ratings: -
In reply to Tim Hunt

Re: the curiously strong allow - a roles/overrides use case

by sam marshall -
Picture of Core developers Picture of Peer reviewers Picture of Plugin developers
Agreed with Tim - I think the 'capability specifically for block' is the right solution.

Adding another permission level is about the last thing Moodle needs imo. smile Even though I agree there's no easy way (without doing the 'add override for every other role' thing) to achieve this specific situation in the current system.

--sam
Average of ratings: -
In reply to sam marshall

Re: the curiously strong allow - a roles/overrides use case

by Dan Poltawski -

I think i'm misunderstanding here, so forgive me..

But assuming the example of the HTML block, where the default would be to allow all roles to see your block. But occasionaly you might want to role-restrict it, wouldn't you be faced with the same problem of needing the problematic override?

Average of ratings: -
In reply to Dan Poltawski

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -
Yup. I suspect that you'd be in the same problem. That's why my 2nd patch needs testers wink
Average of ratings: -
In reply to Martin Dougiamas

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -

unless someone suddenly has a massive performance-retaining brainwave

Not quite a brainwave, but I sure got a patch for this. Works pretty well, and it fixes a suble bug with CAP_PROHIBIT too.

With this patch, locality of RAs still reigns supreme, but if you have 2 RAs in conflict at the same context, the locality of the rdef wins.

The commit msg says more: Subject: [PATCH] accesslib: has_capability_in_accessdata() respects rdef locality a bit more

With this patch, we respect rdef locality when two roles assignments in the same context have conflicting rdefs. In that case, the most local rdef wins.

For a use case, see the discussion here http://moodle.org/mod/forum/discuss.php?d=84472

Notes:

  • If we wanted to have locality of RDEF trump everything we can. A comment in this patch shows how.

  • I don't know how to reproduce this in pure SQL.

And Also:

This patch also fixes a bug where if CAP_PROHIBIT was set and another role added to it in the same context, we would add or substract 1 to CAP_PROHIBIT, and it would lose its magic.

And while at it, tighten the code to avoid casts. All the ints are unambiguously ints.

Average of ratings: -
In reply to Martín Langhoff

Re: the curiously strong allow - a roles/overrides use case

by sam marshall -
Picture of Core developers Picture of Peer reviewers Picture of Plugin developers
Mmm. This seems reasonable. I'm trying to remember the use case why this wasn't in the original spec, but all I can come up with now is rather contrived:
  • Mary is a Student on course CF101.
  • Student has capability blah not set (Inherit)
  • 3 specific module instances do Allow Students to blah. (Override at module level to allow blah.)
  • Mary has been naughty with her blah-ing, so we assign her a Naughty role at course level. This sets blah to Prevent.
  • This role worked in the old system (the Prevent and Allow cancelled each other out, leaving 0 = no permission). If you do locality, though, the Allow now wins.
You could just make Naughty use Prohibit, but I think the theoretical situation there was that what if in one particular (assessed, presumably) activity, we actually did have to allow Mary to blah... you can't override Prohibit, so that isn't possible any more. (In the old system it was possible by overriding Naughty to Allow blah in the necessary activity.)

In practice I think using Prohibit would probably be OK. Can anyone else come up with a more convincing use case for why the current system is needed and overrides should take effect at same level as role?

Note: To clarify what I think Martin L is doing, both in case I'm wrong and so it might make more sense to others:

1. More specific definitions (e.g. at module level instead of course) usually 'win'.
2. (The current situation.) However a role override takes effect at the same level where the role was assigned (e.g. the course), and not at the level where the override was added. So it is only equally powerful to the role-assigned permissions, not more powerful.
3. Martin L is proposing changing this so that role overrides take effect at the level where the override occurs (meaning an override always wins, unless there is a Prohibit somewhere).

If you think it would be useful, on Monday I can try to find the original role specs, which had some use cases for various odd things.

--sam
Average of ratings: -
In reply to sam marshall

Re: the curiously strong allow - a roles/overrides use case

by Martín Langhoff -

Sam,

your first explanation is fine describing a "single role" setup, but note that nothing has changed on that front between 1.7,1.8 and 1.9 with or without my patch.

It might be a good idea to re-read my earlier posts.

Note that things are tricky because you are rarely in a single role situation.

When you say

More specific definitions (e.g. at module level instead of course) usually 'win'.

We have to differentiate RA from RDef. Locality of RA is king (except for CAP_PROHIBIT). If you are "authenticated user" sitewide, and "student" at the course level, and authenticated user has X:allow, but student has X:deny, it's deny.

In my performance patches, I removed all importance from locality of RDef, but I now think that it was wrong. The patch that I am proposing brings it back, in the following manner:

When you have 2 conflicting RAs at the same scope, the locality of the relevant RDef wins. This is a bit cryptic, I think it is better explained in my earlier posts.

Martin L is proposing changing this so that role overrides take effect at the level where the override occurs

No.

See the example use case about block visibility that opens this discussion, and MD's proposal that failed to work. Might help clarify -- if not, let me know.

Edit: Sorry about the negative tone -- perhaps my explanations earlier in this thread are jumbled. This stuff is hard to explain and visualise...

Average of ratings: -