diff --git a/setup.py b/setup.py --- a/setup.py +++ b/setup.py @@ -108,6 +108,7 @@ trac.ticket.web_ui = trac.ticket.web_ui trac.timeline = trac.timeline.web_ui trac.versioncontrol.admin = trac.versioncontrol.admin + trac.versioncontrol.svn_authz = trac.versioncontrol.svn_authz trac.versioncontrol.svn_fs = trac.versioncontrol.svn_fs trac.versioncontrol.svn_prop = trac.versioncontrol.svn_prop trac.versioncontrol.web_ui = trac.versioncontrol.web_ui diff --git a/trac/env.py b/trac/env.py --- a/trac/env.py +++ b/trac/env.py @@ -319,7 +319,7 @@ @param authname: user name for authorization """ - return RepositoryManager(self).get_repository(reponame, authname) + return RepositoryManager(self).get_repository(reponame) def create(self, options=[]): """Create the basic directory structure of the environment, initialize diff --git a/trac/versioncontrol/admin.py b/trac/versioncontrol/admin.py --- a/trac/versioncontrol/admin.py +++ b/trac/versioncontrol/admin.py @@ -83,11 +83,11 @@ def _do_changeset_added(self, reponame, *revs): rm = RepositoryManager(self.env) - rm.notify('changeset_added', reponame, revs, None) + rm.notify('changeset_added', reponame, revs) def _do_changeset_modified(self, reponame, *revs): rm = RepositoryManager(self.env) - rm.notify('changeset_modified', reponame, revs, None) + rm.notify('changeset_modified', reponame, revs) def _do_list(self): rm = RepositoryManager(self.env) @@ -106,11 +106,11 @@ if rev is not None: raise TracError(_('Cannot synchronize a single revision ' 'on multiple repositories')) - repositories = rm.get_real_repositories(None) + repositories = rm.get_real_repositories() else: if reponame == '(default)': reponame = '' - repos = rm.get_repository(reponame, None) + repos = rm.get_repository(reponame) if repos is None: raise TracError(_("Unknown repository '%(reponame)s'", reponame=reponame or '(default)')) @@ -284,7 +284,7 @@ info['editable'] = editable if not info.get('alias'): try: - repos = RepositoryManager(self.env).get_repository(reponame, None) + repos = RepositoryManager(self.env).get_repository(reponame) info['rev'] = repos.get_youngest_rev() except Exception: pass diff --git a/trac/versioncontrol/api.py b/trac/versioncontrol/api.py --- a/trac/versioncontrol/api.py +++ b/trac/versioncontrol/api.py @@ -26,8 +26,7 @@ from trac.admin import AdminCommandError, IAdminCommandProvider from trac.config import ListOption, Option from trac.core import * -from trac.perm import PermissionError -from trac.resource import IResourceManager, ResourceNotFound +from trac.resource import IResourceManager, Resource, ResourceNotFound from trac.util.text import printout, to_unicode from trac.util.translation import _ from trac.web.api import IRequestFilter @@ -318,7 +317,7 @@ if is_default(reponame): reponame = '' try: - repo = self.get_repository(reponame, req.authname) + repo = self.get_repository(reponame) if repo: repo.sync() except TracError, e: @@ -422,7 +421,7 @@ if prio >= 0) return list(types) - def get_repositories_by_dir(self, directory, authname): + def get_repositories_by_dir(self, directory): """Retrieve the repositories based on the given directory. :param directory: the key for identifying the repositories. @@ -435,7 +434,7 @@ if dir: dir = os.path.join(os.path.normcase(dir), '') if dir.startswith(directory): - repos = self.get_repository(reponame, authname) + repos = self.get_repository(reponame) if repos: repositories.append(repos) return repositories @@ -459,13 +458,12 @@ db.commit() return id - def get_repository(self, reponame, authname): + def get_repository(self, reponame): """Retrieve the appropriate Repository for the given name. :param reponame: the key for specifying the repository. If no name is given, take the default repository. - :param authname: deprecated (use fine grained permissions) :return: if no corresponding repository was defined, simply return `None`. """ @@ -499,7 +497,7 @@ finally: self._lock.release() - def get_repository_by_path(self, path, authname): + def get_repository_by_path(self, path): """Retrieve a matching Repository for the given path. :param path: the eventually scoped repository-scoped path @@ -519,7 +517,7 @@ path = path[length:] else: reponame = '' - return (reponame, self.get_repository(reponame, authname), + return (reponame, self.get_repository(reponame), path.rstrip('/') or '/') def get_default_repository(self, context): @@ -549,12 +547,12 @@ self._all_repositories[reponame] = info return self._all_repositories - def get_real_repositories(self, authname): + def get_real_repositories(self): """Return a set of all real repositories (i.e. excluding aliases).""" repositories = set() for reponame in self.get_all_repositories(): try: - repos = self.get_repository(reponame, authname) + repos = self.get_repository(reponame) if repos is not None: repositories.add(repos) except TracError: @@ -572,7 +570,7 @@ self._lock.release() self.config.touch() # Force environment reload - def notify(self, event, reponame, revs, authname): + def notify(self, event, reponame, revs): """Notify repositories and change listeners about repository events. The supported events are the names of the methods defined in the @@ -583,18 +581,18 @@ # Notify a repository by name, and all repositories with the same # base, or all repositories by base or by repository dir - repos = self.get_repository(reponame, None) + repos = self.get_repository(reponame) repositories = [] if repos: base = repos.get_base() else: - repositories = self.get_repositories_by_dir(reponame, None) + repositories = self.get_repositories_by_dir(reponame) if repositories: base = None else: base = reponame if base: - repositories = [r for r in self.get_real_repositories(authname) + repositories = [r for r in self.get_real_repositories() if r.get_base() == base] if not repositories: self.log.warn("Found no repositories matching '%s' base.", @@ -676,7 +674,7 @@ class Repository(object): """Base class for a repository provided by a version control system.""" - def __init__(self, name, params, authz, log): + def __init__(self, name, params, log): """Initialize a repository. :param name: a unique name identifying the repository, usually a @@ -686,15 +684,14 @@ the name of the repository under the key "name" and the surrogate key that identifies the repository in the database under the key "id". - :param authz: a repository authorizer (deprecated). :param log: a logger instance. """ self.name = name self.params = params self.reponame = params['name'] self.id = params['id'] - self.authz = authz or Authorizer() self.log = log + self.resource = Resource('repository', self.reponame) def close(self): """Close the connection to the repository.""" @@ -771,12 +768,11 @@ """ rev = self.youngest_rev while rev: - if self.authz.has_permission_for_changeset(rev): - chgset = self.get_changeset(rev) - if chgset.date < start: - return - if chgset.date < stop: - yield chgset + chgset = self.get_changeset(rev) + if chgset.date < start: + return + if chgset.date < stop: + yield chgset rev = self.previous_rev(rev) def has_node(self, path, rev=None): @@ -883,6 +879,10 @@ """ raise NotImplementedError + def can_view(self, perm): + """Return True if view permission is granted on the repository.""" + return 'BROWSER_VIEW' in perm(self.resource.child('source', '/')) + class Node(object): """Represents a directory or file in the repository at a given revision.""" @@ -890,6 +890,10 @@ DIRECTORY = "dir" FILE = "file" + resource = property(lambda self: Resource('source', self.created_path, + version=self.created_rev, + parent=self.repos.resource)) + # created_path and created_rev properties refer to the Node "creation" # in the Subversion meaning of a Node in a versioned tree (see #3340). # @@ -898,9 +902,10 @@ created_rev = None created_path = None - def __init__(self, path, rev, kind): + def __init__(self, repos, path, rev, kind): assert kind in (Node.DIRECTORY, Node.FILE), \ "Unknown node kind %s" % kind + self.repos = repos self.path = to_unicode(path) self.rev = rev self.kind = kind @@ -990,6 +995,11 @@ isdir = property(lambda x: x.kind == Node.DIRECTORY) isfile = property(lambda x: x.kind == Node.FILE) + def can_view(self, perm): + """Return True if view permission is granted on the node.""" + return (self.isdir and 'BROWSER_VIEW' or 'FILE_VIEW') \ + in perm(self.resource) + class Changeset(object): """Represents a set of changes committed at once in a repository.""" @@ -1005,7 +1015,11 @@ OTHER_CHANGES = (ADD, DELETE) ALL_CHANGES = DIFF_CHANGES + OTHER_CHANGES - def __init__(self, rev, message, author, date): + resource = property(lambda self: Resource('changeset', self.rev, + parent=self.repos.resource)) + + def __init__(self, repos, rev, message, author, date): + self.repos = repos self.rev = rev self.message = message or '' self.author = author or '' @@ -1036,37 +1050,12 @@ """ raise NotImplementedError + def can_view(self, perm): + """Return True if view permission is granted on the changeset.""" + return 'CHANGESET_VIEW' in perm(self.resource) -class PermissionDenied(PermissionError): - """Exception raised by an authorizer. - - This exception is raise if the user has insufficient permissions - to view a specific part of the repository. - """ - def __str__(self): - return self.action - - -class Authorizer(object): - """Controls the view access to parts of the repository. - - Base class for authorizers that are responsible to granting or denying - access to view certain parts of a repository. - """ - - def assert_permission(self, path): - if not self.has_permission(path): - raise PermissionDenied(_('Insufficient permissions to access ' - '%(path)s', path=path)) - - def assert_permission_for_changeset(self, rev): - if not self.has_permission_for_changeset(rev): - raise PermissionDenied(_('Insufficient permissions to access ' - 'changeset %(id)s', id=rev)) - - def has_permission(self, path): - return True - - def has_permission_for_changeset(self, rev): - return True +# Note: Since Trac 0.12, Exception PermissionDenied class is gone, +# and class Authorizer is gone as well. +# +# Fine-grained permissions are now handled via normal permission policies. diff --git a/trac/versioncontrol/cache.py b/trac/versioncontrol/cache.py --- a/trac/versioncontrol/cache.py +++ b/trac/versioncontrol/cache.py @@ -16,14 +16,12 @@ from datetime import datetime import os -import posixpath from trac.cache import CacheProxy from trac.core import TracError from trac.util.datefmt import utc, to_timestamp from trac.util.translation import _ -from trac.versioncontrol import Changeset, Node, Repository, Authorizer, \ - NoSuchChangeset +from trac.versioncontrol import Changeset, Node, Repository, NoSuchChangeset _kindmap = {'D': Node.DIRECTORY, 'F': Node.FILE} @@ -43,14 +41,14 @@ scope = property(lambda self: self.repos.scope) - def __init__(self, env, repos, authz, log): + def __init__(self, env, repos, log): self.env = env self.repos = repos self.metadata = CacheProxy(self.__class__.__module__ + '.' + self.__class__.__name__ + '.metadata:' + str(self.repos.id), self._metadata, self.env) - Repository.__init__(self, repos.name, repos.params, authz, log) + Repository.__init__(self, repos.name, repos.params, log) def close(self): self.repos.close() @@ -65,8 +63,7 @@ return self.repos.get_path_url(path, rev) def get_changeset(self, rev): - return CachedChangeset(self.repos, self.normalize_rev(rev), - self.env, self.authz) + return CachedChangeset(self.repos, self.normalize_rev(rev), self.env) def get_changeset_uid(self, rev): return self.repos.get_changeset_uid(rev) @@ -81,8 +78,7 @@ to_timestamp(stop))) for rev, in cursor: try: - if self.authz.has_permission_for_changeset(rev): - yield self.get_changeset(rev) + yield self.get_changeset(rev) except NoSuchChangeset: pass # skip changesets currently being resync'ed @@ -220,70 +216,63 @@ # 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 - self.log.info("Trying to sync revision [%s]" % - next_youngest) - cset = self.repos.get_changeset(next_youngest) - try: - cursor.execute("INSERT INTO revision " - " (repos,rev,time,author,message) " - "VALUES (%s,%s,%s,%s,%s)", - (self.id, str(next_youngest), - to_timestamp(cset.date), - cset.author, cset.message)) - except Exception, e: # *another* 1.1. resync attempt won - self.log.warning('Revision %s already cached: %s' % - (next_youngest, e)) - # also potentially in progress, so keep ''previous'' - # notion of 'youngest' - self.repos.clear(youngest_rev=youngest) - db.rollback() - return + while next_youngest is not None: + + # 1.1 Attempt to resync the 'revision' table + self.log.info("Trying to sync revision [%s]" % + next_youngest) + cset = self.repos.get_changeset(next_youngest) + try: + cursor.execute("INSERT INTO revision " + " (repos,rev,time,author,message) " + "VALUES (%s,%s,%s,%s,%s)", + (self.id, str(next_youngest), + to_timestamp(cset.date), + cset.author, cset.message)) + except Exception, e: # *another* 1.1. resync attempt won + self.log.warning('Revision %s already cached: %s' % + (next_youngest, e)) + # also potentially in progress, so keep ''previous'' + # notion of 'youngest' + self.repos.clear(youngest_rev=youngest) + db.rollback() + return - # 1.2. now *only* one process was able to get there - # (i.e. there *shouldn't* be any race condition here) + # 1.2. now *only* one process was able to get there + # (i.e. there *shouldn't* be any race condition here) - for path, kind, action, bpath, brev in cset.get_changes(): - self.log.debug("Caching node change in [%s]: %s" - % (next_youngest, - (path,kind,action,bpath,brev))) - kind = kindmap[kind] - action = actionmap[action] - cursor.execute("INSERT INTO node_change " - " (repos,rev,path,node_type," - " change_type,base_path,base_rev) " - "VALUES (%s,%s,%s,%s,%s,%s,%s)", - (self.id, str(next_youngest), - path, kind, action, bpath, brev)) + for path, kind, action, bpath, brev in cset.get_changes(): + self.log.debug("Caching node change in [%s]: %s" + % (next_youngest, + (path,kind,action,bpath,brev))) + kind = kindmap[kind] + action = actionmap[action] + cursor.execute("INSERT INTO node_change " + " (repos,rev,path,node_type," + " change_type,base_path,base_rev) " + "VALUES (%s,%s,%s,%s,%s,%s,%s)", + (self.id, str(next_youngest), + path, kind, action, bpath, brev)) - # 1.3. iterate (1.1 should always succeed now) - youngest = next_youngest - next_youngest = self.repos.next_rev(next_youngest) + # 1.3. iterate (1.1 should always succeed now) + youngest = next_youngest + next_youngest = self.repos.next_rev(next_youngest) - # 1.4. update 'youngest_rev' metadata - # (minimize possibility of failures at point 0.) - cursor.execute("UPDATE repository SET value=%s " - "WHERE id=%s AND name=%s", - (str(youngest), self.id, - CACHE_YOUNGEST_REV)) - self.metadata.invalidate(db) - db.commit() + # 1.4. update 'youngest_rev' metadata + # (minimize possibility of failures at point 0.) + cursor.execute("UPDATE repository SET value=%s " + "WHERE id=%s AND name=%s", + (str(youngest), self.id, + CACHE_YOUNGEST_REV)) + self.metadata.invalidate(db) + db.commit() - # 1.5. provide some feedback - if feedback: - feedback(youngest) - finally: - # 3. restore permission checking (after 1.) - self.repos.authz = authz + # 1.5. provide some feedback + if feedback: + feedback(youngest) def get_node(self, path, rev=None): return self.repos.get_node(path, self.normalize_rev(rev)) @@ -397,20 +386,18 @@ class CachedChangeset(Changeset): - def __init__(self, repos, rev, env, authz): - self.repos = repos + def __init__(self, repos, rev, env): self.env = env - self.authz = authz db = self.env.get_db_cnx() cursor = db.cursor() cursor.execute("SELECT time,author,message FROM revision " "WHERE repos=%s AND rev=%s", - (self.repos.id, str(rev))) + (repos.id, str(rev))) row = cursor.fetchone() if row: _date, author, message = row date = datetime.fromtimestamp(_date, utc) - Changeset.__init__(self, rev, message, author, date) + Changeset.__init__(self, repos, rev, message, author, date) else: raise NoSuchChangeset(rev) self.scope = getattr(repos, 'scope', '') @@ -422,10 +409,6 @@ "FROM node_change WHERE repos=%s AND rev=%s " "ORDER BY path", (self.repos.id, str(self.rev))) for path, kind, change, base_path, base_rev in cursor: - if not self.authz.has_permission(posixpath.join(self.scope, - path.strip('/'))): - # FIXME: what about the base_path? - continue kind = _kindmap[kind] change = _actionmap[change] yield path, kind, change, base_path, base_rev 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 @@ -18,34 +18,17 @@ import os.path -from trac.config import Option +from trac.config import Option, PathOption from trac.core import * -from trac.versioncontrol import Authorizer +from trac.perm import IPermissionPolicy +from trac.resource import Resource +from trac.util import read_file +from trac.util.compat import any +from trac.util.text import exception_to_unicode, to_unicode +from trac.util.translation import _ +from trac.versioncontrol.api import RepositoryManager -class SvnAuthzOptions(Component): - - authz_file = Option('trac', 'authz_file', '', - """Path to Subversion - [http://svnbook.red-bean.com/en/1.1/ch06s04.html#svn-ch-6-sect-4.4.2 authorization (authz) file] - """) - - authz_module_name = Option('trac', 'authz_module_name', '', - """The module prefix used in the authz_file.""") - - -def SubversionAuthorizer(env, repos, authname): - authz_file = env.config.get('trac', 'authz_file') - if not authz_file: - return Authorizer() - if not os.path.isabs(authz_file): - authz_file = os.path.join(env.path, authz_file) - if not os.path.exists(authz_file): - env.log.error('[trac] authz_file (%s) does not exist.' % authz_file) - - module_name = env.config.get('trac', 'authz_module_name') - return RealSubversionAuthorizer(repos, authname, module_name, authz_file) - def parent_iter(path): path = path.strip('/') if path: @@ -56,114 +39,167 @@ while 1: yield path if path == '/': - raise StopIteration() + return path = path[:-1] yield path idx = path.rfind('/') path = path[:idx + 1] -class RealSubversionAuthorizer(Authorizer): - """FIXME: this should become a IPermissionPolicy, of course. +class ParseError(Exception): + """Exception thrown for parse errors in authz files""" - `check_permission(username, action, resource)` should be able to - replace `has_permission(path)` when resource is a `('source', path)` - and `has_permission_for_changeset` when resource is a `('changeset', rev)`. + +def parse(authz): + """Parse a Subversion authorization file. + + Return a dict of modules, each containing a dict of paths, each containing + a dict mapping users to permissions. + """ + groups = {} + aliases = {} + sections = {} + section = None + lineno = 0 + for line in authz.splitlines(): + lineno += 1 + line = to_unicode(line.strip()) + if not line or line.startswith('#') or line.startswith(';'): + continue + if line.startswith('[') and line.endswith(']'): + section = line[1:-1] + continue + if section is None: + raise ParseError(_('Line %(lineno)d: Entry before first ' + 'section header', lineno=lineno)) + parts = line.split('=', 1) + if len(parts) != 2: + raise ParseError(_('Line %(lineno)d: Invalid entry', + lineno=lineno)) + name, value = parts + name = name.strip() + if section == 'groups': + group = groups.setdefault(name, set()) + group.update(each.strip() for each in value.split(',')) + elif section == 'aliases': + aliases[name] = value.strip() + else: + sections.setdefault(section, []).append((name.strip(), value)) + + def resolve(subject, done): + if subject.startswith('@'): + done.add(subject) + for members in groups[subject[1:]] - done: + for each in resolve(members, done): + yield each + elif subject.startswith('&'): + yield aliases[subject[1:]] + else: + yield subject + + authz = {} + for name, items in sections.iteritems(): + parts = name.split(':', 1) + module = authz.setdefault(len(parts) > 1 and parts[0] or '', {}) + section = module.setdefault(parts[-1], {}) + for subject, perms in items: + for user in resolve(subject, set()): + section.setdefault(user, 'r' in perms) # The first match wins + + return authz + + + +class AuthzSourcePolicy(Component): + """Permission policy for `source:` and `changeset:` resources using a + Subversion authz file. + + `FILE_VIEW` and `BROWSER_VIEW` permissions are granted as specified in the + authz file. + + `CHANGESET_VIEW` permission is granted for changesets where `FILE_VIEW` is + granted on at least one modified file, as well as empty for changesets. """ - auth_name = '' - module_name = '' - conf_authz = None + implements(IPermissionPolicy) + + authz_file = PathOption('trac', 'authz_file', '', + """Path to the Subversion + [http://svnbook.red-bean.com/en/1.1/ch06s04.html#svn-ch-6-sect-4.4.2 authorization (authz) file] + """) - def __init__(self, repos, auth_name, module_name, cfg_file, cfg_fp=None): - self.repos = repos - self.auth_name = auth_name - self.module_name = module_name - - from ConfigParser import ConfigParser - self.conf_authz = ConfigParser() - if cfg_fp: - self.conf_authz.readfp(cfg_fp, cfg_file) - elif cfg_file: - self.conf_authz.read(cfg_file) + authz_module_name = Option('trac', 'authz_module_name', '', + """The module prefix used in the `authz_file` for the default + repository. + """) - self.groups = self._groups() + _mtime = 0 + _authz = {} + _users = set() + + # IPermissionPolicy methods - def has_permission(self, path): - if path is None: - return 1 + def check_permission(self, action, username, resource, perm): + if action == 'FILE_VIEW' or action == 'BROWSER_VIEW': + authz, users = self._get_authz_info() + if authz is None: + return False + if resource is None: + return users is True or username in users + if resource.realm == 'source': + modules = [resource.parent.id or self.authz_module_name] + if modules[0]: + modules.append('') + for p in parent_iter(resource.id): + for module in modules: + section = authz.get(module, {}).get(p, {}) + result = section.get(username) + if result is not None: + return result + result = section.get('*') + if result is not None: + return result + return False + elif action == 'CHANGESET_VIEW': + authz, users = self._get_authz_info() + if authz is None: + return False + if resource is None: + return users is True or username in users + if resource.realm == 'changeset': + rm = RepositoryManager(self.env) + repos = rm.get_repository(resource.parent.id) + changes = list(repos.get_changeset(resource.id).get_changes()) + if not changes: + return True + source = Resource('source', version=resource.id, + parent=resource.parent) + return any('FILE_VIEW' in perm(source(id=change[0])) + for change in changes) - for p in parent_iter(path): - if self.module_name: - for perm in self._get_section(self.module_name + ':' + p): - if perm is not None: - return perm - for perm in self._get_section(p): - if perm is not None: - return perm - - return 0 - - 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 - return 1 - return 0 - - # Internal API - - def _groups(self): - if not self.conf_authz.has_section('groups'): - return [] - - grp_parents = {} - usr_grps = [] - - for group in self.conf_authz.options('groups'): - for member in self.conf_authz.get('groups', group).split(','): - member = member.strip() - if member == self.auth_name: - usr_grps.append(group) - elif member.startswith('@'): - grp_parents.setdefault(member[1:], []).append(group) - - expanded = {} - - def expand_group(group): - if group in expanded: - return - expanded[group] = True - for parent in grp_parents.get(group, []): - expand_group(parent) - - for g in usr_grps: - expand_group(g) - - # expand groups - return expanded.keys() - - def _get_section(self, section): - if not self.conf_authz.has_section(section): - return - - yield self._get_permission(section, self.auth_name) - - group_perm = None - for g in self.groups: - p = self._get_permission(section, '@' + g) - if p is not None: - group_perm = p - - if group_perm: - yield 1 - - yield group_perm - - yield self._get_permission(section, '*') - - def _get_permission(self, section, subject): - if self.conf_authz.has_option(section, subject): - return 'r' in self.conf_authz.get(section, subject) - return None + def _get_authz_info(self): + try: + mtime = os.path.getmtime(self.authz_file) + except OSError, e: + if self._authz is not None: + self.log.error('Error accessing authz file: %s', + exception_to_unicode(e)) + self._mtime = mtime = 0 + self._authz = None + self._users = set() + if mtime > self._mtime: + self._mtime = mtime + self.log.info('Parsing authz file: %s' % self.authz_file) + try: + self._authz = parse(read_file(self.authz_file)) + users = set(user for module in self._authz.itervalues() + for path in module.itervalues() + for user, result in path.iteritems() if result) + self._users = '*' in users or users + except Exception, e: + self._authz = None + self._users = set() + self.log.error('Error parsing authz file: %s', + exception_to_unicode(e)) + return self._authz, self._users + \ No newline at end of file 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 @@ -54,7 +54,6 @@ IRepositoryConnector, \ NoSuchChangeset, NoSuchNode from trac.versioncontrol.cache import CachedRepository -from trac.versioncontrol.svn_authz import SubversionAuthorizer from trac.util import embedded_numbers from trac.util.text import exception_to_unicode, to_unicode from trac.util.translation import _ @@ -280,17 +279,12 @@ self._version = self._get_version() self.env.systeminfo.append(('Subversion', self._version)) params.update(tags=self.tags, branches=self.branches) - fs_repos = SubversionRepository(dir, params, None, self.log) + fs_repos = SubversionRepository(dir, params, self.log) if type == 'direct-svnfs': repos = fs_repos else: - repos = CachedRepository(self.env, fs_repos, None, self.log) + repos = CachedRepository(self.env, fs_repos, self.log) repos.has_linear_changesets = True - # FIXME: convert SubversionAuthorizer to a PermissionPolicy - if 'authname' in params: - authz = SubversionAuthorizer(self.env, weakref.proxy(repos), - params['authname']) - repos.authz = fs_repos.authz = authz return repos def _get_version(self): @@ -305,7 +299,7 @@ class SubversionRepository(Repository): """Repository implementation based on the svn.fs API.""" - def __init__(self, path, params, authz, log): + def __init__(self, path, params, log): self.log = log self.pool = Pool() @@ -335,7 +329,7 @@ self.base = 'svn:%s:%s' % (self.uuid, _from_svn(root_path_utf8)) name = 'svn:%s:%s' % (self.uuid, self.path) - Repository.__init__(self, name, params, authz, log) + Repository.__init__(self, name, params, log) # if root_path_utf8 is shorter than the path_utf8, the difference is # this scope (which always starts with a '/') @@ -430,16 +424,13 @@ def get_changeset(self, rev): rev = self.normalize_rev(rev) - return SubversionChangeset(rev, self.authz, self.scope, - self.fs_ptr, self.pool) + return SubversionChangeset(self, rev, self.scope, self.pool) def get_changeset_uid(self, rev): return (self.uuid, rev) def get_node(self, path, rev=None): path = path or '' - self.authz.assert_permission(posixpath.join(self.scope, - path.strip('/'))) if path and path[-1] == '/': path = path[:-1] @@ -490,8 +481,6 @@ if rev < end: break path = _from_svn(path_utf8) - if not self.authz.has_permission(path): - break yield path, rev del tmp1 del tmp2 @@ -667,9 +656,7 @@ class SubversionNode(Node): def __init__(self, path, rev, repos, pool=None, parent_root=None): - self.repos = repos self.fs_ptr = repos.fs_ptr - self.authz = repos.authz self.scope = repos.scope self._scoped_path_utf8 = _to_svn(self.scope, path) self.pool = Pool(pool) @@ -700,7 +687,7 @@ self.created_rev, self.created_path = rev, path self.rev = self.created_rev # TODO: check node id - Node.__init__(self, path, self.rev, _kindmap[node_type]) + Node.__init__(self, repos, path, self.rev, _kindmap[node_type]) def get_content(self): if self.isdir: @@ -719,9 +706,6 @@ entries = fs.dir_entries(self.root, self._scoped_path_utf8, pool()) for item in entries.keys(): path = posixpath.join(self.path, _from_svn(item)) - if not self.authz.has_permission(posixpath.join(self.scope, - path.strip('/'))): - continue yield SubversionNode(path, self._requested_rev, self.repos, self.pool, self.root) @@ -846,11 +830,10 @@ class SubversionChangeset(Changeset): - def __init__(self, rev, authz, scope, fs_ptr, pool=None): + def __init__(self, repos, rev, scope, pool=None): self.rev = rev - self.authz = authz self.scope = scope - self.fs_ptr = fs_ptr + self.fs_ptr = repos.fs_ptr self.pool = Pool(pool) try: message = self._get_prop(core.SVN_PROP_REVISION_LOG) @@ -866,7 +849,7 @@ date = datetime.fromtimestamp(ts, utc) else: date = None - Changeset.__init__(self, rev, message, author, date) + Changeset.__init__(self, repos, rev, message, author, date) def get_properties(self): props = fs.revision_proplist(self.fs_ptr, self.rev, self.pool()) @@ -896,8 +879,7 @@ 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, path): continue path_utf8 = change.path @@ -907,8 +889,7 @@ 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)): + if not _is_path_within_scope(self.scope, base_path): base_path, base_rev = None, -1 # Determine the action diff --git a/trac/versioncontrol/templates/browser.html b/trac/versioncontrol/templates/browser.html --- a/trac/versioncontrol/templates/browser.html +++ b/trac/versioncontrol/templates/browser.html @@ -49,7 +49,7 @@ -

Default Repository

+

Default Repository

 diff --git a/trac/versioncontrol/templates/changeset.html b/trac/versioncontrol/templates/changeset.html --- a/trac/versioncontrol/templates/changeset.html +++ b/trac/versioncontrol/templates/changeset.html @@ -122,8 +122,7 @@   - ${wiki_to_html(context('changeset', changeset.rev, parent=repos_resource), - changeset.message, escape_newlines=True)} + ${wiki_to_html(context, changeset.message, escape_newlines=True)} 
${changeset.message}
 diff --git a/trac/versioncontrol/templates/dir_entries.html b/trac/versioncontrol/templates/dir_entries.html --- a/trac/versioncontrol/templates/dir_entries.html +++ b/trac/versioncontrol/templates/dir_entries.html @@ -6,7 +6,9 @@ - + $entry.rev   - - ${change and dateinfo(change.date) or '-'} + ${('–', dateinfo(change.date))[chgset_view]} - - ${authorinfo(change.author)}: - - - - - ${change and wiki_to_oneliner(chgset_context, change.message, shorten=True)} - - ${change and shorten_line(change.message)} - + + + ${authorinfo(change.author)}: + + + ${wiki_to_oneliner(chgset_context, change.message, shorten=True)} + + ${shorten_line(change.message)} + + + – diff --git a/trac/versioncontrol/templates/repository_index.html b/trac/versioncontrol/templates/repository_index.html --- a/trac/versioncontrol/templates/repository_index.html +++ b/trac/versioncontrol/templates/repository_index.html @@ -5,7 +5,9 @@ - + - - diff --git a/trac/versioncontrol/templates/revisionlog.html b/trac/versioncontrol/templates/revisionlog.html --- a/trac/versioncontrol/templates/revisionlog.html +++ b/trac/versioncontrol/templates/revisionlog.html @@ -110,8 +110,7 @@ @@ -148,7 +147,7 @@
@@ -22,21 +24,22 @@   - ${change and dateinfo(change.date) or '-'} + + ${('–', dateinfo(change.date))[chgset_view]} - ${authorinfo(change.author)}: - - - - - - ${change and wiki_to_oneliner(chgset_context, change.message, shorten=True)} - - ${change and shorten_line(change.message)} - + + + + ${authorinfo(change.author)}: + + + ${wiki_to_oneliner(chgset_context, change.message, shorten=True)} + + ${shorten_line(change.message)} + + + –
- + ${wiki_to_oneliner(chgset_context, change.message, shorten=True)} diff --git a/trac/versioncontrol/templates/revisionlog.rss b/trac/versioncontrol/templates/revisionlog.rss --- a/trac/versioncontrol/templates/revisionlog.rss +++ b/trac/versioncontrol/templates/revisionlog.rss @@ -17,7 +17,7 @@ + item_context = context('changeset', change.rev, parent=context.resource.parent)"> ${author_or_creator(change.author, email_map)} ${http_date(change.date)} Revision $item.rev: ${shorten_line(change.message)} diff --git a/trac/versioncontrol/templates/revisionlog.txt b/trac/versioncontrol/templates/revisionlog.txt --- a/trac/versioncontrol/templates/revisionlog.txt +++ b/trac/versioncontrol/templates/revisionlog.txt @@ -8,8 +8,10 @@ {% with change = changes[item.rev]; extra = extra_changes[item.rev] %}\ ${http_date(change.date)} ${format_author(change.author)} [$item.rev] {% for idx, file in enumerate(extra.files) %}\ +{% if 'FILE_VIEW' in perm(repos.resource.child('source', file, version=change.rev)) %}\ * $file (${dict(edit='modified', add='added', delete='deleted', copy='copied', move='moved')[extra.actions[idx]]}) +{% end %}\ {% end %}\ ${verbose and extra.message or shorten_line(extra.message)} diff --git a/trac/versioncontrol/tests/api.py b/trac/versioncontrol/tests/api.py --- a/trac/versioncontrol/tests/api.py +++ b/trac/versioncontrol/tests/api.py @@ -22,7 +22,7 @@ def setUp(self): self.repo_base = Repository('testrepo', {'name': 'testrepo', 'id': 1}, - None, None) + None) def test_raise_NotImplementedError_close(self): self.failUnlessRaises(NotImplementedError, self.repo_base.close) diff --git a/trac/versioncontrol/tests/cache.py b/trac/versioncontrol/tests/cache.py --- a/trac/versioncontrol/tests/cache.py +++ b/trac/versioncontrol/tests/cache.py @@ -44,13 +44,13 @@ raise NoSuchChangeset(rev) repos = Mock(Repository, 'test-repos', {'name': 'test-repos', 'id': 1}, - None, self.log, + self.log, get_changeset=no_changeset, get_oldest_rev=lambda: 1, get_youngest_rev=lambda: 0, normalize_rev=no_changeset, next_rev=lambda x: None) - cache = CachedRepository(self.env, repos, None, self.log) + cache = CachedRepository(self.env, repos, self.log) cache.sync() cursor = self.db.cursor() @@ -64,18 +64,18 @@ t2 = datetime(2002, 1, 1, 1, 1, 1, 0, utc) changes = [('trunk', Node.DIRECTORY, Changeset.ADD, None, None), ('trunk/README', Node.FILE, Changeset.ADD, None, None)] - changesets = [Mock(Changeset, 0, '', '', t1, - get_changes=lambda: []), - Mock(Changeset, 1, 'Import', 'joe', t2, - get_changes=lambda: iter(changes))] repos = Mock(Repository, 'test-repos', {'name': 'test-repos', 'id': 1}, - None, self.log, + self.log, get_changeset=lambda x: changesets[int(x)], get_oldest_rev=lambda: 0, get_youngest_rev=lambda: 1, normalize_rev=lambda x: x, next_rev=lambda x: int(x) == 0 and 1 or None) - cache = CachedRepository(self.env, repos, None, self.log) + changesets = [Mock(Changeset, repos, 0, '', '', t1, + get_changes=lambda: []), + Mock(Changeset, repos, 1, 'Import', 'joe', t2, + get_changes=lambda: iter(changes))] + cache = CachedRepository(self.env, repos, self.log) cache.sync() cursor = self.db.cursor() @@ -111,16 +111,16 @@ "WHERE id=1 AND name='youngest_rev'") changes = [('trunk/README', Node.FILE, Changeset.EDIT, 'trunk/README', 1)] - changeset = Mock(Changeset, 2, 'Update', 'joe', t3, - get_changes=lambda: iter(changes)) repos = Mock(Repository, 'test-repos', {'name': 'test-repos', 'id': 1}, - None, self.log, + self.log, get_changeset=lambda x: changeset, get_youngest_rev=lambda: 2, get_oldest_rev=lambda: 0, normalize_rev=lambda x: x, next_rev=lambda x: x and int(x) == 1 and 2 or None) - cache = CachedRepository(self.env, repos, None, self.log) + changeset = Mock(Changeset, repos, 2, 'Update', 'joe', t3, + get_changes=lambda: iter(changes)) + cache = CachedRepository(self.env, repos, self.log) cache.sync() cursor = self.db.cursor() @@ -152,13 +152,13 @@ "WHERE id=1 AND name='youngest_rev'") repos = Mock(Repository, 'test-repos', {'name': 'test-repos', 'id': 1}, - None, self.log, + self.log, get_changeset=lambda x: None, get_youngest_rev=lambda: 1, get_oldest_rev=lambda: 0, next_rev=lambda x: None, normalize_rev=lambda rev: rev) - cache = CachedRepository(self.env, repos, None, self.log) + cache = CachedRepository(self.env, repos, self.log) self.assertEqual('1', cache.youngest_rev) changeset = cache.get_changeset(1) self.assertEqual('joe', changeset.author) diff --git a/trac/versioncontrol/tests/svn_authz.py b/trac/versioncontrol/tests/svn_authz.py --- a/trac/versioncontrol/tests/svn_authz.py +++ b/trac/versioncontrol/tests/svn_authz.py @@ -1,221 +1,268 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2010 Edgewall Software +# All rights reserved. +# +# This software is licensed as described in the file COPYING, which +# you should have received as part of this distribution. The terms +# are also available at http://trac.edgewall.org/wiki/TracLicense. +# +# This software consists of voluntary contributions made by many +# individuals. For the exact contribution history, see the revision +# history and logs, available at http://trac.edgewall.org/log/. + +import os.path +import tempfile import unittest -import sys -def tests(): - """ - Subversion Authz File Permissions - ================================= - - Setup code - ---------- - We'll use the ``make_auth`` method to create Authorizer objects - for testing the use of authz files. ``make_auth`` takes a module name - and a string for the authz configuration contents. - - >>> from trac.versioncontrol.svn_authz import RealSubversionAuthorizer - >>> from StringIO import StringIO - >>> make_auth = lambda mod, cfg: RealSubversionAuthorizer(None, - ... 'user', mod, None, StringIO(cfg)) - - - Simple operation - ---------------- - Returns 1 if no path is given: - >>> int(make_auth('', '').has_permission(None)) - 1 - - By default read permission is not enabled: - >>> int(make_auth('', '').has_permission('/')) - 0 - - Read and Write Permissions - ---------------------- - Trac is only concerned about read permissions. - >>> a = make_auth('', ''' - ... [/readonly] - ... user = r - ... [/writeonly] - ... user = w - ... [/readwrite] - ... user = rw - ... [/empty] - ... user = - ... ''') - - Permissions of 'r' or 'rw' will allow access: - >>> int(a.has_permission('/readonly')) - 1 - >>> int(a.has_permission('/readwrite')) - 1 - - If only 'w' permission is given, Trac does not allow access: - >>> int(a.has_permission('/writeonly')) - 0 - - And an empty permission does not give access: - >>> int(a.has_permission('/empty')) - 0 - - Trailing Slashes - ---------------- - Checks all combinations of trailing slashes in the configuration - or in the path parameter: - >>> a = make_auth('', ''' - ... [/a] - ... user = r - ... [/b/] - ... user = r - ... ''') - >>> int(a.has_permission('/a')) - 1 - >>> int(a.has_permission('/a/')) - 1 - >>> int(a.has_permission('/b')) - 1 - >>> int(a.has_permission('/b/')) - 1 - - - Module Usage - ------------ - If a module name is specified, the rules used are specific to the module. - >>> a = make_auth('module', ''' - ... [module:/a] - ... user = r - ... [other:/b] - ... user = r - ... ''') - >>> int(a.has_permission('/a')) - 1 - >>> int(a.has_permission('/b')) - 0 - - If a module is specified, but the configuration contains a non-module - path, the non-module path can still apply: - >>> int(make_auth('module', ''' - ... [/a] - ... user = r - ... ''').has_permission('/a')) - 1 - - However, the module-specific rule will take precedence if both exist: - >>> int(make_auth('module', ''' - ... [module:/a] - ... user = - ... [/a] - ... user = r - ... ''').has_permission('/a')) - 0 - - - Groups and Wildcards - -------------------- - Authz provides a * wildcard for matching any user: - >>> int(make_auth('', ''' - ... [/a] - ... * = r - ... ''').has_permission('/a')) - 1 - - Groups are specified in a separate section and used with an @ prefix: - >>> int(make_auth('', ''' - ... [groups] - ... grp = user - ... [/a] - ... @grp = r - ... ''').has_permission('/a')) - 1 +from trac.resource import Resource +from trac.test import EnvironmentStub +from trac.util import create_file +from trac.versioncontrol.svn_authz import AuthzSourcePolicy, ParseError, \ + parse - Groups can also be members of other groups: - >>> int(make_auth('', ''' - ... [groups] - ... grp1 = user - ... grp2 = @grp1 - ... [/a] - ... @grp2 = r - ... ''').has_permission('/a')) - 1 - Groups should not be defined cyclically, but they are handled appropriately - to avoid infinite loops: - >>> int(make_auth('', ''' - ... [groups] - ... grp1 = @grp2 - ... grp2 = @grp3 - ... grp3 = @grp1, user - ... [/a] - ... @grp1 = r - ... ''').has_permission('/a')) - 1 - - If more than one group matches at the specific path, access is granted - if any of the group rules allow access. - >>> a = make_auth('', ''' - ... [groups] - ... grp1 = user - ... grp2 = user - ... [/a] - ... @grp1 = r - ... @grp2 = - ... [/b] - ... @grp1 = - ... @grp2 = r - ... ''') - >>> int(a.has_permission('/a')) - 1 - >>> int(a.has_permission('/b')) - 1 - - - Precedence - ---------- - Precedence is user, group, then *: - >>> a = make_auth('', ''' - ... [groups] - ... grp = user - ... [/a] - ... @grp = r - ... user = - ... [/b] - ... * = r - ... @grp = - ... ''') - - User specific permission overrides the group permission: - >>> int(a.has_permission('/a')) - 0 - - And group permission overrides the * permission: - >>> int(a.has_permission('/b')) - 0 - - The most specific matching path takes precedence: - >>> a = make_auth('', ''' - ... [/] - ... * = r - ... [/b] - ... user = - ... ''') - >>> int(a.has_permission('/')) - 1 - >>> int(a.has_permission('/a')) - 1 - >>> int(a.has_permission('/b')) - 0 - - Changeset Permissions - --------------------- - A test should go here for the changeset permissions. - """ +class AuthzParserTestCase(unittest.TestCase): + + def test_parse_file(self): + authz = parse("""\ +[groups] +developers = foo, bar +users = @developers, &baz + +[aliases] +baz = CN=Hàröld Hacker,OU=Enginéers,DC=red-bean,DC=com + +# Applies to all repositories +[/] +* = r + +[/trunk] +@developers = rw +&baz = +@users = r + +[/branches] +bar = rw + +# Applies only to module +[module:/trunk] +foo = rw +&baz = r +""") + self.assertEqual({ + '': { + '/': { + '*': True, + }, + '/trunk': { + 'foo': True, + 'bar': True, + u'CN=Hàröld Hacker,OU=Enginéers,DC=red-bean,DC=com': False, + }, + '/branches': { + 'bar': True, + }, + }, + 'module': { + '/trunk': { + 'foo': True, + u'CN=Hàröld Hacker,OU=Enginéers,DC=red-bean,DC=com': True, + }, + }, + }, authz) + + def test_parse_errors(self): + self.assertRaises(ParseError, parse, """\ +user = r + +[module:/trunk] +user = r +""") + self.assertRaises(ParseError, parse, """\ +[module:/trunk] +user +""") + + +class AuthzSourcePolicyTestCase(unittest.TestCase): + + def setUp(self): + tmpdir = os.path.realpath(tempfile.gettempdir()) + self.authz = os.path.join(tmpdir, 'trac-authz') + create_file(self.authz, """\ +[groups] +group1 = user +group2 = @group1 + +cycle1 = @cycle2 +cycle2 = @cycle3 +cycle3 = @cycle1, user + +alias1 = &jekyll +alias2 = @alias1 + +[aliases] +jekyll = Mr Hyde + +# Read / write permissions +[/readonly] +user = r +[/writeonly] +user = w +[/readwrite] +user = rw +[/empty] +user = + +# Trailing slashes +[/trailing_a] +user = r +[/trailing_b/] +user = r + +# Sub-paths +[/sub/path] +user = r + +# Module usage +[module:/module_a] +user = r +[other:/module_b] +user = r +[/module_c] +user = r +[module:/module_d] +user = +[/module_d] +user = r + +# Wildcards +[/wildcard] +* = r + +# Groups +[/groups_a] +@group1 = r +[/groups_b] +@group2 = r +[/cyclic] +@cycle1 = r + +# Precedence +[module:/precedence_a] +user = +[/precedence_a] +user = r +[/precedence_b] +user = r +[/precedence_b/sub] +user = +[/precedence_b/sub/test] +user = r +[/precedence_c] +user = +@group1 = r +[/precedence_d] +@group1 = r +user = + +# Aliases +[/aliases_a] +&jekyll = r +[/aliases_b] +@alias2 = r +""") + self.env = EnvironmentStub(enable=[AuthzSourcePolicy]) + self.env.config.set('trac', 'authz_file', self.authz) + self.policy = AuthzSourcePolicy(self.env) + + def tearDown(self): + self.env.reset_db() + os.remove(self.authz) + + def assertPermission(self, result, user, reponame, path): + """Assert that `user` is granted access `result` to `path` within + the repository `reponame`. + """ + resource = Resource('source', path, + parent=Resource('repository', reponame)) + check = self.policy.check_permission('FILE_VIEW', user, resource, None) + self.assertEqual(result, check) + + def test_default_permission(self): + # By default, no permission is granted + self.assertPermission(False, 'joe', '', '/not_defined') + self.assertPermission(False, 'jane', 'repo', '/not/defined/either') + + def test_read_write(self): + # Allow 'r' and 'rw' entries, deny 'w' and empty entries + self.assertPermission(True, 'user', '', '/readonly') + self.assertPermission(True, 'user', '', '/readwrite') + self.assertPermission(False, 'user', '', '/writeonly') + self.assertPermission(False, 'user', '', '/empty') + + def test_trailing_slashes(self): + # Combinations of trailing slashes in the file and in the path + self.assertPermission(True, 'user', '', '/trailing_a') + self.assertPermission(True, 'user', '', '/trailing_a/') + self.assertPermission(True, 'user', '', '/trailing_b') + self.assertPermission(True, 'user', '', '/trailing_b/') + + def test_sub_path(self): + # Permissions are inherited from containing directories + self.assertPermission(True, 'user', '', '/sub/path') + self.assertPermission(True, 'user', '', '/sub/path/test') + self.assertPermission(True, 'user', '', '/sub/path/other/sub') + + def test_module_usage(self): + # If a module name is specified, the rules are specific to the module + self.assertPermission(True, 'user', 'module', '/module_a') + self.assertPermission(False, 'user', 'module', '/module_b') + # If a module is specified, but the configuration contains a non-module + # path, the non-module path can still apply + self.assertPermission(True, 'user', 'module', '/module_c') + # The module-specific rule takes precedence + self.assertPermission(False, 'user', 'module', '/module_d') + + def test_wildcard(self): + # The * wildcard matches all users + self.assertPermission(True, 'joe', '', '/wildcard') + self.assertPermission(True, 'jane', '', '/wildcard') + + def test_groups(self): + # Groups are specified in a separate section and used with an @ prefix + self.assertPermission(True, 'user', '', '/groups_a') + # Groups can also be members of other groups + self.assertPermission(True, 'user', '', '/groups_b') + # Groups should not be defined cyclically, but they are still handled + # correctly to avoid infinite loops + self.assertPermission(True, 'user', '', '/cyclic') + + def test_precedence(self): + # Module-specific sections take precedence over non-module sections + self.assertPermission(False, 'user', 'module', '/precedence_a') + # The most specific section applies + self.assertPermission(True, 'user', '', '/precedence_b/sub/test') + self.assertPermission(False, 'user', '', '/precedence_b/sub') + self.assertPermission(True, 'user', '', '/precedence_b') + # Within a section, the first matching rule applies + self.assertPermission(False, 'user', '', '/precedence_c') + self.assertPermission(True, 'user', '', '/precedence_d') + + def test_aliases(self): + # Aliases are specified in a separate section and used with an & prefix + self.assertPermission(True, 'Mr Hyde', '', '/aliases_a') + # Aliases can also be used in groups + self.assertPermission(True, 'Mr Hyde', '', '/aliases_b') + def suite(): - try: - from doctest import DocTestSuite - return DocTestSuite(sys.modules[__name__]) - except ImportError: - print >> sys.stderr, "WARNING: DocTestSuite required to run these " \ - "tests" - return unittest.TestSuite() + suite = unittest.TestSuite() + suite.addTest(unittest.makeSuite(AuthzParserTestCase, 'test')) + suite.addTest(unittest.makeSuite(AuthzSourcePolicyTestCase, 'test')) + return suite + if __name__ == '__main__': runner = unittest.TextTestRunner() diff --git a/trac/versioncontrol/tests/svn_fs.py b/trac/versioncontrol/tests/svn_fs.py --- a/trac/versioncontrol/tests/svn_fs.py +++ b/trac/versioncontrol/tests/svn_fs.py @@ -87,7 +87,7 @@ def setUp(self): self.repos = SubversionRepository(REPOS_PATH, {'name': 'repo', 'id': 1}, - None, logger_factory('test')) + logger_factory('test')) def tearDown(self): self.repos = None @@ -510,7 +510,7 @@ def setUp(self): self.repos = SubversionRepository(REPOS_PATH + u'/tête', {'name': 'repo', 'id': 1}, - None, logger_factory('test')) + logger_factory('test')) def tearDown(self): self.repos = None @@ -760,7 +760,7 @@ def setUp(self): self.repos = SubversionRepository(REPOS_PATH + u'/tête/dir1', {'name': 'repo', 'id': 1}, - None, logger_factory('test')) + logger_factory('test')) def tearDown(self): self.repos = None @@ -781,7 +781,7 @@ def setUp(self): self.repos = SubversionRepository(REPOS_PATH + '/tags/v1', {'name': 'repo', 'id': 1}, - None, logger_factory('test')) + logger_factory('test')) def tearDown(self): self.repos = None @@ -800,7 +800,7 @@ def setUp(self): self.repos = SubversionRepository(REPOS_PATH + '/branches', {'name': 'repo', 'id': 1}, - None, logger_factory('test')) + logger_factory('test')) def tearDown(self): self.repos = None diff --git a/trac/versioncontrol/web_ui/browser.py b/trac/versioncontrol/web_ui/browser.py --- a/trac/versioncontrol/web_ui/browser.py +++ b/trac/versioncontrol/web_ui/browser.py @@ -28,7 +28,7 @@ from trac.perm import IPermissionRequestor from trac.resource import Resource, ResourceNotFound from trac.util import embedded_numbers -from trac.util.compat import all +from trac.util.compat import any from trac.util.datefmt import http_date, utc from trac.util.html import escape, Markup from trac.util.text import exception_to_unicode, shorten_line @@ -290,12 +290,13 @@ return 'browser' def get_navigation_items(self, req): - if 'BROWSER_VIEW' in req.perm: - all_repos = RepositoryManager(self.env).get_all_repositories() - if all_repos and not all(info.get('hidden', False) - for info in all_repos.itervalues()): - yield ('mainnav', 'browser', - tag.a(_('Browse Source'), href=req.href.browser())) + rm = RepositoryManager(self.env) + all_repos = rm.get_all_repositories() + if any(info.get('hidden') not in _TRUE_VALUES + and rm.get_repository(reponame).can_view(req.perm) + for reponame, info in all_repos.iteritems()): + yield ('mainnav', 'browser', + tag.a(_('Browse Source'), href=req.href.browser())) # IPermissionRequestor methods @@ -332,14 +333,14 @@ xhr = req.get_header('X-Requested-With') == 'XMLHttpRequest' rm = RepositoryManager(self.env) - reponame, repos, path = rm.get_repository_by_path(path, req.authname) + reponame, repos, path = rm.get_repository_by_path(path) # Repository index all_repositories = None if not reponame and path == '/': all_repositories = rm.get_all_repositories() - if all_repositories and repos \ - and all_repositories[''].get('hidden') in _TRUE_VALUES: + if repos and (all_repositories[''].get('hidden') in _TRUE_VALUES + or not repos.can_view(req.perm)): repos = None if not repos and reponame: @@ -467,8 +468,6 @@ # Internal methods def _render_repository_index(self, context, all_repositories, order, desc): - context.perm.require('BROWSER_VIEW') - # Color scale for the age column timerange = custom_colorizer = None if self.color_scale: @@ -480,8 +479,10 @@ if not reponame or repoinfo.get('hidden') in _TRUE_VALUES: continue try: - repos = rm.get_repository(reponame, context.perm.username) + repos = rm.get_repository(reponame) if repos: + if not repos.can_view(context.perm): + continue youngest = repos.get_changeset(repos.youngest_rev) if self.color_scale and youngest: if not timerange: @@ -509,7 +510,7 @@ 'timerange': timerange, 'colorize_age': custom_colorizer} def _render_dir(self, req, reponame, repos, node, rev, order, desc): - req.perm.require('BROWSER_VIEW') + req.perm(node.resource).require('BROWSER_VIEW') # Entries metadata class entry(object): @@ -518,7 +519,8 @@ for f in entry.__slots__: setattr(self, f, getattr(node, f)) - entries = [entry(n) for n in node.get_entries()] + entries = [entry(n) for n in node.get_entries() + if n.can_view(req.perm)] changes = get_changes(repos, [i.rev for i in entries]) if rev: @@ -577,7 +579,7 @@ } def _render_file(self, req, context, reponame, repos, node, rev=None): - req.perm(context.resource).require('FILE_VIEW') + req.perm(node.resource).require('FILE_VIEW') mimeview = Mimeview(self.env) @@ -777,12 +779,13 @@ order = kwargs.get('order') desc = kwargs.get('desc', 0) - all_repositories = [rdata for rdata in RepositoryManager(self.env). - get_all_repositories().items() - if fnmatchcase(rdata[0], glob)] + rm = RepositoryManager(self.env) + all_repos = dict(rdata for rdata in rm.get_all_repositories().items() + if fnmatchcase(rdata[0], glob)) + if format == 'table': data = self._render_repository_index( - formatter.context, all_repositories, order, desc) + formatter.context, all_repos, order, desc) add_stylesheet(formatter.req, 'common/css/browser.css') from trac.web.chrome import Chrome @@ -791,19 +794,24 @@ {'repo': data}, None, fragment=True) def repolink(reponame): - return Markup(tag.a(reponame, - title=_('View repository %(repo)s', repo=reponame), + label = reponame or _('(default)') + return Markup(tag.a(label, + title=_('View repository %(repo)s', repo=label), href=formatter.href.browser(reponame or None))) + all_repos = sorted( + (reponame, info) for reponame, info in all_repos.iteritems() + if info.get('hidden') not in _TRUE_VALUE + and rm.get_repository(reponame).can_view(formatter.perm)) + if format == 'list': return tag.dl([ tag(tag.dt(repolink(reponame)), tag.dd(repoinfo.get('description'))) - for reponame, repoinfo in all_repositories]) + for reponame, repoinfo in all_repos]) else: # compact - return Markup(', ').join([ - repolink(reponame) for reponame, repoinfo in all_repositories - if reponame]) + return Markup(', ').join([repolink(reponame) + for reponame, repoinfo in all_repos]) diff --git a/trac/versioncontrol/web_ui/changeset.py b/trac/versioncontrol/web_ui/changeset.py --- a/trac/versioncontrol/web_ui/changeset.py +++ b/trac/versioncontrol/web_ui/changeset.py @@ -26,7 +26,6 @@ from StringIO import StringIO from genshi.builder import tag -from genshi.core import Markup from trac.config import Option, BoolOption, IntOption, _TRUE_VALUES from trac.core import * @@ -229,14 +228,13 @@ rm = RepositoryManager(self.env) if reponame: - repos = rm.get_repository(reponame, req.authname) + repos = rm.get_repository(reponame) else: - reponame, repos, new_path = rm.get_repository_by_path( - new_path, req.authname) + reponame, repos, new_path = rm.get_repository_by_path(new_path) if old_path: - old_reponame, old_repos, old_path = rm.get_repository_by_path( - old_path, req.authname) + old_reponame, old_repos, old_path = \ + rm.get_repository_by_path(old_path) if old_repos != repos: raise TracError(_("Can't compare across different " "repositories: %(old)s vs. %(new)s", @@ -254,9 +252,6 @@ try: new_path = repos.normalize_path(new_path) new = repos.normalize_rev(new) - - repos.authz.assert_permission_for_changeset(new) - old_path = repos.normalize_path(old_path or new_path) old = repos.normalize_rev(old or new) except NoSuchChangeset, e: @@ -310,9 +305,8 @@ data['wiki_format_messages'] = self.wiki_format_messages if chgset: - resource = Resource('repository', reponame).child('changeset', new) - req.perm(resource).require('CHANGESET_VIEW') chgset = repos.get_changeset(new) + req.perm(chgset.resource).require('CHANGESET_VIEW') # TODO: find a cheaper way to reimplement r2636 req.check_modified(chgset.date, [ @@ -324,7 +318,6 @@ format = req.args.get('format') if format in ['diff', 'zip']: - req.perm.require('FILE_VIEW') # choosing an appropriate filename rpath = new_path.replace('/','_') if chgset: @@ -413,9 +406,9 @@ # Support for revision properties (#2545) repos_resource = Resource('repository', reponame) - data['repos_resource'] = repos_resource context = Context.from_request(req, 'changeset', chgset.rev, parent=repos_resource) + data['context'] = context revprops = chgset.get_properties() data['properties'] = browser.render_properties('revprop', context, revprops) @@ -498,22 +491,12 @@ # with _that_ node specific history... options = data['diff']['options'] - repos_resource = Resource('repository', reponame) def _prop_changes(old_node, new_node): - old_source = Resource('source', old_node.created_path, - version=old_node.created_rev, - parent=repos_resource) - new_source = Resource('source', new_node.created_path, - version=new_node.created_rev, - parent=repos_resource) - old_props = new_props = [] - if 'FILE_VIEW' in req.perm(old_source): - old_props = old_node.get_properties() - if 'FILE_VIEW' in req.perm(new_source): - new_props = new_node.get_properties() - old_ctx = Context.from_request(req, old_source) - new_ctx = Context.from_request(req, new_source) + old_props = old_node.get_properties() + new_props = new_node.get_properties() + old_ctx = Context.from_request(req, old_node.resource) + new_ctx = Context.from_request(req, new_node.resource) changed_properties = [] if old_props != new_props: for k, v in sorted(old_props.items()): @@ -584,20 +567,19 @@ else: return [] - if 'FILE_VIEW' in req.perm: - diff_bytes = diff_files = 0 - if self.max_diff_bytes or self.max_diff_files: - for old_node, new_node, kind, change in get_changes(): - if change in Changeset.DIFF_CHANGES and kind == Node.FILE: - diff_files += 1 - diff_bytes += _estimate_changes(old_node, new_node) - show_diffs = (not self.max_diff_files or \ - diff_files <= self.max_diff_files) and \ - (not self.max_diff_bytes or \ - diff_bytes <= self.max_diff_bytes or \ - diff_files == 1) - else: - show_diffs = False + diff_bytes = diff_files = 0 + if self.max_diff_bytes or self.max_diff_files: + for old_node, new_node, kind, change in get_changes(): + if change in Changeset.DIFF_CHANGES and kind == Node.FILE \ + and old_node.can_view(req.perm) \ + and new_node.can_view(req.perm): + diff_files += 1 + diff_bytes += _estimate_changes(old_node, new_node) + show_diffs = (not self.max_diff_files or \ + 0 < diff_files <= self.max_diff_files) and \ + (not self.max_diff_bytes or \ + diff_bytes <= self.max_diff_bytes or \ + diff_files == 1) # XHR is used for blame support: display the changeset view without # the navigation and with the changes concerning the annotated file @@ -613,10 +595,12 @@ for old_node, new_node, kind, change in get_changes(): props = [] diffs = [] + show_old = old_node and old_node.can_view(req.perm) + show_new = new_node and new_node.can_view(req.perm) show_entry = change != Changeset.EDIT show_diff = show_diffs or (new_node and new_node.path == annotated) - if change in Changeset.DIFF_CHANGES and 'FILE_VIEW' in req.perm: + if change in Changeset.DIFF_CHANGES and show_old and show_new: assert old_node and new_node props = _prop_changes(old_node, new_node) if props: @@ -628,7 +612,7 @@ has_diffs = True # elif None (means: manually compare to (previous)) show_entry = True - if show_entry or not show_diff: + if (show_old or show_new) and (show_entry or not show_diff): info = {'change': change, 'old': old_node and node_info(old_node, annotated), 'new': new_node and node_info(new_node, annotated), @@ -692,8 +676,8 @@ mimeview = Mimeview(self.env) for old_node, new_node, kind, change in repos.get_changes( - new_path=data['new_path'], new_rev=data['new_rev'], - old_path=data['old_path'], old_rev=data['old_rev']): + new_path=data['new_path'], new_rev=data['new_rev'], + old_path=data['old_path'], old_rev=data['old_rev']): # TODO: Property changes # Content changes @@ -702,9 +686,10 @@ new_content = old_content = '' new_node_info = old_node_info = ('','') - mimeview = Mimeview(self.env) if old_node: + if not old_node.can_view(req.perm): + continue if mimeview.is_binary(old_node.content_type, old_node.path): continue old_content = old_node.get_content().read() @@ -714,6 +699,8 @@ old_content = mimeview.to_unicode(old_content, old_node.content_type) if new_node: + if not new_node.can_view(req.perm): + continue if mimeview.is_binary(new_node.content_type, new_node.path): continue new_content = new_node.get_content().read() @@ -770,8 +757,8 @@ for old_node, new_node, kind, change in repos.get_changes( new_path=data['new_path'], new_rev=data['new_rev'], old_path=data['old_path'], old_rev=data['old_rev']): - if kind == Node.FILE and change != Changeset.DELETE: - assert new_node + if kind == Node.FILE and change != Changeset.DELETE \ + and new_node.can_view(req.perm): zipinfo = ZipInfo() zipinfo.filename = new_node.path.strip('/').encode('utf-8') # Note: unicode filenames are not supported by zipfile. @@ -860,8 +847,10 @@ rm = RepositoryManager(self.env) repositories = rm.get_all_repositories() if len(repositories) > 1: - visible_repos = set(name for name, info in repositories.items() - if info.get('hidden') not in _TRUE_VALUES) + visible_repos = set( + name for name, info in repositories.items() + if info.get('hidden') not in _TRUE_VALUES + and rm.get_repository(name).can_view(req.perm)) default_is_aliased = any(info.get('alias') == '' and name in visible_repos for name, info in repositories.items()) @@ -912,7 +901,7 @@ for cset in changesets: cset_resource = Resource('changeset', cset.rev, parent=repos_resource) - if 'CHANGESET_VIEW' in req.perm(cset_resource): + if cset.can_view(req.perm): repos_for_uid = [reponame] uid = repos.get_changeset_uid(cset.rev) if uid: @@ -933,7 +922,7 @@ for reponame in rm.get_all_repositories(): if all_repos or ('repo-' + reponame) in repo_filters: try: - repos = rm.get_repository(reponame, req.authname) + repos = rm.get_repository(reponame) for event in generate_changesets(reponame, repos): yield event except TracError, e: @@ -971,6 +960,10 @@ filestats = self._prepare_filestats() for c, r, repos_for_c in changesets: for chg in c.get_changes(): + resource = c.resource.parent.child('source', + chg[0] or '/', r.id) + if not 'FILE_VIEW' in context.perm(resource): + continue filestats[chg[2]] += 1 files.append(chg[0]) stats = [(tag.div(class_=kind), @@ -987,6 +980,10 @@ elif show_files: for c, r, repos_for_c in changesets: for chg in c.get_changes(): + resource = c.resource.parent.child('source', + chg[0] or '/', r.id) + if not 'FILE_VIEW' in context.perm(resource): + continue if show_files > 0 and len(files) > show_files: break files.append(tag.li(tag.div(class_=chg[2]), @@ -1046,7 +1043,6 @@ # identifying repository rm = RepositoryManager(self.env) - authname = formatter.perm.username chgset, params, fragment = formatter.split_link(chgset) sep = chgset.find('/') if sep > 0: @@ -1055,21 +1051,25 @@ rev, path = chgset, '/' reponame = rm.get_default_repository(formatter.context) if reponame is not None: - repos = rm.get_repository(reponame, authname) + repos = rm.get_repository(reponame) else: - reponame, repos, path = rm.get_repository_by_path(path, authname) + reponame, repos, path = rm.get_repository_by_path(path) if path == '/': path = None # rendering changeset link - resource = Resource('repository', reponame).child('changeset', rev) - if repos and 'CHANGESET_VIEW' in formatter.perm(resource): + if repos: try: changeset = repos.get_changeset(rev) - href = formatter.href.changeset(rev, reponame or None, path) - return tag.a(label, class_="changeset", - title=shorten_line(changeset.message), - href=href + params + fragment) + if changeset.can_view(formatter.perm): + href = formatter.href.changeset(rev, reponame or None, + path) + return tag.a(label, class_="changeset", + title=shorten_line(changeset.message), + href=href + params + fragment) + errmsg = _("No permission to view changset %(rev)s " + "on %(repos)s", rev=rev, + repos=reponame or _('(default)')) except TracError, e: errmsg = to_unicode(e) elif reponame: @@ -1128,13 +1128,8 @@ repos = self.env.get_repository(reponame) if not repos: continue # revisions for a no longer active repository - if not repos.authz.has_permission_for_changeset(rev): - continue - # FIXME get rid of .authz and use only the normal Permission system - #cset = Resource('repository', reponame).child('changeset' , rev) - #cset = repos.resource.child('changeset' , rev) - #cset = repos.changeset_resource(rev) - cset = Resource('repository', reponame).child('changeset', rev) + cset = Resource('repository', repos.reponame).child('changeset', + rev) if 'CHANGESET_VIEW' in req.perm(cset): yield (req.href.changeset(rev, reponame or None), '[%s]: %s' % (rev, shorten_line(log)), @@ -1157,20 +1152,20 @@ if req.get_header('X-Requested-With') == 'XMLHttpRequest': dirname, prefix = posixpath.split(req.args.get('q')) prefix = prefix.lower() - reponame, repos, path = rm.get_repository_by_path(dirname, - req.authname) + reponame, repos, path = rm.get_repository_by_path(dirname) # an entry is a (isdir, name, path) tuple def kind_order(entry): return (not entry[0], embedded_numbers(entry[1])) if repos: - node = repos.get_node(path) entries = [(e.isdir, e.name, '/' + posixpath.join(reponame, e.path)) - for e in repos.get_node(path).get_entries()] - else: - entries = [(True, r, '/' + r) - for r in rm.get_all_repositories()] + for e in repos.get_node(path).get_entries() + if e.can_view(req.perm)] + if not reponame: + entries.extend((True, name, '/' + name) + for name in rm.get_all_repositories() + if rm.get_repository(name).can_view(req.perm)) elem = tag.ul( [tag.li(isdir and tag.b(path) or path) @@ -1190,16 +1185,12 @@ # -- normalize new_reponame, new_repos, new_path = \ - rm.get_repository_by_path(new_path, req.authname) + rm.get_repository_by_path(new_path) old_reponame, old_repos, old_path = \ - rm.get_repository_by_path(old_path, req.authname) + rm.get_repository_by_path(old_path) new_rev = new_repos.normalize_rev(new_rev) old_rev = old_repos.normalize_rev(old_rev) - # FIXME: replace by fine grained permission checks - new_repos.authz.assert_permission_for_changeset(new_rev) - old_repos.authz.assert_permission_for_changeset(old_rev) - # -- prepare rendering data = {'new_path': posixpath.join(new_reponame, new_path), 'new_rev': new_rev, diff --git a/trac/versioncontrol/web_ui/log.py b/trac/versioncontrol/web_ui/log.py --- a/trac/versioncontrol/web_ui/log.py +++ b/trac/versioncontrol/web_ui/log.py @@ -71,7 +71,7 @@ return True def process_request(self, req): - req.perm.assert_permission('LOG_VIEW') + req.perm.require('LOG_VIEW') mode = req.args.get('mode', 'stop_on_copy') path = req.args.get('path', '/') @@ -83,8 +83,7 @@ limit = int(req.args.get('limit') or self.default_log_limit) reponame, repos, path = RepositoryManager(self.env).\ - get_repository_by_path(path, req.authname) - repos_resource = Resource('repository', reponame) + get_repository_by_path(path) normpath = repos.normalize_path(path) # if `revs` parameter is given, then we're restricted to the @@ -105,12 +104,14 @@ # unless explicit ranges have been specified # * for ''show only add, delete'' we're using # `Repository.get_path_history()` + cset_resource = repos.resource.child('changeset') if mode == 'path_history': - def history(limit): - for h in repos.get_path_history(path, rev, limit): - yield h + def history(): + for h in repos.get_path_history(path, rev): + if 'CHANGESET_VIEW' in req.perm(cset_resource(id=h[1])): + yield h elif revranges: - def history(limit): + def history(): prevpath = path expected_next_item = None ranges = list(revranges.pairs) @@ -123,14 +124,15 @@ p, rev, chg = node_history[0] if rev < a: break # simply skip, no separator - if expected_next_item: - # check whether we're continuing previous range - np, nrev, nchg = expected_next_item - if rev != nrev: # no, we need a separator - yield (np, nrev, None) - yield node_history[0] + if 'CHANGESET_VIEW' in req.perm(cset_resource(id=rev)): + if expected_next_item: + # check whether we're continuing previous range + np, nrev, nchg = expected_next_item + if rev != nrev: # no, we need a separator + yield (np, nrev, None) + yield node_history[0] prevpath = node_history[-1][0] # follow copy - b = rev-1 + b = rev - 1 if len(node_history) > 1: expected_next_item = node_history[-1] else: @@ -138,14 +140,18 @@ if expected_next_item: yield (expected_next_item[0], expected_next_item[1], None) else: - history = get_existing_node(req, repos, path, rev).get_history + def history(): + node = get_existing_node(req, repos, path, rev) + for h in node.get_history(): + if 'CHANGESET_VIEW' in req.perm(cset_resource(id=h[1])): + yield h # -- retrieve history, asking for limit+1 results info = [] depth = 1 previous_path = normpath count = 0 - for old_path, old_rev, old_chg in history(limit+1): + for old_path, old_rev, old_chg in history(): if stop_rev and repos.rev_older_than(old_rev, stop_rev): break old_path = repos.normalize_path(old_path) @@ -241,11 +247,11 @@ cs['actions'] = actions extra_changes[rev] = cs + repos_resource = Resource('repository', reponame) data = { 'context': Context.from_request(req, 'source', path, parent=repos_resource), - 'reponame': reponame or None, 'repos_resource': repos_resource, - 'path': path, 'rev': rev, 'stop_rev': stop_rev, + 'reponame': reponame or None, 'repos': repos, 'path': path, 'rev': rev, 'stop_rev': stop_rev, 'revranges': revranges, 'mode': mode, 'verbose': verbose, 'limit' : limit, @@ -350,12 +356,11 @@ path, revs = match[:idx], match[idx+1:] rm = RepositoryManager(self.env) - authname = formatter.perm.username reponame = rm.get_default_repository(formatter.context) if reponame is not None: - repos = rm.get_repository(reponame, authname) + repos = rm.get_repository(reponame) else: - reponame, repos, path = rm.get_repository_by_path(path, authname) + reponame, repos, path = rm.get_repository_by_path(path) revranges = None if any(c for c in ':-,' if c in revs): diff --git a/trac/versioncontrol/web_ui/tests/wikisyntax.py b/trac/versioncontrol/web_ui/tests/wikisyntax.py --- a/trac/versioncontrol/web_ui/tests/wikisyntax.py +++ b/trac/versioncontrol/web_ui/tests/wikisyntax.py @@ -11,7 +11,7 @@ def _get_changeset(rev): if rev == '1': - return Mock(message="start") + return Mock(message="start", can_view=lambda perm: True) else: raise NoSuchChangeset(rev)