Refuse a portal login for an address that already belongs to an operator
Customer::ensureUser() enforced uniqueness only within users, so a customer provisioned (or registering publicly) with an operator's email got a second, colliding identity for the same address — the exact thing R21 exists to prevent. Checks Operator by address now, not by session (the deleted assertNotAdmin() checked who was signed in, which could never be the right question). Public registration gets the same guard via a plain unique() validation rule, reusing Laravel's stock "already taken" message rather than a dedicated one — naming the collision specifically would tell a public visitor an address belongs to staff, a worse leak than the generic refusal every other collision here already gives them. Impersonation answers 409 instead of crashing into it. The Stripe webhook keeps recording the paid order (same principle as openContract()'s own comment: the order is proof money changed hands) and withholds only the colliding login, logged for an operator to resolve by hand. Verified the reverse direction Codex flagged as already covered: two Admin\Settings actions and the clupilot:create-operator command all already refuse an operator email that collides with a customer's, and that check is sound today because every users row is created in lockstep with a customers row of the same email (CreateNewUser and ensureUser are the only two creation sites, both confirmed by grep) and nothing in this app can change a user's email afterwards (Fortify's updateProfileInformation feature is disabled, and no other code calls that action).feat/operator-identity
parent
913d19d39c
commit
20329b176b
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Actions\Fortify;
|
||||
|
||||
use App\Models\Customer;
|
||||
use App\Models\Operator;
|
||||
use App\Models\User;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Hash;
|
||||
|
|
@ -36,6 +37,14 @@ class CreateNewUser implements CreatesNewUsers
|
|||
// provisioned) customer's email — ensureUser() would later link
|
||||
// that account and hand over the customer's portal.
|
||||
Rule::unique(Customer::class, 'email'),
|
||||
// Nor an operator's — R21: one address must not represent both
|
||||
// identities. No dedicated message: Laravel's stock "already
|
||||
// taken" line (translated in both languages already) is what
|
||||
// the two rules above fall back to as well, and telling a
|
||||
// public visitor specifically that an address belongs to
|
||||
// STAFF would be a worse leak than the generic refusal every
|
||||
// other collision here already gives them.
|
||||
Rule::unique(Operator::class, 'email'),
|
||||
],
|
||||
'password' => $this->passwordRules(),
|
||||
])->validate();
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
namespace App\Actions;
|
||||
|
||||
use App\Exceptions\IdentityCollisionException;
|
||||
use App\Models\Customer;
|
||||
use App\Models\Order;
|
||||
use App\Models\ProvisioningRun;
|
||||
|
|
@ -39,7 +40,21 @@ class StartCustomerProvisioning
|
|||
}
|
||||
|
||||
$customer = $this->resolveCustomer($event);
|
||||
$customer->ensureUser(); // portal login (also enables admin impersonation)
|
||||
|
||||
try {
|
||||
$customer->ensureUser(); // portal login (also enables admin impersonation)
|
||||
} catch (IdentityCollisionException) {
|
||||
// Same principle as openContract() below: the order is the record
|
||||
// that money changed hands, so a paid order still gets recorded
|
||||
// and provisioned. Only the colliding portal LOGIN is withheld —
|
||||
// R21 — until an operator resolves the address conflict by hand
|
||||
// (rename the operator, or the customer). Until then this
|
||||
// customer has no self-service sign-in and cannot be
|
||||
// impersonated either; both call ensureUser() the same way.
|
||||
Log::error('Portal login withheld: this customer email already belongs to an operator account.', [
|
||||
'customer_id' => $customer->id, 'email' => $customer->email,
|
||||
]);
|
||||
}
|
||||
|
||||
try {
|
||||
[$order, $run] = DB::transaction(function () use ($event, $customer) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,21 @@
|
|||
<?php
|
||||
|
||||
namespace App\Exceptions;
|
||||
|
||||
use RuntimeException;
|
||||
|
||||
/**
|
||||
* Refused: this address already belongs to the other identity table.
|
||||
*
|
||||
* R21 — operators and customers are two groups of people, never one row that
|
||||
* could pass for both. Keyed on the address itself, not on who happens to be
|
||||
* signed in when the check runs (see the deleted Customer::assertNotAdmin(),
|
||||
* which checked the caller's session and mis-fired for exactly that reason).
|
||||
*/
|
||||
class IdentityCollisionException extends RuntimeException
|
||||
{
|
||||
public function __construct(public readonly string $email)
|
||||
{
|
||||
parent::__construct("Refusing to create or link a portal login for {$email}: that address already belongs to an operator (R21).");
|
||||
}
|
||||
}
|
||||
|
|
@ -2,10 +2,11 @@
|
|||
|
||||
namespace App\Http\Controllers;
|
||||
|
||||
use App\Exceptions\IdentityCollisionException;
|
||||
use App\Models\Customer;
|
||||
use App\Models\Operator;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Http\RedirectResponse;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Support\Facades\Auth;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\URL;
|
||||
|
|
@ -79,7 +80,15 @@ class ImpersonationController extends Controller
|
|||
|
||||
abort_unless(Cache::add($marker, true, self::WINDOW), 403);
|
||||
|
||||
$target = Customer::where('uuid', $customer)->firstOrFail()->ensureUser();
|
||||
// Stale data, not a normal outcome: a customer row sharing its email
|
||||
// with an operator should never have been possible to reach this far
|
||||
// (see Customer::ensureUser() and CreateNewUser, R21) — refuse rather
|
||||
// than hand the operator's own address a portal session.
|
||||
try {
|
||||
$target = Customer::where('uuid', $customer)->firstOrFail()->ensureUser();
|
||||
} catch (IdentityCollisionException) {
|
||||
abort(409);
|
||||
}
|
||||
$who = Operator::where('uuid', $operator)->firstOrFail();
|
||||
|
||||
Auth::guard('web')->login($target);
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@
|
|||
|
||||
namespace App\Models;
|
||||
|
||||
use App\Exceptions\IdentityCollisionException;
|
||||
use App\Models\Concerns\HasUuid;
|
||||
use Database\Factories\CustomerFactory;
|
||||
use Illuminate\Database\Eloquent\Factories\HasFactory;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use Illuminate\Database\Eloquent\Relations\BelongsTo;
|
||||
|
|
@ -38,7 +40,7 @@ class Customer extends Model
|
|||
&& $this->normalisedVatId() === self::normaliseVatId($this->vat_id_verified_value);
|
||||
}
|
||||
|
||||
/** @use HasFactory<\Database\Factories\CustomerFactory> */
|
||||
/** @use HasFactory<CustomerFactory> */
|
||||
use HasFactory, HasUuid;
|
||||
|
||||
protected $fillable = [
|
||||
|
|
@ -97,10 +99,14 @@ class Customer extends Model
|
|||
|
||||
/**
|
||||
* Find or create the portal login account for this customer and link it.
|
||||
* Race-safe against the unique email index. Never links an operator
|
||||
* account by construction, not by a runtime check: operators live in
|
||||
* `operators`, on their own guard, and are never rows in `users` at all
|
||||
* (see R21) — there is no longer an admin `users` row this could find.
|
||||
* Race-safe against the unique email index. Refuses by construction to
|
||||
* create or link a `users` row for an address that already belongs to an
|
||||
* `operators` row (R21) — that table split killed the old "an operator
|
||||
* account never has an is_admin users row" guarantee this method used to
|
||||
* lean on, and left this one open: nothing stops the SAME address from
|
||||
* being provisioned as a customer independently of its operator row.
|
||||
*
|
||||
* @throws IdentityCollisionException when the address is already an operator's.
|
||||
*/
|
||||
public function ensureUser(): User
|
||||
{
|
||||
|
|
@ -108,6 +114,10 @@ class Customer extends Model
|
|||
return $this->user;
|
||||
}
|
||||
|
||||
if (Operator::query()->where('email', $this->email)->exists()) {
|
||||
throw new IdentityCollisionException($this->email);
|
||||
}
|
||||
|
||||
if (($existing = User::query()->where('email', $this->email)->first()) !== null) {
|
||||
$this->update(['user_id' => $existing->id]);
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,11 @@ return [
|
|||
'body' => 'Die Anfrage passt nicht zu dieser Adresse.',
|
||||
'hint' => null,
|
||||
],
|
||||
'409' => [
|
||||
'title' => 'Dieses Konto lässt sich nicht verbinden.',
|
||||
'body' => 'Diese E-Mail-Adresse des Kunden gehört bereits zu einem Mitarbeiterkonto. Ein Inhaber muss diesen Konflikt auflösen, bevor dieses Portal erreichbar ist.',
|
||||
'hint' => null,
|
||||
],
|
||||
'419' => [
|
||||
'title' => 'Die Sitzung ist abgelaufen.',
|
||||
'body' => 'Aus Sicherheitsgründen wird ein Formular nach längerer Zeit ungültig.',
|
||||
|
|
|
|||
|
|
@ -27,6 +27,11 @@ return [
|
|||
'body' => 'The request does not match this address.',
|
||||
'hint' => null,
|
||||
],
|
||||
'409' => [
|
||||
'title' => 'This account cannot be connected.',
|
||||
'body' => 'This customer\'s email address already belongs to a staff account. An owner needs to resolve that conflict before this portal can be reached.',
|
||||
'hint' => null,
|
||||
],
|
||||
'419' => [
|
||||
'title' => 'The session has expired.',
|
||||
'body' => 'For security, a form stops being valid after a while.',
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
@include('errors.layout', ['code' => 409])
|
||||
|
|
@ -1,5 +1,7 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Customer;
|
||||
use App\Models\Operator;
|
||||
use App\Models\User;
|
||||
|
||||
it('shows the signup page to guests', function () {
|
||||
|
|
@ -26,12 +28,12 @@ it('registers a new account and signs in', function () {
|
|||
$this->assertAuthenticatedAs($user);
|
||||
|
||||
// A linked customer is created so the portal has something to work with.
|
||||
$customer = \App\Models\Customer::query()->where('user_id', $user->id)->first();
|
||||
$customer = Customer::query()->where('user_id', $user->id)->first();
|
||||
expect($customer)->not->toBeNull()->and($customer->email)->toBe('new@signup.test');
|
||||
});
|
||||
|
||||
it('refuses to register with an existing customer email (account-claim guard)', function () {
|
||||
\App\Models\Customer::factory()->create(['email' => 'claim@signup.test']);
|
||||
Customer::factory()->create(['email' => 'claim@signup.test']);
|
||||
|
||||
$this->post(route('register.store'), [
|
||||
'name' => 'Claimer', 'email' => 'claim@signup.test',
|
||||
|
|
@ -41,6 +43,19 @@ it('refuses to register with an existing customer email (account-claim guard)',
|
|||
expect(User::query()->where('email', 'claim@signup.test')->exists())->toBeFalse();
|
||||
});
|
||||
|
||||
it('refuses to register with an existing operator email', function () {
|
||||
// R21: the operator side of the same account-claim guard above — an
|
||||
// address that already belongs to staff must not become a customer too.
|
||||
Operator::factory()->create(['email' => 'staff@signup.test']);
|
||||
|
||||
$this->post(route('register.store'), [
|
||||
'name' => 'Claimer', 'email' => 'staff@signup.test',
|
||||
'password' => 'password1234', 'password_confirmation' => 'password1234',
|
||||
])->assertSessionHasErrors('email');
|
||||
|
||||
expect(User::query()->where('email', 'staff@signup.test')->exists())->toBeFalse();
|
||||
});
|
||||
|
||||
it('throttles repeated registration attempts', function () {
|
||||
$hit429 = false;
|
||||
for ($i = 0; $i < 24; $i++) {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
<?php
|
||||
|
||||
use App\Exceptions\IdentityCollisionException;
|
||||
use App\Models\Customer;
|
||||
use App\Models\Operator;
|
||||
use App\Models\User;
|
||||
|
|
@ -48,6 +49,34 @@ it('impersonates a customer who already has a portal login, with the operator se
|
|||
->and(Auth::guard('web')->user()->email)->toBe($customer->email);
|
||||
});
|
||||
|
||||
it('refuses to create or link a portal login when the email already belongs to an operator', function () {
|
||||
// R21: one address must not represent both identities. This is the
|
||||
// provisioning/impersonation-time half of the rule; CreateNewUser enforces
|
||||
// the same thing at public registration (see RegisterTest).
|
||||
Operator::factory()->create(['email' => 'shared@example.test']);
|
||||
$customer = Customer::factory()->create(['email' => 'shared@example.test']);
|
||||
|
||||
expect(fn () => $customer->ensureUser())
|
||||
->toThrow(IdentityCollisionException::class);
|
||||
|
||||
expect(User::query()->where('email', 'shared@example.test')->exists())->toBeFalse()
|
||||
->and($customer->fresh()->user_id)->toBeNull();
|
||||
});
|
||||
|
||||
it('refuses to impersonate into an email that already belongs to an operator, instead of crashing', function () {
|
||||
$operator = Operator::factory()->role('Owner')->create();
|
||||
Operator::factory()->create(['email' => 'shared@example.test']);
|
||||
$customer = Customer::factory()->create(['email' => 'shared@example.test']);
|
||||
|
||||
$url = URL::temporarySignedRoute('impersonate.enter', now()->addSeconds(60), [
|
||||
'customer' => $customer->uuid, 'operator' => $operator->uuid,
|
||||
]);
|
||||
|
||||
$this->get($url)->assertStatus(409);
|
||||
|
||||
expect(Auth::guard('web')->check())->toBeFalse();
|
||||
});
|
||||
|
||||
it('signs the customer in on the web guard when the link is followed', function () {
|
||||
$operator = Operator::factory()->role('Owner')->create();
|
||||
$customer = Customer::factory()->create();
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Operator;
|
||||
use App\Models\Order;
|
||||
use App\Models\ProvisioningRun;
|
||||
use App\Models\User;
|
||||
use App\Provisioning\Jobs\AdvanceRunJob;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
|
||||
|
|
@ -106,6 +108,25 @@ it('ignores a completed but unpaid checkout session', function () {
|
|||
Queue::assertNothingPushed();
|
||||
});
|
||||
|
||||
it('still provisions a paid order when the customer email already belongs to an operator, but withholds the portal login', function () {
|
||||
// R21: one address must not represent both an operator and a customer.
|
||||
// The payment already happened, so — same principle as openContract()'s
|
||||
// own "the order is the record that money changed hands" — the order and
|
||||
// run are still created; only the colliding portal LOGIN is withheld.
|
||||
Queue::fake();
|
||||
|
||||
Operator::factory()->create(['email' => 'kunde@example.com']);
|
||||
|
||||
$this->postJson(route('webhooks.stripe'), stripeEvent('evt_opcollide'))->assertOk();
|
||||
|
||||
$order = Order::query()->where('stripe_event_id', 'cs_evt_opcollide')->first();
|
||||
expect($order)->not->toBeNull()
|
||||
->and(ProvisioningRun::query()->where('subject_id', $order->id)->where('pipeline', 'customer')->exists())->toBeTrue()
|
||||
->and(User::query()->where('email', 'kunde@example.com')->exists())->toBeFalse();
|
||||
|
||||
Queue::assertPushed(AdvanceRunJob::class, 1);
|
||||
});
|
||||
|
||||
it('rejects an invalid signature when a secret is configured', function () {
|
||||
config()->set('services.stripe.webhook_secret', 'whsec_test');
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue