Skip to content
Snippets Groups Projects
Unverified Commit 6e530e28 authored by Jesse Devaney's avatar Jesse Devaney Committed by GitHub
Browse files

Create Admin Setting for MB_SESSION_COOKIE_SAMESITE (#35392)

* initial frontend commit

* add authorized origins check with note

* update interactive embedding unit test

* add samesite cookie to embedding E2E

* add unit tests for conditional note display

* fix types

* fix translations

* use alert instead of custom component
parent 63f43a6e
Branches
Tags
No related merge requests found
Showing
with 257 additions and 2 deletions
......@@ -184,6 +184,12 @@ describe("scenarios > embedding > smoke tests", { tags: "@OSS" }, () => {
"Enter the origins for the websites or web apps where you want to allow embedding, separated by a space. Here are the exact specifications for what can be entered.",
).should("not.exist");
cy.findByPlaceholderText("https://*.example.com").should("not.exist");
cy.findByTestId("session-cookie-samesite-setting").should("not.exist");
cy.contains(
"Determines whether or not cookies are allowed to be sent on cross-site requests. You’ll likely need to change this to None if your embedding application is hosted under a different domain than Metabase. Otherwise, leave it set to Lax, as it's more secure.",
).should("not.exist");
cy.findByDisplayValue("Lax (default)").should("not.exist");
});
});
......
import styled from "@emotion/styled";
import Alert from "metabase/core/components/Alert";
export const SameSiteAlert = styled(Alert)`
display: block;
padding: 0.75rem;
`;
import { jt, t } from "ttag";
import { useSelector } from "metabase/lib/redux";
import { getDocsUrl, getSetting } from "metabase/selectors/settings";
import { Box, Center, Stack, Text } from "metabase/ui";
import ExternalLink from "metabase/core/components/ExternalLink";
import { isEmpty } from "metabase/lib/utils";
import { isSameOrigin } from "metabase/lib/dom";
import { SameSiteAlert } from "./EmbeddingAppSameSiteCookieDescription.styled";
export const EmbeddingAppSameSiteCookieDescription = () => {
const docsUrl = useSelector(state =>
getDocsUrl(state, {
page: "embedding/interactive-embedding#embedding-metabase-in-a-different-domain",
}),
);
const embeddingSameSiteCookieSetting = useSelector(state =>
getSetting(state, "session-cookie-samesite"),
);
const embeddingAuthorizedOrigins = useSelector(state =>
getSetting(state, "embedding-app-origin"),
);
const shouldDisplayNote =
embeddingSameSiteCookieSetting !== "none" &&
authorizedOriginsContainsNonInstanceDomain(embeddingAuthorizedOrigins);
return (
<Stack mb="1rem" spacing="sm">
<Text fw="bold">{t`SameSite cookie setting`}</Text>
{shouldDisplayNote && <AuthorizedOriginsNote />}
<Text>{t`Determines whether or not cookies are allowed to be sent on cross-site requests. You’ll likely need to change this to None if your embedding application is hosted under a different domain than Metabase. Otherwise, leave it set to Lax, as it's more secure.`}</Text>
<Text>{jt`If you set this to None, you'll have to use HTTPS (unless you're just embedding locally), or browsers will reject the request. ${(
<ExternalLink key="learn-more" href={docsUrl}>
{t`Learn more`}
</ExternalLink>
)}`}</Text>
</Stack>
);
};
function AuthorizedOriginsNote() {
return (
<Box data-testid="authorized-origins-note" w="22rem">
<SameSiteAlert variant="warning" hasBorder>
<Center>
<Text>{jt`You should probably change this setting to ${(
<Text span fw="bold">
{t`None`}
</Text>
)}.`}</Text>
</Center>
</SameSiteAlert>
</Box>
);
}
function authorizedOriginsContainsNonInstanceDomain(
authorizedOriginsString: string,
): boolean {
if (isEmpty(authorizedOriginsString)) {
return false;
}
const origins = authorizedOriginsString.split(" ");
return origins.some(origin => !isSameOrigin(origin));
}
import { useState } from "react";
import { Button, Group, Menu, Text } from "metabase/ui";
import { color } from "metabase/lib/colors";
import { Icon } from "metabase/core/components/Icon";
import type { SessionCookieSameSite } from "metabase-types/api";
interface Options {
value: SessionCookieSameSite;
name: string;
description: string;
}
interface SameSiteSelectWidgetProps {
onChange: (value: SessionCookieSameSite) => void;
setting: {
key: "session-cookie-samesite";
value?: SessionCookieSameSite;
defaultValue: SessionCookieSameSite;
options: Options[];
};
}
export function SameSiteSelectWidget({
setting,
onChange,
}: SameSiteSelectWidgetProps) {
const [opened, setOpened] = useState(false);
const selectedValue = setting.value ?? setting.defaultValue;
const selectedOption = setting.options.find(
({ value }) => value === selectedValue,
);
return (
<Menu
opened={opened}
onChange={setOpened}
position="bottom-start"
shadow="sm"
>
<Menu.Target>
<Button variant={opened ? "outline" : "default"}>
<Group position="apart" miw="10rem">
<span>{selectedOption?.name}</span>
<Icon name="chevrondown" size="12" />
</Group>
</Button>
</Menu.Target>
<Menu.Dropdown maw={"21rem"}>
{setting.options.map(({ value, name, description }) => (
<Menu.Item key="value" onClick={() => onChange(value)}>
<Text>{name}</Text>
<Text c={color("text-light")}>{description}</Text>
</Menu.Item>
))}
</Menu.Dropdown>
</Menu>
);
}
export { EmbeddingAppSameSiteCookieDescription } from "./EmbeddingAppSameSiteCookieDescription";
export { SameSiteSelectWidget } from "./SameSiteSelectWidget";
......@@ -5,6 +5,10 @@ import {
PLUGIN_EMBEDDING,
} from "metabase/plugins";
import { EmbeddingAppOriginDescription } from "./components/EmbeddingAppOriginDescription";
import {
EmbeddingAppSameSiteCookieDescription,
SameSiteSelectWidget,
} from "./components/EmbeddingAppSameSiteCookieDescription";
const SLUG = "embedding-in-other-applications/full-app";
......@@ -27,6 +31,32 @@ if (hasPremiumFeature("embedding")) {
getHidden: (_, derivedSettings) =>
!derivedSettings["enable-embedding"],
},
{
key: "session-cookie-samesite",
description: <EmbeddingAppSameSiteCookieDescription />,
type: "select",
options: [
{
value: "lax",
name: t`Lax (default)`,
description: t`Allows cookies to be sent when a user is navigating to the origin site from an external site (like when following a link).`,
},
{
value: "strict",
name: t`Strict (not recommended)`,
description: t`Never allows cookies to be sent on a cross-site request. Warning: this will prevent users from following external links to Metabase.`,
},
{
value: "none",
name: t`None`,
description: t`Allows all cross-site requests. Incompatible with most Safari and iOS-based browsers.`,
},
],
defaultValue: "lax",
widget: SameSiteSelectWidget,
getHidden: (_, derivedSettings) =>
!derivedSettings["enable-embedding"],
},
],
},
};
......
......@@ -144,6 +144,7 @@ export const createMockSettings = (opts?: Partial<Settings>): Settings => ({
"custom-homepage-dashboard": null,
"deprecation-notice-version": undefined,
"email-configured?": false,
"embedding-app-origin": "",
"enable-embedding": false,
"enable-enhancements?": false,
"enable-nested-queries": true,
......@@ -185,6 +186,7 @@ export const createMockSettings = (opts?: Partial<Settings>): Settings => ({
"search-typeahead-enabled": true,
"setup-token": null,
"session-cookies": null,
"session-cookie-samesite": "lax",
"snowplow-enabled": false,
"show-database-syncing-modal": false,
"show-homepage-data": false,
......
......@@ -172,6 +172,8 @@ export type PasswordComplexity = {
digit?: number;
};
export type SessionCookieSameSite = "lax" | "strict" | "none";
export interface SettingDefinition {
key: string;
env_name?: string;
......@@ -201,6 +203,7 @@ export interface Settings {
"deprecation-notice-version"?: string;
"dismissed-custom-dashboard-toast"?: boolean;
"email-configured?": boolean;
"embedding-app-origin": string;
"embedding-secret-key"?: string;
"enable-embedding": boolean;
"enable-enhancements?": boolean;
......@@ -242,6 +245,7 @@ export interface Settings {
"search-typeahead-enabled": boolean;
"setup-token": string | null;
"session-cookies": boolean | null;
"session-cookie-samesite": SessionCookieSameSite;
"snowplow-enabled": boolean;
"snowplow-url": string;
"show-database-syncing-modal": boolean;
......
......@@ -19,6 +19,9 @@ describe("SettingsEditor", () => {
userEvent.click(screen.getByText("Interactive embedding"));
expect(screen.getByText(/some of our paid plans/)).toBeInTheDocument();
expect(screen.queryByText("Authorized origins")).not.toBeInTheDocument();
expect(
screen.queryByText("SameSite cookie setting"),
).not.toBeInTheDocument();
});
it("should redirect from the full-app embedding page if embedding is not enabled", async () => {
......
......@@ -19,23 +19,94 @@ const setupEmbedding = async (opts?: SetupOpts) => {
};
describe("SettingsEditor", () => {
it("should allow to configure the origin for interactive embedding", async () => {
it("should allow to configure the origin and SameSite cookie setting for interactive embedding", async () => {
await setupEmbedding({
settings: [
createMockSettingDefinition({ key: "enable-embedding" }),
createMockSettingDefinition({ key: "embedding-app-origin" }),
createMockSettingDefinition({ key: "session-cookie-samesite" }),
],
settingValues: createMockSettings({
"enable-embedding": true,
"session-cookie-samesite": "lax",
}),
});
userEvent.click(screen.getByText("Embedding"));
userEvent.click(screen.getByText("Interactive embedding"));
expect(screen.getByText("Interactive embedding")).toBeInTheDocument();
expect(screen.getByText("Authorized origins")).toBeInTheDocument();
expect(screen.getByText("SameSite cookie setting")).toBeInTheDocument();
expect(
screen.queryByText(/some of our paid plans/),
).not.toBeInTheDocument();
});
describe("SameSite cookie note check with authorized origins", () => {
it("should display a note if any authorized origins do not match the instance domain", async () => {
await setupEmbedding({
settings: [
createMockSettingDefinition({ key: "enable-embedding" }),
createMockSettingDefinition({ key: "embedding-app-origin" }),
createMockSettingDefinition({ key: "session-cookie-samesite" }),
],
settingValues: createMockSettings({
"embedding-app-origin": "https://example.com",
"enable-embedding": true,
"session-cookie-samesite": "lax",
}),
});
userEvent.click(screen.getByText("Embedding"));
userEvent.click(screen.getByText("Interactive embedding"));
expect(screen.getByTestId("authorized-origins-note")).toBeInTheDocument();
});
it("should not display a note if all authorized origins match the instance domain", async () => {
await setupEmbedding({
settings: [
createMockSettingDefinition({ key: "enable-embedding" }),
createMockSettingDefinition({ key: "embedding-app-origin" }),
createMockSettingDefinition({ key: "session-cookie-samesite" }),
],
settingValues: createMockSettings({
"embedding-app-origin": "",
"enable-embedding": true,
"session-cookie-samesite": "lax",
}),
});
userEvent.click(screen.getByText("Embedding"));
userEvent.click(screen.getByText("Interactive embedding"));
expect(
screen.queryByTestId("authorized-origins-note"),
).not.toBeInTheDocument();
});
it("should not display a note if SameSite cookie is set to 'none'", async () => {
await setupEmbedding({
settings: [
createMockSettingDefinition({ key: "enable-embedding" }),
createMockSettingDefinition({ key: "embedding-app-origin" }),
createMockSettingDefinition({ key: "session-cookie-samesite" }),
],
settingValues: createMockSettings({
"embedding-app-origin": "https://example.com",
"enable-embedding": true,
"session-cookie-samesite": "none",
}),
});
userEvent.click(screen.getByText("Embedding"));
userEvent.click(screen.getByText("Interactive embedding"));
expect(
screen.queryByTestId("authorized-origins-note"),
).not.toBeInTheDocument();
});
});
});
......@@ -16,7 +16,7 @@ const setupEnterprise = async (opts?: SetupOpts) => {
};
describe("SettingsEditor", () => {
it("should not allow to configure the origin for interactive embedding", async () => {
it("should not allow to configure the origin and SameSite cookie for interactive embedding", async () => {
await setupEnterprise({
settings: [
createMockSettingDefinition({ key: "enable-embedding" }),
......@@ -29,6 +29,9 @@ describe("SettingsEditor", () => {
userEvent.click(screen.getByText("Interactive embedding"));
expect(screen.getByText(/some of our paid plans/)).toBeInTheDocument();
expect(screen.queryByText("Authorized origins")).not.toBeInTheDocument();
expect(
screen.queryByText("SameSite cookie setting"),
).not.toBeInTheDocument();
});
it("should not allow to toggle off password login", async () => {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment