On 13 May 2015 at 15:24, C. Michael Pilato <cmpil...@collab.net> wrote: > On 05/13/2015 02:21 AM, Stefan Fuhrmann wrote: >> Hi devs, >> >> At WANdisco, people have been playing with the new >> wildcard support for authz (see authz-performance branch) >> and ran into an interesting problem. > > [Details snipped. Welcome to 2004, where CollabNet ran into the same > issues with the proprietary regexp-based authz implementation we shipped > in our enterprise products.] > >> My suggestion is to relax the requirements as follows: >> COPY & MOVE still require recursive read access on >> the source but only non-recursive write access on the >> destination. Thus it becomes possible to protect data >> in branches and tags right from their inception without >> relaxing read access requirements to existing data. The >> attached patch achieves just that. >> >> I have 3 questions: >> >> (1) Is there something fundamentally wrong with this >> approach, e.g. braking major use-cases? >> (2) Should we require write access to target and target's >> parent folder (as done by the patch) or just to the target's >> parent folder? >> (3) Should we (optionally?) reduce the access rights reqs >> for DELETE similarly such that no recursive write access >> is needed? That seems risky but would be symmetrical >> to creating copies with r/o or n/a sub-paths. MOVE would >> be updated accordingly (always the same reqs as a COPY >> + DELETE). > > Well, the use-case being broken here is kinda the obvious one: I > shouldn't have permission to create/delete some path /foo/bar (it's a > system-critical file that shouldn't go away, or a password-bearing file > path blocked by the VC system because devs keep stupidly committing > them, or...), but you've provided and end-around for me to do so. :-) > In the copy and move scenarios, you're allowing me to create a path I > can't later delete or further modify. Now, while this is a very handy > semantic for tag creation[1], it's also something an administrator > should be able to make a conscious decision about, not just an > unfortunate side-effect of an effort to make the code faster.
This proposal really has nothing to do with performance; it's about intended/consistent authz semantics, which we never really bothered to define, let alone prove. IMO, the fact that you can have authz rules that allow you to create a directory but not its children is unintended. I bet no-one thought about it during the initial development of mod_authz_svn. > Subversion's path-based authz subsystem is about paths, not nodes as we > devs think of them. Our users have generally not been asked to consider > a copy, addition, or deletion of an item to be changes made to the > parent of that item. So the changes you're suggesting are not merely > ones of functionality, but ones of philosophy. Interesting point, but I disagree: we're still talking about authz in terms of paths, the proposed change just removes the restriction that you cannot copy something to a location where you can create something. Currently create requires non-recursive write access to the target, but copy (and move) require recursive write access. IMO, that's inconsistent and unintuitive, something I'd classify as a design bug rather than a change in philosophy. > I lean towards thinking > that falls outside the scope of acceptable changes in behavior in the > 1.x line *unless* you can find a way to, via configuration, allow > administrators to explicitly toggle this new paradigm. Assuming we agree that it's not a new paradigm, would proper documentation, as I proposed, be sufficient? > [1] Along with regexp access rules, CollabNet has for many years > supported this tag creation use-case by splitting the "write" permission > into multiple ones: "add", "copy", "delete", and "modify". Something > Subversion should consider? That's what the authzperf branch is for, indeed. Hopefully many of these things will land in 1.10. -- Brane
