diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index c07651b75a..a7f9bac4ed 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -240,7 +240,7 @@ App::post('/v1/account/sessions/email') $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); - $secret = Auth::tokenGenerator(); + $secret = Auth::tokenGenerator(Auth::TOKEN_LENGTH_SESSION); $session = new Document(array_merge( [ '$id' => ID::unique(), @@ -814,7 +814,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') } else { $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); - $secret = Auth::tokenGenerator(); + $secret = Auth::tokenGenerator(Auth::TOKEN_LENGTH_SESSION); $session = new Document(array_merge([ '$id' => ID::unique(), @@ -1228,7 +1228,7 @@ $createSession = function (string $userId, string $secret, Request $request, Res $duration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); - $sessionSecret = Auth::tokenGenerator(); + $sessionSecret = Auth::tokenGenerator(Auth::TOKEN_LENGTH_SESSION); $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $session = new Document(array_merge( @@ -1615,7 +1615,7 @@ App::post('/v1/account/sessions/anonymous') $duration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); - $secret = Auth::tokenGenerator(); + $secret = Auth::tokenGenerator(Auth::TOKEN_LENGTH_SESSION); $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $session = new Document(array_merge( @@ -2606,7 +2606,7 @@ App::post('/v1/account/recovery') $expire = DateTime::addSeconds(new \DateTime(), Auth::TOKEN_EXPIRATION_RECOVERY); - $secret = Auth::tokenGenerator(); + $secret = Auth::tokenGenerator(Auth::TOKEN_LENGTH_RECOVERY); $recovery = new Document([ '$id' => ID::unique(), 'userId' => $profile->getId(), @@ -2850,7 +2850,7 @@ App::post('/v1/account/verification') $roles = Authorization::getRoles(); $isPrivilegedUser = Auth::isPrivilegedUser($roles); $isAppUser = Auth::isAppUser($roles); - $verificationSecret = Auth::tokenGenerator(); + $verificationSecret = Auth::tokenGenerator(Auth::TOKEN_LENGTH_VERIFICATION); $expire = DateTime::addSeconds(new \DateTime(), Auth::TOKEN_EXPIRATION_CONFIRM); $verification = new Document([ diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 2ba27efcb3..a7e05507e2 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -474,6 +474,7 @@ App::post('/v1/teams/:teamId/memberships') 'phone' => empty($phone) ? null : $phone, 'emailVerification' => false, 'status' => true, + // TODO: Set password empty? 'password' => Auth::passwordHash(Auth::passwordGenerator(), Auth::DEFAULT_ALGO, Auth::DEFAULT_ALGO_OPTIONS), 'hash' => Auth::DEFAULT_ALGO, 'hashOptions' => Auth::DEFAULT_ALGO_OPTIONS, diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 8653f1df4a..e090b32300 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -1447,58 +1447,8 @@ App::post('/v1/users/:userId/sessions') ->inject('queueForEvents') ->action(function (string $userId, Request $request, Response $response, Database $dbForProject, Document $project, Locale $locale, Reader $geodb, Event $queueForEvents) { $user = $dbForProject->getDocument('users', $userId); - - if ($user !== false && !$user->isEmpty()) { - $user->setAttributes($user->getArrayCopy()); - } else { - $limit = $project->getAttribute('auths', [])['limit'] ?? 0; - - if ($limit !== 0) { - $total = $dbForProject->count('users', max: APP_LIMIT_USERS); - - if ($total >= $limit) { - throw new Exception(Exception::USER_COUNT_EXCEEDED); - } - } - - if ($userId !== 'unique()') { - $existingUser = $dbForProject->findOne('users', [ - Query::equal('$id', [$userId]), - ]); - - if ($existingUser !== false && !$existingUser->isEmpty()) { - throw new Exception(Exception::USER_ALREADY_EXISTS); - } - } - - $userId = $userId === 'unique()' ? ID::unique() : $userId; - - $user->setAttributes([ - '$id' => $userId, - '$permissions' => [ - Permission::read(Role::any()), - Permission::update(Role::user($userId)), - Permission::delete(Role::user($userId)), - ], - 'email' => null, - 'emailVerification' => false, - 'status' => true, - 'password' => null, - 'hash' => Auth::DEFAULT_ALGO, - 'hashOptions' => Auth::DEFAULT_ALGO_OPTIONS, - 'passwordUpdate' => null, - 'registration' => DateTime::now(), - 'reset' => false, - 'prefs' => new \stdClass(), - 'sessions' => null, - 'tokens' => null, - 'memberships' => null, - 'search' => implode(' ', [$userId]), - 'accessedAt' => DateTime::now(), - ]); - - $user->removeAttribute('$internalId'); - Authorization::skip(fn () => $dbForProject->createDocument('users', $user)); + if ($user === false || $user->isEmpty()) { + throw new Exception(Exception::USER_NOT_FOUND); } $secret = Auth::codeGenerator(); @@ -1557,69 +1507,18 @@ App::post('/v1/users/:userId/tokens') ->label('sdk.response.code', Response::STATUS_CODE_CREATED) ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_TOKEN) - ->param('userId', '', new CustomId(), 'User ID. Choose a custom ID or generate a random ID with `ID.unique()`. Valid chars are a-z, A-Z, 0-9, period, hyphen, and underscore. Can\'t start with a special char. Max length is 36 chars.') + ->param('userId', '', new UID(), 'User ID.') ->param('length', 6, new Range(4, 128), 'Token length in characters. The default length is 6 characters', true) ->param('expire', Auth::TOKEN_EXPIRATION_GENERIC, new Range(60, Auth::TOKEN_EXPIRATION_LOGIN_LONG), 'Token expiration period in seconds. The default expiration is 15 minutes.', true) ->inject('request') ->inject('response') - ->inject('project') ->inject('dbForProject') ->inject('queueForEvents') - ->action(function (string $userId, int $length, int $expire, Request $request, Response $response, Document $project, Database $dbForProject, Event $queueForEvents) { + ->action(function (string $userId, int $length, int $expire, Request $request, Response $response, Database $dbForProject, Event $queueForEvents) { $user = $dbForProject->getDocument('users', $userId); - if ($user !== false && !$user->isEmpty()) { - $user->setAttributes($user->getArrayCopy()); - } else { - $limit = $project->getAttribute('auths', [])['limit'] ?? 0; - - if ($limit !== 0) { - $total = $dbForProject->count('users', max: APP_LIMIT_USERS); - - if ($total >= $limit) { - throw new Exception(Exception::USER_COUNT_EXCEEDED); - } - } - - if ($userId !== 'unique()') { - $existingUser = $dbForProject->findOne('users', [ - Query::equal('$id', [$userId]), - ]); - - if ($existingUser !== false && !$existingUser->isEmpty()) { - throw new Exception(Exception::USER_ALREADY_EXISTS); - } - } - - $userId = $userId === 'unique()' ? ID::unique() : $userId; - - - $user->setAttributes([ - '$id' => $userId, - '$permissions' => [ - Permission::read(Role::any()), - Permission::update(Role::user($userId)), - Permission::delete(Role::user($userId)), - ], - 'email' => null, - 'emailVerification' => false, - 'status' => true, - 'password' => null, - 'hash' => Auth::DEFAULT_ALGO, - 'hashOptions' => Auth::DEFAULT_ALGO_OPTIONS, - 'passwordUpdate' => null, - 'registration' => DateTime::now(), - 'reset' => false, - 'prefs' => new \stdClass(), - 'sessions' => null, - 'tokens' => null, - 'memberships' => null, - 'search' => implode(' ', [$userId]), - 'accessedAt' => DateTime::now(), - ]); - - $user->removeAttribute('$internalId'); - Authorization::skip(fn () => $dbForProject->createDocument('users', $user)); + if ($user === false || $user->isEmpty()) { + throw new Exception(Exception::USER_NOT_FOUND); } $secret = Auth::tokenGenerator($length); diff --git a/src/Appwrite/Auth/Auth.php b/src/Appwrite/Auth/Auth.php index 5ee89c6ac3..ba3eea7625 100644 --- a/src/Appwrite/Auth/Auth.php +++ b/src/Appwrite/Auth/Auth.php @@ -79,10 +79,11 @@ class Auth /** * Token Lengths. */ - public const TOKEN_LENGTH_MAGIC_URL = 32; - public const TOKEN_LENGTH_OAUTH2 = 32; - public const TOKEN_LENGTH_SESSION = 128; - + public const TOKEN_LENGTH_MAGIC_URL = 64; + public const TOKEN_LENGTH_VERIFICATION = 64; + public const TOKEN_LENGTH_RECOVERY = 64; + public const TOKEN_LENGTH_OAUTH2 = 64; + public const TOKEN_LENGTH_SESSION = 256; /** * @var string @@ -305,13 +306,20 @@ class Auth * * Generate random password string * - * @param int $length + * @param int $length Length of returned token * * @return string */ - public static function tokenGenerator(int $length = Auth::TOKEN_LENGTH_SESSION): string + public static function tokenGenerator(int $length = 256): string { - return \bin2hex(\random_bytes($length)); + if ($length <= 0) { + throw new \Exception('Token length must be greater than 0'); + } + + $bytesLength = (int) ceil($length / 2); + $token = \bin2hex(\random_bytes($bytesLength)); + + return substr($token, 0, $length); } /** diff --git a/tests/e2e/Services/Users/UsersBase.php b/tests/e2e/Services/Users/UsersBase.php index 78f705660e..ddd6e22470 100644 --- a/tests/e2e/Services/Users/UsersBase.php +++ b/tests/e2e/Services/Users/UsersBase.php @@ -239,15 +239,53 @@ trait UsersBase * Test for SUCCESS */ $token = $this->client->call(Client::METHOD_POST, '/users/' . $data['userId'] . '/tokens', array_merge([ + 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'expire' => 60, - ]); + ], $this->getHeaders())); $this->assertEquals(201, $token['headers']['status-code']); $this->assertEquals($data['userId'], $token['body']['userId']); $this->assertNotEmpty($token['body']['secret']); $this->assertNotEmpty($token['body']['expire']); + + $token = $this->client->call(Client::METHOD_POST, '/users/' . $data['userId'] . '/tokens', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'length' => 15, + 'expire' => 60, + ]); + + $this->assertEquals(201, $token['headers']['status-code']); + $this->assertEquals($data['userId'], $token['body']['userId']); + $this->assertEquals(15, strlen($token['body']['secret'])); + $this->assertNotEmpty($token['body']['expire']); + + /** + * Test for FAILURE + */ + $token = $this->client->call(Client::METHOD_POST, '/users/' . $data['userId'] . '/tokens', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'length' => 1, + 'expire' => 1, + ]); + + $this->assertEquals(400, $token['headers']['status-code']); + $this->assertArrayNotHasKey('userId', $token['body']); + $this->assertArrayNotHasKey('secret', $token['body']); + + $token = $this->client->call(Client::METHOD_POST, '/users/' . $data['userId'] . '/tokens', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'expire' => 999999999999999, + ]); + + $this->assertEquals(400, $token['headers']['status-code']); + $this->assertArrayNotHasKey('userId', $token['body']); + $this->assertArrayNotHasKey('secret', $token['body']); } diff --git a/tests/unit/Auth/AuthTest.php b/tests/unit/Auth/AuthTest.php index ec3e5569d0..4c9ebe6bf2 100644 --- a/tests/unit/Auth/AuthTest.php +++ b/tests/unit/Auth/AuthTest.php @@ -189,8 +189,8 @@ class AuthTest extends TestCase public function testTokenGenerator(): void { - $this->assertEquals(\mb_strlen(Auth::tokenGenerator()), 256); - $this->assertEquals(\mb_strlen(Auth::tokenGenerator(5)), 10); + $this->assertEquals(\strlen(Auth::tokenGenerator()), 128); + $this->assertEquals(\strlen(Auth::tokenGenerator(5)), 5); } public function testCodeGenerator(): void