diff --git a/server/src/Http/Controllers/Api/v1/DriverController.php b/server/src/Http/Controllers/Api/v1/DriverController.php index e1b331c50..cdb187245 100644 --- a/server/src/Http/Controllers/Api/v1/DriverController.php +++ b/server/src/Http/Controllers/Api/v1/DriverController.php @@ -91,11 +91,12 @@ public function create(CreateDriverRequest $request) // get user details for driver $userDetails = $request->only(['name', 'password', 'email', 'phone', 'timezone']); - // Get current company session - $company = $request->has('company') ? $this->companyFromRequest($request) : $this->currentCompany(); - - // Debugging: Ensure company is retrieved correctly - if (!$company) { + // Drivers are created in the credential's own organization. A request + // may still name it, but naming any other organization is refused the + // same way as one that does not exist. + $company = $this->currentCompany(); + $requestedCompany = $request->input('company'); + if (!$company || ($request->has('company') && !in_array($requestedCompany, [$company->uuid, $company->public_id], true))) { return $this->apiError('Company not found.'); } @@ -206,7 +207,7 @@ public function update($id, UpdateDriverRequest $request) // update the driver's login account through its proxy fields try { - ProfileAccountManager::syncProxyFields($driver->getUser(), $userDetails); + ProfileAccountManager::syncProxyFields($driver->getUser(), $userDetails, $driver->company_uuid); } catch (ProfileIdentityConflictException $exception) { return $this->jsonResponse(['error' => $exception->getMessage()], 422); } @@ -996,11 +997,6 @@ protected function driverInputFromRequest(Request $request, ?string $companyUuid return $input; } - protected function companyFromRequest(Request $request): ?Company - { - return Auth::getCompanyFromRequest($request); - } - protected function currentCompany(): ?Company { return Auth::getCompany(); diff --git a/server/src/Http/Controllers/Internal/v1/CustomerController.php b/server/src/Http/Controllers/Internal/v1/CustomerController.php index b20ed00b4..03851bb39 100644 --- a/server/src/Http/Controllers/Internal/v1/CustomerController.php +++ b/server/src/Http/Controllers/Internal/v1/CustomerController.php @@ -36,7 +36,7 @@ public function createPortalLogin(Request $request) return response()->error('Unable to create customer portal login.'); } - if ($error = $this->staffLinkedError($user)) { + if ($error = $this->staffLinkedError($user, $customer)) { return $error; } @@ -58,7 +58,7 @@ public function sendCredentials(Request $request) return response()->error('Unable to send customer portal credentials.'); } - if ($error = $this->staffLinkedError($user)) { + if ($error = $this->staffLinkedError($user, $customer)) { return $error; } @@ -80,7 +80,7 @@ public function deactivatePortalLogin(Request $request) return response()->error('Customer portal login not found.'); } - if ($error = $this->staffLinkedError($user)) { + if ($error = $this->staffLinkedError($user, $customer)) { return $error; } @@ -100,7 +100,7 @@ public function reactivatePortalLogin(Request $request) return response()->error('Customer portal login not found.'); } - if ($error = $this->staffLinkedError($user)) { + if ($error = $this->staffLinkedError($user, $customer)) { return $error; } @@ -180,7 +180,7 @@ public function resetCredentials(Request $request) return response()->error('Unable to reset customer credentials'); } - if ($error = $this->staffLinkedError($user)) { + if ($error = $this->staffLinkedError($user, $customer)) { return $error; } @@ -200,14 +200,19 @@ public function resetCredentials(Request $request) /** * A customer linked to a team member's account signs in with that account, - * which is managed in IAM. + * which is managed in IAM. A login another organization also uses is not + * this organization's to change either. */ - protected function staffLinkedError(User $user): ?\Illuminate\Http\JsonResponse + protected function staffLinkedError(User $user, Contact $customer): ?\Illuminate\Http\JsonResponse { if (ProfileAccountManager::isStaffAccount($user)) { return response()->error('This customer signs in with a team member account. Manage this login in IAM.', 422); } + if ($customer->company_uuid && ProfileAccountManager::isSharedWithAnotherCompany($user, $customer->company_uuid)) { + return response()->error(ProfileAccountManager::SHARED_LOGIN_MESSAGE, 422); + } + return null; } diff --git a/server/src/Http/Controllers/Internal/v1/DriverController.php b/server/src/Http/Controllers/Internal/v1/DriverController.php index 6e4dcc212..f121aeb83 100644 --- a/server/src/Http/Controllers/Internal/v1/DriverController.php +++ b/server/src/Http/Controllers/Internal/v1/DriverController.php @@ -179,7 +179,7 @@ function (&$request, &$driver, &$input) { $input = $input->except(['name', 'password', 'email', 'phone', 'meta', 'avatar_uuid', 'photo_uuid', 'user_uuid', 'user'])->toArray(); // Update the driver's login account through its proxy fields - ProfileAccountManager::syncProxyFields($driver->getUser(), $userInput); + ProfileAccountManager::syncProxyFields($driver->getUser(), $userInput, $driver->company_uuid); // Flush cache $driver->flushAttributesCache(); @@ -333,6 +333,11 @@ protected function resolveDriverLogin(string $id): array return [$driver, $user, response()->error('This driver signs in with a team member account. Manage this login in IAM.', 422)]; } + // Its password and status are not this organization's to change + if (ProfileAccountManager::isSharedWithAnotherCompany($user, $driver->company_uuid)) { + return [$driver, $user, response()->error(ProfileAccountManager::SHARED_LOGIN_MESSAGE, 422)]; + } + return [$driver, $user, null]; } diff --git a/server/src/Models/Contact.php b/server/src/Models/Contact.php index 9f0af9374..60c75c6d5 100644 --- a/server/src/Models/Contact.php +++ b/server/src/Models/Contact.php @@ -580,7 +580,7 @@ public function syncWithUser(): bool return false; } - return ProfileAccountManager::syncProxyFields($this->getUser(), $updates); + return ProfileAccountManager::syncProxyFields($this->getUser(), $updates, $this->company_uuid); } /** diff --git a/server/src/Support/ProfileAccountManager.php b/server/src/Support/ProfileAccountManager.php index 8619a957b..c54db2be9 100644 --- a/server/src/Support/ProfileAccountManager.php +++ b/server/src/Support/ProfileAccountManager.php @@ -24,6 +24,11 @@ * staff account (type `user` or `admin`) of the same organization. A staff * account is managed in IAM, so a profile never changes its email or phone, * resets its password, deactivates it or deletes it. + * + * Emails and phone numbers identify one account across the whole instance, + * but an organization only ever reuses an account that already belongs to it. + * An account of another organization is never linked, and the refusal does + * not say whose it is. */ class ProfileAccountManager { @@ -32,6 +37,12 @@ class ProfileAccountManager */ public const MANAGED_TYPES = ['driver', 'contact', 'customer']; + /** + * Why an organization can't change the password or status of a login that + * another organization's profile also holds. + */ + public const SHARED_LOGIN_MESSAGE = 'This login is also used with another organization, so it can only be managed by its holder.'; + /** * The organization role given to each managed account type. */ @@ -126,8 +137,8 @@ public static function lookup(string $companyUuid, string $type, ?string $email, /** * Resolve the account for a new profile. A staff member of the organization - * or a managed account of the same type with the email/phone is linked; - * otherwise a managed account is created. + * or a managed account of the same type in the organization with the + * email/phone is linked; otherwise a managed account is created. * * @param array $attributes extra user attributes: password, status, timezone, avatar_uuid, country, ip_address, meta * @@ -191,18 +202,21 @@ public static function createManagedAccount(string $companyUuid, string $type, ? /** * Push the profile's proxy fields to its account. A staff account only - * takes the name: its email and phone are its console identity. + * takes the name: its email and phone are its console identity. So does a + * managed account that another organization's profile also holds, since + * its sign-in details are not this organization's to change. * * @throws ProfileIdentityConflictException */ - public static function syncProxyFields(?User $user, array $attributes): bool + public static function syncProxyFields(?User $user, array $attributes, ?string $companyUuid = null): bool { if (!$user) { return false; } - $fields = static::isStaffAccount($user) ? ['name'] : ['name', 'email', 'phone', 'timezone', 'avatar_uuid', 'password']; - $input = array_intersect_key($attributes, array_flip($fields)); + $nameOnly = static::isStaffAccount($user) || ($companyUuid && static::isSharedWithAnotherCompany($user, $companyUuid)); + $fields = $nameOnly ? ['name'] : ['name', 'email', 'phone', 'timezone', 'avatar_uuid', 'password']; + $input = array_intersect_key($attributes, array_flip($fields)); if (array_key_exists('email', $input)) { $input['email'] = static::normalizeEmail($input['email']); @@ -312,11 +326,13 @@ public static function deliverCredentials(Driver|Contact $profile, User $user, s */ public static function assertAccountCanHoldProfile(User $account, string $companyUuid, string $type, string $field = 'email'): void { - if (static::isStaffAccount($account)) { - if (!static::isCompanyMember($account, $companyUuid)) { - throw new ProfileIdentityConflictException(static::takenMessage($field), $field); - } - } elseif ($account->type !== $type) { + // An account of another organization is never linked, and nothing + // about it (its name, type or which organization holds it) is told + if (!static::isCompanyMember($account, $companyUuid) && !(static::isManagedAccount($account) && static::isUnowned($account))) { + throw new ProfileIdentityConflictException(static::takenMessage($field), $field); + } + + if (static::isManagedAccount($account) && $account->type !== $type) { throw new ProfileIdentityConflictException('This ' . static::fieldLabel($field) . ' is already used by a ' . $account->type . '.', $field); } @@ -345,6 +361,29 @@ public static function profileCompanies(User $user): \Illuminate\Support\Collect return $drivers->merge($contacts)->filter()->unique()->values(); } + /** + * Whether another organization holds the account through a membership or a + * profile. The account's own `company_uuid` is left out: it is only where + * the account was first created, and stays after that organization lets go. + */ + public static function isSharedWithAnotherCompany(User $user, string $companyUuid): bool + { + $memberships = CompanyUser::where('user_uuid', $user->uuid)->pluck('company_uuid'); + + return $memberships->merge(static::profileCompanies($user))->filter()->contains(fn ($uuid) => $uuid !== $companyUuid); + } + + /** + * Whether the account belongs to no organization at all: no organization of + * its own, no membership and no live profile anywhere. + */ + public static function isUnowned(User $user): bool + { + return !$user->company_uuid + && !CompanyUser::where('user_uuid', $user->uuid)->exists() + && static::profileCompanies($user)->isEmpty(); + } + public static function isCompanyMember(User $user, ?string $companyUuid): bool { if (!$companyUuid) { diff --git a/server/tests/ApiDriverControllerContractsTest.php b/server/tests/ApiDriverControllerContractsTest.php index 27f52381e..39ead8ea5 100644 --- a/server/tests/ApiDriverControllerContractsTest.php +++ b/server/tests/ApiDriverControllerContractsTest.php @@ -41,13 +41,6 @@ class FleetOpsApiDriverControllerProbe extends DriverController public array $resolvedFiles = []; public string $sessionCompanyUuid = 'company-uuid'; - protected function companyFromRequest(Request $request): ?Company - { - $this->companyCalls[] = ['request', $request->input('company')]; - - return $this->missingCompany ? null : $this->company(); - } - protected function currentCompany(): ?Company { $this->companyCalls[] = ['current']; @@ -401,7 +394,7 @@ public function or(array $keys, mixed $default = null): mixed ])); expect($response)->toBe(['resource' => 'driver', 'driver' => $controller->driver]) - ->and($controller->companyCalls)->toBe([['request', 'company_public']]) + ->and($controller->companyCalls)->toBe([['current']]) // The login account is resolved by ProfileAccountManager in the driver's company ->and($controller->resolvedAccounts)->toBe(['company-uuid']) ->and($controller->createdUsers[0])->toMatchArray([ @@ -444,6 +437,28 @@ public function or(array $keys, mixed $default = null): mixed ->and($controller->createdUsers)->toBe([]); }); +test('api driver controller only creates drivers in the credential organization', function (string $company) { + // Naming the credential's own organization, by uuid or public id, is fine + $controller = new FleetOpsApiDriverControllerProbe(); + $controller->create(new CreateDriverRequest(['company' => $company, 'name' => 'Driver One'])); + + expect($controller->resolvedAccounts)->toBe(['company-uuid']) + ->and($controller->createdDrivers[0]['company_uuid'])->toBe('company-uuid'); +})->with(['company-uuid', 'company_public']); + +test('api driver controller refuses to create drivers in another organization', function (mixed $company) { + // Any other organization is refused exactly like one that does not exist, + // so the endpoint can't be used to probe or write into another tenant + $controller = new FleetOpsApiDriverControllerProbe(); + + expect($controller->create(new CreateDriverRequest(['company' => $company, 'name' => 'Driver One'])))->toBe([ + 'apiError' => 'Company not found.', + 'status' => 400, + ]) + ->and($controller->resolvedAccounts)->toBe([]) + ->and($controller->createdDrivers)->toBe([]); +})->with(['other-company-uuid', 'company_other', '', null]); + test('api driver controller updates drivers user details assignments location and photo', function () { fleetopsApiDriverContractsUsersTable(); diff --git a/server/tests/Feature/Http/Api/DriverControllerSessionHelpersTest.php b/server/tests/Feature/Http/Api/DriverControllerSessionHelpersTest.php index 5a7fdcae9..257c8918d 100644 --- a/server/tests/Feature/Http/Api/DriverControllerSessionHelpersTest.php +++ b/server/tests/Feature/Http/Api/DriverControllerSessionHelpersTest.php @@ -174,12 +174,8 @@ public function parameters() expect($probe->callHelper('sessionCompany'))->toBe('22222222-2222-4222-8222-222222222222') ->and($probe->callHelper('currentCompany')?->public_id)->toBe('company_one') - ->and(true)->toBeTrue(); - - // With no identifying inputs the core auth resolver finds nothing and - // violates its own non-nullable declaration — the helper body still - // executes, which is the covered contract here. - expect(fn () => $probe->callHelper('companyFromRequest', Request::create('/x', 'GET')))->toThrow(TypeError::class); + // A create request can no longer pick its organization from the input + ->and(method_exists($probe, 'companyFromRequest'))->toBeFalse(); }); test('user and driver persistence helpers write records', function () { diff --git a/server/tests/Feature/Http/Internal/CustomerControllerCredentialsTest.php b/server/tests/Feature/Http/Internal/CustomerControllerCredentialsTest.php index 82dc5f916..ef2979ccb 100644 --- a/server/tests/Feature/Http/Internal/CustomerControllerCredentialsTest.php +++ b/server/tests/Feature/Http/Internal/CustomerControllerCredentialsTest.php @@ -127,6 +127,8 @@ public function __call($method, $arguments) 'companies' => ['uuid', 'public_id', 'name', 'country'], // session_status reads the user's membership of the session company 'company_users' => ['uuid', 'company_uuid', 'user_uuid', 'status'], + // a login held by another organization's driver profile is shared + 'drivers' => ['uuid', 'company_uuid', 'user_uuid'], 'files' => ['uuid', 'public_id', 'company_uuid', 'subject_uuid', 'subject_type', 'name', 'path', 'disk', 'type', '_key'], ]; foreach ($tables as $table => $columns) { @@ -180,6 +182,25 @@ public function __call($method, $arguments) ->and($GLOBALS['fleetopsCustomerMailFake']->sent)->not->toBeEmpty(); }); +test('credential actions refuse a login another organization also holds', function () { + $connection = fleetopsCustomerCredentialsBoot(); + $connection->table('company_users')->insert(['uuid' => 'cu-elsewhere', 'company_uuid' => 'company-2', 'user_uuid' => 'user-1', 'status' => 'active']); + $connection->table('users')->where('uuid', 'user-1')->update(['password' => 'hashed:original']); + $controller = new CustomerController(); + $request = Request::create('/x', 'POST', ['customer' => 'contact_custcred1', 'password' => 'takeover', 'password_confirmation' => 'takeover']); + + foreach (['resetCredentials', 'sendCredentials', 'createPortalLogin', 'deactivatePortalLogin', 'reactivatePortalLogin'] as $action) { + $response = $controller->{$action}($request); + + expect($response->getStatusCode())->toBe(422) + ->and($response->getData(true))->toBe(['error' => 'This login is also used with another organization, so it can only be managed by its holder.']); + } + + expect($connection->table('users')->value('password'))->toBe('hashed:original') + ->and($connection->table('users')->value('status'))->toBe('active') + ->and($GLOBALS['fleetopsCustomerMailFake']->sent)->toBeEmpty(); +}); + test('customer resolution and payload helpers expose user details', function () { fleetopsCustomerCredentialsBoot(); $probe = new FleetOpsInternalCustomerProbe(); diff --git a/server/tests/Feature/Http/Internal/DriverControllerExistingUserAdoptionTest.php b/server/tests/Feature/Http/Internal/DriverControllerExistingUserAdoptionTest.php index 38aa20dd3..3cff3d1bf 100644 --- a/server/tests/Feature/Http/Internal/DriverControllerExistingUserAdoptionTest.php +++ b/server/tests/Feature/Http/Internal/DriverControllerExistingUserAdoptionTest.php @@ -408,13 +408,14 @@ function fleetopsDriverAdoptionRoles(SQLiteConnection $connection, string $userU ->and($connection->table('drivers')->value('company_uuid'))->toBe('company-1'); }); -test('a managed driver account of another organization is linked and joins the company', function () { +test('a managed driver account of the organization is linked', function () { + // e.g. a driver whose profile was deleted while the account stayed a member $connection = fleetopsDriverAdoptionBoot([]); - $connection->table('users')->insert(['uuid' => '11111111-1111-4111-8111-111111111112', 'company_uuid' => 'company-2', 'name' => 'Shared Driver', 'phone' => '+6591234568', 'slug' => 'shared', 'type' => 'driver']); - $connection->table('drivers')->insert(['uuid' => 'driver-elsewhere', 'company_uuid' => 'company-2', 'user_uuid' => '11111111-1111-4111-8111-111111111112']); + $connection->table('users')->insert(['uuid' => '11111111-1111-4111-8111-111111111112', 'company_uuid' => 'company-2', 'name' => 'Returning Driver', 'phone' => '+6591234568', 'slug' => 'returning', 'type' => 'driver']); + $connection->table('company_users')->insert(['uuid' => 'cu-returning', 'company_uuid' => 'company-1', 'user_uuid' => '11111111-1111-4111-8111-111111111112', 'status' => 'active']); $result = (new DriverController())->createRecord(fleetopsDriverAdoptionRequest([ - 'name' => 'Shared Driver', + 'name' => 'Returning Driver', 'phone' => '+6591234568', ])); @@ -422,7 +423,7 @@ function fleetopsDriverAdoptionRoles(SQLiteConnection $connection, string $userU ->and($result['driver']->resource->user_uuid)->toBe('11111111-1111-4111-8111-111111111112') ->and($connection->table('drivers')->where('company_uuid', 'company-1')->count())->toBe(1) ->and($connection->table('company_users')->where(['company_uuid' => 'company-1', 'user_uuid' => '11111111-1111-4111-8111-111111111112'])->count())->toBe(1) - ->and(fleetopsDriverAdoptionRoles($connection, '11111111-1111-4111-8111-111111111112'))->toBe(['Driver']) + ->and($connection->table('users')->count())->toBe(1) ->and($connection->table('invites')->count())->toBe(0); }); @@ -438,7 +439,11 @@ function fleetopsDriverAdoptionRoles(SQLiteConnection $connection, string $userU expect($result)->toBeInstanceOf(Illuminate\Http\JsonResponse::class) ->and($result->getStatusCode())->toBe(422) ->and($result->getData(true)['error'])->toBe($message) + // Nothing about the existing account is echoed back + ->and($result->getContent())->not->toContain('Existing') + ->and($result->getContent())->not->toContain('11111111-1111-4111-8111-111111111111') ->and($connection->table('drivers')->where('company_uuid', 'company-1')->where('user_uuid', '!=', '11111111-1111-4111-8111-111111111111')->count())->toBe(0) + ->and($connection->table('company_users')->where('company_uuid', 'company-1')->count())->toBe(0) ->and($connection->table('users')->count())->toBe(1); })->with([ 'staff of another organization' => [ @@ -447,6 +452,24 @@ function fleetopsDriverAdoptionRoles(SQLiteConnection $connection, string $userU ['email' => 'outsider@example.com'], 'This email is already in use by another account.', ], + 'a driver of another organization, by email' => [ + ['company_uuid' => 'company-2', 'email' => 'erin@example.com', 'type' => 'driver'], + ['uuid' => 'driver-elsewhere', 'company_uuid' => 'company-2', 'user_uuid' => '11111111-1111-4111-8111-111111111111'], + ['email' => 'Erin@Example.com', 'phone' => '+6599990000'], + 'This email is already in use by another account.', + ], + 'a driver of another organization, by phone' => [ + ['company_uuid' => 'company-2', 'email' => 'erin@example.com', 'phone' => '+6591234568', 'type' => 'driver'], + ['uuid' => 'driver-elsewhere', 'company_uuid' => 'company-2', 'user_uuid' => '11111111-1111-4111-8111-111111111111'], + ['email' => 'olive@example.com', 'phone' => '+65 9123 4568'], + 'This phone number is already in use by another account.', + ], + 'a customer of another organization does not reveal its type' => [ + ['company_uuid' => 'company-2', 'phone' => '+6591112222', 'type' => 'customer'], + null, + ['phone' => '+6591112222'], + 'This phone number is already in use by another account.', + ], 'a customer account' => [ ['company_uuid' => 'company-1', 'phone' => '+6591112222', 'type' => 'customer'], null, diff --git a/server/tests/Feature/Http/Internal/DriverControllerLoginManagementTest.php b/server/tests/Feature/Http/Internal/DriverControllerLoginManagementTest.php index 16bb6cd74..df8e93e0d 100644 --- a/server/tests/Feature/Http/Internal/DriverControllerLoginManagementTest.php +++ b/server/tests/Feature/Http/Internal/DriverControllerLoginManagementTest.php @@ -474,6 +474,32 @@ function fleetopsDriverLoginUser(SQLiteConnection $connection, string $uuid = '1 ->and(Illuminate\Support\Facades\Mail::getFacadeRoot()->sent)->toBe([]); }); +test('login management endpoints refuse a login another organization also holds', function () { + $connection = fleetopsDriverLoginFixture(); + // The managed driver account also drives for another organization + $connection->table('drivers')->insert(['uuid' => 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa9', 'public_id' => 'driver_elsewhere', 'company_uuid' => 'company-2', 'user_uuid' => '11111111-1111-4111-8111-111111111111']); + $before = fleetopsDriverLoginUser($connection); + $controller = new DriverController(); + $reset = fn (string $id) => $controller->resetCredentials(Request::create('/x', 'POST', ['password' => 'taken-over', 'password_confirmation' => 'taken-over']), $id); + + foreach ([ + fn (string $id) => $controller->sendCredentials($id), + $reset, + fn (string $id) => $controller->deactivateLogin($id), + fn (string $id) => $controller->reactivateLogin($id), + ] as $endpoint) { + $shared = $endpoint('driver_managed'); + + expect($shared->getStatusCode())->toBe(422) + ->and($shared->getData(true))->toBe(['error' => 'This login is also used with another organization, so it can only be managed by its holder.']); + } + + $after = fleetopsDriverLoginUser($connection); + expect($after->password)->toBe($before->password) + ->and($after->status)->toBe($before->status) + ->and(Illuminate\Support\Facades\Mail::getFacadeRoot()->sent)->toBe([]); +}); + test('reset credentials validates the password and optionally sends it', function () { $connection = fleetopsDriverLoginFixture(); $controller = new DriverController(); diff --git a/server/tests/ModelAccessorContractsTest.php b/server/tests/ModelAccessorContractsTest.php index ca9fd6052..2e034c2ff 100644 --- a/server/tests/ModelAccessorContractsTest.php +++ b/server/tests/ModelAccessorContractsTest.php @@ -159,7 +159,7 @@ function fleetopsModelAccessorsUseProfileAccountTables(): void $table->string('phone')->nullable(); $table->timestamp('deleted_at')->nullable(); }); - foreach (['drivers', 'contacts'] as $profileTable) { + foreach (['drivers', 'contacts', 'company_users'] as $profileTable) { $schema->create($profileTable, function ($table) { $table->string('uuid')->nullable(); $table->string('user_uuid')->nullable(); diff --git a/server/tests/Unit/Models/SmallModelSurfacesTest.php b/server/tests/Unit/Models/SmallModelSurfacesTest.php index 6aecd0683..a782c58a6 100644 --- a/server/tests/Unit/Models/SmallModelSurfacesTest.php +++ b/server/tests/Unit/Models/SmallModelSurfacesTest.php @@ -212,6 +212,14 @@ public function __call($method, $arguments) test('contact observer rejects saves whose email or phone belongs to another account', function () { $connection = fleetopsSmallModelBoot(); + // The sync checks whether another organization holds the account + $connection->getSchemaBuilder()->create('drivers', function ($blueprint) { + $blueprint->increments('id'); + $blueprint->string('uuid')->nullable(); + $blueprint->string('company_uuid')->nullable(); + $blueprint->string('user_uuid')->nullable(); + $blueprint->timestamp('deleted_at')->nullable(); + }); $connection->table('users')->insert([ ['uuid' => '11111111-1111-4111-8111-111111111111', 'company_uuid' => 'company-1', 'type' => 'contact', 'email' => 'free@example.test', 'phone' => '+6599999999'], ['uuid' => '22222222-2222-4222-8222-222222222222', 'company_uuid' => 'company-1', 'type' => 'user', 'email' => 'taken@example.test', 'phone' => '+6512345678'], diff --git a/server/tests/Unit/Support/ProfileAccountManagerTest.php b/server/tests/Unit/Support/ProfileAccountManagerTest.php index 079beea04..fa1eda3c9 100644 --- a/server/tests/Unit/Support/ProfileAccountManagerTest.php +++ b/server/tests/Unit/Support/ProfileAccountManagerTest.php @@ -312,11 +312,10 @@ function fleetopsProfileAccountUser(array $attributes): User ->and($staff['conflict'])->toBeNull() ->and($outsider['staff'])->toBeNull() ->and($outsider['conflict'])->toBe('This email is already in use by another account.') - ->and($customer['conflict'])->toBe('This phone number is already used by a driver.') - // A driver account of another organization is linked, not reported as staff - ->and($sameType['account']?->uuid)->toBe('driver-1') + ->and($customer['conflict'])->toBe('This phone number is already in use by another account.') + // A driver account of another organization is never linked, whatever its type ->and($sameType['staff'])->toBeNull() - ->and($sameType['conflict'])->toBeNull() + ->and($sameType['conflict'])->toBe('This phone number is already in use by another account.') // An existing profile can't swap its account for another one ->and($taken['conflict'])->toBe('This email is already in use by another account.') ->and($taken['staff'])->toBeNull() @@ -369,15 +368,16 @@ function fleetopsProfileAccountUser(array $attributes): User ->and($connection->table('company_users')->where('user_uuid', $contact->uuid)->count())->toBe(0); }); -test('resolve for profile links staff members and managed accounts of the same type', function () { +test('resolve for profile links staff members and managed accounts of the organization', function () { $connection = fleetopsProfileAccountBoot(); fleetopsProfileAccountRows($connection, 'users', [ ['uuid' => 'staff-1', 'company_uuid' => 'company-2', 'name' => 'Staff', 'email' => 'staff@example.com', 'type' => 'user'], ['uuid' => 'driver-1', 'company_uuid' => 'company-2', 'phone' => '+6590001111', 'type' => 'driver'], ['uuid' => 'driver-2', 'company_uuid' => null, 'phone' => '+6590003333', 'type' => 'driver'], ]); - // The staff member belongs to company-1 through a membership row + // The staff member and the driver account belong to company-1 through membership rows $connection->table('company_users')->insert(['uuid' => 'cu-staff', 'company_uuid' => 'company-1', 'user_uuid' => 'staff-1', 'status' => 'active']); + $connection->table('company_users')->insert(['uuid' => 'cu-driver', 'company_uuid' => 'company-1', 'user_uuid' => 'driver-1', 'status' => 'active']); $staff = ProfileAccountManager::resolveForProfile('company-1', 'driver', 'Other Name', 'STAFF@example.com', null); $driver = ProfileAccountManager::resolveForProfile('company-1', 'driver', 'Driver', null, '+6590001111'); @@ -434,6 +434,109 @@ function fleetopsProfileAccountUser(array $attributes): User ->and(new ProfileIdentityConflictException('Taken'))->getField()->toBe('email'); }); +test('resolve for profile never links an account of another organization nor tells whose it is', function () { + $connection = fleetopsProfileAccountBoot(); + fleetopsProfileAccountRows($connection, 'users', [ + ['uuid' => 'erin-1', 'company_uuid' => 'company-2', 'name' => 'Erin Driver', 'email' => 'erin@example.com', 'phone' => '+6590001111', 'type' => 'driver'], + ['uuid' => 'cust-1', 'company_uuid' => 'company-2', 'name' => 'Carla Customer', 'email' => 'carla@example.com', 'type' => 'customer'], + ['uuid' => 'member-1', 'company_uuid' => null, 'name' => 'Member Elsewhere', 'phone' => '+6590002222', 'type' => 'driver'], + ]); + $connection->table('drivers')->insert(['uuid' => 'd-erin', 'company_uuid' => 'company-2', 'user_uuid' => 'erin-1']); + // An account without its own organization but a membership elsewhere is held too + $connection->table('company_users')->insert(['uuid' => 'cu-member', 'company_uuid' => 'company-2', 'user_uuid' => 'member-1', 'status' => 'active']); + + $conflict = function (callable $callback): ?ProfileIdentityConflictException { + try { + $callback(); + } catch (ProfileIdentityConflictException $exception) { + return $exception; + } + + return null; + }; + + $byPhone = $conflict(fn () => ProfileAccountManager::resolveForProfile('company-1', 'driver', 'Olive Odoo', 'olive@example.com', '+65 9000 1111')); + $byEmail = $conflict(fn () => ProfileAccountManager::resolveForProfile('company-1', 'driver', 'Olive Odoo', 'ERIN@example.com', '+6599999999')); + $customer = $conflict(fn () => ProfileAccountManager::resolveForProfile('company-1', 'customer', 'Carla', 'carla@example.com', null)); + $wrongType = $conflict(fn () => ProfileAccountManager::resolveForProfile('company-1', 'contact', 'Carla', 'carla@example.com', null)); + $membership = $conflict(fn () => ProfileAccountManager::resolveForProfile('company-1', 'driver', 'M', null, '+6590002222')); + + expect($byPhone?->getMessage())->toBe('This phone number is already in use by another account.') + ->and($byPhone?->getField())->toBe('phone') + ->and($byEmail?->getMessage())->toBe('This email is already in use by another account.') + ->and($byEmail?->getField())->toBe('email') + ->and($customer?->getMessage())->toBe('This email is already in use by another account.') + // Not "already used by a customer": the type of another organization's account is not told + ->and($wrongType?->getMessage())->toBe('This email is already in use by another account.') + ->and($membership?->getMessage())->toBe('This phone number is already in use by another account.'); + + foreach ([$byPhone, $byEmail, $customer, $wrongType, $membership] as $exception) { + expect(json_encode([$exception->getMessage(), $exception->getErrors()])) + ->not->toContain('Erin') + ->not->toContain('erin@') + ->not->toContain('Carla') + ->not->toContain('company-2'); + } + + // Nothing was linked, created or added to company-1 + expect($connection->table('users')->count())->toBe(3) + ->and($connection->table('company_users')->where('company_uuid', 'company-1')->count())->toBe(0) + ->and($connection->table('users')->where('uuid', 'member-1')->value('company_uuid'))->toBeNull(); +}); + +test('accounts held by another organization are told apart from unowned ones', function () { + $connection = fleetopsProfileAccountBoot(); + $own = fleetopsProfileAccountUser(['uuid' => 'own-1', 'company_uuid' => 'company-1', 'type' => 'driver']); + $created = fleetopsProfileAccountUser(['uuid' => 'created-1', 'company_uuid' => 'company-2', 'type' => 'driver']); + $member = fleetopsProfileAccountUser(['uuid' => 'member-1', 'company_uuid' => 'company-1', 'type' => 'driver']); + $profile = fleetopsProfileAccountUser(['uuid' => 'profile-1', 'company_uuid' => 'company-1', 'type' => 'contact']); + $orphan = fleetopsProfileAccountUser(['uuid' => 'orphan-1', 'company_uuid' => null, 'type' => 'driver']); + // No organization of its own and no membership, but a live profile elsewhere (a self-signup customer) + $signup = fleetopsProfileAccountUser(['uuid' => 'signup-1', 'company_uuid' => null, 'email' => 'signup@example.com', 'type' => 'customer']); + $connection->table('company_users')->insert(['uuid' => 'cu-1', 'company_uuid' => 'company-2', 'user_uuid' => 'member-1', 'status' => 'active']); + $connection->table('contacts')->insert(['uuid' => 'c-1', 'company_uuid' => 'company-2', 'user_uuid' => 'profile-1', 'type' => 'contact']); + $connection->table('contacts')->insert(['uuid' => 'c-2', 'company_uuid' => 'company-2', 'user_uuid' => 'signup-1', 'type' => 'customer']); + + expect(ProfileAccountManager::isSharedWithAnotherCompany($own, 'company-1'))->toBeFalse() + // Being created by another organization that no longer holds it is not sharing + ->and(ProfileAccountManager::isSharedWithAnotherCompany($created, 'company-1'))->toBeFalse() + ->and(ProfileAccountManager::isSharedWithAnotherCompany($member, 'company-1'))->toBeTrue() + ->and(ProfileAccountManager::isSharedWithAnotherCompany($profile, 'company-1'))->toBeTrue() + ->and(ProfileAccountManager::isSharedWithAnotherCompany($orphan, 'company-1'))->toBeFalse() + ->and(ProfileAccountManager::isUnowned($orphan))->toBeTrue() + ->and(ProfileAccountManager::isUnowned($own))->toBeFalse() + ->and(ProfileAccountManager::isUnowned($member))->toBeFalse() + ->and(ProfileAccountManager::isUnowned($signup))->toBeFalse(); + + // So another organization can't adopt it by its email + $conflict = null; + try { + ProfileAccountManager::resolveForProfile('company-1', 'customer', 'X', $signup->email, null); + } catch (ProfileIdentityConflictException $exception) { + $conflict = $exception; + } + expect($conflict?->getMessage())->toBe('This email is already in use by another account.') + ->and($connection->table('company_users')->where('user_uuid', 'signup-1')->count())->toBe(0); +}); + +test('sync proxy fields only pushes the name to a managed account another organization also holds', function () { + $connection = fleetopsProfileAccountBoot(); + $shared = fleetopsProfileAccountUser(['uuid' => 'shared-1', 'company_uuid' => 'company-2', 'name' => 'Shared', 'email' => 'shared@example.com', 'phone' => '+6590000000', 'type' => 'driver']); + $connection->table('company_users')->insert([ + ['uuid' => 'cu-1', 'company_uuid' => 'company-1', 'user_uuid' => 'shared-1', 'status' => 'active'], + ['uuid' => 'cu-2', 'company_uuid' => 'company-2', 'user_uuid' => 'shared-1', 'status' => 'active'], + ]); + + expect(ProfileAccountManager::syncProxyFields($shared, ['name' => 'Renamed', 'email' => 'mine@example.com', 'phone' => '+6591111111', 'password' => 'taken-over'], 'company-1'))->toBeTrue() + ->and($connection->table('users')->where('uuid', 'shared-1')->first()) + ->name->toBe('Renamed') + ->email->toBe('shared@example.com') + ->phone->toBe('+6590000000') + ->password->not->toBe('hashed:taken-over') + // Nor can its own organization while another one also holds it + ->and(ProfileAccountManager::syncProxyFields($shared, ['email' => 'new@example.com'], 'company-2'))->toBeFalse(); +}); + test('sync proxy fields pushes normalized changes to managed accounts and only the name to staff', function () { $connection = fleetopsProfileAccountBoot(); $connection->table('users')->insert(['uuid' => 'other-1', 'email' => 'taken@example.com', 'phone' => '+6599990000', 'type' => 'user']);