mirror of
https://github.com/BookStackApp/BookStack.git
synced 2025-04-29 22:29:57 +00:00
Apply fixes from StyleCI
This commit is contained in:
parent
cac31b2074
commit
64785ed9da
31 changed files with 173 additions and 161 deletions
app
Auth/Access
EmailConfirmationService.phpLoginService.php
Mfa
RegistrationService.phpSaml2Service.phpSocialAuthService.phpConsole/Commands
Exceptions
Http
Controllers/Auth
ConfirmEmailController.phpLoginController.phpMfaBackupCodesController.phpMfaController.phpMfaTotpController.phpRegisterController.phpSocialController.php
Middleware
Translation
routes
tests
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
@ -94,6 +98,7 @@ class LoginService
|
|||
$hourAgo = time() - (60 * 60);
|
||||
if ($time < $hourAgo) {
|
||||
$this->clearLastLoginAttempted();
|
||||
|
||||
return ['user_id' => null, 'method' => null];
|
||||
}
|
||||
|
||||
|
@ -156,5 +161,4 @@ class LoginService
|
|||
|
||||
return $result;
|
||||
}
|
||||
|
||||
}
|
|
@ -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,5 +57,4 @@ class MfaSession
|
|||
{
|
||||
return 'mfa-verification-passed:' . $user->id;
|
||||
}
|
||||
|
||||
}
|
|
@ -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();
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
|
|
@ -6,7 +6,6 @@ use Illuminate\Contracts\Validation\Rule;
|
|||
|
||||
class TotpValidationRule implements Rule
|
||||
{
|
||||
|
||||
protected $secret;
|
||||
protected $totpService;
|
||||
|
||||
|
|
|
@ -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');
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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('/');
|
||||
}
|
||||
|
||||
|
|
|
@ -45,6 +45,7 @@ class ResetMfa extends Command
|
|||
$email = $this->option('email');
|
||||
if (!$id && !$email) {
|
||||
$this->error('Either a --id=<number> or --email=<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;
|
||||
}
|
||||
|
||||
|
|
|
@ -9,7 +9,6 @@ use Illuminate\Http\Request;
|
|||
|
||||
class StoppedAuthenticationException extends \Exception implements Responsable
|
||||
{
|
||||
|
||||
protected $user;
|
||||
protected $loginService;
|
||||
|
||||
|
|
|
@ -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]);
|
||||
}
|
||||
|
||||
|
|
|
@ -145,12 +145,15 @@ class LoginController extends Controller
|
|||
* 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')
|
||||
$this->credentials($request),
|
||||
auth()->getDefaultDriver(),
|
||||
$request->filled('remember')
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
@ -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)
|
||||
{
|
||||
|
@ -37,6 +37,7 @@ class MfaBackupCodesController extends Controller
|
|||
|
||||
/**
|
||||
* 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);
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -43,6 +43,7 @@ class MfaTotpController extends Controller
|
|||
/**
|
||||
* 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);
|
||||
|
|
|
@ -48,8 +48,7 @@ class RegisterController extends Controller
|
|||
SocialAuthService $socialAuthService,
|
||||
RegistrationService $registrationService,
|
||||
LoginService $loginService
|
||||
)
|
||||
{
|
||||
) {
|
||||
$this->middleware('guest');
|
||||
$this->middleware('guard:standard');
|
||||
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -9,7 +9,6 @@ use Illuminate\Http\Request;
|
|||
|
||||
class ApiAuthenticate
|
||||
{
|
||||
|
||||
/**
|
||||
* Handle an incoming request.
|
||||
*/
|
||||
|
|
|
@ -16,6 +16,7 @@ class Authenticate
|
|||
if ($request->ajax()) {
|
||||
return response('Unauthorized.', 401);
|
||||
}
|
||||
|
||||
return redirect()->guest(url('/login'));
|
||||
}
|
||||
|
||||
|
|
|
@ -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
|
||||
*
|
||||
* @return mixed
|
||||
*/
|
||||
public function handle($request, Closure $next)
|
||||
|
|
|
@ -27,6 +27,7 @@ class FileLoader extends BaseLoader
|
|||
$themePath = theme_path('lang');
|
||||
$themeTranslations = $themePath ? $this->loadPath($themePath, $locale, $group) : [];
|
||||
$originalTranslations = $this->loadPath($this->path, $locale, $group);
|
||||
|
||||
return array_merge($originalTranslations, $themeTranslations);
|
||||
}
|
||||
|
||||
|
|
|
@ -223,7 +223,6 @@ Route::group(['middleware' => 'auth'], function () {
|
|||
Route::get('/roles/{id}', 'RoleController@edit');
|
||||
Route::put('/roles/{id}', 'RoleController@update');
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
// MFA routes
|
||||
|
|
|
@ -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');
|
||||
|
|
|
@ -10,7 +10,6 @@ use Tests\TestCase;
|
|||
|
||||
class MfaConfigurationTest extends TestCase
|
||||
{
|
||||
|
||||
public function test_totp_setup()
|
||||
{
|
||||
$editor = $this->getEditor();
|
||||
|
@ -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());
|
||||
}
|
||||
|
||||
}
|
|
@ -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');
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -275,5 +275,4 @@ class MfaVerificationTest extends TestCase
|
|||
|
||||
return [$user, $codes, $loginResp];
|
||||
}
|
||||
|
||||
}
|
|
@ -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);
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue