diff --git a/trac/versioncontrol/api.py b/trac/versioncontrol/api.py --- a/trac/versioncontrol/api.py +++ b/trac/versioncontrol/api.py @@ -185,6 +185,14 @@ ((msg and '%s: ' % msg) or '', path, rev), _('No such node')) +class ChangesetDenied(ResourceNotFound): + def __init__(self, rev): + ResourceNotFound.__init__(self, + _('Access to changeset %(rev)s is denied', + rev=rev), + _('Changeset not authorized')) + + class Repository(object): """Base class for a repository provided by a version control system.""" @@ -230,7 +238,7 @@ return [] def get_changeset(self, rev): - """Retrieve a Changeset corresponding to the given revision `rev`.""" + """Retrieve a Changeset corresponding to the given revision `rev`.""" raise NotImplementedError def get_changesets(self, start, stop): diff --git a/trac/versioncontrol/cache.py b/trac/versioncontrol/cache.py --- a/trac/versioncontrol/cache.py +++ b/trac/versioncontrol/cache.py @@ -22,7 +22,7 @@ from trac.util.datefmt import utc, to_timestamp from trac.util.translation import _ from trac.versioncontrol import Changeset, Node, Repository, Authorizer, \ - NoSuchChangeset + NoSuchChangeset, ChangesetDenied _kindmap = {'D': Node.DIRECTORY, 'F': Node.FILE} @@ -136,44 +136,45 @@ if self.youngest != repos_youngest: self.log.info("repos rev [%s] != cached rev [%s]" % (repos_youngest, self.youngest)) - if self.youngest: - next_youngest = self.repos.next_rev(self.youngest) - else: - next_youngest = None - try: - next_youngest = self.repos.oldest_rev - # Ugly hack needed because doing that everytime in - # oldest_rev suffers from horrendeous performance (#5213) - if hasattr(self.repos, 'scope'): - if self.repos.scope != '/': - next_youngest = self.repos.next_rev(next_youngest, - find_initial_rev=True) - next_youngest = self.repos.normalize_rev(next_youngest) - except TracError: - return # can't normalize oldest_rev: repository was empty + authz = self.repos.authz + self.repos.authz = Authorizer() # remove permission checking + try: + if self.youngest: + next_youngest = self.repos.next_rev(self.youngest) + else: + next_youngest = None + try: + next_youngest = self.repos.oldest_rev + # Ugly hack needed because doing that everytime in + # oldest_rev suffers from horrendeous performance + # (#5213) + if hasattr(self.repos, 'scope'): + if self.repos.scope != '/': + next_youngest = self.repos.next_rev( + next_youngest, find_initial_rev=True) + next_youngest = self.repos.normalize_rev(next_youngest) + except TracError: + # can't normalize oldest_rev: repository was empty + return + + if next_youngest is None: # nothing to cache yet + return + + # 0. first check if there's no (obvious) resync in progress + cursor.execute("SELECT rev FROM revision WHERE rev=%s", + (str(next_youngest),)) + for rev, in cursor: + # already there, but in progress, so keep ''previous'' + # notion of 'youngest' + self.repos.clear(youngest_rev=self.youngest) + return + + # 1. prepare for resyncing + # (there still might be a race condition at this point) + + kindmap = dict(zip(_kindmap.values(), _kindmap.keys())) + actionmap = dict(zip(_actionmap.values(), _actionmap.keys())) - if next_youngest is None: # nothing to cache yet - return - - # 0. first check if there's no (obvious) resync in progress - cursor.execute("SELECT rev FROM revision WHERE rev=%s", - (str(next_youngest),)) - for rev, in cursor: - # already there, but in progress, so keep ''previous'' - # notion of 'youngest' - self.repos.clear(youngest_rev=self.youngest) - return - - # 1. prepare for resyncing - # (there still might be a race condition at this point) - - authz = self.repos.authz - self.repos.authz = Authorizer() # remove permission checking - - kindmap = dict(zip(_kindmap.values(), _kindmap.keys())) - actionmap = dict(zip(_actionmap.values(), _actionmap.keys())) - - try: while next_youngest is not None: # 1.1 Attempt to resync the 'revision' table @@ -270,7 +271,7 @@ args.append(path) sql += " OR " # changes on path children - sql += "path "+db.like() + sql += "path " + db.like() args.append(db.like_escape(path+'/') + '%') sql += " OR " # deletion of path ancestors @@ -282,12 +283,13 @@ sql += ")" sql += " ORDER BY " + db.cast('rev', 'int') + \ - (direction == '<' and " DESC" or "") + " LIMIT 1" + (direction == '<' and " DESC" or "") cursor = db.cursor() cursor.execute(sql, args) for rev, in cursor: - return rev + if self.authz.has_permission_for_changeset(rev): + return rev def rev_older_than(self, rev1, rev2): return self.repos.rev_older_than(rev1, rev2) @@ -332,14 +334,20 @@ cursor.execute("SELECT path,node_type,change_type,base_path,base_rev " "FROM node_change WHERE rev=%s " "ORDER BY path", (str(self.rev),)) + empty = True + any_returned = False for path, kind, change, base_path, base_rev in cursor: + empty = False if not self.authz.has_permission(posixpath.join(self.scope, path.strip('/'))): # FIXME: what about the base_path? continue + any_returned = True kind = _kindmap[kind] change = _actionmap[change] yield path, kind, change, base_path, base_rev + if not empty and not any_returned: + raise ChangesetDenied(self.rev) def get_properties(self): return self.repos.get_changeset(self.rev).get_properties() diff --git a/trac/versioncontrol/svn_authz.py b/trac/versioncontrol/svn_authz.py --- a/trac/versioncontrol/svn_authz.py +++ b/trac/versioncontrol/svn_authz.py @@ -20,7 +20,7 @@ from trac.config import Option from trac.core import * -from trac.versioncontrol import Authorizer +from trac.versioncontrol import Authorizer, ChangesetDenied class SvnAuthzOptions(Component): @@ -106,11 +106,12 @@ def has_permission_for_changeset(self, rev): changeset = self.repos.get_changeset(rev) - for change in changeset.get_changes(): - # the repository checks permissions for each change, so just check - # if any changes can be accessed + try: + for change in changeset.get_changes(): + break return 1 - return 0 + except ChangesetDenied: + return 0 # Internal API diff --git a/trac/versioncontrol/svn_fs.py b/trac/versioncontrol/svn_fs.py --- a/trac/versioncontrol/svn_fs.py +++ b/trac/versioncontrol/svn_fs.py @@ -56,7 +56,8 @@ from trac.core import * from trac.versioncontrol import Changeset, Node, Repository, \ IRepositoryConnector, \ - NoSuchChangeset, NoSuchNode + NoSuchChangeset, NoSuchNode, \ + ChangesetDenied from trac.versioncontrol.cache import CachedRepository from trac.versioncontrol.svn_authz import SubversionAuthorizer from trac.versioncontrol.web_ui.browser import IPropertyRenderer @@ -544,6 +545,8 @@ tmp1, tmp2 = tmp2, tmp1 if history_ptr: path_utf8, rev = fs.history_location(history_ptr, tmp2()) + if not self.authz.has_permission_for_changeset(rev): + continue tmp2.clear() if rev < end: break @@ -560,7 +563,6 @@ return prev return None - def get_oldest_rev(self): if self.oldest is None: self.oldest = 1 @@ -909,12 +911,12 @@ copies, deletions = {}, {} changes = [] revroots = {} + empty = True for path_utf8, change in editor.changes.items(): - path = _from_svn(path_utf8) + change_path = _from_svn(path_utf8) # Filtering on `path` - if not (_is_path_within_scope(self.scope, path) and - self.authz.has_permission(path)): + if not _is_path_within_scope(self.scope, change_path): continue path_utf8 = change.path @@ -922,12 +924,19 @@ path = _from_svn(path_utf8) base_path = _from_svn(base_path_utf8) base_rev = change.base_rev - + # Ensure `base_path` is within the scope if not (_is_path_within_scope(self.scope, base_path) and self.authz.has_permission(base_path)): base_path, base_rev = None, -1 + if not path and not base_path and self.scope != '/': + continue # deletion outside of scope, ignore + + empty = False + if not self.authz.has_permission(change_path): + continue + # Determine the action if not path: # deletion if base_path: @@ -937,8 +946,6 @@ deletions[base_path] = idx elif self.scope == '/': # root property change action = Changeset.EDIT - else: # deletion outside of scope, ignore - continue elif change.added or not base_path: # add or copy action = Changeset.ADD if base_path and base_rev: @@ -979,6 +986,8 @@ del changes[i - offset] offset += 1 + if not empty and not changes: + raise ChangesetDenied(self.rev) changes.sort() for change in changes: yield tuple(change) diff --git a/trac/versioncontrol/web_ui/util.py b/trac/versioncontrol/web_ui/util.py --- a/trac/versioncontrol/web_ui/util.py +++ b/trac/versioncontrol/web_ui/util.py @@ -34,10 +34,12 @@ for rev in revs: if rev in changes: continue - try: - changeset = repos.get_changeset(rev) - except NoSuchChangeset: - changeset = {} + changeset = {} + if repos.authz.has_permission_for_changeset(rev): + try: + changeset = repos.get_changeset(rev) + except NoSuchChangeset: + pass changes[rev] = changeset return changes