diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a1f89e..665a820 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,7 +70,7 @@ jobs: php-version: '7.4' extensions: mbstring, intl coverage: none - tools: cs2pr, psalm:^4.7, phpstan:^0.12 + tools: cs2pr, psalm:4.7, phpstan:0.12 - name: Composer Install run: composer install diff --git a/README.md b/README.md index 8566ba4..d542c78 100644 --- a/README.md +++ b/README.md @@ -268,17 +268,10 @@ class SocialAuthListener implements EventListenerInterface */ public function beforeRedirect(EventInterface $event, $url, string $status, ServerRequest $request): void { - $messages = (array)$request->getSession()->read('Flash.flash'); - // Set flash message switch ($status) { case SocialAuthMiddleware::AUTH_STATUS_SUCCESS: - $messages[] = [ - 'message' => __('You are now logged in'), - 'key' => 'flash', - 'element' => 'flash/success', - 'params' => [], - ]; + $request->getFlash()->error('You are now logged in.'); break; // Auth through provider failed. Details will be logged in @@ -289,16 +282,13 @@ class SocialAuthListener implements EventListenerInterface // user has been authenticated through provider but your finder has // a condition to not return an inactivated user. case SocialAuthMiddleware::AUTH_STATUS_FINDER_FAILURE: - $messages[] = [ - 'message' => __('Authentication failed'), - 'key' => 'flash', - 'element' => 'flash/error', - 'params' => [], - ]; + $request->getFlash()->error('Authentication failed.'); break; - } - $request->getSession()->write('Flash.flash', $messages); + case SocialAuthMiddleware::AUTH_STATUS_IDENTITY_MISMATCH: + $request->getFlash()->error('The social profile is already linked to another user.'); + break; + } // You can return a modified redirect URL if needed. } diff --git a/src/Middleware/SocialAuthMiddleware.php b/src/Middleware/SocialAuthMiddleware.php index 84672b6..d1f184b 100644 --- a/src/Middleware/SocialAuthMiddleware.php +++ b/src/Middleware/SocialAuthMiddleware.php @@ -96,6 +96,13 @@ class SocialAuthMiddleware implements MiddlewareInterface, EventDispatcherInterf */ public const AUTH_STATUS_FINDER_FAILURE = 'finder_failure'; + /** + * User associated with social profile mismatches with user record in session. + * + * @var string + */ + public const AUTH_STATUS_IDENTITY_MISMATCH = 'identity_mismatch'; + /** * Default config. * @@ -251,6 +258,7 @@ protected function _handleCallbackAction(ServerRequest $request): Response $config = $this->getConfig(); $providerName = $request->getParam('provider'); $response = new Response(); + $session = $request->getSession(); $profile = $this->_getProfile($providerName, $request); if (!$profile) { @@ -261,7 +269,7 @@ protected function _handleCallbackAction(ServerRequest $request): Response ); } - $user = $this->_getUser($profile, $request->getSession()); + $user = $this->_getUser($profile, $session); if (!$user) { $redirectUrl = $this->_triggerBeforeRedirect($request, $config['loginUrl'], $this->_error); @@ -282,7 +290,10 @@ protected function _handleCallbackAction(ServerRequest $request): Response $user = $user->toArray(); } - $request->getSession()->write($config['sessionKey'], $user); + if (!$session->check($config['sessionKey'])) { + $session->renew(); + $session->write($config['sessionKey'], $user); + } $redirectUrl = $this->_triggerBeforeRedirect($request, $this->_getRedirectUrl($request)); @@ -312,6 +323,36 @@ protected function _setupModelInstances(): void * @return \Cake\Datasource\EntityInterface|null */ protected function _getProfile($providerName, ServerRequest $request): ?EntityInterface + { + $return = $this->_getSocialIdentity($providerName, $request); + if ($return === null) { + return null; + } + + /** @var \Cake\Datasource\EntityInterface|null $profile */ + $profile = $this->_profileModel->find() + ->where([ + $this->_profileModel->aliasField('provider') => $providerName, + $this->_profileModel->aliasField('identifier') => $return['identity']->id, + ]) + ->first(); + + return $this->_patchProfile( + $providerName, + $return['identity'], + $return['access_token'], + $profile ?: null + ); + } + + /** + * Get social identity. + * + * @param string $providerName Provider name. + * @param \Cake\Http\ServerRequest $request Request instance. + * @return array{identity: \SocialConnect\Common\Entity\User, access_token: \SocialConnect\Provider\AccessTokenInterface}|null + */ + protected function _getSocialIdentity($providerName, ServerRequest $request): ?array { try { $provider = $this->_getService($request)->getProvider($providerName); @@ -333,20 +374,7 @@ protected function _getProfile($providerName, ServerRequest $request): ?EntityIn return null; } - /** @var \Cake\Datasource\EntityInterface|null $profile */ - $profile = $this->_profileModel->find() - ->where([ - $this->_profileModel->aliasField('provider') => $providerName, - $this->_profileModel->aliasField('identifier') => $identity->id, - ]) - ->first(); - - return $this->_patchProfile( - $providerName, - $identity, - $accessToken, - $profile ?: null - ); + return ['identity' => $identity, 'access_token' => $accessToken]; } /** @@ -359,12 +387,27 @@ protected function _getProfile($providerName, ServerRequest $request): ?EntityIn */ protected function _getUser(EntityInterface $profile, CakeSession $session): ?EntityInterface { - $user = null; - /** @var string $userPkField */ $userPkField = $this->_userModel->getPrimaryKey(); - if ($profile->get('user_id')) { + $sessionUserId = $session->read($this->getConfig('sessionKey') . '.' . $userPkField); + $profileUserId = $profile->get('user_id'); + + // If the social profile is already connected to a user and that user + // is not the same as one currently set in session then it's an error. + if ( + $sessionUserId && + $profileUserId && + $sessionUserId !== $profileUserId + ) { + $this->_error = self::AUTH_STATUS_IDENTITY_MISMATCH; + + return null; + } + + $user = null; + + if ($profileUserId) { /** @var \Cake\Datasource\EntityInterface $user */ $user = $this->_userModel->find() ->where([ diff --git a/tests/Fixture/UsersFixture.php b/tests/Fixture/UsersFixture.php new file mode 100644 index 0000000..8360893 --- /dev/null +++ b/tests/Fixture/UsersFixture.php @@ -0,0 +1,43 @@ + ['type' => 'integer', 'length' => null, 'unsigned' => false, 'null' => false, 'default' => null, 'comment' => '', 'autoIncrement' => true, 'precision' => null], + 'email' => ['type' => 'string', 'length' => 255, 'null' => true, 'default' => null, 'collate' => 'utf8mb4_unicode_ci', 'comment' => '', 'precision' => null], + '_constraints' => [ + 'primary' => ['type' => 'primary', 'columns' => ['id'], 'length' => []], + ], + '_options' => [ + 'engine' => 'InnoDB', + 'collation' => 'utf8mb4_unicode_ci' + ], + ]; + // phpcs:enable + + /** + * Init method + * + * @return void + */ + public function init(): void + { + $this->records = []; + + parent::init(); + } +} diff --git a/tests/TestCase/Middleware/SocialAuthMiddlewareTest.php b/tests/TestCase/Middleware/SocialAuthMiddlewareTest.php index 358a823..b336a6f 100644 --- a/tests/TestCase/Middleware/SocialAuthMiddlewareTest.php +++ b/tests/TestCase/Middleware/SocialAuthMiddlewareTest.php @@ -4,10 +4,16 @@ namespace ADmad\SocialAuth\Test\TestCase\Middleware; use ADmad\SocialAuth\Middleware\SocialAuthMiddleware; +use ADmad\SocialAuth\Model\Entity\SocialProfile; +use ADmad\SocialAuth\Model\Table\SocialProfilesTable; +use Cake\Event\EventInterface; +use Cake\Event\EventManager; use Cake\Http\Exception\MethodNotAllowedException; use Cake\Http\Response; use Cake\Http\ServerRequestFactory; use Cake\TestSuite\TestCase; +use SocialConnect\Common\Entity\User; +use SocialConnect\Provider\AccessTokenInterface; use SocialConnect\Provider\Session\Dummy; use TestApp\Http\TestRequestHandler; @@ -17,6 +23,11 @@ */ class SocialAuthMiddlewareTest extends TestCase { + protected $fixtures = [ + 'plugin.ADmad/SocialAuth.Users', + 'plugin.ADmad/SocialAuth.SocialProfiles', + ]; + public function setUp(): void { parent::setUp(); @@ -97,4 +108,59 @@ public function testLoginUrlException() $middleware = new SocialAuthMiddleware(); $middleware->process($request, $this->handler); } + + /** + * Test that IDENTITY_MISMATCH_ERROR occurs when a social profile is already + * associated with a user and that user does not match the user record set + * in the session. + * + * @return void + * @see https://github.com/ADmad/cakephp-social-auth/pull/108 + */ + public function testIdentityMismatchError() + { + $request = ServerRequestFactory::fromGlobals([ + 'REQUEST_URI' => '/social-auth/callback/facebook', + ]); + $request = $request->withAttribute('params', [ + 'plugin' => 'ADmad/SocialAuth', + 'controller' => 'Auth', + 'action' => 'callback', + 'provider' => 'facebook', + ]); + $request->getSession()->write('Auth', ['id' => 1]); + + $this->getTableLocator()->get(SocialProfilesTable::class) + ->save(new SocialProfile([ + 'user_id' => 2, + 'provider' => 'facebook', + 'identifier' => 'fbid', + 'email' => 'fb@fb.test', + 'access_token' => '', + ])); + + $middleware = $this->getMockBuilder(SocialAuthMiddleware::class) + ->onlyMethods(['_getSocialIdentity']) + ->getMock(); + + $accessToken = $this->getMockBuilder(AccessTokenInterface::class) + ->getMock(); + + $identity = new User(); + $identity->id = 'fbid'; + $identity->email = 'fb@fb.test'; + + $middleware->expects($this->once()) + ->method('_getSocialIdentity') + ->willReturn(['identity' => $identity, 'access_token' => $accessToken]); + + EventManager::instance()->on( + SocialAuthMiddleware::EVENT_BEFORE_REDIRECT, + function (EventInterface $event, $url, string $status) { + $this->assertSame(SocialAuthMiddleware::AUTH_STATUS_IDENTITY_MISMATCH, $status); + } + ); + + $middleware->process($request, $this->handler); + } }