From c58506aeaace694534a13b40c558a670aa8230c5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 13 Dec 2018 10:13:56 -0800 Subject: [PATCH] Give sessions real PHIDs and slightly modernize session queries Summary: Ref T13222. See PHI873. I'm preparing to introduce a new MFA "Challenge" table which stores state about challenges we've issued (to bind challenges to sessions and prevent most challenge reuse). This table will reference sessions (since each challenge will be bound to a particular session) but sessions currently don't have PHIDs. Give them PHIDs and slightly modernize some related code. Test Plan: - Ran migrations. - Verified table got PHIDs. - Used `var_dump()` to dump an organic user session. Reviewers: amckinley Reviewed By: amckinley Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13222 Differential Revision: https://secure.phabricator.com/D19881 --- .../20181213.auth.01.sessionphid.sql | 2 + .../20181213.auth.02.populatephid.php | 18 +++++++++ .../autopatches/20181213.auth.03.phidkey.sql | 2 + src/__phutil_library_map__.php | 2 + .../engine/PhabricatorAuthSessionEngine.php | 1 + .../phid/PhabricatorAuthSessionPHIDType.php | 34 +++++++++++++++++ .../query/PhabricatorAuthSessionQuery.php | 38 ++++++++++--------- .../auth/storage/PhabricatorAuthSession.php | 5 +++ 8 files changed, 85 insertions(+), 17 deletions(-) create mode 100644 resources/sql/autopatches/20181213.auth.01.sessionphid.sql create mode 100644 resources/sql/autopatches/20181213.auth.02.populatephid.php create mode 100644 resources/sql/autopatches/20181213.auth.03.phidkey.sql create mode 100644 src/applications/auth/phid/PhabricatorAuthSessionPHIDType.php diff --git a/resources/sql/autopatches/20181213.auth.01.sessionphid.sql b/resources/sql/autopatches/20181213.auth.01.sessionphid.sql new file mode 100644 index 0000000000..34b5aa5bf6 --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.01.sessionphid.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.phabricator_session + ADD phid VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20181213.auth.02.populatephid.php b/resources/sql/autopatches/20181213.auth.02.populatephid.php new file mode 100644 index 0000000000..314eaf87a3 --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.02.populatephid.php @@ -0,0 +1,18 @@ +establishConnection('w'); + +foreach ($iterator as $session) { + if (strlen($session->getPHID())) { + continue; + } + + queryfx( + $conn, + 'UPDATE %R SET phid = %s WHERE id = %d', + $table, + $session->generatePHID(), + $session->getID()); +} diff --git a/resources/sql/autopatches/20181213.auth.03.phidkey.sql b/resources/sql/autopatches/20181213.auth.03.phidkey.sql new file mode 100644 index 0000000000..6bc11b3e55 --- /dev/null +++ b/resources/sql/autopatches/20181213.auth.03.phidkey.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.phabricator_session + ADD UNIQUE KEY `key_phid` (phid); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 09430367d9..420457f525 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2296,6 +2296,7 @@ phutil_register_library_map(array( 'PhabricatorAuthSessionEngineExtensionModule' => 'applications/auth/engine/PhabricatorAuthSessionEngineExtensionModule.php', 'PhabricatorAuthSessionGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthSessionGarbageCollector.php', 'PhabricatorAuthSessionInfo' => 'applications/auth/data/PhabricatorAuthSessionInfo.php', + 'PhabricatorAuthSessionPHIDType' => 'applications/auth/phid/PhabricatorAuthSessionPHIDType.php', 'PhabricatorAuthSessionQuery' => 'applications/auth/query/PhabricatorAuthSessionQuery.php', 'PhabricatorAuthSessionRevoker' => 'applications/auth/revoker/PhabricatorAuthSessionRevoker.php', 'PhabricatorAuthSetPasswordController' => 'applications/auth/controller/PhabricatorAuthSetPasswordController.php', @@ -7948,6 +7949,7 @@ phutil_register_library_map(array( 'PhabricatorAuthSessionEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorAuthSessionGarbageCollector' => 'PhabricatorGarbageCollector', 'PhabricatorAuthSessionInfo' => 'Phobject', + 'PhabricatorAuthSessionPHIDType' => 'PhabricatorPHIDType', 'PhabricatorAuthSessionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorAuthSessionRevoker' => 'PhabricatorAuthRevoker', 'PhabricatorAuthSetPasswordController' => 'PhabricatorAuthController', diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php index 4ce86e8f97..ada4162067 100644 --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -119,6 +119,7 @@ final class PhabricatorAuthSessionEngine extends Phobject { $conn_r, 'SELECT s.id AS s_id, + s.phid AS s_phid, s.sessionExpires AS s_sessionExpires, s.sessionStart AS s_sessionStart, s.highSecurityUntil AS s_highSecurityUntil, diff --git a/src/applications/auth/phid/PhabricatorAuthSessionPHIDType.php b/src/applications/auth/phid/PhabricatorAuthSessionPHIDType.php new file mode 100644 index 0000000000..e031c4ae88 --- /dev/null +++ b/src/applications/auth/phid/PhabricatorAuthSessionPHIDType.php @@ -0,0 +1,34 @@ +withPHIDs($phids); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + return; + } + +} diff --git a/src/applications/auth/query/PhabricatorAuthSessionQuery.php b/src/applications/auth/query/PhabricatorAuthSessionQuery.php index 25928e72c1..a2101201e2 100644 --- a/src/applications/auth/query/PhabricatorAuthSessionQuery.php +++ b/src/applications/auth/query/PhabricatorAuthSessionQuery.php @@ -4,6 +4,7 @@ final class PhabricatorAuthSessionQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; + private $phids; private $identityPHIDs; private $sessionKeys; private $sessionTypes; @@ -28,19 +29,17 @@ final class PhabricatorAuthSessionQuery return $this; } + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + public function newResultObject() { + return new PhabricatorAuthSession(); + } + protected function loadPage() { - $table = new PhabricatorAuthSession(); - $conn_r = $table->establishConnection('r'); - - $data = queryfx_all( - $conn_r, - 'SELECT * FROM %T %Q %Q %Q', - $table->getTableName(), - $this->buildWhereClause($conn_r), - $this->buildOrderClause($conn_r), - $this->buildLimitClause($conn_r)); - - return $table->loadAllFromArray($data); + return $this->loadStandardPage($this->newResultObject()); } protected function willFilterPage(array $sessions) { @@ -65,8 +64,8 @@ final class PhabricatorAuthSessionQuery return $sessions; } - protected function buildWhereClause(AphrontDatabaseConnection $conn) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); if ($this->ids !== null) { $where[] = qsprintf( @@ -75,6 +74,13 @@ final class PhabricatorAuthSessionQuery $this->ids); } + if ($this->phids !== null) { + $where[] = qsprintf( + $conn, + 'phid IN (%Ls)', + $this->phids); + } + if ($this->identityPHIDs !== null) { $where[] = qsprintf( $conn, @@ -100,9 +106,7 @@ final class PhabricatorAuthSessionQuery $this->sessionTypes); } - $where[] = $this->buildPagingClause($conn); - - return $this->formatWhereClause($conn, $where); + return $where; } public function getQueryApplicationClass() { diff --git a/src/applications/auth/storage/PhabricatorAuthSession.php b/src/applications/auth/storage/PhabricatorAuthSession.php index cf707a053d..c4731b669b 100644 --- a/src/applications/auth/storage/PhabricatorAuthSession.php +++ b/src/applications/auth/storage/PhabricatorAuthSession.php @@ -20,6 +20,7 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO protected function getConfiguration() { return array( self::CONFIG_TIMESTAMPS => false, + self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( 'type' => 'text32', 'sessionKey' => 'bytes40', @@ -74,6 +75,10 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO } } + public function getPHIDType() { + return PhabricatorAuthSessionPHIDType::TYPECONST; + } + public function isHighSecuritySession() { $until = $this->getHighSecurityUntil();