Rework some tests, still missing significant coverage

adamwathan · Dec 27, 2014 · 9b28d0f · 9b28d0f
1 parent 383c0f4
commit 9b28d0f
Show file tree

Hide file tree

Showing 8 changed files with 136 additions and 150 deletions.
diff --git a/src/AdamWathan/EloquentOAuth/Authenticator.php b/src/AdamWathan/EloquentOAuth/Authenticator.php
@@ -9,46 +9,32 @@
 class Authenticator
 {
     protected $auth;
-    protected $stateManager;
     protected $users;
     protected $identities;
 
-    public function __construct(Auth $auth, StateManager $stateManager, UserStore $users, IdentityStore $identities)
+    public function __construct(Auth $auth, UserStore $users, IdentityStore $identities)
     {
         $this->auth = $auth;
-        $this->stateManager = $stateManager;
         $this->users = $users;
         $this->identities = $identities;
     }
 
-    public function login($providerAlias, $provider, Closure $callback = null)
+    public function login($providerAlias, $userDetails, Closure $callback = null)
     {
-        $this->verifyState();
-        $details = $provider->getUserDetails();
-        $user = $this->getUser($providerAlias, $details);
+        $user = $this->getUser($providerAlias, $userDetails);
         if ($callback) {
-            $callback($user, $details);
+            $callback($user, $userDetails);
         }
-        $this->updateUser($user, $provider, $details);
+        $this->updateUser($user, $providerAlias, $userDetails);
         $this->auth->login($user);
-        return $user;
-    }
-
-    protected function verifyState()
-    {
-        if (! $this->stateManager->verifyState()) {
-            throw new InvalidAuthorizationCodeException;
-        }
     }
 
     protected function getUser($provider, $details)
     {
-        if ($this->userExists($provider, $details)) {
-            $user = $this->getExistingUser($provider, $details);
-        } else {
-            $user = $this->createUser();
+        if ($this->identities->userExists($provider, $details)) {
+            return $this->getExistingUser($provider, $details);
         }
-        return $user;
+        return $this->users->create();
     }
 
     protected function updateUser($user, $provider, $details)
@@ -57,28 +43,12 @@ protected function updateUser($user, $provider, $details)
         $this->updateAccessToken($user, $provider, $details);
     }
 
-    protected function userExists($provider, ProviderUserDetails $details)
-    {
-        return $this->identities->userExists($provider, $details);
-    }
-
     protected function getExistingUser($provider, $details)
     {
-        $identity = $this->getIdentity($provider, $details);
+        $identity = $this->identities->getByProvider($provider, $details);
         return $this->users->findByIdentity($identity);
     }
 
-    protected function getIdentity($provider, ProviderUserDetails $details)
-    {
-        return $this->identities->getByProvider($provider, $details);
-    }
-
-    protected function createUser()
-    {
-        $user = $this->users->create();
-        return $user;
-    }
-
     protected function updateAccessToken($user, $provider, ProviderUserDetails $details)
     {
         $this->flushAccessTokens($user, $provider);

diff --git a/src/AdamWathan/EloquentOAuth/Authorizer.php b/src/AdamWathan/EloquentOAuth/Authorizer.php
@@ -6,22 +6,14 @@
 class Authorizer
 {
     protected $redirect;
-    protected $stateManager;
 
-    public function __construct(Redirect $redirect, StateManager $stateManager)
+    public function __construct(Redirect $redirect)
     {
         $this->redirect = $redirect;
-        $this->stateManager = $stateManager;
     }
 
-    public function authorize(ProviderInterface $provider)
+    public function authorize(ProviderInterface $provider, $state)
     {
-        $state = $this->generateState();
         return $this->redirect->to($provider->authorizeUrl($state));
     }
-
-    protected function generateState()
-    {
-        return $this->stateManager->generateState();
-    }
 }
diff --git a/src/AdamWathan/EloquentOAuth/EloquentOAuthServiceProvider.php b/src/AdamWathan/EloquentOAuth/EloquentOAuthServiceProvider.php
@@ -47,9 +47,9 @@ protected function registerOAuthManager()
         $this->configureOAuthIdentitiesTable();
         $users = new UserStore($app['config']['auth.model']);
         $stateManager = new StateManager($app['session.store'], $app['request']);
-        $authorizer = new Authorizer($app['redirect'], $stateManager);
-        $authenticator = new Authenticator($app['auth'], $stateManager, $users, new IdentityStore);
-        $oauth = new OAuthManager($authorizer, new ProviderRegistrar, $authenticator);
+        $authorizer = new Authorizer($app['redirect']);
+        $authenticator = new Authenticator($app['auth'], $users, new IdentityStore);
+        $oauth = new OAuthManager($authorizer, $authenticator, $stateManager, new ProviderRegistrar);
         $this->registerProviders($oauth);
         return $oauth;
     });

