Tech-debt batch 20: follow-ups — admin refusal reasons, OAuth refusal page, eager outbox serialization, refresh-family max age, token purge on user delete, bearer /me (#653–#658)

CHANGELOG.md

@@ -22,6 +22,19 @@ All notable changes to this project are documented in this file. The format is b
   across pages, and recover concurrent duplicate share grants without a 500 (#640).
 - Share UTC timestamp helpers across sharing, tenants, background tasks, API tokens
   and users (#640).
+- Background tasks: serialize eager outbox dispatches so chained deliveries do not
+  overlap on shared in-memory SQLite connections or trip another handler's commit
+  guard during tenant teardown (#655).
+- Api tokens: durably purge a deleted user's personal tokens across all tenants
+  on `UserDeleted`, preserving service keys and other users' tokens (#657).
+- Users: cap refresh-token families at a DB-configurable 90 days, refusing refreshes
+  and revoking/pruning aged families while preserving reuse evidence for live families (#656).
+- Users admin actions show translated server refusal details instead of generic errors;
+  unknown details retain the existing fallback (#653).
+- Users: GET and PATCH `/api/users/me` now accept bearer access tokens from
+  `/api/users/auth/token` alongside browser sessions (#658).
+- Users: redirect refused OAuth/OIDC browser sign-ins to login with translated English
+  and German messages; preserve JSON errors for API callers (#654).
 - Users: publish deletion, disabling and new role assignments transactionally through
   the durable outbox (inline cleanup without background tasks). Deletion purges tenant
   memberships, shares to the user and direct permission grants; tenants left without

docs/modules/users.md

@@ -196,6 +196,7 @@ Everything else is DB-backed (initial values are pydantic defaults; edit under U
 | `verification_token_lifetime_seconds` | `604_800` (7 days) |
 | `bearer_token_lifetime_seconds` | `900` (15 min) |
 | `refresh_token_lifetime_seconds` | `2_592_000` (30 days) |
+| `refresh_token_family_max_age_days` | `90` (≥ 1) — absolute age from the first token; requires a new login after expiry |
 | `cookie_name` | `"sm_auth"` |
 | `cookie_max_age_seconds` | `1_209_600` (14 days) |
 | `cookie_secure` | `True` (flipped to `False` in dev at startup) |

modules/api_tokens/api_tokens/module.py

@@ -38,10 +38,13 @@ class ApiTokensModule(ModuleBase):
 
     def register_event_handlers(self, bus: EventBus, app: FastAPI | None = None) -> None:
         from simple_module_core.tenant_events import TenantDeleted
+        from simple_module_core.user_events import UserDeleted
 
         from api_tokens.tenant_cleanup import purge_tenant_data
+        from api_tokens.user_cleanup import purge_user_tokens
 
         bus.subscribe_durable(TenantDeleted, purge_tenant_data)
+        bus.subscribe_durable(UserDeleted, purge_user_tokens)
 
     def register_settings(self, app: FastAPI) -> None:
         from api_tokens.services import ApiTokensServices

modules/api_tokens/api_tokens/user_cleanup.py

@@ -0,0 +1,19 @@
+"""Durable cleanup of a deleted user's personal tokens."""
+
+from simple_module_core.durable_events import DurableContext
+from simple_module_core.user_events import UserDeleted
+from simple_module_db.tenancy import all_tenants
+from sqlmodel import col, delete
+
+from api_tokens.constants import TokenKind
+from api_tokens.models import ApiToken
+
+
+async def purge_user_tokens(event: UserDeleted, ctx: DurableContext) -> None:
+    with all_tenants():
+        await ctx.db.execute(
+            delete(ApiToken).where(
+                col(ApiToken.kind) == TokenKind.PERSONAL,
+                col(ApiToken.owner_user_id) == event.user_id,
+            )
+        )

modules/api_tokens/tests/test_api_tokens_user_cleanup.py

@@ -0,0 +1,61 @@
+"""User deletion purges personal tokens across tenants transactionally (#657)."""
+
+import uuid
+from datetime import UTC, datetime, timedelta
+
+import pytest
+from api_tokens.constants import TokenKind
+from api_tokens.models import ApiToken
+from api_tokens.module import ApiTokensModule
+from simple_module_core.events import EventBus
+from simple_module_core.user_events import UserDeleted
+from simple_module_db import all_tenants, tenant_context
+from simple_module_db.transaction import finalize_session, rollback_session
+from sqlmodel import select
+
+
+@pytest.mark.parametrize("rollback", [False, True])
+async def test_api_tokens_user_deleted_cleanup_is_scoped_idempotent_and_transactional(
+    db_session, rollback
+):
+    deleted_id, other_id = uuid.uuid4(), uuid.uuid4()
+    kept_ids = set()
+    for tenant_id in ("first", "second"):
+        with tenant_context(tenant_id):
+            for kind, owner in (
+                (TokenKind.PERSONAL, deleted_id),
+                (TokenKind.PERSONAL, other_id),
+                (TokenKind.SERVICE, deleted_id),
+            ):
+                token = ApiToken(
+                    kind=kind,
+                    owner_user_id=owner,
+                    name="token",
+                    prefix="test",
+                    token_hash=uuid.uuid4().hex,
+                    expires_at=datetime.now(UTC) + timedelta(days=1),
+                )
+                db_session.add(token)
+                if kind == TokenKind.SERVICE or owner == other_id:
+                    kept_ids.add(token.id)
+            await db_session.flush()
+    await finalize_session(db_session)
+    bus = EventBus()
+    # Workers register without an app; inline delivery uses the same durable subscription.
+    ApiTokensModule().register_event_handlers(bus)
+    with tenant_context("first"):
+        for _ in range(2):
+            await bus.publish_durable_or_inline(UserDeleted(deleted_id), db_session)
+    with all_tenants():
+        remaining = (await db_session.execute(select(ApiToken.id))).scalars().all()
+    assert set(remaining) == kept_ids
+    if rollback:
+        await rollback_session(db_session)
+    else:
+        await finalize_session(db_session)
+    with all_tenants():
+        remaining = (await db_session.execute(select(ApiToken.id))).scalars().all()
+    if rollback:
+        assert len(remaining) == 6
+    else:
+        assert set(remaining) == kept_ids

modules/background_tasks/background_tasks/outbox_transport.py

@@ -132,6 +132,8 @@ def __init__(
         # Eager in-process deliveries still running (kept so they aren't
         # garbage-collected mid-flight, and so ``drain`` can await them).
         self._tasks: set[asyncio.Task[Any]] = set()
+        # Chained eager publishes must not share a StaticPool transaction (#655).
+        self._dispatch_lock = asyncio.Lock()
 
     def policy(self) -> OutboxPolicy:
         s = self._services.settings
@@ -155,8 +157,9 @@ def schedule(self, db: AsyncSession | Session, ids: Sequence[uuid.UUID]) -> None
         id_strs = [str(i) for i in id_list]
         after_commit(db, [lambda: self._send(id_strs)])
 
-    def _dispatch(self, ids: list[uuid.UUID]) -> Any:
-        return dispatch(self._db_state.session_factory, self._bus, self.policy(), ids=ids)
+    async def _dispatch(self, ids: list[uuid.UUID]) -> Any:
+        async with self._dispatch_lock:
+            return await dispatch(self._db_state.session_factory, self._bus, self.policy(), ids=ids)
 
     def _dispatch_soon(self, ids: list[uuid.UUID]) -> None:
         """Eager delivery from a session's ``after_commit`` event.

modules/background_tasks/tests/test_bg_outbox_eager_serialization.py

@@ -0,0 +1,55 @@
+"""Chained eager deliveries must not overlap on in-memory SQLite (#655)."""
+
+import asyncio
+from dataclasses import dataclass
+
+from background_tasks.constants import OutboxStatus
+from background_tasks.models import TaskExecution
+from simple_module_core import DurableContext
+from simple_module_core.events import Event
+
+
+@dataclass
+class EagerParent(Event):
+    pass
+
+
+@dataclass
+class EagerChild(Event):
+    pass
+
+
+async def test_chained_eager_delivery_waits_for_parent_dispatch(app, outbox_rows, effect_names):
+    app.state.background_tasks.settings.task_always_eager = True
+    bus = app.state.sm.event_bus
+    active = False
+
+    async def publish_child(event: EagerParent, ctx: DurableContext) -> None:
+        await bus.publish_durable(EagerChild(), ctx.db)
+
+    async def effect(event: Event, ctx: DurableContext) -> None:
+        nonlocal active
+        assert not active, "eager dispatchers overlapped"
+        active = True
+        try:
+            await ctx.db.connection()
+            await asyncio.sleep(0.05)  # intentional-sleep: child dispatcher claims while guarded
+            ctx.db.add(TaskExecution(task_name=f"effect:{type(event).__name__}"))
+            await ctx.db.flush()
+        finally:
+            active = False
+
+    bus.subscribe_durable(EagerParent, publish_child, name="eager-publish-child")
+    bus.subscribe_durable(EagerParent, effect, name="eager-parent-effect")
+    bus.subscribe_durable(EagerChild, effect, name="eager-child-effect")
+    async with app.state.sm.db.session_factory() as db:
+        await bus.publish_durable(EagerParent(), db)
+        await db.commit()
+
+    await bus.durable_transport.drain()
+
+    rows = await outbox_rows()
+    assert len(rows) == 3
+    assert all(row.status == OutboxStatus.DELIVERED for row in rows)
+    assert all(row.attempts == 1 for row in rows)
+    assert await effect_names() == ["effect:EagerChild", "effect:EagerParent"]

modules/tenants/tests/test_tenants_teardown_integration.py

@@ -1,20 +1,31 @@
 """A real tenant deletion delivers every installed owner's durable teardown."""
 
+from dataclasses import replace
 from datetime import UTC, datetime, timedelta
 
+import pytest
 from api_tokens.models import ApiToken
 from audit_log.models import AuditEntry
 from background_tasks.outbox_claim import OutboxPolicy
 from background_tasks.outbox_dispatch import dispatch
 from feature_flags.models import FeatureFlagOverride
 from file_storage.backends.filesystem import FilesystemBackend
 from file_storage.models import StoredFile
+from sharing.module import SharingModule
 from simple_module_db import all_tenants, tenant_context
 from simple_module_db.stored_settings import SCOPE_TENANT, Setting
 from sqlmodel import select
 from tenants.service import TenantService
 
 
+@pytest.fixture(autouse=True, params=[False, True], ids=["default-order", "sharing-after-users"])
+def _sharing_load_order(request, monkeypatch):
+    if request.param:
+        monkeypatch.setattr(
+            SharingModule, "meta", replace(SharingModule.meta, optional_depends_on=["Users"])
+        )
+
+
 async def test_tenants_delete_removes_owned_data_but_retains_audit(
     app, tenant_client, client, tmp_path, monkeypatch
 ):

modules/users/tests-js/users_Login.test.tsx

@@ -42,6 +42,22 @@ afterEach(() => {
 });
 
 describe('users Login', () => {
+  it.each([
+    ['account_not_verified', keys.users.login.oauth_account_not_verified],
+    ['signup_disabled', keys.users.login.oauth_signup_disabled],
+    ['signin_failed', keys.users.login.oauth_signin_failed],
+  ])('shows the translated OAuth message for %s', (code, key) => {
+    configureI18n({ locale: 'en', messages: { [key]: 'Sign-in was refused.' } });
+    render(<Login {...props} oauth_error={code} />);
+    expect(screen.getByRole('alert')).toHaveTextContent('Sign-in was refused.');
+    configureI18n({ locale: 'en', messages: {} });
+  });
+
+  it('ignores unknown OAuth codes instead of displaying query text', () => {
+    render(<Login {...props} oauth_error="<script>untrusted text</script>" />);
+    expect(screen.queryByRole('alert')).toBeNull();
+  });
+
   it('posts the form-encoded credentials and follows the server redirect on 204', async () => {
     const mock = stubFetch(() => new Response(null, { status: 204 }));
     render(<Login {...props} />);

modules/users/tests-js/users_admin_error_details.test.tsx

@@ -0,0 +1,112 @@
+import { configureI18n, t } from '@simple-module-py/i18n';
+import { act, renderHook, waitFor } from '@testing-library/react';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+vi.mock('@inertiajs/react', () => ({ router: { reload: vi.fn(), on: vi.fn(() => vi.fn()) } }));
+vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
+
+import { toast } from 'sonner';
+import { useUserRowActions } from '../users/admin/components/useUserRowActions';
+import de from '../users/locales/de.json';
+import en from '../users/locales/en.json';
+import { adminFailureMessage } from '../users/pages/failureMessage';
+import { useEditUserForm } from '../users/pages/Users/components/useEditUserForm';
+import { useUserActions } from '../users/pages/Users/components/useUserActions';
+import { json, stubFetch } from './users_fixtures';
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.clearAllMocks();
+});
+
+describe.each(['en', 'de'])('users admin errors (%s)', (locale) => {
+  function configure() {
+    const messages: Record<string, string> = {};
+    function flatten(value: object, prefix = 'users') {
+      for (const [key, child] of Object.entries(value)) {
+        if (typeof child === 'string') messages[`${prefix}.${key}`] = child;
+        else flatten(child, `${prefix}.${key}`);
+      }
+    }
+    flatten(locale === 'en' ? en : de);
+    configureI18n({ locale, messages });
+  }
+
+  const privileged = () =>
+    locale === 'en'
+      ? 'You cannot manage a more privileged account.'
+      : 'Sie können kein Konto mit höheren Berechtigungen verwalten.';
+
+  it.each(['row', 'edit'])('translates the server refusal in the %s action toast', async (page) => {
+    configure();
+    stubFetch(() => json(403, { detail: 'You cannot manage a more privileged account.' }));
+    const { result } = renderHook(() => {
+      const row = useUserRowActions();
+      const edit = useUserActions('target', true, false, null);
+      return { row, edit };
+    });
+    await act(async () => {
+      if (page === 'row') await result.current.row.setActive('target', false);
+      else result.current.edit.disable();
+    });
+    await waitFor(() => expect(toast.error).toHaveBeenCalledWith(privileged()));
+    expect(result.current.edit.isActive).toBe(true);
+  });
+
+  it.each([false, true])(
+    'explains a refused role grant after details saved: %s',
+    async (detailsDirty) => {
+      configure();
+      stubFetch((request) =>
+        request.method === 'PATCH'
+          ? json(200, {})
+          : json(403, { detail: 'You cannot assign roles you do not hold.' }),
+      );
+      const user = { id: 'target', email: 'a@example.com', full_name: null, roles: [] };
+      const { result } = renderHook(() => useEditUserForm(user));
+      act(() => {
+        result.current.toggleRole('Admin');
+        if (detailsDirty) result.current.setForm((prev) => ({ ...prev, fullName: 'New name' }));
+      });
+      await act(async () => result.current.save());
+      const reason =
+        locale === 'en'
+          ? 'You cannot assign roles you do not hold.'
+          : 'Sie können keine Rollen zuweisen, die Sie selbst nicht innehaben.';
+      expect(result.current.error).toBe(
+        detailsDirty ? `${(locale === 'en' ? en : de).edit.error_partial} ${reason}` : reason,
+      );
+      expect(result.current.dirtyCount).toBe(1);
+    },
+  );
+
+  it.each(Object.entries(en.admin_errors))('translates the known refusal %s', (key, detail) => {
+    configure();
+    expect(adminFailureMessage({ status: 400, detail }, 'fallback', 'network', t)).toBe(
+      (locale === 'en' ? en : de).admin_errors[key as keyof typeof en.admin_errors],
+    );
+  });
+
+  it.each([null, '', 'constructor', 'unexpected server internals', [{ msg: 'invalid' }]])(
+    'keeps the fallback for unknown detail %s',
+    async (detail) => {
+      configure();
+      stubFetch(() => json(400, { detail }));
+      const { result } = renderHook(() => useUserRowActions());
+      await act(async () => result.current.setActive('target', false));
+      expect(toast.error).toHaveBeenCalledWith(
+        (locale === 'en' ? en : de).user_row.toast_status_failed,
+      );
+    },
+  );
+
+  it('keeps the network failure message', async () => {
+    configure();
+    stubFetch(() => {
+      throw new TypeError('offline');
+    });
+    const { result } = renderHook(() => useUserRowActions());
+    await act(async () => result.current.setActive('target', false));
+    expect(toast.error).toHaveBeenCalledWith((locale === 'en' ? en : de).common.error_occurred);
+  });
+});