diff --git a/app/Access/RegistrationService.php b/app/Access/RegistrationService.php index 1e40a9844..68992fbc6 100644 --- a/app/Access/RegistrationService.php +++ b/app/Access/RegistrationService.php @@ -14,20 +14,14 @@ use Illuminate\Support\Str; class RegistrationService { - protected $userRepo; - protected $emailConfirmationService; - - /** - * RegistrationService constructor. - */ - public function __construct(UserRepo $userRepo, EmailConfirmationService $emailConfirmationService) - { - $this->userRepo = $userRepo; - $this->emailConfirmationService = $emailConfirmationService; + public function __construct( + protected UserRepo $userRepo, + protected EmailConfirmationService $emailConfirmationService, + ) { } /** - * Check whether or not registrations are allowed in the app settings. + * Check if registrations are allowed in the app settings. * * @throws UserRegistrationException */ @@ -84,6 +78,7 @@ class RegistrationService public function registerUser(array $userData, ?SocialAccount $socialAccount = null, bool $emailConfirmed = false): User { $userEmail = $userData['email']; + $authSystem = $socialAccount ? $socialAccount->driver : auth()->getDefaultDriver(); // Email restriction $this->ensureEmailDomainAllowed($userEmail); @@ -94,6 +89,12 @@ class RegistrationService throw new UserRegistrationException(trans('errors.error_user_exists_different_creds', ['email' => $userEmail]), '/login'); } + /** @var ?bool $shouldRegister */ + $shouldRegister = Theme::dispatch(ThemeEvents::AUTH_PRE_REGISTER, $authSystem, $userData); + if ($shouldRegister === false) { + throw new UserRegistrationException(trans('errors.auth_pre_register_theme_prevention'), '/login'); + } + // Create the user $newUser = $this->userRepo->createWithoutActivity($userData, $emailConfirmed); $newUser->attachDefaultRole(); @@ -104,7 +105,7 @@ class RegistrationService } Activity::add(ActivityType::AUTH_REGISTER, $socialAccount ?? $newUser); - Theme::dispatch(ThemeEvents::AUTH_REGISTER, $socialAccount ? $socialAccount->driver : auth()->getDefaultDriver(), $newUser); + Theme::dispatch(ThemeEvents::AUTH_REGISTER, $authSystem, $newUser); // Start email confirmation flow if required if ($this->emailConfirmationService->confirmationRequired() && !$emailConfirmed) { @@ -138,7 +139,7 @@ class RegistrationService } $restrictedEmailDomains = explode(',', str_replace(' ', '', $registrationRestrict)); - $userEmailDomain = $domain = mb_substr(mb_strrchr($userEmail, '@'), 1); + $userEmailDomain = mb_substr(mb_strrchr($userEmail, '@'), 1); if (!in_array($userEmailDomain, $restrictedEmailDomains)) { $redirect = $this->registrationAllowed() ? '/register' : '/login'; diff --git a/app/Theming/ThemeEvents.php b/app/Theming/ThemeEvents.php index ff9e86e16..2d4900c96 100644 --- a/app/Theming/ThemeEvents.php +++ b/app/Theming/ThemeEvents.php @@ -23,7 +23,7 @@ class ThemeEvents * The provided $detail can be a string or a loggable type of model. You should check * the type before making use of this parameter. * - * @param string $type + * @param string $type * @param string|\BookStack\Activity\Models\Loggable $detail */ const ACTIVITY_LOGGED = 'activity_logged'; @@ -42,18 +42,37 @@ class ThemeEvents * system as a standard app user. This includes a user becoming logged in * after registration. This is not emitted upon API usage. * - * @param string $authSystem + * @param string $authSystem * @param \BookStack\Users\Models\User $user */ const AUTH_LOGIN = 'auth_login'; + /** + * Auth pre-register event. + * Runs right before a new user account is registered in the system by any authentication + * system as a standard app user including auto-registration systems used by LDAP, + * SAML, OIDC and social systems. It only includes self-registrations, + * not accounts created by others in the UI or via the REST API. + * It runs after any other normal validation steps. + * Any account/email confirmation occurs post-registration. + * The provided $userData contains the main details that would be used to create + * the account, and may depend on authentication method. + * If false is returned from the event, registration will be prevented and the user + * will be returned to the login page. + * + * @param string $authSystem + * @param array $userData + * @returns bool|null + */ + const AUTH_PRE_REGISTER = 'auth_pre_register'; + /** * Auth register event. * Runs right after a user is newly registered to the application by any authentication * system as a standard app user. This includes auto-registration systems used - * by LDAP, SAML and social systems. It only includes self-registrations. + * by LDAP, SAML, OIDC and social systems. It only includes self-registrations. * - * @param string $authSystem + * @param string $authSystem * @param \BookStack\Users\Models\User $user */ const AUTH_REGISTER = 'auth_register'; @@ -91,8 +110,8 @@ class ThemeEvents * * @param string $tagReference * @param string $replacementHTML - * @param \BookStack\Entities\Models\Page $currentPage - * @param ?\BookStack\Entities\Models\Page $referencedPage + * @param \BookStack\Entities\Models\Page $currentPage + * @param ?\BookStack\Entities\Models\Page $referencedPage */ const PAGE_INCLUDE_PARSE = 'page_include_parse'; @@ -133,7 +152,7 @@ class ThemeEvents * Provides both the original request and the currently resolved response. * Return values, if provided, will be used as a new response to use. * - * @param \Illuminate\Http\Request $request + * @param \Illuminate\Http\Request $request * @param \Illuminate\Http\Response|\Symfony\Component\HttpFoundation\BinaryFileResponse $response * @returns \Illuminate\Http\Response|null */ @@ -149,11 +168,11 @@ class ThemeEvents * If the listener returns a non-null value, that will be used as the POST data instead * of the system default. * - * @param string $event - * @param \BookStack\Activity\Models\Webhook $webhook + * @param string $event + * @param \BookStack\Activity\Models\Webhook $webhook * @param string|\BookStack\Activity\Models\Loggable $detail - * @param \BookStack\Users\Models\User $initiator - * @param int $initiatedTime + * @param \BookStack\Users\Models\User $initiator + * @param int $initiatedTime * @returns array|null */ const WEBHOOK_CALL_BEFORE = 'webhook_call_before'; diff --git a/lang/en/errors.php b/lang/en/errors.php index 607b6ea83..8773a78cb 100644 --- a/lang/en/errors.php +++ b/lang/en/errors.php @@ -10,6 +10,7 @@ return [ // Auth 'error_user_exists_different_creds' => 'A user with the email :email already exists but with different credentials.', + 'auth_pre_register_theme_prevention' => 'User account could not be registered for the provided details', 'email_already_confirmed' => 'Email has already been confirmed, Try logging in.', 'email_confirmation_invalid' => 'This confirmation token is not valid or has already been used, Please try registering again.', 'email_confirmation_expired' => 'The confirmation token has expired, A new confirmation email has been sent.', @@ -23,7 +24,6 @@ return [ 'saml_invalid_response_id' => 'The request from the external authentication system is not recognised by a process started by this application. Navigating back after a login could cause this issue.', 'saml_fail_authed' => 'Login using :system failed, system did not provide successful authorization', 'oidc_already_logged_in' => 'Already logged in', - 'oidc_user_not_registered' => 'The user :name is not registered and automatic registration is disabled', 'oidc_no_email_address' => 'Could not find an email address, for this user, in the data provided by the external authentication system', 'oidc_fail_authed' => 'Login using :system failed, system did not provide successful authorization', 'social_no_action_defined' => 'No action defined', diff --git a/tests/ThemeTest.php b/tests/ThemeTest.php index a7a46521a..8e12b88d8 100644 --- a/tests/ThemeTest.php +++ b/tests/ThemeTest.php @@ -178,6 +178,43 @@ class ThemeTest extends TestCase $this->assertInstanceOf(User::class, $args[1]); } + public function test_event_auth_pre_register() + { + $args = []; + $callback = function (...$eventArgs) use (&$args) { + $args = $eventArgs; + }; + Theme::listen(ThemeEvents::AUTH_PRE_REGISTER, $callback); + $this->setSettings(['registration-enabled' => 'true']); + + $user = User::factory()->make(); + $this->post('/register', ['email' => $user->email, 'name' => $user->name, 'password' => 'password']); + + $this->assertCount(2, $args); + $this->assertEquals('standard', $args[0]); + $this->assertEquals([ + 'email' => $user->email, + 'name' => $user->name, + 'password' => 'password', + ], $args[1]); + $this->assertDatabaseHas('users', ['email' => $user->email]); + } + + public function test_event_auth_pre_register_with_false_return_blocks_registration() + { + $callback = function () { + return false; + }; + Theme::listen(ThemeEvents::AUTH_PRE_REGISTER, $callback); + $this->setSettings(['registration-enabled' => 'true']); + + $user = User::factory()->make(); + $resp = $this->post('/register', ['email' => $user->email, 'name' => $user->name, 'password' => 'password']); + $resp->assertRedirect('/login'); + $this->assertSessionError('User account could not be registered for the provided details'); + $this->assertDatabaseMissing('users', ['email' => $user->email]); + } + public function test_event_webhook_call_before() { $args = [];