Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import TenantCIAMEmailVerification, {
import { otp6 } from "@/lib/crypto/otp";
import { service_role } from "@/lib/supabase/server";
import { select_lang } from "@/i18n/utils";
import { getLocale } from "@/i18n/server";
import {
challengeEmailStateKey,
loadChallengeEmailContext,
Expand Down Expand Up @@ -153,12 +154,14 @@ export async function POST(
: undefined;
const brand_support_contact = publisher.includes("@") ? publisher : undefined;

const langCandidate =
www && typeof www.lang === "string" && www.lang ? www.lang : formDoc.lang;
const emailLang: CIAMVerificationEmailLang = select_lang(
langCandidate,
supported_languages,
"en"
// Prefer the per-form document language when set; otherwise fall back to tenant/published `www.lang`.
// (Treat empty strings as "unset".)
const langCandidate = formDoc.lang?.trim() || www?.lang?.trim() || null;
// Prefer the visitor's device language. If unsupported, fall back to the form/tenant default.
const fallback_lang = select_lang(langCandidate, supported_languages, "en");
const emailLang: CIAMVerificationEmailLang = await getLocale(
[...supported_languages],
fallback_lang
);
const { error: resend_err } = await resend.emails.send({
from: `${brand_name} <[email protected]>`,
Expand Down
145 changes: 106 additions & 39 deletions editor/app/(api)/(public)/v1/submit/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,15 @@ async function submit({
formdata: FormData | URLSearchParams | Map<string, string>;
meta: SessionMeta;
}) {
function isRecord(v: unknown): v is Record<string, unknown> {
return typeof v === "object" && v !== null;
}

function readStringProp(obj: unknown, key: string): string | null {
if (!isRecord(obj)) return null;
const v = obj[key];
return typeof v === "string" && v ? v : null;
}
// console.log("form_id", form_id);

// check for mandatory meta
Expand Down Expand Up @@ -204,6 +213,54 @@ async function submit({
: null;

// customer handling
// NOTE: challenge_email can bind a verified customer to the response_session.
// When that happens, we must reuse that customer instead of creating a new
// fingerprint-based customer (which causes duplicate customers for one respondent).

const challenge_email_fields = fields.filter(
(f) => f.type === "challenge_email"
);

// These are also used later for persistence/binding.
let session_raw: Record<string, unknown> | null = null;
let session_customer_id: string | null = null;
let verified_customer_id: string | null = null;

if (challenge_email_fields.length > 0 && meta.session) {
const { data: response_session, error: response_session_err } =
await service_role.forms
.from("response_session")
.select("id, raw, customer_id")
.eq("id", meta.session)
.eq("form_id", form_id)
.single();

if (response_session_err || !response_session) {
console.error("submit/err/session", response_session_err);
return error(ERR.SERVICE_ERROR.code, { form_id }, meta);
}

session_raw =
(response_session.raw as Record<string, unknown> | null) ?? null;
session_customer_id = response_session.customer_id ?? null;

// If verification succeeded, the challenge state stores `customer_uid`.
// Use it as an authoritative identity fallback (even when response_session.customer_id
// wasn't bound due to stricter binding rules).
for (const f of challenge_email_fields) {
const key = `__challenge_email__${f.id}`;
const obj = session_raw?.[key];
const state = readStringProp(obj, "state");
const customer_uid = readStringProp(obj, "customer_uid");
if (state === "challenge-success" && customer_uid) {
verified_customer_id = customer_uid;
break;
}
}
}

const effective_session_customer_id =
session_customer_id ?? verified_customer_id ?? null;

const _gf_customer_uuid: string | null = qval(
formdata.get(SYSTEM_GF_CUSTOMER_UUID_KEY) as string
Expand All @@ -227,16 +284,50 @@ async function submit({

// console.log("/submit::_gf_customer_uuid:", _gf_customer_uuid);

const customer = await upsert_customer_with({
project_id: form_reference.project_id,
uuid: _gf_customer_uuid,
hints: {
_fp_fingerprintjs_visitorid,
email: _gf_customer_email || undefined,
phone: _gf_customer_phone || undefined,
name: _gf_customer_name || undefined,
},
});
// Only create/upsert a customer when we do not already have a trusted customer id.
// This prevents duplicate customer rows for challenge_email flows.
const customer = effective_session_customer_id
? null
: await upsert_customer_with({
project_id: form_reference.project_id,
uuid: _gf_customer_uuid,
hints: {
_fp_fingerprintjs_visitorid,
email: _gf_customer_email || undefined,
phone: _gf_customer_phone || undefined,
name: _gf_customer_name || undefined,
},
});

// Best-effort: if we have a verified/session customer, enrich it with submitted hints.
// Avoid writing empty values; keep this non-blocking.
if (effective_session_customer_id) {
const patch: {
last_seen_at: string;
email?: string;
phone?: string;
name?: string;
} = { last_seen_at: new Date().toISOString() };
if (_gf_customer_email) patch.email = _gf_customer_email;
if (_gf_customer_phone) patch.phone = _gf_customer_phone;
if (_gf_customer_name) patch.name = _gf_customer_name;

try {
const { error: cus_update_err } = await service_role.workspace
.from("customer")
.update(patch)
.eq("uid", effective_session_customer_id)
.eq("project_id", form_reference.project_id);
if (cus_update_err) {
console.error(
"customer::session_customer_update_error:",
cus_update_err
);
}
} catch (e) {
console.error("customer::session_customer_update_unexpected_error:", e);
}
}

// console.log("/submit::customer:", customer);

Expand Down Expand Up @@ -285,7 +376,7 @@ async function submit({
// TODO: this also needs to be migrated to db constraints
const max_access_by_customer_error = await validate_max_access_by_customer({
form_id,
customer_id: customer?.uid,
customer_id: effective_session_customer_id ?? customer?.uid ?? null,
is_max_form_responses_by_customer_enabled,
max_form_responses_by_customer,
});
Expand Down Expand Up @@ -325,36 +416,10 @@ async function submit({
// ==================================================
// challenge_email gating (session-scoped verification)
// ==================================================
const challenge_email_fields = fields.filter(
(f) => f.type === "challenge_email"
);
const required_challenge_email_fields = challenge_email_fields.filter(
(f) => f.required
);

// These are also used later for persistence/binding.
let session_raw: Record<string, unknown> | null = null;
let session_customer_id: string | null = null;

if (challenge_email_fields.length > 0 && meta.session) {
const { data: response_session, error: response_session_err } =
await service_role.forms
.from("response_session")
.select("id, raw, customer_id")
.eq("id", meta.session)
.eq("form_id", form_id)
.single();

if (response_session_err || !response_session) {
console.error("submit/err/session", response_session_err);
return error(ERR.SERVICE_ERROR.code, { form_id }, meta);
}

session_raw =
(response_session.raw as Record<string, unknown> | null) ?? null;
session_customer_id = response_session.customer_id ?? null;
}

if (required_challenge_email_fields.length > 0) {
if (!meta.session || !session_raw) {
return error(ERR.CHALLENGE_EMAIL_NOT_VERIFIED.code, { form_id }, meta);
Expand Down Expand Up @@ -508,7 +573,8 @@ async function submit({
ip: meta.ip,
// Prefer explicit customer from the submit payload; otherwise use session binding
// (set only by trusted verification flows).
customer_id: customer?.uid ?? session_customer_id,
customer_id:
effective_session_customer_id ?? customer?.uid ?? session_customer_id,
x_referer: meta.referer,
x_useragent: meta.useragent,
x_ipinfo: ipinfo_data as {},
Expand Down Expand Up @@ -545,7 +611,8 @@ async function submit({
form_id,
...({
fingerprint: _fp_fingerprintjs_visitorid,
customer_id: customer?.uid,
customer_id:
effective_session_customer_id ?? customer?.uid ?? undefined,
session_id: meta.session || undefined,
} satisfies FormLinkURLParams["alreadyresponded"]),
},
Expand Down
11 changes: 9 additions & 2 deletions editor/app/(api)/private/editor/[form_id]/fields/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export async function POST(
autocomplete: init.autocomplete,
data: safe_data_field({
type: init.type,
data: init.data as any,
data: init.data,
}) as any,
accept: init.accept,
multiple: init.multiple,
Expand Down Expand Up @@ -323,9 +323,16 @@ function safe_data_field({
data,
}: {
type: FormInputType;
data?: FormFieldDataSchema;
data?: FormFieldDataSchema | null;
}): FormFieldDataSchema | undefined | null {
switch (type) {
case "tel": {
// Keep `data` as an object (if provided) so we can safely persist tel configs
// such as `default_country` without losing shape.
if (!data) return data;
if (typeof data === "object") return data;
return {};
}
case "payment": {
// TODO: enhance the schema validation with external libraries
if (!data || !(data as PaymentFieldData).type) {
Expand Down
61 changes: 56 additions & 5 deletions editor/app/(tenant)/~/[tenant]/(p)/p/login/page.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,66 @@
import React from "react";
import PortalLogin from "./login";
import { getLocale } from "@/i18n/server";
import Link from "next/link";
import { createWWWClient } from "@/lib/supabase/server";
import type { Database } from "@app/database";

export default async function CustomerPortalLoginPage() {
type Params = {
tenant: string;
};

type WwwPublicRow = Database["grida_www"]["Views"]["www_public"]["Row"];

async function fetchPortalTitle(tenant: string) {
const client = await createWWWClient();

const { data: wwwPublic } = await client
.from("www_public")
.select("title")
.eq("name", tenant)
.single()
.returns<Pick<WwwPublicRow, "title">>();

const title =
typeof wwwPublic?.title === "string" && wwwPublic.title.trim()
? wwwPublic.title
: "Customer Portal";

return title;
}

export default async function CustomerPortalLoginPage({
params,
}: {
params: Promise<Params>;
}) {
const { tenant } = await params;
const locale = await getLocale(["en", "ko"]);
const title = await fetchPortalTitle(tenant);

return (
<div className="flex min-h-svh flex-col items-center justify-center gap-6 bg-background p-6 md:p-10">
<div className="w-full max-w-sm">
<PortalLogin locale={locale} />
</div>
<div className="min-h-svh bg-background flex flex-col">
<header className="h-14 border-b" data-testid="portal-login-navbar">
<div className="mx-auto h-full w-full max-w-screen-sm px-6 flex items-center">
<Link
href="#"
className="flex items-center gap-2 min-w-0"
data-testid="portal-login-navbar-brand-link"
>
<span
className="font-semibold truncate"
data-testid="portal-login-navbar-title"
>
{title}
</span>
</Link>
Comment on lines +45 to +56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n editor/app/\(tenant\)/~/\[tenant\]/\(p\)/p/login/page.tsx | head -80

Repository: gridaco/grida

Length of output: 2353


🌐 Web query:

Next.js Link component href="#" accessibility screen reader issues

💡 Result:

Short answer: using href="#" (or an anchor with no real destination) inside a Next.js causes accessibility and focus/announcement problems for screen-reader and keyboard users. Prefer a real fragment (href="#id") or an actual URL for navigation; use a (not a link) for JS actions; and ensure semantic/proper attributes (aria-label, aria-disabled, id, passHref when needed). Sources and quick fixes below.

Why it's a problem

  • href="#" is an empty/same-page target that can confuse screen readers and keyboard focus (focus may remain on the trigger, and AT may not announce a page change). [1][3][4]
  • Next.js renders/extends an actual , and the docs expect real hrefs (including fragments). Using non-semantic or missing hrefs harms semantics/SEO and AT behaviour. [2][5]

Recommended fixes (concise)

  • If you need to jump to an element on the page, use a real fragment linking to an element id:
  • If the UI element triggers a JS action (open modal, toggle) and is not navigation, use a
(with aria attributes) instead of a link:
Open. Buttons expose correct semantics to AT. [1][3]

  • If you must present a disabled link, use aria-disabled and prevent pointer/keyboard activation, but prefer hiding or replacing it with non-interactive text:
    • <a href={url} aria-disabled="true" onClick={(e)=>e.preventDefault()}>Label and style pointer-events:none; also announce with aria-disabled. Note: many screen readers treat aria-disabled on anchors inconsistently—test with real AT. [1][3]
  • If using a custom child component that renders an , include passHref/legacyBehavior per Next.js docs so the underlying gets an href (important for accessibility/SEO). [2][4]
  • References

    • MDN: ARIA link role / prefer native semantics. [1] (developer.mozilla.org)
    • Next.js Link docs (Link renders to , supports fragments; passHref notes). [2] (nextjs.org)
    • Discussion on empty href/# accessibility problems. [3] (stackoverflow.com)
    • Reports of Next.js Link + same-page/hash focus behavior. [4] (stackoverflow.com)
    • Next.js accessibility/route announcer guidance (client navigation announcements). [5] (nextjs.org)

    If you want, tell me the exact code you're using and I’ll show the minimal fix.

    Citations:


    🌐 Web query:

    Next.js Link best practices when there's no destination href empty string

    💡 Result:

    Short answer — don't render a Next.js with an empty/undefined href. Instead conditionally render a real link only when you have a destination; otherwise render a semantic non-link control (button, span with role="button", or an anchor with aria-disabled and no href) and handle click behavior/accessibility explicitly.

    Concrete best practices

    • Condition the Link: only output when href is a non-empty, valid URL/path. This avoids invalid anchors and client-side navigation problems. [1][2]
    • If you need a non-navigating control that looks like a link, use a (preferred for actions) or a / (if purely presentational), and implement keyboard and screen-reader behavior. Ensure aria-disabled, tabindex, and onKeyDown/onClick semantics are correct for accessibility. [3]
    • If href may be external, use a plain with target/rel instead of next/link (next/link is for internal client-side navigation). [4]
    • Avoid using next/link with empty string or undefined because href is required for correct client-side routing and accessibility; conditionally compute/validate href first (e.g., href && <Link ... />). [1][2]
    • If you must render an anchor that shouldn’t navigate, preventDefault onClick and set aria-disabled="true", but prefer rendering a non-link element for actions. [3]

    References

    • Next.js Link docs / API (href is the navigation prop; use Link for internal client-side navigation). [1]
    • Next.js guidance / component docs and ESLint rule about proper Link usage (wrap anchors / passHref and only use Link for navigation). [2]
    • Accessibility guidance and best-practice recommendations for link vs. button semantics and disabled links. [3]
    • Advice: use plain for external links (next/link is for internal routes). [4]

    Would you like a short code example showing the conditional rendering + accessible alternative?


    Replace decorative Link with <div> to fix accessibility.
    Using href="#" creates a non-functional anchor that confuses screen readers and keyboard navigation. Since this is a brand element with no navigation target or action, use a plain <div> instead.

    Suggested fix
    -          <Link
    -            href="#"
    -            className="flex items-center gap-2 min-w-0"
    -            data-testid="portal-login-navbar-brand-link"
    -          >
    +          <div
    +            className="flex items-center gap-2 min-w-0"
    +            data-testid="portal-login-navbar-brand-link"
    +          >
                 <span
                   className="font-semibold truncate"
                   data-testid="portal-login-navbar-title"
                 >
                   {title}
                 </span>
    -          </Link>
    +          </div>
    📝 Committable suggestion

    ‼️ IMPORTANT
    Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    Suggested change
    <Link
    href="#"
    className="flex items-center gap-2 min-w-0"
    data-testid="portal-login-navbar-brand-link"
    >
    <span
    className="font-semibold truncate"
    data-testid="portal-login-navbar-title"
    >
    {title}
    </span>
    </Link>
    <div
    className="flex items-center gap-2 min-w-0"
    data-testid="portal-login-navbar-brand-link"
    >
    <span
    className="font-semibold truncate"
    data-testid="portal-login-navbar-title"
    >
    {title}
    </span>
    </div>
    🤖 Prompt for AI Agents
    In `@editor/app/`(tenant)/~/[tenant]/(p)/p/login/page.tsx around lines 45 - 56,
    The brand element currently uses the Link component with href="#" (Link) which
    is decorative and non-functional; replace the Link wrapper with a plain div
    (preserving className="flex items-center gap-2 min-w-0" and
    data-testid="portal-login-navbar-brand-link") so the element is semantic and
    accessible, keep the inner span (data-testid="portal-login-navbar-title")
    unchanged; also remove or adjust any now-unused Link import if applicable.
    

    </div>
    </header>
    <main className="flex-1 flex items-center justify-center p-6 md:p-10">
    <div className="w-full max-w-sm">
    <PortalLogin locale={locale} />
    </div>
    </main>
    </div>
    );
    }
    20 changes: 20 additions & 0 deletions editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx
    Original file line number Diff line number Diff line change
    Expand Up @@ -7,6 +7,12 @@ import type { Database } from "@app/database";
    import type { PostgrestError } from "@supabase/supabase-js";
    import CampaignReferrerCard from "../west-campaign-referrer-card";

    // TODO(portal): The portal session page is not design-complete yet, but is used in production.
    // Today enterprise customers typically only have one active campaign, so we temporarily redirect
    // directly to the campaign page when there's only a single campaign in the portal session.
    // Remove this once the session page UX is finalized and multi-campaign is common.
    const TMP_SHOULD_REDIRECT_WHEN_SINGLE = true;

    type Params = {
    token: string;
    };
    Expand Down Expand Up @@ -124,6 +130,20 @@ export default async function CustomerPortalSessionPage({
    console.error("[ciam]/referrer error", referrer_err);
    }

    if (
    TMP_SHOULD_REDIRECT_WHEN_SINGLE &&
    iam_referrers &&
    iam_referrers.length === 1
    ) {
    const r = iam_referrers[0];
    const path = r?.campaign?.www_route_path;
    const code = r?.code;
    if (typeof path === "string" && path && typeof code === "string" && code) {
    // FIXME: tenant url
    return redirect(`${path}/t/${code}`);
    }
    }
    Comment on lines +133 to +145
    Copy link

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    ⚠️ Potential issue | 🟠 Major

    🧩 Analysis chain

    🏁 Script executed:

    # First, let's look at the actual file and the context around lines 133-145
    cat -n "editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx" | sed -n '120,160p'

    Repository: gridaco/grida

    Length of output: 1506


    🏁 Script executed:

    # Get the full file size to understand what we're working with
    wc -l "editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx"

    Repository: gridaco/grida

    Length of output: 122


    🏁 Script executed:

    # Search for type definitions related to 'campaign' or 'www_route_path'
    rg -i "www_route_path|campaign" --type ts --type tsx -A 2 -B 2 | head -100

    Repository: gridaco/grida

    Length of output: 84


    🏁 Script executed:

    # Search for the iam_referrers variable source and its type
    rg -i "iam_referrers" --type ts --type tsx -B 3 -A 3 | head -80

    Repository: gridaco/grida

    Length of output: 84


    🏁 Script executed:

    # Look for existing URL/path validation patterns in the codebase
    rg "(startsWith|redirect|protocol|absolute)" --type ts --type tsx -B 1 -A 1 | grep -i "path\|url\|redirect" | head -50

    Repository: gridaco/grida

    Length of output: 84


    🏁 Script executed:

    # Search without type filter for www_route_path and campaign type info
    rg "www_route_path" -A 2 -B 2

    Repository: gridaco/grida

    Length of output: 5231


    🏁 Script executed:

    # Look for type definitions or interfaces related to campaign
    rg "campaign" --type ts -A 2 -B 1 | head -150

    Repository: gridaco/grida

    Length of output: 10853


    🏁 Script executed:

    # Search for supabase schema or type definitions
    find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.sql" \) | xargs grep -l "campaign_public" | head -10

    Repository: gridaco/grida

    Length of output: 576


    🏁 Script executed:

    # Look for redirect usage and URL validation patterns throughout codebase
    rg "redirect\(" -A 1 -B 1 | grep -A 1 -B 1 "startsWith\|url\|path" | head -60

    Repository: gridaco/grida

    Length of output: 2196


    🏁 Script executed:

    # Search for URL validation helpers or utility functions
    find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "isSafeUrl\|validateUrl\|sanitizeUrl" | head -10

    Repository: gridaco/grida

    Length of output: 94


    🏁 Script executed:

    # Check if there are other usages of www_route_path in the session page that need attention
    sed -n '160,175p' "editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx"

    Repository: gridaco/grida

    Length of output: 534


    🏁 Script executed:

    # Look at the schema definition for route_path to see if there are any constraints
    rg "route_path" supabase/schemas/ supabase/migrations/ -A 2 -B 2

    Repository: gridaco/grida

    Length of output: 2425


    Validate www_route_path before redirect to prevent open redirects.

    The campaign.www_route_path field can contain absolute URLs or protocol-relative paths without database constraints. If base_path or name (which compose this field) contain values like https://attacker.com or //attacker.com, the redirect becomes exploitable. Add validation to ensure the path is relative and safe:

    Suggested guard
    -    if (typeof path === "string" && path && typeof code === "string" && code) {
    +    const isSafePath =
    +      typeof path === "string" &&
    +      path &&
    +      path.startsWith("/") &&
    +      !path.startsWith("//") &&
    +      !path.includes("://");
    +    if (isSafePath && typeof code === "string" && code) {
           // FIXME: tenant url
           return redirect(`${path}/t/${code}`);
         }

    Note: The same vulnerable pattern appears in the JSX at line 169 (${r.campaign.www_route_path}/t/${r.code}); apply the same validation there.

    🤖 Prompt for AI Agents
    In `@editor/app/`(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx around lines
    133 - 145, The redirect uses campaign.www_route_path without validating it (see
    TMP_SHOULD_REDIRECT_WHEN_SINGLE, iam_referrers, redirect and the JSX
    interpolation `${r.campaign.www_route_path}/t/${r.code}`), creating an
    open-redirect risk; fix by validating and normalizing www_route_path before use:
    ensure it is a safe relative path (e.g., starts with a single '/' and does not
    start with '//' or contain a scheme like 'http:'), reject or fallback for any
    absolute or protocol-relative values, and apply the same
    validation/normalization both in the redirect branch (where return redirect(...)
    is called) and in the JSX rendering that builds
    `${r.campaign.www_route_path}/t/${r.code}` so only validated relative paths are
    used.
    


    console.info("[ciam]/portal session redeemed", {
    tokenPreview,
    tokenLength: token.length,
    Expand Down
    Original file line number Diff line number Diff line change
    Expand Up @@ -8,6 +8,7 @@ import TenantCIAMEmailVerification, {
    } from "@/theme/templates-email/ciam-verifiaction/default";
    import { otp6 } from "@/lib/crypto/otp";
    import { select_lang } from "@/i18n/utils";
    import { getLocale } from "@/i18n/server";

    /**
    * POST /api/ciam/auth/challenge/with-email
    Expand Down Expand Up @@ -118,10 +119,11 @@ export async function POST(
    ? publisher
    : undefined;

    const emailLang: CIAMVerificationEmailLang = select_lang(
    www.lang,
    supported_languages,
    "en"
    // Prefer the visitor's device language. If unsupported, fall back to the tenant default.
    const fallback_lang = select_lang(www.lang, supported_languages, "en");
    const emailLang: CIAMVerificationEmailLang = await getLocale(
    [...supported_languages],
    fallback_lang
    );
    const { error: resend_err } = await resend.emails.send({
    from: `${brand_name} <[email protected]>`,
    Expand Down
    Loading