From c7f23f522a79f0847903958c0cb37c6899bb9cbe Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 26 Oct 2013 07:56:17 -0700 Subject: [PATCH] Accept and route VCS HTTP requests Summary: Mostly ripped from D7391, with some changes: - Serve repositories at `/diffusion/X/`, with no special `/git/` or `/serve/` URI component. - This requires a little bit of magic, but I got the magic working for Git, Mercurial and SVN, and it seems reasonable. - I think having one URI for everything will make it easier for users to understand. - One downside is that git will clone into `X` by default, but I think that's not a big deal, and we can work around that in the future easily enough. - Accept HTTP requests for Git, SVN and Mercurial repositories. - Auth logic is a little different in order to be more consistent with how other things work. - Instead of AphrontBasicAuthResponse, added "VCSResponse". Mercurial can print strings we send it on the CLI if we're careful, so support that. I did a fair amount of digging and didn't have any luck with git or svn. - Commands we don't know about are assumed to require "Push" capability by default. No actual VCS data going over the wire yet. Test Plan: Ran a bunch of stuff like this: $ hg clone http://local.aphront.com:8080/diffusion/P/ abort: HTTP Error 403: This repository is not available over HTTP. ...and got pretty reasonable-seeming errors in all cases. All this can do is produce errors for now. Reviewers: hach-que, btrahan Reviewed By: hach-que CC: aran Maniphest Tasks: T2230 Differential Revision: https://secure.phabricator.com/D7417 --- src/__phutil_library_map__.php | 4 + src/aphront/response/AphrontResponse.php | 4 + src/aphront/sink/AphrontHTTPSink.php | 10 +- src/aphront/sink/AphrontIsolatedHTTPSink.php | 2 +- src/aphront/sink/AphrontPHPHTTPSink.php | 8 +- .../base/controller/PhabricatorController.php | 2 +- .../PhabricatorApplicationDiffusion.php | 7 + .../controller/DiffusionController.php | 162 ++++++++++++++++++ .../DiffusionRepositoryDefaultController.php | 11 ++ .../response/PhabricatorVCSResponse.php | 62 +++++++ 10 files changed, 264 insertions(+), 8 deletions(-) create mode 100644 src/applications/diffusion/controller/DiffusionRepositoryDefaultController.php create mode 100644 src/applications/repository/response/PhabricatorVCSResponse.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 328a634aad..93183e2352 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -509,6 +509,7 @@ phutil_register_library_map(array( 'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/DiffusionRenameHistoryQuery.php', 'DiffusionRepositoryController' => 'applications/diffusion/controller/DiffusionRepositoryController.php', 'DiffusionRepositoryCreateController' => 'applications/diffusion/controller/DiffusionRepositoryCreateController.php', + 'DiffusionRepositoryDefaultController' => 'applications/diffusion/controller/DiffusionRepositoryDefaultController.php', 'DiffusionRepositoryEditActionsController' => 'applications/diffusion/controller/DiffusionRepositoryEditActionsController.php', 'DiffusionRepositoryEditActivateController' => 'applications/diffusion/controller/DiffusionRepositoryEditActivateController.php', 'DiffusionRepositoryEditBasicController' => 'applications/diffusion/controller/DiffusionRepositoryEditBasicController.php', @@ -1867,6 +1868,7 @@ phutil_register_library_map(array( 'PhabricatorUserTestCase' => 'applications/people/storage/__tests__/PhabricatorUserTestCase.php', 'PhabricatorUserTitleField' => 'applications/people/customfield/PhabricatorUserTitleField.php', 'PhabricatorUserTransaction' => 'applications/people/storage/PhabricatorUserTransaction.php', + 'PhabricatorVCSResponse' => 'applications/repository/response/PhabricatorVCSResponse.php', 'PhabricatorWorker' => 'infrastructure/daemon/workers/PhabricatorWorker.php', 'PhabricatorWorkerActiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php', 'PhabricatorWorkerArchiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php', @@ -2691,6 +2693,7 @@ phutil_register_library_map(array( 'DiffusionRemarkupRule' => 'PhabricatorRemarkupRuleObject', 'DiffusionRepositoryController' => 'DiffusionController', 'DiffusionRepositoryCreateController' => 'DiffusionRepositoryEditController', + 'DiffusionRepositoryDefaultController' => 'DiffusionController', 'DiffusionRepositoryEditActionsController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryEditActivateController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryEditBasicController' => 'DiffusionRepositoryEditController', @@ -4202,6 +4205,7 @@ phutil_register_library_map(array( 'PhabricatorUserTestCase' => 'PhabricatorTestCase', 'PhabricatorUserTitleField' => 'PhabricatorUserCustomField', 'PhabricatorUserTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorVCSResponse' => 'AphrontResponse', 'PhabricatorWorkerActiveTask' => 'PhabricatorWorkerTask', 'PhabricatorWorkerArchiveTask' => 'PhabricatorWorkerTask', 'PhabricatorWorkerDAO' => 'PhabricatorLiskDAO', diff --git a/src/aphront/response/AphrontResponse.php b/src/aphront/response/AphrontResponse.php index 20ce8f7a62..67da4aee93 100644 --- a/src/aphront/response/AphrontResponse.php +++ b/src/aphront/response/AphrontResponse.php @@ -49,6 +49,10 @@ abstract class AphrontResponse { return $this->responseCode; } + public function getHTTPResponseMessage() { + return ''; + } + public function setFrameable($frameable) { $this->frameable = $frameable; return $this; diff --git a/src/aphront/sink/AphrontHTTPSink.php b/src/aphront/sink/AphrontHTTPSink.php index 875aafa3f0..15822051e4 100644 --- a/src/aphront/sink/AphrontHTTPSink.php +++ b/src/aphront/sink/AphrontHTTPSink.php @@ -25,13 +25,13 @@ abstract class AphrontHTTPSink { * @param int Numeric HTTP status code. * @return void */ - final public function writeHTTPStatus($code) { + final public function writeHTTPStatus($code, $message = '') { if (!preg_match('/^\d{3}$/', $code)) { throw new Exception("Malformed HTTP status code '{$code}'!"); } $code = (int)$code; - $this->emitHTTPStatus($code); + $this->emitHTTPStatus($code, $message); } @@ -103,7 +103,9 @@ abstract class AphrontHTTPSink { $response->getHeaders(), $response->getCacheHeaders()); - $this->writeHTTPStatus($response->getHTTPResponseCode()); + $this->writeHTTPStatus( + $response->getHTTPResponseCode(), + $response->getHTTPResponseMessage()); $this->writeHeaders($all_headers); $this->writeData($response_string); } @@ -112,7 +114,7 @@ abstract class AphrontHTTPSink { /* -( Emitting the Response )---------------------------------------------- */ - abstract protected function emitHTTPStatus($code); + abstract protected function emitHTTPStatus($code, $message = ''); abstract protected function emitHeader($name, $value); abstract protected function emitData($data); } diff --git a/src/aphront/sink/AphrontIsolatedHTTPSink.php b/src/aphront/sink/AphrontIsolatedHTTPSink.php index ced5536553..1f8a48f016 100644 --- a/src/aphront/sink/AphrontIsolatedHTTPSink.php +++ b/src/aphront/sink/AphrontIsolatedHTTPSink.php @@ -11,7 +11,7 @@ final class AphrontIsolatedHTTPSink extends AphrontHTTPSink { private $headers; private $data; - protected function emitHTTPStatus($code) { + protected function emitHTTPStatus($code, $message = '') { $this->status = $code; } diff --git a/src/aphront/sink/AphrontPHPHTTPSink.php b/src/aphront/sink/AphrontPHPHTTPSink.php index d94e2b6558..f983abac95 100644 --- a/src/aphront/sink/AphrontPHPHTTPSink.php +++ b/src/aphront/sink/AphrontPHPHTTPSink.php @@ -7,9 +7,13 @@ */ final class AphrontPHPHTTPSink extends AphrontHTTPSink { - protected function emitHTTPStatus($code) { + protected function emitHTTPStatus($code, $message = '') { if ($code != 200) { - header("HTTP/1.0 {$code}"); + $header = "HTTP/1.0 {$code}"; + if (strlen($message)) { + $header .= " {$message}"; + } + header($header); } } diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index 60abcfff8d..149f40ee25 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -24,7 +24,7 @@ abstract class PhabricatorController extends AphrontController { return PhabricatorUserEmail::isEmailVerificationRequired(); } - final public function willBeginExecution() { + public function willBeginExecution() { $request = $this->getRequest(); if ($request->getUser()) { diff --git a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php index bf0faf3d5e..708070f9df 100644 --- a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php +++ b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php @@ -79,6 +79,13 @@ final class PhabricatorApplicationDiffusion extends PhabricatorApplication { '(?Pserve)/' => 'DiffusionRepositoryEditHostingController', ), ), + + // NOTE: This must come after the rule above; it just gives us a + // catch-all for serving repositories over HTTP. We must accept + // requests without the trailing "/" because SVN commands don't + // necessarily include it. + '(?P[A-Z]+)(/|$).*' => 'DiffusionRepositoryDefaultController', + 'inline/' => array( 'edit/(?P[^/]+)/' => 'DiffusionInlineCommentController', 'preview/(?P[^/]+)/' => diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index 7778375028..2fb58b32fa 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -4,6 +4,168 @@ abstract class DiffusionController extends PhabricatorController { protected $diffusionRequest; + public function willBeginExecution() { + $request = $this->getRequest(); + $uri = $request->getRequestURI(); + + // Check if this is a VCS request, e.g. from "git clone", "hg clone", or + // "svn checkout". If it is, we jump off into repository serving code to + // process the request. + + $regex = '@^/diffusion/(?P[A-Z]+)(/|$)@'; + $matches = null; + if (preg_match($regex, (string)$uri, $matches)) { + $vcs = null; + + if ($request->getExists('__vcs__')) { + // This is magic to make it easier for us to debug stuff by telling + // users to run: + // + // curl http://example.phabricator.com/diffusion/X/?__vcs__=1 + // + // ...to get a human-readable error. + $vcs = $request->getExists('__vcs__'); + } else if ($request->getExists('service')) { + // Git also gives us a User-Agent like "git/1.8.2.3". + $vcs = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT; + } else if ($request->getExists('cmd')) { + // Mercurial also sends an Accept header like + // "application/mercurial-0.1", and a User-Agent like + // "mercurial/proto-1.0". + $vcs = PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL; + } else { + // Subversion also sends an initial OPTIONS request (vs GET/POST), and + // has a User-Agent like "SVN/1.8.3 (x86_64-apple-darwin11.4.2) + // serf/1.3.2". + $dav = $request->getHTTPHeader('DAV'); + $dav = new PhutilURI($dav); + if ($dav->getDomain() === 'subversion.tigris.org') { + $vcs = PhabricatorRepositoryType::REPOSITORY_TYPE_SVN; + } + } + + if ($vcs) { + return $this->processVCSRequest($matches['callsign']); + } + } + + parent::willBeginExecution(); + } + + private function processVCSRequest($callsign) { + + // TODO: Authenticate user. + + $viewer = new PhabricatorUser(); + + $allow_public = PhabricatorEnv::getEnvConfig('policy.allow-public'); + if (!$allow_public) { + if (!$viewer->isLoggedIn()) { + return new PhabricatorVCSResponse( + 403, + pht('You must log in to access repositories.')); + } + } + + try { + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->withCallsigns(array($callsign)) + ->executeOne(); + if (!$repository) { + return new PhabricatorVCSResponse( + 404, + pht('No such repository exists.')); + } + } catch (PhabricatorPolicyException $ex) { + if ($viewer->isLoggedIn()) { + return new PhabricatorVCSResponse( + 403, + pht('You do not have permission to access this repository.')); + } else { + return new PhabricatorVCSResponse( + 401, + pht('You must log in to access this repository.')); + } + } + + $is_push = !$this->isReadOnlyRequest($repository); + + switch ($repository->getServeOverHTTP()) { + case PhabricatorRepository::SERVE_READONLY: + if ($is_push) { + return new PhabricatorVCSResponse( + 403, + pht('This repository is read-only over HTTP.')); + } + break; + case PhabricatorRepository::SERVE_READWRITE: + if ($is_push) { + $can_push = PhabricatorPolicyFilter::hasCapability( + $viewer, + $repository, + DiffusionCapabilityPush::CAPABILITY); + if (!$can_push) { + if ($viewer->isLoggedIn()) { + return new PhabricatorVCSResponse( + 403, + pht('You do not have permission to push to this repository.')); + } else { + return new PhabricatorVCSResponse( + 401, + pht('You must log in to push to this repository.')); + } + } + } + break; + case PhabricatorRepository::SERVE_OFF: + default: + return new PhabricatorVCSResponse( + 403, + pht('This repository is not available over HTTP.')); + } + + return new PhabricatorVCSResponse( + 999, + pht('TODO: Implement meaningful responses.')); + } + + private function isReadOnlyRequest( + PhabricatorRepository $repository) { + $request = $this->getRequest(); + + // TODO: This implementation is safe by default, but very incomplete. + + switch ($repository->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $service = $request->getStr('service'); + // NOTE: Service names are the reverse of what you might expect, as they + // are from the point of view of the server. The main read service is + // "git-upload-pack", and the main write service is "git-receive-pack". + switch ($service) { + case 'git-upload-pack': + return true; + case 'git-receive-pack': + default: + return false; + } + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $cmd = $request->getStr('cmd'); + switch ($cmd) { + case 'capabilities': + return true; + default: + return false; + } + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SUBVERSION: + break; + } + + return false; + } + public function willProcessRequest(array $data) { if (isset($data['callsign'])) { $drequest = DiffusionRequest::newFromAphrontRequestDictionary( diff --git a/src/applications/diffusion/controller/DiffusionRepositoryDefaultController.php b/src/applications/diffusion/controller/DiffusionRepositoryDefaultController.php new file mode 100644 index 0000000000..e939d0e54b --- /dev/null +++ b/src/applications/diffusion/controller/DiffusionRepositoryDefaultController.php @@ -0,0 +1,11 @@ +code = $code; + + $message = head(phutil_split_lines($message)); + $this->message = $message; + } + + public function getMessage() { + return $this->message; + } + + public function buildResponseString() { + return $this->code.' '.$this->message; + } + + public function getHeaders() { + $headers = array(); + + if ($this->getHTTPResponseCode() == 401) { + $headers[] = array( + 'WWW-Authenticate', + 'Basic realm="Phabricator Repositories"', + ); + } + + return $headers; + } + + public function getCacheHeaders() { + return array(); + } + + public function getHTTPResponseCode() { + return $this->code; + } + + public function getHTTPResponseMessage() { + return $this->message; + } + +}