Mirrors/BookStack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2025-02-01 12:41:37 +01:00
Code Issues Releases Wiki Activity

Prevented email confirmation exception throw on registration

Browse Source 
Was preventing any other registration actions from taking place such as
LDAP/SAML group sync. Email confirmation should be actioned by
middleware on post-registration redirect.

Added testing to cover.
Tested for LDAP, SAML and normal registration with email confirmation
required to ensure flows work as expected.

Fixes #2082
This commit is contained in:
Dan Brown 2020-08-04 17:54:50 +01:00
parent c076ca408c
commit 87a5340a05
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
4 changed files with 70 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/Auth/Access/RegistrationService.php
View File

@ -71,15 +71,14 @@ class RegistrationService
// Start email confirmation flow if required
if ($this->emailConfirmationService->confirmationRequired() && !$emailConfirmed) {
$newUser->save();
$message = '';
try {
$this->emailConfirmationService->sendConfirmation($newUser);
} catch (Exception $e) {
$message = trans('auth.email_confirm_send_error');
throw new UserRegistrationException($message, '/register/confirm');
}
throw new UserRegistrationException($message, '/register/confirm');
}
return $newUser;

1
app/Auth/Access/Saml2Service.php
View File

@ -311,7 +311,6 @@ class Saml2Service extends ExternalAuthService
/**
* Get the user from the database for the specified details.
* @throws SamlException
* @throws UserRegistrationException
*/
protected function getOrRegisterUser(array $userDetails): ?User

42
tests/Auth/LdapTest.php
View File

@ -594,6 +594,48 @@ class LdapTest extends BrowserKitTest
$this->see('A user with the email tester@example.com already exists but with different credentials');
}
public function test_login_with_email_confirmation_required_maps_groups_but_shows_confirmation_screen()
{
$roleToReceive = factory(Role::class)->create(['display_name' => 'LdapTester']);
$user = factory(User::class)->make();
setting()->put('registration-confirmation', 'true');
app('config')->set([
'services.ldap.user_to_groups' => true,
'services.ldap.group_attribute' => 'memberOf',
'services.ldap.remove_from_groups' => true,
]);
$this->commonLdapMocks(1, 1, 3, 4, 3, 2);
$this->mockLdap->shouldReceive('searchAndGetEntries')
->times(3)
->andReturn(['count' => 1, 0 => [
'uid' => [$user->name],
'cn' => [$user->name],
'dn' => ['dc=test' . config('services.ldap.base_dn')],
'mail' => [$user->email],
'memberof' => [
'count' => 1,
0 => "cn=ldaptester,ou=groups,dc=example,dc=com",
]
]]);
$this->mockUserLogin()->seePageIs('/register/confirm/awaiting');
$this->seeInDatabase('users', [
'email' => $user->email,
'email_confirmed' => false,
]);
$user = User::query()->where('email', '=', $user->email)->first();
$this->seeInDatabase('role_user', [
'user_id' => $user->id,
'role_id' => $roleToReceive->id
]);
$homePage = $this->get('/');
$homePage->assertRedirectedTo('/register/confirm/awaiting');
}
public function test_failed_logins_are_logged_when_message_configured()
{
$log = $this->withTestLogger();

27
tests/Auth/Saml2Test.php
View File

@ -290,6 +290,33 @@ class Saml2Test extends TestCase
});
}
public function test_group_sync_functions_when_email_confirmation_required()
{
setting()->put('registration-confirmation', 'true');
config()->set([
'saml2.onelogin.strict' => false,
'saml2.user_to_groups' => true,
'saml2.remove_from_groups' => false,
]);
$memberRole = factory(Role::class)->create(['external_auth_id' => 'member']);
$adminRole = Role::getSystemRole('admin');
$this->withPost(['SAMLResponse' => $this->acsPostData], function () use ($memberRole, $adminRole) {
$acsPost = $this->followingRedirects()->post('/saml2/acs');
$acsPost->assertSee('Your email address has not yet been confirmed');
$user = User::query()->where('external_auth_id', '=', 'user')->first();
$userRoleIds = $user->roles()->pluck('id');
$this->assertContains($memberRole->id, $userRoleIds, 'User was assigned to member role');
$this->assertContains($adminRole->id, $userRoleIds, 'User was assigned to admin role');
$this->assertTrue($user->email_confirmed == false, 'User email remains unconfirmed');
});
$homeGet = $this->get('/');
$homeGet->assertRedirect('/register/confirm/awaiting');
}
protected function withGet(array $options, callable $callback)
{
return $this->withGlobal($_GET, $options, $callback);