From 788fa04606c8d202257ee1856c279199fc6b66a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Wed, 10 Jan 2024 11:33:55 +0100 Subject: [PATCH 01/22] Improve magic URL design --- app/config/locale/translations/en.json | 2 +- app/controllers/api/account.php | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/config/locale/translations/en.json b/app/config/locale/translations/en.json index 60a6bacd2..3c355ffca 100644 --- a/app/config/locale/translations/en.json +++ b/app/config/locale/translations/en.json @@ -11,7 +11,7 @@ "emails.verification.signature": "{{project}} team", "emails.magicSession.subject": "{{project}} Login", "emails.magicSession.hello": "Hello,", - "emails.magicSession.optionButton": "Click the button below to securely sign in to your {{project}} account. It will expire in 1 hour.", + "emails.magicSession.optionButton": "Click the button below to securely sign in to your {{projectBold}} account. It will expire in 1 hour.", "emails.magicSession.buttonText": "Sign in to {{project}}", "emails.magicSession.optionUrl": "If the button above doesn't show, use the following link:", "emails.magicSession.clientInfo": "This sign in was requested using {{agentClient}} on {{agentDevice}} {{agentOs}}. If you didn't request the sign in, you can safely ignore this email.", diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 8364063f1..1784bafc1 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1035,7 +1035,6 @@ App::post('/v1/account/sessions/magic-url') $url['query'] = Template::mergeQuery(((isset($url['query'])) ? $url['query'] : ''), ['userId' => $user->getId(), 'secret' => $loginSecret, 'expire' => $expire, 'project' => $project->getId()]); $url = Template::unParseURL($url); - $body = $locale->getText("emails.magicSession.body"); $subject = $locale->getText("emails.magicSession.subject"); $customTemplate = $project->getAttribute('templates', [])['email.magicSession-' . $locale->default] ?? []; @@ -1046,7 +1045,6 @@ App::post('/v1/account/sessions/magic-url') $message = Template::fromFile(__DIR__ . '/../../config/locale/templates/email-magic-url.tpl'); $message - ->setParam('{{body}}', $body) ->setParam('{{hello}}', $locale->getText("emails.magicSession.hello")) ->setParam('{{optionButton}}', $locale->getText("emails.magicSession.optionButton")) ->setParam('{{buttonText}}', $locale->getText("emails.magicSession.buttonText")) @@ -1108,10 +1106,11 @@ App::post('/v1/account/sessions/magic-url') 'user' => '', 'team' => '', 'project' => $project->getAttribute('name'), + 'projectBold' => '' . $project->getAttribute('name') . '', 'redirect' => $url, - 'agentDevice' => $agentDevice['deviceBrand'] ?? $agentDevice['deviceBrand'] ?? 'UNKNOWN', - 'agentClient' => $agentClient['clientName'] ?? 'UNKNOWN', - 'agentOs' => $agentOs['osName'] ?? 'UNKNOWN' + 'agentDevice' => '' . ( $agentDevice['deviceBrand'] ?? $agentDevice['deviceBrand'] ?? 'UNKNOWN') . '', + 'agentClient' => '' . ($agentClient['clientName'] ?? 'UNKNOWN') . '', + 'agentOs' => '' . ($agentOs['osName'] ?? 'UNKNOWN') . '' ]; $queueForMails From 4e73c7653857606c3f53b53a3cb3b858c61a4438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Wed, 10 Jan 2024 13:28:49 +0000 Subject: [PATCH 02/22] Add OTP email template --- app/config/locale/templates/email-otp.tpl | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 app/config/locale/templates/email-otp.tpl diff --git a/app/config/locale/templates/email-otp.tpl b/app/config/locale/templates/email-otp.tpl new file mode 100644 index 000000000..e4c9bd226 --- /dev/null +++ b/app/config/locale/templates/email-otp.tpl @@ -0,0 +1,10 @@ +

{{hello}}

+ +

{{optionOtp}}

+ +

{{otp}}

+ +

{{clientInfo}}

+ +

{{thanks}}

+

{{signature}}

