From 1da3f7770fae6e6df2eff710e79de2dd831da985 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Tue, 6 Dec 2022 10:45:04 +0000 Subject: [PATCH 01/16] feat (projects): auth session limit Add endpoint on project to set max number of sessions for project users. --- app/controllers/api/projects.php | 31 +++++++++++++++++++++++++++++++ app/init.php | 1 + 2 files changed, 32 insertions(+) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 1fc60c372..d48fe20de 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -576,6 +576,37 @@ App::patch('/v1/projects/:projectId/auth/:method') $response->dynamic($project, Response::MODEL_PROJECT); }); +App::patch('/v1/projects/:projectId/auth/max-sessions') + ->desc('Update Project users limit') + ->groups(['api', 'projects']) + ->label('scope', 'projects.write') + ->label('sdk.auth', [APP_AUTH_TYPE_ADMIN]) + ->label('sdk.namespace', 'projects') + ->label('sdk.method', 'updateAuthLimit') + ->label('sdk.response.code', Response::STATUS_CODE_OK) + ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) + ->label('sdk.response.model', Response::MODEL_PROJECT) + ->param('projectId', '', new UID(), 'Project unique ID.') + ->param('limit', false, new Range(1, APP_LIMIT_USER_SESSIONS), 'Set the max number of users allowed in this project. Use 0 for unlimited.') + ->inject('response') + ->inject('dbForConsole') + ->action(function (string $projectId, int $limit, Response $response, Database $dbForConsole) { + + $project = $dbForConsole->getDocument('projects', $projectId); + + if ($project->isEmpty()) { + throw new Exception(Exception::PROJECT_NOT_FOUND); + } + + $auths = $project->getAttribute('auths', []); + $auths['max-sessions'] = $limit; + + $dbForConsole->updateDocument('projects', $project->getId(), $project + ->setAttribute('auths', $auths)); + + $response->dynamic($project, Response::MODEL_PROJECT); + }); + App::delete('/v1/projects/:projectId') ->desc('Delete Project') ->groups(['api', 'projects']) diff --git a/app/init.php b/app/init.php index ef9246e36..00423f557 100644 --- a/app/init.php +++ b/app/init.php @@ -84,6 +84,7 @@ const APP_MODE_ADMIN = 'admin'; const APP_PAGING_LIMIT = 12; const APP_LIMIT_COUNT = 5000; const APP_LIMIT_USERS = 10000; +const APP_LIMIT_USER_SESSIONS = 10; const APP_LIMIT_ANTIVIRUS = 20000000; //20MB const APP_LIMIT_ENCRYPTION = 20000000; //20MB const APP_LIMIT_COMPRESSION = 20000000; //20MB From 27d7f4f0836eedbf8ce8c7570d854365a79ce99b Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Tue, 6 Dec 2022 10:52:17 +0000 Subject: [PATCH 02/16] fix (projects): session limits Set max session limits to 100 and default session limits to 10 --- app/controllers/api/projects.php | 2 +- app/init.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index d48fe20de..850ab4128 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -81,7 +81,7 @@ App::post('/v1/projects') } $auth = Config::getParam('auth', []); - $auths = ['limit' => 0, 'duration' => Auth::TOKEN_EXPIRATION_LOGIN_LONG]; + $auths = ['limit' => 0, 'max-sessions' => 100, 'duration' => Auth::TOKEN_EXPIRATION_LOGIN_LONG]; foreach ($auth as $index => $method) { $auths[$method['key'] ?? ''] = true; } diff --git a/app/init.php b/app/init.php index 00423f557..6fc8839c7 100644 --- a/app/init.php +++ b/app/init.php @@ -84,7 +84,7 @@ const APP_MODE_ADMIN = 'admin'; const APP_PAGING_LIMIT = 12; const APP_LIMIT_COUNT = 5000; const APP_LIMIT_USERS = 10000; -const APP_LIMIT_USER_SESSIONS = 10; +const APP_LIMIT_USER_SESSIONS = 100; const APP_LIMIT_ANTIVIRUS = 20000000; //20MB const APP_LIMIT_ENCRYPTION = 20000000; //20MB const APP_LIMIT_COMPRESSION = 20000000; //20MB From f070f0974a561298fac94e4b2946b76696481590 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Fri, 9 Dec 2022 11:33:26 +0000 Subject: [PATCH 03/16] session limit check on shutdown --- app/controllers/api/projects.php | 2 +- app/controllers/shared/api.php | 30 ++++++++++++++++ composer.lock | 61 ++++++++++++++++---------------- 3 files changed, 62 insertions(+), 31 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 850ab4128..5840a39cf 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -599,7 +599,7 @@ App::patch('/v1/projects/:projectId/auth/max-sessions') } $auths = $project->getAttribute('auths', []); - $auths['max-sessions'] = $limit; + $auths['maxSessions'] = $limit; $dbForConsole->updateDocument('projects', $project->getId(), $project ->setAttribute('auths', $auths)); diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 6a7502522..f8d4ccf86 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -318,6 +318,36 @@ App::init() } }); +App::shutdown() + ->groups(['auth']) + ->inject('utopia') + ->inject('request') + ->inject('response') + ->inject('project') + ->inject('dbForProject') + ->action(function(App $utopia, Request $request, Response $response, Document $project, Database $dbForProject) { + // Get user total sessions + // check if endpoint is creating new session + // && sessions >= $auth['max-sessions'] + // if yes -> remove oldest active session + $route = $utopia->match($request); + $event = $route->getLabel('event', ''); + if($event === 'users.[userId].sessions.[sessionId].create' && $project->getId() != 'console') { + $sessionLimit = $project->getAttribute('auth', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS; + $session = $response->getPayload(); + $userId = $session['userId'] ?? ''; + if(empty($userId)) return; + $user = $dbForProject->getDocument('users', $userId); + $sessions = $user->getAttribute('sessions', []); + $count = \count($sessions); + if($count <= $sessionLimit) return; + for($i = 0; $i < ($count - $sessionLimit); $i++) { + $session = array_pop($sessions); + $dbForProject->deleteDocument('sessions', $session->getId()); + } + } + }); + App::shutdown() ->groups(['api']) ->inject('utopia') diff --git a/composer.lock b/composer.lock index e97546287..fd8aea14f 100644 --- a/composer.lock +++ b/composer.lock @@ -805,16 +805,16 @@ }, { "name": "laravel/pint", - "version": "v1.2.0", + "version": "v1.2.1", "source": { "type": "git", "url": "https://github.com/laravel/pint.git", - "reference": "1d276e4c803397a26cc337df908f55c2a4e90d86" + "reference": "e60e2112ee779ce60f253695b273d1646a17d6f1" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laravel/pint/zipball/1d276e4c803397a26cc337df908f55c2a4e90d86", - "reference": "1d276e4c803397a26cc337df908f55c2a4e90d86", + "url": "https://api.github.com/repos/laravel/pint/zipball/e60e2112ee779ce60f253695b273d1646a17d6f1", + "reference": "e60e2112ee779ce60f253695b273d1646a17d6f1", "shasum": "" }, "require": { @@ -826,10 +826,10 @@ }, "require-dev": { "friendsofphp/php-cs-fixer": "^3.11.0", - "illuminate/view": "^9.27", - "laravel-zero/framework": "^9.1.3", - "mockery/mockery": "^1.5.0", - "nunomaduro/larastan": "^2.2", + "illuminate/view": "^9.32.0", + "laravel-zero/framework": "^9.2.0", + "mockery/mockery": "^1.5.1", + "nunomaduro/larastan": "^2.2.0", "nunomaduro/termwind": "^1.14.0", "pestphp/pest": "^1.22.1" }, @@ -867,7 +867,7 @@ "issues": "https://github.com/laravel/pint/issues", "source": "https://github.com/laravel/pint" }, - "time": "2022-09-13T15:07:15+00:00" + "time": "2022-11-29T16:25:20+00:00" }, { "name": "matomo/device-detector", @@ -1461,16 +1461,16 @@ }, { "name": "symfony/deprecation-contracts", - "version": "v3.1.1", + "version": "v3.2.0", "source": { "type": "git", "url": "https://github.com/symfony/deprecation-contracts.git", - "reference": "07f1b9cc2ffee6aaafcf4b710fbc38ff736bd918" + "reference": "1ee04c65529dea5d8744774d474e7cbd2f1206d3" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/deprecation-contracts/zipball/07f1b9cc2ffee6aaafcf4b710fbc38ff736bd918", - "reference": "07f1b9cc2ffee6aaafcf4b710fbc38ff736bd918", + "url": "https://api.github.com/repos/symfony/deprecation-contracts/zipball/1ee04c65529dea5d8744774d474e7cbd2f1206d3", + "reference": "1ee04c65529dea5d8744774d474e7cbd2f1206d3", "shasum": "" }, "require": { @@ -1479,7 +1479,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-main": "3.1-dev" + "dev-main": "3.3-dev" }, "thanks": { "name": "symfony/contracts", @@ -1508,7 +1508,7 @@ "description": "A generic function and convention to trigger deprecation notices", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/deprecation-contracts/tree/v3.1.1" + "source": "https://github.com/symfony/deprecation-contracts/tree/v3.2.0" }, "funding": [ { @@ -1524,7 +1524,7 @@ "type": "tidelift" } ], - "time": "2022-02-25T11:15:52+00:00" + "time": "2022-11-25T10:21:52+00:00" }, { "name": "utopia-php/abuse", @@ -1945,24 +1945,25 @@ }, { "name": "utopia-php/framework", - "version": "0.25.0", + "version": "0.25.1", "source": { "type": "git", "url": "https://github.com/utopia-php/framework.git", - "reference": "c524f681254255c8204fbf7919c53bf3b4982636" + "reference": "2391b397135586b2100d39e338827bef8d2f4ad0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/framework/zipball/c524f681254255c8204fbf7919c53bf3b4982636", - "reference": "c524f681254255c8204fbf7919c53bf3b4982636", + "url": "https://api.github.com/repos/utopia-php/framework/zipball/2391b397135586b2100d39e338827bef8d2f4ad0", + "reference": "2391b397135586b2100d39e338827bef8d2f4ad0", "shasum": "" }, "require": { "php": ">=8.0.0" }, "require-dev": { + "laravel/pint": "^1.2", "phpunit/phpunit": "^9.5.25", - "vimeo/psalm": "^4.27.0" + "vimeo/psalm": "4.27.0" }, "type": "library", "autoload": { @@ -1982,9 +1983,9 @@ ], "support": { "issues": "https://github.com/utopia-php/framework/issues", - "source": "https://github.com/utopia-php/framework/tree/0.25.0" + "source": "https://github.com/utopia-php/framework/tree/0.25.1" }, - "time": "2022-11-02T09:49:57+00:00" + "time": "2022-11-23T18:22:23+00:00" }, { "name": "utopia-php/image", @@ -3291,21 +3292,21 @@ }, { "name": "phpspec/prophecy", - "version": "v1.15.0", + "version": "v1.16.0", "source": { "type": "git", "url": "https://github.com/phpspec/prophecy.git", - "reference": "bbcd7380b0ebf3961ee21409db7b38bc31d69a13" + "reference": "be8cac52a0827776ff9ccda8c381ac5b71aeb359" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpspec/prophecy/zipball/bbcd7380b0ebf3961ee21409db7b38bc31d69a13", - "reference": "bbcd7380b0ebf3961ee21409db7b38bc31d69a13", + "url": "https://api.github.com/repos/phpspec/prophecy/zipball/be8cac52a0827776ff9ccda8c381ac5b71aeb359", + "reference": "be8cac52a0827776ff9ccda8c381ac5b71aeb359", "shasum": "" }, "require": { "doctrine/instantiator": "^1.2", - "php": "^7.2 || ~8.0, <8.2", + "php": "^7.2 || 8.0.* || 8.1.* || 8.2.*", "phpdocumentor/reflection-docblock": "^5.2", "sebastian/comparator": "^3.0 || ^4.0", "sebastian/recursion-context": "^3.0 || ^4.0" @@ -3352,9 +3353,9 @@ ], "support": { "issues": "https://github.com/phpspec/prophecy/issues", - "source": "https://github.com/phpspec/prophecy/tree/v1.15.0" + "source": "https://github.com/phpspec/prophecy/tree/v1.16.0" }, - "time": "2021-12-08T12:19:24+00:00" + "time": "2022-11-29T15:06:56+00:00" }, { "name": "phpunit/php-code-coverage", From cbcc17ded6e613e4e17221148c6496c400f79149 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Fri, 9 Dec 2022 11:36:39 +0000 Subject: [PATCH 04/16] refactor: add whitespace --- app/controllers/shared/api.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index f8d4ccf86..a0212f770 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -326,10 +326,6 @@ App::shutdown() ->inject('project') ->inject('dbForProject') ->action(function(App $utopia, Request $request, Response $response, Document $project, Database $dbForProject) { - // Get user total sessions - // check if endpoint is creating new session - // && sessions >= $auth['max-sessions'] - // if yes -> remove oldest active session $route = $utopia->match($request); $event = $route->getLabel('event', ''); if($event === 'users.[userId].sessions.[sessionId].create' && $project->getId() != 'console') { @@ -337,10 +333,14 @@ App::shutdown() $session = $response->getPayload(); $userId = $session['userId'] ?? ''; if(empty($userId)) return; + $user = $dbForProject->getDocument('users', $userId); + if($user->isEmpty()) return; + $sessions = $user->getAttribute('sessions', []); $count = \count($sessions); if($count <= $sessionLimit) return; + for($i = 0; $i < ($count - $sessionLimit); $i++) { $session = array_pop($sessions); $dbForProject->deleteDocument('sessions', $session->getId()); From cda8e533865474a069c0034ed0f7a65f6c69ddb6 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Fri, 9 Dec 2022 11:54:23 +0000 Subject: [PATCH 05/16] feat: add test for max sessions --- app/controllers/shared/api.php | 24 ++-- .../Projects/ProjectsConsoleClientTest.php | 105 ++++++++++++++++++ 2 files changed, 120 insertions(+), 9 deletions(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index a0212f770..20d5976ef 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -325,23 +325,29 @@ App::shutdown() ->inject('response') ->inject('project') ->inject('dbForProject') - ->action(function(App $utopia, Request $request, Response $response, Document $project, Database $dbForProject) { + ->action(function (App $utopia, Request $request, Response $response, Document $project, Database $dbForProject) { $route = $utopia->match($request); $event = $route->getLabel('event', ''); - if($event === 'users.[userId].sessions.[sessionId].create' && $project->getId() != 'console') { + if ($event === 'users.[userId].sessions.[sessionId].create' && $project->getId() != 'console') { $sessionLimit = $project->getAttribute('auth', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS; $session = $response->getPayload(); $userId = $session['userId'] ?? ''; - if(empty($userId)) return; - + if (empty($userId)) { + return; + } + $user = $dbForProject->getDocument('users', $userId); - if($user->isEmpty()) return; - + if ($user->isEmpty()) { + return; + } + $sessions = $user->getAttribute('sessions', []); $count = \count($sessions); - if($count <= $sessionLimit) return; - - for($i = 0; $i < ($count - $sessionLimit); $i++) { + if ($count <= $sessionLimit) { + return; + } + + for ($i = 0; $i < ($count - $sessionLimit); $i++) { $session = array_pop($sessions); $dbForProject->deleteDocument('sessions', $session->getId()); } diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 413b66589..9501cc3af 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -874,6 +874,111 @@ class ProjectsConsoleClientTest extends Scope return $data; } + /** + * @depends testUpdateProjectAuthLimit + */ + public function testUpdateProjectAuthSessionLimit($data): array + { + $id = $data['projectId'] ?? ''; + + /** + * Test for failure + */ + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/max-sessions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'limit' => 0, + ]); + + $this->assertEquals(400, $response['headers']['status-code']); + + /** + * Test for SUCCESS + */ + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/max-sessions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'limit' => 1, + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertNotEmpty($response['body']['$id']); + + $email = uniqid() . 'user@localhost.test'; + $password = 'password'; + $name = 'User Name'; + + /** + * Create new user + */ + $response = $this->client->call(Client::METHOD_POST, '/account', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $id, + ]), [ + 'userId' => ID::unique(), + 'email' => $email, + 'password' => $password, + 'name' => $name, + ]); + + $this->assertEquals($response['headers']['status-code'], 501); + + /** + * create new session + */ + $response = $this->client->call(Client::METHOD_POST, '/account/sessions/email', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $id, + ]), [ + 'email' => $email, + 'password' => $password, + ]); + + + $this->assertEquals(200, $response['headers']['status-code']); + $sessionId1 = $response['body']['$id']; + + /** + * create new session + */ + $response = $this->client->call(Client::METHOD_POST, '/account/sessions/email', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $id, + ]), [ + 'email' => $email, + 'password' => $password, + ]); + + + $this->assertEquals(200, $response['headers']['status-code']); + $sessionCookie = $response['headers']['set-cookie']; + $sessionId2 = $response['body']['$id']; + + /** + * List sessions + */ + $response = $this->client->call(Client::METHOD_GET, '/account/sessions', [ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $id, + 'Cookie' => $sessionCookie, + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + $sessions = $response['body']['sessions']; + + $this->assertEquals(1, count($sessions)); + $this->assertEquals($sessionId2, $sessions[0]['$id']); + + + return $data; + } + public function testUpdateProjectServiceStatusAdmin(): array { $team = $this->client->call(Client::METHOD_POST, '/teams', array_merge([ From 65bacc69835f0f670a9b47be686db914452f47b3 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 11 Dec 2022 07:13:11 +0000 Subject: [PATCH 06/16] fix: naming and use constant --- app/controllers/api/projects.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 5840a39cf..3e5c6e78b 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -81,7 +81,7 @@ App::post('/v1/projects') } $auth = Config::getParam('auth', []); - $auths = ['limit' => 0, 'max-sessions' => 100, 'duration' => Auth::TOKEN_EXPIRATION_LOGIN_LONG]; + $auths = ['limit' => 0, 'maxSessions' => APP_LIMIT_USER_SESSIONS, 'duration' => Auth::TOKEN_EXPIRATION_LOGIN_LONG]; foreach ($auth as $index => $method) { $auths[$method['key'] ?? ''] = true; } @@ -587,7 +587,7 @@ App::patch('/v1/projects/:projectId/auth/max-sessions') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_PROJECT) ->param('projectId', '', new UID(), 'Project unique ID.') - ->param('limit', false, new Range(1, APP_LIMIT_USER_SESSIONS), 'Set the max number of users allowed in this project. Use 0 for unlimited.') + ->param('limit', false, new Range(1, APP_LIMIT_USER_SESSIONS), 'Set the max number of users allowed in this project.') ->inject('response') ->inject('dbForConsole') ->action(function (string $projectId, int $limit, Response $response, Database $dbForConsole) { From 509aef16bd4e9d8c0922c7d26b3e7c675967f0c9 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 11 Dec 2022 07:13:39 +0000 Subject: [PATCH 07/16] fix: delete user document cache --- app/controllers/shared/api.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 20d5976ef..f3f41962e 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -351,6 +351,7 @@ App::shutdown() $session = array_pop($sessions); $dbForProject->deleteDocument('sessions', $session->getId()); } + $dbForProject->deleteCachedDocument('users', $userId); } }); From fd93c8aebd8cc228322d0000e6ff0d0717dfc39d Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 11 Dec 2022 07:15:18 +0000 Subject: [PATCH 08/16] feat: add auth session limit param --- src/Appwrite/Utopia/Response/Model/Project.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Appwrite/Utopia/Response/Model/Project.php b/src/Appwrite/Utopia/Response/Model/Project.php index 42f360d6a..dab80af3c 100644 --- a/src/Appwrite/Utopia/Response/Model/Project.php +++ b/src/Appwrite/Utopia/Response/Model/Project.php @@ -114,6 +114,12 @@ class Project extends Model 'default' => 0, 'example' => 100, ]) + ->addRule('authSessionsLimit', [ + 'type' => self::TYPE_INTEGER, + 'description' => 'Max sessions allowed per user. 100 maximum.', + 'default' => 100, + 'example' => 10, + ]) ->addRule('providers', [ 'type' => Response::MODEL_PROVIDER, 'description' => 'List of Providers.', @@ -233,6 +239,7 @@ class Project extends Model $document->setAttribute('authLimit', $authValues['limit'] ?? 0); $document->setAttribute('authDuration', $authValues['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG); + $document->setAttribute('authSessionLimit', $authValues['maxSessions'] ?? APP_LIMIT_USER_SESSIONS); foreach ($auth as $index => $method) { $key = $method['key']; From a6afc41c99431c9d66b81192c3649b3b93fbc3a6 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 11 Dec 2022 07:15:47 +0000 Subject: [PATCH 09/16] fix: update test --- tests/e2e/Services/Projects/ProjectsConsoleClientTest.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 9501cc3af..57ccd6d89 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -905,6 +905,7 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); $this->assertNotEmpty($response['body']['$id']); + $this->assertNotEmpty($response['body']['$id']); $email = uniqid() . 'user@localhost.test'; $password = 'password'; @@ -924,7 +925,7 @@ class ProjectsConsoleClientTest extends Scope 'name' => $name, ]); - $this->assertEquals($response['headers']['status-code'], 501); + $this->assertEquals(201, $response['headers']['status-code']); /** * create new session @@ -939,7 +940,7 @@ class ProjectsConsoleClientTest extends Scope ]); - $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals(201, $response['headers']['status-code']); $sessionId1 = $response['body']['$id']; /** @@ -955,7 +956,7 @@ class ProjectsConsoleClientTest extends Scope ]); - $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals(201, $response['headers']['status-code']); $sessionCookie = $response['headers']['set-cookie']; $sessionId2 = $response['body']['$id']; From 3885e0452c08ee5810182b80085f170aba5fd80b Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 11 Dec 2022 08:26:31 +0000 Subject: [PATCH 10/16] fix: session limit test --- app/controllers/shared/api.php | 4 ++-- tests/e2e/Services/Projects/ProjectsConsoleClientTest.php | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index f3f41962e..0f156f9dd 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -329,7 +329,7 @@ App::shutdown() $route = $utopia->match($request); $event = $route->getLabel('event', ''); if ($event === 'users.[userId].sessions.[sessionId].create' && $project->getId() != 'console') { - $sessionLimit = $project->getAttribute('auth', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS; + $sessionLimit = $project->getAttribute('auths', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS; $session = $response->getPayload(); $userId = $session['userId'] ?? ''; if (empty($userId)) { @@ -348,7 +348,7 @@ App::shutdown() } for ($i = 0; $i < ($count - $sessionLimit); $i++) { - $session = array_pop($sessions); + $session = array_shift($sessions); $dbForProject->deleteDocument('sessions', $session->getId()); } $dbForProject->deleteCachedDocument('users', $userId); diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 57ccd6d89..9c4f693e2 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -960,6 +960,9 @@ class ProjectsConsoleClientTest extends Scope $sessionCookie = $response['headers']['set-cookie']; $sessionId2 = $response['body']['$id']; + // request was called in parallel and test failed + sleep(5); + /** * List sessions */ @@ -976,7 +979,6 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(1, count($sessions)); $this->assertEquals($sessionId2, $sessions[0]['$id']); - return $data; } From e0be09ded69eccc334bcb71bc51ed3993adc29d8 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Sun, 11 Dec 2022 08:33:52 +0000 Subject: [PATCH 11/16] refactor: use session group for session limit --- app/controllers/api/account.php | 10 +++---- app/controllers/shared/api.php | 50 +++++++++++++++------------------ 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 45fb03062..af5c8e4f5 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -140,7 +140,7 @@ App::post('/v1/account') App::post('/v1/account/sessions/email') ->alias('/v1/account/sessions') ->desc('Create Email Session') - ->groups(['api', 'account', 'auth']) + ->groups(['api', 'account', 'auth', 'session']) ->label('event', 'users.[userId].sessions.[sessionId].create') ->label('scope', 'public') ->label('auth.type', 'emailPassword') @@ -365,7 +365,7 @@ App::post('/v1/account/sessions/oauth2/callback/:provider/:projectId') App::get('/v1/account/sessions/oauth2/:provider/redirect') ->desc('OAuth2 Redirect') - ->groups(['api', 'account']) + ->groups(['api', 'account', 'session']) ->label('error', __DIR__ . '/../../views/general/error.phtml') ->label('event', 'users.[userId].sessions.[sessionId].create') ->label('scope', 'public') @@ -739,7 +739,7 @@ App::post('/v1/account/sessions/magic-url') App::put('/v1/account/sessions/magic-url') ->desc('Create Magic URL session (confirmation)') - ->groups(['api', 'account']) + ->groups(['api', 'account', 'session']) ->label('scope', 'public') ->label('event', 'users.[userId].sessions.[sessionId].create') ->label('audits.event', 'session.update') @@ -981,7 +981,7 @@ App::post('/v1/account/sessions/phone') App::put('/v1/account/sessions/phone') ->desc('Create Phone Session (confirmation)') - ->groups(['api', 'account']) + ->groups(['api', 'account', 'session']) ->label('scope', 'public') ->label('event', 'users.[userId].sessions.[sessionId].create') ->label('usage.metric', 'sessions.{scope}.requests.create') @@ -1096,7 +1096,7 @@ App::put('/v1/account/sessions/phone') App::post('/v1/account/sessions/anonymous') ->desc('Create Anonymous Session') - ->groups(['api', 'account', 'auth']) + ->groups(['api', 'account', 'auth', 'session']) ->label('event', 'users.[userId].sessions.[sessionId].create') ->label('scope', 'public') ->label('auth.type', 'anonymous') diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 0f156f9dd..dddf8cd8f 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -319,40 +319,36 @@ App::init() }); App::shutdown() - ->groups(['auth']) + ->groups(['session']) ->inject('utopia') ->inject('request') ->inject('response') ->inject('project') ->inject('dbForProject') ->action(function (App $utopia, Request $request, Response $response, Document $project, Database $dbForProject) { - $route = $utopia->match($request); - $event = $route->getLabel('event', ''); - if ($event === 'users.[userId].sessions.[sessionId].create' && $project->getId() != 'console') { - $sessionLimit = $project->getAttribute('auths', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS; - $session = $response->getPayload(); - $userId = $session['userId'] ?? ''; - if (empty($userId)) { - return; - } - - $user = $dbForProject->getDocument('users', $userId); - if ($user->isEmpty()) { - return; - } - - $sessions = $user->getAttribute('sessions', []); - $count = \count($sessions); - if ($count <= $sessionLimit) { - return; - } - - for ($i = 0; $i < ($count - $sessionLimit); $i++) { - $session = array_shift($sessions); - $dbForProject->deleteDocument('sessions', $session->getId()); - } - $dbForProject->deleteCachedDocument('users', $userId); + $sessionLimit = $project->getAttribute('auths', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS; + $session = $response->getPayload(); + $userId = $session['userId'] ?? ''; + if (empty($userId)) { + return; } + + $user = $dbForProject->getDocument('users', $userId); + if ($user->isEmpty()) { + return; + } + + $sessions = $user->getAttribute('sessions', []); + $count = \count($sessions); + if ($count <= $sessionLimit) { + return; + } + + for ($i = 0; $i < ($count - $sessionLimit); $i++) { + $session = array_shift($sessions); + $dbForProject->deleteDocument('sessions', $session->getId()); + } + $dbForProject->deleteCachedDocument('users', $userId); }); App::shutdown() From ca39ed37d87d851e25d379e8ae866a4aed94f988 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 12 Dec 2022 05:02:48 +0000 Subject: [PATCH 12/16] reset limit on test --- .../Services/Projects/ProjectsConsoleClientTest.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 9c4f693e2..889b73d61 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -979,6 +979,16 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(1, count($sessions)); $this->assertEquals($sessionId2, $sessions[0]['$id']); + /** + * Reset Limit + */ + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/max-sessions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'limit' => 10, + ]); + return $data; } From c60a3a66a087d44d5e281df7c2d704e5da2a9994 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 12 Dec 2022 05:07:18 +0000 Subject: [PATCH 13/16] feat: session limit separate default and max value --- app/controllers/api/projects.php | 4 ++-- app/init.php | 3 ++- src/Appwrite/Utopia/Response/Model/Project.php | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 3e5c6e78b..26def36fa 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -81,7 +81,7 @@ App::post('/v1/projects') } $auth = Config::getParam('auth', []); - $auths = ['limit' => 0, 'maxSessions' => APP_LIMIT_USER_SESSIONS, 'duration' => Auth::TOKEN_EXPIRATION_LOGIN_LONG]; + $auths = ['limit' => 0, 'maxSessions' => APP_LIMIT_USER_SESSIONS_DEFAULT, 'duration' => Auth::TOKEN_EXPIRATION_LOGIN_LONG]; foreach ($auth as $index => $method) { $auths[$method['key'] ?? ''] = true; } @@ -587,7 +587,7 @@ App::patch('/v1/projects/:projectId/auth/max-sessions') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_PROJECT) ->param('projectId', '', new UID(), 'Project unique ID.') - ->param('limit', false, new Range(1, APP_LIMIT_USER_SESSIONS), 'Set the max number of users allowed in this project.') + ->param('limit', false, new Range(1, APP_LIMIT_USER_SESSIONS_MAX), 'Set the max number of users allowed in this project. Value allowed is between 1-' . APP_LIMIT_USER_SESSIONS_MAX . '. Default is ' . APP_LIMIT_USER_SESSIONS_DEFAULT) ->inject('response') ->inject('dbForConsole') ->action(function (string $projectId, int $limit, Response $response, Database $dbForConsole) { diff --git a/app/init.php b/app/init.php index 6fc8839c7..2ab86e9b1 100644 --- a/app/init.php +++ b/app/init.php @@ -84,7 +84,8 @@ const APP_MODE_ADMIN = 'admin'; const APP_PAGING_LIMIT = 12; const APP_LIMIT_COUNT = 5000; const APP_LIMIT_USERS = 10000; -const APP_LIMIT_USER_SESSIONS = 100; +const APP_LIMIT_USER_SESSIONS_MAX = 100; +const APP_LIMIT_USER_SESSIONS_DEFAULT = 10; const APP_LIMIT_ANTIVIRUS = 20000000; //20MB const APP_LIMIT_ENCRYPTION = 20000000; //20MB const APP_LIMIT_COMPRESSION = 20000000; //20MB diff --git a/src/Appwrite/Utopia/Response/Model/Project.php b/src/Appwrite/Utopia/Response/Model/Project.php index dab80af3c..b1aff08e9 100644 --- a/src/Appwrite/Utopia/Response/Model/Project.php +++ b/src/Appwrite/Utopia/Response/Model/Project.php @@ -239,7 +239,7 @@ class Project extends Model $document->setAttribute('authLimit', $authValues['limit'] ?? 0); $document->setAttribute('authDuration', $authValues['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG); - $document->setAttribute('authSessionLimit', $authValues['maxSessions'] ?? APP_LIMIT_USER_SESSIONS); + $document->setAttribute('authSessionLimit', $authValues['maxSessions'] ?? APP_LIMIT_USER_SESSIONS_DEFAULT); foreach ($auth as $index => $method) { $key = $method['key']; From 2c46cdcc3cb3aab2a903c33c2f4e5f14d6500145 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Tue, 13 Dec 2022 06:08:38 +0000 Subject: [PATCH 14/16] add session default and comment --- app/controllers/shared/api.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index dddf8cd8f..227c3e7e2 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -318,6 +318,12 @@ App::init() } }); +/** + * Limit user session + * + * Delete older sessions if the number of sessions have crossed + * the session limit set for the project + */ App::shutdown() ->groups(['session']) ->inject('utopia') @@ -326,7 +332,7 @@ App::shutdown() ->inject('project') ->inject('dbForProject') ->action(function (App $utopia, Request $request, Response $response, Document $project, Database $dbForProject) { - $sessionLimit = $project->getAttribute('auths', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS; + $sessionLimit = $project->getAttribute('auths', [])['maxSessions'] ?? APP_LIMIT_USER_SESSIONS_DEFAULT; $session = $response->getPayload(); $userId = $session['userId'] ?? ''; if (empty($userId)) { From ae24689b89461d053138276f7e8edf56f02f69f6 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Tue, 13 Dec 2022 06:26:25 +0000 Subject: [PATCH 15/16] default auth session limit 10 --- src/Appwrite/Utopia/Response/Model/Project.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Appwrite/Utopia/Response/Model/Project.php b/src/Appwrite/Utopia/Response/Model/Project.php index b1aff08e9..28a9c86ff 100644 --- a/src/Appwrite/Utopia/Response/Model/Project.php +++ b/src/Appwrite/Utopia/Response/Model/Project.php @@ -117,7 +117,7 @@ class Project extends Model ->addRule('authSessionsLimit', [ 'type' => self::TYPE_INTEGER, 'description' => 'Max sessions allowed per user. 100 maximum.', - 'default' => 100, + 'default' => 10, 'example' => 10, ]) ->addRule('providers', [ From 255c4043d659e6451222cc638f539f4acae87555 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Tue, 13 Dec 2022 06:33:26 +0000 Subject: [PATCH 16/16] fix lint errors --- app/controllers/shared/api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index 227c3e7e2..7c224667b 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -320,7 +320,7 @@ App::init() /** * Limit user session - * + * * Delete older sessions if the number of sessions have crossed * the session limit set for the project */