From 730b815f72381f78b1e4ea5a27ffa4de7bbf5a54 Mon Sep 17 00:00:00 2001 From: Thomas Lathuiliere Date: Tue, 27 Feb 2024 15:20:33 +0100 Subject: [PATCH] feat(auth): improve error handling --- .../app-builder/public/locales/en/cases.json | 3 +- .../app-builder/public/locales/en/common.json | 3 ++ .../public/locales/en/scheduledExecution.json | 3 +- .../src/components/Auth/PopupBlockedError.tsx | 41 +++++++++++++++++ .../Auth/SignInWithEmailAndPassword.tsx | 3 ++ .../src/components/Auth/SignInWithGoogle.tsx | 8 ++++ .../components/Auth/SignInWithMicrosoft.tsx | 8 ++++ .../Auth/SignUpWithEmailAndPassword.tsx | 3 ++ .../src/components/Cases/CaseFiles.tsx | 10 +++- .../src/components/MarbleToaster.tsx | 3 +- .../ScheduledExecutionDetails.tsx | 12 ++++- .../routes/_builder+/upload+/$objectType.tsx | 14 +++++- .../routes/ressources+/cases+/upload-file.tsx | 18 ++++---- .../src/services/DownloadCaseFilesService.ts | 16 +++++-- .../src/services/DownloadDecisionsService.ts | 17 +++++-- .../src/services/auth/auth.client.ts | 46 +++++++++++++------ packages/app-builder/src/utils/browser.ts | 20 ++++++++ 17 files changed, 191 insertions(+), 37 deletions(-) create mode 100644 packages/app-builder/src/components/Auth/PopupBlockedError.tsx create mode 100644 packages/app-builder/src/utils/browser.ts diff --git a/packages/app-builder/public/locales/en/cases.json b/packages/app-builder/public/locales/en/cases.json index a4751be18..e4f2b2054 100644 --- a/packages/app-builder/public/locales/en/cases.json +++ b/packages/app-builder/public/locales/en/cases.json @@ -11,7 +11,8 @@ "case.file.added_date": "Added on", "case.file.download": "Download", "case.file.downloading": "Downloading...", - "case.file.errors.downloading_decisions_link": "An unknown error as occured while generating the download link. Please try again later.", + "case.file.errors.downloading_decisions_link.auth_error": "An auth error occured while generating the download link. You may need to log in again.", + "case.file.errors.downloading_decisions_link.unknown": "An unknown error as occured while generating the download link. Please try again later.", "case.inbox": "Inbox", "case.inboxes": "Inboxes", "case.status.open": "open", diff --git a/packages/app-builder/public/locales/en/common.json b/packages/app-builder/public/locales/en/common.json index b6789c170..e44705254 100644 --- a/packages/app-builder/public/locales/en/common.json +++ b/packages/app-builder/public/locales/en/common.json @@ -9,6 +9,7 @@ "validation_error_other": "{{count}} validation errors", "errors.unknown": "An unknown error as occured", "errors.account_exists_with_different_credential": "An account already exists with the same email address but different sign-in credentials. Sign in using a provider associated with this email address.", + "errors.popup_blocked_by_client": "The popup has been blocked. Please enable popups and try again.", "errors.not_found": "This page could not be found.", "errors.edit.forbidden_not_draft": "You can only edit a draft version of a scenario.", "errors.list.duplicate_list_name": "A list with this name already exist", @@ -17,6 +18,8 @@ "errors.data.duplicate_table_name": "A table with this name already exist", "errors.data.duplicate_link_name": "A link with this name already exist", "errors.add_to_case.invalid": "A decision already belongs to a case", + "errors.firebase_auth_error": "An auth error occured. Try to log in again.", + "errors.firebase_network_error": "A network error occured. Try to log in again.", "cancel": "Cancel", "save": "Save", "delete": "Delete", diff --git a/packages/app-builder/public/locales/en/scheduledExecution.json b/packages/app-builder/public/locales/en/scheduledExecution.json index 0e3044684..9d67d25b9 100644 --- a/packages/app-builder/public/locales/en/scheduledExecution.json +++ b/packages/app-builder/public/locales/en/scheduledExecution.json @@ -8,7 +8,8 @@ "finished_at": "Finished at", "download_decisions": "Download Decisions", "downloading_decisions": "Downloading decisions...", - "errors.downloading_decisions_link": "An unknown error as occured while generating the download link. Please try again later.", + "errors.downloading_decisions_link.auth_error": "An auth error occured while generating the download link. You may need to log in again.", + "errors.downloading_decisions_link.unknown": "An unknown error as occured while generating the download link. Please try again later.", "status_pending": "Pending", "status_success": "Success", "status_failure": "Failure" diff --git a/packages/app-builder/src/components/Auth/PopupBlockedError.tsx b/packages/app-builder/src/components/Auth/PopupBlockedError.tsx new file mode 100644 index 000000000..c443fb12d --- /dev/null +++ b/packages/app-builder/src/components/Auth/PopupBlockedError.tsx @@ -0,0 +1,41 @@ +import { getCurrentBrowser } from '@app-builder/utils/browser'; +import { Trans, useTranslation } from 'react-i18next'; + +export function PopupBlockedError() { + const { t } = useTranslation(['common']); + return ( +
+ , + }} + /> +
+ ); +} + +const hrefMap = { + Safari: 'https://support.apple.com/guide/safari/sfri40696/mac', + Firefox: + 'https://support.mozilla.org/en-US/kb/pop-blocker-settings-exceptions-troubleshooting', + Chrome: 'https://support.google.com/chrome/answer/95472', +}; + +function EnablePopup({ children }: { children?: React.ReactNode }) { + const browser = getCurrentBrowser(navigator.userAgent); + if (browser in hrefMap) { + return ( + + {children} + + ); + } + return {children}; +} diff --git a/packages/app-builder/src/components/Auth/SignInWithEmailAndPassword.tsx b/packages/app-builder/src/components/Auth/SignInWithEmailAndPassword.tsx index 35fc19e16..3f9da47e7 100644 --- a/packages/app-builder/src/components/Auth/SignInWithEmailAndPassword.tsx +++ b/packages/app-builder/src/components/Auth/SignInWithEmailAndPassword.tsx @@ -8,6 +8,7 @@ import { import { EmailUnverified, InvalidLoginCredentials, + NetworkRequestFailed, useEmailAndPasswordSignIn, UserNotFoundError, WrongPasswordError, @@ -164,6 +165,8 @@ function ClientSignInWithEmailAndPasswordForm({ setError('credentials', { message: t('auth:sign_in.errors.invalid_login_credentials'), }); + } else if (error instanceof NetworkRequestFailed) { + toast.error(t('common:errors.firebase_network_error')); } else { Sentry.captureException(error); toast.error(t('common:errors.unknown')); diff --git a/packages/app-builder/src/components/Auth/SignInWithGoogle.tsx b/packages/app-builder/src/components/Auth/SignInWithGoogle.tsx index 337a0bedb..a16d15e2e 100644 --- a/packages/app-builder/src/components/Auth/SignInWithGoogle.tsx +++ b/packages/app-builder/src/components/Auth/SignInWithGoogle.tsx @@ -1,5 +1,7 @@ import { AccountExistsWithDifferentCredential, + NetworkRequestFailed, + PopupBlockedByClient, useGoogleSignIn, } from '@app-builder/services/auth/auth.client'; import { type AuthPayload } from '@app-builder/services/auth/auth.server'; @@ -10,6 +12,8 @@ import { useTranslation } from 'react-i18next'; import { ClientOnly } from 'remix-utils/client-only'; import { Logo } from 'ui-icons'; +import { PopupBlockedError } from './PopupBlockedError'; + function SignInWithGoogleButton({ onClick }: { onClick?: () => void }) { const { t } = useTranslation(['auth']); @@ -52,6 +56,10 @@ function ClientSignInWithGoogle({ toast.error( t('common:errors.account_exists_with_different_credential'), ); + } else if (error instanceof PopupBlockedByClient) { + toast.error(); + } else if (error instanceof NetworkRequestFailed) { + toast.error(t('common:errors.firebase_network_error')); } else { Sentry.captureException(error); toast.error(t('common:errors.unknown')); diff --git a/packages/app-builder/src/components/Auth/SignInWithMicrosoft.tsx b/packages/app-builder/src/components/Auth/SignInWithMicrosoft.tsx index 865bb135e..d65ab06c2 100644 --- a/packages/app-builder/src/components/Auth/SignInWithMicrosoft.tsx +++ b/packages/app-builder/src/components/Auth/SignInWithMicrosoft.tsx @@ -1,5 +1,7 @@ import { AccountExistsWithDifferentCredential, + NetworkRequestFailed, + PopupBlockedByClient, useMicrosoftSignIn, } from '@app-builder/services/auth/auth.client'; import { type AuthPayload } from '@app-builder/services/auth/auth.server'; @@ -10,6 +12,8 @@ import { useTranslation } from 'react-i18next'; import { ClientOnly } from 'remix-utils/client-only'; import { Logo } from 'ui-icons'; +import { PopupBlockedError } from './PopupBlockedError'; + function SignInWithMicrosoftButton({ onClick }: { onClick?: () => void }) { const { t } = useTranslation(['auth']); @@ -52,6 +56,10 @@ function ClientSignInWithMicrosoft({ toast.error( t('common:errors.account_exists_with_different_credential'), ); + } else if (error instanceof PopupBlockedByClient) { + toast.error(); + } else if (error instanceof NetworkRequestFailed) { + toast.error(t('common:errors.firebase_network_error')); } else { Sentry.captureException(error); toast.error(t('common:errors.unknown')); diff --git a/packages/app-builder/src/components/Auth/SignUpWithEmailAndPassword.tsx b/packages/app-builder/src/components/Auth/SignUpWithEmailAndPassword.tsx index 3b3738dfd..6959426c9 100644 --- a/packages/app-builder/src/components/Auth/SignUpWithEmailAndPassword.tsx +++ b/packages/app-builder/src/components/Auth/SignUpWithEmailAndPassword.tsx @@ -7,6 +7,7 @@ import { } from '@app-builder/components/Form'; import { EmailExistsError, + NetworkRequestFailed, useEmailAndPasswordSignUp, WeakPasswordError, } from '@app-builder/services/auth/auth.client'; @@ -144,6 +145,8 @@ function ClientSignUpWithEmailAndPasswordForm({ }, { shouldFocus: true }, ); + } else if (error instanceof NetworkRequestFailed) { + toast.error(t('common:errors.firebase_network_error')); } else { Sentry.captureException(error); toast.error(t('common:errors.unknown')); diff --git a/packages/app-builder/src/components/Cases/CaseFiles.tsx b/packages/app-builder/src/components/Cases/CaseFiles.tsx index ff230f643..e646e31ec 100644 --- a/packages/app-builder/src/components/Cases/CaseFiles.tsx +++ b/packages/app-builder/src/components/Cases/CaseFiles.tsx @@ -1,5 +1,6 @@ import { AlreadyDownloadingError, + AuthRequestError, useDownloadCaseFiles, } from '@app-builder/services/DownloadCaseFilesService'; import { formatDateTime, useFormatLanguage } from '@app-builder/utils/format'; @@ -112,8 +113,15 @@ function FileLink({ caseFileId }: { caseFileId: string }) { if (e instanceof AlreadyDownloadingError) { // Already downloading, do nothing return; + } else if (e instanceof AuthRequestError) { + toast.error( + t('cases:case.file.errors.downloading_decisions_link.auth_error'), + ); + } else { + toast.error( + t('cases:case.file.errors.downloading_decisions_link.unknown'), + ); } - toast.error(t('cases:case.file.errors.downloading_decisions_link')); }, }, ); diff --git a/packages/app-builder/src/components/MarbleToaster.tsx b/packages/app-builder/src/components/MarbleToaster.tsx index 0e61eb098..b520db651 100644 --- a/packages/app-builder/src/components/MarbleToaster.tsx +++ b/packages/app-builder/src/components/MarbleToaster.tsx @@ -74,8 +74,9 @@ export function MarbleToaster({ ) : null} diff --git a/packages/app-builder/src/components/ScheduledExecutions/ScheduledExecutionDetails.tsx b/packages/app-builder/src/components/ScheduledExecutions/ScheduledExecutionDetails.tsx index 1ee93663a..02f036177 100644 --- a/packages/app-builder/src/components/ScheduledExecutions/ScheduledExecutionDetails.tsx +++ b/packages/app-builder/src/components/ScheduledExecutions/ScheduledExecutionDetails.tsx @@ -1,5 +1,6 @@ import { AlreadyDownloadingError, + AuthRequestError, useDownloadDecisions, } from '@app-builder/services/DownloadDecisionsService'; import { toast } from 'react-hot-toast'; @@ -37,8 +38,17 @@ function ScheduledExecutionDetailsInternal({ if (e instanceof AlreadyDownloadingError) { // Already downloading, do nothing return; + } else if (e instanceof AuthRequestError) { + toast.error( + t( + 'scheduledExecution:errors.downloading_decisions_link.auth_error', + ), + ); + } else { + toast.error( + t('scheduledExecution:errors.downloading_decisions_link.unknown'), + ); } - toast.error(t('scheduledExecution:errors.downloading_decisions_link')); }, }, ); diff --git a/packages/app-builder/src/routes/_builder+/upload+/$objectType.tsx b/packages/app-builder/src/routes/_builder+/upload+/$objectType.tsx index 7df12aa03..c885ac645 100644 --- a/packages/app-builder/src/routes/_builder+/upload+/$objectType.tsx +++ b/packages/app-builder/src/routes/_builder+/upload+/$objectType.tsx @@ -66,7 +66,7 @@ const UploadForm = ({ objectType }: { objectType: string }) => { }); const revalidator = useRevalidator(); - const { accessToken, backendUrl } = useBackendInfo( + const { getAccessToken, backendUrl } = useBackendInfo( clientServices.authenticationClientService, ); @@ -108,13 +108,23 @@ const UploadForm = ({ objectType }: { objectType: string }) => { const formData = new FormData(); formData.append('file', file); + const tokenResponse = await getAccessToken(); + if (!tokenResponse.success) { + setIsModalOpen(true); + computeModalMessage({ + success: false, + errorMessage: t('common:errors.firebase_auth_error'), + }); + return; + } + const response = await fetch( `${backendUrl}/ingestion/${objectType}/batch`, { method: 'POST', body: formData, headers: { - Authorization: `Bearer ${await accessToken()}`, + Authorization: `Bearer ${tokenResponse.accessToken}`, }, }, ); diff --git a/packages/app-builder/src/routes/ressources+/cases+/upload-file.tsx b/packages/app-builder/src/routes/ressources+/cases+/upload-file.tsx index 79a5b63a8..6f3aff6ea 100644 --- a/packages/app-builder/src/routes/ressources+/cases+/upload-file.tsx +++ b/packages/app-builder/src/routes/ressources+/cases+/upload-file.tsx @@ -47,10 +47,6 @@ export function UploadFile({ caseDetail }: { caseDetail: CaseDetail }) { ); } -function toastError(error: string): void { - toast(error, { icon: '❌' }); -} - function UploadFileContent({ caseDetail, setOpen, @@ -62,7 +58,7 @@ function UploadFileContent({ const [loading, setLoading] = useState(false); const revalidator = useRevalidator(); - const { accessToken, backendUrl } = useBackendInfo( + const { getAccessToken, backendUrl } = useBackendInfo( clientServices.authenticationClientService, ); @@ -94,6 +90,12 @@ function UploadFileContent({ const file = acceptedFiles[0]; try { setLoading(true); + const tokenResponse = await getAccessToken(); + if (!tokenResponse.success) { + toast.error(t('common:errors.firebase_auth_error')); + return; + } + const formData = new FormData(); formData.append('file', file); @@ -103,13 +105,13 @@ function UploadFileContent({ method: 'POST', body: formData, headers: { - Authorization: `Bearer ${await accessToken()}`, + Authorization: `Bearer ${tokenResponse.accessToken}`, }, }, ); if (!response.ok) { Sentry.captureException(await response.text()); - toastError('An error occurred while trying to upload the file.'); + toast.error('An error occurred while trying to upload the file.'); return; } @@ -117,7 +119,7 @@ function UploadFileContent({ setOpen(false); } catch (error) { Sentry.captureException(error); - toastError('An error occurred while trying to upload the file.'); + toast.error('An error occurred while trying to upload the file.'); } finally { setLoading(false); } diff --git a/packages/app-builder/src/services/DownloadCaseFilesService.ts b/packages/app-builder/src/services/DownloadCaseFilesService.ts index b621ccd14..eb61d28a9 100644 --- a/packages/app-builder/src/services/DownloadCaseFilesService.ts +++ b/packages/app-builder/src/services/DownloadCaseFilesService.ts @@ -8,11 +8,13 @@ import { clientServices } from './init.client'; export class AlreadyDownloadingError extends Error {} export class FetchLinkError extends Error {} +export class AuthRequestError extends Error {} type DownloadFileError = | AlreadyDownloadingError | FetchLinkError | DownloadError - | UnknownError; + | UnknownError + | AuthRequestError; const fileDownloadUrlSchema = z.object({ url: z.string(), @@ -23,7 +25,7 @@ export function useDownloadCaseFiles( { onError }: { onError?: (error: DownloadFileError) => void } = {}, ) { const [downloading, setDownloading] = useState(false); - const { backendUrl, accessToken } = useBackendInfo( + const { backendUrl, getAccessToken } = useBackendInfo( clientServices.authenticationClientService, ); @@ -36,13 +38,18 @@ export function useDownloadCaseFiles( } setDownloading(true); + const tokenResponse = await getAccessToken(); + if (!tokenResponse.success) { + throw new AuthRequestError(); + } + const downloadLink = `${backendUrl}/cases/files/${encodeURIComponent( caseFileId, )}/download_link`; const response = await fetch(downloadLink, { method: 'GET', headers: { - Authorization: `Bearer ${await accessToken()}`, + Authorization: `Bearer ${tokenResponse.accessToken}`, }, }); @@ -57,7 +64,8 @@ export function useDownloadCaseFiles( if ( error instanceof AlreadyDownloadingError || error instanceof FetchLinkError || - error instanceof DownloadError + error instanceof DownloadError || + error instanceof AuthRequestError ) { onError?.(error); } else { diff --git a/packages/app-builder/src/services/DownloadDecisionsService.ts b/packages/app-builder/src/services/DownloadDecisionsService.ts index 2f4835769..06be9c6ca 100644 --- a/packages/app-builder/src/services/DownloadDecisionsService.ts +++ b/packages/app-builder/src/services/DownloadDecisionsService.ts @@ -7,18 +7,20 @@ import { clientServices } from './init.client'; export class AlreadyDownloadingError extends Error {} export class FetchLinkError extends Error {} +export class AuthRequestError extends Error {} type DownloadDecisionsError = | AlreadyDownloadingError | FetchLinkError | DownloadError - | UnknownError; + | UnknownError + | AuthRequestError; export function useDownloadDecisions( scheduleExecutionId: string, { onError }: { onError?: (error: DownloadDecisionsError) => void } = {}, ) { const [downloading, setDownloading] = useState(false); - const { backendUrl, accessToken } = useBackendInfo( + const { backendUrl, getAccessToken } = useBackendInfo( clientServices.authenticationClientService, ); @@ -31,13 +33,19 @@ export function useDownloadDecisions( } setDownloading(true); + const tokenResponse = await getAccessToken(); + if (!tokenResponse.success) { + throw new AuthRequestError(); + } + const downloadLink = `${backendUrl}/scheduled-executions/${encodeURIComponent( scheduleExecutionId, )}/decisions.zip`; + const response = await fetch(downloadLink, { method: 'GET', headers: { - Authorization: `Bearer ${await accessToken()}`, + Authorization: `Bearer ${tokenResponse.accessToken}`, }, }); @@ -54,7 +62,8 @@ export function useDownloadDecisions( if ( error instanceof AlreadyDownloadingError || error instanceof FetchLinkError || - error instanceof DownloadError + error instanceof DownloadError || + error instanceof AuthRequestError ) { onError?.(error); } else { diff --git a/packages/app-builder/src/services/auth/auth.client.ts b/packages/app-builder/src/services/auth/auth.client.ts index d18311c87..9e69825a4 100644 --- a/packages/app-builder/src/services/auth/auth.client.ts +++ b/packages/app-builder/src/services/auth/auth.client.ts @@ -41,6 +41,10 @@ export function useGoogleSignIn({ return; case AuthErrorCodes.NEED_CONFIRMATION: throw new AccountExistsWithDifferentCredential(); + case AuthErrorCodes.POPUP_BLOCKED: + throw new PopupBlockedByClient(); + case AuthErrorCodes.NETWORK_REQUEST_FAILED: + throw new NetworkRequestFailed(); } } throw error; @@ -71,6 +75,10 @@ export function useMicrosoftSignIn({ return; case AuthErrorCodes.NEED_CONFIRMATION: throw new AccountExistsWithDifferentCredential(); + case AuthErrorCodes.POPUP_BLOCKED: + throw new PopupBlockedByClient(); + case AuthErrorCodes.NETWORK_REQUEST_FAILED: + throw new NetworkRequestFailed(); } } throw error; @@ -79,6 +87,8 @@ export function useMicrosoftSignIn({ } export class AccountExistsWithDifferentCredential extends Error {} +export class PopupBlockedByClient extends Error {} +export class NetworkRequestFailed extends Error {} export function useEmailAndPasswordSignIn({ authenticationClientRepository, @@ -109,6 +119,8 @@ export function useEmailAndPasswordSignIn({ throw new WrongPasswordError(); case AuthErrorCodes.INVALID_LOGIN_CREDENTIALS: throw new InvalidLoginCredentials(); + case AuthErrorCodes.NETWORK_REQUEST_FAILED: + throw new NetworkRequestFailed(); } } } @@ -144,6 +156,8 @@ export function useEmailAndPasswordSignUp({ throw new EmailExistsError(); case AuthErrorCodes.WEAK_PASSWORD: throw new WeakPasswordError(); + case AuthErrorCodes.NETWORK_REQUEST_FAILED: + throw new NetworkRequestFailed(); } } throw error; @@ -189,23 +203,27 @@ export function useBackendInfo({ }: AuthenticationClientService) { const backendUrl = getClientEnv('MARBLE_API_DOMAIN'); - const accessToken = async () => { - const firebaseIdToken = - await authenticationClientRepository.firebaseIdToken(); - const token = await marbleApi.postToken( - { - authorization: `Bearer ${firebaseIdToken}`, - }, - { - baseUrl: backendUrl, - }, - ); - - return token.access_token; + const getAccessToken = async () => { + try { + const firebaseIdToken = + await authenticationClientRepository.firebaseIdToken(); + const token = await marbleApi.postToken( + { + authorization: `Bearer ${firebaseIdToken}`, + }, + { + baseUrl: backendUrl, + }, + ); + + return { accessToken: token.access_token, success: true as const }; + } catch (error) { + return { error, success: false as const }; + } }; return { - accessToken, + getAccessToken, backendUrl, }; } diff --git a/packages/app-builder/src/utils/browser.ts b/packages/app-builder/src/utils/browser.ts new file mode 100644 index 000000000..cf0bd6cb5 --- /dev/null +++ b/packages/app-builder/src/utils/browser.ts @@ -0,0 +1,20 @@ +export function getCurrentBrowser(userAgent: string) { + if (userAgent.includes('Chrome')) { + return 'Chrome'; + } else if (userAgent.includes('Firefox')) { + return 'Firefox'; + } else if (userAgent.includes('Safari')) { + return 'Safari'; + } else if (userAgent.includes('Edge')) { + return 'Edge'; + } else if (userAgent.includes('Opera')) { + return 'Opera'; + } else { + return 'Unknown Browser'; + } +} + +export type KnownBrowser = Exclude< + ReturnType, + 'Unknown Browser' +>;