Edgewall Software
Modify

Ticket #9566 (closed defect: fixed)

Opened 18 months ago

Last modified 17 months ago

Improper handling of scoped repositories in svn AuthzSourcePolicy

Reported by: itamaro Owned by: rblank
Priority: high Milestone: 0.12.1
Component: version control Version: 0.12
Severity: major Keywords: svn authz
Cc:
Release Notes:
API 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

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

Download all attachments as: .zip

Change History

comment:1 Changed 18 months ago by rblank

  • Milestone set to 0.12.1
  • Owner set to rblank
  • Priority changed from normal to high

Changed 18 months ago by rblank

Support for scoped repositories.

comment:2 follow-up: Changed 18 months ago by 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.

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?

comment:3 in reply to: ↑ 2 ; follow-up: Changed 18 months ago by itamaro

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

comment:4 in reply to: ↑ 3 Changed 18 months ago by rblank

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 Changed 18 months ago by itamaro

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'

Changed 18 months ago by rblank

Handle coarse permissions

comment:6 follow-up: Changed 18 months ago by rblank

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

comment:7 in reply to: ↑ 6 Changed 18 months ago by itamaro

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 Changed 18 months ago by rblank

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.

Changed 18 months ago by rblank

Formalize the repository scope.

comment:9 Changed 17 months ago by rblank

  • Resolution set to fixed
  • Status changed from new to closed
View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from rblank. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.