Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions frontend/apps/web/src/pages/CreatePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
});
});
}

Expand Down
21 changes: 15 additions & 6 deletions frontend/apps/web/src/pages/DetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
type InlineWriteItem,
type InlineWritePayload,
type ObjectActionDescriptor,
type ObjectActionRunResponse,
type WriteValue,
} from '@dar/data';
import { detailCollapseKey, usePersistedState } from '@dar/customization';
Expand All @@ -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';

Expand Down Expand Up @@ -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)}
/>
Expand Down Expand Up @@ -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);
Expand All @@ -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.');
}
Expand Down
56 changes: 56 additions & 0 deletions frontend/apps/web/src/pages/prepopulate.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
37 changes: 37 additions & 0 deletions frontend/apps/web/src/pages/prepopulate.ts
Original file line number Diff line number Diff line change
@@ -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;
}
67 changes: 67 additions & 0 deletions frontend/apps/web/src/toast.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
21 changes: 21 additions & 0 deletions frontend/apps/web/src/toast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions frontend/packages/api/src/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions frontend/packages/data/src/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } : {}),
};
}
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
66 changes: 66 additions & 0 deletions tests/test_login_next_redirect.py
Original file line number Diff line number Diff line change
@@ -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/"
Loading