diff --git a/app/config/collections2.php b/app/config/collections2.php index 4f407f3ee4..e99233c4a3 100644 --- a/app/config/collections2.php +++ b/app/config/collections2.php @@ -578,7 +578,7 @@ $collections = [ '$id' => 'email', 'type' => Database::VAR_STRING, 'format' => '', - 'size' => 1024, + 'size' => 320, 'signed' => true, 'required' => false, 'default' => null, @@ -695,15 +695,33 @@ $collections = [ 'array' => true, 'filters' => ['json'], ], + [ + '$id' => 'deleted', + 'type' => Database::VAR_BOOLEAN, + 'format' => '', + 'size' => 0, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], + ], ], 'indexes' => [ [ '$id' => '_key_email', 'type' => Database::INDEX_UNIQUE, 'attributes' => ['email'], - 'lengths' => [1024], + 'lengths' => [320], 'orders' => [Database::ORDER_ASC], ], + [ + '$id' => '_key_deleted_email', + 'type' => Database::INDEX_KEY, + 'attributes' => ['deleted', 'email'], + 'lengths' => [0, 320], + 'orders' => [Database::ORDER_ASC, Database::ORDER_ASC], + ], ], ], diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index bc34f81dda..3ad04efe81 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -78,7 +78,9 @@ App::post('/v1/account') $limit = $project->getAttribute('auths', [])['limit'] ?? 0; if ($limit !== 0) { - $sum = $dbForInternal->count('users', [], APP_LIMIT_USERS); + $sum = $dbForInternal->count('users', [ + new Query('deleted', Query::TYPE_EQUAL, [false]), + ], APP_LIMIT_USERS); if ($sum >= $limit) { throw new Exception('Project registration is restricted. Contact your administrator for more information.', 501); @@ -105,6 +107,7 @@ App::post('/v1/account') 'sessions' => [], 'tokens' => [], 'memberships' => [], + 'deleted' => false ])); } catch (Duplicate $th) { throw new Exception('Account already exists', 409); @@ -165,7 +168,7 @@ App::post('/v1/account/sessions') $email = \strtolower($email); $protocol = $request->getProtocol(); - $profile = $dbForInternal->findOne('users', [new Query('email', Query::TYPE_EQUAL, [$email])]); // Get user by email address + $profile = $dbForInternal->findOne('users', [new Query('deleted', Query::TYPE_EQUAL, [false]), new Query('email', Query::TYPE_EQUAL, [$email])]); // Get user by email address if (!$profile || !Auth::passwordVerify($password, $profile->getAttribute('password'))) { $audits @@ -462,13 +465,13 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $name = $oauth2->getUserName($accessToken); $email = $oauth2->getUserEmail($accessToken); - $user = $dbForInternal->findOne('users', [new Query('email', Query::TYPE_EQUAL, [$email])]); // Get user by email address + $user = $dbForInternal->findOne('users', [new Query('deleted', Query::TYPE_EQUAL, [false]), new Query('email', Query::TYPE_EQUAL, [$email])]); // Get user by email address if ($user === false || $user->isEmpty()) { // Last option -> create the user, generate random password $limit = $project->getAttribute('auths', [])['limit'] ?? 0; if ($limit !== 0) { - $sum = $dbForInternal->count('users', [], APP_LIMIT_COUNT); + $sum = $dbForInternal->count('users', [ new Query('deleted', Query::TYPE_EQUAL, [false]),], APP_LIMIT_COUNT); if ($sum >= $limit) { throw new Exception('Project registration is restricted. Contact your administrator for more information.', 501); @@ -495,6 +498,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'sessions' => [], 'tokens' => [], 'memberships' => [], + 'deleted' => false ])); } catch (Duplicate $th) { throw new Exception('Account already exists', 409); @@ -639,7 +643,9 @@ App::post('/v1/account/sessions/anonymous') $limit = $project->getAttribute('auths', [])['limit'] ?? 0; if ($limit !== 0) { - $sum = $dbForInternal->count('users', [], APP_LIMIT_COUNT); + $sum = $dbForInternal->count('users', [ + new Query('deleted', Query::TYPE_EQUAL, [false]), + ], APP_LIMIT_COUNT); if ($sum >= $limit) { throw new Exception('Project registration is restricted. Contact your administrator for more information.', 501); @@ -665,6 +671,7 @@ App::post('/v1/account/sessions/anonymous') 'sessions' => [], 'tokens' => [], 'memberships' => [], + 'deleted' => false ])); Authorization::reset(); @@ -1221,6 +1228,8 @@ App::delete('/v1/account') $protocol = $request->getProtocol(); $user = $dbForInternal->updateDocument('users', $user->getId(), $user->setAttribute('status', false)); + // TODO Seems to be related to users.php/App::delete('/v1/users/:userId'). Can we share code between these two? Do todos below apply to users.php? + // TODO delete all tokens or only current session? // TODO delete all user data according to GDPR. Make sure everything is backed up and backups are deleted later /* @@ -1463,7 +1472,7 @@ App::post('/v1/account/recovery') $isAppUser = Auth::isAppUser(Authorization::$roles); $email = \strtolower($email); - $profile = $dbForInternal->findOne('users', [new Query('email', Query::TYPE_EQUAL, [$email])]); // Get user by email address + $profile = $dbForInternal->findOne('users', [new Query('deleted', Query::TYPE_EQUAL, [false]), new Query('email', Query::TYPE_EQUAL, [$email])]); // Get user by email address if (!$profile) { throw new Exception('User not found', 404); @@ -1566,7 +1575,7 @@ App::put('/v1/account/recovery') $profile = $dbForInternal->getDocument('users', $userId); - if ($profile->isEmpty()) { + if ($profile->isEmpty() || $profile->getAttribute('deleted')) { throw new Exception('User not found', 404); } diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index c6c8207e21..86319ea5ec 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -66,6 +66,7 @@ App::post('/v1/users') 'sessions' => [], 'tokens' => [], 'memberships' => [], + 'deleted' => false ])); } catch (Duplicate $th) { throw new Exception('Account already exists', 409); @@ -106,13 +107,17 @@ App::get('/v1/users') if (!empty($after)) { $afterUser = $dbForInternal->getDocument('users', $after); - if ($afterUser->isEmpty()) { + if ($afterUser->isEmpty() || $afterUser->getAttribute('deleted')) { throw new Exception('User for after not found', 400); } } - $results = $dbForInternal->find('users', [], $limit, $offset, [], [$orderType], $afterUser ?? null); - $sum = $dbForInternal->count('users', [], APP_LIMIT_COUNT); + $results = $dbForInternal->find('users', [ + new Query('deleted', Query::TYPE_EQUAL, [false]), + ], $limit, $offset, [], [$orderType], $afterUser ?? null); + $sum = $dbForInternal->count('users', [ + new Query('deleted', Query::TYPE_EQUAL, [false]), + ], APP_LIMIT_COUNT); $usage ->setParam('users.read', 1) @@ -146,7 +151,7 @@ App::get('/v1/users/:userId') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -178,7 +183,7 @@ App::get('/v1/users/:userId/prefs') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -214,7 +219,7 @@ App::get('/v1/users/:userId/sessions') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -266,7 +271,7 @@ App::get('/v1/users/:userId/logs') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -374,7 +379,7 @@ App::patch('/v1/users/:userId/status') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -410,7 +415,7 @@ App::patch('/v1/users/:userId/verification') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -446,7 +451,7 @@ App::patch('/v1/users/:userId/name') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -485,7 +490,7 @@ App::patch('/v1/users/:userId/password') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -525,7 +530,7 @@ App::patch('/v1/users/:userId/email') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -569,7 +574,7 @@ App::patch('/v1/users/:userId/prefs') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -606,7 +611,7 @@ App::delete('/v1/users/:userId/sessions/:sessionId') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -661,7 +666,7 @@ App::delete('/v1/users/:userId/sessions') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } @@ -710,26 +715,35 @@ App::delete('/v1/users/:userId') $user = $dbForInternal->getDocument('users', $userId); - if ($user->isEmpty()) { + if ($user->isEmpty() || $user->getAttribute('deleted')) { throw new Exception('User not found', 404); } - if (!$dbForInternal->deleteDocument('users', $userId)) { - throw new Exception('Failed to remove user from DB', 500); - } + // clone user object to send to workers + $clone = clone $user; + + $user + ->setAttribute("name", null) + ->setAttribute("email", null) + ->setAttribute("password", null) + ->setAttribute("deleted", true) + ; + + $dbForInternal->updateDocument('users', $userId, $user); $deletes ->setParam('type', DELETE_TYPE_DOCUMENT) - ->setParam('document', $user) + ->setParam('document', $clone) ; $events - ->setParam('eventData', $response->output($user, Response::MODEL_USER)) + ->setParam('eventData', $response->output($clone, Response::MODEL_USER)) ; $usage ->setParam('users.delete', 1) ; + $response->noContent(); }); diff --git a/docker-compose.yml b/docker-compose.yml index 539799aa9a..ade61dc1f8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -63,7 +63,7 @@ services: - ./psalm.xml:/usr/src/code/psalm.xml - ./tests:/usr/src/code/tests - ./app:/usr/src/code/app - # - ./vendor/utopia-php/database:/usr/src/code/vendor/utopia-php/database + # - ./vendor:/usr/src/code/vendor - ./docs:/usr/src/code/docs - ./public:/usr/src/code/public - ./src:/usr/src/code/src diff --git a/tests/e2e/Services/Users/UsersCustomServerTest.php b/tests/e2e/Services/Users/UsersCustomServerTest.php index c5e4ff8c1a..cee654e6c3 100644 --- a/tests/e2e/Services/Users/UsersCustomServerTest.php +++ b/tests/e2e/Services/Users/UsersCustomServerTest.php @@ -2,6 +2,7 @@ namespace Tests\E2E\Services\Users; +use Tests\E2E\Client; use Tests\E2E\Scopes\ProjectCustom; use Tests\E2E\Scopes\Scope; use Tests\E2E\Scopes\SideServer; @@ -11,4 +12,49 @@ class UsersCustomServerTest extends Scope use UsersBase; use ProjectCustom; use SideServer; + + public function testDeprecatedUsers():array + { + /** + * Test for FAILURE (don't allow recreating account with same custom ID) + */ + + // Create user with custom ID 'meldiron' + $response = $this->client->call(Client::METHOD_POST, '/users', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'userId' => 'meldiron', + 'email' => 'matej@appwrite.io', + 'password' => 'my-superstr0ng-password', + 'name' => 'Matej Bačo' + ]); + + $this->assertEquals(201, $response['headers']['status-code']); + + // Delete user with custom ID 'meldiron' + $response = $this->client->call(Client::METHOD_DELETE, '/users/meldiron', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(204, $response['headers']['status-code']); + + // Try to create user with custom ID 'meldiron' again, but now it should fail + $response1 = $this->client->call(Client::METHOD_POST, '/users', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'userId' => 'meldiron', + 'email' => 'matej2@appwrite.io', + 'password' => 'someones-superstr0ng-password', + 'name' => 'Matej Bačo Second' + ]); + + $this->assertEquals(409, $response1['headers']['status-code']); + $this->assertEquals('Account already exists', $response1['body']['message']); + + return []; + } + } \ No newline at end of file