Skip to content

Commit

Permalink
Merge pull request #53407 from Kalydosos/fix-51888-cors-errors-are-di…
Browse files Browse the repository at this point in the history
…splayed-for-attachments

Fix 51888 cors errors are displayed for attachments
  • Loading branch information
rlinoz authored Jan 8, 2025
2 parents 235f6b2 + f026b07 commit ab92780
Show file tree
Hide file tree
Showing 18 changed files with 236 additions and 40 deletions.
3 changes: 3 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,8 @@ const CONST = {
UNKNOWN: 'unknown',
},
},
// The number of milliseconds for an idle session to expire
SESSION_EXPIRATION_TIME_MS: 2 * 3600 * 1000, // 2 hours
WEEK_STARTS_ON: 1, // Monday
DEFAULT_TIME_ZONE: {automatic: true, selected: 'America/Los_Angeles'},
DEFAULT_ACCOUNT_DATA: {errors: null, success: '', isLoading: false},
Expand Down Expand Up @@ -1560,6 +1562,7 @@ const CONST = {
ATTACHMENT_PREVIEW_ATTRIBUTE: 'src',
ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE: 'data-name',
ATTACHMENT_LOCAL_URL_PREFIX: ['blob:', 'file:'],
ATTACHMENT_OR_RECEIPT_LOCAL_URL: /^https:\/\/(www\.)?([a-z0-9_-]+\.)*expensify.com(:[0-9]+)?\/(chat-attachments|receipts)/,
ATTACHMENT_THUMBNAIL_URL_ATTRIBUTE: 'data-expensify-thumbnail-url',
ATTACHMENT_THUMBNAIL_WIDTH_ATTRIBUTE: 'data-expensify-width',
ATTACHMENT_THUMBNAIL_HEIGHT_ATTRIBUTE: 'data-expensify-height',
Expand Down
2 changes: 1 addition & 1 deletion src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ const ROUTES = {
ATTACHMENTS: {
route: 'attachment',
getRoute: (
reportID: string,
reportID: string | undefined,
type: ValueOf<typeof CONST.ATTACHMENT_TYPE>,
url: string,
accountID?: number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function extractAttachments(
}

if (name === 'img' && attribs.src) {
const expensifySource = attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE];
const expensifySource = attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] ?? (new RegExp(CONST.ATTACHMENT_OR_RECEIPT_LOCAL_URL, 'i').test(attribs.src) ? attribs.src : null);
const source = tryResolveUrlFromApiRoot(expensifySource || attribs.src);
const previewSource = tryResolveUrlFromApiRoot(attribs.src);
const sourceLinkKey = `${source}|${currentLink}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ function ImageRenderer({tnode}: ImageRendererProps) {
// Concierge responder attachments are uploaded to S3 without any access
// control and thus require no authToken to verify access.
//
const attachmentSourceAttribute = htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE];
const attachmentSourceAttribute =
htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] ?? (new RegExp(CONST.ATTACHMENT_OR_RECEIPT_LOCAL_URL, 'i').test(htmlAttribs.src) ? htmlAttribs.src : null);
const isAttachmentOrReceipt = !!attachmentSourceAttribute;

// Files created/uploaded/hosted by App should resolve from API ROOT. Other URLs aren't modified
Expand Down Expand Up @@ -105,14 +106,14 @@ function ImageRenderer({tnode}: ImageRendererProps) {
}

const attachmentLink = tnode.parent?.attributes?.href;
const route = ROUTES.ATTACHMENTS?.getRoute(reportID ?? '-1', type, source, accountID, isAttachmentOrReceipt, fileName, attachmentLink);
const route = ROUTES.ATTACHMENTS?.getRoute(reportID, type, source, accountID, isAttachmentOrReceipt, fileName, attachmentLink);
Navigation.navigate(route);
}}
onLongPress={(event) => {
if (isDisabled) {
return;
}
showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs));
showContextMenuForReport(event, anchor, report?.reportID, action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs));
}}
shouldUseHapticsOnLongPress
accessibilityRole={CONST.ROLE.BUTTON}
Expand Down
101 changes: 80 additions & 21 deletions src/components/Image/index.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import React, {useCallback, useContext, useMemo, useState} from 'react';
import {withOnyx} from 'react-native-onyx';
import React, {useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import {useSession} from '@components/OnyxProvider';
import {isExpiredSession} from '@libs/actions/Session';
import activateReauthenticator from '@libs/actions/Session/AttachmentImageReauthenticator';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import BaseImage from './BaseImage';
import {ImageBehaviorContext} from './ImageBehaviorContextProvider';
import type {ImageOnLoadEvent, ImageOnyxProps, ImageOwnProps, ImageProps} from './types';
import type {ImageOnLoadEvent, ImageProps} from './types';

function Image({source: propsSource, isAuthTokenRequired = false, session, onLoad, objectPosition = CONST.IMAGE_OBJECT_POSITION.INITIAL, style, ...forwardedProps}: ImageProps) {
function Image({source: propsSource, isAuthTokenRequired = false, onLoad, objectPosition = CONST.IMAGE_OBJECT_POSITION.INITIAL, style, ...forwardedProps}: ImageProps) {
const [aspectRatio, setAspectRatio] = useState<string | number | null>(null);
const isObjectPositionTop = objectPosition === CONST.IMAGE_OBJECT_POSITION.TOP;
const session = useSession();

const {shouldSetAspectRatioInStyle} = useContext(ImageBehaviorContext);

Expand Down Expand Up @@ -37,6 +40,49 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa
},
[onLoad, updateAspectRatio],
);

// accepted sessions are sessions of a certain criteria that we think can necessitate a reload of the images
// because images sources barely changes unless specific events occur like network issues (offline/online) per example.
// Here we target new session received less than 60s after the previous session (that could be from fresh reauthentication, the previous session was not necessarily expired)
// or new session after the previous session was expired (based on timestamp gap between the 2 creationDate and the freshness of the new session).
const isAcceptedSession = useCallback((sessionCreationDateDiff: number, sessionCreationDate: number) => {
return sessionCreationDateDiff < 60000 || (sessionCreationDateDiff >= CONST.SESSION_EXPIRATION_TIME_MS && new Date().getTime() - sessionCreationDate < 60000);
}, []);

/**
* trying to figure out if the current session is expired or fresh from a necessary reauthentication
*/
const previousSessionAge = useRef<number | undefined>();
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const validSessionAge: number | undefined = useMemo(() => {
// Authentication is required only for certain types of images (attachments and receipts),
// so we only calculate the session age for those
if (!isAuthTokenRequired) {
return undefined;
}
if (session?.creationDate) {
if (previousSessionAge.current) {
// Most likely a reauthentication happened, but unless the calculated source is different from the previous, the image won't reload
if (isAcceptedSession(session.creationDate - previousSessionAge.current, session.creationDate)) {
return session.creationDate;
}
return previousSessionAge.current;
}
if (isExpiredSession(session.creationDate)) {
// reset the countdown to now so future sessions creationDate can be compared to that time
return new Date().getTime();
}
return session.creationDate;
}
return undefined;
}, [session, isAuthTokenRequired, isAcceptedSession]);
useEffect(() => {
if (!isAuthTokenRequired) {
return;
}
previousSessionAge.current = validSessionAge;
});

/**
* Check if the image source is a URL - if so the `encryptedAuthToken` is appended
* to the source.
Expand All @@ -48,24 +94,44 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa
}
const authToken = session?.encryptedAuthToken ?? null;
if (isAuthTokenRequired && authToken) {
return {
...propsSource,
headers: {
[CONST.CHAT_ATTACHMENT_TOKEN_KEY]: authToken,
},
};
if (!!session?.creationDate && !isExpiredSession(session.creationDate)) {
return {
...propsSource,
headers: {
[CONST.CHAT_ATTACHMENT_TOKEN_KEY]: authToken,
},
};
}
if (session) {
activateReauthenticator(session);
}
return undefined;
}
}
return propsSource;
// The session prop is not required, as it causes the image to reload whenever the session changes. For more information, please refer to issue #26034.
// but we still need the image to reload sometimes (example : when the current session is expired)
// by forcing a recalculation of the source (which value could indeed change) through the modification of the variable validSessionAge
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [propsSource, isAuthTokenRequired]);
}, [propsSource, isAuthTokenRequired, validSessionAge]);
useEffect(() => {
if (!isAuthTokenRequired || source !== undefined) {
return;
}
forwardedProps?.waitForSession?.();
}, [source, isAuthTokenRequired, forwardedProps]);

/**
* If the image fails to load and the object position is top, we should hide the image by setting the opacity to 0.
*/
const shouldOpacityBeZero = isObjectPositionTop && !aspectRatio;

if (source === undefined && !!forwardedProps?.waitForSession) {
return undefined;
}
if (source === undefined) {
return <FullScreenLoadingIndicator />;
}
return (
<BaseImage
// eslint-disable-next-line react/jsx-props-no-spreading
Expand All @@ -77,18 +143,11 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa
);
}

function imagePropsAreEqual(prevProps: ImageOwnProps, nextProps: ImageOwnProps) {
function imagePropsAreEqual(prevProps: ImageProps, nextProps: ImageProps) {
return prevProps.source === nextProps.source;
}

const ImageWithOnyx = React.memo(
withOnyx<ImageProps, ImageOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(Image),
imagePropsAreEqual,
);
const ImageWithOnyx = React.memo(Image, imagePropsAreEqual);

ImageWithOnyx.displayName = 'Image';

Expand Down
18 changes: 9 additions & 9 deletions src/components/Image/types.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
import type {ImageSource} from 'expo-image';
import type {ImageRequireSource, ImageResizeMode, ImageStyle, ImageURISource, StyleProp} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type CONST from '@src/CONST';
import type {Session} from '@src/types/onyx';

type ExpoImageSource = ImageSource | number | ImageSource[];

type ImageObjectPosition = ValueOf<typeof CONST.IMAGE_OBJECT_POSITION>;

type ImageOnyxProps = {
/** Session info for the currently logged in user. */
session: OnyxEntry<Session>;
};

type ImageOnLoadEvent = {
nativeEvent: {
width: number;
Expand Down Expand Up @@ -53,8 +46,15 @@ type ImageOwnProps = BaseImageProps & {

/** The object position of image */
objectPosition?: ImageObjectPosition;

/**
* Called when the image should wait for a valid session to reload
* At the moment this function is called, the image is not in cache anymore
* cf https://github.com/Expensify/App/issues/51888
*/
waitForSession?: () => void;
};

type ImageProps = ImageOnyxProps & ImageOwnProps;
type ImageProps = ImageOwnProps;

export type {BaseImageProps, ImageOwnProps, ImageOnyxProps, ImageProps, ExpoImageSource, ImageOnLoadEvent, ImageObjectPosition};
export type {BaseImageProps, ImageOwnProps, ImageProps, ExpoImageSource, ImageOnLoadEvent, ImageObjectPosition};
6 changes: 6 additions & 0 deletions src/components/ImageView/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ function ImageView({isAuthTokenRequired = false, url, fileName, onError}: ImageV
/>
);
}

return (
<View
// eslint-disable-next-line react-compiler/react-compiler
Expand All @@ -238,6 +239,11 @@ function ImageView({isAuthTokenRequired = false, url, fileName, onError}: ImageV
resizeMode={RESIZE_MODES.contain}
onLoadStart={imageLoadingStart}
onLoad={imageLoad}
waitForSession={() => {
setIsLoading(true);
setZoomScale(0);
setIsZoomed(false);
}}
onError={onError}
/>
</PressableWithoutFeedback>
Expand Down
7 changes: 7 additions & 0 deletions src/components/ImageWithSizeCalculation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ function ImageWithSizeCalculation({url, altText, style, onMeasure, onLoadFailure
}}
onError={onError}
onLoad={imageLoadedSuccessfully}
waitForSession={() => {
// Called when the image should wait for a valid session to reload
// At the moment this function is called, the image is not in cache anymore
isLoadedRef.current = false;
setIsImageCached(false);
setIsLoading(true);
}}
objectPosition={objectPosition}
/>
{isLoading && !isImageCached && !isOffline && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
Expand Down
8 changes: 8 additions & 0 deletions src/components/Lightbox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,14 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
updateContentSize(e);
setLightboxImageLoaded(true);
}}
waitForSession={() => {
// only active lightbox should call this function
if (!isActive || isFallbackVisible || !isLightboxVisible) {
return;
}
setContentSize(cachedImageDimensions.get(uri));
setLightboxImageLoaded(false);
}}
/>
</MultiGestureCanvas>
</View>
Expand Down
4 changes: 2 additions & 2 deletions src/components/ShowContextMenuContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ ShowContextMenuContext.displayName = 'ShowContextMenuContext';
function showContextMenuForReport(
event: GestureResponderEvent | MouseEvent,
anchor: ContextMenuAnchor,
reportID: string,
reportID: string | undefined,
action: OnyxEntry<ReportAction>,
checkIfContextMenuActive: () => void,
isArchivedRoom = false,
Expand All @@ -60,7 +60,7 @@ function showContextMenuForReport(
anchor,
reportID,
action?.reportActionID,
ReportUtils.getOriginalReportID(reportID, action),
reportID ? ReportUtils.getOriginalReportID(reportID, action) : undefined,
undefined,
checkIfContextMenuActive,
checkIfContextMenuActive,
Expand Down
9 changes: 9 additions & 0 deletions src/components/TestToolMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ function TestToolMenu() {
/>
</TestToolRow>

{/* Sends an expired session to the FE and invalidates the session by the same time in the BE. Action is delayed for 15s */}
<TestToolRow title={translate('initialSettingsPage.troubleshoot.authenticationStatus')}>
<Button
small
text={translate('initialSettingsPage.troubleshoot.invalidateWithDelay')}
onPress={() => Session.expireSessionWithDelay()}
/>
</TestToolRow>

<TestCrash />
</>
);
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,7 @@ const translations = {
debugMode: 'Debug mode',
invalidFile: 'Invalid file',
invalidFileDescription: 'The file you are trying to import is not valid. Please try again.',
invalidateWithDelay: 'Invalidate with delay',
},
debugConsole: {
saveLog: 'Save log',
Expand Down
3 changes: 2 additions & 1 deletion src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,8 @@ const translations = {
usingImportedState: 'Estás utilizando el estado importado. Pulsa aquí para borrarlo.',
debugMode: 'Modo depuración',
invalidFile: 'Archivo inválido',
invalidFileDescription: 'El archivo que estás intentando importar no es válido. Por favor, inténtalo de nuevo.',
invalidFileDescription: 'El archivo que ests intentando importar no es válido. Por favor, inténtalo de nuevo.',
invalidateWithDelay: 'Invalidar con retraso',
},
debugConsole: {
saveLog: 'Guardar registro',
Expand Down
1 change: 1 addition & 0 deletions src/libs/E2E/actions/e2eLogin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export default function (): Promise<boolean> {
.then((response) => {
Onyx.merge(ONYXKEYS.SESSION, {
authToken: response.authToken,
creationDate: new Date().getTime(),
email: e2eUserCredentials.email,
});
console.debug('[E2E] Signed in finished!');
Expand Down
Loading

0 comments on commit ab92780

Please sign in to comment.