Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional value to ISession for passed proxy values #2330

Merged
merged 15 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/imperative/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to the Imperative package will be documented in this file.

## Recent Changes

- Enhancement: Added optional `proxy` object to ISession interface for extenders to pass a ProxyVariables object that would override the environment variables if in place. [#2330](https://github.com/zowe/zowe-cli/pull/2330)

## `8.6.1`

- BugFix: Handled an HTTP 1.1 race condition where an SDK user may experience an ECONNRESET error if a session was reused on Node 20 and above due to HTTP Keep-Alive. [#2339](https://github.com/zowe/zowe-cli/pull/2339)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1472,14 +1472,13 @@ describe("AbstractRestClient tests", () => {
});

describe('buildOptions', () => {
const privateRestClient = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_CERT_PEM,
cert: "FakePemCert",
certKey: "FakePemCertKey"
})
) as any;
const restSession = new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_CERT_PEM,
cert: "FakePemCert",
certKey: "FakePemCertKey"
});
const privateRestClient = new RestClient(restSession) as any;
let getSystemProxyUrlSpy: jest.SpyInstance;
let getProxyAgentSpy: jest.SpyInstance;
let setCertPemAuthSpy: jest.SpyInstance;
Expand All @@ -1503,6 +1502,22 @@ describe("AbstractRestClient tests", () => {
const result = privateRestClient.buildOptions(resource, request, reqHeaders);
expect(Object.keys(result)).toContain('agent');
});

it('Should use session proxy options over env vars for proxy agent', () => {
restSession.ISession.proxy = { proxy_authorization: 'proxy_auth_string'};
const resource = '/resource';
const request = '';
const reqHeaders: any[] = [];
const url = new URL('https://www.zowe.com');
const proxyAgent = new HttpsProxyAgent(url, { rejectUnauthorized: true });
getSystemProxyUrlSpy.mockReturnValue(url);
getProxyAgentSpy.mockReturnValue(proxyAgent);
setCertPemAuthSpy.mockReturnValue(true);
const headerSpy = jest.spyOn(privateRestClient, "appendHeaders");
const result = privateRestClient.buildOptions(resource, request, reqHeaders);
expect(Object.keys(result)).toContain('agent');
expect(headerSpy).toHaveBeenCalledWith([{'Proxy-Authorization': restSession.ISession.proxy.proxy_authorization}]);
})
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,50 @@ describe("Proxy tests", () => {
const httpUrl = "http://www.zowe.com";
const httpsUrl = "https://www.zowe.com";
const noProxyList = "www.zowe.com, fake.com,ibm.com,broadcom.com ";
const passedUrl = "passedurl.com";
let getProxySettingsSpy: jest.SpyInstance;
let checkUrlSpy: jest.SpyInstance;

describe("recognise passed proxy values in session", () => {
const noProxySpy = jest.spyOn(privateProxy, "matchesNoProxySettings");
const httpEnvVarSpy = jest.spyOn(privateProxy, "getHttpEnvVariables");
const httpsEnvVarSpy = jest.spyOn(privateProxy, "getHttpsEnvVariables");
checkUrlSpy = jest.spyOn(privateProxy, "checkUrl");
const expected = {
proxyUrl: passedUrl,
protocol: HTTPS_PROTOCOL
}

beforeEach(() => {
jest.clearAllMocks();
checkUrlSpy.mockClear();
});
it("Should use the HTTP proxy agent passed with session", () => {
expected.protocol = HTTP_PROTOCOL;
session.proxy = { http_proxy: passedUrl };
session.protocol = HTTP_PROTOCOL;
noProxySpy.mockReturnValueOnce(false);
expect(httpEnvVarSpy).not.toHaveBeenCalled();
expect(httpsEnvVarSpy).not.toHaveBeenCalled();
checkUrlSpy.mockReturnValueOnce(passedUrl);
expect(JSON.stringify(ProxySettings["getProxySettings"](session))).toEqual(JSON.stringify(expected));
noProxySpy.mockClear();
checkUrlSpy.mockClear();
});
it("Should use the HTTPS proxy agent passed with session", () => {
expected.protocol = HTTPS_PROTOCOL;
session.proxy = { https_proxy: passedUrl };
session.protocol = HTTPS_PROTOCOL;
noProxySpy.mockReturnValueOnce(false);
expect(httpEnvVarSpy).not.toHaveBeenCalled();
expect(httpsEnvVarSpy).not.toHaveBeenCalled();
checkUrlSpy.mockReturnValueOnce(passedUrl);
expect(JSON.stringify(ProxySettings["getProxySettings"](session))).toEqual(JSON.stringify(expected));
noProxySpy.mockClear();
checkUrlSpy.mockClear();
});
});

describe("getProxyAgent", () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -108,13 +149,22 @@ describe("Proxy tests", () => {
expect(ProxySettings["matchesNoProxySettings"](session)).toEqual(expected);
process.env["NO_PROXY"] = undefined;
});

it("Should return true for match with no_proxy passed with session proxy", () => {
session.proxy = { http_proxy: passedUrl, no_proxy: ["fake.com"] };
session.protocol = HTTP_PROTOCOL;
expect(ProxySettings["matchesNoProxySettings"](session)).toEqual(true);
});
it("Should not match session hostname with no_proxy", () => {
const expected = false;
process.env["NO_PROXY"] = noProxyList;
session.hostname = "microsoft.com";
expect(ProxySettings["matchesNoProxySettings"](session)).toEqual(expected);
process.env["NO_PROXY"] = undefined;
});
it("Should return false for match with no_proxy passed with session proxy", () => {
session.proxy = { http_proxy: passedUrl, no_proxy: ["false.com", "blah.com"] };
session.protocol = HTTP_PROTOCOL;
expect(ProxySettings["matchesNoProxySettings"](session)).toEqual(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,27 @@ describe("Session tests", () => {
expect(session.ISession).toMatchSnapshot();
});

it("should build a session with passed proxy settings updating http_proxy and session's strictSSL", () => {
let error;
let session;
let mySettings;
try {
mySettings = {
http_proxy: "example.com",
https_proxy: undefined,
no_proxy: undefined,
proxy_authorization: undefined,
proxy_strict_ssl: false
};
session = new Session({hostname: "localhost", type: "bearer", tokenValue: "blahblahblah", proxy: mySettings});
} catch (thrownError) {
error = thrownError;
}
expect(error).toBeUndefined();
expect(session.ISession.proxy).toEqual(mySettings);
expect(session.ISession.strictSSL).toEqual(false);
});

it("should require proper type", () => {
let error;
try {
Expand Down
1 change: 1 addition & 0 deletions packages/imperative/src/rest/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export * as SessConstants from "./src/session/SessConstants";
export * from "./src/session/doc/ISession";
export * from "./src/session/doc/IOptionsForAddConnProps";
export * from "./src/session/doc/IOverridePromptConnProps";
export * from "./src/session/doc/ProxyVariables";
export * from "./src/session/AbstractSession";
export * from "./src/session/ConnectionPropsForSessCfg";
export * from "./src/session/Session";
3 changes: 3 additions & 0 deletions packages/imperative/src/rest/src/client/AbstractRestClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,9 @@ export abstract class AbstractRestClient {
this.mLogger.info(`Proxy setting "${proxyUrl.href}" will not be used as hostname was found listed under "no_proxy" setting.`);
} else {
this.mLogger.info(`Using the following proxy setting for the request: ${proxyUrl.href}`);
if (this.session.ISession?.proxy?.proxy_authorization) {
reqHeaders.push({ 'Proxy-Authorization': this.session.ISession.proxy.proxy_authorization});
}
options.agent = ProxySettings.getProxyAgent(this.session.ISession);
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/imperative/src/rest/src/client/ProxySettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class ProxySettings {
* @memberof ProxySettings
*/
public static matchesNoProxySettings(session: ISession): boolean {
const noProxyValues = this.getNoProxyEnvVariables();
const noProxyValues = session.proxy?.no_proxy ?? this.getNoProxyEnvVariables();
if (!noProxyValues) {
return false;
}
Expand All @@ -103,10 +103,10 @@ export class ProxySettings {
const protocol = session.protocol ?? HTTPS_PROTOCOL;
let envVariable: string | undefined;
if (protocol === HTTP_PROTOCOL) {
envVariable = this.getHttpEnvVariables();
envVariable = session.proxy?.http_proxy ?? this.getHttpEnvVariables();
}
else if (protocol === HTTPS_PROTOCOL) {
envVariable = this.getHttpsEnvVariables();
envVariable = session.proxy?.https_proxy ?? this.getHttpsEnvVariables();
}
const proxyUrl = this.checkUrl(envVariable);
if (proxyUrl) {
Expand Down
8 changes: 6 additions & 2 deletions packages/imperative/src/rest/src/session/AbstractSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,12 @@ export abstract class AbstractSession {
}

// set strictSSL
if (populatedSession.strictSSL === undefined || populatedSession.strictSSL === null) {
populatedSession.strictSSL = AbstractSession.DEFAULT_STRICT_SSL;
if (populatedSession.proxy?.proxy_strict_ssl === false) {
populatedSession.strictSSL = false;
} else {
if (populatedSession.proxy?.proxy_strict_ssl || populatedSession.strictSSL === undefined || populatedSession.strictSSL === null) {
populatedSession.strictSSL = AbstractSession.DEFAULT_STRICT_SSL;
}
}

// set port if not set
Expand Down
10 changes: 10 additions & 0 deletions packages/imperative/src/rest/src/session/doc/ISession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

import * as SessConstants from "../SessConstants";
import { ProxyVariables } from "./ProxyVariables";

/**
* Session interface for maintaining cookie and protocol information
Expand Down Expand Up @@ -185,4 +186,13 @@ export interface ISession {
* @memberof ISession
*/
authTypeOrder?: string[];

/**
* Specifies external proxy settings
* values will override environment variable proxy settings
*
* @type {ProxyVariables[]}
* @memberof ISession
*/
proxy?: ProxyVariables;
}
35 changes: 35 additions & 0 deletions packages/imperative/src/rest/src/session/doc/ProxyVariables.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* This program and the accompanying materials are made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Copyright Contributors to the Zowe Project.
*
*/

export interface ProxyVariables {
/**
* HTTP_PROXY/http_proxy value
*/
http_proxy?: string,
/**
* HTTPS_PROXY/https_proxy value
*/
https_proxy?: string,
/**
* string[] of NO_PROXY/no_proxy values
*/
no_proxy?: string[],
/**
* The value to send as the Proxy-Authorization
* header for every network request
*/
proxy_authorization?: string,
/**
* boolean value for whether the proxy server certificate
* should be verified against the list of supplied CAs
*/
proxy_strict_ssl?: boolean
}