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.

Sign up for GitHub

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
Merged

Fix MM-61975 #8459

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
45bb57e
Fix MM-61975
larkox Jan 8, 2025
ce69419
Merge branch 'main' into alertLogout
larkox Jan 17, 2025
3dd0a15
Add missing strings
larkox Jan 17, 2025
1332315
Merge branch 'main' into alertLogout
larkox Jan 21, 2025
8ab3bb5
Ensure the app gets logged out after not accepting the ToS
larkox Jan 21, 2025
29d4046
Fix i18n and add comment
larkox Feb 4, 2025
4d60f6b
Merge branch 'main' into alertLogout
larkox Feb 4, 2025
6ddbf13
Fix tests and address feedback
larkox Feb 4, 2025
505782c
Check for the right value coming from status
larkox Feb 6, 2025
90b978c
Merge branch 'main' into alertLogout
larkox Feb 10, 2025
3be85fa
Update texts
larkox Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/actions/remote/session.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ describe('sessions', () => {
});

it('logout - base case', async () => {
const result = await logout(serverUrl, true, true, true);
const result = await logout(serverUrl, undefined, true, true, true);
expect(result).toBeDefined();
expect(result.data).toBeDefined();
});
Expand Down
39 changes: 35 additions & 4 deletions app/actions/remote/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// See LICENSE.txt for license information.

import NetInfo from '@react-native-community/netinfo';
import {DeviceEventEmitter, Platform} from 'react-native';
import {Alert, DeviceEventEmitter, Platform} from 'react-native';

import {Database, Events} from '@constants';
import {SYSTEM_IDENTIFIERS} from '@constants/database';
Expand All @@ -22,6 +22,7 @@ import {getCSRFFromCookie} from '@utils/security';
import {loginEntry} from './entry';

import type {LoginArgs} from '@typings/database/database';
import type {IntlShape} from 'react-intl';

const HTTP_UNAUTHORIZED = 401;

Expand Down Expand Up @@ -56,7 +57,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, true);
return {error: null, logout: true};
}

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

export const logout = async (serverUrl: string, skipServerLogout = false, removeServer = false, skipEvents = false) => {
export const logout = async (serverUrl: string, intl: IntlShape | undefined, skipServerLogout = false, removeServer = false, skipEvents = false) => {
if (!skipServerLogout) {
let loggedOut = false;
try {
const client = NetworkManager.getClient(serverUrl);
await client.logout();
const response = await client.logout();
if (response.status === 200) {
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({id: 'logout.fail.title', defaultMessage: 'Logout Failed'}) || 'Logout Failed';
const body = intl?.formatMessage({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?'}) || '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?';
const cancel = intl?.formatMessage({id: 'logout.fail.cancel', defaultMessage: 'Cancel'}) || 'Cancel';
const confirm = intl?.formatMessage({id: 'logout.fail.confirm', defaultMessage: 'Confirm'}) || 'Confirm';

Alert.alert(
title,
body,
[
{
text: cancel,
style: 'cancel',
},
{
text: confirm,
onPress: async () => {
logout(serverUrl, intl, true);
},
},
],
);

return {data: false};
}
}

if (!skipEvents) {
Expand Down
3 changes: 3 additions & 0 deletions app/client/rest/tracking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {DeviceEventEmitter, Platform} from 'react-native';
import {Events} from '@constants';
import {t} from '@i18n';
import {setServerCredentials} from '@init/credentials';
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 @@ -245,6 +246,7 @@ export default class ClientTracking {
url,
});
} 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 @@ -253,6 +255,7 @@ export default class ClientTracking {
},
url,
details: error,
status_code,
});
} finally {
if (groupLabel) {
Expand Down
2 changes: 1 addition & 1 deletion app/managers/session_manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ class SessionManager {

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure why we call this, we changed skipServerLogout to true and skipEvents is true. We won't do anything in logout apart from return {data: true};? Unless changing skipServerLogout to true was a mistake?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are completely right. But it makes kind of sense 😅

If the session expired, in theory it expired on the server, so we don't need to contact the server.
And if we are in this part of the code, is because the SESSION_EXPIRED event was triggered, so we don't have to trigger it again.

I guess we can remove the logout completely, yes, but I have mixed feelings because "logically" it makes sense to have it here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess this call is here in case there are changes to the logout function in future and we forget to add the call back to onSessionExpired? Should we add a comment above this call in case someone else removes it?

await this.terminateSession(serverUrl, false);

const activeServerUrl = await DatabaseManager.getActiveServerUrl();
Expand Down
2 changes: 1 addition & 1 deletion app/screens/home/account/components/options/logout/index.tsx
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions app/screens/home/channel_list/categories_list/header/header.tsx
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions app/screens/home/channel_list/servers/servers_list/server_item/server_item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ const ServerItem = ({

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

if (isActive) {
dismissBottomSheet();
Expand All @@ -194,7 +194,7 @@ const ServerItem = ({
const skipLogoutFromServer = server.lastActiveAt === 0;
await dismissBottomSheet();
Navigation.updateProps(Screens.HOME, {extra: undefined});
await logout(server.url, skipLogoutFromServer, true);
await logout(server.url, intl, skipLogoutFromServer, true);
};

const startTutorial = () => {
Expand Down
4 changes: 2 additions & 2 deletions app/screens/select_team/header.tsx
View file
Open in desktop
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
View file
Open in desktop
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);
}, [serverUrl, componentId, intl]);

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