github/deer-flow
1
0
Fork 0
mirror of https://github.com/bytedance/deer-flow.git synced 2026-07-26 07:57:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(auth): resolve email accounts case-insensitively (#4101)

Browse Source 
* fix(auth): handle email addresses case-insensitively

Normalize new and changed addresses, use case-insensitive lookup, and keep legacy case-variant rows stable during unrelated updates.

* fix(auth): reject legacy case-variant registrations

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Daoyuan Li 2026-07-25 18:52:16 -07:00 committed by GitHub
parent 5d073991c2
commit b5cc3a81c3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 376 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
backend/app/gateway/auth/repositories/base.py
View File

@ -30,7 +30,10 @@ class UserRepository(ABC):
user: User object to create
Returns:
Created User with ID assigned
Created User with ID assigned. ``email`` is the canonical
(lowercase) stored form, which may differ in case from what was
passed in -- implementations mutate the input ``user`` in place
to reflect this rather than returning a fresh object.
Raises:
ValueError: If email already exists
@ -69,7 +72,9 @@ class UserRepository(ABC):
user: User object with updated fields
Returns:
Updated User
Updated User. ``email`` is the canonical (lowercase) stored
form -- implementations mutate the input ``user`` in place to
reflect this rather than returning a fresh object.
Raises:
UserNotFoundError: If no row exists for ``user.id``. This is

62
backend/app/gateway/auth/repositories/sqlite.py
View File

@ -24,6 +24,25 @@ from app.gateway.auth.repositories.base import UserNotFoundError, UserRepository
from deerflow.persistence.user.model import UserRow
def _normalize_email(email: str) -> str:
"""Canonicalise an email address for storage and lookup.
An email identifies exactly one account regardless of the case the client
sends. The two write paths would otherwise disagree: local registration
normalises through ``EmailStr``, which lowercases only the *domain* and
keeps the local-part case (``Victim@X.COM`` -> ``Victim@x.com``), while OIDC
provisioning lowercases the whole address (``-> victim@x.com``). Combined
with the previous case-sensitive lookup, ``Victim@x.com`` and
``victim@x.com`` resolved to two separate rows, defeating the invariant
that a local account blocks an SSO login on the same email.
Canonicalising to lowercase at every write site and matching
case-insensitively on read closes that gap for new accounts while letting
existing mixed-case rows keep resolving, without a destructive bulk rewrite.
"""
return email.lower()
class SQLiteUserRepository(UserRepository):
"""Async user repository backed by the shared SQLAlchemy engine."""
@ -65,9 +84,20 @@ class SQLiteUserRepository(UserRepository):
# ── CRUD ──────────────────────────────────────────────────────────
async def create_user(self, user: User) -> User:
"""Insert a new user. Raises ``ValueError`` on duplicate email."""
"""Insert a new user. Raises ``ValueError`` on duplicate email.
The email is canonicalised to lowercase before insert so the existing
unique constraint enforces case-insensitive uniqueness for new rows and
the returned ``User`` reflects the stored form.
"""
user.email = _normalize_email(user.email)
row = self._user_to_row(user)
async with self._sf() as session:
# The unique constraint is case-sensitive, so it cannot catch a
# canonical address colliding with a mixed-case legacy row.
existing = select(UserRow.id).where(func.lower(UserRow.email) == user.email).limit(1)
if await session.scalar(existing) is not None:
raise ValueError(f"Email already registered: {user.email}")
session.add(row)
try:
await session.commit()
@ -82,10 +112,17 @@ class SQLiteUserRepository(UserRepository):
return self._row_to_user(row) if row is not None else None
async def get_user_by_email(self, email: str) -> User | None:
stmt = select(UserRow).where(UserRow.email == email)
# Case-insensitive match: an account is keyed by its email regardless of
# the case the caller supplies (see ``_normalize_email``). ``.first()``
# with a deterministic ``created_at`` ordering resolves to the oldest
# account instead of raising if a pre-fix database already holds two
# rows differing only in case, so the fix never turns a legacy duplicate
# pair into a 500. ``id`` is a secondary tiebreaker so the choice stays
# deterministic even if two legacy rows share the same ``created_at``.
stmt = select(UserRow).where(func.lower(UserRow.email) == _normalize_email(email)).order_by(UserRow.created_at, UserRow.id).limit(1)
async with self._sf() as session:
result = await session.execute(stmt)
row = result.scalar_one_or_none()
row = result.scalars().first()
return self._row_to_user(row) if row is not None else None
async def update_user(self, user: User) -> User:
@ -99,7 +136,24 @@ class SQLiteUserRepository(UserRepository):
# success would let the caller log "password reset" for
# a row that no longer exists.
raise UserNotFoundError(f"User {user.id} no longer exists")
row.email = user.email
# Canonicalise the email only when it actually changes, comparing
# case-insensitively against the stored value, then mirror the
# persisted value back onto the returned object. Re-lowercasing an
# *unchanged* legacy mixed-case email — e.g. a password-only update
# on a pre-fix ``Victim@x.com`` row while a canonical ``victim@x.com``
# row also exists — would rewrite it onto the other row's unique
# email and raise IntegrityError, surfacing as a 500 on the
# change-password / reset-admin paths that do not catch it. Guarding
# against the *raw* stored value (``canonical != row.email``) is not
# enough: the mixed-case row's canonical form still differs from its
# own stored casing, so it would rewrite and collide anyway. A genuine
# change still normalises, so the unique constraint keeps enforcing
# case-insensitive uniqueness for updated rows; get_user_by_email
# already resolves legacy mixed-case rows case-insensitively on read.
canonical_email = _normalize_email(user.email)
if canonical_email != _normalize_email(row.email):
row.email = canonical_email
user.email = row.email
row.password_hash = user.password_hash
row.system_role = user.system_role
row.oauth_provider = user.oauth_provider

