diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -104,15 +104,29 @@ try { $remote_addr = $request->getRemoteAddress(); + if ($request->isHTTPS()) { + $remote_protocol = PhabricatorRepositoryPullEvent::PROTOCOL_HTTPS; + } else { + $remote_protocol = PhabricatorRepositoryPullEvent::PROTOCOL_HTTP; + } + $pull_event = id(new PhabricatorRepositoryPullEvent()) ->setEpoch(PhabricatorTime::getNow()) ->setRemoteAddress($remote_addr) - ->setRemoteProtocol('http'); + ->setRemoteProtocol($remote_protocol); if ($response) { - $pull_event - ->setResultType('wild') - ->setResultCode($response->getHTTPResponseCode()); + $response_code = $response->getHTTPResponseCode(); + + if ($response_code == 200) { + $pull_event + ->setResultType(PhabricatorRepositoryPullEvent::RESULT_PULL) + ->setResultCode($response_code); + } else { + $pull_event + ->setResultType(PhabricatorRepositoryPullEvent::RESULT_ERROR) + ->setResultCode($response_code); + } if ($response instanceof PhabricatorVCSResponse) { $pull_event->setProperties( @@ -122,7 +136,7 @@ } } else { $pull_event - ->setResultType('exception') + ->setResultType(PhabricatorRepositoryPullEvent::RESULT_EXCEPTION) ->setResultCode(500) ->setProperties( array( diff --git a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php --- a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php @@ -61,11 +61,11 @@ if ($err) { $pull_event - ->setResultType('error') + ->setResultType(PhabricatorRepositoryPullEvent::RESULT_ERROR) ->setResultCode($err); } else { $pull_event - ->setResultType('pull') + ->setResultType(PhabricatorRepositoryPullEvent::RESULT_PULL) ->setResultCode(0); } diff --git a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php --- a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php @@ -272,7 +272,7 @@ return id(new PhabricatorRepositoryPullEvent()) ->setEpoch(PhabricatorTime::getNow()) ->setRemoteAddress($remote_address) - ->setRemoteProtocol('ssh') + ->setRemoteProtocol(PhabricatorRepositoryPullEvent::PROTOCOL_SSH) ->setPullerPHID($viewer->getPHID()) ->setRepositoryPHID($repository->getPHID()); } diff --git a/src/applications/diffusion/view/DiffusionPullLogListView.php b/src/applications/diffusion/view/DiffusionPullLogListView.php --- a/src/applications/diffusion/view/DiffusionPullLogListView.php +++ b/src/applications/diffusion/view/DiffusionPullLogListView.php @@ -94,7 +94,7 @@ pht('From'), pht('Via'), null, - pht('Error'), + pht('Code'), pht('Date'), )) ->setColumnClasses( diff --git a/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php b/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php --- a/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php +++ b/src/applications/repository/storage/PhabricatorRepositoryPullEvent.php @@ -15,6 +15,14 @@ private $repository = self::ATTACHABLE; + const RESULT_PULL = 'pull'; + const RESULT_ERROR = 'error'; + const RESULT_EXCEPTION = 'exception'; + + const PROTOCOL_HTTP = 'http'; + const PROTOCOL_HTTPS = 'https'; + const PROTOCOL_SSH = 'ssh'; + public static function initializeNewEvent(PhabricatorUser $viewer) { return id(new PhabricatorRepositoryPushEvent()) ->setPusherPHID($viewer->getPHID()); @@ -62,8 +70,9 @@ public function getRemoteProtocolDisplayName() { $map = array( - 'ssh' => pht('SSH'), - 'http' => pht('HTTP'), + self::PROTOCOL_SSH => pht('SSH'), + self::PROTOCOL_HTTP => pht('HTTP'), + self::PROTOCOL_HTTPS => pht('HTTPS'), ); $protocol = $this->getRemoteProtocol(); @@ -74,25 +83,53 @@ public function newResultIcon() { $icon = new PHUIIconView(); $type = $this->getResultType(); + $code = $this->getResultCode(); + + $protocol = $this->getRemoteProtocol(); + + $is_any_http = + ($protocol === self::PROTOCOL_HTTP) || + ($protocol === self::PROTOCOL_HTTPS); + + // If this was an HTTP request and we responded with a 401, that means + // the user didn't provide credentials. This is technically an error, but + // it's routine and just causes the client to prompt them. Show a more + // comforting icon and description in the UI. + if ($is_any_http) { + if ($code == 401) { + return $icon + ->setIcon('fa-key blue') + ->setTooltip(pht('Authentication Required')); + } + } switch ($type) { - case 'wild': + case self::RESULT_ERROR: $icon - ->setIcon('fa-question indigo') - ->setTooltip(pht('Unknown ("%s")', $type)); + ->setIcon('fa-times red') + ->setTooltip(pht('Error')); break; - case 'pull': + case self::RESULT_EXCEPTION: + $icon + ->setIcon('fa-exclamation-triangle red') + ->setTooltip(pht('Exception')); + break; + case self::RESULT_PULL: $icon ->setIcon('fa-download green') ->setTooltip(pht('Pull')); break; + default: + $icon + ->setIcon('fa-question indigo') + ->setTooltip(pht('Unknown ("%s")', $type)); + break; } return $icon; } - /* -( PhabricatorPolicyInterface )----------------------------------------- */