diff --git a/messages/en-US.json b/messages/en-US.json index 0961a81dc..9d9f0633f 100644 --- a/messages/en-US.json +++ b/messages/en-US.json @@ -4392,6 +4392,7 @@ "redirectMatchPathDescription": "Which incoming paths this redirect applies to", "redirectRewritePathDescription": "Optionally change the path before redirecting. Leave unset to keep the original path.", "redirectRewritePathRequired": "Enter a rewrite path, or choose Strip Prefix", + "redirectMatchPathInvalidRegex": "Match path must be a valid regular expression", "redirectCreate": "Create Redirect", "redirectCreateDescription": "Forward requests matching a path to another URL", "redirectEditDescription": "Update how this redirect forwards incoming requests", diff --git a/server/db/pg/schema/schema.ts b/server/db/pg/schema/schema.ts index 4540546be..1783140ff 100644 --- a/server/db/pg/schema/schema.ts +++ b/server/db/pg/schema/schema.ts @@ -248,7 +248,7 @@ export const redirects = pgTable("redirects", { .$type<"exact" | "prefix" | "regex">() .notNull() .default("regex"), // exact, prefix, regex - matchPath: varchar("matchPath").notNull().default(".*"), + matchPath: varchar("matchPath"), rewritePath: varchar("rewritePath"), // if set, rewrites the path to this value, // else, the original path will be kept rewritePathType: varchar("rewritePathType").$type< diff --git a/server/db/sqlite/schema/schema.ts b/server/db/sqlite/schema/schema.ts index b29a83f71..75231f083 100644 --- a/server/db/sqlite/schema/schema.ts +++ b/server/db/sqlite/schema/schema.ts @@ -264,13 +264,13 @@ export const redirects = sqliteTable("redirects", { .$type<"exact" | "prefix" | "regex">() .notNull() .default("regex"), // exact, prefix, regex - matchPath: text("matchPath").notNull().default("*"), + matchPath: text("matchPath"), rewritePath: text("rewritePath"), // if set, rewrites the path to this value, // else, the original path will be kept rewritePathType: text("rewritePathType").$type< "exact" | "prefix" | "regex" | "stripPrefix" >(), // exact, prefix, regex, stripPrefix - + priority: integer("priority").default(100), permanent: integer("permanent", { mode: "boolean" }) .notNull() .default(false), diff --git a/server/lib/traefik/redirect.ts b/server/lib/traefik/redirect.ts index 7af82549a..8f1b81c7e 100644 --- a/server/lib/traefik/redirect.ts +++ b/server/lib/traefik/redirect.ts @@ -11,25 +11,24 @@ export type RedirectRouteRow = { /** Host the redirect listens on (resource fullDomain or subdomain.baseDomain). */ fullDomain: string; hasSubdomain: boolean; + attachedTo: "resource" | "domain"; + enabled: boolean; + name: string; wildcard: boolean | null; ssl: boolean; - matchPath: string; + matchPath: string | null; pathMatchType: string; priority: number | null; domainCertResolver?: string | null; preferWildcardCert?: boolean | null; }; -/** - * Add Traefik routers for redirects. Like resources, every request is sent - * through badger, which looks up the redirect by host/path, applies any - * path rewrite and answers with the redirect itself - Traefik only has to - * match the host (+ path) and terminate TLS. Redirects have no backend, so - * the routers point at Traefik's built-in noop@internal service. - * TLS/cert-resolver handling differs between the OSS and private - * (pangolin-dns aware) config generators, so callers resolve that via - * resolveTls - returning null skips the redirect (no valid cert yet). - */ +// Traefik requires a service on every router, but a redirect router's +// middleware chain always terminates the request with a 30x, so the service +// is never reached. noop@internal answers 418 if it ever is - treat that as +// a bug in the middleware chain, not something to route around. +const NOOP_SERVICE = "noop@internal"; + export function buildRedirectConfig(params: { config_output: any; redirects: RedirectRouteRow[]; @@ -56,7 +55,18 @@ export function buildRedirectConfig(params: { const routerMiddlewares = [badgerMiddlewareName, ...additionalMiddlewares]; for (const redirect of redirects) { - const routerName = `redirect-${redirect.redirectId}-router`; + const routerName = `${redirect.redirectId}-redirect-${redirect.name}-router`; + + logger.debug( + `Processing redirect ${redirect.name} with domain ${redirect.fullDomain}` + ); + + if (!redirect.enabled) { + logger.debug( + `Redirect ${redirect.name} is disabled, skipping Traefik config` + ); + continue; + } let tls: any = {}; if (redirect.ssl) { @@ -70,7 +80,7 @@ export function buildRedirectConfig(params: { config_output.http.routers = {}; } - if (redirect.pathMatchType === "regex") { + if (redirect.matchPath && redirect.pathMatchType === "regex") { try { new RegExp(redirect.matchPath); } catch { @@ -99,11 +109,13 @@ export function buildRedirectConfig(params: { redirect.pathMatchType ) + (hasExplicitPriority ? 0 : 1); - if (redirect.ssl) { + // if resource is already attached to resource, we don't need to add the https redirect + // as it is already added in the resource traefik config + if (redirect.attachedTo !== "resource" && redirect.ssl) { config_output.http.routers[`${routerName}-redirect`] = { entryPoints: [httpEntrypoint], middlewares: [redirectHttpsMiddlewareName], - service: "noop@internal", + service: NOOP_SERVICE, rule, priority }; @@ -112,7 +124,7 @@ export function buildRedirectConfig(params: { config_output.http.routers[routerName] = { entryPoints: [redirect.ssl ? httpsEntrypoint : httpEntrypoint], middlewares: routerMiddlewares, - service: "noop@internal", + service: NOOP_SERVICE, rule, priority, ...(redirect.ssl ? { tls } : {}) diff --git a/server/lib/traefik/types.ts b/server/lib/traefik/types.ts index ab19ee653..122746419 100644 --- a/server/lib/traefik/types.ts +++ b/server/lib/traefik/types.ts @@ -1,8 +1,8 @@ -import { Target } from "@server/db"; +import type { Domain, Resource, Target } from "@server/db"; -// Extended target type with site information, shared between the OSS and +// Target subset with site information, shared between the OSS and // private getTraefikConfig implementations. -export type TargetWithSite = Target & { +export type TargetWithSite = { resourceId: number; targetId: number; ip: string | null; @@ -19,3 +19,42 @@ export type TargetWithSite = Target & { online: boolean; }; }; + +// A resource grouped with its targets for router/service generation. Every +// target in a group shares the same path/rewrite config, so those columns +// live on the resource rather than on each target. +export type ResourceWithTargets = Pick< + Resource, + | "resourceId" + | "fullDomain" + | "ssl" + | "proxyPort" + | "subdomain" + | "domainId" + | "enabled" + | "stickySession" + | "tlsServerName" + | "setHostHeader" + | "enableProxy" + | "headers" + | "proxyProtocol" + | "wildcard" + | "mode" + | "maintenanceModeEnabled" + | "maintenanceModeType" + | "maintenanceTitle" + | "maintenanceMessage" + | "maintenanceEstimatedTime" +> & + Pick & { + /** Sanitized resource name used in router/service names */ + name: string; + /** Sanitized resourceId + path config, unique per router */ + key: string; + priority: number; + proxyProtocolVersion: number; + // Left-joined from the resource's domain, so absent when there is none + domainCertResolver: Domain["certResolver"] | null; + preferWildcardCert: Domain["preferWildcardCert"] | null; + targets: TargetWithSite[]; + }; diff --git a/server/private/lib/traefik/getTraefikConfig.ts b/server/private/lib/traefik/getTraefikConfig.ts index 828b3bda2..ed45a05b2 100644 --- a/server/private/lib/traefik/getTraefikConfig.ts +++ b/server/private/lib/traefik/getTraefikConfig.ts @@ -56,7 +56,7 @@ import { } from "@server/lib/certificates"; import { build } from "@server/build"; import regionalCache from "#private/lib/cache"; -import { TargetWithSite } from "@server/lib/traefik/types"; +import { ResourceWithTargets } from "@server/lib/traefik/types"; import { buildWildcardTls } from "@server/lib/traefik/certResolver"; import { buildHostRule, @@ -218,7 +218,7 @@ export async function getTraefikConfig( .orderBy(desc(targets.priority), targets.targetId); // stable ordering // Group by resource and include targets with their unique site data - const resourcesMap = new Map(); + const resourcesMap = new Map(); for (const row of resourcesWithTargetsAndSites) { if (!["http", "tcp", "udp"].includes(row.mode)) { @@ -246,7 +246,7 @@ export async function getTraefikConfig( .filter(Boolean) .join("-"); const mapKey = [resourceId, pathKey].filter(Boolean).join("-"); - const key = sanitize(mapKey); + const key = sanitize(mapKey) ?? ""; if (!resourcesMap.has(mapKey)) { const validation = validatePathRewriteConfig( @@ -300,7 +300,7 @@ export async function getTraefikConfig( } // Add target with its associated site data - resourcesMap.get(mapKey).targets.push({ + resourcesMap.get(mapKey)!.targets.push({ resourceId: row.resourceId, targetId: row.targetId, ip: row.ip, @@ -406,6 +406,8 @@ export async function getTraefikConfig( // domain; the domain join resolves to whichever one applies. const redirectRows = await db .select({ + name: redirects.name, + enabled: redirects.enabled, redirectId: redirects.redirectId, subdomain: redirects.subdomain, matchPath: redirects.matchPath, @@ -432,7 +434,10 @@ export async function getTraefikConfig( sql`coalesce(${redirects.domainId}, ${resources.domainId})` ) ) - .leftJoin(domainNamespaces, eq(domainNamespaces.domainId, domains.domainId)) + .leftJoin( + domainNamespaces, + eq(domainNamespaces.domainId, domains.domainId) + ) .where( and( eq(redirects.enabled, true), @@ -459,6 +464,8 @@ export async function getTraefikConfig( } redirectRoutes.push({ + enabled: row.enabled, + name: sanitize(row.name) || "", redirectId: row.redirectId, fullDomain, hasSubdomain: attachedToResource @@ -467,6 +474,7 @@ export async function getTraefikConfig( wildcard: row.resourceWildcard, // Domain-attached redirects always get a certificate on creation ssl: attachedToResource ? !!row.resourceSsl : true, + attachedTo: attachedToResource ? "resource" : "domain", matchPath: row.matchPath, pathMatchType: row.pathMatchType, priority: row.priority, @@ -475,6 +483,14 @@ export async function getTraefikConfig( }); } + console.dir( + { + redirectRoutes, + redirectRows + }, + { depth: null } + ); + let validCerts: CertificateResult[] = []; if (privateConfig.getRawPrivateConfig().flags.use_pangolin_dns) { // create a list of all domains to get certs for @@ -552,7 +568,7 @@ export async function getTraefikConfig( // get the key and the resource for (const [, resource] of resourcesMap.entries()) { - const targets = resource.targets as TargetWithSite[]; + const targets = resource.targets; const key = resource.key; const routerName = `${key}-${resource.name}-router`; diff --git a/server/routers/redirect/createRedirect.ts b/server/routers/redirect/createRedirect.ts index 97303066f..1131243e2 100644 --- a/server/routers/redirect/createRedirect.ts +++ b/server/routers/redirect/createRedirect.ts @@ -14,6 +14,7 @@ import { redirectMatchPathSchema, redirectPathMatchTypeSchema, redirectRewritePathSchema, + isValidMatchPath, redirectRewritePathTypeSchema } from "@server/routers/redirect/validation"; import { getUniqueRedirectName } from "@server/db/names"; @@ -35,7 +36,7 @@ const bodySchema = z subdomain: z.string().nonempty().optional().nullable(), destinationDomain: redirectDestinationDomainSchema, pathMatchType: redirectPathMatchTypeSchema.optional(), - matchPath: redirectMatchPathSchema, + matchPath: redirectMatchPathSchema.optional().nullable(), rewritePath: redirectRewritePathSchema.optional().nullable(), rewritePathType: redirectRewritePathTypeSchema.optional().nullable(), permanent: z.boolean().optional(), @@ -57,6 +58,10 @@ const bodySchema = z .refine((data) => Boolean(data.resourceId) !== Boolean(data.domainId), { message: "Exactly one of resourceId or domainId must be provided", path: ["resourceId"] + }) + .refine((data) => isValidMatchPath(data.matchPath, data.pathMatchType), { + message: "matchPath must be a valid regular expression", + path: ["matchPath"] }); registry.registerPath({ @@ -187,7 +192,7 @@ export async function createRedirect( subdomain: subdomain ?? null, destinationDomain, pathMatchType: pathMatchType ?? "regex", - matchPath, + matchPath: matchPath ?? null, rewritePath: rewritePath ?? null, rewritePathType: rewritePathType ?? null, permanent: permanent ?? false, diff --git a/server/routers/redirect/getRedirect.ts b/server/routers/redirect/getRedirect.ts index cba2048e9..f12a4b0af 100644 --- a/server/routers/redirect/getRedirect.ts +++ b/server/routers/redirect/getRedirect.ts @@ -19,7 +19,7 @@ export type GetRedirectResponse = { subdomain: string | null; destinationDomain: string; pathMatchType: "exact" | "prefix" | "regex"; - matchPath: string; + matchPath: string | null; rewritePath: string | null; rewritePathType: "exact" | "prefix" | "regex" | "stripPrefix" | null; permanent: boolean; diff --git a/server/routers/redirect/listRedirects.ts b/server/routers/redirect/listRedirects.ts index a6970e279..d90acd6f6 100644 --- a/server/routers/redirect/listRedirects.ts +++ b/server/routers/redirect/listRedirects.ts @@ -19,7 +19,7 @@ export type ListRedirectsResponse = PaginatedResponse<{ subdomain: string | null; destinationDomain: string; pathMatchType: "exact" | "prefix" | "regex"; - matchPath: string; + matchPath: string | null; rewritePath: string | null; rewritePathType: "exact" | "prefix" | "regex" | "stripPrefix" | null; permanent: boolean; diff --git a/server/routers/redirect/updateRedirect.ts b/server/routers/redirect/updateRedirect.ts index 59b87f59c..139db0e46 100644 --- a/server/routers/redirect/updateRedirect.ts +++ b/server/routers/redirect/updateRedirect.ts @@ -15,7 +15,8 @@ import { redirectMatchPathSchema, redirectPathMatchTypeSchema, redirectRewritePathSchema, - redirectRewritePathTypeSchema + redirectRewritePathTypeSchema, + isValidMatchPath } from "@server/routers/redirect/validation"; import { createCertificate } from "../certificates"; @@ -36,7 +37,7 @@ const bodySchema = z.strictObject({ subdomain: z.string().nonempty().optional().nullable(), destinationDomain: redirectDestinationDomainSchema.optional(), pathMatchType: redirectPathMatchTypeSchema.optional(), - matchPath: redirectMatchPathSchema.optional(), + matchPath: redirectMatchPathSchema.optional().nullable(), rewritePath: redirectRewritePathSchema.optional().nullable(), rewritePathType: redirectRewritePathTypeSchema.optional().nullable(), permanent: z.boolean().optional(), @@ -206,6 +207,22 @@ export async function updateRedirect( } } + if ( + !isValidMatchPath( + body.matchPath !== undefined + ? body.matchPath + : existing.matchPath, + body.pathMatchType ?? existing.pathMatchType + ) + ) { + return next( + createHttpError( + HttpCode.BAD_REQUEST, + "matchPath must be a valid regular expression" + ) + ); + } + const updateData: Partial = {}; if (body.name !== undefined) { diff --git a/server/routers/redirect/validation.ts b/server/routers/redirect/validation.ts index 7c9122fab..93129eaf8 100644 --- a/server/routers/redirect/validation.ts +++ b/server/routers/redirect/validation.ts @@ -19,7 +19,29 @@ export const redirectRewritePathTypeSchema = z.enum([ "stripPrefix" ]); -export const redirectMatchPathSchema = z.string().nonempty().default("*"); +export const redirectMatchPathSchema = z.string().nonempty(); + +export function isValidRegex(pattern: string): boolean { + try { + new RegExp(pattern); + return true; + } catch { + return false; + } +} + +/** + * A regex match path is fed straight to `new RegExp` when building routes, + * so reject patterns that would throw there. + */ +export function isValidMatchPath( + matchPath: string | null | undefined, + pathMatchType: string | null | undefined +): boolean { + return ( + pathMatchType !== "regex" || !matchPath || isValidRegex(matchPath) + ); +} export const redirectRewritePathSchema = z.string().nonempty(); diff --git a/src/components/RedirectForm.tsx b/src/components/RedirectForm.tsx index 827018ee8..c024e560a 100644 --- a/src/components/RedirectForm.tsx +++ b/src/components/RedirectForm.tsx @@ -41,6 +41,7 @@ import { useEnvContext } from "@app/hooks/useEnvContext"; import { toast } from "@app/hooks/useToast"; import { createApiClient, formatAxiosError } from "@app/lib/api"; import { isValidDomain } from "@server/lib/validators"; +import { isValidRegex } from "@server/routers/redirect/validation"; import { cn } from "@app/lib/cn"; import { CaretSortIcon } from "@radix-ui/react-icons"; import { zodResolver } from "@hookform/resolvers/zod"; @@ -65,7 +66,6 @@ import { Plus } from "lucide-react"; import DomainPicker from "@app/components/DomainPicker"; import Link from "next/link"; -const DEFAULT_MATCH_PATH = ".*"; const DEFAULT_PATH_MATCH_TYPE = "regex" as const; export type ExistingRedirect = GetRedirectResponse["redirect"]; @@ -127,7 +127,7 @@ export default function RedirectForm({ message: t("redirectDestinationDomainInvalid") }), pathMatchType: z.enum(["exact", "prefix", "regex"]), - matchPath: z.string().trim().min(1), + matchPath: z.string().trim().nullable(), rewritePath: z.string().nullable(), rewritePathType: z .enum(["exact", "prefix", "regex", "stripPrefix"]) @@ -150,6 +150,17 @@ export default function RedirectForm({ path: ["resourceId"] }); } + if ( + data.pathMatchType === "regex" && + data.matchPath && + !isValidRegex(data.matchPath) + ) { + ctx.addIssue({ + code: "custom", + message: t("redirectMatchPathInvalidRegex"), + path: ["matchPath"] + }); + } // stripPrefix drops the matched prefix outright, so it is // the one rewrite type that needs no replacement value. if ( @@ -179,7 +190,7 @@ export default function RedirectForm({ resourceId: redirect?.resourceId ?? null, destinationDomain: redirect?.destinationDomain ?? "", pathMatchType: redirect?.pathMatchType ?? DEFAULT_PATH_MATCH_TYPE, - matchPath: redirect?.matchPath ?? DEFAULT_MATCH_PATH, + matchPath: redirect?.matchPath ?? null, rewritePath: redirect?.rewritePath ?? null, rewritePathType: redirect?.rewritePathType ?? null, permanent: redirect?.permanent ?? false, @@ -234,7 +245,7 @@ export default function RedirectForm({ values.attachTo === "resource" ? values.resourceId : null, destinationDomain: values.destinationDomain.trim(), pathMatchType: values.pathMatchType, - matchPath: values.matchPath.trim(), + matchPath: values.matchPath?.trim() || null, rewritePath: values.rewritePath?.trim() || null, rewritePathType: values.rewritePathType, permanent: values.permanent, @@ -650,15 +661,16 @@ export default function RedirectForm({ onChange={( config ) => { - // matchPath and - // pathMatchType are - // NOT NULL, so a - // clear falls back - // to the defaults - // rather than null. + // No match path + // means the + // redirect applies + // to every path; + // pathMatchType is + // NOT NULL so it + // keeps a default. field.onChange( config.path || - DEFAULT_MATCH_PATH + null ); form.setValue( "pathMatchType", @@ -675,13 +687,22 @@ export default function RedirectForm({ variant="outline" className="flex items-center gap-2 p-2 w-full text-left cursor-pointer" > - + {field.value ? ( + + ) : ( + <> + + {t( + "matchPath" + )} + + )} } /> diff --git a/src/components/RedirectsTable.tsx b/src/components/RedirectsTable.tsx index e0b2c2802..2ecc28205 100644 --- a/src/components/RedirectsTable.tsx +++ b/src/components/RedirectsTable.tsx @@ -31,7 +31,15 @@ import { import { useTranslations } from "next-intl"; import Link from "next/link"; import { useRouter } from "next/navigation"; -import { useEffect, useMemo, useState, useTransition } from "react"; +import { + useEffect, + useMemo, + useOptimistic, + useRef, + useState, + useTransition, + type ComponentRef +} from "react"; import { useDebouncedCallback } from "use-debounce"; export type RedirectRow = { @@ -41,7 +49,7 @@ export type RedirectRow = { subdomain: string | null; destinationDomain: string; pathMatchType: "exact" | "prefix" | "regex"; - matchPath: string; + matchPath: string | null; rewritePath: string | null; rewritePathType: "exact" | "prefix" | "regex" | "stripPrefix" | null; permanent: boolean; @@ -121,15 +129,9 @@ export default function RedirectsTable({ return path.endsWith("/") ? `${path}*` : `${path}/*`; } - async function toggleEnabled(row: RedirectRow, enabled: boolean) { - setRows((prev) => - prev.map((r) => - r.redirectId === row.redirectId ? { ...r, enabled } : r - ) - ); - + async function toggleEnabled(enabled: boolean, redirectId: number) { try { - await api.post(`/org/${orgId}/redirects/${row.redirectId}`, { + await api.post(`/org/${orgId}/redirects/${redirectId}`, { enabled }); toast({ @@ -138,13 +140,6 @@ export default function RedirectsTable({ }); router.refresh(); } catch (e) { - setRows((prev) => - prev.map((r) => - r.redirectId === row.redirectId - ? { ...r, enabled: row.enabled } - : r - ) - ); toast({ variant: "destructive", title: t("redirectErrorUpdate"), @@ -288,11 +283,15 @@ export default function RedirectsTable({ ) : null} {host ?? ""} - - {redirect.pathMatchType === "prefix" - ? withPrefixGlob(redirect.matchPath) - : redirect.matchPath} - + {redirect.matchPath && ( + + {redirect.pathMatchType === "prefix" + ? withPrefixGlob( + redirect.matchPath + ) + : redirect.matchPath} + + )} ); @@ -338,11 +337,9 @@ export default function RedirectsTable({ friendlyName: t("enabled"), header: () => {t("enabled")}, cell: ({ row }) => ( - - toggleEnabled(row.original, checked) - } + ) }, @@ -451,3 +448,36 @@ export default function RedirectsTable({ ); } + +type RedirectEnabledFormProps = { + redirect: RedirectRow; + onToggleEnabled: (val: boolean, redirectId: number) => Promise; +}; + +function RedirectEnabledForm({ + redirect, + onToggleEnabled +}: RedirectEnabledFormProps) { + const [optimisticEnabled, setOptimisticEnabled] = useOptimistic( + redirect.enabled + ); + + const formRef = useRef>(null); + + async function submitAction(formData: FormData) { + const newEnabled = !(formData.get("enabled") === "on"); + setOptimisticEnabled(newEnabled); + await onToggleEnabled(newEnabled, redirect.redirectId); + } + + return ( +
+ formRef.current?.requestSubmit()} + /> + + ); +}