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 1/8] 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 b3e3417555..18bf22e14b 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 38742bffdc..a861f3151f 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 2ba27efcb3..8e6ddb3277 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 98080fda55..55226fe0bd 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 31448c4df5..5675d1a5f7 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 2/8] 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 a861f3151f..e95719329d 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 61077c0719..c9ffdbe65f 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 f7fc7070a4..663e8c9e7f 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 5675d1a5f7..5914109881 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 9cbd4227fb..01a9b19944 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 4d844c7551..adec297a03 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 3/8] 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 3e6a760b25..2199e935d7 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 4/8] 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 e95719329d..b55630a6ce 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 adec297a03..8c4c8889f4 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 5/8] 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 b55630a6ce..01d50f94e5 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 fc186c69a1a0588e1b73af590c2b7e747704cb41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 22 Jan 2024 08:13:46 +0000 Subject: [PATCH 6/8] PR review changes --- app/controllers/api/account.php | 48 +++++++++++++-------------------- 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 01d50f94e5..a0e82f97d4 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -2437,13 +2437,12 @@ 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.') - ->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('queueForEvents') - ->action(function (?string $sessionId, bool $identity, Response $response, Document $user, Database $dbForProject, Document $project, Event $queueForEvents) { + ->action(function (?string $sessionId, Response $response, Document $user, Database $dbForProject, Document $project, Event $queueForEvents) { $sessionId = ($sessionId === 'current') ? Auth::sessionVerify($user->getAttribute('sessions'), Auth::$secret) @@ -2467,40 +2466,29 @@ App::patch('/v1/account/sessions/:sessionId') $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 + $provider = $session->getAttribute('provider'); + $refreshToken = $session->getAttribute('providerRefreshToken'); - // $expireAt = (int) $session->getAttribute('providerAccessTokenExpiry'); - // if(\time() < $expireAt - 5) { // 5 seconds time-sync and networking gap, to be safe - // return $response->noContent(); - // } + $appId = $project->getAttribute('oAuthProviders', [])[$provider . 'Appid'] ?? ''; + $appSecret = $project->getAttribute('oAuthProviders', [])[$provider . 'Secret'] ?? '{}'; - $provider = $session->getAttribute('provider'); - $refreshToken = $session->getAttribute('providerRefreshToken'); + $className = 'Appwrite\\Auth\\OAuth2\\' . \ucfirst($provider); - $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(''))); + if (!\class_exists($className)) { + throw new Exception(Exception::PROJECT_PROVIDER_UNSUPPORTED); } - $dbForProject->updateDocument('sessions', $sessionId, $session); + $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(''))); + + // Save changes + $dbForProject->updateDocument('sessions', $sessionId, $session); $dbForProject->deleteCachedDocument('users', $user->getId()); $queueForEvents From 9ad5aa79d0bf91b7abe305e217badd4dccafdd36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 22 Jan 2024 11:20:33 +0000 Subject: [PATCH 7/8] Fix failing tests --- app/controllers/api/account.php | 34 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index e47cd5347b..b5ee99fb8b 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -828,6 +828,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())); $session = $dbForProject->createDocument('sessions', $session->setAttribute('$permissions', [ @@ -1230,7 +1231,6 @@ $createSession = function (string $userId, string $secret, Request $request, Res $detector = new Detector($request->getUserAgent('UNKNOWN')); $record = $geodb->get($request->getIP()); $sessionSecret = Auth::tokenGenerator(Auth::TOKEN_LENGTH_SESSION); - $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $session = new Document(array_merge( [ @@ -1242,6 +1242,7 @@ $createSession = function (string $userId, string $secret, Request $request, Res 'userAgent' => $request->getUserAgent('UNKNOWN'), 'ip' => $request->getIP(), 'countryCode' => ($record) ? \strtolower($record['country']['iso_code']) : '--', + 'expire' => DateTime::addSeconds(new \DateTime(), $duration) ], $detector->getOS(), $detector->getClient(), @@ -1283,6 +1284,7 @@ $createSession = function (string $userId, string $secret, Request $request, Res $response->addHeader('X-Fallback-Cookies', \json_encode([Auth::$cookieName => Auth::encodeSession($user->getId(), $sessionSecret)])); } + $expire = DateTime::formatTz(DateTime::addSeconds(new \DateTime(), $duration)); $protocol = $request->getProtocol(); $response @@ -2438,27 +2440,23 @@ App::patch('/v1/account/sessions/:sessionId') $session->setAttribute('expire', DateTime::addSeconds(new \DateTime(), $authDuration)); // Refresh OAuth access token - $provider = $session->getAttribute('provider'); - $refreshToken = $session->getAttribute('providerRefreshToken'); - - $appId = $project->getAttribute('oAuthProviders', [])[$provider . 'Appid'] ?? ''; - $appSecret = $project->getAttribute('oAuthProviders', [])[$provider . 'Secret'] ?? '{}'; - + $provider = $session->getAttribute('provider', ''); + $refreshToken = $session->getAttribute('providerRefreshToken', ''); $className = 'Appwrite\\Auth\\OAuth2\\' . \ucfirst($provider); - if (!\class_exists($className)) { - throw new Exception(Exception::PROJECT_PROVIDER_UNSUPPORTED); + if (!empty($provider) && \class_exists($className)) { + $appId = $project->getAttribute('oAuthProviders', [])[$provider . 'Appid'] ?? ''; + $appSecret = $project->getAttribute('oAuthProviders', [])[$provider . 'Secret'] ?? '{}'; + + $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(''))); } - $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(''))); - // Save changes $dbForProject->updateDocument('sessions', $sessionId, $session); $dbForProject->deleteCachedDocument('users', $user->getId()); From 3bf89a931d2a3c2ffb56e95b26d08042fc22ce16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 22 Jan 2024 13:20:48 +0000 Subject: [PATCH 8/8] PR review changes --- app/controllers/api/account.php | 8 ++++---- tests/e2e/Services/Account/AccountCustomClientTest.php | 4 +--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index b5ee99fb8b..e049ccf4e2 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -2394,7 +2394,7 @@ App::delete('/v1/account/sessions/:sessionId') }); App::patch('/v1/account/sessions/:sessionId') - ->desc('Renew session') + ->desc('Update (or renew) a session') ->groups(['api', 'account']) ->label('scope', 'accounts.write') ->label('event', 'users.[userId].sessions.[sessionId].update') @@ -2424,9 +2424,9 @@ App::patch('/v1/account/sessions/:sessionId') $sessions = $user->getAttribute('sessions', []); $session = null; - foreach ($sessions as $key => $loopSession) { - if ($sessionId === $loopSession->getId()) { - $session = $loopSession; + foreach ($sessions as $key => $value) { + if ($sessionId === $value->getId()) { + $session = $value; break; } } diff --git a/tests/e2e/Services/Account/AccountCustomClientTest.php b/tests/e2e/Services/Account/AccountCustomClientTest.php index 197c1db67a..acd4e51b31 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -1785,9 +1785,7 @@ 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']);