diff --git a/src/AdamWathan/EloquentOAuth/OAuthManager.php b/src/AdamWathan/EloquentOAuth/OAuthManager.php
@@ -9,14 +9,16 @@
 class OAuthManager
 {
     protected $authorizer;
-    protected $providers;
     protected $authenticator;
+    protected $stateManager;
+    protected $providers;
 
-    public function __construct(Authorizer $authorizer, ProviderRegistrar $providers, Authenticator $authenticator)
+    public function __construct(Authorizer $authorizer, Authenticator $authenticator, StateManager $stateManager, ProviderRegistrar $providers)
     {
         $this->authorizer = $authorizer;
-        $this->providers = $providers;
         $this->authenticator = $authenticator;
+        $this->stateManager = $stateManager;
+        $this->providers = $providers;
     }
 
     public function registerProvider($alias, ProviderInterface $provider)
@@ -26,12 +28,17 @@ public function registerProvider($alias, ProviderInterface $provider)
 
     public function authorize($providerAlias)
     {
-        return $this->authorizer->authorize($this->getProvider($providerAlias));
+        $state = $this->stateManager->generateState();
+        return $this->authorizer->authorize($this->getProvider($providerAlias), $state);
     }
 
     public function login($providerAlias, Closure $callback = null)
     {
-        return $this->authenticator->login($providerAlias, $this->getProvider($providerAlias), $callback);
+        if (! $this->stateManager->verifyState()) {
+            throw new InvalidAuthorizationCodeException;
+        }
+        $details = $this->getProvider($providerAlias)->getUserDetails();
+        return $this->authenticator->login($providerAlias, $details, $callback);
     }
 
     protected function getProvider($providerAlias)

diff --git a/tests/AuthenticatorTest.php b/tests/AuthenticatorTest.php
@@ -0,0 +1,53 @@
+<?php
+
+use Mockery as M;
+use AdamWathan\EloquentOAuth\Authenticator;
+
+class AuthenticatorTest extends PHPUnit_Framework_TestCase
+{
+    public function tearDown()
+    {
+        M::close();
+    }
+
+    public function test_login_creates_new_user_if_no_matching_user_exists()
+    {
+        $auth = M::mock('Illuminate\\Auth\\AuthManager');
+        $users  = M::mock('AdamWathan\\EloquentOAuth\\UserStore');
+        $identities  = M::mock('AdamWathan\\EloquentOAuth\\IdentityStore')->shouldIgnoreMissing();
+
+        $userDetails = M::mock('AdamWathan\\EloquentOAuth\\ProviderUserDetails');
+
+        $user = M::mock('stdClass')->shouldIgnoreMissing();
+
+        $authenticator = new Authenticator($auth, $users, $identities);
+
+        $users->shouldReceive('create')->andReturn($user);
+        $users->shouldReceive('store')->once();
+        $auth->shouldReceive('login')->with($user)->once();
+
+        $authenticator->login('provider', $userDetails);
+    }
+
+    public function test_login_uses_existing_user_if_matching_user_exists()
+    {
+        $auth = M::mock('Illuminate\\Auth\\AuthManager');
+        $users  = M::mock('AdamWathan\\EloquentOAuth\\UserStore')->shouldIgnoreMissing();
+        $identities  = M::mock('AdamWathan\\EloquentOAuth\\IdentityStore')->shouldIgnoreMissing();
+
+        $userDetails = M::mock('AdamWathan\\EloquentOAuth\\ProviderUserDetails');
+        $identity = M::mock('AdamWathan\\EloquentOAuth\\OAuthIdentity');
+
+        $user = M::mock('stdClass')->shouldIgnoreMissing();
+
+        $authenticator = new Authenticator($auth, $users, $identities);
+
+        $identities->shouldReceive('userExists')->andReturn(true);
+        $identities->shouldReceive('getByProvider')->andReturn($identity);
+        $users->shouldReceive('create')->never();
+        $users->shouldReceive('findByIdentity')->andReturn($user);
+        $auth->shouldReceive('login')->with($user)->once();
+
+        $authenticator->login('provider', $userDetails);
+    }
+}
diff --git a/tests/AuthorizerTest.php b/tests/AuthorizerTest.php
@@ -0,0 +1,14 @@
+<?php
+
+use Mockery as M;
+use AdamWathan\EloquentOAuth\Authorizer;
+
+class AuthorizerTest extends PHPUnit_Framework_TestCase
+{
+    public function tearDown()
+    {
+        M::close();
+    }
+
+    public function test_placeholder() {}
+}
diff --git a/tests/IdentityStoreTest.php b/tests/IdentityStoreTest.php
@@ -114,4 +114,46 @@ public function test_store()
 
         $this->assertEquals(1, OAuthIdentity::count());
     }
