mirror/formbricks
1
0
Fork 0
mirror of https://github.com/formbricks/formbricks.git synced 2026-05-06 11:20:56 -05:00
Code Issues Packages Projects Releases 169 Wiki Activity

fix: address envoy auth review findings

Browse Source
This commit is contained in:
Bhagya Amarasinghe
2026-04-27 19:31:43 +05:30
parent bcc3789ce8
commit be80db8418
7 changed files with 84 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/app/api/(internal)/envoy-auth/[...path]/route.test.ts
+54 -8
View File
@@ -1,11 +1,11 @@
import { NextRequest } from "next/server";
import { beforeEach, describe, expect, test, vi } from "vitest";
import { DELETE, GET, PATCH, POST } from "./route";
import { DELETE, GET, HEAD, OPTIONS, PATCH, POST } from "./route";
const {
mockAuthenticateApiKeyFromHeaders,
mockGetApiKeyFromHeaders,
mockGetFeedbackRecordsGatewayJwtFromHeaders,
mockGetBearerTokenFromHeaders,
mockGetProxySession,
mockVerifyFeedbackRecordsGatewayToken,
mockGetFeedbackRecordDirectoryAuthContext,
@@ -15,7 +15,7 @@ const {
} = vi.hoisted(() => ({
mockAuthenticateApiKeyFromHeaders: vi.fn(),
mockGetApiKeyFromHeaders: vi.fn(),
mockGetFeedbackRecordsGatewayJwtFromHeaders: vi.fn(),
mockGetBearerTokenFromHeaders: vi.fn(),
mockGetProxySession: vi.fn(),
mockVerifyFeedbackRecordsGatewayToken: vi.fn(),
mockGetFeedbackRecordDirectoryAuthContext: vi.fn(),
@@ -27,7 +27,7 @@ const {
vi.mock("@/modules/api/lib/api-key-auth", () => ({
authenticateApiKeyFromHeaders: mockAuthenticateApiKeyFromHeaders,
getApiKeyFromHeaders: mockGetApiKeyFromHeaders,
getFeedbackRecordsGatewayJwtFromHeaders: mockGetFeedbackRecordsGatewayJwtFromHeaders,
getBearerTokenFromHeaders: mockGetBearerTokenFromHeaders,
}));
vi.mock("@/modules/auth/lib/proxy-session", () => ({
@@ -95,7 +95,7 @@ describe("Envoy auth route", () => {
beforeEach(() => {
vi.resetAllMocks();
mockGetApiKeyFromHeaders.mockReturnValue(null);
mockGetFeedbackRecordsGatewayJwtFromHeaders.mockReturnValue(null);
mockGetBearerTokenFromHeaders.mockReturnValue(null);
mockAuthenticateApiKeyFromHeaders.mockResolvedValue(null);
mockGetProxySession.mockResolvedValue(null);
mockVerifyFeedbackRecordsGatewayToken.mockImplementation(() => {
@@ -239,7 +239,7 @@ describe("Envoy auth route", () => {
});
test("returns 401 for invalid explicit JWT even when a session cookie exists", async () => {
mockGetFeedbackRecordsGatewayJwtFromHeaders.mockReturnValue("header.payload.signature");
mockGetBearerTokenFromHeaders.mockReturnValue("header.payload.signature");
mockGetProxySession.mockResolvedValue({
userId: "user_1",
});
@@ -261,7 +261,7 @@ describe("Envoy auth route", () => {
});
test("allows PATCH requests with a valid gateway JWT", async () => {
mockGetFeedbackRecordsGatewayJwtFromHeaders.mockReturnValue("header.payload.signature");
mockGetBearerTokenFromHeaders.mockReturnValue("header.payload.signature");
mockVerifyFeedbackRecordsGatewayToken.mockReturnValue({ userId: "user_1" });
const response = await PATCH(
@@ -352,7 +352,7 @@ describe("Envoy auth route", () => {
});
test("returns 403 for archived directories", async () => {
mockGetFeedbackRecordsGatewayJwtFromHeaders.mockReturnValue("header.payload.signature");
mockGetBearerTokenFromHeaders.mockReturnValue("header.payload.signature");
mockVerifyFeedbackRecordsGatewayToken.mockReturnValue({ userId: "user_1" });
mockGetFeedbackRecordDirectoryAuthContext.mockResolvedValue({
organizationId: "org_1",
@@ -398,4 +398,50 @@ describe("Envoy auth route", () => {
expect(response.status).toBe(400);
});
test("handles HEAD requests through the generic route instead of 405ing at Next.js", async () => {
mockGetApiKeyFromHeaders.mockReturnValue("fbk_test");
mockAuthenticateApiKeyFromHeaders.mockResolvedValue({
type: "apiKey",
apiKeyId: "key_1",
organizationId: "org_1",
organizationAccess: { accessControl: { read: true, write: true } },
workspacePermissions: [],
feedbackRecordDirectoryPermissions: [],
});
const response = await HEAD(
createRequest(`http://localhost/api/envoy-auth/v1/feedback-records/${feedbackRecordId}`, {
method: "HEAD",
headers: {
"x-api-key": "fbk_test",
},
})
);
expect(response.status).toBe(400);
});
test("handles OPTIONS requests through the generic route instead of 405ing at Next.js", async () => {
mockGetApiKeyFromHeaders.mockReturnValue("fbk_test");
mockAuthenticateApiKeyFromHeaders.mockResolvedValue({
type: "apiKey",
apiKeyId: "key_1",
organizationId: "org_1",
organizationAccess: { accessControl: { read: true, write: true } },
workspacePermissions: [],
feedbackRecordDirectoryPermissions: [],
});
const response = await OPTIONS(
createRequest("http://localhost/api/envoy-auth/v1/feedback-records", {
method: "OPTIONS",
headers: {
authorization: "Bearer fbk_test",
},
})
);
expect(response.status).toBe(400);
});
});
apps/web/app/api/(internal)/envoy-auth/[...path]/route.ts
+2
View File
@@ -10,3 +10,5 @@ export const POST = handler;
export const PUT = handler;
export const PATCH = handler;
export const DELETE = handler;
export const HEAD = handler;
export const OPTIONS = handler;
apps/web/modules/api/lib/api-key-auth.test.ts
+13 -4
View File
@@ -1,5 +1,5 @@
import { describe, expect, test } from "vitest";
import { getApiKeyFromHeaders, getFeedbackRecordsGatewayJwtFromHeaders } from "./api-key-auth";
import { getApiKeyFromHeaders, getBearerTokenFromHeaders } from "./api-key-auth";
describe("api-key-auth helpers", () => {
test("prefers x-api-key over bearer authorization", () => {
@@ -17,15 +17,24 @@ describe("api-key-auth helpers", () => {
});
expect(getApiKeyFromHeaders(headers)).toBe("fbk_from_bearer");
expect(getFeedbackRecordsGatewayJwtFromHeaders(headers)).toBeNull();
expect(getBearerTokenFromHeaders(headers)).toBe("fbk_from_bearer");
});
test("treats jwt-shaped bearer tokens as gateway JWTs, not API keys", () => {
test("does not treat jwt-shaped bearer tokens as API keys", () => {
const headers = new Headers({
authorization: "Bearer header.payload.signature",
});
expect(getApiKeyFromHeaders(headers)).toBeNull();
expect(getFeedbackRecordsGatewayJwtFromHeaders(headers)).toBe("header.payload.signature");
expect(getBearerTokenFromHeaders(headers)).toBe("header.payload.signature");
});
test("does not treat opaque bearer tokens as API keys", () => {
const headers = new Headers({
authorization: "Bearer opaque_service_token",
});
expect(getApiKeyFromHeaders(headers)).toBeNull();
expect(getBearerTokenFromHeaders(headers)).toBe("opaque_service_token");
});
});
apps/web/modules/api/lib/api-key-auth.ts
+4 -18
View File
@@ -1,11 +1,12 @@
import { TAuthenticationApiKey } from "@formbricks/types/auth";
import { parseApiKeyV2 } from "@/lib/crypto";
import { getApiKeyWithPermissions } from "@/modules/organization/settings/api-keys/lib/api-key";
type THeadersLike = Pick<Headers, "get">;
const BEARER_PREFIX = "bearer ";
const getBearerToken = (headers: THeadersLike): string | null => {
export const getBearerTokenFromHeaders = (headers: THeadersLike): string | null => {
const authorizationHeader = headers.get("authorization")?.trim();
if (!authorizationHeader) {
return null;
@@ -19,29 +20,14 @@ const getBearerToken = (headers: THeadersLike): string | null => {
const token = authorizationHeader.slice(BEARER_PREFIX.length).trim();
return token.length > 0 ? token : null;
};
const isJwtLikeToken = (token: string): boolean => {
const segments = token.split(".");
return segments.length === 3 && segments.every((segment) => segment.length > 0);
};
export const getApiKeyFromHeaders = (headers: THeadersLike): string | null => {
const apiKeyHeader = headers.get("x-api-key")?.trim();
if (apiKeyHeader) {
return apiKeyHeader;
}
const bearerToken = getBearerToken(headers);
if (!bearerToken || isJwtLikeToken(bearerToken)) {
return null;
}
return bearerToken;
};
export const getFeedbackRecordsGatewayJwtFromHeaders = (headers: THeadersLike): string | null => {
const bearerToken = getBearerToken(headers);
if (!bearerToken || !isJwtLikeToken(bearerToken)) {
const bearerToken = getBearerTokenFromHeaders(headers);
if (!bearerToken || !parseApiKeyV2(bearerToken)) {
return null;
}
apps/web/modules/hub/feedback-records-gateway.ts
+5 -1
View File
@@ -9,7 +9,7 @@ import { AuthorizationError } from "@formbricks/types/errors";
import { verifyFeedbackRecordsGatewayToken } from "@/lib/jwt";
import { checkAuthorizationUpdated } from "@/lib/utils/action-client/action-client-middleware";
import {
getFeedbackRecordsGatewayJwtFromHeaders,
getBearerTokenFromHeaders,
} from "@/modules/api/lib/api-key-auth";
import {
buildAllowResponse,
@@ -165,6 +165,10 @@ const parseJsonBody = async (request: NextRequest): Promise<Record<string, unkno
}
};
const getFeedbackRecordsGatewayJwtFromHeaders = (headers: Headers): string | null => {
return getBearerTokenFromHeaders(headers);
};
const hasFeedbackRecordDirectoryPermission = (
authentication: TAuthenticationApiKey,
feedbackRecordDirectoryId: string,
charts/formbricks/templates/_helpers.tpl
+4 -1
View File
@@ -157,9 +157,12 @@ If `namespaceOverride` is provided, it will be used; otherwise, it defaults to `
{{- end }}
{{- define "formbricks.hubApiKey" -}}
{{- $secret := (lookup "v1" "Secret" .Release.Namespace (include "formbricks.hubSecretName" .)) }}
{{- $hubSecretName := include "formbricks.hubSecretName" . }}
{{- $secret := (lookup "v1" "Secret" .Release.Namespace $hubSecretName) }}
{{- if and $secret (index $secret.data "HUB_API_KEY") }}
{{- index $secret.data "HUB_API_KEY" | b64dec -}}
{{- else if .Values.hub.existingSecret }}
{{- fail (printf "hub.existingSecret %q must already exist in namespace %q and contain HUB_API_KEY when rendering the generated app secret. Disable secret.enabled and provide app-secrets externally, or pre-create the Hub secret." $hubSecretName .Release.Namespace) -}}
{{- else }}
{{- randAlphaNum 32 -}}
{{- end -}}
charts/formbricks/values.yaml
+2
View File
@@ -574,6 +574,8 @@ hub:
# Defaults to the generated app secret (<release>-app-secrets), which contains DATABASE_URL and HUB_API_KEY.
# If you set this, the custom secret must provide DATABASE_URL and HUB_API_KEY.
# The app secret still needs HUB_API_KEY as well because the web app talks to Hub directly.
# When secret.enabled=true, the custom Hub secret must already exist at render time so Helm can copy HUB_API_KEY
# into the generated app secret and Envoy credential.
existingSecret: ""
# Optional env vars (non-secret). Use existingSecret for secret values such as DATABASE_URL and HUB_API_KEY.