Skip to content

fix(security): never link a profile to another organization's login account - #414

Merged
roncodes merged 2 commits into
release/v0.6.75from
fix/profile-account-cross-tenant-linking
Oct 23, 2026
Merged

roncodes merged 2 commits into
release/v0.6.75from
fix/profile-account-cross-tenant-linking

Conversation

@roncodes

Copy link
Copy Markdown
Member

Security: profiles could take over another organization's login accounts

Impact

Fixes a cross-tenant identity bug in driver, customer and contact creation.

ProfileAccountManager::resolveForProfile() looked up the login account by email or phone across the whole instance. If it found a managed account (a driver, contact or customer user) of the same type, it linked that account to the new profile and added it to the caller's organization, whichever organization the account belonged to. As a result, anyone holding an API key or a console session could:

  • Find out who another tenant's people are. The driver resource that comes back includes the linked account's name, email and phone. Creating a driver with a guessed phone number or email was enough.
  • Attach another tenant's driver, customer or contact login to their own organization. The account got a membership and role in the caller's organization.
  • Take the login over afterwards. The profile's update path pushes email, phone and password to the linked account (syncProxyFields). The console's reset-credentials, send-credentials and deactivate/reactivate actions also work on that account, for drivers and for customers. So the second organization could set a new password, or deactivate the login for the first.

A separate problem was in the same endpoint: POST /v1/drivers accepted a company parameter (a uuid or public id) and created the driver in that company, without checking that it belonged to the API credential.

Reproduction (local stack, 2026-10-22)

  1. "ERPNext Test Co" has a driver, "Erin Driver", with phone +12065550201.
  2. Using the "Odoo Test Co" API key: POST /v1/drivers {"name":"Olive Odoo","email":"<unique>","phone":"+12065550201"}.
  3. The response is a driver in Odoo Test Co, linked to ERPNext Test Co's user. It returns Erin's name, email and phone, and Erin's account now belongs to Odoo Test Co too.

Fix

Rule: an organization only reuses a login account that already belongs to it.

  • assertAccountCanHoldProfile() now requires company membership (users.company_uuid or a company_users row) for every account, not only for staff accounts. A managed account that belongs to no organization at all (no company_uuid and no memberships) can still be adopted, because no tenant owns it.
  • An account of another organization is refused with the generic This email/phone number is already in use by another account. error (422). The error never includes the other account's name, email, uuid or organization. It also doesn't include its type: the old already used by a customer message is now shown only for accounts in the caller's own organization.
  • Linking within the same organization still works: a team member, or a managed account that is a member of the organization but has no profile of that type yet (for example, a driver whose profile was deleted).
  • POST /v1/drivers always creates the driver in the credential's organization. A company value that names any other organization, or none, gets the same Company not found. error as an unknown company, so the endpoint can't be used to check which companies exist. companyFromRequest() is removed.
  • Accounts already shared by the old behaviour. These are accounts with a membership or profile in more than one organization.
    • syncProxyFields() takes the profile's organization. For a shared account it only pushes the name, the same as for a staff account.
    • The driver and customer credential actions (send or reset credentials, create, deactivate or reactivate login) return 422 with This login is also used with another organization, so it can only be managed by its holder.
    • The holder can still use forgot-password.
  • The same rule covers the console (internal v1) driver create, contact and customer create (Contact::createUserFromContact), and the profile-identity lookup hint, because they all go through ProfileAccountManager.

Intentionally unchanged: public customer self-signup (POST /v1/customers after request-creation-code) can still attach to an existing account. That flow proves the person owns the identity by sending a verification code to it.

Behaviour change: one person can no longer be added as a driver (or customer or contact) to a second organization just by entering their phone or email. That was the old intent: the test was named "a managed driver account of another organization is linked". Supporting it safely needs a flow where the account holder consents, such as an invite they accept. That would be a follow-up.

No core-api changes are needed.

