Skip to content
4 changes: 4 additions & 0 deletions app/client/src/ce/constants/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
102 changes: 102 additions & 0 deletions app/client/src/ce/pages/AdminSettings/config/configuration.test.tsx
Original file line number Diff line number Diff line change
@@ -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 <Radio setting={APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING} />;
}

// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const Parent = reduxForm<any, any>({
validate: () => ({}),
form: SETTINGS_FORM_NAME,
touchOnBlur: true,
})(FrameAncestorsRadio);

return render(<Parent />, {
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<HTMLInputElement>("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'");
});
});
28 changes: 20 additions & 8 deletions app/client/src/ce/pages/AdminSettings/config/configuration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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: <TagInput input={{}} placeholder={""} type={"text"} />,
node: <FrameAncestorsTagInput placeholder={""} type={"text"} />,
nodeInputPath: "input",
nodeParentClass: "tag-input",
},
Expand All @@ -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 (
Expand All @@ -139,7 +151,7 @@ export const APPSMITH_ALLOWED_FRAME_ANCESTORS_SETTING: Setting = {
) {
return "'none'";
} else {
return sources;
return limitedSources;
}
},
validate: (value: string) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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 (
<>
<TagInput {...rest} input={tagInputProps} />
{error && (
<ErrorText kind="body-s" renderAs="span">
{error}
</ErrorText>
)}
</>
);
}

export default FrameAncestorsTagInput;
Loading
Loading