Merge pull request #3085 from marcschaeferger-org/security-updates

Normalize request parameters and update dependencies for Security
2026-06-05 07:16:24 +00:00 · 2026-05-27 21:37:50 -07:00
parent ddabfb5ca1
commit 2946df3b8e
37 changed files with 2656 additions and 3609 deletions
--- a/server/middlewares/integration/verifyAccessTokenAccess.ts
+++ b/server/middlewares/integration/verifyAccessTokenAccess.ts
@@ -4,6 +4,7 @@ import { resourceAccessToken, resources, apiKeyOrg } from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeyAccessTokenAccess(
    req: Request,
@@ -12,7 +13,7 @@ export async function verifyApiKeyAccessTokenAccess(
 ) {
    try {
        const apiKey = req.apiKey;
-        const accessTokenId = req.params.accessTokenId;
+        const accessTokenId = getFirstString(req.params.accessTokenId);

        if (!apiKey) {
            return next(
@@ -20,6 +21,12 @@ export async function verifyApiKeyAccessTokenAccess(
            );
        }

+        if (!accessTokenId) {
+            return next(
+                createHttpError(HttpCode.BAD_REQUEST, "Invalid access token ID")
+            );
+        }
+
        const [accessToken] = await db
            .select()
            .from(resourceAccessToken)
--- a/server/middlewares/integration/verifyApiKeyApiKeyAccess.ts
+++ b/server/middlewares/integration/verifyApiKeyApiKeyAccess.ts
@@ -4,6 +4,7 @@ import { apiKeys, apiKeyOrg } from "@server/db";
 import { and, eq, or } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeyApiKeyAccess(
    req: Request,
@@ -14,8 +15,10 @@ export async function verifyApiKeyApiKeyAccess(
        const { apiKey: callerApiKey } = req;

        const apiKeyId =
-            req.params.apiKeyId || req.body.apiKeyId || req.query.apiKeyId;
-        const orgId = req.params.orgId;
+            getFirstString(req.params.apiKeyId) ||
+            getFirstString(req.body.apiKeyId) ||
+            getFirstString(req.query.apiKeyId);
+        const orgId = getFirstString(req.params.orgId);

        if (!callerApiKey) {
            return next(
--- a/server/middlewares/integration/verifyApiKeyDomainAccess.ts
+++ b/server/middlewares/integration/verifyApiKeyDomainAccess.ts
@@ -3,6 +3,7 @@ import { db, domains, orgDomains, apiKeyOrg } from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeyDomainAccess(
    req: Request,
@@ -12,8 +13,10 @@ export async function verifyApiKeyDomainAccess(
    try {
        const apiKey = req.apiKey;
        const domainId =
-            req.params.domainId || req.body.domainId || req.query.domainId;
-        const orgId = req.params.orgId;
+            getFirstString(req.params.domainId) ||
+            getFirstString(req.body.domainId) ||
+            getFirstString(req.query.domainId);
+        const orgId = getFirstString(req.params.orgId);

        if (!apiKey) {
            return next(
@@ -27,6 +30,12 @@ export async function verifyApiKeyDomainAccess(
            );
        }

+        if (!orgId) {
+            return next(
+                createHttpError(HttpCode.BAD_REQUEST, "Invalid organization ID")
+            );
+        }
+
        if (apiKey.isRoot) {
            // Root keys can access any domain in any org
            return next();
--- a/server/middlewares/integration/verifyApiKeyIdpAccess.ts
+++ b/server/middlewares/integration/verifyApiKeyIdpAccess.ts
@@ -4,6 +4,7 @@ import { idp, idpOrg, apiKeyOrg } from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeyIdpAccess(
    req: Request,
@@ -12,8 +13,12 @@ export async function verifyApiKeyIdpAccess(
 ) {
    try {
        const apiKey = req.apiKey;
-        const idpId = req.params.idpId || req.body.idpId || req.query.idpId;
-        const orgId = req.params.orgId;
+        const idpIdRaw =
+            getFirstString(req.params.idpId) ||
+            getFirstString(req.body.idpId) ||
+            getFirstString(req.query.idpId);
+        const idpId = Number.parseInt(idpIdRaw ?? "", 10);
+        const orgId = getFirstString(req.params.orgId);

        if (!apiKey) {
            return next(
@@ -27,7 +32,7 @@ export async function verifyApiKeyIdpAccess(
            );
        }

-        if (!idpId) {
+        if (Number.isNaN(idpId)) {
            return next(
                createHttpError(HttpCode.BAD_REQUEST, "Invalid IDP ID")
            );
--- a/server/middlewares/integration/verifyApiKeyOrgAccess.ts
+++ b/server/middlewares/integration/verifyApiKeyOrgAccess.ts
@@ -4,6 +4,7 @@ import { apiKeyOrg } from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeyOrgAccess(
    req: Request,
@@ -12,7 +13,7 @@ export async function verifyApiKeyOrgAccess(
 ) {
    try {
        const apiKeyId = req.apiKey?.apiKeyId;
-        const orgId = req.params.orgId;
+        const orgId = getFirstString(req.params.orgId);

        if (!apiKeyId) {
            return next(
@@ -45,7 +46,7 @@ export async function verifyApiKeyOrgAccess(
        }

        if (!req.apiKeyOrg) {
-            next(
+            return next(
                createHttpError(
                    HttpCode.FORBIDDEN,
                    "Key does not have access to this organization"
--- a/server/middlewares/integration/verifyApiKeySiteResourceAccess.ts
+++ b/server/middlewares/integration/verifyApiKeySiteResourceAccess.ts
@@ -4,6 +4,7 @@ import { siteResources, apiKeyOrg } from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeySiteResourceAccess(
    req: Request,
@@ -12,7 +13,8 @@ export async function verifyApiKeySiteResourceAccess(
 ) {
    try {
        const apiKey = req.apiKey;
-        const siteResourceId = parseInt(req.params.siteResourceId);
+        const siteResourceIdRaw = getFirstString(req.params.siteResourceId);
+        const siteResourceId = Number.parseInt(siteResourceIdRaw ?? "", 10);

        if (!apiKey) {
            return next(
@@ -20,7 +22,7 @@ export async function verifyApiKeySiteResourceAccess(
            );
        }

-        if (!siteResourceId) {
+        if (Number.isNaN(siteResourceId)) {
            return next(
                createHttpError(
                    HttpCode.BAD_REQUEST,
--- a/server/middlewares/integration/verifyApiKeyTargetAccess.ts
+++ b/server/middlewares/integration/verifyApiKeyTargetAccess.ts
@@ -4,6 +4,7 @@ import { resources, targets, apiKeyOrg } from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeyTargetAccess(
    req: Request,
@@ -12,7 +13,8 @@ export async function verifyApiKeyTargetAccess(
 ) {
    try {
        const apiKey = req.apiKey;
-        const targetId = parseInt(req.params.targetId);
+        const targetIdRaw = getFirstString(req.params.targetId);
+        const targetId = Number.parseInt(targetIdRaw ?? "", 10);

        if (!apiKey) {
            return next(
@@ -20,7 +22,7 @@ export async function verifyApiKeyTargetAccess(
            );
        }

-        if (isNaN(targetId)) {
+        if (Number.isNaN(targetId)) {
            return next(
                createHttpError(HttpCode.BAD_REQUEST, "Invalid target ID")
            );
--- a/server/middlewares/verifyAccessTokenAccess.ts
+++ b/server/middlewares/verifyAccessTokenAccess.ts
@@ -7,6 +7,7 @@ import HttpCode from "@server/types/HttpCode";
 import { canUserAccessResource } from "@server/auth/canUserAccessResource";
 import { checkOrgAccessPolicy } from "#dynamic/lib/checkOrgAccessPolicy";
 import { getUserOrgRoleIds } from "@server/lib/userOrgRoles";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyAccessTokenAccess(
    req: Request,
@@ -14,7 +15,7 @@ export async function verifyAccessTokenAccess(
    next: NextFunction
 ) {
    const userId = req.user!.userId;
-    const accessTokenId = req.params.accessTokenId;
+    const accessTokenId = getFirstString(req.params.accessTokenId);

    if (!userId) {
        return next(
@@ -22,6 +23,12 @@ export async function verifyAccessTokenAccess(
        );
    }

+    if (!accessTokenId) {
+        return next(
+            createHttpError(HttpCode.BAD_REQUEST, "Invalid access token ID")
+        );
+    }
+
    const [accessToken] = await db
        .select()
        .from(resourceAccessToken)
@@ -87,7 +94,7 @@ export async function verifyAccessTokenAccess(
        }

        if (!req.userOrg) {
-            next(
+            return next(
                createHttpError(
                    HttpCode.FORBIDDEN,
                    "User does not have access to this organization"
--- a/server/middlewares/verifyApiKeyAccess.ts
+++ b/server/middlewares/verifyApiKeyAccess.ts
@@ -6,6 +6,7 @@ import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
 import { checkOrgAccessPolicy } from "#dynamic/lib/checkOrgAccessPolicy";
 import { getUserOrgRoleIds } from "@server/lib/userOrgRoles";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyApiKeyAccess(
    req: Request,
@@ -14,9 +15,24 @@ export async function verifyApiKeyAccess(
 ) {
    try {
        const userId = req.user!.userId;
-        const apiKeyId =
-            req.params.apiKeyId || req.body.apiKeyId || req.query.apiKeyId;
-        const orgId = req.params.orgId;
+        const apiKeyIdFromParams = getFirstString(req.params?.apiKeyId);
+        const apiKeyIdFromBody = getFirstString(req.body?.apiKeyId);
+
+        if (
+            apiKeyIdFromParams &&
+            apiKeyIdFromBody &&
+            apiKeyIdFromParams !== apiKeyIdFromBody
+        ) {
+            return next(
+                createHttpError(
+                    HttpCode.BAD_REQUEST,
+                    "API key ID provided in both URL and body with different values"
+                )
+            );
+        }
+
+        const apiKeyId = apiKeyIdFromParams || apiKeyIdFromBody;
+        const orgId = getFirstString(req.params.orgId);

        if (!userId) {
            return next(
@@ -104,10 +120,7 @@ export async function verifyApiKeyAccess(
            }
        }

-        req.userOrgRoleIds = await getUserOrgRoleIds(
-            req.userOrg.userId,
-            orgId
-        );
+        req.userOrgRoleIds = await getUserOrgRoleIds(req.userOrg.userId, orgId);

        return next();
    } catch (error) {
--- a/server/middlewares/verifyDomainAccess.ts
+++ b/server/middlewares/verifyDomainAccess.ts
@@ -6,6 +6,7 @@ import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
 import { checkOrgAccessPolicy } from "#dynamic/lib/checkOrgAccessPolicy";
 import { getUserOrgRoleIds } from "@server/lib/userOrgRoles";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyDomainAccess(
    req: Request,
@@ -14,9 +15,8 @@ export async function verifyDomainAccess(
 ) {
    try {
        const userId = req.user!.userId;
-        const domainId =
-            req.params.domainId;
-        const orgId = req.params.orgId;
+        const domainId = getFirstString(req.params.domainId);
+        const orgId = getFirstString(req.params.orgId);

        if (!userId) {
            return next(
@@ -62,10 +62,7 @@ export async function verifyDomainAccess(
                .select()
                .from(userOrgs)
                .where(
-                    and(
-                        eq(userOrgs.userId, userId),
-                        eq(userOrgs.orgId, orgId)
-                    )
+                    and(eq(userOrgs.userId, userId), eq(userOrgs.orgId, orgId))
                )
                .limit(1);
            req.userOrg = userOrgRole[0];
--- a/server/middlewares/verifyLimits.ts
+++ b/server/middlewares/verifyLimits.ts
@@ -3,6 +3,7 @@ import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
 import { usageService } from "@server/lib/billing/usageService";
 import { build } from "@server/build";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyLimits(
    req: Request,
@@ -13,7 +14,10 @@ export async function verifyLimits(
        return next();
    }

-    const orgId = req.userOrgId || req.apiKeyOrg?.orgId || req.params.orgId;
+    const orgId =
+        req.userOrgId ||
+        req.apiKeyOrg?.orgId ||
+        getFirstString(req.params.orgId);

    if (!orgId) {
        return next(); // its fine if we silently fail here because this is not critical to operation or security and its better user experience if we dont fail
--- a/server/middlewares/verifyOrgAccess.ts
+++ b/server/middlewares/verifyOrgAccess.ts
@@ -6,6 +6,7 @@ import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
 import { checkOrgAccessPolicy } from "#dynamic/lib/checkOrgAccessPolicy";
 import { getUserOrgRoleIds } from "@server/lib/userOrgRoles";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyOrgAccess(
    req: Request,
@@ -13,7 +14,7 @@ export async function verifyOrgAccess(
    next: NextFunction
 ) {
    const userId = req.user!.userId;
-    const orgId = req.params.orgId;
+    const orgId = getFirstString(req.params.orgId);

    if (!userId) {
        return next(
--- a/server/middlewares/verifySiteProvisioningKeyAccess.ts
+++ b/server/middlewares/verifySiteProvisioningKeyAccess.ts
@@ -1,10 +1,16 @@
 import { Request, Response, NextFunction } from "express";
-import { db, userOrgs, siteProvisioningKeys, siteProvisioningKeyOrg } from "@server/db";
+import {
+    db,
+    userOrgs,
+    siteProvisioningKeys,
+    siteProvisioningKeyOrg
+} from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
 import { checkOrgAccessPolicy } from "#dynamic/lib/checkOrgAccessPolicy";
 import { getUserOrgRoleIds } from "@server/lib/userOrgRoles";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifySiteProvisioningKeyAccess(
    req: Request,
@@ -13,8 +19,10 @@ export async function verifySiteProvisioningKeyAccess(
 ) {
    try {
        const userId = req.user!.userId;
-        const siteProvisioningKeyId = req.params.siteProvisioningKeyId;
-        const orgId = req.params.orgId;
+        const siteProvisioningKeyId = getFirstString(
+            req.params.siteProvisioningKeyId
+        );
+        const orgId = getFirstString(req.params.orgId);

        if (!userId) {
            return next(
@@ -80,10 +88,7 @@ export async function verifySiteProvisioningKeyAccess(
                .where(
                    and(
                        eq(userOrgs.userId, userId),
-                        eq(
-                            userOrgs.orgId,
-                            row.siteProvisioningKeyOrg.orgId
-                        )
+                        eq(userOrgs.orgId, row.siteProvisioningKeyOrg.orgId)
                    )
                )
                .limit(1);
--- a/server/middlewares/verifyTargetAccess.ts
+++ b/server/middlewares/verifyTargetAccess.ts
@@ -7,6 +7,7 @@ import HttpCode from "@server/types/HttpCode";
 import { canUserAccessResource } from "../auth/canUserAccessResource";
 import { checkOrgAccessPolicy } from "#dynamic/lib/checkOrgAccessPolicy";
 import { getUserOrgRoleIds } from "@server/lib/userOrgRoles";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyTargetAccess(
    req: Request,
@@ -14,7 +15,8 @@ export async function verifyTargetAccess(
    next: NextFunction
 ) {
    const userId = req.user!.userId;
-    const targetId = parseInt(req.params.targetId);
+    const targetIdRaw = getFirstString(req.params.targetId);
+    const targetId = Number.parseInt(targetIdRaw ?? "", 10);

    if (!userId) {
        return next(
--- a/server/middlewares/verifyUserIsOrgOwner.ts
+++ b/server/middlewares/verifyUserIsOrgOwner.ts
@@ -4,6 +4,7 @@ import { userOrgs } from "@server/db";
 import { and, eq } from "drizzle-orm";
 import createHttpError from "http-errors";
 import HttpCode from "@server/types/HttpCode";
+import { getFirstString } from "@server/lib/requestParams";

 export async function verifyUserIsOrgOwner(
    req: Request,
@@ -11,7 +12,7 @@ export async function verifyUserIsOrgOwner(
    next: NextFunction
 ) {
    const userId = req.user!.userId;
-    const orgId = req.params.orgId;
+    const orgId = getFirstString(req.params.orgId);

    if (!userId) {
        return next(