mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-07-15 12:58:04 +00:00
fix(security): scope owner-less email accounts to a mailbox match in route guards (#5238)
The HTTP email route guard `_assert_owns_account` and the explicit-account_id path in `_get_email_config` gated cross-tenant access with `if row.owner and row.owner != owner` -- which skips the check entirely when the account row is owner-less (owner NULL or ""). `email_accounts` is the one owner-scoped table left out of the legacy-owner migration backfill (core/database.py), so such rows persist on multi-user deploys: an account configured while auth was disabled, or an imported legacy row. Any authenticated user could then pass that account's id to read/send/update-credentials/delete another tenant's mailbox and read its decrypted IMAP/SMTP creds. Both sibling paths already enforce the intended contract -- the same-file `_owner_or_matching_legacy_account` fallback and the MCP `_account_visible_to_owner` gate (whose comment says it mirrors "the HTTP email route fallback") only expose an owner-less account when its own mailbox (imap_user / from_address) is the caller's. Factor that row-level predicate into `_account_visible_to_owner` and use it in both guards, so owner-less accounts are visible only on a mailbox match. Owned accounts, the legacy-claim path, and single-user mode (owner == "") are unchanged. Complements #5234 (which fixes the same class on the MCP tool layer); this is the HTTP route layer it does not touch.
This commit is contained in:
+25
-4
@@ -349,7 +349,7 @@ def _assert_owns_account(account_id: str, owner: str) -> None:
|
||||
row = db.query(_EA).filter(_EA.id == account_id).first()
|
||||
if row is None:
|
||||
raise HTTPException(404, "Account not found")
|
||||
if row.owner and row.owner != owner:
|
||||
if not _account_visible_to_owner(row, owner):
|
||||
# Treat as 404 (not 403) so we don't leak existence.
|
||||
raise HTTPException(404, "Account not found")
|
||||
finally:
|
||||
@@ -362,6 +362,26 @@ def _assert_owns_account(account_id: str, owner: str) -> None:
|
||||
logger.error(f"Account-owner check failed: {e}")
|
||||
raise HTTPException(503, "Account check failed")
|
||||
|
||||
|
||||
def _account_visible_to_owner(row, owner: str) -> bool:
|
||||
"""Whether an authenticated `owner` may act on this EmailAccount row.
|
||||
|
||||
Mirrors the SQL predicate in `_get_email_config`'s
|
||||
`_owner_or_matching_legacy_account`: a caller sees an account they own, or a
|
||||
legacy owner-less account (owner NULL/"") only when its own mailbox
|
||||
(`imap_user` / `from_address`) is the caller's. `email_accounts` is the one
|
||||
owner-scoped table deliberately left out of the legacy-owner migration
|
||||
backfill, so ownerless rows persist on multi-user deploys — making this the
|
||||
gate that keeps one tenant off another's imported mailbox and its decrypted
|
||||
IMAP/SMTP credentials."""
|
||||
row_owner = getattr(row, "owner", None) or ""
|
||||
if row_owner:
|
||||
return row_owner == owner
|
||||
return owner in {
|
||||
getattr(row, "imap_user", None) or "",
|
||||
getattr(row, "from_address", None) or "",
|
||||
}
|
||||
|
||||
def _q(name: str) -> str:
|
||||
"""Quote an IMAP mailbox name. Defensive: escapes `\\` and `"` and wraps
|
||||
in double quotes so user-supplied folder names with spaces or quotes can't
|
||||
@@ -903,12 +923,13 @@ def _get_email_config(account_id: str | None = None, owner: str = "") -> dict:
|
||||
try:
|
||||
if account_id:
|
||||
row = db.query(_EA).filter(_EA.id == account_id, _EA.enabled == True).first() # noqa: E712
|
||||
# If the resolved row belongs to a different owner, treat as
|
||||
# If the resolved row isn't visible to this owner, treat as
|
||||
# not-found rather than silently serving it. This is a defense
|
||||
# in depth — `require_owner` already calls `_assert_owns_account`
|
||||
# for query-param account_ids, but other callers (cookbook
|
||||
# rules, scheduled poller) may not.
|
||||
if row is not None and owner and row.owner and row.owner != owner:
|
||||
# rules, scheduled poller) may not. Ownerless legacy rows are
|
||||
# only visible on a mailbox match, same as the fallback below.
|
||||
if row is not None and owner and not _account_visible_to_owner(row, owner):
|
||||
row = None
|
||||
# Fallback path — restrict to this owner's accounts so we don't
|
||||
# leak another user's default mailbox to an unconfigured user.
|
||||
|
||||
Reference in New Issue
Block a user