diff --git a/app/Access/LoginService.php b/app/Access/LoginService.php index cc48e0f9b..6607969af 100644 --- a/app/Access/LoginService.php +++ b/app/Access/LoginService.php @@ -5,6 +5,7 @@ namespace BookStack\Access; use BookStack\Access\Mfa\MfaSession; use BookStack\Activity\ActivityType; use BookStack\Exceptions\LoginAttemptException; +use BookStack\Exceptions\LoginAttemptInvalidUserException; use BookStack\Exceptions\StoppedAuthenticationException; use BookStack\Facades\Activity; use BookStack\Facades\Theme; @@ -29,10 +30,14 @@ class LoginService * a reason to (MFA or Unconfirmed Email). * Returns a boolean to indicate the current login result. * - * @throws StoppedAuthenticationException + * @throws StoppedAuthenticationException|LoginAttemptInvalidUserException */ public function login(User $user, string $method, bool $remember = false): void { + if ($user->isGuest()) { + throw new LoginAttemptInvalidUserException('Login not allowed for guest user'); + } + if ($this->awaitingEmailConfirmation($user) || $this->needsMfaVerification($user)) { $this->setLastLoginAttemptedForUser($user, $method, $remember); @@ -58,7 +63,7 @@ class LoginService * * @throws Exception */ - public function reattemptLoginFor(User $user) + public function reattemptLoginFor(User $user): void { if ($user->id !== ($this->getLastLoginAttemptUser()->id ?? null)) { throw new Exception('Login reattempt user does align with current session state'); @@ -152,16 +157,40 @@ class LoginService */ public function attempt(array $credentials, string $method, bool $remember = false): bool { + if ($this->areCredentialsForGuest($credentials)) { + return false; + } + $result = auth()->attempt($credentials, $remember); if ($result) { $user = auth()->user(); auth()->logout(); - $this->login($user, $method, $remember); + try { + $this->login($user, $method, $remember); + } catch (LoginAttemptInvalidUserException $e) { + // Catch and return false for non-login accounts + // so it looks like a normal invalid login. + return false; + } } return $result; } + /** + * Check if the given credentials are likely for the system guest account. + */ + protected function areCredentialsForGuest(array $credentials): bool + { + if (isset($credentials['email'])) { + return User::query()->where('email', '=', $credentials['email']) + ->where('system_name', '=', 'public') + ->exists(); + } + + return false; + } + /** * Logs the current user out of the application. * Returns an app post-redirect path. diff --git a/app/Exceptions/LoginAttemptInvalidUserException.php b/app/Exceptions/LoginAttemptInvalidUserException.php new file mode 100644 index 000000000..163484c5a --- /dev/null +++ b/app/Exceptions/LoginAttemptInvalidUserException.php @@ -0,0 +1,7 @@ +assertSee('Too many login attempts. Please try again in'); } + public function test_login_specifically_disabled_for_guest_account() + { + $guest = $this->users->guest(); + + $resp = $this->post('/login', ['email' => $guest->email, 'password' => 'password']); + $resp->assertRedirect('/login'); + $resp = $this->followRedirects($resp); + $resp->assertSee('These credentials do not match our records.'); + + // Test login even with password somehow set + $guest->password = Hash::make('password'); + $guest->save(); + + $resp = $this->post('/login', ['email' => $guest->email, 'password' => 'password']); + $resp->assertRedirect('/login'); + $resp = $this->followRedirects($resp); + $resp->assertSee('These credentials do not match our records.'); + } + /** * Perform a login. */