Fix MM-61975 (#8459) (#8599)

* Fix MM-61975

* Add missing strings

* Ensure the app gets logged out after not accepting the ToS

* Fix i18n and add comment

* Fix tests and address feedback

* Check for the right value coming from status

* Update texts

(cherry picked from commit 779fe31c3b)

Co-authored-by: Daniel Espino García <larkox@gmail.com>

app/actions/remote/session.test.ts

@@ -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,
@@ -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;
 
@@ -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;
@@ -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
@@ -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();
@@ -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();
+        }
+    });
+});

app/actions/remote/session.ts

@@ -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';
@@ -25,6 +26,33 @@ import type {LoginArgs} from '@typings/database/database';
 
 const HTTP_UNAUTHORIZED = 401;
 
+const logoutMessages = defineMessages({
+    title: {
+        id: 'logout.fail.title',
+        defaultMessage: 'Logout not complete',
+    },
+    bodyForced: {
+        id: 'logout.fail.message.forced',
+        defaultMessage: 'We could not log you out of the server. Some data may continue to be accessible to this device once the device goes back online.',
+    },
+    body: {
+        id: 'logout.fail.message',
+        defaultMessage: 'You’re not fully logged out. Some data may continue to be accessible to this device once the device goes back online. What do you want to do?',
+    },
+    cancel: {
+        id: 'logout.fail.cancel',
+        defaultMessage: 'Cancel',
+    },
+    continue: {
+        id: 'logout.fail.continue_anyway',
+        defaultMessage: 'Continue Anyway',
+    },
+    ok: {
+        id: 'logout.fail.ok',
+        defaultMessage: 'OK',
+    },
+});
+
 export const addPushProxyVerificationStateFromLogin = async (serverUrl: string) => {
     try {
         const {operator} = DatabaseManager.getServerDatabaseAndOperator(serverUrl);
@@ -56,7 +84,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};
     }
 
@@ -135,15 +163,61 @@ 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 bodyMessage = logoutOnAlert ? logoutMessages.bodyForced : logoutMessages.body;
+            const confirmMessage = logoutOnAlert ? logoutMessages.ok : logoutMessages.continue;
+            const body = intl?.formatMessage(bodyMessage) || bodyMessage.defaultMessage;
+            const cancel = intl?.formatMessage(logoutMessages.cancel) || logoutMessages.cancel.defaultMessage;
+            const confirm = intl?.formatMessage(confirmMessage) || confirmMessage.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) {

app/client/rest/tracking.ts

@@ -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';
@@ -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: {
@@ -384,6 +386,7 @@ export default class ClientTracking {
                 },
                 url,
                 details: error,
+                status_code,
             });
         } finally {
             if (groupLabel && CollectNetworkMetrics) {

app/managers/session_manager.test.ts

@@ -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();
         });
     });

app/managers/session_manager.ts

@@ -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();

app/screens/home/account/components/options/logout/index.tsx

@@ -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 (

app/screens/home/channel_list/categories_list/header/header.tsx

@@ -169,8 +169,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) {

app/screens/home/channel_list/servers/servers_list/server_item/server_item.tsx

@@ -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) => {

app/screens/select_team/header.tsx

@@ -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();

app/screens/terms_of_service/terms_of_service.tsx

@@ -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(

assets/base/i18n/en.json

@@ -465,6 +465,12 @@
   "login.signIn": "Log In",
   "login.signingIn": "Logging In",
   "login.username": "Username",
+  "logout.fail.cancel": "Cancel",
+  "logout.fail.continue_anyway": "Continue Anyway",
+  "logout.fail.message": "You’re not fully logged out. Some data may continue to be accessible to this device once the device goes back online. What do you want to do?",
+  "logout.fail.message.forced": "We could not log you out of the server. Some data may continue to be accessible to this device once the device goes back online.",
+  "logout.fail.ok": "OK",
+  "logout.fail.title": "Logout not complete",
   "markdown.latex.error": "Latex render error",
   "markdown.max_nodes.error": "This message is too long to by shown fully on a mobile device. Please view it on desktop or contact an admin to increase this limit.",
   "markdown.parse_error": "An error occurred while parsing this text",