From d099d58f77a1d43d0e21ef7d3d31887c51e1d031 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 28 Feb 2025 11:38:12 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82(frontend)=20secure=20download=20bu?= =?UTF-8?q?tton?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocknote download button opens the file in a new tab, which could be not secure because of XSS attacks. We replace the download button with a new one that downloads the file instead of opening it in a new tab. Some files are flags as unsafe (SVG / js / exe), for these files we add a confirmation modal before downloading the file to prevent the user from downloading a file that could be harmful. In the future, we could add other security layers from this model, to analyze the file before downloading it by example. --- CHANGELOG.md | 1 + .../e2e/__tests__/app-impress/assets/test.svg | 13 ++ .../__tests__/app-impress/doc-editor.spec.ts | 44 ++++++- .../components/BlockNoteToolbar.tsx | 57 +++++++-- .../components/FileDownloadButton.tsx | 111 ++++++++++++++++++ .../components/ModalConfirmDownloadUnsafe.tsx | 74 ++++++++++++ .../src/features/docs/doc-export/index.ts | 1 + 7 files changed, 288 insertions(+), 13 deletions(-) create mode 100644 src/frontend/apps/e2e/__tests__/app-impress/assets/test.svg create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/FileDownloadButton.tsx create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/components/ModalConfirmDownloadUnsafe.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ced3121..850fd07b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to ## Added - 💄(frontend) add error pages #643 +- 🔒️ Manage unsafe attachments #663 - ✨(frontend) Custom block quote with export #646 - ✨(frontend) add open source section homepage #666 diff --git a/src/frontend/apps/e2e/__tests__/app-impress/assets/test.svg b/src/frontend/apps/e2e/__tests__/app-impress/assets/test.svg new file mode 100644 index 00000000..f413e27d --- /dev/null +++ b/src/frontend/apps/e2e/__tests__/app-impress/assets/test.svg @@ -0,0 +1,13 @@ + + + + Hello svg + diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts index 470dfadd..92753afd 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts @@ -1,8 +1,7 @@ -/* eslint-disable playwright/no-conditional-expect */ -/* eslint-disable playwright/no-conditional-in-test */ import path from 'path'; import { expect, test } from '@playwright/test'; +import cs from 'convert-stream'; import { createDoc, @@ -415,6 +414,8 @@ test.describe('Doc Editor', () => { const editor = page.locator('.ProseMirror'); await editor.getByText('Hello').dblclick(); + /* eslint-disable playwright/no-conditional-expect */ + /* eslint-disable playwright/no-conditional-in-test */ if (!ai_transform && !ai_translate) { await expect(page.getByRole('button', { name: 'AI' })).toBeHidden(); return; @@ -441,6 +442,45 @@ test.describe('Doc Editor', () => { page.getByRole('menuitem', { name: 'Language' }), ).toBeHidden(); } + /* eslint-enable playwright/no-conditional-expect */ + /* eslint-enable playwright/no-conditional-in-test */ }); }); + + test('it downloads unsafe files', async ({ page, browserName }) => { + const [randomDoc] = await createDoc(page, 'doc-editor', browserName, 1); + + const fileChooserPromise = page.waitForEvent('filechooser'); + const downloadPromise = page.waitForEvent('download', (download) => { + return download.suggestedFilename().includes(`svg`); + }); + + await verifyDocName(page, randomDoc); + + await page.locator('.ProseMirror.bn-editor').click(); + await page.locator('.ProseMirror.bn-editor').fill('Hello World'); + + await page.keyboard.press('Enter'); + await page.locator('.bn-block-outer').last().fill('/'); + await page.getByText('Resizable image with caption').click(); + await page.getByText('Upload image').click(); + + const fileChooser = await fileChooserPromise; + await fileChooser.setFiles(path.join(__dirname, 'assets/test.svg')); + + await page.locator('.bn-block-content[data-name="test.svg"]').click(); + await page.getByRole('button', { name: 'Download image' }).click(); + + await expect( + page.getByText('This file is flagged as unsafe.'), + ).toBeVisible(); + + await page.getByRole('button', { name: 'Download' }).click(); + + const download = await downloadPromise; + expect(download.suggestedFilename()).toContain(`-unsafe.svg`); + + const svgBuffer = await cs.toBuffer(await download.createReadStream()); + expect(svgBuffer.toString()).toContain('Hello svg'); + }); }); diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolbar.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolbar.tsx index aa0eb1e9..c1f106d9 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolbar.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolbar.tsx @@ -6,24 +6,50 @@ import { getFormattingToolbarItems, useDictionary, } from '@blocknote/react'; -import React, { useCallback } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { AIGroupButton } from './AIButton'; +import { FileDownloadButton } from './FileDownloadButton'; import { MarkdownButton } from './MarkdownButton'; +import { ModalConfirmDownloadUnsafe } from './ModalConfirmDownloadUnsafe'; import { getQuoteFormattingToolbarItems } from './custom-blocks'; export const BlockNoteToolbar = () => { const dict = useDictionary(); + const [confirmOpen, setIsConfirmOpen] = useState(false); + const [onConfirm, setOnConfirm] = useState<() => void | Promise>(); const { t } = useTranslation(); - const formattingToolbar = useCallback( - () => ( + const toolbarItems = useMemo(() => { + const toolbarItems = getFormattingToolbarItems([ + ...blockTypeSelectItems(dict), + getQuoteFormattingToolbarItems(t), + ]); + const fileDownloadButtonIndex = toolbarItems.findIndex( + (item) => item.key === 'fileDownloadButton', + ); + if (fileDownloadButtonIndex !== -1) { + toolbarItems.splice( + fileDownloadButtonIndex, + 1, + { + setIsConfirmOpen(true); + setOnConfirm(() => onConfirm); + }} + />, + ); + } + + return toolbarItems; + }, [dict, t]); + + const formattingToolbar = useCallback(() => { + return ( - {getFormattingToolbarItems([ - ...blockTypeSelectItems(dict), - getQuoteFormattingToolbarItems(t), - ])} + {toolbarItems} {/* Extra button to do some AI powered actions */} @@ -31,9 +57,18 @@ export const BlockNoteToolbar = () => { {/* Extra button to convert from markdown to json */} - ), - [dict, t], - ); + ); + }, [toolbarItems]); - return ; + return ( + <> + + {confirmOpen && ( + setIsConfirmOpen(false)} + onConfirm={onConfirm} + /> + )} + + ); }; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/FileDownloadButton.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/FileDownloadButton.tsx new file mode 100644 index 00000000..c34eb244 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/FileDownloadButton.tsx @@ -0,0 +1,111 @@ +import { + BlockSchema, + InlineContentSchema, + StyleSchema, + checkBlockIsFileBlock, + checkBlockIsFileBlockWithPlaceholder, +} from '@blocknote/core'; +import { + useBlockNoteEditor, + useComponentsContext, + useDictionary, + useSelectedBlocks, +} from '@blocknote/react'; +import { useCallback, useMemo } from 'react'; +import { RiDownload2Fill } from 'react-icons/ri'; + +import { downloadFile, exportResolveFileUrl } from '@/features/docs/doc-export'; + +export const FileDownloadButton = ({ + open, +}: { + open: (onConfirm: () => Promise | void) => void; +}) => { + const dict = useDictionary(); + const Components = useComponentsContext(); + + const editor = useBlockNoteEditor< + BlockSchema, + InlineContentSchema, + StyleSchema + >(); + + const selectedBlocks = useSelectedBlocks(editor); + + const fileBlock = useMemo(() => { + // Checks if only one block is selected. + if (selectedBlocks.length !== 1) { + return undefined; + } + + const block = selectedBlocks[0]; + + if (checkBlockIsFileBlock(block, editor)) { + return block; + } + + return undefined; + }, [editor, selectedBlocks]); + + const onClick = useCallback(async () => { + if (fileBlock && fileBlock.props.url) { + editor.focus(); + + const url = fileBlock.props.url as string; + + /** + * If not hosted on our domain, means not a file uploaded by the user, + * we do what Blocknote was doing initially. + */ + if (!url.includes(window.location.hostname)) { + if (!editor.resolveFileUrl) { + window.open(url); + } else { + void editor + .resolveFileUrl(url) + .then((downloadUrl) => window.open(downloadUrl)); + } + + return; + } + + if (!url.includes('-unsafe')) { + const blob = (await exportResolveFileUrl(url, undefined)) as Blob; + downloadFile(blob, url.split('/').pop() || 'file'); + } else { + const onConfirm = async () => { + const blob = (await exportResolveFileUrl(url, undefined)) as Blob; + downloadFile(blob, url.split('/').pop() || 'file (unsafe)'); + }; + + open(onConfirm); + } + } + }, [editor, fileBlock, open]); + + if ( + !fileBlock || + checkBlockIsFileBlockWithPlaceholder(fileBlock, editor) || + !Components + ) { + return null; + } + + return ( + <> + } + onClick={() => void onClick()} + /> + + ); +}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/ModalConfirmDownloadUnsafe.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/ModalConfirmDownloadUnsafe.tsx new file mode 100644 index 00000000..da4d5e43 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/ModalConfirmDownloadUnsafe.tsx @@ -0,0 +1,74 @@ +import { Button, Modal, ModalSize } from '@openfun/cunningham-react'; +import { useTranslation } from 'react-i18next'; + +import { Box, Text } from '@/components'; + +interface ModalConfirmDownloadUnsafeProps { + onClose: () => void; + onConfirm?: () => Promise | void; +} + +export const ModalConfirmDownloadUnsafe = ({ + onConfirm, + onClose, +}: ModalConfirmDownloadUnsafeProps) => { + const { t } = useTranslation(); + + return ( + onClose()} + rightActions={ + <> + + + + } + size={ModalSize.SMALL} + title={ + + + warning + + {t('Warning')} + + } + > + + + + {t('This file is flagged as unsafe.')} + + {t('Please download it only if it comes from a trusted source.')} + + + + + + ); +}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-export/index.ts b/src/frontend/apps/impress/src/features/docs/doc-export/index.ts index 07635cbb..590a7f4c 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-export/index.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-export/index.ts @@ -1 +1,2 @@ export * from './components'; +export * from './utils';