Skip to content

Implement proper account recovery #62

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.

Sign up for GitHub

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

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Implement proper account recovery #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
981b225
WIP
claudiodekker Aug 5, 2023
47f285b
WIP
claudiodekker Aug 5, 2023
82ef62f
WIP
claudiodekker Aug 5, 2023
41cd7e5
WIP
claudiodekker Aug 5, 2023
f66be1e
WIP
claudiodekker Aug 6, 2023
578a1c7
Fix styling
claudiodekker Aug 6, 2023
a16ab2d
WIP
claudiodekker Aug 6, 2023
688c49e
WIP
claudiodekker Aug 6, 2023
7a79c27
WIP
claudiodekker Aug 6, 2023
7de0d22
WIP
claudiodekker Aug 6, 2023
71ae504
WIP
claudiodekker Aug 6, 2023
13a6818
WIP
claudiodekker Aug 6, 2023
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
8 changes: 4 additions & 4 deletions packages/bladebones/src/Testing/BladeViewTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

namespace ClaudioDekker\LaravelAuthBladebones\Testing;

use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\AccountRecoveryChallengeViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\AccountRecoveryRequestViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\CredentialsOverviewViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\LoginViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\MultiFactorChallengeViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\RecoveryChallengeViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\RecoveryRequestViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\RegisterPublicKeyCredentialViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\RegisterTotpCredentialViewTests;
use ClaudioDekker\LaravelAuthBladebones\Testing\Partials\RegisterViewTests;
Expand All @@ -16,11 +16,11 @@ trait BladeViewTests
{
use RegisterViewTests;
use LoginViewTests;
use AccountRecoveryRequestViewTests;
use RecoveryRequestViewTests;

// Challenges
use AccountRecoveryChallengeViewTests;
use MultiFactorChallengeViewTests;
use RecoveryChallengeViewTests;
use SudoModeChallengeViewTests;

// Settings
Expand Down
4 changes: 2 additions & 2 deletions ...als/AccountRecoveryChallengeViewTests.php → ...g/Partials/RecoveryChallengeViewTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use Illuminate\Support\Facades\Password;

trait AccountRecoveryChallengeViewTests
trait RecoveryChallengeViewTests
{
/** @test */
public function the_account_recovery_challenge_page_uses_blade_views(): void
Expand All @@ -14,7 +14,7 @@ public function the_account_recovery_challenge_page_uses_blade_views(): void

$response = $this->get(route('recover-account.challenge', [
'token' => $token,
'email' => $user->getEmailForPasswordReset(),
$this->usernameField() => $user->{$this->usernameField()},
]));

$response->assertViewIs('auth.challenges.recovery');
Expand Down
2 changes: 1 addition & 1 deletion ...tials/AccountRecoveryRequestViewTests.php → ...ing/Partials/RecoveryRequestViewTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace ClaudioDekker\LaravelAuthBladebones\Testing\Partials;

trait AccountRecoveryRequestViewTests
trait RecoveryRequestViewTests
{
/** @test */
public function the_account_recovery_request_page_uses_blade_views(): void
Expand Down
16 changes: 10 additions & 6 deletions packages/bladebones/stubs/defaults/routes/web.stub
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<?php

use App\Http\Controllers\Auth\AccountRecoveryRequestController;
use App\Http\Controllers\Auth\Challenges\AccountRecoveryChallengeController;
use App\Http\Controllers\Auth\Challenges\MultiFactorChallengeController;
use App\Http\Controllers\Auth\Challenges\RecoveryChallengeController;
use App\Http\Controllers\Auth\Challenges\SudoModeChallengeController;
use App\Http\Controllers\Auth\LoginController;
use App\Http\Controllers\Auth\RecoveryRequestController;
use App\Http\Controllers\Auth\RegisterController;
use App\Http\Controllers\Auth\Settings\ChangePasswordController;
use App\Http\Controllers\Auth\Settings\CredentialsController;
Expand Down Expand Up @@ -41,10 +41,14 @@ Route::middleware('guest')->group(function () {
Route::post('/auth/login/challenge', [MultiFactorChallengeController::class, 'store']);
});

Route::get('/auth/recover-account', [AccountRecoveryRequestController::class, 'create'])->name('recover-account');
Route::post('/auth/recover-account', [AccountRecoveryRequestController::class, 'store']);
Route::get('/auth/recover-account/{token}', [AccountRecoveryChallengeController::class, 'create'])->name('recover-account.challenge');
Route::post('/auth/recover-account/{token}', [AccountRecoveryChallengeController::class, 'store']);
Route::get('/auth/recover-account', [RecoveryRequestController::class, 'create'])->name('recover-account');
Route::post('/auth/recover-account', [RecoveryRequestController::class, 'store']);
Route::get('/auth/recover-account/{token}', [RecoveryChallengeController::class, 'create'])->name('recover-account.challenge');
Route::post('/auth/recover-account/{token}', [RecoveryChallengeController::class, 'store']);

Route::middleware([])->group(function () {
Route::get('/auth/reset', ['foo', 'bar'])->name('recover-account.reset');
});
Comment on lines +48 to +51
Copy link
Contributor

@Jubeki Jubeki Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Route::middleware([])->group(function () {
Route::get('/auth/reset', ['foo', 'bar'])->name('recover-account.reset');
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay it seems the route is needed, but there is no Controller associated and an empty middleware array.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is on purpose - hence the PR still being a draft. The actual 'reset' part is still TODO


Route::get('/auth/register', [RegisterController::class, 'create'])->name('register');
Route::post('/auth/register', [RegisterController::class, 'store']);
Expand Down
47 changes: 34 additions & 13 deletions ...ountRecoveryChallengeController.bladetmpl → ...ges/RecoveryChallengeController.bladetmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,41 @@
@php
$flavorTrait = str_replace("-", "", \Illuminate\Support\Str::title($flavor));
@endphp

namespace App\Http\Controllers\Auth\Challenges;

use ClaudioDekker\LaravelAuth\Http\Controllers\Challenges\AccountRecoveryChallengeController as BaseController;
use ClaudioDekker\LaravelAuth\Http\Controllers\Challenges\RecoveryChallengeController as BaseController;
@if ($flavor !== 'email-based')
use ClaudioDekker\LaravelAuth\Http\Modifiers\{{ $flavorTrait }};
@endif
use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Contracts\View\View;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Validation\ValidationException;

class AccountRecoveryChallengeController extends BaseController
class RecoveryChallengeController extends BaseController
{
@php
$uses = [];
if ($flavor !== 'email-based') {
$uses[] = $flavorTrait;
}

asort($uses);
@endphp
@if (count($uses) > 0)
@foreach($uses as $use)
use {{ $use }};
@endforeach

@endif
/**
* Handle an incoming request to view the account recovery challenge page.
*
* {!! '@' !!}see static::sendAccountRecoveredResponse()
* {!! '@' !!}see static::sendChallengePageResponse()
* {!! '@' !!}see static::sendInvalidRecoveryLinkResponse()
* {!! '@' !!}see static::sendChallengePageResponse()
* {!! '@' !!}see static::sendRecoveryModeEnabledResponse()
*/
public function create(Request $request, string $token): RedirectResponse|View
{
Expand All @@ -26,9 +46,9 @@ class AccountRecoveryChallengeController extends BaseController
* Handle an incoming account recovery challenge response.
*
* {!! '@' !!}see static::sendRateLimitedResponse()
* {!! '@' !!}see static::sendAccountRecoveredResponse()
* {!! '@' !!}see static::sendInvalidRecoveryCodeResponse()
* {!! '@' !!}see static::sendInvalidRecoveryLinkResponse()
* {!! '@' !!}see static::sendInvalidRecoveryCodeResponse()
* {!! '@' !!}see static::sendRecoveryModeEnabledResponse()
*/
public function store(Request $request, string $token): RedirectResponse
{
Expand All @@ -52,7 +72,11 @@ class AccountRecoveryChallengeController extends BaseController
*/
protected function sendInvalidRecoveryLinkResponse(Request $request): void
{
abort(403, 'The given email and recovery token combination are invalid.');
@if ($flavor === 'username-based')
abort(403, __('laravel-auth::auth.recovery.invalid', ['field' => 'username']));
@else
abort(403, __('laravel-auth::auth.recovery.invalid', ['field' => 'email']));
@endif
}

/**
Expand All @@ -68,14 +92,11 @@ class AccountRecoveryChallengeController extends BaseController
}

/**
* Sends a response indicating that the user's account has been recovered.
*
* Typically, you'd want this response to redirect the user to their account's security settings page,
* where they can adjust whatever is causing them to be unable to authenticate using normal means.
* Sends a response indicating that recovery mode has been enabled for the given user.
*/
protected function sendAccountRecoveredResponse(Request $request, Authenticatable $user): RedirectResponse
protected function sendRecoveryModeEnabledResponse(Request $request, Authenticatable $user): RedirectResponse
{
return redirect()->route('auth.settings');
return redirect()->route('recover-account.reset');
}

/**
Expand Down
10 changes: 7 additions & 3 deletions ...ccountRecoveryRequestController.bladetmpl → ...llers/RecoveryRequestController.bladetmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

namespace App\Http\Controllers\Auth;

use ClaudioDekker\LaravelAuth\Http\Controllers\AccountRecoveryRequestController as BaseController;
use ClaudioDekker\LaravelAuth\Http\Controllers\RecoveryRequestController as BaseController;
@if (count($traits) > 0)
@foreach($traits as $trait)
use {{ $trait }};
Expand All @@ -27,7 +27,7 @@ use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Validation\ValidationException;

class AccountRecoveryRequestController extends BaseController
class RecoveryRequestController extends BaseController
{
@php
$uses = [];
Expand Down Expand Up @@ -79,7 +79,11 @@ class AccountRecoveryRequestController extends BaseController
*/
public function sendRecoveryLinkSentResponse(Request $request): RedirectResponse
{
return redirect()->back()->with('status', __('laravel-auth::auth.recovery.sent'));
@if ($flavor === 'username-based')
return redirect()->back()->with('status', __('laravel-auth::auth.recovery.sent', ['field' => 'username']));
@else
return redirect()->back()->with('status', __('laravel-auth::auth.recovery.sent', ['field' => 'email']));
@endif
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/bladebones/templates/Tests/AuthenticationTest.bladetmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
@endphp
namespace Tests\Feature;

use ClaudioDekker\LaravelAuth\Testing\AccountRecoveryChallengeTests;
use ClaudioDekker\LaravelAuth\Testing\AccountRecoveryRequestTests;
use ClaudioDekker\LaravelAuth\Testing\EmailVerification\{{ $verificationTrait }};
use ClaudioDekker\LaravelAuth\Testing\EmailVerificationTests;
use ClaudioDekker\LaravelAuth\Testing\Flavors\{{ $flavorTrait }};
use ClaudioDekker\LaravelAuth\Testing\GenerateRecoveryCodesTests;
use ClaudioDekker\LaravelAuth\Testing\LoginTests;
use ClaudioDekker\LaravelAuth\Testing\LogoutTests;
use ClaudioDekker\LaravelAuth\Testing\MultiFactorChallengeTests;
use ClaudioDekker\LaravelAuth\Testing\RecoveryChallengeTests;
use ClaudioDekker\LaravelAuth\Testing\RecoveryRequestTests;
use ClaudioDekker\LaravelAuth\Testing\RegisterPublicKeyCredentialTests;
use ClaudioDekker\LaravelAuth\Testing\RegisterTotpCredentialTests;
use ClaudioDekker\LaravelAuth\Testing\RegistrationTests;
Expand Down Expand Up @@ -48,14 +48,14 @@ class AuthenticationTest extends TestCase
@endforeach

// Basic Auth
use AccountRecoveryRequestTests;
use RecoveryRequestTests;
use RegistrationTests;
use LoginTests;
use LogoutTests;

// Challenges
use AccountRecoveryChallengeTests;
use MultiFactorChallengeTests;
use RecoveryChallengeTests;
use SudoModeChallengeTests;

// Settings
Expand Down
Loading