1 files changed, 65 insertions, 41 deletions

diff --git a/app/Controller/Oauth.php b/app/Controller/Oauth.php
index 452faecd..12b91144 100644
--- a/app/Controller/Oauth.php
+++ b/app/Controller/Oauth.php
@@ -2,6 +2,8 @@
 
 namespace Kanboard\Controller;
 
+use Kanboard\Core\Security\OAuthAuthenticationProviderInterface;
+
 /**
  * OAuth controller
  *
@@ -11,25 +13,6 @@ namespace Kanboard\Controller;
 class Oauth extends Base
 {
     /**
-     * Unlink external account
-     *
-     * @access public
-     */
-    public function unlink()
-    {
-        $backend = $this->request->getStringParam('backend');
-        $this->checkCSRFParam();
-
-        if ($this->authenticationManager->getProvider($backend)->unlink($this->userSession->getId())) {
-            $this->flash->success(t('Your external account is not linked anymore to your profile.'));
-        } else {
-            $this->flash->failure(t('Unable to unlink your external account.'));
-        }
-
-        $this->response->redirect($this->helper->url->to('user', 'external', array('user_id' => $this->userSession->getId())));
-    }
-
-    /**
      * Redirect to the provider if no code received
      *
      * @access private
@@ -38,9 +21,10 @@ class Oauth extends Base
     protected function step1($provider)
     {
         $code = $this->request->getStringParam('code');
+        $state = $this->request->getStringParam('state');
 
         if (! empty($code)) {
-            $this->step2($provider, $code);
+            $this->step2($provider, $code, $state);
         } else {
             $this->response->redirect($this->authenticationManager->getProvider($provider)->getService()->getAuthorizationUrl());
         }
@@ -50,34 +34,44 @@ class Oauth extends Base
      * Link or authenticate the user
      *
      * @access protected
-     * @param string $provider
+     * @param string $providerName
      * @param string $code
+     * @param string $state
      */
-    protected function step2($provider, $code)
+    protected function step2($providerName, $code, $state)
     {
-        $this->authenticationManager->getProvider($provider)->setCode($code);
+        $provider = $this->authenticationManager->getProvider($providerName);
+        $provider->setCode($code);
+        $hasValidState = $provider->getService()->isValidateState($state);
 
         if ($this->userSession->isLogged()) {
-            $this->link($provider);
+            if ($hasValidState) {
+                $this->link($provider);
+            } else {
+                $this->flash->failure(t('The OAuth2 state parameter is invalid'));
+                $this->response->redirect($this->helper->url->to('user', 'external', array('user_id' => $this->userSession->getId())));
+            }
+        } else {
+            if ($hasValidState) {
+                $this->authenticate($providerName);
+            } else {
+                $this->authenticationFailure(t('The OAuth2 state parameter is invalid'));
+            }
         }
-
-        $this->authenticate($provider);
     }
 
     /**
      * Link the account
      *
      * @access protected
-     * @param string $provider
+     * @param  OAuthAuthenticationProviderInterface $provider
      */
-    protected function link($provider)
+    protected function link(OAuthAuthenticationProviderInterface $provider)
     {
-        $authProvider = $this->authenticationManager->getProvider($provider);
-
-        if (! $authProvider->authenticate()) {
+        if (! $provider->authenticate()) {
             $this->flash->failure(t('External authentication failed'));
         } else {
-            $this->userProfile->assign($this->userSession->getId(), $authProvider->getUser());
+            $this->userProfile->assign($this->userSession->getId(), $provider->getUser());
             $this->flash->success(t('Your external account is linked to your profile successfully.'));
         }
 
@@ -85,22 +79,52 @@ class Oauth extends Base
     }
 
     /**
+     * Unlink external account
+     *
+     * @access public
+     */
+    public function unlink()
+    {
+        $backend = $this->request->getStringParam('backend');
+        $this->checkCSRFParam();
+
+        if ($this->authenticationManager->getProvider($backend)->unlink($this->userSession->getId())) {
+            $this->flash->success(t('Your external account is not linked anymore to your profile.'));
+        } else {
+            $this->flash->failure(t('Unable to unlink your external account.'));
+        }
+
+        $this->response->redirect($this->helper->url->to('user', 'external', array('user_id' => $this->userSession->getId())));
+    }
+
+    /**
      * Authenticate the account
      *
      * @access protected
-     * @param string $provider
+     * @param string $providerName
      */
-    protected function authenticate($provider)
+    protected function authenticate($providerName)
     {
-        if ($this->authenticationManager->oauthAuthentication($provider)) {
+        if ($this->authenticationManager->oauthAuthentication($providerName)) {
             $this->response->redirect($this->helper->url->to('app', 'index'));
         } else {
-            $this->response->html($this->helper->layout->app('auth/index', array(
-                'errors' => array('login' => t('External authentication failed')),
-                'values' => array(),
-                'no_layout' => true,
-                'title' => t('Login')
-            )));
+            $this->authenticationFailure(t('External authentication failed'));
         }
     }
+
+    /**
+     * Show login failure page
+     *
+     * @access protected
+     * @param  string $message
+     */
+    protected function authenticationFailure($message)
+    {
+        $this->response->html($this->helper->layout->app('auth/index', array(
+            'errors' => array('login' => $message),
+            'values' => array(),
+            'no_layout' => true,
+            'title' => t('Login')
+        )));
+    }
 }