diff --git a/app/Auth/Access/EmailConfirmationService.php b/app/Auth/Access/EmailConfirmationService.php index 75c03b318..9c357d95f 100644 --- a/app/Auth/Access/EmailConfirmationService.php +++ b/app/Auth/Access/EmailConfirmationService.php @@ -14,6 +14,7 @@ class EmailConfirmationService extends UserTokenService /** * Create new confirmation for a user, * Also removes any existing old ones. + * * @throws ConfirmationEmailException */ public function sendConfirmation(User $user) diff --git a/app/Auth/Access/LoginService.php b/app/Auth/Access/LoginService.php index b251e4cc3..e02296b37 100644 --- a/app/Auth/Access/LoginService.php +++ b/app/Auth/Access/LoginService.php @@ -13,7 +13,6 @@ use Exception; class LoginService { - protected const LAST_LOGIN_ATTEMPTED_SESSION_KEY = 'auth-login-last-attempted'; protected $mfaSession; @@ -30,12 +29,14 @@ class LoginService * 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); } @@ -55,6 +56,7 @@ class LoginService /** * Reattempt a system login after a previous stopped attempt. + * * @throws Exception */ public function reattemptLoginFor(User $user) @@ -75,12 +77,14 @@ class LoginService 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 @@ -91,9 +95,10 @@ class LoginService } [$id, $method, $remember, $time] = explode(':', $value); - $hourAgo = time() - (60*60); + $hourAgo = time() - (60 * 60); if ($time < $hourAgo) { $this->clearLastLoginAttempted(); + return ['user_id' => null, 'method' => null]; } @@ -156,5 +161,4 @@ class LoginService return $result; } - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Mfa/BackupCodeService.php b/app/Auth/Access/Mfa/BackupCodeService.php index ca58e7404..d58d28ae1 100644 --- a/app/Auth/Access/Mfa/BackupCodeService.php +++ b/app/Auth/Access/Mfa/BackupCodeService.php @@ -29,6 +29,7 @@ class BackupCodeService { $cleanCode = $this->cleanInputCode($code); $codes = json_decode($codeSet); + return in_array($cleanCode, $codes); } @@ -42,6 +43,7 @@ class BackupCodeService $codes = json_decode($codeSet); $pos = array_search($cleanCode, $codes, true); array_splice($codes, $pos, 1); + return json_encode($codes); } @@ -57,4 +59,4 @@ class BackupCodeService { 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 index 8821dbb9d..72163b58e 100644 --- a/app/Auth/Access/Mfa/MfaSession.php +++ b/app/Auth/Access/Mfa/MfaSession.php @@ -57,5 +57,4 @@ class MfaSession { 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 index ec0d5d563..8f07c6657 100644 --- a/app/Auth/Access/Mfa/MfaValue.php +++ b/app/Auth/Access/Mfa/MfaValue.php @@ -7,8 +7,8 @@ use Carbon\Carbon; use Illuminate\Database\Eloquent\Model; /** - * @property int $id - * @property int $user_id + * @property int $id + * @property int $user_id * @property string $method * @property string $value * @property Carbon $created_at @@ -38,7 +38,7 @@ class MfaValue extends Model /** @var MfaValue $mfaVal */ $mfaVal = static::query()->firstOrNew([ 'user_id' => $user->id, - 'method' => $method + 'method' => $method, ]); $mfaVal->setValue($value); $mfaVal->save(); diff --git a/app/Auth/Access/Mfa/TotpService.php b/app/Auth/Access/Mfa/TotpService.php index d1013978b..a3e9fc827 100644 --- a/app/Auth/Access/Mfa/TotpService.php +++ b/app/Auth/Access/Mfa/TotpService.php @@ -51,10 +51,11 @@ class TotpService 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 + new SvgImageBackEnd() ) ))->writeString($url); } @@ -68,4 +69,4 @@ class TotpService /** @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 index 7fe3d8504..22cb7da9b 100644 --- a/app/Auth/Access/Mfa/TotpValidationRule.php +++ b/app/Auth/Access/Mfa/TotpValidationRule.php @@ -6,7 +6,6 @@ use Illuminate\Contracts\Validation\Rule; class TotpValidationRule implements Rule { - protected $secret; protected $totpService; diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index 4c8b53c0d..16e3edbb4 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -88,6 +88,7 @@ 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 3f0f40ccc..6cbfdac0b 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -2,15 +2,11 @@ namespace BookStack\Auth\Access; -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; -use BookStack\Theming\ThemeEvents; use Exception; use Illuminate\Support\Str; use OneLogin\Saml2\Auth; @@ -392,6 +388,7 @@ class Saml2Service extends ExternalAuthService } $this->loginService->login($user, 'saml2'); + return $user; } } diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index 70f3fc3d3..8cf243fe7 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -142,6 +142,7 @@ class SocialAuthService // Simply log the user into the application. if (!$isLoggedIn && $socialAccount !== null) { $this->loginService->login($socialAccount->user, $socialAccount); + return redirect()->intended('/'); } diff --git a/app/Console/Commands/ResetMfa.php b/app/Console/Commands/ResetMfa.php index feb477943..031bec04b 100644 --- a/app/Console/Commands/ResetMfa.php +++ b/app/Console/Commands/ResetMfa.php @@ -45,6 +45,7 @@ class ResetMfa extends Command $email = $this->option('email'); if (!$id && !$email) { $this->error('Either a --id= or --email= option must be provided.'); + return 1; } @@ -57,6 +58,7 @@ class ResetMfa extends Command if (!$user) { $this->error("A user where {$field}={$value} could not be found."); + return 1; } @@ -66,6 +68,7 @@ class ResetMfa extends Command if ($confirm) { $user->mfaValues()->delete(); $this->info('User MFA methods have been reset.'); + return 0; } diff --git a/app/Exceptions/StoppedAuthenticationException.php b/app/Exceptions/StoppedAuthenticationException.php index 7a978ffc6..ef7f24017 100644 --- a/app/Exceptions/StoppedAuthenticationException.php +++ b/app/Exceptions/StoppedAuthenticationException.php @@ -9,7 +9,6 @@ use Illuminate\Http\Request; class StoppedAuthenticationException extends \Exception implements Responsable { - protected $user; protected $loginService; @@ -50,7 +49,7 @@ class StoppedAuthenticationException extends \Exception implements Responsable if ($request->wantsJson()) { return response()->json([ 'error' => [ - 'code' => 401, + 'code' => 401, 'message' => trans('errors.email_confirmation_awaiting'), ], ], 401); @@ -62,4 +61,4 @@ class StoppedAuthenticationException extends \Exception implements Responsable 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 520efdf88..02b9ef276 100644 --- a/app/Http/Controllers/Auth/ConfirmEmailController.php +++ b/app/Http/Controllers/Auth/ConfirmEmailController.php @@ -28,8 +28,7 @@ class ConfirmEmailController extends Controller EmailConfirmationService $emailConfirmationService, LoginService $loginService, UserRepo $userRepo - ) - { + ) { $this->emailConfirmationService = $emailConfirmationService; $this->loginService = $loginService; $this->userRepo = $userRepo; @@ -51,6 +50,7 @@ class ConfirmEmailController extends Controller public function showAwaiting() { $user = $this->loginService->getLastLoginAttemptUser(); + return view('auth.user-unconfirmed', ['user' => $user]); } diff --git a/app/Http/Controllers/Auth/HandlesPartialLogins.php b/app/Http/Controllers/Auth/HandlesPartialLogins.php index 4ce67d236..c7f362151 100644 --- a/app/Http/Controllers/Auth/HandlesPartialLogins.php +++ b/app/Http/Controllers/Auth/HandlesPartialLogins.php @@ -22,4 +22,4 @@ trait HandlesPartialLogins 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 174770bf9..01cc77d84 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -144,13 +144,16 @@ class LoginController extends Controller /** * Attempt to log the user into the application. * - * @param \Illuminate\Http\Request $request + * @param \Illuminate\Http\Request $request + * * @return bool */ protected function attemptLogin(Request $request) { return $this->loginService->attempt( - $this->credentials($request), auth()->getDefaultDriver(), $request->filled('remember') + $this->credentials($request), + auth()->getDefaultDriver(), + $request->filled('remember') ); } diff --git a/app/Http/Controllers/Auth/MfaBackupCodesController.php b/app/Http/Controllers/Auth/MfaBackupCodesController.php index 4b4e11659..d92029bf1 100644 --- a/app/Http/Controllers/Auth/MfaBackupCodesController.php +++ b/app/Http/Controllers/Auth/MfaBackupCodesController.php @@ -20,7 +20,7 @@ class MfaBackupCodesController extends Controller protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-backup-codes'; /** - * Show a view that generates and displays backup codes + * Show a view that generates and displays backup codes. */ public function generate(BackupCodeService $codeService) { @@ -30,13 +30,14 @@ class MfaBackupCodesController extends Controller $downloadUrl = 'data:application/octet-stream;base64,' . base64_encode(implode("\n\n", $codes)); return view('mfa.backup-codes-generate', [ - 'codes' => $codes, + 'codes' => $codes, 'downloadUrl' => $downloadUrl, ]); } /** * Confirm the setup of backup codes, storing them against the user. + * * @throws Exception */ public function confirm() @@ -52,6 +53,7 @@ class MfaBackupCodesController extends Controller if (!auth()->check()) { $this->showSuccessNotification(trans('auth.mfa_setup_login_notification')); + return redirect('/login'); } @@ -60,6 +62,7 @@ class MfaBackupCodesController extends Controller /** * Verify the MFA method submission on check. + * * @throws NotFoundException * @throws ValidationException */ @@ -76,8 +79,8 @@ class MfaBackupCodesController extends Controller if (!$codeService->inputCodeExistsInSet($value, $codes)) { $fail(trans('validation.backup_codes')); } - } - ] + }, + ], ]); $updatedCodes = $codeService->removeInputCodeFromSet($request->get('code'), $codes); diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php index 75cd46eef..ca77cc708 100644 --- a/app/Http/Controllers/Auth/MfaController.php +++ b/app/Http/Controllers/Auth/MfaController.php @@ -20,6 +20,7 @@ class MfaController extends Controller ->mfaValues() ->get(['id', 'method']) ->groupBy('method'); + return view('mfa.setup', [ 'userMethods' => $userMethods, ]); @@ -27,6 +28,7 @@ class MfaController extends Controller /** * Remove an MFA method for the current user. + * * @throws \Exception */ public function remove(string $method) @@ -56,13 +58,13 @@ class MfaController extends Controller // 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) { + $otherMethods = $userMethods->keys()->filter(function ($userMethod) use ($method) { return $method !== $userMethod; })->all(); return view('mfa.verify', [ - 'userMethods' => $userMethods, - 'method' => $method, + 'userMethods' => $userMethods, + 'method' => $method, 'otherMethods' => $otherMethods, ]); } diff --git a/app/Http/Controllers/Auth/MfaTotpController.php b/app/Http/Controllers/Auth/MfaTotpController.php index d55f08cff..5a932d6e9 100644 --- a/app/Http/Controllers/Auth/MfaTotpController.php +++ b/app/Http/Controllers/Auth/MfaTotpController.php @@ -36,13 +36,14 @@ class MfaTotpController extends Controller return view('mfa.totp-generate', [ 'secret' => $totpSecret, - 'svg' => $svg, + 'svg' => $svg, ]); } /** * Confirm the setup of TOTP and save the auth method secret * against the current user. + * * @throws ValidationException * @throws NotFoundException */ @@ -54,7 +55,7 @@ class MfaTotpController extends Controller 'required', 'max:12', 'min:4', new TotpValidationRule($totpSecret), - ] + ], ]); MfaValue::upsertWithValue($this->currentOrLastAttemptedUser(), MfaValue::METHOD_TOTP, $totpSecret); @@ -63,6 +64,7 @@ class MfaTotpController extends Controller if (!auth()->check()) { $this->showSuccessNotification(trans('auth.mfa_setup_login_notification')); + return redirect('/login'); } @@ -71,6 +73,7 @@ class MfaTotpController extends Controller /** * Verify the MFA method submission on check. + * * @throws NotFoundException */ public function verify(Request $request, LoginService $loginService, MfaSession $mfaSession) @@ -83,7 +86,7 @@ class MfaTotpController extends Controller 'required', 'max:12', 'min:4', new TotpValidationRule($totpSecret), - ] + ], ]); $mfaSession->markVerifiedForUser($user); diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index e372b38ef..bd1ffeac2 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -48,8 +48,7 @@ class RegisterController extends Controller SocialAuthService $socialAuthService, RegistrationService $registrationService, LoginService $loginService - ) - { + ) { $this->middleware('guest'); $this->middleware('guard:standard'); diff --git a/app/Http/Controllers/Auth/SocialController.php b/app/Http/Controllers/Auth/SocialController.php index 2e9e62162..1691668a2 100644 --- a/app/Http/Controllers/Auth/SocialController.php +++ b/app/Http/Controllers/Auth/SocialController.php @@ -27,8 +27,7 @@ class SocialController extends Controller SocialAuthService $socialAuthService, RegistrationService $registrationService, LoginService $loginService - ) - { + ) { $this->middleware('guest')->only(['getRegister', 'postRegister']); $this->socialAuthService = $socialAuthService; $this->registrationService = $registrationService; diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php index 73192b0cf..bc584d3c5 100644 --- a/app/Http/Middleware/ApiAuthenticate.php +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -9,7 +9,6 @@ use Illuminate\Http\Request; class ApiAuthenticate { - /** * Handle an incoming request. */ diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index dede8f260..a32029112 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -16,6 +16,7 @@ class Authenticate if ($request->ajax()) { return response('Unauthorized.', 401); } + return redirect()->guest(url('/login')); } diff --git a/app/Http/Middleware/AuthenticatedOrPendingMfa.php b/app/Http/Middleware/AuthenticatedOrPendingMfa.php index febfef207..0a0588864 100644 --- a/app/Http/Middleware/AuthenticatedOrPendingMfa.php +++ b/app/Http/Middleware/AuthenticatedOrPendingMfa.php @@ -8,7 +8,6 @@ use Closure; class AuthenticatedOrPendingMfa { - protected $loginService; protected $mfaSession; @@ -18,12 +17,12 @@ class AuthenticatedOrPendingMfa $this->mfaSession = $mfaSession; } - /** * Handle an incoming request. * - * @param \Illuminate\Http\Request $request - * @param \Closure $next + * @param \Illuminate\Http\Request $request + * @param \Closure $next + * * @return mixed */ public function handle($request, Closure $next) diff --git a/app/Theming/ThemeEvents.php b/app/Theming/ThemeEvents.php index 76bf649f9..1965556a9 100644 --- a/app/Theming/ThemeEvents.php +++ b/app/Theming/ThemeEvents.php @@ -41,7 +41,7 @@ class ThemeEvents * Provides both the original request and the currently resolved response. * Return values, if provided, will be used as a new response to use. * - * @param \Illuminate\Http\Request $request + * @param \Illuminate\Http\Request $request * @param \Illuminate\Http\Response|Symfony\Component\HttpFoundation\BinaryFileResponse $response * @returns \Illuminate\Http\Response|null */ diff --git a/app/Translation/FileLoader.php b/app/Translation/FileLoader.php index 775eefc47..de1124046 100644 --- a/app/Translation/FileLoader.php +++ b/app/Translation/FileLoader.php @@ -26,7 +26,8 @@ class FileLoader extends BaseLoader if (is_null($namespace) || $namespace === '*') { $themePath = theme_path('lang'); $themeTranslations = $themePath ? $this->loadPath($themePath, $locale, $group) : []; - $originalTranslations = $this->loadPath($this->path, $locale, $group); + $originalTranslations = $this->loadPath($this->path, $locale, $group); + return array_merge($originalTranslations, $themeTranslations); } diff --git a/routes/web.php b/routes/web.php index a70d0d818..853b1ee4c 100644 --- a/routes/web.php +++ b/routes/web.php @@ -223,18 +223,17 @@ 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::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::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'); diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index b4b99d130..657728c17 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -332,7 +332,7 @@ class AuthTest extends BrowserKitTest $user = $this->getEditor(); $mfaSession = $this->app->make(MfaSession::class); - $mfaSession->markVerifiedForUser($user);; + $mfaSession->markVerifiedForUser($user); $this->assertTrue($mfaSession->isVerifiedForUser($user)); $this->asAdmin()->visit('/logout'); diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 3bd6988e4..9e0729a8e 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,8 +646,8 @@ 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, ]); @@ -655,20 +655,20 @@ class LdapTest extends TestCase $this->mockLdap->shouldReceive('searchAndGetEntries') ->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', ], ]]); $login = $this->followingRedirects()->mockUserLogin(); $login->assertSee('Thanks for registering!'); $this->assertDatabaseHas('users', [ - 'email' => $user->email, + 'email' => $user->email, 'email_confirmed' => false, ]); @@ -704,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 index bdf26fb20..eb0e2faf0 100644 --- a/tests/Auth/MfaConfigurationTest.php +++ b/tests/Auth/MfaConfigurationTest.php @@ -10,7 +10,6 @@ use Tests\TestCase; class MfaConfigurationTest extends TestCase { - public function test_totp_setup() { $editor = $this->getEditor(); @@ -54,7 +53,7 @@ class MfaConfigurationTest extends TestCase $this->assertDatabaseHas('mfa_values', [ 'user_id' => $editor->id, - 'method' => 'totp', + 'method' => 'totp', ]); $this->assertFalse(session()->has('mfa-setup-totp-secret')); $value = MfaValue::query()->where('user_id', '=', $editor->id) @@ -79,7 +78,7 @@ class MfaConfigurationTest extends TestCase $codes = decrypt(session()->get('mfa-setup-backup-codes')); // Check code format $this->assertCount(16, $codes); - $this->assertEquals(16*11, strlen(implode('', $codes))); + $this->assertEquals(16 * 11, strlen(implode('', $codes))); // Check download link $resp->assertSee(base64_encode(implode("\n\n", $codes))); @@ -94,7 +93,7 @@ class MfaConfigurationTest extends TestCase $this->assertDatabaseHas('mfa_values', [ 'user_id' => $editor->id, - 'method' => 'backup_codes', + 'method' => 'backup_codes', ]); $this->assertFalse(session()->has('mfa-setup-backup-codes')); $value = MfaValue::query()->where('user_id', '=', $editor->id) @@ -155,13 +154,12 @@ class MfaConfigurationTest extends TestCase $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->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 index e63094303..ee6f3ecc8 100644 --- a/tests/Auth/MfaVerificationTest.php +++ b/tests/Auth/MfaVerificationTest.php @@ -149,7 +149,7 @@ class MfaVerificationTest extends TestCase /** @var TestResponse $mfaView */ $mfaView = $this->followingRedirects()->post('/login', [ - 'email' => $user->email, + 'email' => $user->email, 'password' => 'password', ]); @@ -179,7 +179,7 @@ class MfaVerificationTest extends TestCase /** @var TestResponse $resp */ $resp = $this->followingRedirects()->post('/login', [ - 'email' => $user->email, + 'email' => $user->email, 'password' => 'password', ]); @@ -197,7 +197,7 @@ class MfaVerificationTest extends TestCase $resp->assertSeeText('Multi-factor method configured, Please now login again using the configured method.'); $resp = $this->followingRedirects()->post('/login', [ - 'email' => $user->email, + 'email' => $user->email, 'password' => 'password', ]); $resp->assertSeeText('Enter one of your remaining backup codes below:'); @@ -227,6 +227,7 @@ class MfaVerificationTest extends TestCase $role = $user->roles->first(); $role->mfa_enforced = true; $role->save(); + try { $loginService->login($user, 'testing'); } catch (StoppedAuthenticationException $e) { @@ -238,7 +239,6 @@ class MfaVerificationTest extends TestCase $resp = $this->call($method, $path); $resp->assertRedirect('/login'); } - } /** @@ -252,7 +252,7 @@ class MfaVerificationTest extends TestCase $user->save(); MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, $secret); $loginResp = $this->post('/login', [ - 'email' => $user->email, + 'email' => $user->email, 'password' => 'password', ]); @@ -262,18 +262,17 @@ class MfaVerificationTest extends TestCase /** * @return Array */ - protected function startBackupCodeLogin($codes = ['kzzu6-1pgll','bzxnf-plygd','bwdsp-ysl51','1vo93-ioy7n','lf7nw-wdyka','xmtrd-oplac']): array + 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, + 'email' => $user->email, 'password' => 'password', ]); return [$user, $codes, $loginResp]; } - -} \ No newline at end of file +} diff --git a/tests/Commands/ResetMfaCommandTest.php b/tests/Commands/ResetMfaCommandTest.php index 550f6d390..e65a048ef 100644 --- a/tests/Commands/ResetMfaCommandTest.php +++ b/tests/Commands/ResetMfaCommandTest.php @@ -58,7 +58,7 @@ class ResetMfaCommandTest extends TestCase public function test_giving_non_existing_user_shows_error_message() { - $this->artisan("bookstack:reset-mfa --email=donkeys@example.com") + $this->artisan('bookstack:reset-mfa --email=donkeys@example.com') ->expectsOutput('A user where email=donkeys@example.com could not be found.') ->assertExitCode(1); }