From 7effed46994a07bbced2c3c7a9b975b07d0b8c0f Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Wed, 12 Dec 2018 15:25:18 +0000 Subject: [PATCH 01/31] Add test for SocialIdentity past confirmation deadline --- ..._000000_create_social_identities_table.php | 10 +++---- src/SocialIdentity.php | 30 +++++++++++-------- tests/DatabaseTestCase.php | 10 +++++++ tests/SocialIdentityTest.php | 18 +++++++++++ tests/TestCase.php | 23 ++++++++++---- tests/factories/SocialIdentityFactory.php | 10 +++++++ 6 files changed, 78 insertions(+), 23 deletions(-) create mode 100644 tests/DatabaseTestCase.php create mode 100644 tests/SocialIdentityTest.php create mode 100644 tests/factories/SocialIdentityFactory.php diff --git a/migrations/2016_11_01_000000_create_social_identities_table.php b/migrations/2016_11_01_000000_create_social_identities_table.php index d89c2a4..1ef26c2 100644 --- a/migrations/2016_11_01_000000_create_social_identities_table.php +++ b/migrations/2016_11_01_000000_create_social_identities_table.php @@ -14,13 +14,13 @@ public function up() { Schema::create('social_identities', function (Blueprint $table) { $table->increments('id'); - $table->integer('user_id')->unsigned()->index(); - $table->string('provider'); - $table->string('reference'); - $table->text('access_token'); + $table->integer('user_id')->unsigned()->index()->nullable(); + $table->string('provider')->nullable(); + $table->string('reference')->nullable(); + $table->text('access_token')->nullable(); $table->string('expires_at')->nullable(); $table->string('refresh_token')->nullable(); - $table->string('confirm_token')->default(''); + $table->string('confirm_token')->nullable(); $table->dateTime('confirm_until')->nullable(); $table->dateTime('confirmed_at')->nullable(); $table->timestamps(); diff --git a/src/SocialIdentity.php b/src/SocialIdentity.php index c1f3cda..0f8201c 100644 --- a/src/SocialIdentity.php +++ b/src/SocialIdentity.php @@ -3,8 +3,8 @@ namespace Konsulting\Butler; use Carbon\Carbon; -use Illuminate\Support\Str; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Str; use Konsulting\Butler\Exceptions\UnableToConfirm; use Laravel\Socialite\Contracts\User as Identity; @@ -52,11 +52,11 @@ public static function createFromOauthIdentity($provider, $user, Identity $ident } return static::create([ - 'provider' => $provider, - 'user_id' => $user->id, - 'reference' => $identity->getId(), - 'access_token' => $identity->token, - 'expires_at' => Carbon::now()->addSeconds($identity->expiresIn), + 'provider' => $provider, + 'user_id' => $user->id, + 'reference' => $identity->getId(), + 'access_token' => $identity->token, + 'expires_at' => Carbon::now()->addSeconds($identity->expiresIn), 'refresh_token' => $identity->refreshToken, 'confirm_token' => Str::random(60), 'confirm_until' => Carbon::now()->addMinutes(30), @@ -64,18 +64,22 @@ public static function createFromOauthIdentity($provider, $user, Identity $ident } /** - * Check if we are byond the confirmation deadline. + * Check if we are beyond the confirmation deadline. If no deadline has been set, treat it as being past the + * deadline. + * + * @return bool */ public function pastConfirmationDeadline() { - return isset($this->conform_until) ? $this->confirm_until->lt(Carbon::now()) : false; + return (! $this->confirm_until instanceof Carbon) ?: $this->confirm_until->lt(Carbon::now()); } /** - * Confirm a SocialIdentity after locating it by it's token. + * Confirm a SocialIdentity after locating it by its token. * * @param $token * + * @return static * @throws \Konsulting\Butler\Exceptions\UnableToConfirm */ public static function confirmByToken($token) @@ -124,8 +128,8 @@ public function askUserToConfirm() public function updateFromOauthIdentity(Identity $identity) { $this->update([ - 'access_token' => $identity->token, - 'expires_at' => Carbon::now()->addSeconds($identity->expiresIn), + 'access_token' => $identity->token, + 'expires_at' => Carbon::now()->addSeconds($identity->expiresIn), 'refresh_token' => $identity->refreshToken, ]); } @@ -136,7 +140,7 @@ public function updateFromOauthIdentity(Identity $identity) * @param $provider * @param \Laravel\Socialite\Contracts\User $identity * - * @return mixed + * @return static|null */ public static function retrieveByOauthIdentity($provider, Identity $identity) { @@ -153,7 +157,7 @@ public static function retrieveByOauthIdentity($provider, Identity $identity) * @param $provider * @param \Laravel\Socialite\Contracts\User $identity * - * @return mixed + * @return static|null */ public static function retrievePossibleByOauthIdentity($provider, Identity $identity) { diff --git a/tests/DatabaseTestCase.php b/tests/DatabaseTestCase.php new file mode 100644 index 0000000..c273156 --- /dev/null +++ b/tests/DatabaseTestCase.php @@ -0,0 +1,10 @@ +create(['confirm_until' => $this->carbonNow->subMinute()]); + $notPastDeadline = factory(SocialIdentity::class)->create(['confirm_until' => $this->carbonNow->addMinute()]); + $noDeadlineSet = factory(SocialIdentity::class)->create(['confirm_until' => null]); + + $this->assertTrue($pastDeadline->pastConfirmationDeadline()); + $this->assertFalse($notPastDeadline->pastConfirmationDeadline()); + $this->assertTrue($noDeadlineSet->pastConfirmationDeadline()); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index ce00405..5167266 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -2,17 +2,25 @@ namespace Konsulting\Butler; -use Route; use Butler; -use Schema; -use Konsulting\Butler\Fake\User; +use Illuminate\Support\Carbon; use Konsulting\Butler\Fake\Identity; use Konsulting\Butler\Fake\Socialite; -use Orchestra\Database\ConsoleServiceProvider; +use Konsulting\Butler\Fake\User; use Laravel\Socialite\Contracts\Factory as SocialiteFactory; +use Orchestra\Database\ConsoleServiceProvider; +use Route; +use Schema; abstract class TestCase extends \Orchestra\Testbench\BrowserKit\TestCase { + /** + * The time that Carbon::now() has been set to for testing. + * + * @var \Carbon\Carbon + */ + protected $carbonNow; + /** * Set up ServiceProviders. * @@ -42,11 +50,14 @@ protected function getPackageAliases($app) /** * Define environment setup. * - * @param \Illuminate\Foundation\Application $app + * @param \Illuminate\Foundation\Application $app * @return void */ protected function getEnvironmentSetUp($app) { + Carbon::setTestNow('2018-01-01 10:30:40'); + $this->carbonNow = Carbon::now(); + // Setup default database to use sqlite :memory: $app['config']->set('database.default', 'testbench'); $app['config']->set('database.connections.testbench', [ @@ -96,6 +107,8 @@ public function setUp() '--database' => 'testbench', '--realpath' => realpath(__DIR__ . '/../migrations'), ]); + + $this->withFactories(__DIR__ . '/factories'); } public function createUsersTable() diff --git a/tests/factories/SocialIdentityFactory.php b/tests/factories/SocialIdentityFactory.php new file mode 100644 index 0000000..3ed3a4e --- /dev/null +++ b/tests/factories/SocialIdentityFactory.php @@ -0,0 +1,10 @@ +define(SocialIdentity::class, function (Faker\Generator $faker) { + return [ + ]; +}); From 11f6faad85b21e5915ee90d066b9f1ee7f41e918 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 13:31:43 +0000 Subject: [PATCH 02/31] Add Scrutinizer config --- .scrutinizer.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .scrutinizer.yml diff --git a/.scrutinizer.yml b/.scrutinizer.yml new file mode 100644 index 0000000..0805943 --- /dev/null +++ b/.scrutinizer.yml @@ -0,0 +1,26 @@ +build: + environment: + php: + version: 7.2 + nodes: + analysis: + tests: + override: + - php-scrutinizer-run + - + command: 'vendor/bin/phpunit --coverage-clover=coverage-file' + coverage: + file: 'coverage-file' + format: 'clover' + tests: true +filter: + excluded_paths: + - 'tests/*' +checks: + php: true +build_failure_conditions: + - 'elements.rating(<= D).exists' # No classes/methods with a rating of D or worse + - 'elements.rating(<= B).new.exists' # No new classes/methods with a rating of B or worse + - 'issues.severity(>= MAJOR).new.exists' # New issues of major or higher severity + - 'project.metric("scrutinizer.quality", < 9.5)' # Code Quality Rating drops below 9.5 + - 'project.metric_change("scrutinizer.test_coverage", < -0.001)' # Code Coverage decreased from previous inspection less than 0.1% From ab965e6cd936dc5dd7d50347469675968f6e868a Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 13:39:16 +0000 Subject: [PATCH 03/31] Update PHPUnit config --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index 4f85fad..e5b2f31 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -11,7 +11,7 @@ > - tests + tests From d87a683fd35ff6700e93203b632c34e5f3de9ace Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 13:43:29 +0000 Subject: [PATCH 04/31] Update PHPUnit config --- phpunit.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/phpunit.xml b/phpunit.xml index e5b2f31..ccc0be0 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,8 +7,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="false" -> + stopOnFailure="false"> tests From 9022cf4b23e4ffd646e081e111705118979114c3 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 13:50:52 +0000 Subject: [PATCH 05/31] Set mail driver to log for tests --- tests/TestCase.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/TestCase.php b/tests/TestCase.php index 5167266..d41a470 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -66,6 +66,8 @@ protected function getEnvironmentSetUp($app) 'prefix' => '', ]); + $app['config']->set('mail.driver', 'log'); + $app['config']->set('butler.providers', ['test' => ['name' => 'Test']]); $app['config']->set('butler.user_class', User::class); From 93a2c658417ce2bc02cd4153f26421f8036e0a7f Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 14:07:30 +0000 Subject: [PATCH 06/31] Swap RefreshDatabase trait for DatabaseTransactions for PHP 5.6 compatibility --- tests/AuthControllerTest.php | 2 +- tests/DatabaseTestCase.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/AuthControllerTest.php b/tests/AuthControllerTest.php index 4946660..2ed9e8e 100644 --- a/tests/AuthControllerTest.php +++ b/tests/AuthControllerTest.php @@ -8,7 +8,7 @@ use Konsulting\Butler\Fake\Socialite; use Konsulting\Butler\Notifications\ConfirmSocialIdentity; -class AuthControllerTest extends TestCase +class AuthControllerTest extends DatabaseTestCase { public function test_it_redirects_to_a_provider() { diff --git a/tests/DatabaseTestCase.php b/tests/DatabaseTestCase.php index c273156..257e5dd 100644 --- a/tests/DatabaseTestCase.php +++ b/tests/DatabaseTestCase.php @@ -2,9 +2,9 @@ namespace Konsulting\Butler; -use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Foundation\Testing\DatabaseTransactions; abstract class DatabaseTestCase extends TestCase { - use RefreshDatabase; + use DatabaseTransactions; } From 34da103920dfdae7d76b2e1fe8e705f2a9186bfb Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 14:21:39 +0000 Subject: [PATCH 07/31] Change Illuminate Carbon to Carbon\Carbon --- tests/TestCase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestCase.php b/tests/TestCase.php index d41a470..993f667 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -3,7 +3,7 @@ namespace Konsulting\Butler; use Butler; -use Illuminate\Support\Carbon; +use Carbon\Carbon; use Konsulting\Butler\Fake\Identity; use Konsulting\Butler\Fake\Socialite; use Konsulting\Butler\Fake\User; From 182fdcdd7ba00f5a7b8fabe4ee5af0d3cb3d3762 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 14:33:13 +0000 Subject: [PATCH 08/31] Update database test case --- tests/DatabaseTestCase.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/DatabaseTestCase.php b/tests/DatabaseTestCase.php index 257e5dd..d28e1fe 100644 --- a/tests/DatabaseTestCase.php +++ b/tests/DatabaseTestCase.php @@ -2,9 +2,9 @@ namespace Konsulting\Butler; -use Illuminate\Foundation\Testing\DatabaseTransactions; +use Illuminate\Foundation\Testing\DatabaseMigrations; abstract class DatabaseTestCase extends TestCase { - use DatabaseTransactions; + use DatabaseMigrations; } From 90a5bbda015d8705370db0cccdf564b334d50912 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Fri, 14 Dec 2018 14:46:19 +0000 Subject: [PATCH 09/31] Drop PHP 5.6 from Travis config --- .travis.yml | 9 +-------- tests/DatabaseTestCase.php | 4 ++-- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index d509519..bd8eb8e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,22 +1,15 @@ language: php php: - - 5.6 - 7.0 - 7.1 - 7.2 + - 7.3 env: global: - setup=basic -matrix: - include: - - php: 5.6.4 - env: setup=lowest - - php: 5.6.4 - env: setup=stable - sudo: false install: diff --git a/tests/DatabaseTestCase.php b/tests/DatabaseTestCase.php index d28e1fe..c273156 100644 --- a/tests/DatabaseTestCase.php +++ b/tests/DatabaseTestCase.php @@ -2,9 +2,9 @@ namespace Konsulting\Butler; -use Illuminate\Foundation\Testing\DatabaseMigrations; +use Illuminate\Foundation\Testing\RefreshDatabase; abstract class DatabaseTestCase extends TestCase { - use DatabaseMigrations; + use RefreshDatabase; } From d0fc3fe0940e1a931e7e5ddcbdf2bbfad432cc54 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Tue, 8 Jan 2019 13:22:09 +0000 Subject: [PATCH 10/31] Add butler driver class and test --- composer.json | 5 +-- config/butler.php | 2 +- src/Butler.php | 68 ++++++++++++++++++++++++++--------- src/ButlerDriver.php | 29 +++++++++++++++ src/ButlerServiceProvider.php | 5 ++- src/EloquentUserProvider.php | 3 +- tests/ButlerDriverTest.php | 43 ++++++++++++++++++++++ 7 files changed, 133 insertions(+), 22 deletions(-) create mode 100644 src/ButlerDriver.php create mode 100644 tests/ButlerDriverTest.php diff --git a/composer.json b/composer.json index 0781b85..60a2a94 100644 --- a/composer.json +++ b/composer.json @@ -4,8 +4,9 @@ "require-dev": { "phpunit/phpunit": "~5.7|~6.0|~7.0", "orchestra/testbench": ">= 3.0 <= 3.7", - "orchestra/testbench-browser-kit": "~3.1@dev", - "orchestra/database": "~3.1@dev" + "orchestra/testbench-browser-kit": "~3.1", + "orchestra/database": "~3.1", + "mockery/mockery": "^1.2" }, "license": "MIT", "authors": [ diff --git a/config/butler.php b/config/butler.php index e2aa226..a790b61 100644 --- a/config/butler.php +++ b/config/butler.php @@ -21,7 +21,7 @@ ], /* - * Can Butler create users if social login is requested for non-exiting user? + * Can Butler create users if social login is requested for non-existing user? */ 'can_create_users' => false, diff --git a/src/Butler.php b/src/Butler.php index 3b81eae..c2e8eb9 100644 --- a/src/Butler.php +++ b/src/Butler.php @@ -2,22 +2,49 @@ namespace Konsulting\Butler; -use stdClass; use Konsulting\Butler\Exceptions\NoUser; +use Konsulting\Butler\Exceptions\SocialIdentityAlreadyAssociated; use Konsulting\Butler\Exceptions\UnknownProvider; -use Laravel\Socialite\Contracts\User as Identity; use Konsulting\Butler\Exceptions\UserAlreadyHasSocialIdentity; -use Konsulting\Butler\Exceptions\SocialIdentityAlreadyAssociated; +use Laravel\Socialite\Contracts\User as Identity; +use Laravel\Socialite\SocialiteManager; +use stdClass; class Butler { + /** + * The Socialite instance. + * + * @var SocialiteManager + */ + protected $socialite; + + /** + * The social provider config. + * + * @var \Illuminate\Support\Collection + */ protected $providers; + + /** + * The mapping between route names and URLs within the host application. + * + * @var \Illuminate\Support\Collection + */ protected $routeNames; - public function __construct($providers, $routeNames) + /** + * Butler constructor. + * + * @param SocialiteManager $socialite + * @param array[] $providers + * @param array $routeNames + */ + public function __construct(SocialiteManager $socialite, $providers, $routeNames) { $this->providers = $this->prepareProviders($providers); $this->routeNames = collect($routeNames); + $this->socialite = $socialite; } /** @@ -37,7 +64,7 @@ protected function prepareProviders($providers) /** * Check that the provider is one that is supported by the app. * - * @param $name + * @param string $name * * @throws \Konsulting\Butler\Exceptions\UnknownProvider */ @@ -74,6 +101,11 @@ public function provider($provider) return $this->providers[$provider]; } + public function driver($providerName) + { + return new ButlerDriver($this->socialite->driver($providerName)); + } + /** * Simple function to include the routes where needed. */ @@ -97,14 +129,14 @@ public function routeName($key) } /** - * Authenticate a Socialite User (Identity) and update the token - * information for a given provider --- only if appropriate - * We also check whether the provider is set up for use. + * Authenticate a Socialite User (Identity) and update the token information for a given provider --- only if + * appropriate. We also check whether the provider is set up for use. * - * @param $provider + * @param string $provider The provider name * @param \Laravel\Socialite\Contracts\User $identity * * @return bool + * @throws UnknownProvider */ public function authenticate($provider, Identity $identity) { @@ -138,16 +170,17 @@ protected function guard() } /** - * Register an Identity with a user. We'll use the authenticated user, - * or if we can't find an appropriate user, create one if allowed. - * Otherwise, we will fail through a graceful Exception :). + * Register an Identity with a user. We'll use the authenticated user, or if we can't find an appropriate user, + * create one if allowed. Otherwise, we will fail through a graceful Exception :). * - * @param $provider + * @param string $provider The provider name * @param \Laravel\Socialite\Contracts\User $identity * * @return SocialIdentity - * @throws \Konsulting\Butler\Exceptions\NoUser - * @throws \Konsulting\Butler\Exceptions\SocialIdentityAlreadyAssociated + * @throws NoUser + * @throws SocialIdentityAlreadyAssociated + * @throws UnknownProvider + * @throws UserAlreadyHasSocialIdentity */ public function register($provider, Identity $identity) { @@ -194,6 +227,9 @@ protected function guardExistingSocialIdentities($provider, Identity $identity) * Confirm a SocialIdentity by providing the token. * * @param $token + * + * @return SocialIdentity + * @throws Exceptions\UnableToConfirm */ public static function confirmIdentityByToken($token) { @@ -203,7 +239,7 @@ public static function confirmIdentityByToken($token) /** * Obtain the UserProvider Instance. * - * @return \Illuminate\Foundation\Application|mixed + * @return EloquentUserProvider */ public function userProvider() { diff --git a/src/ButlerDriver.php b/src/ButlerDriver.php new file mode 100644 index 0000000..0dfbcfb --- /dev/null +++ b/src/ButlerDriver.php @@ -0,0 +1,29 @@ +socialiteProvider = $socialiteProvider; + } + + /** + * @return \Illuminate\Http\RedirectResponse + */ + public function redirect() + { + return $this->socialiteProvider->redirect(); + } +} diff --git a/src/ButlerServiceProvider.php b/src/ButlerServiceProvider.php index 299cc35..322af14 100644 --- a/src/ButlerServiceProvider.php +++ b/src/ButlerServiceProvider.php @@ -2,7 +2,9 @@ namespace Konsulting\Butler; +use Illuminate\Foundation\Application; use Illuminate\Support\ServiceProvider; +use Laravel\Socialite\SocialiteManager; class ButlerServiceProvider extends ServiceProvider { @@ -25,8 +27,9 @@ public function register() { $this->mergeConfigFrom(__DIR__ . '/../config/butler.php', 'butler'); - $this->app->singleton('butler', function ($app) { + $this->app->singleton('butler', function (Application $app) { return new Butler( + new SocialiteManager($app), $app['config']['butler.providers'], $app['config']['butler.route_map'] ); diff --git a/src/EloquentUserProvider.php b/src/EloquentUserProvider.php index 12269b4..bce3b2e 100644 --- a/src/EloquentUserProvider.php +++ b/src/EloquentUserProvider.php @@ -50,8 +50,7 @@ public function retrieveByOauthIdentity(Identity $oauthId) } /** - * Create a new user from the provided Identity. - * Record it's creation within the instance. + * Create a new user from the provided Identity. Record its creation within the instance. * * @param \Laravel\Socialite\Contracts\User $oauthId * diff --git a/tests/ButlerDriverTest.php b/tests/ButlerDriverTest.php new file mode 100644 index 0000000..475b197 --- /dev/null +++ b/tests/ButlerDriverTest.php @@ -0,0 +1,43 @@ +socialite = Mockery::mock(SocialiteManager::class); + $this->socialiteProvider = Mockery::mock(Provider::class); + $this->butler = new Butler($this->socialite, [ + 'my-service' => ['name' => 'My Service'], + ], []); + } + + /** @test */ + public function it_performs_a_redirect_on_the_socialite_driver() + { + $this->socialite->shouldReceive('driver')->with('my-service') + ->andReturn($this->socialiteProvider)->once(); + $this->socialiteProvider->shouldReceive('redirect')->withNoArgs() + ->andReturn(new RedirectResponse('/'))->once(); + + $this->assertInstanceOf(RedirectResponse::class, $this->butler->driver('my-service')->redirect()); + } +} From ddd87e0ee70b08bcfd68ed1790e0e169152e9275 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Tue, 8 Jan 2019 14:21:04 +0000 Subject: [PATCH 11/31] Add user() method to butler driver --- src/ButlerDriver.php | 18 +++++++++++++++--- tests/AuthControllerTest.php | 14 +++++++------- tests/ButlerDriverTest.php | 31 +++++++++++++++++++++++++++---- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/ButlerDriver.php b/src/ButlerDriver.php index 0dfbcfb..716e7c7 100644 --- a/src/ButlerDriver.php +++ b/src/ButlerDriver.php @@ -5,10 +5,10 @@ use Laravel\Socialite\Contracts\Provider; use Laravel\Socialite\Two\AbstractProvider; -class ButlerDriver +class ButlerDriver implements Provider { /** - * The Socialite provider instance, used for performing actions. + * The Socialite provider instance. * * @var AbstractProvider */ @@ -20,10 +20,22 @@ public function __construct(Provider $socialiteProvider) } /** - * @return \Illuminate\Http\RedirectResponse + * Redirect the user to the authentication page for the provider. + * + * @return \Symfony\Component\HttpFoundation\RedirectResponse */ public function redirect() { return $this->socialiteProvider->redirect(); } + + /** + * Get the User instance for the authenticated user. + * + * @return \Laravel\Socialite\Contracts\User + */ + public function user() + { + return $this->socialiteProvider->user(); + } } diff --git a/tests/AuthControllerTest.php b/tests/AuthControllerTest.php index 2ed9e8e..5d9964c 100644 --- a/tests/AuthControllerTest.php +++ b/tests/AuthControllerTest.php @@ -10,6 +10,13 @@ class AuthControllerTest extends DatabaseTestCase { + public function setUp() + { + parent::setUp(); + + $this->app->bind('\Laravel\Socialite\Contracts\Factory', Socialite::class); + } + public function test_it_redirects_to_a_provider() { $this->visitRoute('butler.redirect', 'test'); @@ -126,13 +133,6 @@ public function test_logging_in_with_the_an_existing_identity_on_your_account_wh $this->dontSee('Identity saved'); } - public function setUp() - { - parent::setUp(); - - $this->app->singleton('\Laravel\Socialite\Contracts\Factory', Socialite::class); - } - public function makeUser2() { return User::create(['name' => 'Roger', 'email' => 'roger@klever.co.uk']); diff --git a/tests/ButlerDriverTest.php b/tests/ButlerDriverTest.php index 475b197..2b715ad 100644 --- a/tests/ButlerDriverTest.php +++ b/tests/ButlerDriverTest.php @@ -4,6 +4,7 @@ use Illuminate\Http\RedirectResponse; use Laravel\Socialite\Contracts\Provider; +use Laravel\Socialite\Contracts\User as SocialiteUser; use Laravel\Socialite\SocialiteManager; use Mockery; use Mockery\Mock; @@ -33,11 +34,33 @@ public function setUp() /** @test */ public function it_performs_a_redirect_on_the_socialite_driver() { - $this->socialite->shouldReceive('driver')->with('my-service') - ->andReturn($this->socialiteProvider)->once(); + $redirect = Mockery::mock(RedirectResponse::class); + $this->socialiteShouldReturnDriver(); $this->socialiteProvider->shouldReceive('redirect')->withNoArgs() - ->andReturn(new RedirectResponse('/'))->once(); + ->andReturn($redirect)->once(); + + $this->assertSame($redirect, $this->butler->driver('my-service')->redirect()); + } + + /** @test */ + public function it_gets_the_user() + { + $user = Mockery::mock(SocialiteUser::class); + $this->socialiteShouldReturnDriver(); + $this->socialiteProvider->shouldReceive('user')->withNoArgs() + ->andReturn($user)->once(); + + $this->assertSame($user, $this->butler->driver('my-service')->user()); + } - $this->assertInstanceOf(RedirectResponse::class, $this->butler->driver('my-service')->redirect()); + /** + * Set the expectation and return value on the Socialite mock. + * + * @return void + */ + protected function socialiteShouldReturnDriver() + { + $this->socialite->shouldReceive('driver')->with('my-service') + ->andReturn($this->socialiteProvider)->once(); } } From d2f645ddb817f52dd3a9ab92ed49d6e693012077 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Tue, 8 Jan 2019 15:17:31 +0000 Subject: [PATCH 12/31] Update AuthController --- src/ButlerFacade.php | 2 +- src/ButlerServiceProvider.php | 4 +++- src/Controllers/AuthController.php | 37 ++++++++++++++++-------------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/ButlerFacade.php b/src/ButlerFacade.php index 9000446..acfa3b9 100644 --- a/src/ButlerFacade.php +++ b/src/ButlerFacade.php @@ -8,6 +8,6 @@ class ButlerFacade extends Facade { protected static function getFacadeAccessor() { - return 'butler'; + return Butler::class; } } diff --git a/src/ButlerServiceProvider.php b/src/ButlerServiceProvider.php index 322af14..575e0ec 100644 --- a/src/ButlerServiceProvider.php +++ b/src/ButlerServiceProvider.php @@ -27,7 +27,7 @@ public function register() { $this->mergeConfigFrom(__DIR__ . '/../config/butler.php', 'butler'); - $this->app->singleton('butler', function (Application $app) { + $this->app->bind(Butler::class, function (Application $app) { return new Butler( new SocialiteManager($app), $app['config']['butler.providers'], @@ -35,6 +35,8 @@ public function register() ); }); + $this->app->alias(Butler::class, 'butler'); + $this->app->singleton('butler_user_provider', function ($app) { $class = $app['config']['butler.user_provider']; diff --git a/src/Controllers/AuthController.php b/src/Controllers/AuthController.php index 21411f5..b5bc9a5 100644 --- a/src/Controllers/AuthController.php +++ b/src/Controllers/AuthController.php @@ -2,8 +2,8 @@ namespace Konsulting\Butler\Controllers; -use Butler; use Illuminate\Http\RedirectResponse; +use Konsulting\Butler\Butler; use Konsulting\Butler\Exceptions\NoUser; use GuzzleHttp\Exception\ClientException; use Laravel\Socialite\Two\InvalidStateException; @@ -38,9 +38,9 @@ public function __construct(SocialiteManager $socialite) public function redirect($provider) { try { - Butler::checkProvider($provider); + \Butler::checkProvider($provider); } catch (UnknownProvider $e) { - return redirect()->route(Butler::routeName('login')) + return redirect()->route(\Butler::routeName('login')) ->with('status.content', 'Unknown Provider.') ->with('status.type', 'warning'); } @@ -51,40 +51,43 @@ public function redirect($provider) /** * Obtain the user information from the provider, store details, and log in if appropriate. * - * @param $provider + * @param Butler $butler + * @param $provider * * @return RedirectResponse + * @throws UnableToConfirm + * @throws UnknownProvider */ - public function callback($provider) + public function callback(Butler $butler, $provider) { try { $oauthId = $this->socialite->driver($provider)->user(); } catch (InvalidStateException $e) { - return redirect()->route(Butler::routeName('login')) - ->with('status.content', 'There was a problem logging in with ' . Butler::provider($provider)->name . ', please try again.') + return redirect()->route($butler->routeName('login')) + ->with('status.content', 'There was a problem logging in with ' . $butler->provider($provider)->name . ', please try again.') ->with('status.type', 'warning'); } catch (ClientException $e) { // There was a problem getting the user. Socialite does not distinguish the reason. // The most likely reason is that someone denied the link-up to our site. So // we will return to login with an appropriate message for that scenario. - return redirect()->route(Butler::routeName('login')) - ->with('status.content', 'You have cancelled the login with ' . Butler::provider($provider)->name . '.') + return redirect()->route($butler->routeName('login')) + ->with('status.content', 'You have cancelled the login with ' . $butler->provider($provider)->name . '.') ->with('status.type', 'warning'); } - if (Butler::authenticate($provider, $oauthId)) { - return redirect()->route(Butler::routeName('authenticated')); + if ($butler->authenticate($provider, $oauthId)) { + return redirect()->route($butler->routeName('authenticated')); } try { - $socialIdentity = Butler::register($provider, $oauthId); + $socialIdentity = $butler->register($provider, $oauthId); // If the user was just created, and we have opted to allow them to // login without confirming the social identify, we'll log them // in. Otherwise, we'll ask to confirm the social identity. - if (Butler::createdUser($socialIdentity->user) + if ($butler->createdUser($socialIdentity->user) && config('butler.confirm_identity_for_new_user', true) == false ) { $socialIdentity->confirm(); @@ -99,15 +102,15 @@ public function callback($provider) ->with('status.content', $message) ->with('status.type', 'success'); } catch (NoUser $e) { - return redirect()->route(Butler::routeName('login')) + return redirect()->route($butler->routeName('login')) ->with('status.content', $e->getMessage()) ->with('status.type', 'danger'); } catch (SocialIdentityAlreadyAssociated $e) { - return redirect()->route(Butler::routeName('login')) + return redirect()->route($butler->routeName('login')) ->with('status.content', $e->getMessage()) ->with('status.type', 'danger'); } catch (UserAlreadyHasSocialIdentity $e) { - return redirect()->route(Butler::routeName('profile')); + return redirect()->route($butler->routeName('profile')); } } @@ -139,7 +142,7 @@ public function confirm($token) protected function loginOrProfile() { - return Butler::routeName($this->guard()->check() ? 'profile' : 'login'); + return \Butler::routeName($this->guard()->check() ? 'profile' : 'login'); } protected function guard() From 58118ed2f9a547a6849ef5f22287033c8edf26c7 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Wed, 9 Jan 2019 12:24:38 +0000 Subject: [PATCH 13/31] Add refresh functionality to butler driver --- src/ButlerDriver.php | 45 ++++++++++ src/Contracts/RefreshableProvider.php | 14 +++ src/Controllers/AuthController.php | 2 +- src/Exceptions/ButlerException.php | 8 ++ src/Exceptions/CouldNotRefreshToken.php | 8 ++ src/Exceptions/NoUser.php | 4 +- .../SocialIdentityAlreadyAssociated.php | 4 +- src/Exceptions/UnableToConfirm.php | 4 +- src/Exceptions/UnknownProvider.php | 4 +- src/Exceptions/UnrefreshableProvider.php | 16 ++++ .../UserAlreadyHasSocialIdentity.php | 4 +- tests/ButlerDriverTest.php | 89 ++++++++++++++++++- 12 files changed, 183 insertions(+), 19 deletions(-) create mode 100644 src/Contracts/RefreshableProvider.php create mode 100644 src/Exceptions/ButlerException.php create mode 100644 src/Exceptions/CouldNotRefreshToken.php create mode 100644 src/Exceptions/UnrefreshableProvider.php diff --git a/src/ButlerDriver.php b/src/ButlerDriver.php index 716e7c7..1173cb8 100644 --- a/src/ButlerDriver.php +++ b/src/ButlerDriver.php @@ -2,6 +2,11 @@ namespace Konsulting\Butler; +use Illuminate\Support\Arr; +use Illuminate\Support\Carbon; +use Konsulting\Butler\Contracts\RefreshableProvider; +use Konsulting\Butler\Exceptions\CouldNotRefreshToken; +use Konsulting\Butler\Exceptions\UnrefreshableProvider; use Laravel\Socialite\Contracts\Provider; use Laravel\Socialite\Two\AbstractProvider; @@ -38,4 +43,44 @@ public function user() { return $this->socialiteProvider->user(); } + + /** + * @param SocialIdentity $socialIdentity + * @return SocialIdentity + * @throws UnrefreshableProvider + * @throws CouldNotRefreshToken + */ + public function refresh(SocialIdentity $socialIdentity) + { + if (! $this->socialiteProvider instanceof RefreshableProvider) { + throw new UnrefreshableProvider($this->socialiteProvider); + } + + $response = $this->socialiteProvider->getRefreshResponse($socialIdentity->refresh_token); + + $this->validateRefreshResponse($response); + + $socialIdentity->update([ + 'access_token' => $response['access_token'], + 'refresh_token' => Arr::get($response, 'refresh_token'), + 'expires_at' => array_key_exists('expires_in', $response) + ? Carbon::now()->addSeconds($response['expires_in']) + : null, + ]); + + return $socialIdentity; + } + + /** + * Check that the response is an array and contains an access token. + * + * @param array $response + * @throws CouldNotRefreshToken + */ + protected function validateRefreshResponse($response) + { + if (! is_array($response) || ! array_key_exists('access_token', $response)) { + throw new CouldNotRefreshToken('Bad response received: ' . serialize($response)); + } + } } diff --git a/src/Contracts/RefreshableProvider.php b/src/Contracts/RefreshableProvider.php new file mode 100644 index 0000000..dee5a20 --- /dev/null +++ b/src/Contracts/RefreshableProvider.php @@ -0,0 +1,14 @@ +guard()->login($socialIdentity->user); } } catch (UnableToConfirm $e) { - return redirect()->route(Butler::routeName('login')) + return redirect()->route(\Butler::routeName('login')) ->with('status.content', 'Unable to confirm identity usage.') ->with('status.type', 'danger'); } diff --git a/src/Exceptions/ButlerException.php b/src/Exceptions/ButlerException.php new file mode 100644 index 0000000..02fa449 --- /dev/null +++ b/src/Exceptions/ButlerException.php @@ -0,0 +1,8 @@ +socialite = Mockery::mock(SocialiteManager::class); $this->socialiteProvider = Mockery::mock(Provider::class); + $this->butler = new Butler($this->socialite, [ 'my-service' => ['name' => 'My Service'], ], []); @@ -34,8 +39,9 @@ public function setUp() /** @test */ public function it_performs_a_redirect_on_the_socialite_driver() { + $this->socialiteShouldReturnMockedProvider(); + $redirect = Mockery::mock(RedirectResponse::class); - $this->socialiteShouldReturnDriver(); $this->socialiteProvider->shouldReceive('redirect')->withNoArgs() ->andReturn($redirect)->once(); @@ -45,20 +51,97 @@ public function it_performs_a_redirect_on_the_socialite_driver() /** @test */ public function it_gets_the_user() { + $this->socialiteShouldReturnMockedProvider(); + $user = Mockery::mock(SocialiteUser::class); - $this->socialiteShouldReturnDriver(); $this->socialiteProvider->shouldReceive('user')->withNoArgs() ->andReturn($user)->once(); $this->assertSame($user, $this->butler->driver('my-service')->user()); } + /** @test */ + public function it_cannot_refresh_a_non_refreshable_provider() + { + $this->socialiteShouldReturnMockedProvider(); + + $this->expectException(UnrefreshableProvider::class); + + $this->butler->driver('my-service')->refresh(Mockery::mock(SocialIdentity::class)); + } + + /** @test */ + public function it_refreshes_a_token() + { + $this->socialiteProvider = Mockery::mock(Provider::class, RefreshableProvider::class); + $this->socialiteShouldReturnMockedProvider(); + + $socialIdentity = factory(SocialIdentity::class)->create(); + $this->socialiteProvider->shouldReceive('getRefreshResponse')->with($socialIdentity->refresh_token) + ->once()->andReturn([ + 'access_token' => 'new access token', + 'expires_in' => 3600, + 'token_type' => 'Bearer', + 'refresh_token' => 'new refresh token', + ]); + + $this->butler->driver('my-service')->refresh($socialIdentity); + + $socialIdentity->refresh(); + $this->assertSame('new access token', $socialIdentity->access_token); + $this->assertSame('new refresh token', $socialIdentity->refresh_token); + $this->assertSame(Carbon::now()->addSeconds(3600)->toAtomString(), $socialIdentity->expires_at->toAtomString()); + } + + /** @test */ + public function it_refreshes_a_token_and_does_not_receive_an_expiry_time_or_refresh_token() + { + $this->socialiteProvider = Mockery::mock(Provider::class, RefreshableProvider::class); + $this->socialiteShouldReturnMockedProvider(); + + $socialIdentity = factory(SocialIdentity::class)->create([ + 'refresh_token' => 'my refresh token', + 'expires_at' => Carbon::now(), + ]); + $this->socialiteProvider->shouldReceive('getRefreshResponse')->once()->andReturn(['access_token' => 'a']); + + $this->butler->driver('my-service')->refresh($socialIdentity); + + $this->assertSame(null, $socialIdentity->expires_at); + $this->assertSame(null, $socialIdentity->refresh_token); + } + + /** + * @test + * @dataProvider badRefreshTokenResponseProvider + */ + public function it_fails_a_refresh_if_the_response_is_not_valid($refreshResponse) + { + $this->socialiteProvider = Mockery::mock(Provider::class, RefreshableProvider::class); + $this->socialiteShouldReturnMockedProvider(); + + $socialIdentity = factory(SocialIdentity::class)->create(); + $this->socialiteProvider->shouldReceive('getRefreshResponse')->once()->andReturn($refreshResponse); + + $this->expectException(CouldNotRefreshToken::class); + + $this->butler->driver('my-service')->refresh($socialIdentity); + } + + public function badRefreshTokenResponseProvider() + { + return [ + [['no access token' => 'a']], + [false], + ]; + } + /** * Set the expectation and return value on the Socialite mock. * * @return void */ - protected function socialiteShouldReturnDriver() + protected function socialiteShouldReturnMockedProvider() { $this->socialite->shouldReceive('driver')->with('my-service') ->andReturn($this->socialiteProvider)->once(); From f4c1d818d69c1c773f93c8da5ae1fd4a76ab717f Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Wed, 9 Jan 2019 13:31:52 +0000 Subject: [PATCH 14/31] Add method to check for token expiry --- src/SocialIdentity.php | 13 +++++++++++++ tests/SocialIdentityTest.php | 27 +++++++++++++++++++++++++-- tests/TestCase.php | 17 +++++++++++------ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/SocialIdentity.php b/src/SocialIdentity.php index 0f8201c..87c4f57 100644 --- a/src/SocialIdentity.php +++ b/src/SocialIdentity.php @@ -171,4 +171,17 @@ public static function retrievePossibleByOauthIdentity($provider, Identity $iden }) ->first(); } + + /** + * Check if the access token's expiry date has passed. Since the expires_in parameter is not required by OAuth, if + * the expiry date is null we'll assume that the token is still valid. + * + * @return bool + */ + public function accessTokenIsExpired() + { + return $this->expires_at instanceof Carbon + ? $this->expires_at->isPast() + : false; + } } diff --git a/tests/SocialIdentityTest.php b/tests/SocialIdentityTest.php index a6b4ff8..7aaa4b2 100644 --- a/tests/SocialIdentityTest.php +++ b/tests/SocialIdentityTest.php @@ -2,17 +2,40 @@ namespace Konsulting\Butler; +use Carbon\Carbon; + class SocialIdentityTest extends DatabaseTestCase { /** @test */ public function it_checks_if_the_confirmation_deadline_has_passed() { - $pastDeadline = factory(SocialIdentity::class)->create(['confirm_until' => $this->carbonNow->subMinute()]); - $notPastDeadline = factory(SocialIdentity::class)->create(['confirm_until' => $this->carbonNow->addMinute()]); + $pastDeadline = factory(SocialIdentity::class)->create(['confirm_until' => static::carbonNow()->subMinute()]); + $notPastDeadline = factory(SocialIdentity::class)->create(['confirm_until' => static::carbonNow()->addMinute()]); $noDeadlineSet = factory(SocialIdentity::class)->create(['confirm_until' => null]); $this->assertTrue($pastDeadline->pastConfirmationDeadline()); $this->assertFalse($notPastDeadline->pastConfirmationDeadline()); $this->assertTrue($noDeadlineSet->pastConfirmationDeadline()); } + + /** + * @test + * @dataProvider accessTokenExpiryProvider + */ + public function it_checks_if_the_access_token_has_expired($expiresAt, $isExpired) + { + $socialIdentity = factory(SocialIdentity::class)->create(['expires_at' => $expiresAt]) + ->fresh(); + + $this->assertSame($isExpired, $socialIdentity->accessTokenIsExpired()); + } + + public function accessTokenExpiryProvider() + { + return [ + [static::carbonNow()->subMonth(), true], + [static::carbonNow()->addMonth(), false], + [null, false], + ]; + } } diff --git a/tests/TestCase.php b/tests/TestCase.php index 993f667..a1bc335 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -14,15 +14,21 @@ abstract class TestCase extends \Orchestra\Testbench\BrowserKit\TestCase { + const CARBON_NOW = '2018-01-01 10:30:40'; + /** - * The time that Carbon::now() has been set to for testing. + * Get the test Carbon::now() instance. For use in data providers where Carbon::setTestNow() has not been called + * (because data providers are resolved before PHPUnit's setUp() method). * - * @var \Carbon\Carbon + * @return Carbon */ - protected $carbonNow; + protected static function carbonNow() + { + return Carbon::parse(static::CARBON_NOW); + } /** - * Set up ServiceProviders. + * Set up service providers. * * @param \Illuminate\Foundation\Application $app * @@ -55,8 +61,7 @@ protected function getPackageAliases($app) */ protected function getEnvironmentSetUp($app) { - Carbon::setTestNow('2018-01-01 10:30:40'); - $this->carbonNow = Carbon::now(); + Carbon::setTestNow(static::carbonNow()); // Setup default database to use sqlite :memory: $app['config']->set('database.default', 'testbench'); From f879a83ea89b19edd20681dba929175cc056f233 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Wed, 9 Jan 2019 13:48:57 +0000 Subject: [PATCH 15/31] Update butler driver doc block --- src/Butler.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Butler.php b/src/Butler.php index c2e8eb9..c5a1666 100644 --- a/src/Butler.php +++ b/src/Butler.php @@ -101,6 +101,13 @@ public function provider($provider) return $this->providers[$provider]; } + /** + * Get the Butler driver for the given Socialite provider. This wraps the underlying Socialite driver and provides + * some extra functionality. + * + * @param string $providerName + * @return ButlerDriver + */ public function driver($providerName) { return new ButlerDriver($this->socialite->driver($providerName)); From 0451a773f07497efbf4d21cf34d420745f63b69d Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Wed, 9 Jan 2019 14:00:40 +0000 Subject: [PATCH 16/31] Retrieve SocialiteManager from container instead of instantiating directly --- src/ButlerServiceProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ButlerServiceProvider.php b/src/ButlerServiceProvider.php index 575e0ec..3a3ae6c 100644 --- a/src/ButlerServiceProvider.php +++ b/src/ButlerServiceProvider.php @@ -4,7 +4,7 @@ use Illuminate\Foundation\Application; use Illuminate\Support\ServiceProvider; -use Laravel\Socialite\SocialiteManager; +use Laravel\Socialite\Contracts\Factory; class ButlerServiceProvider extends ServiceProvider { @@ -29,7 +29,7 @@ public function register() $this->app->bind(Butler::class, function (Application $app) { return new Butler( - new SocialiteManager($app), + $app->make(Factory::class), $app['config']['butler.providers'], $app['config']['butler.route_map'] ); From 468afd24aa06446d5a0b9e33a4507b30d05785cd Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Wed, 9 Jan 2019 14:06:29 +0000 Subject: [PATCH 17/31] Amend Socialite type declaration --- src/Butler.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Butler.php b/src/Butler.php index c5a1666..bad63f2 100644 --- a/src/Butler.php +++ b/src/Butler.php @@ -6,6 +6,7 @@ use Konsulting\Butler\Exceptions\SocialIdentityAlreadyAssociated; use Konsulting\Butler\Exceptions\UnknownProvider; use Konsulting\Butler\Exceptions\UserAlreadyHasSocialIdentity; +use Laravel\Socialite\Contracts\Factory as SocialiteFactory; use Laravel\Socialite\Contracts\User as Identity; use Laravel\Socialite\SocialiteManager; use stdClass; @@ -36,11 +37,11 @@ class Butler /** * Butler constructor. * - * @param SocialiteManager $socialite + * @param SocialiteFactory $socialite * @param array[] $providers * @param array $routeNames */ - public function __construct(SocialiteManager $socialite, $providers, $routeNames) + public function __construct(SocialiteFactory $socialite, $providers, $routeNames) { $this->providers = $this->prepareProviders($providers); $this->routeNames = collect($routeNames); From 83caa5c048eb38b75a179fa196478e5673291ea7 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Thu, 10 Jan 2019 11:41:49 +0000 Subject: [PATCH 18/31] Add Butler Job --- src/Jobs/ButlerJob.php | 153 ++++++++++++++++++++++++++++++++++++++++ tests/ButlerJobTest.php | 125 ++++++++++++++++++++++++++++++++ 2 files changed, 278 insertions(+) create mode 100644 src/Jobs/ButlerJob.php create mode 100644 tests/ButlerJobTest.php diff --git a/src/Jobs/ButlerJob.php b/src/Jobs/ButlerJob.php new file mode 100644 index 0000000..f82e0e4 --- /dev/null +++ b/src/Jobs/ButlerJob.php @@ -0,0 +1,153 @@ +user = $user; + } + + /** + * Get the name of the socialite provider. + * + * @return string + */ + abstract protected function getSocialProviderName(); + + /** + * The main body of the job. This will be attempted multiple times following token refreshes if $butlerTryLimit > 1. + * + * @param string $token + * @return bool + */ + abstract protected function doAction($token); + + /** + * Handle the exception caught in the process of executing the action or refreshing the token. + * + * @param \Exception $e + * @throws \Exception + * @return mixed + */ + protected function handleException(\Exception $e) + { + throw $e; + } + + /** + * Handle the job. + * + * @return bool + * @throws \Exception + */ + public function handle() + { + try { + $socialIdentity = $this->getSocialIdentityFromUser(); + $token = $this->getToken($socialIdentity); + + return $this->actionLoop($token, $socialIdentity); + } catch (\Exception $e) { + return $this->handleException($e); + } + } + + /** + * Get the access token for the social identity, refreshing if necessary. + * + * @param SocialIdentity $socialIdentity + * @param bool $forceRefresh + * @return string The access token + */ + private function getToken(SocialIdentity $socialIdentity, $forceRefresh = false) + { + if ($forceRefresh || $socialIdentity->accessTokenIsExpired()) { + $socialIdentity = $this->refreshSocialIdentityTokens($socialIdentity); + } + + return $socialIdentity->access_token; + } + + /** + * Refresh the social identity and get the access token. + * + * @param SocialIdentity $socialIdentity + * @return string + */ + private function getRefreshedToken(SocialIdentity $socialIdentity) + { + return $this->getToken($socialIdentity, true); + } + + /** + * Refresh the tokens on the social identity model. + * + * @param SocialIdentity $socialIdentity + * @return SocialIdentity + */ + private function refreshSocialIdentityTokens($socialIdentity) + { + return \Butler::driver($this->getSocialProviderName())->refresh($socialIdentity); + } + + /** + * Loop through the action for the specified number of tries, or until successful. + * + * @param string $token + * @param SocialIdentity $socialIdentity + * @return bool + */ + private function actionLoop($token, $socialIdentity) + { + $tries = 1; + do { + $success = $this->doAction($token); + + if ($success || $tries >= $this->butlerTryLimit) { + break; + } + + $token = $this->getRefreshedToken($socialIdentity); + $tries++; + } while (true); + + return $success; + } + + /** + * Retrieve the social identity from the user model. + * + * @return SocialIdentity|Model + * @throws \Exception + */ + private function getSocialIdentityFromUser() + { + $socialIdentity = SocialIdentity::query()->where('user_id', $this->user->getAuthIdentifier()) + ->where('provider', $this->getSocialProviderName())->first(); + + if (! $socialIdentity) { + throw new \Exception('Identity not found'); + } + + return $socialIdentity; + } +} diff --git a/tests/ButlerJobTest.php b/tests/ButlerJobTest.php new file mode 100644 index 0000000..f70114b --- /dev/null +++ b/tests/ButlerJobTest.php @@ -0,0 +1,125 @@ +socialiteProvider = Mockery::mock(SocialiteProvider::class, RefreshableProvider::class); + + $socialite->extend('my-service', function () { + return $this->socialiteProvider; + }); + + $this->user = Mockery::mock(Authenticatable::class); + $this->user->shouldReceive('getAuthIdentifier')->andReturn(1); + + $this->socialIdentity = SocialIdentity::create([ + 'access_token' => 'the access token', + 'refresh_token' => 'the refresh token', + 'user_id' => $this->user->getAuthIdentifier(), + 'provider' => 'my-service', + ]); + } + + + /** @test */ + public function it_executes_a_job_with_a_valid_access_token() + { + $task = Mockery::mock(); + + $task->shouldReceive('run')->with($this->socialIdentity->access_token)->andReturn(true)->once(); + MyButlerJob::dispatch($this->user, $task); + } + + /** @test */ + public function it_refreshes_an_expired_token_before_running_the_task_and_tries_multiple_times() + { + $this->socialIdentity->update(['expires_at' => Carbon::now()->subDay()]); + $task = Mockery::mock(); + + // Refresh before loop + $this->socialiteProvider->shouldReceive('getRefreshResponse')->with('the refresh token') + ->andReturn([ + 'access_token' => 'new token 1', + 'refresh_token' => 'new refresh 1', + ])->once()->ordered(); + + // First run, fail + $task->shouldReceive('run')->with('new token 1')->andReturn(false)->once()->ordered(); + + // Refresh after failed run + $this->socialiteProvider->shouldReceive('getRefreshResponse')->with('new refresh 1') + ->andReturn(['access_token' => 'new token 2'])->once()->ordered(); + + // Second run, succeed + $task->shouldReceive('run')->with('new token 2')->andReturn(true)->once()->ordered(); + + MyButlerJob::dispatch($this->user, $task); + } + + /** @test */ + public function it_fails_if_it_exceeds_the_try_limit() + { + $this->socialiteProvider->shouldReceive('getRefreshResponse')->andReturn(['access_token' => 'a'])->once(); + + // $butlerTryLimit is 2 so run should be called twice + $task = Mockery::mock(); + $task->shouldReceive('run')->andReturn(false)->twice(); + + MyButlerJob::dispatch($this->user, $task); + } +} + + +class MyButlerJob extends ButlerJob +{ + protected $butlerTryLimit = 2; + + private $task; + + public function __construct(Authenticatable $user, $task) + { + parent::__construct($user); + $this->task = $task; + } + + protected function getSocialProviderName() + { + return 'my-service'; + } + + protected function doAction($token) + { + // Pass the token to our 'task' so we can assert against it + return $this->task->run($token); + } +} From b034a2e67ab1f8633da80478251a8434d3b228d1 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Thu, 10 Jan 2019 12:14:13 +0000 Subject: [PATCH 19/31] Add tests for exception handling on butler job --- src/Exceptions/SocialIdentityNotFound.php | 8 ++++++ src/Jobs/ButlerJob.php | 12 ++++++-- tests/ButlerJobTest.php | 35 ++++++++++++++++++++++- 3 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 src/Exceptions/SocialIdentityNotFound.php diff --git a/src/Exceptions/SocialIdentityNotFound.php b/src/Exceptions/SocialIdentityNotFound.php new file mode 100644 index 0000000..27781a6 --- /dev/null +++ b/src/Exceptions/SocialIdentityNotFound.php @@ -0,0 +1,8 @@ +where('provider', $this->getSocialProviderName())->first(); if (! $socialIdentity) { - throw new \Exception('Identity not found'); + throw new SocialIdentityNotFound('User ' . $this->user->getAuthIdentifier() . + ' does not have a social identity for ' . $this->getSocialProviderName()); } return $socialIdentity; diff --git a/tests/ButlerJobTest.php b/tests/ButlerJobTest.php index f70114b..bbd4a9e 100644 --- a/tests/ButlerJobTest.php +++ b/tests/ButlerJobTest.php @@ -5,6 +5,8 @@ use Carbon\Carbon; use Illuminate\Contracts\Auth\Authenticatable; use Konsulting\Butler\Contracts\RefreshableProvider; +use Konsulting\Butler\Exceptions\ButlerException; +use Konsulting\Butler\Exceptions\SocialIdentityNotFound; use Konsulting\Butler\Jobs\ButlerJob; use Laravel\Socialite\Contracts\Factory; use Laravel\Socialite\Contracts\Provider as SocialiteProvider; @@ -17,7 +19,7 @@ class ButlerJobTest extends DatabaseTestCase /** @var Authenticatable|Mock */ protected $user; - /** @var SocialIdentity|Mock */ + /** @var SocialIdentity */ protected $socialIdentity; /** @var SocialiteProvider|Mock */ @@ -97,6 +99,27 @@ public function it_fails_if_it_exceeds_the_try_limit() MyButlerJob::dispatch($this->user, $task); } + + /** @test */ + public function it_throws_an_exception_if_the_social_identity_is_not_found() + { + $this->socialIdentity->delete(); + + $this->expectException(SocialIdentityNotFound::class); + MyButlerJob::dispatch($this->user, Mockery::mock()); + } + + /** @test */ + public function it_handles_thrown_exceptions() + { + $exception = new TestException; + + $task = Mockery::mock(); + $task->shouldReceive('run')->andThrow($exception); + $task->shouldReceive('handleException')->with($exception); + + MyButlerJob::dispatch($this->user, $task); + } } @@ -122,4 +145,14 @@ protected function doAction($token) // Pass the token to our 'task' so we can assert against it return $this->task->run($token); } + + protected function handleException(\Exception $e) + { + $this->task->handleException($e); + } +} + +class TestException extends ButlerException +{ + } From ac18aa85c0a271c57a21d16b2b7a94927a7fe8d1 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Thu, 10 Jan 2019 12:35:30 +0000 Subject: [PATCH 20/31] Fix exception handling tests --- tests/ButlerJobTest.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/ButlerJobTest.php b/tests/ButlerJobTest.php index bbd4a9e..7272a9a 100644 --- a/tests/ButlerJobTest.php +++ b/tests/ButlerJobTest.php @@ -115,8 +115,8 @@ public function it_handles_thrown_exceptions() $exception = new TestException; $task = Mockery::mock(); - $task->shouldReceive('run')->andThrow($exception); - $task->shouldReceive('handleException')->with($exception); + $task->shouldReceive('run')->andThrow($exception)->once(); + $task->shouldReceive('handleException')->with($exception)->once(); MyButlerJob::dispatch($this->user, $task); } @@ -148,7 +148,11 @@ protected function doAction($token) protected function handleException(\Exception $e) { - $this->task->handleException($e); + if ($e instanceof TestException) { + return $this->task->handleException($e); + } + + return parent::handleException($e); } } From 011c651b738f5d84d716bb84556ac331f27d6209 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Tue, 15 Jan 2019 17:13:31 +0000 Subject: [PATCH 21/31] Treat access token as expired if expires_at is not set --- src/SocialIdentity.php | 12 +++++++----- tests/SocialIdentityTest.php | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/SocialIdentity.php b/src/SocialIdentity.php index 87c4f57..ebd2527 100644 --- a/src/SocialIdentity.php +++ b/src/SocialIdentity.php @@ -173,15 +173,17 @@ public static function retrievePossibleByOauthIdentity($provider, Identity $iden } /** - * Check if the access token's expiry date has passed. Since the expires_in parameter is not required by OAuth, if - * the expiry date is null we'll assume that the token is still valid. + * Check if the access token's expiry date has passed. If the expiry date is null we'll assume that the token has + * expired. * * @return bool */ public function accessTokenIsExpired() { - return $this->expires_at instanceof Carbon - ? $this->expires_at->isPast() - : false; + if ($this->expires_at instanceof Carbon) { + return $this->expires_at->isPast(); + } + + return true; } } diff --git a/tests/SocialIdentityTest.php b/tests/SocialIdentityTest.php index 7aaa4b2..e01b000 100644 --- a/tests/SocialIdentityTest.php +++ b/tests/SocialIdentityTest.php @@ -35,7 +35,7 @@ public function accessTokenExpiryProvider() return [ [static::carbonNow()->subMonth(), true], [static::carbonNow()->addMonth(), false], - [null, false], + [null, true], ]; } } From 49ca750cce1c67842ce7386a70014203d1ffc669 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Wed, 16 Jan 2019 12:14:04 +0000 Subject: [PATCH 22/31] Update butler job to remove internal looping --- src/Jobs/ButlerJob.php | 48 +++++++++++------------------------------ src/SocialIdentity.php | 10 +++++++++ tests/ButlerJobTest.php | 31 +++++++------------------- 3 files changed, 30 insertions(+), 59 deletions(-) diff --git a/src/Jobs/ButlerJob.php b/src/Jobs/ButlerJob.php index b06e9dc..2ba6933 100644 --- a/src/Jobs/ButlerJob.php +++ b/src/Jobs/ButlerJob.php @@ -60,6 +60,17 @@ protected function handleException(\Exception $e) throw $e; } + /** + * Invalidate the access token. Useful if the response indicates that the access token is invalid or expired, and + * should be refreshed upon retry. + * + * @return bool + */ + protected function invalidateAccessToken() + { + return $this->getSocialIdentityFromUser()->invalidateAccessToken(); + } + /** * Handle the job. * @@ -72,7 +83,7 @@ public function handle() $socialIdentity = $this->getSocialIdentityFromUser(); $token = $this->getToken($socialIdentity); - return $this->actionLoop($token, $socialIdentity); + return $this->doAction($token); } catch (\Exception $e) { return $this->handleException($e); } @@ -94,17 +105,6 @@ private function getToken(SocialIdentity $socialIdentity, $forceRefresh = false) return $socialIdentity->access_token; } - /** - * Refresh the social identity and get the access token. - * - * @param SocialIdentity $socialIdentity - * @return string - */ - private function getRefreshedToken(SocialIdentity $socialIdentity) - { - return $this->getToken($socialIdentity, true); - } - /** * Refresh the tokens on the social identity model. * @@ -116,30 +116,6 @@ private function refreshSocialIdentityTokens($socialIdentity) return \Butler::driver($this->getSocialProviderName())->refresh($socialIdentity); } - /** - * Loop through the action for the specified number of tries, or until successful. - * - * @param string $token - * @param SocialIdentity $socialIdentity - * @return bool - */ - private function actionLoop($token, $socialIdentity) - { - $tries = 1; - do { - $success = $this->doAction($token); - - if ($success || $tries >= $this->butlerTryLimit) { - break; - } - - $token = $this->getRefreshedToken($socialIdentity); - $tries++; - } while (true); - - return $success; - } - /** * Retrieve the social identity from the user model. * diff --git a/src/SocialIdentity.php b/src/SocialIdentity.php index ebd2527..f21be44 100644 --- a/src/SocialIdentity.php +++ b/src/SocialIdentity.php @@ -186,4 +186,14 @@ public function accessTokenIsExpired() return true; } + + /** + * Remove the access token and expiry date, usually to force a token refresh after a failed request. + * + * @return bool + */ + public function invalidateAccessToken() + { + return $this->update(['access_token' => null, 'expires_at' => null]); + } } diff --git a/tests/ButlerJobTest.php b/tests/ButlerJobTest.php index 7272a9a..55cc435 100644 --- a/tests/ButlerJobTest.php +++ b/tests/ButlerJobTest.php @@ -46,13 +46,13 @@ public function setUp() $this->socialIdentity = SocialIdentity::create([ 'access_token' => 'the access token', + 'expires_at' => Carbon::now()->addDay(), 'refresh_token' => 'the refresh token', 'user_id' => $this->user->getAuthIdentifier(), 'provider' => 'my-service', ]); } - /** @test */ public function it_executes_a_job_with_a_valid_access_token() { @@ -63,7 +63,7 @@ public function it_executes_a_job_with_a_valid_access_token() } /** @test */ - public function it_refreshes_an_expired_token_before_running_the_task_and_tries_multiple_times() + public function it_refreshes_an_expired_token_before_running_the_task_and_invalidates_the_token() { $this->socialIdentity->update(['expires_at' => Carbon::now()->subDay()]); $task = Mockery::mock(); @@ -78,26 +78,11 @@ public function it_refreshes_an_expired_token_before_running_the_task_and_tries_ // First run, fail $task->shouldReceive('run')->with('new token 1')->andReturn(false)->once()->ordered(); - // Refresh after failed run - $this->socialiteProvider->shouldReceive('getRefreshResponse')->with('new refresh 1') - ->andReturn(['access_token' => 'new token 2'])->once()->ordered(); - - // Second run, succeed - $task->shouldReceive('run')->with('new token 2')->andReturn(true)->once()->ordered(); - MyButlerJob::dispatch($this->user, $task); - } - - /** @test */ - public function it_fails_if_it_exceeds_the_try_limit() - { - $this->socialiteProvider->shouldReceive('getRefreshResponse')->andReturn(['access_token' => 'a'])->once(); - - // $butlerTryLimit is 2 so run should be called twice - $task = Mockery::mock(); - $task->shouldReceive('run')->andReturn(false)->twice(); - MyButlerJob::dispatch($this->user, $task); + $this->socialIdentity->refresh(); + $this->assertNull($this->socialIdentity->access_token); + $this->assertNull($this->socialIdentity->expires_at); } /** @test */ @@ -125,8 +110,6 @@ public function it_handles_thrown_exceptions() class MyButlerJob extends ButlerJob { - protected $butlerTryLimit = 2; - private $task; public function __construct(Authenticatable $user, $task) @@ -143,7 +126,9 @@ protected function getSocialProviderName() protected function doAction($token) { // Pass the token to our 'task' so we can assert against it - return $this->task->run($token); + if (! $this->task->run($token)) { + $this->invalidateAccessToken(); + } } protected function handleException(\Exception $e) From 5c23be925ab19c475670e3840b69119be54ce2c8 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Tue, 5 Nov 2019 12:27:23 +0000 Subject: [PATCH 23/31] Update dependency version constraints. Min PHP 7.1, allow Laravel 6 --- composer.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index 60a2a94..fa81459 100644 --- a/composer.json +++ b/composer.json @@ -2,8 +2,8 @@ "name": "konsulting/laravel-butler", "description": "Butler manages multiple Socialite authentication for your Laravel app.", "require-dev": { - "phpunit/phpunit": "~5.7|~6.0|~7.0", - "orchestra/testbench": ">= 3.0 <= 3.7", + "phpunit/phpunit": "^5.7 || ^6.0 || ^7.0", + "orchestra/testbench": "^3.0 || ^4.0", "orchestra/testbench-browser-kit": "~3.1", "orchestra/database": "~3.1", "mockery/mockery": "^1.2" @@ -16,10 +16,10 @@ } ], "require": { - "php": "^5.6|^7.0", - "nesbot/carbon": "^1.21 || ^2.0 ", - "laravel/socialite": "~3.0", - "laravel/framework": "~5.3.30|~5.4.0|~5.5.0|~5.6.0|~5.7.0|~5.8.0" + "php": "^7.1", + "nesbot/carbon": "^1.21 || ^2.0", + "laravel/socialite": "^3.0", + "laravel/framework": "^5.3.30 || ^6.0" }, "autoload": { "psr-4": { From e852c25159cd5a8a055be0f30406d5d525a310b6 Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Tue, 5 Nov 2019 12:36:17 +0000 Subject: [PATCH 24/31] Bump PHPUnit to 8, Testbench to 4 --- .gitignore | 1 + composer.json | 12 ++++++------ tests/AuthControllerTest.php | 2 +- tests/ButlerDriverTest.php | 2 +- tests/ButlerJobTest.php | 2 +- tests/TestCase.php | 4 ++-- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 3a9875b..0794651 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /vendor/ composer.lock +.phpunit.result.cache diff --git a/composer.json b/composer.json index fa81459..4e6859a 100644 --- a/composer.json +++ b/composer.json @@ -2,10 +2,10 @@ "name": "konsulting/laravel-butler", "description": "Butler manages multiple Socialite authentication for your Laravel app.", "require-dev": { - "phpunit/phpunit": "^5.7 || ^6.0 || ^7.0", - "orchestra/testbench": "^3.0 || ^4.0", - "orchestra/testbench-browser-kit": "~3.1", - "orchestra/database": "~3.1", + "phpunit/phpunit": "^8.0", + "orchestra/testbench": "^4.0", + "orchestra/testbench-browser-kit": "^4.0", + "orchestra/database": "^4.0", "mockery/mockery": "^1.2" }, "license": "MIT", @@ -18,8 +18,8 @@ "require": { "php": "^7.1", "nesbot/carbon": "^1.21 || ^2.0", - "laravel/socialite": "^3.0", - "laravel/framework": "^5.3.30 || ^6.0" + "laravel/socialite": "^4.0", + "laravel/framework": "^5.7 || ^6.0" }, "autoload": { "psr-4": { diff --git a/tests/AuthControllerTest.php b/tests/AuthControllerTest.php index 5d9964c..40ccb55 100644 --- a/tests/AuthControllerTest.php +++ b/tests/AuthControllerTest.php @@ -10,7 +10,7 @@ class AuthControllerTest extends DatabaseTestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/ButlerDriverTest.php b/tests/ButlerDriverTest.php index febe3d2..e1042d1 100644 --- a/tests/ButlerDriverTest.php +++ b/tests/ButlerDriverTest.php @@ -24,7 +24,7 @@ class ButlerDriverTest extends DatabaseTestCase /** @var Butler */ protected $butler; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/ButlerJobTest.php b/tests/ButlerJobTest.php index 55cc435..1b987d3 100644 --- a/tests/ButlerJobTest.php +++ b/tests/ButlerJobTest.php @@ -30,7 +30,7 @@ protected function getPackageProviders($app) return array_merge(parent::getPackageProviders($app), [SocialiteServiceProvider::class]); } - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/TestCase.php b/tests/TestCase.php index a1bc335..ef7c6ae 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -104,7 +104,7 @@ protected function getEnvironmentSetUp($app) /** * Setup the test environment. */ - public function setUp() + public function setUp(): void { parent::setUp(); @@ -112,7 +112,7 @@ public function setUp() $this->loadMigrationsFrom([ '--database' => 'testbench', - '--realpath' => realpath(__DIR__ . '/../migrations'), + '--path' => realpath(__DIR__ . '/../migrations'), ]); $this->withFactories(__DIR__ . '/factories'); From b78a7a34ca6cc85e991db2a33f296084ee6fe93d Mon Sep 17 00:00:00 2001 From: Robin D'Arcy Date: Tue, 5 Nov 2019 12:51:02 +0000 Subject: [PATCH 25/31] Allow the Socialite manager mock to be turned off for tests --- tests/ButlerJobTest.php | 2 ++ tests/Fakes/Socialite.php | 5 +++-- tests/TestCase.php | 18 +++++++++++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/ButlerJobTest.php b/tests/ButlerJobTest.php index 1b987d3..643b661 100644 --- a/tests/ButlerJobTest.php +++ b/tests/ButlerJobTest.php @@ -25,6 +25,8 @@ class ButlerJobTest extends DatabaseTestCase /** @var SocialiteProvider|Mock */ protected $socialiteProvider; + protected $mockSocialiteManager = false; + protected function getPackageProviders($app) { return array_merge(parent::getPackageProviders($app), [SocialiteServiceProvider::class]); diff --git a/tests/Fakes/Socialite.php b/tests/Fakes/Socialite.php index d444b3a..57bcd9c 100644 --- a/tests/Fakes/Socialite.php +++ b/tests/Fakes/Socialite.php @@ -3,8 +3,9 @@ namespace Konsulting\Butler\Fake; use Laravel\Socialite\Contracts\Factory; +use Laravel\Socialite\SocialiteManager; -class Socialite implements Factory +class Socialite extends SocialiteManager implements Factory { protected $driver; @@ -22,6 +23,6 @@ public function redirect() public function user() { - return new Identity(['id' => 1, 'name'=> 'Keoghan', 'email' => 'keoghan@klever.co.uk']); + return new Identity(['id' => 1, 'name' => 'Keoghan', 'email' => 'keoghan@klever.co.uk']); } } diff --git a/tests/TestCase.php b/tests/TestCase.php index ef7c6ae..4106df5 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -4,6 +4,7 @@ use Butler; use Carbon\Carbon; +use Illuminate\Contracts\Container\Container; use Konsulting\Butler\Fake\Identity; use Konsulting\Butler\Fake\Socialite; use Konsulting\Butler\Fake\User; @@ -16,6 +17,8 @@ abstract class TestCase extends \Orchestra\Testbench\BrowserKit\TestCase { const CARBON_NOW = '2018-01-01 10:30:40'; + protected $mockSocialiteManager = true; + /** * Get the test Carbon::now() instance. For use in data providers where Carbon::setTestNow() has not been called * (because data providers are resolved before PHPUnit's setUp() method). @@ -56,7 +59,7 @@ protected function getPackageAliases($app) /** * Define environment setup. * - * @param \Illuminate\Foundation\Application $app + * @param \Illuminate\Foundation\Application $app * @return void */ protected function getEnvironmentSetUp($app) @@ -78,9 +81,14 @@ protected function getEnvironmentSetUp($app) $app['config']->set('auth.providers.users.model', User::class); - $app->singleton(SocialiteFactory::class, function () { - return new Socialite('test'); - }); + if ($this->mockSocialiteManager) { + $app->singleton(SocialiteFactory::class, function () { + $container = \Mockery::mock(Container::class); + $container->shouldIgnoreMissing(); + + return new Socialite($container); + }); + } Route::group([ 'middleware' => 'web', @@ -112,7 +120,7 @@ public function setUp(): void $this->loadMigrationsFrom([ '--database' => 'testbench', - '--path' => realpath(__DIR__ . '/../migrations'), + '--path' => realpath(__DIR__ . '/../migrations'), ]); $this->withFactories(__DIR__ . '/factories'); From cf5b6e0104ff8316c9d22720c068b622c0b4a4d6 Mon Sep 17 00:00:00 2001 From: agrover86 <32781068+agrover86@users.noreply.github.com> Date: Mon, 16 Mar 2020 13:39:57 +0000 Subject: [PATCH 26/31] Upgrade to Laravel 7 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 4e6859a..aca316d 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "php": "^7.1", "nesbot/carbon": "^1.21 || ^2.0", "laravel/socialite": "^4.0", - "laravel/framework": "^5.7 || ^6.0" + "laravel/framework": "^5.7 || ^6.0 || ^7.0" }, "autoload": { "psr-4": { From cc8d946ca24dd804caecaf7c4d0e24d9f4e238df Mon Sep 17 00:00:00 2001 From: Keoghan Litchfield Date: Wed, 3 Jun 2020 14:25:31 +0100 Subject: [PATCH 27/31] Allow override of table for SocialIdentity --- config/butler.php | 5 +++++ src/SocialIdentity.php | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/config/butler.php b/config/butler.php index a790b61..a4b9c9e 100644 --- a/config/butler.php +++ b/config/butler.php @@ -7,6 +7,11 @@ */ 'user_provider' => \Konsulting\Butler\EloquentUserProvider::class, + /* + * Social Identities Table Name + */ + 'social_identities_table_name' => 'social_identities', + /* * The user class for mapping identities to. */ diff --git a/src/SocialIdentity.php b/src/SocialIdentity.php index f21be44..b081727 100644 --- a/src/SocialIdentity.php +++ b/src/SocialIdentity.php @@ -16,6 +16,14 @@ class SocialIdentity extends Model protected $visible = ['id', 'user_id', 'provider', 'confirmed_at']; + /** + * @inheritDoc + */ + public function getTable() + { + return config('butler.social_identities_table_name') ?: 'social_identities'; + } + /** * A Social Identity belongs to a User. * From 41abccf3f6da2b5af377126e3193b649f702460e Mon Sep 17 00:00:00 2001 From: Keoghan Litchfield Date: Wed, 3 Jun 2020 13:28:40 +0000 Subject: [PATCH 28/31] Apply fixes from StyleCI --- ...016_11_01_000000_create_social_identities_table.php | 2 +- src/Contracts/RefreshableProvider.php | 2 +- src/Controllers/AuthController.php | 10 +++++----- src/Exceptions/ButlerException.php | 1 - src/Exceptions/CouldNotRefreshToken.php | 1 - src/Exceptions/SocialIdentityNotFound.php | 1 - src/Notifications/ConfirmSocialIdentity.php | 4 ++-- src/SocialIdentity.php | 2 +- tests/AuthControllerTest.php | 4 ++-- tests/BasicButlerTest.php | 4 ++-- tests/ButlerJobTest.php | 2 -- tests/Fakes/User.php | 2 +- tests/SocialIdentityTest.php | 2 -- 13 files changed, 15 insertions(+), 22 deletions(-) diff --git a/migrations/2016_11_01_000000_create_social_identities_table.php b/migrations/2016_11_01_000000_create_social_identities_table.php index 1ef26c2..4e309c7 100644 --- a/migrations/2016_11_01_000000_create_social_identities_table.php +++ b/migrations/2016_11_01_000000_create_social_identities_table.php @@ -1,7 +1,7 @@ Date: Fri, 5 Jun 2020 11:58:30 +0100 Subject: [PATCH 29/31] Allow identities to be associated to users without emails needing to match. --- config/butler.php | 5 +++++ src/Butler.php | 8 ++++++++ src/Controllers/AuthController.php | 5 +++++ .../SocialIdentityAssociatedToLoggedInUser.php | 8 ++++++++ tests/AuthControllerTest.php | 12 ++++++++++++ tests/TestCase.php | 4 ++-- 6 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php diff --git a/config/butler.php b/config/butler.php index a4b9c9e..928854e 100644 --- a/config/butler.php +++ b/config/butler.php @@ -45,6 +45,11 @@ */ 'login_immediately_after_confirm' => false, + /** + * Allow Butler to assicate a new identity to the logged in user, rather than matching on email address + */ + 'can_associate_to_logged_in_user' => false, + /* * The application routes for us to use when redirecting the user after actions */ diff --git a/src/Butler.php b/src/Butler.php index bad63f2..1cfa9c7 100644 --- a/src/Butler.php +++ b/src/Butler.php @@ -10,6 +10,7 @@ use Laravel\Socialite\Contracts\User as Identity; use Laravel\Socialite\SocialiteManager; use stdClass; +use Konsulting\Butler\Exceptions\SocialIdentityAssociatedToLoggedInUser; class Butler { @@ -194,6 +195,13 @@ public function register($provider, Identity $identity) { $this->checkProvider($provider); $this->guardExistingSocialIdentities($provider, $identity); + + if (config('butler.can_associate_to_logged_in_user', false) === true) { + SocialIdentity::createFromOauthIdentity($provider, $this->guard()->user(), $identity); + + throw new SocialIdentityAssociatedToLoggedInUser('Social Identity linked to your account'); + } + $user = $this->userProvider()->retrieveByOauthIdentity($identity); if (! $user) { diff --git a/src/Controllers/AuthController.php b/src/Controllers/AuthController.php index 5e519a4..7bbb8da 100644 --- a/src/Controllers/AuthController.php +++ b/src/Controllers/AuthController.php @@ -8,6 +8,7 @@ use Konsulting\Butler\Butler; use Konsulting\Butler\Exceptions\NoUser; use Konsulting\Butler\Exceptions\SocialIdentityAlreadyAssociated; +use Konsulting\Butler\Exceptions\SocialIdentityAssociatedToLoggedInUser; use Konsulting\Butler\Exceptions\UnableToConfirm; use Konsulting\Butler\Exceptions\UnknownProvider; use Konsulting\Butler\Exceptions\UserAlreadyHasSocialIdentity; @@ -111,6 +112,10 @@ public function callback(Butler $butler, $provider) ->with('status.type', 'danger'); } catch (UserAlreadyHasSocialIdentity $e) { return redirect()->route($butler->routeName('profile')); + } catch (SocialIdentityAssociatedToLoggedInUser $e) { + return redirect()->route($butler->routeName('profile')) + ->with('status.content', $e->getMessage()) + ->with('status.type', 'danger'); } } diff --git a/src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php b/src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php new file mode 100644 index 0000000..b4e5e21 --- /dev/null +++ b/src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php @@ -0,0 +1,8 @@ +dontSee('Identity saved'); } + public function test_it_will_associate_to_a_logged_in_user_if_allowed_bypassing_the_email_association() + { + $this->app['config']->set('butler.can_associate_to_logged_in_user', true); + + $user = $this->makeUser('Fred', 'fred@klever.co.uk'); + + $this->actingAs($user) + ->visitRoute('butler.callback', 'test') + ->seeRouteIs(Butler::routeName('profile')) + ->seeInDatabase('social_identities', ['user_id' => 1]); + } + public function makeUser2() { return User::create(['name' => 'Roger', 'email' => 'roger@klever.co.uk']); diff --git a/tests/TestCase.php b/tests/TestCase.php index 4106df5..0e7f22e 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -136,9 +136,9 @@ public function createUsersTable() }); } - protected function makeUser() + protected function makeUser($name = 'Keoghan', $email = 'keoghan@klever.co.uk') { - return User::create(['name' => 'Keoghan', 'email' => 'keoghan@klever.co.uk']); + return User::create(['name' => $name, 'email' => $email]); } protected function makeIdentity() From 5debc666471e6421f5580695a74ac14e72229831 Mon Sep 17 00:00:00 2001 From: Keoghan Litchfield Date: Mon, 19 Apr 2021 11:21:44 +0100 Subject: [PATCH 30/31] Allow Laravel 8 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index aca316d..d843051 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "php": "^7.1", "nesbot/carbon": "^1.21 || ^2.0", "laravel/socialite": "^4.0", - "laravel/framework": "^5.7 || ^6.0 || ^7.0" + "laravel/framework": "^5.7 || ^6.0 || ^7.0 || ^8.0" }, "autoload": { "psr-4": { From 5d8deca02444073c865a3519968879dfdb2a84d3 Mon Sep 17 00:00:00 2001 From: Keoghan Litchfield Date: Mon, 19 Apr 2021 11:24:25 +0100 Subject: [PATCH 31/31] Style changes --- config/butler.php | 2 +- src/Butler.php | 2 +- src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/config/butler.php b/config/butler.php index 928854e..c9205d7 100644 --- a/config/butler.php +++ b/config/butler.php @@ -46,7 +46,7 @@ 'login_immediately_after_confirm' => false, /** - * Allow Butler to assicate a new identity to the logged in user, rather than matching on email address + * Allow Butler to assicate a new identity to the logged in user, rather than matching on email address. */ 'can_associate_to_logged_in_user' => false, diff --git a/src/Butler.php b/src/Butler.php index 1cfa9c7..d880d18 100644 --- a/src/Butler.php +++ b/src/Butler.php @@ -4,13 +4,13 @@ use Konsulting\Butler\Exceptions\NoUser; use Konsulting\Butler\Exceptions\SocialIdentityAlreadyAssociated; +use Konsulting\Butler\Exceptions\SocialIdentityAssociatedToLoggedInUser; use Konsulting\Butler\Exceptions\UnknownProvider; use Konsulting\Butler\Exceptions\UserAlreadyHasSocialIdentity; use Laravel\Socialite\Contracts\Factory as SocialiteFactory; use Laravel\Socialite\Contracts\User as Identity; use Laravel\Socialite\SocialiteManager; use stdClass; -use Konsulting\Butler\Exceptions\SocialIdentityAssociatedToLoggedInUser; class Butler { diff --git a/src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php b/src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php index b4e5e21..3caf049 100644 --- a/src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php +++ b/src/Exceptions/SocialIdentityAssociatedToLoggedInUser.php @@ -4,5 +4,4 @@ class SocialIdentityAssociatedToLoggedInUser extends ButlerException { - }