fix(spa): detail-page action toast picks colour by message level (#632)

Plus lock-in tests for two audit verifications that already worked
(#629 / #636 — close-with-tests).

## #632 — toast colour by Django level

The detail page's `ObjectActionButton.onSuccess` was unconditionally
calling `toast.success(message || 'Done')` — so a
`message_user(..., level=messages.ERROR)` from a per-object action
toasted GREEN instead of red. The list page already dispatched by
level correctly; the asymmetry was hidden behind the legacy
`runObjectAction` adapter in `@dar/data/mutations.ts`, which collapsed
the `ActionRunResponse.messages[]` (each with a level) into a single
`message` string and dropped the level.

Fix:

- `ObjectActionRunResponse` now carries `messages?: UserMessage[]`
  alongside the legacy `message?: string`.
- `runObjectAction` propagates the full list.
- A new shared `toastMessages(toast, messages)` helper lives next to
  `useToast` and maps each level to the right toast method
  (`error` / `warning` → red, `info` / `debug` → blue,
  `success` / unknown → green) — mirrors what the list page already
  does inline.
- `DetailPage`'s `onSuccess` now uses the helper when `messages[]` is
  present; falls back to `toast.success(message || 'Done')` for
  back-compat with v1.4.x APIs that don't emit messages.

## #629 — prepopulated_fields verify-and-lock

The wire already carries `prepopulated_fields`; `CreatePage`
already slugifies the target from sources on each keystroke and
opts the target out of further auto-fill once the operator edits it
by hand. Pulled the logic out of `handleFieldChange` into a pure
`applyPrepopulate(...)` helper so a vitest can pin the four
behaviours (single source, joined sources, manual-edit lock, empty
fallback) without rendering the whole CreatePage.

## #636 — open-redirect protection on `?next=`

`DarLoginView` is a `LoginView` subclass that doesn't override
`get_success_url` / `form_valid` / `success_url_allowed_hosts`, so
the parent's same-host check on `?next=` is intact today. Added a
pytest that POSTs the login with `?next=https://evil.example.com/`
and asserts the redirect target is NOT off-host — locks the
property so a future override can't silently re-introduce the
open-redirect.

## Verification

- `pnpm -r typecheck` ✓
- `pnpm lint` (js + css + darkmode) ✓
- `pnpm test` — 174 / 174 (up from 163; +11 new) ✓
- `poetry run pytest tests/test_login_next_redirect.py -v` — 2 / 2 ✓
- `pnpm -w build` ✓

Closes #629, closes #632, closes #636.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: martin-castro-laminr-ai

frontend/apps/web/src/pages/CreatePage.tsx

@@ -20,7 +20,7 @@ import { Breadcrumb, Button, Card, EmptyState } from '@dar/ui';
 import { FieldInput } from '@dar/form';
 
 import { RecordSkeleton } from '../components/RecordSkeleton';
-import { slugify } from '../slugify';
+import { applyPrepopulate } from './prepopulate';
 import { useModelMeta } from '../useModelMeta';
 import { useToast } from '../toast';
 import { useUnsavedGuard } from '../useUnsavedGuard';
@@ -184,16 +184,13 @@ function CreateForm({ schema, onCreate, onCancel }: CreateFormProps) {
 
   function handleFieldChange(name: string, v: WriteValue): void {
     setValues((prev) => {
-      const next = { ...prev, [name]: v };
       // A direct edit of a target field opts it out of further auto-fill.
       if (prepopTargets.has(name)) editedTargets.current.add(name);
-      for (const [target, sources] of Object.entries(prepopulated)) {
-        if (editedTargets.current.has(target)) continue;
-        if (!sources.includes(name)) continue;
-        const joined = sources.map((s) => (next[s] == null ? '' : String(next[s]))).join(' ');
-        next[target] = slugify(joined);
-      }
-      return next;
+      return applyPrepopulate({
+        values: { ...prev, [name]: v },
+        prepopulated,
+        editedTargets: editedTargets.current,
+      });
     });
   }
 

frontend/apps/web/src/pages/DetailPage.tsx

@@ -38,6 +38,7 @@ import {
   type InlineWriteItem,
   type InlineWritePayload,
   type ObjectActionDescriptor,
+  type ObjectActionRunResponse,
   type WriteValue,
 } from '@dar/data';
 import { detailCollapseKey, usePersistedState } from '@dar/customization';
@@ -48,7 +49,7 @@ import { HistoryModal } from '@dar/history';
 
 import { RecordSkeleton } from '../components/RecordSkeleton';
 import { useModelMeta } from '../useModelMeta';
-import { useToast } from '../toast';
+import { toastMessages, useToast } from '../toast';
 import { carryPreservedFilters, listPathWithPreservedFilters } from '../changelistFilters';
 import { useUnsavedGuard } from '../useUnsavedGuard';
 
@@ -310,13 +311,21 @@ export function DetailPage({
                 onRun={() =>
                   runObjectAction({ client, appLabel, modelName, pk, name: action.name })
                 }
-                onSuccess={async (message, redirect) => {
+                onSuccess={async ({ message, messages, redirect }) => {
                   if (redirect) {
                     navigate(redirect);
                     return;
                   }
                   await refresh();
-                  toast.success(message || 'Done');
+                  // Dispatch by Django level tag (#632) — error /
+                  // warning levels must NOT toast green. Falls back to a
+                  // single-line success toast for v1.4.x APIs that don't
+                  // emit `messages[]`.
+                  if (messages && messages.length > 0) {
+                    toastMessages(toast, messages);
+                  } else {
+                    toast.success(message || 'Done');
+                  }
                 }}
                 onError={(message) => toast.error(message)}
               />
@@ -444,8 +453,8 @@ function ObjectActionButton({
   onError,
 }: {
   action: ObjectActionDescriptor;
-  onRun: () => Promise<{ ok: boolean; message?: string; redirect?: string }>;
-  onSuccess: (message: string | undefined, redirect: string | undefined) => Promise<void> | void;
+  onRun: () => Promise<ObjectActionRunResponse>;
+  onSuccess: (result: ObjectActionRunResponse) => Promise<void> | void;
   onError: (message: string) => void;
 }) {
   const [busy, setBusy] = useState(false);
@@ -460,7 +469,7 @@ function ObjectActionButton({
         try {
           const result = await onRun();
           if (result.ok) {
-            await onSuccess(result.message, result.redirect);
+            await onSuccess(result);
           } else {
             onError(result.message || 'The action could not be completed.');
           }

frontend/apps/web/src/pages/prepopulate.test.ts

@@ -0,0 +1,56 @@
+// Lock the prepopulated_fields behaviour (#245 / #629). Verifies:
+//   1. typing in a source field slugifies the target on every keystroke
+//   2. multiple sources are joined with a space, then slugified together
+//   3. once the operator edits a target by hand, further changes to
+//      its sources DO NOT overwrite — the manual edit wins
+//   4. a target with no source values stays an empty string
+import { describe, expect, it } from 'vitest';
+
+import { applyPrepopulate } from './prepopulate';
+
+describe('applyPrepopulate', () => {
+  const prepopulated = { slug: ['title'] };
+
+  it('slugifies the target from a single source on first edit', () => {
+    const out = applyPrepopulate({
+      values: { title: 'Hello World' },
+      prepopulated,
+      editedTargets: new Set(),
+    });
+    expect(out.slug).toBe('hello-world');
+  });
+
+  it('joins multiple sources with a space before slugifying', () => {
+    const out = applyPrepopulate({
+      values: { first: 'Acme', second: 'Holdings Inc' },
+      prepopulated: { slug: ['first', 'second'] },
+      editedTargets: new Set(),
+    });
+    expect(out.slug).toBe('acme-holdings-inc');
+  });
+
+  it('does not overwrite a target the operator has edited by hand', () => {
+    const out = applyPrepopulate({
+      values: { title: 'New Title', slug: 'custom-slug' },
+      prepopulated,
+      editedTargets: new Set(['slug']),
+    });
+    expect(out.slug).toBe('custom-slug');
+  });
+
+  it('leaves the target as an empty string when every source is empty', () => {
+    const out = applyPrepopulate({
+      values: { title: '' },
+      prepopulated,
+      editedTargets: new Set(),
+    });
+    expect(out.slug).toBe('');
+  });
+
+  it('returns a new object — does not mutate the input', () => {
+    const values = { title: 'X' };
+    const out = applyPrepopulate({ values, prepopulated, editedTargets: new Set() });
+    expect(out).not.toBe(values);
+    expect(values).not.toHaveProperty('slug');
+  });
+});

frontend/apps/web/src/pages/prepopulate.ts

@@ -0,0 +1,37 @@
+// prepopulated_fields helper (#245 / #629). Pure function so the
+// CreatePage form logic is testable without rendering the page —
+// CreatePage owns the per-target "user has edited this" set and
+// passes it in; this just applies the slugify-from-sources rule
+// to every still-untouched target.
+//
+// Mirrors Django admin's add-form behaviour: typing in a source
+// field auto-fills the target until the operator edits the target
+// by hand, after which auto-fill stops for that record.
+import type { WriteValue } from '@dar/data';
+
+import { slugify } from '../slugify';
+
+export interface PrepopulateArgs {
+  /** Current draft values (the function is non-mutating — returns
+   *  a NEW object with the updated targets). */
+  values: Record<string, WriteValue>;
+  /** From `schema.prepopulated_fields`: `{target: [...sourceFields]}`. */
+  prepopulated: Record<string, string[]>;
+  /** Target field names the operator has edited directly — these
+   *  are SKIPPED so a manual edit isn't overwritten. */
+  editedTargets: ReadonlySet<string>;
+}
+
+export function applyPrepopulate({
+  values,
+  prepopulated,
+  editedTargets,
+}: PrepopulateArgs): Record<string, WriteValue> {
+  const next = { ...values };
+  for (const [target, sources] of Object.entries(prepopulated)) {
+    if (editedTargets.has(target)) continue;
+    const joined = sources.map((s) => (next[s] == null ? '' : String(next[s]))).join(' ');
+    next[target] = slugify(joined);
+  }
+  return next;
+}

frontend/apps/web/src/toast.test.ts

@@ -0,0 +1,67 @@
+// Lock the level → toast-method dispatch (#632). The detail page
+// previously toasted success-green for every action, including
+// `message_user(..., level=messages.ERROR)`, because the legacy
+// `runObjectAction` adapter dropped the level field. This test
+// pins the per-level routing so a regression turns red, not silent.
+import { describe, expect, it, vi } from 'vitest';
+
+import { toastMessages } from './toast';
+
+function makeToast() {
+  return {
+    success: vi.fn<(message: string) => void>(),
+    error: vi.fn<(message: string) => void>(),
+    info: vi.fn<(message: string) => void>(),
+  };
+}
+
+describe('toastMessages', () => {
+  it('routes `error` to toast.error (red)', () => {
+    const t = makeToast();
+    toastMessages(t, [{ level: 'error', message: 'boom' }]);
+    expect(t.error).toHaveBeenCalledWith('boom');
+    expect(t.success).not.toHaveBeenCalled();
+    expect(t.info).not.toHaveBeenCalled();
+  });
+
+  it('routes `warning` to toast.error (red — closest available)', () => {
+    const t = makeToast();
+    toastMessages(t, [{ level: 'warning', message: 'careful' }]);
+    expect(t.error).toHaveBeenCalledWith('careful');
+  });
+
+  it('routes `info` to toast.info (blue)', () => {
+    const t = makeToast();
+    toastMessages(t, [{ level: 'info', message: 'fyi' }]);
+    expect(t.info).toHaveBeenCalledWith('fyi');
+  });
+
+  it('routes `debug` to toast.info (blue)', () => {
+    const t = makeToast();
+    toastMessages(t, [{ level: 'debug', message: 'trace' }]);
+    expect(t.info).toHaveBeenCalledWith('trace');
+  });
+
+  it('routes `success` (and unknown levels) to toast.success (green)', () => {
+    const t = makeToast();
+    toastMessages(t, [
+      { level: 'success', message: 'ok' },
+      { level: 'unknown', message: 'mystery' },
+    ]);
+    expect(t.success).toHaveBeenCalledWith('ok');
+    expect(t.success).toHaveBeenCalledWith('mystery');
+  });
+
+  it('dispatches every entry in order — no merging, no dedupe', () => {
+    const t = makeToast();
+    toastMessages(t, [
+      { level: 'error', message: 'first' },
+      { level: 'success', message: 'second' },
+      { level: 'error', message: 'third' },
+    ]);
+    expect(t.error).toHaveBeenCalledTimes(2);
+    expect(t.error).toHaveBeenNthCalledWith(1, 'first');
+    expect(t.error).toHaveBeenNthCalledWith(2, 'third');
+    expect(t.success).toHaveBeenCalledWith('second');
+  });
+});

frontend/apps/web/src/toast.tsx

@@ -32,6 +32,27 @@ export function useToast(): ToastApi {
   return ctx;
 }
 
+/**
+ * Dispatch a `message_user`-emitted message list to the toast API,
+ * picking the colour per Django's level tag (#632 — list AND detail
+ * page now share this; the detail page previously dropped the level
+ * via the legacy `runObjectAction` adapter and toasted success-green
+ * for every action even when the level was `error` / `warning`).
+ *
+ * Mirrors the legacy admin: `error` / `warning` → red, `info` /
+ * `debug` → blue, `success` (and any unknown level) → green.
+ */
+export function toastMessages(
+  toast: ToastApi,
+  messages: ReadonlyArray<{ level: string; message: string }>,
+): void {
+  for (const m of messages) {
+    if (m.level === 'error' || m.level === 'warning') toast.error(m.message);
+    else if (m.level === 'info' || m.level === 'debug') toast.info(m.message);
+    else toast.success(m.message);
+  }
+}
+
 export function ToastProvider({ children }: { children: ReactNode }) {
   const [toasts, setToasts] = useState<Toast[]>([]);
   const idRef = useRef(0);

frontend/packages/api/src/contract.ts

@@ -319,7 +319,13 @@ export type ObjectActionDescriptor = ActionDescriptor;
  */
 export interface ObjectActionRunResponse {
   ok: boolean;
+  /** Convenience: first `messages[]` entry's text — kept for back-compat
+   *  with v1.4.x consumers that only ever toasted one line. */
   message?: string;
+  /** Every `message_user` line with its Django level tag (#632 — needed
+   *  so the SPA can pick red / amber / blue / green per level instead of
+   *  always toasting green on the detail page). */
+  messages?: UserMessage[];
   redirect?: string;
 }
 

frontend/packages/data/src/mutations.ts

@@ -89,6 +89,11 @@ export async function runObjectAction(
   return {
     ok: res.executed,
     ...(message !== undefined ? { message } : {}),
+    // Propagate the full level-tagged messages list (#632) so the
+    // detail page can pick the toast colour per level — the legacy
+    // single-`message` field collapses every message into one string
+    // with no level information.
+    ...(res.messages !== undefined ? { messages: res.messages } : {}),
     ...(res.redirect !== undefined ? { redirect: res.redirect } : {}),
   };
 }

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "django-admin-react"
-version = "1.4.11"
+version = "1.4.12"
 description = "A drop-in React single-page admin for Django, driven entirely by ModelAdmin."
 authors = ["django-admin-react contributors"]
 license = "MIT"

tests/test_login_next_redirect.py

@@ -0,0 +1,66 @@
+"""Lock the ``?next=`` open-redirect protection on ``DarLoginView`` (#636).
+
+``DarLoginView`` subclasses ``django.contrib.auth.views.LoginView``,
+which defaults ``success_url_allowed_hosts = set()`` and rejects any
+``next`` query value that points off-host. The risk: a subclass that
+overrides ``get_success_url`` / ``form_valid`` without preserving the
+allow-list check would silently re-introduce an open redirect — every
+"please log in, then return to <X>" mail-out would become a phishing
+weapon.
+
+This test posts a login with ``?next=https://evil.example.com/`` and
+asserts the post-login redirect target is NOT off-host. If a future
+change overrides the redirect helpers without the safe-host gate,
+this test fails red and the regression doesn't ship.
+"""
+
+from __future__ import annotations
+
+from urllib.parse import urlparse
+
+import pytest
+from django.contrib.auth import get_user_model
+from django.test import Client
+
+LOGIN_URL = "/admin-react/login/"
+
+
+@pytest.fixture
+def staff_user(db):
+    u = get_user_model().objects.create_user(username="alice", password="pw12345!")
+    u.is_staff = True
+    u.is_active = True
+    u.save()
+    return u
+
+
+def test_next_param_rejects_external_host(staff_user):
+    """An off-host ``next`` value MUST NOT survive into the redirect target."""
+    c = Client()
+    response = c.post(
+        f"{LOGIN_URL}?next=https://evil.example.com/phish",
+        data={"username": "alice", "password": "pw12345!"},
+    )
+    # Successful login should redirect to the safe default, not off-host.
+    assert response.status_code == 302, "Login POST should 302 on success."
+    target = response.headers["Location"]
+    parsed = urlparse(target)
+    # An empty netloc means same-host (a path-only redirect like `/admin/`).
+    # Any non-empty netloc must NOT be the attacker's host.
+    assert parsed.netloc in ("", "testserver"), (
+        f"Login redirected to off-host URL {target!r} — open redirect."
+    )
+
+
+def test_next_param_accepts_same_host_path(staff_user):
+    """A same-host ``next`` path IS honoured (the legitimate use case)."""
+    c = Client()
+    response = c.post(
+        f"{LOGIN_URL}?next=/admin-react/auth/user/",
+        data={"username": "alice", "password": "pw12345!"},
+    )
+    assert response.status_code == 302
+    target = response.headers["Location"]
+    parsed = urlparse(target)
+    assert parsed.netloc in ("", "testserver")
+    assert parsed.path == "/admin-react/auth/user/"