Skip to content

Commit 6f6ca01

Browse files
author
epriestley
committed
Send forced mail on SSH key edits
Summary: Ref T10917. This cheats fairly heavily to generate SSH key mail: - Generate normal transaction mail. - Force it to go to the user. - Use `setForceDelivery()` to force it to actually be delivered. - Add some warning language to the mail body. This doesn't move us much closer to Glorious Infrastructure for this whole class of events, but should do what it needs to for now and doesn't really require anything sketchy. Test Plan: Created and edited SSH keys, got security notice mail. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10917 Differential Revision: https://secure.phabricator.com/D15948
1 parent da6b3de commit 6f6ca01

File tree

9 files changed

+134
-7
lines changed

9 files changed

+134
-7
lines changed

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,6 +1883,7 @@
18831883
'PhabricatorAuthSSHKeyListController' => 'applications/auth/controller/PhabricatorAuthSSHKeyListController.php',
18841884
'PhabricatorAuthSSHKeyPHIDType' => 'applications/auth/phid/PhabricatorAuthSSHKeyPHIDType.php',
18851885
'PhabricatorAuthSSHKeyQuery' => 'applications/auth/query/PhabricatorAuthSSHKeyQuery.php',
1886+
'PhabricatorAuthSSHKeyReplyHandler' => 'applications/auth/mail/PhabricatorAuthSSHKeyReplyHandler.php',
18861887
'PhabricatorAuthSSHKeySearchEngine' => 'applications/auth/query/PhabricatorAuthSSHKeySearchEngine.php',
18871888
'PhabricatorAuthSSHKeyTableView' => 'applications/auth/view/PhabricatorAuthSSHKeyTableView.php',
18881889
'PhabricatorAuthSSHKeyTransaction' => 'applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php',
@@ -6318,6 +6319,7 @@
63186319
'PhabricatorAuthSSHKeyListController' => 'PhabricatorAuthSSHKeyController',
63196320
'PhabricatorAuthSSHKeyPHIDType' => 'PhabricatorPHIDType',
63206321
'PhabricatorAuthSSHKeyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
6322+
'PhabricatorAuthSSHKeyReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
63216323
'PhabricatorAuthSSHKeySearchEngine' => 'PhabricatorApplicationSearchEngine',
63226324
'PhabricatorAuthSSHKeyTableView' => 'AphrontView',
63236325
'PhabricatorAuthSSHKeyTransaction' => 'PhabricatorApplicationTransaction',

src/applications/almanac/storage/AlmanacDevice.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,14 @@ public function getSSHKeyDefaultName() {
227227
return $this->getName();
228228
}
229229

230+
public function getSSHKeyNotifyPHIDs() {
231+
// Devices don't currently have anyone useful to notify about SSH key
232+
// edits, and they're usually a difficult vector to attack since you need
233+
// access to a cluster host. However, it would be nice to make them
234+
// subscribable at some point.
235+
return array();
236+
}
237+
230238

231239
/* -( PhabricatorDestructibleInterface )----------------------------------- */
232240

