From df064adce3f2819881172e36882e990c1888c6fd Mon Sep 17 00:00:00 2001 From: Steven Nguyen <1477010+stnguyen90@users.noreply.github.com> Date: Wed, 8 May 2024 17:36:46 +0000 Subject: [PATCH 1/4] feat(auth): forward OAuth2 callback params The only place Apple includes the user's name is in the params so we need to forward the params to the redirect endpoint so they can be used when creating the user. --- app/controllers/api/account.php | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 8c0f25be09..11af53008e 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1069,17 +1069,15 @@ App::get('/v1/account/sessions/oauth2/callback/:provider/:projectId') $domain = $request->getHostname(); $protocol = $request->getProtocol(); + $params = $request->getParams(); + $params['project'] = $projectId; + unset($params['projectId']); + $response ->addHeader('Cache-Control', 'no-store, no-cache, must-revalidate, max-age=0') ->addHeader('Pragma', 'no-cache') ->redirect($protocol . '://' . $domain . '/v1/account/sessions/oauth2/' . $provider . '/redirect?' - . \http_build_query([ - 'project' => $projectId, - 'code' => $code, - 'state' => $state, - 'error' => $error, - 'error_description' => $error_description - ])); + . \http_build_query($params)); }); App::post('/v1/account/sessions/oauth2/callback/:provider/:projectId') @@ -1102,17 +1100,15 @@ App::post('/v1/account/sessions/oauth2/callback/:provider/:projectId') $domain = $request->getHostname(); $protocol = $request->getProtocol(); + $params = $request->getParams(); + $params['project'] = $projectId; + unset($params['projectId']); + $response ->addHeader('Cache-Control', 'no-store, no-cache, must-revalidate, max-age=0') ->addHeader('Pragma', 'no-cache') ->redirect($protocol . '://' . $domain . '/v1/account/sessions/oauth2/' . $provider . '/redirect?' - . \http_build_query([ - 'project' => $projectId, - 'code' => $code, - 'state' => $state, - 'error' => $error, - 'error_description' => $error_description - ])); + . \http_build_query($params)); }); App::get('/v1/account/sessions/oauth2/:provider/redirect') From c76e29077cac774100bcbd150b151bc144c054ea Mon Sep 17 00:00:00 2001 From: Steven Nguyen <1477010+stnguyen90@users.noreply.github.com> Date: Wed, 8 May 2024 17:51:45 +0000 Subject: [PATCH 2/4] feat(auth): try to get user name from request param if not from oauth2 This is only applicable for Apple OAuth2 because this is the only provider that does not return user name from an API call and only returns the name in the callback URL. Reference: * https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_js/incorporating_sign_in_with_apple_into_other_platforms#3332115 --- app/controllers/api/account.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 11af53008e..f379dd3023 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1235,7 +1235,17 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $failureRedirect(Exception::USER_MISSING_ID); } - $name = $oauth2->getUserName($accessToken); + $name = ''; + $nameOAuth = $oauth2->getUserName($accessToken); + $userParam = \json_decode($request->getParam('user'), true); + if (!empty($nameOAuth)) { + $name = $nameOAuth; + } elseif (is_array($userParam)) { + $nameParam = $userParam['name']; + if (is_array($nameParam) && isset($nameParam['firstName']) && isset($nameParam['lastName'])) { + $name = $nameParam['firstName'] . ' ' . $nameParam['lastName']; + } + } $email = $oauth2->getUserEmail($accessToken); // Check if this identity is connected to a different user From c52bf2a0a125f5fb658a00ede709f56e217b4de5 Mon Sep 17 00:00:00 2001 From: Steven Nguyen <1477010+stnguyen90@users.noreply.github.com> Date: Wed, 8 May 2024 17:57:10 +0000 Subject: [PATCH 3/4] fix(auth): Don't use email in place for name for Apple OAuth2 Apple OAuth2 does not return the user's name in the claims and so we used email instead, but this can look broken to users and developers to see an email where the name should be. --- src/Appwrite/Auth/OAuth2/Apple.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Appwrite/Auth/OAuth2/Apple.php b/src/Appwrite/Auth/OAuth2/Apple.php index 2abf61c947..0b4ec50881 100644 --- a/src/Appwrite/Auth/OAuth2/Apple.php +++ b/src/Appwrite/Auth/OAuth2/Apple.php @@ -160,15 +160,6 @@ class Apple extends OAuth2 */ public function getUserName(string $accessToken): string { - if ( - isset($this->claims['email']) && - !empty($this->claims['email']) && - isset($this->claims['email_verified']) && - $this->claims['email_verified'] === 'true' - ) { - return $this->claims['email']; - } - return ''; } From 1626168d3794cbacc31c9244c43436c2642dec68 Mon Sep 17 00:00:00 2001 From: Steven Nguyen <1477010+stnguyen90@users.noreply.github.com> Date: Mon, 13 May 2024 14:17:39 -0700 Subject: [PATCH 4/4] fix(project): set limit to retrieve all stats for the usage range Because limit was not passed for the find() query, the limit defaulted to 25. As such, when requesting stats for the last 30 days, only the last 25 were retrieved. --- app/controllers/api/project.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/project.php b/app/controllers/api/project.php index 208e288614..a3e07927d6 100644 --- a/app/controllers/api/project.php +++ b/app/controllers/api/project.php @@ -70,7 +70,7 @@ App::get('/v1/project/usage') '1d' => 'Y-m-d\T00:00:00.000P', }; - Authorization::skip(function () use ($dbForProject, $firstDay, $lastDay, $period, $metrics, &$total, &$stats) { + Authorization::skip(function () use ($dbForProject, $firstDay, $lastDay, $period, $metrics, $limit, &$total, &$stats) { foreach ($metrics['total'] as $metric) { $result = $dbForProject->findOne('stats', [ Query::equal('metric', [$metric]), @@ -85,6 +85,7 @@ App::get('/v1/project/usage') Query::equal('period', [$period]), Query::greaterThanEqual('time', $firstDay), Query::lessThan('time', $lastDay), + Query::limit($limit), Query::orderDesc('time'), ]);