Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MM-61975 #8459

Merged
merged 11 commits into from
Feb 17, 2025
122 changes: 115 additions & 7 deletions app/actions/remote/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@

/* eslint-disable max-lines */

import {Platform} from 'react-native';
import {createIntl} from 'react-intl';
import {Alert, DeviceEventEmitter, Platform} from 'react-native';

import {Events} from '@constants';
import {GLOBAL_IDENTIFIERS, SYSTEM_IDENTIFIERS} from '@constants/database';
import DatabaseManager from '@database/manager';
import NetworkManager from '@managers/network_manager';
import {logWarning} from '@utils/log';

import {
addPushProxyVerificationStateFromLogin,
Expand All @@ -25,6 +28,11 @@ import {
import type ServerDataOperator from '@database/operator/server_data_operator';
import type {LoginArgs} from '@typings/database/database';

const intl = createIntl({
locale: 'en',
messages: {},
});

const serverUrl = 'baseHandler.test.com';
let operator: ServerDataOperator;

Expand All @@ -45,6 +53,7 @@ const mockClient = {
getSessions: jest.fn(() => [session1]),
sendPasswordResetEmail: jest.fn(() => ({status: 200})),
getMe: jest.fn(() => user1),
logout: jest.fn(),
};

let mockGetPushProxyVerificationState: jest.Mock;
Expand Down Expand Up @@ -90,6 +99,14 @@ jest.mock('@utils/security', () => {
};
});

jest.mock('@utils/log', () => {
const original = jest.requireActual('@utils/log');
return {
...original,
logWarning: jest.fn(),
};
});

beforeAll(() => {
// eslint-disable-next-line
// @ts-ignore
Expand Down Expand Up @@ -194,12 +211,6 @@ describe('sessions', () => {
expect(result.failed).toBe(false);
});

it('logout - base case', async () => {
const result = await logout(serverUrl, true, true, true);
expect(result).toBeDefined();
expect(result.data).toBeDefined();
});

it('cancelSessionNotification - handle not found database', async () => {
const result = await cancelSessionNotification('foo');
expect(result).toBeDefined();
Expand Down Expand Up @@ -370,3 +381,100 @@ describe('sessions', () => {
expect(result).toBeUndefined();
});
});

describe('logout', () => {
const mockEmit = jest.spyOn(DeviceEventEmitter, 'emit').mockImplementation(() => true);
const mockAlert = jest.spyOn(Alert, 'alert').mockImplementation(() => true);

type TestCase = {
options: {
skipServerLogout: boolean;
logoutOnAlert: boolean;
removeServer: boolean;
skipEvents: boolean;
};
withIntl: boolean;
clientReturnError: boolean;
}

const testCases: TestCase[] = [];
for (const skipServerLogout of [false, true]) {
for (const logoutOnAlert of [false, true]) {
for (const removeServer of [false, true]) {
for (const skipEvents of [false, true]) {
for (const withIntl of [false, true]) {
for (const clientReturnError of [false, true]) {
testCases.push({options: {
skipServerLogout,
logoutOnAlert,
removeServer,
skipEvents},
withIntl,
clientReturnError});
}
}
}
}
}
}

test.each(testCases)('%j', async ({clientReturnError, options, withIntl}) => {
if (clientReturnError) {
mockClient.logout.mockImplementationOnce(() => {
throw new Error('logout error');
});
} else {
mockClient.logout = jest.fn(() => ({status: 'OK'}));
}

const mockFormatMessage = jest.spyOn(intl, 'formatMessage');

const shouldCallClient = !options.skipServerLogout;
const shouldLogWarning = shouldCallClient && clientReturnError;
const shouldShowAlert = shouldCallClient && clientReturnError;
const shouldEmit = !options.skipEvents;
const shouldEmitBeforeAlert = shouldEmit && (!shouldShowAlert || options.logoutOnAlert);
const shouldEmitAfterAlert = shouldEmit && !shouldEmitBeforeAlert;
const expectedResult = !(shouldShowAlert && !options.logoutOnAlert);

const clientCalls = shouldCallClient ? 1 : 0;
const alertButtons = options.logoutOnAlert ? 1 : 2;

const result = await logout(serverUrl, withIntl ? intl : undefined, {...options});

expect(result.data).toBe(expectedResult);
expect(mockClient.logout).toHaveBeenCalledTimes(clientCalls);
if (shouldLogWarning) {
expect(logWarning).toHaveBeenCalled();
}

if (shouldEmitBeforeAlert) {
expect(mockEmit).toHaveBeenCalledWith(Events.SERVER_LOGOUT, {serverUrl, removeServer: options.removeServer});
} else {
expect(mockEmit).not.toHaveBeenCalled();
}
if (shouldShowAlert) {
if (withIntl) {
expect(mockFormatMessage).toHaveBeenCalled();
}
expect(mockAlert).toHaveBeenCalled();
expect(mockAlert.mock.calls[0][2]).toHaveLength(alertButtons);
if (alertButtons === 2) {
mockAlert.mock.calls[0][2]?.[0].onPress?.();
expect(mockClient.logout).toHaveBeenCalledTimes(clientCalls);
expect(mockEmit).toHaveBeenCalledTimes(0);
}

mockAlert.mock.calls[0][2]?.[alertButtons - 1].onPress?.(); // Last button should be confirm
expect(mockClient.logout).toHaveBeenCalledTimes(clientCalls);
if (shouldEmitAfterAlert) {
expect(mockEmit).toHaveBeenCalledWith(Events.SERVER_LOGOUT, {serverUrl, removeServer: options.removeServer});
} else {
expect(mockEmit).toHaveBeenCalledTimes(shouldEmitBeforeAlert ? 1 : 0);
}
} else {
expect(mockFormatMessage).not.toHaveBeenCalled();
expect(mockAlert).not.toHaveBeenCalled();
}
});
});
76 changes: 72 additions & 4 deletions app/actions/remote/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// See LICENSE.txt for license information.

import NetInfo from '@react-native-community/netinfo';
import {DeviceEventEmitter, Platform} from 'react-native';
import {defineMessages, type IntlShape} from 'react-intl';
import {Alert, DeviceEventEmitter, Platform, type AlertButton} from 'react-native';

import {Database, Events} from '@constants';
import {SYSTEM_IDENTIFIERS} from '@constants/database';
Expand All @@ -25,6 +26,29 @@ import type {LoginArgs} from '@typings/database/database';

const HTTP_UNAUTHORIZED = 401;

const logoutMessages = defineMessages({
title: {
id: 'logout.fail.title',
defaultMessage: 'Logout Failed',
},
bodyForced: {
id: 'logout.fail.message.forced',
defaultMessage: 'We could not log you out of the server. Data may continue to be accessible to this device once the device goes back online.',
},
body: {
id: 'logout.fail.message',
defaultMessage: 'We could not log you out of the server. If you log out now, data may continue to be accessible to this device once the device goes back online. Do you still want to continue?',
},
cancel: {
id: 'logout.fail.cancel',
defaultMessage: 'Cancel',
},
confirm: {
id: 'logout.fail.confirm',
defaultMessage: 'Confirm',
},
});

export const addPushProxyVerificationStateFromLogin = async (serverUrl: string) => {
try {
const {operator} = DatabaseManager.getServerDatabaseAndOperator(serverUrl);
Expand Down Expand Up @@ -56,7 +80,7 @@ export const forceLogoutIfNecessary = async (serverUrl: string, err: unknown) =>
const currentUserId = await getCurrentUserId(database);

if (isErrorWithStatusCode(err) && err.status_code === HTTP_UNAUTHORIZED && isErrorWithUrl(err) && err.url?.indexOf('/login') === -1 && currentUserId) {
await logout(serverUrl);
await logout(serverUrl, undefined, {skipServerLogout: true});
return {error: null, logout: true};
}

Expand Down Expand Up @@ -135,15 +159,59 @@ export const login = async (serverUrl: string, {ldapOnly = false, loginId, mfaTo
}
};

export const logout = async (serverUrl: string, skipServerLogout = false, removeServer = false, skipEvents = false) => {
type LogoutOptions = {
skipServerLogout?: boolean;
removeServer?: boolean;
skipEvents?: boolean;
logoutOnAlert?: boolean;
};

export const logout = async (
serverUrl: string,
intl: IntlShape | undefined,
{
skipServerLogout = false,
removeServer = false,
skipEvents = false,
logoutOnAlert = false,
}: LogoutOptions = {}) => {
if (!skipServerLogout) {
let loggedOut = false;
try {
const client = NetworkManager.getClient(serverUrl);
await client.logout();
const response = await client.logout();
if (response.status === 'OK') {
loggedOut = true;
}
} catch (error) {
// We want to log the user even if logging out from the server failed
logWarning('An error occurred logging out from the server', serverUrl, getFullErrorMessage(error));
}

if (!loggedOut) {
const title = intl?.formatMessage(logoutMessages.title) || logoutMessages.title.defaultMessage;

const body = logoutOnAlert ? intl?.formatMessage(logoutMessages.bodyForced) || logoutMessages.bodyForced.defaultMessage : intl?.formatMessage(logoutMessages.body) || logoutMessages.body.defaultMessage;
const cancel = intl?.formatMessage(logoutMessages.cancel) || logoutMessages.cancel.defaultMessage;
const confirm = intl?.formatMessage(logoutMessages.confirm) || logoutMessages.confirm.defaultMessage;

const buttons: AlertButton[] = logoutOnAlert ? [] : [{text: cancel, style: 'cancel'}];
buttons.push({
text: confirm,
onPress: logoutOnAlert ? undefined : () => {
logout(serverUrl, intl, {skipEvents, removeServer, logoutOnAlert, skipServerLogout: true});
},
});
Alert.alert(
title,
body,
buttons,
);

if (!logoutOnAlert) {
return {data: false};
}
}
}

if (!skipEvents) {
Expand Down
3 changes: 3 additions & 0 deletions app/client/rest/tracking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {t} from '@i18n';
import {setServerCredentials} from '@init/credentials';
import PerformanceMetricsManager from '@managers/performance_metrics_manager';
import {NetworkRequestMetrics} from '@managers/performance_metrics_manager/constant';
import {isErrorWithStatusCode} from '@utils/errors';
import {getFormattedFileSize} from '@utils/file';
import {logDebug, logInfo} from '@utils/log';
import {semverFromServerVersion} from '@utils/server';
Expand Down Expand Up @@ -376,6 +377,7 @@ export default class ClientTracking {
try {
response = await request!(url, this.buildRequestOptions(options));
} catch (error) {
const status_code = isErrorWithStatusCode(error) ? error.status_code : undefined;
throw new ClientError(this.apiClient.baseUrl, {
message: 'Received invalid response from the server.',
intl: {
Expand All @@ -384,6 +386,7 @@ export default class ClientTracking {
},
url,
details: error,
status_code,
});
} finally {
if (groupLabel && CollectNetworkMetrics) {
Expand Down
2 changes: 1 addition & 1 deletion app/managers/session_manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe('SessionManager', () => {

await new Promise((resolve) => setImmediate(resolve));

expect(logout).toHaveBeenCalledWith(mockServerUrl, false, false, true);
expect(logout).toHaveBeenCalledWith(mockServerUrl, undefined, {skipEvents: true, skipServerLogout: true});
expect(relaunchApp).toHaveBeenCalled();
});
});
Expand Down
6 changes: 5 additions & 1 deletion app/managers/session_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,11 @@ export class SessionManager {

private onSessionExpired = async (serverUrl: string) => {
this.terminatingSessionUrl.add(serverUrl);
await logout(serverUrl, false, false, true);

// logout is not doing anything in this scenario, but we keep it
// to keep the same flow as other logout scenarios.
await logout(serverUrl, undefined, {skipServerLogout: true, skipEvents: true});

await this.terminateSession(serverUrl, false);

const activeServerUrl = await DatabaseManager.getActiveServerUrl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const LogOut = () => {

const onLogout = useCallback(preventDoubleTap(() => {
Navigation.updateProps(Screens.HOME, {extra: undefined});
alertServerLogout(serverDisplayName, () => logout(serverUrl), intl);
alertServerLogout(serverDisplayName, () => logout(serverUrl, intl), intl);
}), [serverDisplayName, serverUrl, intl]);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ const ChannelListHeader = ({
}, [pushProxyStatus, intl]);

const onLogoutPress = useCallback(() => {
alertServerLogout(serverDisplayName, () => logout(serverUrl), intl);
}, []);
alertServerLogout(serverDisplayName, () => logout(serverUrl, intl), intl);
}, [intl, serverDisplayName, serverUrl]);

let header;
if (displayName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,21 +181,21 @@ const ServerItem = ({

const logoutServer = useCallback(async () => {
Navigation.updateProps(Screens.HOME, {extra: undefined});
await logout(server.url);
await logout(server.url, intl);

if (isActive) {
dismissBottomSheet();
} else {
DeviceEventEmitter.emit(Events.SWIPEABLE, '');
}
}, [isActive, server.url]);
}, [intl, isActive, server.url]);

const removeServer = useCallback(async () => {
const skipLogoutFromServer = server.lastActiveAt === 0;
const skipServerLogout = server.lastActiveAt === 0;
await dismissBottomSheet();
Navigation.updateProps(Screens.HOME, {extra: undefined});
await logout(server.url, skipLogoutFromServer, true);
}, [server.lastActiveAt, server.url]);
await logout(server.url, intl, {skipServerLogout, removeServer: true});
}, [intl, server.lastActiveAt, server.url]);

const startTutorial = () => {
viewRef.current?.measureInWindow((x, y, w, h) => {
Expand Down
4 changes: 2 additions & 2 deletions app/screens/select_team/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ function Header() {

const headerStyle = useMemo(() => ({...styles.header, marginLeft: canAddOtherServers ? MARGIN_WITH_SERVER_ICON : undefined}), [canAddOtherServers]);
const onLogoutPress = useCallback(() => {
alertServerLogout(serverDisplayName, () => logout(serverUrl), intl);
}, [serverUrl, serverDisplayName]);
alertServerLogout(serverDisplayName, () => logout(serverUrl, intl), intl);
}, [serverDisplayName, intl, serverUrl]);

const onLabelPress = useCallback(() => {
serverButtonRef.current?.openServers();
Expand Down
4 changes: 2 additions & 2 deletions app/screens/terms_of_service/terms_of_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ const TermsOfService = ({

const closeTermsAndLogout = useCallback(() => {
dismissOverlay(componentId);
logout(serverUrl);
}, [serverUrl, componentId]);
logout(serverUrl, intl, {logoutOnAlert: true});
}, [serverUrl, componentId, intl]);

const alertError = useCallback((retry: () => void) => {
Alert.alert(
Expand Down
Loading
Loading