mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 00:55:07 +00:00
1dc274e896
EnforceTwoFactor exempts by route name, and only the GET half of confirm-password has one. routes/auth.php:95 names the form `password.confirm`; :98 registers its submission with no name at all, and Route::named() answers false for a null name. So the loop the exemption exists to prevent is still there, one step further along. With Setting::TwoFactorEnforcement set to staff, clients or all, an un-enrolled account walks: GET /dashboard -> two-factor.show GET /system/settings/security -> two-factor.show PATCH /system/settings/security -> two-factor.show POST /settings/two-factor -> /confirm-password (RequirePassword) GET /confirm-password -> 200, the form renders POST /confirm-password -> two-factor.show <- not exempt `auth.password_confirmed_at` is never written, so enrolling can never start, and every route that is not on the exemption list stays shut -- including Settings -> Security, the one screen that could turn enforcement back off. Logout is the only door left; recovery is CLI or database access. It takes one administrator turning the setting on to reach it, and it reaches every account on the installation at once, including their own. The fix is the name. `password.confirm*` then covers both halves of one screen, matching `two-factor.*` in the same expression; the namespace belongs entirely to a flow enrolment already depends on being reachable, and the route table has nothing else under it -- `password.confirm` (GET) and `password.confirm.store` (POST) are the two it reaches. Exempting the submission grants nothing further. store() validates the password, writes a session flag and redirects; the redirect it issues enters this middleware like any other request, so Settings -> Security is still answered with two-factor.show after confirming. What changes is that enrolment can now be started. Two tests, both measured red against the unfixed middleware: the password confirmation sticks, and enrolment can be started afterwards (the secret is written and the screen reports `pending`). Also named the redirect the existing test settles for. `->assertRedirect()` with no target passes on this middleware bouncing the request back to two-factor.show, which is the shape that file exists to refuse. It is a clarification rather than a guard -- that assertion is green either way, since the redirect it sees comes from RequirePassword. Full suite passes (2050 passed / 2 skipped), PHPStan level 8 clean.
108 lines
5.0 KiB
PHP
108 lines
5.0 KiB
PHP
<?php
|
|
|
|
use App\Http\Controllers\Auth\AuthenticatedSessionController;
|
|
use App\Http\Controllers\Auth\ConfirmablePasswordController;
|
|
use App\Http\Controllers\Auth\EmailVerificationNotificationController;
|
|
use App\Http\Controllers\Auth\EmailVerificationPromptController;
|
|
use App\Http\Controllers\Auth\NewPasswordController;
|
|
use App\Http\Controllers\Auth\PasswordResetLinkController;
|
|
use App\Http\Controllers\Auth\VerifyEmailController;
|
|
use App\Modules\Clients\Http\Controllers\RegistrationController;
|
|
use App\Modules\Identity\Http\Controllers\SocialLoginController;
|
|
use App\Modules\Identity\Http\Controllers\TwoFactorChallengeController;
|
|
use Illuminate\Support\Facades\Route;
|
|
|
|
// NOTE: there is deliberately no staff registration route. /register is
|
|
// CLIENT self-registration (v1's register.php), gated by the
|
|
// clients_can_register setting inside the controller.
|
|
//
|
|
// **Every `throttle:` below names its own bucket, and must.** The bare
|
|
// two-argument form does not key on the route at all — Laravel keys it on
|
|
// `sha1(domain|ip)` for a guest and `sha1(user_id)` for a signed-in user
|
|
// (ThrottleRequests::resolveRequestSignature) — so all of these counted
|
|
// into one number together with the public share links in web.php, and the
|
|
// tightest limit on that number applied to all of them. Opening six share
|
|
// links locked the visitor out of the two-factor challenge. The numbers
|
|
// here are unchanged; the third argument is what makes each of them mean
|
|
// what it says.
|
|
//
|
|
// POST login is deliberately absent from this: it is rate-limited per
|
|
// email *and* IP inside LoginRequest, which is a stronger boundary than a
|
|
// per-IP count and does not lock out a whole office behind one address.
|
|
Route::middleware('guest')->group(function () {
|
|
Route::get('register', [RegistrationController::class, 'create'])
|
|
->name('register');
|
|
|
|
Route::post('register', [RegistrationController::class, 'store'])
|
|
->middleware('throttle:6,1,register');
|
|
|
|
Route::get('login', [AuthenticatedSessionController::class, 'create'])
|
|
->name('login');
|
|
|
|
Route::post('login', [AuthenticatedSessionController::class, 'store']);
|
|
|
|
// Beginning a provider exchange is a guest action; completing one is
|
|
// not necessarily — see the callback below, which sits outside every
|
|
// group.
|
|
Route::get('auth/{provider}/redirect', [SocialLoginController::class, 'redirect'])
|
|
->middleware('throttle:20,1,social-redirect')
|
|
->name('social.redirect');
|
|
|
|
Route::get('forgot-password', [PasswordResetLinkController::class, 'create'])
|
|
->name('password.request');
|
|
|
|
// The broker's own throttle is per-address (config/auth.php), which
|
|
// does nothing to stop one host walking a list of addresses — so the
|
|
// endpoint is throttled per IP as well, same as register/2FA below.
|
|
Route::post('forgot-password', [PasswordResetLinkController::class, 'store'])
|
|
->middleware('throttle:6,1,password-email')
|
|
->name('password.email');
|
|
|
|
Route::get('reset-password/{token}', [NewPasswordController::class, 'create'])
|
|
->name('password.reset');
|
|
|
|
Route::post('reset-password', [NewPasswordController::class, 'store'])
|
|
->middleware('throttle:6,1,password-reset')
|
|
->name('password.store');
|
|
|
|
Route::get('two-factor-challenge', [TwoFactorChallengeController::class, 'create'])
|
|
->name('two-factor.challenge');
|
|
|
|
Route::post('two-factor-challenge', [TwoFactorChallengeController::class, 'store'])
|
|
->middleware('throttle:6,1,two-factor');
|
|
});
|
|
|
|
// Deliberately in neither group. Signing in through a provider must not
|
|
// require a session, and connecting one to an existing account requires
|
|
// exactly that — so the guard is the intent written into the session
|
|
// before the redirect, which also refuses a callback nobody asked for.
|
|
Route::get('auth/{provider}/callback', [SocialLoginController::class, 'callback'])
|
|
->middleware('throttle:20,1,social-callback')
|
|
->name('social.callback');
|
|
|
|
Route::middleware('auth')->group(function () {
|
|
Route::get('verify-email', EmailVerificationPromptController::class)
|
|
->name('verification.notice');
|
|
|
|
Route::get('verify-email/{id}/{hash}', VerifyEmailController::class)
|
|
->middleware(['signed', 'throttle:6,1,verify-email'])
|
|
->name('verification.verify');
|
|
|
|
Route::post('email/verification-notification', [EmailVerificationNotificationController::class, 'store'])
|
|
->middleware('throttle:6,1,verification-send')
|
|
->name('verification.send');
|
|
|
|
Route::get('confirm-password', [ConfirmablePasswordController::class, 'show'])
|
|
->name('password.confirm');
|
|
|
|
// Named so EnforceTwoFactor can exempt it. Its exemption list matches
|
|
// on route names, and an unnamed route matches nothing -- which left
|
|
// the form reachable and its submission not, closing the enrolment
|
|
// path enforcement depends on.
|
|
Route::post('confirm-password', [ConfirmablePasswordController::class, 'store'])
|
|
->name('password.confirm.store');
|
|
|
|
Route::post('logout', [AuthenticatedSessionController::class, 'destroy'])
|
|
->name('logout');
|
|
});
|