From e3a9d73fe140ca53d010c2582df861f31d33e538 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 6 Sep 2011 09:37:35 -0700 Subject: [PATCH] Add keyfile and HTTP Basic auth support to repositories Summary: I still need to go through all the daemon and Diffusion code and change the bare execx() calls to $repository->execxXXX() to actually make this work, but we're getting close. Test Plan: Configured repositories with various HTTP / SVN setups and ran the test_connection.php script to verify keys were located and added and username/password information was supplied. Reviewers: jungejason, nh, tuomaspelkonen, aran Reviewed By: nh CC: aran, nh, jungejason Differential Revision: 902 --- .../PhabricatorRepositoryEditController.php | 75 ++++++++++++++++--- .../repository/controller/edit/__init__.php | 1 - .../repository/PhabricatorRepository.php | 62 ++++++++++++--- webroot/rsrc/css/aphront/form-view.css | 12 +++ 4 files changed, 131 insertions(+), 19 deletions(-) diff --git a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php index 91d3715d2d..414f8bd94e 100644 --- a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php +++ b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php @@ -205,6 +205,9 @@ class PhabricatorRepositoryEditController $is_git = false; $is_svn = false; + $e_ssh_key = null; + $e_ssh_keyfile = null; + switch ($repository->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: $is_git = true; @@ -241,6 +244,19 @@ class PhabricatorRepositoryEditController $repository->setDetail('ssh-login', $request->getStr('ssh-login')); $repository->setDetail('ssh-key', $request->getStr('ssh-key')); + $repository->setDetail('ssh-keyfile', $request->getStr('ssh-keyfile')); + + $repository->setDetail('http-login', $request->getStr('http-login')); + $repository->setDetail('http-pass', $request->getStr('http-pass')); + + if ($repository->getDetail('ssh-key') && + $repository->getDetail('ssh-keyfile')) { + $errors[] = + "Specify only one of 'SSH Private Key' and 'SSH Private Key File', ". + "not both."; + $e_ssh_key = 'Choose Only One'; + $e_ssh_keyfile = 'Choose Only One'; + } $repository->setDetail( 'herald-disabled', @@ -392,8 +408,8 @@ class PhabricatorRepositoryEditController '
'. '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.'. + 'the repository does not use SSH.'. + ' NOTE: This feature is not yet fully supported.'. '
'); $form @@ -407,14 +423,55 @@ class PhabricatorRepositoryEditController ->setName('ssh-key') ->setLabel('SSH Private Key') ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT) - ->setValue($repository->getDetail('ssh-key'))) + ->setValue($repository->getDetail('ssh-key')) + ->setError($e_ssh_key) + ->setCaption('Specify the entire private key, or...')) ->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()).'')); + id(new AphrontFormTextControl()) + ->setName('ssh-keyfile') + ->setLabel('SSH Private Key File') + ->setValue($repository->getDetail('ssh-keyfile')) + ->setError($e_ssh_keyfile) + ->setCaption( + '...specify a path on disk where the daemon should '. + 'look for a private key.')); + + $supports_http = $is_svn; + + if ($supports_http) { + $form + ->appendChild( + '
'. + 'If you want to connect to this repository over HTTP Basic Auth, '. + 'enter the username and password to use. You can leave these '. + 'fields blank if the repository does not use HTTP Basic Auth.'. + ' NOTE: This feature is not yet fully supported.'. + '
') + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('http-login') + ->setLabel('HTTP Basic Login') + ->setValue($repository->getDetail('http-login'))) + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('http-pass') + ->setLabel('HTTP Basic Password') + ->setValue($repository->getDetail('http-pass'))); + } + + $form + ->appendChild( + '
'. + 'To test your authentication configuration, save this '. + 'form and then run this script:'. + ''. + 'phabricator/ $ ./scripts/repository/test_connection.php '. + phutil_escape_html($repository->getCallsign()). + ''. + 'This will verify that your configuration is correct and the '. + 'daemons can connect to the remote repository and pull changes '. + 'from it.'. + '
'); $form->appendChild(''); diff --git a/src/applications/repository/controller/edit/__init__.php b/src/applications/repository/controller/edit/__init__.php index 4f61cde518..7da55db338 100644 --- a/src/applications/repository/controller/edit/__init__.php +++ b/src/applications/repository/controller/edit/__init__.php @@ -17,7 +17,6 @@ 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 bb1181ceea..d0e5e29808 100644 --- a/src/applications/repository/storage/repository/PhabricatorRepository.php +++ b/src/applications/repository/storage/repository/PhabricatorRepository.php @@ -128,7 +128,7 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { if ($this->shouldUseSSH()) { switch ($this->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - $pattern = "SVN_SSH=%s svn {$pattern}"; + $pattern = "SVN_SSH=%s svn --non-interactive {$pattern}"; array_unshift( $args, csprintf( @@ -160,10 +160,30 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { default: throw new Exception("Unrecognized version control system."); } + } else if ($this->shouldUseHTTP()) { + switch ($this->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $pattern = + "svn ". + "--non-interactive ". + "--no-auth-cache ". + "--trust-server-cert ". + "--username %s ". + "--password %s ". + $pattern; + array_unshift( + $args, + $this->getDetail('http-login'), + $this->getDetail('http-pass')); + break; + default: + throw new Exception( + "No support for HTTP Basic Auth in this version control system."); + } } else { switch ($this->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - $pattern = "svn {$pattern}"; + $pattern = "svn --non-interactive {$pattern}"; break; case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: $pattern = "git {$pattern}"; @@ -187,7 +207,7 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { switch ($this->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - $pattern = "(cd %s && svn {$pattern})"; + $pattern = "(cd %s && svn --non-interactive {$pattern})"; array_unshift($args, $this->getLocalPath()); break; case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: @@ -212,11 +232,21 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { } 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; + if ($this->sshKeyfile === null) { + $key = $this->getDetail('ssh-key'); + $keyfile = $this->getDetail('ssh-keyfile'); + if ($keyfile) { + // Make sure we can read the file, that it exists, etc. + Filesystem::readFile($keyfile); + $this->sshKeyfile = $keyfile; + } else if ($key) { + $keyfile = new TempFile('phabricator-repository-ssh-key'); + chmod($keyfile, 0600); + Filesystem::writeFile($keyfile, $key); + $this->sshKeyfile = $keyfile; + } else { + $this->sshKeyfile = ''; + } } return (string)$this->sshKeyfile; @@ -226,7 +256,17 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { $uri = new PhutilURI($this->getRemoteURI()); $protocol = $uri->getProtocol(); if ($this->isSSHProtocol($protocol)) { - return (bool)$this->getDetail('ssh-key'); + return (bool)$this->getSSHKeyfile(); + } else { + return false; + } + } + + public function shouldUseHTTP() { + $uri = new PhutilURI($this->getRemoteURI()); + $protocol = $uri->getProtocol(); + if ($this->isHTTPProtocol($protocol)) { + return (bool)$this->getDetail('http-login'); } else { return false; } @@ -236,4 +276,8 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { return ($protocol == 'ssh' || $protocol == 'svn+ssh'); } + private function isHTTPProtocol($protocol) { + return ($protocol == 'http' || $protocol == 'https'); + } + } diff --git a/webroot/rsrc/css/aphront/form-view.css b/webroot/rsrc/css/aphront/form-view.css index 6c228cc51a..f05e957d3f 100644 --- a/webroot/rsrc/css/aphront/form-view.css +++ b/webroot/rsrc/css/aphront/form-view.css @@ -87,6 +87,18 @@ margin: 0.75em 3% 1.25em; } +.aphront-form-important { + margin: .5em 0; + background: #ffffdd; + padding: .5em 1em; +} +.aphront-form-important code { + display: block; + padding: .25em; + margin: .5em 2em; +} + + .aphront-form-control-static .aphront-form-input { padding-top: 4px; font-size: 13px;