From 31b00ee6a60b3ffe7a4f0aef4135b6f51ea0fa73 Mon Sep 17 00:00:00 2001 From: Kanishk Kakar Date: Thu, 13 Jun 2019 00:26:32 +0530 Subject: [PATCH] Update server validation logic. This PR removes .ogg file check (supported only by very old servers). Other enhancements in server validation logic - * Reject domains with no organizations. * Convert validation methods to async await * Add messages.js for returning error message strings. Fixes: #596, #573. --- app/renderer/js/utils/domain-util.js | 129 +++++++++++++-------------- app/resources/messages.js | 27 ++++++ 2 files changed, 88 insertions(+), 68 deletions(-) create mode 100644 app/resources/messages.js diff --git a/app/renderer/js/utils/domain-util.js b/app/renderer/js/utils/domain-util.js index d571f5cf..fb405f26 100644 --- a/app/renderer/js/utils/domain-util.js +++ b/app/renderer/js/utils/domain-util.js @@ -10,6 +10,7 @@ const escape = require('escape-html'); const Logger = require('./logger-util'); const RequestUtil = require(__dirname + '/../utils/request-util.js'); +const Messages = require(__dirname + '/../../../resources/messages.js'); const logger = new Logger({ file: `domain-util.log`, @@ -101,19 +102,50 @@ class DomainUtil { return false; } + async checkCertError(domain, serverConf, error, silent) { + if (silent) { + // since getting server settings has already failed + return serverConf; + } else { + // Report error to sentry to get idea of possible certificate errors + // users get when adding the servers + logger.reportSentry(new Error(error)); + const certErrorMessage = Messages.certErrorMessage(domain, error); + const certErrorDetail = Messages.certErrorDetail(); + + const response = await dialog.showMessageBox({ + type: 'warning', + buttons: ['Yes', 'No'], + defaultId: 1, + message: certErrorMessage, + detail: certErrorDetail + }); + if (response === 0) { + // set ignoreCerts parameter to true in case user responds with yes + serverConf.ignoreCerts = true; + try { + return await this.getServerSettings(domain, serverConf.ignoreCerts); + } catch (err) { + if (error === Messages.noOrgsError(domain)) { + throw new Error(error); + } + return serverConf; + } + } else { + throw new Error('Untrusted certificate.'); + } + } + } + // ignoreCerts parameter helps in fetching server icon and // other server details when user chooses to ignore certificate warnings - checkDomain(domain, ignoreCerts = false, silent = false) { + async checkDomain(domain, ignoreCerts = false, silent = false) { if (!silent && this.duplicateDomain(domain)) { // Do not check duplicate in silent mode - return Promise.reject('This server has been added.'); + throw new Error('This server has been added.'); } domain = this.formatUrl(domain); - const checkDomain = { - url: domain + '/static/audio/zulip.ogg', - ...RequestUtil.requestOptions(domain, ignoreCerts) - }; const serverConf = { icon: defaultIconUrl, @@ -122,70 +154,29 @@ class DomainUtil { ignoreCerts }; - return new Promise((resolve, reject) => { - request(checkDomain, (error, response) => { - // If the domain contains following strings we just bypass the server - const whitelistDomains = [ - 'zulipdev.org' - ]; + try { + return await this.getServerSettings(domain, serverConf.ignoreCerts); + } catch (err) { + // If the domain contains following strings we just bypass the server + const whitelistDomains = [ + 'zulipdev.org' + ]; - // make sure that error is an error or string not undefined - // so validation does not throw error. - error = error || ''; + // make sure that error is an error or string not undefined + // so validation does not throw error. + const error = err || ''; - const certsError = error.toString().includes('certificate'); - if (!error && response.statusCode < 400) { - // Correct - this.getServerSettings(domain, serverConf.ignoreCerts).then(serverSettings => { - resolve(serverSettings); - }, () => { - resolve(serverConf); - }); - } else if (domain.indexOf(whitelistDomains) >= 0 || certsError) { - if (silent) { - this.getServerSettings(domain, serverConf.ignoreCerts).then(serverSettings => { - resolve(serverSettings); - }, () => { - resolve(serverConf); - }); - } else { - // Report error to sentry to get idea of possible certificate errors - // users get when adding the servers - logger.reportSentry(new Error(error)); - const certErrorMessage = `Do you trust certificate from ${domain}? \n ${error}`; - const certErrorDetail = `The organization you're connecting to is either someone impersonating the Zulip server you entered, or the server you're trying to connect to is configured in an insecure way. - \nIf you have a valid certificate please add it from Settings>Organizations and try to add the organization again. - \nUnless you have a good reason to believe otherwise, you should not proceed. - \nYou can click here if you'd like to proceed with the connection.`; - - dialog.showMessageBox({ - type: 'warning', - buttons: ['Yes', 'No'], - defaultId: 1, - message: certErrorMessage, - detail: certErrorDetail - }, response => { - if (response === 0) { - // set ignoreCerts parameter to true in case user responds with yes - serverConf.ignoreCerts = true; - this.getServerSettings(domain, serverConf.ignoreCerts).then(serverSettings => { - resolve(serverSettings); - }, () => { - resolve(serverConf); - }); - } else { - reject('Untrusted Certificate.'); - } - }); - } - } else { - const invalidZulipServerError = `${domain} does not appear to be a valid Zulip server. Make sure that \ - \n (1) you can connect to that URL in a web browser and \n (2) if you need a proxy to connect to the Internet, that you've configured your proxy in the Network settings \n (3) its a zulip server \ - \n (4) the server has a valid certificate, you can add custom certificates in Settings>Organizations`; - reject(invalidZulipServerError); + const certsError = error.toString().includes('certificate'); + if (domain.indexOf(whitelistDomains) >= 0 || certsError) { + try { + return await this.checkCertError(domain, serverConf, error, silent); + } catch (err) { + throw err; } - }); - }); + } else { + throw Messages.invalidZulipServerError(domain); + } + } } getServerSettings(domain, ignoreCerts = false) { @@ -207,9 +198,11 @@ class DomainUtil { alias: escape(data.realm_name), ignoreCerts }); + } else { + reject(Messages.noOrgsError(domain)); } } else { - reject('Zulip server version < 1.6.'); + reject(response); } }); }); diff --git a/app/resources/messages.js b/app/resources/messages.js new file mode 100644 index 00000000..5791080a --- /dev/null +++ b/app/resources/messages.js @@ -0,0 +1,27 @@ +class Messages { + invalidZulipServerError(domain) { + return `${domain} does not appear to be a valid Zulip server. Make sure that + \n • You can connect to that URL in a web browser.\ + \n • If you need a proxy to connect to the Internet, that you've configured your proxy in the Network settings.\ + \n • It's a Zulip server. (The oldest supported version is 1.6).\ + \n • The server has a valid certificate. (You can add custom certificates in Settings > Organizations).`; + } + + noOrgsError(domain) { + return `${domain} does not have any organizations added.\ + \nPlease contact your server administrator.`; + } + + certErrorMessage(domain, error) { + return `Do you trust certificate from ${domain}? \n ${error}`; + } + + certErrorDetail() { + return `The organization you're connecting to is either someone impersonating the Zulip server you entered, or the server you're trying to connect to is configured in an insecure way. + \nIf you have a valid certificate please add it from Settings>Organizations and try to add the organization again. + \nUnless you have a good reason to believe otherwise, you should not proceed. + \nYou can click here if you'd like to proceed with the connection.`; + } +} + +module.exports = new Messages();