From f7cc66891fca1f187f9bd235ae5721765eaf7044 Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Thu, 24 Aug 2023 10:22:24 -0400 Subject: [PATCH 1/8] initial commit to make oembedallowedorigins configurable --- .../Advanced/AdvancedConfigContainer.tsx | 3 ++ .../OEmbedAllowedOriginsConfigContainer.tsx | 47 +++++++++++++++++++ .../sections/Sites/AllowedOriginsTextarea.tsx | 8 +++- .../sections/Sites/CreateSiteForm.tsx | 2 +- .../Configure/sections/Sites/EditSiteForm.tsx | 5 +- src/core/server/graph/resolvers/Settings.ts | 1 + src/core/server/graph/schema/schema.graphql | 11 ++++- src/core/server/models/settings/settings.ts | 1 + 8 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx index 73b1a6d764..8d4eceefb9 100644 --- a/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx +++ b/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx @@ -15,6 +15,7 @@ import CommentStreamLiveUpdatesContainer from "./CommentStreamLiveUpdatesContain import CustomCSSConfig from "./CustomCSSConfig"; import EmbeddedCommentRepliesConfig from "./EmbeddedCommentRepliesConfig"; import ForReviewQueueConfig from "./ForReviewQueueConfig"; +import OEmbedAllowedOriginsConfigContainer from "./OEmbedAllowedOriginsConfigContainer"; import StoryCreationConfig from "./StoryCreationConfig"; interface Props { @@ -32,6 +33,7 @@ const AdvancedConfigContainer: React.FunctionComponent = ({ + ({ fragment AdvancedConfigContainer_settings on Settings { ...CustomCSSConfig_formValues @relay(mask: false) ...EmbeddedCommentRepliesConfig_formValues @relay(mask: false) + ...OEmbedAllowedOriginsConfigContainer_formValues @relay(mask: false) ...CommentStreamLiveUpdates_formValues @relay(mask: false) ...StoryCreationConfig_formValues @relay(mask: false) ...CommentStreamLiveUpdatesContainer_settings diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx new file mode 100644 index 0000000000..8d960d128a --- /dev/null +++ b/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx @@ -0,0 +1,47 @@ +import { Localized } from "@fluent/react/compat"; +import React, { FunctionComponent } from "react"; +import { graphql } from "react-relay"; + +import { FormField, FormFieldDescription, Label } from "coral-ui/components/v2"; + +import ConfigBox from "../../ConfigBox"; +import Header from "../../Header"; +import AllowedOriginsTextarea from "../Sites/AllowedOriginsTextarea"; + +// eslint-disable-next-line no-unused-expressions +graphql` + fragment OEmbedAllowedOriginsConfigContainer_formValues on Settings { + oEmbedAllowedOrigins + } +`; + +interface Props { + disabled: boolean; +} + +const OEmbedAllowedOriginsConfigContainer: FunctionComponent = ({ + disabled, +}) => ( + +
oEmbed permitted domains
+ + } + > + + + + Domains that are permitted to make calls to the oEmbed API. + + + + + + + + +); + +export default OEmbedAllowedOriginsConfigContainer; diff --git a/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx b/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx index 3c513fe16d..8c5c7caf4f 100644 --- a/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx +++ b/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx @@ -10,12 +10,16 @@ import ValidationMessage from "../../ValidationMessage"; import styles from "./AllowedOriginsTextarea.css"; interface Props { + name: string; defaultValue?: ReadonlyArray; } -const AllowedOriginsTextarea: FunctionComponent = ({ defaultValue }) => ( +const AllowedOriginsTextarea: FunctionComponent = ({ + name, + defaultValue, +}) => ( = ({ onCreate }) => { - + {submitError && ( diff --git a/src/core/client/admin/routes/Configure/sections/Sites/EditSiteForm.tsx b/src/core/client/admin/routes/Configure/sections/Sites/EditSiteForm.tsx index 4461581f88..ffb1f455de 100644 --- a/src/core/client/admin/routes/Configure/sections/Sites/EditSiteForm.tsx +++ b/src/core/client/admin/routes/Configure/sections/Sites/EditSiteForm.tsx @@ -90,7 +90,10 @@ const EditSiteForm: FunctionComponent = ({ - + diff --git a/src/core/server/graph/resolvers/Settings.ts b/src/core/server/graph/resolvers/Settings.ts index 4c922a80ca..b6f4425da5 100644 --- a/src/core/server/graph/resolvers/Settings.ts +++ b/src/core/server/graph/resolvers/Settings.ts @@ -69,4 +69,5 @@ export const Settings: GQLSettingsTypeResolver = { } return flairBadges; }, + oEmbedAllowedOrigins: ({ oEmbedAllowedOrigins = [] }) => oEmbedAllowedOrigins, }; diff --git a/src/core/server/graph/schema/schema.graphql b/src/core/server/graph/schema/schema.graphql index 23cd467c9a..5f15866dfb 100644 --- a/src/core/server/graph/schema/schema.graphql +++ b/src/core/server/graph/schema/schema.graphql @@ -1710,7 +1710,6 @@ type BadgeConfiguration { FlairBadgeConfiguration specifies the configuration for flair badges, including whether they are enabled and any configured image urls. """ - type FlairBadge { name: String! url: String! @@ -2222,6 +2221,11 @@ type Settings @cacheControl(maxAge: 30) { they are enabled and any configured image urls """ flairBadges: FlairBadgeConfiguration + + """ + oEmbedAllowedOrigins are the allowed origins for oEmbed API calls. + """ + oEmbedAllowedOrigins: [String!]! @auth(roles: [ADMIN, MODERATOR]) } ################################################################################ @@ -5802,6 +5806,11 @@ input SettingsInput { they are enabled and any configured image urls """ flairBadges: FlairBadgeConfigurationInput + + """ + oEmbedAllowedOrigins are the allowed origins for oEmbed API calls. + """ + oEmbedAllowedOrigins: [String!] } """ diff --git a/src/core/server/models/settings/settings.ts b/src/core/server/models/settings/settings.ts index 99ba0821a9..89f0a3f082 100644 --- a/src/core/server/models/settings/settings.ts +++ b/src/core/server/models/settings/settings.ts @@ -321,6 +321,7 @@ export type Settings = GlobalModerationSettings & | "announcement" | "memberBios" | "embeddedComments" + | "oEmbedAllowedOrigins" > & { /** * auth is the set of configured authentication integrations. From c05b0f6cb0d119b326376e6ed13d6117c35d99a9 Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Thu, 24 Aug 2023 10:59:36 -0400 Subject: [PATCH 2/8] check for oembedallowedorigins on oembed api calls; add to defaults --- src/core/server/app/middleware/commentEmbedWhitelisted.ts | 8 +++++++- src/core/server/app/router/api/index.ts | 2 +- src/core/server/models/tenant/tenant.ts | 1 + src/core/server/test/fixtures.ts | 1 + 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/core/server/app/middleware/commentEmbedWhitelisted.ts b/src/core/server/app/middleware/commentEmbedWhitelisted.ts index 0ccb3732e7..ce479676e3 100644 --- a/src/core/server/app/middleware/commentEmbedWhitelisted.ts +++ b/src/core/server/app/middleware/commentEmbedWhitelisted.ts @@ -9,7 +9,7 @@ import { AppOptions } from ".."; import { getRequesterOrigin } from "../helpers"; export const commentEmbedWhitelisted = - ({ mongo }: Pick): RequestHandler => + ({ mongo }: Pick, oembedAPI = false): RequestHandler => async (req, res, next) => { // First try to get the commentID from the query params let { commentID } = req.query; @@ -37,6 +37,12 @@ export const commentEmbedWhitelisted = origin = req.header("Origin"); } if (origin) { + // if oEmbed API call, we also check oEmbed allowed origins on tenant + if (oembedAPI) { + if (tenant.oEmbedAllowedOrigins.includes(origin)) { + return next(); + } + } if (site.allowedOrigins.includes(origin)) { return next(); } diff --git a/src/core/server/app/router/api/index.ts b/src/core/server/app/router/api/index.ts index 34bc5793c8..afe215a9eb 100644 --- a/src/core/server/app/router/api/index.ts +++ b/src/core/server/app/router/api/index.ts @@ -97,7 +97,7 @@ export function createAPIRouter(app: AppOptions, options: RouterOptions) { router.get("/oembed", cspSiteMiddleware(app), oembedHandler(app)); router.get( "/services/oembed", - commentEmbedWhitelisted(app), + commentEmbedWhitelisted(app, true), cors(createCommentEmbedCorsOptionsDelegate(app.mongo)), oembedProviderHandler(app) ); diff --git a/src/core/server/models/tenant/tenant.ts b/src/core/server/models/tenant/tenant.ts index 192762a10f..8c08ce494e 100644 --- a/src/core/server/models/tenant/tenant.ts +++ b/src/core/server/models/tenant/tenant.ts @@ -298,6 +298,7 @@ export async function createTenant( flairBadgesEnabled: false, badges: [], }, + oEmbedAllowedOrigins: [], }; // Create the new Tenant by merging it together with the defaults. diff --git a/src/core/server/test/fixtures.ts b/src/core/server/test/fixtures.ts index 7efe082648..25701d5de8 100644 --- a/src/core/server/test/fixtures.ts +++ b/src/core/server/test/fixtures.ts @@ -184,6 +184,7 @@ export const createTenantFixture = ( flattenReplies: false, disableDefaultFonts: false, emailDomainModeration: [], + oEmbedAllowedOrigins: [], }; return merge(fixture, defaults); From 6cda411453831fe7fd65554bdf630b2d6316f4ed Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Thu, 24 Aug 2023 11:24:34 -0400 Subject: [PATCH 3/8] add test and localizations --- .../OEmbedAllowedOriginsConfigContainer.tsx | 14 ++++--- .../sections/Sites/AllowedOriginsTextarea.tsx | 3 ++ .../admin/test/configure/advanced.spec.tsx | 40 +++++++++++++++++++ src/core/client/admin/test/fixtures.ts | 1 + src/locales/en-US/admin.ftl | 4 ++ 5 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx index 8d960d128a..803b7d856d 100644 --- a/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx +++ b/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx @@ -23,23 +23,25 @@ const OEmbedAllowedOriginsConfigContainer: FunctionComponent = ({ disabled, }) => ( -
oEmbed permitted domains
+ +
+ oEmbed permitted domains +
} > - + Domains that are permitted to make calls to the oEmbed API. - + - + ); diff --git a/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx b/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx index 8c5c7caf4f..b0735ad4b1 100644 --- a/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx +++ b/src/core/client/admin/routes/Configure/sections/Sites/AllowedOriginsTextarea.tsx @@ -12,11 +12,13 @@ import styles from "./AllowedOriginsTextarea.css"; interface Props { name: string; defaultValue?: ReadonlyArray; + disabled?: boolean; } const AllowedOriginsTextarea: FunctionComponent = ({ name, defaultValue, + disabled = false, }) => ( = ({ autoCapitalize="off" spellCheck={false} fullwidth + disabled={disabled} /> diff --git a/src/core/client/admin/test/configure/advanced.spec.tsx b/src/core/client/admin/test/configure/advanced.spec.tsx index 9f19e303b3..043e4d0ca2 100644 --- a/src/core/client/admin/test/configure/advanced.spec.tsx +++ b/src/core/client/admin/test/configure/advanced.spec.tsx @@ -241,3 +241,43 @@ it("change embedded comments allow replies", async () => { expect(resolvers.Mutation!.updateSettings!.called).toBe(true); }); }); + +it("change oembed permitted domains", async () => { + const resolvers = createResolversStub({ + Mutation: { + updateSettings: ({ variables }) => { + expectAndFail(variables.settings.oEmbedAllowedOrigins).toEqual([ + "http://localhost:8080", + ]); + return { + settings: pureMerge(settings, variables.settings), + }; + }, + }, + }); + const { advancedContainer, saveChangesButton } = await createTestRenderer({ + resolvers, + }); + + const oembedAllowedOriginsConfig = within(advancedContainer).getByTestId( + "oembed-allowed-origins-config" + ); + + const allowedOriginsTextArea = within(oembedAllowedOriginsConfig).getByRole( + "textbox" + ); + + userEvent.type(allowedOriginsTextArea, "http://"); + + userEvent.click(saveChangesButton); + + expect(within(advancedContainer).getByText("Invalid URL")); + + userEvent.type(allowedOriginsTextArea, "localhost:8080"); + + userEvent.click(saveChangesButton); + + await waitFor(() => { + expect(resolvers.Mutation!.updateSettings!.called).toBe(true); + }); +}); diff --git a/src/core/client/admin/test/fixtures.ts b/src/core/client/admin/test/fixtures.ts index 7102c1f35a..8213c5d950 100644 --- a/src/core/client/admin/test/fixtures.ts +++ b/src/core/client/admin/test/fixtures.ts @@ -229,6 +229,7 @@ export const settings = createFixture({ flairBadgesEnabled: false, badges: [], }, + oEmbedAllowedOrigins: [], }); export const settingsWithMultisite = createFixture( diff --git a/src/locales/en-US/admin.ftl b/src/locales/en-US/admin.ftl index 1ba67c3b5e..a8d3ce2e46 100644 --- a/src/locales/en-US/admin.ftl +++ b/src/locales/en-US/admin.ftl @@ -911,6 +911,10 @@ configure-advanced-embeddedCommentReplies-explanation = When enabled, a reply bu specific comment or story. configure-advanced-embeddedCommentReplies-label = Allow replies to embedded comments +configure-advanced-oembedAllowedOrigins-header = oEmbed permitted domains +configure-advanced-oembedAllowedOrigins-description = Domains that are permitted to make calls to the oEmbed API. +configure-advanced-oembedAllowedOrigins-label = oEmbed permitted domains + configure-advanced-permittedDomains = Permitted domains configure-advanced-permittedDomains-description = Domains where your { -product-name } instance is allowed to be embedded From fa1c429846a34f27ac0eb23d1910bc02cea05637 Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Thu, 24 Aug 2023 15:36:37 -0400 Subject: [PATCH 4/8] update config layout and copy --- .../Advanced/AdvancedConfigContainer.tsx | 9 +-- .../Advanced/EmbeddedCommentRepliesConfig.tsx | 52 -------------- .../Advanced/EmbeddedCommentsConfig.tsx | 72 +++++++++++++++++++ .../OEmbedAllowedOriginsConfigContainer.tsx | 49 ------------- .../admin/test/configure/advanced.spec.tsx | 12 ++-- src/core/client/admin/test/fixtures.ts | 2 +- .../app/middleware/commentEmbedWhitelisted.ts | 4 +- src/core/server/graph/resolvers/Settings.ts | 10 ++- src/core/server/graph/schema/schema.graphql | 18 +++-- src/core/server/models/settings/settings.ts | 1 - src/core/server/models/tenant/tenant.ts | 2 +- src/core/server/test/fixtures.ts | 5 +- src/locales/en-US/admin.ftl | 4 +- 13 files changed, 108 insertions(+), 132 deletions(-) delete mode 100644 src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentRepliesConfig.tsx create mode 100644 src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx delete mode 100644 src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx index 8d4eceefb9..4c89753386 100644 --- a/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx +++ b/src/core/client/admin/routes/Configure/sections/Advanced/AdvancedConfigContainer.tsx @@ -13,9 +13,8 @@ import { AdvancedConfigContainer_settings } from "coral-admin/__generated__/Adva import AMPConfig from "./AMPConfig"; import CommentStreamLiveUpdatesContainer from "./CommentStreamLiveUpdatesContainer"; import CustomCSSConfig from "./CustomCSSConfig"; -import EmbeddedCommentRepliesConfig from "./EmbeddedCommentRepliesConfig"; +import EmbeddedCommentsConfig from "./EmbeddedCommentsConfig"; import ForReviewQueueConfig from "./ForReviewQueueConfig"; -import OEmbedAllowedOriginsConfigContainer from "./OEmbedAllowedOriginsConfigContainer"; import StoryCreationConfig from "./StoryCreationConfig"; interface Props { @@ -32,8 +31,7 @@ const AdvancedConfigContainer: React.FunctionComponent = ({ return ( - - + ({ settings: graphql` fragment AdvancedConfigContainer_settings on Settings { ...CustomCSSConfig_formValues @relay(mask: false) - ...EmbeddedCommentRepliesConfig_formValues @relay(mask: false) - ...OEmbedAllowedOriginsConfigContainer_formValues @relay(mask: false) + ...EmbeddedCommentsConfig_formValues @relay(mask: false) ...CommentStreamLiveUpdates_formValues @relay(mask: false) ...StoryCreationConfig_formValues @relay(mask: false) ...CommentStreamLiveUpdatesContainer_settings diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentRepliesConfig.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentRepliesConfig.tsx deleted file mode 100644 index f78b905743..0000000000 --- a/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentRepliesConfig.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import { Localized } from "@fluent/react/compat"; -import React, { FunctionComponent } from "react"; -import { graphql } from "react-relay"; - -import { FormField, FormFieldDescription, Label } from "coral-ui/components/v2"; - -import ConfigBox from "../../ConfigBox"; -import Header from "../../Header"; -import OnOffField from "../../OnOffField"; - -// eslint-disable-next-line no-unused-expressions -graphql` - fragment EmbeddedCommentRepliesConfig_formValues on Settings { - embeddedComments { - allowReplies - } - } -`; - -interface Props { - disabled: boolean; -} - -const EmbeddedCommentRepliesConfig: FunctionComponent = ({ - disabled, -}) => ( - -
- Embedded comment replies -
- - } - > - - - - When enabled, a reply button will appear with each embedded comment to - encourage additional discussion on that specific comment or story. - - - - - - - - -); - -export default EmbeddedCommentRepliesConfig; diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx new file mode 100644 index 0000000000..b86ef7065c --- /dev/null +++ b/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx @@ -0,0 +1,72 @@ +import { Localized } from "@fluent/react/compat"; +import React, { FunctionComponent } from "react"; +import { graphql } from "react-relay"; + +import { FormField, FormFieldDescription, Label } from "coral-ui/components/v2"; + +import ConfigBox from "../../ConfigBox"; +import Header from "../../Header"; +import OnOffField from "../../OnOffField"; +import AllowedOriginsTextarea from "../Sites/AllowedOriginsTextarea"; +import Subheader from "../../Subheader"; + +// eslint-disable-next-line no-unused-expressions +graphql` + fragment EmbeddedCommentsConfig_formValues on Settings { + embeddedComments { + allowReplies + oEmbedAllowedOrigins + } + } +`; + +interface Props { + disabled: boolean; +} + +const EmbeddedCommentsConfig: FunctionComponent = ({ disabled }) => ( + +
+ Embedded comments +
+ + } + > + + + + + + + When enabled, a reply button will appear with each embedded comment to + encourage additional discussion on that specific comment or story. + + + + + + For sites using oEmbed + + + + + + + + Domains that are permitted to make calls to the oEmbed API (ex. + http://localhost:3000, https://staging.domain.com, + https://domain.com). + + + + + +); + +export default EmbeddedCommentsConfig; diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx deleted file mode 100644 index 803b7d856d..0000000000 --- a/src/core/client/admin/routes/Configure/sections/Advanced/OEmbedAllowedOriginsConfigContainer.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import { Localized } from "@fluent/react/compat"; -import React, { FunctionComponent } from "react"; -import { graphql } from "react-relay"; - -import { FormField, FormFieldDescription, Label } from "coral-ui/components/v2"; - -import ConfigBox from "../../ConfigBox"; -import Header from "../../Header"; -import AllowedOriginsTextarea from "../Sites/AllowedOriginsTextarea"; - -// eslint-disable-next-line no-unused-expressions -graphql` - fragment OEmbedAllowedOriginsConfigContainer_formValues on Settings { - oEmbedAllowedOrigins - } -`; - -interface Props { - disabled: boolean; -} - -const OEmbedAllowedOriginsConfigContainer: FunctionComponent = ({ - disabled, -}) => ( - -
- oEmbed permitted domains -
- - } - > - - - - Domains that are permitted to make calls to the oEmbed API. - - - - - - - - -); - -export default OEmbedAllowedOriginsConfigContainer; diff --git a/src/core/client/admin/test/configure/advanced.spec.tsx b/src/core/client/admin/test/configure/advanced.spec.tsx index 043e4d0ca2..582ededa84 100644 --- a/src/core/client/admin/test/configure/advanced.spec.tsx +++ b/src/core/client/admin/test/configure/advanced.spec.tsx @@ -59,7 +59,7 @@ it("renders configure advanced", async () => { const { configureContainer } = await createTestRenderer(); expect(within(configureContainer).getByLabelText("Custom CSS")).toBeDefined(); expect( - within(configureContainer).getByText("Embedded comment replies") + within(configureContainer).getByText("Embedded comments") ).toBeDefined(); expect( within(configureContainer).getByText("Comment stream live updates") @@ -223,7 +223,7 @@ it("change embedded comments allow replies", async () => { }); const embeddedCommentReplies = within(advancedContainer).getByTestId( - "embedded-comment-replies-config" + "embedded-comments-config" ); const offField = within(embeddedCommentReplies).getByText("Off"); @@ -246,9 +246,9 @@ it("change oembed permitted domains", async () => { const resolvers = createResolversStub({ Mutation: { updateSettings: ({ variables }) => { - expectAndFail(variables.settings.oEmbedAllowedOrigins).toEqual([ - "http://localhost:8080", - ]); + expectAndFail( + variables.settings.embeddedComments?.oEmbedAllowedOrigins + ).toEqual(["http://localhost:8080"]); return { settings: pureMerge(settings, variables.settings), }; @@ -260,7 +260,7 @@ it("change oembed permitted domains", async () => { }); const oembedAllowedOriginsConfig = within(advancedContainer).getByTestId( - "oembed-allowed-origins-config" + "embedded-comments-config" ); const allowedOriginsTextArea = within(oembedAllowedOriginsConfig).getByRole( diff --git a/src/core/client/admin/test/fixtures.ts b/src/core/client/admin/test/fixtures.ts index 8213c5d950..c32159eafc 100644 --- a/src/core/client/admin/test/fixtures.ts +++ b/src/core/client/admin/test/fixtures.ts @@ -224,12 +224,12 @@ export const settings = createFixture({ emailDomainModeration: [], embeddedComments: { allowReplies: true, + oEmbedAllowedOrigins: [], }, flairBadges: { flairBadgesEnabled: false, badges: [], }, - oEmbedAllowedOrigins: [], }); export const settingsWithMultisite = createFixture( diff --git a/src/core/server/app/middleware/commentEmbedWhitelisted.ts b/src/core/server/app/middleware/commentEmbedWhitelisted.ts index ce479676e3..13ca25e99a 100644 --- a/src/core/server/app/middleware/commentEmbedWhitelisted.ts +++ b/src/core/server/app/middleware/commentEmbedWhitelisted.ts @@ -39,7 +39,9 @@ export const commentEmbedWhitelisted = if (origin) { // if oEmbed API call, we also check oEmbed allowed origins on tenant if (oembedAPI) { - if (tenant.oEmbedAllowedOrigins.includes(origin)) { + if ( + tenant.embeddedComments?.oEmbedAllowedOrigins.includes(origin) + ) { return next(); } } diff --git a/src/core/server/graph/resolvers/Settings.ts b/src/core/server/graph/resolvers/Settings.ts index b6f4425da5..7988ef3b09 100644 --- a/src/core/server/graph/resolvers/Settings.ts +++ b/src/core/server/graph/resolvers/Settings.ts @@ -54,10 +54,15 @@ export const Settings: GQLSettingsTypeResolver = { return deprecated; }, embeddedComments: ( - { embeddedComments = { allowReplies: true } }, + { embeddedComments = { allowReplies: true, oEmbedAllowedOrigins: [] } }, args, ctx - ) => embeddedComments, + ) => { + return { + allowReplies: embeddedComments.allowReplies ?? true, + oEmbedAllowedOrigins: embeddedComments.oEmbedAllowedOrigins ?? [], + }; + }, flairBadges: ({ flairBadges = { flairBadgesEnabled: false, badges: [] }, }) => { @@ -69,5 +74,4 @@ export const Settings: GQLSettingsTypeResolver = { } return flairBadges; }, - oEmbedAllowedOrigins: ({ oEmbedAllowedOrigins = [] }) => oEmbedAllowedOrigins, }; diff --git a/src/core/server/graph/schema/schema.graphql b/src/core/server/graph/schema/schema.graphql index 5f15866dfb..cf20728d62 100644 --- a/src/core/server/graph/schema/schema.graphql +++ b/src/core/server/graph/schema/schema.graphql @@ -1673,6 +1673,10 @@ EmbeddedCommentsConfiguration specifies the configuration for embedded comments. """ type EmbeddedCommentsConfiguration { allowReplies: Boolean + """ + oEmbedAllowedOrigins are the allowed origins for oEmbed API calls. + """ + oEmbedAllowedOrigins: [String!]! @auth(roles: [ADMIN, MODERATOR]) } """ @@ -2221,11 +2225,6 @@ type Settings @cacheControl(maxAge: 30) { they are enabled and any configured image urls """ flairBadges: FlairBadgeConfiguration - - """ - oEmbedAllowedOrigins are the allowed origins for oEmbed API calls. - """ - oEmbedAllowedOrigins: [String!]! @auth(roles: [ADMIN, MODERATOR]) } ################################################################################ @@ -5572,6 +5571,10 @@ EmbeddedCommentsConfigurationInput specifies the configuration for comment embed """ input EmbeddedCommentsConfigurationInput { allowReplies: Boolean + """ + oEmbedAllowedOrigins are the allowed origins for oEmbed API calls. + """ + oEmbedAllowedOrigins: [String!] } """ @@ -5806,11 +5809,6 @@ input SettingsInput { they are enabled and any configured image urls """ flairBadges: FlairBadgeConfigurationInput - - """ - oEmbedAllowedOrigins are the allowed origins for oEmbed API calls. - """ - oEmbedAllowedOrigins: [String!] } """ diff --git a/src/core/server/models/settings/settings.ts b/src/core/server/models/settings/settings.ts index 89f0a3f082..99ba0821a9 100644 --- a/src/core/server/models/settings/settings.ts +++ b/src/core/server/models/settings/settings.ts @@ -321,7 +321,6 @@ export type Settings = GlobalModerationSettings & | "announcement" | "memberBios" | "embeddedComments" - | "oEmbedAllowedOrigins" > & { /** * auth is the set of configured authentication integrations. diff --git a/src/core/server/models/tenant/tenant.ts b/src/core/server/models/tenant/tenant.ts index 8c08ce494e..3103cc6d00 100644 --- a/src/core/server/models/tenant/tenant.ts +++ b/src/core/server/models/tenant/tenant.ts @@ -293,12 +293,12 @@ export async function createTenant( emailDomainModeration: [], embeddedComments: { allowReplies: true, + oEmbedAllowedOrigins: [], }, flairBadges: { flairBadgesEnabled: false, badges: [], }, - oEmbedAllowedOrigins: [], }; // Create the new Tenant by merging it together with the defaults. diff --git a/src/core/server/test/fixtures.ts b/src/core/server/test/fixtures.ts index 25701d5de8..81187cec43 100644 --- a/src/core/server/test/fixtures.ts +++ b/src/core/server/test/fixtures.ts @@ -184,7 +184,10 @@ export const createTenantFixture = ( flattenReplies: false, disableDefaultFonts: false, emailDomainModeration: [], - oEmbedAllowedOrigins: [], + embeddedComments: { + allowReplies: true, + oEmbedAllowedOrigins: [], + }, }; return merge(fixture, defaults); diff --git a/src/locales/en-US/admin.ftl b/src/locales/en-US/admin.ftl index a8d3ce2e46..48ded2a1df 100644 --- a/src/locales/en-US/admin.ftl +++ b/src/locales/en-US/admin.ftl @@ -905,6 +905,8 @@ configure-advanced-customCSS-containsFontFace = URL to a custom CSS stylesheet that contains all @font-face definitions needed by above stylesheet. +configure-advanced-embeddedComments = Embedded comments +configure-advanced-embeddedComments-subheader = For sites using oEmbed configure-advanced-embeddedCommentReplies = Embedded comment replies configure-advanced-embeddedCommentReplies-explanation = When enabled, a reply button will appear with each embedded comment to encourage additional discussion on that @@ -912,7 +914,7 @@ configure-advanced-embeddedCommentReplies-explanation = When enabled, a reply bu configure-advanced-embeddedCommentReplies-label = Allow replies to embedded comments configure-advanced-oembedAllowedOrigins-header = oEmbed permitted domains -configure-advanced-oembedAllowedOrigins-description = Domains that are permitted to make calls to the oEmbed API. +configure-advanced-oembedAllowedOrigins-description = Domains that are permitted to make calls to the oEmbed API (ex. http://localhost:3000, https://staging.domain.com, https://domain.com). configure-advanced-oembedAllowedOrigins-label = oEmbed permitted domains configure-advanced-permittedDomains = Permitted domains From 4d0aa5bab85f6f1a4685d255252bb83d1a57700f Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Fri, 25 Aug 2023 10:37:11 -0400 Subject: [PATCH 5/8] fix import order --- .../Configure/sections/Advanced/EmbeddedCommentsConfig.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx index b86ef7065c..518cd674a9 100644 --- a/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx +++ b/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx @@ -7,8 +7,8 @@ import { FormField, FormFieldDescription, Label } from "coral-ui/components/v2"; import ConfigBox from "../../ConfigBox"; import Header from "../../Header"; import OnOffField from "../../OnOffField"; -import AllowedOriginsTextarea from "../Sites/AllowedOriginsTextarea"; import Subheader from "../../Subheader"; +import AllowedOriginsTextarea from "../Sites/AllowedOriginsTextarea"; // eslint-disable-next-line no-unused-expressions graphql` From 3f79f6b69a1ed5dcf225ba96b06f70bc3fdc29ff Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Fri, 25 Aug 2023 10:49:10 -0400 Subject: [PATCH 6/8] update descs to helpertext to match design --- .../sections/Advanced/EmbeddedCommentsConfig.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx b/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx index 518cd674a9..a22d78332b 100644 --- a/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx +++ b/src/core/client/admin/routes/Configure/sections/Advanced/EmbeddedCommentsConfig.tsx @@ -2,7 +2,7 @@ import { Localized } from "@fluent/react/compat"; import React, { FunctionComponent } from "react"; import { graphql } from "react-relay"; -import { FormField, FormFieldDescription, Label } from "coral-ui/components/v2"; +import { FormField, HelperText, Label } from "coral-ui/components/v2"; import ConfigBox from "../../ConfigBox"; import Header from "../../Header"; @@ -40,10 +40,10 @@ const EmbeddedCommentsConfig: FunctionComponent = ({ disabled }) => ( - + When enabled, a reply button will appear with each embedded comment to encourage additional discussion on that specific comment or story. - + @@ -55,11 +55,11 @@ const EmbeddedCommentsConfig: FunctionComponent = ({ disabled }) => ( - + Domains that are permitted to make calls to the oEmbed API (ex. http://localhost:3000, https://staging.domain.com, https://domain.com). - + Date: Fri, 25 Aug 2023 11:08:16 -0400 Subject: [PATCH 7/8] remove no-longer-needed localization --- src/locales/en-US/admin.ftl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/locales/en-US/admin.ftl b/src/locales/en-US/admin.ftl index 48ded2a1df..33246960de 100644 --- a/src/locales/en-US/admin.ftl +++ b/src/locales/en-US/admin.ftl @@ -907,7 +907,6 @@ configure-advanced-customCSS-containsFontFace = configure-advanced-embeddedComments = Embedded comments configure-advanced-embeddedComments-subheader = For sites using oEmbed -configure-advanced-embeddedCommentReplies = Embedded comment replies configure-advanced-embeddedCommentReplies-explanation = When enabled, a reply button will appear with each embedded comment to encourage additional discussion on that specific comment or story. From c21177a3e42662638a9e0257a67d25aaea31d040 Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Mon, 28 Aug 2023 12:50:29 -0400 Subject: [PATCH 8/8] combine oembed allowed origins checks --- .../server/app/middleware/commentEmbedWhitelisted.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/server/app/middleware/commentEmbedWhitelisted.ts b/src/core/server/app/middleware/commentEmbedWhitelisted.ts index 13ca25e99a..0f6fd29f1f 100644 --- a/src/core/server/app/middleware/commentEmbedWhitelisted.ts +++ b/src/core/server/app/middleware/commentEmbedWhitelisted.ts @@ -38,12 +38,11 @@ export const commentEmbedWhitelisted = } if (origin) { // if oEmbed API call, we also check oEmbed allowed origins on tenant - if (oembedAPI) { - if ( - tenant.embeddedComments?.oEmbedAllowedOrigins.includes(origin) - ) { - return next(); - } + if ( + oembedAPI && + tenant.embeddedComments?.oEmbedAllowedOrigins.includes(origin) + ) { + return next(); } if (site.allowedOrigins.includes(origin)) { return next();