mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-25 12:54:59 +00:00
3e24ccd42f
ConfirmablePasswordController checked the local hash and nothing else:
Auth::guard('web')->validate(['email' => ..., 'password' => ...])
An account provisioned from a directory has no local password. It holds a
Str::password(64) generated at provisioning time that nobody has ever
seen, and the application knows this -- LdapAuthenticator::isDirectoryAccount()
is the question, and the sign-in form asks it before deciding what to
check. This screen did not, so it refused those accounts the only password
they have.
That is not a cosmetic refusal. `password.confirm` stands in front of
enrolling in two-factor, so a directory-provisioned client could not enrol
at all. Set TwoFactorEnforcement to `clients` or `all` and EnforceTwoFactor
redirects every request they make to two-factor.show -- a screen whose
"enable" button leads to a door they cannot open. PR #1708 fixed the
routing half of that ("Let an enforced user reach the far side of the
confirm-password screen"); this is the credential half.
The rule now lives in one place. PasswordVerification is the sibling of
SignIn on the other side of the line SignIn draws -- SignIn is everything
after a credential checks out, this is the one question asked before it --
and it exists for the reason SignIn gives for existing: "the way they get
broken is by being written twice". LoginRequest keeps its ordering, its
provisioning and its rate limiting, and delegates the check itself.
Behaviour preserved exactly on the sign-in path: local hash first so an
account that answers locally generates no directory traffic, directory
only for accounts whose credentials live there, the stale-hash re-hash on
the local branch only, and the ldap_dn stamp on the directory branch. All
23 existing LDAP sign-in tests pass unchanged.
One thing this closes on the way past. Because the old check went straight
to the local hash, a directory account's placeholder *would* have confirmed
if anybody ever learned it -- a door the sign-in form does not have, since
it skips the local branch for those accounts. It now behaves the same on
both screens; there is a test.
**What this does not fix, and should be read as a limitation.** Accounts
provisioned by a social provider are in the same position -- a random local
password nobody holds -- and they are not directory accounts, so this
changes nothing for them. Their route to a local password is the password
reset, which #1748 made work end to end by moving auth_source to Local when
the reset completes. A social account that has never done that still cannot
confirm a password, and so still cannot enrol in two-factor.
Tests: three fail against the unfixed pair, including the placeholder case
above. Two more pin what must not change -- a wrong directory password is
still refused, and a local account with LDAP switched on still confirms
against its own hash.
175 lines
5.5 KiB
PHP
175 lines
5.5 KiB
PHP
<?php
|
|
|
|
namespace App\Http\Requests\Auth;
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Identity\Ldap\LdapProvisioner;
|
|
use App\Modules\Identity\PasswordVerification;
|
|
use App\Modules\Identity\SignIn;
|
|
use App\Modules\Platform\Captcha\CaptchaForm;
|
|
use App\Support\Rules;
|
|
use Illuminate\Auth\Events\Lockout;
|
|
use Illuminate\Contracts\Validation\ValidationRule;
|
|
use Illuminate\Foundation\Http\FormRequest;
|
|
use Illuminate\Support\Facades\Auth;
|
|
use Illuminate\Support\Facades\RateLimiter;
|
|
use Illuminate\Support\Str;
|
|
use Illuminate\Validation\ValidationException;
|
|
|
|
class LoginRequest extends FormRequest
|
|
{
|
|
/**
|
|
* Determine if the user is authorized to make this request.
|
|
*/
|
|
public function authorize(): bool
|
|
{
|
|
return true;
|
|
}
|
|
|
|
/**
|
|
* Get the validation rules that apply to the request.
|
|
*
|
|
* @return array<string, ValidationRule|array<mixed>|string>
|
|
*/
|
|
public function rules(): array
|
|
{
|
|
return [
|
|
'email' => ['required', 'string', 'email'],
|
|
'password' => ['required', 'string'],
|
|
// Deliberately here rather than inside authenticate(): rules
|
|
// run first, so a bot never reaches the credential check, and
|
|
// an honest visitor whose token expired never burns one of
|
|
// their five attempts.
|
|
...Rules::captcha(CaptchaForm::Login),
|
|
];
|
|
}
|
|
|
|
/**
|
|
* Attempt to authenticate the request's credentials.
|
|
*
|
|
* Returns true when the credentials are valid but the account has
|
|
* two-factor authentication enabled: no session is created and the
|
|
* pending user id is stored for the challenge step.
|
|
*
|
|
* Three phases, deliberately in this order:
|
|
*
|
|
* 1. Identify and verify — is this password correct, from any source
|
|
* this installation accepts?
|
|
* 2. Account state — is this account allowed to sign in at all?
|
|
* 3. Two-factor, then the session.
|
|
*
|
|
* Splitting 1 from 2 is what lets a directory be consulted without
|
|
* restating anything. The property that account state is only revealed
|
|
* to somebody holding the right password now falls out of the ordering,
|
|
* rather than being re-established by a second Auth::validate() inside
|
|
* each branch — and rate limiting covers every credential source,
|
|
* because every failure funnels through one refusal.
|
|
*
|
|
* @throws ValidationException
|
|
*/
|
|
public function authenticate(): bool
|
|
{
|
|
$this->ensureIsNotRateLimited();
|
|
|
|
$user = User::query()->where('email', $this->string('email'))->first();
|
|
|
|
// A directory identity with no local account yet. Returns null
|
|
// unless LDAP is on, auto-provisioning is on, and the bind
|
|
// succeeds — so an unknown email costs nothing on an installation
|
|
// that does not use a directory.
|
|
if ($user === null) {
|
|
$user = app(LdapProvisioner::class)->provision(
|
|
(string) $this->string('email'),
|
|
(string) $this->string('password'),
|
|
);
|
|
}
|
|
|
|
$verified = $this->verifyCredentials($user);
|
|
|
|
if ($verified === null) {
|
|
$this->failWithInvalidCredentials();
|
|
}
|
|
|
|
$signIn = app(SignIn::class);
|
|
|
|
$refusal = $signIn->refusalReason($verified);
|
|
|
|
if ($refusal !== null) {
|
|
// Reached only with correct credentials, so this reveals the
|
|
// account state to its owner and to nobody else.
|
|
throw ValidationException::withMessages(['email' => $refusal]);
|
|
}
|
|
|
|
// Phases 2 and 3 are shared with every other way into this
|
|
// application — see SignIn. Rate limiting stays here, because it
|
|
// is a property of this form (keyed on email and IP) rather than
|
|
// of signing in.
|
|
$pendingTwoFactor = $signIn->begin($verified, $this->boolean('remember'));
|
|
|
|
RateLimiter::clear($this->throttleKey());
|
|
|
|
return $pendingTwoFactor;
|
|
}
|
|
|
|
/**
|
|
* The account whose password checks out, or null.
|
|
*
|
|
* The rule itself -- local hash first, directory when the credentials
|
|
* live there -- is PasswordVerification's, because this is no longer
|
|
* the only screen that has to ask it. See that class.
|
|
*/
|
|
private function verifyCredentials(?User $user): ?User
|
|
{
|
|
if ($user === null) {
|
|
return null;
|
|
}
|
|
|
|
return app(PasswordVerification::class)->verify($user, (string) $this->string('password'))
|
|
? $user
|
|
: null;
|
|
}
|
|
|
|
/**
|
|
* @throws ValidationException
|
|
*/
|
|
protected function failWithInvalidCredentials(): never
|
|
{
|
|
RateLimiter::hit($this->throttleKey());
|
|
|
|
throw ValidationException::withMessages([
|
|
'email' => __('auth.failed'),
|
|
]);
|
|
}
|
|
|
|
/**
|
|
* Ensure the login request is not rate limited.
|
|
*
|
|
* @throws ValidationException
|
|
*/
|
|
public function ensureIsNotRateLimited(): void
|
|
{
|
|
if (! RateLimiter::tooManyAttempts($this->throttleKey(), 5)) {
|
|
return;
|
|
}
|
|
|
|
event(new Lockout($this));
|
|
|
|
$seconds = RateLimiter::availableIn($this->throttleKey());
|
|
|
|
throw ValidationException::withMessages([
|
|
'email' => __('auth.throttle', [
|
|
'seconds' => $seconds,
|
|
'minutes' => ceil($seconds / 60),
|
|
]),
|
|
]);
|
|
}
|
|
|
|
/**
|
|
* Get the rate limiting throttle key for the request.
|
|
*/
|
|
public function throttleKey(): string
|
|
{
|
|
return Str::transliterate(Str::lower($this->string('email')).'|'.$this->ip());
|
|
}
|
|
}
|