diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index 9136b37b5..00ad630be 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -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; diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index 8f9a24cde..89ddd0011 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -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 diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index df3fd8d21..02b33ecd6 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -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(); diff --git a/tests/Auth/Saml2Test.php b/tests/Auth/Saml2Test.php index d0da45297..df0bb81c1 100644 --- a/tests/Auth/Saml2Test.php +++ b/tests/Auth/Saml2Test.php @@ -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);