Edgewall Software
Modify

Opened 14 years ago

Closed 14 years ago

#9566 closed defect (fixed)

Improper handling of scoped repositories in svn AuthzSourcePolicy

Reported by: Itamar Oren Owned by: Remy Blank
Priority: high Milestone: 0.12.1
Component: version control Version: 0.12
Severity: major Keywords: svn authz
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

The AuthzSourcePolicy component uses the resource.id for matching a repository path in the authz file (trunk/trac/versioncontrol/svn_authz.py?rev=10005#L154), which is only the relative path from the scope in case of scoped repositories.

As a result, it is not possible to reuse the authz file created for serving the SVN repository, which uses full paths.

This can be fixed, at least for SVN-scoped-repositories, by replacing the mentioned line with:

for p in parent_iter(self.env.get_repository(resource.parent.id).scope + resource.id.strip('/')):

Attachments (3)

9566-scoped-repos-r10006.patch (13.8 KB ) - added by Remy Blank 14 years ago.
Support for scoped repositories.
9566-scoped-repos-2-r10006.patch (15.5 KB ) - added by Remy Blank 14 years ago.
Handle coarse permissions
9566-formal-scope-r10007.patch (2.9 KB ) - added by Remy Blank 14 years ago.
Formalize the repository scope.

Download all attachments as: .zip

Change History (12)

comment:1 by Remy Blank, 14 years ago

Milestone: 0.12.1
Owner: set to Remy Blank
Priority: normalhigh

by Remy Blank, 14 years ago

Support for scoped repositories.

comment:2 by Remy Blank, 14 years ago

Could you please test 9566-scoped-repos-r10006.patch? It should add support for scoped repositories. It also refactors the permission checking to remove duplicate code.

As CachedRepository already checks for a .scope attribute, I wonder if we shouldn't "standardize" this practice by defining a scope = '/' class attribute in Repository, so that the scope is always present and "empty" by default?

in reply to:  2 ; comment:3 by Itamar Oren, 14 years ago

Replying to rblank:

Could you please test 9566-scoped-repos-r10006.patch? It should add support for scoped repositories. It also refactors the permission checking to remove duplicate code.

Any chance you could generate the patch using svn instead of git? (or alternatively explain how I apply the patch to a svn working copy)

As CachedRepository already checks for a .scope attribute, I wonder if we shouldn't "standardize" this practice by defining a scope = '/' class attribute in Repository, so that the scope is always present and "empty" by default?

+1

in reply to:  3 comment:4 by Remy Blank, 14 years ago

Replying to itamaro:

Any chance you could generate the patch using svn instead of git? (or alternatively explain how I apply the patch to a svn working copy)

You can apply it normally with patch:

cd /path/to/working/copy
patch -p1 <9566-scoped-repos-r10006.patch

You may have to adjust the end-of-lines if you are on Windows (and find a suitable patch.exe).

comment:5 by Itamar Oren, 14 years ago

OK, it wasn't a svn-git issue with applying the patch, it was a branch-issue. I have trunk, and it appears that the patch is against branches/0.12-stable …

Anyway, I tried taking svn_authz.py from the branch (only that module, without touching any other one) and then applying the patch, which caused an exception on every request, similar to:

Traceback (most recent call last):
  ...
  File "c:\var\trac\trac-trunk\trac\versioncontrol\svn_authz.py", line 148, in check_permission
    if (resource.realm, action) in self._handled_perms:
AttributeError: 'NoneType' object has no attribute 'realm'

by Remy Blank, 14 years ago

Handle coarse permissions

comment:6 by Remy Blank, 14 years ago

Right, I forgot to handle coarse permissions. Please try 9566-scoped-repos-2-r10006.patch.

in reply to:  6 comment:7 by Itamar Oren, 14 years ago

Replying to rblank:

Right, I forgot to handle coarse permissions. Please try 9566-scoped-repos-2-r10006.patch.

That seems to do the trick. This works for me now.

Thanks!

comment:8 by Remy Blank, 14 years ago

Thanks for testing. Patch applied in [10007].

That leaves the question of formalizing the scope, something like 9566-formal-scope-r10007.patch. All tests pass.

by Remy Blank, 14 years ago

Formalize the repository scope.

comment:9 by Remy Blank, 14 years ago

Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.