[PATCH] permissions determined by svn_authz aren't the same as determined by Subversion

[ash@…]

It's necessary for me to restrict access to areas of the Subversion repository using a Subversion path-based authorization file. Therefore, I need Trac to restrict access in the same way.

However, the Trac AuthzSourcePolicy doesn't interpret the authorization file in the same way that Subversion does. Specifically, Subversion will check all the entries for a particular path and grant access to a particular entity if any of those entries gives permission. Trac on the other hand uses the permissions for the first entry that matches and doesn't check subsequent entries.

Example: if I have a file along the lines of

[groups]
everyone = dave.public, joe.private

[repo:/]
@everyone = rw

[repo:/hidden]
@everyone =
joe.private = rw

then Subversion would allow joe.private access to /hidden, but Trac would deny it. Obviously, I would like the visibility to Subversion clients to be the same as the visibility in the Trac source browser.

Using Trac 1.0.1 with Python 2.7.

The attached patch appears to fix the problem for me. So far, I have only tested it with a minimal configuration like the one above.

[ash@…]

Patch to make svn_authz behave more like Subversion

[ash@…]

Note that I have been using this patch for more than two years now with a folder permissions structure that is reasonably complex. I haven't observed any further differences in how Trac and Subversion handle permissions.

[Jun Omae]

I'll verify it. We should add unit tests for it.

[Jun Omae]

In [c2ecc3ec0/jomae.git], applied the suggested patch and added unit tests. However, the unit tests fail. The patch is incomplete.

$ python setup.py -q test -s trac.versioncontrol.tests.svn_authz.suite
........F.......
======================================================================
FAIL: test_multiple_entries (trac.versioncontrol.tests.svn_authz.AuthzSourcePolicyTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/src/tracdev/git/trac/versioncontrol/tests/svn_authz.py", line 400, in test_multiple_entries
    self.assertPathPerm(True,  'joe',       '',      '/multiple/foo')
  File "/src/tracdev/git/trac/versioncontrol/tests/svn_authz.py", line 266, in assertPathPerm
    self.assertEqual(result, check)
AssertionError: True != False

----------------------------------------------------------------------
Ran 16 tests in 0.114s

FAILED (failures=1)

We could confirm behavior of svnauthz file using svn.repos.authz_check_access().

>>> from tracopt.versioncontrol.svn.svn_fs import _import_svn
>>> _import_svn()
>>> from tracopt.versioncontrol.svn.svn_fs import Pool, repos
>>> from trac.util.text import print_table
>>>
>>> with open('/tmp/svnauthz.txt', 'w') as f:
...     f.write("""\
... [/multiple]
... $authenticated = r
... [/multiple/foo]
... joe =
... $authenticated =
... * = r
... [/multiple/bar]
... * =
... john = r
... $anonymous = r
... [/multiple/baz]
... $anonymous = r
... * =
... jane = r
... [repos:/multiple/bar]
... joe = r
... john =
... """)
...
>>> def f():
...   pool = Pool()
...   authzdb = repos.authz_read('/tmp/svnauthz.txt', True, pool())
...   header = ['Path', 'anonymous', 'joe', 'john', 'jane', 'someone']
...   data = []
...   for repos_name in ('', 'repos'):
...     for path in ('/multiple/foo', '/multiple/bar', '/multiple/baz'):
...       line = [repos_name + ':' + path if repos_name else path]
...       line.extend(
...         repos.authz_check_access(authzdb, repos_name, path, user, repos.svn_authz_read, pool())
...         for user in (None, 'joe', 'john', 'jane', 'someone'))
...       data.append(line)
...   print_table(data, header)
...
>>> f()

Path                 anonymous  joe  john  jane  someone
--------------------------------------------------------
/multiple/foo        1          1    1     1     1
/multiple/bar        1          0    1     0     0
/multiple/baz        1          0    0     1     0
repos:/multiple/foo  1          1    1     1     1
repos:/multiple/bar  1          1    0     0     0
repos:/multiple/baz  1          0    0     1     0

[Jun Omae]

Proposed changes in jomae.git@00208b7fc:366e4330d (jomae.git@t11744).

[ash@…]

Thanks for looking at this and fixing it. On my site, I haven't (until now, but I expect that to change in future) used repository names in front of the path names in the authorizations file, just the path names on their own. Is that sufficient to explain why I haven't seen the fault that your unit tests have revealed?

[Jun Omae]

Before the changes, the policy component would allow john access to [repos:/path/to] with the following configuration:

[/path/to]
john = r
[repos:/path/to]
john =

However, svnauthz feature wouldn't allow john access to [repos:/path/to].

[Jun Omae]

Additional small changes in [30ed3e03f/jomae.git].

I'll push proposed changes if there are no objections from other developers.

[Jun Omae]

Committed in [15300] and merged in [15301-15302].

[Ryan J Ollos]

One small thing, on trunk we have mkdtemp​, which could be used in [15301#file2].

[Jun Omae]

Thanks for review and reminding. I forgot to execute svn ci on trunk ;-) I just pushed with your suggestion in [15302].