+
+    public function test_user_exists_returns_true_when_user_exists()
+    {
+        OAuthIdentity::create(array(
+            'user_id' => 2,
+            'provider' => 'facebook',
+            'provider_user_id' => 'bazfoo',
+            'access_token' => 'def456',
+            ));
+        $details = new ProviderUserDetails(array(
+            'accessToken' => 'new-token',
+            'userId' => 'bazfoo',
+            'nickname' => 'john.doe',
+            'firstName' => 'John',
+            'lastName' => 'Doe',
+            'email' => '[email protected]',
+            'imageUrl' => 'http://example.com/photos/john_doe.jpg',
+        ));
+        $identities = new IdentityStore;
+        $this->assertTrue($identities->userExists('facebook', $details));
+    }
+
+    public function test_user_exists_returns_false_when_user_doesnt_exist()
+    {
+        OAuthIdentity::create(array(
+            'user_id' => 2,
+            'provider' => 'facebook',
+            'provider_user_id' => 'foobar',
+            'access_token' => 'def456',
+            ));
+        $details = new ProviderUserDetails(array(
+            'accessToken' => 'new-token',
+            'userId' => 'bazfoo',
+            'nickname' => 'john.doe',
+            'firstName' => 'John',
+            'lastName' => 'Doe',
+            'email' => '[email protected]',
+            'imageUrl' => 'http://example.com/photos/john_doe.jpg',
+        ));
+        $identities = new IdentityStore;
+        $this->assertFalse($identities->userExists('facebook', $details));
+    }
 }
diff --git a/tests/OAuthManagerTest.php b/tests/OAuthManagerTest.php
@@ -10,97 +10,5 @@ public function tearDown()
         M::close();
     }
 
