diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1447,12 +1447,14 @@ 'HeraldWebhookEditController' => 'applications/herald/controller/HeraldWebhookEditController.php', 'HeraldWebhookEditEngine' => 'applications/herald/editor/HeraldWebhookEditEngine.php', 'HeraldWebhookEditor' => 'applications/herald/editor/HeraldWebhookEditor.php', + 'HeraldWebhookKeyController' => 'applications/herald/controller/HeraldWebhookKeyController.php', 'HeraldWebhookListController' => 'applications/herald/controller/HeraldWebhookListController.php', 'HeraldWebhookManagementWorkflow' => 'applications/herald/management/HeraldWebhookManagementWorkflow.php', 'HeraldWebhookNameTransaction' => 'applications/herald/xaction/HeraldWebhookNameTransaction.php', 'HeraldWebhookPHIDType' => 'applications/herald/phid/HeraldWebhookPHIDType.php', 'HeraldWebhookQuery' => 'applications/herald/query/HeraldWebhookQuery.php', 'HeraldWebhookRequest' => 'applications/herald/storage/HeraldWebhookRequest.php', + 'HeraldWebhookRequestGarbageCollector' => 'applications/herald/garbagecollector/HeraldWebhookRequestGarbageCollector.php', 'HeraldWebhookRequestListView' => 'applications/herald/view/HeraldWebhookRequestListView.php', 'HeraldWebhookRequestPHIDType' => 'applications/herald/phid/HeraldWebhookRequestPHIDType.php', 'HeraldWebhookRequestQuery' => 'applications/herald/query/HeraldWebhookRequestQuery.php', @@ -6735,12 +6737,14 @@ 'PhabricatorPolicyInterface', 'PhabricatorApplicationTransactionInterface', 'PhabricatorDestructibleInterface', + 'PhabricatorProjectInterface', ), 'HeraldWebhookCallManagementWorkflow' => 'HeraldWebhookManagementWorkflow', 'HeraldWebhookController' => 'HeraldController', 'HeraldWebhookEditController' => 'HeraldWebhookController', 'HeraldWebhookEditEngine' => 'PhabricatorEditEngine', 'HeraldWebhookEditor' => 'PhabricatorApplicationTransactionEditor', + 'HeraldWebhookKeyController' => 'HeraldWebhookController', 'HeraldWebhookListController' => 'HeraldWebhookController', 'HeraldWebhookManagementWorkflow' => 'PhabricatorManagementWorkflow', 'HeraldWebhookNameTransaction' => 'HeraldWebhookTransactionType', @@ -6751,6 +6755,7 @@ 'PhabricatorPolicyInterface', 'PhabricatorExtendedPolicyInterface', ), + 'HeraldWebhookRequestGarbageCollector' => 'PhabricatorGarbageCollector', 'HeraldWebhookRequestListView' => 'AphrontView', 'HeraldWebhookRequestPHIDType' => 'PhabricatorPHIDType', 'HeraldWebhookRequestQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', diff --git a/src/applications/herald/application/PhabricatorHeraldApplication.php b/src/applications/herald/application/PhabricatorHeraldApplication.php --- a/src/applications/herald/application/PhabricatorHeraldApplication.php +++ b/src/applications/herald/application/PhabricatorHeraldApplication.php @@ -68,10 +68,8 @@ 'HeraldWebhookViewController', $this->getEditRoutePattern('edit/') => 'HeraldWebhookEditController', 'test/(?P\d+)/' => 'HeraldWebhookTestController', - 'key/' => array( - 'view/(?P\d+)/' => 'HeraldWebhookViewKeyController', - 'cycle/(?P\d+)/' => 'HeraldWebhookCycleKeyController', - ), + 'key/(?Pview|cycle)/(?P\d+)/' => + 'HeraldWebhookKeyController', ), ), ); diff --git a/src/applications/herald/controller/HeraldWebhookKeyController.php b/src/applications/herald/controller/HeraldWebhookKeyController.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/controller/HeraldWebhookKeyController.php @@ -0,0 +1,56 @@ +getViewer(); + + $hook = id(new HeraldWebhookQuery()) + ->setViewer($viewer) + ->withIDs(array($request->getURIData('id'))) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$hook) { + return new Aphront404Response(); + } + + $action = $request->getURIData('action'); + if ($action === 'cycle') { + if (!$request->isFormPost()) { + return $this->newDialog() + ->setTitle(pht('Regenerate HMAC Key')) + ->appendParagraph( + pht( + 'Regenerate the HMAC key used to sign requests made by this '. + 'webhook?')) + ->appendParagraph( + pht( + 'Requests which are currently authenticated with the old key '. + 'may fail.')) + ->addCancelButton($hook->getURI()) + ->addSubmitButton(pht('Regnerate Key')); + } else { + $hook->regenerateHMACKey()->save(); + } + } + + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendControl( + id(new AphrontFormTextControl()) + ->setLabel(pht('HMAC Key')) + ->setValue($hook->getHMACKey())); + + return $this->newDialog() + ->setTitle(pht('Webhook HMAC Key')) + ->appendForm($form) + ->addCancelButton($hook->getURI(), pht('Done')); + } + + +} diff --git a/src/applications/herald/controller/HeraldWebhookTestController.php b/src/applications/herald/controller/HeraldWebhookTestController.php --- a/src/applications/herald/controller/HeraldWebhookTestController.php +++ b/src/applications/herald/controller/HeraldWebhookTestController.php @@ -19,26 +19,74 @@ return new Aphront404Response(); } + $v_object = null; + $e_object = null; + $errors = array(); if ($request->isFormPost()) { - $object = $hook; - $request = HeraldWebhookRequest::initializeNewWebhookRequest($hook) - ->setObjectPHID($object->getPHID()) - ->save(); + $v_object = $request->getStr('object'); + if (!strlen($v_object)) { + $object = $hook; + } else { + $objects = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withNames(array($v_object)) + ->execute(); + if ($objects) { + $object = head($objects); + } else { + $e_object = pht('Invalid'); + $errors[] = pht('Specified object could not be loaded.'); + } + } - $request->queueCall(); + if (!$errors) { + $xaction_query = + PhabricatorApplicationTransactionQuery::newQueryForObject($object); - $next_uri = $hook->getURI().'request/'.$request->getID().'/'; + $xactions = $xaction_query + ->withObjectPHIDs(array($object->getPHID())) + ->setViewer($viewer) + ->setLimit(10) + ->execute(); - return id(new AphrontRedirectResponse())->setURI($next_uri); + $request = HeraldWebhookRequest::initializeNewWebhookRequest($hook) + ->setObjectPHID($object->getPHID()) + ->setIsTestAction(true) + ->setTransactionPHIDs(mpull($xactions, 'getPHID')) + ->save(); + + $request->queueCall(); + + $next_uri = $hook->getURI().'request/'.$request->getID().'/'; + + return id(new AphrontRedirectResponse())->setURI($next_uri); + } } + $instructions = <<setViewer($viewer) + ->appendControl( + id(new AphrontFormTextControl()) + ->setLabel(pht('Object')) + ->setName('object') + ->setError($e_object) + ->setValue($v_object)); + return $this->newDialog() + ->setErrors($errors) + ->setWidth(AphrontDialogView::WIDTH_FORM) ->setTitle(pht('New Test Request')) - ->appendParagraph( - pht('This will make a new test request to the configured URI.')) + ->appendParagraph(new PHUIRemarkupView($viewer, $instructions)) + ->appendForm($form) ->addCancelButton($hook->getURI()) - ->addSubmitButton(pht('Make Request')); + ->addSubmitButton(pht('Test Webhook')); } diff --git a/src/applications/herald/controller/HeraldWebhookViewController.php b/src/applications/herald/controller/HeraldWebhookViewController.php --- a/src/applications/herald/controller/HeraldWebhookViewController.php +++ b/src/applications/herald/controller/HeraldWebhookViewController.php @@ -94,10 +94,15 @@ $title = $hook->getName(); + $status_icon = $hook->getStatusIcon(); + $status_color = $hook->getStatusColor(); + $status_name = $hook->getStatusDisplayName(); + $header = id(new PHUIHeaderView()) ->setHeader($title) ->setViewer($viewer) ->setPolicyObject($hook) + ->setStatus($status_icon, $status_color, $status_name) ->setHeaderIcon('fa-cloud-upload'); return $header; @@ -117,6 +122,9 @@ $edit_uri = $this->getApplicationURI("webhook/edit/{$id}/"); $test_uri = $this->getApplicationURI("webhook/test/{$id}/"); + $key_view_uri = $this->getApplicationURI("webhook/key/view/{$id}/"); + $key_cycle_uri = $this->getApplicationURI("webhook/key/cycle/{$id}/"); + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Webhook')) @@ -133,6 +141,22 @@ ->setWorkflow(true) ->setHref($test_uri)); + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName(pht('View HMAC Key')) + ->setIcon('fa-key') + ->setDisabled(!$can_edit) + ->setWorkflow(true) + ->setHref($key_view_uri)); + + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Regenerate HMAC Key')) + ->setIcon('fa-refresh') + ->setDisabled(!$can_edit) + ->setWorkflow(true) + ->setHref($key_cycle_uri)); + return $curtain; } diff --git a/src/applications/herald/garbagecollector/HeraldWebhookRequestGarbageCollector.php b/src/applications/herald/garbagecollector/HeraldWebhookRequestGarbageCollector.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/garbagecollector/HeraldWebhookRequestGarbageCollector.php @@ -0,0 +1,29 @@ +establishConnection('w'); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE dateCreated < %d LIMIT 100', + $table->getTableName(), + $this->getGarbageEpoch()); + + return ($conn_w->getAffectedRows() == 100); + } + +} diff --git a/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php b/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php --- a/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php +++ b/src/applications/herald/management/HeraldWebhookCallManagementWorkflow.php @@ -6,7 +6,7 @@ protected function didConstruct() { $this ->setName('call') - ->setExamples('**call** --id __id__') + ->setExamples('**call** --id __id__ [--object __object__]') ->setSynopsis(pht('Call a webhook.')) ->setArguments( array( @@ -15,6 +15,19 @@ 'param' => 'id', 'help' => pht('Webhook ID to call'), ), + array( + 'name' => 'object', + 'param' => 'object', + 'help' => pht('Submit transactions for a particular object.'), + ), + array( + 'name' => 'silent', + 'help' => pht('Set the "silent" flag on the request.'), + ), + array( + 'name' => 'secure', + 'help' => pht('Set the "secure" flag on the request.'), + ), )); } @@ -39,12 +52,38 @@ $id)); } - $object = $hook; + $object_name = $args->getArg('object'); + if ($object_name === null) { + $object = $hook; + } else { + $objects = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withNames(array($object_name)) + ->execute(); + if (!$objects) { + throw new PhutilArgumentUsageException( + pht( + 'Unable to load specified object ("%s").', + $object_name)); + } + $object = head($objects); + } + + $xaction_query = + PhabricatorApplicationTransactionQuery::newQueryForObject($object); - $application_phid = id(new PhabricatorHeraldApplication())->getPHID(); + $xactions = $xaction_query + ->withObjectPHIDs(array($object->getPHID())) + ->setViewer($viewer) + ->setLimit(10) + ->execute(); $request = HeraldWebhookRequest::initializeNewWebhookRequest($hook) ->setObjectPHID($object->getPHID()) + ->setIsTestAction(true) + ->setIsSilentAction((bool)$args->getArg('silent')) + ->setIsSecureAction((bool)$args->getArg('secure')) + ->setTransactionPHIDs(mpull($xactions, 'getPHID')) ->save(); PhabricatorWorker::setRunAllTasksInProcess(true); diff --git a/src/applications/herald/query/HeraldWebhookSearchEngine.php b/src/applications/herald/query/HeraldWebhookSearchEngine.php --- a/src/applications/herald/query/HeraldWebhookSearchEngine.php +++ b/src/applications/herald/query/HeraldWebhookSearchEngine.php @@ -80,9 +80,16 @@ ->setViewer($viewer); foreach ($hooks as $hook) { $item = id(new PHUIObjectItemView()) - ->setObjectName(pht('Hook %d', $hook->getID())) + ->setObjectName(pht('Webhook %d', $hook->getID())) ->setHeader($hook->getName()) - ->setHref($hook->getURI()); + ->setHref($hook->getURI()) + ->addAttribute($hook->getWebhookURI()); + + $item->addIcon($hook->getStatusIcon(), $hook->getStatusDisplayName()); + + if ($hook->isDisabled()) { + $item->setDisabled(true); + } $list->addItem($item); } diff --git a/src/applications/herald/storage/HeraldWebhook.php b/src/applications/herald/storage/HeraldWebhook.php --- a/src/applications/herald/storage/HeraldWebhook.php +++ b/src/applications/herald/storage/HeraldWebhook.php @@ -5,7 +5,8 @@ implements PhabricatorPolicyInterface, PhabricatorApplicationTransactionInterface, - PhabricatorDestructibleInterface { + PhabricatorDestructibleInterface, + PhabricatorProjectInterface { protected $name; protected $webhookURI; @@ -44,7 +45,7 @@ ->setStatus(self::HOOKSTATUS_ENABLED) ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()) ->setEditPolicy($viewer->getPHID()) - ->setHmacKey(Filesystem::readRandomCharacters(32)); + ->regenerateHMACKey(); } public function getURI() { @@ -56,16 +57,79 @@ } public static function getStatusDisplayNameMap() { + $specs = self::getStatusSpecifications(); + return ipull($specs, 'name', 'key'); + } + + private static function getStatusSpecifications() { + $specs = array( + array( + 'key' => self::HOOKSTATUS_FIREHOSE, + 'name' => pht('Firehose'), + 'color' => 'orange', + 'icon' => 'fa-star-o', + ), + array( + 'key' => self::HOOKSTATUS_ENABLED, + 'name' => pht('Enabled'), + 'color' => 'bluegrey', + 'icon' => 'fa-check', + ), + array( + 'key' => self::HOOKSTATUS_DISABLED, + 'name' => pht('Disabled'), + 'color' => 'dark', + 'icon' => 'fa-ban', + ), + ); + + return ipull($specs, null, 'key'); + } + + + private static function getSpecificationForStatus($status) { + $specs = self::getStatusSpecifications(); + + if (isset($specs[$status])) { + return $specs[$status]; + } + return array( - self::HOOKSTATUS_FIREHOSE => pht('Firehose'), - self::HOOKSTATUS_ENABLED => pht('Enabled'), - self::HOOKSTATUS_DISABLED => pht('Disabled'), + 'key' => $status, + 'name' => pht('Unknown ("%s")', $status), + 'icon' => 'fa-question', + 'color' => 'indigo', ); } + public static function getDisplayNameForStatus($status) { + $spec = self::getSpecificationForStatus($status); + return $spec['name']; + } + + public static function getIconForStatus($status) { + $spec = self::getSpecificationForStatus($status); + return $spec['icon']; + } + + public static function getColorForStatus($status) { + $spec = self::getSpecificationForStatus($status); + return $spec['color']; + } + public function getStatusDisplayName() { $status = $this->getStatus(); - return idx($this->getStatusDisplayNameMap(), $status); + return self::getDisplayNameForStatus($status); + } + + public function getStatusIcon() { + $status = $this->getStatus(); + return self::getIconForStatus($status); + } + + public function getStatusColor() { + $status = $this->getStatus(); + return self::getColorForStatus($status); } public function getErrorBackoffWindow() { @@ -101,6 +165,9 @@ return false; } + public function regenerateHMACKey() { + return $this->setHMACKey(Filesystem::readRandomCharacters(32)); + } /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/herald/storage/HeraldWebhookRequest.php b/src/applications/herald/storage/HeraldWebhookRequest.php --- a/src/applications/herald/storage/HeraldWebhookRequest.php +++ b/src/applications/herald/storage/HeraldWebhookRequest.php @@ -116,6 +116,30 @@ return $this->getProperty('transactionPHIDs', array()); } + public function setIsSilentAction($bool) { + return $this->setProperty('silent', $bool); + } + + public function getIsSilentAction() { + return $this->getProperty('silent', false); + } + + public function setIsTestAction($bool) { + return $this->setProperty('test', $bool); + } + + public function getIsTestAction() { + return $this->getProperty('test', false); + } + + public function setIsSecureAction($bool) { + return $this->setProperty('secure', $bool); + } + + public function getIsSecureAction() { + return $this->getProperty('secure', false); + } + public function queueCall() { PhabricatorWorker::scheduleTask( 'HeraldWebhookWorker', diff --git a/src/applications/herald/view/HeraldWebhookRequestListView.php b/src/applications/herald/view/HeraldWebhookRequestListView.php --- a/src/applications/herald/view/HeraldWebhookRequestListView.php +++ b/src/applications/herald/view/HeraldWebhookRequestListView.php @@ -44,12 +44,20 @@ $rowc[] = null; } + $last_epoch = $request->getLastRequestEpoch(); + if ($request->getLastRequestEpoch()) { + $last_request = phabricator_datetime($last_epoch, $viewer); + } else { + $last_request = null; + } + $rows[] = array( $request->getID(), $icon, $handles[$request->getObjectPHID()]->renderLink(), $request->getErrorType(), $request->getErrorCode(), + $last_request, ); } @@ -62,6 +70,7 @@ pht('Object'), pht('Type'), pht('Code'), + pht('Requested At'), )) ->setColumnClasses( array( @@ -70,6 +79,7 @@ 'wide', '', '', + '', )); return $table; diff --git a/src/applications/herald/worker/HeraldWebhookWorker.php b/src/applications/herald/worker/HeraldWebhookWorker.php --- a/src/applications/herald/worker/HeraldWebhookWorker.php +++ b/src/applications/herald/worker/HeraldWebhookWorker.php @@ -143,10 +143,15 @@ 'object' => array( 'phid' => $object->getPHID(), ), + 'action' => array( + 'test' => $request->getIsTestAction(), + 'silent' => $request->getIsSilentAction(), + 'secure' => $request->getIsSecureAction(), + ), 'transactions' => $xaction_data, ); - $payload = phutil_json_encode($payload); + $payload = id(new PhutilJSON())->encodeFormatted($payload); $key = $hook->getHmacKey(); $signature = PhabricatorHash::digestHMACSHA256($payload, $key); $uri = $hook->getWebhookURI(); diff --git a/src/applications/herald/xaction/HeraldWebhookStatusTransaction.php b/src/applications/herald/xaction/HeraldWebhookStatusTransaction.php --- a/src/applications/herald/xaction/HeraldWebhookStatusTransaction.php +++ b/src/applications/herald/xaction/HeraldWebhookStatusTransaction.php @@ -14,20 +14,32 @@ } public function getTitle() { + $old_value = $this->getOldValue(); + $new_value = $this->getNewValue(); + + $old_status = HeraldWebhook::getDisplayNameForStatus($old_value); + $new_status = HeraldWebhook::getDisplayNameForStatus($new_value); + return pht( '%s changed hook status from %s to %s.', $this->renderAuthor(), - $this->renderOldValue(), - $this->renderNewValue()); + $this->renderValue($old_status), + $this->renderValue($new_status)); } public function getTitleForFeed() { + $old_value = $this->getOldValue(); + $new_value = $this->getNewValue(); + + $old_status = HeraldWebhook::getDisplayNameForStatus($old_value); + $new_status = HeraldWebhook::getDisplayNameForStatus($new_value); + return pht( '%s changed %s from %s to %s.', $this->renderAuthor(), $this->renderObject(), - $this->renderOldValue(), - $this->renderNewValue()); + $this->renderValue($old_status), + $this->renderValue($new_status)); } public function validateTransactions($object, array $xactions) {