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
79 changes: 79 additions & 0 deletions frontend/apps/web/src/action-redirect.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
68 changes: 68 additions & 0 deletions frontend/apps/web/src/action-redirect.ts
Original file line number Diff line number Diff line change
@@ -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 `<meta name="dar-mount">` 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);
}
}
15 changes: 14 additions & 1 deletion frontend/apps/web/src/pages/DetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<HTMLMetaElement>('meta[name="dar-mount"]')
?.content?.trim();
const mount = (mountMeta ?? '/').replace(/\/?$/, '/');
followActionRedirect({ redirect, mount, navigate });
return;
}
await refresh();
Expand Down
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.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"
Expand Down
Loading