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.

Sign up for GitHub

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

Jump to bottom

Restore precedence of token over password in AbstractRestClient - V3 #2125

Merged
merged 2 commits into from
May 3, 2024
Merged

Restore precedence of token over password in AbstractRestClient - V3 #2125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc86022
Restore precedence of token over password in AbstractRestClient
gejohnston May 1, 2024
7272153
Merge remote-tracking branch 'origin/next' into cred-order-v3
gejohnston May 3, 2024
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
View file
Open in desktop
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

- BugFix: Restore the previous precedence of token over password in AbstractRestClient [#2109](https://github.com/zowe/zowe-cli/issues/2109)

## `8.0.0-next.202404301428`

- Enhancement: Add informative messages before prompting for connection property values in the CLI callback function getValuesBack.
Expand Down
257 changes: 257 additions & 0 deletions packages/imperative/src/rest/__tests__/client/AbstractRestClient.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@
*
*/

import * as fs from "fs";
import * as https from "https";
import * as http from "http";
import { Session } from "../../src/session/Session";
import {
AUTH_TYPE_BASIC, AUTH_TYPE_BEARER, AUTH_TYPE_CERT_PEM, AUTH_TYPE_TOKEN
} from "../../src/session/SessConstants";
import { RestClient } from "../../src/client/RestClient";
import { Headers } from "../../src/client/Headers";
import { ProcessUtils } from "../../../utilities";
Expand All @@ -34,6 +38,13 @@ import { IO } from "../../../io";
*/

describe("AbstractRestClient tests", () => {
let setPasswordAuthSpy: any;

beforeEach(() => {
// pretend that basic auth was successfully set
setPasswordAuthSpy = jest.spyOn(AbstractRestClient.prototype as any, "setPasswordAuth");
setPasswordAuthSpy.mockReturnValue(true);
});

it("should not append any headers to a request by default", () => {
const client = new RestClient(new Session({hostname: "test"}));
Expand Down Expand Up @@ -65,6 +76,23 @@ describe("AbstractRestClient tests", () => {
expect(error.message).toMatchSnapshot();
});

it("should throw an error when when no creds are in the session", async () => {
// restore setPasswordAuth spy to its original implementation
setPasswordAuthSpy.mockRestore();

let caughtError;
try {
await RestClient.getExpectString(new Session({
hostname: "test"
}), "/resource");
} catch (error) {
caughtError = error;
}

expect(caughtError instanceof ImperativeError).toBe(true);
expect(caughtError.message).toContain("No credentials for a BASIC or TOKEN type of session");
});

it("should not error when chunking data and payload data are present in outgoing request", async () => {

interface IPayload {
Expand Down Expand Up @@ -667,6 +695,9 @@ describe("AbstractRestClient tests", () => {

(https.request as any) = httpsRequestFnc;

// restore setPasswordAuth spy to its original implementation
setPasswordAuthSpy.mockRestore();

let error;
try {
await RestClient.getExpectString(
Expand Down Expand Up @@ -701,6 +732,9 @@ describe("AbstractRestClient tests", () => {

(https.request as any) = httpsRequestFnc;

// restore setPasswordAuth spy to its original implementation
setPasswordAuthSpy.mockRestore();

let error;
try {
await RestClient.getExpectString(
Expand Down Expand Up @@ -738,6 +772,9 @@ describe("AbstractRestClient tests", () => {

(https.request as any) = httpsRequestFnc;

// restore setPasswordAuth spy to its original implementation
setPasswordAuthSpy.mockRestore();

let error;
try {
await RestClient.getExpectString(
Expand Down Expand Up @@ -1118,4 +1155,224 @@ describe("AbstractRestClient tests", () => {
expect(result).toBe("\r\nabc\r\ndef\r\n");
});
});

describe("private functions", () => {
beforeEach(() => {
// restore setPasswordAuth spy to its original implementation
if (setPasswordAuthSpy) {
setPasswordAuthSpy.mockRestore();
}
});

describe("setTokenAuth", () => {
it("should return true when a session specifies a token", () => {
// pretend that the session was created for a token
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_TOKEN,
tokenType: "FakeTokenType",
tokenValue: "FakeTokenValue"
})
);

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const tokenWasSet: boolean = restClient["setTokenAuth"](restOptions);
expect(tokenWasSet).toEqual(true);
expect(restOptions.headers["Cookie"]).toBeDefined();

});

it("should return false when a token session has no token value", () => {
// pretend that the session was created for a token, but with no value
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_TOKEN,
tokenType: "FakeTokenType",
tokenValue: "FakeTokenValue"
})
);
delete restClient["mSession"]["mISession"].tokenValue;

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const tokenWasSet: boolean = restClient["setTokenAuth"](restOptions);
expect(tokenWasSet).toEqual(false);
expect(restOptions.headers["Cookie"]).not.toBeDefined();
});
});

describe("setPasswordAuth", () => {
it("should return true when a session specifies user and password", () => {
// pretend that the session was created with a user and password
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_BASIC,
user: "FakeUser",
password: "FakePassword"
})
);

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const passwordWasSet: boolean = restClient["setPasswordAuth"](restOptions);
expect(passwordWasSet).toEqual(true);
expect(restOptions.headers["Authorization"]).toBeDefined();
});

it("should return true when a session specifies base64EncodedAuth", () => {
// pretend that the session was created with a base64 cred
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_BASIC,
base64EncodedAuth: "FakeBase64EncodedCred"
})
);

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const passwordWasSet: boolean = restClient["setPasswordAuth"](restOptions);
expect(passwordWasSet).toEqual(true);
expect(restOptions.headers["Authorization"]).toBeDefined();
});

it("should return false when a basic auth session has no user, password, or Base64Cred", () => {
// pretend that the session was created for basic auth, but has no creds
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_BASIC,
user: "FakeUser",
password: "FakePassword"
})
);
delete restClient["mSession"]["mISession"].user;
delete restClient["mSession"]["mISession"].password;
delete restClient["mSession"]["mISession"].base64EncodedAuth;

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const passwordWasSet: boolean = restClient["setPasswordAuth"](restOptions);
expect(passwordWasSet).toEqual(false);
expect(restOptions.headers["Authorization"]).not.toBeDefined();
});
});

describe("setBearerAuth", () => {
it("should return true when a session has a bearer token", () => {
// pretend that the session was created for a bearer token
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_BEARER,
tokenValue: "FakeBearerTokenValue"
})
);

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const bearerWasSet: boolean = restClient["setBearerAuth"](restOptions);
expect(bearerWasSet).toEqual(true);
expect(restOptions.headers["Authorization"]).toBeDefined();
});

it("should return false when a bearer token session has no token value", () => {
// pretend that the session was created for a bearer token, but with no value
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_BEARER,
tokenValue: "FakeBearerTokenValue"
})
);
delete restClient["mSession"]["mISession"].tokenValue;

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const bearerWasSet: boolean = restClient["setBearerAuth"](restOptions);
expect(bearerWasSet).toEqual(false);
expect(restOptions.headers["Authorization"]).not.toBeDefined();
});
});

