From ad98c0aa04ad432d00928ac4b68005f7a45fa8f2 Mon Sep 17 00:00:00 2001 From: Nik Graf Date: Thu, 21 Mar 2024 10:25:14 +0100 Subject: [PATCH] fix attachment download for share links --- apps/app/components/editor/Editor.tsx | 5 ++- apps/app/components/editor/Editor.web.tsx | 5 ++- apps/app/components/editor/types.ts | 1 + apps/app/components/sharePage/SharePage.tsx | 1 + apps/app/generated/graphql.ts | 14 +++--- .../queries/commentsByDocumentId.graphql | 2 +- apps/app/graphql/queries/fileUrl.graphql | 8 +++- .../createDownloadAndDecryptFileFunction.ts | 6 +-- apps/backend/src/database/file/getFileUrl.ts | 44 +++++++++++++------ .../comment/commentsByDocumentId.test.ts | 2 +- .../queries/comment/commentsByDocumentId.ts | 4 +- .../src/graphql/queries/file/fileUrl.test.ts | 20 +++------ .../src/graphql/queries/file/fileUrl.ts | 10 ++--- 13 files changed, 71 insertions(+), 51 deletions(-) diff --git a/apps/app/components/editor/Editor.tsx b/apps/app/components/editor/Editor.tsx index 137bbf429..d6f93c033 100644 --- a/apps/app/components/editor/Editor.tsx +++ b/apps/app/components/editor/Editor.tsx @@ -95,6 +95,7 @@ export default function Editor({ documentState, canComment, currentDeviceSigningPublicKey, + documentShareLinkToken, }: EditorProps) { const webViewRef = useRef(null); // leveraging a ref here since the injectedJavaScriptBeforeContentLoaded @@ -123,10 +124,10 @@ export default function Editor({ const downloadAndDecryptFile = useMemo(() => { return createDownloadAndDecryptFileFunction({ - workspaceId, + documentShareLinkToken, documentId, }); - }, [workspaceId, documentId]); + }, [documentId, documentShareLinkToken]); const workspaceDevicesToUsernames = useWorkspaceMemberDevicesToUsernames({ workspaceId, diff --git a/apps/app/components/editor/Editor.web.tsx b/apps/app/components/editor/Editor.web.tsx index 7651efb20..8ab582522 100644 --- a/apps/app/components/editor/Editor.web.tsx +++ b/apps/app/components/editor/Editor.web.tsx @@ -66,6 +66,7 @@ export default function Editor({ editable, documentState, canComment, + documentShareLinkToken, }: EditorProps) { const [editorBottombarState, setEditorBottombarState] = useState(initialEditorBottombarState); @@ -184,10 +185,10 @@ export default function Editor({ const downloadAndDecryptFile = useMemo(() => { return createDownloadAndDecryptFileFunction({ - workspaceId, documentId, + documentShareLinkToken, }); - }, [workspaceId, documentId]); + }, [documentId, documentShareLinkToken]); if (!documentLoaded) { return ; diff --git a/apps/app/components/editor/types.ts b/apps/app/components/editor/types.ts index 0fe17b383..ad35c55c6 100644 --- a/apps/app/components/editor/types.ts +++ b/apps/app/components/editor/types.ts @@ -15,4 +15,5 @@ export type EditorProps = { documentState: DocumentState; canComment: boolean; currentDeviceSigningPublicKey: string; + documentShareLinkToken?: string; }; diff --git a/apps/app/components/sharePage/SharePage.tsx b/apps/app/components/sharePage/SharePage.tsx index 4fe9a9720..2980230c9 100644 --- a/apps/app/components/sharePage/SharePage.tsx +++ b/apps/app/components/sharePage/SharePage.tsx @@ -346,6 +346,7 @@ export const SharePage: React.FC = ({ currentDeviceSigningPublicKey={sodium.to_base64( signatureKeyPair.publicKey )} + documentShareLinkToken={token} /> ); diff --git a/apps/app/generated/graphql.ts b/apps/app/generated/graphql.ts index a762c44d0..55ce9933a 100644 --- a/apps/app/generated/graphql.ts +++ b/apps/app/generated/graphql.ts @@ -967,8 +967,8 @@ export type QueryEncryptedWebDeviceArgs = { export type QueryFileUrlArgs = { documentId: Scalars['ID']['input']; + documentShareLinkToken?: InputMaybe; fileId: Scalars['ID']['input']; - workspaceId: Scalars['ID']['input']; }; @@ -1814,8 +1814,8 @@ export type EncryptedWebDeviceQuery = { __typename?: 'Query', encryptedWebDevice export type FileUrlQueryVariables = Exact<{ fileId: Scalars['ID']['input']; - workspaceId: Scalars['ID']['input']; documentId: Scalars['ID']['input']; + documentShareLinkToken?: InputMaybe; }>; @@ -2507,7 +2507,7 @@ export function useVerifyRegistrationMutation() { return Urql.useMutation(VerifyRegistrationDocument); }; export const CommentsByDocumentIdDocument = gql` - query commentsByDocumentId($documentId: ID!, $documentShareLinkToken: String, $first: Int = 50, $after: String) { + query commentsByDocumentId($documentId: ID!, $documentShareLinkToken: String, $first: Int = 200, $after: String) { commentsByDocumentId( documentId: $documentId documentShareLinkToken: $documentShareLinkToken @@ -2741,8 +2741,12 @@ export function useEncryptedWebDeviceQuery(options: Omit({ query: EncryptedWebDeviceDocument, ...options }); }; export const FileUrlDocument = gql` - query fileUrl($fileId: ID!, $workspaceId: ID!, $documentId: ID!) { - fileUrl(fileId: $fileId, workspaceId: $workspaceId, documentId: $documentId) { + query fileUrl($fileId: ID!, $documentId: ID!, $documentShareLinkToken: String) { + fileUrl( + fileId: $fileId + documentId: $documentId + documentShareLinkToken: $documentShareLinkToken + ) { id downloadUrl } diff --git a/apps/app/graphql/queries/commentsByDocumentId.graphql b/apps/app/graphql/queries/commentsByDocumentId.graphql index 3099f2af9..ea074d2ab 100644 --- a/apps/app/graphql/queries/commentsByDocumentId.graphql +++ b/apps/app/graphql/queries/commentsByDocumentId.graphql @@ -1,7 +1,7 @@ query commentsByDocumentId( $documentId: ID! $documentShareLinkToken: String - $first: Int = 50 + $first: Int = 200 $after: String ) { commentsByDocumentId( diff --git a/apps/app/graphql/queries/fileUrl.graphql b/apps/app/graphql/queries/fileUrl.graphql index c55a3b8cf..7e005d0e9 100644 --- a/apps/app/graphql/queries/fileUrl.graphql +++ b/apps/app/graphql/queries/fileUrl.graphql @@ -1,5 +1,9 @@ -query fileUrl($fileId: ID!, $workspaceId: ID!, $documentId: ID!) { - fileUrl(fileId: $fileId, workspaceId: $workspaceId, documentId: $documentId) { +query fileUrl($fileId: ID!, $documentId: ID!, $documentShareLinkToken: String) { + fileUrl( + fileId: $fileId + documentId: $documentId + documentShareLinkToken: $documentShareLinkToken + ) { id downloadUrl } diff --git a/apps/app/utils/file/createDownloadAndDecryptFileFunction.ts b/apps/app/utils/file/createDownloadAndDecryptFileFunction.ts index bf612696b..d97103fcc 100644 --- a/apps/app/utils/file/createDownloadAndDecryptFileFunction.ts +++ b/apps/app/utils/file/createDownloadAndDecryptFileFunction.ts @@ -2,8 +2,8 @@ import { runFileUrlQuery } from "../../generated/graphql"; import { decryptFile } from "./decryptFile"; type CreateDownloadAndDecryptFileFunctionProps = { - workspaceId: string; documentId: string; + documentShareLinkToken?: string; }; export type Props = { @@ -13,11 +13,11 @@ export type Props = { }; export const createDownloadAndDecryptFileFunction = ({ documentId, - workspaceId, + documentShareLinkToken, }: CreateDownloadAndDecryptFileFunctionProps) => { return async ({ fileId, publicNonce, key }: Props): Promise => { const result = await runFileUrlQuery( - { fileId, documentId, workspaceId }, + { fileId, documentId, documentShareLinkToken }, {} ); if (!result.data?.fileUrl?.downloadUrl) { diff --git a/apps/backend/src/database/file/getFileUrl.ts b/apps/backend/src/database/file/getFileUrl.ts index c9bc6e6ca..3dac1cb15 100644 --- a/apps/backend/src/database/file/getFileUrl.ts +++ b/apps/backend/src/database/file/getFileUrl.ts @@ -27,30 +27,48 @@ export type Props = { userId: string; fileId: string; documentId: string; - workspaceId: string; + documentShareLinkToken?: string | null | undefined; }; export const getFileUrl = async ({ userId, fileId, documentId, - workspaceId, + documentShareLinkToken, }: Props) => { - const user2Workspace = await prisma.usersToWorkspaces.findFirst({ - where: { userId, workspaceId }, - select: { role: true }, - }); - if (!user2Workspace) { - throw new ForbiddenError("Unauthorized"); - } + // verify the document exists const document = await prisma.document.findFirst({ - where: { id: documentId, workspaceId }, - select: { id: true }, + where: { id: documentId }, }); if (!document) { - throw new UserInputError("Invalid documentId"); + throw new ForbiddenError("Unauthorized"); + } + // if the user has a documentShareLinkToken, verify it + let documentShareLink: any = null; + if (documentShareLinkToken) { + documentShareLink = await prisma.documentShareLink.findFirst({ + where: { + token: documentShareLinkToken, + documentId, + }, + }); + if (!documentShareLink) { + throw new UserInputError("Invalid documentShareLinkToken"); + } + } else { + // if no documentShareLinkToken, the user must have access to the workspace + const user2Workspace = await prisma.usersToWorkspaces.findFirst({ + where: { + userId, + workspaceId: document.workspaceId, + }, + }); + if (!user2Workspace) { + throw new ForbiddenError("Unauthorized"); + } } - const fileName = `${workspaceId}/${documentId}/${fileId}.blob`; + + const fileName = `${document.workspaceId}/${documentId}/${fileId}.blob`; const downloadUrl = await getSignedUrl( s3Client, new GetObjectCommand({ diff --git a/apps/backend/src/graphql/queries/comment/commentsByDocumentId.test.ts b/apps/backend/src/graphql/queries/comment/commentsByDocumentId.test.ts index c23c7fe0e..08021a38f 100644 --- a/apps/backend/src/graphql/queries/comment/commentsByDocumentId.test.ts +++ b/apps/backend/src/graphql/queries/comment/commentsByDocumentId.test.ts @@ -175,7 +175,7 @@ test("too many", async () => { await commentsByDocumentId({ graphql, documentId: userData1.document.id, - first: 51, + first: 201, authorizationHeader: deriveSessionAuthorization({ sessionKey: userData1.sessionKey, }).authorization, diff --git a/apps/backend/src/graphql/queries/comment/commentsByDocumentId.ts b/apps/backend/src/graphql/queries/comment/commentsByDocumentId.ts index f5376c56f..9d802a7c6 100644 --- a/apps/backend/src/graphql/queries/comment/commentsByDocumentId.ts +++ b/apps/backend/src/graphql/queries/comment/commentsByDocumentId.ts @@ -13,9 +13,9 @@ export const commentsByDocumentIdQuery = queryField((t) => { documentShareLinkToken: stringArg(), }, async nodes(root, args, context) { - if (args.first && args.first > 50) { + if (args.first && args.first > 200) { throw new UserInputError( - "Requested too many devices. First value exceeds 50." + "Requested too many comments. First value exceeds 200." ); } if (!context.user && typeof args.documentShareLinkToken !== "string") { diff --git a/apps/backend/src/graphql/queries/file/fileUrl.test.ts b/apps/backend/src/graphql/queries/file/fileUrl.test.ts index b9ca8efe5..941711c4a 100644 --- a/apps/backend/src/graphql/queries/file/fileUrl.test.ts +++ b/apps/backend/src/graphql/queries/file/fileUrl.test.ts @@ -37,7 +37,6 @@ beforeAll(async () => { type Props = { graphql: TestContext; fileId: string; - workspaceId: string; documentId: string; authorization: string; }; @@ -45,16 +44,11 @@ const getFileUrl = async ({ graphql, fileId, documentId, - workspaceId, authorization, }: Props) => { const query = gql` - query fileUrl($fileId: ID!, $documentId: ID!, $workspaceId: ID!) { - fileUrl( - fileId: $fileId - documentId: $documentId - workspaceId: $workspaceId - ) { + query fileUrl($fileId: ID!, $documentId: ID!) { + fileUrl(fileId: $fileId, documentId: $documentId) { id downloadUrl } @@ -62,7 +56,7 @@ const getFileUrl = async ({ `; return graphql.client.request( query, - { fileId, documentId, workspaceId }, + { fileId, documentId }, { authorization } ); }; @@ -72,7 +66,6 @@ test("get file url", async () => { graphql, fileId: fileUploadData.fileId, documentId: userData.document.id, - workspaceId: userData.workspace.id, authorization: deriveSessionAuthorization({ sessionKey: userData.sessionKey, }).authorization, @@ -95,7 +88,6 @@ test("invalid access", async () => { graphql, fileId: fileUploadData.fileId, documentId: userData.document.id, - workspaceId: userData.workspace.id, authorization: deriveSessionAuthorization({ sessionKey: otherUser.sessionKey, }).authorization, @@ -110,7 +102,6 @@ test("Unauthenticated", async () => { graphql, fileId: fileUploadData.fileId, documentId: userData.document.id, - workspaceId: userData.workspace.id, authorization: "invalid-session-key", }))() ).rejects.toThrowError(/UNAUTHENTICATED/); @@ -139,14 +130,13 @@ describe("Input errors", () => { input: { fileId: null, documentId: userData.document.id, - workspaceId: userData.workspace.id, }, }, { authorization: userData.sessionKey } ))() ).rejects.toThrowError(); }); - test("Invalid document id", async () => { + test("Invalid file id", async () => { const userData = await createUserWithWorkspace({ username: `${generateId()}@example.com`, password, @@ -157,8 +147,8 @@ describe("Input errors", () => { query, { input: { + fileId: null, documentId: userData.document.id, - workspaceId: null, }, }, { authorization: userData.sessionKey } diff --git a/apps/backend/src/graphql/queries/file/fileUrl.ts b/apps/backend/src/graphql/queries/file/fileUrl.ts index 5a90ea712..a1cb7f1d7 100644 --- a/apps/backend/src/graphql/queries/file/fileUrl.ts +++ b/apps/backend/src/graphql/queries/file/fileUrl.ts @@ -1,5 +1,5 @@ import { AuthenticationError } from "apollo-server-express"; -import { idArg, nonNull, queryField } from "nexus"; +import { idArg, nonNull, queryField, stringArg } from "nexus"; import { getFileUrl } from "../../../database/file/getFileUrl"; import { File } from "../../types/file"; @@ -9,17 +9,17 @@ export const fileUrlQuery = queryField((t) => { args: { fileId: nonNull(idArg()), documentId: nonNull(idArg()), - workspaceId: nonNull(idArg()), + documentShareLinkToken: stringArg(), }, async resolve(root, args, context) { - if (!context.user) { + if (!context.user && typeof args.documentShareLinkToken !== "string") { throw new AuthenticationError("Not authenticated"); } const downloadUrl = await getFileUrl({ - userId: context.user.id, + userId: context.user?.id, fileId: args.fileId, documentId: args.documentId, - workspaceId: args.workspaceId, + documentShareLinkToken: args.documentShareLinkToken, }); return { id: args.fileId,