\ No newline at end of file From 4f5755e7d09ef369d1e8b330be130932159338a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 15 Jan 2024 13:43:21 +0000 Subject: [PATCH 03/22] Implement session renewal --- app/config/collections.php | 11 ++ app/controllers/api/account.php | 133 +++++++++++----------- app/controllers/api/teams.php | 1 + docs/references/account/update-session.md | 2 +- src/Appwrite/Auth/Auth.php | 2 +- 5 files changed, 80 insertions(+), 69 deletions(-) diff --git a/app/config/collections.php b/app/config/collections.php index b3e341755..18bf22e14 100644 --- a/app/config/collections.php +++ b/app/config/collections.php @@ -681,6 +681,17 @@ $commonCollections = [ 'array' => false, 'filters' => [], ], + [ + '$id' => ID::custom('expire'), + 'type' => Database::VAR_DATETIME, + 'format' => '', + 'size' => 0, + 'signed' => false, + 'required' => true, + 'default' => null, + 'array' => false, + 'filters' => ['datetime'], + ], ], 'indexes' => [ [ diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 38742bffd..a861f3151 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -232,7 +232,6 @@ App::post('/v1/account/sessions/email') $user->setAttributes($profile->getArrayCopy()); $duration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; - $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); @@ -248,6 +247,7 @@ App::post('/v1/account/sessions/email') 'userAgent' => $request->getUserAgent('UNKNOWN'), 'ip' => $request->getIP(), 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', + 'expire' => DateTime::addSeconds(new \DateTime(), $duration) ], $detector->getOS(), $detector->getClient(), @@ -291,7 +291,6 @@ App::post('/v1/account/sessions/email') $session ->setAttribute('current', true) ->setAttribute('countryName', $countryName) - ->setAttribute('expire', $expire) ; $queueForEvents @@ -753,7 +752,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $record = $geodb->get($request->getIP()); $secret = Auth::tokenGenerator(); $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); - $session = new Document(array_merge([ '$id' => ID::unique(), 'userId' => $user->getId(), @@ -767,6 +765,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'userAgent' => $request->getUserAgent('UNKNOWN'), 'ip' => $request->getIP(), 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', + 'expire' => DateTime::addSeconds(new \DateTime(), $duration) ], $detector->getOS(), $detector->getClient(), $detector->getDevice())); if (empty($user->getAttribute('email'))) { @@ -793,8 +792,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $dbForProject->deleteCachedDocument('users', $user->getId()); - $session->setAttribute('expire', $expire); - $queueForEvents ->setParam('userId', $user->getId()) ->setParam('sessionId', $session->getId()) @@ -1208,7 +1205,6 @@ App::put('/v1/account/sessions/magic-url') $record = $geodb->get($request->getIP()); $secret = Auth::tokenGenerator(); $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); - $session = new Document(array_merge( [ '$id' => ID::unique(), @@ -1219,6 +1215,7 @@ App::put('/v1/account/sessions/magic-url') 'userAgent' => $request->getUserAgent('UNKNOWN'), 'ip' => $request->getIP(), 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', + 'expire' => DateTime::addSeconds(new \DateTime(), $duration) ], $detector->getOS(), $detector->getClient(), @@ -1272,8 +1269,7 @@ App::put('/v1/account/sessions/magic-url') $session ->setAttribute('current', true) - ->setAttribute('countryName', $countryName) - ->setAttribute('expire', $expire); + ->setAttribute('countryName', $countryName); $response->dynamic($session, Response::MODEL_SESSION); }); @@ -1488,7 +1484,6 @@ App::put('/v1/account/sessions/phone') $record = $geodb->get($request->getIP()); $secret = Auth::tokenGenerator(); $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); - $session = new Document(array_merge( [ '$id' => ID::unique(), @@ -1499,6 +1494,7 @@ App::put('/v1/account/sessions/phone') 'userAgent' => $request->getUserAgent('UNKNOWN'), 'ip' => $request->getIP(), 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', + 'expire' => DateTime::addSeconds(new \DateTime(), $duration) ], $detector->getOS(), $detector->getClient(), @@ -1553,7 +1549,6 @@ App::put('/v1/account/sessions/phone') $session ->setAttribute('current', true) ->setAttribute('countryName', $countryName) - ->setAttribute('expire', $expire) ; $response->dynamic($session, Response::MODEL_SESSION); @@ -1654,6 +1649,7 @@ App::post('/v1/account/sessions/anonymous') 'userAgent' => $request->getUserAgent('UNKNOWN'), 'ip' => $request->getIP(), 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', + 'expire' => DateTime::addSeconds(new \DateTime(), $duration) ], $detector->getOS(), $detector->getClient(), @@ -1690,7 +1686,6 @@ App::post('/v1/account/sessions/anonymous') $session ->setAttribute('current', true) ->setAttribute('countryName', $countryName) - ->setAttribute('expire', $expire) ; $response->dynamic($session, Response::MODEL_SESSION); @@ -1890,8 +1885,6 @@ App::get('/v1/account/sessions') $session->setAttribute('countryName', $countryName); $session->setAttribute('current', ($current == $session->getId()) ? true : false); - $session->setAttribute('expire', DateTime::formatTz(DateTime::addSeconds(new \DateTime($session->getCreatedAt()), $authDuration))); - $sessions[$key] = $session; } @@ -1997,7 +1990,6 @@ App::get('/v1/account/sessions/:sessionId') $session ->setAttribute('current', ($session->getAttribute('secret') == Auth::hash(Auth::$secret))) ->setAttribute('countryName', $countryName) - ->setAttribute('expire', DateTime::formatTz(DateTime::addSeconds(new \DateTime($session->getCreatedAt()), $authDuration))) ; return $response->dynamic($session, Response::MODEL_SESSION); @@ -2429,7 +2421,7 @@ App::delete('/v1/account/sessions/:sessionId') }); App::patch('/v1/account/sessions/:sessionId') - ->desc('Update OAuth session (refresh tokens)') + ->desc('Renew session') ->groups(['api', 'account']) ->label('scope', 'account') ->label('event', 'users.[userId].sessions.[sessionId].update') @@ -2446,72 +2438,80 @@ App::patch('/v1/account/sessions/:sessionId') ->label('sdk.response.model', Response::MODEL_SESSION) ->label('abuse-limit', 10) ->param('sessionId', '', new UID(), 'Session ID. Use the string \'current\' to update the current device session.') - ->inject('request') + ->param('identity', false, new Boolean(), 'Refresh OAuth2 identity. If enabled and session was created using an OAuth provider, the access token of the identity will be refreshed', true) ->inject('response') ->inject('user') ->inject('dbForProject') ->inject('project') - ->inject('locale') ->inject('queueForEvents') - ->action(function (?string $sessionId, Request $request, Response $response, Document $user, Database $dbForProject, Document $project, Locale $locale, Event $queueForEvents) { + ->action(function (?string $sessionId, bool $identity, Response $response, Document $user, Database $dbForProject, Document $project, Event $queueForEvents) { + $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $sessionId = ($sessionId === 'current') ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret, $authDuration) : $sessionId; - $sessions = $user->getAttribute('sessions', []); + $session = null; foreach ($sessions as $key => $session) {/** @var Document $session */ - if ($sessionId == $session->getId()) { - // Comment below would skip re-generation if token is still valid - // We decided to not include this because developer can get expiration date from the session - // I kept code in comment because it might become relevant in the future - - // $expireAt = (int) $session->getAttribute('providerAccessTokenExpiry'); - // if(\time() < $expireAt - 5) { // 5 seconds time-sync and networking gap, to be safe - // return $response->noContent(); - // } - - $provider = $session->getAttribute('provider'); - $refreshToken = $session->getAttribute('providerRefreshToken'); - - $appId = $project->getAttribute('oAuthProviders', [])[$provider . 'Appid'] ?? ''; - $appSecret = $project->getAttribute('oAuthProviders', [])[$provider . 'Secret'] ?? '{}'; - - $className = 'Appwrite\\Auth\\OAuth2\\' . \ucfirst($provider); - - if (!\class_exists($className)) { - throw new Exception(Exception::PROJECT_PROVIDER_UNSUPPORTED); - } - - $oauth2 = new $className($appId, $appSecret, '', [], []); - - $oauth2->refreshTokens($refreshToken); - - $session - ->setAttribute('providerAccessToken', $oauth2->getAccessToken('')) - ->setAttribute('providerRefreshToken', $oauth2->getRefreshToken('')) - ->setAttribute('providerAccessTokenExpiry', DateTime::addSeconds(new \DateTime(), (int)$oauth2->getAccessTokenExpiry(''))); - - $dbForProject->updateDocument('sessions', $sessionId, $session); - - $dbForProject->deleteCachedDocument('users', $user->getId()); - - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; - - $session->setAttribute('expire', DateTime::formatTz(DateTime::addSeconds(new \DateTime($session->getCreatedAt()), $authDuration))); - - $queueForEvents - ->setParam('userId', $user->getId()) - ->setParam('sessionId', $session->getId()) - ->setPayload($response->output($session, Response::MODEL_SESSION)) - ; - - return $response->dynamic($session, Response::MODEL_SESSION); + if ($sessionId === $session->getId()) { + $session = $session; + break; } } - throw new Exception(Exception::USER_SESSION_NOT_FOUND); + if ($session === null) { + throw new Exception(Exception::USER_SESSION_NOT_FOUND); + } + + // Extend session + $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; + $session->setAttribute('expire', DateTime::addSeconds(new \DateTime(), $authDuration)); + + // Refresh OAuth access token + if ($identity) { + // Comment below would skip re-generation if token is still valid + // We decided to not include this because developer can get expiration date from the session + // I kept code in comment because it might become relevant in the future + + // $expireAt = (int) $session->getAttribute('providerAccessTokenExpiry'); + // if(\time() < $expireAt - 5) { // 5 seconds time-sync and networking gap, to be safe + // return $response->noContent(); + // } + + $provider = $session->getAttribute('provider'); + $refreshToken = $session->getAttribute('providerRefreshToken'); + + $appId = $project->getAttribute('oAuthProviders', [])[$provider . 'Appid'] ?? ''; + $appSecret = $project->getAttribute('oAuthProviders', [])[$provider . 'Secret'] ?? '{}'; + + $className = 'Appwrite\\Auth\\OAuth2\\' . \ucfirst($provider); + + if (!\class_exists($className)) { + throw new Exception(Exception::PROJECT_PROVIDER_UNSUPPORTED); + } + + $oauth2 = new $className($appId, $appSecret, '', [], []); + + $oauth2->refreshTokens($refreshToken); + + $session + ->setAttribute('providerAccessToken', $oauth2->getAccessToken('')) + ->setAttribute('providerRefreshToken', $oauth2->getRefreshToken('')) + ->setAttribute('providerAccessTokenExpiry', DateTime::addSeconds(new \DateTime(), (int)$oauth2->getAccessTokenExpiry(''))); + } + + $dbForProject->updateDocument('sessions', $sessionId, $session); + + $dbForProject->deleteCachedDocument('users', $user->getId()); + + $queueForEvents + ->setParam('userId', $user->getId()) + ->setParam('sessionId', $session->getId()) + ->setPayload($response->output($session, Response::MODEL_SESSION)) + ; + + return $response->dynamic($session, Response::MODEL_SESSION); }); App::delete('/v1/account/sessions') @@ -2554,7 +2554,6 @@ App::delete('/v1/account/sessions') if ($session->getAttribute('secret') == Auth::hash(Auth::$secret)) { $session->setAttribute('current', true); - $session->setAttribute('expire', DateTime::addSeconds(new \DateTime($session->getCreatedAt()), Auth::TOKEN_EXPIRATION_LOGIN_LONG)); // If current session delete the cookies too $response diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 2ba27efcb..8e6ddb327 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -962,6 +962,7 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId/status') 'userAgent' => $request->getUserAgent('UNKNOWN'), 'ip' => $request->getIP(), 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', + 'expire' => DateTime::addSeconds(new \DateTime(), $authDuration) ], $detector->getOS(), $detector->getClient(), $detector->getDevice())); $session = $dbForProject->createDocument('sessions', $session diff --git a/docs/references/account/update-session.md b/docs/references/account/update-session.md index 98080fda5..55226fe0b 100644 --- a/docs/references/account/update-session.md +++ b/docs/references/account/update-session.md @@ -1 +1 @@ -Access tokens have limited lifespan and expire to mitigate security risks. If session was created using an OAuth provider, this route can be used to "refresh" the access token. \ No newline at end of file +Extend session's expiry to increase it's lifespan. Extending a session is useful when session length is short such as 5 minutes. \ No newline at end of file diff --git a/src/Appwrite/Auth/Auth.php b/src/Appwrite/Auth/Auth.php index 31448c4df..5675d1a5f 100644 --- a/src/Appwrite/Auth/Auth.php +++ b/src/Appwrite/Auth/Auth.php @@ -363,7 +363,7 @@ class Auth $session->isSet('secret') && $session->isSet('provider') && $session->getAttribute('secret') === self::hash($secret) && - DateTime::formatTz(DateTime::addSeconds(new \DateTime($session->getCreatedAt()), $expires)) >= DateTime::formatTz(DateTime::now()) + DateTime::formatTz(DateTime::addSeconds(new \DateTime($session->getAttribute('expire')), $expires)) >= DateTime::formatTz(DateTime::now()) ) { return $session->getId(); } From 39997c817f462e92b5dd50966e5fa8161f852c03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 15 Jan 2024 14:37:47 +0000 Subject: [PATCH 04/22] Fix failing tests --- app/controllers/api/account.php | 14 +++++--------- app/init.php | 4 +--- app/realtime.php | 3 +-- src/Appwrite/Auth/Auth.php | 5 ++--- .../Services/Account/AccountCustomClientTest.php | 4 +++- .../Projects/ProjectsConsoleClientTest.php | 2 +- 6 files changed, 13 insertions(+), 19 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index a861f3151..e95719329 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -584,8 +584,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') } $sessions = $user->getAttribute('sessions', []); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; - $current = Auth::sessionVerify($sessions, Auth::$secret, $authDuration); + $current = Auth::sessionVerify($sessions, Auth::$secret); if ($current) { // Delete current session of new one. $currentDocument = $dbForProject->getDocument('sessions', $current); @@ -1877,8 +1876,7 @@ App::get('/v1/account/sessions') ->action(function (Response $response, Document $user, Locale $locale, Document $project) { $sessions = $user->getAttribute('sessions', []); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; - $current = Auth::sessionVerify($sessions, Auth::$secret, $authDuration); + $current = Auth::sessionVerify($sessions, Auth::$secret); foreach ($sessions as $key => $session) {/** @var Document $session */ $countryName = $locale->getText('countries.' . strtolower($session->getAttribute('countryCode')), $locale->getText('locale.country.unknown')); @@ -1978,9 +1976,8 @@ App::get('/v1/account/sessions/:sessionId') ->action(function (?string $sessionId, Response $response, Document $user, Locale $locale, Database $dbForProject, Document $project) { $sessions = $user->getAttribute('sessions', []); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $sessionId = ($sessionId === 'current') - ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret, $authDuration) + ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret) : $sessionId; foreach ($sessions as $session) {/** @var Document $session */ @@ -2371,9 +2368,8 @@ App::delete('/v1/account/sessions/:sessionId') ->action(function (?string $sessionId, ?\DateTime $requestTimestamp, Request $request, Response $response, Document $user, Database $dbForProject, Locale $locale, Event $queueForEvents, Document $project) { $protocol = $request->getProtocol(); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $sessionId = ($sessionId === 'current') - ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret, $authDuration) + ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret) : $sessionId; $sessions = $user->getAttribute('sessions', []); @@ -2448,7 +2444,7 @@ App::patch('/v1/account/sessions/:sessionId') $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $sessionId = ($sessionId === 'current') - ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret, $authDuration) + ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret) : $sessionId; $sessions = $user->getAttribute('sessions', []); diff --git a/app/init.php b/app/init.php index 61077c071..c9ffdbe65 100644 --- a/app/init.php +++ b/app/init.php @@ -1099,11 +1099,9 @@ App::setResource('user', function ($mode, $project, $console, $request, $respons Authorization::setDefaultStatus(true); Auth::setCookieName('a_session_' . $project->getId()); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; if (APP_MODE_ADMIN === $mode) { Auth::setCookieName('a_session_' . $console->getId()); - $authDuration = Auth::TOKEN_EXPIRATION_LOGIN_LONG; } $session = Auth::decodeSession( @@ -1146,7 +1144,7 @@ App::setResource('user', function ($mode, $project, $console, $request, $respons if ( $user->isEmpty() // Check a document has been found in the DB - || !Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret, $authDuration) + || !Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret) ) { // Validate user has valid login token $user = new Document([]); } diff --git a/app/realtime.php b/app/realtime.php index f7fc7070a..663e8c9e7 100644 --- a/app/realtime.php +++ b/app/realtime.php @@ -545,11 +545,10 @@ $server->onMessage(function (int $connection, string $message) use ($server, $re Auth::$secret = $session['secret'] ?? ''; $user = $database->getDocument('users', Auth::$unique); - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; if ( empty($user->getId()) // Check a document has been found in the DB - || !Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret, $authDuration) // Validate user has valid login token + || !Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret) // Validate user has valid login token ) { // cookie not valid throw new Exception(Exception::REALTIME_MESSAGE_FORMAT_INVALID, 'Session is not valid.'); diff --git a/src/Appwrite/Auth/Auth.php b/src/Appwrite/Auth/Auth.php index 5675d1a5f..591410988 100644 --- a/src/Appwrite/Auth/Auth.php +++ b/src/Appwrite/Auth/Auth.php @@ -351,11 +351,10 @@ class Auth * * @param array $sessions * @param string $secret - * @param string $expires * * @return bool|string */ - public static function sessionVerify(array $sessions, string $secret, int $expires) + public static function sessionVerify(array $sessions, string $secret) { foreach ($sessions as $session) { /** @var Document $session */ @@ -363,7 +362,7 @@ class Auth $session->isSet('secret') && $session->isSet('provider') && $session->getAttribute('secret') === self::hash($secret) && - DateTime::formatTz(DateTime::addSeconds(new \DateTime($session->getAttribute('expire')), $expires)) >= DateTime::formatTz(DateTime::now()) + DateTime::formatTz(DateTime::format(new \DateTime($session->getAttribute('expire')))) >= DateTime::formatTz(DateTime::now()) ) { return $session->getId(); } diff --git a/tests/e2e/Services/Account/AccountCustomClientTest.php b/tests/e2e/Services/Account/AccountCustomClientTest.php index 9cbd4227f..01a9b1994 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -613,7 +613,9 @@ class AccountCustomClientTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $session, - ])); + ]), [ + 'identity' => true + ]); $this->assertEquals(200, $response['headers']['status-code']); $this->assertEquals('123456', $response['body']['providerAccessToken']); diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 4d844c755..adec297a0 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -691,7 +691,7 @@ class ProjectsConsoleClientTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ - 'duration' => 60, // Set session duration to 2 minutes + 'duration' => 60, // Set session duration to 1 minute ]); $this->assertEquals(200, $response['headers']['status-code']); From 98d84a6887b56f2bd53b19a1e38df74445fcf3a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 15 Jan 2024 14:53:37 +0000 Subject: [PATCH 05/22] Fix unit tests --- tests/unit/Auth/AuthTest.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/unit/Auth/AuthTest.php b/tests/unit/Auth/AuthTest.php index 3e6a760b2..2199e935d 100644 --- a/tests/unit/Auth/AuthTest.php +++ b/tests/unit/Auth/AuthTest.php @@ -213,12 +213,14 @@ class AuthTest extends TestCase 'secret' => $hash, 'provider' => Auth::SESSION_PROVIDER_EMAIL, 'providerUid' => 'test@example.com', + 'expire' => DateTime::addSeconds(new \DateTime(), $expireTime1), ]), new Document([ '$id' => ID::custom('token2'), 'secret' => 'secret2', 'provider' => Auth::SESSION_PROVIDER_EMAIL, 'providerUid' => 'test@example.com', + 'expire' => DateTime::addSeconds(new \DateTime(), $expireTime1), ]), ]; @@ -230,19 +232,21 @@ class AuthTest extends TestCase 'secret' => $hash, 'provider' => Auth::SESSION_PROVIDER_EMAIL, 'providerUid' => 'test@example.com', + 'expire' => DateTime::addSeconds(new \DateTime(), $expireTime2), ]), new Document([ '$id' => ID::custom('token2'), 'secret' => 'secret2', 'provider' => Auth::SESSION_PROVIDER_EMAIL, 'providerUid' => 'test@example.com', + 'expire' => DateTime::addSeconds(new \DateTime(), $expireTime2), ]), ]; - $this->assertEquals(Auth::sessionVerify($tokens1, $secret, $expireTime1), 'token1'); - $this->assertEquals(Auth::sessionVerify($tokens1, 'false-secret', $expireTime1), false); - $this->assertEquals(Auth::sessionVerify($tokens2, $secret, $expireTime2), false); - $this->assertEquals(Auth::sessionVerify($tokens2, 'false-secret', $expireTime2), false); + $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(): void From 12a0596c6e84f77b6a41c7e546cc940e599eb73c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 15 Jan 2024 20:04:36 +0000 Subject: [PATCH 06/22] Implement session renewal test --- app/controllers/api/account.php | 1 - .../Projects/ProjectsConsoleClientTest.php | 55 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index e95719329..b55630a6c 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -2442,7 +2442,6 @@ App::patch('/v1/account/sessions/:sessionId') ->inject('queueForEvents') ->action(function (?string $sessionId, bool $identity, Response $response, Document $user, Database $dbForProject, Document $project, Event $queueForEvents) { - $authDuration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $sessionId = ($sessionId === 'current') ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret) : $sessionId; diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index adec297a0..8c4c8889f 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -765,6 +765,61 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(401, $response['headers']['status-code']); + // Set session duration to 15s + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/duration', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'duration' => 15, // seconds + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals(15, $response['body']['authDuration']); + + // Create session + $response = $this->client->call(Client::METHOD_POST, '/account/sessions/email', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $projectId, + ]), [ + 'email' => $userEmail, + 'password' => 'password', + ]); + + $this->assertEquals(201, $response['headers']['status-code']); + + $sessionCookie = $response['headers']['set-cookie']; + + // Wait 10 seconds, ensure valid session, extend session + \sleep(10); + + $response = $this->client->call(Client::METHOD_GET, '/account', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $projectId, + 'Cookie' => $sessionCookie, + ])); + + $this->assertEquals(200, $response['headers']['status-code']); + + $response = $this->client->call(Client::METHOD_PATCH, '/account/sessions/current', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $projectId, + 'cookie' => $sessionCookie, + ])); + + $this->assertEquals(200, $response['headers']['status-code']); + + // Wait 20 seconds, ensure non-valid session + \sleep(20); + + $response = $this->client->call(Client::METHOD_GET, '/account', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $projectId, + 'Cookie' => $sessionCookie, + ])); + + $this->assertEquals(401, $response['headers']['status-code']); + // Return project back to normal $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $id . '/auth/duration', array_merge([ 'content-type' => 'application/json', From 240f20b2cfb20be998d087d4ab0f299aaf64831d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Wed, 17 Jan 2024 13:20:47 +0000 Subject: [PATCH 07/22] PR review changes --- app/controllers/api/account.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index b55630a6c..01d50f94e 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -234,7 +234,6 @@ App::post('/v1/account/sessions/email') $duration = $project->getAttribute('auths', [])['duration'] ?? Auth::TOKEN_EXPIRATION_LOGIN_LONG; $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); - $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $secret = Auth::tokenGenerator(); $session = new Document(array_merge( [ @@ -280,6 +279,8 @@ App::post('/v1/account/sessions/email') ; } + $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); + $response ->addCookie(Auth::$cookieName . '_legacy', Auth::encodeSession($user->getId(), $secret), (new \DateTime($expire))->getTimestamp(), '/', Config::getParam('cookieDomain'), ('https' == $protocol), true, null) ->addCookie(Auth::$cookieName, Auth::encodeSession($user->getId(), $secret), (new \DateTime($expire))->getTimestamp(), '/', Config::getParam('cookieDomain'), ('https' == $protocol), true, Config::getParam('cookieSamesite')) @@ -750,7 +751,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); $secret = Auth::tokenGenerator(); - $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $session = new Document(array_merge([ '$id' => ID::unique(), 'userId' => $user->getId(), @@ -813,6 +813,8 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $state['success'] = URLParser::unparse($state['success']); } + $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); + $response ->addHeader('Cache-Control', 'no-store, no-cache, must-revalidate, max-age=0') ->addHeader('Pragma', 'no-cache') @@ -1203,7 +1205,6 @@ App::put('/v1/account/sessions/magic-url') $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); $secret = Auth::tokenGenerator(); - $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $session = new Document(array_merge( [ '$id' => ID::unique(), @@ -1257,6 +1258,7 @@ App::put('/v1/account/sessions/magic-url') $response->addHeader('X-Fallback-Cookies', \json_encode([Auth::$cookieName => Auth::encodeSession($user->getId(), $secret)])); } + $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $protocol = $request->getProtocol(); $response @@ -1482,7 +1484,6 @@ App::put('/v1/account/sessions/phone') $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); $secret = Auth::tokenGenerator(); - $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $session = new Document(array_merge( [ '$id' => ID::unique(), @@ -1535,6 +1536,7 @@ App::put('/v1/account/sessions/phone') $response->addHeader('X-Fallback-Cookies', \json_encode([Auth::$cookieName => Auth::encodeSession($user->getId(), $secret)])); } + $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $protocol = $request->getProtocol(); $response @@ -1636,7 +1638,6 @@ App::post('/v1/account/sessions/anonymous') $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); $secret = Auth::tokenGenerator(); - $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $session = new Document(array_merge( [ @@ -1674,6 +1675,8 @@ App::post('/v1/account/sessions/anonymous') $response->addHeader('X-Fallback-Cookies', \json_encode([Auth::$cookieName => Auth::encodeSession($user->getId(), $secret)])); } + $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); + $response ->addCookie(Auth::$cookieName . '_legacy', Auth::encodeSession($user->getId(), $secret), (new \DateTime($expire))->getTimestamp(), '/', Config::getParam('cookieDomain'), ('https' == $protocol), true, null) ->addCookie(Auth::$cookieName, Auth::encodeSession($user->getId(), $secret), (new \DateTime($expire))->getTimestamp(), '/', Config::getParam('cookieDomain'), ('https' == $protocol), true, Config::getParam('cookieSamesite')) @@ -2448,9 +2451,9 @@ App::patch('/v1/account/sessions/:sessionId') $sessions = $user->getAttribute('sessions', []); $session = null; - foreach ($sessions as $key => $session) {/** @var Document $session */ - if ($sessionId === $session->getId()) { - $session = $session; + foreach ($sessions as $key => $loopSession) { + if ($sessionId === $loopSession->getId()) { + $session = $loopSession; break; } } From f2291f16860f37772767242608c3b26698d3a293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Fri, 19 Jan 2024 12:11:20 +0000 Subject: [PATCH 08/22] Add OTP translations --- app/config/locale/templates/email-otp.tpl | 4 ++-- app/config/locale/translations/en.json | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/config/locale/templates/email-otp.tpl b/app/config/locale/templates/email-otp.tpl index e4c9bd226..45564fbf8 100644 --- a/app/config/locale/templates/email-otp.tpl +++ b/app/config/locale/templates/email-otp.tpl @@ -1,8 +1,8 @@

{{hello}}

-

{{optionOtp}}

+

{{description}}

-

{{otp}}

+

{{otp}}

{{clientInfo}}

diff --git a/app/config/locale/translations/en.json b/app/config/locale/translations/en.json index 3c355ffca..fa1b96b57 100644 --- a/app/config/locale/translations/en.json +++ b/app/config/locale/translations/en.json @@ -17,6 +17,12 @@ "emails.magicSession.clientInfo": "This sign in was requested using {{agentClient}} on {{agentDevice}} {{agentOs}}. If you didn't request the sign in, you can safely ignore this email.", "emails.magicSession.thanks": "Thanks,", "emails.magicSession.signature": "{{project}} team", + "emails.otpSession.subject": "{{project}} Login", + "emails.otpSession.hello": "Hello,", + "emails.otpSession.optionButton": "Enter the following verification code when prompted to securely sign in to your {{projectBold}} account. It will expire in 15 minutes.", + "emails.otpSession.clientInfo": "This sign in was requested using {{agentClient}} on {{agentDevice}} {{agentOs}}. If you didn't request the sign in, you can safely ignore this email.", + "emails.otpSession.thanks": "Thanks,", + "emails.otpSession.signature": "{{project}} team", "emails.recovery.subject": "Password Reset", "emails.recovery.hello": "Hello {{user}}", "emails.recovery.body": "Follow this link to reset your {{project}} password.", From df9bc6df56452798a486a132e0684ebfb5f5d8d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Fri, 19 Jan 2024 13:42:26 +0000 Subject: [PATCH 09/22] Implement OTP email endpoint + tests --- app/config/auth.php | 7 + app/config/locale/templates/email-base.tpl | 1 + app/config/locale/templates/email-otp.tpl | 14 +- app/config/locale/translations/en.json | 3 +- app/controllers/api/account.php | 231 ++++++++++++++++++++- src/Appwrite/Auth/Auth.php | 3 +- tests/e2e/Services/Account/AccountBase.php | 100 +++++++++ 7 files changed, 354 insertions(+), 5 deletions(-) diff --git a/app/config/auth.php b/app/config/auth.php index ca62256dc..930df226e 100644 --- a/app/config/auth.php +++ b/app/config/auth.php @@ -17,6 +17,13 @@ return [ 'docs' => 'https://appwrite.io/docs/references/cloud/client-web/account#accountCreateMagicURLToken', 'enabled' => true, ], + 'email' => [ + 'name' => 'Email (OTP)', + 'key' => 'email', + 'icon' => '/images/users/email.png', + 'docs' => 'https://appwrite.io/docs/references/cloud/client-web/account#accountCreateEmailToken', + 'enabled' => true, + ], 'anonymous' => [ 'name' => 'Anonymous', 'key' => 'anonymous', diff --git a/app/config/locale/templates/email-base.tpl b/app/config/locale/templates/email-base.tpl index 346f2f158..8c571c2b5 100644 --- a/app/config/locale/templates/email-base.tpl +++ b/app/config/locale/templates/email-base.tpl @@ -10,6 +10,7 @@