diff --git a/apps/docs/content/docs/dev/advanced/auth.mdx b/apps/docs/content/docs/dev/advanced/auth.mdx index 83e5104be..19dc6f685 100644 --- a/apps/docs/content/docs/dev/advanced/auth.mdx +++ b/apps/docs/content/docs/dev/advanced/auth.mdx @@ -75,3 +75,44 @@ import { TypeTable } from 'fumadocs-ui/components/type-table'; }, }} /> + +## Back to where you were + +The admin cookie is short-lived by design (a day, by default), so an admin's +session tends to run out mid-task. When it does, VitNode remembers the page they +were on in the sign-in URL: + +```txt +/admin?redirect=%2Fadmin%2Fcore%2Fusers%3Fpage%3D2 +``` + +Sign in, and you land back on that page - query string and all - instead of the +dashboard. With nothing to return to, sign-in goes to `/admin/core` as before. + +The path comes from the Proxy, which stamps every request with an +`x-vitnode-pathname` header (locale prefix already removed) because a Server +Component only ever sees the rewritten, internal URL. You get this for free with +`createVitNodeProxy`. + +The `?redirect=` value is untrusted - it is in the URL, after all - so it is +validated twice: once when the sign-in page reads it, and again in the Server +Action that signs you in. Only paths inside `/admin` survive; an absolute URL, a +protocol-relative `//somewhere-else.example`, a `..` segment or the sign-in page +itself are all thrown away in favour of `/admin/core`. + + + Your `src/app/[locale]/admin/page.tsx` has to forward the page props for the + sign-in view to see the query: + +```tsx title="src/app/[locale]/admin/page.tsx" +import { SignInAdminView } from "@vitnode/core/views/admin/sign-in/sign-in-admin-view"; +import React from "react"; + +export default function Page( + props: React.ComponentProps, +) { + return ; +} +``` + + diff --git a/apps/docs/src/app/[locale]/admin/page.tsx b/apps/docs/src/app/[locale]/admin/page.tsx index 5e82810dd..5d9d3b0f0 100644 --- a/apps/docs/src/app/[locale]/admin/page.tsx +++ b/apps/docs/src/app/[locale]/admin/page.tsx @@ -1,5 +1,8 @@ import { SignInAdminView } from "@vitnode/core/views/admin/sign-in/sign-in-admin-view"; +import React from "react"; -export default function Page() { - return ; +export default function Page( + props: React.ComponentProps, +) { + return ; } diff --git a/packages/create-vitnode-app/copy-of-vitnode-app/root/src/app/[locale]/admin/page.tsx b/packages/create-vitnode-app/copy-of-vitnode-app/root/src/app/[locale]/admin/page.tsx index 5e82810dd..5d9d3b0f0 100644 --- a/packages/create-vitnode-app/copy-of-vitnode-app/root/src/app/[locale]/admin/page.tsx +++ b/packages/create-vitnode-app/copy-of-vitnode-app/root/src/app/[locale]/admin/page.tsx @@ -1,5 +1,8 @@ import { SignInAdminView } from "@vitnode/core/views/admin/sign-in/sign-in-admin-view"; +import React from "react"; -export default function Page() { - return ; +export default function Page( + props: React.ComponentProps, +) { + return ; } diff --git a/packages/vitnode/src/lib/admin-redirect.test.ts b/packages/vitnode/src/lib/admin-redirect.test.ts new file mode 100644 index 000000000..a351b51ef --- /dev/null +++ b/packages/vitnode/src/lib/admin-redirect.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; + +import { + ADMIN_SIGN_IN_PATH, + getAdminSignInHref, + sanitizeAdminRedirect, +} from "./admin-redirect"; + +describe("sanitizeAdminRedirect", () => { + it("keeps AdminCP paths, query string included", () => { + expect(sanitizeAdminRedirect("/admin/core/users")).toBe( + "/admin/core/users", + ); + expect(sanitizeAdminRedirect("/admin/core/users?page=2&sort=name")).toBe( + "/admin/core/users?page=2&sort=name", + ); + }); + + it("rejects anything that would leave the origin", () => { + expect(sanitizeAdminRedirect("//evil.example/admin")).toBeUndefined(); + expect(sanitizeAdminRedirect("/\\evil.example/admin")).toBeUndefined(); + expect(sanitizeAdminRedirect("https://evil.example")).toBeUndefined(); + expect(sanitizeAdminRedirect("/admin/core\\..\\..\\evil")).toBeUndefined(); + expect(sanitizeAdminRedirect("admin/core")).toBeUndefined(); + }); + + it("rejects paths outside the AdminCP", () => { + expect(sanitizeAdminRedirect("/settings")).toBeUndefined(); + expect(sanitizeAdminRedirect("/administrators")).toBeUndefined(); + expect(sanitizeAdminRedirect("/admin/../settings")).toBeUndefined(); + }); + + it("rejects the sign-in page, which would be a loop", () => { + expect(sanitizeAdminRedirect("/admin")).toBeUndefined(); + expect( + sanitizeAdminRedirect("/admin?redirect=/admin/core"), + ).toBeUndefined(); + }); + + it("rejects a missing value", () => { + expect(sanitizeAdminRedirect(null)).toBeUndefined(); + expect(sanitizeAdminRedirect(undefined)).toBeUndefined(); + expect(sanitizeAdminRedirect("")).toBeUndefined(); + }); +}); + +describe("getAdminSignInHref", () => { + it("remembers where the admin was", () => { + expect(getAdminSignInHref("/admin/core/users?page=2")).toEqual({ + pathname: ADMIN_SIGN_IN_PATH, + query: { redirect: "/admin/core/users?page=2" }, + }); + }); + + it("falls back to the bare sign-in page", () => { + expect(getAdminSignInHref("/admin")).toBe(ADMIN_SIGN_IN_PATH); + expect(getAdminSignInHref(null)).toBe(ADMIN_SIGN_IN_PATH); + }); +}); diff --git a/packages/vitnode/src/lib/admin-redirect.ts b/packages/vitnode/src/lib/admin-redirect.ts new file mode 100644 index 000000000..21960aab6 --- /dev/null +++ b/packages/vitnode/src/lib/admin-redirect.ts @@ -0,0 +1,53 @@ +/** Query parameter carrying the page to return to after signing back in. */ +export const ADMIN_REDIRECT_PARAM = "redirect"; + +/** The AdminCP sign-in page. */ +export const ADMIN_SIGN_IN_PATH = "/admin"; + +/** Where an admin lands when there is nothing to return to. */ +export const ADMIN_HOME_PATH = "/admin/core"; + +/** + * Narrows a `?redirect=` value down to a path inside the AdminCP, or + * `undefined` when it is anything else. + * + * The value is untrusted on both hops - it arrives in the URL, and again in the + * sign-in form's payload - so both sanitise it. Without this an expired session + * would hand anyone a link that signs an admin in and drops them on + * `//evil.example`, which is exactly the open redirect a "take me back where I + * was" feature invites. Only locale-less paths under `/admin` survive, and the + * sign-in page itself is rejected so a successful sign-in never lands back on + * the form. + */ +export const sanitizeAdminRedirect = ( + value: null | string | undefined, +): string | undefined => { + if (typeof value !== "string" || !value.startsWith("/")) return undefined; + + // `//evil.example` and `/\evil.example` are both protocol-relative URLs to + // another origin, and a backslash anywhere else is no more legitimate. + if (value.includes("\\") || value.startsWith("//")) return undefined; + + const [pathname] = value.split(/[#?]/); + + if (!pathname.startsWith(`${ADMIN_SIGN_IN_PATH}/`)) return undefined; + // `/admin/../..` stays same-origin, but it leaves the AdminCP. + if (pathname.split("/").includes("..")) return undefined; + + return value; +}; + +/** + * The sign-in href for an admin who was on `pathname` when their session ran + * out, remembering it as `?redirect=` when it is worth returning to. + */ +export const getAdminSignInHref = (pathname: null | string | undefined) => { + const redirectTo = sanitizeAdminRedirect(pathname); + + if (!redirectTo) return ADMIN_SIGN_IN_PATH; + + return { + pathname: ADMIN_SIGN_IN_PATH, + query: { [ADMIN_REDIRECT_PARAM]: redirectTo }, + }; +}; diff --git a/packages/vitnode/src/lib/api/get-session-admin-api.ts b/packages/vitnode/src/lib/api/get-session-admin-api.ts index 6d19d8897..f3d3e1d7a 100644 --- a/packages/vitnode/src/lib/api/get-session-admin-api.ts +++ b/packages/vitnode/src/lib/api/get-session-admin-api.ts @@ -1,3 +1,4 @@ +import { headers } from "next/headers"; import { cache } from "react"; import type { PermissionsStaffArgs } from "@/api/lib/permission-staff"; @@ -5,7 +6,9 @@ import type { PermissionsStaffArgs } from "@/api/lib/permission-staff"; import { hasStaffPermission } from "@/api/lib/staff-permission"; import { adminModule } from "@/api/modules/admin/admin.module"; import { CONFIG_PLUGIN } from "@/config"; +import { getAdminSignInHref } from "@/lib/admin-redirect"; import { fetcher } from "@/lib/fetcher"; +import { VITNODE_PATHNAME_HEADER } from "@/lib/request-pathname"; import { redirect } from "../navigation"; @@ -33,7 +36,10 @@ export const getSessionAdminApi = cache(async () => { }); if (res.status !== 200) { - await redirect("/admin"); + // Remembered as `?redirect=`, so signing back in returns the admin to the + // page they were on instead of dropping them on the dashboard. + const pathname = (await headers()).get(VITNODE_PATHNAME_HEADER); + await redirect(getAdminSignInHref(pathname)); return; } diff --git a/packages/vitnode/src/lib/i18n/proxy.test.ts b/packages/vitnode/src/lib/i18n/proxy.test.ts new file mode 100644 index 000000000..f3f425b00 --- /dev/null +++ b/packages/vitnode/src/lib/i18n/proxy.test.ts @@ -0,0 +1,44 @@ +import { NextRequest, NextResponse } from "next/server"; +import { describe, expect, it, vi } from "vitest"; + +import { VITNODE_PATHNAME_HEADER } from "../request-pathname"; +import { createVitNodeProxy } from "./proxy"; + +let stamped: null | string = null; + +vi.mock("next-intl/middleware", () => ({ + default: () => (request: NextRequest) => { + stamped = request.headers.get(VITNODE_PATHNAME_HEADER); + + return NextResponse.next(); + }, +})); + +const proxy = createVitNodeProxy({ + defaultLocale: "en", + locales: [ + { code: "en", name: "English" }, + { code: "pl", name: "Polski" }, + ], +}); + +/** The path next-intl - and with it the app - is handed for `url`. */ +const stampedPathname = (url: string) => { + proxy(new NextRequest(url)); + + return stamped; +}; + +describe("createVitNodeProxy", () => { + it("stamps the requested path, query string included", () => { + expect( + stampedPathname("https://vitnode.test/admin/core/users?page=2"), + ).toBe("/admin/core/users?page=2"); + }); + + it("strips the locale prefix, so the path can be localized again later", () => { + expect(stampedPathname("https://vitnode.test/pl/admin/core")).toBe( + "/admin/core", + ); + }); +}); diff --git a/packages/vitnode/src/lib/i18n/proxy.ts b/packages/vitnode/src/lib/i18n/proxy.ts index 30e85c443..9ea0adeba 100644 --- a/packages/vitnode/src/lib/i18n/proxy.ts +++ b/packages/vitnode/src/lib/i18n/proxy.ts @@ -1,7 +1,14 @@ +import type { NextRequest } from "next/server"; + import createMiddleware from "next-intl/middleware"; import type { VitNodeI18nConfig } from "./types"; +import { + stripLocalePrefix, + VITNODE_PATHNAME_HEADER, +} from "../request-pathname"; + /** * The next-intl Proxy (middleware), built from the app's i18n config alone. * @@ -10,10 +17,27 @@ import type { VitNodeI18nConfig } from "./types"; * through the request config - cannot be imported. Keeping `vitnode.config.ts` * out of the Proxy's module graph is what lets the request config read root * params at all. + * + * It also stamps {@link VITNODE_PATHNAME_HEADER} onto the request, which is the + * only place the requested path is still intact: next-intl rewrites the URL + * from here on, and it copies the request headers onto the response it forwards, + * so a Server Component can read the header back out of `headers()`. */ -export const createVitNodeProxy = (i18n: VitNodeI18nConfig) => - createMiddleware({ - locales: i18n.locales.map(locale => locale.code), +export const createVitNodeProxy = (i18n: VitNodeI18nConfig) => { + const locales = i18n.locales.map(locale => locale.code); + const proxy = createMiddleware({ + locales, defaultLocale: i18n.defaultLocale, localePrefix: i18n.localePrefix ?? "as-needed", }); + + return (request: NextRequest) => { + request.headers.set( + VITNODE_PATHNAME_HEADER, + stripLocalePrefix(request.nextUrl.pathname, locales) + + request.nextUrl.search, + ); + + return proxy(request); + }; +}; diff --git a/packages/vitnode/src/lib/request-pathname.test.ts b/packages/vitnode/src/lib/request-pathname.test.ts new file mode 100644 index 000000000..453b42dae --- /dev/null +++ b/packages/vitnode/src/lib/request-pathname.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; + +import { stripLocalePrefix } from "./request-pathname"; + +describe("stripLocalePrefix", () => { + const locales = ["en", "pl"]; + + it("drops a leading locale segment", () => { + expect(stripLocalePrefix("/pl/admin/core", locales)).toBe("/admin/core"); + expect(stripLocalePrefix("/en", locales)).toBe("/"); + }); + + it("leaves an unprefixed path alone", () => { + expect(stripLocalePrefix("/admin/core", locales)).toBe("/admin/core"); + expect(stripLocalePrefix("/", locales)).toBe("/"); + }); + + it("only matches whole segments", () => { + expect(stripLocalePrefix("/entries/1", locales)).toBe("/entries/1"); + expect(stripLocalePrefix("/admin/en/core", locales)).toBe("/admin/en/core"); + }); +}); diff --git a/packages/vitnode/src/lib/request-pathname.ts b/packages/vitnode/src/lib/request-pathname.ts new file mode 100644 index 000000000..bc3e8e5df --- /dev/null +++ b/packages/vitnode/src/lib/request-pathname.ts @@ -0,0 +1,33 @@ +/** + * Request header the Proxy stamps with the path the visitor actually asked for + * - pathname plus query string, locale prefix removed. + * + * A Server Component never sees that path on its own: by the time it renders, + * the Proxy has rewritten the URL to the internal, locale-prefixed one, and + * nothing in `headers()` carries the original. Anything that has to know where + * the visitor was - sending an admin back to the page they were on after their + * session expired, for one - reads it from here. + */ +export const VITNODE_PATHNAME_HEADER = "x-vitnode-pathname"; + +/** + * Drops a leading locale segment, so `/pl/admin/core` and `/admin/core` both + * come out as `/admin/core`. + * + * Stored paths stay locale-less on purpose: `Link` and `redirect` from + * `@/lib/navigation` add the current locale back themselves, and a path that + * already carried one would end up prefixed twice. + */ +export const stripLocalePrefix = ( + pathname: string, + locales: string[], +): string => { + for (const locale of locales) { + if (pathname === `/${locale}`) return "/"; + if (pathname.startsWith(`/${locale}/`)) { + return pathname.slice(locale.length + 1); + } + } + + return pathname; +}; diff --git a/packages/vitnode/src/views/admin/sign-in/sign-in-admin-view.tsx b/packages/vitnode/src/views/admin/sign-in/sign-in-admin-view.tsx index 2ebd90675..eef399184 100644 --- a/packages/vitnode/src/views/admin/sign-in/sign-in-admin-view.tsx +++ b/packages/vitnode/src/views/admin/sign-in/sign-in-admin-view.tsx @@ -1,15 +1,31 @@ import { I18nProvider } from "@/components/i18n-provider"; import { LogoVitNode } from "@/components/logo-vitnode"; import { Card } from "@/components/ui/card"; +import { + ADMIN_REDIRECT_PARAM, + sanitizeAdminRedirect, +} from "@/lib/admin-redirect"; import { FormSignIn } from "@/views/auth/sign-in/form/form"; -export const SignInAdminView = () => { +export const SignInAdminView = async ({ + searchParams, +}: { + searchParams?: Promise>; +}) => { + const redirectParam = (await searchParams)?.[ADMIN_REDIRECT_PARAM]; + return (
- +
diff --git a/packages/vitnode/src/views/auth/sign-in/form/form.tsx b/packages/vitnode/src/views/auth/sign-in/form/form.tsx index 7cae30a4a..f640e2a59 100644 --- a/packages/vitnode/src/views/auth/sign-in/form/form.tsx +++ b/packages/vitnode/src/views/auth/sign-in/form/form.tsx @@ -15,13 +15,19 @@ import { useFormSignIn } from "./use-form"; export const FormSignIn = ({ isAdmin, isEmail, + redirectTo, }: { isAdmin?: boolean; isEmail: boolean; + /** AdminCP path to return to after signing in, from `?redirect=`. */ + redirectTo?: string; }) => { const t = useTranslations("core.auth.sign_in"); const shouldReduceMotion = useReducedMotion(); - const { onSubmit, error, formSchema } = useFormSignIn({ isAdmin }); + const { onSubmit, error, formSchema } = useFormSignIn({ + isAdmin, + redirectTo, + }); return (
diff --git a/packages/vitnode/src/views/auth/sign-in/form/mutation-api.server.ts b/packages/vitnode/src/views/auth/sign-in/form/mutation-api.server.ts index ffcf66c8f..08126eb59 100644 --- a/packages/vitnode/src/views/auth/sign-in/form/mutation-api.server.ts +++ b/packages/vitnode/src/views/auth/sign-in/form/mutation-api.server.ts @@ -7,14 +7,17 @@ import { revalidatePath } from "next/cache"; import type { zodSignInSchema } from "@/api/modules/users/routes/sign-in.route"; import { usersModule } from "@/api/modules/users/users.module"; +import { ADMIN_HOME_PATH, sanitizeAdminRedirect } from "@/lib/admin-redirect"; import { fetcher } from "@/lib/fetcher"; import { redirect } from "@/lib/navigation"; -export const mutationApi = async ( - input: z.infer & { - isAdmin?: boolean; - }, -) => { +export const mutationApi = async ({ + redirectTo, + ...input +}: z.infer & { + isAdmin?: boolean; + redirectTo?: string; +}) => { const res = await fetcher(usersModule, { path: "/sign_in", method: "post", @@ -35,7 +38,9 @@ export const mutationApi = async ( if (input.isAdmin) { revalidatePath("/[locale]/admin", "layout"); - await redirect("/admin/core"); + // `redirectTo` crossed the network as part of the form payload, so it is + // sanitised again here rather than trusted from the page that rendered it. + await redirect(sanitizeAdminRedirect(redirectTo) ?? ADMIN_HOME_PATH); return; } diff --git a/packages/vitnode/src/views/auth/sign-in/form/use-form.ts b/packages/vitnode/src/views/auth/sign-in/form/use-form.ts index 6d6d67a03..b18944a9b 100644 --- a/packages/vitnode/src/views/auth/sign-in/form/use-form.ts +++ b/packages/vitnode/src/views/auth/sign-in/form/use-form.ts @@ -7,7 +7,13 @@ import type { AutoFormOnSubmit } from "@/components/form/auto-form"; import { mutationApi } from "./mutation-api.server"; -export const useFormSignIn = ({ isAdmin = false }: { isAdmin?: boolean }) => { +export const useFormSignIn = ({ + isAdmin = false, + redirectTo, +}: { + isAdmin?: boolean; + redirectTo?: string; +}) => { const [error, setError] = React.useState<"" | "access_denied">(""); const t = useTranslations<"core.auth.sign_in">("core.auth.sign_in"); const tErrors = useTranslations("core.global.errors"); @@ -21,7 +27,7 @@ export const useFormSignIn = ({ isAdmin = false }: { isAdmin?: boolean }) => { const onSubmit: AutoFormOnSubmit = async values => { setError(""); - const mutation = await mutationApi({ ...values, isAdmin }); + const mutation = await mutationApi({ ...values, isAdmin, redirectTo }); if (!mutation?.message) return; if (mutation?.message !== "Internal Server Error") {