describe("setCertPemAuth", () => {
let readFileSyncSpy: any;

beforeEach(() => {
// pretend that readFileSync can read the cert and the cert key
readFileSyncSpy = jest.spyOn(fs, "readFileSync").mockReturnValue(
"Some fake data from ReadFileSync"
);
});

afterEach(() => {
// restore readFileSync to its original implementation
readFileSyncSpy.mockRestore();
});

it("should return true when a session has a PEM cert", () => {
// pretend that the session was created for a PEM cert
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_CERT_PEM,
cert: "FakePemCert",
certKey: "FakePemCertKey"
})
);


// call the function that we want to test
const restOptions: any = {
headers: {}
};
const pemCertWasSet: boolean = restClient["setCertPemAuth"](restOptions);

expect(readFileSyncSpy).toHaveBeenCalledWith(restClient["mSession"]["mISession"].cert);
expect(readFileSyncSpy).toHaveBeenCalledWith(restClient["mSession"]["mISession"].certKey);
expect(pemCertWasSet).toEqual(true);
});

it("should return false when a PEM cert session has no type", () => {
// pretend that the session was created for a PEM cert, but with no type
const restClient: any = new RestClient(
new Session({
hostname: "FakeHostName",
type: AUTH_TYPE_CERT_PEM,
cert: "FakePemCert",
certKey: "FakePemCertKey"
})
);
delete restClient["mSession"]["mISession"].type;

// call the function that we want to test
const restOptions: any = {
headers: {}
};
const pemCertWasSet: boolean = restClient["setCertPemAuth"](restOptions);

expect(pemCertWasSet).toEqual(false);
expect(readFileSyncSpy).not.toHaveBeenCalledWith(restClient["mSession"]["mISession"].cert);
expect(readFileSyncSpy).not.toHaveBeenCalledWith(restClient["mSession"]["mISession"].certKey);
});
});
});
});
7 changes: 7 additions & 0 deletions packages/imperative/src/rest/__tests__/client/RestClient.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,22 @@ import { RestClientError } from "../../src/client/RestClientError";
import { IOptionsFullResponse } from "../../src/client/doc/IOptionsFullResponse";
import { IRestClientResponse } from "../../src/client/doc/IRestClientResponse";
import { CLIENT_PROPERTY } from "../../src/client/types/AbstractRestClientProperties";
import { AbstractRestClient } from "../../src/client/AbstractRestClient";

/**
* RestClient is already tested vie the AbstractRestClient test, so we will extend RestClient
* with CustomRestClient to test things above and beyond RestClient.
*/

describe("RestClient tests", () => {
let setPasswordAuthSpy: any;

beforeEach(() => {
jest.clearAllMocks();

// pretend that basic auth was successfully set
setPasswordAuthSpy = jest.spyOn(AbstractRestClient.prototype as any, "setPasswordAuth");
setPasswordAuthSpy.mockReturnValue(true);
});

it("should add our custom header", async () => {
Expand Down
Loading
Loading