Menu
▾
▴
Re: [TWiki-Dev] Access Controls
|
From: Kenneth L. <ke...@la...> - 2007-02-28 17:59:19
|
Crawford Currie wrote:
> Last night I was invited to an IRC conference to discuss the recent
> issue with the interpretation of access controls. I gave up and left
> the conference in sheer frustration at having yet again to explain the
> issues, in an environment that was once again refusing to hear
> anything except "Cairo compatibility". After sleeping on it, I decided
> to make one last attempt to explain ACLs again, for the benefit of all
> of TWiki-Dev.
It is sad that you left instead of staying and listening what I had to
say. If you had stayed you would have understood what I wanted instead
of assuming what I wanted.
>
> For those not familiar with the recent discussions, this relates to
> the interpretation of an empty value in an access control setting.
> This interpretation was intended to change between Cairo and TWiki-4,
> as described in the release notes and documentation, but a bug crept
> in [1] which meant that the interpretation of ALLOW didn't change as
> planned.
I am happy the code was changed because I do not like the spec.
>
> Note that this is *not* intended as the opener for a debate about
> access controls. If you want to raise new proposals, please do so in
> Codev, and good luck finding someone to implement them.
>
>
> TWiki ACLs
>
> You have to think of TWiki ACLs in terms of two lists. One list is a
> list of people / groups who are *denied* access to something, the
> other is a list of people / groups who are *allowed* access.
>
> There has to be a sequence between these two lists, so that if a user
> exists in both lists, there is a decision-making order. The order is
> DENY then ALLOW.
>
> Now, you need operations to manipulate these lists. Since there is no
> way in TWiki to *add to* or *remove from* a list defined in a TWiki
> variable, the manipulations have to be in terms of *redefining* the
> lists. * Set ALLOW and * Set DENY are those operations.
>
> Further, there needs to be a priority in the way these lists are
> built, so that global (web) permissions can be overridden at the local
> (topic) level.
>
> OK, so far so uncontroversial.
Yep, agree
>
>
> The Cairo Permissions Model
>
>
snipped out to make this mail shoter
>
>
> Hierarchical Webs
>
Snip - agree that hierarchical web feature required that the access
control model was changed. We could not stay with working with Cairo. I
never said that.
>
>
> The Dakar Permissions Model
>
snip
>
> The reason I selected this model was that I felt it *minimised* the
> impact on users upgrading from Cairo. A change was going to have to
> happen. All they had to do was remove the spurious empty settings from
> their WebPreferences topics. At least this way the changes were
> minimal, and the interpretation of permissions was normalised to the
> expectations set by preferences.
Access rights and their indirect impact is hard to overview and I
personally never noticed the issues that I now react againt.
>
> Reviewing the use cases for this model:
>
> A web in which all users are *denied* access. Just allow no-one
>
> o Set ALLOWWEBx =
> * How do you grant a single user / small number of users access to
> a single topic?
> o Set ALLOWTOPICx = SameAsCairo, ListTheUsers
> * How do you grant all users access to a single topic? Simple,
> deny no-one:
> o Set DENYTOPICx =
>
> Now, consider a web in which all users are *allowed* access (i.e.
> no-one is denied)
>
> * Set DENYWEBx =
>
> * How do you deny a single user access to a single topic? DENY them
> o Set DENYTOPICx = TheDeniedUserSameAsCairo
> * How do you deny all users access to a single topic? Just ALLOW
> no-one
>
> * Set ALLOWTOPICx =
>
You have some very good use cases. And the Cairo model sucked with
respect to these use cases.
What I cannot accept in short.
I cannot accept that
* Set DENYWEBCHANGE =
* Set ALLOWWEBCHANGE = Someone
means allow anyone access because the DenyWebChange = blank means Deny
noone and overrule the Allow setting. This is so geeky and so much
reverse logic that noone really understands this even when you read the
docs. An empty Deny should not overrule a defined Allow.
Cairo webs had all 6 Web settings defined as blanks and upgraders will
be left with a wide open web thinking that they are protected. This I
cannot accept either.
And fortunately I do not have to worry. Because as I said yesterday - 10
seconds before you slammed the door on us - it does not work as you say
it works.
A Set DENYWEBCHANGE = blank does not allow anyone access!!! You can
argue from here till eternity how it should work. But test it yourself.
It does not work that way - thank God. So upgraders are not left with
wide open webs. And I am not in a state of horror.
Same with DENYWEBVIEW and DENYWEBRENAME. At a base web (not subweb)
setting all these blank means the same as commenting them out.
All is exactly like I want it to be.
Going to topic level things change.
* Set DENYTOPICVIEW =
* Set ALLOWTOPICVIEW = Yes
* Result: Anyone can see the topic and even edit it. I would expect
only <http://merlin.lavrsen.dk/twiki41/bin/view/Main/YeS>YeS to
have access.
But I am not panicing because of this because I doubt many users will
have defined such topic prefs in practical. Unless they want to take
advantage of the Dakar behavior.
Same with
* Set DENYTOPICCHANGE =
* Set ALLOWTOPICCHANGE = Yes
<http://merlin.lavrsen.dk/twiki41/bin/view/Main/YeS>
* Result: Anyone can see the topic and even edit it. I would expect
only YeS to have access.
And again I do not panic. I just find the behavior silly. I do not
understand this reverse logic. Here I prefer that things work same way
as with ALLOWWEB/DENYWEB and like it worked in Cairo.
It is pretty easy to revert the behavior back to Cairo. Just remove 4
lines of code.
Index: Access.pm
===================================================================
--- Access.pm (revision 12993)
+++ Access.pm (working copy)
@@ -157,10 +157,6 @@
#print STDERR $this->{failure},"\n";
return 0;
}
- } else {
- # If DENYTOPIC is empty, don't deny _anyone_
- #print STDERR "DENYTOPIC is empty\n";
- return 1;
}
}
And for a moment I felt happy and thought - yes that is what we do.
BUT BUT BUT !!!!! then I continued testing all night (only got two hours
of sleep) and came to very similar conclusions as Crawford gives. The
Cairo behavior sucks when it comes to closing an entire web and then
grant access to a selection of topics. You cannot do this in practical.
So something has to change. And the way Dakar implents this on
DENYTOPICXXXX enables you to cancel a strict web level access right. But
I do not like the syntax. But I do not want to just revert back to Cairo
ad loose this important new feature.
> My Position
>
> While I understand and subscribe to the goal of compatibility with old
> releases, there are instances where things *have* to change in order
> to work. This is one of those instances; Cairo compatibility is not an
> option, because Cairo was *broken*. I have no objection whatsoever to
> an /improvement/ in the permissions system. I don't even object that
> strongly to the proposal to change the doc of the ALLOW setting to
> compensate for the introduced bug, despite the confusion it engenders,
> because I can still do what I need to. But /reverting to a broken
> model is a bad idea.
>
> /If you are determined that upgrading from Cairo is still such a big
> issue, then you may want to consider Michael Sparks' 2003 proposal for
> an implicit /all users/ pseudo-group (see [6]). At least that wouldn't
> stuff anyone who has adopted TWiki-4. This pseudo-group would be
> synonymous with the current "empty" setting, so instead of
>
> * Set DENYTOPICCHANGE =
>
> (i.e. "deny nobody") you would write
>
> * Set ALLOWTOPICCHANGE = *
>
> (i.e. "allow everybody"). Of course all TWiki-4 adopters would have to
> change, and all default webs, and the code as well. And the doc. And
> you would still have to use the DENY = x hack to override a DENY set
> at a higher level.
I came very much to a similar conclusion.
I think we should introduce in 4.2 a wild card syntax. I would like to
see an all authenticated users group (all users except guest) and the *
for all users. That would give the flexibility and a syntax that is
logical. I would not implement the ALLOWTOPICCHANGE = blank. And I would
somehow like to deprecate/remove the DENYTOPICCHANGE = blank behavour.
But Dakar/Edinburgh has been out for a year now and some people will
have used the DENY = blank so we cannot just remove it.
For 4.1.2 I propose leaving code as it is.
Document the behavour as it is.
Warn that the DENYTOPICXXXX = blank syntax is deprecated and will be
replaced by something better.
Then in 4.2 we can decide if we want to have a configurable legacy mode
for the DENY = blank syntax
We need time to think and discuss this properly. Especially the subweb
behavior needs lots of thinking because if we do it wrong the whole idea
of subwebs go down the drain. Cairo behavior is not an option.
Finally - on my Merlin test server I documented my testing and my
thoughts. This is from before I slept and I was tired so the tone may be
sharp. But take a look. There are 6 hours of test results and it may
trigger some ideas.
http://merlin.lavrsen.dk/twiki41/bin/view/Myweb/AccessRightsTest
And if my test server is dead I mirrored it on my production server.
http://www.lavrsen.dk/twiki/bin/view/Kenneth/AccessRightsTest
If you want to try things yourself remember to not test as an admin.
That gives false results on who is grated access. You need two users.
One that gets access and one that does not. I called mine YeS and NoOb.
Kenneth
|