From 1df7d4039e20c18e9460d37ee29399dcaffeac45 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 2 Sep 2011 17:20:14 -0700 Subject: [PATCH] Store repository credentials with repositories Summary: Move toward storing credentials in configuration so it's easier to get the daemons working. This should eventually solve all the key juggling junk you have to do right now. This only gets us part of the way to actually using these credentials in the daemons since I have to go swap everything for $repository->execBlah(). I tried to write a web "Test Connection" button but it was too much of a mess to get git to work since git doesn't give you access to its SSH command and SSH has a bunch of interactive prompts which you can't really do anything about without it or a bunch of ~/.ssh/config editing. This is what Git recommends: https://git.wiki.kernel.org/index.php/GitFaq#How_do_I_specify_what_ssh_key_git_should_use.3F ..but it's not a great match for this use case. Test Plan: - Only partial. - Ran "test_connection.php" on a Git repo with and without SSH, and with and without valid credentials. This part works properly. - Ran "test_connection.php" on a public SVN repo, but I don't have private or WEBDAV repos set up at the moment. - Mercurial doesn't work yet. - Daemons haven't been converted yet. Reviewers: jungejason, tuomaspelkonen, aran Reviewed By: jungejason CC: aran, abdul, nmalcolm, epriestley, jungejason Differential Revision: 888 --- scripts/repository/test_connection.php | 90 +++++++++ src/__celerity_resource_map__.php | 20 +- .../PhabricatorRepositoryEditController.php | 77 ++++++-- .../repository/controller/edit/__init__.php | 1 + .../repository/PhabricatorRepository.php | 181 ++++++++++++++++++ .../storage/repository/__init__.php | 6 + webroot/index.php | 2 +- 7 files changed, 355 insertions(+), 22 deletions(-) create mode 100755 scripts/repository/test_connection.php diff --git a/scripts/repository/test_connection.php b/scripts/repository/test_connection.php new file mode 100755 index 0000000000..3f511d40c5 --- /dev/null +++ b/scripts/repository/test_connection.php @@ -0,0 +1,90 @@ +#!/usr/bin/env php +\n"; + exit(1); +} + +echo phutil_console_wrap( + phutil_console_format( + 'This script will test that you have configured valid credentials for '. + 'access to a repository, so the Phabricator daemons can pull from it. '. + 'You should run this as the **same user you will run the daemons as**, '. + 'from the **same machine they will run from**. Doing this will help '. + 'detect various problems with your configuration, such as SSH issues.')); + +list($whoami) = execx('whoami'); +$whoami = trim($whoami); + +$ok = phutil_console_confirm("Do you want to continue as '{$whoami}'?"); +if (!$ok) { + die(1); +} + +$callsign = $argv[1]; +echo "Loading '{$callsign}' repository...\n"; +$repository = id(new PhabricatorRepository())->loadOneWhere( + 'callsign = %s', + $argv[1]); +if (!$repository) { + throw new Exception("No such repository exists!"); +} + +$vcs = $repository->getVersionControlSystem(); + +PhutilServiceProfiler::installEchoListener(); + +echo "Trying to connect to the remote...\n"; +switch ($vcs) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $err = $repository->passthruRemoteCommand( + '--limit 1 log %s', + $repository->getRemoteURI()); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + // Do an ls-remote on a nonexistent ref, which we expect to just return + // nothing. + $err = $repository->passthruRemoteCommand( + 'ls-remote %s %s', + $repository->getRemoteURI(), + 'just-testing'); + break; + default: + throw new Exception("Unsupported repository type."); +} + +if ($err) { + echo phutil_console_format( + "** FAIL ** Connection failed. The credentials for this ". + "repository appear to be incorrectly configured.\n"); + exit(1); +} else { + echo phutil_console_format( + "** OKAY ** Connection successful. The credentials for ". + "this repository appear to be correctly configured.\n"); +} + diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 21038f4976..83d7e1f9cc 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -652,6 +652,17 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/maniphest/behavior-transaction-preview.js', ), + 0 => + array( + 'uri' => '/res/1da00bfe/rsrc/js/javelin/lib/__tests__/URI.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-uri', + 1 => 'javelin-php-serializer', + ), + 'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js', + ), 'javelin-behavior-owners-path-editor' => array( 'uri' => '/res/9cf78ffc/rsrc/js/application/owners/owners-path-editor.js', @@ -1283,15 +1294,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/core/Prefab.js', ), - 0 => - array( - 'uri' => '/res/936e8e81/rsrc/js/javelin/docs/onload.js', - 'type' => 'js', - 'requires' => - array( - ), - 'disk' => '/rsrc/js/javelin/docs/onload.js', - ), 'phabricator-profile-css' => array( 'uri' => '/res/ebe1ac2f/rsrc/css/application/profile/profile-view.css', diff --git a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php index 159cc45c0c..91d3715d2d 100644 --- a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php +++ b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php @@ -239,6 +239,9 @@ class PhabricatorRepositoryEditController 'default-owners-path', '/')); + $repository->setDetail('ssh-login', $request->getStr('ssh-login')); + $repository->setDetail('ssh-key', $request->getStr('ssh-key')); + $repository->setDetail( 'herald-disabled', $request->getInt('herald-disabled', 0)); @@ -329,10 +332,14 @@ class PhabricatorRepositoryEditController 'Differential, Diffusion, Herald, and other services. To enable '. 'tracking for a repository, configure it here and then start (or '. 'restart) the daemons. More information is available in the '. - ''.$user_guide_link.'.

') + ''.$user_guide_link.'.

'); + + $form + ->appendChild( + '

Basics

') ->appendChild( id(new AphrontFormStaticControl()) - ->setLabel('Repository') + ->setLabel('Repository Name') ->setValue($repository->getName())) ->appendChild( id(new AphrontFormSelectControl()) @@ -345,13 +352,19 @@ class PhabricatorRepositoryEditController ->setValue( $repository->getDetail('tracking-enabled') ? 'enabled' - : 'disabled')); + : 'disabled')) + ->appendChild('
'); + + $form->appendChild( + '

Remote URI

'. + '
'); $uri_label = 'Repository URI'; if ($is_git) { $instructions = - 'NOTE: The user the tracking daemon runs as must have permission to '. - 'git clone from this URI.'; + 'Enter the URI to clone this repository from. It should look like '. + 'git@github.com:example/example.git or '. + 'ssh://user@host.com/git/example.git'; $form->appendChild( '

'.$instructions.'

'); } else if ($is_svn) { @@ -360,10 +373,7 @@ class PhabricatorRepositoryEditController 'You can figure this out by running svn info and looking at '. 'the value in the Repository Root field. It should be a URI '. 'and look like http://svn.example.org/svn/ or '. - 'svn+ssh://svn.example.com/svnroot/.'. - '

'. - 'NOTE: The user the daemons run as must be able to execute '. - 'svn log against this URI.'; + 'svn+ssh://svn.example.com/svnroot/'; $form->appendChild( '

'.$instructions.'

'); $uri_label = 'Repository Root'; @@ -374,9 +384,44 @@ class PhabricatorRepositoryEditController id(new AphrontFormTextControl()) ->setName('uri') ->setLabel($uri_label) + ->setID('remote-uri') ->setValue($repository->getDetail('remote-uri')) ->setError($e_uri)); + $form->appendChild( + '
'. + 'If you want to connect to this repository over SSH, enter the '. + 'username and private key to use. You can leave these fields blank if '. + 'the repository does not use SSH. NOTE: This feature is not '. + 'yet fully supported.'. + '
'); + + $form + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('ssh-login') + ->setLabel('SSH User') + ->setValue($repository->getDetail('ssh-login'))) + ->appendChild( + id(new AphrontFormTextAreaControl()) + ->setName('ssh-key') + ->setLabel('SSH Private Key') + ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT) + ->setValue($repository->getDetail('ssh-key'))) + ->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel('Test Connection') + ->setValue( + 'To test these credentials, save this form and '. + 'then run: phabricator/scripts/repository/test_connection'. + '.php '.phutil_escape_html($repository->getCallsign()).'')); + + $form->appendChild('
'); + + $form->appendChild( + '

Importing Repository Information

'. + '
'); + if ($is_git) { $form->appendChild( '

Select a path on local disk '. @@ -415,6 +460,12 @@ class PhabricatorRepositoryEditController 'Number of seconds daemon should sleep between requests. Larger '. 'numbers reduce load but also decrease responsiveness.')); + $form->appendChild('

'); + + $form->appendChild( + '

Application Configuration

'. + '
'); + if ($is_git) { $form ->appendChild( @@ -452,8 +503,8 @@ class PhabricatorRepositoryEditController 1 => 'Disabled - Do Not Send Email', )) ->setCaption( - 'You can temporarily disable Herald notifications when reparsing '. - 'a repository or importing a new repository.')); + 'You can temporarily disable Herald commit notifications when '. + 'reparsing a repository or importing a new repository.')); $parsers = id(new PhutilSymbolLoader()) ->setAncestorClass('PhabricatorRepositoryCommitMessageDetailParser') @@ -487,10 +538,12 @@ class PhabricatorRepositoryEditController ->setCaption('Repository UUID from svn info.')); } + $form->appendChild('
'); + $form ->appendChild( id(new AphrontFormSubmitControl()) - ->setValue('Save')); + ->setValue('Save Configuration')); $panel = new AphrontPanelView(); $panel->setHeader('Repository Tracking'); diff --git a/src/applications/repository/controller/edit/__init__.php b/src/applications/repository/controller/edit/__init__.php index 7da55db338..4f61cde518 100644 --- a/src/applications/repository/controller/edit/__init__.php +++ b/src/applications/repository/controller/edit/__init__.php @@ -17,6 +17,7 @@ phutil_require_module('phabricator', 'applications/repository/storage/repository phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'view/control/table'); phutil_require_module('phabricator', 'view/form/base'); +phutil_require_module('phabricator', 'view/form/control/markup'); phutil_require_module('phabricator', 'view/form/control/select'); phutil_require_module('phabricator', 'view/form/control/static'); phutil_require_module('phabricator', 'view/form/control/submit'); diff --git a/src/applications/repository/storage/repository/PhabricatorRepository.php b/src/applications/repository/storage/repository/PhabricatorRepository.php index 0235e1a435..bb1181ceea 100644 --- a/src/applications/repository/storage/repository/PhabricatorRepository.php +++ b/src/applications/repository/storage/repository/PhabricatorRepository.php @@ -32,6 +32,8 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { protected $versionControlSystem; protected $details = array(); + private $sshKeyfile; + public function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -55,4 +57,183 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { return $this; } + public function getRemoteURI() { + $raw_uri = $this->getDetail('remote-uri'); + + $vcs = $this->getVersionControlSystem(); + $is_git = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT); + + // If there's no protocol (git implicit SSH) reformat the URI to be a + // normal URI. These git URIs look like "user@domain.com:path" instead of + // "ssh://user@domain/path". + $uri = new PhutilURI($raw_uri); + if ($is_git && !$uri->getProtocol()) { + list($domain, $path) = explode(':', $raw_uri, 2); + $uri = new PhutilURI('ssh://'.$domain.'/'.$path); + } + + if ($this->isSSHProtocol($uri->getProtocol())) { + if ($this->getSSHLogin()) { + $uri->setUser($this->getSSHLogin()); + } + } + + return (string)$uri; + } + + public function getLocalPath() { + return $this->getDetail('local-path'); + } + + public function execRemoteCommand($pattern /*, $arg, ... */) { + $args = func_get_args(); + $args = $this->formatRemoteCommand($args); + return call_user_func_array('exec_manual', $args); + } + + public function execxRemoteCommand($pattern /*, $arg, ... */) { + $args = func_get_args(); + $args = $this->formatRemoteCommand($args); + return call_user_func_array('execx', $args); + } + + public function passthruRemoteCommand($pattern /*, $arg, ... */) { + $args = func_get_args(); + $args = $this->formatRemoteCommand($args); + return call_user_func_array('phutil_passthru', $args); + } + + public function execLocalCommand($pattern /*, $arg, ... */) { + $args = func_get_args(); + $args = $this->formatLocalCommand($args); + return call_user_func_array('exec_manual', $args); + } + + public function execxLocalCommand($pattern /*, $arg, ... */) { + $args = func_get_args(); + $args = $this->formatLocalCommand($args); + return call_user_func_array('execx', $args); + } + + public function passthruLocalCommand($pattern /*, $arg, ... */) { + $args = func_get_args(); + $args = $this->formatLocalCommand($args); + return call_user_func_array('phutil_passthru', $args); + } + + private function formatRemoteCommand(array $args) { + $pattern = $args[0]; + $args = array_slice($args, 1); + + if ($this->shouldUseSSH()) { + switch ($this->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $pattern = "SVN_SSH=%s svn {$pattern}"; + array_unshift( + $args, + csprintf( + 'ssh -l %s -i %s', + $this->getSSHLogin(), + $this->getSSHKeyfile())); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $command = call_user_func_array( + 'csprintf', + array_merge( + array( + "(ssh-add %s && git {$pattern})", + $this->getSSHKeyfile(), + ), + $args)); + $pattern = "ssh-agent sh -c %s"; + $args = array($command); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $pattern = "hg --config ui.ssh=%s {$pattern}"; + array_unshift( + $args, + csprintf( + 'ssh -l %s -i %s', + $this->getSSHLogin(), + $this->getSSHKeyfile())); + break; + default: + throw new Exception("Unrecognized version control system."); + } + } else { + switch ($this->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $pattern = "svn {$pattern}"; + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $pattern = "git {$pattern}"; + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $pattern = "hg {$pattern}"; + break; + default: + throw new Exception("Unrecognized version control system."); + } + } + + array_unshift($args, $pattern); + + return $args; + } + + private function formatLocalCommand(array $args) { + $pattern = $args[0]; + $args = array_slice($args, 1); + + switch ($this->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $pattern = "(cd %s && svn {$pattern})"; + array_unshift($args, $this->getLocalPath()); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $pattern = "(cd %s && git {$pattern})"; + array_unshift($args, $this->getLocalPath()); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $pattern = "(cd %s && hg {$pattern})"; + array_unshift($args, $this->getLocalPath()); + break; + default: + throw new Exception("Unrecognized version control system."); + } + + array_unshift($args, $pattern); + + return $args; + } + + private function getSSHLogin() { + return $this->getDetail('ssh-login'); + } + + private function getSSHKeyfile() { + if (!$this->sshKeyfile) { + $keyfile = new TempFile('phabricator-repository-ssh-key'); + chmod($keyfile, 0600); + Filesystem::writeFile($keyfile, $this->getDetail('ssh-key')); + $this->sshKeyfile = $keyfile; + } + + return (string)$this->sshKeyfile; + } + + public function shouldUseSSH() { + $uri = new PhutilURI($this->getRemoteURI()); + $protocol = $uri->getProtocol(); + if ($this->isSSHProtocol($protocol)) { + return (bool)$this->getDetail('ssh-key'); + } else { + return false; + } + } + + private function isSSHProtocol($protocol) { + return ($protocol == 'ssh' || $protocol == 'svn+ssh'); + } + } diff --git a/src/applications/repository/storage/repository/__init__.php b/src/applications/repository/storage/repository/__init__.php index 6480f38dac..e08a467030 100644 --- a/src/applications/repository/storage/repository/__init__.php +++ b/src/applications/repository/storage/repository/__init__.php @@ -8,9 +8,15 @@ phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); +phutil_require_module('phabricator', 'applications/repository/constants/repositorytype'); phutil_require_module('phabricator', 'applications/repository/storage/base'); +phutil_require_module('phutil', 'filesystem'); +phutil_require_module('phutil', 'filesystem/tempfile'); +phutil_require_module('phutil', 'future/exec'); +phutil_require_module('phutil', 'parser/uri'); phutil_require_module('phutil', 'utils'); +phutil_require_module('phutil', 'xsprintf/csprintf'); phutil_require_source('PhabricatorRepository.php'); diff --git a/webroot/index.php b/webroot/index.php index 44159da28b..2c2154fadb 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -250,7 +250,7 @@ function phabricator_shutdown() { return; } - if ($event['type'] != E_ERROR) { + if ($event['type'] != E_ERROR && $event['type'] != E_PARSE) { return; }