fix: prevent removal of pre-auth secret on logout (#9411)

* fix: prevent removal of pre-auth secret on logout

* fix: use pre-auth secret when reconnecting to server after logout

* test: verify pre-auth secret usage in switchToServerAndLogin
This commit is contained in:
Felipe Martin 2026-01-27 15:52:37 +01:00 committed by GitHub
parent ca117805be
commit fbaf13c7aa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 490 additions and 1 deletions

View file

@ -10,6 +10,7 @@ import {fetchConfigAndLicense} from '@actions/remote/systems';
import {Events, Preferences, Screens} from '@constants'; import {Events, Preferences, Screens} from '@constants';
import DatabaseManager from '@database/manager'; import DatabaseManager from '@database/manager';
import {DEFAULT_LOCALE, getTranslations} from '@i18n'; import {DEFAULT_LOCALE, getTranslations} from '@i18n';
import {getPreauthSecret} from '@init/credentials';
import SecurityManager from '@managers/security_manager'; import SecurityManager from '@managers/security_manager';
import WebsocketManager from '@managers/websocket_manager'; import WebsocketManager from '@managers/websocket_manager';
import {getServer, getServerByIdentifier} from '@queries/app/servers'; import {getServer, getServerByIdentifier} from '@queries/app/servers';
@ -26,6 +27,7 @@ jest.mock('@queries/app/servers');
jest.mock('@queries/servers/system'); jest.mock('@queries/servers/system');
jest.mock('@database/manager'); jest.mock('@database/manager');
jest.mock('@managers/security_manager'); jest.mock('@managers/security_manager');
jest.mock('@init/credentials');
jest.mock('@managers/websocket_manager'); jest.mock('@managers/websocket_manager');
jest.mock('@utils/log'); jest.mock('@utils/log');
@ -182,4 +184,54 @@ describe('switchToServerAndLogin', () => {
expect(SecurityManager.isDeviceJailbroken).toHaveBeenCalledWith('serverUrl'); expect(SecurityManager.isDeviceJailbroken).toHaveBeenCalledWith('serverUrl');
expect(callback).toHaveBeenCalled(); expect(callback).toHaveBeenCalled();
}); });
it('should retrieve and use pre-auth secret when reconnecting after logout', async () => {
const server = {url: 'serverUrl', displayName: 'Server'} as ServersModel;
const config = {DiagnosticId: 'diagId'} as ClientConfig;
const license = {} as ClientLicense;
const preauthSecret = 'test-secret-123';
jest.mocked(getServer).mockResolvedValueOnce(server);
jest.mocked(getPreauthSecret).mockResolvedValueOnce(preauthSecret);
jest.mocked(doPing).mockResolvedValueOnce({});
jest.mocked(fetchConfigAndLicense).mockResolvedValueOnce({config, license});
jest.mocked(getServerByIdentifier).mockResolvedValueOnce(undefined);
await Actions.switchToServerAndLogin('serverUrl', theme, intl, jest.fn());
expect(getPreauthSecret).toHaveBeenCalledWith('serverUrl');
expect(doPing).toHaveBeenCalledWith('serverUrl', true, 5000, preauthSecret);
});
it('should work correctly when no pre-auth secret is stored', async () => {
const server = {url: 'serverUrl', displayName: 'Server'} as ServersModel;
const config = {DiagnosticId: 'diagId'} as ClientConfig;
const license = {} as ClientLicense;
jest.mocked(getServer).mockResolvedValueOnce(server);
jest.mocked(getPreauthSecret).mockResolvedValueOnce(undefined);
jest.mocked(doPing).mockResolvedValueOnce({});
jest.mocked(fetchConfigAndLicense).mockResolvedValueOnce({config, license});
jest.mocked(getServerByIdentifier).mockResolvedValueOnce(undefined);
await Actions.switchToServerAndLogin('serverUrl', theme, intl, jest.fn());
expect(getPreauthSecret).toHaveBeenCalledWith('serverUrl');
expect(doPing).toHaveBeenCalledWith('serverUrl', true, 5000, undefined);
});
it('should pass pre-auth secret to doPing even when ping fails', async () => {
const server = {url: 'serverUrl'} as ServersModel;
const preauthSecret = 'test-secret-456';
jest.mocked(getServer).mockResolvedValueOnce(server);
jest.mocked(getPreauthSecret).mockResolvedValueOnce(preauthSecret);
jest.mocked(doPing).mockResolvedValueOnce({error: 'ping error'});
await Actions.switchToServerAndLogin('serverUrl', theme, intl, jest.fn());
expect(getPreauthSecret).toHaveBeenCalledWith('serverUrl');
expect(doPing).toHaveBeenCalledWith('serverUrl', true, 5000, preauthSecret);
expect(alertServerError).toHaveBeenCalledWith(intl, 'ping error');
});
}); });

