diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php index a19f143b7..60b1630e0 100644 --- a/app/Actions/ActivityType.php +++ b/app/Actions/ActivityType.php @@ -50,4 +50,7 @@ class ActivityType const AUTH_PASSWORD_RESET_UPDATE = 'auth_password_reset_update'; const AUTH_LOGIN = 'auth_login'; const AUTH_REGISTER = 'auth_register'; + + const MFA_SETUP_METHOD = 'mfa_setup_method'; + const MFA_REMOVE_METHOD = 'mfa_remove_method'; } diff --git a/app/Api/ApiTokenGuard.php b/app/Api/ApiTokenGuard.php index 75ed5cb35..8b9cbc8e1 100644 --- a/app/Api/ApiTokenGuard.php +++ b/app/Api/ApiTokenGuard.php @@ -2,6 +2,7 @@ namespace BookStack\Api; +use BookStack\Auth\Access\LoginService; use BookStack\Exceptions\ApiAuthException; use Illuminate\Auth\GuardHelpers; use Illuminate\Contracts\Auth\Authenticatable; @@ -19,6 +20,11 @@ class ApiTokenGuard implements Guard */ protected $request; + /** + * @var LoginService + */ + protected $loginService; + /** * The last auth exception thrown in this request. * @@ -29,9 +35,10 @@ class ApiTokenGuard implements Guard /** * ApiTokenGuard constructor. */ - public function __construct(Request $request) + public function __construct(Request $request, LoginService $loginService) { $this->request = $request; + $this->loginService = $loginService; } /** @@ -95,6 +102,10 @@ class ApiTokenGuard implements Guard $this->validateToken($token, $secret); + if ($this->loginService->awaitingEmailConfirmation($token->user)) { + throw new ApiAuthException(trans('errors.email_confirmation_awaiting')); + } + return $token->user; } diff --git a/app/Auth/Access/EmailConfirmationService.php b/app/Auth/Access/EmailConfirmationService.php index 3425c1e72..75c03b318 100644 --- a/app/Auth/Access/EmailConfirmationService.php +++ b/app/Auth/Access/EmailConfirmationService.php @@ -14,9 +14,6 @@ class EmailConfirmationService extends UserTokenService /** * Create new confirmation for a user, * Also removes any existing old ones. - * - * @param User $user - * * @throws ConfirmationEmailException */ public function sendConfirmation(User $user) @@ -33,8 +30,6 @@ class EmailConfirmationService extends UserTokenService /** * Check if confirmation is required in this instance. - * - * @return bool */ public function confirmationRequired(): bool { diff --git a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php index 9c3b47e97..99bfd2e79 100644 --- a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php +++ b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php @@ -186,12 +186,8 @@ class ExternalBaseSessionGuard implements StatefulGuard */ public function loginUsingId($id, $remember = false) { - if (!is_null($user = $this->provider->retrieveById($id))) { - $this->login($user, $remember); - - return $user; - } - + // Always return false as to disable this method, + // Logins should route through LoginService. return false; } diff --git a/app/Auth/Access/LoginService.php b/app/Auth/Access/LoginService.php new file mode 100644 index 000000000..b251e4cc3 --- /dev/null +++ b/app/Auth/Access/LoginService.php @@ -0,0 +1,160 @@ +<?php + +namespace BookStack\Auth\Access; + +use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\Mfa\MfaSession; +use BookStack\Auth\User; +use BookStack\Exceptions\StoppedAuthenticationException; +use BookStack\Facades\Activity; +use BookStack\Facades\Theme; +use BookStack\Theming\ThemeEvents; +use Exception; + +class LoginService +{ + + protected const LAST_LOGIN_ATTEMPTED_SESSION_KEY = 'auth-login-last-attempted'; + + protected $mfaSession; + protected $emailConfirmationService; + + public function __construct(MfaSession $mfaSession, EmailConfirmationService $emailConfirmationService) + { + $this->mfaSession = $mfaSession; + $this->emailConfirmationService = $emailConfirmationService; + } + + /** + * Log the given user into the system. + * Will start a login of the given user but will prevent if there's + * a reason to (MFA or Unconfirmed Email). + * Returns a boolean to indicate the current login result. + * @throws StoppedAuthenticationException + */ + public function login(User $user, string $method, bool $remember = false): void + { + if ($this->awaitingEmailConfirmation($user) || $this->needsMfaVerification($user)) { + $this->setLastLoginAttemptedForUser($user, $method, $remember); + throw new StoppedAuthenticationException($user, $this); + } + + $this->clearLastLoginAttempted(); + auth()->login($user, $remember); + Activity::add(ActivityType::AUTH_LOGIN, "{$method}; {$user->logDescriptor()}"); + Theme::dispatch(ThemeEvents::AUTH_LOGIN, $method, $user); + + // Authenticate on all session guards if a likely admin + if ($user->can('users-manage') && $user->can('user-roles-manage')) { + $guards = ['standard', 'ldap', 'saml2']; + foreach ($guards as $guard) { + auth($guard)->login($user); + } + } + } + + /** + * Reattempt a system login after a previous stopped attempt. + * @throws Exception + */ + public function reattemptLoginFor(User $user) + { + if ($user->id !== ($this->getLastLoginAttemptUser()->id ?? null)) { + throw new Exception('Login reattempt user does align with current session state'); + } + + $lastLoginDetails = $this->getLastLoginAttemptDetails(); + $this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember'] ?? false); + } + + /** + * Get the last user that was attempted to be logged in. + * Only exists if the last login attempt had correct credentials + * but had been prevented by a secondary factor. + */ + public function getLastLoginAttemptUser(): ?User + { + $id = $this->getLastLoginAttemptDetails()['user_id']; + return User::query()->where('id', '=', $id)->first(); + } + + /** + * Get the details of the last login attempt. + * Checks upon a ttl of about 1 hour since that last attempted login. + * @return array{user_id: ?string, method: ?string, remember: bool} + */ + protected function getLastLoginAttemptDetails(): array + { + $value = session()->get(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY); + if (!$value) { + return ['user_id' => null, 'method' => null]; + } + + [$id, $method, $remember, $time] = explode(':', $value); + $hourAgo = time() - (60*60); + if ($time < $hourAgo) { + $this->clearLastLoginAttempted(); + return ['user_id' => null, 'method' => null]; + } + + return ['user_id' => $id, 'method' => $method, 'remember' => boolval($remember)]; + } + + /** + * Set the last login attempted user. + * Must be only used when credentials are correct and a login could be + * achieved but a secondary factor has stopped the login. + */ + protected function setLastLoginAttemptedForUser(User $user, string $method, bool $remember) + { + session()->put( + self::LAST_LOGIN_ATTEMPTED_SESSION_KEY, + implode(':', [$user->id, $method, $remember, time()]) + ); + } + + /** + * Clear the last login attempted session value. + */ + protected function clearLastLoginAttempted(): void + { + session()->remove(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY); + } + + /** + * Check if MFA verification is needed. + */ + public function needsMfaVerification(User $user): bool + { + return !$this->mfaSession->isVerifiedForUser($user) && $this->mfaSession->isRequiredForUser($user); + } + + /** + * Check if the given user is awaiting email confirmation. + */ + public function awaitingEmailConfirmation(User $user): bool + { + return $this->emailConfirmationService->confirmationRequired() && !$user->email_confirmed; + } + + /** + * Attempt the login of a user using the given credentials. + * Meant to mirror Laravel's default guard 'attempt' method + * but in a manner that always routes through our login system. + * May interrupt the flow if extra authentication requirements are imposed. + * + * @throws StoppedAuthenticationException + */ + public function attempt(array $credentials, string $method, bool $remember = false): bool + { + $result = auth()->attempt($credentials, $remember); + if ($result) { + $user = auth()->user(); + auth()->logout(); + $this->login($user, $method, $remember); + } + + return $result; + } + +} \ No newline at end of file diff --git a/app/Auth/Access/Mfa/BackupCodeService.php b/app/Auth/Access/Mfa/BackupCodeService.php new file mode 100644 index 000000000..ca58e7404 --- /dev/null +++ b/app/Auth/Access/Mfa/BackupCodeService.php @@ -0,0 +1,60 @@ +<?php + +namespace BookStack\Auth\Access\Mfa; + +use Illuminate\Support\Str; + +class BackupCodeService +{ + /** + * Generate a new set of 16 backup codes. + */ + public function generateNewSet(): array + { + $codes = []; + while (count($codes) < 16) { + $code = Str::random(5) . '-' . Str::random(5); + if (!in_array($code, $codes)) { + $codes[] = strtolower($code); + } + } + + return $codes; + } + + /** + * Check if the given code matches one of the available options. + */ + public function inputCodeExistsInSet(string $code, string $codeSet): bool + { + $cleanCode = $this->cleanInputCode($code); + $codes = json_decode($codeSet); + return in_array($cleanCode, $codes); + } + + /** + * Remove the given input code from the given available options. + * Will return a JSON string containing the codes. + */ + public function removeInputCodeFromSet(string $code, string $codeSet): string + { + $cleanCode = $this->cleanInputCode($code); + $codes = json_decode($codeSet); + $pos = array_search($cleanCode, $codes, true); + array_splice($codes, $pos, 1); + return json_encode($codes); + } + + /** + * Count the number of codes in the given set. + */ + public function countCodesInSet(string $codeSet): int + { + return count(json_decode($codeSet)); + } + + protected function cleanInputCode(string $code): string + { + return strtolower(str_replace(' ', '-', trim($code))); + } +} \ No newline at end of file diff --git a/app/Auth/Access/Mfa/MfaSession.php b/app/Auth/Access/Mfa/MfaSession.php new file mode 100644 index 000000000..8821dbb9d --- /dev/null +++ b/app/Auth/Access/Mfa/MfaSession.php @@ -0,0 +1,61 @@ +<?php + +namespace BookStack\Auth\Access\Mfa; + +use BookStack\Auth\User; + +class MfaSession +{ + /** + * Check if MFA is required for the given user. + */ + public function isRequiredForUser(User $user): bool + { + // TODO - Test both these cases + return $user->mfaValues()->exists() || $this->userRoleEnforcesMfa($user); + } + + /** + * Check if the given user is pending MFA setup. + * (MFA required but not yet configured). + */ + public function isPendingMfaSetup(User $user): bool + { + return $this->isRequiredForUser($user) && !$user->mfaValues()->exists(); + } + + /** + * Check if a role of the given user enforces MFA. + */ + protected function userRoleEnforcesMfa(User $user): bool + { + return $user->roles() + ->where('mfa_enforced', '=', true) + ->exists(); + } + + /** + * Check if the current MFA session has already been verified for the given user. + */ + public function isVerifiedForUser(User $user): bool + { + return session()->get($this->getMfaVerifiedSessionKey($user)) === 'true'; + } + + /** + * Mark the current session as MFA-verified. + */ + public function markVerifiedForUser(User $user): void + { + session()->put($this->getMfaVerifiedSessionKey($user), 'true'); + } + + /** + * Get the session key in which the MFA verification status is stored. + */ + protected function getMfaVerifiedSessionKey(User $user): string + { + return 'mfa-verification-passed:' . $user->id; + } + +} \ No newline at end of file diff --git a/app/Auth/Access/Mfa/MfaValue.php b/app/Auth/Access/Mfa/MfaValue.php new file mode 100644 index 000000000..ec0d5d563 --- /dev/null +++ b/app/Auth/Access/Mfa/MfaValue.php @@ -0,0 +1,76 @@ +<?php + +namespace BookStack\Auth\Access\Mfa; + +use BookStack\Auth\User; +use Carbon\Carbon; +use Illuminate\Database\Eloquent\Model; + +/** + * @property int $id + * @property int $user_id + * @property string $method + * @property string $value + * @property Carbon $created_at + * @property Carbon $updated_at + */ +class MfaValue extends Model +{ + protected static $unguarded = true; + + const METHOD_TOTP = 'totp'; + const METHOD_BACKUP_CODES = 'backup_codes'; + + /** + * Get all the MFA methods available. + */ + public static function allMethods(): array + { + return [self::METHOD_TOTP, self::METHOD_BACKUP_CODES]; + } + + /** + * Upsert a new MFA value for the given user and method + * using the provided value. + */ + public static function upsertWithValue(User $user, string $method, string $value): void + { + /** @var MfaValue $mfaVal */ + $mfaVal = static::query()->firstOrNew([ + 'user_id' => $user->id, + 'method' => $method + ]); + $mfaVal->setValue($value); + $mfaVal->save(); + } + + /** + * Easily get the decrypted MFA value for the given user and method. + */ + public static function getValueForUser(User $user, string $method): ?string + { + /** @var MfaValue $mfaVal */ + $mfaVal = static::query() + ->where('user_id', '=', $user->id) + ->where('method', '=', $method) + ->first(); + + return $mfaVal ? $mfaVal->getValue() : null; + } + + /** + * Decrypt the value attribute upon access. + */ + protected function getValue(): string + { + return decrypt($this->value); + } + + /** + * Encrypt the value attribute upon access. + */ + protected function setValue($value): void + { + $this->value = encrypt($value); + } +} diff --git a/app/Auth/Access/Mfa/TotpService.php b/app/Auth/Access/Mfa/TotpService.php new file mode 100644 index 000000000..d1013978b --- /dev/null +++ b/app/Auth/Access/Mfa/TotpService.php @@ -0,0 +1,71 @@ +<?php + +namespace BookStack\Auth\Access\Mfa; + +use BaconQrCode\Renderer\Color\Rgb; +use BaconQrCode\Renderer\Image\SvgImageBackEnd; +use BaconQrCode\Renderer\ImageRenderer; +use BaconQrCode\Renderer\RendererStyle\Fill; +use BaconQrCode\Renderer\RendererStyle\RendererStyle; +use BaconQrCode\Writer; +use PragmaRX\Google2FA\Google2FA; +use PragmaRX\Google2FA\Support\Constants; + +class TotpService +{ + protected $google2fa; + + public function __construct(Google2FA $google2fa) + { + $this->google2fa = $google2fa; + // Use SHA1 as a default, Personal testing of other options in 2021 found + // many apps lack support for other algorithms yet still will scan + // the code causing a confusing UX. + $this->google2fa->setAlgorithm(Constants::SHA1); + } + + /** + * Generate a new totp secret key. + */ + public function generateSecret(): string + { + /** @noinspection PhpUnhandledExceptionInspection */ + return $this->google2fa->generateSecretKey(); + } + + /** + * Generate a TOTP URL from secret key. + */ + public function generateUrl(string $secret): string + { + return $this->google2fa->getQRCodeUrl( + setting('app-name'), + user()->email, + $secret + ); + } + + /** + * Generate a QR code to display a TOTP URL. + */ + public function generateQrCodeSvg(string $url): string + { + $color = Fill::uniformColor(new Rgb(255, 255, 255), new Rgb(32, 110, 167)); + return (new Writer( + new ImageRenderer( + new RendererStyle(192, 0, null, null, $color), + new SvgImageBackEnd + ) + ))->writeString($url); + } + + /** + * Verify that the user provided code is valid for the secret. + * The secret must be known, not user-provided. + */ + public function verifyCode(string $code, string $secret): bool + { + /** @noinspection PhpUnhandledExceptionInspection */ + return $this->google2fa->verifyKey($secret, $code); + } +} \ No newline at end of file diff --git a/app/Auth/Access/Mfa/TotpValidationRule.php b/app/Auth/Access/Mfa/TotpValidationRule.php new file mode 100644 index 000000000..7fe3d8504 --- /dev/null +++ b/app/Auth/Access/Mfa/TotpValidationRule.php @@ -0,0 +1,38 @@ +<?php + +namespace BookStack\Auth\Access\Mfa; + +use Illuminate\Contracts\Validation\Rule; + +class TotpValidationRule implements Rule +{ + + protected $secret; + protected $totpService; + + /** + * Create a new rule instance. + * Takes the TOTP secret that must be system provided, not user provided. + */ + public function __construct(string $secret) + { + $this->secret = $secret; + $this->totpService = app()->make(TotpService::class); + } + + /** + * Determine if the validation rule passes. + */ + public function passes($attribute, $value) + { + return $this->totpService->verifyCode($value, $this->secret); + } + + /** + * Get the validation error message. + */ + public function message() + { + return trans('validation.totp'); + } +} diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index 16e3edbb4..4c8b53c0d 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -88,7 +88,6 @@ class RegistrationService session()->flash('sent-email-confirmation', true); } catch (Exception $e) { $message = trans('auth.email_confirm_send_error'); - throw new UserRegistrationException($message, '/register/confirm'); } } diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index 28d4d4030..3f0f40ccc 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -6,6 +6,7 @@ use BookStack\Actions\ActivityType; use BookStack\Auth\User; use BookStack\Exceptions\JsonDebugException; use BookStack\Exceptions\SamlException; +use BookStack\Exceptions\StoppedAuthenticationException; use BookStack\Exceptions\UserRegistrationException; use BookStack\Facades\Activity; use BookStack\Facades\Theme; @@ -25,16 +26,16 @@ class Saml2Service extends ExternalAuthService { protected $config; protected $registrationService; - protected $user; + protected $loginService; /** * Saml2Service constructor. */ - public function __construct(RegistrationService $registrationService, User $user) + public function __construct(RegistrationService $registrationService, LoginService $loginService) { $this->config = config('saml2'); $this->registrationService = $registrationService; - $this->user = $user; + $this->loginService = $loginService; } /** @@ -332,7 +333,7 @@ class Saml2Service extends ExternalAuthService */ protected function getOrRegisterUser(array $userDetails): ?User { - $user = $this->user->newQuery() + $user = User::query() ->where('external_auth_id', '=', $userDetails['external_id']) ->first(); @@ -357,6 +358,7 @@ class Saml2Service extends ExternalAuthService * @throws SamlException * @throws JsonDebugException * @throws UserRegistrationException + * @throws StoppedAuthenticationException */ public function processLoginCallback(string $samlID, array $samlAttributes): User { @@ -389,10 +391,7 @@ class Saml2Service extends ExternalAuthService $this->syncWithGroups($user, $groups); } - auth()->login($user); - Activity::add(ActivityType::AUTH_LOGIN, "saml2; {$user->logDescriptor()}"); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, 'saml2', $user); - + $this->loginService->login($user, 'saml2'); return $user; } } diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index 2f1a6876a..70f3fc3d3 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -2,15 +2,11 @@ namespace BookStack\Auth\Access; -use BookStack\Actions\ActivityType; use BookStack\Auth\SocialAccount; use BookStack\Auth\User; use BookStack\Exceptions\SocialDriverNotConfigured; use BookStack\Exceptions\SocialSignInAccountNotUsed; use BookStack\Exceptions\UserRegistrationException; -use BookStack\Facades\Activity; -use BookStack\Facades\Theme; -use BookStack\Theming\ThemeEvents; use Illuminate\Support\Facades\Event; use Illuminate\Support\Str; use Laravel\Socialite\Contracts\Factory as Socialite; @@ -28,6 +24,11 @@ class SocialAuthService */ protected $socialite; + /** + * @var LoginService + */ + protected $loginService; + /** * The default built-in social drivers we support. * @@ -59,9 +60,10 @@ class SocialAuthService /** * SocialAuthService constructor. */ - public function __construct(Socialite $socialite) + public function __construct(Socialite $socialite, LoginService $loginService) { $this->socialite = $socialite; + $this->loginService = $loginService; } /** @@ -139,10 +141,7 @@ class SocialAuthService // When a user is not logged in and a matching SocialAccount exists, // Simply log the user into the application. if (!$isLoggedIn && $socialAccount !== null) { - auth()->login($socialAccount->user); - Activity::add(ActivityType::AUTH_LOGIN, $socialAccount); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, $socialDriver, $socialAccount->user); - + $this->loginService->login($socialAccount->user, $socialAccount); return redirect()->intended('/'); } diff --git a/app/Auth/Permissions/PermissionsRepo.php b/app/Auth/Permissions/PermissionsRepo.php index 4d191679d..988146700 100644 --- a/app/Auth/Permissions/PermissionsRepo.php +++ b/app/Auth/Permissions/PermissionsRepo.php @@ -57,6 +57,7 @@ class PermissionsRepo public function saveNewRole(array $roleData): Role { $role = $this->role->newInstance($roleData); + $role->mfa_enforced = ($roleData['mfa_enforced'] ?? 'false') === 'true'; $role->save(); $permissions = isset($roleData['permissions']) ? array_keys($roleData['permissions']) : []; @@ -90,6 +91,7 @@ class PermissionsRepo $this->assignRolePermissions($role, $permissions); $role->fill($roleData); + $role->mfa_enforced = ($roleData['mfa_enforced'] ?? 'false') === 'true'; $role->save(); $this->permissionService->buildJointPermissionForRole($role); Activity::add(ActivityType::ROLE_UPDATE, $role); diff --git a/app/Auth/Role.php b/app/Auth/Role.php index 94ba39d1d..dcd960948 100644 --- a/app/Auth/Role.php +++ b/app/Auth/Role.php @@ -18,6 +18,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany; * @property string $description * @property string $external_auth_id * @property string $system_name + * @property bool $mfa_enforced */ class Role extends Model implements Loggable { diff --git a/app/Auth/User.php b/app/Auth/User.php index 1a3560691..0a6849fe0 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -4,6 +4,7 @@ namespace BookStack\Auth; use BookStack\Actions\Favourite; use BookStack\Api\ApiToken; +use BookStack\Auth\Access\Mfa\MfaValue; use BookStack\Entities\Tools\SlugGenerator; use BookStack\Interfaces\Loggable; use BookStack\Interfaces\Sluggable; @@ -38,6 +39,7 @@ use Illuminate\Support\Collection; * @property string $external_auth_id * @property string $system_name * @property Collection $roles + * @property Collection $mfaValues */ class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, Sluggable { @@ -265,6 +267,14 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon return $this->hasMany(Favourite::class); } + /** + * Get the MFA values belonging to this use. + */ + public function mfaValues(): HasMany + { + return $this->hasMany(MfaValue::class); + } + /** * Get the last activity time for this user. */ diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index 61ca12dcc..e1a040fc2 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -71,6 +71,7 @@ class UserRepo $query = User::query()->select(['*']) ->withLastActivityAt() ->with(['roles', 'avatar']) + ->withCount('mfaValues') ->orderBy($sort, $sortData['order']); if ($sortData['search']) { @@ -188,6 +189,7 @@ class UserRepo $user->socialAccounts()->delete(); $user->apiTokens()->delete(); $user->favourites()->delete(); + $user->mfaValues()->delete(); $user->delete(); // Delete user profile images diff --git a/app/Console/Commands/ResetMfa.php b/app/Console/Commands/ResetMfa.php new file mode 100644 index 000000000..feb477943 --- /dev/null +++ b/app/Console/Commands/ResetMfa.php @@ -0,0 +1,74 @@ +<?php + +namespace BookStack\Console\Commands; + +use BookStack\Auth\User; +use Illuminate\Console\Command; + +class ResetMfa extends Command +{ + /** + * The name and signature of the console command. + * + * @var string + */ + protected $signature = 'bookstack:reset-mfa + {--id= : Numeric ID of the user to reset MFA for} + {--email= : Email address of the user to reset MFA for} + '; + + /** + * The console command description. + * + * @var string + */ + protected $description = 'Reset & Clear any configured MFA methods for the given user'; + + /** + * Create a new command instance. + * + * @return void + */ + public function __construct() + { + parent::__construct(); + } + + /** + * Execute the console command. + * + * @return mixed + */ + public function handle() + { + $id = $this->option('id'); + $email = $this->option('email'); + if (!$id && !$email) { + $this->error('Either a --id=<number> or --email=<email> option must be provided.'); + return 1; + } + + /** @var User $user */ + $field = $id ? 'id' : 'email'; + $value = $id ?: $email; + $user = User::query() + ->where($field, '=', $value) + ->first(); + + if (!$user) { + $this->error("A user where {$field}={$value} could not be found."); + return 1; + } + + $this->info("This will delete any configure multi-factor authentication methods for user: \n- ID: {$user->id}\n- Name: {$user->name}\n- Email: {$user->email}\n"); + $this->info('If multi-factor authentication is required for this user they will be asked to reconfigure their methods on next login.'); + $confirm = $this->confirm('Are you sure you want to proceed?'); + if ($confirm) { + $user->mfaValues()->delete(); + $this->info('User MFA methods have been reset.'); + return 0; + } + + return 1; + } +} diff --git a/app/Exceptions/StoppedAuthenticationException.php b/app/Exceptions/StoppedAuthenticationException.php new file mode 100644 index 000000000..7a978ffc6 --- /dev/null +++ b/app/Exceptions/StoppedAuthenticationException.php @@ -0,0 +1,65 @@ +<?php + +namespace BookStack\Exceptions; + +use BookStack\Auth\Access\LoginService; +use BookStack\Auth\User; +use Illuminate\Contracts\Support\Responsable; +use Illuminate\Http\Request; + +class StoppedAuthenticationException extends \Exception implements Responsable +{ + + protected $user; + protected $loginService; + + /** + * StoppedAuthenticationException constructor. + */ + public function __construct(User $user, LoginService $loginService) + { + $this->user = $user; + $this->loginService = $loginService; + parent::__construct(); + } + + /** + * @inheritdoc + */ + public function toResponse($request) + { + $redirect = '/login'; + + if ($this->loginService->awaitingEmailConfirmation($this->user)) { + return $this->awaitingEmailConfirmationResponse($request); + } + + if ($this->loginService->needsMfaVerification($this->user)) { + $redirect = '/mfa/verify'; + } + + return redirect($redirect); + } + + /** + * Provide an error response for when the current user's email is not confirmed + * in a system which requires it. + */ + protected function awaitingEmailConfirmationResponse(Request $request) + { + if ($request->wantsJson()) { + return response()->json([ + 'error' => [ + 'code' => 401, + 'message' => trans('errors.email_confirmation_awaiting'), + ], + ], 401); + } + + if (session()->get('sent-email-confirmation') === true) { + return redirect('/register/confirm'); + } + + return redirect('/register/confirm/awaiting'); + } +} \ No newline at end of file diff --git a/app/Http/Controllers/Auth/ConfirmEmailController.php b/app/Http/Controllers/Auth/ConfirmEmailController.php index 63f220a68..520efdf88 100644 --- a/app/Http/Controllers/Auth/ConfirmEmailController.php +++ b/app/Http/Controllers/Auth/ConfirmEmailController.php @@ -2,15 +2,13 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Actions\ActivityType; use BookStack\Auth\Access\EmailConfirmationService; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\UserRepo; use BookStack\Exceptions\ConfirmationEmailException; use BookStack\Exceptions\UserTokenExpiredException; use BookStack\Exceptions\UserTokenNotFoundException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Exception; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; @@ -20,14 +18,20 @@ use Illuminate\View\View; class ConfirmEmailController extends Controller { protected $emailConfirmationService; + protected $loginService; protected $userRepo; /** * Create a new controller instance. */ - public function __construct(EmailConfirmationService $emailConfirmationService, UserRepo $userRepo) + public function __construct( + EmailConfirmationService $emailConfirmationService, + LoginService $loginService, + UserRepo $userRepo + ) { $this->emailConfirmationService = $emailConfirmationService; + $this->loginService = $loginService; $this->userRepo = $userRepo; } @@ -43,12 +47,11 @@ class ConfirmEmailController extends Controller /** * Shows a notice that a user's email address has not been confirmed, * Also has the option to re-send the confirmation email. - * - * @return View */ public function showAwaiting() { - return view('auth.user-unconfirmed'); + $user = $this->loginService->getLastLoginAttemptUser(); + return view('auth.user-unconfirmed', ['user' => $user]); } /** @@ -87,11 +90,9 @@ class ConfirmEmailController extends Controller $user->email_confirmed = true; $user->save(); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); - $this->showSuccessNotification(trans('auth.email_confirm_success')); $this->emailConfirmationService->deleteByUser($user); + $this->showSuccessNotification(trans('auth.email_confirm_success')); + $this->loginService->login($user, auth()->getDefaultDriver()); return redirect('/'); } diff --git a/app/Http/Controllers/Auth/HandlesPartialLogins.php b/app/Http/Controllers/Auth/HandlesPartialLogins.php new file mode 100644 index 000000000..4ce67d236 --- /dev/null +++ b/app/Http/Controllers/Auth/HandlesPartialLogins.php @@ -0,0 +1,25 @@ +<?php + +namespace BookStack\Http\Controllers\Auth; + +use BookStack\Auth\Access\LoginService; +use BookStack\Auth\User; +use BookStack\Exceptions\NotFoundException; + +trait HandlesPartialLogins +{ + /** + * @throws NotFoundException + */ + protected function currentOrLastAttemptedUser(): User + { + $loginService = app()->make(LoginService::class); + $user = auth()->user() ?? $loginService->getLastLoginAttemptUser(); + + if (!$user) { + throw new NotFoundException('A user for this action could not be found'); + } + + return $user; + } +} \ No newline at end of file diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 5154f7e97..174770bf9 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -3,13 +3,11 @@ namespace BookStack\Http\Controllers\Auth; use Activity; -use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Exceptions\LoginAttemptEmailNeededException; use BookStack\Exceptions\LoginAttemptException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Illuminate\Foundation\Auth\AuthenticatesUsers; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; @@ -37,16 +35,19 @@ class LoginController extends Controller protected $redirectAfterLogout = '/login'; protected $socialAuthService; + protected $loginService; /** * Create a new controller instance. */ - public function __construct(SocialAuthService $socialAuthService) + public function __construct(SocialAuthService $socialAuthService, LoginService $loginService) { $this->middleware('guest', ['only' => ['getLogin', 'login']]); $this->middleware('guard:standard,ldap', ['only' => ['login', 'logout']]); $this->socialAuthService = $socialAuthService; + $this->loginService = $loginService; + $this->redirectPath = url('/'); $this->redirectAfterLogout = url('/login'); } @@ -140,6 +141,19 @@ class LoginController extends Controller return $this->sendFailedLoginResponse($request); } + /** + * Attempt to log the user into the application. + * + * @param \Illuminate\Http\Request $request + * @return bool + */ + protected function attemptLogin(Request $request) + { + return $this->loginService->attempt( + $this->credentials($request), auth()->getDefaultDriver(), $request->filled('remember') + ); + } + /** * The user has been authenticated. * @@ -150,17 +164,6 @@ class LoginController extends Controller */ protected function authenticated(Request $request, $user) { - // Authenticate on all session guards if a likely admin - if ($user->can('users-manage') && $user->can('user-roles-manage')) { - $guards = ['standard', 'ldap', 'saml2']; - foreach ($guards as $guard) { - auth($guard)->login($user); - } - } - - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); - return redirect()->intended($this->redirectPath()); } diff --git a/app/Http/Controllers/Auth/MfaBackupCodesController.php b/app/Http/Controllers/Auth/MfaBackupCodesController.php new file mode 100644 index 000000000..4b4e11659 --- /dev/null +++ b/app/Http/Controllers/Auth/MfaBackupCodesController.php @@ -0,0 +1,95 @@ +<?php + +namespace BookStack\Http\Controllers\Auth; + +use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; +use BookStack\Auth\Access\Mfa\BackupCodeService; +use BookStack\Auth\Access\Mfa\MfaSession; +use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Exceptions\NotFoundException; +use BookStack\Http\Controllers\Controller; +use Exception; +use Illuminate\Http\Request; +use Illuminate\Validation\ValidationException; + +class MfaBackupCodesController extends Controller +{ + use HandlesPartialLogins; + + protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-backup-codes'; + + /** + * Show a view that generates and displays backup codes + */ + public function generate(BackupCodeService $codeService) + { + $codes = $codeService->generateNewSet(); + session()->put(self::SETUP_SECRET_SESSION_KEY, encrypt($codes)); + + $downloadUrl = 'data:application/octet-stream;base64,' . base64_encode(implode("\n\n", $codes)); + + return view('mfa.backup-codes-generate', [ + 'codes' => $codes, + 'downloadUrl' => $downloadUrl, + ]); + } + + /** + * Confirm the setup of backup codes, storing them against the user. + * @throws Exception + */ + public function confirm() + { + if (!session()->has(self::SETUP_SECRET_SESSION_KEY)) { + return response('No generated codes found in the session', 500); + } + + $codes = decrypt(session()->pull(self::SETUP_SECRET_SESSION_KEY)); + MfaValue::upsertWithValue($this->currentOrLastAttemptedUser(), MfaValue::METHOD_BACKUP_CODES, json_encode($codes)); + + $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'backup-codes'); + + if (!auth()->check()) { + $this->showSuccessNotification(trans('auth.mfa_setup_login_notification')); + return redirect('/login'); + } + + return redirect('/mfa/setup'); + } + + /** + * Verify the MFA method submission on check. + * @throws NotFoundException + * @throws ValidationException + */ + public function verify(Request $request, BackupCodeService $codeService, MfaSession $mfaSession, LoginService $loginService) + { + $user = $this->currentOrLastAttemptedUser(); + $codes = MfaValue::getValueForUser($user, MfaValue::METHOD_BACKUP_CODES) ?? '[]'; + + $this->validate($request, [ + 'code' => [ + 'required', + 'max:12', 'min:8', + function ($attribute, $value, $fail) use ($codeService, $codes) { + if (!$codeService->inputCodeExistsInSet($value, $codes)) { + $fail(trans('validation.backup_codes')); + } + } + ] + ]); + + $updatedCodes = $codeService->removeInputCodeFromSet($request->get('code'), $codes); + MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, $updatedCodes); + + $mfaSession->markVerifiedForUser($user); + $loginService->reattemptLoginFor($user); + + if ($codeService->countCodesInSet($updatedCodes) < 5) { + $this->showWarningNotification(trans('auth.mfa_backup_codes_usage_limit_warning')); + } + + return redirect()->intended(); + } +} diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php new file mode 100644 index 000000000..75cd46eef --- /dev/null +++ b/app/Http/Controllers/Auth/MfaController.php @@ -0,0 +1,69 @@ +<?php + +namespace BookStack\Http\Controllers\Auth; + +use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Http\Controllers\Controller; +use Illuminate\Http\Request; + +class MfaController extends Controller +{ + use HandlesPartialLogins; + + /** + * Show the view to setup MFA for the current user. + */ + public function setup() + { + $userMethods = $this->currentOrLastAttemptedUser() + ->mfaValues() + ->get(['id', 'method']) + ->groupBy('method'); + return view('mfa.setup', [ + 'userMethods' => $userMethods, + ]); + } + + /** + * Remove an MFA method for the current user. + * @throws \Exception + */ + public function remove(string $method) + { + if (in_array($method, MfaValue::allMethods())) { + $value = user()->mfaValues()->where('method', '=', $method)->first(); + if ($value) { + $value->delete(); + $this->logActivity(ActivityType::MFA_REMOVE_METHOD, $method); + } + } + + return redirect('/mfa/setup'); + } + + /** + * Show the page to start an MFA verification. + */ + public function verify(Request $request) + { + $desiredMethod = $request->get('method'); + $userMethods = $this->currentOrLastAttemptedUser() + ->mfaValues() + ->get(['id', 'method']) + ->groupBy('method'); + + // Basic search for the default option for a user. + // (Prioritises totp over backup codes) + $method = $userMethods->has($desiredMethod) ? $desiredMethod : $userMethods->keys()->sort()->reverse()->first(); + $otherMethods = $userMethods->keys()->filter(function($userMethod) use ($method) { + return $method !== $userMethod; + })->all(); + + return view('mfa.verify', [ + 'userMethods' => $userMethods, + 'method' => $method, + 'otherMethods' => $otherMethods, + ]); + } +} diff --git a/app/Http/Controllers/Auth/MfaTotpController.php b/app/Http/Controllers/Auth/MfaTotpController.php new file mode 100644 index 000000000..d55f08cff --- /dev/null +++ b/app/Http/Controllers/Auth/MfaTotpController.php @@ -0,0 +1,94 @@ +<?php + +namespace BookStack\Http\Controllers\Auth; + +use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; +use BookStack\Auth\Access\Mfa\MfaSession; +use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Auth\Access\Mfa\TotpService; +use BookStack\Auth\Access\Mfa\TotpValidationRule; +use BookStack\Exceptions\NotFoundException; +use BookStack\Http\Controllers\Controller; +use Illuminate\Http\Request; +use Illuminate\Validation\ValidationException; + +class MfaTotpController extends Controller +{ + use HandlesPartialLogins; + + protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-totp-secret'; + + /** + * Show a view that generates and displays a TOTP QR code. + */ + public function generate(TotpService $totp) + { + if (session()->has(static::SETUP_SECRET_SESSION_KEY)) { + $totpSecret = decrypt(session()->get(static::SETUP_SECRET_SESSION_KEY)); + } else { + $totpSecret = $totp->generateSecret(); + session()->put(static::SETUP_SECRET_SESSION_KEY, encrypt($totpSecret)); + } + + $qrCodeUrl = $totp->generateUrl($totpSecret); + $svg = $totp->generateQrCodeSvg($qrCodeUrl); + + return view('mfa.totp-generate', [ + 'secret' => $totpSecret, + 'svg' => $svg, + ]); + } + + /** + * Confirm the setup of TOTP and save the auth method secret + * against the current user. + * @throws ValidationException + * @throws NotFoundException + */ + public function confirm(Request $request) + { + $totpSecret = decrypt(session()->get(static::SETUP_SECRET_SESSION_KEY)); + $this->validate($request, [ + 'code' => [ + 'required', + 'max:12', 'min:4', + new TotpValidationRule($totpSecret), + ] + ]); + + MfaValue::upsertWithValue($this->currentOrLastAttemptedUser(), MfaValue::METHOD_TOTP, $totpSecret); + session()->remove(static::SETUP_SECRET_SESSION_KEY); + $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'totp'); + + if (!auth()->check()) { + $this->showSuccessNotification(trans('auth.mfa_setup_login_notification')); + return redirect('/login'); + } + + return redirect('/mfa/setup'); + } + + /** + * Verify the MFA method submission on check. + * @throws NotFoundException + */ + public function verify(Request $request, LoginService $loginService, MfaSession $mfaSession) + { + $user = $this->currentOrLastAttemptedUser(); + $totpSecret = MfaValue::getValueForUser($user, MfaValue::METHOD_TOTP); + + $this->validate($request, [ + 'code' => [ + 'required', + 'max:12', 'min:4', + new TotpValidationRule($totpSecret), + ] + ]); + + $mfaSession->markVerifiedForUser($user); + $loginService->reattemptLoginFor($user); + + return redirect()->intended(); + } +} diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 1728ece32..e372b38ef 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -2,14 +2,13 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\RegistrationService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Auth\User; +use BookStack\Exceptions\StoppedAuthenticationException; use BookStack\Exceptions\UserRegistrationException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Illuminate\Foundation\Auth\RegistersUsers; use Illuminate\Http\Request; use Illuminate\Support\Facades\Hash; @@ -32,6 +31,7 @@ class RegisterController extends Controller protected $socialAuthService; protected $registrationService; + protected $loginService; /** * Where to redirect users after login / registration. @@ -44,13 +44,18 @@ class RegisterController extends Controller /** * Create a new controller instance. */ - public function __construct(SocialAuthService $socialAuthService, RegistrationService $registrationService) + public function __construct( + SocialAuthService $socialAuthService, + RegistrationService $registrationService, + LoginService $loginService + ) { $this->middleware('guest'); $this->middleware('guard:standard'); $this->socialAuthService = $socialAuthService; $this->registrationService = $registrationService; + $this->loginService = $loginService; $this->redirectTo = url('/'); $this->redirectPath = url('/'); @@ -89,6 +94,7 @@ class RegisterController extends Controller * Handle a registration request for the application. * * @throws UserRegistrationException + * @throws StoppedAuthenticationException */ public function postRegister(Request $request) { @@ -98,9 +104,7 @@ class RegisterController extends Controller try { $user = $this->registrationService->registerUser($userData); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); + $this->loginService->login($user, auth()->getDefaultDriver()); } catch (UserRegistrationException $exception) { if ($exception->getMessage()) { $this->showErrorNotification($exception->getMessage()); diff --git a/app/Http/Controllers/Auth/SocialController.php b/app/Http/Controllers/Auth/SocialController.php index 1d1468698..2e9e62162 100644 --- a/app/Http/Controllers/Auth/SocialController.php +++ b/app/Http/Controllers/Auth/SocialController.php @@ -2,16 +2,14 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\RegistrationService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Exceptions\SocialDriverNotConfigured; use BookStack\Exceptions\SocialSignInAccountNotUsed; use BookStack\Exceptions\SocialSignInException; use BookStack\Exceptions\UserRegistrationException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Illuminate\Http\Request; use Illuminate\Support\Str; use Laravel\Socialite\Contracts\User as SocialUser; @@ -20,15 +18,21 @@ class SocialController extends Controller { protected $socialAuthService; protected $registrationService; + protected $loginService; /** * SocialController constructor. */ - public function __construct(SocialAuthService $socialAuthService, RegistrationService $registrationService) + public function __construct( + SocialAuthService $socialAuthService, + RegistrationService $registrationService, + LoginService $loginService + ) { $this->middleware('guest')->only(['getRegister', 'postRegister']); $this->socialAuthService = $socialAuthService; $this->registrationService = $registrationService; + $this->loginService = $loginService; } /** @@ -136,11 +140,8 @@ class SocialController extends Controller } $user = $this->registrationService->registerUser($userData, $socialAccount, $emailVerified); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, $socialDriver, $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); - $this->showSuccessNotification(trans('auth.register_success')); + $this->loginService->login($user, $socialDriver); return redirect('/'); } diff --git a/app/Http/Controllers/Auth/UserInviteController.php b/app/Http/Controllers/Auth/UserInviteController.php index 253e25507..bd1912b0b 100644 --- a/app/Http/Controllers/Auth/UserInviteController.php +++ b/app/Http/Controllers/Auth/UserInviteController.php @@ -2,14 +2,12 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\UserInviteService; use BookStack\Auth\UserRepo; use BookStack\Exceptions\UserTokenExpiredException; use BookStack\Exceptions\UserTokenNotFoundException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Exception; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; @@ -18,17 +16,19 @@ use Illuminate\Routing\Redirector; class UserInviteController extends Controller { protected $inviteService; + protected $loginService; protected $userRepo; /** * Create a new controller instance. */ - public function __construct(UserInviteService $inviteService, UserRepo $userRepo) + public function __construct(UserInviteService $inviteService, LoginService $loginService, UserRepo $userRepo) { $this->middleware('guest'); $this->middleware('guard:standard'); $this->inviteService = $inviteService; + $this->loginService = $loginService; $this->userRepo = $userRepo; } @@ -72,11 +72,9 @@ class UserInviteController extends Controller $user->email_confirmed = true; $user->save(); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); - $this->showSuccessNotification(trans('auth.user_invite_success', ['appName' => setting('app-name')])); $this->inviteService->deleteByUser($user); + $this->showSuccessNotification(trans('auth.user_invite_success', ['appName' => setting('app-name')])); + $this->loginService->login($user, auth()->getDefaultDriver()); return redirect('/'); } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index f7b2afef8..a0da220ee 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -123,17 +123,20 @@ class UserController extends Controller { $this->checkPermissionOrCurrentUser('users-manage', $id); - $user = $this->user->newQuery()->with(['apiTokens'])->findOrFail($id); + /** @var User $user */ + $user = $this->user->newQuery()->with(['apiTokens', 'mfaValues'])->findOrFail($id); $authMethod = ($user->system_name) ? 'system' : config('auth.method'); $activeSocialDrivers = $socialAuthService->getActiveDrivers(); + $mfaMethods = $user->mfaValues->groupBy('method'); $this->setPageTitle(trans('settings.user_profile')); $roles = $this->userRepo->getAllRoles(); return view('users.edit', [ 'user' => $user, 'activeSocialDrivers' => $activeSocialDrivers, + 'mfaMethods' => $mfaMethods, 'authMethod' => $authMethod, 'roles' => $roles, ]); diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 4f9bfc1e6..d1f5de917 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -53,5 +53,6 @@ class Kernel extends HttpKernel 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, 'perm' => \BookStack\Http\Middleware\PermissionMiddleware::class, 'guard' => \BookStack\Http\Middleware\CheckGuard::class, + 'mfa-setup' => \BookStack\Http\Middleware\AuthenticatedOrPendingMfa::class, ]; } diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php index 21d69810f..73192b0cf 100644 --- a/app/Http/Middleware/ApiAuthenticate.php +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -9,7 +9,6 @@ use Illuminate\Http\Request; class ApiAuthenticate { - use ChecksForEmailConfirmation; /** * Handle an incoming request. @@ -37,7 +36,6 @@ class ApiAuthenticate // Return if the user is already found to be signed in via session-based auth. // This is to make it easy to browser the API via browser after just logging into the system. if (signedInUser() || session()->isStarted()) { - $this->ensureEmailConfirmedIfRequested(); if (!user()->can('access-api')) { throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403); } @@ -50,7 +48,6 @@ class ApiAuthenticate // Validate the token and it's users API access auth()->authenticate(); - $this->ensureEmailConfirmedIfRequested(); } /** diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index 3b018cde0..dede8f260 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -7,47 +7,18 @@ use Illuminate\Http\Request; class Authenticate { - use ChecksForEmailConfirmation; - /** * Handle an incoming request. */ public function handle(Request $request, Closure $next) { - if ($this->awaitingEmailConfirmation()) { - return $this->emailConfirmationErrorResponse($request); - } - if (!hasAppAccess()) { if ($request->ajax()) { return response('Unauthorized.', 401); - } else { - return redirect()->guest(url('/login')); } + return redirect()->guest(url('/login')); } return $next($request); } - - /** - * Provide an error response for when the current user's email is not confirmed - * in a system which requires it. - */ - protected function emailConfirmationErrorResponse(Request $request) - { - if ($request->wantsJson()) { - return response()->json([ - 'error' => [ - 'code' => 401, - 'message' => trans('errors.email_confirmation_awaiting'), - ], - ], 401); - } - - if (session()->get('sent-email-confirmation') === true) { - return redirect('/register/confirm'); - } - - return redirect('/register/confirm/awaiting'); - } } diff --git a/app/Http/Middleware/AuthenticatedOrPendingMfa.php b/app/Http/Middleware/AuthenticatedOrPendingMfa.php new file mode 100644 index 000000000..febfef207 --- /dev/null +++ b/app/Http/Middleware/AuthenticatedOrPendingMfa.php @@ -0,0 +1,41 @@ +<?php + +namespace BookStack\Http\Middleware; + +use BookStack\Auth\Access\LoginService; +use BookStack\Auth\Access\Mfa\MfaSession; +use Closure; + +class AuthenticatedOrPendingMfa +{ + + protected $loginService; + protected $mfaSession; + + public function __construct(LoginService $loginService, MfaSession $mfaSession) + { + $this->loginService = $loginService; + $this->mfaSession = $mfaSession; + } + + + /** + * Handle an incoming request. + * + * @param \Illuminate\Http\Request $request + * @param \Closure $next + * @return mixed + */ + public function handle($request, Closure $next) + { + $user = auth()->user(); + $loggedIn = $user !== null; + $lastAttemptUser = $this->loginService->getLastLoginAttemptUser(); + + if ($loggedIn || ($lastAttemptUser && $this->mfaSession->isPendingMfaSetup($lastAttemptUser))) { + return $next($request); + } + + return redirect()->to(url('/login')); + } +} diff --git a/app/Http/Middleware/ChecksForEmailConfirmation.php b/app/Http/Middleware/ChecksForEmailConfirmation.php deleted file mode 100644 index 2eabeca16..000000000 --- a/app/Http/Middleware/ChecksForEmailConfirmation.php +++ /dev/null @@ -1,36 +0,0 @@ -<?php - -namespace BookStack\Http\Middleware; - -use BookStack\Exceptions\UnauthorizedException; - -trait ChecksForEmailConfirmation -{ - /** - * Check if the current user has a confirmed email if the instance deems it as required. - * Throws if confirmation is required by the user. - * - * @throws UnauthorizedException - */ - protected function ensureEmailConfirmedIfRequested() - { - if ($this->awaitingEmailConfirmation()) { - throw new UnauthorizedException(trans('errors.email_confirmation_awaiting')); - } - } - - /** - * Check if email confirmation is required and the current user is awaiting confirmation. - */ - protected function awaitingEmailConfirmation(): bool - { - if (auth()->check()) { - $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict')); - if ($requireConfirmation && !auth()->user()->email_confirmed) { - return true; - } - } - - return false; - } -} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index abee6bcda..f7c8fe492 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -3,6 +3,7 @@ namespace BookStack\Providers; use Blade; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Entities\BreadcrumbsViewComposer; use BookStack\Entities\Models\Book; @@ -68,7 +69,7 @@ class AppServiceProvider extends ServiceProvider }); $this->app->singleton(SocialAuthService::class, function ($app) { - return new SocialAuthService($app->make(SocialiteFactory::class)); + return new SocialAuthService($app->make(SocialiteFactory::class), $app->make(LoginService::class)); }); } } diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index 1a78214dc..71b7ab016 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -8,6 +8,7 @@ use BookStack\Auth\Access\ExternalBaseUserProvider; use BookStack\Auth\Access\Guards\LdapSessionGuard; use BookStack\Auth\Access\Guards\Saml2SessionGuard; use BookStack\Auth\Access\LdapService; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\RegistrationService; use Illuminate\Support\ServiceProvider; @@ -21,7 +22,7 @@ class AuthServiceProvider extends ServiceProvider public function boot() { Auth::extend('api-token', function ($app, $name, array $config) { - return new ApiTokenGuard($app['request']); + return new ApiTokenGuard($app['request'], $app->make(LoginService::class)); }); Auth::extend('ldap-session', function ($app, $name, array $config) { @@ -30,7 +31,7 @@ class AuthServiceProvider extends ServiceProvider return new LdapSessionGuard( $name, $provider, - $this->app['session.store'], + $app['session.store'], $app[LdapService::class], $app[RegistrationService::class] ); @@ -42,7 +43,7 @@ class AuthServiceProvider extends ServiceProvider return new Saml2SessionGuard( $name, $provider, - $this->app['session.store'], + $app['session.store'], $app[RegistrationService::class] ); }); diff --git a/composer.json b/composer.json index bbd689454..7362a085d 100644 --- a/composer.json +++ b/composer.json @@ -13,6 +13,7 @@ "ext-json": "*", "ext-mbstring": "*", "ext-xml": "*", + "bacon/bacon-qr-code": "^2.0", "barryvdh/laravel-dompdf": "^0.9.0", "barryvdh/laravel-snappy": "^0.4.8", "doctrine/dbal": "^2.12.1", @@ -26,6 +27,7 @@ "league/html-to-markdown": "^5.0.0", "nunomaduro/collision": "^3.1", "onelogin/php-saml": "^4.0", + "pragmarx/google2fa": "^8.0", "predis/predis": "^1.1.6", "socialiteproviders/discord": "^4.1", "socialiteproviders/gitlab": "^4.1", diff --git a/composer.lock b/composer.lock index a9ba1b0a4..306068abb 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d1109d0dc4a6ab525cdbf64ed21f6dd4", + "content-hash": "4d845f3c8b77c8d73bf92c9223ddd805", "packages": [ { "name": "aws/aws-sdk-php", @@ -96,6 +96,59 @@ }, "time": "2021-08-04T18:12:21+00:00" }, + { + "name": "bacon/bacon-qr-code", + "version": "2.0.4", + "source": { + "type": "git", + "url": "https://github.com/Bacon/BaconQrCode.git", + "reference": "f73543ac4e1def05f1a70bcd1525c8a157a1ad09" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Bacon/BaconQrCode/zipball/f73543ac4e1def05f1a70bcd1525c8a157a1ad09", + "reference": "f73543ac4e1def05f1a70bcd1525c8a157a1ad09", + "shasum": "" + }, + "require": { + "dasprid/enum": "^1.0.3", + "ext-iconv": "*", + "php": "^7.1 || ^8.0" + }, + "require-dev": { + "phly/keep-a-changelog": "^1.4", + "phpunit/phpunit": "^7 | ^8 | ^9", + "squizlabs/php_codesniffer": "^3.4" + }, + "suggest": { + "ext-imagick": "to generate QR code images" + }, + "type": "library", + "autoload": { + "psr-4": { + "BaconQrCode\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-2-Clause" + ], + "authors": [ + { + "name": "Ben Scholzen 'DASPRiD'", + "email": "mail@dasprids.de", + "homepage": "https://dasprids.de/", + "role": "Developer" + } + ], + "description": "BaconQrCode is a QR code generator for PHP.", + "homepage": "https://github.com/Bacon/BaconQrCode", + "support": { + "issues": "https://github.com/Bacon/BaconQrCode/issues", + "source": "https://github.com/Bacon/BaconQrCode/tree/2.0.4" + }, + "time": "2021-06-18T13:26:35+00:00" + }, { "name": "barryvdh/laravel-dompdf", "version": "v0.9.0", @@ -227,6 +280,53 @@ }, "time": "2020-09-07T12:33:10+00:00" }, + { + "name": "dasprid/enum", + "version": "1.0.3", + "source": { + "type": "git", + "url": "https://github.com/DASPRiD/Enum.git", + "reference": "5abf82f213618696dda8e3bf6f64dd042d8542b2" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/DASPRiD/Enum/zipball/5abf82f213618696dda8e3bf6f64dd042d8542b2", + "reference": "5abf82f213618696dda8e3bf6f64dd042d8542b2", + "shasum": "" + }, + "require-dev": { + "phpunit/phpunit": "^7 | ^8 | ^9", + "squizlabs/php_codesniffer": "^3.4" + }, + "type": "library", + "autoload": { + "psr-4": { + "DASPRiD\\Enum\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-2-Clause" + ], + "authors": [ + { + "name": "Ben Scholzen 'DASPRiD'", + "email": "mail@dasprids.de", + "homepage": "https://dasprids.de/", + "role": "Developer" + } + ], + "description": "PHP 7.1 enum implementation", + "keywords": [ + "enum", + "map" + ], + "support": { + "issues": "https://github.com/DASPRiD/Enum/issues", + "source": "https://github.com/DASPRiD/Enum/tree/1.0.3" + }, + "time": "2020-10-02T16:03:48+00:00" + }, { "name": "doctrine/cache", "version": "2.1.1", @@ -2801,6 +2901,73 @@ }, "time": "2021-04-09T13:42:10+00:00" }, + { + "name": "paragonie/constant_time_encoding", + "version": "v2.4.0", + "source": { + "type": "git", + "url": "https://github.com/paragonie/constant_time_encoding.git", + "reference": "f34c2b11eb9d2c9318e13540a1dbc2a3afbd939c" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/paragonie/constant_time_encoding/zipball/f34c2b11eb9d2c9318e13540a1dbc2a3afbd939c", + "reference": "f34c2b11eb9d2c9318e13540a1dbc2a3afbd939c", + "shasum": "" + }, + "require": { + "php": "^7|^8" + }, + "require-dev": { + "phpunit/phpunit": "^6|^7|^8|^9", + "vimeo/psalm": "^1|^2|^3|^4" + }, + "type": "library", + "autoload": { + "psr-4": { + "ParagonIE\\ConstantTime\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Paragon Initiative Enterprises", + "email": "security@paragonie.com", + "homepage": "https://paragonie.com", + "role": "Maintainer" + }, + { + "name": "Steve 'Sc00bz' Thomas", + "email": "steve@tobtu.com", + "homepage": "https://www.tobtu.com", + "role": "Original Developer" + } + ], + "description": "Constant-time Implementations of RFC 4648 Encoding (Base-64, Base-32, Base-16)", + "keywords": [ + "base16", + "base32", + "base32_decode", + "base32_encode", + "base64", + "base64_decode", + "base64_encode", + "bin2hex", + "encoding", + "hex", + "hex2bin", + "rfc4648" + ], + "support": { + "email": "info@paragonie.com", + "issues": "https://github.com/paragonie/constant_time_encoding/issues", + "source": "https://github.com/paragonie/constant_time_encoding" + }, + "time": "2020-12-06T15:14:20+00:00" + }, { "name": "paragonie/random_compat", "version": "v9.99.99", @@ -3107,6 +3274,58 @@ ], "time": "2020-07-20T17:29:33+00:00" }, + { + "name": "pragmarx/google2fa", + "version": "8.0.0", + "source": { + "type": "git", + "url": "https://github.com/antonioribeiro/google2fa.git", + "reference": "26c4c5cf30a2844ba121760fd7301f8ad240100b" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/antonioribeiro/google2fa/zipball/26c4c5cf30a2844ba121760fd7301f8ad240100b", + "reference": "26c4c5cf30a2844ba121760fd7301f8ad240100b", + "shasum": "" + }, + "require": { + "paragonie/constant_time_encoding": "^1.0|^2.0", + "php": "^7.1|^8.0" + }, + "require-dev": { + "phpstan/phpstan": "^0.12.18", + "phpunit/phpunit": "^7.5.15|^8.5|^9.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "PragmaRX\\Google2FA\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Antonio Carlos Ribeiro", + "email": "acr@antoniocarlosribeiro.com", + "role": "Creator & Designer" + } + ], + "description": "A One Time Password Authentication package, compatible with Google Authenticator.", + "keywords": [ + "2fa", + "Authentication", + "Two Factor Authentication", + "google2fa" + ], + "support": { + "issues": "https://github.com/antonioribeiro/google2fa/issues", + "source": "https://github.com/antonioribeiro/google2fa/tree/8.0.0" + }, + "time": "2020-04-05T10:47:18+00:00" + }, { "name": "predis/predis", "version": "v1.1.7", diff --git a/database/migrations/2021_06_30_173111_create_mfa_values_table.php b/database/migrations/2021_06_30_173111_create_mfa_values_table.php new file mode 100644 index 000000000..937fd31d9 --- /dev/null +++ b/database/migrations/2021_06_30_173111_create_mfa_values_table.php @@ -0,0 +1,34 @@ +<?php + +use Illuminate\Database\Migrations\Migration; +use Illuminate\Database\Schema\Blueprint; +use Illuminate\Support\Facades\Schema; + +class CreateMfaValuesTable extends Migration +{ + /** + * Run the migrations. + * + * @return void + */ + public function up() + { + Schema::create('mfa_values', function (Blueprint $table) { + $table->increments('id'); + $table->integer('user_id')->index(); + $table->string('method', 20)->index(); + $table->text('value'); + $table->timestamps(); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists('mfa_values'); + } +} diff --git a/database/migrations/2021_07_03_085038_add_mfa_enforced_to_roles_table.php b/database/migrations/2021_07_03_085038_add_mfa_enforced_to_roles_table.php new file mode 100644 index 000000000..c14d47ea7 --- /dev/null +++ b/database/migrations/2021_07_03_085038_add_mfa_enforced_to_roles_table.php @@ -0,0 +1,32 @@ +<?php + +use Illuminate\Database\Migrations\Migration; +use Illuminate\Database\Schema\Blueprint; +use Illuminate\Support\Facades\Schema; + +class AddMfaEnforcedToRolesTable extends Migration +{ + /** + * Run the migrations. + * + * @return void + */ + public function up() + { + Schema::table('roles', function (Blueprint $table) { + $table->boolean('mfa_enforced'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('roles', function (Blueprint $table) { + $table->dropColumn('mfa_enforced'); + }); + } +} diff --git a/readme.md b/readme.md index 1b8c66061..c0bef7c7a 100644 --- a/readme.md +++ b/readme.md @@ -189,4 +189,6 @@ These are the great open-source projects used to help build BookStack: * [OneLogin's SAML PHP Toolkit](https://github.com/onelogin/php-saml) * [League/CommonMark](https://commonmark.thephpleague.com/) * [League/Flysystem](https://flysystem.thephpleague.com) -* [StyleCI](https://styleci.io/) \ No newline at end of file +* [StyleCI](https://styleci.io/) +* [pragmarx/google2fa](https://github.com/antonioribeiro/google2fa) +* [Bacon/BaconQrCode](https://github.com/Bacon/BaconQrCode) \ No newline at end of file diff --git a/resources/lang/en/activities.php b/resources/lang/en/activities.php index 5917de2cf..50bda60bd 100644 --- a/resources/lang/en/activities.php +++ b/resources/lang/en/activities.php @@ -47,6 +47,10 @@ return [ 'favourite_add_notification' => '":name" has been added to your favourites', 'favourite_remove_notification' => '":name" has been removed from your favourites', + // MFA + 'mfa_setup_method_notification' => 'Multi-factor method successfully configured', + 'mfa_remove_method_notification' => 'Multi-factor method successfully removed', + // Other 'commented_on' => 'commented on', 'permissions_update' => 'updated permissions', diff --git a/resources/lang/en/auth.php b/resources/lang/en/auth.php index d64fce93a..e4d4c425b 100644 --- a/resources/lang/en/auth.php +++ b/resources/lang/en/auth.php @@ -73,5 +73,40 @@ return [ 'user_invite_page_welcome' => 'Welcome to :appName!', 'user_invite_page_text' => 'To finalise your account and gain access you need to set a password which will be used to log-in to :appName on future visits.', 'user_invite_page_confirm_button' => 'Confirm Password', - 'user_invite_success' => 'Password set, you now have access to :appName!' + 'user_invite_success' => 'Password set, you now have access to :appName!', + + // Multi-factor Authentication + 'mfa_setup' => 'Setup Multi-Factor Authentication', + 'mfa_setup_desc' => 'Setup multi-factor authentication as an extra layer of security for your user account.', + 'mfa_setup_configured' => 'Already configured', + 'mfa_setup_reconfigure' => 'Reconfigure', + 'mfa_setup_remove_confirmation' => 'Are you sure you want to remove this multi-factor authentication method?', + 'mfa_setup_action' => 'Setup', + 'mfa_backup_codes_usage_limit_warning' => 'You have less than 5 backup codes remaining, Please generate and store a new set before you run out of codes to prevent being locked out of your account.', + 'mfa_option_totp_title' => 'Mobile App', + 'mfa_option_totp_desc' => 'To use multi-factor authentication you\'ll need a mobile application that supports TOTP such as Google Authenticator, Authy or Microsoft Authenticator.', + 'mfa_option_backup_codes_title' => 'Backup Codes', + 'mfa_option_backup_codes_desc' => 'Securely store a set of one-time-use backup codes which you can enter to verify your identity.', + 'mfa_gen_confirm_and_enable' => 'Confirm and Enable', + 'mfa_gen_backup_codes_title' => 'Backup Codes Setup', + 'mfa_gen_backup_codes_desc' => 'Store the below list of codes in a safe place. When accessing the system you\'ll be able to use one of the codes as a second authentication mechanism.', + 'mfa_gen_backup_codes_download' => 'Download Codes', + 'mfa_gen_backup_codes_usage_warning' => 'Each code can only be used once', + 'mfa_gen_totp_title' => 'Mobile App Setup', + 'mfa_gen_totp_desc' => 'To use multi-factor authentication you\'ll need a mobile application that supports TOTP such as Google Authenticator, Authy or Microsoft Authenticator.', + 'mfa_gen_totp_scan' => 'Scan the QR code below using your preferred authentication app to get started.', + 'mfa_gen_totp_verify_setup' => 'Verify Setup', + 'mfa_gen_totp_verify_setup_desc' => 'Verify that all is working by entering a code, generated within your authentication app, in the input box below:', + 'mfa_gen_totp_provide_code_here' => 'Provide your app generated code here', + 'mfa_verify_access' => 'Verify Access', + 'mfa_verify_access_desc' => 'Your user account requires you to confirm your identity via an additional level of verification before you\'re granted access. Verify using one of your configured methods to continue.', + 'mfa_verify_no_methods' => 'No Methods Configured', + 'mfa_verify_no_methods_desc' => 'No multi-factor authentication methods could be found for your account. You\'ll need to set up at least one method before you gain access.', + 'mfa_verify_use_totp' => 'Verify using a mobile app', + 'mfa_verify_use_backup_codes' => 'Verify using a backup code', + 'mfa_verify_backup_code' => 'Backup Code', + 'mfa_verify_backup_code_desc' => 'Enter one of your remaining backup codes below:', + 'mfa_verify_backup_code_enter_here' => 'Enter backup code here', + 'mfa_verify_totp_desc' => 'Enter the code, generated using your mobile app, below:', + 'mfa_setup_login_notification' => 'Multi-factor method configured, Please now login again using the configured method.', ]; \ No newline at end of file diff --git a/resources/lang/en/common.php b/resources/lang/en/common.php index 1861869e3..f93fb034b 100644 --- a/resources/lang/en/common.php +++ b/resources/lang/en/common.php @@ -39,6 +39,7 @@ return [ 'reset' => 'Reset', 'remove' => 'Remove', 'add' => 'Add', + 'configure' => 'Configure', 'fullscreen' => 'Fullscreen', 'favourite' => 'Favourite', 'unfavourite' => 'Unfavourite', diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 789ef9d1b..87f672e4d 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -138,6 +138,7 @@ return [ 'role_details' => 'Role Details', 'role_name' => 'Role Name', 'role_desc' => 'Short Description of Role', + 'role_mfa_enforced' => 'Requires Multi-Factor Authentication', 'role_external_auth_id' => 'External Authentication IDs', 'role_system' => 'System Permissions', 'role_manage_users' => 'Manage users', @@ -204,6 +205,10 @@ return [ 'users_api_tokens_create' => 'Create Token', 'users_api_tokens_expires' => 'Expires', 'users_api_tokens_docs' => 'API Documentation', + 'users_mfa' => 'Multi-Factor Authentication', + 'users_mfa_desc' => 'Setup multi-factor authentication as an extra layer of security for your user account.', + 'users_mfa_x_methods' => ':count method configured|:count methods configured', + 'users_mfa_configure' => 'Configure Methods', // API Tokens 'user_api_token_create' => 'Create API Token', diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index 4031de2ae..1963b0df2 100644 --- a/resources/lang/en/validation.php +++ b/resources/lang/en/validation.php @@ -15,6 +15,7 @@ return [ 'alpha_dash' => 'The :attribute may only contain letters, numbers, dashes and underscores.', 'alpha_num' => 'The :attribute may only contain letters and numbers.', 'array' => 'The :attribute must be an array.', + 'backup_codes' => 'The provided code is not valid or has already been used.', 'before' => 'The :attribute must be a date before :date.', 'between' => [ 'numeric' => 'The :attribute must be between :min and :max.', @@ -98,6 +99,7 @@ return [ ], 'string' => 'The :attribute must be a string.', 'timezone' => 'The :attribute must be a valid zone.', + 'totp' => 'The provided code is not valid or has expired.', 'unique' => 'The :attribute has already been taken.', 'url' => 'The :attribute format is invalid.', 'uploaded' => 'The file could not be uploaded. The server may not accept files of this size.', diff --git a/resources/sass/_forms.scss b/resources/sass/_forms.scss index 953d1d060..bb6d17f82 100644 --- a/resources/sass/_forms.scss +++ b/resources/sass/_forms.scss @@ -29,6 +29,10 @@ } } +.input-fill-width { + width: 100% !important; +} + .fake-input { @extend .input-base; overflow: auto; diff --git a/resources/sass/_layout.scss b/resources/sass/_layout.scss index 516d7d612..e26948301 100644 --- a/resources/sass/_layout.scss +++ b/resources/sass/_layout.scss @@ -181,6 +181,10 @@ body.flexbox { display: inline-block !important; } +.relative { + position: relative; +} + .hidden { display: none !important; } diff --git a/resources/views/auth/user-unconfirmed.blade.php b/resources/views/auth/user-unconfirmed.blade.php index 85473685b..5f1edd2b9 100644 --- a/resources/views/auth/user-unconfirmed.blade.php +++ b/resources/views/auth/user-unconfirmed.blade.php @@ -17,8 +17,8 @@ {!! csrf_field() !!} <div class="form-group"> <label for="email">{{ trans('auth.email') }}</label> - @if(auth()->check()) - @include('form.text', ['name' => 'email', 'model' => auth()->user()]) + @if($user) + @include('form.text', ['name' => 'email', 'model' => $user]) @else @include('form.text', ['name' => 'email']) @endif diff --git a/resources/views/mfa/backup-codes-generate.blade.php b/resources/views/mfa/backup-codes-generate.blade.php new file mode 100644 index 000000000..279ca68a0 --- /dev/null +++ b/resources/views/mfa/backup-codes-generate.blade.php @@ -0,0 +1,36 @@ +@extends('simple-layout') + +@section('body') + + <div class="container very-small py-xl"> + <div class="card content-wrap auto-height"> + <h1 class="list-heading">{{ trans('auth.mfa_gen_backup_codes_title') }}</h1> + <p>{{ trans('auth.mfa_gen_backup_codes_desc') }}</p> + + <div class="text-center mb-xs"> + <div class="text-bigger code-base p-m" style="column-count: 2"> + @foreach($codes as $code) + {{ $code }} <br> + @endforeach + </div> + </div> + + <p class="text-right"> + <a href="{{ $downloadUrl }}" download="backup-codes.txt" class="button outline small">{{ trans('auth.mfa_gen_backup_codes_download') }}</a> + </p> + + <p class="callout warning"> + {{ trans('auth.mfa_gen_backup_codes_usage_warning') }} + </p> + + <form action="{{ url('/mfa/backup_codes/confirm') }}" method="POST"> + {{ csrf_field() }} + <div class="mt-s text-right"> + <a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('common.cancel') }}</a> + <button class="button">{{ trans('auth.mfa_gen_confirm_and_enable') }}</button> + </div> + </form> + </div> + </div> + +@stop diff --git a/resources/views/mfa/setup-method-row.blade.php b/resources/views/mfa/setup-method-row.blade.php new file mode 100644 index 000000000..e195174c1 --- /dev/null +++ b/resources/views/mfa/setup-method-row.blade.php @@ -0,0 +1,30 @@ +<div class="grid half gap-xl"> + <div> + <div class="setting-list-label">{{ trans('auth.mfa_option_' . $method . '_title') }}</div> + <p class="small"> + {{ trans('auth.mfa_option_' . $method . '_desc') }} + </p> + </div> + <div class="pt-m"> + @if($userMethods->has($method)) + <div class="text-pos"> + @icon('check-circle') + {{ trans('auth.mfa_setup_configured') }} + </div> + <a href="{{ url('/mfa/' . $method . '/generate') }}" class="button outline small">{{ trans('auth.mfa_setup_reconfigure') }}</a> + <div component="dropdown" class="inline relative"> + <button type="button" refs="dropdown@toggle" class="button outline small">{{ trans('common.remove') }}</button> + <div refs="dropdown@menu" class="dropdown-menu"> + <p class="text-neg small px-m mb-xs">{{ trans('auth.mfa_setup_remove_confirmation') }}</p> + <form action="{{ url('/mfa/' . $method . '/remove') }}" method="post"> + {{ csrf_field() }} + {{ method_field('delete') }} + <button class="text-primary small delete">{{ trans('common.confirm') }}</button> + </form> + </div> + </div> + @else + <a href="{{ url('/mfa/' . $method . '/generate') }}" class="button outline">{{ trans('auth.mfa_setup_action') }}</a> + @endif + </div> +</div> \ No newline at end of file diff --git a/resources/views/mfa/setup.blade.php b/resources/views/mfa/setup.blade.php new file mode 100644 index 000000000..f3f290d4b --- /dev/null +++ b/resources/views/mfa/setup.blade.php @@ -0,0 +1,18 @@ +@extends('simple-layout') + +@section('body') + <div class="container small py-xl"> + + <div class="card content-wrap auto-height"> + <h1 class="list-heading">{{ trans('auth.mfa_setup') }}</h1> + <p class="mb-none"> {{ trans('auth.mfa_setup_desc') }}</p> + + <div class="setting-list"> + @foreach(['totp', 'backup_codes'] as $method) + @include('mfa.setup-method-row', ['method' => $method]) + @endforeach + </div> + + </div> + </div> +@stop diff --git a/resources/views/mfa/totp-generate.blade.php b/resources/views/mfa/totp-generate.blade.php new file mode 100644 index 000000000..8c470516f --- /dev/null +++ b/resources/views/mfa/totp-generate.blade.php @@ -0,0 +1,37 @@ +@extends('simple-layout') + +@section('body') + + <div class="container very-small py-xl"> + <div class="card content-wrap auto-height"> + <h1 class="list-heading">{{ trans('auth.mfa_gen_totp_title') }}</h1> + <p>{{ trans('auth.mfa_gen_totp_desc') }}</p> + <p>{{ trans('auth.mfa_gen_totp_scan') }}</p> + + <div class="text-center"> + <div class="block inline"> + {!! $svg !!} + </div> + </div> + + <h2 class="list-heading">{{ trans('auth.mfa_gen_totp_verify_setup') }}</h2> + <p id="totp-verify-input-details" class="mb-s">{{ trans('auth.mfa_gen_totp_verify_setup_desc') }}</p> + <form action="{{ url('/mfa/totp/confirm') }}" method="POST"> + {{ csrf_field() }} + <input type="text" + name="code" + aria-labelledby="totp-verify-input-details" + placeholder="{{ trans('auth.mfa_gen_totp_provide_code_here') }}" + class="input-fill-width {{ $errors->has('code') ? 'neg' : '' }}"> + @if($errors->has('code')) + <div class="text-neg text-small px-xs">{{ $errors->first('code') }}</div> + @endif + <div class="mt-s text-right"> + <a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('common.cancel') }}</a> + <button class="button">{{ trans('auth.mfa_gen_confirm_and_enable') }}</button> + </div> + </form> + </div> + </div> + +@stop diff --git a/resources/views/mfa/verify.blade.php b/resources/views/mfa/verify.blade.php new file mode 100644 index 000000000..3a1905e97 --- /dev/null +++ b/resources/views/mfa/verify.blade.php @@ -0,0 +1,35 @@ +@extends('simple-layout') + +@section('body') + <div class="container very-small py-xl"> + + <div class="card content-wrap auto-height"> + <h1 class="list-heading">{{ trans('auth.mfa_verify_access') }}</h1> + <p class="mb-none">{{ trans('auth.mfa_verify_access_desc') }}</p> + + @if(!$method) + <hr class="my-l"> + <h5>{{ trans('auth.mfa_verify_no_methods') }}</h5> + <p class="small">{{ trans('auth.mfa_verify_no_methods_desc') }}</p> + <div> + <a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('common.configure') }}</a> + </div> + @endif + + @if($method) + <hr class="my-l"> + @include('mfa.verify.' . $method) + @endif + + @if(count($otherMethods) > 0) + <hr class="my-l"> + @foreach($otherMethods as $otherMethod) + <div class="text-center"> + <a href="{{ url("/mfa/verify?method={$otherMethod}") }}">{{ trans('auth.mfa_verify_use_' . $otherMethod) }}</a> + </div> + @endforeach + @endif + + </div> + </div> +@stop diff --git a/resources/views/mfa/verify/backup_codes.blade.php b/resources/views/mfa/verify/backup_codes.blade.php new file mode 100644 index 000000000..0e5b82086 --- /dev/null +++ b/resources/views/mfa/verify/backup_codes.blade.php @@ -0,0 +1,17 @@ +<div class="setting-list-label">{{ trans('auth.mfa_verify_backup_code') }}</div> + +<p class="small mb-m">{{ trans('auth.mfa_verify_backup_code_desc') }}</p> + +<form action="{{ url('/mfa/backup_codes/verify') }}" method="post"> + {{ csrf_field() }} + <input type="text" + name="code" + placeholder="{{ trans('auth.mfa_verify_backup_code_enter_here') }}" + class="input-fill-width {{ $errors->has('code') ? 'neg' : '' }}"> + @if($errors->has('code')) + <div class="text-neg text-small px-xs">{{ $errors->first('code') }}</div> + @endif + <div class="mt-s text-right"> + <button class="button">{{ trans('common.confirm') }}</button> + </div> +</form> \ No newline at end of file diff --git a/resources/views/mfa/verify/totp.blade.php b/resources/views/mfa/verify/totp.blade.php new file mode 100644 index 000000000..9a861fc6c --- /dev/null +++ b/resources/views/mfa/verify/totp.blade.php @@ -0,0 +1,17 @@ +<div class="setting-list-label">{{ trans('auth.mfa_option_totp_title') }}</div> + +<p class="small mb-m">{{ trans('auth.mfa_verify_totp_desc') }}</p> + +<form action="{{ url('/mfa/totp/verify') }}" method="post"> + {{ csrf_field() }} + <input type="text" + name="code" + placeholder="{{ trans('auth.mfa_gen_totp_provide_code_here') }}" + class="input-fill-width {{ $errors->has('code') ? 'neg' : '' }}"> + @if($errors->has('code')) + <div class="text-neg text-small px-xs">{{ $errors->first('code') }}</div> + @endif + <div class="mt-s text-right"> + <button class="button">{{ trans('common.confirm') }}</button> + </div> +</form> \ No newline at end of file diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index 604acbb16..d1a61f0cd 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -11,13 +11,16 @@ </div> <div> <div class="form-group"> - <label for="name">{{ trans('settings.role_name') }}</label> + <label for="display_name">{{ trans('settings.role_name') }}</label> @include('form.text', ['name' => 'display_name']) </div> <div class="form-group"> - <label for="name">{{ trans('settings.role_desc') }}</label> + <label for="description">{{ trans('settings.role_desc') }}</label> @include('form.text', ['name' => 'description']) </div> + <div class="form-group"> + @include('form.checkbox', ['name' => 'mfa_enforced', 'label' => trans('settings.role_mfa_enforced') ]) + </div> @if(config('auth.method') === 'ldap' || config('auth.method') === 'saml2') <div class="form-group"> diff --git a/resources/views/settings/roles/index.blade.php b/resources/views/settings/roles/index.blade.php index 47cd8c920..898a96eef 100644 --- a/resources/views/settings/roles/index.blade.php +++ b/resources/views/settings/roles/index.blade.php @@ -27,7 +27,12 @@ @foreach($roles as $role) <tr> <td><a href="{{ url("/settings/roles/{$role->id}") }}">{{ $role->display_name }}</a></td> - <td>{{ $role->description }}</td> + <td> + @if($role->mfa_enforced) + <span title="{{ trans('settings.role_mfa_enforced') }}">@icon('lock') </span> + @endif + {{ $role->description }} + </td> <td class="text-center">{{ $role->users->count() }}</td> </tr> @endforeach diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index 5712855e6..d882558a4 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -63,6 +63,27 @@ </form> </section> + <section class="card content-wrap auto-height"> + <h2 class="list-heading">{{ trans('settings.users_mfa') }}</h2> + <p>{{ trans('settings.users_mfa_desc') }}</p> + <div class="grid half gap-xl v-center pb-s"> + <div> + @if ($mfaMethods->count() > 0) + <span class="text-pos">@icon('check-circle')</span> + @else + <span class="text-neg">@icon('cancel')</span> + @endif + {{ trans_choice('settings.users_mfa_x_methods', $mfaMethods->count()) }} + </div> + <div class="text-m-right"> + @if($user->id === user()->id) + <a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('settings.users_mfa_configure') }}</a> + @endif + </div> + </div> + + </section> + @if(user()->id === $user->id && count($activeSocialDrivers) > 0) <section class="card content-wrap auto-height"> <h2 class="list-heading">{{ trans('settings.users_social_accounts') }}</h2> diff --git a/resources/views/users/index.blade.php b/resources/views/users/index.blade.php index 5eef51175..9a9221242 100644 --- a/resources/views/users/index.blade.php +++ b/resources/views/users/index.blade.php @@ -43,7 +43,12 @@ <td class="text-center" style="line-height: 0;"><img class="avatar med" src="{{ $user->getAvatar(40)}}" alt="{{ $user->name }}"></td> <td> <a href="{{ url("/settings/users/{$user->id}") }}"> - {{ $user->name }} <br> <span class="text-muted">{{ $user->email }}</span> + {{ $user->name }} + <br> + <span class="text-muted">{{ $user->email }}</span> + @if($user->mfa_values_count > 0) + <span title="MFA Configured" class="text-pos">@icon('lock')</span> + @endif </a> </td> <td> diff --git a/routes/web.php b/routes/web.php index bc9705e10..a70d0d818 100644 --- a/routes/web.php +++ b/routes/web.php @@ -223,14 +223,28 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/roles/{id}', 'RoleController@edit'); Route::put('/roles/{id}', 'RoleController@update'); }); + }); +// MFA routes +Route::group(['middleware' => 'mfa-setup'], function() { + Route::get('/mfa/setup', 'Auth\MfaController@setup'); + Route::get('/mfa/totp/generate', 'Auth\MfaTotpController@generate'); + Route::post('/mfa/totp/confirm', 'Auth\MfaTotpController@confirm'); + Route::get('/mfa/backup_codes/generate', 'Auth\MfaBackupCodesController@generate'); + Route::post('/mfa/backup_codes/confirm', 'Auth\MfaBackupCodesController@confirm'); +}); +Route::group(['middleware' => 'guest'], function() { + Route::get('/mfa/verify', 'Auth\MfaController@verify'); + Route::post('/mfa/totp/verify', 'Auth\MfaTotpController@verify'); + Route::post('/mfa/backup_codes/verify', 'Auth\MfaBackupCodesController@verify'); +}); +Route::delete('/mfa/{method}/remove', 'Auth\MfaController@remove')->middleware('auth'); + // Social auth routes Route::get('/login/service/{socialDriver}', 'Auth\SocialController@login'); Route::get('/login/service/{socialDriver}/callback', 'Auth\SocialController@callback'); -Route::group(['middleware' => 'auth'], function () { - Route::post('/login/service/{socialDriver}/detach', 'Auth\SocialController@detach'); -}); +Route::post('/login/service/{socialDriver}/detach', 'Auth\SocialController@detach')->middleware('auth'); Route::get('/register/service/{socialDriver}', 'Auth\SocialController@register'); // Login/Logout routes diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index febf58399..b4b99d130 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -2,6 +2,7 @@ namespace Tests\Auth; +use BookStack\Auth\Access\Mfa\MfaSession; use BookStack\Auth\Role; use BookStack\Auth\User; use BookStack\Entities\Models\Page; @@ -131,7 +132,8 @@ class AuthTest extends BrowserKitTest ->seePageIs('/register/confirm/awaiting') ->see('Resend') ->visit('/books') - ->seePageIs('/register/confirm/awaiting') + ->seePageIs('/login') + ->visit('/register/confirm/awaiting') ->press('Resend Confirmation Email'); // Get confirmation and confirm notification matches @@ -172,10 +174,7 @@ class AuthTest extends BrowserKitTest ->seePageIs('/register/confirm') ->seeInDatabase('users', ['name' => $user->name, 'email' => $user->email, 'email_confirmed' => false]); - $this->visit('/') - ->seePageIs('/register/confirm/awaiting'); - - auth()->logout(); + $this->assertNull(auth()->user()); $this->visit('/')->seePageIs('/login') ->type($user->email, '#email') @@ -209,10 +208,8 @@ class AuthTest extends BrowserKitTest ->seePageIs('/register/confirm') ->seeInDatabase('users', ['name' => $user->name, 'email' => $user->email, 'email_confirmed' => false]); - $this->visit('/') - ->seePageIs('/register/confirm/awaiting'); + $this->assertNull(auth()->user()); - auth()->logout(); $this->visit('/')->seePageIs('/login') ->type($user->email, '#email') ->type($user->password, '#password') @@ -330,6 +327,18 @@ class AuthTest extends BrowserKitTest ->seePageIs('/login'); } + public function test_mfa_session_cleared_on_logout() + { + $user = $this->getEditor(); + $mfaSession = $this->app->make(MfaSession::class); + + $mfaSession->markVerifiedForUser($user);; + $this->assertTrue($mfaSession->isVerifiedForUser($user)); + + $this->asAdmin()->visit('/logout'); + $this->assertFalse($mfaSession->isVerifiedForUser($user)); + } + public function test_reset_password_flow() { Notification::fake(); @@ -410,6 +419,14 @@ class AuthTest extends BrowserKitTest $login->assertRedirectedTo('http://localhost'); } + public function test_login_intended_redirect_does_not_factor_mfa_routes() + { + $this->get('/books')->assertRedirectedTo('/login'); + $this->get('/mfa/setup')->assertRedirectedTo('/login'); + $login = $this->post('/login', ['email' => 'admin@admin.com', 'password' => 'password']); + $login->assertRedirectedTo('/books'); + } + public function test_login_authenticates_admins_on_all_guards() { $this->post('/login', ['email' => 'admin@admin.com', 'password' => 'password']); diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 9d491fd0f..3bd6988e4 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -27,18 +27,18 @@ class LdapTest extends TestCase define('LDAP_OPT_REFERRALS', 1); } config()->set([ - 'auth.method' => 'ldap', - 'auth.defaults.guard' => 'ldap', - 'services.ldap.base_dn' => 'dc=ldap,dc=local', - 'services.ldap.email_attribute' => 'mail', + 'auth.method' => 'ldap', + 'auth.defaults.guard' => 'ldap', + 'services.ldap.base_dn' => 'dc=ldap,dc=local', + 'services.ldap.email_attribute' => 'mail', 'services.ldap.display_name_attribute' => 'cn', - 'services.ldap.id_attribute' => 'uid', - 'services.ldap.user_to_groups' => false, - 'services.ldap.version' => '3', - 'services.ldap.user_filter' => '(&(uid=${user}))', - 'services.ldap.follow_referrals' => false, - 'services.ldap.tls_insecure' => false, - 'services.ldap.thumbnail_attribute' => null, + 'services.ldap.id_attribute' => 'uid', + 'services.ldap.user_to_groups' => false, + 'services.ldap.version' => '3', + 'services.ldap.user_filter' => '(&(uid=${user}))', + 'services.ldap.follow_referrals' => false, + 'services.ldap.tls_insecure' => false, + 'services.ldap.thumbnail_attribute' => null, ]); $this->mockLdap = \Mockery::mock(Ldap::class); $this->app[Ldap::class] = $this->mockLdap; @@ -70,9 +70,9 @@ class LdapTest extends TestCase protected function mockUserLogin(?string $email = null): TestResponse { return $this->post('/login', [ - 'username' => $this->mockUser->name, - 'password' => $this->mockUser->password, - ] + ($email ? ['email' => $email] : [])); + 'username' => $this->mockUser->name, + 'password' => $this->mockUser->password, + ] + ($email ? ['email' => $email] : [])); } /** @@ -95,8 +95,8 @@ class LdapTest extends TestCase ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], ]]); $resp = $this->mockUserLogin(); @@ -109,8 +109,8 @@ class LdapTest extends TestCase $resp->assertElementExists('#home-default'); $resp->assertSee($this->mockUser->name); $this->assertDatabaseHas('users', [ - 'email' => $this->mockUser->email, - 'email_confirmed' => false, + 'email' => $this->mockUser->email, + 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, ]); } @@ -126,8 +126,8 @@ class LdapTest extends TestCase ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], ]]); $resp = $this->mockUserLogin(); @@ -150,8 +150,8 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'cn' => [$this->mockUser->name], - 'dn' => $ldapDn, + 'cn' => [$this->mockUser->name], + 'dn' => $ldapDn, 'mail' => [$this->mockUser->email], ]]); @@ -170,10 +170,10 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'cn' => [$this->mockUser->name], - 'dn' => $ldapDn, + 'cn' => [$this->mockUser->name], + 'dn' => $ldapDn, 'my_custom_id' => ['cooluser456'], - 'mail' => [$this->mockUser->email], + 'mail' => [$this->mockUser->email], ]]); $resp = $this->mockUserLogin(); @@ -189,8 +189,8 @@ class LdapTest extends TestCase ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], ]]); $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true, false); @@ -219,14 +219,14 @@ class LdapTest extends TestCase $userForm->assertDontSee('Password'); $save = $this->post('/settings/users/create', [ - 'name' => $this->mockUser->name, + 'name' => $this->mockUser->name, 'email' => $this->mockUser->email, ]); $save->assertSessionHasErrors(['external_auth_id' => 'The external auth id field is required.']); $save = $this->post('/settings/users/create', [ - 'name' => $this->mockUser->name, - 'email' => $this->mockUser->email, + 'name' => $this->mockUser->name, + 'email' => $this->mockUser->email, 'external_auth_id' => $this->mockUser->name, ]); $save->assertRedirect('/settings/users'); @@ -241,8 +241,8 @@ class LdapTest extends TestCase $editPage->assertDontSee('Password'); $update = $this->put("/settings/users/{$editUser->id}", [ - 'name' => $editUser->name, - 'email' => $editUser->email, + 'name' => $editUser->name, + 'email' => $editUser->email, 'external_auth_id' => 'test_auth_id', ]); $update->assertRedirect('/settings/users'); @@ -271,8 +271,8 @@ class LdapTest extends TestCase $this->mockUser->attachRole($existingRole); app('config')->set([ - 'services.ldap.user_to_groups' => true, - 'services.ldap.group_attribute' => 'memberOf', + 'services.ldap.user_to_groups' => true, + 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => false, ]); @@ -280,14 +280,14 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], - 'mail' => [$this->mockUser->email], + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'mail' => [$this->mockUser->email], 'memberof' => [ 'count' => 2, - 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com', - 1 => 'cn=ldaptester-second,ou=groups,dc=example,dc=com', + 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com', + 1 => 'cn=ldaptester-second,ou=groups,dc=example,dc=com', ], ]]); @@ -316,8 +316,8 @@ class LdapTest extends TestCase $this->mockUser->attachRole($existingRole); app('config')->set([ - 'services.ldap.user_to_groups' => true, - 'services.ldap.group_attribute' => 'memberOf', + 'services.ldap.user_to_groups' => true, + 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); @@ -325,13 +325,13 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], - 'mail' => [$this->mockUser->email], + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'mail' => [$this->mockUser->email], 'memberof' => [ 'count' => 1, - 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com', + 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com', ], ]]); @@ -361,8 +361,8 @@ class LdapTest extends TestCase $roleToNotReceive = factory(Role::class)->create(['display_name' => 'ex-auth-a', 'external_auth_id' => 'test-second-param']); app('config')->set([ - 'services.ldap.user_to_groups' => true, - 'services.ldap.group_attribute' => 'memberOf', + 'services.ldap.user_to_groups' => true, + 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); @@ -370,13 +370,13 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], - 'mail' => [$this->mockUser->email], + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'mail' => [$this->mockUser->email], 'memberof' => [ 'count' => 1, - 0 => 'cn=ex-auth-a,ou=groups,dc=example,dc=com', + 0 => 'cn=ex-auth-a,ou=groups,dc=example,dc=com', ], ]]); @@ -402,8 +402,8 @@ class LdapTest extends TestCase setting()->put('registration-role', $roleToReceive->id); app('config')->set([ - 'services.ldap.user_to_groups' => true, - 'services.ldap.group_attribute' => 'memberOf', + 'services.ldap.user_to_groups' => true, + 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); @@ -411,14 +411,14 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], - 'mail' => [$this->mockUser->email], + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'mail' => [$this->mockUser->email], 'memberof' => [ 'count' => 2, - 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com', - 1 => 'cn=ldaptester-second,ou=groups,dc=example,dc=com', + 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com', + 1 => 'cn=ldaptester-second,ou=groups,dc=example,dc=com', ], ]]); @@ -445,9 +445,9 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], 'displayname' => 'displayNameAttribute', ]]); @@ -471,8 +471,8 @@ class LdapTest extends TestCase ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], ]]); $this->mockUserLogin()->assertRedirect('/login'); @@ -482,10 +482,10 @@ class LdapTest extends TestCase $resp->assertRedirect('/'); $this->get('/')->assertSee($this->mockUser->name); $this->assertDatabaseHas('users', [ - 'email' => $this->mockUser->email, - 'email_confirmed' => false, + 'email' => $this->mockUser->email, + 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, - 'name' => $this->mockUser->name, + 'name' => $this->mockUser->name, ]); } @@ -499,8 +499,8 @@ class LdapTest extends TestCase $this->commonLdapMocks(0, 1, 1, 2, 1); $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], ]]); $this->mockLdap->shouldReceive('connect')->once() @@ -566,8 +566,8 @@ class LdapTest extends TestCase ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], ]]); $resp = $this->post('/login', [ @@ -575,7 +575,7 @@ class LdapTest extends TestCase 'password' => $this->mockUser->password, ]); $resp->assertJsonStructure([ - 'details_from_ldap' => [], + 'details_from_ldap' => [], 'details_bookstack_parsed' => [], ]); } @@ -605,8 +605,8 @@ class LdapTest extends TestCase ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), ['cn', 'dn', 'uid', 'mail', 'cn']) ->andReturn(['count' => 1, 0 => [ 'uid' => [hex2bin('FFF8F7')], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], ]]); $details = $ldapService->getUserDetails('test'); @@ -619,14 +619,14 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'uid' => [$this->mockUser->name], - 'cn' => [$this->mockUser->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')], 'mail' => 'tester@example.com', ]], ['count' => 1, 0 => [ - 'uid' => ['Barry'], - 'cn' => ['Scott'], - 'dn' => ['dc=bscott' . config('services.ldap.base_dn')], + 'uid' => ['Barry'], + 'cn' => ['Scott'], + 'dn' => ['dc=bscott' . config('services.ldap.base_dn')], 'mail' => 'tester@example.com', ]]); @@ -646,28 +646,29 @@ class LdapTest extends TestCase setting()->put('registration-confirmation', 'true'); app('config')->set([ - 'services.ldap.user_to_groups' => true, - 'services.ldap.group_attribute' => 'memberOf', + '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->commonLdapMocks(1, 1, 6, 8, 6, 4); $this->mockLdap->shouldReceive('searchAndGetEntries') - ->times(3) + ->times(6) ->andReturn(['count' => 1, 0 => [ - 'uid' => [$user->name], - 'cn' => [$user->name], - 'dn' => ['dc=test' . config('services.ldap.base_dn')], - 'mail' => [$user->email], + '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', + 0 => 'cn=ldaptester,ou=groups,dc=example,dc=com', ], ]]); - $this->followingRedirects()->mockUserLogin()->assertSee('Thanks for registering!'); + $login = $this->followingRedirects()->mockUserLogin(); + $login->assertSee('Thanks for registering!'); $this->assertDatabaseHas('users', [ - 'email' => $user->email, + 'email' => $user->email, 'email_confirmed' => false, ]); @@ -677,8 +678,13 @@ class LdapTest extends TestCase 'role_id' => $roleToReceive->id, ]); + $this->assertNull(auth()->user()); + $homePage = $this->get('/'); - $homePage->assertRedirect('/register/confirm/awaiting'); + $homePage->assertRedirect('/login'); + + $login = $this->followingRedirects()->mockUserLogin(); + $login->assertSee('Email Address Not Confirmed'); } public function test_failed_logins_are_logged_when_message_configured() @@ -698,8 +704,8 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ - 'cn' => [$this->mockUser->name], - 'dn' => $ldapDn, + 'cn' => [$this->mockUser->name], + 'dn' => $ldapDn, 'jpegphoto' => [base64_decode('/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8Q EBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=')], 'mail' => [$this->mockUser->email], diff --git a/tests/Auth/MfaConfigurationTest.php b/tests/Auth/MfaConfigurationTest.php new file mode 100644 index 000000000..bdf26fb20 --- /dev/null +++ b/tests/Auth/MfaConfigurationTest.php @@ -0,0 +1,167 @@ +<?php + +namespace Tests\Auth; + +use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Auth\User; +use PragmaRX\Google2FA\Google2FA; +use Tests\TestCase; + +class MfaConfigurationTest extends TestCase +{ + + public function test_totp_setup() + { + $editor = $this->getEditor(); + $this->assertDatabaseMissing('mfa_values', ['user_id' => $editor->id]); + + // Setup page state + $resp = $this->actingAs($editor)->get('/mfa/setup'); + $resp->assertElementContains('a[href$="/mfa/totp/generate"]', 'Setup'); + + // Generate page access + $resp = $this->get('/mfa/totp/generate'); + $resp->assertSee('Mobile App Setup'); + $resp->assertSee('Verify Setup'); + $resp->assertElementExists('form[action$="/mfa/totp/confirm"] button'); + $this->assertSessionHas('mfa-setup-totp-secret'); + $svg = $resp->getElementHtml('#main-content .card svg'); + + // Validation error, code should remain the same + $resp = $this->post('/mfa/totp/confirm', [ + 'code' => 'abc123', + ]); + $resp->assertRedirect('/mfa/totp/generate'); + $resp = $this->followRedirects($resp); + $resp->assertSee('The provided code is not valid or has expired.'); + $revisitSvg = $resp->getElementHtml('#main-content .card svg'); + $this->assertTrue($svg === $revisitSvg); + + // Successful confirmation + $google2fa = new Google2FA(); + $secret = decrypt(session()->get('mfa-setup-totp-secret')); + $otp = $google2fa->getCurrentOtp($secret); + $resp = $this->post('/mfa/totp/confirm', [ + 'code' => $otp, + ]); + $resp->assertRedirect('/mfa/setup'); + + // Confirmation of setup + $resp = $this->followRedirects($resp); + $resp->assertSee('Multi-factor method successfully configured'); + $resp->assertElementContains('a[href$="/mfa/totp/generate"]', 'Reconfigure'); + + $this->assertDatabaseHas('mfa_values', [ + 'user_id' => $editor->id, + 'method' => 'totp', + ]); + $this->assertFalse(session()->has('mfa-setup-totp-secret')); + $value = MfaValue::query()->where('user_id', '=', $editor->id) + ->where('method', '=', 'totp')->first(); + $this->assertEquals($secret, decrypt($value->value)); + } + + public function test_backup_codes_setup() + { + $editor = $this->getEditor(); + $this->assertDatabaseMissing('mfa_values', ['user_id' => $editor->id]); + + // Setup page state + $resp = $this->actingAs($editor)->get('/mfa/setup'); + $resp->assertElementContains('a[href$="/mfa/backup_codes/generate"]', 'Setup'); + + // Generate page access + $resp = $this->get('/mfa/backup_codes/generate'); + $resp->assertSee('Backup Codes'); + $resp->assertElementContains('form[action$="/mfa/backup_codes/confirm"]', 'Confirm and Enable'); + $this->assertSessionHas('mfa-setup-backup-codes'); + $codes = decrypt(session()->get('mfa-setup-backup-codes')); + // Check code format + $this->assertCount(16, $codes); + $this->assertEquals(16*11, strlen(implode('', $codes))); + // Check download link + $resp->assertSee(base64_encode(implode("\n\n", $codes))); + + // Confirm submit + $resp = $this->post('/mfa/backup_codes/confirm'); + $resp->assertRedirect('/mfa/setup'); + + // Confirmation of setup + $resp = $this->followRedirects($resp); + $resp->assertSee('Multi-factor method successfully configured'); + $resp->assertElementContains('a[href$="/mfa/backup_codes/generate"]', 'Reconfigure'); + + $this->assertDatabaseHas('mfa_values', [ + 'user_id' => $editor->id, + 'method' => 'backup_codes', + ]); + $this->assertFalse(session()->has('mfa-setup-backup-codes')); + $value = MfaValue::query()->where('user_id', '=', $editor->id) + ->where('method', '=', 'backup_codes')->first(); + $this->assertEquals($codes, json_decode(decrypt($value->value))); + } + + public function test_backup_codes_cannot_be_confirmed_if_not_previously_generated() + { + $resp = $this->asEditor()->post('/mfa/backup_codes/confirm'); + $resp->assertStatus(500); + } + + public function test_mfa_method_count_is_visible_on_user_edit_page() + { + $user = $this->getEditor(); + $resp = $this->actingAs($this->getAdmin())->get($user->getEditUrl()); + $resp->assertSee('0 methods configured'); + + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, 'test'); + $resp = $this->get($user->getEditUrl()); + $resp->assertSee('1 method configured'); + + MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, 'test'); + $resp = $this->get($user->getEditUrl()); + $resp->assertSee('2 methods configured'); + } + + public function test_mfa_setup_link_only_shown_when_viewing_own_user_edit_page() + { + $admin = $this->getAdmin(); + $resp = $this->actingAs($admin)->get($admin->getEditUrl()); + $resp->assertElementExists('a[href$="/mfa/setup"]'); + + $resp = $this->actingAs($admin)->get($this->getEditor()->getEditUrl()); + $resp->assertElementNotExists('a[href$="/mfa/setup"]'); + } + + public function test_mfa_indicator_shows_in_user_list() + { + $admin = $this->getAdmin(); + User::query()->where('id', '!=', $admin->id)->delete(); + + $resp = $this->actingAs($admin)->get('/settings/users'); + $resp->assertElementNotExists('[title="MFA Configured"] svg'); + + MfaValue::upsertWithValue($admin, MfaValue::METHOD_TOTP, 'test'); + $resp = $this->actingAs($admin)->get('/settings/users'); + $resp->assertElementExists('[title="MFA Configured"] svg'); + } + + public function test_remove_mfa_method() + { + $admin = $this->getAdmin(); + + MfaValue::upsertWithValue($admin, MfaValue::METHOD_TOTP, 'test'); + $this->assertEquals(1, $admin->mfaValues()->count()); + $resp = $this->actingAs($admin)->get('/mfa/setup'); + $resp->assertElementExists('form[action$="/mfa/totp/remove"]'); + + $resp = $this->delete("/mfa/totp/remove"); + $resp->assertRedirect("/mfa/setup"); + $resp = $this->followRedirects($resp); + $resp->assertSee('Multi-factor method successfully removed'); + + $this->assertActivityExists(ActivityType::MFA_REMOVE_METHOD); + $this->assertEquals(0, $admin->mfaValues()->count()); + } + +} \ No newline at end of file diff --git a/tests/Auth/MfaVerificationTest.php b/tests/Auth/MfaVerificationTest.php new file mode 100644 index 000000000..e63094303 --- /dev/null +++ b/tests/Auth/MfaVerificationTest.php @@ -0,0 +1,279 @@ +<?php + +namespace Tests\Auth; + +use BookStack\Auth\Access\LoginService; +use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Auth\Access\Mfa\TotpService; +use BookStack\Auth\Role; +use BookStack\Auth\User; +use BookStack\Exceptions\StoppedAuthenticationException; +use Illuminate\Support\Facades\Hash; +use PragmaRX\Google2FA\Google2FA; +use Tests\TestCase; +use Tests\TestResponse; + +class MfaVerificationTest extends TestCase +{ + public function test_totp_verification() + { + [$user, $secret, $loginResp] = $this->startTotpLogin(); + $loginResp->assertRedirect('/mfa/verify'); + + $resp = $this->get('/mfa/verify'); + $resp->assertSee('Verify Access'); + $resp->assertSee('Enter the code, generated using your mobile app, below:'); + $resp->assertElementExists('form[action$="/mfa/totp/verify"] input[name="code"]'); + + $google2fa = new Google2FA(); + $resp = $this->post('/mfa/totp/verify', [ + 'code' => $google2fa->getCurrentOtp($secret), + ]); + $resp->assertRedirect('/'); + $this->assertEquals($user->id, auth()->user()->id); + } + + public function test_totp_verification_fails_on_missing_invalid_code() + { + [$user, $secret, $loginResp] = $this->startTotpLogin(); + + $resp = $this->get('/mfa/verify'); + $resp = $this->post('/mfa/totp/verify', [ + 'code' => '', + ]); + $resp->assertRedirect('/mfa/verify'); + + $resp = $this->get('/mfa/verify'); + $resp->assertSeeText('The code field is required.'); + $this->assertNull(auth()->user()); + + $resp = $this->post('/mfa/totp/verify', [ + 'code' => '123321', + ]); + $resp->assertRedirect('/mfa/verify'); + $resp = $this->get('/mfa/verify'); + + $resp->assertSeeText('The provided code is not valid or has expired.'); + $this->assertNull(auth()->user()); + } + + public function test_backup_code_verification() + { + [$user, $codes, $loginResp] = $this->startBackupCodeLogin(); + $loginResp->assertRedirect('/mfa/verify'); + + $resp = $this->get('/mfa/verify'); + $resp->assertSee('Verify Access'); + $resp->assertSee('Backup Code'); + $resp->assertSee('Enter one of your remaining backup codes below:'); + $resp->assertElementExists('form[action$="/mfa/backup_codes/verify"] input[name="code"]'); + + $resp = $this->post('/mfa/backup_codes/verify', [ + 'code' => $codes[1], + ]); + + $resp->assertRedirect('/'); + $this->assertEquals($user->id, auth()->user()->id); + // Ensure code no longer exists in available set + $userCodes = MfaValue::getValueForUser($user, MfaValue::METHOD_BACKUP_CODES); + $this->assertStringNotContainsString($codes[1], $userCodes); + $this->assertStringContainsString($codes[0], $userCodes); + } + + public function test_backup_code_verification_fails_on_missing_or_invalid_code() + { + [$user, $codes, $loginResp] = $this->startBackupCodeLogin(); + + $resp = $this->get('/mfa/verify'); + $resp = $this->post('/mfa/backup_codes/verify', [ + 'code' => '', + ]); + $resp->assertRedirect('/mfa/verify'); + + $resp = $this->get('/mfa/verify'); + $resp->assertSeeText('The code field is required.'); + $this->assertNull(auth()->user()); + + $resp = $this->post('/mfa/backup_codes/verify', [ + 'code' => 'ab123-ab456', + ]); + $resp->assertRedirect('/mfa/verify'); + + $resp = $this->get('/mfa/verify'); + $resp->assertSeeText('The provided code is not valid or has already been used.'); + $this->assertNull(auth()->user()); + } + + public function test_backup_code_verification_fails_on_attempted_code_reuse() + { + [$user, $codes, $loginResp] = $this->startBackupCodeLogin(); + + $this->post('/mfa/backup_codes/verify', [ + 'code' => $codes[0], + ]); + $this->assertNotNull(auth()->user()); + auth()->logout(); + session()->flush(); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']); + $this->get('/mfa/verify'); + $resp = $this->post('/mfa/backup_codes/verify', [ + 'code' => $codes[0], + ]); + $resp->assertRedirect('/mfa/verify'); + $this->assertNull(auth()->user()); + + $resp = $this->get('/mfa/verify'); + $resp->assertSeeText('The provided code is not valid or has already been used.'); + } + + public function test_backup_code_verification_shows_warning_when_limited_codes_remain() + { + [$user, $codes, $loginResp] = $this->startBackupCodeLogin(['abc12-def45', 'abc12-def46']); + + $resp = $this->post('/mfa/backup_codes/verify', [ + 'code' => $codes[0], + ]); + $resp = $this->followRedirects($resp); + $resp->assertSeeText('You have less than 5 backup codes remaining, Please generate and store a new set before you run out of codes to prevent being locked out of your account.'); + } + + public function test_both_mfa_options_available_if_set_on_profile() + { + $user = $this->getEditor(); + $user->password = Hash::make('password'); + $user->save(); + + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, 'abc123'); + MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, '["abc12-def456"]'); + + /** @var TestResponse $mfaView */ + $mfaView = $this->followingRedirects()->post('/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + // Totp shown by default + $mfaView->assertElementExists('form[action$="/mfa/totp/verify"] input[name="code"]'); + $mfaView->assertElementContains('a[href$="/mfa/verify?method=backup_codes"]', 'Verify using a backup code'); + + // Ensure can view backup_codes view + $resp = $this->get('/mfa/verify?method=backup_codes'); + $resp->assertElementExists('form[action$="/mfa/backup_codes/verify"] input[name="code"]'); + $resp->assertElementContains('a[href$="/mfa/verify?method=totp"]', 'Verify using a mobile app'); + } + + public function test_mfa_required_with_no_methods_leads_to_setup() + { + $user = $this->getEditor(); + $user->password = Hash::make('password'); + $user->save(); + /** @var Role $role */ + $role = $user->roles->first(); + $role->mfa_enforced = true; + $role->save(); + + $this->assertDatabaseMissing('mfa_values', [ + 'user_id' => $user->id, + ]); + + /** @var TestResponse $resp */ + $resp = $this->followingRedirects()->post('/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $resp->assertSeeText('No Methods Configured'); + $resp->assertElementContains('a[href$="/mfa/setup"]', 'Configure'); + + $this->get('/mfa/backup_codes/generate'); + $resp = $this->post('/mfa/backup_codes/confirm'); + $resp->assertRedirect('/login'); + $this->assertDatabaseHas('mfa_values', [ + 'user_id' => $user->id, + ]); + + $resp = $this->get('/login'); + $resp->assertSeeText('Multi-factor method configured, Please now login again using the configured method.'); + + $resp = $this->followingRedirects()->post('/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + $resp->assertSeeText('Enter one of your remaining backup codes below:'); + } + + public function test_mfa_setup_route_access() + { + $routes = [ + ['get', '/mfa/setup'], + ['get', '/mfa/totp/generate'], + ['post', '/mfa/totp/confirm'], + ['get', '/mfa/backup_codes/generate'], + ['post', '/mfa/backup_codes/confirm'], + ]; + + // Non-auth access + foreach ($routes as [$method, $path]) { + $resp = $this->call($method, $path); + $resp->assertRedirect('/login'); + } + + // Attempted login user, who has configured mfa, access + // Sets up user that has MFA required after attempted login. + $loginService = $this->app->make(LoginService::class); + $user = $this->getEditor(); + /** @var Role $role */ + $role = $user->roles->first(); + $role->mfa_enforced = true; + $role->save(); + try { + $loginService->login($user, 'testing'); + } catch (StoppedAuthenticationException $e) { + } + $this->assertNotNull($loginService->getLastLoginAttemptUser()); + + MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, '[]'); + foreach ($routes as [$method, $path]) { + $resp = $this->call($method, $path); + $resp->assertRedirect('/login'); + } + + } + + /** + * @return Array<User, string, TestResponse> + */ + protected function startTotpLogin(): array + { + $secret = $this->app->make(TotpService::class)->generateSecret(); + $user = $this->getEditor(); + $user->password = Hash::make('password'); + $user->save(); + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, $secret); + $loginResp = $this->post('/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + return [$user, $secret, $loginResp]; + } + + /** + * @return Array<User, string, TestResponse> + */ + protected function startBackupCodeLogin($codes = ['kzzu6-1pgll','bzxnf-plygd','bwdsp-ysl51','1vo93-ioy7n','lf7nw-wdyka','xmtrd-oplac']): array + { + $user = $this->getEditor(); + $user->password = Hash::make('password'); + $user->save(); + MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, json_encode($codes)); + $loginResp = $this->post('/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + return [$user, $codes, $loginResp]; + } + +} \ No newline at end of file diff --git a/tests/Auth/Saml2Test.php b/tests/Auth/Saml2Test.php index 096f862e7..8ace3e2ee 100644 --- a/tests/Auth/Saml2Test.php +++ b/tests/Auth/Saml2Test.php @@ -289,16 +289,18 @@ class Saml2Test extends TestCase $this->assertEquals('http://localhost/register/confirm', url()->current()); $acsPost->assertSee('Please check your email and click the confirmation button to access BookStack.'); + /** @var User $user */ $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'); + $this->assertFalse(boolval($user->email_confirmed), 'User email remains unconfirmed'); }); + $this->assertNull(auth()->user()); $homeGet = $this->get('/'); - $homeGet->assertRedirect('/register/confirm/awaiting'); + $homeGet->assertRedirect('/login'); } public function test_login_where_existing_non_saml_user_shows_warning() diff --git a/tests/Commands/ResetMfaCommandTest.php b/tests/Commands/ResetMfaCommandTest.php new file mode 100644 index 000000000..550f6d390 --- /dev/null +++ b/tests/Commands/ResetMfaCommandTest.php @@ -0,0 +1,65 @@ +<?php + +namespace Tests\Commands; + +use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Auth\User; +use Tests\TestCase; + +class ResetMfaCommandTest extends TestCase +{ + public function test_command_requires_email_or_id_option() + { + $this->artisan('bookstack:reset-mfa') + ->expectsOutput('Either a --id=<number> or --email=<email> option must be provided.') + ->assertExitCode(1); + } + + public function test_command_runs_with_provided_email() + { + /** @var User $user */ + $user = User::query()->first(); + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, 'test'); + + $this->assertEquals(1, $user->mfaValues()->count()); + $this->artisan("bookstack:reset-mfa --email={$user->email}") + ->expectsQuestion('Are you sure you want to proceed?', true) + ->expectsOutput('User MFA methods have been reset.') + ->assertExitCode(0); + $this->assertEquals(0, $user->mfaValues()->count()); + } + + public function test_command_runs_with_provided_id() + { + /** @var User $user */ + $user = User::query()->first(); + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, 'test'); + + $this->assertEquals(1, $user->mfaValues()->count()); + $this->artisan("bookstack:reset-mfa --id={$user->id}") + ->expectsQuestion('Are you sure you want to proceed?', true) + ->expectsOutput('User MFA methods have been reset.') + ->assertExitCode(0); + $this->assertEquals(0, $user->mfaValues()->count()); + } + + public function test_saying_no_to_confirmation_does_not_reset_mfa() + { + /** @var User $user */ + $user = User::query()->first(); + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, 'test'); + + $this->assertEquals(1, $user->mfaValues()->count()); + $this->artisan("bookstack:reset-mfa --id={$user->id}") + ->expectsQuestion('Are you sure you want to proceed?', false) + ->assertExitCode(1); + $this->assertEquals(1, $user->mfaValues()->count()); + } + + public function test_giving_non_existing_user_shows_error_message() + { + $this->artisan("bookstack:reset-mfa --email=donkeys@example.com") + ->expectsOutput('A user where email=donkeys@example.com could not be found.') + ->assertExitCode(1); + } +} diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php index 09c3233e3..b9b1805b6 100644 --- a/tests/Permissions/RolesTest.php +++ b/tests/Permissions/RolesTest.php @@ -64,15 +64,16 @@ class RolesTest extends BrowserKitTest ->type('Test Role', 'display_name') ->type('A little test description', 'description') ->press('Save Role') - ->seeInDatabase('roles', ['display_name' => $testRoleName, 'description' => $testRoleDesc]) + ->seeInDatabase('roles', ['display_name' => $testRoleName, 'description' => $testRoleDesc, 'mfa_enforced' => false]) ->seePageIs('/settings/roles'); // Updating $this->asAdmin()->visit('/settings/roles') ->see($testRoleDesc) ->click($testRoleName) ->type($testRoleUpdateName, '#display_name') + ->check('#mfa_enforced') ->press('Save Role') - ->seeInDatabase('roles', ['display_name' => $testRoleUpdateName, 'description' => $testRoleDesc]) + ->seeInDatabase('roles', ['display_name' => $testRoleUpdateName, 'description' => $testRoleDesc, 'mfa_enforced' => true]) ->seePageIs('/settings/roles'); // Deleting $this->asAdmin()->visit('/settings/roles') diff --git a/tests/TestResponse.php b/tests/TestResponse.php index 39a9d796b..79f173c9b 100644 --- a/tests/TestResponse.php +++ b/tests/TestResponse.php @@ -26,6 +26,14 @@ class TestResponse extends BaseTestResponse return $this->crawlerInstance; } + /** + * Get the HTML of the first element at the given selector. + */ + public function getElementHtml(string $selector): string + { + return $this->crawler()->filter($selector)->first()->outerHtml(); + } + /** * Assert the response contains the specified element. *