refactor: update Clerk authentication integration and improve organization handling

backend/tests/test_auth_claims.py

@@ -0,0 +1,165 @@
+# ruff: noqa: SLF001
+
+from __future__ import annotations
+
+from dataclasses import dataclass, field
+from types import SimpleNamespace
+from typing import Any
+
+import pytest
+
+from app.core import auth
+from app.models.users import User
+
+
+@dataclass
+class _FakeSession:
+    added: list[Any] = field(default_factory=list)
+    committed: int = 0
+    refreshed: list[Any] = field(default_factory=list)
+
+    def add(self, value: Any) -> None:
+        self.added.append(value)
+
+    async def commit(self) -> None:
+        self.committed += 1
+
+    async def refresh(self, value: Any) -> None:
+        self.refreshed.append(value)
+
+
+def test_extract_claim_email_prefers_direct_email() -> None:
+    claims: dict[str, object] = {
+        "email": " User@Example.com ",
+        "primary_email_address": "ignored@example.com",
+    }
+    assert auth._extract_claim_email(claims) == "user@example.com"
+
+
+def test_extract_claim_email_from_primary_id() -> None:
+    claims: dict[str, object] = {
+        "primary_email_address_id": "id-2",
+        "email_addresses": [
+            {"id": "id-1", "email_address": "first@example.com"},
+            {"id": "id-2", "email_address": "chosen@example.com"},
+        ],
+    }
+    assert auth._extract_claim_email(claims) == "chosen@example.com"
+
+
+def test_extract_claim_email_falls_back_to_first_address() -> None:
+    claims: dict[str, object] = {
+        "email_addresses": [
+            {"id": "id-1", "email_address": "first@example.com"},
+            {"id": "id-2", "email_address": "second@example.com"},
+        ],
+    }
+    assert auth._extract_claim_email(claims) == "first@example.com"
+
+
+def test_extract_claim_name_from_parts() -> None:
+    claims: dict[str, object] = {
+        "given_name": "Alex",
+        "family_name": "Morgan",
+    }
+    assert auth._extract_claim_name(claims) == "Alex Morgan"
+
+
+def test_extract_clerk_profile_prefers_primary_email() -> None:
+    profile = SimpleNamespace(
+        primary_email_address_id="e2",
+        email_addresses=[
+            SimpleNamespace(id="e1", email_address="first@example.com"),
+            SimpleNamespace(id="e2", email_address="primary@example.com"),
+        ],
+        first_name="Asha",
+        last_name="Rao",
+    )
+    email, name = auth._extract_clerk_profile(profile)
+    assert email == "primary@example.com"
+    assert name == "Asha"
+
+
+@pytest.mark.asyncio
+async def test_get_or_sync_user_updates_email_and_name(monkeypatch: pytest.MonkeyPatch) -> None:
+    existing = User(clerk_user_id="user_123", email="old@example.com", name=None)
+
+    async def _fake_get_or_create(*_args: Any, **_kwargs: Any) -> tuple[User, bool]:
+        return existing, False
+
+    async def _fake_fetch(_clerk_user_id: str) -> tuple[str | None, str | None]:
+        return "new@example.com", "New Name"
+
+    monkeypatch.setattr(auth.crud, "get_or_create", _fake_get_or_create)
+    monkeypatch.setattr(auth, "_fetch_clerk_profile", _fake_fetch)
+
+    session = _FakeSession()
+    out = await auth._get_or_sync_user(
+        session,  # type: ignore[arg-type]
+        clerk_user_id="user_123",
+        claims={},
+    )
+
+    assert out is existing
+    assert existing.email == "new@example.com"
+    assert existing.name == "New Name"
+    assert session.committed == 1
+    assert session.refreshed == [existing]
+
+
+@pytest.mark.asyncio
+async def test_get_or_sync_user_uses_clerk_profile_when_claims_are_minimal(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    existing = User(clerk_user_id="user_123", email=None, name=None)
+
+    async def _fake_get_or_create(*_args: Any, **_kwargs: Any) -> tuple[User, bool]:
+        return existing, False
+
+    async def _fake_fetch(_clerk_user_id: str) -> tuple[str | None, str | None]:
+        return "from-clerk@example.com", "From Clerk"
+
+    monkeypatch.setattr(auth.crud, "get_or_create", _fake_get_or_create)
+    monkeypatch.setattr(auth, "_fetch_clerk_profile", _fake_fetch)
+
+    session = _FakeSession()
+    out = await auth._get_or_sync_user(
+        session,  # type: ignore[arg-type]
+        clerk_user_id="user_123",
+        claims={"sub": "user_123"},
+    )
+
+    assert out is existing
+    assert existing.email == "from-clerk@example.com"
+    assert existing.name == "From Clerk"
+    assert session.committed == 1
+    assert session.refreshed == [existing]
+
+
+@pytest.mark.asyncio
+async def test_get_or_sync_user_skips_commit_when_unchanged(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    existing = User(clerk_user_id="user_123", email="same@example.com", name="Name")
+
+    async def _fake_get_or_create(*_args: Any, **_kwargs: Any) -> tuple[User, bool]:
+        return existing, False
+
+    async def _fake_fetch(_clerk_user_id: str) -> tuple[str | None, str | None]:
+        return "same@example.com", "Different Name"
+
+    monkeypatch.setattr(auth.crud, "get_or_create", _fake_get_or_create)
+    monkeypatch.setattr(auth, "_fetch_clerk_profile", _fake_fetch)
+
+    session = _FakeSession()
+    out = await auth._get_or_sync_user(
+        session,  # type: ignore[arg-type]
+        clerk_user_id="user_123",
+        claims={},
+    )
+
+    assert out is existing
+    assert existing.email == "same@example.com"
+    assert existing.name == "Name"
+    assert session.committed == 0
+    assert session.refreshed == []

backend/tests/test_organizations_service.py

@@ -43,12 +43,14 @@ class _FakeExecResult:
 class _FakeSession:
     exec_results: list[Any]
     get_results: dict[tuple[type[Any], Any], Any] = field(default_factory=dict)
+    commit_side_effects: list[Exception] = field(default_factory=list)
 
     added: list[Any] = field(default_factory=list)
     added_all: list[list[Any]] = field(default_factory=list)
     executed: list[Any] = field(default_factory=list)
 
     committed: int = 0
+    rolled_back: int = 0
     flushed: int = 0
     refreshed: list[Any] = field(default_factory=list)
 
@@ -71,8 +73,14 @@ class _FakeSession:
         self.added_all.append(values)
 
     async def commit(self) -> None:
+        if self.commit_side_effects:
+            effect = self.commit_side_effects.pop(0)
+            raise effect
         self.committed += 1
 
+    async def rollback(self) -> None:
+        self.rolled_back += 1
+
     async def flush(self) -> None:
         self.flushed += 1
 
@@ -132,7 +140,7 @@ async def test_ensure_member_for_user_returns_existing_membership(
 
     monkeypatch.setattr(organizations, "get_active_membership", _fake_get_active)
 
-    session = _FakeSession(exec_results=[])
+    session = _FakeSession(exec_results=[_FakeExecResult()])
     out = await organizations.ensure_member_for_user(session, user)
     assert out is existing
 
@@ -156,6 +164,9 @@ async def test_ensure_member_for_user_accepts_pending_invite(
     async def _fake_find(_session: Any, _email: str) -> OrganizationInvite:
         return invite
 
+    async def _fake_get_first(_session: Any, _user_id: Any) -> None:
+        return None
+
     accepted = OrganizationMember(
         organization_id=org_id,
         user_id=user.id,
@@ -172,39 +183,73 @@ async def test_ensure_member_for_user_accepts_pending_invite(
         return accepted
 
     monkeypatch.setattr(organizations, "get_active_membership", _fake_get_active)
+    monkeypatch.setattr(organizations, "get_first_membership", _fake_get_first)
     monkeypatch.setattr(organizations, "_find_pending_invite", _fake_find)
     monkeypatch.setattr(organizations, "accept_invite", _fake_accept)
 
-    session = _FakeSession(exec_results=[])
+    session = _FakeSession(exec_results=[_FakeExecResult()])
     out = await organizations.ensure_member_for_user(session, user)
     assert out is accepted
 
 
 @pytest.mark.asyncio
-async def test_ensure_member_for_user_creates_default_org_and_first_owner(
+async def test_ensure_member_for_user_creates_personal_org_and_owner(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
     user = User(clerk_user_id="u1", email=None)
-    org = Organization(id=uuid4(), name=organizations.DEFAULT_ORG_NAME)
 
     async def _fake_get_active(_session: Any, _user: User) -> None:
         return None
 
-    async def _fake_ensure_default(_session: Any) -> Organization:
-        return org
+    async def _fake_get_first(_session: Any, _user_id: Any) -> None:
+        return None
 
     monkeypatch.setattr(organizations, "get_active_membership", _fake_get_active)
-    monkeypatch.setattr(organizations, "ensure_default_org", _fake_ensure_default)
+    monkeypatch.setattr(organizations, "get_first_membership", _fake_get_first)
 
-    # member_count query returns 0 -> first member becomes owner
-    session = _FakeSession(exec_results=[_FakeExecResult(one_value=0)])
+    session = _FakeSession(exec_results=[_FakeExecResult()])
 
     out = await organizations.ensure_member_for_user(session, user)
-    assert out.organization_id == org.id
     assert out.user_id == user.id
     assert out.role == "owner"
     assert out.all_boards_read is True
     assert out.all_boards_write is True
+    assert out.organization_id == user.active_organization_id
+    assert any(
+        isinstance(item, Organization) and item.id == out.organization_id
+        for item in session.added
+    )
+    assert session.committed == 1
+
+
+@pytest.mark.asyncio
+async def test_ensure_member_for_user_reuses_existing_membership_after_lock(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    user = User(clerk_user_id="u1")
+    org = Organization(id=uuid4(), name=organizations.DEFAULT_ORG_NAME)
+    existing = OrganizationMember(
+        organization_id=org.id,
+        user_id=user.id,
+        role="member",
+    )
+
+    async def _fake_get_active(_session: Any, _user: User) -> None:
+        return None
+
+    async def _fake_get_first(
+        _session: Any,
+        _user_id: Any,
+    ) -> OrganizationMember | None:
+        return existing
+
+    monkeypatch.setattr(organizations, "get_active_membership", _fake_get_active)
+    monkeypatch.setattr(organizations, "get_first_membership", _fake_get_first)
+
+    session = _FakeSession(exec_results=[_FakeExecResult()])
+
+    out = await organizations.ensure_member_for_user(session, user)
+    assert out is existing
     assert user.active_organization_id == org.id
     assert session.committed == 1