View file

@ -7,6 +7,7 @@ import {doPing} from '@actions/remote/general';
import {fetchConfigAndLicense} from '@actions/remote/systems'; import {fetchConfigAndLicense} from '@actions/remote/systems';
import {Screens} from '@constants'; import {Screens} from '@constants';
import DatabaseManager from '@database/manager'; import DatabaseManager from '@database/manager';
import {getPreauthSecret} from '@init/credentials';
import SecurityManager from '@managers/security_manager'; import SecurityManager from '@managers/security_manager';
import WebsocketManager from '@managers/websocket_manager'; import WebsocketManager from '@managers/websocket_manager';
import {getServer, getServerByIdentifier} from '@queries/app/servers'; import {getServer, getServerByIdentifier} from '@queries/app/servers';
@ -53,7 +54,10 @@ export async function switchToServerAndLogin(serverUrl: string, theme: Theme, in
return; return;
} }
const result = await doPing(server.url, true); // Retrieve pre-auth secret from keychain if it exists
const preauthSecret = await getPreauthSecret(server.url);
const result = await doPing(server.url, true, 5000, preauthSecret);
if (result.error) { if (result.error) {
alertServerError(intl, result.error); alertServerError(intl, result.error);
callback?.(); callback?.();

View file

@ -342,5 +342,29 @@ describe('session actions', () => {
expect(resetMomentLocale).toHaveBeenCalledWith(); expect(resetMomentLocale).toHaveBeenCalledWith();
}); });
it('should remove user credentials but preserve pre-auth secret on logout (removeServer=false)', async () => {
await terminateSession(mockServerUrl, false);
// Verify user credentials are removed
expect(removeServerCredentials).toHaveBeenCalledWith(mockServerUrl);
// Verify database is deleted (not destroyed)
expect(DatabaseManager.deleteServerDatabase).toHaveBeenCalledWith(mockServerUrl);
expect(DatabaseManager.destroyServerDatabase).not.toHaveBeenCalled();
// Note: removeServerCredentials now only removes the token, not the pre-auth secret
// The pre-auth secret is only removed when destroyServerDatabase is called
});
it('should call destroyServerDatabase which removes pre-auth secret on server removal (removeServer=true)', async () => {
await terminateSession(mockServerUrl, true);
// Verify database is destroyed (which will remove pre-auth secret internally)
expect(DatabaseManager.destroyServerDatabase).toHaveBeenCalledWith(mockServerUrl);
expect(DatabaseManager.deleteServerDatabase).not.toHaveBeenCalled();
// Note: destroyServerDatabase internally calls removePreauthSecret
});
}); });
}); });

View file

@ -24,6 +24,7 @@ import ServerDataOperator from '@database/operator/server_data_operator';
import {schema as appSchema} from '@database/schema/app'; import {schema as appSchema} from '@database/schema/app';
import {serverSchema} from '@database/schema/server'; import {serverSchema} from '@database/schema/server';
import {beforeUpgrade} from '@helpers/database/upgrade'; import {beforeUpgrade} from '@helpers/database/upgrade';
import {removePreauthSecret} from '@init/credentials';
import {PlaybookRunModel, PlaybookChecklistModel, PlaybookChecklistItemModel, PlaybookRunPropertyFieldModel, PlaybookRunPropertyValueModel} from '@playbooks/database/models'; import {PlaybookRunModel, PlaybookChecklistModel, PlaybookChecklistItemModel, PlaybookRunPropertyFieldModel, PlaybookRunPropertyValueModel} from '@playbooks/database/models';
import {getActiveServer, getServer, getServerByIdentifier} from '@queries/app/servers'; import {getActiveServer, getServer, getServerByIdentifier} from '@queries/app/servers';
import {logDebug, logError} from '@utils/log'; import {logDebug, logError} from '@utils/log';
@ -391,6 +392,9 @@ class DatabaseManagerSingleton {
delete this.serverDatabases[serverUrl]; delete this.serverDatabases[serverUrl];
this.deleteServerDatabaseFiles(serverUrl); this.deleteServerDatabaseFiles(serverUrl);
// Remove pre-auth secret when server is destroyed
await removePreauthSecret(serverUrl);
} }
} }
}; };

