mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-18 17:45:09 +00:00
Refuse an invitation whose address was taken while the link was live
An invitation stays live for days -- 72 hours by default -- and the address it names can be claimed in that window: staff got impatient and created the account by hand, or the person used the public registration form instead. Redemption never asked, so User::create() met the unique index on users.email and raised a QueryException. A 500, on the screen of somebody who had just chosen a password, having done nothing wrong. ClientProvisioning::addressIsFree() exists for exactly this, and its docblock says who must call it: the paths with no form to validate. LDAP asks. Redemption is the third such path and did not. It now refuses with a message that says what happened, rather than the generic "this invitation is no longer valid" the expired case uses. There is nothing to withhold here -- whoever holds the link already knows the address, because it is the one the invitation was sent to -- and being told to sign in instead is the only useful thing to say. The invitation stays pending rather than being retired. It is the account that resolved the situation, not the link, and a retired row would only make the second attempt read as expired. The address rule spans soft-deleted accounts, the same as it does everywhere else, so a deleted account still holds its address until erasure takes the row away. Both cases are tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CPk8qAs38pudYGWwmGkYPe
This commit is contained in:
@@ -69,6 +69,24 @@ class InvitationRedemptionController extends Controller
|
||||
]);
|
||||
}
|
||||
|
||||
// An invitation is live for days, and the address it names can be
|
||||
// taken in the meantime — staff got impatient and made the account
|
||||
// by hand, or the person registered through the public form. The
|
||||
// unique index on users.email spans trashed rows, so provision()
|
||||
// would raise a QueryException here rather than refusing: a 500 on
|
||||
// the screen of somebody who has just typed a password. Every other
|
||||
// caller with no form to validate asks this first, for this reason
|
||||
// — see ClientProvisioning::addressIsFree().
|
||||
if (! $this->provisioning->addressIsFree($invitation->email)) {
|
||||
throw ValidationException::withMessages([
|
||||
// Says what happened, because the person holding this link
|
||||
// already knows the address is theirs — it is the one the
|
||||
// invitation was sent to. There is nothing here to disclose
|
||||
// that the invitation itself did not.
|
||||
'token' => [__('An account already exists for this email address. Try signing in instead, or reset your password.')],
|
||||
]);
|
||||
}
|
||||
|
||||
$client = $this->provisioning->provision(
|
||||
name: $validated['name'],
|
||||
email: $invitation->email,
|
||||
|
||||
@@ -238,3 +238,35 @@ test('resending an expired invitation issues a fresh token and emails it, withou
|
||||
$this->post('/invite/not-a-real-token/resend')->assertRedirect();
|
||||
Notification::assertNothingSent();
|
||||
});
|
||||
|
||||
test('an invitation whose address was taken while the link was live refuses rather than failing on the unique index', function () {
|
||||
$invitation = Invitation::issue('invited@example.com', null, null, $this->admin, now()->addDay());
|
||||
|
||||
// Staff got impatient, or the person used the public form instead.
|
||||
User::factory()->client()->create(['email' => 'invited@example.com']);
|
||||
|
||||
$this->post("/invite/{$invitation->token}", [
|
||||
'token' => $invitation->token,
|
||||
'name' => 'Invited Person',
|
||||
'password' => 'super-secret-password',
|
||||
'password_confirmation' => 'super-secret-password',
|
||||
])->assertSessionHasErrors('token');
|
||||
|
||||
expect(User::query()->where('email', 'invited@example.com')->count())->toBe(1)
|
||||
->and($invitation->fresh()->status)->toBe(Invitation::STATUS_PENDING);
|
||||
});
|
||||
|
||||
test('an address held by a deleted account is still taken, the same as it is everywhere else', function () {
|
||||
$invitation = Invitation::issue('invited@example.com', null, null, $this->admin, now()->addDay());
|
||||
|
||||
User::factory()->client()->create(['email' => 'invited@example.com'])->delete();
|
||||
|
||||
$this->post("/invite/{$invitation->token}", [
|
||||
'token' => $invitation->token,
|
||||
'name' => 'Invited Person',
|
||||
'password' => 'super-secret-password',
|
||||
'password_confirmation' => 'super-secret-password',
|
||||
])->assertSessionHasErrors('token');
|
||||
|
||||
expect(User::query()->where('email', 'invited@example.com')->exists())->toBeFalse();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user