From 69053a40f94706c5a71f44c542bf8c88e005caea Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 14 Apr 2017 05:06:20 -0700 Subject: [PATCH] Dirty the SSH key cache when usernames change Summary: Fixes T12554. The SSH key cache contains usernames, but is not currently dirtied on username changes. An alternative solution would be to use user PHIDs instead of usernames in the file, which would make this unnecessary, but that would make debugging a bit harder. For now, I think this small added complexity is worth the easier debugging, but we could look at this again if cache management gets harder in the future. Test Plan: - Added a key as `ducksey`, ran `bin/ssh-auth`, saw key immediately. - Renamed `ducksey` to `ducker`, ran `bin/ssh-auth`, saw username change immediately. - Added another key as `ducker`, ran `bin/ssh-auth`, saw key immediately. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12554 Differential Revision: https://secure.phabricator.com/D17687 --- .../auth/editor/PhabricatorAuthSSHKeyEditor.php | 4 +--- src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php | 6 ++++++ src/applications/people/editor/PhabricatorUserEditor.php | 4 ++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php b/src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php index 4d04707598..0962b7b56a 100644 --- a/src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php +++ b/src/applications/auth/editor/PhabricatorAuthSSHKeyEditor.php @@ -197,9 +197,7 @@ final class PhabricatorAuthSSHKeyEditor // After making any change to an SSH key, drop the authfile cache so it // is regenerated the next time anyone authenticates. - $cache = PhabricatorCaches::getMutableCache(); - $authfile_key = PhabricatorAuthSSHKeyQuery::AUTHFILE_CACHEKEY; - $cache->deleteKey($authfile_key); + PhabricatorAuthSSHKeyQuery::deleteSSHKeyCache(); return $xactions; } diff --git a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php index 6ba047d100..77b666ea44 100644 --- a/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php +++ b/src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php @@ -11,6 +11,12 @@ final class PhabricatorAuthSSHKeyQuery private $keys; private $isActive; + public static function deleteSSHKeyCache() { + $cache = PhabricatorCaches::getMutableCache(); + $authfile_key = self::AUTHFILE_CACHEKEY; + $cache->deleteKey($authfile_key); + } + public function withIDs(array $ids) { $this->ids = $ids; return $this; diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php index 3370fb428b..cb5ed65f8d 100644 --- a/src/applications/people/editor/PhabricatorUserEditor.php +++ b/src/applications/people/editor/PhabricatorUserEditor.php @@ -195,6 +195,10 @@ final class PhabricatorUserEditor extends PhabricatorEditor { $user->saveTransaction(); + // The SSH key cache currently includes usernames, so dirty it. See T12554 + // for discussion. + PhabricatorAuthSSHKeyQuery::deleteSSHKeyCache(); + $user->sendUsernameChangeEmail($actor, $old_username); }