View file

@ -0,0 +1,390 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {Platform} from 'react-native';
import * as KeyChain from 'react-native-keychain';
import {
getAllServerCredentials,
getServerCredentials,
setServerCredentials,
removeServerCredentials,
removePreauthSecret,
} from './credentials';
jest.mock('react-native-keychain', () => ({
SECURITY_LEVEL: {
SECURE_SOFTWARE: 'SECURE_SOFTWARE',
},
STORAGE_TYPE: {
FB: 'FB',
AES: 'AES',
RSA: 'RSA',
},
setInternetCredentials: jest.fn(),
getInternetCredentials: jest.fn(),
resetInternetCredentials: jest.fn(),
setGenericPassword: jest.fn(),
getGenericPassword: jest.fn(),
resetGenericPassword: jest.fn(),
getAllInternetPasswordServers: jest.fn(),
getAllGenericPasswordServices: jest.fn(),
}));
jest.mock('@utils/log');
jest.mock('@utils/mattermost_managed', () => ({
getIOSAppGroupDetails: jest.fn().mockReturnValue({
appGroupIdentifier: 'group.com.mattermost.test',
}),
}));
jest.mock('@database/manager', () => ({
getActiveServerUrl: jest.fn().mockResolvedValue('https://example.com'),
serverDatabases: {},
}));
describe('credentials', () => {
const mockServerUrl = 'https://example.com';
const mockToken = 'test-token-123';
const mockUserId = 'user-id-123';
const mockPreauthSecret = 'preauth-secret-123';
beforeEach(() => {
jest.clearAllMocks();
});
describe('setServerCredentials', () => {
it('should store credentials with pre-auth secret', () => {
setServerCredentials(mockServerUrl, mockToken, mockPreauthSecret);
expect(KeyChain.setInternetCredentials).toHaveBeenCalledWith(
mockServerUrl,
mockToken,
mockToken,
expect.objectContaining({
securityLevel: KeyChain.SECURITY_LEVEL.SECURE_SOFTWARE,
}),
);
expect(KeyChain.setGenericPassword).toHaveBeenCalledWith(
'preshared_secret',
mockPreauthSecret,
expect.objectContaining({
server: mockServerUrl,
securityLevel: KeyChain.SECURITY_LEVEL.SECURE_SOFTWARE,
}),
);
});
it('should store credentials without pre-auth secret', () => {
setServerCredentials(mockServerUrl, mockToken);
expect(KeyChain.setInternetCredentials).toHaveBeenCalledWith(
mockServerUrl,
mockToken,
mockToken,
expect.any(Object),
);
expect(KeyChain.resetGenericPassword).toHaveBeenCalledWith(
expect.objectContaining({
server: mockServerUrl,
}),
);
});
it('should not store credentials when serverUrl is missing', () => {
setServerCredentials('', mockToken, mockPreauthSecret);
expect(KeyChain.setInternetCredentials).not.toHaveBeenCalled();
expect(KeyChain.setGenericPassword).not.toHaveBeenCalled();
});
it('should not store credentials when token is missing', () => {
setServerCredentials(mockServerUrl, '', mockPreauthSecret);
expect(KeyChain.setInternetCredentials).not.toHaveBeenCalled();
expect(KeyChain.setGenericPassword).not.toHaveBeenCalled();
});
it('should use iOS app group on iOS platform', () => {
Platform.OS = 'ios';
setServerCredentials(mockServerUrl, mockToken, mockPreauthSecret);
expect(KeyChain.setInternetCredentials).toHaveBeenCalledWith(
mockServerUrl,
mockToken,
mockToken,
expect.objectContaining({
accessGroup: 'group.com.mattermost.test',
}),
);
});
it('should not use app group on Android platform', () => {
Platform.OS = 'android';
setServerCredentials(mockServerUrl, mockToken, mockPreauthSecret);
expect(KeyChain.setInternetCredentials).toHaveBeenCalledWith(
mockServerUrl,
mockToken,
mockToken,
expect.objectContaining({
accessGroup: undefined,
}),
);
});
});
describe('getServerCredentials', () => {
it('should retrieve credentials with pre-auth secret', async () => {
jest.mocked(KeyChain.getInternetCredentials).mockResolvedValue({
username: mockUserId,
password: mockToken,
service: mockServerUrl,
storage: 'keychain' as any,
});
jest.mocked(KeyChain.getGenericPassword).mockResolvedValue({
username: 'preshared_secret',
password: mockPreauthSecret,
service: mockServerUrl,
storage: 'keychain' as any,
});
const result = await getServerCredentials(mockServerUrl);
expect(result).toEqual({
serverUrl: mockServerUrl,
userId: mockUserId,
token: mockToken,
preauthSecret: mockPreauthSecret,
});
});
it('should retrieve credentials without pre-auth secret', async () => {
jest.mocked(KeyChain.getInternetCredentials).mockResolvedValue({
username: mockUserId,
password: mockToken,
service: mockServerUrl,
storage: 'keychain' as any,
});
jest.mocked(KeyChain.getGenericPassword).mockResolvedValue(false);
const result = await getServerCredentials(mockServerUrl);
expect(result).toEqual({
serverUrl: mockServerUrl,
userId: mockUserId,
token: mockToken,
preauthSecret: undefined,
});
});
it('should handle legacy token format with device token', async () => {
jest.mocked(KeyChain.getInternetCredentials).mockResolvedValue({
username: 'device-token-123,user-id-456',
password: mockToken,
service: mockServerUrl,
storage: 'keychain' as any,
});
jest.mocked(KeyChain.getGenericPassword).mockResolvedValue(false);
const result = await getServerCredentials(mockServerUrl);
expect(result).toEqual({
serverUrl: mockServerUrl,
userId: 'user-id-456',
token: mockToken,
preauthSecret: undefined,
});
});
it('should return null when credentials do not exist', async () => {
jest.mocked(KeyChain.getInternetCredentials).mockResolvedValue(false);
const result = await getServerCredentials(mockServerUrl);
expect(result).toBeNull();
});
it('should return null when token is undefined', async () => {
jest.mocked(KeyChain.getInternetCredentials).mockResolvedValue({
username: mockUserId,
password: 'undefined',
service: mockServerUrl,
storage: 'keychain' as any,
});
const result = await getServerCredentials(mockServerUrl);
expect(result).toBeNull();
});
it('should gracefully handle errors when retrieving pre-auth secret', async () => {
jest.mocked(KeyChain.getInternetCredentials).mockResolvedValue({
username: mockUserId,
password: mockToken,
service: mockServerUrl,
storage: 'keychain' as any,
});
jest.mocked(KeyChain.getGenericPassword).mockRejectedValue(new Error('Keychain error'));
const result = await getServerCredentials(mockServerUrl);
expect(result).toEqual({
serverUrl: mockServerUrl,
userId: mockUserId,
token: mockToken,
preauthSecret: undefined,
});
});
it('should return null on error retrieving main credentials', async () => {
jest.mocked(KeyChain.getInternetCredentials).mockRejectedValue(new Error('Keychain error'));
const result = await getServerCredentials(mockServerUrl);
expect(result).toBeNull();
});
});
describe('removeServerCredentials', () => {
it('should remove internet credentials only', async () => {
await removeServerCredentials(mockServerUrl);
expect(KeyChain.resetInternetCredentials).toHaveBeenCalledWith({
server: mockServerUrl,
});
// Should NOT remove pre-auth secret
expect(KeyChain.resetGenericPassword).not.toHaveBeenCalled();
});
});
describe('removePreauthSecret', () => {
it('should remove pre-auth secret', async () => {
await removePreauthSecret(mockServerUrl);
expect(KeyChain.resetGenericPassword).toHaveBeenCalledWith({
server: mockServerUrl,
});
});
it('should gracefully handle errors when pre-auth secret does not exist', async () => {
jest.mocked(KeyChain.resetGenericPassword).mockRejectedValue(new Error('Not found'));
await expect(removePreauthSecret(mockServerUrl)).resolves.not.toThrow();
expect(KeyChain.resetGenericPassword).toHaveBeenCalledWith({
server: mockServerUrl,
});
});
});
describe('getAllServerCredentials', () => {
beforeEach(() => {
Platform.OS = 'ios';
});
it('should retrieve all server credentials on iOS', async () => {
const serverUrls = [
'https://server1.com',
'https://server2.com',
];
jest.mocked((KeyChain as any).getAllInternetPasswordServers).mockResolvedValue(serverUrls);
jest.mocked(KeyChain.getInternetCredentials).mockImplementation(async (url) => {
if (url === 'https://server1.com') {
return {
username: 'user1',
password: 'token1',
service: url,
storage: 'keychain' as any,
};
} else if (url === 'https://server2.com') {
return {
username: 'user2',
password: 'token2',
service: url,
storage: 'keychain' as any,
};
}
return false;
});
jest.mocked(KeyChain.getGenericPassword).mockResolvedValue(false);
const result = await getAllServerCredentials();
expect(result).toHaveLength(2);
expect(result[0]).toEqual({
serverUrl: 'https://server1.com',
userId: 'user1',
token: 'token1',
preauthSecret: undefined,
});
expect(result[1]).toEqual({
serverUrl: 'https://server2.com',
userId: 'user2',
token: 'token2',
preauthSecret: undefined,
});
});
it('should retrieve all server credentials on Android', async () => {
Platform.OS = 'android';
const serverUrls = ['https://server1.com'];
jest.mocked(KeyChain.getAllGenericPasswordServices).mockResolvedValue(serverUrls);
jest.mocked(KeyChain.getInternetCredentials).mockResolvedValue({
username: 'user1',
password: 'token1',
service: 'https://server1.com',
storage: 'keychain' as any,
});
jest.mocked(KeyChain.getGenericPassword).mockResolvedValue(false);
const result = await getAllServerCredentials();
expect(result).toHaveLength(1);
expect(KeyChain.getAllGenericPasswordServices).toHaveBeenCalled();
expect((KeyChain as any).getAllInternetPasswordServers).not.toHaveBeenCalled();
});
it('should filter out null credentials', async () => {
const serverUrls = [
'https://server1.com',
'https://server2.com',
];
jest.mocked((KeyChain as any).getAllInternetPasswordServers).mockResolvedValue(serverUrls);
jest.mocked(KeyChain.getInternetCredentials).mockImplementation(async (url) => {
if (url === 'https://server1.com') {
return {
username: 'user1',
password: 'token1',
service: url,
storage: 'keychain' as any,
};
}
return false;
});
jest.mocked(KeyChain.getGenericPassword).mockResolvedValue(false);
const result = await getAllServerCredentials();
expect(result).toHaveLength(1);
expect(result[0].serverUrl).toBe('https://server1.com');
});
});
});

View file

@ -84,6 +84,9 @@ export const setServerCredentials = (serverUrl: string, token: string, preauthSe
export const removeServerCredentials = async (serverUrl: string) => { export const removeServerCredentials = async (serverUrl: string) => {
await KeyChain.resetInternetCredentials({server: serverUrl}); await KeyChain.resetInternetCredentials({server: serverUrl});
};
export const removePreauthSecret = async (serverUrl: string) => {
try { try {
await KeyChain.resetGenericPassword({server: serverUrl}); await KeyChain.resetGenericPassword({server: serverUrl});
} catch (e) { } catch (e) {
@ -91,6 +94,18 @@ export const removeServerCredentials = async (serverUrl: string) => {
} }
}; };
export const getPreauthSecret = async (serverUrl: string): Promise<string | undefined> => {
try {
const preauthCredentials = await KeyChain.getGenericPassword({
server: serverUrl,
});
const secret = preauthCredentials ? preauthCredentials.password : undefined;
return secret;
} catch (e) {
return undefined;
}
};
export const removeActiveServerCredentials = async () => { export const removeActiveServerCredentials = async () => {
const serverUrl = await getActiveServerUrl(); const serverUrl = await getActiveServerUrl();
if (serverUrl) { if (serverUrl) {