Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15393644
D19046.id45670.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
21 KB
Referenced Files
None
Subscribers
None
D19046.id45670.diff
View Options
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<id>\d+)/' => 'HeraldWebhookTestController',
- 'key/' => array(
- 'view/(?P<id>\d+)/' => 'HeraldWebhookViewKeyController',
- 'cycle/(?P<id>\d+)/' => 'HeraldWebhookCycleKeyController',
- ),
+ 'key/(?P<action>view|cycle)/(?P<id>\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 @@
+<?php
+
+final class HeraldWebhookKeyController
+ extends HeraldWebhookController {
+
+ public function handleRequest(AphrontRequest $request) {
+ $viewer = $this->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 = <<<EOREMARKUP
+Optionally, choose an object to generate test data for (like `D123` or `T234`).
+
+The 10 most recent transactions for the object will be submitted to the webhook.
+EOREMARKUP;
+
+ $form = id(new AphrontFormView())
+ ->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 @@
+<?php
+
+final class HeraldWebhookRequestGarbageCollector
+ extends PhabricatorGarbageCollector {
+
+ const COLLECTORCONST = 'herald.webhooks';
+
+ public function getCollectorName() {
+ return pht('Herald Webhooks');
+ }
+
+ public function getDefaultRetentionPolicy() {
+ return phutil_units('7 days in seconds');
+ }
+
+ protected function collectGarbage() {
+ $table = new HeraldWebhookRequest();
+ $conn_w = $table->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) {
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sun, Mar 16, 10:57 PM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7223212
Default Alt Text
D19046.id45670.diff (21 KB)
Attached To
Mode
D19046: Refine core webhook implementation somewhat
Attached
Detach File
Event Timeline
Log In to Comment