311
backend/tests/test_auth.py
View File

@ -478,6 +478,317 @@ def test_update_user_raises_when_row_concurrently_deleted(tmp_path):
asyncio.run(_run())
# ── Email case-insensitivity (account collision invariant) ──────────────────
#
# Regression coverage for the case-collision gap: local registration normalises
# email through ``EmailStr`` (lowercases only the domain) while OIDC lowercases
# the whole address, and the repo lookup used to be case-sensitive, so
# ``Victim@x.com`` and ``victim@x.com`` became two separate accounts — defeating
# the invariant that a local account blocks an SSO login on the same email
# (flagged on PR #3506, fixed there only OIDC-side). The repo now canonicalises
# to lowercase on write and matches case-insensitively on read.
def test_email_lookup_is_case_insensitive(tmp_path):
"""A user registered with mixed case resolves for any-case lookup."""
import asyncio
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
async def _run() -> None:
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
repo = SQLiteUserRepository(get_session_factory())
created = await repo.create_user(User(email="Victim@x.com", password_hash="h", system_role="user"))
# Stored canonical (lowercase) and reflected back on the returned object.
assert created.email == "victim@x.com"
for variant in ("victim@x.com", "VICTIM@X.COM", "Victim@x.com"):
found = await repo.get_user_by_email(variant)
assert found is not None, f"lookup missed {variant!r}"
assert str(found.id) == str(created.id)
finally:
await close_engine()
asyncio.run(_run())
def test_create_user_rejects_email_differing_only_in_case(tmp_path):
"""The second case-variant registration collides on the canonical email."""
import asyncio
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
async def _run() -> None:
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
repo = SQLiteUserRepository(get_session_factory())
await repo.create_user(User(email="Victim@x.com", password_hash="h", system_role="user"))
with pytest.raises(ValueError):
await repo.create_user(User(email="victim@x.com", password_hash="h", system_role="user"))
assert await repo.count_users() == 1
finally:
await close_engine()
asyncio.run(_run())
def test_create_user_rejects_legacy_mixed_case_email(tmp_path):
"""Registration must not duplicate a mixed-case row created before normalization."""
import asyncio
from uuid import uuid4
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
async def _run() -> None:
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
from deerflow.persistence.user.model import UserRow
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
sf = get_session_factory()
repo = SQLiteUserRepository(sf)
async with sf() as session:
session.add(
UserRow(
id=str(uuid4()),
email="Victim@x.com",
password_hash="h",
system_role="user",
needs_setup=False,
token_version=0,
)
)
await session.commit()
with pytest.raises(ValueError, match="Email already registered"):
await repo.create_user(User(email="victim@x.com", password_hash="h", system_role="user"))
assert await repo.count_users() == 1
finally:
await close_engine()
asyncio.run(_run())
def test_update_user_normalizes_email(tmp_path):
"""Changing an email through update_user stores the canonical lowercase form."""
import asyncio
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
async def _run() -> None:
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
repo = SQLiteUserRepository(get_session_factory())
user = await repo.create_user(User(email="user@x.com", password_hash="h", system_role="user"))
user.email = "New@Mixed.COM"
await repo.update_user(user)
refetched = await repo.get_user_by_id(str(user.id))
assert refetched is not None
assert refetched.email == "new@mixed.com"
assert await repo.get_user_by_email("NEW@MIXED.com") is not None
finally:
await close_engine()
asyncio.run(_run())
def test_distinct_emails_remain_distinct(tmp_path):
"""Case-folding must not collapse genuinely different addresses."""
import asyncio
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
async def _run() -> None:
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
repo = SQLiteUserRepository(get_session_factory())
alice = await repo.create_user(User(email="alice@x.com", password_hash="h", system_role="user"))
bob = await repo.create_user(User(email="bob@x.com", password_hash="h", system_role="user"))
assert await repo.count_users() == 2
fa = await repo.get_user_by_email("Alice@x.com")
fb = await repo.get_user_by_email("BOB@x.com")
assert fa is not None and str(fa.id) == str(alice.id)
assert fb is not None and str(fb.id) == str(bob.id)
assert str(fa.id) != str(fb.id)
finally:
await close_engine()
asyncio.run(_run())
def test_legacy_mixed_case_duplicate_rows_resolve_without_error(tmp_path):
"""A pre-fix DB with two case-variant rows resolves to the oldest, never 500s.
Migration-safety: existing installations may already hold ``Victim@x.com``
and ``victim@x.com`` as separate rows. The case-insensitive lookup must not
raise ``MultipleResultsFound``; it deterministically returns the oldest
(most-established) account.
"""
import asyncio
from datetime import UTC, datetime, timedelta
from uuid import uuid4
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
async def _run() -> None:
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
from deerflow.persistence.user.model import UserRow
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
sf = get_session_factory()
repo = SQLiteUserRepository(sf)
older = datetime.now(UTC) - timedelta(days=5)
newer = datetime.now(UTC)
# Insert raw rows (bypassing create_user's normalisation) to mimic
# data written before this fix.
async with sf() as session:
session.add(UserRow(id=str(uuid4()), email="Victim@x.com", password_hash="h", system_role="user", created_at=older, needs_setup=False, token_version=0))
session.add(UserRow(id=str(uuid4()), email="victim@x.com", password_hash="h", system_role="user", created_at=newer, needs_setup=False, token_version=0))
await session.commit()
found = await repo.get_user_by_email("VICTIM@X.COM")
assert found is not None
assert found.email == "Victim@x.com" # oldest wins, deterministically
finally:
await close_engine()
asyncio.run(_run())
def test_update_user_on_legacy_mixed_case_row_does_not_collide(tmp_path):
"""A password-only update on a legacy mixed-case row must not 500.
Migration-safety, write side. A pre-fix DB may already hold two rows
differing only in case (``Victim@x.com`` + ``victim@x.com``). A password
change or admin reset reloads the row and calls ``update_user`` with the
email unchanged. ``update_user`` must not opportunistically re-lowercase the
mixed-case email, because that collides with the already-canonical row's
unique email and raises ``IntegrityError`` — which surfaces as a 500 on the
change-password / reset-admin paths that don't catch it. The read path was
hardened for this legacy state; the write path must match, so only a genuine
email change (differing case-insensitively from the stored value) rewrites
the column.
"""
import asyncio
from datetime import UTC, datetime, timedelta
from uuid import uuid4
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
async def _run() -> None:
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
from deerflow.persistence.user.model import UserRow
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
sf = get_session_factory()
repo = SQLiteUserRepository(sf)
mixed_id = str(uuid4())
canonical_id = str(uuid4())
older = datetime.now(UTC) - timedelta(days=5)
newer = datetime.now(UTC)
# Raw rows written before the fix (the mixed-case row is the oldest).
async with sf() as session:
session.add(UserRow(id=mixed_id, email="Victim@x.com", password_hash="old-hash", system_role="user", created_at=older, needs_setup=False, token_version=0))
session.add(UserRow(id=canonical_id, email="victim@x.com", password_hash="canonical-hash", system_role="user", created_at=newer, needs_setup=False, token_version=0))
await session.commit()
# Simulate a password change on the mixed-case row: reload it, keep
# the email as-stored, set a new hash + bump the token version.
mixed = await repo.get_user_by_id(mixed_id)
assert mixed is not None
assert mixed.email == "Victim@x.com"
mixed.password_hash = "new-hash-after-change"
mixed.token_version += 1
# Must not raise IntegrityError even though lowercasing the email
# would collide with the canonical row's unique email.
await repo.update_user(mixed)
# The mixed-case row kept its stored casing and took the new password.
refetched = await repo.get_user_by_id(mixed_id)
assert refetched is not None
assert refetched.email == "Victim@x.com"
assert refetched.password_hash == "new-hash-after-change"
assert refetched.token_version == 1
# The canonical row is untouched, and no row was lost or merged.
canonical = await repo.get_user_by_id(canonical_id)
assert canonical is not None
assert canonical.email == "victim@x.com"
assert canonical.password_hash == "canonical-hash"
assert await repo.count_users() == 2
# Case-insensitive lookup still resolves the mixed-case row (oldest wins).
found = await repo.get_user_by_email("VICTIM@X.COM")
assert found is not None
assert str(found.id) == mixed_id
finally:
await close_engine()
asyncio.run(_run())
def test_oidc_login_blocked_by_existing_local_account_across_case(tmp_path):
"""End-to-end invariant: an SSO login cannot create a duplicate of a local
account whose email differs only in case.
Uses the real repository + provider + provisioning (no mocks), so it covers
the cross-path gap the mock-based OIDC tests could not: local registration
keeps the local-part case (``Victim@x.com``) while OIDC lowercases the whole
address (``victim@x.com``).
"""
import asyncio
from app.gateway.auth.local_provider import LocalAuthProvider
from app.gateway.auth.repositories.sqlite import SQLiteUserRepository
from app.gateway.auth.user_provisioning import get_or_provision_oidc_user
from deerflow.config.auth_config import OIDCProviderConfig
async def _run() -> None:
from fastapi import HTTPException
from app.gateway.auth.oidc import OIDCIdentity
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
url = f"sqlite+aiosqlite:///{tmp_path}/scratch.db"
await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path))
try:
provider = LocalAuthProvider(SQLiteUserRepository(get_session_factory()))
await provider.create_user(email="Victim@x.com", password="pw-abc-123!", system_role="user")
cfg = OIDCProviderConfig(display_name="Test SSO", issuer="https://issuer.example.com", client_id="deer-flow", auto_create_users=True)
identity = OIDCIdentity(provider="keycloak", subject="sub-1", email="Victim@x.com", email_verified=True, name="Victim", claims={})
with pytest.raises(HTTPException) as exc_info:
await get_or_provision_oidc_user(provider_id="keycloak", provider_config=cfg, identity=identity, local_provider=provider)
assert exc_info.value.status_code == 409
# No duplicate auto-created — the local account still owns the email.
assert await provider.count_users() == 1
finally:
await close_engine()
asyncio.run(_run())
# ── Token Versioning ───────────────────────────────────────────────────────