src/applications/auth/controller/PhabricatorAuthSSHKeyGenerateController.php

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,31 @@ public function handleRequest(AphrontRequest $request) {
3636

3737
$type = $public_key->getType();
3838
$body = $public_key->getBody();
39+
$comment = pht('Generated');
3940

40-
$key
41-
->setName($default_name)
42-
->setKeyType($type)
43-
->setKeyBody($body)
44-
->setKeyComment(pht('Generated'))
45-
->save();
41+
$entire_key = "{$type} {$body} {$comment}";
42+
43+
$type_create = PhabricatorTransactions::TYPE_CREATE;
44+
$type_name = PhabricatorAuthSSHKeyTransaction::TYPE_NAME;
45+
$type_key = PhabricatorAuthSSHKeyTransaction::TYPE_KEY;
46+
47+
$xactions = array();
48+
49+
$xactions[] = id(new PhabricatorAuthSSHKeyTransaction())
50+
->setTransactionType(PhabricatorTransactions::TYPE_CREATE);
51+
52+
$xactions[] = id(new PhabricatorAuthSSHKeyTransaction())
53+
->setTransactionType($type_name)
54+
->setNewValue($default_name);
55+
56+
$xactions[] = id(new PhabricatorAuthSSHKeyTransaction())
57+
->setTransactionType($type_key)
58+
->setNewValue($entire_key);
59+
60+
$editor = id(new PhabricatorAuthSSHKeyEditor())
61+
->setActor($viewer)
62+
->setContentSourceFromRequest($request)
63+
->applyTransactions($key, $xactions);
4664

4765
// NOTE: We're disabling workflow on submit so the download works. We're
4866
// disabling workflow on cancel so the page reloads, showing the new

src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,4 +177,68 @@ protected function didCatchDuplicateKeyException(
177177
}
178178

179179

180+
protected function shouldSendMail(
181+
PhabricatorLiskDAO $object,
182+
array $xactions) {
183+
return true;
184+
}
185+
186+
protected function getMailSubjectPrefix() {
187+
return pht('[SSH Key]');
188+
}
189+
190+
protected function getMailThreadID(PhabricatorLiskDAO $object) {
191+
return 'ssh-key-'.$object->getPHID();
192+
}
193+
194+
protected function getMailTo(PhabricatorLiskDAO $object) {
195+
return $object->getObject()->getSSHKeyNotifyPHIDs();
196+
}
197+
198+
protected function getMailCC(PhabricatorLiskDAO $object) {
199+
return array();
200+
}
201+
202+
protected function buildReplyHandler(PhabricatorLiskDAO $object) {
203+
return id(new PhabricatorAuthSSHKeyReplyHandler())
204+
->setMailReceiver($object);
205+
}
206+
207+
protected function buildMailTemplate(PhabricatorLiskDAO $object) {
208+
$id = $object->getID();
209+
$name = $object->getName();
210+
$phid = $object->getPHID();
211+
212+
$mail = id(new PhabricatorMetaMTAMail())
213+
->setSubject(pht('SSH Key %d: %s', $id, $name))
214+
->addHeader('Thread-Topic', $phid);
215+
216+
// The primary value of this mail is alerting users to account compromises,
217+
// so force delivery. In particular, this mail should still be delievered
218+
// even if "self mail" is disabled.
219+
$mail->setForceDelivery(true);
220+
221+
return $mail;
222+
}
223+
224+
protected function buildMailBody(
225+
PhabricatorLiskDAO $object,
226+
array $xactions) {
227+
228+
$body = parent::buildMailBody($object, $xactions);
229+
230+
$body->addLinkSection(
231+
pht('SECURITY WARNING'),
232+
pht(
233+
'If you do not recognize this change, it may indicate your account '.
234+
'has been compromised.'));
235+
236+
$detail_uri = $object->getURI();
237+
$detail_uri = PhabricatorEnv::getProductionURI($detail_uri);
238+
239+
$body->addLinkSection(pht('SSH KEY DETAIL'), $detail_uri);
240+
241+
return $body;
242+
}
243+
180244
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
final class PhabricatorAuthSSHKeyReplyHandler
4+
extends PhabricatorApplicationTransactionReplyHandler {
5+
6+
public function validateMailReceiver($mail_receiver) {
7+
if (!($mail_receiver instanceof PhabricatorAuthSSHKey)) {
8+
throw new Exception(
9+
pht('Mail receiver is not a %s!', 'PhabricatorAuthSSHKey'));
10+
}
11+
}
12+
13+
public function getObjectPrefix() {
14+
return 'SSHKEY';
15+
}
16+
17+
}

src/applications/auth/sshkey/PhabricatorSSHPublicKeyInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,6 @@ public function getSSHPublicKeyManagementURI(PhabricatorUser $viewer);
1717
*/
1818
public function getSSHKeyDefaultName();
1919

20+
public function getSSHKeyNotifyPHIDs();
21+
2022
}

src/applications/auth/storage/PhabricatorAuthSSHKey.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ public function save() {
7070
return parent::save();
7171
}
7272

73+
public function getMailKey() {
74+
// NOTE: We don't actually receive mail for these objects. It's OK for
75+
// the mail key to be predictable until we do.
76+
return PhabricatorHash::digestForIndex($this->getPHID());
77+
}
78+
7379
public function toPublicKey() {
7480
return PhabricatorAuthSSHPublicKey::newFromStoredKey($this);
7581
}
@@ -164,7 +170,7 @@ public function getApplicationTransactionObject() {
164170
}
165171

166172
public function getApplicationTransactionTemplate() {
167-
return new PhabricatorAuthProviderConfigTransaction();
173+
return new PhabricatorAuthSSHKeyTransaction();
168174
}
169175

170176
public function willRenderTimeline(

src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ public function getTitle() {
2626
$new = $this->getNewValue();
2727

2828
switch ($this->getTransactionType()) {
29+
case PhabricatorTransactions::TYPE_CREATE:
30+
return pht(
31+
'%s created this key.',
32+
$this->renderHandleLink($author_phid));
2933
case self::TYPE_NAME:
3034
return pht(
3135
'%s renamed this key from "%s" to "%s".',

src/applications/people/storage/PhabricatorUser.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,6 +1342,12 @@ public function getSSHKeyDefaultName() {
13421342
return 'id_rsa_phabricator';
13431343
}
13441344

1345+
public function getSSHKeyNotifyPHIDs() {
1346+
return array(
1347+
$this->getPHID(),
1348+
);
1349+
}
1350+
13451351

13461352
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
13471353

0 commit comments

Comments
 (0)