From d4bd9c0d06f3f8032ef166b989f2de58cd39a0d3 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 19 Feb 2021 13:12:47 +0100 Subject: [PATCH] decouple user session from tokens --- app/config/collections.php | 93 ++++++++++++++++++++++++++---- app/controllers/api/account.php | 71 +++++++++++------------ app/controllers/api/teams.php | 7 +-- app/controllers/api/users.php | 17 +++--- app/init.php | 2 +- src/Appwrite/Auth/Auth.php | 30 +++++++++- src/Appwrite/Database/Database.php | 1 + tests/unit/Auth/AuthTest.php | 71 ++++++++++++++++++----- 8 files changed, 214 insertions(+), 78 deletions(-) diff --git a/app/config/collections.php b/app/config/collections.php index 9164cd93b4..92e687cd17 100644 --- a/app/config/collections.php +++ b/app/config/collections.php @@ -271,6 +271,16 @@ $collections = [ 'required' => true, 'array' => false, ], + [ + '$collection' => Database::SYSTEM_COLLECTION_RULES, + 'label' => 'Sessions', + 'key' => 'sessions', + 'type' => Database::SYSTEM_VAR_TYPE_DOCUMENT, + 'default' => [], + 'required' => false, + 'array' => true, + 'list' => [Database::SYSTEM_COLLECTION_SESSIONS], + ], [ '$collection' => Database::SYSTEM_COLLECTION_RULES, 'label' => 'Tokens', @@ -293,11 +303,11 @@ $collections = [ ], ], ], - Database::SYSTEM_COLLECTION_TOKENS => [ + Database::SYSTEM_COLLECTION_SESSIONS => [ '$collection' => Database::SYSTEM_COLLECTION_COLLECTIONS, - '$id' => Database::SYSTEM_COLLECTION_TOKENS, + '$id' => Database::SYSTEM_COLLECTION_SESSIONS, '$permissions' => ['read' => ['*']], - 'name' => 'Token', + 'name' => 'Sessions', 'structure' => true, 'rules' => [ [ @@ -306,15 +316,15 @@ $collections = [ 'key' => 'userId', 'type' => Database::SYSTEM_VAR_TYPE_TEXT, 'default' => null, - 'required' => false, + 'required' => true, 'array' => false, ], [ '$collection' => Database::SYSTEM_COLLECTION_RULES, - 'label' => 'Type', - 'key' => 'type', - 'type' => Database::SYSTEM_VAR_TYPE_NUMERIC, - 'default' => null, + 'label' => 'Secret', + 'key' => 'secret', + 'type' => Database::SYSTEM_VAR_TYPE_TEXT, + 'default' => '', 'required' => true, 'array' => false, ], @@ -324,7 +334,7 @@ $collections = [ 'key' => 'provider', 'type' => Database::SYSTEM_VAR_TYPE_TEXT, 'default' => '', - 'required' => false, + 'required' => true, 'array' => false, ], [ @@ -333,7 +343,7 @@ $collections = [ 'key' => 'providerUid', 'type' => Database::SYSTEM_VAR_TYPE_TEXT, 'default' => '', - 'required' => false, + 'required' => true, 'array' => false, ], [ @@ -500,6 +510,69 @@ $collections = [ ], ], ], + Database::SYSTEM_COLLECTION_TOKENS => [ + '$collection' => Database::SYSTEM_COLLECTION_COLLECTIONS, + '$id' => Database::SYSTEM_COLLECTION_TOKENS, + '$permissions' => ['read' => ['*']], + 'name' => 'Token', + 'structure' => true, + 'rules' => [ + [ + '$collection' => Database::SYSTEM_COLLECTION_RULES, + 'label' => 'User ID', + 'key' => 'userId', + 'type' => Database::SYSTEM_VAR_TYPE_TEXT, + 'default' => null, + 'required' => false, + 'array' => false, + ], + [ + '$collection' => Database::SYSTEM_COLLECTION_RULES, + 'label' => 'Type', + 'key' => 'type', + 'type' => Database::SYSTEM_VAR_TYPE_NUMERIC, + 'default' => null, + 'required' => true, + 'array' => false, + ], + [ + '$collection' => Database::SYSTEM_COLLECTION_RULES, + 'label' => 'Secret', + 'key' => 'secret', + 'type' => Database::SYSTEM_VAR_TYPE_TEXT, + 'default' => '', + 'required' => true, + 'array' => false, + ], + [ + '$collection' => Database::SYSTEM_COLLECTION_RULES, + 'label' => 'Expire', + 'key' => 'expire', + 'type' => Database::SYSTEM_VAR_TYPE_NUMERIC, + 'default' => 0, + 'required' => true, + 'array' => false, + ], + [ + '$collection' => Database::SYSTEM_COLLECTION_RULES, + 'label' => 'User Agent', + 'key' => 'userAgent', + 'type' => Database::SYSTEM_VAR_TYPE_TEXT, + 'default' => '', + 'required' => true, + 'array' => false, + ], + [ + '$collection' => Database::SYSTEM_COLLECTION_RULES, + 'label' => 'IP', + 'key' => 'ip', + 'type' => Database::SYSTEM_VAR_TYPE_IP, + 'default' => '', + 'required' => true, + 'array' => false, + ], + ], + ], Database::SYSTEM_COLLECTION_MEMBERSHIPS => [ '$collection' => Database::SYSTEM_COLLECTION_COLLECTIONS, '$id' => Database::SYSTEM_COLLECTION_MEMBERSHIPS, diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 26c76a7f30..a81e38288a 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -190,11 +190,10 @@ App::post('/v1/account/sessions') $secret = Auth::tokenGenerator(); $session = new Document(array_merge( [ - '$collection' => Database::SYSTEM_COLLECTION_TOKENS, + '$collection' => Database::SYSTEM_COLLECTION_SESSIONS, '$permissions' => ['read' => ['user:'.$profile->getId()], 'write' => ['user:'.$profile->getId()]], 'userId' => $profile->getId(), - 'type' => Auth::TOKEN_TYPE_LOGIN, - 'provider' => Auth::TOKEN_PROVIDER_EMAIL, + 'provider' => Auth::SESSION_PROVIDER_EMAIL, 'providerUid' => $email, 'secret' => Auth::hash($secret), // One way hash encryption to protect DB leak 'expire' => $expiry, @@ -212,7 +211,7 @@ App::post('/v1/account/sessions') throw new Exception('Failed saving session to DB', 500); } - $profile->setAttribute('tokens', $session, Document::SET_TYPE_APPEND); + $profile->setAttribute('sessions', $session, Document::SET_TYPE_APPEND); $profile = $projectDB->updateDocument($profile->getArrayCopy()); @@ -441,7 +440,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') throw new Exception('Missing ID from OAuth2 provider', 400); } - $current = Auth::tokenVerify($user->getAttribute('tokens', []), Auth::TOKEN_TYPE_LOGIN, Auth::$secret); + $current = Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret); if ($current) { $projectDB->deleteDocument($current); //throw new Exception('User already logged in', 401); @@ -451,8 +450,8 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'limit' => 1, 'filters' => [ '$collection='.Database::SYSTEM_COLLECTION_USERS, - 'tokens.provider='.$provider, - 'tokens.providerUid='.$oauth2ID + 'sessions.provider='.$provider, + 'sessions.providerUid='.$oauth2ID ], ]) : $user; @@ -507,10 +506,9 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $secret = Auth::tokenGenerator(); $expiry = \time() + Auth::TOKEN_EXPIRATION_LOGIN_LONG; $session = new Document(array_merge([ - '$collection' => Database::SYSTEM_COLLECTION_TOKENS, + '$collection' => Database::SYSTEM_COLLECTION_SESSIONS, '$permissions' => ['read' => ['user:'.$user['$id']], 'write' => ['user:'.$user['$id']]], 'userId' => $user->getId(), - 'type' => Auth::TOKEN_TYPE_LOGIN, 'provider' => $provider, 'providerUid' => $oauth2ID, 'providerToken' => $accessToken, @@ -523,7 +521,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $user ->setAttribute('status', Auth::USER_STATUS_ACTIVATED) - ->setAttribute('tokens', $session, Document::SET_TYPE_APPEND) + ->setAttribute('sessions', $session, Document::SET_TYPE_APPEND) ; Authorization::setRole('user:'.$user->getId()); @@ -585,16 +583,18 @@ App::post('/v1/account/jwt') /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Document $user */ - $tokens = $user->getAttribute('tokens', []); - $session = new Document(); + $sessions = $user->getAttribute('sessions', []); + $current = new Document(); - foreach ($tokens as $token) { /** @var Appwrite\Database\Document $token */ - if ($token->getAttribute('secret') == Auth::hash(Auth::$secret)) { // If current session delete the cookies too - $session = $token; + foreach ($sessions as $session) { + /** @var Appwrite\Database\Document $session */ + + if ($session->getAttribute('secret') == Auth::hash(Auth::$secret)) { // If current session delete the cookies too + $current = $session; } } - if($session->isEmpty()) { + if($current->isEmpty()) { throw new Exception('No valid session found', 401); } @@ -608,7 +608,7 @@ App::post('/v1/account/jwt') // 'scopes' => ['user'], // 'iss' => 'http://api.mysite.com', 'userId' => $user->getId(), - 'sessionId' => $session->getId(), + 'sessionId' => $current->getId(), ])]), Response::MODEL_JWT); }); @@ -673,22 +673,19 @@ App::get('/v1/account/sessions') /** @var Appwrite\Database\Document $user */ /** @var Utopia\Locale\Locale $locale */ - $tokens = $user->getAttribute('tokens', []); - $sessions = []; + $sessions = $user->getAttribute('sessions', []); $countries = $locale->getText('countries'); - $current = Auth::tokenVerify($tokens, Auth::TOKEN_TYPE_LOGIN, Auth::$secret); + $current = Auth::sessionVerify($sessions, Auth::$secret); - foreach ($tokens as $token) { /* @var $token Document */ - if (Auth::TOKEN_TYPE_LOGIN != $token->getAttribute('type')) { - continue; - } + foreach ($sessions as $key => $session) { + /** @var Document $session */ - $token->setAttribute('countryName', (isset($countries[$token->getAttribute('contryCode')])) - ? $countries[$token->getAttribute('contryCode')] + $session->setAttribute('countryName', (isset($countries[$session->getAttribute('contryCode')])) + ? $countries[$session->getAttribute('contryCode')] : $locale->getText('locale.country.unknown')); - $token->setAttribute('current', ($current == $token->getId()) ? true : false); + $session->setAttribute('current', ($current == $session->getId()) ? true : false); - $sessions[] = $token; + $sessions[$key] = $session; } $response->dynamic(new Document([ @@ -1052,14 +1049,14 @@ App::delete('/v1/account/sessions/:sessionId') $protocol = $request->getProtocol(); $sessionId = ($sessionId === 'current') - ? Auth::tokenVerify($user->getAttribute('tokens'), Auth::TOKEN_TYPE_LOGIN, Auth::$secret) + ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret) : $sessionId; - $tokens = $user->getAttribute('tokens', []); + $sessions = $user->getAttribute('sessions', []); - foreach ($tokens as $token) { /* @var $token Document */ - if (($sessionId == $token->getId()) && Auth::TOKEN_TYPE_LOGIN == $token->getAttribute('type')) { - if (!$projectDB->deleteDocument($token->getId())) { + foreach ($sessions as $session) { /** @var Document $session */ + if (($sessionId == $session->getId())) { + if (!$projectDB->deleteDocument($session->getId())) { throw new Exception('Failed to remove token from DB', 500); } @@ -1075,10 +1072,10 @@ App::delete('/v1/account/sessions/:sessionId') ; } - $token->setAttribute('current', false); + $session->setAttribute('current', false); - if ($token->getAttribute('secret') == Auth::hash(Auth::$secret)) { // If current session delete the cookies too - $token->setAttribute('current', true); + if ($session->getAttribute('secret') == Auth::hash(Auth::$secret)) { // If current session delete the cookies too + $session->setAttribute('current', true); $response ->addCookie(Auth::$cookieName.'_legacy', '', \time() - 3600, '/', Config::getParam('cookieDomain'), ('https' == $protocol), true, null) @@ -1087,7 +1084,7 @@ App::delete('/v1/account/sessions/:sessionId') } $events - ->setParam('payload', $response->output($token, Response::MODEL_SESSION)) + ->setParam('payload', $response->output($session, Response::MODEL_SESSION)) ; return $response->noContent(); diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index a674eacf0d..775d9b47a3 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -595,11 +595,10 @@ App::patch('/v1/teams/:teamId/memberships/:inviteId/status') $expiry = \time() + Auth::TOKEN_EXPIRATION_LOGIN_LONG; $secret = Auth::tokenGenerator(); $session = new Document(array_merge([ - '$collection' => Database::SYSTEM_COLLECTION_TOKENS, + '$collection' => Database::SYSTEM_COLLECTION_SESSIONS, '$permissions' => ['read' => ['user:'.$user->getId()], 'write' => ['user:'.$user->getId()]], 'userId' => $user->getId(), - 'type' => Auth::TOKEN_TYPE_LOGIN, - 'provider' => Auth::TOKEN_PROVIDER_EMAIL, + 'provider' => Auth::SESSION_PROVIDER_EMAIL, 'providerUid' => $user->getAttribute('email'), 'secret' => Auth::hash($secret), // One way hash encryption to protect DB leak 'expire' => $expiry, @@ -608,7 +607,7 @@ App::patch('/v1/teams/:teamId/memberships/:inviteId/status') 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', ], $detector->getOS(), $detector->getClient(), $detector->getDevice())); - $user->setAttribute('tokens', $session, Document::SET_TYPE_APPEND); + $user->setAttribute('sessions', $session, Document::SET_TYPE_APPEND); Authorization::setRole('user:'.$userId); diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index efb0041cee..b8e79ccb95 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -196,21 +196,18 @@ App::get('/v1/users/:userId/sessions') throw new Exception('User not found', 404); } - $tokens = $user->getAttribute('tokens', []); - $sessions = []; + $sessions = $user->getAttribute('sessions', []); $countries = $locale->getText('countries'); - foreach ($tokens as $token) { /* @var $token Document */ - if (Auth::TOKEN_TYPE_LOGIN != $token->getAttribute('type')) { - continue; - } + foreach ($sessions as $key => $session) { + /** @var Document $session */ - $token->setAttribute('countryName', (isset($countries[$token->getAttribute('contryCode')])) - ? $countries[$token->getAttribute('contryCode')] + $session->setAttribute('countryName', (isset($countries[$session->getAttribute('contryCode')])) + ? $countries[$session->getAttribute('contryCode')] : $locale->getText('locale.country.unknown')); - $token->setAttribute('current', false); + $session->setAttribute('current', false); - $sessions[] = $token; + $sessions[$key] = $session; } $response->dynamic(new Document([ diff --git a/app/init.php b/app/init.php index 7cf2f9f0fb..372a3857b0 100644 --- a/app/init.php +++ b/app/init.php @@ -419,7 +419,7 @@ App::setResource('user', function($mode, $project, $console, $request, $response if (empty($user->getId()) // Check a document has been found in the DB || Database::SYSTEM_COLLECTION_USERS !== $user->getCollection() // Validate returned document is really a user document - || !Auth::tokenVerify($user->getAttribute('tokens', []), Auth::TOKEN_TYPE_LOGIN, Auth::$secret)) { // Validate user has valid login token + || !Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret)) { // Validate user has valid login token $user = new Document(['$id' => '', '$collection' => Database::SYSTEM_COLLECTION_USERS]); } diff --git a/src/Appwrite/Auth/Auth.php b/src/Appwrite/Auth/Auth.php index b9da8cde7d..18a2837f5a 100644 --- a/src/Appwrite/Auth/Auth.php +++ b/src/Appwrite/Auth/Auth.php @@ -28,7 +28,7 @@ class Auth /** * Token Types. */ - const TOKEN_TYPE_LOGIN = 1; + const TOKEN_TYPE_LOGIN = 1; // Deprecated const TOKEN_TYPE_VERIFICATION = 2; const TOKEN_TYPE_RECOVERY = 3; const TOKEN_TYPE_INVITE = 4; @@ -36,8 +36,8 @@ class Auth /** * Session Providers. */ - const TOKEN_PROVIDER_EMAIL = 'email'; - const TOKEN_PROVIDER_ANONYMOUS = 'anonymous'; + const SESSION_PROVIDER_EMAIL = 'email'; + const SESSION_PROVIDER_ANONYMOUS = 'anonymous'; /** * Token Expiration times. @@ -213,6 +213,30 @@ class Auth return false; } + /** + * Verify session and check that its not expired. + * + * @param array $sessions + * @param string $secret + * + * @return bool|string + */ + public static function sessionVerify(array $sessions, string $secret) + { + foreach ($sessions as $session) { /** @var Document $session */ + if ($session->isSet('secret') && + $session->isSet('expire') && + $session->isSet('provider') && + $session->isSet('providerUid') && + $session->getAttribute('secret') === self::hash($secret) && + $session->getAttribute('expire') >= \time()) { + return (string)$session->getId(); + } + } + + return false; + } + /** * Is Previligged User? * diff --git a/src/Appwrite/Database/Database.php b/src/Appwrite/Database/Database.php index 4154214b82..d0defdec03 100644 --- a/src/Appwrite/Database/Database.php +++ b/src/Appwrite/Database/Database.php @@ -27,6 +27,7 @@ class Database // Auth, Account and Users (private to user) const SYSTEM_COLLECTION_USERS = 'users'; + const SYSTEM_COLLECTION_SESSIONS = 'sessions'; const SYSTEM_COLLECTION_TOKENS = 'tokens'; // Teams (shared among team members) diff --git a/tests/unit/Auth/AuthTest.php b/tests/unit/Auth/AuthTest.php index 5860d1efb9..6a9b8ab3b0 100644 --- a/tests/unit/Auth/AuthTest.php +++ b/tests/unit/Auth/AuthTest.php @@ -62,41 +62,55 @@ class AuthTest extends TestCase $this->assertEquals(\mb_strlen(Auth::tokenGenerator(5)), 10); } - public function testTokenVerify() + public function testSessionVerify() { $secret = 'secret1'; $hash = Auth::hash($secret); $tokens1 = [ new Document([ '$id' => 'token1', - 'type' => Auth::TOKEN_TYPE_LOGIN, 'expire' => time() + 60 * 60 * 24, 'secret' => $hash, + 'provider' => Auth::SESSION_PROVIDER_EMAIL, + 'providerUid' => 'test@example.com', ]), new Document([ '$id' => 'token2', - 'type' => Auth::TOKEN_TYPE_LOGIN, 'expire' => time() - 60 * 60 * 24, 'secret' => 'secret2', + 'provider' => Auth::SESSION_PROVIDER_EMAIL, + 'providerUid' => 'test@example.com', ]), ]; $tokens2 = [ new Document([ // Correct secret and type time, wrong expire time '$id' => 'token1', - 'type' => Auth::TOKEN_TYPE_LOGIN, 'expire' => time() - 60 * 60 * 24, 'secret' => $hash, + 'provider' => Auth::SESSION_PROVIDER_EMAIL, + 'providerUid' => 'test@example.com', ]), new Document([ '$id' => 'token2', - 'type' => Auth::TOKEN_TYPE_LOGIN, 'expire' => time() - 60 * 60 * 24, 'secret' => 'secret2', + 'provider' => Auth::SESSION_PROVIDER_EMAIL, + 'providerUid' => 'test@example.com', ]), ]; - $tokens3 = [ // Correct secret and expire time, wrong type + $this->assertEquals(Auth::sessionVerify($tokens1, $secret), 'token1'); + $this->assertEquals(Auth::sessionVerify($tokens1, 'false-secret'), false); + $this->assertEquals(Auth::sessionVerify($tokens2, $secret), false); + $this->assertEquals(Auth::sessionVerify($tokens2, 'false-secret'), false); + } + + public function testTokenVerify() + { + $secret = 'secret1'; + $hash = Auth::hash($secret); + $tokens1 = [ new Document([ '$id' => 'token1', 'type' => Auth::TOKEN_TYPE_RECOVERY, @@ -105,20 +119,51 @@ class AuthTest extends TestCase ]), new Document([ '$id' => 'token2', - 'type' => Auth::TOKEN_TYPE_LOGIN, + 'type' => Auth::TOKEN_TYPE_RECOVERY, 'expire' => time() - 60 * 60 * 24, 'secret' => 'secret2', ]), ]; - $this->assertEquals(Auth::tokenVerify($tokens1, Auth::TOKEN_TYPE_LOGIN, $secret), 'token1'); - $this->assertEquals(Auth::tokenVerify($tokens1, Auth::TOKEN_TYPE_LOGIN, 'false-secret'), false); - $this->assertEquals(Auth::tokenVerify($tokens2, Auth::TOKEN_TYPE_LOGIN, $secret), false); - $this->assertEquals(Auth::tokenVerify($tokens2, Auth::TOKEN_TYPE_LOGIN, 'false-secret'), false); - $this->assertEquals(Auth::tokenVerify($tokens3, Auth::TOKEN_TYPE_LOGIN, $secret), false); - $this->assertEquals(Auth::tokenVerify($tokens3, Auth::TOKEN_TYPE_LOGIN, 'false-secret'), false); + $tokens2 = [ + new Document([ // Correct secret and type time, wrong expire time + '$id' => 'token1', + 'type' => Auth::TOKEN_TYPE_RECOVERY, + 'expire' => time() - 60 * 60 * 24, + 'secret' => $hash, + ]), + new Document([ + '$id' => 'token2', + 'type' => Auth::TOKEN_TYPE_RECOVERY, + 'expire' => time() - 60 * 60 * 24, + 'secret' => 'secret2', + ]), + ]; + + $tokens3 = [ // Correct secret and expire time, wrong type + new Document([ + '$id' => 'token1', + 'type' => Auth::TOKEN_TYPE_INVITE, + 'expire' => time() + 60 * 60 * 24, + 'secret' => $hash, + ]), + new Document([ + '$id' => 'token2', + 'type' => Auth::TOKEN_TYPE_RECOVERY, + 'expire' => time() - 60 * 60 * 24, + 'secret' => 'secret2', + ]), + ]; + + $this->assertEquals(Auth::tokenVerify($tokens1, Auth::TOKEN_TYPE_RECOVERY, $secret), 'token1'); + $this->assertEquals(Auth::tokenVerify($tokens1, Auth::TOKEN_TYPE_RECOVERY, 'false-secret'), false); + $this->assertEquals(Auth::tokenVerify($tokens2, Auth::TOKEN_TYPE_RECOVERY, $secret), false); + $this->assertEquals(Auth::tokenVerify($tokens2, Auth::TOKEN_TYPE_RECOVERY, 'false-secret'), false); + $this->assertEquals(Auth::tokenVerify($tokens3, Auth::TOKEN_TYPE_RECOVERY, $secret), false); + $this->assertEquals(Auth::tokenVerify($tokens3, Auth::TOKEN_TYPE_RECOVERY, 'false-secret'), false); } + public function testIsPreviliggedUser() { $this->assertEquals(false, Auth::isPreviliggedUser([]));