-    public function test_authorize_returns_correct_redirect_when_provider_is_registered()
-    {
-        $auth = M::mock('Illuminate\\Auth\\AuthManager')->shouldIgnoreMissing();
-        $redirector = M::mock('Illuminate\\Routing\\Redirector')->shouldIgnoreMissing();
-        $stateManager  = M::mock('AdamWathan\\EloquentOAuth\\StateManager')->shouldIgnoreMissing();
-        $users  = M::mock('AdamWathan\\EloquentOAuth\\UserStore')->shouldIgnoreMissing();
-        $identities  = M::mock('AdamWathan\\EloquentOAuth\\IdentityStore')->shouldIgnoreMissing();
-
-        $oauth = new OAuthManager($auth, $redirector, $stateManager, $users, $identities);
-
-        $redirectResponse = M::mock('Illuminate\\Http\\RedirectResponse');
-        $redirectUrl = 'http://example.com/authorize';
-        $provider = M::mock('AdamWathan\\EloquentOAuth\\Providers\\ProviderInterface');
-        $provider->shouldReceive('authorizeUrl')->andReturn($redirectUrl);
-        $redirector->shouldReceive('to')->with($redirectUrl)->andReturn($redirectResponse);
-
-        $oauth->registerProvider('provider', $provider);
-
-        $result = $oauth->authorize('provider');
-        $expected = $redirectResponse;
-
-        $this->assertEquals($expected, $result);
-    }
-
-    /**
-     * @expectedException AdamWathan\EloquentOAuth\Exceptions\ProviderNotRegisteredException
-     */
-    public function test_authorize_throws_exception_when_provider_is_not_registered()
-    {
-        $auth = M::mock('Illuminate\\Auth\\AuthManager')->shouldIgnoreMissing();
-        $redirector = M::mock('Illuminate\\Routing\\Redirector')->shouldIgnoreMissing();
-        $stateManager  = M::mock('AdamWathan\\EloquentOAuth\\StateManager')->shouldIgnoreMissing();
-        $users  = M::mock('AdamWathan\\EloquentOAuth\\UserStore')->shouldIgnoreMissing();
-        $identities  = M::mock('AdamWathan\\EloquentOAuth\\IdentityStore')->shouldIgnoreMissing();
-
-        $oauth = new OAuthManager($auth, $redirector, $stateManager, $users, $identities);
-
-        $result = $oauth->authorize('missingProvider');
-    }
-
-    public function test_login_creates_new_user_if_no_matching_user_exists()
-    {
-        $auth = M::mock('Illuminate\\Auth\\AuthManager');
-        $redirector = M::mock('Illuminate\\Routing\\Redirector');
-        $stateManager  = M::mock('AdamWathan\\EloquentOAuth\\StateManager')->shouldIgnoreMissing();
-        $users  = M::mock('AdamWathan\\EloquentOAuth\\UserStore');
-        $identities  = M::mock('AdamWathan\\EloquentOAuth\\IdentityStore')->shouldIgnoreMissing();
-
-        $provider = M::mock('AdamWathan\\EloquentOAuth\\Providers\\ProviderInterface');
-        $userDetails = M::mock('AdamWathan\\EloquentOAuth\\ProviderUserDetails');
-
-        $user = M::mock('stdClass')->shouldIgnoreMissing();
-
-        $oauth = new OAuthManager($auth, $redirector, $stateManager, $users, $identities);
-        $oauth->registerProvider('provider', $provider);
-
-        $stateManager->shouldReceive('verifyState')->andReturn(true);
-        $provider->shouldReceive('getUserDetails')->andReturn($userDetails);
-        $users->shouldReceive('create')->andReturn($user);
-        $users->shouldReceive('store')->andReturn(true);
-
-        $auth->shouldReceive('login')->with($user)->once();
-        $result = $oauth->login('provider');
-    }
-
-    public function test_login_uses_existing_user_if_matching_user_exists()
-    {
-        $auth = M::mock('Illuminate\\Auth\\AuthManager');
-        $redirector = M::mock('Illuminate\\Routing\\Redirector');
-        $stateManager  = M::mock('AdamWathan\\EloquentOAuth\\StateManager')->shouldIgnoreMissing();
-        $users  = M::mock('AdamWathan\\EloquentOAuth\\UserStore');
-        $identities  = M::mock('AdamWathan\\EloquentOAuth\\IdentityStore')->shouldIgnoreMissing();
-
-        $provider = M::mock('AdamWathan\\EloquentOAuth\\Providers\\ProviderInterface');
-        $freshUserDetails = M::mock('AdamWathan\\EloquentOAuth\\ProviderUserDetails');
-        $existingUserDetails = M::mock('AdamWathan\\EloquentOAuth\\ProviderUserDetails');
-
-        $user = M::mock('stdClass')->shouldIgnoreMissing();
-
-        $oauth = new OAuthManager($auth, $redirector, $stateManager, $users, $identities);
-        $oauth->registerProvider('provider', $provider);
-
-        $stateManager->shouldReceive('verifyState')->andReturn(true);
-        $provider->shouldReceive('getUserDetails')->andReturn($freshUserDetails);
-        $identities->shouldReceive('userExists')->with('provider', $freshUserDetails)->andReturn(true);
-        $identities->shouldReceive('getByProvider')->with('provider', $freshUserDetails)->andReturn($existingUserDetails);
-        $users->shouldReceive('create')->never();
-        $users->shouldReceive('findByIdentity')->once()->andReturn($user);
-        $users->shouldReceive('store')->andReturn(true);
-
-        $auth->shouldReceive('login')->with($user)->once();
-        $result = $oauth->login('provider');
-    }
+    public function test_placeholder() {}
 }