Merge pull request #1033 from telerik/vladimirov/proxy

Proxy improvements

Author: rosen-vladimirov

appbuilder/services/npm-service.ts

@@ -4,6 +4,7 @@ import * as constants from "../../constants";
 import { fromWindowsRelativePathToUnix, toBoolean } from "../../helpers";
 import { exported } from "../../decorators";
 import * as url from "url";
+const { getCredentialsFromAuth } = require("proxy-lib/lib/utils");
 
 export class NpmService implements INpmService {
 	private static TYPES_DIRECTORY = "@types/";
@@ -347,10 +348,14 @@ export class NpmService implements INpmService {
 				if (npmProxy && npmProxy !== "null") {
 					const strictSslString = (await this.$childProcess.exec("npm config get strict-ssl") || "").toString().trim();
 					const uri = url.parse(npmProxy);
+					const { username, password } = getCredentialsFromAuth(uri.auth || "");
+
 					this._proxySettings = {
 						hostname: uri.hostname,
 						port: uri.port,
-						rejectUnauthorized: toBoolean(strictSslString)
+						rejectUnauthorized: toBoolean(strictSslString),
+						username,
+						password
 					};
 				}
 			} catch (err) {

bootstrap.ts

@@ -95,7 +95,6 @@ $injector.require("devicePlatformsConstants", "./mobile/device-platforms-constan
 $injector.require("helpService", "./services/help-service");
 $injector.require("messageContractGenerator", "./services/message-contract-generator");
 $injector.require("proxyService", "./services/proxy-service");
-$injector.require("credentialsService", "./services/credentials-service");
 $injector.requireCommand("dev-preuninstall", "./commands/preuninstall");
 $injector.requireCommand("dev-generate-messages", "./commands/generate-messages");
 $injector.requireCommand("doctor", "./commands/doctor");

commands/proxy/proxy-clear.ts

@@ -9,7 +9,7 @@ export class ProxyClearCommand extends ProxyCommandBase {
 	}
 
 	public async execute(args: string[]): Promise<void> {
-		this.$proxyService.clearCache();
+		await this.$proxyService.clearCache();
 		this.$logger.out("Successfully cleared proxy.");
 		await this.tryTrackUsage();
 	}

commands/proxy/proxy-set.ts

@@ -4,6 +4,7 @@ import { ProxyCommandBase } from "./proxy-base";
 import { HttpProtocolToPort } from "../../constants";
 import { parse } from "url";
 import { platform, EOL } from "os";
+const { getCredentialsFromAuth } = require("proxy-lib/lib/utils");
 
 const proxySetCommandName = "proxy|set";
 
@@ -53,7 +54,7 @@ export class ProxySetCommand extends ProxyCommandBase {
 
 		let port = urlObj.port && +urlObj.port || HttpProtocolToPort[urlObj.protocol];
 		const noPort = !port || !this.isValidPort(port);
-		const authCredentials = this.getCredentialsFromAuth(urlObj.auth || "");
+		const authCredentials = getCredentialsFromAuth(urlObj.auth || "");
 		if ((username && authCredentials.username && username !== authCredentials.username) ||
 			password && authCredentials.password && password !== authCredentials.password) {
 			this.$errors.fail("The credentials you have provided in the url address mismatch those passed as command line arguments.");
@@ -86,18 +87,11 @@ export class ProxySetCommand extends ProxyCommandBase {
 			password = await this.$prompter.getPassword("Password");
 		}
 
-		if (username && password) {
-			await this.$proxyService.setCredentials({
-				password,
-				username
-			});
-		}
-
-		const proxyCache: IProxyCache = {
-			PROXY_HOSTNAME: urlObj.hostname,
-			PROXY_PORT: port,
-			PROXY_PROTOCOL: urlObj.protocol,
-			ALLOW_INSECURE: this.$options.insecure
+		const settings: IProxyLibSettings = {
+			proxyUrl: urlString,
+			username,
+			password,
+			rejectUnauthorized: !this.$options.insecure
 		};
 
 		if (!this.$hostInfo.isWindows) {
@@ -111,24 +105,12 @@ export class ProxySetCommand extends ProxyCommandBase {
 
 		this.$logger.warn(`${messageNote}Run '${clientName} proxy set --help' for more information.`);
 
-		this.$proxyService.setCache(proxyCache);
+		await this.$proxyService.setCache(settings);
 		this.$logger.out(`Successfully setup proxy.${EOL}`);
 		this.$logger.out(await this.$proxyService.getInfo());
 		await this.tryTrackUsage();
 	}
 
-	private getCredentialsFromAuth(auth: string): ICredentials {
-		const colonIndex = auth.indexOf(":");
-		let username = "";
-		let password = "";
-		if (colonIndex > -1) {
-			username = auth.substring(0, colonIndex);
-			password = auth.substring(colonIndex + 1);
-		}
-
-		return { username, password };
-	}
-
 	private isPasswordRequired(username: string, password: string): boolean {
 		return !!(username && !password);
 	}

declarations.d.ts

@@ -550,13 +550,24 @@ interface IOpener {
 
 interface IErrors {
 	fail(formatStr: string, ...args: any[]): never;
-	fail(opts: { formatStr?: string; errorCode?: number; suppressCommandHelp?: boolean }, ...args: any[]): never;
+	fail(opts: { formatStr?: string; errorCode?: number; suppressCommandHelp?: boolean, proxyAuthenticationRequired?: boolean }, ...args: any[]): never;
 	failWithoutHelp(message: string, ...args: any[]): never;
 	beginCommand(action: () => Promise<boolean>, printCommandHelp: () => Promise<void>): Promise<boolean>;
 	verifyHeap(message: string): void;
 	printCallStack: boolean;
 }
 
+/**
+ * Describes error raised when making http requests.
+ */
+interface IHttpRequestError extends Error {
+
+	/**
+	 * Defines if the error is caused by the proxy requiring authentication.
+	 */
+	proxyAuthenticationRequired: boolean;
+}
+
 interface ICommandOptions {
 	disableAnalytics?: boolean;
 	enableHooks?: boolean;
@@ -843,61 +854,47 @@ interface IDynamicHelpService {
 }
 
 /**
- * Describes cache information about proxy settings.
+ * Describes standard username/password type credentials.
  */
-interface IProxyCache {
+interface ICredentials {
+	username: string;
+	password: string;
+}
+
+interface IRejectUnauthorized {
 	/**
-	 * Hostname of the proxy
+	 * Defines if NODE_TLS_REJECT_UNAUTHORIZED should be set to true or false. Default value is true.
 	 */
+	rejectUnauthorized: boolean;
+}
 
-	PROXY_HOSTNAME: string;
+/**
+ * Proxy settings required for http request.
+ */
+interface IProxySettings extends IRejectUnauthorized, ICredentials  {
 	/**
-	 * Port of the proxy
+	 * Hostname of the machine used for proxy.
 	 */
-	PROXY_PORT: number;
+	hostname: string;
 
 	/**
-	 * Protocol of the proxy - http or https
+	 * Port of the machine used for proxy that allows connections.
 	 */
-	PROXY_PROTOCOL: string;
+	port: string;
 
 	/**
 	 * Protocol of the proxy - http or https
 	 */
-	ALLOW_INSECURE: boolean;
-}
+	protocol?: string;
 
-/**
- * Describes standard username/password type credentials.
- */
-interface ICredentials {
-	username: string;
-	password: string;
+
+	proxy?: string;
 }
 
-/**
- * Describes Service used for interaction with the OS' secure storage (Windows Credentials manager for example).
- */
-interface ICredentialsService {
-	/**
-	 * Sets the provided credentials in the OS' secure storage.
-	 * @param key {string} A key which can later be used to retrieve the credentials.
-	 * @param credentials {ICredentials} Credentials to be stored.
-	 * @returns {Promise<ICredentials>} The stored credentials.
-	 */
-	setCredentials(key: string, credentials: ICredentials): Promise<ICredentials>;
-	/**
-	 * Retrieves credentials from the OS' secure storage with the given key.
-	 * @param key {string} A key based on which to retrieve the credentials.
-	 * @returns {Promise<ICredentials>} The stored credentials.
-	 */
-	getCredentials(key: string): Promise<ICredentials>;
-	/**
-	 * Clears credentials from the OS' secure storage with the given key.
-	 * @param key {string} A key based on which to clear the credentials.
-	 * @returns {Promise<void>}
-	 */
-	clearCredentials(key: string): Promise<void>;
+interface IProxyLibSettings extends IRejectUnauthorized, ICredentials {
+	proxyUrl: string;
+	credentialsKey?: string;
+	userSpecifiedSettingsFilePath?: string;
 }
 
 /**
@@ -906,35 +903,22 @@ interface ICredentialsService {
 interface IProxyService {
 	/**
 	 * Caches proxy data.
-	 * @param cacheData {IProxyCache} Data to be cached.
-	 * @returns {Promise<ICredentials>} The cache.
+	 * @param cacheData {IProxyLibSettings} Data to be cached.
+	 * @returns {Promise<void>} The cache.
 	 */
-	setCache(cacheData: IProxyCache): IProxyCache;
+	setCache(settings: IProxyLibSettings): Promise<void>;
 
 	/**
 	 * Retrieves proxy cache data.
-	 * @returns {IProxyCache} The cache.
+	 * @returns {Promise<IProxySettings>} Proxy data.
 	 */
-	getCache(): IProxyCache;
+	getCache(): Promise<IProxySettings>;
 
 	/**
 	 * Clears proxy cache data.
-	 * @returns {void}
-	 */
-	clearCache(): void;
-
-	/**
-	 * Sets the provided proxy credentials in the OS' secure storage.
-	 * @param credentials {ICredentials} Proxy credentials to be stored.
-	 * @returns {Promise<ICredentials>} The stored proxy credentials.
-	 */
-	setCredentials(credentials: ICredentials): Promise<ICredentials>;
-
-	/**
-	 * Retrieves proxy credentials from the OS' secure storage with the given key.
-	 * @returns {Promise<ICredentials>} The stored proxy credentials.
+	 * @returns {Promise<void>}
 	 */
-	getCredentials(): Promise<ICredentials>;
+	clearCache(): Promise<void>;
 
 	/**
 	 * Gets info about the proxy that can be printed and shown to the user.
@@ -1918,31 +1902,6 @@ interface ITypeScriptTranspileOptions {
 	useLocalTypeScriptCompiler?: boolean;
 }
 
-/**
- * Proxy settings required for http request.
- */
-interface IProxySettings {
-	/**
-	 * Hostname of the machine used for proxy.
-	 */
-	hostname: string;
-
-	/**
-	 * Port of the machine used for proxy that allows connections.
-	 */
-	port: string;
-
-	/**
-	 * Protocol of the proxy - http or https
-	 */
-	protocol?: string;
-
-	/**
-	 * Defines if NODE_TLS_REJECT_UNAUTHORIZED should be set to true or false. Default value is true.
-	 */
-	rejectUnauthorized: boolean;
-}
-
 /**
  * Describes operating system-related utility methods
  */

errors.ts

@@ -142,6 +142,7 @@ export class Errors implements IErrors {
 		exception.stack = (new Error(exception.message)).stack;
 		exception.errorCode = opts.errorCode || ErrorCodes.UNKNOWN;
 		exception.suppressCommandHelp = opts.suppressCommandHelp;
+		exception.proxyAuthenticationRequired = !!opts.proxyAuthenticationRequired;
 		this.$injector.resolve("logger").trace(opts.formatStr);
 		throw exception;
 	}

http-client.ts

@@ -43,12 +43,12 @@ export class HttpClient implements Server.IHttpClient {
 		let pipeTo = options.pipeTo;
 		delete options.pipeTo;
 
-		const proxyCache = this.$proxyService.getCache();
+		const cliProxySettings = await this.$proxyService.getCache();
 
 		options.headers = options.headers || {};
 		const headers = options.headers;
 
-		await this.useProxySettings(proxySettings, proxyCache, options, headers, requestProto);
+		await this.useProxySettings(proxySettings, cliProxySettings, options, headers, requestProto);
 
 		if (!headers.Accept || headers.Accept.indexOf("application/json") < 0) {
 			if (headers.Accept) {
@@ -98,7 +98,7 @@ export class HttpClient implements Server.IHttpClient {
 			const requestObj = request(options);
 
 			requestObj
-				.on("error", (err: Error) => {
+				.on("error", (err: IHttpRequestError) => {
 					this.$logger.trace("An error occurred while sending the request:", err);
 					// In case we get a 4xx error code there seems to be no better way than this regex to get the error code
 					// the tunnel-agent module that request is using is obscuring the response and hence the statusCode by throwing an error message
@@ -107,6 +107,7 @@ export class HttpClient implements Server.IHttpClient {
 					const errorMessageMatch = err.message.match(HttpClient.STATUS_CODE_REGEX);
 					const errorMessageStatusCode = errorMessageMatch && errorMessageMatch[1] && +errorMessageMatch[1];
 					const errorMessage = this.getErrorMessage(errorMessageStatusCode, null);
+					err.proxyAuthenticationRequired = errorMessageStatusCode === HttpStatusCodes.PROXY_AUTHENTICATION_REQUIRED;
 					err.message = errorMessage || err.message;
 					this.setResponseResult(promiseActions, timerId, { err });
 				})
@@ -239,7 +240,8 @@ export class HttpClient implements Server.IHttpClient {
 	private getErrorMessage(statusCode: number, body: string): string {
 		if (statusCode === HttpStatusCodes.PROXY_AUTHENTICATION_REQUIRED) {
 			const clientNameLowerCase = this.$staticConfig.CLIENT_NAME.toLowerCase();
-			return `Your proxy requires authentication. You can run ${EOL}\t${clientNameLowerCase} proxy set <url> <username> <password>.${EOL}In order to supply ${clientNameLowerCase} with the credentials needed.`;
+			this.$logger.error(`You can run ${EOL}\t${clientNameLowerCase} proxy set <url> <username> <password>.${EOL}In order to supply ${clientNameLowerCase} with the credentials needed.`);
+			return "Your proxy requires authentication.";
 		} else if (statusCode === HttpStatusCodes.PAYMENT_REQUIRED) {
 			const subscriptionUrl = util.format("%s://%s/appbuilder/account/subscription", this.$config.AB_SERVER_PROTO, this.$config.AB_SERVER);
 			return util.format("Your subscription has expired. Go to %s to manage your subscription. Note: After you renew your subscription, " +
@@ -253,10 +255,10 @@ export class HttpClient implements Server.IHttpClient {
 					return err;
 				}
 
-				if (err.ExceptionMessage) {
+				if (err && err.ExceptionMessage) {
 					return err.ExceptionMessage;
 				}
-				if (err.Message) {
+				if (err && err.Message) {
 					return err.Message;
 				}
 			} catch (parsingFailed) {
@@ -270,25 +272,24 @@ export class HttpClient implements Server.IHttpClient {
 	/**
 	 * This method respects the proxySettings (or proxyCache) by modifying headers and options passed to http(s) module.
 	 * @param {IProxySettings} proxySettings The settings passed for this specific call.
-	 * @param {IProxyCache} proxyCache The globally set proxy for this CLI.
+	 * @param {IProxySettings} cliProxySettings The globally set proxy for this CLI.
 	 * @param {any}options The object that will be passed to http(s) module.
 	 * @param {any} headers Headers of the current request.
 	 * @param {string} requestProto The protocol used for the current request - http or https.
 	 */
-	private async useProxySettings(proxySettings: IProxySettings, proxyCache: IProxyCache, options: any, headers: any, requestProto: string): Promise<void> {
-		if (proxySettings || proxyCache) {
-			const proto = (proxySettings && proxySettings.protocol) || proxyCache.PROXY_PROTOCOL || "http:";
-			const host = (proxySettings && proxySettings.hostname) || proxyCache.PROXY_HOSTNAME;
-			const port = (proxySettings && proxySettings.port) || proxyCache.PROXY_PORT;
+	private async useProxySettings(proxySettings: IProxySettings, cliProxySettings: IProxySettings, options: any, headers: any, requestProto: string): Promise<void> {
+		if (proxySettings || cliProxySettings) {
+			const proto = (proxySettings && proxySettings.protocol) || cliProxySettings.protocol || "http:";
+			const host = (proxySettings && proxySettings.hostname) || cliProxySettings.hostname;
+			const port = (proxySettings && proxySettings.port) || cliProxySettings.port;
 			let credentialsPart = "";
-			const proxyCredentials = await this.$proxyService.getCredentials();
-			if (proxyCredentials && proxyCredentials.username && proxyCredentials.password) {
-				credentialsPart = `${proxyCredentials.username}:${proxyCredentials.password}@`;
+			if (cliProxySettings.username && cliProxySettings.password) {
+				credentialsPart = `${cliProxySettings.username}:${cliProxySettings.password}@`;
 			}
 
 			// Note that proto ends with :
 			options.proxy = `${proto}//${credentialsPart}${host}:${port}`;
-			options.rejectUnauthorized = proxySettings ? proxySettings.rejectUnauthorized : (proxyCache ? !proxyCache.ALLOW_INSECURE : true);
+			options.rejectUnauthorized = proxySettings ? proxySettings.rejectUnauthorized : cliProxySettings.rejectUnauthorized;
 
 			this.$logger.trace("Using proxy: %s", options.proxy);
 		}