Tests

  • ProfileAccountManagerTest
    • Refusal by phone and by email.
    • A cross-organization customer, and a cross-organization account of a different type that must not reveal its type.
    • An account held only through a membership.
    • Asserts that nothing is linked, created or attached, and that messages and errors contain no name, email or organization.
    • Same-organization reuse, and adoption of an unowned account.
    • isSharedWithAnotherCompany and isUnowned.
    • syncProxyFields pushes only the name to a shared account.
  • DriverControllerExistingUserAdoptionTest (internal create): new datasets for another organization's driver by email, by phone, and a customer. They assert the 422 body doesn't echo the existing account, and that no company_users row is created. The "another organization is linked" test is replaced by "a managed driver account of the organization is linked".
  • ApiDriverControllerContractsTest: the own organization is accepted by uuid and by public id. Another organization, an empty value and a null value are refused before anything is resolved or created.
  • DriverControllerLoginManagementTest and CustomerControllerCredentialsTest: every credential/login action refuses a shared login, and the password, status and mail are left untouched.

Every changed statement in server/src is covered (per-file clover check).

I ran the full suite one file at a time (507 files) against the branch. Three fixtures needed updating because the shared-account check queries company_users/drivers, or because companyFromRequest was removed: ModelAccessorContractsTest, SmallModelSurfacesTest and DriverControllerSessionHelpersTest. After that, every file passes.

…ccount

Creating a driver, customer or contact looked up the login account by email
or phone across the whole instance and linked any managed account of the same
type to the new profile, adding it to the caller's organization. Any API key
holder could reveal another tenant's driver (name, email, phone came back in
the response) by guessing a phone number, attach that login to their own
organization, and then change its email, phone or password through the
profile update and credential endpoints.

- ProfileAccountManager only reuses an account that already belongs to the
  requesting organization (or belongs to none). Another organization's
  account is refused with the generic "already in use by another account"
  error, which reveals neither its details nor its type.
- Accounts already shared with another organization only take the name from
  profile updates, and driver/customer credential and login actions refuse
  them.
- POST /v1/drivers no longer creates drivers in whatever company the
  `company` parameter names: anything but the credential's own organization
  is refused like an unknown company.
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (33032e5) to head (4161375).
⚠️ Report is 1 commits behind head on release/v0.6.75.

Additional details and impacted files
@@                 Coverage Diff                 @@
##             release/v0.6.75      #414   +/-   ##
===================================================
  Coverage             100.00%   100.00%           
- Complexity             13596     13607   +11     
===================================================
  Files                    634       634           
  Lines                  49342     49352   +10     
===================================================
+ Hits                   49342     49352   +10     
Flag Coverage Δ
backend 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

isUnowned() only looked at users.company_uuid and company_users, so an account
with neither but a live driver or contact profile in another organization (a
customer who signed up through a storefront, for example) could still be
adopted by any organization that knew its email or phone. It now also requires
that the account holds no profile anywhere.
@roncodes roncodes mentioned this pull request Oct 23, 2026
@roncodes
roncodes changed the base branch from main to release/v0.6.75 October 23, 2026 03:21
@roncodes

Copy link
Copy Markdown
Member Author

Review: the rule (only reuse an account that already belongs to the organization) is right, and the refusals don't leak anything. I pushed one hardening fix (4161375):

  • Gap fixed: isUnowned() only checked users.company_uuid and company_users. An account with neither, but with a live driver or contact profile in another organization, still counted as unowned. A storefront self-signup customer could look like this. So any organization that knew its email or phone could still adopt it and read its name, email and phone back. isUnowned() now also requires profileCompanies($user) to be empty. The new test checks that such an account is refused with the generic message and gets no membership.
  • I ran 42 test files that touch ProfileAccountManager and the driver, customer and contact controllers against the branch with a real vendor copy. All pass.

Notes, not blocking:

  • syncProxyFields() still pushes name to a shared login, so one organization can rename a person the other organization also sees. That's low impact, and it matches the staff-account behaviour, but it could be locked down too.
  • POST /v1/drivers with company: "" or null now returns Company not found.. Before, a null value also failed through companyFromRequest(), so I don't expect API clients to be affected.

Retargeted to release/v0.6.75 (#415).

@roncodes
roncodes merged commit fe0a691 into release/v0.6.75 Oct 23, 2026
11 checks passed
@roncodes
roncodes deleted the fix/profile-account-cross-tenant-linking branch October 23, 2026 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant