Skip to content

[feature/2fa] Simplify 2fa features #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: feature/2fa
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions app/Actions/TwoFactorAuth/CompleteTwoFactorAuthentication.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ class CompleteTwoFactorAuthentication
/**
* Complete the two-factor authentication process.
*
* @param mixed $user The user to authenticate
* @return void
* @param mixed $user The user to authenticate
*/
public function __invoke($user): void
{
// Get the remember preference from the session (default to false if not set)
$remember = Session::get('login.remember', false);

// Log the user in with the remember preference
Auth::login($user, $remember);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ public function __invoke($user)
])->save();
}
}
}
}
2 changes: 1 addition & 1 deletion app/Actions/TwoFactorAuth/GenerateNewRecoveryCodes.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ public function generate()
{
return Str::random(10).'-'.Str::random(10);
}
}
}
20 changes: 10 additions & 10 deletions app/Actions/TwoFactorAuth/GenerateQrCodeAndSecretKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

namespace App\Actions\TwoFactorAuth;

use App\Models\User;
use BaconQrCode\Renderer\Image\SvgImageBackEnd;
use BaconQrCode\Renderer\ImageRenderer;
use BaconQrCode\Renderer\RendererStyle\RendererStyle;
use BaconQrCode\Writer;
use App\Models\User;
use PragmaRX\Google2FA\Google2FA;

class GenerateQrCodeAndSecretKey
Expand All @@ -21,34 +21,34 @@ class GenerateQrCodeAndSecretKey
public function __invoke($user): array
{
// Create a new Google2FA instance with explicit configuration
$google2fa = new Google2FA();
$google2fa = new Google2FA;
$google2fa->setOneTimePasswordLength(6);

// Generate a standard 16-character secret key
$secret_key = $google2fa->generateSecretKey(16);

// Set company name from config
$this->companyName = config('app.name', 'Laravel');

// Generate the QR code URL
$g2faUrl = $google2fa->getQRCodeUrl(
$this->companyName,
$user->email,
$secret_key
);

// Create the QR code image
$writer = new Writer(
new ImageRenderer(
new RendererStyle(400),
new SvgImageBackEnd()
new SvgImageBackEnd
)
);

// Generate the QR code as a base64 encoded SVG
$qrcode_image = base64_encode($writer->writeString($g2faUrl));

return [$qrcode_image, $secret_key];

}
}
}
20 changes: 10 additions & 10 deletions app/Actions/TwoFactorAuth/ProcessRecoveryCode.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,28 @@ class ProcessRecoveryCode
/**
* Verify a recovery code and remove it from the list if valid.
*
* @param array $recoveryCodes The array of recovery codes
* @param string $submittedCode The code submitted by the user
* @param array $recoveryCodes The array of recovery codes
* @param string $submittedCode The code submitted by the user
* @return array|false Returns the updated array of recovery codes if valid, or false if invalid
*/
public function __invoke(array $recoveryCodes, string $submittedCode)
{
// Clean the submitted code
$submittedCode = trim($submittedCode);

// If the user has entered multiple codes, only validate the first one
$submittedCode = explode(" ", $submittedCode)[0];
$submittedCode = explode(' ', $submittedCode)[0];

// Check if the code is valid
if (!in_array($submittedCode, $recoveryCodes)) {
if (! in_array($submittedCode, $recoveryCodes)) {
return false;
}

// Remove the used recovery code from the list
$updatedCodes = array_values(array_filter($recoveryCodes, function($code) use ($submittedCode) {
return !hash_equals($code, $submittedCode);
$updatedCodes = array_values(array_filter($recoveryCodes, function ($code) use ($submittedCode) {
return ! hash_equals($code, $submittedCode);
}));

return $updatedCodes;
}
}
11 changes: 5 additions & 6 deletions app/Actions/TwoFactorAuth/VerifyTwoFactorCode.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,19 @@ class VerifyTwoFactorCode
/**
* Verify a two-factor authentication code.
*
* @param string $secret The decrypted secret key
* @param string $code The code to verify
* @return bool
* @param string $secret The decrypted secret key
* @param string $code The code to verify
*/
public function __invoke(string $secret, string $code): bool
{
// Clean the code (remove spaces and non-numeric characters)
$code = preg_replace('/[^0-9]/', '', $code);

// Create a new Google2FA instance with explicit configuration
$google2fa = new Google2FA();
$google2fa = new Google2FA;
$google2fa->setWindow(8); // Allow for some time drift
$google2fa->setOneTimePasswordLength(6); // Ensure 6-digit codes

try {
return $google2fa->verify($code, $secret);
} catch (\Exception $e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function store(LoginRequest $request): RedirectResponse
// Store the user ID and remember preference in the session
$request->session()->put([
'login.id' => $user->getKey(),
'login.remember' => $request->boolean('remember')
'login.remember' => $request->boolean('remember'),
]);

return redirect()->route('two-factor.challenge');
Expand Down
42 changes: 17 additions & 25 deletions app/Http/Controllers/Auth/TwoFactorAuthChallengeController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
use App\Models\User;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\RateLimiter;
use Illuminate\Validation\ValidationException;
use Illuminate\Support\Str;
use Illuminate\Validation\ValidationException;

class TwoFactorAuthChallengeController extends Controller
{
/**
* Attempt to authenticate a new session using the two factor authentication code.
*
* @param \Illuminate\Http\Request $request
* @return mixed
*/
public function store(Request $request)
Expand Down Expand Up @@ -49,64 +48,61 @@ public function store(Request $request)
/**
* Authenticate using a one-time password (OTP).
*
* @param \Illuminate\Http\Request $request
* @param \App\Models\User $user
* @return \Illuminate\Http\Response
*/
protected function authenticateUsingCode(Request $request, User $user)
{
$secret = decrypt($user->two_factor_secret);
$secret = $user->two_factor_secret;
$valid = app(VerifyTwoFactorCode::class)($secret, $request->code);

if ($valid) {
app(CompleteTwoFactorAuthentication::class)($user);
RateLimiter::clear($this->throttleKey($user));

return redirect()->intended(route('dashboard', absolute: false));
}

RateLimiter::hit($this->throttleKey($user));

return back()->withErrors(['code' => __('The provided two factor authentication code was invalid.')]);
}

/**
* Authenticate using a recovery code.
*
* @param \Illuminate\Http\Request $request
* @param \App\Models\User $user
* @return \Illuminate\Http\Response
*/
protected function authenticateUsingRecoveryCode(Request $request, User $user)
{
$recoveryCodes = json_decode(decrypt($user->two_factor_recovery_codes), true);
$recoveryCodes = $user->two_factor_recovery_codes;

// Process the recovery code - this handles validation and removing the used code
$updatedCodes = app(ProcessRecoveryCode::class)($recoveryCodes, $request->recovery_code);

// If ProcessRecoveryCode returns false, the code was invalid
if ($updatedCodes === false) {
RateLimiter::hit($this->throttleKey($user));

return back()->withErrors(['recovery_code' => __('The provided two factor authentication recovery code was invalid.')]);
}

// Update the user's recovery codes, removing the used code
$user->two_factor_recovery_codes = encrypt(json_encode($updatedCodes));
$user->two_factor_recovery_codes = $updatedCodes;
$user->save();

// Complete the authentication process
app(CompleteTwoFactorAuthentication::class)($user);

// Clear rate limiter after successful authentication
RateLimiter::clear($this->throttleKey($user));

// Redirect to the intended page
return redirect()->intended(route('dashboard', absolute: false));
}

/**
* Ensure the 2FA challenge is not rate limited.
*
* @param \App\Models\User $user
* @return void
*
* @throws \Illuminate\Validation\ValidationException
*/
Expand All @@ -127,13 +123,9 @@ protected function ensureIsNotRateLimited(User $user): void

/**
* Get the rate limiting throttle key for the given user.
*
* @param \App\Models\User $user
* @return string
*/
protected function throttleKey(User $user): string
{
return Str::transliterate($user->id . '|2fa|' . request()->ip());
return Str::transliterate($user->id.'|2fa|'.request()->ip());
}
}

Loading