# HG changeset patch # User Daniel Dourvaris # Date 2020-03-31 07:25:37 # Node ID 5da17e74f2ad4eacc891bac2975e8b5b2076bf11 # Parent 4c0a23dbf1c31d49fb757859f5f266f17681eab7 svn: fixed case of wrong extracted repository name for SSH backend. In cases where we commited to a nested subdirs SVN reported the access path with the subdir paths in it. We couldn't then match that extended name into proper rhodecode repository for ACL checks. - Current implementation gives an slight overhead as we have to lookup all repositories - fixes #5606 diff --git a/rhodecode/apps/ssh_support/lib/backends/__init__.py b/rhodecode/apps/ssh_support/lib/backends/__init__.py --- a/rhodecode/apps/ssh_support/lib/backends/__init__.py +++ b/rhodecode/apps/ssh_support/lib/backends/__init__.py @@ -91,7 +91,6 @@ class SshWrapper(object): def get_repo_details(self, mode): vcs_type = mode if mode in ['svn', 'hg', 'git'] else None - mode = mode repo_name = None hg_pattern = r'^hg\s+\-R\s+(\S+)\s+serve\s+\-\-stdio$' @@ -101,8 +100,7 @@ class SshWrapper(object): repo_name = hg_match.group(1).strip('/') return vcs_type, repo_name, mode - git_pattern = ( - r'^git-(receive-pack|upload-pack)\s\'[/]?(\S+?)(|\.git)\'$') + git_pattern = r'^git-(receive-pack|upload-pack)\s\'[/]?(\S+?)(|\.git)\'$' git_match = re.match(git_pattern, self.command) if git_match is not None: vcs_type = 'git' @@ -115,7 +113,8 @@ class SshWrapper(object): if svn_match is not None: vcs_type = 'svn' - # Repo name should be extracted from the input stream + # Repo name should be extracted from the input stream, we're unable to + # extract it at this point in execution return vcs_type, repo_name, mode return vcs_type, repo_name, mode @@ -188,8 +187,7 @@ class SshWrapper(object): log.debug('SSH Connection info %s', self.get_connection_info()) if shell and self.command is None: - log.info( - 'Dropping to shell, no command given and shell is allowed') + log.info('Dropping to shell, no command given and shell is allowed') os.execl('/bin/bash', '-l') exit_code = 1 @@ -216,8 +214,7 @@ class SshWrapper(object): exit_code = -1 else: - log.error( - 'Unhandled Command: "%s" Aborting.', self.command) + log.error('Unhandled Command: "%s" Aborting.', self.command) exit_code = -1 return exit_code diff --git a/rhodecode/apps/ssh_support/lib/backends/base.py b/rhodecode/apps/ssh_support/lib/backends/base.py --- a/rhodecode/apps/ssh_support/lib/backends/base.py +++ b/rhodecode/apps/ssh_support/lib/backends/base.py @@ -66,9 +66,8 @@ class VcsServer(object): def _check_permissions(self, action): permission = self.user_permissions.get(self.repo_name) - log.debug( - 'permission for %s on %s are: %s', - self.user, self.repo_name, permission) + log.debug('permission for %s on %s are: %s', + self.user, self.repo_name, permission) if not permission: log.error('user `%s` permissions to repo:%s are empty. Forbidding access.', diff --git a/rhodecode/apps/ssh_support/lib/backends/git.py b/rhodecode/apps/ssh_support/lib/backends/git.py --- a/rhodecode/apps/ssh_support/lib/backends/git.py +++ b/rhodecode/apps/ssh_support/lib/backends/git.py @@ -68,8 +68,7 @@ class GitServer(VcsServer): self.store = store self.ini_path = ini_path self.repo_name = repo_name - self._path = self.git_path = config.get( - 'app:main', 'ssh.executable.git') + self._path = self.git_path = config.get('app:main', 'ssh.executable.git') self.repo_mode = repo_mode self.tunnel = GitTunnelWrapper(server=self) diff --git a/rhodecode/apps/ssh_support/lib/backends/svn.py b/rhodecode/apps/ssh_support/lib/backends/svn.py --- a/rhodecode/apps/ssh_support/lib/backends/svn.py +++ b/rhodecode/apps/ssh_support/lib/backends/svn.py @@ -95,9 +95,8 @@ class SubversionTunnelWrapper(object): signal.alarm(self.timeout) first_response = self._read_first_client_response() signal.alarm(0) - return ( - self._parse_first_client_response(first_response) - if first_response else None) + return (self._parse_first_client_response(first_response) + if first_response else None) def patch_first_client_response(self, response, **kwargs): self.create_hooks_env() @@ -112,9 +111,8 @@ class SubversionTunnelWrapper(object): self.process.stdin.write(buffer_) def fail(self, message): - print( - "( failure ( ( 210005 {message} 0: 0 ) ) )".format( - message=self._svn_string(message))) + print("( failure ( ( 210005 {message} 0: 0 ) ) )".format( + message=self._svn_string(message))) self.remove_configs() self.process.kill() return 1 @@ -139,6 +137,7 @@ class SubversionTunnelWrapper(object): brackets_stack.pop() elif next_byte == " " and not brackets_stack: break + return buffer_ def _parse_first_client_response(self, buffer_): @@ -149,8 +148,7 @@ class SubversionTunnelWrapper(object): ( version:number ( cap:word ... ) url:string ? ra-client:string ( ? client:string ) ) - Please check https://svn.apache.org/repos/asf/subversion/trunk/ - subversion/libsvn_ra_svn/protocol + Please check https://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/protocol """ version_re = r'(?P\d+)' capabilities_re = r'\(\s(?P[\w\d\-\ ]+)\s\)' @@ -163,8 +161,35 @@ class SubversionTunnelWrapper(object): version=version_re, capabilities=capabilities_re, url=url_re, ra_client=ra_client_re, client=client_re)) matcher = regex.match(buffer_) + return matcher.groupdict() if matcher else None + def _match_repo_name(self, url): + """ + Given an server url, try to match it against ALL known repository names. + This handles a tricky SVN case for SSH and subdir commits. + E.g if our repo name is my-svn-repo, a svn commit on file in a subdir would + result in the url with this subdir added. + """ + # case 1 direct match, we don't do any "heavy" lookups + if url in self.server.user_permissions: + return url + + log.debug('Extracting repository name from subdir path %s', url) + # case 2 we check all permissions, and match closes possible case... + # NOTE(dan): In this case we only know that url has a subdir parts, it's safe + # to assume that it will have the repo name as prefix, we ensure the prefix + # for similar repositories isn't matched by adding a / + # e.g subgroup/repo-name/ and subgroup/repo-name-1/ would work correct. + for repo_name in self.server.user_permissions: + repo_name_prefix = repo_name + '/' + if url.startswith(repo_name_prefix): + log.debug('Found prefix %s match, returning proper repository name', + repo_name_prefix) + return repo_name + + return + def run(self, extras): action = 'pull' self.create_svn_config() @@ -175,7 +200,8 @@ class SubversionTunnelWrapper(object): return self.fail("Repository name cannot be extracted") url_parts = urlparse.urlparse(first_response['url']) - self.server.repo_name = url_parts.path.strip('/') + + self.server.repo_name = self._match_repo_name(url_parts.path.strip('/')) exit_code = self.server._check_permissions(action) if exit_code: @@ -200,10 +226,10 @@ class SubversionServer(VcsServer): .__init__(user, user_permissions, config, env) self.store = store self.ini_path = ini_path - # this is set in .run() from input stream + # NOTE(dan): repo_name at this point is empty, + # this is set later in .run() based from parsed input stream self.repo_name = repo_name - self._path = self.svn_path = config.get( - 'app:main', 'ssh.executable.svn') + self._path = self.svn_path = config.get('app:main', 'ssh.executable.svn') self.tunnel = SubversionTunnelWrapper(server=self) diff --git a/rhodecode/apps/ssh_support/tests/test_server_svn.py b/rhodecode/apps/ssh_support/tests/test_server_svn.py --- a/rhodecode/apps/ssh_support/tests/test_server_svn.py +++ b/rhodecode/apps/ssh_support/tests/test_server_svn.py @@ -89,6 +89,86 @@ class TestSubversionServer(object): result = server._check_permissions(action) assert result is code + @pytest.mark.parametrize('permissions, access_paths, expected_match', [ + # not matched repository name + ({ + 'test-svn': '' + }, ['test-svn-1', 'test-svn-1/subpath'], + None), + + # exact match + ({ + 'test-svn': '' + }, + ['test-svn'], + 'test-svn'), + + # subdir commits + ({ + 'test-svn': '' + }, + ['test-svn/foo', + 'test-svn/foo/test-svn', + 'test-svn/trunk/development.txt', + ], + 'test-svn'), + + # subgroups + similar patterns + ({ + 'test-svn': '', + 'test-svn-1': '', + 'test-svn-subgroup/test-svn': '', + + }, + ['test-svn-1', + 'test-svn-1/foo/test-svn', + 'test-svn-1/test-svn', + ], + 'test-svn-1'), + + # subgroups + similar patterns + ({ + 'test-svn-1': '', + 'test-svn-10': '', + 'test-svn-100': '', + }, + ['test-svn-10', + 'test-svn-10/foo/test-svn', + 'test-svn-10/test-svn', + ], + 'test-svn-10'), + + # subgroups + similar patterns + ({ + 'name': '', + 'nameContains': '', + 'nameContainsThis': '', + }, + ['nameContains', + 'nameContains/This', + 'nameContains/This/test-svn', + ], + 'nameContains'), + + # subgroups + similar patterns + ({ + 'test-svn': '', + 'test-svn-1': '', + 'test-svn-subgroup/test-svn': '', + + }, + ['test-svn-subgroup/test-svn', + 'test-svn-subgroup/test-svn/foo/test-svn', + 'test-svn-subgroup/test-svn/trunk/example.txt', + ], + 'test-svn-subgroup/test-svn'), + ]) + def test_repo_extraction_on_subdir(self, svn_server, permissions, access_paths, expected_match): + server = svn_server.create(user_permissions=permissions) + for path in access_paths: + repo_name = server.tunnel._match_repo_name(path) + assert repo_name == expected_match + def test_run_returns_executes_command(self, svn_server): server = svn_server.create() from rhodecode.apps.ssh_support.lib.backends.svn import SubversionTunnelWrapper