diff --git a/frontend/apps/web/src/action-redirect.test.ts b/frontend/apps/web/src/action-redirect.test.ts new file mode 100644 index 0000000..fd73b65 --- /dev/null +++ b/frontend/apps/web/src/action-redirect.test.ts @@ -0,0 +1,79 @@ +// Lock the action-redirect routing (#620). `navigate` for SPA-internal +// paths, `window.location.assign` (injected as `assignLocation` for +// testability) for everything else. The legacy behaviour piped every +// redirect through `navigate` — which silently no-op'd for any URL +// outside the SPA mount. +import { describe, expect, it, vi } from 'vitest'; + +import { followActionRedirect } from './action-redirect'; + +const MOUNT = '/admin-react/'; +const ORIGIN = 'http://localhost:3000'; + +function makeArgs(redirect: string) { + const navigate = vi.fn(); + const assignLocation = vi.fn(); + return { + args: { redirect, mount: MOUNT, navigate, currentOrigin: ORIGIN, assignLocation }, + navigate, + assignLocation, + }; +} + +describe('followActionRedirect', () => { + it('uses navigate for a same-origin path inside the SPA mount', () => { + const { args, navigate, assignLocation } = makeArgs('/admin-react/auth/user/42/'); + followActionRedirect(args); + // The mount prefix is stripped so BrowserRouter's basename + // doesn't double up. Trailing slash before the relative path is + // preserved. + expect(navigate).toHaveBeenCalledWith('/auth/user/42/'); + expect(assignLocation).not.toHaveBeenCalled(); + }); + + it('preserves search + hash when navigating client-side', () => { + const { args, navigate } = makeArgs('/admin-react/auth/user/42/?tab=audit#log'); + followActionRedirect(args); + expect(navigate).toHaveBeenCalledWith('/auth/user/42/?tab=audit#log'); + }); + + it('falls back to assignLocation for a same-origin path OUTSIDE the mount', () => { + // Legacy admin path — must be a full browser navigation since + // React Router only routes within the SPA mount. + const { args, navigate, assignLocation } = makeArgs('/admin/auth/user/42/change/'); + followActionRedirect(args); + expect(assignLocation).toHaveBeenCalledWith('/admin/auth/user/42/change/'); + expect(navigate).not.toHaveBeenCalled(); + }); + + it('falls back to assignLocation for cross-origin URLs', () => { + // The signed-S3-download case — must be a real browser navigation + // so the download starts on the operator's machine. + const { args, navigate, assignLocation } = makeArgs('https://s3.example.com/signed/file.pdf'); + followActionRedirect(args); + expect(assignLocation).toHaveBeenCalledWith('https://s3.example.com/signed/file.pdf'); + expect(navigate).not.toHaveBeenCalled(); + }); + + it('falls back to assignLocation for a hijack-style /hijack/... URL', () => { + // Common third-party pattern (`django-hijack`) — action returns + // `HttpResponseRedirect("/hijack/release-user/?next=...")`. + const { args, navigate, assignLocation } = makeArgs('/hijack/release-user/?next=/admin/foo/'); + followActionRedirect(args); + expect(assignLocation).toHaveBeenCalledWith('/hijack/release-user/?next=/admin/foo/'); + expect(navigate).not.toHaveBeenCalled(); + }); + + it('falls back to assignLocation on an unparseable URL', () => { + // A malformed input shouldn't make the operator's click disappear + // into the void — surface it as a real navigation and let the + // browser report the failure to the user. + // + // Using "http://" alone — bare scheme with no authority — produces + // a TypeError in the URL constructor on all majors. + const { args, navigate, assignLocation } = makeArgs('http://'); + followActionRedirect(args); + expect(assignLocation).toHaveBeenCalledWith('http://'); + expect(navigate).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/apps/web/src/action-redirect.ts b/frontend/apps/web/src/action-redirect.ts new file mode 100644 index 0000000..9ccc1ea --- /dev/null +++ b/frontend/apps/web/src/action-redirect.ts @@ -0,0 +1,68 @@ +// Action-handler redirect routing (#620). Django ModelAdmin actions +// can return any HttpResponse (HttpResponseRedirect, file streams, +// HTML pages); the API extracts the response's Location header into +// the JSON envelope's `redirect` field. Before this fix the SPA piped +// `redirect` straight into React Router's `navigate` — which is +// scoped to the SPA mount, so any redirect pointing OUTSIDE the SPA +// silently no-op'd: legacy `/admin/.../` URLs, hijack/impersonate +// flows, signed S3 download URLs all looked broken from the operator's +// POV. This helper picks the right navigation primitive per URL. +import type { NavigateFunction } from 'react-router-dom'; + +/** + * Follow a redirect returned by a ModelAdmin action. + * + * - Same origin AND under the SPA mount → React Router `navigate` + * (no full reload; the operator stays in the SPA). + * - Anything else (off-origin, or same-origin outside the mount) → + * `window.location.assign` (full browser navigation; the only way + * to reach a non-SPA page). + * + * `mount` is the SPA's URL prefix (e.g. `/admin-react/`), already + * read once at boot from the `` tag in + * `main.tsx` — pass it through; this helper doesn't query the DOM + * so it stays unit-testable. + * + * If the redirect URL is unparseable (a defensive case — the API + * should always emit a real URL) we fall back to `window.location + * .assign` and let the browser report the failure to the user. + */ +export interface FollowRedirectArgs { + redirect: string; + mount: string; + navigate: NavigateFunction; + /** Current page origin (`http://localhost:3000` etc.). Defaults to + * `window.location.origin` in production; the test injects a known + * value because jsdom's `window.location.origin` is hard-coded. */ + currentOrigin?: string; + /** Full-page navigation. Defaults to `window.location.assign` in + * production; the test injects a spy because jsdom's + * `window.location.assign` is non-configurable on most jsdom + * versions and resists `vi.spyOn`. */ + assignLocation?: (url: string) => void; +} + +export function followActionRedirect(args: FollowRedirectArgs): void { + const { redirect, mount, navigate } = args; + const origin = args.currentOrigin ?? window.location.origin; + const assignLocation = args.assignLocation ?? ((url: string) => window.location.assign(url)); + let parsed: URL; + try { + parsed = new URL(redirect, origin); + } catch { + assignLocation(redirect); + return; + } + const sameOrigin = parsed.origin === origin; + const inSpaMount = parsed.pathname.startsWith(mount); + if (sameOrigin && inSpaMount) { + // BrowserRouter is initialised with `basename={mount}` (main.tsx) + // so React Router routes are relative to that. Strip the mount + // prefix before passing — otherwise the basename auto-prepends + // and we get `/admin-react/admin-react/...` which 404s. + const relative = parsed.pathname.slice(mount.length - 1) + parsed.search + parsed.hash; + navigate(relative); + } else { + assignLocation(redirect); + } +} diff --git a/frontend/apps/web/src/pages/DetailPage.tsx b/frontend/apps/web/src/pages/DetailPage.tsx index b378c09..c9680d0 100644 --- a/frontend/apps/web/src/pages/DetailPage.tsx +++ b/frontend/apps/web/src/pages/DetailPage.tsx @@ -50,6 +50,7 @@ import { HistoryModal } from '@dar/history'; import { RecordSkeleton } from '../components/RecordSkeleton'; import { useModelMeta } from '../useModelMeta'; import { toastMessages, useToast } from '../toast'; +import { followActionRedirect } from '../action-redirect'; import { carryPreservedFilters, listPathWithPreservedFilters } from '../changelistFilters'; import { useUnsavedGuard } from '../useUnsavedGuard'; @@ -313,7 +314,19 @@ export function DetailPage({ } onSuccess={async ({ message, messages, redirect }) => { if (redirect) { - navigate(redirect); + // Smart-route the redirect (#620): an admin action + // can hand back any URL — a same-SPA path, a legacy + // `/admin/.../change/` page, a hijack URL, a signed + // S3 download. React Router's `navigate` only + // routes WITHIN the SPA mount, so non-SPA URLs + // silently no-op'd before. `followActionRedirect` + // picks `navigate` vs `window.location.assign` + // based on origin + mount. + const mountMeta = document + .querySelector('meta[name="dar-mount"]') + ?.content?.trim(); + const mount = (mountMeta ?? '/').replace(/\/?$/, '/'); + followActionRedirect({ redirect, mount, navigate }); return; } await refresh(); diff --git a/pyproject.toml b/pyproject.toml index 6287331..e5d8b73 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "django-admin-react" -version = "1.4.12" +version = "1.4.13" description = "A drop-in React single-page admin for Django, driven entirely by ModelAdmin." authors = ["django-admin-react contributors"] license = "MIT"