Most of these issues have already been addressed by Brane,
but as he wished for my input, here it is.
These are the guiding principles for the 1.10 authz design:
(1) ACLs are only evaluated on a per-user bases; ACLs that
don't mention this user (or any of their groups) are ignored.
Rationale: We don't want to explicitly repeat inherited access
specs that don't change for the respective path / section.
(2) A more specific rule (fully) replaces any more general rule.
Rationale: You want to specify access exactly where it applies
and be sure that those are exactly the rights that will be applied.
(3) If there are multiple, equally specific rules, the last one
replaces any previous one.
Rationale: The last one, so you can specify catch-all denial ACLs.
Replacement, again, to ensure that exactly the rights specified
last will be in effect.
Those principles seemed quite reasonable and, most importantly,
workable with all the potential confusion caused by glob-rules.
Given that the previous authz code lacked similarly explicit
guidelines, I felt that any discrepancy between 1.9 and 1.10
would be a strong indication of a undesirable behavior in 1.9.
I still think this is true but we must neither break 1.9 authz.
On 20.07.2018 14:59, Philip Martin wrote:
> In 1.9 it was possible to repeat, or reopen, a section:
>
> [/some/path]
> user = r
> [/some/path]
> otheruser = rw
> This was equivalent to a single section:
>
> [/some/path]
> user = r
> otheruser = rw
>
> In 1.10 this is rejected by the parser and cannot be used. Is this a
> bug in 1.10 or an acceptable behaviour change?
Collides with rule (3), which is essential for reasoning on glob-rules.
Since it seemed an accidental feature inherited from the config parser,
we explicitly flagged it as invalid in 1.10. It would otherwise become
[/some/path]
otheruser = rw
> In 1.9 any repeat acl lines that were the exact same match, such as:
>
> [/some/path]
> user = rw
> user = r
>
> resulted in the last line overriding all the other lines, so user=r in
> the example above. In 1.10 the lines combine, so user=rw in the example
> above. Is this a bug in 1.10, or a bug in 1.9 that is fixed in 1.10, or
> an acceptable behaviour change?
Ouch. That is a bad one and an oversight in the design - I think.
According to (3), 1.9 behaves correctly. I guess we consider it
an unordered collection in 1.10 and then a union is the only thing
that approximates intent when a user is a member of different
groups with different access rights. Strict ordering becomes
very useful when assigning rights to groups:
[/some/path]
@Users = rw
@BadUsers = r
I guess the fix in 1.10 is simple but will change 1.10 behavior.
My proposal here:
* apply replacement semantics here as in 1.9
* error out / warn on repeated lines with the same user or group
(this is hardly ever intentional)
* provide a script / tool that takes 1.10 authz and checks for
changed behavior ("r" after "rw" rules etc.)
The last one is a bit of work but would be really handy.
> Finally, issue 4762. In 1.9 if both global and per-repository sections
> matched they were combined, so:
>
> [/some/path]
> user = rw
> [repos:/some/path]
> user = r
>
> resulted in user=rw. The issue 4762 problem is that in 1.10 the
> per-repository section overrides any global section, so user=r above. I
> believe this is a 1.10 bug and that the 1.9 behaviour should be
> reinstated.
According to (2), 1.10 behaves correctly: "user" has rw access,
except for a specific repository. I was not aware that 1.9 has
different behavior.
Now, the crux is how to unbreak 1.9. I suggest the following.
* Introduce a resolution option in the authz code:
- "union" (1.9 behavior)
- "most specific" (1.10 behavior)
- "error out" (new, will work in all non-ambiguous cases)
* default to the "error out"
* optionally specify the desired behavior as a config option
> Glob sections are new so they could have different behaviour from
> non-glob sections, but is that what we want? There is a wiki page
> https://cwiki.apache.org/confluence/display/SVN/AuthzImprovements but
> given issue 4762 I not sure whether it describes the correct behaviour.
>
User documentation on the new authz is inadequate.
-- Stefan^2.
Received on 2018-09-08 11:17:26 CEST