diff --git a/apps/web/app/api/auth/sso/recovery/complete/route.ts b/apps/web/app/api/auth/sso/recovery/complete/route.ts index 56cee728e3ed..5a62d2d9f885 100644 --- a/apps/web/app/api/auth/sso/recovery/complete/route.ts +++ b/apps/web/app/api/auth/sso/recovery/complete/route.ts @@ -1,12 +1,12 @@ import { NextResponse } from "next/server"; import { logger } from "@formbricks/logger"; import { verifySsoRelinkIntent } from "@/lib/jwt"; -import { deleteSessionBySessionToken } from "@/modules/auth/lib/auth-session-repository"; import { getSession } from "@/modules/auth/lib/session"; import { BETTER_AUTH_SESSION_COOKIE_NAMES, getSessionTokenFromCookieHeader, } from "@/modules/auth/lib/session-cookie"; +import { revokeSessionByToken } from "@/modules/auth/lib/session-revocation"; import { completeSsoRecovery, getSsoRecoveryFailureRedirectUrl } from "@/modules/ee/sso/lib/sso-recovery"; const clearSessionCookies = (response: NextResponse) => { @@ -31,7 +31,9 @@ const buildFailedRecoveryResponse = async (request: Request, callbackUrl?: strin } try { - await deleteSessionBySessionToken(sessionToken); + // Through the two-store revocation, not a raw Prisma delete: sessions live in Redis too, and a + // DB-only delete would leave this one resolvable by `getSession` until its TTL (ENG-2557). + await revokeSessionByToken(sessionToken); } catch (error) { logger.error(error, "Failed to delete SSO recovery session after recovery completion error"); } @@ -52,6 +54,8 @@ export const GET = async (request: Request) => { const callbackUrl = await completeSsoRecovery({ intentToken, sessionUserId: session?.user.id, + // Spared by the post-commit session sweep, so the redirect below still lands signed in. + sessionToken: getSessionTokenFromCookieHeader(request.headers.get("cookie")) ?? undefined, }); return NextResponse.redirect(callbackUrl); diff --git a/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.test.ts b/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.test.ts index e8d404748d5a..69f8bb0b6906 100644 --- a/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.test.ts +++ b/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.test.ts @@ -3,6 +3,7 @@ import { prisma } from "@formbricks/database"; import { Prisma } from "@formbricks/database/prisma"; import { logger } from "@formbricks/logger"; import { DatabaseError, ResourceNotFoundError } from "@formbricks/types/errors"; +import { PUBLIC_API_SURVEY_NAME_PLACEHOLDER } from "@formbricks/types/js-constants"; import { getWorkspaceStateData } from "./data"; vi.mock("server-only", () => ({})); @@ -120,7 +121,7 @@ describe("getWorkspaceStateData", () => { surveys: [ { ...mockWorkspaceData.surveys[0], - name: "[deprecated] survey name omitted from public API - will be removed soon", + name: PUBLIC_API_SURVEY_NAME_PLACEHOLDER, }, ], actionClasses: mockWorkspaceData.actionClasses, diff --git a/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.ts b/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.ts index 0fbb557f7c99..8234aa45759d 100644 --- a/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.ts +++ b/apps/web/app/api/v1/client/[workspaceId]/environment/lib/data.ts @@ -9,6 +9,7 @@ import { TJsWorkspaceStateSurvey, TJsWorkspaceStateWorkspaceSetting, } from "@formbricks/types/js"; +import { PUBLIC_API_SURVEY_NAME_PLACEHOLDER } from "@formbricks/types/js-constants"; import { type TBaseFilters, buildSurveyInteractionRefreshMap } from "@formbricks/types/segment"; import { toLegacyLanguageCodes } from "@/lib/i18n/utils"; import { validateInputs } from "@/lib/utils/validate"; @@ -283,7 +284,7 @@ export const getWorkspaceStateData = async (workspaceId: string): Promise> + > +): EnrichmentStatusResponse => ({ + tenant_id: "frd-1", + translation: { enabled: false, eligible: 0, done: 0, ...overrides.translation }, + sentiment: { enabled: false, eligible: 0, done: 0, ...overrides.sentiment }, + emotions: { enabled: false, eligible: 0, done: 0, ...overrides.emotions }, +}); + +describe("aggregateEnrichmentStatus", () => { + test("derives pending as eligible minus done", () => { + const result = aggregateEnrichmentStatus([ + status({ translation: { enabled: true, eligible: 500, done: 480 } }), + ]); + + expect(result).toEqual([ + { kind: "translation", eligible: 500, done: 480, failedTerminal: 0, pending: 20 }, + ]); + }); + + test("drops enrichments that are disabled everywhere", () => { + const result = aggregateEnrichmentStatus([ + status({ + translation: { enabled: true, eligible: 500, done: 500 }, + sentiment: { enabled: true, eligible: 500, done: 200 }, + }), + ]); + + expect(result.map((enrichment) => enrichment.kind)).toEqual(["translation", "sentiment"]); + }); + + test("returns nothing when no enrichment is enabled", () => { + expect(aggregateEnrichmentStatus([status({})])).toEqual([]); + }); + + test("drops an enrichment that's enabled but has no eligible records anywhere", () => { + // e.g. translation on, but every record is already in the target language — a 0/0 bar can never + // move, the same reason a fully-disabled enrichment is dropped. + const result = aggregateEnrichmentStatus([ + status({ translation: { enabled: true, eligible: 0, done: 0 } }), + ]); + + expect(result).toEqual([]); + }); + + test("sums the counts across directories", () => { + const result = aggregateEnrichmentStatus([ + status({ sentiment: { enabled: true, eligible: 300, done: 100 } }), + status({ sentiment: { enabled: true, eligible: 200, done: 150 } }), + ]); + + expect(result).toEqual([ + { kind: "sentiment", eligible: 500, done: 250, failedTerminal: 0, pending: 250 }, + ]); + }); + + test("ignores directories where the enrichment is switched off", () => { + // The disabled directory reports zeros; counting it would only inflate the denominator with + // records that can never be enriched. + const result = aggregateEnrichmentStatus([ + status({ emotions: { enabled: true, eligible: 400, done: 100 } }), + status({ emotions: { enabled: false, eligible: 0, done: 0 } }), + ]); + + expect(result).toEqual([{ kind: "emotions", eligible: 400, done: 100, failedTerminal: 0, pending: 300 }]); + }); + + test("keeps an enrichment enabled on only one of several directories", () => { + const result = aggregateEnrichmentStatus([ + status({ translation: { enabled: false, eligible: 0, done: 0 } }), + status({ translation: { enabled: true, eligible: 120, done: 120 } }), + ]); + + expect(result).toEqual([ + { kind: "translation", eligible: 120, done: 120, failedTerminal: 0, pending: 0 }, + ]); + }); + + test("never reports a negative pending count", () => { + const result = aggregateEnrichmentStatus([ + status({ sentiment: { enabled: true, eligible: 10, done: 12 } }), + ]); + + expect(result[0].pending).toBe(0); + }); + + test("treats an enrichment missing from the Hub response as disabled", () => { + // sentiment/emotions genuinely absent, not just zeroed — legal per the response type, which + // types all three keys as optional for exactly this case. + const partial: EnrichmentStatusResponse = { + tenant_id: "frd-1", + translation: { enabled: true, eligible: 50, done: 25 }, + }; + + const result = aggregateEnrichmentStatus([partial]); + + expect(result).toEqual([{ kind: "translation", eligible: 50, done: 25, failedTerminal: 0, pending: 25 }]); + }); + + test("returns nothing when there are no directories", () => { + expect(aggregateEnrichmentStatus([])).toEqual([]); + }); + + // ENG-2375: a record whose enrichment permanently gave up (content filter, refusal, truncation) + // used to be silently folded into `eligible - done` and read as "still in progress" forever. + test("excludes permanently-failed records from pending", () => { + const result = aggregateEnrichmentStatus([ + status({ sentiment: { enabled: true, eligible: 100, done: 80, failed_terminal: 15 } }), + ]); + + expect(result).toEqual([{ kind: "sentiment", eligible: 100, done: 80, failedTerminal: 15, pending: 5 }]); + }); + + test("sums failed_terminal across directories", () => { + const result = aggregateEnrichmentStatus([ + status({ emotions: { enabled: true, eligible: 200, done: 150, failed_terminal: 10 } }), + status({ emotions: { enabled: true, eligible: 100, done: 60, failed_terminal: 5 } }), + ]); + + expect(result).toEqual([{ kind: "emotions", eligible: 300, done: 210, failedTerminal: 15, pending: 75 }]); + }); + + test("never reports a negative pending count when failed_terminal alone exceeds the remainder", () => { + // Shouldn't happen per the Hub's own invariant (done + failed + failed_terminal <= eligible), but + // clamp defensively rather than surface a negative number if it ever does. + const result = aggregateEnrichmentStatus([ + status({ translation: { enabled: true, eligible: 10, done: 5, failed_terminal: 8 } }), + ]); + + expect(result[0].pending).toBe(0); + }); + + test("treats a missing failed_terminal as zero, for a Hub that predates the field", () => { + const result = aggregateEnrichmentStatus([ + status({ sentiment: { enabled: true, eligible: 50, done: 30 } }), + ]); + + expect(result[0].failedTerminal).toBe(0); + expect(result[0].pending).toBe(20); + }); +}); diff --git a/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/aggregate.ts b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/aggregate.ts new file mode 100644 index 000000000000..35690858a42b --- /dev/null +++ b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/aggregate.ts @@ -0,0 +1,51 @@ +import { + ENRICHMENT_KINDS, + type TEnrichmentProgress, +} from "@/modules/ee/unify-feedback/enrichment-status/lib/enrichment"; +import type { EnrichmentStatusResponse, EnrichmentTypeStatus } from "@/modules/hub/types"; + +/** + * Fold the per-directory Hub responses into one progress row per enrichment. + * + * The Feedback Data page merges records from every directory assigned to the workspace and offers no + * directory selector, so the indicator above the table has to speak for the same set. Records are + * partitioned by directory, so summing the per-directory counts gives workspace totals without + * double-counting. + * + * Only directories where the enrichment is switched on contribute: a disabled directory reports zeros, + * and folding those in would drag a shared progress bar toward a denominator that can never grow. An + * enrichment disabled everywhere is dropped entirely — and so is one that's enabled but has no eligible + * records anywhere (e.g. translation on, but every record already in the target language), for the + * same reason: a 0/0 bar can never move, whether the zero comes from being off or from having nothing + * to do. + */ +export function aggregateEnrichmentStatus( + statuses: readonly EnrichmentStatusResponse[] +): TEnrichmentProgress[] { + const enrichments: TEnrichmentProgress[] = []; + + for (const kind of ENRICHMENT_KINDS) { + // Tolerate a Hub that predates a given enrichment: an absent key reads as disabled, not as NaN. + const enabledStatuses = statuses + .map((status) => status[kind]) + .filter((status): status is EnrichmentTypeStatus => Boolean(status?.enabled)); + if (enabledStatuses.length === 0) continue; + + const eligible = enabledStatuses.reduce((sum, status) => sum + (status.eligible || 0), 0); + if (eligible === 0) continue; + + const done = enabledStatuses.reduce((sum, status) => sum + (status.done || 0), 0); + // Bridged field (ENG-2375) — absent on a Hub that predates it, reads as 0 rather than NaN. + const failedTerminal = enabledStatuses.reduce((sum, status) => sum + (status.failed_terminal || 0), 0); + + enrichments.push({ + kind, + eligible, + done, + failedTerminal, + pending: Math.max(0, eligible - done - failedTerminal), + }); + } + + return enrichments; +} diff --git a/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/operations.test.ts b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/operations.test.ts new file mode 100644 index 000000000000..cbf1cfbc92bf --- /dev/null +++ b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/operations.test.ts @@ -0,0 +1,117 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { requireUnifyFeedbackWorkspaceAccess } from "@/app/api/v3/lib/feedback-access"; +import type { V3WorkspaceContext } from "@/app/api/v3/lib/workspace-context"; +import { getFeedbackDirectoriesByWorkspaceId } from "@/modules/ee/feedback-directory/lib/feedback-directory"; +import type { TEnrichmentStatusResponse } from "@/modules/ee/unify-feedback/enrichment-status/lib/enrichment"; +import { getEnrichmentStatus } from "@/modules/hub/service"; +import type { EnrichmentStatusResponse } from "@/modules/hub/types"; +import { NO_CONFIG_ERROR } from "@/modules/hub/utils"; +import { getV3EnrichmentStatus } from "./operations"; + +vi.mock("server-only", () => ({})); + +vi.mock("@/app/api/v3/lib/feedback-access", () => ({ + requireUnifyFeedbackWorkspaceAccess: vi.fn(), +})); + +vi.mock("@/modules/ee/feedback-directory/lib/feedback-directory", () => ({ + getFeedbackDirectoriesByWorkspaceId: vi.fn(), +})); + +vi.mock("@/modules/hub/service", () => ({ + getEnrichmentStatus: vi.fn(), +})); + +const workspaceId = "clxx1234567890123456789012"; +const context: V3WorkspaceContext = { workspaceId, organizationId: "org_1" }; +const base = { authentication: null, workspaceId, requestId: "req_1", instance: "/x" }; + +const hubStatus = (tenantId: string, done: number): EnrichmentStatusResponse => ({ + tenant_id: tenantId, + translation: { enabled: true, eligible: 100, done }, + sentiment: { enabled: false, eligible: 0, done: 0 }, + emotions: { enabled: false, eligible: 0, done: 0 }, +}); + +const readBody = async (response: Response): Promise => + ((await response.json()) as { data: TEnrichmentStatusResponse }).data; + +describe("getV3EnrichmentStatus", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(requireUnifyFeedbackWorkspaceAccess).mockResolvedValue(context); + }); + + test("short-circuits on the authorization response without reaching the Hub", async () => { + const forbidden = new Response(null, { status: 403 }); + vi.mocked(requireUnifyFeedbackWorkspaceAccess).mockResolvedValue(forbidden); + + const response = await getV3EnrichmentStatus(base); + + expect(response).toBe(forbidden); + expect(getFeedbackDirectoriesByWorkspaceId).not.toHaveBeenCalled(); + expect(getEnrichmentStatus).not.toHaveBeenCalled(); + }); + + test("resolves the tenant ids from the workspace's own directories", async () => { + vi.mocked(getFeedbackDirectoriesByWorkspaceId).mockResolvedValue([ + { id: "frd-1" }, + { id: "frd-2" }, + ] as Awaited>); + vi.mocked(getEnrichmentStatus).mockImplementation(async (tenantId: string) => ({ + data: hubStatus(tenantId, 40), + error: null, + })); + + const response = await getV3EnrichmentStatus(base); + + expect(getFeedbackDirectoriesByWorkspaceId).toHaveBeenCalledWith(workspaceId); + expect(getEnrichmentStatus).toHaveBeenCalledWith("frd-1"); + expect(getEnrichmentStatus).toHaveBeenCalledWith("frd-2"); + await expect(readBody(response)).resolves.toEqual({ + enrichments: [{ kind: "translation", eligible: 200, done: 80, failedTerminal: 0, pending: 120 }], + unavailable: false, + }); + }); + + test("reports no enrichments when the workspace has no directory", async () => { + vi.mocked(getFeedbackDirectoriesByWorkspaceId).mockResolvedValue([]); + + const response = await getV3EnrichmentStatus(base); + + expect(getEnrichmentStatus).not.toHaveBeenCalled(); + await expect(readBody(response)).resolves.toEqual({ enrichments: [], unavailable: false }); + }); + + test("degrades to unavailable rather than erroring when every directory fails", async () => { + vi.mocked(getFeedbackDirectoriesByWorkspaceId).mockResolvedValue([{ id: "frd-1" }] as Awaited< + ReturnType + >); + vi.mocked(getEnrichmentStatus).mockResolvedValue({ data: null, error: { ...NO_CONFIG_ERROR } }); + + const response = await getV3EnrichmentStatus(base); + + expect(response.status).toBe(200); + await expect(readBody(response)).resolves.toEqual({ enrichments: [], unavailable: true }); + }); + + test("keeps the directories that answered when one of them fails", async () => { + vi.mocked(getFeedbackDirectoriesByWorkspaceId).mockResolvedValue([ + { id: "frd-1" }, + { id: "frd-2" }, + ] as Awaited>); + vi.mocked(getEnrichmentStatus).mockImplementation(async (tenantId: string) => + tenantId === "frd-1" + ? { data: hubStatus(tenantId, 25), error: null } + : { data: null, error: { ...NO_CONFIG_ERROR } } + ); + + const response = await getV3EnrichmentStatus(base); + + // The failed directory is left out entirely — counting it as zero-done would invent a backlog. + await expect(readBody(response)).resolves.toEqual({ + enrichments: [{ kind: "translation", eligible: 100, done: 25, failedTerminal: 0, pending: 75 }], + unavailable: false, + }); + }); +}); diff --git a/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/operations.ts b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/operations.ts new file mode 100644 index 000000000000..f07ee06dfd17 --- /dev/null +++ b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/operations.ts @@ -0,0 +1,61 @@ +import "server-only"; +import { requireUnifyFeedbackWorkspaceAccess } from "@/app/api/v3/lib/feedback-access"; +import { successResponse } from "@/app/api/v3/lib/response"; +import type { TV3Authentication } from "@/app/api/v3/lib/types"; +import { getFeedbackDirectoriesByWorkspaceId } from "@/modules/ee/feedback-directory/lib/feedback-directory"; +import type { TEnrichmentStatusResponse } from "@/modules/ee/unify-feedback/enrichment-status/lib/enrichment"; +import { getEnrichmentStatus } from "@/modules/hub/service"; +import type { EnrichmentStatusResponse } from "@/modules/hub/types"; +import { aggregateEnrichmentStatus } from "./aggregate"; + +/** + * Enrichment progress for every feedback directory assigned to a workspace. + * + * The Hub authenticates with a single shared API key and trusts the `tenant_id` it is handed, so the + * authorization has to happen here: workspace access plus the `feedbackDirectories` entitlement, then + * the tenant ids are read off the workspace's own directories. Nothing tenant-identifying comes from + * the caller. + * + * The indicator is a non-critical enhancement above the records table, so a Hub that is down or + * unconfigured degrades to `unavailable: true` with a 200 rather than erroring — the client reads that + * flag to render nothing and stop polling, exactly as the taxonomy fields read does. + */ +export async function getV3EnrichmentStatus(params: { + authentication: TV3Authentication; + workspaceId: string; + requestId: string; + instance?: string; +}): Promise { + const { authentication, workspaceId, requestId, instance } = params; + + const context = await requireUnifyFeedbackWorkspaceAccess( + authentication, + workspaceId, + "read", + requestId, + instance + ); + if (context instanceof Response) return context; + + const directories = await getFeedbackDirectoriesByWorkspaceId(context.workspaceId); + if (directories.length === 0) { + return successResponse({ enrichments: [], unavailable: false }, { requestId }); + } + + const results = await Promise.all(directories.map((directory) => getEnrichmentStatus(directory.id))); + + // A directory that failed is left out rather than counted as zero — folding a failure in as "nothing + // done yet" would invent a backlog. If every directory failed there is nothing to show at all. + const statuses = results + .map((result) => result.data) + .filter((data): data is EnrichmentStatusResponse => data !== null); + + if (statuses.length === 0) { + return successResponse({ enrichments: [], unavailable: true }, { requestId }); + } + + return successResponse( + { enrichments: aggregateEnrichmentStatus(statuses), unavailable: false }, + { requestId } + ); +} diff --git a/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/schemas.ts b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/schemas.ts new file mode 100644 index 000000000000..2c97a4df8f9a --- /dev/null +++ b/apps/web/app/api/v3/unify-feedback/enrichment-status/lib/schemas.ts @@ -0,0 +1,9 @@ +import { z } from "zod"; + +/** + * The enrichment-status read is addressed by workspace alone: the directories it covers are resolved + * server-side from that workspace, so the Hub `tenant_id` is never taken from the caller. + */ +export const ZEnrichmentStatusQuery = z.object({ workspaceId: z.cuid2() }).strict(); + +export type TEnrichmentStatusQuery = z.infer; diff --git a/apps/web/app/api/v3/unify-feedback/enrichment-status/route.ts b/apps/web/app/api/v3/unify-feedback/enrichment-status/route.ts new file mode 100644 index 000000000000..79a00035962c --- /dev/null +++ b/apps/web/app/api/v3/unify-feedback/enrichment-status/route.ts @@ -0,0 +1,22 @@ +/** + * GET /api/v3/unify-feedback/enrichment-status — per-enrichment progress (translation, sentiment, + * emotions) across a workspace's feedback directories. Feeds the indicator above the records table. + * Session-only. + */ +import { withV3ApiWrapper } from "@/app/api/v3/lib/api-wrapper"; +import { getV3EnrichmentStatus } from "./lib/operations"; +import { ZEnrichmentStatusQuery } from "./lib/schemas"; + +export const GET = withV3ApiWrapper({ + auth: "session", + schemas: { + query: ZEnrichmentStatusQuery, + }, + handler: async ({ authentication, parsedInput, requestId, instance }) => + getV3EnrichmentStatus({ + authentication, + workspaceId: parsedInput.query.workspaceId, + requestId, + instance, + }), +}); diff --git a/apps/web/i18n.lock b/apps/web/i18n.lock index d368706808c5..5856e624172d 100644 --- a/apps/web/i18n.lock +++ b/apps/web/i18n.lock @@ -3859,6 +3859,11 @@ checksums: workspace/unify/edit_csv_mapping: 4f3bad444664d58ffe8ace3dc9e200f9 workspace/unify/edit_source_connection: eee85426384e6569665ac2b342ca02e4 workspace/unify/emotions: 130553c4f824567f63931b26e7340a78 + workspace/unify/enrichment_failed_count: 4b4e86695f1ac024d7551558ec362838 + workspace/unify/enrichment_failed_summary_title: 029f2aae3e3df5963b2701f7f4bf3810 + workspace/unify/enrichment_in_progress: 66710e3dfb3ed4554250e18c2f598b56 + workspace/unify/enrichment_pending_summary: d984f31ed0cd99996b5e06dc8eea8a77 + workspace/unify/enrichment_progress_count: e2550a14c74fdbc0a946563c46e79fd0 workspace/unify/enter_name_for_source: de6d02a0a8ccc99204ad831ca6dcdbd3 workspace/unify/enum: 96fc644f35edd6b1c09d1d503f078acc workspace/unify/error_source_directory_not_assigned: 74aa6d4705a984b37f0ad84f1b40b6dc @@ -4047,6 +4052,7 @@ checksums: workspace/unify/translated_text: aa93e50a0559cbe99aedc954d649bd34 workspace/unify/translated_text_hint: 95276e457e89bf50e4fc1ccf9023b128 workspace/unify/translated_to: ad58c74e181eba2101928648fa5f2457 + workspace/unify/translation: 9276816e7bc1a6ff00f388faa0863986 workspace/unify/unify_feedback: bdb518a1e62f51049ccc4366b909fb0a workspace/unify/update_mapping_description: 58d5966c0c9b406c037dff3aa8bcb396 workspace/unify/updated_at: 8fdb85248e591254973403755dcc3724 diff --git a/apps/web/lib/user/password.test.ts b/apps/web/lib/user/password.test.ts index 145c5df0379a..583c74bcd5db 100644 --- a/apps/web/lib/user/password.test.ts +++ b/apps/web/lib/user/password.test.ts @@ -2,12 +2,13 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; import { prisma } from "@formbricks/database"; import { InvalidInputError } from "@formbricks/types/errors"; import { verifyPassword } from "@/modules/auth/lib/utils"; -import { getCredentialPasswordHash, verifyUserPassword } from "./password"; +import { getCredentialPasswordHash, hasCredentialAccount, verifyUserPassword } from "./password"; vi.mock("@formbricks/database", () => ({ prisma: { account: { findUnique: vi.fn(), + count: vi.fn(), }, }, })); @@ -74,4 +75,27 @@ describe("user password helpers", () => { expect(mockVerifyPassword).not.toHaveBeenCalled(); }); + + test("hasCredentialAccount uses Better Auth's own credential-account predicate", async () => { + vi.mocked(prisma.account.count).mockResolvedValue(1); + + await expect(hasCredentialAccount("user-1")).resolves.toBe(true); + // The same four-column predicate Better Auth's `findCredentialAccount` uses. Anything broader would + // answer "may reset" for a row `resetPassword` cannot then find, which turns into a unique-constraint + // collision on its create branch rather than a reset. + expect(prisma.account.count).toHaveBeenCalledWith({ + where: { + userId: "user-1", + provider: "credential", + issuer: "local:credential", + providerAccountId: "user-1", + }, + }); + }); + + test("hasCredentialAccount is false for an SSO-only user", async () => { + vi.mocked(prisma.account.count).mockResolvedValue(0); + + await expect(hasCredentialAccount("user-2")).resolves.toBe(false); + }); }); diff --git a/apps/web/lib/user/password.ts b/apps/web/lib/user/password.ts index f255b387403c..1eceaaead995 100644 --- a/apps/web/lib/user/password.ts +++ b/apps/web/lib/user/password.ts @@ -1,4 +1,5 @@ import "server-only"; +import { createLocalAccountIssuer } from "@better-auth/core/db"; import { cache as reactCache } from "react"; import { prisma } from "@formbricks/database"; import { InvalidInputError } from "@formbricks/types/errors"; @@ -32,3 +33,36 @@ export const verifyUserPassword = async (userId: string, password: string): Prom return await verifyPassword(password, passwordHash); }; + +/** + * Whether the user has a Better Auth `credential` Account row at all — i.e. whether they are (or once + * were) a password user, independently of whether a password is currently set on it. + * + * Deliberately the SAME predicate Better Auth's own `findCredentialAccount` uses — `userId`, provider + * `credential`, the `local:credential` issuer, and `accountId` equal to the user id + * (`better-auth/dist/db/internal-adapter.mjs`). That match matters because this answers "may this user + * reset a password?", and the only consumer of the answer is `resetPassword`, which locates the row with + * that exact predicate: erring broader would return true for a row whose key has drifted, the reset + * would then take its create-a-row branch, and that can collide with `@@unique([provider, + * providerAccountId])` — a 500 instead of a reset. + * + * Note this is the opposite trade-off from the SSO-recovery strip, which scopes by `userId` alone on + * purpose: a strip must never miss a live hash, so over-matching is the safe direction there. Here + * over-matching promises something the next call cannot deliver, so under-matching is. Same schema, two + * different questions. + * + * `createLocalAccountIssuer` rather than the `"local:credential"` literal so this tracks upstream if the + * namespace ever changes — the same reason the Playwright user fixture uses it. + */ +export const hasCredentialAccount = reactCache(async (userId: string): Promise => { + const count = await prisma.account.count({ + where: { + userId, + provider: "credential", + issuer: createLocalAccountIssuer("credential"), + providerAccountId: userId, + }, + }); + + return count > 0; +}); diff --git a/apps/web/locales/de-DE.json b/apps/web/locales/de-DE.json index 96bcba96740a..35a273d98c06 100644 --- a/apps/web/locales/de-DE.json +++ b/apps/web/locales/de-DE.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "CSV-Zuordnung bearbeiten", "edit_source_connection": "Feedback-Quelle bearbeiten", "emotions": "Emotionen", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} fehlgeschlagen} other {{failedCount, number} fehlgeschlagen}}", + "enrichment_failed_summary_title": "Einige Feedbacks konnten nicht angereichert werden", + "enrichment_in_progress": "Feedback wird angereichert...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} Datensatz ausstehend} other {{pending, number} Datensätze ausstehend}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} Datensatz} other {{done, number} / {eligible, number} Datensätze}}", "enter_name_for_source": "Gib einen Namen für diese Quelle ein", "enum": "Aufzählung", "error_source_directory_not_assigned": "Dieses Feedback-Verzeichnis ist diesem Workspace nicht zugewiesen.", @@ -4207,6 +4212,7 @@ "translated_text": "Übersetzter Text", "translated_text_hint": "Automatisch aus dem ursprünglichen Feedback übersetzt.", "translated_to": "Übersetzt auf {language}", + "translation": "Übersetzung", "unify_feedback": "Vereinheitlichen", "update_mapping_description": "Aktualisiere die Zuordnungskonfiguration für diese Quelle.", "updated_at": "Aktualisiert am", diff --git a/apps/web/locales/en-US.json b/apps/web/locales/en-US.json index 9125715bd1ab..edb6213b982e 100644 --- a/apps/web/locales/en-US.json +++ b/apps/web/locales/en-US.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Edit CSV mapping", "edit_source_connection": "Edit feedback source", "emotions": "Emotions", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} failed} other {{failedCount, number} failed}}", + "enrichment_failed_summary_title": "Some feedback couldn't be enriched", + "enrichment_in_progress": "Enriching feedback...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} record pending} other {{pending, number} records pending}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} record} other {{done, number} / {eligible, number} records}}", "enter_name_for_source": "Enter a name for this source", "enum": "enum", "error_source_directory_not_assigned": "This feedback directory is not assigned to this workspace.", @@ -4207,6 +4212,7 @@ "translated_text": "Translated text", "translated_text_hint": "Automatically translated from the original feedback.", "translated_to": "Translated to {language}", + "translation": "Translation", "unify_feedback": "Unify", "update_mapping_description": "Update the mapping configuration for this source.", "updated_at": "Updated at", diff --git a/apps/web/locales/es-ES.json b/apps/web/locales/es-ES.json index fe00b690c3ce..987c2416c52d 100644 --- a/apps/web/locales/es-ES.json +++ b/apps/web/locales/es-ES.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Editar mapeo de CSV", "edit_source_connection": "Editar fuente de comentarios", "emotions": "Emociones", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} con error} other {{failedCount, number} con error}}", + "enrichment_failed_summary_title": "No se pudo enriquecer parte del feedback", + "enrichment_in_progress": "Enriqueciendo comentarios...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} registro pendiente} other {{pending, number} registros pendientes}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} registro} other {{done, number} / {eligible, number} registros}}", "enter_name_for_source": "Introduce un nombre para este origen", "enum": "enum", "error_source_directory_not_assigned": "Este directorio de feedback no está asignado a este espacio de trabajo.", @@ -4207,6 +4212,7 @@ "translated_text": "Texto traducido", "translated_text_hint": "Traducido automáticamente del feedback original.", "translated_to": "Traducido a {language}", + "translation": "Traducción", "unify_feedback": "Unificar", "update_mapping_description": "Actualiza la configuración de mapeo para esta fuente.", "updated_at": "Actualizado el", diff --git a/apps/web/locales/fr-FR.json b/apps/web/locales/fr-FR.json index 1397baf4a6c5..1a88f2cfcf64 100644 --- a/apps/web/locales/fr-FR.json +++ b/apps/web/locales/fr-FR.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Modifier le mappage CSV", "edit_source_connection": "Modifier la source de retours", "emotions": "Émotions", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} échec} other {{failedCount, number} échecs}}", + "enrichment_failed_summary_title": "Certains retours n'ont pas pu être enrichis", + "enrichment_in_progress": "Enrichissement des retours en cours...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} enregistrement en attente} other {{pending, number} enregistrements en attente}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} enregistrement} other {{done, number} / {eligible, number} enregistrements}}", "enter_name_for_source": "Entrez un nom pour cette source", "enum": "enum", "error_source_directory_not_assigned": "Ce répertoire de commentaires n'est pas assigné à cet espace de travail.", @@ -4207,6 +4212,7 @@ "translated_text": "Texte traduit", "translated_text_hint": "Traduit automatiquement à partir du retour initial.", "translated_to": "Traduit en {language}", + "translation": "Traduction", "unify_feedback": "Unifier", "update_mapping_description": "Mettre à jour la configuration de mappage pour cette source.", "updated_at": "Mis à jour à", diff --git a/apps/web/locales/hu-HU.json b/apps/web/locales/hu-HU.json index af6e274ad667..935a4cf49892 100644 --- a/apps/web/locales/hu-HU.json +++ b/apps/web/locales/hu-HU.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "CSV-leképezés szerkesztése", "edit_source_connection": "Visszajelzési forrás szerkesztése", "emotions": "Érzelmek", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} sikertelen} other {{failedCount, number} sikertelen}}", + "enrichment_failed_summary_title": "Néhány visszajelzést nem sikerült gazdagítani", + "enrichment_in_progress": "Visszajelzések feldolgozása folyamatban...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} rekord függőben} other {{pending, number} rekord függőben}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} rekord} other {{done, number} / {eligible, number} rekord}}", "enter_name_for_source": "Adjon nevet ennek a forrásnak", "enum": "felsorolás", "error_source_directory_not_assigned": "Ez a visszajelzési könyvtár nincs hozzárendelve ehhez a munkaterülethez.", @@ -4207,6 +4212,7 @@ "translated_text": "Lefordított szöveg", "translated_text_hint": "Automatikusan lefordítva az eredeti visszajelzésből.", "translated_to": "Lefordítva erre: {language}", + "translation": "Fordítás", "unify_feedback": "Egyesítés", "update_mapping_description": "A leképezési beállítás frissítése ennél a forrásnál.", "updated_at": "Frissítve", diff --git a/apps/web/locales/ja-JP.json b/apps/web/locales/ja-JP.json index d22b1806a8e8..02960dec3bb6 100644 --- a/apps/web/locales/ja-JP.json +++ b/apps/web/locales/ja-JP.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "CSVマッピングを編集", "edit_source_connection": "フィードバックソースを編集", "emotions": "感情", + "enrichment_failed_count": "{failedCount, plural, other {{failedCount, number}件が失敗しました}}", + "enrichment_failed_summary_title": "一部のフィードバックをエンリッチできませんでした", + "enrichment_in_progress": "フィードバックを強化中...", + "enrichment_pending_summary": "{pending, plural, other {{pending, number}件のレコードが保留中}}", + "enrichment_progress_count": "{eligible, plural, other {{done, number} / {eligible, number}件のレコード}}", "enter_name_for_source": "このソースの名前を入力", "enum": "列挙型", "error_source_directory_not_assigned": "このフィードバックディレクトリはこのワークスペースに割り当てられていません。", @@ -4207,6 +4212,7 @@ "translated_text": "翻訳されたテキスト", "translated_text_hint": "元のフィードバックから自動的に翻訳されました。", "translated_to": "{language}に翻訳", + "translation": "翻訳", "unify_feedback": "統合", "update_mapping_description": "このソースのマッピング設定を更新します。", "updated_at": "更新日時", diff --git a/apps/web/locales/nl-NL.json b/apps/web/locales/nl-NL.json index 1b15322ba184..bd7b51a63c0e 100644 --- a/apps/web/locales/nl-NL.json +++ b/apps/web/locales/nl-NL.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "CSV-mapping bewerken", "edit_source_connection": "Feedbackbron bewerken", "emotions": "Emoties", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} mislukt} other {{failedCount, number} mislukt}}", + "enrichment_failed_summary_title": "Sommige feedback kon niet worden verrijkt", + "enrichment_in_progress": "Feedback verrijken...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} record in behandeling} other {{pending, number} records in behandeling}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} record} other {{done, number} / {eligible, number} records}}", "enter_name_for_source": "Voer een naam in voor deze bron", "enum": "enum", "error_source_directory_not_assigned": "Deze feedbackdirectory is niet toegewezen aan deze workspace.", @@ -4207,6 +4212,7 @@ "translated_text": "Vertaalde tekst", "translated_text_hint": "Automatisch vertaald vanuit de oorspronkelijke feedback.", "translated_to": "Vertaald naar {language}", + "translation": "Vertaling", "unify_feedback": "Unify", "update_mapping_description": "Werk de mappingconfiguratie voor deze bron bij.", "updated_at": "Bijgewerkt op", diff --git a/apps/web/locales/pt-BR.json b/apps/web/locales/pt-BR.json index 7ecba507043b..3805c036e4db 100644 --- a/apps/web/locales/pt-BR.json +++ b/apps/web/locales/pt-BR.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Editar mapeamento CSV", "edit_source_connection": "Editar fonte de feedback", "emotions": "Emoções", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} falhou} other {{failedCount, number} falharam}}", + "enrichment_failed_summary_title": "Alguns feedbacks não puderam ser enriquecidos", + "enrichment_in_progress": "Enriquecendo feedback...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} registro pendente} other {{pending, number} registros pendentes}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} registro} other {{done, number} / {eligible, number} registros}}", "enter_name_for_source": "Digite um nome para esta origem", "enum": "enum", "error_source_directory_not_assigned": "Este diretório de feedback não está atribuído a este workspace.", @@ -4207,6 +4212,7 @@ "translated_text": "Texto traduzido", "translated_text_hint": "Traduzido automaticamente do feedback original.", "translated_to": "Traduzido para {language}", + "translation": "Tradução", "unify_feedback": "Unificar", "update_mapping_description": "Atualize a configuração de mapeamento para esta fonte.", "updated_at": "Atualizado em", diff --git a/apps/web/locales/pt-PT.json b/apps/web/locales/pt-PT.json index 45e83d537848..df299ba38843 100644 --- a/apps/web/locales/pt-PT.json +++ b/apps/web/locales/pt-PT.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Editar mapeamento CSV", "edit_source_connection": "Editar fonte de feedback", "emotions": "Emoções", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} falhou} other {{failedCount, number} falharam}}", + "enrichment_failed_summary_title": "Alguns feedbacks não puderam ser enriquecidos", + "enrichment_in_progress": "A enriquecer feedback...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} registo pendente} other {{pending, number} registos pendentes}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} registo} other {{done, number} / {eligible, number} registos}}", "enter_name_for_source": "Introduz um nome para esta origem", "enum": "enum", "error_source_directory_not_assigned": "Este diretório de feedback não está atribuído a este espaço de trabalho.", @@ -4207,6 +4212,7 @@ "translated_text": "Texto traduzido", "translated_text_hint": "Traduzido automaticamente a partir do feedback original.", "translated_to": "Traduzido para {language}", + "translation": "Tradução", "unify_feedback": "Unificar", "update_mapping_description": "Atualiza a configuração de mapeamento para esta origem.", "updated_at": "Atualizado em", diff --git a/apps/web/locales/ro-RO.json b/apps/web/locales/ro-RO.json index b842224325fa..1a393a24dc75 100644 --- a/apps/web/locales/ro-RO.json +++ b/apps/web/locales/ro-RO.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Editează maparea CSV", "edit_source_connection": "Editează sursa de feedback", "emotions": "Emoții", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} eșuat} few {{failedCount, number} eșuate} other {{failedCount, number} de eșuate}}", + "enrichment_failed_summary_title": "Unele feedback-uri nu au putut fi îmbogățite", + "enrichment_in_progress": "Se îmbogățește feedback-ul...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} înregistrare în așteptare} few {{pending, number} înregistrări în așteptare} other {{pending, number} de înregistrări în așteptare}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} înregistrare} few {{done, number} / {eligible, number} înregistrări} other {{done, number} / {eligible, number} de înregistrări}}", "enter_name_for_source": "Introdu un nume pentru această sursă", "enum": "enum", "error_source_directory_not_assigned": "Acest director de feedback nu este atribuit acestui spațiu de lucru.", @@ -4207,6 +4212,7 @@ "translated_text": "Text tradus", "translated_text_hint": "Tradus automat din feedbackul original.", "translated_to": "Tradus în {language}", + "translation": "Traducere", "unify_feedback": "Unifică", "update_mapping_description": "Actualizează configurația de mapare pentru această sursă.", "updated_at": "Actualizat la", diff --git a/apps/web/locales/ru-RU.json b/apps/web/locales/ru-RU.json index 5a0b4b2d5d1f..fca3291da41c 100644 --- a/apps/web/locales/ru-RU.json +++ b/apps/web/locales/ru-RU.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Редактировать сопоставление CSV", "edit_source_connection": "Редактировать источник отзывов", "emotions": "Эмоции", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} не удалось обработать} few {{failedCount, number} не удалось обработать} many {{failedCount, number} не удалось обработать} other {{failedCount, number} не удалось обработать}}", + "enrichment_failed_summary_title": "Не удалось обогатить некоторые отзывы", + "enrichment_in_progress": "Обогащение отзывов...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} запись ожидает обработки} few {{pending, number} записи ожидают обработки} many {{pending, number} записей ожидают обработки} other {{pending, number} записи ожидают обработки}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} из {eligible, number} записи} few {{done, number} из {eligible, number} записей} many {{done, number} из {eligible, number} записей} other {{done, number} из {eligible, number} записи}}", "enter_name_for_source": "Введи имя для этого источника", "enum": "enum", "error_source_directory_not_assigned": "Этот каталог отзывов не назначен этому рабочему пространству.", @@ -4207,6 +4212,7 @@ "translated_text": "Переведённый текст", "translated_text_hint": "Автоматически переведено из исходного отзыва.", "translated_to": "Переведено на {language}", + "translation": "Перевод", "unify_feedback": "Объединить", "update_mapping_description": "Обнови настройки сопоставления для этого источника.", "updated_at": "Обновлено", diff --git a/apps/web/locales/sv-SE.json b/apps/web/locales/sv-SE.json index 96a0d297e672..d7d77524be3d 100644 --- a/apps/web/locales/sv-SE.json +++ b/apps/web/locales/sv-SE.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "Redigera CSV-mappning", "edit_source_connection": "Redigera feedbackkälla", "emotions": "Känslor", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} misslyckades} other {{failedCount, number} misslyckades}}", + "enrichment_failed_summary_title": "En del feedback kunde inte berikas", + "enrichment_in_progress": "Berikar feedback...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} post väntar} other {{pending, number} poster väntar}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} post} other {{done, number} / {eligible, number} poster}}", "enter_name_for_source": "Ange ett namn för denna källa", "enum": "enum", "error_source_directory_not_assigned": "Den här feedbackkatalogen är inte tilldelad den här arbetsytan.", @@ -4207,6 +4212,7 @@ "translated_text": "Översatt text", "translated_text_hint": "Översatt automatiskt från den ursprungliga feedbacken.", "translated_to": "Översatt till {language}", + "translation": "Översättning", "unify_feedback": "Förena", "update_mapping_description": "Uppdatera mappningskonfigurationen för den här källan.", "updated_at": "Uppdaterad", diff --git a/apps/web/locales/tr-TR.json b/apps/web/locales/tr-TR.json index e192dd58ce30..0b542fe6b5f1 100644 --- a/apps/web/locales/tr-TR.json +++ b/apps/web/locales/tr-TR.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "CSV eşlemesini düzenle", "edit_source_connection": "Geri bildirim kaynağını düzenle", "emotions": "Duygular", + "enrichment_failed_count": "{failedCount, plural, one {{failedCount, number} başarısız} other {{failedCount, number} başarısız}}", + "enrichment_failed_summary_title": "Bazı geri bildirimler zenginleştirilemedi", + "enrichment_in_progress": "Geri bildirim zenginleştiriliyor...", + "enrichment_pending_summary": "{pending, plural, one {{pending, number} kayıt bekliyor} other {{pending, number} kayıt bekliyor}}", + "enrichment_progress_count": "{eligible, plural, one {{done, number} / {eligible, number} kayıt} other {{done, number} / {eligible, number} kayıt}}", "enter_name_for_source": "Bu kaynak için bir ad girin", "enum": "enum", "error_source_directory_not_assigned": "Bu geri bildirim dizini bu çalışma alanına atanmamış.", @@ -4207,6 +4212,7 @@ "translated_text": "Çevrilen metin", "translated_text_hint": "Orijinal geri bildirimden otomatik olarak çevrildi.", "translated_to": "{language} diline çevrildi", + "translation": "Çeviri", "unify_feedback": "Birleştir", "update_mapping_description": "Bu kaynak için eşleme yapılandırmasını güncelle.", "updated_at": "Güncellenme tarihi", diff --git a/apps/web/locales/zh-Hans-CN.json b/apps/web/locales/zh-Hans-CN.json index 658f4f60b969..f10e8c6f4431 100644 --- a/apps/web/locales/zh-Hans-CN.json +++ b/apps/web/locales/zh-Hans-CN.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "编辑 CSV 映射", "edit_source_connection": "编辑反馈来源", "emotions": "情绪", + "enrichment_failed_count": "{failedCount, plural, other {{failedCount, number} 条失败}}", + "enrichment_failed_summary_title": "部分反馈无法丰富", + "enrichment_in_progress": "正在丰富反馈内容...", + "enrichment_pending_summary": "{pending, plural, other {{pending, number} 条记录待处理}}", + "enrichment_progress_count": "{eligible, plural, other {{done, number} / {eligible, number} 条记录}}", "enter_name_for_source": "为此来源输入名称", "enum": "枚举", "error_source_directory_not_assigned": "此反馈目录未分配给此工作区。", @@ -4207,6 +4212,7 @@ "translated_text": "翻译后的文本", "translated_text_hint": "根据原始反馈自动翻译。", "translated_to": "已翻译为{language}", + "translation": "翻译", "unify_feedback": "统一", "update_mapping_description": "更新此来源的映射配置。", "updated_at": "更新于", diff --git a/apps/web/locales/zh-Hant-TW.json b/apps/web/locales/zh-Hant-TW.json index aef47f96cc88..d388ae8b1662 100644 --- a/apps/web/locales/zh-Hant-TW.json +++ b/apps/web/locales/zh-Hant-TW.json @@ -4019,6 +4019,11 @@ "edit_csv_mapping": "編輯 CSV 對應", "edit_source_connection": "編輯回饋來源", "emotions": "情緒", + "enrichment_failed_count": "{failedCount, plural, other {{failedCount, number} 個失敗}}", + "enrichment_failed_summary_title": "部分意見回饋無法進行擴充", + "enrichment_in_progress": "正在豐富化回饋內容...", + "enrichment_pending_summary": "{pending, plural, other {{pending, number} 筆記錄待處理}}", + "enrichment_progress_count": "{eligible, plural, other {{done, number} / {eligible, number} 筆記錄}}", "enter_name_for_source": "請輸入此來源的名稱", "enum": "enum", "error_source_directory_not_assigned": "此回饋目錄尚未指派給此工作區。", @@ -4207,6 +4212,7 @@ "translated_text": "翻譯後的文字", "translated_text_hint": "根據原始回饋自動翻譯。", "translated_to": "已翻譯為{language}", + "translation": "翻譯", "unify_feedback": "統整", "update_mapping_description": "更新此來源的對應設定。", "updated_at": "更新時間", diff --git a/apps/web/modules/auth/forgot-password/actions.test.ts b/apps/web/modules/auth/forgot-password/actions.test.ts index 723af153c7d1..e7694860da59 100644 --- a/apps/web/modules/auth/forgot-password/actions.test.ts +++ b/apps/web/modules/auth/forgot-password/actions.test.ts @@ -7,14 +7,49 @@ import { applyIPRateLimit } from "@/modules/core/rate-limit/helpers"; import { rateLimitConfigs } from "@/modules/core/rate-limit/rate-limit-configs"; import { forgotPasswordAction } from "./actions"; +const mocks = vi.hoisted(() => ({ + hasCredentialAccount: vi.fn(), + // Held in a box and exposed through a getter below so a single test can flip it: `EMAIL_AUTH_ENABLED` is + // a const import in the action, and the getter keeps the live binding readable per call. + emailAuthEnabled: { value: true }, + // The wrapper is applied once at module import, so `vi.resetAllMocks()` in beforeEach would wipe the + // call history before any test could read it. A plain array on the hoisted object survives the reset. + auditWrapperArgs: [] as [string, string][], +})); + const allowedRateLimitResponse = { allowed: true }; + +/** Fresh audit context per call — the action writes `userId` / `suppressEvent` onto it. */ +let auditLoggingCtx: Record; +const callAction = (input: { email: string } = { email: "test@example.com" }) => { + auditLoggingCtx = {}; + return forgotPasswordAction({ ctx: { auditLoggingCtx }, parsedInput: input } as any); +}; const RESET_REDIRECT = "http://localhost:3000/auth/forgot-password/reset"; vi.mock("@/lib/constants", () => ({ + get EMAIL_AUTH_ENABLED() { + return mocks.emailAuthEnabled.value; + }, PASSWORD_RESET_DISABLED: false, WEBAPP_URL: "http://localhost:3000", })); +// Mocked at the module boundary rather than letting the real one load: `lib/user/password` pulls in +// `lib/crypto`, which reads ENCRYPTION_KEY from the (fully replaced) constants mock at import time. +vi.mock("@/lib/user/password", () => ({ + hasCredentialAccount: mocks.hasCredentialAccount, +})); + +// Passthrough so the handler runs directly, matching modules/ee/billing/actions.test.ts. Importing the +// real handler would drag the audit-log graph (and its POSTHOG_KEY constant read) into this suite. +vi.mock("@/modules/ee/audit-logs/lib/handler", () => ({ + withAuditLogging: vi.fn((action: string, target: string, fn: unknown) => { + mocks.auditWrapperArgs.push([action, target]); + return fn; + }), +})); + vi.mock("@/modules/core/rate-limit/helpers", () => ({ applyIPRateLimit: vi.fn(), })); @@ -57,6 +92,10 @@ describe("forgotPasswordAction", () => { beforeEach(() => { vi.resetAllMocks(); + mocks.emailAuthEnabled.value = true; + // `vi.resetAllMocks()` does not touch this, so reset it here too: a future test that asserts on + // `auditLoggingCtx` without calling `callAction` would otherwise read the previous test's object. + auditLoggingCtx = {}; }); afterEach(() => { @@ -67,7 +106,7 @@ describe("forgotPasswordAction", () => { test("applies rate limiting (with the right config) before looking up the user", async () => { vi.mocked(getUserByEmail).mockResolvedValue(mockUser as any); - await forgotPasswordAction({ parsedInput: validInput } as any); + await callAction(validInput); expect(applyIPRateLimit).toHaveBeenCalledWith(rateLimitConfigs.auth.forgotPassword); expect(applyIPRateLimit).toHaveBeenCalledBefore(getUserByEmail as any); @@ -78,7 +117,7 @@ describe("forgotPasswordAction", () => { new Error("Maximum number of requests reached. Please try again later.") ); - await expect(forgotPasswordAction({ parsedInput: validInput } as any)).rejects.toThrow( + await expect(callAction(validInput)).rejects.toThrow( "Maximum number of requests reached. Please try again later." ); @@ -92,7 +131,7 @@ describe("forgotPasswordAction", () => { vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); vi.mocked(getUserByEmail).mockResolvedValue(mockUser as any); - const result = await forgotPasswordAction({ parsedInput: validInput } as any); + const result = await callAction(validInput); expect(getUserByEmail).toHaveBeenCalledWith(validInput.email); expect(auth.api.requestPasswordReset).toHaveBeenCalledWith({ @@ -106,21 +145,130 @@ describe("forgotPasswordAction", () => { vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); vi.mocked(getUserByEmail).mockResolvedValue(null); - const result = await forgotPasswordAction({ parsedInput: validInput } as any); + const result = await callAction(validInput); expect(auth.api.requestPasswordReset).not.toHaveBeenCalled(); expect(result).toEqual({ success: true }); }); - test("does not request a reset for a non-email identity provider", async () => { + test("does not request a reset for an SSO user with no credential account", async () => { + vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); + vi.mocked(getUserByEmail).mockResolvedValue({ ...mockUser, identityProvider: "google" } as any); + mocks.hasCredentialAccount.mockResolvedValue(false); + + const result = await callAction(validInput); + + expect(auth.api.requestPasswordReset).not.toHaveBeenCalled(); + expect(result).toEqual({ success: true }); + }); + }); + + /** + * SSO recovery is one-way: it flips `identityProvider` to the SSO provider and nothing flips it back, + * while clearing the password it found. Gated on `identityProvider` alone these users could never ask + * for a reset again, so the surviving credential `Account` row is what lets them back in (ENG-2557). + */ + describe("Recovered SSO users (ENG-2557)", () => { + beforeEach(() => { vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); + }); + + test("requests a reset for an SSO-identity user who still has a credential account", async () => { vi.mocked(getUserByEmail).mockResolvedValue({ ...mockUser, identityProvider: "google" } as any); + mocks.hasCredentialAccount.mockResolvedValue(true); + + const result = await callAction(validInput); - const result = await forgotPasswordAction({ parsedInput: validInput } as any); + expect(mocks.hasCredentialAccount).toHaveBeenCalledWith(mockUser.id); + expect(auth.api.requestPasswordReset).toHaveBeenCalledWith({ + body: { email: mockUser.email, redirectTo: RESET_REDIRECT }, + headers: expect.any(Headers), + }); + expect(result).toEqual({ success: true }); + }); + test("stays shut on an SSO-only instance, even with a credential account present", async () => { + mocks.emailAuthEnabled.value = false; + vi.mocked(getUserByEmail).mockResolvedValue({ ...mockUser, identityProvider: "google" } as any); + mocks.hasCredentialAccount.mockResolvedValue(true); + + const result = await callAction(validInput); + + // Handing a password back where the operator disabled credential auth would be the "sign in around + // the IdP" bypass that switching it off exists to prevent. expect(auth.api.requestPasswordReset).not.toHaveBeenCalled(); expect(result).toEqual({ success: true }); }); + + test("does not need the credential lookup for an email-identity user", async () => { + vi.mocked(getUserByEmail).mockResolvedValue(mockUser as any); + + await callAction(validInput); + + // Short-circuits on `identityProvider === "email"`, so the extra query never runs for the common case. + expect(mocks.hasCredentialAccount).not.toHaveBeenCalled(); + expect(auth.api.requestPasswordReset).toHaveBeenCalledOnce(); + }); + }); + + /** + * The action answers `{ success: true }` whether or not a reset was actually requested, so the audit + * wrapper cannot tell the two apart on its own — without `suppressEvent` it would record a + * `passwordReset` for an address that never got one. Same false-record problem as duplicate sign-up + * (ENG-2091), and these pin both directions. + */ + describe("Audit record", () => { + beforeEach(() => { + vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); + }); + + test("targets the audited event at the user when a reset is actually requested", async () => { + vi.mocked(getUserByEmail).mockResolvedValue(mockUser as any); + + await callAction(validInput); + + expect(auditLoggingCtx.userId).toBe(mockUser.id); + expect(auditLoggingCtx.suppressEvent).toBeUndefined(); + }); + + test("suppresses the event for an address with no account", async () => { + vi.mocked(getUserByEmail).mockResolvedValue(null); + + await callAction(validInput); + + expect(auditLoggingCtx.suppressEvent).toBe(true); + expect(auditLoggingCtx.userId).toBeUndefined(); + }); + + /** + * Without this the whole audit story is unobserved: `withAuditLogging` is mocked as a passthrough and + * `actionClient.action` returns the handler, so deleting the wrapper from the action entirely would + * leave every other test in this file green — including the ones below. This is the only assertion + * that the event is emitted under the right action and target at all. + */ + test("wires the wrapper with the right audit action and target", () => { + expect(mocks.auditWrapperArgs).toContainEqual(["passwordReset", "user"]); + }); + + test("suppresses the event when the reset email fails to send", async () => { + vi.mocked(getUserByEmail).mockResolvedValue(mockUser as any); + vi.mocked(auth.api.requestPasswordReset).mockRejectedValue(new Error("smtp down")); + + const result = await callAction(validInput); + + // The action still reports success, so an unsuppressed event would claim a link was mailed. + expect(result).toEqual({ success: true }); + expect(auditLoggingCtx.suppressEvent).toBe(true); + }); + + test("suppresses the event for a user with no password to reset", async () => { + vi.mocked(getUserByEmail).mockResolvedValue({ ...mockUser, identityProvider: "google" } as any); + mocks.hasCredentialAccount.mockResolvedValue(false); + + await callAction(validInput); + + expect(auditLoggingCtx.suppressEvent).toBe(true); + }); }); describe("Error Handling", () => { @@ -129,7 +277,7 @@ describe("forgotPasswordAction", () => { vi.mocked(getUserByEmail).mockResolvedValue(mockUser as any); vi.mocked(auth.api.requestPasswordReset).mockRejectedValue(new Error("BA request failed")); - await expect(forgotPasswordAction({ parsedInput: validInput } as any)).resolves.toEqual({ + await expect(callAction(validInput)).resolves.toEqual({ success: true, }); expect(logger.error).toHaveBeenCalledWith( @@ -138,13 +286,21 @@ describe("forgotPasswordAction", () => { ); }); + test("still reports success when the credential lookup throws", async () => { + vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); + vi.mocked(getUserByEmail).mockResolvedValue({ ...mockUser, identityProvider: "google" } as any); + mocks.hasCredentialAccount.mockRejectedValue(new Error("db down")); + + // The whole point of the fail-closed catch: no reset, no escaping error. + await expect(callAction(validInput)).resolves.toEqual({ success: true }); + expect(auth.api.requestPasswordReset).not.toHaveBeenCalled(); + }); + test("propagates a user-lookup error", async () => { vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); vi.mocked(getUserByEmail).mockRejectedValue(new Error("Database error")); - await expect(forgotPasswordAction({ parsedInput: validInput } as any)).rejects.toThrow( - "Database error" - ); + await expect(callAction(validInput)).rejects.toThrow("Database error"); }); }); @@ -153,14 +309,15 @@ describe("forgotPasswordAction", () => { vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); vi.mocked(getUserByEmail).mockResolvedValue(null); - expect(await forgotPasswordAction({ parsedInput: validInput } as any)).toEqual({ success: true }); + expect(await callAction(validInput)).toEqual({ success: true }); }); test("always returns success for an SSO user and never requests a reset", async () => { vi.mocked(applyIPRateLimit).mockResolvedValue(allowedRateLimitResponse); vi.mocked(getUserByEmail).mockResolvedValue({ ...mockUser, identityProvider: "github" } as any); + mocks.hasCredentialAccount.mockResolvedValue(false); - const result = await forgotPasswordAction({ parsedInput: validInput } as any); + const result = await callAction(validInput); expect(result).toEqual({ success: true }); expect(auth.api.requestPasswordReset).not.toHaveBeenCalled(); diff --git a/apps/web/modules/auth/forgot-password/actions.ts b/apps/web/modules/auth/forgot-password/actions.ts index a67407dc969e..ec70f12623c1 100644 --- a/apps/web/modules/auth/forgot-password/actions.ts +++ b/apps/web/modules/auth/forgot-password/actions.ts @@ -2,23 +2,75 @@ import { headers } from "next/headers"; import { z } from "zod"; +import type { IdentityProvider } from "@formbricks/database/prisma"; import { logger } from "@formbricks/logger"; import { OperationNotAllowedError } from "@formbricks/types/errors"; import { ZUserEmail } from "@formbricks/types/user"; -import { PASSWORD_RESET_DISABLED, WEBAPP_URL } from "@/lib/constants"; +import { EMAIL_AUTH_ENABLED, PASSWORD_RESET_DISABLED, WEBAPP_URL } from "@/lib/constants"; +import { hasCredentialAccount } from "@/lib/user/password"; import { actionClient } from "@/lib/utils/action-client"; import { auth } from "@/modules/auth/lib/auth"; import { getUserByEmail } from "@/modules/auth/lib/user"; import { applyIPRateLimit } from "@/modules/core/rate-limit/helpers"; import { rateLimitConfigs } from "@/modules/core/rate-limit/rate-limit-configs"; +import { withAuditLogging } from "@/modules/ee/audit-logs/lib/handler"; + +/** + * Whether this user has a password to reset. Pure SSO users do not, and are silently skipped — Better + * Auth's request endpoint is enumeration-safe and the action always reports success either way. + * + * The second arm exists because SSO recovery is one-way (ENG-2557): completing it flips + * `identityProvider` to the SSO provider and nothing ever flips it back, while the recovery also clears + * the password it found. Gated on `identityProvider` alone, those users could never ask for a reset again + * — locked to an IdP they might lose access to, with `auth.api.setPassword` being `serverOnly` and + * unwired. The surviving credential `Account` row identifies them: recovery nulls the password, it does + * not delete the row. + * + * Kept as narrow as that problem, deliberately: gating on the credential row rather than on + * `emailVerified` means this action grants nothing to a user who has only ever signed in via SSO, and + * `EMAIL_AUTH_ENABLED` switches the second arm off entirely on an SSO-only instance. Note that gate + * covers only the second arm — an `identityProvider === "email"` user still receives reset mail on an + * SSO-only instance, which is pre-existing behaviour this change deliberately leaves alone. So the flag + * here is about not *widening* that surface, not about closing it. + * + * Both are belt-and-braces rather than the enforcement boundary, and it is worth not mistaking one for + * the other. Better Auth's native `POST /api/auth/request-password-reset` is mounted by the `[...all]` + * catch-all unconditionally — it is NOT gated on `emailAndPassword.enabled` — and its `resetPassword` + * CREATES a credential row when none exists. So a password can be minted for any registered address + * regardless of this action. What actually contains that is `/sign-in/email`, which IS gated, so a minted + * password is unusable on an SSO-only instance. This relaxation therefore grants no reach that was not + * already there; it just stops the UI lying to a recovered user. + */ +const canResetPassword = async (user: { + id: string; + identityProvider: IdentityProvider; +}): Promise => { + if (user.identityProvider === "email") { + return true; + } + if (!EMAIL_AUTH_ENABLED) { + return false; + } + + try { + return await hasCredentialAccount(user.id); + } catch (error) { + // Fail closed rather than letting this escape. Note the action's `{ success: true }` is not an + // absolute invariant — `getUserByEmail` above it is unguarded and its `DatabaseError` is not an + // expected error, so it surfaces as a server error. That is address-independent, so it is not an + // enumeration oracle; this catch just avoids adding a second, narrower failure mode on a path that + // only runs for non-email identity providers. + logger.error({ error, userId: user.id }, "Credential-account lookup failed during password reset"); + return false; + } +}; const ZForgotPasswordAction = z.object({ email: ZUserEmail, }); -export const forgotPasswordAction = actionClient - .inputSchema(ZForgotPasswordAction) - .action(async ({ parsedInput }) => { +export const forgotPasswordAction = actionClient.inputSchema(ZForgotPasswordAction).action( + withAuditLogging("passwordReset", "user", async ({ ctx, parsedInput }) => { await applyIPRateLimit(rateLimitConfigs.auth.forgotPassword); if (PASSWORD_RESET_DISABLED) { @@ -27,18 +79,32 @@ export const forgotPasswordAction = actionClient const user = await getUserByEmail(parsedInput.email); - // Only credential (email-identity) users have a password to reset; SSO users are silently skipped. - // Better Auth's request endpoint is itself enumeration-safe, and we always return success below. - if (user && user.identityProvider === "email") { + if (user && (await canResetPassword(user))) { + // Target the audited event at the account the reset was requested for. The ACTOR stays + // `UNKNOWN_DATA` because this action is unauthenticated by design — which is the honest record: + // someone who knows the address asked for a reset. + ctx.auditLoggingCtx.userId = user.id; try { await auth.api.requestPasswordReset({ body: { email: user.email, redirectTo: `${WEBAPP_URL}/auth/forgot-password/reset` }, headers: await headers(), }); } catch (error) { + // The send failed but the action still answers `{ success: true }`, so without suppressing here + // the trail would claim a reset link was mailed to this user — the same false record the `else` + // branch guards, in the other direction. SMTP being down should not read as "we mailed them". + ctx.auditLoggingCtx.suppressEvent = true; logger.error({ error, userId: user.id }, "Password reset request failed"); } + } else { + // No reset was requested — unknown address, or a user with no password to reset. The action still + // answers `{ success: true }` to stay enumeration-safe, so without this the wrapper's fixed + // `passwordReset` action would record a reset that never happened (the same false-record problem + // `suppressEvent` was added for on duplicate sign-up, ENG-2091). A thrown failure is audited + // regardless, so this cannot hide one. + ctx.auditLoggingCtx.suppressEvent = true; } return { success: true }; - }); + }) +); diff --git a/apps/web/modules/auth/lib/auth-session-repository.test.ts b/apps/web/modules/auth/lib/auth-session-repository.test.ts index 0c2ada685460..1c9320f17418 100644 --- a/apps/web/modules/auth/lib/auth-session-repository.test.ts +++ b/apps/web/modules/auth/lib/auth-session-repository.test.ts @@ -2,67 +2,47 @@ import { afterEach, describe, expect, test, vi } from "vitest"; import { prisma } from "@formbricks/database"; import { Prisma } from "@formbricks/database/prisma"; import { DatabaseError } from "@formbricks/types/errors"; -import { deleteSessionBySessionToken } from "./auth-session-repository"; +import { getSessionTokensByUserId } from "./auth-session-repository"; vi.mock("@formbricks/database", () => ({ prisma: { session: { - deleteMany: vi.fn(), + findMany: vi.fn(), }, }, })); describe("auth-session-repository", () => { - const sessionToken = "session-token-cm8z6bn2q000008l34h8g7k9m"; - afterEach(() => { vi.clearAllMocks(); }); - test("deletes the session matching the given token", async () => { - vi.mocked(prisma.session.deleteMany).mockResolvedValue({ count: 1 }); - - const result = await deleteSessionBySessionToken(sessionToken); - - expect(result).toBe(1); - expect(prisma.session.deleteMany).toHaveBeenCalledWith({ - where: { sessionToken }, + test("lists a user's unexpired session tokens", async () => { + vi.mocked(prisma.session.findMany).mockResolvedValue([ + { sessionToken: "token-a" }, + { sessionToken: "token-b" }, + ] as never); + + await expect(getSessionTokensByUserId("user_1")).resolves.toEqual(["token-a", "token-b"]); + // Expired rows are excluded on purpose: the count feeds the SSO-recovery audit event, where it is + // read as "how many sessions the squatter was holding". + expect(prisma.session.findMany).toHaveBeenCalledWith({ + where: { userId: "user_1", expires: { gt: expect.any(Date) } }, + select: { sessionToken: true }, }); }); - test("returns zero when no session matches the token", async () => { - vi.mocked(prisma.session.deleteMany).mockResolvedValue({ count: 0 }); + test("returns an empty list for a user with no sessions", async () => { + vi.mocked(prisma.session.findMany).mockResolvedValue([] as never); - const result = await deleteSessionBySessionToken(sessionToken); - - expect(result).toBe(0); - }); - - test("uses the provided transaction client when available", async () => { - const txDeleteMany = vi.fn().mockResolvedValue({ count: 1 }); - const tx = { - session: { - deleteMany: txDeleteMany, - }, - } as unknown as Prisma.TransactionClient; - - const result = await deleteSessionBySessionToken(sessionToken, tx); - - expect(result).toBe(1); - expect(txDeleteMany).toHaveBeenCalledWith({ - where: { sessionToken }, - }); - expect(prisma.session.deleteMany).not.toHaveBeenCalled(); + await expect(getSessionTokensByUserId("user_1")).resolves.toEqual([]); }); test("wraps prisma known errors in DatabaseError", async () => { - vi.mocked(prisma.session.deleteMany).mockRejectedValue( - new Prisma.PrismaClientKnownRequestError("database failed", { - code: "P2021", - clientVersion: "test", - }) + vi.mocked(prisma.session.findMany).mockRejectedValue( + new Prisma.PrismaClientKnownRequestError("boom", { code: "P2024", clientVersion: "7.x" }) ); - await expect(deleteSessionBySessionToken(sessionToken)).rejects.toThrow(DatabaseError); + await expect(getSessionTokensByUserId("user_1")).rejects.toThrow(DatabaseError); }); }); diff --git a/apps/web/modules/auth/lib/auth-session-repository.ts b/apps/web/modules/auth/lib/auth-session-repository.ts index b1628a917e59..7455184628ea 100644 --- a/apps/web/modules/auth/lib/auth-session-repository.ts +++ b/apps/web/modules/auth/lib/auth-session-repository.ts @@ -1,14 +1,10 @@ import "server-only"; import { z } from "zod"; import { prisma } from "@formbricks/database"; -import { Prisma, PrismaClient } from "@formbricks/database/prisma"; +import { Prisma } from "@formbricks/database/prisma"; import { DatabaseError } from "@formbricks/types/errors"; import { validateInputs } from "@/lib/utils/validate"; -type TAuthSessionDbClient = PrismaClient | Prisma.TransactionClient; - -const getDbClient = (tx?: Prisma.TransactionClient): TAuthSessionDbClient => tx ?? prisma; - const handleDatabaseError = (error: unknown): never => { if (error instanceof Prisma.PrismaClientKnownRequestError) { throw new DatabaseError(error.message); @@ -17,20 +13,32 @@ const handleDatabaseError = (error: unknown): never => { throw error; }; -export const deleteSessionBySessionToken = async ( - sessionToken: string, - tx?: Prisma.TransactionClient -): Promise => { - validateInputs([sessionToken, z.string().min(1)]); +/** + * Every UNEXPIRED session token belonging to a user, read from Postgres. + * + * Postgres is the authoritative enumeration source here, deliberately: `session.storeSessionInDatabase` + * is on (auth.ts), so every session has a row, whereas Better Auth's own `internalAdapter.listSessions` + * reads the `active-sessions-` index out of `secondaryStorage` and returns an empty list when + * that key is missing or evicted — which would silently revoke nothing. + */ +export const getSessionTokensByUserId = async (userId: string): Promise => { + validateInputs([userId, z.string().min(1)]); try { - const result = await getDbClient(tx).session.deleteMany({ + const sessions = await prisma.session.findMany({ where: { - sessionToken, + userId, + // Expired rows are already unusable, and including them would inflate the revocation count that + // lands in the SSO-recovery audit event — the one place that number is read as "how many + // sessions the squatter was holding". + expires: { gt: new Date() }, + }, + select: { + sessionToken: true, }, }); - return result.count; + return sessions.map((session) => session.sessionToken); } catch (error) { return handleDatabaseError(error); } diff --git a/apps/web/modules/auth/lib/session-revocation.test.ts b/apps/web/modules/auth/lib/session-revocation.test.ts new file mode 100644 index 000000000000..9c4dfb56ac18 --- /dev/null +++ b/apps/web/modules/auth/lib/session-revocation.test.ts @@ -0,0 +1,128 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { getSessionTokensByUserId } from "@/modules/auth/lib/auth-session-repository"; +import { revokeSessionByToken, revokeUserSessionsExcept } from "@/modules/auth/lib/session-revocation"; + +const mocks = vi.hoisted(() => ({ + deleteSessions: vi.fn(), +})); + +vi.mock("@/modules/auth/lib/auth-session-repository", () => ({ + getSessionTokensByUserId: vi.fn(), +})); + +vi.mock("@/modules/auth/lib/auth", () => ({ + auth: { + $context: Promise.resolve({ + internalAdapter: { deleteSessions: mocks.deleteSessions }, + }), + }, +})); + +vi.mock("@formbricks/logger", () => ({ + logger: { info: vi.fn(), error: vi.fn() }, +})); + +describe("revokeUserSessionsExcept", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.deleteSessions.mockResolvedValue(undefined); + }); + + /** + * The whole point of this helper: sessions live in Postgres AND Redis, and `findSession` reads Redis + * first. Going through Better Auth's `deleteSessions` is what clears both — a bare Prisma delete would + * leave every revoked session still resolvable by `auth.api.getSession`. + */ + test("revokes through Better Auth's adapter, so both session stores are cleared", async () => { + vi.mocked(getSessionTokensByUserId).mockResolvedValue(["token-a", "token-b"]); + + const revoked = await revokeUserSessionsExcept({ userId: "user_1" }); + + expect(mocks.deleteSessions).toHaveBeenCalledWith(["token-a", "token-b"]); + expect(revoked).toBe(2); + }); + + test("spares the caller's own session", async () => { + vi.mocked(getSessionTokensByUserId).mockResolvedValue(["token-a", "keep-me", "token-b"]); + + const revoked = await revokeUserSessionsExcept({ userId: "user_1", keepSessionToken: "keep-me" }); + + expect(mocks.deleteSessions).toHaveBeenCalledWith(["token-a", "token-b"]); + expect(revoked).toBe(2); + }); + + test("does not touch the adapter when the only session is the one being kept", async () => { + vi.mocked(getSessionTokensByUserId).mockResolvedValue(["keep-me"]); + + const revoked = await revokeUserSessionsExcept({ userId: "user_1", keepSessionToken: "keep-me" }); + + expect(mocks.deleteSessions).not.toHaveBeenCalled(); + expect(revoked).toBe(0); + }); + + /** + * Fail-safe direction: `keepSessionToken` is only ever SUBTRACTED from the user's own token set, so an + * absent or foreign value can only over-revoke the caller's sessions — never under-revoke, and never + * reach another user. These two pin that, because the opposite would be a real hole. + */ + test("sweeps the caller too when no token is supplied", async () => { + vi.mocked(getSessionTokensByUserId).mockResolvedValue(["token-a", "token-b"]); + + const revoked = await revokeUserSessionsExcept({ userId: "user_1", keepSessionToken: undefined }); + + expect(mocks.deleteSessions).toHaveBeenCalledWith(["token-a", "token-b"]); + expect(revoked).toBe(2); + }); + + test("a token belonging to someone else spares nothing and reaches nothing", async () => { + vi.mocked(getSessionTokensByUserId).mockResolvedValue(["token-a", "token-b"]); + + const revoked = await revokeUserSessionsExcept({ + userId: "user_1", + keepSessionToken: "some-other-users-token", + }); + + // Everything of user_1's goes; the foreign token is never passed to the adapter. + expect(mocks.deleteSessions).toHaveBeenCalledWith(["token-a", "token-b"]); + expect(revoked).toBe(2); + }); + + test("is a no-op for a user with no sessions", async () => { + vi.mocked(getSessionTokensByUserId).mockResolvedValue([]); + + expect(await revokeUserSessionsExcept({ userId: "user_1" })).toBe(0); + expect(mocks.deleteSessions).not.toHaveBeenCalled(); + }); + + test("enumerates from Postgres, which is the store that always has every session", async () => { + vi.mocked(getSessionTokensByUserId).mockResolvedValue(["token-a"]); + + await revokeUserSessionsExcept({ userId: "user_1" }); + + // Not `internalAdapter.listSessions`: under `secondaryStorage` that reads only the + // `active-sessions-` Redis index and returns [] if it was evicted, revoking nothing. + expect(getSessionTokensByUserId).toHaveBeenCalledWith("user_1"); + }); +}); + +describe("revokeSessionByToken", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.deleteSessions.mockResolvedValue(undefined); + }); + + test("revokes the one session through the adapter, so both stores are cleared", async () => { + await revokeSessionByToken("token-a"); + + expect(mocks.deleteSessions).toHaveBeenCalledWith(["token-a"]); + }); + + // The guard the retired `deleteSessionBySessionToken` carried. An empty token would otherwise reach + // `secondaryStorage.delete("")` and a `deleteMany` on `token IN ('')` — a no-op that looks like a + // successful revocation, which is the worst possible failure mode for this function. + test.each([[""], [" "]])("refuses a blank token (%j) instead of no-oping", async (token) => { + await expect(revokeSessionByToken(token)).rejects.toThrow(); + + expect(mocks.deleteSessions).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/modules/auth/lib/session-revocation.ts b/apps/web/modules/auth/lib/session-revocation.ts new file mode 100644 index 000000000000..1c75cf23f092 --- /dev/null +++ b/apps/web/modules/auth/lib/session-revocation.ts @@ -0,0 +1,82 @@ +import "server-only"; +import { z } from "zod"; +import { logger } from "@formbricks/logger"; +import { validateInputs } from "@/lib/utils/validate"; +import { getSessionTokensByUserId } from "@/modules/auth/lib/auth-session-repository"; + +/** + * Revoke every session a user holds except one, across BOTH session stores. + * + * Sessions live in two places (see `auth.ts`): Postgres, because `session.storeSessionInDatabase` is + * required for the forward-auth proxies, and Redis via `secondaryStorage`. `findSession` consults Redis + * first, so a bare `prisma.session.deleteMany()` is NOT a revocation — it leaves the session fully valid + * for `auth.api.getSession` while breaking only the proxy path, which is strictly worse than doing + * nothing. Better Auth's `internalAdapter.deleteSessions` clears both: `secondaryStorage.delete(token)` + * per token, then the rows. + * + * Enumeration comes from Postgres (`getSessionTokensByUserId`) rather than `internalAdapter.listSessions`, + * which under `secondaryStorage` reads only the `active-sessions-` index and returns an empty + * list if that key was evicted — silently revoking nothing. + * + * Known residual, shared with `revokeSessionsOnPasswordReset`: `cookieCache` serves a still-valid + * `session_data` cookie from its signature alone, so a revoked session's holder keeps a cached session + * for up to `cookieCache.maxAge` (5 min in auth.ts) without ever hitting either store. Stateless by + * design upstream (ENG-2591). + * + * Do not read that as "5 minutes of exposure": the window is bounded but what can be done inside it is + * not. A still-cached session can mint an API key or complete an OAuth authorization, neither of which + * expires with the session. That is why the ENG-2557 strip revokes OAuth grants in its transaction + * rather than relying on the sweep alone. + * + * One upstream wrinkle: `deleteSessions` prunes the per-token keys and the rows but does not rewrite the + * `active-sessions-` index, so `auth.api.listSessions` keeps listing revoked sessions until their + * TTL. No security consequence — `getSession` needs the per-token key, which is gone — but an "active + * sessions" view lies immediately after a revocation. + * + * `auth` is imported dynamically on purpose: `auth.ts` → `better-auth-hooks.ts` → `sso-recovery.ts`, so a + * static import here would close an import cycle for this module's callers. Same reason `auth.ts` reaches + * for `await import("@/modules/email")`. + * + * @returns the number of sessions revoked. + */ +export const revokeUserSessionsExcept = async ({ + userId, + keepSessionToken, +}: { + userId: string; + keepSessionToken?: string; +}): Promise => { + const tokens = (await getSessionTokensByUserId(userId)).filter((token) => token !== keepSessionToken); + + if (tokens.length === 0) { + return 0; + } + + const { auth } = await import("@/modules/auth/lib/auth"); + const ctx = await auth.$context; + await ctx.internalAdapter.deleteSessions(tokens); + + logger.info({ userId, revoked: tokens.length }, "Revoked user sessions"); + + return tokens.length; +}; + +/** + * Revoke one session by its (unsigned) token, across both stores. Same rationale as above — a raw + * Prisma delete leaves the Redis copy resolvable by `getSession` until its TTL — for callers that hold a + * token rather than a user id, like the SSO-recovery failure path killing the session its cookie names. + */ +export const revokeSessionByToken = async (sessionToken: string): Promise => { + // Restores the guard the retired `deleteSessionBySessionToken` carried. Not reachable from the one + // caller today — the recovery-completion route returns early on a missing cookie — but this is an + // exported helper in an auth module, and a blank token would reach `secondaryStorage.delete("")` plus a + // `deleteMany` on `token IN ('')`: a no-op that reports a successful revocation, which is the worst + // failure mode this function has. `.trim()` before `.min(1)` is deliberately stricter than the original + // `min(1)`, since a whitespace-only token is exactly as meaningless as an empty one and session tokens + // are opaque values that never contain whitespace. + validateInputs([sessionToken, z.string().trim().min(1)]); + + const { auth } = await import("@/modules/auth/lib/auth"); + const ctx = await auth.$context; + await ctx.internalAdapter.deleteSessions([sessionToken]); +}; diff --git a/apps/web/modules/ee/sso/lib/sso-recovery.integration.test.ts b/apps/web/modules/ee/sso/lib/sso-recovery.integration.test.ts new file mode 100644 index 000000000000..29643383ab61 --- /dev/null +++ b/apps/web/modules/ee/sso/lib/sso-recovery.integration.test.ts @@ -0,0 +1,324 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { prisma } from "@formbricks/database"; +import { resetDb } from "@/integration/reset-db"; +import { ENCRYPTION_KEY, WEBAPP_URL } from "@/lib/constants"; +import { symmetricEncrypt } from "@/lib/crypto"; +import { createSsoRelinkIntent } from "@/lib/jwt"; +import { auth } from "@/modules/auth/lib/auth"; +import { getSessionTokenFromCookieHeader } from "@/modules/auth/lib/session-cookie"; +import { completeSsoRecovery } from "@/modules/ee/sso/lib/sso-recovery"; +import { sendPasswordResetLinkEmail } from "@/modules/email"; + +/** + * The real red-green proof for ENG-2557, against real Postgres and real Redis. + * + * The bug was invisible to the unit suite for a reason worth restating: the control looked present, wrote + * to `User.password` / `User.twoFactorSecret`, and asserted exactly that — but ENG-1054 had moved both + * factors elsewhere, so it stripped nothing. Only a test that drives Better Auth's own sign-in against a + * real database can tell "the password is gone" from "we nulled a column nobody reads". + * + * Every security assertion here therefore goes through `auth.api.*` rather than reading columns. + */ + +// EMAIL_VERIFICATION_DISABLED differs between environments — `0` in a dev `.env`, `1` in the `.env.example` +// CI copies — and `auth.ts` turns it into `requireEmailVerification` at import time. Pinning it keeps this +// file identical everywhere, and `1` is deliberately the *vulnerable* posture: it is what lets an account +// with an unproven address hold a live session, which is the state the session sweep exists for. +vi.mock("@/lib/constants", async (importOriginal) => ({ + ...(await importOriginal()), + EMAIL_VERIFICATION_DISABLED: true, + // A dev `.env` ships PASSWORD_RESET_DISABLED=1 (CI's fixups flip it); the lockout test below drives a + // real reset, so pin it open. + PASSWORD_RESET_DISABLED: false, +})); + +// Fires inside setImmediate and calls getClientIpFromHeaders() outside a request scope. +vi.mock("@/modules/ee/audit-logs/lib/handler", async (importOriginal) => ({ + ...(await importOriginal()), + queueAuditEventBackground: vi.fn(async () => undefined), +})); + +const ATTACKER_PASSWORD = "Passw0rd!squatter"; +const VICTIM_EMAIL = "victim@example.com"; + +/** + * Only the session-token cookie from a sign-in response, deliberately dropping `session_data`: with + * `cookieCache` enabled, `getSession` serves a still-valid `session_data` cookie straight from its + * signature without consulting Redis or Postgres. Carrying it along would make every revocation + * assertion below pass OR fail for the wrong reason — the cache window (5 min) is a documented residual + * of any revocation here, `revokeSessionsOnPasswordReset` included, not something these tests can bind. + */ +const sessionTokenCookie = (response: Response): string => { + const cookie = response.headers + .getSetCookie() + .map((setCookie) => setCookie.split(";")[0]) + .find((pair) => pair.startsWith("formbricks.session_token=")); + expect(cookie).toBeTruthy(); + return cookie!; +}; + +const signIn = (password: string): Promise => + auth.api.signInEmail({ body: { email: VICTIM_EMAIL, password }, asResponse: true }); + +/** + * The squatter's starting state: an account on someone else's address, with a working password and an + * unproven email. Built through `signUpEmail` rather than by hand on purpose — 1.7 filters + * `findCredentialAccount` on `issuer`, so a hand-seeded credential row would reject a *correct* password + * and every assertion below would pass for the wrong reason. + * + * Deliberately does NOT enrol 2FA: for a `twoFactorEnabled` user Better Auth's sign-in hook deletes the + * just-minted session and answers with a challenge instead, so any test that needs a real session cookie + * from `signIn` must enrol 2FA only AFTER minting it (`enrollTwoFactor` below). + */ +const seedUnprovenAccountWithPassword = async () => { + await auth.api.signUpEmail({ + body: { email: VICTIM_EMAIL, password: ATTACKER_PASSWORD, name: "Squatter" }, + asResponse: true, + }); + const user = await prisma.user.findUniqueOrThrow({ where: { email: VICTIM_EMAIL } }); + expect(user.emailVerified).toBe(false); + + return user; +}; + +/** + * A 2FA enrolment in both stores, the shape `enableTwoFactorAuth` leaves behind. The `TwoFactor` row's + * contents never need to verify (assertions only count rows), but the LEGACY columns must be genuinely + * decryptable: the backfill shim re-encodes them on sign-in inside a swallow-all try/catch, so a + * placeholder there would make the resurrection assertion below pass because the shim crashed, not + * because recovery disarmed it. + */ +const enrollTwoFactor = async (userId: string) => { + await prisma.twoFactor.create({ + data: { userId, secret: "encrypted-secret", backupCodes: "encrypted-backup-codes" }, + }); + await prisma.user.update({ + where: { id: userId }, + data: { + twoFactorEnabled: true, + twoFactorSecret: symmetricEncrypt("JBSWY3DPEHPK3PXP", ENCRYPTION_KEY), + backupCodes: symmetricEncrypt(JSON.stringify(["aaaaabbbbb", "cccccddddd"]), ENCRYPTION_KEY), + }, + }); +}; + +/** The session the recovery link mints, which completion runs on and the sweep must spare. */ +const createRecoverySession = async (userId: string): Promise => { + const ctx = await auth.$context; + const session = await ctx.internalAdapter.createSession(userId, false); + return session.token; +}; + +const runRecovery = (user: { id: string; email: string }, sessionToken: string) => + completeSsoRecovery({ + intentToken: createSsoRelinkIntent({ + userId: user.id, + email: user.email, + provider: "google", + providerAccountId: "google-sub-recovery-1", + callbackUrl: WEBAPP_URL, + }), + sessionUserId: user.id, + sessionToken, + }); + +beforeEach(async () => { + await resetDb(); + vi.mocked(sendPasswordResetLinkEmail).mockClear(); +}); + +describe("SSO recovery strips the live local auth factors (real Postgres + Redis)", () => { + test("the squatter's password no longer authenticates after the address is proven", async () => { + const user = await seedUnprovenAccountWithPassword(); + + // Anti-vacuity: the password genuinely signs in before recovery. Without this the assertion below + // can pass because the fixture silently failed — the exact failure mode that let this ship. + expect((await signIn(ATTACKER_PASSWORD)).status).toBe(200); + // Test-only cleanup of the precondition session's row, so this test asserts only the password strip + // and stays green even if the session sweep were to regress (it has its own tests below). + await prisma.session.deleteMany({}); + + await runRecovery(user, await createRecoverySession(user.id)); + + // Pre-fix this was 200: the strip nulled `User.password` while the live hash sat on `Account`. + expect((await signIn(ATTACKER_PASSWORD)).status).toBe(401); + // A rejection alone can be right for the wrong reason, so also check nothing was minted: the + // recovery session must be the only one that exists. + expect(await prisma.session.count({ where: { userId: user.id } })).toBe(1); + }); + + test("the 2FA enrolment does not survive on an account that changed hands", async () => { + const user = await seedUnprovenAccountWithPassword(); + await enrollTwoFactor(user.id); + + await runRecovery(user, await createRecoverySession(user.id)); + + // Pre-fix this was 1. Dormant rather than exploitable (Better Auth gates its challenge on + // `user.twoFactorEnabled`, which the legacy update already cleared), but a stale TOTP secret and + // backup codes must not outlive the handover. + expect(await prisma.twoFactor.count({ where: { userId: user.id } })).toBe(0); + // The legacy columns stay null too: `better-auth-two-factor-backfill` rebuilds a `TwoFactor` row from + // them on the next credential sign-in, so leaving them set would re-arm the factor. + const after = await prisma.user.findUniqueOrThrow({ where: { id: user.id } }); + expect(after.twoFactorEnabled).toBe(false); + expect(after.twoFactorSecret).toBeNull(); + expect(after.backupCodes).toBeNull(); + }); + + test("a session the squatter was holding stops resolving, in both session stores", async () => { + const user = await seedUnprovenAccountWithPassword(); + const squatterCookie = sessionTokenCookie(await signIn(ATTACKER_PASSWORD)); + expect(await auth.api.getSession({ headers: { cookie: squatterCookie } })).not.toBeNull(); + + await runRecovery(user, await createRecoverySession(user.id)); + + // `getSession` reads Redis first, so this is the assertion a raw `prisma.session.deleteMany()` would + // fail: it would clear the row and leave the session perfectly valid here. + expect(await auth.api.getSession({ headers: { cookie: squatterCookie } })).toBeNull(); + }); + + /** + * Binds the seam the route relies on and nothing else exercises live: `route.ts` derives + * `keepSessionToken` from the request cookie via `getSessionTokenFromCookieHeader`, and the sweep + * compares it against `Session.sessionToken` rows. Better Auth signs its cookie as + * `token.signature` — if the helper ever returned the signed form (or the secret resolution drifted + * from auth.ts's), the filter would match nothing and recovery would sign its own caller out. + */ + test("the route's cookie-derived keep-token matches the stored session token", async () => { + const user = await seedUnprovenAccountWithPassword(); + const cookie = sessionTokenCookie(await signIn(ATTACKER_PASSWORD)); + + const keepToken = getSessionTokenFromCookieHeader(cookie); + + expect(keepToken).toBeTruthy(); + const stored = await prisma.session.findMany({ + where: { userId: user.id }, + select: { sessionToken: true }, + }); + expect(stored.map((row) => row.sessionToken)).toContain(keepToken); + }); + + test("the recovering user's own session is spared, so the redirect stays signed in", async () => { + const user = await seedUnprovenAccountWithPassword(); + await signIn(ATTACKER_PASSWORD); // a second session, to be swept + const recoverySessionToken = await createRecoverySession(user.id); + + await runRecovery(user, recoverySessionToken); + + const remaining = await prisma.session.findMany({ where: { userId: user.id } }); + expect(remaining.map((session) => session.sessionToken)).toEqual([recoverySessionToken]); + }); + + /** + * The refresh token is the persistence that outlives every session: `oauthProvider` is registered + * unconditionally with open dynamic client registration and a 30-day refresh lifetime, and both token + * tables' `session` FK is `onDelete: SetNull`, so sweeping sessions blanks the liveness check instead + * of failing it. Revoking is the only thing that stops it, and `handleRefreshTokenGrant` reads + * `revoked`, so this asserts a value the grant path genuinely consults. + * + * Access tokens are deliberately NOT asserted here. Our config sets `resources` and never sets + * `disableJwtPlugin`, so every access token is a self-contained JWT that is signed and never + * persisted, and `/api/mcp` verifies bearers against JWKS without reading the table. Seeding a row + * production never writes and asserting its column would be exactly the "assert a column nothing + * reads" mistake this whole file exists to catch. The 15-minute JWT residual is called out in the + * strip's docblock and in the PR's open gaps instead. + */ + test("the refresh token and consent do not survive recovery", async () => { + const user = await seedUnprovenAccountWithPassword(); + const client = await prisma.oauthClient.create({ + data: { + clientId: "mcp-client-live-1", + name: "smoke client", + redirectUris: [`${WEBAPP_URL}/cb`], + disabled: false, + }, + }); + await prisma.oauthRefreshToken.create({ + data: { + token: "refresh-token-live-1", + clientId: client.clientId, + userId: user.id, + scopes: ["openid"], + expiresAt: new Date(Date.now() + 30 * 24 * 60 * 60 * 1000), + createdAt: new Date(), + }, + }); + // Consent is what lets `/authorize` skip the consent screen, so leaving it would let a + // still-cookie-cached session mint a replacement refresh token and undo the revocation. + await prisma.oauthConsent.create({ + data: { + clientId: client.clientId, + userId: user.id, + scopes: ["openid"], + createdAt: new Date(), + updatedAt: new Date(), + }, + }); + + await runRecovery(user, await createRecoverySession(user.id)); + + const refreshed = await prisma.oauthRefreshToken.findFirstOrThrow({ where: { userId: user.id } }); + expect(refreshed.revoked).not.toBeNull(); + expect(await prisma.oauthConsent.count({ where: { userId: user.id } })).toBe(0); + }); + + /** + * Recovery flips `identityProvider` to the SSO provider and nothing ever flips it back, so without this + * the fix would trade a takeover for a lockout: no password, and no way to ask for one. + */ + test("a recovered user can still get a password back", async () => { + const user = await seedUnprovenAccountWithPassword(); + await enrollTwoFactor(user.id); + + await runRecovery(user, await createRecoverySession(user.id)); + + // The credential row survives with a null password — that row is what identifies them as a password + // user to `forgotPasswordAction` after `identityProvider` has moved on. + const credential = await prisma.account.findFirstOrThrow({ + where: { userId: user.id, provider: "credential" }, + }); + expect(credential.password).toBeNull(); + + await auth.api.requestPasswordReset({ + body: { email: VICTIM_EMAIL, redirectTo: `${WEBAPP_URL}/auth/forgot-password/reset` }, + }); + const [{ verifyLink }] = vi.mocked(sendPasswordResetLinkEmail).mock.calls.at(-1)!; + // Better Auth builds `${baseURL}/reset-password/${token}?callbackURL=…`, so the token is a path + // segment — reading it as a query param yields null and the reset below would fail as unauthenticated. + const token = new URL(verifyLink).pathname.split("/").pop(); + expect(token).toBeTruthy(); + + const newPassword = "Passw0rd!rightful-owner"; + await auth.api.resetPassword({ body: { newPassword, token: token! } }); + + expect((await signIn(newPassword)).status).toBe(200); + expect((await signIn(ATTACKER_PASSWORD)).status).toBe(401); + + // The resurrection channel: that successful credential sign-in is exactly when the backfill shim + // (`better-auth-two-factor-backfill`) would rebuild a `TwoFactor` row from the legacy `User` columns. + // Recovery cleared both halves — the row AND the legacy columns — so nothing may come back. This is + // the assertion that goes red if a refactor ever drops the "redundant" legacy nulls from the strip. + expect(await prisma.twoFactor.count({ where: { userId: user.id } })).toBe(0); + }); + + test("an already-proven account keeps its password, sessions and 2FA", async () => { + const user = await seedUnprovenAccountWithPassword(); + await prisma.user.update({ where: { id: user.id }, data: { emailVerified: true } }); + // Mint the owner's session BEFORE enrolling 2FA — after enrolment, `signIn` answers with a 2FA + // challenge instead of a session cookie. + const ownerCookie = sessionTokenCookie(await signIn(ATTACKER_PASSWORD)); + expect(await auth.api.getSession({ headers: { cookie: ownerCookie } })).not.toBeNull(); + await enrollTwoFactor(user.id); + + await runRecovery(user, await createRecoverySession(user.id)); + + // Linking another provider to a legitimate account must not strip it or sign it out everywhere. + // With 2FA enrolled, a correct password now yields the challenge (still 200; a stripped password + // would 401 before the 2FA hook runs), so the 200 proves the credential survived. + expect((await signIn(ATTACKER_PASSWORD)).status).toBe(200); + expect(await auth.api.getSession({ headers: { cookie: ownerCookie } })).not.toBeNull(); + expect(await prisma.twoFactor.count({ where: { userId: user.id } })).toBe(1); + const after = await prisma.user.findUniqueOrThrow({ where: { id: user.id } }); + expect(after.twoFactorEnabled).toBe(true); + }); +}); diff --git a/apps/web/modules/ee/sso/lib/sso-recovery.test.ts b/apps/web/modules/ee/sso/lib/sso-recovery.test.ts index 52df5b3dc330..6f7e5a5edd13 100644 --- a/apps/web/modules/ee/sso/lib/sso-recovery.test.ts +++ b/apps/web/modules/ee/sso/lib/sso-recovery.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; import { prisma } from "@formbricks/database"; import { Prisma } from "@formbricks/database/prisma"; +import { revokeUserSessionsExcept } from "@/modules/auth/lib/session-revocation"; import { finalizeSuccessfulSignIn } from "@/modules/auth/lib/sign-in-tracking"; import { buildVerificationRequestedPath } from "@/modules/auth/lib/verification-links"; import { sendVerificationEmail } from "@/modules/email"; @@ -37,6 +38,10 @@ vi.mock("@/lib/jwt", () => ({ verifySsoRelinkIntent: mocks.verifySsoRelinkIntent, })); +vi.mock("@/modules/auth/lib/session-revocation", () => ({ + revokeUserSessionsExcept: vi.fn(), +})); + vi.mock("@/modules/auth/lib/sign-in-tracking", () => ({ finalizeSuccessfulSignIn: vi.fn(), })); @@ -82,14 +87,42 @@ vi.mock("./account-linking", () => ({ describe("sso-recovery", () => { const txUserUpdate = vi.fn(); + const txTwoFactorDeleteMany = vi.fn(); + const txAccountUpdateMany = vi.fn(); + const txOauthAccessUpdateMany = vi.fn(); + const txOauthRefreshUpdateMany = vi.fn(); + const txOauthConsentDeleteMany = vi.fn(); + // Both new stores belong in the stub: post-ENG-1054 the password lives on `Account` and the 2FA secret + // in `TwoFactor`, so a strip that only touched `user` is exactly the bug ENG-2557 fixed. const tx = { user: { update: txUserUpdate, }, + twoFactor: { + deleteMany: txTwoFactorDeleteMany, + }, + account: { + updateMany: txAccountUpdateMany, + }, + oauthAccessToken: { + updateMany: txOauthAccessUpdateMany, + }, + oauthRefreshToken: { + updateMany: txOauthRefreshUpdateMany, + }, + oauthConsent: { + deleteMany: txOauthConsentDeleteMany, + }, }; beforeEach(() => { vi.clearAllMocks(); + txTwoFactorDeleteMany.mockResolvedValue({ count: 1 }); + txAccountUpdateMany.mockResolvedValue({ count: 1 }); + txOauthAccessUpdateMany.mockResolvedValue({ count: 1 }); + txOauthRefreshUpdateMany.mockResolvedValue({ count: 2 }); + txOauthConsentDeleteMany.mockResolvedValue({ count: 1 }); + vi.mocked(revokeUserSessionsExcept).mockResolvedValue(2); vi.mocked(prisma.$transaction).mockImplementation( async (callback: (txClient: Prisma.TransactionClient) => Promise) => await callback(tx as unknown as Prisma.TransactionClient) @@ -187,21 +220,21 @@ describe("sso-recovery", () => { id: "user_1", email: "john.doe@example.com", locale: "en-US", - emailVerified: null, + emailVerified: false, isActive: true, identityProvider: "email", identityProviderAccountId: null, - password: "hashed-password", - twoFactorEnabled: true, - twoFactorSecret: "encrypted-secret", - backupCodes: "encrypted-codes", } as any); const callbackUrl = await completeSsoRecovery({ intentToken: "test-intent", sessionUserId: "user_1", + sessionToken: "current-session-token", }); + // Exact-match on purpose: the legacy nulls are load-bearing, not leftovers. `better-auth-two-factor-backfill` + // re-materialises a `TwoFactor` row from `twoFactorEnabled && twoFactorSecret` on the next credential + // sign-in, so dropping them from this payload would let the stripped factor come back. expect(txUserUpdate).toHaveBeenCalledWith({ where: { id: "user_1", @@ -214,6 +247,48 @@ describe("sso-recovery", () => { twoFactorSecret: null, }, }); + // The live stores, which the pre-ENG-2557 implementation never touched. + expect(txTwoFactorDeleteMany).toHaveBeenCalledWith({ where: { userId: "user_1" } }); + // Scoped by owner, NOT by `providerAccountId`/`issuer`: those are account-key columns and a drifted key + // (ENG-2555) would make a key-filtered query walk past a row still holding a live hash. + expect(txAccountUpdateMany).toHaveBeenCalledWith({ + where: { userId: "user_1", provider: "credential" }, + data: { password: null }, + }); + // Post-commit, sparing the caller's own session so the redirect still lands signed in. + expect(revokeUserSessionsExcept).toHaveBeenCalledWith({ + userId: "user_1", + keepSessionToken: "current-session-token", + }); + // The OAuth grants the account minted while unproven: a refresh token outlives every session, so + // the sweep is incomplete without this. + expect(txOauthAccessUpdateMany).toHaveBeenCalledWith({ + where: { userId: "user_1", revoked: null }, + data: { revoked: expect.any(Date) }, + }); + expect(txOauthRefreshUpdateMany).toHaveBeenCalledWith({ + where: { userId: "user_1", revoked: null }, + data: { revoked: expect.any(Date) }, + }); + // Consent too: `/authorize` skips the consent screen when a row exists, so leaving it would let a + // still-cookie-cached session mint a replacement refresh token and undo the revocation above. + expect(txOauthConsentDeleteMany).toHaveBeenCalledWith({ where: { userId: "user_1" } }); + expect(mocks.queueAuditEventBackground).toHaveBeenCalledWith( + expect.objectContaining({ + action: "sso_recovery_completed", + status: "success", + newObject: expect.objectContaining({ + credentialPasswordsCleared: 1, + twoFactorRowsRemoved: 1, + // Distinct fields, and the stub returns distinct counts (1 access / 2 refresh) on purpose: a + // summed field would read 3 either way and could not catch the two being swapped. + oauthAccessTokensRevoked: 1, + oauthRefreshTokensRevoked: 2, + oauthConsentsRevoked: 1, + sessionsRevoked: 2, + }), + }) + ); expect(syncSsoIdentityForUser).toHaveBeenCalledWith({ userId: "user_1", provider: "google", @@ -237,25 +312,112 @@ describe("sso-recovery", () => { id: "user_1", email: "john.doe@example.com", locale: "en-US", - emailVerified: new Date("2024-01-01T00:00:00.000Z"), + emailVerified: true, isActive: true, identityProvider: "email", identityProviderAccountId: null, - password: "hashed-password", - twoFactorEnabled: true, - twoFactorSecret: "encrypted-secret", - backupCodes: "encrypted-codes", } as any); await completeSsoRecovery({ intentToken: "test-intent", sessionUserId: "user_1", + sessionToken: "current-session-token", }); expect(txUserUpdate).not.toHaveBeenCalled(); + expect(txTwoFactorDeleteMany).not.toHaveBeenCalled(); + expect(txAccountUpdateMany).not.toHaveBeenCalled(); + expect(txOauthAccessUpdateMany).not.toHaveBeenCalled(); + expect(txOauthRefreshUpdateMany).not.toHaveBeenCalled(); + expect(txOauthConsentDeleteMany).not.toHaveBeenCalled(); + // A proven account is a legitimate one: linking another provider to it must not sign its other + // sessions out, and must not report a strip that did not happen. + expect(revokeUserSessionsExcept).not.toHaveBeenCalled(); + expect(mocks.queueAuditEventBackground).toHaveBeenCalledWith( + expect.objectContaining({ + action: "sso_recovery_completed", + newObject: expect.not.objectContaining({ credentialPasswordsCleared: expect.anything() }), + }) + ); expect(syncSsoIdentityForUser).toHaveBeenCalledOnce(); }); + /** + * The guard keys on `emailVerified` alone. It used to also require `identityProvider === "email"`, but + * that column is denormalized onto `User` by an `account.create.after` hook, so a security control + * resting on it goes silently dead if it ever drifts. An unproven address is unproven whatever the + * denormalized column happens to say. + */ + test("strips an unverified account even when identityProvider says otherwise", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + id: "user_1", + email: "john.doe@example.com", + locale: "en-US", + emailVerified: false, + isActive: true, + identityProvider: "google", + identityProviderAccountId: "provider-account-1", + } as any); + + await completeSsoRecovery({ + intentToken: "test-intent", + sessionUserId: "user_1", + sessionToken: "current-session-token", + }); + + expect(txAccountUpdateMany).toHaveBeenCalledWith({ + where: { userId: "user_1", provider: "credential" }, + data: { password: null }, + }); + expect(txTwoFactorDeleteMany).toHaveBeenCalledOnce(); + expect(revokeUserSessionsExcept).toHaveBeenCalledOnce(); + }); + + /** + * The strip has already committed by the time sessions are swept, so a revocation failure must not undo + * it or fail the recovery — the user would be left with a stripped password and no way through. + */ + test("still completes recovery when the session sweep fails", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + id: "user_1", + email: "john.doe@example.com", + locale: "en-US", + emailVerified: false, + isActive: true, + identityProvider: "email", + identityProviderAccountId: null, + } as any); + vi.mocked(revokeUserSessionsExcept).mockRejectedValue(new Error("redis unavailable")); + + const callbackUrl = await completeSsoRecovery({ + intentToken: "test-intent", + sessionUserId: "user_1", + sessionToken: "current-session-token", + }); + + expect(callbackUrl).toBe("http://localhost:3000/environments/env_1"); + expect(txAccountUpdateMany).toHaveBeenCalledOnce(); + // A failed sweep must NOT be recorded as "0 sessions revoked" — same number, opposite incident. + expect(mocks.queueAuditEventBackground).toHaveBeenCalledWith( + expect.objectContaining({ + action: "sso_recovery_completed", + newObject: expect.objectContaining({ + credentialPasswordsCleared: 1, + sessionRevocationFailed: true, + }), + }) + ); + // `action` discriminator is load-bearing: `toHaveBeenCalledWith` passes when ANY call matches, so + // without it this would be satisfied by any other queued event lacking the key, and could go green + // without ever proving the completion event omits it. + expect(mocks.queueAuditEventBackground).toHaveBeenCalledWith( + expect.objectContaining({ + action: "sso_recovery_completed", + newObject: expect.not.objectContaining({ sessionsRevoked: expect.anything() }), + }) + ); + }); + test("rejects recovery when the signed-in user does not match the intent owner", async () => { await expect( completeSsoRecovery({ diff --git a/apps/web/modules/ee/sso/lib/sso-recovery.ts b/apps/web/modules/ee/sso/lib/sso-recovery.ts index 7b989ce786e7..ae57f71fbeeb 100644 --- a/apps/web/modules/ee/sso/lib/sso-recovery.ts +++ b/apps/web/modules/ee/sso/lib/sso-recovery.ts @@ -5,6 +5,7 @@ import type { Account } from "@formbricks/types/auth"; import { WEBAPP_URL } from "@/lib/constants"; import { createEmailToken, createSsoRelinkIntent, verifySsoRelinkIntent } from "@/lib/jwt"; import { getValidatedCallbackUrl } from "@/lib/utils/url"; +import { revokeUserSessionsExcept } from "@/modules/auth/lib/session-revocation"; import { finalizeSuccessfulSignIn } from "@/modules/auth/lib/sign-in-tracking"; import { buildVerificationRequestedPath } from "@/modules/auth/lib/verification-links"; import { queueAuditEventBackground } from "@/modules/ee/audit-logs/lib/handler"; @@ -35,6 +36,8 @@ const queueSsoRecoveryAuditEvent = ({ provider, callbackUrl, failureReason, + reclaimed, + sessionsRevoked, }: { action: "sso_recovery_started" | "sso_recovery_completed" | "sso_recovery_failed"; status: "success" | "failure"; @@ -43,6 +46,9 @@ const queueSsoRecoveryAuditEvent = ({ provider: string; callbackUrl?: string; failureReason?: string; + reclaimed?: TReclaimOutcome; + /** `null` means the sweep threw — deliberately not conflated with "there were none". */ + sessionsRevoked?: number | null; }) => { queueAuditEventBackground({ action, @@ -57,39 +63,138 @@ const queueSsoRecoveryAuditEvent = ({ provider, ...(callbackUrl ? { callbackUrl } : {}), ...(failureReason ? { failureReason } : {}), + // The one moment an account can change hands, so record what was taken away rather than only that + // recovery succeeded. Marker keys in `newObject` are the house idiom (`passwordResetMarker`, + // `twoFactorAuth: "disabled"`); `redactPII` matches exact lowercased keys, so these survive while + // `email` above is redacted. + ...(reclaimed + ? { + credentialPasswordsCleared: reclaimed.credentialPasswordsCleared, + twoFactorRowsRemoved: reclaimed.twoFactorRowsRemoved, + // Reported separately, not summed. In this configuration access tokens are self-contained + // JWTs that are never persisted (see the revocation block below), so the access count is + // ~always 0 and a combined "grants" total would be the refresh count wearing a plural name — + // unreadable for the one audience these fields exist for. Keeping both means an + // access-token row appearing at all is itself visible, which would mean the opaque-token + // configuration is in play. + oauthAccessTokensRevoked: reclaimed.oauthAccessTokensRevoked, + oauthRefreshTokensRevoked: reclaimed.oauthRefreshTokensRevoked, + oauthConsentsRevoked: reclaimed.oauthConsentsRevoked, + // `sessionsRevoked: 0` and "the sweep failed" are the same number but opposite incidents, + // and this is the field a responder reads to confirm the squatter was actually kicked out. + ...(sessionsRevoked === null ? { sessionRevocationFailed: true } : { sessionsRevoked }), + } + : {}), }, }); }; -const SSO_RECOVERY_USER_SELECT = { - ...LINKED_SSO_LOOKUP_SELECT, - backupCodes: true, - password: true, - twoFactorEnabled: true, - twoFactorSecret: true, -} as const; - -type TSsoRecoveryUser = Prisma.UserGetPayload<{ - select: typeof SSO_RECOVERY_USER_SELECT; -}>; +/** + * What `reclaimUnverifiedLocalAuthIfNeeded` actually removed, for the audit record. `null` means the + * account was already proven and nothing was touched. + */ +type TReclaimOutcome = { + credentialPasswordsCleared: number; + twoFactorRowsRemoved: number; + oauthAccessTokensRevoked: number; + oauthRefreshTokensRevoked: number; + oauthConsentsRevoked: number; +} | null; +/** + * Strip the local auth factors of an account whose email was never proven, at the moment an SSO identity + * proves it (ENG-554, ENG-2557). + * + * The threat: an attacker registers on a victim's address, sets a password, and is held out only by + * `requireEmailVerification`. Recovery then sets `emailVerified: true` — removing the very thing keeping + * them out — so the untrusted factors have to go with it. The proof authorising this is NOT the IdP's + * assertion: `startSsoRecovery` mails the address on record and completion requires the session that link + * mints, plus `sessionUserId === intent.userId`. Upstream Better Auth does the same thing in + * `revoke-unproven-account-access.mjs` for magic-link and email-OTP. + * + * THE FACTORS LIVE IN TWO PLACES EACH, and this is where ENG-2557 came from: the original control (#7755) + * predates ENG-1054, which moved the password to `Account.password` and 2FA into the `TwoFactor` table. + * It kept nulling the legacy `User` columns only, so post-cutover it stripped nothing at all — for any + * user created after the cutover those columns are already null, because `signUpEmail` never writes them. + * A mitigation that reads as present and does nothing is worse than none; write BOTH stores. + * + * The legacy `User` nulls are therefore kept deliberately, not left as dead code: + * `better-auth-two-factor-backfill.ts` re-materialises a `TwoFactor` row from + * `twoFactorEnabled && twoFactorSecret` on every successful credential sign-in, so dropping them would let + * a later sign-in re-arm the attacker's factor. Both halves together disarm it twice. + * + * Two deliberate shapes worth not "simplifying": + * + * - The password is NULLED, not the row deleted. Sign-in behaviour is identical either way + * (`!currentPassword` → the same 401 after the same dummy hash), but the surviving row is what marks + * this user as a credential user, which is what lets `forgotPasswordAction` still offer them a reset — + * recovery flips `identityProvider` to the SSO provider and nothing ever flips it back, so deleting the + * row would lock them out of every password route with no self-service way back. + * - The `where` is scoped by `userId`, NOT by `providerAccountId` / `issuer`, even though Better Auth's own + * `findCredentialAccount` filters on all four. Those are account-KEY columns and a drifted key is a real + * failure mode here (ENG-2555 was exactly that): a row whose key drifted still holds a live hash, and a + * query filtering on the key would walk straight past it. Owner-scoping cannot reach another user's row + * and does not go blind. + * + * The 2FA half is the weaker of the two, and worth stating honestly: Better Auth gates its challenge on + * `user.twoFactorEnabled`, which the legacy update already clears, so the orphaned `TwoFactor` row was + * never reachable at sign-in. Removing it is about not leaving a stale TOTP secret and backup codes at rest + * on an account that has changed hands — not a live bypass. + * + * Sessions are revoked by the caller, after commit — Better Auth resolves its adapter from its own + * AsyncLocalStorage, so a revocation issued in here would execute outside `tx` and survive a rollback. + * + * Not locked against concurrent recoveries (upstream takes a DB advisory lock for its equivalent). Every + * write here is idempotent — two `deleteMany`/`updateMany` calls and an update to fixed values — so a race + * converges on the same state rather than corrupting it. + */ const reclaimUnverifiedLocalAuthIfNeeded = async ({ tx, user, }: { tx: Prisma.TransactionClient; - user: TSsoRecoveryUser; -}) => { - if (user.identityProvider !== "email" || user.emailVerified) { - return; + user: TSsoLookupUser; +}): Promise => { + // Keyed on `emailVerified` alone. The old guard also required `identityProvider === "email"`, but that + // column is denormalized onto `User` by an `account.create.after` hook — resting a security control on a + // denormalized value means drift silently disables it. + // + // Worth being honest about the limit of this test: `emailVerified` is a one-bit latch, and it says the + // address was proven, NOT that the account's local factors were ever proven by their owner. Anyone who + // knows the account's password can have Better Auth re-send a verification mail (`sendOnSignIn`), so a + // single victim click flips this and the strip below stops firing. That is the pre-hijacking vector in + // ENG-2562, tracked separately, and closing it means invalidating the credential at verification time + // too — not a different guard here. + // + // The INVERSE case matters just as much and is not hypothetical: `requireEmailVerification` is + // `!EMAIL_VERIFICATION_DISABLED` (auth.ts) and `EMAIL_VERIFICATION_DISABLED=1` ships as the default in + // `.env.example` and `docker/docker-compose.yml`. The verification mail still goes out but blocks + // nothing, so on a default self-hosted install a user has no reason to click it and `emailVerified` + // stays false for the life of the account. This guard's population there is not squatters — it is + // every credential user who never bothered. + // + // For them a first-time SSO sign-in runs recovery and permanently removes their second factor: the + // `TwoFactor` row goes, and the legacy `twoFactorEnabled`/`twoFactorSecret` nulls below (kept + // deliberately, so the backfill shim cannot re-arm an attacker's factor) are exactly what stop it + // being re-armed for a legitimate owner either. They can recover the password via + // `forgotPasswordAction`, and then hold a one-factor account where two were enrolled, without being + // told. Correct for a squatter, a silent downgrade for the owner. + // + // Left as-is on purpose: there is no signal here that separates the two populations, and weakening the + // guard would reopen the takeover. What is missing is telling the user — mail them what was removed and + // prompt re-enrolment. That needs a new transactional template, so it is tracked separately rather than + // widened into a fix that backports to two release branches. + if (user.emailVerified) { + return null; } - // Inbox ownership is now proven, so strip any untrusted local auth factors before the SSO - // account becomes the canonical way back in. + // Sequential, not `Promise.all`: an interactive transaction is bound to a single connection, so + // parallel writes on `tx` buy nothing here and only risk interleaving. + // + // The legacy columns: the 2FA pair is load-bearing (see the backfill note above); `password` is a no-op + // for post-cutover users and kept only so a pre-cutover row cannot survive here. await tx.user.update({ - where: { - id: user.id, - }, + where: { id: user.id }, data: { backupCodes: null, emailVerified: true, @@ -98,6 +203,53 @@ const reclaimUnverifiedLocalAuthIfNeeded = async ({ twoFactorSecret: null, }, }); + const twoFactorRows = await tx.twoFactor.deleteMany({ where: { userId: user.id } }); + const credentialRows = await tx.account.updateMany({ + where: { userId: user.id, provider: "credential" }, + data: { password: null }, + }); + + // MCP OAuth grants the account minted while its address was unproven. Without this the sweep is + // incomplete in the one direction that outlives it: `oauthProvider` is registered unconditionally + // (auth.ts) with open dynamic client registration, so a holder of a live session can bank a refresh + // token good for 30 days — far longer than the session revoked below, and unreachable by it because + // both token tables' `session` FK is `onDelete: SetNull`, which blanks the liveness check rather than + // failing it. + // + // The REFRESH token is the one that matters and the one this actually stops: `handleRefreshTokenGrant` + // reads `revoked`, so revoking it ends the 30-day persistence. + // + // ACCESS tokens are a different story, and worth stating plainly rather than implying this covers them. + // Our config sets `resources` and never sets `disableJwtPlugin`, so `isJwtAccessToken` is always true + // and every access token is a self-contained JWT: `createJwtAccessToken` signs without persisting, so + // there is normally no row here to update, and `/api/mcp` verifies bearers against JWKS + // (`modules/mcp/auth.ts`) without reading this table at all. Upstream's own revoke endpoint says as + // much — "JWT access tokens are self-contained and cannot be revoked server-side". The write below is + // therefore defence for the opaque-token configuration only; the residual is that a squatter's JWT + // stays valid for up to `accessTokenExpiresIn` (15 min) after recovery. Shortening that, or checking + // revocation at the resource server, is the only thing that would close it. + // + // Consent goes too: `/authorize` skips the consent screen when a matching `oauthConsent` row exists, + // so leaving it would let a still-cookie-cached session (see session-revocation.ts) silently mint a + // fresh 30-day refresh token and undo the revocation above. + const revokedAt = new Date(); + const accessRows = await tx.oauthAccessToken.updateMany({ + where: { userId: user.id, revoked: null }, + data: { revoked: revokedAt }, + }); + const refreshRows = await tx.oauthRefreshToken.updateMany({ + where: { userId: user.id, revoked: null }, + data: { revoked: revokedAt }, + }); + const consentRows = await tx.oauthConsent.deleteMany({ where: { userId: user.id } }); + + return { + credentialPasswordsCleared: credentialRows.count, + twoFactorRowsRemoved: twoFactorRows.count, + oauthAccessTokensRevoked: accessRows.count, + oauthRefreshTokensRevoked: refreshRows.count, + oauthConsentsRevoked: consentRows.count, + }; }; const createSsoRecoveryCompletionUrl = (intentToken: string): string => { @@ -198,9 +350,15 @@ export const startSsoRecovery = async ({ export const completeSsoRecovery = async ({ intentToken, sessionUserId, + sessionToken, }: { intentToken: string; sessionUserId?: string; + /** + * The recovering user's own session token, so the post-commit revocation can spare it. Everything else + * the account accrued while its address was unproven is swept. + */ + sessionToken?: string; }): Promise => { let intent: ReturnType; @@ -285,7 +443,7 @@ export const completeSsoRecovery = async ({ where: { id: intent.userId, }, - select: SSO_RECOVERY_USER_SELECT, + select: LINKED_SSO_LOOKUP_SELECT, }); if (user?.email !== intent.email) { @@ -308,8 +466,8 @@ export const completeSsoRecovery = async ({ throw new Error(OAUTH_ACCOUNT_NOT_LINKED_ERROR); } - await prisma.$transaction(async (tx) => { - await reclaimUnverifiedLocalAuthIfNeeded({ + const reclaimed = await prisma.$transaction(async (tx) => { + const outcome = await reclaimUnverifiedLocalAuthIfNeeded({ tx, user, }); @@ -326,8 +484,35 @@ export const completeSsoRecovery = async ({ account: recoveryAccount, tx, }); + + return outcome; }); + // Only when factors were actually stripped: this is the account changing hands, so any session the + // squatter still holds has to go. Reachable in practice because `signUpEmail` writes + // `emailVerified: false` regardless of `requireEmailVerification`, so on an instance with + // EMAIL_VERIFICATION_DISABLED=1 (the shipped .env.example and docker-compose default) an unproven + // account can sign in and hold a live session for up to SESSION_MAX_AGE. + // + // After commit, never inside the transaction: Better Auth resolves its adapter from its own + // AsyncLocalStorage, so this would run outside `tx` and outlive a rollback. Best-effort for the same + // reason the strip must not be undone by a revocation failure — it has already committed. + let sessionsRevoked: number | null = 0; + if (reclaimed) { + try { + sessionsRevoked = await revokeUserSessionsExcept({ + userId: user.id, + keepSessionToken: sessionToken, + }); + } catch (error) { + sessionsRevoked = null; + logger.error( + { error, userId: user.id }, + "Failed to revoke sessions after reclaiming unverified local auth" + ); + } + } + try { await finalizeSuccessfulSignIn({ userId: user.id, @@ -353,6 +538,8 @@ export const completeSsoRecovery = async ({ email: user.email, provider, callbackUrl: intent.callbackUrl, + reclaimed, + sessionsRevoked, }); return getValidatedCallbackUrl(intent.callbackUrl, WEBAPP_URL) ?? WEBAPP_URL; diff --git a/apps/web/modules/ee/unify-feedback/components/feedback-records-page-client.tsx b/apps/web/modules/ee/unify-feedback/components/feedback-records-page-client.tsx index 62fe8ef80484..08a58c2f9d64 100644 --- a/apps/web/modules/ee/unify-feedback/components/feedback-records-page-client.tsx +++ b/apps/web/modules/ee/unify-feedback/components/feedback-records-page-client.tsx @@ -2,6 +2,8 @@ import { useTranslation } from "react-i18next"; import type { TFeedbackSourceFieldMapping } from "@formbricks/types/feedback-source"; +import { EnrichmentStatus } from "@/modules/ee/unify-feedback/enrichment-status/components/enrichment-status"; +import { EnrichmentStatusQueryClientProvider } from "@/modules/ee/unify-feedback/enrichment-status/query-client-provider"; import type { FeedbackRecordData } from "@/modules/hub/types"; import { PageContentWrapper } from "@/modules/ui/components/page-content-wrapper"; import { PageHeader } from "@/modules/ui/components/page-header"; @@ -37,16 +39,23 @@ export function FeedbackRecordsPageClient({ - + {/* Lifted above the table (rather than owned by EnrichmentStatus itself) so the CSV import + flow inside FeedbackRecordsTable can invalidate this same query client when it creates new + pending work — see EnrichmentStatus's doc comment. */} + + + + + ); } diff --git a/apps/web/modules/ee/unify-feedback/components/feedback-records-table.tsx b/apps/web/modules/ee/unify-feedback/components/feedback-records-table.tsx index ace994307d98..83c56507eb81 100644 --- a/apps/web/modules/ee/unify-feedback/components/feedback-records-table.tsx +++ b/apps/web/modules/ee/unify-feedback/components/feedback-records-table.tsx @@ -1,5 +1,6 @@ "use client"; +import { useQueryClient } from "@tanstack/react-query"; import { TFunction } from "i18next"; import { ChevronDownIcon, MessageSquareTextIcon, PlusIcon, RefreshCwIcon } from "lucide-react"; import Link from "next/link"; @@ -10,6 +11,7 @@ import type { TFeedbackSourceFieldMapping } from "@formbricks/types/feedback-sou import { getFeedbackRecordContactsAction, listFeedbackRecordsAction } from "@/lib/feedback-source/actions"; import { formatDateForDisplay, formatDateTimeForDisplay } from "@/lib/utils/datetime"; import { getFormattedErrorMessage } from "@/lib/utils/helper"; +import { enrichmentStatusKeys } from "@/modules/ee/unify-feedback/enrichment-status/lib/query"; import type { FeedbackRecordData } from "@/modules/hub/types"; import { Badge } from "@/modules/ui/components/badge"; import { Button } from "@/modules/ui/components/button"; @@ -83,6 +85,10 @@ export const FeedbackRecordsTable = ({ canDeleteRecords, }: Readonly) => { const { t, i18n } = useTranslation(); + // Reaches the same query client as the enrichment-status banner (provided above this table by + // `feedback-records-page-client.tsx`), so a CSV import here can invalidate that read instead of + // leaving it stale until a manual reload — see EnrichmentStatus's doc comment. + const queryClient = useQueryClient(); const [records, setRecords] = useState(initialRecords); const [cursors, setCursors] = useState>(initialCursors); const [contactIdByUserId, setContactIdByUserId] = @@ -235,6 +241,15 @@ export const FeedbackRecordsTable = ({ void resolveContactsForRecords(mergedRecords); }; + // A CSV import creates records this list and the enrichment-status banner above it don't know + // about yet — neither refetches on its own. Refresh the list the same way the manual Refresh + // button does, and invalidate the banner's read so its next poll (or mount) picks up the new + // backlog instead of staying dark until someone reloads the page. + const handleImportComplete = () => { + void handleRefresh(); + void queryClient.invalidateQueries({ queryKey: enrichmentStatusKeys.all }); + }; + const handleLoadMore = async () => { if (isLoadingMore || isRefreshing || !hasMore) return; setIsLoadingMore(true); @@ -494,6 +509,7 @@ export const FeedbackRecordsTable = ({ feedbackSourceId={csvImportSource.id} workspaceId={workspaceId} fieldMappings={csvImportSource.fieldMappings} + onImportComplete={handleImportComplete} /> )} diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/components/enrichment-status-banner.tsx b/apps/web/modules/ee/unify-feedback/enrichment-status/components/enrichment-status-banner.tsx new file mode 100644 index 000000000000..c4fc671812bc --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/components/enrichment-status-banner.tsx @@ -0,0 +1,112 @@ +"use client"; + +import { TFunction } from "i18next"; +import { AlertCircleIcon, Loader2Icon } from "lucide-react"; +import { useTranslation } from "react-i18next"; +import { ProgressBar } from "@/modules/ui/components/progress-bar"; +import type { TEnrichmentKind, TEnrichmentProgress } from "../lib/enrichment"; +import { totalFailedTerminalEnrichments, totalPendingEnrichments } from "../lib/enrichment"; + +// Spelled out per kind rather than built from the kind: the translation-key scanner only sees literal +// `t("…")` calls, so a computed key would be reported as unused and never reach the other locales. +const enrichmentLabel = (kind: TEnrichmentKind, t: TFunction): string => { + switch (kind) { + case "translation": + return t("workspace.unify.translation"); + case "sentiment": + return t("workspace.unify.sentiment"); + case "emotions": + return t("workspace.unify.emotions"); + } +}; + +/** + * Progress of the record-level AI enrichments (translation, sentiment, emotions) running behind the + * Feedback Data table. + * + * Shown while something is outstanding *or* while a permanent failure (ENG-2375) has nothing left to + * report but itself — a bar keyed only on `pending` would hide the failure count at the exact moment + * it becomes the final answer, which defeats the point of tracking it separately. With neither, there + * is no job to report, and a permanent "all caught up" bar would just be a strip of chrome above every + * page load. Enrichments that have caught up stay listed while the banner is up, so the reader sees + * the whole picture rather than a lone straggler with no context. + */ +export const EnrichmentStatusBanner = ({ enrichments }: Readonly<{ enrichments: TEnrichmentProgress[] }>) => { + const { t } = useTranslation(); + + const totalPending = totalPendingEnrichments(enrichments); + const totalFailedTerminal = totalFailedTerminalEnrichments(enrichments); + if (totalPending === 0 && totalFailedTerminal === 0) return null; + + const isInProgress = totalPending > 0; + + return ( +
+
+
+ {isInProgress ? ( +
+ + {isInProgress + ? t("workspace.unify.enrichment_pending_summary", { pending: totalPending }) + : t("workspace.unify.enrichment_failed_count", { failedCount: totalFailedTerminal })} + +
+ +
+ {enrichments.map((enrichment) => { + const label = enrichmentLabel(enrichment.kind, t); + const progressLabel = t("workspace.unify.enrichment_progress_count", { + done: enrichment.done, + eligible: enrichment.eligible, + }); + + return ( +
+
+ {label} +
+ {enrichment.failedTerminal > 0 && ( + + {t("workspace.unify.enrichment_failed_count", { + failedCount: enrichment.failedTerminal, + })} + + )} + {progressLabel} +
+
+
+ {/* Native carries the accessible semantics (Sonar S6819 — role="progressbar" + on a div is not exposed as a progress bar on every platform); visually hidden since + the styled bar below renders the same value for sighted users. */} + + +
+
+ ); + })} +
+
+ ); +}; diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/components/enrichment-status.tsx b/apps/web/modules/ee/unify-feedback/enrichment-status/components/enrichment-status.tsx new file mode 100644 index 000000000000..26f79daf9401 --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/components/enrichment-status.tsx @@ -0,0 +1,23 @@ +"use client"; + +import { useEnrichmentStatus } from "../hooks/use-enrichment-status"; +import { EnrichmentStatusBanner } from "./enrichment-status-banner"; + +/** + * Background-job status for the Feedback Data page (ENG-2128): how far the record-level AI enrichments + * have got through the workspace's feedback records. Renders nothing until it has a usable answer — a + * Hub that is down or unconfigured reports `unavailable`, which must not surface as an error banner, + * since enrichment progress is context for the table below, not something the page depends on. + * + * Requires an `EnrichmentStatusQueryClientProvider` ancestor. That provider is supplied by the page + * (`feedback-records-page-client.tsx`) rather than by this component, so a sibling that creates new + * pending work — the CSV import flow in the records table — can reach the same query client to + * invalidate this read instead of leaving the indicator stale until a manual reload. + */ +export const EnrichmentStatus = ({ workspaceId }: Readonly<{ workspaceId: string }>) => { + const { data } = useEnrichmentStatus({ workspaceId }); + + if (!data || data.unavailable) return null; + + return ; +}; diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/hooks/use-enrichment-status.test.ts b/apps/web/modules/ee/unify-feedback/enrichment-status/hooks/use-enrichment-status.test.ts new file mode 100644 index 000000000000..6eedb041f5e8 --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/hooks/use-enrichment-status.test.ts @@ -0,0 +1,146 @@ +/** + * @vitest-environment jsdom + */ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook, waitFor } from "@testing-library/react"; +import { type ReactNode, createElement } from "react"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import type { TEnrichmentStatusResponse } from "../lib/enrichment"; +import { useEnrichmentStatus } from "./use-enrichment-status"; + +function createWrapper(queryClient: QueryClient) { + const Wrapper = ({ children }: Readonly<{ children: ReactNode }>) => + createElement(QueryClientProvider, { client: queryClient }, children); + Wrapper.displayName = "UseEnrichmentStatusTestWrapper"; + return Wrapper; +} + +const createQueryClient = () => new QueryClient({ defaultOptions: { queries: { retry: false } } }); + +const jsonResponse = (data: TEnrichmentStatusResponse) => + new Response(JSON.stringify({ data }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + +describe("useEnrichmentStatus", () => { + beforeEach(() => { + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = + true; + vi.stubGlobal("fetch", vi.fn()); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + test("fetches the enrichment status for the workspace", async () => { + const fetchMock = vi.mocked(global.fetch); + fetchMock.mockResolvedValue( + jsonResponse({ + enrichments: [{ kind: "translation", eligible: 500, done: 480, failedTerminal: 0, pending: 20 }], + unavailable: false, + }) + ); + + const { result } = renderHook(() => useEnrichmentStatus({ workspaceId: "w" }), { + wrapper: createWrapper(createQueryClient()), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(result.current.data?.enrichments).toHaveLength(1); + expect(fetchMock).toHaveBeenCalledWith( + "/api/v3/unify-feedback/enrichment-status?workspaceId=w", + expect.objectContaining({ method: "GET", cache: "no-store" }) + ); + }); + + test("keeps polling while work is pending", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const fetchMock = vi.mocked(global.fetch); + fetchMock.mockImplementation(async () => + jsonResponse({ + enrichments: [{ kind: "sentiment", eligible: 500, done: 100, failedTerminal: 0, pending: 400 }], + unavailable: false, + }) + ); + + const { result } = renderHook(() => useEnrichmentStatus({ workspaceId: "w" }), { + wrapper: createWrapper(createQueryClient()), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await vi.advanceTimersByTimeAsync(11_000); + + expect(fetchMock.mock.calls.length).toBeGreaterThan(1); + }); + + test("stops polling once nothing is pending", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const fetchMock = vi.mocked(global.fetch); + fetchMock.mockImplementation(async () => + jsonResponse({ + enrichments: [{ kind: "sentiment", eligible: 500, done: 500, failedTerminal: 0, pending: 0 }], + unavailable: false, + }) + ); + + const { result } = renderHook(() => useEnrichmentStatus({ workspaceId: "w" }), { + wrapper: createWrapper(createQueryClient()), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await vi.advanceTimersByTimeAsync(30_000); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + test("stops polling once the only remainder is permanently-failed records", async () => { + // ENG-2375: before failedTerminal was subtracted, this shape (eligible=500, done=480, 20 + // permanently failed) reported pending=20 and polled forever for work that would never complete. + vi.useFakeTimers({ shouldAdvanceTime: true }); + const fetchMock = vi.mocked(global.fetch); + fetchMock.mockImplementation(async () => + jsonResponse({ + enrichments: [{ kind: "sentiment", eligible: 500, done: 480, failedTerminal: 20, pending: 0 }], + unavailable: false, + }) + ); + + const { result } = renderHook(() => useEnrichmentStatus({ workspaceId: "w" }), { + wrapper: createWrapper(createQueryClient()), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await vi.advanceTimersByTimeAsync(30_000); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + test("stops polling when the Hub is unavailable", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const fetchMock = vi.mocked(global.fetch); + fetchMock.mockImplementation(async () => jsonResponse({ enrichments: [], unavailable: true })); + + const { result } = renderHook(() => useEnrichmentStatus({ workspaceId: "w" }), { + wrapper: createWrapper(createQueryClient()), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await vi.advanceTimersByTimeAsync(30_000); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + test("stays idle (no fetch) when workspaceId is empty", () => { + const fetchMock = vi.mocked(global.fetch); + + const { result } = renderHook(() => useEnrichmentStatus({ workspaceId: "" }), { + wrapper: createWrapper(createQueryClient()), + }); + + expect(result.current.fetchStatus).toBe("idle"); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/hooks/use-enrichment-status.ts b/apps/web/modules/ee/unify-feedback/enrichment-status/hooks/use-enrichment-status.ts new file mode 100644 index 000000000000..c8d3bd4a2ff9 --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/hooks/use-enrichment-status.ts @@ -0,0 +1,29 @@ +"use client"; + +import { useQuery } from "@tanstack/react-query"; +import { getEnrichmentStatus } from "../lib/api-client"; +import { ENRICHMENT_POLL_INTERVAL_MS, totalPendingEnrichments } from "../lib/enrichment"; +import { enrichmentStatusKeys } from "../lib/query"; + +/** + * Enrichment progress for a workspace's feedback directories — feeds the indicator above the records + * table. The route is a live query on the Hub, so there is no staleness to work around; it self-polls + * only while work is outstanding and stops at zero (or when the Hub is unavailable), the same way the + * taxonomy fields query polls while embeddings catch up. + */ +export const useEnrichmentStatus = ({ + workspaceId, + enabled = true, +}: Readonly<{ workspaceId: string; enabled?: boolean }>) => + useQuery({ + queryKey: enrichmentStatusKeys.status(workspaceId), + enabled: enabled && workspaceId.length > 0, + queryFn: ({ signal }) => getEnrichmentStatus({ workspaceId, signal }), + refetchInterval: (query) => { + const data = query.state.data; + if (!data || data.unavailable) { + return false; + } + return totalPendingEnrichments(data.enrichments) > 0 ? ENRICHMENT_POLL_INTERVAL_MS : false; + }, + }); diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/lib/api-client.ts b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/api-client.ts new file mode 100644 index 000000000000..28d7c48ecfa9 --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/api-client.ts @@ -0,0 +1,21 @@ +import { parseV3ApiError } from "@/modules/api/lib/v3-client"; +import type { TEnrichmentStatusResponse } from "./enrichment"; + +/** Client fetcher for the enrichment-status v3 route. Forwards the TanStack `signal`. */ +const ENDPOINT = "/api/v3/unify-feedback/enrichment-status"; + +export async function getEnrichmentStatus(params: { + workspaceId: string; + signal?: AbortSignal; +}): Promise { + const query = new URLSearchParams({ workspaceId: params.workspaceId }); + const response = await fetch(`${ENDPOINT}?${query.toString()}`, { + method: "GET", + cache: "no-store", + signal: params.signal, + }); + if (!response.ok) { + throw await parseV3ApiError(response); + } + return ((await response.json()) as { data: TEnrichmentStatusResponse }).data; +} diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/lib/enrichment.test.ts b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/enrichment.test.ts new file mode 100644 index 000000000000..7d123d070c23 --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/enrichment.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, test } from "vitest"; +import { totalFailedTerminalEnrichments, totalPendingEnrichments } from "./enrichment"; +import type { TEnrichmentProgress } from "./enrichment"; + +const progress = (overrides: Partial): TEnrichmentProgress => ({ + kind: "sentiment", + eligible: 0, + done: 0, + failedTerminal: 0, + pending: 0, + ...overrides, +}); + +describe("totalPendingEnrichments", () => { + test("sums pending across enrichments", () => { + expect( + totalPendingEnrichments([progress({ pending: 5 }), progress({ kind: "emotions", pending: 3 })]) + ).toBe(8); + }); + + test("is zero for an empty list", () => { + expect(totalPendingEnrichments([])).toBe(0); + }); +}); + +describe("totalFailedTerminalEnrichments", () => { + test("sums failedTerminal across enrichments", () => { + expect( + totalFailedTerminalEnrichments([ + progress({ failedTerminal: 20 }), + progress({ kind: "emotions", failedTerminal: 5 }), + ]) + ).toBe(25); + }); + + test("is zero when nothing has failed permanently", () => { + expect(totalFailedTerminalEnrichments([progress({ pending: 40 })])).toBe(0); + }); +}); diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/lib/enrichment.ts b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/enrichment.ts new file mode 100644 index 000000000000..83af2c5496c4 --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/enrichment.ts @@ -0,0 +1,58 @@ +/** + * Shape and pure helpers for the Unify Feedback enrichment-status indicator (ENG-1670 / ENG-2128). + * + * Imported by both the v3 route that builds the payload and the client that renders it, so it stays + * free of server-only and React imports. + */ + +export const ENRICHMENT_KINDS = ["translation", "sentiment", "emotions"] as const; + +export type TEnrichmentKind = (typeof ENRICHMENT_KINDS)[number]; + +/** + * One enrichment's progress, already aggregated across the workspace's feedback directories. + * + * `eligible`/`done` are data-derived counts of feedback records (how many qualify for the enrichment + * vs. how many carry it), not queue depth. `pending` is `eligible - done - failedTerminal` rather than + * the plain difference: a record whose enrichment permanently gave up (content filter, refusal, + * truncation — ENG-2375) would otherwise read as "still in progress" forever, since nothing about it + * changes on its own. `failedTerminal` is reported separately so the UI can say so rather than count + * it as work still moving. + * + * This still isn't the full picture — a record whose enrichment was switched on after it already + * existed was never enqueued at all, and neither `done` nor `failedTerminal` accounts for it, so it + * remains indistinguishable from genuinely in-flight work until ENG-2376 (auto-requeue) ships. + */ +export type TEnrichmentProgress = { + kind: TEnrichmentKind; + eligible: number; + done: number; + failedTerminal: number; + pending: number; +}; + +export type TEnrichmentStatusResponse = { + /** + * Only the enrichments enabled for at least one of the workspace's directories. A disabled + * enrichment is omitted rather than reported as 0/0 — it will never progress, so a bar for it would + * be permanently stuck at zero. + */ + enrichments: TEnrichmentProgress[]; + /** The Hub could not be reached. Render nothing and stop polling rather than showing a false zero. */ + unavailable: boolean; +}; + +/** Poll cadence while enrichment work is still outstanding. */ +export const ENRICHMENT_POLL_INTERVAL_MS = 5000; + +export const totalPendingEnrichments = (enrichments: TEnrichmentProgress[]): number => + enrichments.reduce((sum, enrichment) => sum + enrichment.pending, 0); + +/** + * Total permanently-failed records across all enrichments. Kept separate from + * `totalPendingEnrichments` so the banner can stay up to report failures even once nothing is left + * that could still complete — otherwise the one moment a permanent failure becomes the final answer + * (pending hits 0) is exactly when it would disappear. + */ +export const totalFailedTerminalEnrichments = (enrichments: TEnrichmentProgress[]): number => + enrichments.reduce((sum, enrichment) => sum + enrichment.failedTerminal, 0); diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/lib/query.ts b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/query.ts new file mode 100644 index 000000000000..61e380d1d0b1 --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/lib/query.ts @@ -0,0 +1,5 @@ +/** Typed query-key factory for the enrichment-status read. Never inline string keys. */ +export const enrichmentStatusKeys = { + all: ["unify-enrichment-status"] as const, + status: (workspaceId: string) => [...enrichmentStatusKeys.all, workspaceId] as const, +}; diff --git a/apps/web/modules/ee/unify-feedback/enrichment-status/query-client-provider.tsx b/apps/web/modules/ee/unify-feedback/enrichment-status/query-client-provider.tsx new file mode 100644 index 000000000000..2ac9607aec1d --- /dev/null +++ b/apps/web/modules/ee/unify-feedback/enrichment-status/query-client-provider.tsx @@ -0,0 +1,17 @@ +"use client"; + +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { type ReactNode, useState } from "react"; + +/** Feature-scoped React Query provider for the enrichment-status indicator. Created once per mount so + * the cache survives re-renders but isn't shared across requests. */ +export const EnrichmentStatusQueryClientProvider = ({ children }: Readonly<{ children: ReactNode }>) => { + const [queryClient] = useState( + () => + new QueryClient({ + defaultOptions: { queries: { retry: 1, refetchOnWindowFocus: false } }, + }) + ); + + return {children}; +}; diff --git a/apps/web/modules/ee/unify-feedback/sources/components/csv-import-modal.tsx b/apps/web/modules/ee/unify-feedback/sources/components/csv-import-modal.tsx index 7555b52953cd..1bd6db79d816 100644 --- a/apps/web/modules/ee/unify-feedback/sources/components/csv-import-modal.tsx +++ b/apps/web/modules/ee/unify-feedback/sources/components/csv-import-modal.tsx @@ -34,6 +34,9 @@ interface CsvImportModalProps { workspaceId: string; fieldMappings: TFeedbackSourceFieldMapping[]; onOpenEditFeedbackSource?: () => void; + /** Called after a successful import, in addition to closing the dialog — e.g. so a caller rendered + * next to a stale records list or enrichment-status read can refresh/invalidate it. */ + onImportComplete?: () => void; } export function CsvImportModal({ @@ -43,6 +46,7 @@ export function CsvImportModal({ workspaceId, fieldMappings, onOpenEditFeedbackSource, + onImportComplete, }: CsvImportModalProps) { const { t } = useTranslation(); const [csvFile, setCsvFile] = useState(null); @@ -122,6 +126,7 @@ export function CsvImportModal({ setParsedData([]); setRowCount(0); onOpenChange(false); + onImportComplete?.(); } else { toast.error( getTranslatedFeedbackSourceError(result.error.error, t, { diff --git a/apps/web/modules/hub/service.ts b/apps/web/modules/hub/service.ts index fa4b64e16558..91d41fc7d45e 100644 --- a/apps/web/modules/hub/service.ts +++ b/apps/web/modules/hub/service.ts @@ -6,6 +6,7 @@ import { assertRepeatedArrayParams, getHubClient } from "./hub-client"; import type { CreateTaxonomyRunInput, CreateTaxonomyRunResponse, + EnrichmentStatusResponse, FeedbackRecordCountParams, FeedbackRecordCountResponse, FeedbackRecordCreateParams, @@ -422,6 +423,27 @@ export const createFeedbackRecordsBatch = async ( return { results }; }; +/** + * Per-tenant enrichment progress (translation, sentiment, emotions) — ENG-1670. + * + * A live query on the Hub side, so there is nothing to cache here: the caller polls it while work is + * outstanding and stops once nothing is pending. + */ +export const getEnrichmentStatus = async (tenantId: string): Promise> => { + const client = getHubClient(); + if (!client) { + return { data: null, error: { ...NO_CONFIG_ERROR } }; + } + + try { + const data = await client.enrichmentStatus.retrieve({ tenant_id: tenantId }); + return { data, error: null }; + } catch (err) { + logger.warn({ err, tenantId, hint: getHubErrorHint(err) }, "Hub: getEnrichmentStatus failed"); + return createHubResultFromError(err); + } +}; + export const listTaxonomyFields = async (tenantId: string): Promise> => { const client = getHubClient(); if (!client) { diff --git a/apps/web/modules/hub/types.ts b/apps/web/modules/hub/types.ts index e673ba0c9347..dcbf8796e049 100644 --- a/apps/web/modules/hub/types.ts +++ b/apps/web/modules/hub/types.ts @@ -42,6 +42,39 @@ export type SimilarRecordsResponse = FormbricksHub.FeedbackRecords.FeedbackRecor export type SimilarRecordsResultItem = FormbricksHub.FeedbackRecords.FeedbackRecordRetrieveSimilarResponse.Data; +// Tenant-scoped enrichment progress (ENG-1670). Counts are data-derived from the directory's feedback +// records — how many qualify for an enrichment vs. how many carry it — not queue depth, so `done` never +// exceeds `eligible` and "in progress" is the difference. `enabled: false` means the enrichment is +// switched off for the tenant or not configured in the deployment, and its counts are zero. +// +// `failed`/`failed_terminal` (ENG-2375, hub PR formbricks/hub#125) split what used to be silently +// folded into `eligible - done`: `failed` is a transient failure River will retry; `failed_terminal` +// gave up for good (content filter, refusal, truncation) and will never complete on its own. Without +// this, a permanently-failed record read as "still in progress" forever and the poll never stopped. +// The published SDK predates the fields, so bridge them as optional reads until it ships them. +// Note: `eligible - done - failed - failed_terminal` is not always 0 — a record whose enrichment was +// enabled after it already existed was never enqueued at all, and neither done nor failed accounts for +// it (ENG-2376 tracks auto-requeueing that residual; out of scope here). +// +// `failed` is intentionally not read by the aggregator: a transient failure is still going to be +// retried by River, so it stays folded into `pending` the same way it always did — only +// `failed_terminal` (which will never resolve on its own) is pulled out and shown separately. +export type EnrichmentTypeStatus = FormbricksHub.TypeStatus & { + failed?: number; + failed_terminal?: number; +}; +// The three per-enrichment keys are re-declared optional (the SDK types them as always-present) +// because the aggregator treats an absent key as "disabled" rather than assuming the Hub always +// answers with all three — matching the `status?.enabled` optional-chaining it already does. +export type EnrichmentStatusResponse = Omit< + FormbricksHub.EnrichmentStatusRetrieveResponse, + "translation" | "sentiment" | "emotions" +> & { + translation?: EnrichmentTypeStatus; + sentiment?: EnrichmentTypeStatus; + emotions?: EnrichmentTypeStatus; +}; + export type TaxonomyScope = { tenant_id: string; source_type: string; diff --git a/apps/web/modules/survey/editor/components/survey-variables-card-item.tsx b/apps/web/modules/survey/editor/components/survey-variables-card-item.tsx index 4ed95d9e4cc7..bbdf9c411072 100644 --- a/apps/web/modules/survey/editor/components/survey-variables-card-item.tsx +++ b/apps/web/modules/survey/editor/components/survey-variables-card-item.tsx @@ -202,6 +202,7 @@ export const SurveyVariablesCardItem = ({ name="type" render={({ field }) => ( form.handleSubmit(editSurveyVariable)() : undefined}>