[MM-45031] Don't use infinite background retry logic when the certificate is invalid (#2153)
This commit is contained in:
@@ -36,6 +36,10 @@ jest.mock('main/certificateStore', () => ({
|
|||||||
jest.mock('main/tray/tray', () => ({}));
|
jest.mock('main/tray/tray', () => ({}));
|
||||||
jest.mock('main/windows/windowManager', () => ({
|
jest.mock('main/windows/windowManager', () => ({
|
||||||
getMainWindow: jest.fn(),
|
getMainWindow: jest.fn(),
|
||||||
|
getViewNameByWebContentsId: jest.fn(),
|
||||||
|
viewManager: {
|
||||||
|
views: new Map(),
|
||||||
|
},
|
||||||
}));
|
}));
|
||||||
|
|
||||||
describe('main/app/app', () => {
|
describe('main/app/app', () => {
|
||||||
@@ -53,6 +57,7 @@ describe('main/app/app', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
WindowManager.viewManager.views.clear();
|
||||||
jest.resetAllMocks();
|
jest.resetAllMocks();
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -139,6 +144,16 @@ describe('main/app/app', () => {
|
|||||||
expect(CertificateStore.save).toHaveBeenCalled();
|
expect(CertificateStore.save).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should load URL using MattermostView when trusting certificate', async () => {
|
||||||
|
dialog.showMessageBox.mockResolvedValue({response: 0});
|
||||||
|
const load = jest.fn();
|
||||||
|
WindowManager.viewManager.views.set('view-name', {load});
|
||||||
|
WindowManager.getViewNameByWebContentsId.mockReturnValue('view-name');
|
||||||
|
await handleAppCertificateError(event, webContents, testURL, 'error-1', certificate, callback);
|
||||||
|
expect(callback).toHaveBeenCalledWith(true);
|
||||||
|
expect(load).toHaveBeenCalledWith(testURL);
|
||||||
|
});
|
||||||
|
|
||||||
it('should explicitly untrust if user selects More Details and then cancel with the checkbox checked', async () => {
|
it('should explicitly untrust if user selects More Details and then cancel with the checkbox checked', async () => {
|
||||||
dialog.showMessageBox.mockResolvedValueOnce({response: 0}).mockResolvedValueOnce({response: 1, checkboxChecked: true});
|
dialog.showMessageBox.mockResolvedValueOnce({response: 0}).mockResolvedValueOnce({response: 1, checkboxChecked: true});
|
||||||
await handleAppCertificateError(event, webContents, testURL, 'error-1', certificate, callback);
|
await handleAppCertificateError(event, webContents, testURL, 'error-1', certificate, callback);
|
||||||
|
@@ -136,7 +136,14 @@ export async function handleAppCertificateError(event: Event, webContents: WebCo
|
|||||||
CertificateStore.add(origin, certificate);
|
CertificateStore.add(origin, certificate);
|
||||||
CertificateStore.save();
|
CertificateStore.save();
|
||||||
certificateErrorCallbacks.get(errorID)(true);
|
certificateErrorCallbacks.get(errorID)(true);
|
||||||
|
|
||||||
|
const viewName = WindowManager.getViewNameByWebContentsId(webContents.id);
|
||||||
|
if (viewName) {
|
||||||
|
const view = WindowManager.viewManager?.views.get(viewName);
|
||||||
|
view?.load(url);
|
||||||
|
} else {
|
||||||
webContents.loadURL(url);
|
webContents.loadURL(url);
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
if (result.checkboxChecked) {
|
if (result.checkboxChecked) {
|
||||||
CertificateStore.add(origin, certificate, true);
|
CertificateStore.add(origin, certificate, true);
|
||||||
|
@@ -97,6 +97,17 @@ describe('main/views/MattermostView', () => {
|
|||||||
expect(mattermostView.view.webContents.loadURL).toBeCalledWith('http://server-1.com/', expect.any(Object));
|
expect(mattermostView.view.webContents.loadURL).toBeCalledWith('http://server-1.com/', expect.any(Object));
|
||||||
expect(mattermostView.loadRetry).toBeCalledWith('http://server-1.com/', error);
|
expect(mattermostView.loadRetry).toBeCalledWith('http://server-1.com/', error);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should not retry when failing to load due to cert error', async () => {
|
||||||
|
const error = new Error('test');
|
||||||
|
error.code = 'ERR_CERT_ERROR';
|
||||||
|
const promise = Promise.reject(error);
|
||||||
|
mattermostView.view.webContents.loadURL.mockImplementation(() => promise);
|
||||||
|
mattermostView.load('a-bad<url');
|
||||||
|
await expect(promise).rejects.toThrow(error);
|
||||||
|
expect(mattermostView.view.webContents.loadURL).toBeCalledWith('http://server-1.com/', expect.any(Object));
|
||||||
|
expect(mattermostView.loadRetry).not.toBeCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('retry', () => {
|
describe('retry', () => {
|
||||||
|
@@ -174,6 +174,13 @@ export class MattermostView extends EventEmitter {
|
|||||||
log.info(`[${Util.shorten(this.tab.name)}] Loading ${loadURL}`);
|
log.info(`[${Util.shorten(this.tab.name)}] Loading ${loadURL}`);
|
||||||
const loading = this.view.webContents.loadURL(loadURL, {userAgent: composeUserAgent()});
|
const loading = this.view.webContents.loadURL(loadURL, {userAgent: composeUserAgent()});
|
||||||
loading.then(this.loadSuccess(loadURL)).catch((err) => {
|
loading.then(this.loadSuccess(loadURL)).catch((err) => {
|
||||||
|
if (err.code && err.code.startsWith('ERR_CERT')) {
|
||||||
|
WindowManager.sendToRenderer(LOAD_FAILED, this.tab.name, err.toString(), loadURL.toString());
|
||||||
|
this.emit(LOAD_FAILED, this.tab.name, err.toString(), loadURL.toString());
|
||||||
|
log.info(`[${Util.shorten(this.tab.name)}] Invalid certificate, stop retrying until the user decides what to do: ${err}.`);
|
||||||
|
this.status = Status.ERROR;
|
||||||
|
return;
|
||||||
|
}
|
||||||
this.loadRetry(loadURL, err);
|
this.loadRetry(loadURL, err);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user