From 95cd7d94f8ba307b119712348398e3f912825e65 Mon Sep 17 00:00:00 2001 From: menachem roytenburd Date: Thu, 28 Sep 2023 04:57:21 +0300 Subject: [PATCH] feat: Support multiple organizations API key. (#68) --- .eslintrc.json | 3 +- src/api-client.ts | 26 +++-- src/auth.ts | 100 +++++++++++++----- src/extension.ts | 3 +- src/get-environments.ts | 21 +--- src/test/integration/mocks/quick-pick.ts | 12 +++ src/test/integration/mocks/server.ts | 6 +- .../suite/authentication.test.it.ts | 69 +++++++++--- .../suite/deployment-logs.test.it.ts | 9 +- .../suite/environment-actions.test.it.ts | 9 +- .../integration/suite/environments.test.it.ts | 15 +-- src/test/unit/mocks/http.ts | 5 +- 12 files changed, 183 insertions(+), 95 deletions(-) create mode 100644 src/test/integration/mocks/quick-pick.ts diff --git a/.eslintrc.json b/.eslintrc.json index 1ea3cba..7652349 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -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" } } diff --git a/src/api-client.ts b/src/api-client.ts index 6849998..adc26cb 100644 --- a/src/api-client.ts +++ b/src/api-client.ts @@ -2,26 +2,27 @@ import axios, { AxiosInstance } from "axios"; import { ENV0_API_URL } from "./common"; import { EnvironmentModel } from "./get-environments"; import { DeploymentStepLogsResponse, DeploymentStepResponse } from "./types"; +import { AuthService } from "./auth"; class ApiClient { - private credentials?: { username: string; password: string }; private readonly instance: AxiosInstance; + private authService?: AuthService; constructor() { this.instance = axios.create({ baseURL: `https://${ENV0_API_URL}` }); this.instance.interceptors.request.use((config) => { - if (this.credentials) { - config.auth = this.credentials; + const credentials = this.authService?.getCredentials(); + if (credentials) { + config.auth = { + username: credentials.username, + password: credentials.password, + }; } return config; }); } - public init(credentials: { username: string; password: string }) { - this.credentials = credentials; - } - - public clearCredentials() { - this.credentials = undefined; + public init(authService: AuthService) { + this.authService = authService; } public async abortDeployment(deploymentId: string) { @@ -62,11 +63,14 @@ class ApiClient { return response.data; } - public async getEnvironments(organizationId: string) { + public async getEnvironments() { const response = await this.instance.get( `/environments`, { - params: { organizationId, isActive: true }, + params: { + organizationId: this.authService?.getCredentials().selectedOrgId, + isActive: true, + }, } ); diff --git a/src/auth.ts b/src/auth.ts index abade05..643ae53 100644 --- a/src/auth.ts +++ b/src/auth.ts @@ -6,11 +6,19 @@ import { showUnexpectedErrorMessage, } from "./notification-messages"; -const env0KeyIdKey = "env0.keyId"; -const env0SecretKey = "env0.secret"; +const env0KeyIdStoreKey = "env0.keyId"; +const env0SecretStoreKey = "env0.secret"; +const selectedOrgIdStoreKey = "env0.selectedOrgId"; export class AuthService { constructor(private readonly context: vscode.ExtensionContext) {} + + private credentials?: { + username: string; + password: string; + selectedOrgId: string; + }; + public registerLoginCommand(onLogin: () => void) { const disposable = vscode.commands.registerCommand( "env0.login", @@ -47,10 +55,9 @@ export class AuthService { }, }); - // 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 - await this.storeAuthData(keyId!, secret!); + const selectedOrgId = await this.pickOrganization(keyId!, secret!); + if (selectedOrgId) { + await this.storeAuthData(keyId!, secret!, selectedOrgId); await onLogin(); } } @@ -70,38 +77,68 @@ export class AuthService { } public async isLoggedIn() { - const { secret, keyId } = await this.getAuthData(); - return !!(secret && keyId); + const { secret, keyId, selectedOrgId } = await this.getAuthData(); + if (!(secret && keyId && selectedOrgId)) { + return false; + } + this.credentials = { username: keyId, password: secret, selectedOrgId }; + return true; } - public async getApiKeyCredentials() { - const { secret, keyId } = await this.getAuthData(); - if (!secret || !keyId) { - throw new Error("Could not read env0 api key values"); + public getCredentials() { + if (!this.credentials) { + // this should happen only if the user is logged out + throw new Error("Could not read credentials"); } - return { username: keyId, password: secret }; + + return this.credentials; } private async getAuthData() { return { - keyId: await this.context.secrets.get(env0KeyIdKey), - secret: await this.context.secrets.get(env0SecretKey), + keyId: await this.context.secrets.get(env0KeyIdStoreKey), + secret: await this.context.secrets.get(env0SecretStoreKey), + selectedOrgId: await this.context.secrets.get(selectedOrgIdStoreKey), }; } - 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) => ({ + name: org.name, + id: org.id, + })); + const items: vscode.QuickPickItem[] = orgs.map((org: any) => ({ + label: org.name, + description: org.id, + })); + + const selectedItem = await vscode.window.showQuickPick(items, { + placeHolder: "Select an organization", + ignoreFocusOut: true, }); - return true; + const selectedOrgId = selectedItem?.description; + 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) { @@ -109,19 +146,26 @@ export class AuthService { } else { showUnexpectedErrorMessage(); } - return false; + return undefined; } } ); } - private async storeAuthData(keyId: string, secret: string) { - await this.context.secrets.store(env0KeyIdKey, keyId); - await this.context.secrets.store(env0SecretKey, secret); + private async storeAuthData( + keyId: string, + secret: string, + selectedOrgId: string + ) { + this.credentials = { username: keyId, password: secret, selectedOrgId }; + await this.context.secrets.store(env0KeyIdStoreKey, keyId); + await this.context.secrets.store(env0SecretStoreKey, secret); + await this.context.secrets.store(selectedOrgIdStoreKey, selectedOrgId); } private async clearAuthData() { - await this.context.secrets.delete(env0KeyIdKey); - await this.context.secrets.delete(env0SecretKey); + await this.context.secrets.delete(env0KeyIdStoreKey); + await this.context.secrets.delete(env0SecretStoreKey); + await this.context.secrets.delete(selectedOrgIdStoreKey); } } diff --git a/src/extension.ts b/src/extension.ts index 0f522b9..0f9d977 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -70,7 +70,7 @@ const init = async ( environmentsTree: vscode.TreeView, authService: AuthService ) => { - apiClient.init(await authService.getApiKeyCredentials()); + apiClient.init(authService); extensionState.setLoggedIn(true); await loadEnvironments(environmentsDataProvider); }; @@ -81,7 +81,6 @@ const onLogOut = async () => { } stopEnvironmentPolling(); environmentsDataProvider.clear(); - apiClient.clearCredentials(); extensionState.setLoggedIn(false); }; diff --git a/src/get-environments.ts b/src/get-environments.ts index 9595eab..2f05807 100644 --- a/src/get-environments.ts +++ b/src/get-environments.ts @@ -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 apiClient.getEnvironments(); if (environments.length > 0) { const { currentBranch, repository } = getGitRepoAndBranch(); @@ -63,18 +59,3 @@ export async function getEnvironmentsForBranch() { } return environments; } - -async function getEnvironments(organizationId: string) { - try { - return apiClient.getEnvironments(organizationId); - } catch (e) { - console.log(e); - } - - return []; -} - -async function getOrganizationId() { - const organizations = await apiClient.getOrganizations(); - return organizations[0]?.id; -} diff --git a/src/test/integration/mocks/quick-pick.ts b/src/test/integration/mocks/quick-pick.ts new file mode 100644 index 0000000..c3fd8dd --- /dev/null +++ b/src/test/integration/mocks/quick-pick.ts @@ -0,0 +1,12 @@ +import * as jestMock from "jest-mock"; +import * as vscode from "vscode"; + +let quickPickSpy: jestMock.SpyInstance; +export const spyOnQuickPick = () => { + quickPickSpy = jestMock.spyOn(vscode.window, "showQuickPick"); + return quickPickSpy; +}; + +export const resetSpyOnQuickPick = () => { + quickPickSpy?.mockReset?.(); +}; diff --git a/src/test/integration/mocks/server.ts b/src/test/integration/mocks/server.ts index c666133..614dc74 100644 --- a/src/test/integration/mocks/server.ts +++ b/src/test/integration/mocks/server.ts @@ -28,8 +28,8 @@ const assertAuth = (credentials: Credentials, authHeader: string | null) => { assert.strictEqual(secret, credentials.secret); }; -export const mockGetOrganization = ( - organizationId: string, +export const mockGetOrganizations = ( + orgs: { id: string; name: string }[], credentials?: Credentials ) => { server.use( @@ -37,7 +37,7 @@ export const mockGetOrganization = ( if (credentials) { assertAuth(credentials, req.headers.get("Authorization")); } - return res(ctx.json([{ id: organizationId }])); + return res(ctx.json(orgs)); }) ); }; diff --git a/src/test/integration/suite/authentication.test.it.ts b/src/test/integration/suite/authentication.test.it.ts index a798712..f8a2e5e 100644 --- a/src/test/integration/suite/authentication.test.it.ts +++ b/src/test/integration/suite/authentication.test.it.ts @@ -2,7 +2,7 @@ import { mockGitRepoAndBranch } from "../mocks/git"; import { mockGetDeploymentStepsApiResponse, mockGetEnvironment, - mockGetOrganization, + mockGetOrganizations, mockRedeployApiResponse, } from "../mocks/server"; import { @@ -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( @@ -30,25 +30,28 @@ const environmentMock = getEnvironmentMock( name: envName, } ); +const selectedOrg = { name: "my org", id: "org-id" }; +const initMocksAndLogin = async (moreOrgs: typeof selectedOrg[] = []) => { + mockGetOrganizations([selectedOrg, ...moreOrgs], auth); + mockGetEnvironment(selectedOrg.id, [environmentMock], auth); + mockGitRepoAndBranch("main", "git@github.com: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", "git@github.com: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); @@ -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); + mockGetOrganizations([selectedOrg], newAuthData); + mockGetEnvironment(selectedOrg.id, [environmentMock], newAuthData); await login(newAuthData); await waitFor(() => expect(getFirstEnvStatus()).toContain("ACTIVE")); @@ -75,6 +79,7 @@ suite("authentication", function () { }); test("should show login message when logout", async () => { + await initMocksAndLogin(); await logout(); await waitFor(() => expect(getEnvironmentsView().message).toContain( @@ -82,4 +87,40 @@ suite("authentication", function () { ) ); }); + + 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, + })), + { + ignoreFocusOut: true, + 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 }) + ); + mockGetOrganizations([selectedOrg, secondOrg], auth); + mockGetEnvironment(secondOrg.id, [environmentMock], auth); + mockGitRepoAndBranch("main", "git@github.com:user/repo.git"); + mockGetDeploymentStepsApiResponse(); + await login(auth); + + await waitFor(() => + expect(getFirstEnvironment().name).toBe(environmentMock.name) + ); + }); }); diff --git a/src/test/integration/suite/deployment-logs.test.it.ts b/src/test/integration/suite/deployment-logs.test.it.ts index 97e8ae4..8a47a97 100644 --- a/src/test/integration/suite/deployment-logs.test.it.ts +++ b/src/test/integration/suite/deployment-logs.test.it.ts @@ -10,7 +10,7 @@ import { mockDeploymentLogsResponses, mockGetDeploymentApiResponse, mockGetEnvironment, - mockGetOrganization, + mockGetOrganizations, } from "../mocks/server"; import { clickOnEnvironmentByName, @@ -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"; @@ -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); + mockGetOrganizations([organization], auth); + mockGetEnvironment(organization.id, environments, auth); await login(auth); await waitFor(() => expect(getFirstEnvStatus()).toBe("ACTIVE")); }); diff --git a/src/test/integration/suite/environment-actions.test.it.ts b/src/test/integration/suite/environment-actions.test.it.ts index 4528ff8..16c2fd9 100644 --- a/src/test/integration/suite/environment-actions.test.it.ts +++ b/src/test/integration/suite/environment-actions.test.it.ts @@ -16,7 +16,7 @@ import { mockGetDeploymentStepsApiResponse, mockDestroyApiResponse, mockGetEnvironment, - mockGetOrganization, + mockGetOrganizations, mockRedeployApiResponse, } from "../mocks/server"; import { mockGitRepoAndBranch } from "../mocks/git"; @@ -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); + mockGetOrganizations([organization], auth); + mockGetEnvironment(organization.id, environments, auth); mockGitRepoAndBranch("main", "git@github.com:user/repo.git"); mockGetDeploymentStepsApiResponse(); spyOnShowMessage(); diff --git a/src/test/integration/suite/environments.test.it.ts b/src/test/integration/suite/environments.test.it.ts index 1996de4..a3239ab 100644 --- a/src/test/integration/suite/environments.test.it.ts +++ b/src/test/integration/suite/environments.test.it.ts @@ -1,4 +1,4 @@ -import { mockGetEnvironment, mockGetOrganization } from "../mocks/server"; +import { mockGetEnvironment, mockGetOrganizations } from "../mocks/server"; import { mockGitRepoAndBranch, mockNoGitRepo } from "../mocks/git"; // @ts-ignore import * as extension from "../../../../dist/extension.js"; @@ -16,10 +16,11 @@ import { EnvironmentModel } from "../../../get-environments"; import expect from "expect"; const auth = { keyId: "key-id", secret: "key-secret" }; -const orgId = "org-id"; +const organization = { name: "my org", id: "org-id" }; + const initTest = async (environments: EnvironmentModel[]) => { - mockGetOrganization(orgId, auth); - mockGetEnvironment(orgId, environments, auth); + mockGetOrganizations([organization], auth); + mockGetEnvironment(organization.id, environments, auth); mockGitRepoAndBranch("main", "git@github.com:user/repo.git"); await login(auth); }; @@ -80,7 +81,7 @@ suite("environments", function () { }), ]; mockNoGitRepo(); - mockGetOrganization(orgId, auth); + mockGetOrganizations([organization], auth); // we don't await on login because we want to test the loading message login(auth); await waitFor( @@ -88,7 +89,7 @@ suite("environments", function () { 10 ); mockGitRepoAndBranch("main", "git@github.com:user/repo.git"); - mockGetEnvironment(orgId, environments, auth, 2000); + mockGetEnvironment(organization.id, environments, auth, 2000); await waitFor(() => expect(getEnvironmentViewMessage()).toBe( "loading environments for branch main..." @@ -99,7 +100,7 @@ suite("environments", function () { test("should show could not find git branch message", async () => { mockNoGitRepo(); - mockGetOrganization(orgId, auth); + mockGetOrganizations([organization], auth); await login(auth); await waitFor(() => expect(getEnvironmentViewMessage()).toBe( diff --git a/src/test/unit/mocks/http.ts b/src/test/unit/mocks/http.ts index cf4ff8d..20d0bf2 100644 --- a/src/test/unit/mocks/http.ts +++ b/src/test/unit/mocks/http.ts @@ -10,7 +10,10 @@ export const mockValidateCredentialsRequest = (auth: Credentials) => { options?.auth?.username === auth.keyId && options?.auth?.password === auth.secret ) { - return Promise.resolve({ status: 200 }); + return Promise.resolve({ + status: 200, + data: [{ id: "org-id", name: "org name" }], + }); } return Promise.reject(new Error("not found")); };