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

feat: Support multiple organizations API key. #68

Merged
merged 9 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
],
"rules": {
"no-useless-constructor": "off",
"@typescript-eslint/ban-ts-comment": "off"
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/no-non-null-assertion": "off"
}
}
12 changes: 9 additions & 3 deletions src/api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { DeploymentStepLogsResponse, DeploymentStepResponse } from "./types";

class ApiClient {
private credentials?: { username: string; password: string };
private currentOrganizationId?: string;
private readonly instance: AxiosInstance;
constructor() {
this.instance = axios.create({ baseURL: `https://${ENV0_API_URL}` });
Expand All @@ -16,11 +17,16 @@ class ApiClient {
});
}

public init(credentials: { username: string; password: string }) {
public init(
credentials: { username: string; password: string },
organizationId: string
) {
this.credentials = credentials;
this.currentOrganizationId = organizationId;
}

public clearCredentials() {
this.currentOrganizationId = undefined;
this.credentials = undefined;
}

Expand Down Expand Up @@ -62,11 +68,11 @@ class ApiClient {
return response.data;
}

public async getEnvironments(organizationId: string) {
public async getEnvironments() {
const response = await this.instance.get<EnvironmentModel[]>(
`/environments`,
{
params: { organizationId, isActive: true },
params: { organizationId: this.currentOrganizationId, isActive: true },
}
);

Expand Down
52 changes: 41 additions & 11 deletions src/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@

export class AuthService {
constructor(private readonly context: vscode.ExtensionContext) {}

private selectedOrgId: string | undefined;

public getSelectedOrg() {
return this.selectedOrgId;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why hold the this id in 2 different places, shouldn't there be a single source of truth? ( probably here and not in api client )

Copy link
Contributor Author

@meniRoy meniRoy Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely agree. I chose this approach because it aligns with our existing method of managing user credentials.

When I was creating the auth service, my intention was to pass an instance of it to the API client to avoid this very issue. However, I received this comment

4c8a496
also saved selected org id in store

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its up to you 👍🏻
this was the only important comment approving 👍🏻


public registerLoginCommand(onLogin: () => void) {
const disposable = vscode.commands.registerCommand(
"env0.login",
Expand Down Expand Up @@ -47,9 +54,9 @@
},
});

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
if (await this.validateUserCredentials(keyId!, secret!)) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const selectedOrgId = await this.pickOrganization(keyId!, secret!);
if (selectedOrgId) {
this.selectedOrgId = selectedOrgId;
await this.storeAuthData(keyId!, secret!);
await onLogin();
}
Expand Down Expand Up @@ -89,27 +96,50 @@
};
}

private async validateUserCredentials(keyId: string, secret: string) {
private async pickOrganization(keyId: string, secret: string) {
// Displaying a loading indicator to inform the user that something is happening
return await vscode.window.withProgress(
{ location: { viewId: ENV0_ENVIRONMENTS_VIEW_ID } },
async () => {
try {
await axios.get(`https://${ENV0_API_URL}/organizations`, {
auth: { username: keyId, password: secret },
validateStatus: function (status) {
return status >= 200 && status < 300;
},
const orgsRes = await axios.get(
`https://${ENV0_API_URL}/organizations`,
{
auth: { username: keyId, password: secret },
validateStatus: function (status) {
return status >= 200 && status < 300;
},
}
);
if (orgsRes.data.length === 1) {
return orgsRes.data[0].id;
}
const orgs = orgsRes.data.map((org: any) => ({

Check warning on line 117 in src/auth.ts

View workflow job for this annotation

GitHub Actions / test

Unexpected any. Specify a different type
name: org.name,
id: org.id,
}));
const items: vscode.QuickPickItem[] = orgs.map((org: any) => ({

Check warning on line 121 in src/auth.ts

View workflow job for this annotation

GitHub Actions / test

Unexpected any. Specify a different type
label: org.name,
description: org.id,
}));

const selectedItem = await vscode.window.showQuickPick(items, {
placeHolder: "Select an organization",
});
return true;
const selectedOrgId = selectedItem?.description;
alonnoga marked this conversation as resolved.
Show resolved Hide resolved
if (!selectedOrgId) {
vscode.window.showErrorMessage("No organization selected");
return undefined;
}
return selectedOrgId;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (e: any) {
if (e?.response?.status >= 400 && e?.response?.status < 500) {
showInvalidCredentialsMessage();
} else {
showUnexpectedErrorMessage();
}
return false;
return undefined;
}
}
);
Expand Down
5 changes: 4 additions & 1 deletion src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ const init = async (
environmentsTree: vscode.TreeView<Environment>,
authService: AuthService
) => {
apiClient.init(await authService.getApiKeyCredentials());
apiClient.init(
await authService.getApiKeyCredentials(),
authService.getSelectedOrg()!
);
extensionState.setLoggedIn(true);
await loadEnvironments(environmentsDataProvider);
};
Expand Down
15 changes: 3 additions & 12 deletions src/get-environments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,7 @@ function repositoriesEqual(rep1: string, rep2: string): boolean {
export async function getEnvironmentsForBranch() {
let environments: EnvironmentModel[] = [];

const organizationId = await getOrganizationId();

if (organizationId) {
environments = await getEnvironments(organizationId);
}
environments = await getEnvironments();

if (environments.length > 0) {
const { currentBranch, repository } = getGitRepoAndBranch();
Expand All @@ -64,17 +60,12 @@ export async function getEnvironmentsForBranch() {
return environments;
}

async function getEnvironments(organizationId: string) {
async function getEnvironments() {
try {
return apiClient.getEnvironments(organizationId);
return apiClient.getEnvironments();
} catch (e) {
console.log(e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where do console logs go in a vs code extension?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

19cf61c
we don’t need this function anymore because we handle errors from getEnvironments here

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just asking for general knowledge 👍🏻

}

return [];
}

async function getOrganizationId() {
const organizations = await apiClient.getOrganizations();
return organizations[0]?.id;
}
12 changes: 12 additions & 0 deletions src/test/integration/mocks/quick-pick.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as jestMock from "jest-mock";
import * as vscode from "vscode";

let quickPickSpy: jestMock.SpyInstance<any>;

Check warning on line 4 in src/test/integration/mocks/quick-pick.ts

View workflow job for this annotation

GitHub Actions / test

Unexpected any. Specify a different type
export const spyOnQuickPick = () => {
quickPickSpy = jestMock.spyOn(vscode.window, "showQuickPick");
return quickPickSpy;
};

export const resetSpyOnQuickPick = () => {
quickPickSpy?.mockReset?.();
};
4 changes: 2 additions & 2 deletions src/test/integration/mocks/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ const assertAuth = (credentials: Credentials, authHeader: string | null) => {
};

export const mockGetOrganization = (
organizationId: string,
orgs: { id: string; name: string }[],
alonnoga marked this conversation as resolved.
Show resolved Hide resolved
credentials?: Credentials
) => {
server.use(
rest.get(`https://${ENV0_API_URL}/organizations`, (req, res, ctx) => {
if (credentials) {
assertAuth(credentials, req.headers.get("Authorization"));
}
return res(ctx.json([{ id: organizationId }]));
return res(ctx.json(orgs));
})
);
};
Expand Down
66 changes: 53 additions & 13 deletions src/test/integration/suite/authentication.test.it.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import {
import * as jestMock from "jest-mock";
import * as vscode from "vscode";
import expect from "expect";
import { afterEach, beforeEach } from "mocha";
import { afterEach } from "mocha";
import { resetSpyOnQuickPick, spyOnQuickPick } from "../mocks/quick-pick";

const orgId = "org-id";
const envName = "my env";
const auth = { keyId: "key-id", secret: "key-secret" };
const environmentMock = getEnvironmentMock(
Expand All @@ -30,25 +30,28 @@ const environmentMock = getEnvironmentMock(
name: envName,
}
);
const selectedOrg = { name: "my org", id: "org-id" };
const initMocksAndLogin = async (moreOrgs: typeof selectedOrg[] = []) => {
mockGetOrganization([selectedOrg, ...moreOrgs], auth);
mockGetEnvironment(selectedOrg.id, [environmentMock], auth);
mockGitRepoAndBranch("main", "[email protected]:user/repo.git");
mockGetDeploymentStepsApiResponse();
await login(auth);

await waitFor(() => expect(getFirstEnvStatus()).toContain("ACTIVE"));
};

suite("authentication", function () {
this.timeout(1000 * 10);
beforeEach(async () => {
mockGetOrganization(orgId, auth);
mockGetEnvironment(orgId, [environmentMock], auth);
mockGitRepoAndBranch("main", "[email protected]:user/repo.git");
mockGetDeploymentStepsApiResponse();

await login(auth);
await waitFor(() => expect(getFirstEnvStatus()).toContain("ACTIVE"));
});

afterEach(async () => {
await logout();
await resetExtension();
resetSpyOnQuickPick();
});

test("should call redeploy with the credentials provided on login", async () => {
await initMocksAndLogin();
const onRedeployCalled = jestMock.fn();
mockRedeployApiResponse(environmentMock.id, auth, onRedeployCalled);

Expand All @@ -57,13 +60,14 @@ suite("authentication", function () {
});

test("should call redeploy with updated credentials when logout and login again ", async () => {
await initMocksAndLogin();
await logout();
const newAuthData = {
keyId: "different-key-id",
secret: "different-key-secret",
};
mockGetOrganization(orgId, newAuthData);
mockGetEnvironment(orgId, [environmentMock], newAuthData);
mockGetOrganization([selectedOrg], newAuthData);
mockGetEnvironment(selectedOrg.id, [environmentMock], newAuthData);

await login(newAuthData);
await waitFor(() => expect(getFirstEnvStatus()).toContain("ACTIVE"));
Expand All @@ -75,11 +79,47 @@ suite("authentication", function () {
});

test("should show login message when logout", async () => {
await initMocksAndLogin();
await logout();
await waitFor(() =>
expect(getEnvironmentsView().message).toContain(
"you are logged out. in order to log in, run the command 'env0.login'"
)
);
});

test("should show pick organization message when login", async () => {
const onQuickPick = spyOnQuickPick();

const secondOrg = { name: "second org", id: "second-org-id" };
initMocksAndLogin([secondOrg]);
await waitFor(() =>
expect(onQuickPick).toHaveBeenCalledWith(
[selectedOrg, secondOrg].map((org) => ({
label: org.name,
description: org.id,
})),
{
placeHolder: "Select an organization",
}
)
);
});

test("should show environments for the selected org", async () => {
const secondOrg = { name: "second org", id: "second-org-id" };
const onQuickPick = spyOnQuickPick();
onQuickPick.mockImplementationOnce(() =>
Promise.resolve({ label: secondOrg.name, description: secondOrg.id })
);
mockGetOrganization([selectedOrg, secondOrg], auth);
mockGetEnvironment(secondOrg.id, [environmentMock], auth);
mockGitRepoAndBranch("main", "[email protected]:user/repo.git");
mockGetDeploymentStepsApiResponse();
await login(auth);

await waitFor(() =>
expect(getFirstEnvironment().name).toBe(environmentMock.name)
);
});
});
7 changes: 4 additions & 3 deletions src/test/integration/suite/deployment-logs.test.it.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import expect from "expect";
import { DeploymentStatus } from "../../../types";

const auth = { keyId: "key-id", secret: "key-secret" };
const orgId = "org-id";
const organization = { name: "my org", id: "org-id" };
const orgId = organization.id;
const firstEnvName = "my env";
const secondEnvName = "my env 2";

Expand Down Expand Up @@ -62,8 +63,8 @@ suite("deployment logs", function () {
mockOutputChannel();
await resetExtension(); // we need to resat because we are mocking the output channel
const environments = [firstEnvironmentMock, secondEnvironmentMock];
mockGetOrganization(orgId, auth);
mockGetEnvironment(orgId, environments, auth);
mockGetOrganization([organization], auth);
mockGetEnvironment(organization.id, environments, auth);
await login(auth);
await waitFor(() => expect(getFirstEnvStatus()).toBe("ACTIVE"));
});
Expand Down
7 changes: 4 additions & 3 deletions src/test/integration/suite/environment-actions.test.it.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ import {
} from "../mocks/notification-message";

const auth = { keyId: "key-id", secret: "key-secret" };
const orgId = "org-id";
const organization = { name: "my org", id: "org-id" };
const orgId = organization.id;

const initTest = async (environments: EnvironmentModel[]) => {
mockGetOrganization(orgId, auth);
mockGetEnvironment(orgId, environments, auth);
mockGetOrganization([organization], auth);
mockGetEnvironment(organization.id, environments, auth);
mockGitRepoAndBranch("main", "[email protected]:user/repo.git");
mockGetDeploymentStepsApiResponse();
spyOnShowMessage();
Expand Down
Loading