Altered social auto-reg to be configurable per service

- Added {$service}_AUTO_REGISTER and {$service}_AUTO_CONFIRM_EMAIL env
options for each social auth system.
- Auto-register will allow registration from login, even if registration
is disabled.
- Auto-confirm-email indicates trust and will mark new registrants as
'email_confirmed' and skip 'confirmation email' flow.
- Also added covering tests.
This commit is contained in:
Dan Brown 2018-09-21 18:05:06 +01:00
parent 7ad8314bd7
commit e60d11ee04
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
9 changed files with 200 additions and 97 deletions

View File

@ -0,0 +1,6 @@
<?php namespace BookStack\Exceptions;
class SocialSignInAccountNotUsed extends SocialSignInException
{
}

View File

@ -2,7 +2,7 @@
namespace BookStack\Http\Controllers\Auth;
use BookStack\Exceptions\ConfirmationEmailException;
use BookStack\Exceptions\SocialSignInAccountNotUsed;
use BookStack\Exceptions\SocialSignInException;
use BookStack\Exceptions\UserRegistrationException;
use BookStack\Repos\UserRepo;
@ -16,6 +16,7 @@ use Illuminate\Http\Response;
use Validator;
use BookStack\Http\Controllers\Controller;
use Illuminate\Foundation\Auth\RegistersUsers;
use Laravel\Socialite\Contracts\User as SocialUser;
class RegisterController extends Controller
{
@ -133,25 +134,28 @@ class RegisterController extends Controller
* The registrations flow for all users.
* @param array $userData
* @param bool|false|SocialAccount $socialAccount
* @param bool $emailVerified
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
* @throws UserRegistrationException
*/
protected function registerUser(array $userData, $socialAccount = false)
protected function registerUser(array $userData, $socialAccount = false, $emailVerified = false)
{
if (setting('registration-restrict')) {
$restrictedEmailDomains = explode(',', str_replace(' ', '', setting('registration-restrict')));
$registrationRestrict = setting('registration-restrict');
if ($registrationRestrict) {
$restrictedEmailDomains = explode(',', str_replace(' ', '', $registrationRestrict));
$userEmailDomain = $domain = substr(strrchr($userData['email'], "@"), 1);
if (!in_array($userEmailDomain, $restrictedEmailDomains)) {
throw new UserRegistrationException(trans('auth.registration_email_domain_invalid'), '/register');
}
}
$newUser = $this->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);
}
}

View File

@ -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
]);
}

View File

@ -1,13 +1,12 @@
<?php namespace BookStack\Services;
use BookStack\Http\Requests\Request;
use GuzzleHttp\Exception\ClientException;
use BookStack\Exceptions\SocialSignInAccountNotUsed;
use Laravel\Socialite\Contracts\Factory as Socialite;
use BookStack\Exceptions\SocialDriverNotConfigured;
use BookStack\Exceptions\SocialSignInException;
use BookStack\Exceptions\UserRegistrationException;
use BookStack\Repos\UserRepo;
use BookStack\SocialAccount;
use Laravel\Socialite\Contracts\User as SocialUser;
class SocialAuthService
{
@ -58,18 +57,13 @@ class SocialAuthService
/**
* Handle the social registration process on callback.
* @param $socialDriver
* @return \Laravel\Socialite\Contracts\User
* @throws SocialDriverNotConfigured
* @param string $socialDriver
* @param SocialUser $socialUser
* @return SocialUser
* @throws UserRegistrationException
*/
public function handleRegistrationCallback($socialDriver)
public function handleRegistrationCallback(string $socialDriver, SocialUser $socialUser)
{
$driver = $this->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)

View File

@ -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' => [

View File

@ -35,8 +35,12 @@
<env name="STORAGE_TYPE" value="local"/>
<env name="GITHUB_APP_ID" value="aaaaaaaaaaaaaa"/>
<env name="GITHUB_APP_SECRET" value="aaaaaaaaaaaaaa"/>
<env name="GITHUB_AUTO_REGISTER" value=""/>
<env name="GITHUB_AUTO_CONFIRM_EMAIL" value=""/>
<env name="GOOGLE_APP_ID" value="aaaaaaaaaaaaaa"/>
<env name="GOOGLE_APP_SECRET" value="aaaaaaaaaaaaaa"/>
<env name="GOOGLE_AUTO_REGISTER" value=""/>
<env name="GOOGLE_AUTO_CONFIRM_EMAIL" value=""/>
<env name="APP_URL" value="http://bookstack.dev"/>
</php>
</phpunit>

View File

@ -43,8 +43,6 @@ return [
'reg_settings' => 'Registration Settings',
'reg_allow' => 'Allow registration?',
'reg_auto_social_allow' => 'Allow auto social registration?',
'reg_auto_social_allow_desc' => 'If the social user doesn\'t exist, automatically sign him up. Domain restriction is respected if set. Email is also automatically validated for this kind of social registration.',
'reg_default_role' => 'Default user role after registration',
'reg_confirm_email' => 'Require email confirmation?',
'reg_confirm_email_desc' => 'If domain restriction is used then email confirmation will be required and the below value will be ignored.',

View File

@ -136,11 +136,6 @@
@endforeach
</select>
</div>
<div class="form-group">
<label for="setting-autosocialregistration-confirmation">{{ trans('settings.reg_auto_social_allow') }}</label>
<p class="small">{{ trans('settings.reg_auto_social_allow_desc') }}</p>
@include('components.toggle-switch', ['name' => 'setting-autosocialregistration-confirmation', 'value' => setting('autosocialregistration-confirmation')])
</div>
<div class="form-group">
<label for="setting-registration-confirmation">{{ trans('settings.reg_confirm_email') }}</label>
<p class="small">{{ trans('settings.reg_confirm_email_desc') }}</p>

View File

@ -1,6 +1,6 @@
<?php namespace Tests;
class SocialAuthTest extends BrowserKitTest
class SocialAuthTest extends TestCase
{
public function test_social_registration()
@ -25,11 +25,11 @@ class SocialAuthTest extends BrowserKitTest
$mockSocialUser->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]);
}
}