Page MenuHomePhabricator

Provide a more straightforward way to revoke SSH keys by finding and destroying the objects
ClosedPublic

Authored by nickz on Dec 19 2015, 7:20 AM.
Tags
None
Referenced Files
F13142689: D14823.diff
Fri, May 3, 6:31 AM
F13140154: D14823.diff
Fri, May 3, 3:31 AM
Unknown Object (File)
Mon, Apr 29, 4:50 PM
Unknown Object (File)
Mon, Apr 29, 4:50 PM
Unknown Object (File)
Mon, Apr 29, 4:50 PM
Unknown Object (File)
Mon, Apr 29, 4:50 PM
Unknown Object (File)
Mon, Apr 29, 3:21 PM
Unknown Object (File)
Thu, Apr 25, 4:39 PM
Subscribers
Tokens
"Piece of Eight" token, awarded by epriestley.

Details

Summary

Ref T9967

Test Plan

Ran migrations.
Verified database populated properly with PHIDs (SELECT * FROM auth_sshkey;).
Ran auth.querypublickeys conduit method to see phids show up
Ran bin/remove destroy <phid>.
Viewed the test key was gone.

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint Passed
Unit
Tests Skipped
Build Status
Buildable 9656
Build 11566: arc lint + arc unit

Event Timeline

nickz retitled this revision from to Provide a more straightforward way to revoke SSH keys by finding and destroying the objects.
nickz updated this object.
nickz edited the test plan for this revision. (Show Details)
nickz added a reviewer: epriestley.
nickz edited edge metadata.
epriestley edited edge metadata.

This is great, thanks! Couple of minor inlines.

src/applications/auth/phid/PhabricatorAuthSSHKeyPHIDType.php
34–35

SSH keys don't really have any interesting properties (like name or URL), but you could maybe do something like this:

$handle->setName(pht('SSH Key %d', $key->getID()));

For example, I think that will make bin/remove destroy render things a little more cleanly. As-is, it will render something like this:

...
This object will be destroyed forever:

    - PHID-AKEY-irxjvgxviobklieu2677 (PhabricatorAuthSSHKey) Unknown Object (SSH Key)

    Are you absolutely certain you want to destroy this object? [y/N]

...but if you use setName() it should render something a little nicer, more like this:

This object will be destroyed forever:

    - PHID-AKEY-irxjvgxviobklieu2677 (PhabricatorAuthSSHKey) SSH Key 234

    Are you absolutely certain you want to destroy this object? [y/N]
src/applications/auth/storage/PhabricatorAuthSSHKey.php
105

Stray debugging print.

106–110

You don't need to do this, since we're only deleting the object itself (SSH Keys don't have any sub-objects or sub-rows, like Owners do), and the $this->delete() call on line 112 will take care of deleting the object.

So the body should be fine as just this:

$this->openTransaction();
  $this->delete();
$this->saveTransaction();
This revision now requires changes to proceed.Dec 19 2015, 11:37 AM
nickz edited edge metadata.

Updated based on Evan's comments.

epriestley edited edge metadata.

Thanks! I added you to Blessed Committers, so you should be able to land this yourself.

(The project page also has some instructions on how to get access configured, now.)

This revision is now accepted and ready to land.Dec 19 2015, 5:58 PM
This revision was automatically updated to reflect the committed changes.

Thanks! I added you to Blessed Committers, so you should be able to land this yourself.

(The project page also has some instructions on how to get access configured, now.)

Thank you! Happy to be a blessed_committer!!!