diff --git a/app/Exceptions/SocialSignInAccountNotUsed.php b/app/Exceptions/SocialSignInAccountNotUsed.php
new file mode 100644
index 000000000..7eaa72bd5
--- /dev/null
+++ b/app/Exceptions/SocialSignInAccountNotUsed.php
@@ -0,0 +1,6 @@
+userRepo->registerNew($userData);
+ $newUser = $this->userRepo->registerNew($userData, $emailVerified);
if ($socialAccount) {
$newUser->socialAccounts()->save($socialAccount);
}
- if (setting('registration-confirmation') || setting('registration-restrict')) {
+ if ((setting('registration-confirmation') || $registrationRestrict) && !$emailVerified) {
$newUser->save();
try {
@@ -250,7 +254,6 @@ class RegisterController extends Controller
* @throws SocialSignInException
* @throws UserRegistrationException
* @throws \BookStack\Exceptions\SocialDriverNotConfigured
- * @throws ConfirmationEmailException
*/
public function socialCallback($socialDriver, Request $request)
{
@@ -267,12 +270,24 @@ class RegisterController extends Controller
}
$action = session()->pull('social-callback');
+
+ // Attempt login or fall-back to register if allowed.
+ $socialUser = $this->socialAuthService->getSocialUser($socialDriver);
if ($action == 'login') {
- return $this->socialAuthService->handleLoginCallback($socialDriver);
+ try {
+ return $this->socialAuthService->handleLoginCallback($socialDriver, $socialUser);
+ } catch (SocialSignInAccountNotUsed $exception) {
+ if ($this->socialAuthService->driverAutoRegisterEnabled($socialDriver)) {
+ return $this->socialRegisterCallback($socialDriver, $socialUser);
+ }
+ throw $exception;
+ }
}
+
if ($action == 'register') {
- return $this->socialRegisterCallback($socialDriver);
+ return $this->socialRegisterCallback($socialDriver, $socialUser);
}
+
return redirect()->back();
}
@@ -288,15 +303,16 @@ class RegisterController extends Controller
/**
* Register a new user after a registration callback.
- * @param $socialDriver
+ * @param string $socialDriver
+ * @param SocialUser $socialUser
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
* @throws UserRegistrationException
- * @throws \BookStack\Exceptions\SocialDriverNotConfigured
*/
- protected function socialRegisterCallback($socialDriver)
+ protected function socialRegisterCallback(string $socialDriver, SocialUser $socialUser)
{
- $socialUser = $this->socialAuthService->handleRegistrationCallback($socialDriver);
+ $socialUser = $this->socialAuthService->handleRegistrationCallback($socialDriver, $socialUser);
$socialAccount = $this->socialAuthService->fillSocialAccount($socialDriver, $socialUser);
+ $emailVerified = $this->socialAuthService->driverAutoConfirmEmailEnabled($socialDriver);
// Create an array of the user data to create a new user instance
$userData = [
@@ -304,6 +320,6 @@ class RegisterController extends Controller
'email' => $socialUser->getEmail(),
'password' => str_random(30)
];
- return $this->registerUser($userData, $socialAccount);
+ return $this->registerUser($userData, $socialAccount, $emailVerified);
}
}
diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php
index 6defe8aa5..6fe8d2619 100644
--- a/app/Repos/UserRepo.php
+++ b/app/Repos/UserRepo.php
@@ -79,12 +79,12 @@ class UserRepo
/**
* Creates a new user and attaches a role to them.
* @param array $data
- * @param boolean autoVerifyEmail
+ * @param boolean $verifyEmail
* @return User
*/
- public function registerNew(array $data, $autoVerifyEmail=false)
+ public function registerNew(array $data, $verifyEmail = false)
{
- $user = $this->create($data, $autoVerifyEmail);
+ $user = $this->create($data, $verifyEmail);
$this->attachDefaultRole($user);
// Get avatar from gravatar and save
@@ -142,17 +142,17 @@ class UserRepo
/**
* Create a new basic instance of user.
* @param array $data
- * @param boolean $autoVerifyEmail
+ * @param boolean $verifyEmail
* @return User
*/
- public function create(array $data, $autoVerifyEmail=false)
+ public function create(array $data, $verifyEmail = false)
{
return $this->user->forceCreate([
'name' => $data['name'],
'email' => $data['email'],
'password' => bcrypt($data['password']),
- 'email_confirmed' => $autoVerifyEmail
+ 'email_confirmed' => $verifyEmail
]);
}
diff --git a/app/Services/SocialAuthService.php b/app/Services/SocialAuthService.php
index 6b4c221b0..9017dfebe 100644
--- a/app/Services/SocialAuthService.php
+++ b/app/Services/SocialAuthService.php
@@ -1,13 +1,12 @@
validateDriver($socialDriver);
-
- // Get user details from social driver
- $socialUser = $this->socialite->driver($driver)->user();
-
// Check social account has not already been used
if ($this->socialAccount->where('driver_id', '=', $socialUser->getId())->exists()) {
throw new UserRegistrationException(trans('errors.social_account_in_use', ['socialAccount'=>$socialDriver]), '/login');
@@ -84,17 +78,26 @@ class SocialAuthService
}
/**
- * Handle the login process on a oAuth callback.
- * @param $socialDriver
- * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
+ * Get the social user details via the social driver.
+ * @param string $socialDriver
+ * @return SocialUser
* @throws SocialDriverNotConfigured
- * @throws SocialSignInException
*/
- public function handleLoginCallback($socialDriver)
+ public function getSocialUser(string $socialDriver)
{
$driver = $this->validateDriver($socialDriver);
- // Get user details from social driver
- $socialUser = $this->socialite->driver($driver)->user();
+ return $this->socialite->driver($driver)->user();
+ }
+
+ /**
+ * Handle the login process on a oAuth callback.
+ * @param $socialDriver
+ * @param SocialUser $socialUser
+ * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
+ * @throws SocialSignInAccountNotUsed
+ */
+ public function handleLoginCallback($socialDriver, SocialUser $socialUser)
+ {
$socialId = $socialUser->getId();
// Get any attached social accounts or users
@@ -109,40 +112,6 @@ class SocialAuthService
return redirect()->intended('/');
}
- // When a user is not logged in and no matching SocialAccount exists,
- // If the auto social registration is enabled, attach the social account, create new user and log him in.
- if (!$isLoggedIn && $socialAccount === null && setting('autosocialregistration-confirmation')) {
-
- // Fill social account
- $socialAccount = $this->fillSocialAccount($socialDriver, $socialUser);
-
- // Create an array of the user data to create a new user instance
- $userData = [
- 'name' => $socialUser->getName(),
- 'email' => $socialUser->getEmail(),
- 'password' => str_random(30)
- ];
-
- // Check domain if domain restriction setting is set
- if (setting('registration-restrict')) {
- $restrictedEmailDomains = explode(',', str_replace(' ', '', setting('registration-restrict')));
- $userEmailDomain = $domain = substr(strrchr($socialUser->getEmail(), "@"), 1);
- if (!in_array($userEmailDomain, $restrictedEmailDomains)) {
- throw new SocialSignInException(trans('auth.registration_email_domain_invalid'), '/login');
- }
- }
-
- // Register new user with autoVerifyEmail set to true and attach the social account
- $newUser = $this->userRepo->registerNew($userData, true);
- $newUser->socialAccounts()->save($socialAccount);
- $newUser->save();
-
- // Log him in
- auth()->login($newUser);
-
- return redirect()->intended('/');
- }
-
// When a user is logged in but the social account does not exist,
// Create the social account and attach it to the user & redirect to the profile page.
if ($isLoggedIn && $socialAccount === null) {
@@ -170,7 +139,7 @@ class SocialAuthService
$message .= trans('errors.social_account_register_instructions', ['socialAccount' => title_case($socialDriver)]);
}
- throw new SocialSignInException($message, '/login');
+ throw new SocialSignInAccountNotUsed($message, '/login');
}
/**
@@ -233,8 +202,28 @@ class SocialAuthService
}
/**
- * @param string $socialDriver
- * @param \Laravel\Socialite\Contracts\User $socialUser
+ * Check if the current config for the given driver allows auto-registration.
+ * @param string $driver
+ * @return bool
+ */
+ public function driverAutoRegisterEnabled(string $driver)
+ {
+ return config('services.' . strtolower($driver) . '.auto_register') === true;
+ }
+
+ /**
+ * Check if the current config for the given driver allow email address auto-confirmation.
+ * @param string $driver
+ * @return bool
+ */
+ public function driverAutoConfirmEmailEnabled(string $driver)
+ {
+ return config('services.' . strtolower($driver) . '.auto_confirm') === true;
+ }
+
+ /**
+ * @param string $socialDriver
+ * @param SocialUser $socialUser
* @return SocialAccount
*/
public function fillSocialAccount($socialDriver, $socialUser)
diff --git a/config/services.php b/config/services.php
index fab0c1d75..2b0f260cd 100644
--- a/config/services.php
+++ b/config/services.php
@@ -48,6 +48,8 @@ return [
'client_secret' => env('GITHUB_APP_SECRET', false),
'redirect' => env('APP_URL') . '/login/service/github/callback',
'name' => 'GitHub',
+ 'auto_register' => env('GITHUB_AUTO_REGISTER', false),
+ 'auto_confirm' => env('GITHUB_AUTO_CONFIRM_EMAIL', false),
],
'google' => [
@@ -55,6 +57,8 @@ return [
'client_secret' => env('GOOGLE_APP_SECRET', false),
'redirect' => env('APP_URL') . '/login/service/google/callback',
'name' => 'Google',
+ 'auto_register' => env('GOOGLE_AUTO_REGISTER', false),
+ 'auto_confirm' => env('GOOGLE_AUTO_CONFIRM_EMAIL', false),
],
'slack' => [
@@ -62,6 +66,8 @@ return [
'client_secret' => env('SLACK_APP_SECRET', false),
'redirect' => env('APP_URL') . '/login/service/slack/callback',
'name' => 'Slack',
+ 'auto_register' => env('SLACK_AUTO_REGISTER', false),
+ 'auto_confirm' => env('SLACK_AUTO_CONFIRM_EMAIL', false),
],
'facebook' => [
@@ -69,6 +75,8 @@ return [
'client_secret' => env('FACEBOOK_APP_SECRET', false),
'redirect' => env('APP_URL') . '/login/service/facebook/callback',
'name' => 'Facebook',
+ 'auto_register' => env('FACEBOOK_AUTO_REGISTER', false),
+ 'auto_confirm' => env('FACEBOOK_AUTO_CONFIRM_EMAIL', false),
],
'twitter' => [
@@ -76,6 +84,8 @@ return [
'client_secret' => env('TWITTER_APP_SECRET', false),
'redirect' => env('APP_URL') . '/login/service/twitter/callback',
'name' => 'Twitter',
+ 'auto_register' => env('TWITTER_AUTO_REGISTER', false),
+ 'auto_confirm' => env('TWITTER_AUTO_CONFIRM_EMAIL', false),
],
'azure' => [
@@ -84,6 +94,8 @@ return [
'tenant' => env('AZURE_TENANT', false),
'redirect' => env('APP_URL') . '/login/service/azure/callback',
'name' => 'Microsoft Azure',
+ 'auto_register' => env('AZURE_AUTO_REGISTER', false),
+ 'auto_confirm' => env('AZURE_AUTO_CONFIRM_EMAIL', false),
],
'okta' => [
@@ -92,6 +104,8 @@ return [
'redirect' => env('APP_URL') . '/login/service/okta/callback',
'base_url' => env('OKTA_BASE_URL'),
'name' => 'Okta',
+ 'auto_register' => env('OKTA_AUTO_REGISTER', false),
+ 'auto_confirm' => env('OKTA_AUTO_CONFIRM_EMAIL', false),
],
'gitlab' => [
@@ -100,6 +114,8 @@ return [
'redirect' => env('APP_URL') . '/login/service/gitlab/callback',
'instance_uri' => env('GITLAB_BASE_URI'), // Needed only for self hosted instances
'name' => 'GitLab',
+ 'auto_register' => env('GITLAB_AUTO_REGISTER', false),
+ 'auto_confirm' => env('GITLAB_AUTO_CONFIRM_EMAIL', false),
],
'twitch' => [
@@ -107,12 +123,17 @@ return [
'client_secret' => env('TWITCH_APP_SECRET'),
'redirect' => env('APP_URL') . '/login/service/twitch/callback',
'name' => 'Twitch',
+ 'auto_register' => env('TWITCH_AUTO_REGISTER', false),
+ 'auto_confirm' => env('TWITCH_AUTO_CONFIRM_EMAIL', false),
],
+
'discord' => [
'client_id' => env('DISCORD_APP_ID'),
'client_secret' => env('DISCORD_APP_SECRET'),
'redirect' => env('APP_URL') . '/login/service/discord/callback',
'name' => 'Discord',
+ 'auto_register' => env('DISCORD_AUTO_REGISTER', false),
+ 'auto_confirm' => env('DISCORD_AUTO_CONFIRM_EMAIL', false),
],
'ldap' => [
diff --git a/phpunit.xml b/phpunit.xml
index 9434b710f..4c1e4f66c 100644
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -35,8 +35,12 @@
{{ trans('settings.reg_auto_social_allow_desc') }}
- @include('components.toggle-switch', ['name' => 'setting-autosocialregistration-confirmation', 'value' => setting('autosocialregistration-confirmation')]) -{{ trans('settings.reg_confirm_email_desc') }}
diff --git a/tests/Auth/SocialAuthTest.php b/tests/Auth/SocialAuthTest.php index e3494d073..5bfe0c222 100644 --- a/tests/Auth/SocialAuthTest.php +++ b/tests/Auth/SocialAuthTest.php @@ -1,6 +1,6 @@ shouldReceive('getName')->once()->andReturn($user->name); $mockSocialUser->shouldReceive('getAvatar')->once()->andReturn('avatar_placeholder'); - $this->visit('/register/service/google'); - $this->visit('/login/service/google/callback'); - $this->seeInDatabase('users', ['name' => $user->name, 'email' => $user->email]); + $this->get('/register/service/google'); + $this->get('/login/service/google/callback'); + $this->assertDatabaseHas('users', ['name' => $user->name, 'email' => $user->email]); $user = $user->whereEmail($user->email)->first(); - $this->seeInDatabase('social_accounts', ['user_id' => $user->id]); + $this->assertDatabaseHas('social_accounts', ['user_id' => $user->id]); } public function test_social_login() @@ -53,17 +53,21 @@ class SocialAuthTest extends BrowserKitTest $mockSocialDriver->shouldReceive('redirect')->twice()->andReturn(redirect('/')); // Test login routes - $this->visit('/login')->seeElement('#social-login-google') - ->click('#social-login-google') - ->seePageIs('/login'); + $resp = $this->get('/login'); + $resp->assertElementExists('a#social-login-google[href$="/login/service/google"]'); + $resp = $this->followingRedirects()->get("/login/service/google"); + $resp->assertSee('login-form'); // Test social callback - $this->visit('/login/service/google/callback')->seePageIs('/login') - ->see(trans('errors.social_account_not_used', ['socialAccount' => 'Google'])); + $resp = $this->followingRedirects()->get('/login/service/google/callback'); + $resp->assertSee('login-form'); + $resp->assertSee(trans('errors.social_account_not_used', ['socialAccount' => 'Google'])); + + $resp = $this->get('/login'); + $resp->assertElementExists('a#social-login-github[href$="/login/service/github"]'); + $resp = $this->followingRedirects()->get("/login/service/github"); + $resp->assertSee('login-form'); - $this->visit('/login')->seeElement('#social-login-github') - ->click('#social-login-github') - ->seePageIs('/login'); // Test social callback with matching social account \DB::table('social_accounts')->insert([ @@ -71,7 +75,77 @@ class SocialAuthTest extends BrowserKitTest 'driver' => 'github', 'driver_id' => 'logintest123' ]); - $this->visit('/login/service/github/callback')->seePageIs('/'); + $resp = $this->followingRedirects()->get('/login/service/github/callback'); + $resp->assertDontSee("login-form"); + } + + public function test_social_autoregister() + { + config([ + 'services.google.client_id' => 'abc123', 'services.google.client_secret' => '123abc', + 'APP_URL' => 'http://localhost' + ]); + + $user = factory(\BookStack\User::class)->make(); + $mockSocialite = \Mockery::mock('Laravel\Socialite\Contracts\Factory'); + $this->app['Laravel\Socialite\Contracts\Factory'] = $mockSocialite; + $mockSocialDriver = \Mockery::mock('Laravel\Socialite\Contracts\Provider'); + $mockSocialUser = \Mockery::mock('\Laravel\Socialite\Contracts\User'); + + $mockSocialUser->shouldReceive('getId')->times(4)->andReturn(1); + $mockSocialUser->shouldReceive('getEmail')->times(2)->andReturn($user->email); + $mockSocialUser->shouldReceive('getName')->once()->andReturn($user->name); + $mockSocialUser->shouldReceive('getAvatar')->once()->andReturn('avatar_placeholder'); + + $mockSocialDriver->shouldReceive('user')->times(2)->andReturn($mockSocialUser); + $mockSocialite->shouldReceive('driver')->times(4)->with('google')->andReturn($mockSocialDriver); + $mockSocialDriver->shouldReceive('redirect')->twice()->andReturn(redirect('/')); + + $googleAccountNotUsedMessage = trans('errors.social_account_not_used', ['socialAccount' => 'Google']); + + $this->get('/login/service/google'); + $resp = $this->followingRedirects()->get('/login/service/google/callback'); + $resp->assertSee($googleAccountNotUsedMessage); + + config(['services.google.auto_register' => true]); + + $this->get('/login/service/google'); + $resp = $this->followingRedirects()->get('/login/service/google/callback'); + $resp->assertDontSee($googleAccountNotUsedMessage); + + $this->assertDatabaseHas('users', ['name' => $user->name, 'email' => $user->email, 'email_confirmed' => false]); + $user = $user->whereEmail($user->email)->first(); + $this->assertDatabaseHas('social_accounts', ['user_id' => $user->id]); + } + + public function test_social_auto_email_confirm() + { + config([ + 'services.google.client_id' => 'abc123', 'services.google.client_secret' => '123abc', + 'APP_URL' => 'http://localhost', 'services.google.auto_register' => true, 'services.google.auto_confirm' => true + ]); + + $user = factory(\BookStack\User::class)->make(); + $mockSocialite = \Mockery::mock('Laravel\Socialite\Contracts\Factory'); + $this->app['Laravel\Socialite\Contracts\Factory'] = $mockSocialite; + $mockSocialDriver = \Mockery::mock('Laravel\Socialite\Contracts\Provider'); + $mockSocialUser = \Mockery::mock('\Laravel\Socialite\Contracts\User'); + + $mockSocialUser->shouldReceive('getId')->times(3)->andReturn(1); + $mockSocialUser->shouldReceive('getEmail')->times(2)->andReturn($user->email); + $mockSocialUser->shouldReceive('getName')->once()->andReturn($user->name); + $mockSocialUser->shouldReceive('getAvatar')->once()->andReturn('avatar_placeholder'); + + $mockSocialDriver->shouldReceive('user')->times(1)->andReturn($mockSocialUser); + $mockSocialite->shouldReceive('driver')->times(2)->with('google')->andReturn($mockSocialDriver); + $mockSocialDriver->shouldReceive('redirect')->once()->andReturn(redirect('/')); + + $this->get('/login/service/google'); + $this->get('/login/service/google/callback'); + + $this->assertDatabaseHas('users', ['name' => $user->name, 'email' => $user->email, 'email_confirmed' => true]); + $user = $user->whereEmail($user->email)->first(); + $this->assertDatabaseHas('social_accounts', ['user_id' => $user->id]); } }