diff --git a/app/client/src/ce/constants/messages.ts b/app/client/src/ce/constants/messages.ts index 93533ed3e330..3a36ee3e489d 100644 --- a/app/client/src/ce/constants/messages.ts +++ b/app/client/src/ce/constants/messages.ts @@ -2093,6 +2093,10 @@ export const IN_APP_EMBED_SETTING = { copied: () => "Copied", limitEmbeddingLabel: () => "Embedding restricted", limitEmbeddingTooltip: () => "This app can be embedded in approved URLs only", + limitEmbeddingBareWildcardError: () => + 'A bare "*" allows embedding on every domain. Select "Allow embedding everywhere" instead, or enter specific URLs.', + limitEmbeddingDisableKeywordError: () => + '"none" disables embedding everywhere and cannot be combined with URLs. Select "Disable embedding everywhere" instead, or enter specific URLs.', disableEmbeddingLabel: () => "Embedding disabled", disableEmbeddingTooltip: () => "This app cannot be embedded anywhere on the internet", diff --git a/app/client/src/ce/pages/AdminSettings/config/configuration.test.tsx b/app/client/src/ce/pages/AdminSettings/config/configuration.test.tsx new file mode 100644 index 000000000000..c75a7fe787bd --- /dev/null +++ b/app/client/src/ce/pages/AdminSettings/config/configuration.test.tsx @@ -0,0 +1,102 @@ +import { render } from "test/testUtils"; +import React from "react"; +import Radio from "pages/AdminSettings/FormGroup/Radio"; +import { SETTINGS_FORM_NAME } from "ee/constants/forms"; +import { reduxForm } from "redux-form"; +import { AppsmithFrameAncestorsSetting } from "pages/Applications/EmbedSnippet/Constants/constants"; +import { APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING } from "./configuration"; + +// Render the real frame-ancestors radio setting with the given form value. +// Passing `undefined` simulates a cold bootstrap of /settings/configuration, +// where the admin-settings store is not yet hydrated and redux-form calls the +// field's format() with undefined. +function renderFrameAncestorsRadio(value: string | undefined) { + function FrameAncestorsRadio() { + return ; + } + + // TODO: Fix this the next time the file is edited + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const Parent = reduxForm({ + validate: () => ({}), + form: SETTINGS_FORM_NAME, + touchOnBlur: true, + })(FrameAncestorsRadio); + + return render(, { + initialState: { + form: { + [SETTINGS_FORM_NAME]: { + values: + value === undefined + ? {} + : { [APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING.id]: value }, + }, + }, + }, + }); +} + +describe("APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING radio", () => { + it("renders without crashing when the setting is not yet hydrated (cold bootstrap)", () => { + // Regression: on a hard refresh of /settings/configuration the value is + // undefined, so format() must tolerate it instead of throwing and taking the + // whole settings page to the error boundary. + expect(() => renderFrameAncestorsRadio(undefined)).not.toThrow(); + expect(document.querySelectorAll("input[type=radio]").length).toBe(3); + }); + + it("selects Allow embedding everywhere for a value containing a bare *", () => { + renderFrameAncestorsRadio("'self' *"); + + const radios = + document.querySelectorAll("input[type=radio]"); + + // Options render in config order: allow / limit / disable. + expect(radios[0].value).toBe( + AppsmithFrameAncestorsSetting.ALLOW_EMBEDDING_EVERYWHERE, + ); + expect(radios[0].checked).toBe(true); + }); +}); + +describe("APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING parse (stored value)", () => { + // The parse step is what actually writes the env value on save. + const parse = APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING.parse as (v: { + value: string; + additionalData?: string; + }) => string; + + it("quotes a bare self typed in the limit list so it is never persisted raw", () => { + expect( + parse({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "self,https://a.com", + }), + ).toBe("'self' https://a.com"); + }); + + it("never combines 'none' with allow-list sources when saving", () => { + // "'none'" is exclusive in CSP; persisting "'none' https://a.com" would + // silently disable embedding despite the "limit" selection. + expect( + parse({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "none,https://a.com", + }), + ).toBe("https://a.com"); + }); + + it("stores the quoted keywords for the allow/disable options", () => { + expect( + parse({ + value: AppsmithFrameAncestorsSetting.ALLOW_EMBEDDING_EVERYWHERE, + }), + ).toBe("*"); + expect( + parse({ + value: AppsmithFrameAncestorsSetting.DISABLE_EMBEDDING_EVERYWHERE, + }), + ).toBe("'none'"); + }); +}); diff --git a/app/client/src/ce/pages/AdminSettings/config/configuration.tsx b/app/client/src/ce/pages/AdminSettings/config/configuration.tsx index bbab80ec5a2c..d8b9b8f309fe 100644 --- a/app/client/src/ce/pages/AdminSettings/config/configuration.tsx +++ b/app/client/src/ce/pages/AdminSettings/config/configuration.tsx @@ -9,11 +9,14 @@ import { SettingSubtype, SettingTypes, } from "ee/pages/AdminSettings/config/types"; -import { TagInput } from "@appsmith/ads-old"; import localStorage from "utils/localStorage"; import isUndefined from "lodash/isUndefined"; import { AppsmithFrameAncestorsSetting } from "pages/Applications/EmbedSnippet/Constants/constants"; -import { formatEmbedSettings } from "pages/Applications/EmbedSnippet/Utils/utils"; +import { + formatEmbedSettings, + sanitizeLimitedFrameAncestors, +} from "pages/Applications/EmbedSnippet/Utils/utils"; +import FrameAncestorsTagInput from "pages/Applications/EmbedSnippet/FrameAncestorsTagInput"; import { isAirgapped } from "ee/utils/airgapHelpers"; import { APPSMITH_BASE_URL_SETUP_DOC } from "constants/ThirdPartyConstants"; @@ -105,7 +108,7 @@ export const APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING: Setting = { label: "Limit embedding to certain URLs", value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, nodeLabel: "You can add one or more URLs", - node: , + node: , nodeInputPath: "input", nodeParentClass: "tag-input", }, @@ -124,10 +127,19 @@ export const APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING: Setting = { ? localStorage.getItem("ALLOWED_FRAME_ANCESTORS") ?? "" : value.additionalData.replaceAll(",", " "); - // If they are one of the other options we don't store it in storage since it will - // set in the env variable on save - if (sources !== "*" && sources !== "'none'") { - localStorage.setItem("ALLOWED_FRAME_ANCESTORS", sources); + // Sanitize the limited list: drop any bare "*" and normalize the disable + // sentinel "'none'" to empty. A stale localStorage value from before allow-all + // detection existed could still hold a "*" (or "* 'none'"), which would + // otherwise round-trip into the stored value and silently reopen embedding or + // emit the disable sentinel from LIMIT mode. Host wildcards are preserved. + const limitedSources = sanitizeLimitedFrameAncestors(sources); + + // Remember only real limited sources; never persist an allow-all/disabled + // value, and clear any stale entry left in storage. + if (limitedSources) { + localStorage.setItem("ALLOWED_FRAME_ANCESTORS", limitedSources); + } else { + localStorage.removeItem("ALLOWED_FRAME_ANCESTORS"); } if ( @@ -139,7 +151,7 @@ export const APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING: Setting = { ) { return "'none'"; } else { - return sources; + return limitedSources; } }, validate: (value: string) => { diff --git a/app/client/src/pages/Applications/EmbedSnippet/FrameAncestorsTagInput.tsx b/app/client/src/pages/Applications/EmbedSnippet/FrameAncestorsTagInput.tsx new file mode 100644 index 000000000000..2d6f5da9a9ef --- /dev/null +++ b/app/client/src/pages/Applications/EmbedSnippet/FrameAncestorsTagInput.tsx @@ -0,0 +1,98 @@ +import React, { useCallback, useMemo, useState } from "react"; +import styled from "styled-components"; +import { TagInput } from "@appsmith/ads-old"; +import { Text } from "@appsmith/ads"; +import { createMessage, IN_APP_EMBED_SETTING } from "ee/constants/messages"; +import { + normalizeFrameAncestorToken, + removeAllowAllFrameAncestorChips, + removeDisableFrameAncestorChips, +} from "./Utils/utils"; + +// Quote bare "self"/"none" keywords in each comma-separated chip so the chip the +// user sees matches what is stored. Chips can hold whitespace-separated tokens +// (pasted values), so normalize per token. +const normalizeChips = (value: string): string => + value + .split(",") + .filter(Boolean) + .map((chip) => + chip + .split(/\s+/) + .filter(Boolean) + .map(normalizeFrameAncestorToken) + .join(" "), + ) + .join(","); + +const ErrorText = styled(Text)` + display: block; + margin-top: 4px; + color: var(--ads-v2-color-fg-error); +`; + +interface FrameAncestorsTagInputProps { + // Injected by the radio field via nodeInputPath. Values are comma-separated. + input?: { + value?: string; + onChange?: (value: string) => void; + }; + placeholder: string; + type: string; +} + +// Wraps the shared TagInput for the "Limit embedding to certain URLs" list and +// rejects any chip containing a keyword source that contradicts the "limit" +// intent: a bare "*" (belongs to "Allow embedding everywhere") or a +// "none"/"'none'" (belongs to "Disable embedding everywhere" - in CSP "'none'" +// is exclusive and would silently disable embedding if combined with URLs). Both +// checks are whitespace-aware because a pasted value can arrive as a single chip +// (e.g. "'self' *" or "none https://a.com"). Host wildcards like +// "https://*.example.com" are left untouched. +function FrameAncestorsTagInput(props: FrameAncestorsTagInputProps) { + const { input = {}, ...rest } = props; + const [error, setError] = useState(""); + const { onChange, value } = input; + + const handleChange = useCallback( + (nextValue: string) => { + const withoutWildcard = removeAllowAllFrameAncestorChips(nextValue); + const withoutDisable = removeDisableFrameAncestorChips( + withoutWildcard.value, + ); + + if (withoutWildcard.removed) { + setError( + createMessage(IN_APP_EMBED_SETTING.limitEmbeddingBareWildcardError), + ); + } else if (withoutDisable.removed) { + setError( + createMessage(IN_APP_EMBED_SETTING.limitEmbeddingDisableKeywordError), + ); + } else { + setError(""); + } + + onChange?.(normalizeChips(withoutDisable.value)); + }, + [onChange], + ); + + const tagInputProps = useMemo( + () => ({ value, onChange: handleChange }), + [value, handleChange], + ); + + return ( + <> + + {error && ( + + {error} + + )} + + ); +} + +export default FrameAncestorsTagInput; diff --git a/app/client/src/pages/Applications/EmbedSnippet/Utils/utils.test.ts b/app/client/src/pages/Applications/EmbedSnippet/Utils/utils.test.ts new file mode 100644 index 000000000000..965a8167a112 --- /dev/null +++ b/app/client/src/pages/Applications/EmbedSnippet/Utils/utils.test.ts @@ -0,0 +1,363 @@ +import { + containsAllowAllFrameAncestor, + containsDisableFrameAncestor, + formatEmbedSettings, + isAllowAllFrameAncestorToken, + isDisableFrameAncestorToken, + normalizeFrameAncestorToken, + removeAllowAllFrameAncestorChips, + removeDisableFrameAncestorChips, + sanitizeLimitedFrameAncestors, + stripAllowAllFrameAncestorTokens, +} from "./utils"; +import { AppsmithFrameAncestorsSetting } from "../Constants/constants"; + +describe("isAllowAllFrameAncestorToken", () => { + it("matches a bare *", () => { + expect(isAllowAllFrameAncestorToken("*")).toBe(true); + expect(isAllowAllFrameAncestorToken(" * ")).toBe(true); + }); + + it("does not match host wildcards or other sources", () => { + expect(isAllowAllFrameAncestorToken("https://*.example.com")).toBe(false); + expect(isAllowAllFrameAncestorToken("'self'")).toBe(false); + expect(isAllowAllFrameAncestorToken("")).toBe(false); + }); + + it("tolerates a null/undefined token", () => { + expect(isAllowAllFrameAncestorToken(undefined as unknown as string)).toBe( + false, + ); + expect(isAllowAllFrameAncestorToken(null as unknown as string)).toBe(false); + }); +}); + +describe("containsAllowAllFrameAncestor", () => { + it("detects a standalone * anywhere in the list", () => { + expect(containsAllowAllFrameAncestor("*")).toBe(true); + expect(containsAllowAllFrameAncestor("'self' *")).toBe(true); + expect(containsAllowAllFrameAncestor("* 'self'")).toBe(true); + }); + + it("does not treat host wildcards as allow-everywhere", () => { + expect(containsAllowAllFrameAncestor("https://*.example.com")).toBe(false); + expect(containsAllowAllFrameAncestor("'self' https://*.example.com")).toBe( + false, + ); + }); + + it("is false for other sources and empty values", () => { + expect(containsAllowAllFrameAncestor("'self'")).toBe(false); + expect(containsAllowAllFrameAncestor("'none'")).toBe(false); + expect(containsAllowAllFrameAncestor("")).toBe(false); + expect(containsAllowAllFrameAncestor(" ")).toBe(false); + }); + + it("tolerates a null/undefined value without throwing", () => { + expect(containsAllowAllFrameAncestor(undefined as unknown as string)).toBe( + false, + ); + expect(containsAllowAllFrameAncestor(null as unknown as string)).toBe( + false, + ); + }); +}); + +describe("formatEmbedSettings", () => { + it("maps an exact * to allow-embedding-everywhere", () => { + expect(formatEmbedSettings("*")).toEqual({ + value: AppsmithFrameAncestorsSetting.ALLOW_EMBEDDING_EVERYWHERE, + }); + }); + + it("maps a value containing a bare * to allow-embedding-everywhere", () => { + // "*" overrides every other source in a CSP frame-ancestors policy, so the + // effective policy is allow-everywhere - show the truthful radio. + expect(formatEmbedSettings("'self' *")).toEqual({ + value: AppsmithFrameAncestorsSetting.ALLOW_EMBEDDING_EVERYWHERE, + }); + expect(formatEmbedSettings("* 'self'")).toEqual({ + value: AppsmithFrameAncestorsSetting.ALLOW_EMBEDDING_EVERYWHERE, + }); + }); + + it("maps 'none' to disable-embedding-everywhere", () => { + expect(formatEmbedSettings("'none'")).toEqual({ + value: AppsmithFrameAncestorsSetting.DISABLE_EMBEDDING_EVERYWHERE, + }); + }); + + it("maps a single source to limit-embedding", () => { + expect(formatEmbedSettings("'self'")).toEqual({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "'self'", + }); + }); + + it("maps multiple sources to limit-embedding with comma-separated chips", () => { + expect(formatEmbedSettings("'self' https://a.com")).toEqual({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "'self',https://a.com", + }); + }); + + it("keeps a host wildcard as a limit-embedding entry", () => { + // "https://*.example.com" only matches subdomains of example.com; it is a + // legitimate limit-list source and must NOT be read as allow-everywhere. + expect(formatEmbedSettings("https://*.example.com")).toEqual({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "https://*.example.com", + }); + }); + + it("maps an empty value to an empty limit-embedding list", () => { + expect(formatEmbedSettings("")).toEqual({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "", + }); + }); + + it("tolerates an unhydrated (undefined/null) value without throwing", () => { + // On a cold load of /settings/configuration the admin-settings store is not + // yet populated, so redux-form calls format() with undefined. This must not + // crash the settings page - it should behave like an empty limit list. + expect(formatEmbedSettings(undefined as unknown as string)).toEqual({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "", + }); + expect(formatEmbedSettings(null as unknown as string)).toEqual({ + value: AppsmithFrameAncestorsSetting.LIMIT_EMBEDDING, + additionalData: "", + }); + }); +}); + +describe("removeAllowAllFrameAncestorChips", () => { + it("drops a bare * committed as its own chip", () => { + expect(removeAllowAllFrameAncestorChips("*")).toEqual({ + value: "", + removed: true, + }); + expect(removeAllowAllFrameAncestorChips("'self',*")).toEqual({ + value: "'self'", + removed: true, + }); + }); + + it("drops a pasted single chip that contains a bare *", () => { + // A pasted value has no comma separator, so the whole "'self' *" arrives as + // one chip. The guard must still catch the bare "*" inside it. + expect(removeAllowAllFrameAncestorChips("'self' *")).toEqual({ + value: "", + removed: true, + }); + expect(removeAllowAllFrameAncestorChips("* https://a.com")).toEqual({ + value: "", + removed: true, + }); + }); + + it("keeps host wildcards and regular URLs", () => { + expect(removeAllowAllFrameAncestorChips("https://*.example.com")).toEqual({ + value: "https://*.example.com", + removed: false, + }); + expect(removeAllowAllFrameAncestorChips("'self',https://a.com")).toEqual({ + value: "'self',https://a.com", + removed: false, + }); + }); + + it("returns an empty result for an empty value", () => { + expect(removeAllowAllFrameAncestorChips("")).toEqual({ + value: "", + removed: false, + }); + }); +}); + +describe("stripAllowAllFrameAncestorTokens", () => { + it("removes bare * tokens while keeping other sources", () => { + expect(stripAllowAllFrameAncestorTokens("'self' *")).toBe("'self'"); + expect(stripAllowAllFrameAncestorTokens("* 'self' https://a.com")).toBe( + "'self' https://a.com", + ); + }); + + it("returns an empty string when only a bare * is present", () => { + expect(stripAllowAllFrameAncestorTokens("*")).toBe(""); + expect(stripAllowAllFrameAncestorTokens("")).toBe(""); + }); + + it("tolerates a null/undefined value", () => { + expect( + stripAllowAllFrameAncestorTokens(undefined as unknown as string), + ).toBe(""); + expect(stripAllowAllFrameAncestorTokens(null as unknown as string)).toBe( + "", + ); + }); + + it("preserves host wildcards and normal sources unchanged", () => { + expect(stripAllowAllFrameAncestorTokens("https://*.example.com")).toBe( + "https://*.example.com", + ); + expect(stripAllowAllFrameAncestorTokens("'self' https://a.com")).toBe( + "'self' https://a.com", + ); + }); +}); + +describe("sanitizeLimitedFrameAncestors", () => { + it("normalizes the disable sentinel to empty so LIMIT mode never emits it", () => { + // Stripping "*" from "* 'none'" would leave "'none'"; LIMIT mode must not + // return the disable-everywhere sentinel. + expect(sanitizeLimitedFrameAncestors("* 'none'")).toBe(""); + expect(sanitizeLimitedFrameAncestors("'none'")).toBe(""); + }); + + it("drops bare * tokens like the underlying strip", () => { + expect(sanitizeLimitedFrameAncestors("*")).toBe(""); + expect(sanitizeLimitedFrameAncestors("* 'self'")).toBe("'self'"); + expect(sanitizeLimitedFrameAncestors("")).toBe(""); + }); + + it("preserves host wildcards and normal sources", () => { + expect(sanitizeLimitedFrameAncestors("https://*.example.com")).toBe( + "https://*.example.com", + ); + expect(sanitizeLimitedFrameAncestors("'self' https://a.com")).toBe( + "'self' https://a.com", + ); + }); + + it("quotes bare self/none keywords so they are never persisted raw", () => { + // A bare "self" is parsed as a hostname and breaks the CSP policy, so it must + // be stored as the quoted "'self'" keyword. + expect(sanitizeLimitedFrameAncestors("self")).toBe("'self'"); + expect(sanitizeLimitedFrameAncestors("self https://a.com")).toBe( + "'self' https://a.com", + ); + expect(sanitizeLimitedFrameAncestors("SELF *")).toBe("'self'"); + // A lone bare "none" normalizes to the disable sentinel, which LIMIT mode + // drops to empty rather than persisting. + expect(sanitizeLimitedFrameAncestors("none")).toBe(""); + }); + + it("strips a none/'none' token combined with allow-list sources", () => { + // In CSP "'none'" is exclusive, so "none https://a.com" must never persist as + // "'none' https://a.com" (which would silently disable embedding). The + // "'none'" is stripped so the allow-list saves as intended. + expect(sanitizeLimitedFrameAncestors("none https://a.com")).toBe( + "https://a.com", + ); + expect(sanitizeLimitedFrameAncestors("'none' https://a.com 'self'")).toBe( + "https://a.com 'self'", + ); + expect(sanitizeLimitedFrameAncestors("* 'none'")).toBe(""); + // Case-insensitive quoted keyword must be stripped too. + expect(sanitizeLimitedFrameAncestors("'NONE' https://a.com")).toBe( + "https://a.com", + ); + }); +}); + +describe("isDisableFrameAncestorToken", () => { + it("matches bare and quoted none (case-insensitive)", () => { + expect(isDisableFrameAncestorToken("none")).toBe(true); + expect(isDisableFrameAncestorToken("NONE")).toBe(true); + expect(isDisableFrameAncestorToken("'none'")).toBe(true); + expect(isDisableFrameAncestorToken("'NONE'")).toBe(true); + }); + + it("does not match other sources", () => { + expect(isDisableFrameAncestorToken("'self'")).toBe(false); + expect(isDisableFrameAncestorToken("https://a.com")).toBe(false); + expect(isDisableFrameAncestorToken("*")).toBe(false); + expect(isDisableFrameAncestorToken("")).toBe(false); + }); +}); + +describe("containsDisableFrameAncestor", () => { + it("detects a none/'none' token anywhere in the list", () => { + expect(containsDisableFrameAncestor("none")).toBe(true); + expect(containsDisableFrameAncestor("none https://a.com")).toBe(true); + expect(containsDisableFrameAncestor("'self' 'none'")).toBe(true); + }); + + it("is false for lists without a disable keyword", () => { + expect(containsDisableFrameAncestor("'self' https://a.com")).toBe(false); + expect(containsDisableFrameAncestor("https://*.example.com")).toBe(false); + expect(containsDisableFrameAncestor("")).toBe(false); + expect(containsDisableFrameAncestor(undefined as unknown as string)).toBe( + false, + ); + }); +}); + +describe("removeDisableFrameAncestorChips", () => { + it("drops a none chip committed on its own", () => { + expect(removeDisableFrameAncestorChips("none")).toEqual({ + value: "", + removed: true, + }); + expect(removeDisableFrameAncestorChips("'self',none")).toEqual({ + value: "'self'", + removed: true, + }); + }); + + it("drops a pasted single chip that contains a disable keyword", () => { + // Pasting "none https://a.com" arrives as one chip; it must be rejected so + // "'none'" is never combined with allow-list sources. + expect(removeDisableFrameAncestorChips("none https://a.com")).toEqual({ + value: "", + removed: true, + }); + expect(removeDisableFrameAncestorChips("'none' https://a.com")).toEqual({ + value: "", + removed: true, + }); + // Quoted-uppercase keyword is still a case-insensitive CSP "'none'". + expect(removeDisableFrameAncestorChips("'NONE' https://a.com")).toEqual({ + value: "", + removed: true, + }); + }); + + it("keeps allow-list sources untouched", () => { + expect(removeDisableFrameAncestorChips("'self',https://a.com")).toEqual({ + value: "'self',https://a.com", + removed: false, + }); + expect(removeDisableFrameAncestorChips("")).toEqual({ + value: "", + removed: false, + }); + }); +}); + +describe("normalizeFrameAncestorToken", () => { + it("quotes the bare self/none keywords (case-insensitive)", () => { + expect(normalizeFrameAncestorToken("self")).toBe("'self'"); + expect(normalizeFrameAncestorToken(" SELF ")).toBe("'self'"); + expect(normalizeFrameAncestorToken("none")).toBe("'none'"); + expect(normalizeFrameAncestorToken("None")).toBe("'none'"); + }); + + it("canonicalizes already-quoted keywords, including uppercase", () => { + expect(normalizeFrameAncestorToken("'self'")).toBe("'self'"); + expect(normalizeFrameAncestorToken("'none'")).toBe("'none'"); + expect(normalizeFrameAncestorToken("'SELF'")).toBe("'self'"); + expect(normalizeFrameAncestorToken("'NONE'")).toBe("'none'"); + }); + + it("leaves host/scheme sources and wildcards unquoted", () => { + expect(normalizeFrameAncestorToken("https://a.com")).toBe("https://a.com"); + expect(normalizeFrameAncestorToken("https://*.example.com")).toBe( + "https://*.example.com", + ); + expect(normalizeFrameAncestorToken("*")).toBe("*"); + expect(normalizeFrameAncestorToken("")).toBe(""); + }); +}); diff --git a/app/client/src/pages/Applications/EmbedSnippet/Utils/utils.ts b/app/client/src/pages/Applications/EmbedSnippet/Utils/utils.ts index 32cf74a06d88..399507e43f69 100644 --- a/app/client/src/pages/Applications/EmbedSnippet/Utils/utils.ts +++ b/app/client/src/pages/Applications/EmbedSnippet/Utils/utils.ts @@ -1,7 +1,130 @@ import { AppsmithFrameAncestorsSetting } from "../Constants/constants"; +// A bare "*" token is a wildcard source in a CSP frame-ancestors policy. When +// present it matches every origin and overrides every other source in the list, +// so the effective policy is "allow embedding everywhere" regardless of the +// other tokens. This is different from a host wildcard like +// "https://*.example.com", which only matches subdomains of a specific host and +// is a legitimate limit-list entry. +export const isAllowAllFrameAncestorToken = (token: string): boolean => + (token ?? "").trim() === "*"; + +// True when the stored frame-ancestors value contains a standalone "*" token. +// Split on whitespace so we match a bare "*" but not a "*" embedded in a host +// pattern such as "https://*.example.com". Tolerates a null/undefined value: +// on a cold load of /settings/configuration the admin-settings store is not yet +// hydrated, so this runs before the value exists. +export const containsAllowAllFrameAncestor = (value: string): boolean => { + const trimmed = (value ?? "").trim(); + + return ( + trimmed.length > 0 && + trimmed.split(/\s+/).some(isAllowAllFrameAncestorToken) + ); +}; + +// Filter the comma-separated limit list emitted by the TagInput, dropping any +// chip that contains a bare "*". Pasted content can arrive as a single chip +// holding whitespace-separated tokens (e.g. "'self' *"), so each chip is checked +// with the whitespace-aware helper rather than a strict token match - otherwise +// a bare "*" inside a pasted chip would slip into the list and silently reopen +// embedding to every origin. A host wildcard like "https://*.example.com" is +// kept. `removed` reports whether anything was dropped so the caller can surface +// an inline message. +export const removeAllowAllFrameAncestorChips = ( + value: string, +): { value: string; removed: boolean } => { + const chips = value ? value.split(",") : []; + const accepted = chips.filter((chip) => !containsAllowAllFrameAncestor(chip)); + + return { + value: accepted.join(","), + removed: accepted.length !== chips.length, + }; +}; + +// Remove any bare "*" tokens from a whitespace-separated frame-ancestors value, +// keeping legitimate sources (including host wildcards like +// "https://*.example.com"). Used to sanitize a limited-embedding list so a stale +// allow-all "*" - e.g. one left in localStorage before allow-all detection +// existed - can never round-trip back into the stored value. +export const stripAllowAllFrameAncestorTokens = (value: string): string => + (value ?? "") + .trim() + .split(/\s+/) + .filter((token) => token.length > 0 && !isAllowAllFrameAncestorToken(token)) + .join(" "); + +// Quote the CSP keyword sources. In a frame-ancestors policy "'self'" and +// "'none'" must be single-quoted; a bare "self"/"none" is parsed by the browser +// as a hostname and silently breaks the policy. Normalize them to the quoted +// form (case-insensitive). Host/scheme sources and wildcards +// ("https://x.com", "*.example.com", "*") must stay unquoted and are left as-is. +export const normalizeFrameAncestorToken = (token: string): string => { + const trimmed = (token ?? "").trim(); + // CSP keywords are case-insensitive, and users may type them already quoted, so + // canonicalize both the bare and quoted forms (e.g. "NONE" and "'NONE'"). + const lower = trimmed.toLowerCase(); + + if (lower === "self" || lower === "'self'") return "'self'"; + + if (lower === "none" || lower === "'none'") return "'none'"; + + return trimmed; +}; + +// True when a single token is the disable-everywhere keyword ("none"/"'none'", +// case-insensitive). +export const isDisableFrameAncestorToken = (token: string): boolean => + normalizeFrameAncestorToken(token) === "'none'"; + +// True when a value contains a disable keyword token. In a CSP frame-ancestors +// policy "'none'" is exclusive: alongside other sources it disables embedding +// entirely, so it must never sit in a limit list. +export const containsDisableFrameAncestor = (value: string): boolean => { + const trimmed = (value ?? "").trim(); + + return ( + trimmed.length > 0 && trimmed.split(/\s+/).some(isDisableFrameAncestorToken) + ); +}; + +// Filter the comma-separated limit list emitted by the TagInput, dropping any +// chip that contains a disable keyword ("none"/"'none'"). Mirrors +// removeAllowAllFrameAncestorChips; the caller steers the admin to the "Disable +// embedding everywhere" radio. `removed` reports whether anything was dropped. +export const removeDisableFrameAncestorChips = ( + value: string, +): { value: string; removed: boolean } => { + const chips = value ? value.split(",") : []; + const accepted = chips.filter((chip) => !containsDisableFrameAncestor(chip)); + + return { + value: accepted.join(","), + removed: accepted.length !== chips.length, + }; +}; + +// Sanitize a whitespace-separated value for use as a limited-embedding list: +// drop bare "*" tokens, quote bare "self"/"none" keywords, and strip every +// "'none'" token. Neither "*" nor "'none'" is a valid limit-list source, and +// because "'none'" is exclusive in CSP a value like "none https://a.com" must +// not persist as "'none' https://a.com" (which would silently disable embedding); +// the "'none'" is removed so the allow-list saves as intended. Host wildcards +// such as "https://*.example.com" are preserved. +export const sanitizeLimitedFrameAncestors = (value: string): string => + stripAllowAllFrameAncestorTokens(value) + .split(/\s+/) + .filter((token) => token.length > 0) + .map(normalizeFrameAncestorToken) + .filter((token) => token !== "'none'") + .join(" "); + export const formatEmbedSettings = (value: string) => { - if (value === "*") { + // A value containing a bare "*" is effectively allow-everywhere, even when it + // also lists other sources (e.g. "'self' *"). Show the truthful radio rather + // than a contradictory "limit" state. + if (containsAllowAllFrameAncestor(value)) { return { value: AppsmithFrameAncestorsSetting.ALLOW_EMBEDDING_EVERYWHERE, };