Add proper error handling to OAuth2 client

diff --git a/wcfsetup/install/files/lib/action/AbstractOauth2Action.class.php b/wcfsetup/install/files/lib/action/AbstractOauth2Action.class.php
index 7e336a153395bb06248e251578ae1b6d800d15cd..2fe725ac0c7dede7254c8c082fdb0bac4bb08c17 100644 (file)
--- a/wcfsetup/install/files/lib/action/AbstractOauth2Action.class.php
+++ b/wcfsetup/install/files/lib/action/AbstractOauth2Action.class.php
@@ -1,11 +1,13 @@
 <?php
 namespace wcf\action;
 use GuzzleHttp\ClientInterface;
+use GuzzleHttp\Exception\GuzzleException;
 use GuzzleHttp\Psr7\Request;
 use ParagonIE\ConstantTime\Hex;
 use wcf\system\exception\NamedUserException;
 use wcf\system\exception\PermissionDeniedException;
 use wcf\system\io\HttpFactory;
+use wcf\system\user\authentication\oauth\exception\StateValidationException;
 use wcf\system\user\authentication\oauth\User as OauthUser;
 use wcf\system\WCF;
 use wcf\util\HeaderUtil;
@@ -105,13 +107,13 @@ abstract class AbstractOauth2Action extends AbstractAction {
         */
        protected function validateState() {
                if (!isset($_GET['state'])) {
-                       throw new \Exception('Missing state parameter');
+                       throw new StateValidationException('Missing state parameter');
                }
                if (!($sessionState = WCF::getSession()->getVar(self::STATE))) {
-                       throw new \Exception('Missing state in session');
+                       throw new StateValidationException('Missing state in session');
                }
                if (!\hash_equals($sessionState, (string) $_GET['state'])) {
-                       throw new \Exception('Mismatching state');
+                       throw new StateValidationException('Mismatching state');
                }
                
                WCF::getSession()->unregister(self::STATE);
@@ -132,12 +134,18 @@ abstract class AbstractOauth2Action extends AbstractAction {
                        'code' => $_GET['code'],
                ], '', '&', PHP_QUERY_RFC1738));
                
