Merge branch '9-improve-oauth' into 'master'

Resolve "Improve OAuth"

Closes #9

See merge request pnx/httpcb!9

app/controllers/AuthController.php

@@ -54,8 +54,19 @@ class AuthController extends ControllerBase
                 // NOTE: Should pass $state here also.
                 $data = $client->authenticate($code);
 
-                $this->auth->loginOauth($data);
+                $result = $this->auth->loginOauth($data);
-                $this->response->redirect('/');
+
+                // There was an error when creating the account
+                if (is_array($result)) {
+                    $msg = '';
+                    foreach($result as $message) {
+                        $msg .= '<li>' . $message->getMessage() . '</li>';
+                    }
+                    $this->flash->message('error', "Failed to create account: <ul>{$msg}</ul>");
+                    $this->response->redirect('/login');
+                } else {
+                    $this->response->redirect('/');
+                }
             } catch(\Exception $e) {
                 $this->flash->message('error', 'Failed to authenticate.');
                 $this->response->redirect('/login');

app/library/Auth.php

@@ -43,34 +43,28 @@ class Auth extends Component
     /**
      * Login using OAuth
      *
-     * @param UserDataInterface $auth
+     * @param UserDataInterface $data
+     * @return bool|\Phalcon\Mvc\Model\MessageInterface[]
      */
-    public function loginOauth(UserDataInterface $auth)
+    public function loginOauth(UserDataInterface $data)
     {
-        // Look for a user with this email.
+        $user = User::findFirstByOAuthID($data);
-        $user = User::findFirstByEmail($auth->getEmail());
 
         if (!$user) {
             // Did not find any user. create him.
-            if (strlen($auth->getUsername()) > 0) {
+            $user = User::createFromOAuthData($data);
-                $name = $auth->getUsername();
+
-            } else if(strlen($auth->getName()) > 0) {
+            if ($user->save() === false) {
-                $name = $auth->getName();
+                return $user->getMessages();
-            } else {
-                $name = '';
             }
-
-            $user = new User();
-            $user->setEmail($auth->getEmail())
-                ->setUsername($name);
-
-            $user->save();
         }
 
         $this->setIdentity($user->getId());
 
         $this->_eventsManager->fire('auth:onLogin', $this,
-            "OAuth {$auth->getProvider()}");
+            "OAuth {$data->getProvider()}");
+
+        return true;
     }
 
     /**

app/library/OAuth/Adapter/League.php

@@ -15,6 +15,7 @@ class League implements AdapterInterface
     protected $_providerClasses = array(
         'github' => '\League\OAuth2\Client\Provider\Github',
         'gitlab' => '\Omines\OAuth2\Client\Provider\Gitlab',
+        'google' => '\League\OAuth2\Client\Provider\Google',
     );
 
     /**
@@ -36,7 +37,7 @@ class League implements AdapterInterface
     public function __construct($provider_name, $options)
     {
         if (!array_key_exists($provider_name, $this->_providerClasses)) {
-            throw new Exception("Provider '{$provider_name}' is not supported.");
+            throw new \Exception("Provider '{$provider_name}' is not supported.");
         }
 
         $className = $this->_providerClasses[$provider_name];
@@ -44,7 +45,7 @@ class League implements AdapterInterface
 
         if (!($provider instanceof AbstractProvider)) {
             // TODO: Throw a better exception class :)
-            throw new Exception("Provider object must be an instance of League\\OAuth2\\Client\\Provider\\AbstractProvider");
+            throw new \Exception("Provider object must be an instance of League\\OAuth2\\Client\\Provider\\AbstractProvider");
         }
 
         $this->_provider = $provider;

app/library/OAuth/UserData/Google.php

@@ -0,0 +1,59 @@
+<?php
+
+namespace Httpcb\OAuth\UserData;
+
+class Google implements UserDataInterface
+{
+    protected $data;
+
+    /**
+     * {@inheritDoc}
+     */
+    public function __construct(array $data)
+    {
+        $this->data = $data;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getProvider()
+    {
+        return 'Google';
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getId()
+    {
+        return (int) $this->data['id'];
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getUsername()
+    {
+        return null;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getName()
+    {
+        return $this->data['displayName'];
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getEmail()
+    {
+        if (isset($this->data['emails'][0]['value'])) {
+            return $this->data['emails'][0]['value'];
+        }
+        return null;
+    }
+}

app/migrations/20180419133825_user_oauth_ids.php

@@ -0,0 +1,22 @@
+<?php
+
+
+use Phinx\Migration\AbstractMigration;
+
+class UserOauthIds extends AbstractMigration
+{
+    public function up()
+    {
+        $this->table('user')
+            ->addColumn('gitlab_id', 'integer', [
+                'limit' => 14,
+                'null' => true,
+                'after' => 'github_user'
+            ])
+            ->addColumn('google_id', 'integer', [
+                'limit' => 21,
+                'null' => true,
+                'after' => 'gitlab_id'
+            ])->save();
+    }
+}

app/migrations/20180607222058_user_remove_github_user.php

@@ -0,0 +1,14 @@
+<?php
+
+
+use Phinx\Migration\AbstractMigration;
+
+class UserRemoveGithubUser extends AbstractMigration
+{
+    public function up()
+    {
+        $this->table('user')
+            ->removeColumn('github_user')
+            ->save();
+    }
+}

app/models/Data/User.php

@@ -2,8 +2,11 @@
 
 namespace App\Model\Data;
 
-use Phalcon\Mvc\Model;
+use Phalcon\Mvc\Model,
-use InvalidArgumentException;
+    Phalcon\Validation,
+    Phalcon\Validation\Validator\Uniqueness,
+    InvalidArgumentException,
+    Httpcb\OAuth\UserData\UserDataInterface;
 
 class User extends Model
 {
@@ -27,6 +30,10 @@ class User extends Model
 
     protected $github_user;
 
+    protected $gitlab_id;
+
+    protected $google_id;
+
     public function initialize()
     {
         $this->useDynamicUpdate(true);
@@ -38,6 +45,17 @@ class User extends Model
         $this->setEventsManager($this->getDI()->get('eventsManager'));
     }
 
+    public function validation()
+    {
+        // Validation
+        $validator = new Validation();
+
+        $validator->add('username', new Uniqueness(['message' => 'The username already exists.']));
+        $validator->add('email', new Uniqueness(['message' => 'The email address already exists.']));
+
+        return $this->validate($validator);
+    }
+
     /**
      * @return mixed
      */
@@ -193,6 +211,55 @@ class User extends Model
         return $this;
     }
 
+    /**
+     * @return mixed
+     */
+    public function getGitlabId()
+    {
+        return $this->gitlab_id;
+    }
+
+    /**
+     * @param mixed $gitlab_id
+     * @return User
+     */
+    public function setGitlabId($gitlab_id)
+    {
+        $this->gitlab_id = $gitlab_id;
+        return $this;
+    }
+
+    /**
+     * @return mixed
+     */
+    public function getGoogleId()
+    {
+        return $this->google_id;
+    }
+
+    /**
+     * @param mixed $google_id
+     * @return User
+     */
+    public function setGoogleId($google_id)
+    {
+        $this->google_id = $google_id;
+        return $this;
+    }
+
+    static public function createFromOAuthData(UserDataInterface $data)
+    {
+        $oauth_id = 'set' . $data->getProvider() . 'Id';
+
+        $user = new self();
+        $user->setUsername($data->getUsername())
+            ->setName($data->getName())
+            ->setEmail($data->getEmail())
+            ->{$oauth_id}($data->getId());
+
+        return $user;
+    }
+
     /**
      * Find the first user by Username or Email
      *
@@ -207,6 +274,16 @@ class User extends Model
         ]);
     }
 
+    static public function findFirstByOAuthID(UserDataInterface $oauth)
+    {
+        $column = strtolower($oauth->getProvider());
+
+        return self::findFirst([
+            "{$column}_id = :id:",
+            "bind" => [ 'id' => $oauth->getId() ]
+        ]);
+    }
+
     public function beforeSave()
     {
         // Fire event on password create/changed.