mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
c72adadc44
POST /confirm-password verified the account's password and counted nothing. Forty wrong guesses, forty identical refusals, no lockout, no Retry-After, no log line. routes/auth.php opens by requiring the opposite: **Every `throttle:` below names its own bucket, and must.** and every other route in the file has one. POST login is the deliberate exception, and the file says why -- LoginRequest limits it per email *and* IP, which is a stronger boundary than a per-IP count. confirm-password had neither of those things. It is the wrong door to leave unlatched. Re-proving the password is what stands between a stolen session and disabling two-factor, regenerating recovery codes, or minting an API token -- credentials that outlive the session, which is the reason routes/settings.php gives for putting those routes behind it. An attacker who already holds the session can sit on this endpoint until the password falls out of it, and then has the password for everything else too. Two more with the same shape, in routes/settings.php: - PUT /settings/password -- update() validates `current_password`. - DELETE /settings/profile -- destroy() validates `current_password`. Both were equally uncounted, and both answer the same question in the same way, so an attacker refused at one door simply used the next. Fixing one of three would have been cosmetic. All three get named buckets at 6/1, matching the credential-facing routes already in auth.php. Named rather than bare: a bare `throttle:` keys on sha1(domain|ip) or sha1(user_id) with no route in it, which is how six share links once locked a visitor out of the two-factor challenge. Not changed: POST /logout has no bucket either and does not need one -- it checks no credential and reveals nothing by being repeated. PATCH /settings/profile likewise. Tests: three that fail against the unthrottled routes, and two that pin what the buckets must not do -- exhausting one must not spend another's, and one account's guesses must not lock a different account out.
120 lines
5.8 KiB
PHP
120 lines
5.8 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.
|
|
//
|
|
// Throttled because it checks a password. It was the one credential
|
|
// check in this file with no bucket at all: not the per-email-and-IP
|
|
// limiter POST login has, not a named `throttle:` like the rest --
|
|
// nothing, so an attacker holding a stolen session could sit on it
|
|
// and guess. That is the wrong door to leave open, because passing it
|
|
// is exactly what re-proving the password is meant to make expensive:
|
|
// beyond it lie disabling two-factor, regenerating recovery codes and
|
|
// minting an API token, and the password is then known for everything
|
|
// else too. Six a minute, matching the other credential-facing
|
|
// buckets here.
|
|
Route::post('confirm-password', [ConfirmablePasswordController::class, 'store'])
|
|
->middleware('throttle:6,1,password-confirm')
|
|
->name('password.confirm.store');
|
|
|
|
Route::post('logout', [AuthenticatedSessionController::class, 'destroy'])
|
|
->name('logout');
|
|
});
|