-               $response = $this->getHttpClient()->send($request);
-               
-               // Validate state. Validation of state is executed after fetching the
-               // access_token to invalidate 'code'.
-               if ($this->supportsState()) {
-                       $this->validateState();
+               try {
+                       $response = $this->getHttpClient()->send($request);
+               }
+               finally {
+                       // Validate state. Validation of state is executed after fetching the
+                       // access_token to invalidate 'code'.
+                       //
+                       // Validation is happening within the `finally` so that the StateValidationException
+                       // overwrites any GuzzleException (improving the error message).
+                       if ($this->supportsState()) {
+                               $this->validateState();
+                       }
                }
                
                $parsed = JSON::decode((string) $response->getBody());
@@ -190,17 +198,42 @@ abstract class AbstractOauth2Action extends AbstractAction {
        public function execute() {
                parent::execute();
                
-               if (isset($_GET['code'])) {
-                       $accessToken = $this->codeToAccessToken($_GET['code']);
-                       $oauthUser = $this->getUser($accessToken);
-                       
-                       $this->processUser($oauthUser);
+               try {
+                       if (isset($_GET['code'])) {
+                               $accessToken = $this->codeToAccessToken($_GET['code']);
+                               $oauthUser = $this->getUser($accessToken);
+                               
+                               $this->processUser($oauthUser);
+                       }
+                       else if (isset($_GET['error'])) {
+                               $this->handleError($_GET['error']);
+                       }
+                       else {
+                               $this->initiate();
+                       }
                }
-               else if (isset($_GET['error'])) {
-                       $this->handleError($_GET['error']);
+               catch (NamedUserException | PermissionDeniedException $e) {
+                       throw $e;
                }
-               else {
-                       $this->initiate();
+               catch (StateValidationException $e) {
+                       throw new NamedUserException(WCF::getLanguage()->getDynamicVariable(
+                               'wcf.user.3rdparty.login.error.stateValidation'
+                       ));
+               }
+               catch (\Exception $e) {
+                       $exceptionID = \wcf\functions\exception\logThrowable($e);
+                       
+                       $type = 'genericException';
+                       if ($e instanceof GuzzleException) {
+                               $type = 'httpError';
+                       }
+                       
+                       throw new NamedUserException(WCF::getLanguage()->getDynamicVariable(
+                               'wcf.user.3rdparty.login.error.'.$type,
+                               [
+                                       'exceptionID' => $exceptionID,
+                               ]
+                       ));
                }
        }
 }

diff --git a/wcfsetup/install/files/lib/system/user/authentication/oauth/exception/StateValidationException.class.php b/wcfsetup/install/files/lib/system/user/authentication/oauth/exception/StateValidationException.class.php
new file mode 100644 (file)
index 0000000..64ed5db
--- /dev/null
+++ b/wcfsetup/install/files/lib/system/user/authentication/oauth/exception/StateValidationException.class.php
@@ -0,0 +1,17 @@
+<?php
+namespace wcf\system\user\authentication\oauth\exception;
+
+/**
+ * Thrown when the CSRF 'state' parameter cannot be validated.
+ * 
+ * @author     Tim Duesterhus
+ * @copyright  2001-2021 WoltLab GmbH
+ * @license    GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
+ * @package    WoltLabSuite\Core\System\User\Authentication\Oauth\Exception
+ * @since      5.4
+ */
+final class StateValidationException extends \UnexpectedValueException {
+       public function __construct($message, ?\Throwable $previous = null) {
+               parent::__construct($message, 0, $previous);
+       }
+}

diff --git a/wcfsetup/install/lang/de.xml b/wcfsetup/install/lang/de.xml
index b80ceac351d5d7d72f95bbe1ae720e7f66818406..f8f1c1529b02e5ad8fb10c0f15fec63eb0720202 100644 (file)
--- a/wcfsetup/install/lang/de.xml
+++ b/wcfsetup/install/lang/de.xml
@@ -5068,6 +5068,9 @@ Die E-Mail-Adresse des neuen Benutzers lautet: {@$user->email}
                <item name="wcf.user.3rdparty"><![CDATA[Drittanbieter]]></item>
                <item name="wcf.user.3rdparty.login.error.access_denied"><![CDATA[Da {if LANGUAGE_USE_INFORMAL_VARIANT}du die Berechtigungen verweigert hast{else}Sie die Berechtigungen verweigert haben{/if}, ist die Anmeldung nicht möglich.]]></item>
                <item name="wcf.user.3rdparty.login.error.denied"><![CDATA[Da {if LANGUAGE_USE_INFORMAL_VARIANT}du die Berechtigungen verweigert hast{else}Sie die Berechtigungen verweigert haben{/if}, ist die Anmeldung nicht möglich.]]></item>
+               <item name="wcf.user.3rdparty.login.error.httpError"><![CDATA[Die Kommunikation mit dem externen Dienst ist fehlgeschlagen. Interner Fehlercode: <code>{$exceptionID}</code>.]]></item>
+               <item name="wcf.user.3rdparty.login.error.genericException"><![CDATA[Bei der Verarbeitung der Anfrage ist ein Fehler aufgetreten. Bitte {if LANGUAGE_USE_INFORMAL_VARIANT}wende dich{else}wenden Sie sich{/if} an den Administrator. Interner Fehlercode: <code>{$exceptionID}</code>.]]></item>
+               <item name="wcf.user.3rdparty.login.error.stateValidation"><![CDATA[Die Anfrage konnte nicht zugeordnet werden. Möglicherweise ist {if LANGUAGE_USE_INFORMAL_VARIANT}deine{else}Ihre{/if} Sitzung abgelaufen.]]></item>
                <item name="wcf.user.3rdparty.github"><![CDATA[GitHub]]></item>
                <item name="wcf.user.3rdparty.github.login"><![CDATA[Mit GitHub anmelden]]></item>
                <item name="wcf.user.3rdparty.github.register"><![CDATA[{if LANGUAGE_USE_INFORMAL_VARIANT}Du erstellst{else}Sie erstellen{/if} ein Benutzerkonto über <span class="icon icon16 fa-github"></span>&nbsp;GitHub. Der Benutzername und {if LANGUAGE_USE_INFORMAL_VARIANT}deine{else}Ihre{/if} E-Mail-Adresse wurden daher bereits ausgefüllt.]]></item>

diff --git a/wcfsetup/install/lang/en.xml b/wcfsetup/install/lang/en.xml
index 4d6b7768d8420f7c0e96cb453b334042793ddf5c..7e659db9a1e340b2113ae6f20aff453cc94acbd2 100644 (file)
--- a/wcfsetup/install/lang/en.xml
+++ b/wcfsetup/install/lang/en.xml
@@ -5065,6 +5065,9 @@ You also received a list of emergency codes to use when your second factor becom
                <item name="wcf.user.3rdparty"><![CDATA[Third-Party Login]]></item>
                <item name="wcf.user.3rdparty.login.error.access_denied"><![CDATA[Access to your external account has been rejected.]]></item>
                <item name="wcf.user.3rdparty.login.error.denied"><![CDATA[Access to your external account has been rejected.]]></item>
+               <item name="wcf.user.3rdparty.login.error.httpError"><![CDATA[Unable to communicate with the external service. Internal error code: <code>{$exceptionID}</code>.]]></item>
+               <item name="wcf.user.3rdparty.login.error.genericException"><![CDATA[An error occurred while processing your authentication request. Please contact the administrator for help. Internal error code: <code>{$exceptionID}</code>.]]></item>
+               <item name="wcf.user.3rdparty.login.error.stateValidation"><![CDATA[Unable to find the authentication request. Maybe your session expired.]]></item>
                <item name="wcf.user.3rdparty.github"><![CDATA[GitHub]]></item>
                <item name="wcf.user.3rdparty.github.login"><![CDATA[Login with GitHub]]></item>
                <item name="wcf.user.3rdparty.github.register"><![CDATA[You are creating an account through <span class="icon icon16 fa-github"></span>&nbsp;GitHub. The username and your email address have therefore already been entered.]]></item>