check idp org ownership on save policy closes #3290

Fix #3484

server/lib/blueprints/resourcePolicies.ts

@@ -1,7 +1,5 @@
 import {
     db,
-    idp,
-    idpOrg,
     resourcePolicies,
     resourcePolicyHeaderAuth,
     resourcePolicyPassword,
@@ -20,6 +18,7 @@ import { Config, ResourcePolicyData } from "./types";
 import logger from "@server/logger";
 import { getUniqueResourcePolicyName } from "@server/db/names";
 import { hashPassword } from "@server/auth/password";
+import { idpExistsForOrg } from "@server/lib/idp/idpExistsForOrg";
 import { isValidCIDR, isValidIP, isValidUrlGlobPattern } from "../validators";
 import { isLicensedOrSubscribed } from "#dynamic/lib/isLicencedOrSubscribed";
 import { tierMatrix } from "../billing/tierMatrix";
@@ -71,19 +70,13 @@ export async function updateResourcePolicies(
 
         // Validate auto-login-idp if provided
         if (policyData["auto-login-idp"]) {
-            const [provider] = await trx
-                .select()
-                .from(idp)
-                .innerJoin(idpOrg, eq(idpOrg.idpId, idp.idpId))
-                .where(
-                    and(
-                        eq(idp.idpId, policyData["auto-login-idp"]),
-                        eq(idpOrg.orgId, orgId)
-                    )
-                )
-                .limit(1);
+            const providerExists = await idpExistsForOrg(
+                policyData["auto-login-idp"],
+                orgId,
+                trx
+            );
 
-            if (!provider) {
+            if (!providerExists) {
                 throw new Error(
                     `Identity provider not found for policy '${policyNiceId}' in this organization`
                 );

server/lib/blueprints/types.ts

@@ -28,7 +28,7 @@ export const TargetHealthCheckSchema = z.object({
     hostname: z.string(),
     port: z.int().min(1).max(65535),
     enabled: z.boolean().optional().default(true),
-    path: z.string().optional(),
+    path: z.string().optional().default("/"),
     scheme: z.string().optional(),
     mode: z.string().default("http"),
     interval: z.int().default(30),

server/lib/certificates.ts

@@ -5,6 +5,7 @@ export async function getValidCertificatesForDomains(
     Array<{
         id: number;
         domain: string;
+        queriedDomain: string;
         wildcard: boolean | null;
         certFile: string | null;
         keyFile: string | null;

server/lib/clientVersionChecks.ts

@@ -18,3 +18,19 @@ export function canCompress(
         return false;
     }
 }
+
+// Whether this newt client understands `tlsCertId` references into the
+// sync message's `certs` array, instead of requiring each target to carry
+// its own inline `tlsCert`/`tlsKey` PEM data. Bump the version floor here to
+// match whatever release first ships the newt-side support.
+export function supportsCertReferences(
+    clientVersion: string | null | undefined
+): boolean {
+    try {
+        if (!clientVersion) return false;
+        if (!semver.valid(clientVersion)) return false;
+        return semver.gte(clientVersion, "1.16.0");
+    } catch {
+        return false;
+    }
+}

server/lib/idp/idpExistsForOrg.ts

@@ -0,0 +1,36 @@
+import { db, idp, idpOrg, Transaction } from "@server/db";
+import { and, eq } from "drizzle-orm";
+
+export function isOrgIdentityProviderMode(): boolean {
+    return process.env.IDENTITY_PROVIDER_MODE === "org";
+}
+
+/**
+ * Checks whether an identity provider can be used for the given org.
+ * In org IdP mode, the provider must be linked via idpOrg.
+ * In global IdP mode, the provider only needs to exist.
+ */
+export async function idpExistsForOrg(
+    idpId: number,
+    orgId: string,
+    dbOrTrx: typeof db | Transaction = db
+): Promise<boolean> {
+    if (isOrgIdentityProviderMode()) {
+        const [provider] = await dbOrTrx
+            .select({ idpId: idp.idpId })
+            .from(idp)
+            .innerJoin(idpOrg, eq(idpOrg.idpId, idp.idpId))
+            .where(and(eq(idp.idpId, idpId), eq(idpOrg.orgId, orgId)))
+            .limit(1);
+
+        return !!provider;
+    }
+
+    const [provider] = await dbOrTrx
+        .select({ idpId: idp.idpId })
+        .from(idp)
+        .where(eq(idp.idpId, idpId))
+        .limit(1);
+
+    return !!provider;
+}

server/lib/ip.ts

@@ -5,6 +5,7 @@ import config from "@server/lib/config";
 import z from "zod";
 import logger from "@server/logger";
 import semver from "semver";
+import { createHash } from "crypto";
 import { getValidCertificatesForDomains } from "#dynamic/lib/certificates";
 import { lockManager } from "#dynamic/lib/lock";
 
@@ -648,21 +649,101 @@ export type SubnetProxyTargetV2 = {
     httpTargets?: HTTPTarget[];
     tlsCert?: string;
     tlsKey?: string;
+    tlsCertId?: string; // references an entry in the sync message's top-level `certs` array instead of inlining tlsCert/tlsKey
 };
 
+export type CertRef = { id: string; cert: string; key: string };
+
+/**
+ * Replaces each target's inline tlsCert/tlsKey with a tlsCertId reference
+ * into a deduplicated certs array, so that many targets sharing the same
+ * certificate (e.g. a wildcard cert used by thousands of site resources)
+ * only need that certificate sent once per sync message.
+ */
+export function dedupeCertsForTargets(
+    targetsV2: SubnetProxyTargetV2[]
+): { targets: SubnetProxyTargetV2[]; certs: CertRef[] } {
+    const idByContent = new Map<string, string>();
+    const certs: CertRef[] = [];
+
+    const targets = targetsV2.map((target) => {
+        if (!target.tlsCert || !target.tlsKey) {
+            return target;
+        }
+
+        const contentKey = `${target.tlsCert}|${target.tlsKey}`;
+        let id = idByContent.get(contentKey);
+        if (!id) {
+            id = createHash("sha1").update(contentKey).digest("hex").slice(0, 16);
+            idByContent.set(contentKey, id);
+            certs.push({ id, cert: target.tlsCert, key: target.tlsKey });
+        }
+
+        const { tlsCert, tlsKey, ...rest } = target;
+        return { ...rest, tlsCertId: id };
+    });
+
+    return { targets, certs };
+}
+
 export type HTTPTarget = {
     destAddr: string; // must be an IP or hostname
     destPort: number;
     scheme: "http" | "https";
 };
 
+export type CertByDomain = Map<string, { certFile: string; keyFile: string }>;
+
+/**
+ * Fetches the TLS certificates for every enabled, SSL-enabled HTTP site
+ * resource's fullDomain in a single batched call, instead of one call per
+ * resource. Many resources commonly resolve to the very same certificate
+ * (e.g. a wildcard covering the org's domain), so batching turns what would
+ * be N concurrent DB/cache round-trips into one, and a lookup failure fails
+ * loudly for the whole batch rather than silently dropping the cert on a
+ * random subset of otherwise-identical resources under load.
+ */
+export async function batchFetchCertsForSiteResources(
+    allSiteResources: SiteResource[]
+): Promise<CertByDomain> {
+    const domains = new Set(
+        allSiteResources
+            .filter((r) => r.enabled && r.mode === "http" && r.ssl && r.fullDomain)
+            .map((r) => r.fullDomain as string)
+    );
+
+    const certByDomain: CertByDomain = new Map();
+    if (domains.size === 0) {
+        return certByDomain;
+    }
+
+    try {
+        const certResults = await getValidCertificatesForDomains(domains, true);
+        for (const cert of certResults) {
+            if (cert.certFile && cert.keyFile) {
+                certByDomain.set(cert.queriedDomain, {
+                    certFile: cert.certFile,
+                    keyFile: cert.keyFile
+                });
+            }
+        }
+    } catch (err) {
+        logger.error(
+            `Failed to batch-retrieve certificates for ${domains.size} domain(s): ${err}`
+        );
+    }
+
+    return certByDomain;
+}
+
 export async function generateSubnetProxyTargetV2(
     siteResource: SiteResource,
     clients: {
         clientId: number;
         pubKey: string | null;
         subnet: string | null;
-    }[]
+    }[],
+    certByDomain?: CertByDomain
 ): Promise<SubnetProxyTargetV2[] | undefined> {
     if (!siteResource.enabled) {
         logger.debug(
@@ -750,23 +831,40 @@ export async function generateSubnetProxyTargetV2(
         let tlsKey: string | undefined;
 
         if (siteResource.ssl && siteResource.fullDomain) {
-            try {
-                const certs = await getValidCertificatesForDomains(
-                    new Set([siteResource.fullDomain]),
-                    true
-                );
-                if (certs.length > 0 && certs[0].certFile && certs[0].keyFile) {
-                    tlsCert = certs[0].certFile;
-                    tlsKey = certs[0].keyFile;
+            if (certByDomain) {
+                // Caller batch-fetched certs for all resources up front (the
+                // common, high-scale path) — just look up this resource's
+                // domain rather than issuing its own DB/cache round-trip.
+                const cert = certByDomain.get(siteResource.fullDomain);
+                if (cert) {
+                    tlsCert = cert.certFile;
+                    tlsKey = cert.keyFile;
                 } else {
                     logger.warn(
                         `No valid certificate found for SSL site resource ${siteResource.siteResourceId} with domain ${siteResource.fullDomain}`
                     );
                 }
-            } catch (err) {
-                logger.error(
-                    `Failed to retrieve certificate for site resource ${siteResource.siteResourceId} domain ${siteResource.fullDomain}: ${err}`
-                );
+            } else {
+                // No batched map supplied by the caller — fall back to a
+                // single-domain lookup for this resource alone.
+                try {
+                    const certs = await getValidCertificatesForDomains(
+                        new Set([siteResource.fullDomain]),
+                        true
+                    );
+                    if (certs.length > 0 && certs[0].certFile && certs[0].keyFile) {
+                        tlsCert = certs[0].certFile;
+                        tlsKey = certs[0].keyFile;
+                    } else {
+                        logger.warn(
+                            `No valid certificate found for SSL site resource ${siteResource.siteResourceId} with domain ${siteResource.fullDomain}`
+                        );
+                    }
+                } catch (err) {
+                    logger.error(
+                        `Failed to retrieve certificate for site resource ${siteResource.siteResourceId} domain ${siteResource.fullDomain}: ${err}`
+                    );
+                }
             }
         }
 

server/private/routers/policy/createResourcePolicy.ts

@@ -14,8 +14,6 @@
 import { hashPassword } from "@server/auth/password";
 import {
     db,
-    idp,
-    idpOrg,
     orgs,
     resourcePolicies,
     resourcePolicyHeaderAuth,
@@ -31,6 +29,7 @@ import {
     type ResourcePolicy
 } from "@server/db";
 import { getUniqueResourcePolicyName } from "@server/db/names";
+import { idpExistsForOrg } from "@server/lib/idp/idpExistsForOrg";
 import response from "@server/lib/response";
 import {
     getResourceRuleValueValidationError,
@@ -204,14 +203,9 @@ export async function createResourcePolicy(
 
         // Check if Identity provider in `skipToIdpId` exists
         if (skipToIdpId) {
-            const [provider] = await db
-                .select()
-                .from(idp)
-                .innerJoin(idpOrg, eq(idpOrg.idpId, idp.idpId))
-                .where(and(eq(idp.idpId, skipToIdpId), eq(idpOrg.orgId, orgId)))
-                .limit(1);
+            const providerExists = await idpExistsForOrg(skipToIdpId, orgId);
 
-            if (!provider) {
+            if (!providerExists) {
                 return next(
                     createHttpError(
                         HttpCode.INTERNAL_SERVER_ERROR,

server/routers/newt/buildConfiguration.ts

@@ -19,6 +19,7 @@ import { eq, and, inArray } from "drizzle-orm";
 import config from "@server/lib/config";
 import { decrypt } from "@server/lib/crypto";
 import {
+    batchFetchCertsForSiteResources,
     formatEndpoint,
     generateSubnetProxyTargetV2,
     SubnetProxyTargetV2
@@ -206,11 +207,18 @@ export async function buildClientConfigurationForNewtClient(
         });
     }
 
+    // Batch-fetch certs for every SSL-enabled HTTP resource's domain in one
+    // call rather than letting each resource fetch its own — with thousands
+    // of resources this avoids a concurrent DB/cache stampede for what is
+    // often the very same (e.g. wildcard) certificate.
+    const certByDomain = await batchFetchCertsForSiteResources(allSiteResources);
+
     const resourceTargetsArr = await Promise.all(
         allSiteResources.map((resource) =>
             generateSubnetProxyTargetV2(
                 resource,
-                clientsByResourceId.get(resource.siteResourceId) ?? []
+                clientsByResourceId.get(resource.siteResourceId) ?? [],
+                certByDomain
             )
         )
     );

server/routers/newt/certs.ts

@@ -0,0 +1,75 @@
+import { sendToClient } from "#dynamic/routers/ws";
+import logger from "@server/logger";
+import {
+    canCompress,
+    supportsCertReferences
+} from "@server/lib/clientVersionChecks";
+import { CertRef } from "@server/lib/ip";
+
+/**
+ * Pushes an incremental set of certs to a newt client outside of a full
+ * newt/sync or newt/wg/receive-config, e.g. after a certificate renewal so
+ * that every target referencing it (by tlsCertId) picks up the new material
+ * without waiting for the next full resync.
+ */
+export async function sendCertsAdd(
+    newtId: string,
+    certs: CertRef[],
+    version?: string | null
+) {
+    if (certs.length === 0) {
+        return;
+    }
+
+    if (!supportsCertReferences(version)) {
+        logger.debug(
+            `Newt ${newtId} (version ${version}) does not support cert references, skipping certs/add`
+        );
+        return;
+    }
+
+    await sendToClient(
+        newtId,
+        {
+            type: "newt/certs/add",
+            data: certs
+        },
+        {
+            incrementConfigVersion: true,
+            compress: canCompress(version, "newt")
+        }
+    );
+}
+
+/**
+ * Tells a newt client to drop the given cert IDs, e.g. once the server knows
+ * no target references them anymore.
+ */
+export async function sendCertsRemove(
+    newtId: string,
+    certIds: string[],
+    version?: string | null
+) {
+    if (certIds.length === 0) {
+        return;
+    }
+
+    if (!supportsCertReferences(version)) {
+        logger.debug(
+            `Newt ${newtId} (version ${version}) does not support cert references, skipping certs/remove`
+        );
+        return;
+    }
+
+    await sendToClient(
+        newtId,
+        {
+            type: "newt/certs/remove",
+            data: { ids: certIds }
+        },
+        {
+            incrementConfigVersion: true,
+            compress: canCompress(version, "newt")
+        }
+    );
+}

server/routers/newt/handleNewtGetConfigMessage.ts

@@ -7,7 +7,11 @@ import { eq } from "drizzle-orm";
 import { sendToExitNode } from "#dynamic/lib/exitNodes";
 import { buildClientConfigurationForNewtClient } from "./buildConfiguration";
 import { convertTargetsIfNecessary } from "../client/targets";
-import { canCompress } from "@server/lib/clientVersionChecks";
+import {
+    canCompress,
+    supportsCertReferences
+} from "@server/lib/clientVersionChecks";
+import { dedupeCertsForTargets } from "@server/lib/ip";
 import config from "@server/lib/config";
 import { waitForSiteRebuildIdle } from "@server/lib/rebuildClientAssociations";
 
@@ -119,7 +123,16 @@ export const handleNewtGetConfigMessage: MessageHandler = async (context) => {
         exitNode
     );
 
-    const targetsToSend = await convertTargetsIfNecessary(newt.newtId, targets); // for backward compatibility with old newt versions that don't support the new target format
+    // Older newt clients only understand inline tlsCert/tlsKey on each
+    // target, so only switch to certId references once we know the client
+    // can resolve them.
+    let dedupedTargets = targets;
+    let certs: { id: string; cert: string; key: string }[] = [];
+    if (supportsCertReferences(newt.version)) {
+        ({ targets: dedupedTargets, certs } = dedupeCertsForTargets(targets));
+    }
+
+    const targetsToSend = await convertTargetsIfNecessary(newt.newtId, dedupedTargets); // for backward compatibility with old newt versions that don't support the new target format
 
     return {
         message: {
@@ -128,6 +141,7 @@ export const handleNewtGetConfigMessage: MessageHandler = async (context) => {
                 ipAddress: site.address,
                 peers,
                 targets: targetsToSend,
+                certs,
                 chainId: chainId
             }
         },

server/routers/newt/sync.ts

@@ -6,7 +6,11 @@ import {
     buildClientConfigurationForNewtClient,
     buildTargetConfigurationForNewtClient
 } from "./buildConfiguration";
-import { canCompress } from "@server/lib/clientVersionChecks";
+import {
+    canCompress,
+    supportsCertReferences
+} from "@server/lib/clientVersionChecks";
+import { dedupeCertsForTargets } from "@server/lib/ip";
 
 export async function sendNewtSyncMessage(newt: Newt, site: Site) {
     const {
@@ -28,6 +32,16 @@ export async function sendNewtSyncMessage(newt: Newt, site: Site) {
         site,
         exitNode
     );
+
+    // Older newt clients only understand inline tlsCert/tlsKey on each
+    // target, so only switch to certId references once we know the client
+    // can resolve them.
+    let clientTargets = targets;
+    let certs: { id: string; cert: string; key: string }[] = [];
+    if (supportsCertReferences(newt.version)) {
+        ({ targets: clientTargets, certs } = dedupeCertsForTargets(targets));
+    }
+
     await sendToClient(
         newt.newtId,
         {
@@ -39,7 +53,8 @@ export async function sendNewtSyncMessage(newt: Newt, site: Site) {
                 },
                 healthCheckTargets: validHealthCheckTargets,
                 peers: peers,
-                clientTargets: targets,
+                clientTargets: clientTargets,
+                certs: certs,
                 browserGatewayTargets: browserGatewayTargets,
                 remoteExitNodeSubnets: remoteExitNodeSubnets
             }

server/routers/policy/setResourcePolicyAccessControl.ts

@@ -2,8 +2,6 @@ import { Request, Response, NextFunction } from "express";
 import { z } from "zod";
 import {
     db,
-    idp,
-    idpOrg,
     resourcePolicies,
     rolePolicies,
     roles,
@@ -18,6 +16,7 @@ import logger from "@server/logger";
 import { fromError } from "zod-validation-error";
 import { and, eq, inArray, ne } from "drizzle-orm";
 import { OpenAPITags, registry } from "@server/openApi";
+import { idpExistsForOrg } from "@server/lib/idp/idpExistsForOrg";
 
 const setResourcePolicyAcccessControlBodySchema = z.strictObject({
     sso: z.boolean(),
@@ -27,7 +26,7 @@ const setResourcePolicyAcccessControlBodySchema = z.strictObject({
     }),
     skipToIdpId: z.int().positive().optional().nullable().openapi({
         type: "integer",
-        description: "Page number to retrieve"
+        description: "Default identity provider ID to skip to on login"
     })
 });
 
@@ -36,8 +35,8 @@ const setResourcePolicyAccessControlParamsSchema = z.strictObject({
 });
 
 registry.registerPath({
-    method: "post",
-    path: "/resource-policy/{resourceId}/access-control",
+    method: "put",
+    path: "/resource-policy/{resourcePolicyId}/access-control",
     description:
         "Set access control users for a resource policy, including SSO, users, roles, Identity provider.",
     tags: [OpenAPITags.PublicResourcePolicyLegacy],
@@ -163,16 +162,9 @@ export async function setResourcePolicyAccessControl(
 
         // Check if Identity provider in `skipToIdpId` exists
         if (idpId) {
-            const [provider] = await db
-                .select()
-                .from(idp)
-                .innerJoin(idpOrg, eq(idpOrg.idpId, idp.idpId))
-                .where(
-                    and(eq(idp.idpId, idpId), eq(idpOrg.orgId, policy.orgId))
-                )
-                .limit(1);
+            const providerExists = await idpExistsForOrg(idpId, policy.orgId);
 
-            if (!provider) {
+            if (!providerExists) {
                 return next(
                     createHttpError(
                         HttpCode.INTERNAL_SERVER_ERROR,

src/app/[orgId]/settings/sites/[niceId]/credentials/page.tsx

@@ -40,6 +40,8 @@ import { NewtSiteInstallCommands } from "@app/components/newt-install-commands";
 import { usePaidStatus } from "@app/hooks/usePaidStatus";
 import { tierMatrix } from "@server/lib/billing/tierMatrix";
 import type { AxiosResponse } from "axios";
+import { useQuery } from "@tanstack/react-query";
+import { productUpdatesQueries } from "@app/lib/queries";
 
 export default function CredentialsPage() {
     const { env } = useEnvContext();
@@ -67,6 +69,12 @@ export default function CredentialsPage() {
 
     const { isPaidUser } = usePaidStatus();
 
+    const { data: latestVersions } = useQuery(
+        productUpdatesQueries.latestVersion(true)
+    );
+    const newtVersion =
+        latestVersions?.data?.newt?.latestVersion ?? "latest";
+
     // Fetch site defaults for wireguard sites to show in obfuscated config
     useEffect(() => {
         const fetchSiteDefaults = async () => {
@@ -302,6 +310,7 @@ export default function CredentialsPage() {
                             id={displayNewtId ?? "**********"}
                             secret={displaySecret ?? "**************"}
                             endpoint={env.app.dashboardUrl}
+                            version={newtVersion}
                         />
                     </>
                 )}

src/app/[orgId]/settings/sites/create/page.tsx

@@ -56,6 +56,8 @@ import { QRCodeCanvas } from "qrcode.react";
 import { useTranslations } from "next-intl";
 import { build } from "@server/build";
 import { NewtSiteInstallCommands } from "@app/components/newt-install-commands";
+import { useQuery } from "@tanstack/react-query";
+import { productUpdatesQueries } from "@app/lib/queries";
 
 type SiteType = "newt" | "wireguard" | "local";
 
@@ -189,9 +191,14 @@ export default function Page() {
     const [wgConfig, setWgConfig] = useState("");
 
     const [createLoading, setCreateLoading] = useState(false);
-    const [newtVersion, setNewtVersion] = useState("latest");
     const [showAdvancedSettings, setShowAdvancedSettings] = useState(false);
 
+    const { data: latestVersions } = useQuery(
+        productUpdatesQueries.latestVersion(true)
+    );
+    const newtVersion =
+        latestVersions?.data?.newt?.latestVersion ?? "latest";
+
     const [siteDefaults, setSiteDefaults] =
         useState<PickSiteDefaultsResponse | null>(null);
 
@@ -302,45 +309,6 @@ export default function Page() {
         const load = async () => {
             setLoadingPage(true);
 
-            let currentNewtVersion = "latest";
-
-            try {
-                const controller = new AbortController();
-                const timeoutId = setTimeout(() => controller.abort(), 3000);
-
-                const response = await fetch(
-                    `https://api.github.com/repos/fosrl/newt/releases/latest`,
-                    { signal: controller.signal }
-                );
-
-                clearTimeout(timeoutId);
-
-                if (!response.ok) {
-                    throw new Error(
-                        t("newtErrorFetchReleases", {
-                            err: response.statusText
-                        })
-                    );
-                }
-                const data = await response.json();
-                const latestVersion = data.tag_name;
-                currentNewtVersion = latestVersion;
-                setNewtVersion(latestVersion);
-            } catch (error) {
-                if (error instanceof Error && error.name === "AbortError") {
-                    console.error(t("newtErrorFetchTimeout"));
-                } else {
-                    console.error(
-                        t("newtErrorFetchLatest", {
-                            err:
-                                error instanceof Error
-                                    ? error.message
-                                    : String(error)
-                        })
-                    );
-                }
-            }
-
             const generatedKeypair = generateKeypair();
 
             const privateKey = generatedKeypair.privateKey;

src/hooks/useOptimisticLabels.ts

@@ -95,7 +95,10 @@ export function useOptimisticLabels({
     }
 
     async function refresh() {
-        router.refresh();
+        // Only refresh if the labels have been modified
+        if (pendingActions.length > 0) {
+            router.refresh();
+        }
         setPendingActions([]);
     }