diff --git a/e2e/specs/server_management/add_server_modal.test.js b/e2e/specs/server_management/add_server_modal.test.js index 44f78a0b..70f68624 100644 --- a/e2e/specs/server_management/add_server_modal.test.js +++ b/e2e/specs/server_management/add_server_modal.test.js @@ -53,42 +53,31 @@ describe('Add Server Modal', function desc() { }); describe('MM-T4389 Invalid messages', () => { - it('MM-T4389_1 should not be valid if no server name or URL has been set', async () => { - await newServerView.click('#saveNewServerModal'); - const existingName = await newServerView.isVisible('#serverNameInput.is-invalid'); - const existingUrl = await newServerView.isVisible('#serverUrlInput.is-invalid'); - existingName.should.be.true; - existingUrl.should.be.true; - }); - - it('should not be valid if a server with the same name exists', async () => { - await newServerView.type('#serverNameInput', config.teams[0].name); - await newServerView.type('#serverUrlInput', 'http://example.org'); - await newServerView.click('#saveNewServerModal'); - const existing = await newServerView.isVisible('#serverNameInput.is-invalid'); + it('MM-T4389_1 should not be valid and save should be disabled if no server name or URL has been set', async () => { + const existing = await newServerView.isVisible('#nameValidation.error'); existing.should.be.true; + const disabled = await newServerView.getAttribute('#saveNewServerModal', 'disabled'); + (disabled === '').should.be.true; }); - it('should not be valid if a server with the same URL exists', async () => { + it('should warn the user if a server with the same URL exists, but still allow them to save', async () => { await newServerView.type('#serverNameInput', 'some-new-server'); await newServerView.type('#serverUrlInput', config.teams[0].url); - await newServerView.click('#saveNewServerModal'); - const existing = await newServerView.isVisible('#serverUrlInput.is-invalid'); + await newServerView.waitForSelector('#urlValidation.warning'); + const existing = await newServerView.isVisible('#urlValidation.warning'); existing.should.be.true; + const disabled = await newServerView.getAttribute('#saveNewServerModal', 'disabled'); + (disabled === '').should.be.false; }); describe('Valid server name', async () => { beforeEach(async () => { await newServerView.type('#serverNameInput', 'TestServer'); - await newServerView.click('#saveNewServerModal'); }); - it('MM-T4389_2 Name should not be marked invalid, URL should be marked invalid', async () => { - const existingName = await newServerView.isVisible('#serverNameInput.is-invalid'); - const existingUrl = await newServerView.isVisible('#serverUrlInput.is-invalid'); + it('MM-T4389_2 Name should not be marked invalid, but should not be able to save', async () => { + await newServerView.waitForSelector('#nameValidation.error', {state: 'detached'}); const disabled = await newServerView.getAttribute('#saveNewServerModal', 'disabled'); - existingName.should.be.false; - existingUrl.should.be.true; (disabled === '').should.be.true; }); }); @@ -96,12 +85,11 @@ describe('Add Server Modal', function desc() { describe('Valid server url', () => { beforeEach(async () => { await newServerView.type('#serverUrlInput', 'http://example.org'); - await newServerView.click('#saveNewServerModal'); }); it('MM-T4389_3 URL should not be marked invalid, name should be marked invalid', async () => { - const existingName = await newServerView.isVisible('#serverNameInput.is-invalid'); - const existingUrl = await newServerView.isVisible('#serverUrlInput.is-invalid'); + const existingUrl = await newServerView.isVisible('#urlValidation.error'); + const existingName = await newServerView.isVisible('#nameValidation.error'); const disabled = await newServerView.getAttribute('#saveNewServerModal', 'disabled'); existingName.should.be.true; existingUrl.should.be.false; @@ -112,8 +100,8 @@ describe('Add Server Modal', function desc() { it('MM-T2826_1 should not be valid if an invalid server address has been set', async () => { await newServerView.type('#serverUrlInput', 'superInvalid url'); - await newServerView.click('#saveNewServerModal'); - const existing = await newServerView.isVisible('#serverUrlInput.is-invalid'); + await newServerView.waitForSelector('#urlValidation.error'); + const existing = await newServerView.isVisible('#urlValidation.error'); existing.should.be.true; }); @@ -121,6 +109,7 @@ describe('Add Server Modal', function desc() { beforeEach(async () => { await newServerView.type('#serverUrlInput', 'http://example.org'); await newServerView.type('#serverNameInput', 'TestServer'); + await newServerView.waitForSelector('#urlValidation.warning'); }); it('should be possible to click add', async () => { diff --git a/e2e/specs/server_management/configure_server_modal.test.js b/e2e/specs/server_management/configure_server_modal.test.js index 47386e29..825d1535 100644 --- a/e2e/specs/server_management/configure_server_modal.test.js +++ b/e2e/specs/server_management/configure_server_modal.test.js @@ -49,7 +49,8 @@ describe('Configure Server Modal', function desc() { it('MM-T5117 should be valid if display name and URL are set', async () => { await configureServerModal.type('#input_name', 'TestServer'); - await configureServerModal.type('#input_url', 'http://example.org'); + await configureServerModal.type('#input_url', 'https://community.mattermost.com'); + await configureServerModal.waitForSelector('#customMessage_url.Input___success'); const connectButtonDisabled = await configureServerModal.getAttribute('#connectConfigureServer', 'disabled'); (connectButtonDisabled === '').should.be.false; @@ -57,11 +58,8 @@ describe('Configure Server Modal', function desc() { it('MM-T5118 should not be valid if an invalid URL has been set', async () => { await configureServerModal.type('#input_name', 'TestServer'); - await configureServerModal.type('#input_url', 'lorem.ipsum.dolor.sit.amet'); - - await configureServerModal.click('#connectConfigureServer'); - - await asyncSleep(1000); + await configureServerModal.type('#input_url', '!@#$%^&*()'); + await configureServerModal.waitForSelector('#customMessage_url.Input___error'); const errorClass = await configureServerModal.getAttribute('#customMessage_url', 'class'); errorClass.should.contain('Input___error'); diff --git a/e2e/specs/server_management/edit_server_modal.test.js b/e2e/specs/server_management/edit_server_modal.test.js index cdb89e0a..43c724e5 100644 --- a/e2e/specs/server_management/edit_server_modal.test.js +++ b/e2e/specs/server_management/edit_server_modal.test.js @@ -101,20 +101,8 @@ describe('EditServerModal', function desc() { it('MM-T2826_3 should not edit server if an invalid server address has been set', async () => { await editServerView.type('#serverUrlInput', 'superInvalid url'); - await editServerView.click('#saveNewServerModal'); - const existing = await editServerView.isVisible('#serverUrlInput.is-invalid'); - existing.should.be.true; - }); - - it('should not edit server if another server with the same name or URL exists', async () => { - await editServerView.fill('#serverNameInput', config.teams[1].name); - await editServerView.click('#saveNewServerModal'); - let existing = await editServerView.isVisible('#serverNameInput.is-invalid'); - existing.should.be.true; - - await editServerView.fill('#serverNameInput', 'NewTestServer'); - await editServerView.fill('#serverUrlInput', config.teams[1].url); - existing = await editServerView.isVisible('#serverUrlInput.is-invalid'); + await editServerView.waitForSelector('#urlValidation.error'); + const existing = await editServerView.isVisible('#urlValidation.error'); existing.should.be.true; }); diff --git a/i18n/en.json b/i18n/en.json index 6150f56a..23fc645c 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -121,13 +121,20 @@ "renderer.components.autoSaveIndicator.saving": "Saving...", "renderer.components.configureServer.cardtitle": "Enter your server details", "renderer.components.configureServer.connect.default": "Connect", + "renderer.components.configureServer.connect.override": "Connect anyway", "renderer.components.configureServer.connect.saving": "Connecting…", "renderer.components.configureServer.name.info": "The name that will be displayed in your server list", "renderer.components.configureServer.name.placeholder": "Server display name", "renderer.components.configureServer.subtitle": "Set up your first server to connect to your

team’s communication hub", "renderer.components.configureServer.title": "Let’s connect to a server", "renderer.components.configureServer.url.info": "The URL of your Mattermost server", + "renderer.components.configureServer.url.insecure": "Your server URL is potentially insecure. For best results, use a URL with the HTTPS protocol.", + "renderer.components.configureServer.url.notMattermost": "The server URL provided does not appear to point to a valid Mattermost server. Please verify the URL and check your connection.", + "renderer.components.configureServer.url.ok": "Server URL is valid. Server version: {serverVersion}", "renderer.components.configureServer.url.placeholder": "Server URL", + "renderer.components.configureServer.url.urlNotMatched": "The server URL provided does not match the configured Site URL on your Mattermost server. Server version: {serverVersion}", + "renderer.components.configureServer.url.urlUpdated": "The server URL provided has been updated to match the configured Site URL on your Mattermost server. Server version: {serverVersion}", + "renderer.components.configureServer.url.validating": "Validating...", "renderer.components.errorView.cannotConnectToAppName": "Cannot connect to {appName}", "renderer.components.errorView.havingTroubleConnecting": "We're having trouble connecting to {appName}. We'll continue to try and establish a connection.", "renderer.components.errorView.refreshThenVerify": "If refreshing this page (Ctrl+R or Command+R) does not work please verify that:", @@ -139,17 +146,21 @@ "renderer.components.mainPage.contextMenu.ariaLabel": "Context menu", "renderer.components.mainPage.titleBar": "Mattermost", "renderer.components.newServerModal.error.nameRequired": "Name is required.", - "renderer.components.newServerModal.error.serverNameExists": "A server with the same name already exists.", "renderer.components.newServerModal.error.serverUrlExists": "A server with the same URL already exists.", "renderer.components.newServerModal.error.urlIncorrectFormatting": "URL is not formatted correctly.", - "renderer.components.newServerModal.error.urlNeedsHttp": "URL should start with http:// or https://.", "renderer.components.newServerModal.error.urlRequired": "URL is required.", "renderer.components.newServerModal.serverDisplayName": "Server Display Name", "renderer.components.newServerModal.serverDisplayName.description": "The name of the server displayed on your desktop app tab bar.", "renderer.components.newServerModal.serverURL": "Server URL", "renderer.components.newServerModal.serverURL.description": "The URL of your Mattermost server. Must start with http:// or https://.", + "renderer.components.newServerModal.success.ok": "Server URL is valid. Server version: {serverVersion}", "renderer.components.newServerModal.title.add": "Add Server", "renderer.components.newServerModal.title.edit": "Edit Server", + "renderer.components.newServerModal.validating": "Validating...", + "renderer.components.newServerModal.warning.insecure": "Your server URL is potentially insecure. For best results, use a URL with the HTTPS protocol.", + "renderer.components.newServerModal.warning.notMattermost": "The server URL provided does not appear to point to a valid Mattermost server. Please verify the URL and check your connection.", + "renderer.components.newServerModal.warning.urlNotMatched": "The server URL does not match the configured Site URL on your Mattermost server. Server version: {serverVersion}", + "renderer.components.newServerModal.warning.urlUpdated": "The server URL provided has been updated to match the configured Site URL on your Mattermost server. Server version: {serverVersion}", "renderer.components.removeServerModal.body": "This will remove the server from your Desktop App but will not delete any of its data - you can add the server back to the app at any time.", "renderer.components.removeServerModal.confirm": "Confirm you wish to remove the {serverName} server?", "renderer.components.removeServerModal.title": "Remove Server", diff --git a/src/common/communication.ts b/src/common/communication.ts index abdcf81a..03049d58 100644 --- a/src/common/communication.ts +++ b/src/common/communication.ts @@ -170,3 +170,5 @@ export const UPDATE_APPSTATE_FOR_VIEW_ID = 'update-appstate-for-view-id'; export const MAIN_WINDOW_CREATED = 'main-window-created'; export const MAIN_WINDOW_RESIZED = 'main-window-resized'; export const MAIN_WINDOW_FOCUSED = 'main-window-focused'; + +export const VALIDATE_SERVER_URL = 'validate-server-url'; diff --git a/src/common/utils/constants.ts b/src/common/utils/constants.ts index 4ea1e899..5599acf8 100644 --- a/src/common/utils/constants.ts +++ b/src/common/utils/constants.ts @@ -44,6 +44,17 @@ export const DOWNLOADS_DROPDOWN_MENU_FULL_HEIGHT = DOWNLOADS_DROPDOWN_MENU_HEIGH export const DOWNLOADS_DROPDOWN_MAX_ITEMS = 50; export const DOWNLOADS_DROPDOWN_AUTOCLOSE_TIMEOUT = 4000; // 4 sec +export const URLValidationStatus = { + OK: 'OK', + Missing: 'MISSING', + Invalid: 'INVALID', + Insecure: 'INSECURE', + URLExists: 'URL_EXISTS', + NotMattermost: 'NOT_MATTERMOST', + URLNotMatched: 'URL_NOT_MATCHED', + URLUpdated: 'URL_UPDATED', +}; + // supported custom login paths (oath, saml) export const customLoginRegexPaths = [ /^\/oauth\/authorize$/i, diff --git a/src/main/app/initialize.ts b/src/main/app/initialize.ts index a6a4055a..c029c2f4 100644 --- a/src/main/app/initialize.ts +++ b/src/main/app/initialize.ts @@ -41,6 +41,7 @@ import { WINDOW_MINIMIZE, WINDOW_RESTORE, DOUBLE_CLICK_ON_WINDOW, + VALIDATE_SERVER_URL, } from 'common/communication'; import Config from 'common/config'; import {isTrustedURL, parseURL} from 'common/utils/url'; @@ -97,6 +98,7 @@ import { handleEditServerModal, handleNewServerModal, handleRemoveServerModal, + handleServerURLValidation, switchServer, } from './servers'; import { @@ -287,6 +289,7 @@ function initializeInterCommunicationEventListeners() { ipcMain.on(SHOW_NEW_SERVER_MODAL, handleNewServerModal); ipcMain.on(SHOW_EDIT_SERVER_MODAL, handleEditServerModal); ipcMain.on(SHOW_REMOVE_SERVER_MODAL, handleRemoveServerModal); + ipcMain.handle(VALIDATE_SERVER_URL, handleServerURLValidation); ipcMain.handle(GET_AVAILABLE_SPELL_CHECKER_LANGUAGES, () => session.defaultSession.availableSpellCheckerLanguages); ipcMain.on(START_UPDATE_DOWNLOAD, handleStartDownload); ipcMain.on(START_UPGRADE, handleStartUpgrade); diff --git a/src/main/app/intercom.test.js b/src/main/app/intercom.test.js index 28bb162b..7ce84648 100644 --- a/src/main/app/intercom.test.js +++ b/src/main/app/intercom.test.js @@ -47,7 +47,6 @@ describe('main/app/intercom', () => { getLocalPreload.mockReturnValue('/some/preload.js'); MainWindow.get.mockReturnValue({}); - ServerManager.getAllServers.mockReturnValue([]); ServerManager.hasServers.mockReturnValue(false); }); @@ -56,7 +55,7 @@ describe('main/app/intercom', () => { ModalManager.addModal.mockReturnValue(promise); handleWelcomeScreenModal(); - expect(ModalManager.addModal).toHaveBeenCalledWith('welcomeScreen', '/some/index.html', '/some/preload.js', [], {}, true); + expect(ModalManager.addModal).toHaveBeenCalledWith('welcomeScreen', '/some/index.html', '/some/preload.js', null, {}, true); }); }); diff --git a/src/main/app/intercom.ts b/src/main/app/intercom.ts index e35263e8..0e5ebb6e 100644 --- a/src/main/app/intercom.ts +++ b/src/main/app/intercom.ts @@ -97,7 +97,7 @@ export function handleWelcomeScreenModal() { if (!mainWindow) { return; } - const modalPromise = ModalManager.addModal('welcomeScreen', html, preload, ServerManager.getAllServers().map((server) => server.toUniqueServer()), mainWindow, !ServerManager.hasServers()); + const modalPromise = ModalManager.addModal('welcomeScreen', html, preload, null, mainWindow, !ServerManager.hasServers()); if (modalPromise) { modalPromise.then((data) => { const newServer = ServerManager.addServer(data); diff --git a/src/main/app/servers.test.js b/src/main/app/servers.test.js index 5476c2cd..46175dd9 100644 --- a/src/main/app/servers.test.js +++ b/src/main/app/servers.test.js @@ -1,9 +1,12 @@ // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import {MattermostServer} from 'common/servers/MattermostServer'; import ServerManager from 'common/servers/serverManager'; +import {URLValidationStatus} from 'common/utils/constants'; import {getDefaultViewsForConfigServer} from 'common/views/View'; +import {ServerInfo} from 'main/server/serverInfo'; import ModalManager from 'main/views/modalManager'; import {getLocalURLString, getLocalPreload} from 'main/utils'; import MainWindow from 'main/windows/mainWindow'; @@ -28,10 +31,17 @@ jest.mock('common/servers/serverManager', () => ({ getView: jest.fn(), getLastActiveTabForServer: jest.fn(), getServerLog: jest.fn(), + lookupViewByURL: jest.fn(), +})); +jest.mock('common/servers/MattermostServer', () => ({ + MattermostServer: jest.fn(), })); jest.mock('common/views/View', () => ({ getDefaultViewsForConfigServer: jest.fn(), })); +jest.mock('main/server/serverInfo', () => ({ + ServerInfo: jest.fn(), +})); jest.mock('main/views/modalManager', () => ({ addModal: jest.fn(), })); @@ -315,4 +325,156 @@ describe('main/app/servers', () => { })); }); }); + + describe('handleServerURLValidation', () => { + beforeEach(() => { + MattermostServer.mockImplementation(({url}) => ({url})); + ServerInfo.mockImplementation(({url}) => ({ + fetchRemoteInfo: jest.fn().mockImplementation(() => ({ + serverVersion: '7.8.0', + siteName: 'Mattermost', + siteURL: url, + })), + })); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should return Missing when you get no URL', async () => { + const result = await Servers.handleServerURLValidation({}); + expect(result.status).toBe(URLValidationStatus.Missing); + }); + + it('should return Invalid when you pass in invalid characters', async () => { + const result = await Servers.handleServerURLValidation({}, '!@#$%^&*()!@#$%^&*()'); + expect(result.status).toBe(URLValidationStatus.Invalid); + }); + + it('should include HTTPS when missing', async () => { + const result = await Servers.handleServerURLValidation({}, 'server.com'); + expect(result.status).toBe(URLValidationStatus.OK); + expect(result.validatedURL).toBe('https://server.com/'); + }); + + it('should correct typos in the protocol', async () => { + const result = await Servers.handleServerURLValidation({}, 'htpst://server.com'); + expect(result.status).toBe(URLValidationStatus.OK); + expect(result.validatedURL).toBe('https://server.com/'); + }); + + it('should replace HTTP with HTTPS when applicable', async () => { + const result = await Servers.handleServerURLValidation({}, 'http://server.com'); + expect(result.status).toBe(URLValidationStatus.OK); + expect(result.validatedURL).toBe('https://server.com/'); + }); + + it('should generate a warning when the server already exists', async () => { + ServerManager.lookupViewByURL.mockReturnValue({server: {id: 'server-1', url: new URL('https://server.com')}}); + const result = await Servers.handleServerURLValidation({}, 'https://server.com'); + expect(result.status).toBe(URLValidationStatus.URLExists); + expect(result.validatedURL).toBe('https://server.com/'); + }); + + it('should generate a warning if the server exists when editing', async () => { + ServerManager.lookupViewByURL.mockReturnValue({server: {name: 'Server 1', id: 'server-1', url: new URL('https://server.com')}}); + const result = await Servers.handleServerURLValidation({}, 'https://server.com', 'server-2'); + expect(result.status).toBe(URLValidationStatus.URLExists); + expect(result.validatedURL).toBe('https://server.com/'); + expect(result.existingServerName).toBe('Server 1'); + }); + + it('should not generate a warning if editing the same server', async () => { + ServerManager.lookupViewByURL.mockReturnValue({server: {name: 'Server 1', id: 'server-1', url: new URL('https://server.com')}}); + const result = await Servers.handleServerURLValidation({}, 'https://server.com', 'server-1'); + expect(result.status).toBe(URLValidationStatus.OK); + expect(result.validatedURL).toBe('https://server.com/'); + }); + + it('should attempt HTTP when HTTPS fails, and generate a warning', async () => { + ServerInfo.mockImplementation(({url}) => ({ + fetchRemoteInfo: jest.fn().mockImplementation(() => { + if (url.startsWith('https:')) { + return undefined; + } + + return { + serverVersion: '7.8.0', + siteName: 'Mattermost', + siteURL: url, + }; + }), + })); + + const result = await Servers.handleServerURLValidation({}, 'http://server.com'); + expect(result.status).toBe(URLValidationStatus.Insecure); + expect(result.validatedURL).toBe('http://server.com/'); + }); + + it('should show a warning when the ping request times out', async () => { + ServerInfo.mockImplementation(() => ({ + fetchRemoteInfo: jest.fn().mockImplementation(() => { + throw new Error(); + }), + })); + + const result = await Servers.handleServerURLValidation({}, 'https://not-server.com'); + expect(result.status).toBe(URLValidationStatus.NotMattermost); + expect(result.validatedURL).toBe('https://not-server.com/'); + }); + + it('should update the users URL when the Site URL is different', async () => { + ServerInfo.mockImplementation(() => ({ + fetchRemoteInfo: jest.fn().mockImplementation(() => { + return { + serverVersion: '7.8.0', + siteName: 'Mattermost', + siteURL: 'https://mainserver.com/', + }; + }), + })); + + const result = await Servers.handleServerURLValidation({}, 'https://server.com'); + expect(result.status).toBe(URLValidationStatus.URLUpdated); + expect(result.validatedURL).toBe('https://mainserver.com/'); + }); + + it('should warn the user when the Site URL is different but unreachable', async () => { + ServerInfo.mockImplementation(({url}) => ({ + fetchRemoteInfo: jest.fn().mockImplementation(() => { + if (url === 'https://mainserver.com/') { + return undefined; + } + return { + serverVersion: '7.8.0', + siteName: 'Mattermost', + siteURL: 'https://mainserver.com/', + }; + }), + })); + + const result = await Servers.handleServerURLValidation({}, 'https://server.com'); + expect(result.status).toBe(URLValidationStatus.URLNotMatched); + expect(result.validatedURL).toBe('https://server.com/'); + }); + + it('should warn the user when the Site URL already exists as another server', async () => { + ServerManager.lookupViewByURL.mockReturnValue({server: {name: 'Server 1', id: 'server-1', url: new URL('https://mainserver.com')}}); + ServerInfo.mockImplementation(() => ({ + fetchRemoteInfo: jest.fn().mockImplementation(() => { + return { + serverVersion: '7.8.0', + siteName: 'Mattermost', + siteURL: 'https://mainserver.com', + }; + }), + })); + + const result = await Servers.handleServerURLValidation({}, 'https://server.com'); + expect(result.status).toBe(URLValidationStatus.URLExists); + expect(result.validatedURL).toBe('https://mainserver.com/'); + expect(result.existingServerName).toBe('Server 1'); + }); + }); }); diff --git a/src/main/app/servers.ts b/src/main/app/servers.ts index 053c09aa..7d2ac05c 100644 --- a/src/main/app/servers.ts +++ b/src/main/app/servers.ts @@ -1,18 +1,23 @@ // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {IpcMainEvent, ipcMain} from 'electron'; +import {IpcMainEvent, IpcMainInvokeEvent, ipcMain} from 'electron'; import {UniqueServer, Server} from 'types/config'; +import {URLValidationResult} from 'types/server'; import {UPDATE_SHORTCUT_MENU} from 'common/communication'; import {Logger} from 'common/log'; import ServerManager from 'common/servers/serverManager'; +import {MattermostServer} from 'common/servers/MattermostServer'; +import {isValidURI, isValidURL, parseURL} from 'common/utils/url'; +import {URLValidationStatus} from 'common/utils/constants'; import ViewManager from 'main/views/viewManager'; import ModalManager from 'main/views/modalManager'; import MainWindow from 'main/windows/mainWindow'; import {getLocalPreload, getLocalURLString} from 'main/utils'; +import {ServerInfo} from 'main/server/serverInfo'; const log = new Logger('App.Servers'); @@ -49,7 +54,7 @@ export const handleNewServerModal = () => { if (!mainWindow) { return; } - const modalPromise = ModalManager.addModal('newServer', html, preload, ServerManager.getAllServers().map((server) => server.toUniqueServer()), mainWindow, !ServerManager.hasServers()); + const modalPromise = ModalManager.addModal('newServer', html, preload, null, mainWindow, !ServerManager.hasServers()); if (modalPromise) { modalPromise.then((data) => { const newServer = ServerManager.addServer(data); @@ -80,14 +85,11 @@ export const handleEditServerModal = (e: IpcMainEvent, id: string) => { if (!server) { return; } - const modalPromise = ModalManager.addModal<{currentServers: UniqueServer[]; server: UniqueServer}, Server>( + const modalPromise = ModalManager.addModal( 'editServer', html, preload, - { - currentServers: ServerManager.getAllServers().map((server) => server.toUniqueServer()), - server: server.toUniqueServer(), - }, + server.toUniqueServer(), mainWindow); if (modalPromise) { modalPromise.then((data) => ServerManager.editServer(id, data)).catch((e) => { @@ -132,3 +134,96 @@ export const handleRemoveServerModal = (e: IpcMainEvent, id: string) => { log.warn('There is already an edit server modal'); } }; + +export const handleServerURLValidation = async (e: IpcMainInvokeEvent, url?: string, currentId?: string): Promise => { + log.debug('handleServerURLValidation', url, currentId); + + // If the URL is missing or null, reject + if (!url) { + return {status: URLValidationStatus.Missing}; + } + + let httpUrl = url; + if (!isValidURL(url)) { + // If it already includes the protocol, tell them it's invalid + if (isValidURI(url)) { + httpUrl = url.replace(/^(.+):/, 'https:'); + } else { + // Otherwise add HTTPS for them + httpUrl = `https://${url}`; + } + } + + // Make sure the final URL is valid + const parsedURL = parseURL(httpUrl); + if (!parsedURL) { + return {status: URLValidationStatus.Invalid}; + } + + // Try and add HTTPS to see if we can get a more secure URL + let secureURL = parsedURL; + if (parsedURL.protocol === 'http:') { + secureURL = parseURL(parsedURL.toString().replace(/^http:/, 'https:')) ?? parsedURL; + } + + // Tell the user if they already have a server for this URL + const existingServer = ServerManager.lookupViewByURL(secureURL, true); + if (existingServer && existingServer.server.id !== currentId) { + return {status: URLValidationStatus.URLExists, existingServerName: existingServer.server.name, validatedURL: existingServer.server.url.toString()}; + } + + // Try and get remote info from the most secure URL, otherwise use the insecure one + let remoteURL = secureURL; + let remoteInfo = await testRemoteServer(secureURL); + if (!remoteInfo) { + if (secureURL.toString() !== parsedURL.toString()) { + remoteURL = parsedURL; + remoteInfo = await testRemoteServer(parsedURL); + } + } + + // If we can't get the remote info, warn the user that this might not be the right URL + // If the original URL was invalid, don't replace that as they probably have a typo somewhere + if (!remoteInfo) { + return {status: URLValidationStatus.NotMattermost, validatedURL: parsedURL.toString()}; + } + + // If we were only able to connect via HTTP, warn the user that the connection is not secure + if (remoteURL.protocol === 'http:') { + return {status: URLValidationStatus.Insecure, serverVersion: remoteInfo.serverVersion, validatedURL: remoteURL.toString()}; + } + + // If the URL doesn't match the Site URL, set the URL to the correct one + if (remoteInfo.siteURL && remoteURL.toString() !== new URL(remoteInfo.siteURL).toString()) { + const parsedSiteURL = parseURL(remoteInfo.siteURL); + if (parsedSiteURL) { + // Check the Site URL as well to see if it's already pre-configured + const existingServer = ServerManager.lookupViewByURL(parsedSiteURL, true); + if (existingServer && existingServer.server.id !== currentId) { + return {status: URLValidationStatus.URLExists, existingServerName: existingServer.server.name, validatedURL: existingServer.server.url.toString()}; + } + + // If we can't reach the remote Site URL, there's probably a configuration issue + const remoteSiteURLInfo = await testRemoteServer(parsedSiteURL); + if (!remoteSiteURLInfo) { + return {status: URLValidationStatus.URLNotMatched, serverVersion: remoteInfo.serverVersion, serverName: remoteInfo.siteName, validatedURL: remoteURL.toString()}; + } + } + + // Otherwise fix it for them and return + return {status: URLValidationStatus.URLUpdated, serverVersion: remoteInfo.serverVersion, serverName: remoteInfo.siteName, validatedURL: remoteInfo.siteURL}; + } + + return {status: URLValidationStatus.OK, serverVersion: remoteInfo.serverVersion, serverName: remoteInfo.siteName, validatedURL: remoteInfo.siteURL}; +}; + +const testRemoteServer = async (parsedURL: URL) => { + const server = new MattermostServer({name: 'temp', url: parsedURL.toString()}, false); + const serverInfo = new ServerInfo(server); + try { + const remoteInfo = await serverInfo.fetchRemoteInfo(); + return remoteInfo; + } catch (error) { + return undefined; + } +}; diff --git a/src/main/preload/desktopAPI.js b/src/main/preload/desktopAPI.js index 9a7b2f54..0692d9a8 100644 --- a/src/main/preload/desktopAPI.js +++ b/src/main/preload/desktopAPI.js @@ -88,6 +88,7 @@ import { GET_ORDERED_SERVERS, GET_ORDERED_TABS_FOR_SERVER, SERVERS_UPDATE, + VALIDATE_SERVER_URL, } from 'common/communication'; console.log('Preload initialized'); @@ -135,6 +136,7 @@ contextBridge.exposeInMainWorld('desktop', { getOrderedServers: () => ipcRenderer.invoke(GET_ORDERED_SERVERS), getOrderedTabsForServer: (serverId) => ipcRenderer.invoke(GET_ORDERED_TABS_FOR_SERVER, serverId), onUpdateServers: (listener) => ipcRenderer.on(SERVERS_UPDATE, () => listener()), + validateServerURL: (url, currentId) => ipcRenderer.invoke(VALIDATE_SERVER_URL, url, currentId), getConfiguration: () => ipcRenderer.invoke(GET_CONFIGURATION), getVersion: () => ipcRenderer.invoke('get-app-version'), diff --git a/src/main/server/serverInfo.ts b/src/main/server/serverInfo.ts index 133d7bb2..df354281 100644 --- a/src/main/server/serverInfo.ts +++ b/src/main/server/serverInfo.ts @@ -49,6 +49,7 @@ export class ServerInfo { private onGetConfig = (data: ClientConfig) => { this.remoteInfo.serverVersion = data.Version; this.remoteInfo.siteURL = data.SiteURL; + this.remoteInfo.siteName = data.SiteName; this.remoteInfo.hasFocalboard = this.remoteInfo.hasFocalboard || data.BuildBoards === 'true'; } diff --git a/src/renderer/components/ConfigureServer.tsx b/src/renderer/components/ConfigureServer.tsx index cc44bd08..9a9684b4 100644 --- a/src/renderer/components/ConfigureServer.tsx +++ b/src/renderer/components/ConfigureServer.tsx @@ -1,14 +1,13 @@ // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useState, useCallback, useEffect} from 'react'; +import React, {useState, useCallback, useEffect, useRef} from 'react'; import {useIntl, FormattedMessage} from 'react-intl'; import classNames from 'classnames'; import {UniqueServer} from 'types/config'; -import {isValidURL, parseURL} from 'common/utils/url'; -import {MODAL_TRANSITION_TIMEOUT} from 'common/utils/constants'; +import {MODAL_TRANSITION_TIMEOUT, URLValidationStatus} from 'common/utils/constants'; import womanLaptop from 'renderer/assets/svg/womanLaptop.svg'; @@ -22,7 +21,6 @@ import 'renderer/css/components/ConfigureServer.scss'; import 'renderer/css/components/LoadingScreen.css'; type ConfigureServerProps = { - currentServers: UniqueServer[]; server?: UniqueServer; mobileView?: boolean; darkMode?: boolean; @@ -36,7 +34,6 @@ type ConfigureServerProps = { }; function ConfigureServer({ - currentServers, server, mobileView, darkMode, @@ -56,35 +53,62 @@ function ConfigureServer({ id, } = server || {}; + const mounted = useRef(false); const [transition, setTransition] = useState<'inFromRight' | 'outToLeft'>(); const [name, setName] = useState(prevName || ''); const [url, setUrl] = useState(prevURL || ''); const [nameError, setNameError] = useState(''); - const [urlError, setURLError] = useState(''); + const [urlError, setURLError] = useState<{type: STATUS; value: string}>(); const [showContent, setShowContent] = useState(false); const [waiting, setWaiting] = useState(false); - const canSave = name && url && !nameError && !urlError; + const [validating, setValidating] = useState(false); + const validationTimestamp = useRef(); + const validationTimeout = useRef(); + const editing = useRef(false); + + const canSave = name && url && !nameError && !validating && urlError && urlError.type !== STATUS.ERROR; useEffect(() => { setTransition('inFromRight'); setShowContent(true); + mounted.current = true; + return () => { + mounted.current = false; + }; }, []); - const checkProtocolInURL = (checkURL: string): Promise => { - if (isValidURL(checkURL)) { - return Promise.resolve(checkURL); - } - return window.desktop.modals.pingDomain(checkURL). - then((result: string) => { - const newURL = `${result}://${checkURL}`; - setUrl(newURL); - return newURL; - }). - catch(() => { - console.error(`Could not ping url: ${checkURL}`); - return checkURL; - }); + const fetchValidationResult = (urlToValidate: string) => { + setValidating(true); + setURLError({ + type: STATUS.INFO, + value: formatMessage({id: 'renderer.components.configureServer.url.validating', defaultMessage: 'Validating...'}), + }); + const requestTime = Date.now(); + validationTimestamp.current = requestTime; + validateURL(urlToValidate).then(({validatedURL, serverName, message}) => { + if (editing.current) { + setValidating(false); + setURLError(undefined); + return; + } + if (!validationTimestamp.current || requestTime < validationTimestamp.current) { + return; + } + if (validatedURL) { + setUrl(validatedURL); + } + if (serverName) { + setName((prev) => { + return prev.length ? prev : serverName; + }); + } + if (message) { + setTransition(undefined); + setURLError(message); + } + setValidating(false); + }); }; const validateName = () => { @@ -97,46 +121,76 @@ function ConfigureServer({ }); } - if (currentServers.find(({name: existingName}) => existingName === newName)) { - return formatMessage({ - id: 'renderer.components.newServerModal.error.serverNameExists', - defaultMessage: 'A server with the same name already exists.', - }); - } - return ''; }; - const validateURL = async (fullURL: string) => { - if (!fullURL) { - return formatMessage({ - id: 'renderer.components.newServerModal.error.urlRequired', - defaultMessage: 'URL is required.', - }); + const validateURL = async (url: string) => { + let message; + const validationResult = await window.desktop.validateServerURL(url); + if (validationResult.validatedURL) { + setUrl(validationResult.validatedURL); } - if (!parseURL(fullURL)) { - return formatMessage({ - id: 'renderer.components.newServerModal.error.urlIncorrectFormatting', - defaultMessage: 'URL is not formatted correctly.', - }); + if (validationResult?.status === URLValidationStatus.Missing) { + message = { + type: STATUS.ERROR, + value: formatMessage({ + id: 'renderer.components.newServerModal.error.urlRequired', + defaultMessage: 'URL is required.', + }), + }; } - if (!isValidURL(fullURL)) { - return formatMessage({ - id: 'renderer.components.newServerModal.error.urlNeedsHttp', - defaultMessage: 'URL should start with http:// or https://.', - }); + if (validationResult?.status === URLValidationStatus.Invalid) { + message = { + type: STATUS.ERROR, + value: formatMessage({ + id: 'renderer.components.newServerModal.error.urlIncorrectFormatting', + defaultMessage: 'URL is not formatted correctly.', + }), + }; } - if (currentServers.find(({url: existingURL}) => parseURL(existingURL)?.toString === parseURL(fullURL)?.toString())) { - return formatMessage({ - id: 'renderer.components.newServerModal.error.serverUrlExists', - defaultMessage: 'A server with the same URL already exists.', - }); + if (validationResult?.status === URLValidationStatus.Insecure) { + message = { + type: STATUS.WARNING, + value: formatMessage({id: 'renderer.components.configureServer.url.insecure', defaultMessage: 'Your server URL is potentially insecure. For best results, use a URL with the HTTPS protocol.'}), + }; } - return ''; + if (validationResult?.status === URLValidationStatus.NotMattermost) { + message = { + type: STATUS.WARNING, + value: formatMessage({id: 'renderer.components.configureServer.url.notMattermost', defaultMessage: 'The server URL provided does not appear to point to a valid Mattermost server. Please verify the URL and check your connection.'}), + }; + } + + if (validationResult?.status === URLValidationStatus.URLNotMatched) { + message = { + type: STATUS.WARNING, + value: formatMessage({id: 'renderer.components.configureServer.url.urlNotMatched', defaultMessage: 'The server URL provided does not match the configured Site URL on your Mattermost server. Server version: {serverVersion}'}, {serverVersion: validationResult.serverVersion}), + }; + } + + if (validationResult?.status === URLValidationStatus.URLUpdated) { + message = { + type: STATUS.INFO, + value: formatMessage({id: 'renderer.components.configureServer.url.urlUpdated', defaultMessage: 'The server URL provided has been updated to match the configured Site URL on your Mattermost server. Server version: {serverVersion}'}, {serverVersion: validationResult.serverVersion}), + }; + } + + if (validationResult?.status === URLValidationStatus.OK) { + message = { + type: STATUS.SUCCESS, + value: formatMessage({id: 'renderer.components.configureServer.url.ok', defaultMessage: 'Server URL is valid. Server version: {serverVersion}'}, {serverVersion: validationResult.serverVersion}), + }; + } + + return { + validatedURL: validationResult.validatedURL, + serverName: validationResult.serverName, + message, + }; }; const handleNameOnChange = ({target: {value}}: React.ChangeEvent) => { @@ -151,8 +205,18 @@ function ConfigureServer({ setUrl(value); if (urlError) { - setURLError(''); + setURLError(undefined); } + + editing.current = true; + clearTimeout(validationTimeout.current as unknown as number); + validationTimeout.current = setTimeout(() => { + if (!mounted.current) { + return; + } + editing.current = false; + fetchValidationResult(value); + }, 1000); }; const handleOnSaveButtonClick = (e: React.MouseEvent) => { @@ -183,21 +247,11 @@ function ConfigureServer({ return; } - const fullURL = await checkProtocolInURL(url.trim()); - const urlError = await validateURL(fullURL); - - if (urlError) { - setTransition(undefined); - setURLError(urlError); - setWaiting(false); - return; - } - setTransition('outToLeft'); setTimeout(() => { onConnect({ - url: fullURL, + url, name, id, }); @@ -269,7 +323,7 @@ function ConfigureServer({ /> -
+
void; onSave?: (server: UniqueServer) => void; server?: UniqueServer; - currentServers?: UniqueServer[]; editMode?: boolean; show?: boolean; restoreFocus?: boolean; @@ -29,11 +31,15 @@ type State = { serverId?: string; serverOrder: number; saveStarted: boolean; + validationStarted: boolean; + validationResult?: URLValidationResult; } class NewServerModal extends React.PureComponent { wasShown?: boolean; serverUrlInputRef?: HTMLInputElement; + validationTimeout?: NodeJS.Timeout; + mounted: boolean; static defaultProps = { restoreFocus: true, @@ -43,48 +49,37 @@ class NewServerModal extends React.PureComponent { super(props); this.wasShown = false; + this.mounted = false; this.state = { serverName: '', serverUrl: '', serverOrder: props.currentOrder || 0, saveStarted: false, + validationStarted: false, }; } - initializeOnShow() { + componentDidMount(): void { + this.mounted = true; + } + + componentWillUnmount(): void { + this.mounted = false; + } + + initializeOnShow = () => { this.setState({ serverName: this.props.server ? this.props.server.name : '', serverUrl: this.props.server ? this.props.server.url : '', serverId: this.props.server?.id, saveStarted: false, + validationStarted: false, + validationResult: undefined, }); - } - getServerNameValidationError() { - if (!this.state.saveStarted) { - return null; + if (this.props.editMode && this.props.server) { + this.validateServerURL(this.props.server.url); } - if (this.props.currentServers) { - const currentServers = [...this.props.currentServers]; - if (currentServers.find((server) => server.id !== this.state.serverId && server.name === this.state.serverName)) { - return ( - - ); - } - } - return this.state.serverName.length > 0 ? null : ( - - ); - } - - getServerNameValidationState() { - return this.getServerNameValidationError() === null ? null : 'error'; } handleServerNameChange = (e: React.ChangeEvent) => { @@ -93,108 +88,205 @@ class NewServerModal extends React.PureComponent { }); } - getServerUrlValidationError() { - if (!this.state.saveStarted) { - return null; - } - if (this.props.currentServers) { - const currentServers = [...this.props.currentServers]; - if (currentServers.find((server) => server.id !== this.state.serverId && server.url === this.state.serverUrl)) { - return ( - - ); - } - } - if (this.state.serverUrl.length === 0) { - return ( - - ); - } - if (!(/^https?:\/\/.*/).test(this.state.serverUrl.trim())) { - return ( - - ); - } - if (!isValidURL(this.state.serverUrl.trim())) { - return ( - - ); - } - return null; - } - - getServerUrlValidationState() { - return this.getServerUrlValidationError() === null ? null : 'error'; - } - handleServerUrlChange = (e: React.ChangeEvent) => { const serverUrl = e.target.value; - this.setState({serverUrl}); + this.setState({serverUrl, validationResult: undefined}); + this.validateServerURL(serverUrl); } - addProtocolToUrl = (serverUrl: string): Promise => { - if (serverUrl.startsWith('http://') || serverUrl.startsWith('https://')) { - return Promise.resolve(undefined); + validateServerURL = (serverUrl: string) => { + clearTimeout(this.validationTimeout as unknown as number); + this.validationTimeout = setTimeout(() => { + if (!this.mounted) { + return; + } + const currentTimeout = this.validationTimeout; + this.setState({validationStarted: true}); + window.desktop.validateServerURL(serverUrl, this.props.server?.id).then((validationResult) => { + if (!this.mounted) { + return; + } + if (currentTimeout !== this.validationTimeout) { + return; + } + this.setState({validationResult, validationStarted: false, serverUrl: validationResult.validatedURL ?? serverUrl, serverName: this.state.serverName ? this.state.serverName : validationResult.serverName ?? ''}); + }); + }, 1000); + } + + isServerURLErrored = () => { + return this.state.validationResult?.status === URLValidationStatus.Invalid || + this.state.validationResult?.status === URLValidationStatus.Missing; + } + + getServerURLMessage = () => { + if (this.state.validationStarted) { + return ( +
+ + +
+ ); } - return window.desktop.modals.pingDomain(serverUrl). - then((result: string) => { - this.setState({serverUrl: `${result}://${this.state.serverUrl}`}); - }). - catch(() => { - console.error(`Could not ping url: ${serverUrl}`); - }); + if (!this.state.validationResult) { + return null; + } + + switch (this.state.validationResult?.status) { + case URLValidationStatus.Missing: + return ( +
+ + +
+ ); + case URLValidationStatus.Invalid: + return ( +
+ + +
+ ); + case URLValidationStatus.URLExists: + return ( +
+ + +
+ ); + case URLValidationStatus.Insecure: + return ( +
+ + +
+ ); + case URLValidationStatus.NotMattermost: + return ( +
+ + +
+ ); + case URLValidationStatus.URLNotMatched: + return ( +
+ + +
+ ); + case URLValidationStatus.URLUpdated: + return ( +
+ + +
+ ); + } + + return ( +
+ + +
+ ); } - getError() { - const nameError = this.getServerNameValidationError(); - const urlError = this.getServerUrlValidationError(); - - if (nameError && urlError) { + getServerNameMessage = () => { + if (!this.state.serverName.length) { return ( - <> - {nameError} -
- {urlError} - +
+ + +
); - } else if (nameError) { - return nameError; - } else if (urlError) { - return urlError; } return null; } - validateForm() { - return this.getServerNameValidationState() === null && - this.getServerUrlValidationState() === null; - } + save = () => { + if (!this.state.validationResult) { + return; + } + + if (this.isServerURLErrored()) { + return; + } - save = async () => { - await this.addProtocolToUrl(this.state.serverUrl); this.setState({ saveStarted: true, }, () => { - if (this.validateForm()) { - this.props.onSave?.({ - url: this.state.serverUrl, - name: this.state.serverName, - id: this.state.serverId, - }); - } + this.props.onSave?.({ + url: this.state.serverUrl, + name: this.state.serverName, + id: this.state.serverId, + }); }); } @@ -291,7 +383,7 @@ class NewServerModal extends React.PureComponent { this.props.setInputRef(ref); } }} - isInvalid={Boolean(this.getServerUrlValidationState())} + isInvalid={this.isServerURLErrored()} autoFocus={true} /> @@ -318,7 +410,7 @@ class NewServerModal extends React.PureComponent { onClick={(e: React.MouseEvent) => { e.stopPropagation(); }} - isInvalid={Boolean(this.getServerNameValidationState())} + isInvalid={!this.state.serverName.length} /> @@ -329,15 +421,15 @@ class NewServerModal extends React.PureComponent { +
+ {this.getServerNameMessage()} + {this.getServerURLMessage()} +
-
- {this.getError()} -
- {this.props.onClose &&