From d88feaaba8563b37c8780606c61e3bad44153d88 Mon Sep 17 00:00:00 2001 From: Anton Shepilov Date: Thu, 21 Sep 2023 14:08:57 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=9F=20Fix=20download=20button=20(#210)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add auth cookie to avoid using auth token in the url * Fix all download button * Refactor some of the places where we download file --- tdrive/backend/node/package-lock.json | 21 ++++++++++ tdrive/backend/node/package.json | 1 + .../core/platform/services/auth/web/jwt.ts | 6 +++ .../node/src/services/documents/web/routes.ts | 4 +- .../app/components/search-popup/common.tsx | 40 ------------------- .../components/search-popup/search-input.tsx | 22 +++++----- .../src/app/features/auth/auth-service.ts | 9 +++++ .../app/features/auth/jwt-storage-service.ts | 2 +- .../src/app/features/auth/login-service.ts | 15 +------ .../features/drive/api-client/api-client.ts | 18 ++------- .../app/features/files/hooks/use-upload.ts | 22 ---------- .../global/framework/requests-api-service.ts | 13 ------ .../views/client/body/drive/context-menu.tsx | 4 +- .../app/views/client/body/drive/shared.tsx | 9 +++-- 14 files changed, 61 insertions(+), 125 deletions(-) diff --git a/tdrive/backend/node/package-lock.json b/tdrive/backend/node/package-lock.json index a440435f..292f35ba 100644 --- a/tdrive/backend/node/package-lock.json +++ b/tdrive/backend/node/package-lock.json @@ -504,6 +504,22 @@ } } }, + "@fastify/cookie": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@fastify/cookie/-/cookie-6.0.0.tgz", + "integrity": "sha512-Luy3Po3dOJmqAuPCiPcWsX0tV5+C3AOnULSdlsGjNGOvyE7jqzysp8kT9ICfsUvove+TeUMgTWl1y9XS3ZPPMg==", + "requires": { + "cookie-signature": "^1.1.0", + "fastify-plugin": "^3.0.1" + }, + "dependencies": { + "fastify-plugin": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/fastify-plugin/-/fastify-plugin-3.0.1.tgz", + "integrity": "sha512-qKcDXmuZadJqdTm6vlCqioEbyewF60b/0LOFCcYN1B6BIZGlYJumWWOYs70SFYLDAH4YqdE1cxH/RKMG7rFxgA==" + } + } + }, "@fastify/error": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@fastify/error/-/error-2.0.0.tgz", @@ -3152,6 +3168,11 @@ "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.2.tgz", "integrity": "sha512-aSWTXFzaKWkvHO1Ny/s+ePFpvKsPnjc551iI41v3ny/ow6tBG5Vd+FuqGNhh1LxOmVzOlGUriIlOaokOvhaStA==" }, + "cookie-signature": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.2.1.tgz", + "integrity": "sha512-78KWk9T26NhzXtuL26cIJ8/qNHANyJ/ZYrmEXFzUmhZdjpBv+DlWlOANRTGBt48YcyslsLrj0bMLFTmXvLRCOw==" + }, "core-util-is": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.3.tgz", diff --git a/tdrive/backend/node/package.json b/tdrive/backend/node/package.json index 7cbd0b83..81eeb8e7 100644 --- a/tdrive/backend/node/package.json +++ b/tdrive/backend/node/package.json @@ -105,6 +105,7 @@ "@fastify/caching": "^7.0.0", "@fastify/formbody": "^6.0.0", "@fastify/static": "^5.0.1", + "@fastify/cookie": "^6.0.0", "@ffprobe-installer/ffprobe": "^1.4.1", "@sentry/node": "^6.19.7", "@sentry/tracing": "^6.19.7", diff --git a/tdrive/backend/node/src/core/platform/services/auth/web/jwt.ts b/tdrive/backend/node/src/core/platform/services/auth/web/jwt.ts index 8af60b6f..ee0c3661 100644 --- a/tdrive/backend/node/src/core/platform/services/auth/web/jwt.ts +++ b/tdrive/backend/node/src/core/platform/services/auth/web/jwt.ts @@ -1,12 +1,18 @@ import { FastifyPluginCallback, FastifyRequest } from "fastify"; import fastifyJwt from "fastify-jwt"; +import cookie from "@fastify/cookie"; import fp from "fastify-plugin"; import config from "../../../../config"; import { JwtType } from "../../types"; const jwtPlugin: FastifyPluginCallback = (fastify, _opts, next) => { + fastify.register(cookie); fastify.register(fastifyJwt, { secret: config.get("auth.jwt.secret"), + cookie: { + cookieName: "X-AuthToken", + signed: false, + }, }); const authenticate = async (request: FastifyRequest) => { diff --git a/tdrive/backend/node/src/services/documents/web/routes.ts b/tdrive/backend/node/src/services/documents/web/routes.ts index fa7890f4..ef5a8c82 100644 --- a/tdrive/backend/node/src/services/documents/web/routes.ts +++ b/tdrive/backend/node/src/services/documents/web/routes.ts @@ -76,14 +76,14 @@ const routes: FastifyPluginCallback = (fastify: FastifyInstance, _options, next) fastify.route({ method: "GET", url: `${serviceUrl}/:id/download`, - preValidation: [fastify.authenticateOptional], + preValidation: [fastify.authenticate], handler: documentsController.download.bind(documentsController), }); fastify.route({ method: "GET", url: `${serviceUrl}/download/zip`, - preValidation: [fastify.authenticateOptional], + preValidation: [fastify.authenticate], handler: documentsController.downloadZip.bind(documentsController), }); diff --git a/tdrive/frontend/src/app/components/search-popup/common.tsx b/tdrive/frontend/src/app/components/search-popup/common.tsx index 876d161f..a8c335c1 100644 --- a/tdrive/frontend/src/app/components/search-popup/common.tsx +++ b/tdrive/frontend/src/app/components/search-popup/common.tsx @@ -1,48 +1,8 @@ -import Strings from 'features/global/utils/strings'; -import FileUploadService from 'features/files/services/file-upload-service'; -import routerService from '@features/router/services/router-service'; import { DriveApiClient } from '@features/drive/api-client/api-client'; import { DriveItem } from '@features/drive/types'; -import { AttachedFileType } from '@features/files/types/file'; -export const highlightText = (text?: string, highlight?: string) => { - if (!text) { - return ''; - } - if (!highlight) { - return text; - } - const reg = new RegExp('(' + Strings.removeAccents(highlight) + ')', 'ig'); - return Strings.removeAccents(text).replace(reg, "$1"); -}; - -export const getFileMessageDownloadRoute = (file: AttachedFileType): string => { - if (file?.metadata?.source === 'internal') - return FileUploadService.getDownloadRoute({ - companyId: file.metadata?.external_id?.company_id, - fileId: file.metadata?.external_id?.id, - }); - return ''; -}; - -export const onFileDownloadClick = (file: AttachedFileType) => { - const url = getFileMessageDownloadRoute(file); - - url && (window.location.href = url); -}; - -export const openDriveItem = (driveItem: DriveItem, workspace_id: string, drive_app_id: string) => { - routerService.push( - routerService.generateRouteFromState({ - companyId: driveItem.company_id, - workspaceId: workspace_id, - channelId: drive_app_id, - }), - ); -}; export const onDriveItemDownloadClick = async (driveItem: DriveItem) => { const url = await DriveApiClient.getDownloadUrl(driveItem.company_id, driveItem.id); - url && (window.location.href = url); }; diff --git a/tdrive/frontend/src/app/components/search-popup/search-input.tsx b/tdrive/frontend/src/app/components/search-popup/search-input.tsx index 46c66148..b411bf55 100755 --- a/tdrive/frontend/src/app/components/search-popup/search-input.tsx +++ b/tdrive/frontend/src/app/components/search-popup/search-input.tsx @@ -1,12 +1,12 @@ -import { SearchIcon } from '@heroicons/react/solid'; -import { useEffect, useRef } from 'react'; -import { useRecoilState } from 'recoil'; -import { InputDecorationIcon } from '@atoms/input/input-decoration-icon'; -import { Input } from '@atoms/input/input-text'; -import { Loader } from '@atoms/loader'; -import Languages from '@features/global/services/languages-service'; -import { useSearchDriveItemsLoading } from '@features/search/hooks/use-search-drive-items'; -import { SearchInputState } from '@features/search/state/search-input'; +import { SearchIcon } from "@heroicons/react/solid"; +import { useEffect, useRef } from "react"; +import { useRecoilState } from "recoil"; +import { InputDecorationIcon } from "@atoms/input/input-decoration-icon"; +import { Input } from "@atoms/input/input-text"; +import { Loader } from "@atoms/loader"; +import Languages from "@features/global/services/languages-service"; +import { useSearchDriveItemsLoading } from "@features/search/hooks/use-search-drive-items"; +import { SearchInputState } from "@features/search/state/search-input"; export const SearchInput = () => { const [input, setInput] = useRecoilState(SearchInputState); @@ -16,9 +16,7 @@ export const SearchInput = () => { if (inputElement.current) inputElement.current.focus(); }, []); - const driveItemsLoading = useSearchDriveItemsLoading(); - - const loading = driveItemsLoading; + const loading = useSearchDriveItemsLoading(); return (
diff --git a/tdrive/frontend/src/app/features/auth/auth-service.ts b/tdrive/frontend/src/app/features/auth/auth-service.ts index 04d0ac8c..c6a29102 100644 --- a/tdrive/frontend/src/app/features/auth/auth-service.ts +++ b/tdrive/frontend/src/app/features/auth/auth-service.ts @@ -19,6 +19,7 @@ import UserAPIClient from '../../features/users/api/user-api-client'; import Application from '@features/applications/services/application-service'; import LocalStorage from '@features/global/framework/local-storage-service'; import Globals from '@features/global/services/globals-tdrive-app-service'; +import { Cookies } from 'react-cookie'; type AccountType = 'remote' | 'internal'; export type LoginState = '' | 'app' | 'error' | 'signin' | 'logged_out' | 'logout'; @@ -26,11 +27,16 @@ type InitState = '' | 'initializing' | 'initialized'; @TdriveService('AuthService') class AuthService { + + public static AUTH_TOKEN_COOKIE = "X-AuthToken"; + private provider: AuthProvider | null = null; private logger: Logger.Logger; private initState: InitState = ''; + private cookies: Cookies = new Cookies(); currentUserId = ''; + constructor() { this.logger = Logger.getLogger('AuthService'); } @@ -125,7 +131,9 @@ class AuthService { onNewToken(token?: JWTDataType): void { if (token) { + console.log("Save auth token to storage and cookie") JWT.updateJWT(token); + this.cookies.set(AuthService.AUTH_TOKEN_COOKIE, JWT.getJWT(), { path: "/" }); // TODO: Update the user from API? // this.updateUser(); } @@ -220,6 +228,7 @@ class AuthService { this.resetCurrentUser(); LocalStorage.clear(); JWT.clear(); + this.cookies.remove(AuthService.AUTH_TOKEN_COOKIE); } setCurrentUser(user: UserType) { diff --git a/tdrive/frontend/src/app/features/auth/jwt-storage-service.ts b/tdrive/frontend/src/app/features/auth/jwt-storage-service.ts index e2ec87e9..738659d5 100755 --- a/tdrive/frontend/src/app/features/auth/jwt-storage-service.ts +++ b/tdrive/frontend/src/app/features/auth/jwt-storage-service.ts @@ -134,7 +134,7 @@ class JWTStorage { this.logger.debug('authenticateCall: Updating user because the access token expired'); this.renew() .then(() => { - LoginService.updateUser(callback); + return LoginService.updateUser(callback); }) .catch(async () => { this.clear(); diff --git a/tdrive/frontend/src/app/features/auth/login-service.ts b/tdrive/frontend/src/app/features/auth/login-service.ts index 4499ed29..4a002762 100755 --- a/tdrive/frontend/src/app/features/auth/login-service.ts +++ b/tdrive/frontend/src/app/features/auth/login-service.ts @@ -11,8 +11,6 @@ import Application from '../applications/services/application-service'; import { UserType } from '@features/users/types/user'; import { Cookies } from 'react-cookie'; import InitService from '../global/services/init-service'; -import { useRecoilState } from "recoil"; -import { CurrentUserState } from "features/users/state/atoms/current-user"; class Login extends Observable { @@ -39,6 +37,7 @@ class Login extends Observable { error_code: any; cookies: Cookies; + // eslint-disable-next-line @typescript-eslint/no-unused-vars recoilUpdateUser = (user: UserType | undefined) => {}; @@ -224,18 +223,6 @@ class Login extends Observable { this.userIsSet = new Promise(resolve => (this.resolveUser = resolve)); } - getIsPublicAccess() { - let publicAccess = false; - const viewParameter = WindowState.findGetParameter('view') || ''; - if ( - (viewParameter && ['drive_publicAccess'].indexOf(viewParameter) >= 0) || - Globals.store_public_access_get_data - ) { - publicAccess = true; - Globals.store_public_access_get_data = WindowState.allGetParameter(); - } - return publicAccess; - } } export default new Login(); diff --git a/tdrive/frontend/src/app/features/drive/api-client/api-client.ts b/tdrive/frontend/src/app/features/drive/api-client/api-client.ts index 3f5aab31..f3236ccb 100644 --- a/tdrive/frontend/src/app/features/drive/api-client/api-client.ts +++ b/tdrive/frontend/src/app/features/drive/api-client/api-client.ts @@ -111,22 +111,10 @@ export class DriveApiClient { ); } - static async getDownloadToken(companyId: string, ids: string[], versionId?: string) { - return Api.get<{ token: string }>( - `/internal/services/documents/v1/companies/${companyId}/item/download/token` + - `?items=${ids.join(',')}&version_id=${versionId}` + - appendTdriveToken(true), - ); - } - - static async getDownloadUrl(companyId: string, id: string, versionId?: string) { + static getDownloadUrl(companyId: string, id: string, versionId?: string) { if (versionId) - return Api.route( - `/internal/services/files/v1/companies/${companyId}/files/${id}/download?version_id=${versionId}`, - ); - return Api.route( - `/internal/services/files/v1/companies/${companyId}/files/${id}/download`, - ); + return Api.route(`/internal/services/documents/v1/companies/${companyId}/item/${id}/download?version_id=${versionId}`); + return Api.route(`/internal/services/documents/v1/companies/${companyId}/item/${id}/download`); } static async getDownloadZipUrl(companyId: string, ids: string[]) { diff --git a/tdrive/frontend/src/app/features/files/hooks/use-upload.ts b/tdrive/frontend/src/app/features/files/hooks/use-upload.ts index 45a596db..a03d50a1 100644 --- a/tdrive/frontend/src/app/features/files/hooks/use-upload.ts +++ b/tdrive/frontend/src/app/features/files/hooks/use-upload.ts @@ -21,27 +21,6 @@ export const useUpload = () => { if (companyId) FileUploadService.deleteOneFile({ companyId, fileId: id }); }; - const downloadOneFile = ({ - companyId, - fileId, - blob, - }: { - companyId: string; - fileId: string; - blob?: boolean; - }) => { - if (blob) { - return FileUploadService.download({ companyId, fileId }); - } - - const url = FileUploadService.getDownloadRoute({ - companyId, - fileId, - }); - - url && (window.location.href = url); - }; - const retryUpload = (id: string) => FileUploadService.retry(id); return { @@ -51,7 +30,6 @@ export const useUpload = () => { getOnePendingFile, currentTask, deleteOneFile, - downloadOneFile, retryUpload, }; }; diff --git a/tdrive/frontend/src/app/features/global/framework/requests-api-service.ts b/tdrive/frontend/src/app/features/global/framework/requests-api-service.ts index c45f1389..b08413a2 100755 --- a/tdrive/frontend/src/app/features/global/framework/requests-api-service.ts +++ b/tdrive/frontend/src/app/features/global/framework/requests-api-service.ts @@ -31,12 +31,10 @@ class Requests { .then(response => { if (options.withBlob) { response.blob().then(blob => { - this.retrieveJWTToken(JSON.stringify(blob)); callback && callback(blob); }); } else { response.text().then(text => { - if (text) this.retrieveJWTToken(text); callback && callback(text); }); } @@ -54,17 +52,6 @@ class Requests { this.request(type, route, data, callback, options); }); } - - retrieveJWTToken(rawBody: string) { - try { - const body = JSON.parse(rawBody); - if (body.access_token) { - JWTStorage.updateJWT(body.access_token); - } - } catch (err) { - console.error('Error while reading jwt tokens from: ' + rawBody, err); - } - } } export default new Requests(); diff --git a/tdrive/frontend/src/app/views/client/body/drive/context-menu.tsx b/tdrive/frontend/src/app/views/client/body/drive/context-menu.tsx index bf6b2403..3bd7772d 100644 --- a/tdrive/frontend/src/app/views/client/body/drive/context-menu.tsx +++ b/tdrive/frontend/src/app/views/client/body/drive/context-menu.tsx @@ -86,7 +86,7 @@ export const useOnBuildContextMenu = (children: DriveItem[], initialParentId?: s downloadZip([item!.id]); console.log(item!.id); } else { - download(item.last_version_cache.file_metadata.external_id); + download(item.id); } } }, @@ -286,7 +286,7 @@ export const useOnBuildContextMenu = (children: DriveItem[], initialParentId?: s downloadZip(idsFromArray); } else if (parent.item) { console.log("Download folder itself"); - download(parent.item.last_version_cache.file_metadata.external_id); + download(parent.item.id); } else { console.error("Very strange, everything is null, you are trying to download undefined"); } diff --git a/tdrive/frontend/src/app/views/client/body/drive/shared.tsx b/tdrive/frontend/src/app/views/client/body/drive/shared.tsx index cbf5fcad..6a918173 100755 --- a/tdrive/frontend/src/app/views/client/body/drive/shared.tsx +++ b/tdrive/frontend/src/app/views/client/body/drive/shared.tsx @@ -16,6 +16,8 @@ import { ToasterService } from 'app/features/global/services/toaster-service'; import { useParams } from 'react-router-dom'; import shortUUID from 'short-uuid'; import Avatar from '../../../../atoms/avatar'; +import AuthService from '@features/auth/auth-service'; + import { DriveApiClient, setPublicLinkToken, @@ -122,12 +124,11 @@ const AccessChecker = ({ throw new Error('Invalid password or token, or expired link.'); } - JWTStorage.updateJWT(access_token); + AuthService.onNewToken(access_token); - //Everything alright, load the drive setAccessGranted(true); - refresh(folderId); - refreshApplications(); + await refresh(folderId); + await refreshApplications(); } catch (e) { console.error(e); ToasterService.error('Unable to access documents: ' + e);