From 7de29e299198e41c7b09722e1d0b039a8467a25a Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Fri, 14 Jul 2023 16:22:30 -0700 Subject: [PATCH 1/4] Fix typing issues in OAuth2 --- src/Appwrite/Auth/OAuth2.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Appwrite/Auth/OAuth2.php b/src/Appwrite/Auth/OAuth2.php index d7c1c99546..28de8dd914 100644 --- a/src/Appwrite/Auth/OAuth2.php +++ b/src/Appwrite/Auth/OAuth2.php @@ -73,6 +73,13 @@ abstract class OAuth2 */ abstract public function refreshTokens(string $refreshToken): array; + /** + * @param string $accessToken + * + * @return string + */ + abstract public function getUserID(string $accessToken): string; + /** * @param string $accessToken * @@ -148,11 +155,11 @@ abstract class OAuth2 * * @return string */ - public function getAccessTokenExpiry(string $code): string + public function getAccessTokenExpiry(string $code): int { $tokens = $this->getTokens($code); - return $tokens['expires_in'] ?? ''; + return $tokens['expires_in'] ?? 0; } // The parseState function was designed specifically for Amazon OAuth2 Adapter to override. From 3499a7028c8866a28a661029c56b2493eabff3cb Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Fri, 14 Jul 2023 17:23:17 -0700 Subject: [PATCH 2/4] Create OAuth2 exception class --- app/config/errors.php | 15 ++++++++ src/Appwrite/Auth/OAuth2/Exception.php | 51 ++++++++++++++++++++++++++ src/Appwrite/Extend/Exception.php | 3 ++ 3 files changed, 69 insertions(+) create mode 100644 src/Appwrite/Auth/OAuth2/Exception.php diff --git a/app/config/errors.php b/app/config/errors.php index 0c5067f1f6..37e4c5eba1 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -195,6 +195,21 @@ return [ 'description' => 'Missing ID from OAuth2 provider.', 'code' => 400, ], + Exception::USER_OAUTH2_BAD_REQUEST => [ + 'name' => Exception::USER_OAUTH2_BAD_REQUEST, + 'description' => 'OAuth2 provider rejected the bad request.', + 'code' => 400, + ], + Exception::USER_OAUTH2_UNAUTHORIZED => [ + 'name' => Exception::USER_OAUTH2_UNAUTHORIZED, + 'description' => 'OAuth2 provider rejected the unauthorized request.', + 'code' => 401, + ], + Exception::USER_OAUTH2_PROVIDER_ERROR => [ + 'name' => Exception::USER_OAUTH2_PROVIDER_ERROR, + 'description' => 'OAuth2 provider returned some error.', + 'code' => 424, + ], /** Teams */ Exception::TEAM_NOT_FOUND => [ diff --git a/src/Appwrite/Auth/OAuth2/Exception.php b/src/Appwrite/Auth/OAuth2/Exception.php new file mode 100644 index 0000000000..af98fa5e9d --- /dev/null +++ b/src/Appwrite/Auth/OAuth2/Exception.php @@ -0,0 +1,51 @@ +response = $response; + $this->message = $response; + $decoded = json_decode($response, true); + if (\is_array($decoded)) { + $this->error = $decoded['error']; + $this->errorDescription = $decoded['error_description']; + $this->message = $this->error . ': ' . $this->errorDescription; + } + $type = match ($code) { + 400 => AppwriteException::USER_OAUTH2_BAD_REQUEST, + 401 => AppwriteException::USER_OAUTH2_UNAUTHORIZED, + default => AppwriteException::USER_OAUTH2_PROVIDER_ERROR + }; + + parent::__construct($type, $this->message, $code, $previous); + } + + /** + * Get the error parameter from the response. + * + * See https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 for more information. + */ + public function getError(): string + { + return $this->error; + } + + /** + * Get the error_description parameter from the response. + * + * See https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 for more information. + */ + public function getErrorDescription(): string + { + return $this->errorDescription; + } +} diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index d97032bd81..07c273b887 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -75,6 +75,9 @@ class Exception extends \Exception public const USER_PHONE_ALREADY_EXISTS = 'user_phone_already_exists'; public const USER_PHONE_NOT_FOUND = 'user_phone_not_found'; public const USER_MISSING_ID = 'user_missing_id'; + public const USER_OAUTH2_BAD_REQUEST = 'user_oauth2_bad_request'; + public const USER_OAUTH2_UNAUTHORIZED = 'user_oauth2_unauthorized'; + public const USER_OAUTH2_PROVIDER_ERROR = 'user_oauth2_provider_error'; /** Teams */ public const TEAM_NOT_FOUND = 'team_not_found'; From a7c9e4bb7e9a9d4fd2d47a7d9c912328e0464caf Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Fri, 14 Jul 2023 17:28:45 -0700 Subject: [PATCH 3/4] Improve OAuth2 Error Handling Update the OAuth2 class to throw an exception if an API call to the OAuth2 provider fails and update the endpoint to redirect to the failure url with the information from the OAuth2 provider. --- app/controllers/api/account.php | 79 +++++++++++++++++++++------------ src/Appwrite/Auth/OAuth2.php | 10 ++++- 2 files changed, 60 insertions(+), 29 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 4869a30dd6..4e862b822e 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -2,6 +2,7 @@ use Ahc\Jwt\JWT; use Appwrite\Auth\Auth; +use Appwrite\Auth\OAuth2\Exception as OAuth2Exception; use Appwrite\Auth\Validator\Password; use Appwrite\Auth\Validator\Phone; use Appwrite\Detector\Detector; @@ -413,27 +414,22 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $appSecret = $project->getAttribute('authProviders', [])[$provider . 'Secret'] ?? '{}'; $providerEnabled = $project->getAttribute('authProviders', [])[$provider . 'Enabled'] ?? false; - if (!$providerEnabled) { - throw new Exception(Exception::PROJECT_PROVIDER_DISABLED, 'This provider is disabled. Please enable the provider from your ' . APP_NAME . ' console to continue.'); - } - - if (!empty($appSecret) && isset($appSecret['version'])) { - $key = App::getEnv('_APP_OPENSSL_KEY_V' . $appSecret['version']); - $appSecret = OpenSSL::decrypt($appSecret['data'], $appSecret['method'], $key, 0, \hex2bin($appSecret['iv']), \hex2bin($appSecret['tag'])); - } - $className = 'Appwrite\\Auth\\OAuth2\\' . \ucfirst($provider); if (!\class_exists($className)) { throw new Exception(Exception::PROJECT_PROVIDER_UNSUPPORTED); } + $providers = Config::getParam('providers'); + $providerName = $providers[$provider]['name'] ?? ''; + + /** @var Appwrite\Auth\OAuth2 $oauth2 */ $oauth2 = new $className($appId, $appSecret, $callback); if (!empty($state)) { try { $state = \array_merge($defaultState, $oauth2->parseState($state)); - } catch (\Exception$exception) { + } catch (\Exception $exception) { throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Failed to parse login state params as passed from OAuth2 provider'); } } else { @@ -447,27 +443,54 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') if (!empty($state['failure']) && !$validateURL->isValid($state['failure'])) { throw new Exception(Exception::PROJECT_INVALID_FAILURE_URL); } - - $accessToken = $oauth2->getAccessToken($code); - $refreshToken = $oauth2->getRefreshToken($code); - $accessTokenExpiry = $oauth2->getAccessTokenExpiry($code); - - if (empty($accessToken)) { - if (!empty($state['failure'])) { - $response->redirect($state['failure'], 301, 0); + $failure = []; + if (!empty($state['failure'])) { + $failure = URLParser::parse($state['failure']); + } + $failureRedirect = (function (string $type, ?string $message = null, ?int $code = null) use ($failure, $response) { + $exception = new Exception($type, $message, $code); + if (!empty($failure)) { + $query = URLParser::parseQuery($failure['query']); + $query['error'] = json_encode([ + 'message' => $exception->getMessage(), + 'type' => $exception->getType(), + 'code' => !\is_null($code) ? $code : $exception->getCode(), + ]); + $failure['query'] = URLParser::unparseQuery($query); + $response->redirect(URLParser::unparse($failure), 301); } - throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Failed to obtain access token'); + throw $exception; + }); + + if (!$providerEnabled) { + $failureRedirect(Exception::PROJECT_PROVIDER_DISABLED, 'This provider is disabled. Please enable the provider from your ' . APP_NAME . ' console to continue.'); + } + + if (!empty($appSecret) && isset($appSecret['version'])) { + $key = App::getEnv('_APP_OPENSSL_KEY_V' . $appSecret['version']); + $appSecret = OpenSSL::decrypt($appSecret['data'], $appSecret['method'], $key, 0, \hex2bin($appSecret['iv']), \hex2bin($appSecret['tag'])); + } + + $accessToken = ''; + $refreshToken = ''; + $accessTokenExpiry = 0; + + try { + $accessToken = $oauth2->getAccessToken($code); + $refreshToken = $oauth2->getRefreshToken($code); + $accessTokenExpiry = $oauth2->getAccessTokenExpiry($code); + } catch (OAuth2Exception $ex) { + $failureRedirect( + $ex->getType(), + 'Failed to obtain access token. The ' . $providerName . ' OAuth2 provider returned an error: ' . $ex->getMessage(), + $ex->getCode(), + ); } $oauth2ID = $oauth2->getUserID($accessToken); - if (empty($oauth2ID)) { - if (!empty($state['failure'])) { - $response->redirect($state['failure'], 301, 0); - } - - throw new Exception(Exception::USER_MISSING_ID); + $failureRedirect(Exception::USER_MISSING_ID); } $sessions = $user->getAttribute('sessions', []); @@ -515,7 +538,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $total = $dbForProject->count('users', max: APP_LIMIT_USERS); if ($total >= $limit) { - throw new Exception(Exception::USER_COUNT_EXCEEDED); + $failureRedirect(Exception::USER_COUNT_EXCEEDED); } } @@ -550,13 +573,13 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') ]); Authorization::skip(fn() => $dbForProject->createDocument('users', $user)); } catch (Duplicate $th) { - throw new Exception(Exception::USER_ALREADY_EXISTS); + $failureRedirect(Exception::USER_ALREADY_EXISTS); } } } if (false === $user->getAttribute('status')) { // Account is blocked - throw new Exception(Exception::USER_BLOCKED); // User is in status blocked + $failureRedirect(Exception::USER_BLOCKED); // User is in status blocked } // Create session token, verify user account and update OAuth2 ID and Access Token diff --git a/src/Appwrite/Auth/OAuth2.php b/src/Appwrite/Auth/OAuth2.php index 28de8dd914..c737e183f8 100644 --- a/src/Appwrite/Auth/OAuth2.php +++ b/src/Appwrite/Auth/OAuth2.php @@ -2,6 +2,8 @@ namespace Appwrite\Auth; +use Appwrite\Auth\OAuth2\Exception; + abstract class OAuth2 { /** @@ -75,7 +77,7 @@ abstract class OAuth2 /** * @param string $accessToken - * + * * @return string */ abstract public function getUserID(string $accessToken): string; @@ -202,8 +204,14 @@ abstract class OAuth2 // Send the request & save response to $response $response = \curl_exec($ch); + $code = curl_getinfo($ch, CURLINFO_HTTP_CODE); + \curl_close($ch); + if ($code != 200) { + throw new Exception($response, $code); + } + return (string)$response; } } From 2621c08c4aa6ec92f94624c1101b4bcc99f011ab Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Thu, 27 Jul 2023 17:12:15 -0700 Subject: [PATCH 4/4] Update OAuth2 params 1. Make code optional so we can show the error via redirect 2. Capture error and error_description params from the OAuth2 provider --- app/controllers/api/account.php | 46 +++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 4e862b822e..5c78898453 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -339,11 +339,13 @@ App::get('/v1/account/sessions/oauth2/callback/:provider/:projectId') ->label('docs', false) ->param('projectId', '', new Text(1024), 'Project ID.') ->param('provider', '', new WhiteList(\array_keys(Config::getParam('providers')), true), 'OAuth2 provider.') - ->param('code', '', new Text(2048), 'OAuth2 code.') + ->param('code', '', new Text(2048, 0), 'OAuth2 code.', true) ->param('state', '', new Text(2048), 'Login state params.', true) + ->param('error', '', new Text(2048, 0), 'Error code returned from the OAuth2 provider.', true) + ->param('error_description', '', new Text(2048, 0), 'Human-readable text providing additional information about the error returned from the OAuth2 provider.', true) ->inject('request') ->inject('response') - ->action(function (string $projectId, string $provider, string $code, string $state, Request $request, Response $response) { + ->action(function (string $projectId, string $provider, string $code, string $state, string $error, string $error_description, Request $request, Response $response) { $domain = $request->getHostname(); $protocol = $request->getProtocol(); @@ -352,7 +354,13 @@ App::get('/v1/account/sessions/oauth2/callback/:provider/:projectId') ->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])); + . \http_build_query([ + 'project' => $projectId, + 'code' => $code, + 'state' => $state, + 'error' => $error, + 'error_description' => $error_description + ])); }); App::post('/v1/account/sessions/oauth2/callback/:provider/:projectId') @@ -364,11 +372,13 @@ App::post('/v1/account/sessions/oauth2/callback/:provider/:projectId') ->label('docs', false) ->param('projectId', '', new Text(1024), 'Project ID.') ->param('provider', '', new WhiteList(\array_keys(Config::getParam('providers')), true), 'OAuth2 provider.') - ->param('code', '', new Text(2048), 'OAuth2 code.') + ->param('code', '', new Text(2048, 0), 'OAuth2 code.', true) ->param('state', '', new Text(2048), 'Login state params.', true) + ->param('error', '', new Text(2048, 0), 'Error code returned from the OAuth2 provider.', true) + ->param('error_description', '', new Text(2048, 0), 'Human-readable text providing additional information about the error returned from the OAuth2 provider.', true) ->inject('request') ->inject('response') - ->action(function (string $projectId, string $provider, string $code, string $state, Request $request, Response $response) { + ->action(function (string $projectId, string $provider, string $code, string $state, string $error, string $error_description, Request $request, Response $response) { $domain = $request->getHostname(); $protocol = $request->getProtocol(); @@ -377,7 +387,13 @@ App::post('/v1/account/sessions/oauth2/callback/:provider/:projectId') ->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])); + . \http_build_query([ + 'project' => $projectId, + 'code' => $code, + 'state' => $state, + 'error' => $error, + 'error_description' => $error_description + ])); }); App::get('/v1/account/sessions/oauth2/:provider/redirect') @@ -395,8 +411,10 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') ->label('usage.metric', 'sessions.{scope}.requests.create') ->label('usage.params', ['provider:{request.provider}']) ->param('provider', '', new WhiteList(\array_keys(Config::getParam('providers')), true), 'OAuth2 provider.') - ->param('code', '', new Text(2048), 'OAuth2 code.') + ->param('code', '', new Text(2048, 0), 'OAuth2 code.', true) ->param('state', '', new Text(2048), 'OAuth2 state params.', true) + ->param('error', '', new Text(2048, 0), 'Error code returned from the OAuth2 provider.', true) + ->param('error_description', '', new Text(2048, 0), 'Human-readable text providing additional information about the error returned from the OAuth2 provider.', true) ->inject('request') ->inject('response') ->inject('project') @@ -404,7 +422,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') ->inject('dbForProject') ->inject('geodb') ->inject('events') - ->action(function (string $provider, string $code, string $state, Request $request, Response $response, Document $project, Document $user, Database $dbForProject, Reader $geodb, Event $events) use ($oauthDefaultSuccess) { + ->action(function (string $provider, string $code, string $state, string $error, string $error_description, Request $request, Response $response, Document $project, Document $user, Database $dbForProject, Reader $geodb, Event $events) use ($oauthDefaultSuccess) { $protocol = $request->getProtocol(); $callback = $protocol . '://' . $request->getHostname() . '/v1/account/sessions/oauth2/callback/' . $provider . '/' . $project->getId(); @@ -467,6 +485,18 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $failureRedirect(Exception::PROJECT_PROVIDER_DISABLED, 'This provider is disabled. Please enable the provider from your ' . APP_NAME . ' console to continue.'); } + if (!empty($error)) { + $message = 'The ' . $providerName . ' OAuth2 provider returned an error: ' . $error; + if (!empty($error_description)) { + $message .= ': ' . $error_description; + } + $failureRedirect(Exception::USER_OAUTH2_PROVIDER_ERROR, $message); + } + + if (empty($code)) { + $failureRedirect(Exception::USER_OAUTH2_PROVIDER_ERROR, 'Missing OAuth2 code. Please contact the Appwrite team for additional support.'); + } + if (!empty($appSecret) && isset($appSecret['version'])) { $key = App::getEnv('_APP_OPENSSL_KEY_V' . $appSecret['version']); $appSecret = OpenSSL::decrypt($appSecret['data'], $appSecret['method'], $key, 0, \hex2bin($appSecret['iv']), \hex2bin($appSecret['tag']));