From bb2786f814a5a1d3a3035b4bf94fa130424bab86 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Fri, 21 Jul 2023 10:21:23 +0200 Subject: [PATCH 1/7] chore: store the user id and org with the auth token on login --- core/package.json | 6 +- core/src/cloud/api.ts | 43 +++- core/src/commands/login.ts | 39 +++- core/src/config-store/global.ts | 2 + .../unit/src/cloud/{api.ts => auth-token.ts} | 24 ++- core/test/unit/src/commands/login.ts | 200 +++++++++++++++++- package-lock.json | 10 +- 7 files changed, 307 insertions(+), 17 deletions(-) rename core/test/unit/src/cloud/{api.ts => auth-token.ts} (76%) diff --git a/core/package.json b/core/package.json index 73db795da2..8e84408b92 100644 --- a/core/package.json +++ b/core/package.json @@ -246,7 +246,7 @@ "is-subset": "^0.1.1", "md5": "^2.3.0", "mocha": "^10.2.0", - "nock": "^12.0.3", + "nock": "^13.3.2", "node-fetch": "^2.7.0", "nodemon": "^3.0.1", "nyc": "^15.1.0", @@ -279,8 +279,8 @@ "integ-local": "GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"remote-only\" npm run _integ", "integ-minikube": "GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"remote-only\" npm run _integ", "integ-remote": "GARDEN_INTEG_TEST_MODE=remote GARDEN_SKIP_TESTS=local-only npm run _integ", - "test": "mocha", - "test:silly": "GARDEN_LOGGER_TYPE=basic GARDEN_LOG_LEVEL=silly mocha" + "test": "NODE_OPTIONS=--no-experimental-fetch mocha", + "test:silly": "NODE_OPTIONS=--no-experimental-fetch GARDEN_LOGGER_TYPE=basic GARDEN_LOG_LEVEL=silly mocha" }, "pkg": { "scripts": [ diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index 9885028010..8c97ec6c3b 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -100,6 +100,13 @@ export interface CloudSessionResponse { shortId: string } +// Internal representation of the cloud user profile +export interface CloudUserProfile { + userId: string + organizationName: string + domain: string +} + export interface CloudSession extends CloudSessionResponse { api: CloudApi id: string @@ -257,7 +264,8 @@ export class CloudApi { log: Log, globalConfigStore: GlobalConfigStore, tokenResponse: AuthTokenResponse, - domain: string + domain: string, + userProfile: CloudUserProfile | undefined = undefined ) { const distroName = getCloudDistributionName(domain) @@ -275,6 +283,8 @@ export class CloudApi { token: tokenResponse.token, refreshToken: tokenResponse.refreshToken, validity: add(new Date(), { seconds: validityMs / 1000 }), + userId: userProfile?.userId, + organizationName: userProfile?.organizationName, }) log.debug("Saved client auth token to config store") } catch (error) { @@ -327,6 +337,27 @@ export class CloudApi { return (await CloudApi.getStoredAuthToken(log, globalConfigStore, domain))?.token } + /** + * Retrieve a user and user organization based on the values stored in the auth token. + */ + static async getAuthTokenUserProfile( + log: Log, + globalConfigStore: GlobalConfigStore, + domain: string + ): Promise { + const authToken = await CloudApi.getStoredAuthToken(log, globalConfigStore, domain) + + if (!authToken) { + return undefined + } + + if (!authToken.userId || !authToken.organizationName) { + return undefined + } + + return { userId: authToken.userId, organizationName: authToken.organizationName, domain } + } + /** * If a persisted client auth token exists, deletes it. */ @@ -461,7 +492,15 @@ export class CloudApi { refreshToken: rt.value || "", tokenValidity: res.data.jwtValidity, } - await CloudApi.saveAuthToken(this.log, this.globalConfigStore, tokenObj, this.domain) + let userProfile: CloudUserProfile | undefined + if (token.userId && token.organizationName) { + userProfile = { + userId: token.userId, + organizationName: token.organizationName, + domain: this.domain, + } + } + await CloudApi.saveAuthToken(this.log, this.globalConfigStore, tokenObj, this.domain, userProfile) } catch (err) { if (!(err instanceof GotHttpError)) { throw err diff --git a/core/src/commands/login.ts b/core/src/commands/login.ts index 9784ec8a53..c58e9575e9 100644 --- a/core/src/commands/login.ts +++ b/core/src/commands/login.ts @@ -9,7 +9,7 @@ import { Command, CommandParams, CommandResult } from "./base" import { printHeader } from "../logger/util" import dedent = require("dedent") -import { AuthTokenResponse, CloudApi, getGardenCloudDomain } from "../cloud/api" +import { AuthTokenResponse, CloudApi, CloudUserProfile, getGardenCloudDomain } from "../cloud/api" import { Log } from "../logger/log-entry" import { ConfigurationError, TimeoutError, InternalError, CloudApiError } from "../exceptions" import { AuthRedirectServer } from "../cloud/auth" @@ -86,8 +86,10 @@ export class LoginCommand extends Command<{}, Opts> { const distroName = getCloudDistributionName(cloudDomain) + let cloudApi: CloudApi | undefined + try { - const cloudApi = await CloudApi.factory({ log, cloudDomain, skipLogging: true, globalConfigStore }) + cloudApi = await CloudApi.factory({ log, cloudDomain, skipLogging: true, globalConfigStore }) if (cloudApi) { log.info({ msg: `You're already logged in to ${cloudDomain}.` }) @@ -111,8 +113,39 @@ export class LoginCommand extends Command<{}, Opts> { log.info({ msg: `Logging in to ${cloudDomain}...` }) const tokenResponse = await login(log, cloudDomain, garden.events) + // Save the token, then try to create a cloud API instance and retrieve the profile await CloudApi.saveAuthToken(log, globalConfigStore, tokenResponse, cloudDomain) - log.info({ msg: `Successfully logged in to ${cloudDomain}.` }) + + try { + cloudApi = await CloudApi.factory({ log, cloudDomain, skipLogging: true, globalConfigStore }) + + // this is a best effort request to retrieve the profile and + // store with the token + let userProfile: CloudUserProfile | undefined + + try { + const remoteProfile = await cloudApi?.getProfile() + + if (remoteProfile && remoteProfile.id && remoteProfile.organization.name) { + userProfile = { + userId: remoteProfile.id, + organizationName: remoteProfile.organization.name, + domain: cloudDomain, + } + } + } catch (err) { + log.silly(`Failed to retreive the user profile after retrieving access token, ${err.toString()}`) + } + + await CloudApi.saveAuthToken(log, globalConfigStore, tokenResponse, cloudDomain, userProfile) + log.info({ msg: `Successfully logged in to ${cloudDomain}.` }) + } catch (err) { + await CloudApi.clearAuthToken(log, globalConfigStore, cloudDomain) + throw new CloudApiError({ + message: `Failed verifying user for ${cloudDomain}. Try logging in again.`, + wrappedErrors: [err], + }) + } return {} } diff --git a/core/src/config-store/global.ts b/core/src/config-store/global.ts index c299b6164d..f91fd013dc 100644 --- a/core/src/config-store/global.ts +++ b/core/src/config-store/global.ts @@ -55,6 +55,8 @@ const clientAuthTokenSchema = z.object({ token: z.string(), refreshToken: z.string(), validity: z.coerce.date(), + userId: z.string().optional(), + organizationName: z.string().optional(), }) const globalConfigSchema = z.object({ diff --git a/core/test/unit/src/cloud/api.ts b/core/test/unit/src/cloud/auth-token.ts similarity index 76% rename from core/test/unit/src/cloud/api.ts rename to core/test/unit/src/cloud/auth-token.ts index 0c08e803bb..ea60aa87b3 100644 --- a/core/test/unit/src/cloud/api.ts +++ b/core/test/unit/src/cloud/auth-token.ts @@ -9,12 +9,12 @@ import { expect } from "chai" import { getRootLogger } from "../../../../src/logger/logger" import { gardenEnv } from "../../../../src/constants" -import { CloudApi } from "../../../../src/cloud/api" +import { CloudApi, CloudUserProfile } from "../../../../src/cloud/api" import { uuidv4 } from "../../../../src/util/random" import { randomString } from "../../../../src/util/string" import { GlobalConfigStore } from "../../../../src/config-store/global" -describe("CloudApi", () => { +describe("AuthToken", () => { const log = getRootLogger().createLog() const domain = "https://garden." + randomString() const globalConfigStore = new GlobalConfigStore() @@ -36,6 +36,26 @@ describe("CloudApi", () => { expect(savedToken).to.eql(testToken.token) }) + it("should return a user profile if stored", async () => { + const testToken = { + token: uuidv4(), + refreshToken: uuidv4(), + tokenValidity: 9999, + } + const userProfile: CloudUserProfile = { + userId: "some-uuid", + organizationName: "some-org-name", + domain, + } + + await CloudApi.saveAuthToken(log, globalConfigStore, testToken, domain, userProfile) + const savedToken = await CloudApi.getAuthToken(log, globalConfigStore, domain) + expect(savedToken).to.eql(testToken.token) + + const savedProfile = await CloudApi.getAuthTokenUserProfile(log, globalConfigStore, domain) + expect(savedProfile).to.eql(userProfile) + }) + it("should return the value of GARDEN_AUTH_TOKEN if it's present", async () => { const tokenBackup = gardenEnv.GARDEN_AUTH_TOKEN const testToken = "token-from-env" diff --git a/core/test/unit/src/commands/login.ts b/core/test/unit/src/commands/login.ts index dc5e1d11ba..7bb820ed6a 100644 --- a/core/test/unit/src/commands/login.ts +++ b/core/test/unit/src/commands/login.ts @@ -6,6 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import nock from "nock" import { expect } from "chai" import td from "testdouble" import { @@ -20,7 +21,7 @@ import { AuthRedirectServer } from "../../../../src/cloud/auth" import { LoginCommand } from "../../../../src/commands/login" import { dedent, randomString } from "../../../../src/util/string" -import { CloudApi } from "../../../../src/cloud/api" +import { CloudApi, CloudApiTokenRefreshError } from "../../../../src/cloud/api" import { LogLevel } from "../../../../src/logger/logger" import { DEFAULT_GARDEN_CLOUD_DOMAIN, gardenEnv } from "../../../../src/constants" import { CloudApiError } from "../../../../src/exceptions" @@ -41,12 +42,45 @@ function loginCommandParams({ garden, opts = { "disable-project-check": false } } } +function makeScope({ domain, withProfileRequest = true }: { domain?: string; withProfileRequest?: boolean }) { + const scope = nock(domain || "http://example.invalid") + + scope.get("/api/token/verify").reply(200, { + status: "success", + data: { + valid: true, + }, + }) + + if (withProfileRequest) { + scope.get("/api/profile").reply(200, { + status: "success", + data: { + id: "1", + createdAt: new Date().toString(), + updatedAt: new Date().toString(), + name: "gordon", + vcsUsername: "gordon@garden.io", + serviceAccount: false, + organization: { + id: "1", + name: "garden", + }, + cachedPermissions: {}, + accessTokens: [], + groups: [], + }, + }) + } + + return scope +} + // In the tests below we stub out the auth redirect server but still emit the // token received event. describe("LoginCommand", () => { let tmpDir: TempDirectory let globalConfigStore: GlobalConfigStore - const loginOpts = { "disable-project-check": false } beforeEach(async () => { td.replace(AuthRedirectServer.prototype, "start", async () => {}) @@ -58,6 +92,7 @@ describe("LoginCommand", () => { afterEach(async () => { await tmpDir.cleanup() + nock.cleanAll() }) it("should log in if the project has a domain without an id", async () => { @@ -74,6 +109,11 @@ describe("LoginCommand", () => { globalConfigStore, }) + // intercept token verification and profile request + const scope = makeScope({ domain: garden.cloudDomain }) + + // emits an event which the local server expects, this is independent of the network + // requests scoped by nock setTimeout(() => { garden.events.emit("receivedToken", testToken) }, 500) @@ -84,6 +124,10 @@ describe("LoginCommand", () => { expect(savedToken).to.exist expect(savedToken!.token).to.eql(testToken.token) expect(savedToken!.refreshToken).to.eql(testToken.refreshToken) + expect(savedToken!.userId).to.eql("1") + expect(savedToken!.organizationName).to.eql("garden") + + expect(scope.done()).to.not.throw }) it("should log in if the project has a domain and an id", async () => { @@ -100,6 +144,9 @@ describe("LoginCommand", () => { globalConfigStore, }) + // intercept token verification and profile request + const scope = makeScope({ domain: garden.cloudDomain }) + setTimeout(() => { garden.events.emit("receivedToken", testToken) }, 500) @@ -110,6 +157,10 @@ describe("LoginCommand", () => { expect(savedToken).to.exist expect(savedToken!.token).to.eql(testToken.token) expect(savedToken!.refreshToken).to.eql(testToken.refreshToken) + expect(savedToken!.userId).to.eql("1") + expect(savedToken!.organizationName).to.eql("garden") + + expect(scope.done()).to.not.throw }) it("should be a no-op if the user is already logged in", async () => { @@ -162,6 +213,9 @@ describe("LoginCommand", () => { const cloudDomain = "https://example.invalid" Object.assign(garden, { cloudDomain }) + // intercept token verification and profile request + const scope = makeScope({ domain: garden.cloudDomain }) + setTimeout(() => { garden.events.emit("receivedToken", testToken) }, 500) @@ -171,6 +225,10 @@ describe("LoginCommand", () => { expect(savedToken).to.exist expect(savedToken!.token).to.eql(testToken.token) expect(savedToken!.refreshToken).to.eql(testToken.refreshToken) + expect(savedToken!.userId).to.eql("1") + expect(savedToken!.organizationName).to.eql("garden") + + expect(scope.done()).to.not.throw }) it("should fall back to the default garden cloud domain when none is defined", async () => { @@ -185,6 +243,9 @@ describe("LoginCommand", () => { commandInfo: { name: "foo", args: {}, opts: {} }, }) + // intercept token verification and profile request + const scope = makeScope({ domain: garden.cloudDomain }) + setTimeout(() => { garden.events.emit("receivedToken", testToken) }, 500) @@ -200,6 +261,10 @@ describe("LoginCommand", () => { expect(savedToken).to.exist expect(savedToken!.token).to.eql(testToken.token) expect(savedToken!.refreshToken).to.eql(testToken.refreshToken) + expect(savedToken!.userId).to.eql("1") + expect(savedToken!.organizationName).to.eql("garden") + + expect(scope.done()).to.not.throw }) it("should throw if the user has an invalid auth token", async () => { @@ -321,6 +386,9 @@ describe("LoginCommand", () => { // Need to override the default because we're using DummyGarden Object.assign(garden, { cloudDomain }) + // intercept token verification and profile request + const scope = makeScope({ domain: garden.cloudDomain }) + setTimeout(() => { garden.events.emit("receivedToken", testToken) }, 500) @@ -334,6 +402,125 @@ describe("LoginCommand", () => { expect(savedToken).to.exist expect(savedToken!.token).to.eql(testToken.token) expect(savedToken!.refreshToken).to.eql(testToken.refreshToken) + + expect(scope.done()).to.not.throw + }) + + it("should fail to login if the token verification to the cloud api fails", async () => { + const postfix = randomString() + const testToken = { + token: `dummy-token-${postfix}`, + refreshToken: `dummy-refresh-token-${postfix}`, + tokenValidity: 60, + } + const command = new LoginCommand() + const garden = await makeTestGarden(getDataDir("test-projects", "login", "has-domain-and-id"), { + noEnterprise: false, + commandInfo: { name: "foo", args: {}, opts: {} }, + globalConfigStore, + }) + + // intercept token verification and profile request + // const scope = makeScope({ domain: garden.cloudDomain, withProfileRequest: false }) + const scope = nock(garden.cloudDomain || "http://example.invalid") + + scope.get("/api/token/verify").reply(500) + + setTimeout(() => { + garden.events.emit("receivedToken", testToken) + }, 500) + + await expectError( + async () => await command.action(loginCommandParams({ garden })), + (err) => { + expect(err.message).to.contain(`Failed verifying user for ${garden.cloudDomain}`) + expect(err.wrappedErrors).to.have.length(1) + expect(err.wrappedErrors[0]).to.be.an.instanceof(CloudApiError) + expect(err.wrappedErrors[0].message).to.contain(`An error occurred while verifying client auth token`) + } + ) + + const savedToken = await CloudApi.getStoredAuthToken(garden.log, garden.globalConfigStore, garden.cloudDomain!) + expect(savedToken).to.not.exist + + expect(scope.done()).to.not.throw + }) + + it("should fail to login if the token refresh fails after a failed verification", async () => { + const postfix = randomString() + const testToken = { + token: `dummy-token-${postfix}`, + refreshToken: `dummy-refresh-token-${postfix}`, + tokenValidity: 60, + } + const command = new LoginCommand() + const garden = await makeTestGarden(getDataDir("test-projects", "login", "has-domain-and-id"), { + noEnterprise: false, + commandInfo: { name: "foo", args: {}, opts: {} }, + globalConfigStore, + }) + + // intercept token verification and profile request + // const scope = makeScope({ domain: garden.cloudDomain, withProfileRequest: false }) + const scope = nock(garden.cloudDomain || "http://example.invalid") + + scope.get("/api/token/verify").reply(401) + + scope.get("/api/token/refresh").reply(401) + + setTimeout(() => { + garden.events.emit("receivedToken", testToken) + }, 500) + + await expectError( + async () => await command.action(loginCommandParams({ garden })), + (err) => { + expect(err.message).to.contain(`Failed verifying user for ${garden.cloudDomain}`) + expect(err.wrappedErrors).to.have.length(1) + expect(err.wrappedErrors[0]).to.be.an.instanceof(CloudApiTokenRefreshError) + expect(err.wrappedErrors[0].message).to.contain(`An error occurred while verifying`) + } + ) + + const savedToken = await CloudApi.getStoredAuthToken(garden.log, garden.globalConfigStore, garden.cloudDomain!) + expect(savedToken).to.not.exist + + expect(scope.done()).to.not.throw + }) + + it("should succeed to login even if the profile retrieval fails", async () => { + const postfix = randomString() + const testToken = { + token: `dummy-token-${postfix}`, + refreshToken: `dummy-refresh-token-${postfix}`, + tokenValidity: 60, + } + const command = new LoginCommand() + const garden = await makeTestGarden(getDataDir("test-projects", "login", "has-domain-and-id"), { + noEnterprise: false, + commandInfo: { name: "foo", args: {}, opts: {} }, + globalConfigStore, + }) + + // intercept token verification and profile request + // const scope = makeScope({ domain: garden.cloudDomain, withProfileRequest: false }) + const scope = makeScope({ domain: garden.cloudDomain, withProfileRequest: false }) + scope.get("/api/profile").reply(500) + + setTimeout(() => { + garden.events.emit("receivedToken", testToken) + }, 500) + + await command.action(loginCommandParams({ garden })) + + const savedToken = await CloudApi.getStoredAuthToken(garden.log, garden.globalConfigStore, garden.cloudDomain!) + expect(savedToken).to.exist + expect(savedToken!.token).to.eql(testToken.token) + expect(savedToken!.refreshToken).to.eql(testToken.refreshToken) + expect(savedToken!.userId).to.be.undefined + expect(savedToken!.organizationName).to.be.undefined + + expect(scope.done()).to.not.throw }) context("GARDEN_AUTH_TOKEN set in env", () => { @@ -400,6 +587,9 @@ describe("LoginCommand", () => { globalConfigStore, }) + // intercept token verification and profile request + const scope = makeScope({ domain: garden.cloudDomain }) + setTimeout(() => { garden.events.emit("receivedToken", testToken) }, 500) @@ -414,6 +604,8 @@ describe("LoginCommand", () => { expect(savedToken).to.exist expect(savedToken!.token).to.eql(testToken.token) expect(savedToken!.refreshToken).to.eql(testToken.refreshToken) + + expect(scope.done()).to.not.throw }) it("should log in using the domain in GARDEN_CLOUD_DOMAIN", async () => { @@ -430,6 +622,9 @@ describe("LoginCommand", () => { globalConfigStore, }) + // intercept token verification and profile request + const scope = makeScope({ domain: garden.cloudDomain }) + setTimeout(() => { garden.events.emit("receivedToken", testToken) }, 500) @@ -448,6 +643,7 @@ describe("LoginCommand", () => { const logOutput = getLogMessages(garden.log, (entry) => entry.level === LogLevel.info).join("\n") expect(logOutput).to.include(`Logging in to ${gardenEnv.GARDEN_CLOUD_DOMAIN}`) + expect(scope.done()).to.not.throw }) after(() => { diff --git a/package-lock.json b/package-lock.json index e7615476ef..40c0168e5a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3491,7 +3491,7 @@ "is-subset": "^0.1.1", "md5": "^2.3.0", "mocha": "^10.2.0", - "nock": "^12.0.3", + "nock": "^13.3.2", "node-fetch": "^2.7.0", "nodemon": "^3.0.1", "nyc": "^15.1.0", @@ -14710,14 +14710,14 @@ } }, "core/node_modules/nock": { - "version": "12.0.3", - "resolved": "https://registry.npmjs.org/nock/-/nock-12.0.3.tgz", - "integrity": "sha512-QNb/j8kbFnKCiyqi9C5DD0jH/FubFGj5rt9NQFONXwQm3IPB0CULECg/eS3AU1KgZb/6SwUa4/DTRKhVxkGABw==", + "version": "13.3.3", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.3.3.tgz", + "integrity": "sha512-z+KUlILy9SK/RjpeXDiDUEAq4T94ADPHE3qaRkf66mpEhzc/ytOMm3Bwdrbq6k1tMWkbdujiKim3G2tfQARuJw==", "dev": true, "dependencies": { "debug": "^4.1.0", "json-stringify-safe": "^5.0.1", - "lodash": "^4.17.13", + "lodash": "^4.17.21", "propagate": "^2.0.0" }, "engines": { From ec8f31a904692ff9e507d54774921d42009d251b Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Wed, 2 Aug 2023 20:31:57 +0200 Subject: [PATCH 2/7] chore(analytics): optionally include user and project metadata for noProject commands --- core/src/analytics/analytics.ts | 85 ++++++++++++++---- core/src/commands/login.ts | 6 +- core/test/unit/src/analytics/analytics.ts | 102 +++++++++++++--------- core/test/unit/src/cli/analytics.ts | 6 +- 4 files changed, 137 insertions(+), 62 deletions(-) diff --git a/core/src/analytics/analytics.ts b/core/src/analytics/analytics.ts index abcda816be..5642e487f8 100644 --- a/core/src/analytics/analytics.ts +++ b/core/src/analytics/analytics.ts @@ -15,19 +15,21 @@ import { getPackageVersion, sleep, getDurationMsec } from "../util/util" import { SEGMENT_PROD_API_KEY, SEGMENT_DEV_API_KEY, gardenEnv } from "../constants" import { Log } from "../logger/log-entry" import hasha = require("hasha") -import { Garden } from "../garden" +import { DummyGarden, Garden } from "../garden" import { AnalyticsCommandResult, AnalyticsEventType } from "./analytics-types" import dedent from "dedent" import { getGitHubUrl } from "../docs/common" import { Profile } from "../util/profiling" import { ModuleConfig } from "../config/module" -import { UserResult } from "@garden-io/platform-api-types" import { uuidv4 } from "../util/random" import { GardenError, NodeJSErrnoErrorCodes, StackTraceMetadata } from "../exceptions" import { ActionConfigMap } from "../actions/types" import { actionKinds } from "../actions/types" import { getResultErrorProperties } from "./helpers" import segmentClient = require("analytics-node") +import { findProjectConfig } from "../config/base" +import { ProjectConfig } from "../config/project" +import { CloudApi, CloudUserProfile, getGardenCloudDomain } from "../cloud/api" const CI_USER = "ci-user" @@ -114,6 +116,7 @@ interface PropertiesBase { isLoggedIn: boolean cloudUserId?: string customer?: string + organizationName?: string ciName: string | null system: SystemInfo isCI: boolean @@ -214,6 +217,7 @@ interface IdentifyEvent { traits: { userIdV2: string customer?: string + organizationName?: string platform: string platformVersion: string gardenVersion: string @@ -270,8 +274,10 @@ export class AnalyticsHandler { private enterpriseProjectIdV2?: string private enterpriseDomainV2?: string private isLoggedIn: boolean + // These are set for a logged in user private cloudUserId?: string - private cloudCustomerName?: string + private cloudOrganizationName?: string + private cloudDomain?: string private ciName: string | null private systemConfig: SystemInfo private isCI: boolean @@ -292,6 +298,8 @@ export class AnalyticsHandler { cloudUser, isEnabled, ciInfo, + projectName, + fallbackCloudDomain, }: { garden: Garden log: Log @@ -300,8 +308,10 @@ export class AnalyticsHandler { moduleConfigs: ModuleConfig[] actionConfigs: ActionConfigMap isEnabled: boolean - cloudUser?: UserResult + cloudUser?: CloudUserProfile ciInfo: CiInfo + projectName: string + fallbackCloudDomain?: string }) { const segmentApiKey = gardenEnv.ANALYTICS_DEV ? SEGMENT_DEV_API_KEY : SEGMENT_PROD_API_KEY @@ -351,10 +361,10 @@ export class AnalyticsHandler { const originName = this.garden.vcsInfo.originUrl - const projectName = this.garden.projectName this.projectName = AnalyticsHandler.hash(projectName) this.projectNameV2 = AnalyticsHandler.hashV2(projectName) + // Note, this is not the project id from the Project config, its referred to as enterpriseProjectId below const projectId = originName || this.projectName this.projectId = AnalyticsHandler.hash(projectId) this.projectIdV2 = AnalyticsHandler.hashV2(projectId) @@ -363,20 +373,22 @@ export class AnalyticsHandler { // in the project level Garden configuration. Not to be confused with the anonymized project ID we generate from // the project name for the purpose of analytics. const enterpriseProjectId = this.garden.projectId - if (enterpriseProjectId) { + const enterpriseDomain = this.garden.cloudDomain + + // we only set this when defined in the project config since it indicates + // that the project has been connected to a cloud project + if (enterpriseProjectId && enterpriseDomain) { this.enterpriseProjectId = AnalyticsHandler.hash(enterpriseProjectId) this.enterpriseProjectIdV2 = AnalyticsHandler.hashV2(enterpriseProjectId) - } - - const enterpriseDomain = this.garden.cloudDomain - if (enterpriseDomain) { this.enterpriseDomain = AnalyticsHandler.hash(enterpriseDomain) this.enterpriseDomainV2 = AnalyticsHandler.hashV2(enterpriseDomain) } + // A user can be logged in to the community tier if (cloudUser) { this.cloudUserId = AnalyticsHandler.makeCloudUserId(cloudUser) - this.cloudCustomerName = cloudUser.organization.name + this.cloudOrganizationName = cloudUser.organizationName + this.cloudDomain = cloudUser.domain } this.isRecurringUser = getIsRecurringUser(analyticsConfig.firstRunAt, analyticsConfig.latestRunAt) @@ -387,7 +399,8 @@ export class AnalyticsHandler { anonymousId: anonymousUserId, traits: { userIdV2, - customer: cloudUser?.organization.name, + customer: cloudUser?.organizationName, + organizationName: cloudUser?.organizationName, platform: platform(), platformVersion: release(), gardenVersion: getPackageVersion(), @@ -445,15 +458,50 @@ export class AnalyticsHandler { const moduleConfigs = await garden.getRawModuleConfigs() const actionConfigs = await garden.getRawActionConfigs() - let cloudUser: UserResult | undefined + let cloudUser: CloudUserProfile | undefined if (garden.cloudApi) { try { - cloudUser = await garden.cloudApi?.getProfile() + const userProfile = await garden.cloudApi?.getProfile() + + if (userProfile && userProfile.id && userProfile.organization.name) { + cloudUser = { + userId: userProfile.id, + organizationName: userProfile.organization.name, + domain: garden.cloudApi.domain, + } + } } catch (err) { log.debug(`Getting profile from API failed with error: ${err}`) } } + // best effort load the project if this is a dummy garden instance + let projectName = garden.projectName + let fallbackCloudDomain: string | undefined + + // Not logged in and this is a dummy instance, try to best effort retrieve + // the user and project metadata + if (!garden.cloudApi && garden instanceof DummyGarden) { + const config: ProjectConfig | undefined = await findProjectConfig({ log, path: garden.projectRoot }) + + if (config) { + fallbackCloudDomain = getGardenCloudDomain(config.domain) + // override the project name since it will default to no-project + projectName = config.name + + // fallback to the stored user profile (this is done without verifying the token and the content) + const userProfile = await CloudApi.getAuthTokenUserProfile(log, garden.globalConfigStore, fallbackCloudDomain) + + if (userProfile) { + cloudUser = { + userId: userProfile.userId, + organizationName: userProfile.organizationName, + domain: fallbackCloudDomain, + } + } + } + } + if (isFirstRun && !ciInfo.isCi) { const gitHubUrl = getGitHubUrl("docs/misc/telemetry.md") const msg = dedent` @@ -502,6 +550,8 @@ export class AnalyticsHandler { isEnabled, ciInfo, anonymousUserId, + projectName, + fallbackCloudDomain, }) } @@ -529,8 +579,8 @@ export class AnalyticsHandler { } } - static makeCloudUserId(cloudUser: UserResult) { - return `${cloudUser.organization.name}_${cloudUser.id}` + static makeCloudUserId(cloudUser: CloudUserProfile) { + return `${cloudUser.organizationName}_${cloudUser.userId}` } /** @@ -548,7 +598,8 @@ export class AnalyticsHandler { enterpriseDomainV2: this.enterpriseDomainV2, isLoggedIn: this.isLoggedIn, ciName: this.ciName, - customer: this.cloudCustomerName, + customer: this.cloudOrganizationName, + organizationName: this.cloudOrganizationName, system: this.systemConfig, isCI: this.isCI, sessionId: this.sessionId, diff --git a/core/src/commands/login.ts b/core/src/commands/login.ts index c58e9575e9..43b2483524 100644 --- a/core/src/commands/login.ts +++ b/core/src/commands/login.ts @@ -11,7 +11,7 @@ import { printHeader } from "../logger/util" import dedent = require("dedent") import { AuthTokenResponse, CloudApi, CloudUserProfile, getGardenCloudDomain } from "../cloud/api" import { Log } from "../logger/log-entry" -import { ConfigurationError, TimeoutError, InternalError, CloudApiError } from "../exceptions" +import { ConfigurationError, TimeoutError, InternalError, CloudApiError, toGardenError } from "../exceptions" import { AuthRedirectServer } from "../cloud/auth" import { EventBus } from "../events/events" import { getCloudDistributionName } from "../util/util" @@ -134,7 +134,7 @@ export class LoginCommand extends Command<{}, Opts> { } } } catch (err) { - log.silly(`Failed to retreive the user profile after retrieving access token, ${err.toString()}`) + log.silly(`Failed to retreive the user profile after retrieving access token, ${err}`) } await CloudApi.saveAuthToken(log, globalConfigStore, tokenResponse, cloudDomain, userProfile) @@ -143,7 +143,7 @@ export class LoginCommand extends Command<{}, Opts> { await CloudApi.clearAuthToken(log, globalConfigStore, cloudDomain) throw new CloudApiError({ message: `Failed verifying user for ${cloudDomain}. Try logging in again.`, - wrappedErrors: [err], + wrappedErrors: [toGardenError(err)], }) } diff --git a/core/test/unit/src/analytics/analytics.ts b/core/test/unit/src/analytics/analytics.ts index eb628d35c4..eceab8001d 100644 --- a/core/test/unit/src/analytics/analytics.ts +++ b/core/test/unit/src/analytics/analytics.ts @@ -14,12 +14,7 @@ import { validate as validateUuid } from "uuid" import { makeTestGardenA, TestGarden, enableAnalytics, getDataDir, makeTestGarden, freezeTime } from "../../../helpers" import { FakeCloudApi, apiProjectName, apiRemoteOriginUrl } from "../../../helpers/api" import { AnalyticsHandler, CommandResultEvent, getAnonymousUserId } from "../../../../src/analytics/analytics" -import { - DEFAULT_BUILD_TIMEOUT_SEC, - DEFAULT_GARDEN_CLOUD_DOMAIN, - GardenApiVersion, - gardenEnv, -} from "../../../../src/constants" +import { DEFAULT_BUILD_TIMEOUT_SEC, GardenApiVersion, gardenEnv } from "../../../../src/constants" import { LogLevel, RootLogger } from "../../../../src/logger/logger" import { AnalyticsGlobalConfig } from "../../../../src/config-store/global" import { QuietWriter } from "../../../../src/logger/writers/quiet-writer" @@ -31,8 +26,6 @@ const host = "https://api.segment.io" const projectNameV2 = "discreet-sudden-struggle_95048f63dc14db38ed4138ffb6ff8999" describe("AnalyticsHandler", () => { - const scope = nock(host) - const time = new Date() const basicConfig: AnalyticsGlobalConfig = { anonymousUserId: "6d87dd61-0feb-4373-8c78-41cd010907e7", @@ -50,7 +43,13 @@ describe("AnalyticsHandler", () => { ciName: null, } + function setupNock() { + return nock(host) + } + before(async () => { + // make sure we don't do any external requests + nock.disableNetConnect() garden = await makeTestGardenA() resetAnalyticsConfig = await enableAnalytics(garden) }) @@ -58,6 +57,7 @@ describe("AnalyticsHandler", () => { after(async () => { await resetAnalyticsConfig() nock.cleanAll() + nock.enableNetConnect() }) describe("factory", () => { @@ -148,7 +148,7 @@ describe("AnalyticsHandler", () => { }) it("should identify the user with an anonymous ID", async () => { let payload: any - scope + const scope = setupNock() .post(`/v1/batch`, (body) => { const events = body.batch.map((event: any) => event.type) payload = body.batch @@ -193,7 +193,7 @@ describe("AnalyticsHandler", () => { }) it("should not identify the user if analytics is disabled", async () => { let payload: any - scope + const scope = setupNock() .post(`/v1/batch`, (body) => { const events = body.batch.map((event: any) => event.type) payload = body.batch @@ -258,6 +258,8 @@ describe("AnalyticsHandler", () => { }) it("should not replace the anonymous user ID with the Cloud user ID", async () => { + setupNock().post(`/v1/batch`).reply(200) + await garden.globalConfigStore.set("analytics", basicConfig) const now = freezeTime() @@ -273,6 +275,8 @@ describe("AnalyticsHandler", () => { }) }) it("should be enabled unless env var for disabling is set", async () => { + setupNock().post(`/v1/batch`).reply(200) + await garden.globalConfigStore.set("analytics", basicConfig) analytics = await AnalyticsHandler.factory({ garden, log: garden.log, ciInfo }) const isEnabledWhenNoEnvVar = analytics.isEnabled @@ -291,7 +295,7 @@ describe("AnalyticsHandler", () => { }) it("should identify the user with a Cloud ID", async () => { let payload: any - scope + const scope = setupNock() .post(`/v1/batch`, (body) => { const events = body.batch.map((event: any) => event.type) payload = body.batch @@ -303,17 +307,28 @@ describe("AnalyticsHandler", () => { await garden.globalConfigStore.set("analytics", basicConfig) analytics = await AnalyticsHandler.factory({ garden, log: garden.log, ciInfo }) + const newConfig = await garden.globalConfigStore.get("analytics") + + expect(newConfig).to.eql({ + anonymousUserId: "6d87dd61-0feb-4373-8c78-41cd010907e7", + firstRunAt: basicConfig.firstRunAt, + latestRunAt: now, + optedOut: false, + cloudProfileEnabled: true, + }) + await analytics.flush() expect(analytics.isEnabled).to.equal(true) expect(scope.isDone()).to.equal(true) expect(payload).to.eql([ { - userId: "garden_1", // This is the imporant part + userId: "garden_1", // This is the important part anonymousId: "6d87dd61-0feb-4373-8c78-41cd010907e7", traits: { userIdV2: AnalyticsHandler.hashV2("6d87dd61-0feb-4373-8c78-41cd010907e7"), customer: "garden", + organizationName: "garden", platform: payload[0].traits.platform, platformVersion: payload[0].traits.platformVersion, gardenVersion: payload[0].traits.gardenVersion, @@ -333,7 +348,7 @@ describe("AnalyticsHandler", () => { }) it("should not identify the user if analytics is disabled via env var", async () => { let payload: any - scope + const scope = setupNock() .post(`/v1/batch`, (body) => { const events = body.batch.map((event: any) => event.type) payload = body.batch @@ -368,7 +383,7 @@ describe("AnalyticsHandler", () => { }) it("should return the event with the correct project metadata", async () => { - scope.post(`/v1/batch`).reply(200) + setupNock().post(`/v1/batch`).reply(200) await garden.globalConfigStore.set("analytics", basicConfig) const now = freezeTime() @@ -385,10 +400,11 @@ describe("AnalyticsHandler", () => { projectNameV2, enterpriseProjectId: undefined, enterpriseProjectIdV2: undefined, - enterpriseDomain: AnalyticsHandler.hash(DEFAULT_GARDEN_CLOUD_DOMAIN), - enterpriseDomainV2: AnalyticsHandler.hashV2(DEFAULT_GARDEN_CLOUD_DOMAIN), + enterpriseDomain: undefined, + enterpriseDomainV2: undefined, isLoggedIn: false, customer: undefined, + organizationName: undefined, ciName: analytics["ciName"], system: analytics["systemConfig"], isCI: analytics["isCI"], @@ -413,7 +429,7 @@ describe("AnalyticsHandler", () => { }) }) it("should set the CI info if applicable", async () => { - scope.post(`/v1/batch`).reply(200) + setupNock().post(`/v1/batch`).reply(200) await garden.globalConfigStore.set("analytics", basicConfig) const now = freezeTime() @@ -430,10 +446,11 @@ describe("AnalyticsHandler", () => { projectNameV2, enterpriseProjectId: undefined, enterpriseProjectIdV2: undefined, - enterpriseDomain: AnalyticsHandler.hash(DEFAULT_GARDEN_CLOUD_DOMAIN), - enterpriseDomainV2: AnalyticsHandler.hashV2(DEFAULT_GARDEN_CLOUD_DOMAIN), + enterpriseDomain: undefined, + enterpriseDomainV2: undefined, isLoggedIn: false, customer: undefined, + organizationName: undefined, system: analytics["systemConfig"], isCI: true, ciName: "foo", @@ -458,7 +475,7 @@ describe("AnalyticsHandler", () => { }) }) it("should handle projects with no services, tests, or tasks", async () => { - scope.post(`/v1/batch`).reply(200) + setupNock().post(`/v1/batch`).reply(200) garden.setModuleConfigs([ { @@ -492,10 +509,11 @@ describe("AnalyticsHandler", () => { projectNameV2, enterpriseProjectId: undefined, enterpriseProjectIdV2: undefined, - enterpriseDomain: AnalyticsHandler.hash(DEFAULT_GARDEN_CLOUD_DOMAIN), - enterpriseDomainV2: AnalyticsHandler.hashV2(DEFAULT_GARDEN_CLOUD_DOMAIN), + enterpriseDomain: undefined, + enterpriseDomainV2: undefined, isLoggedIn: false, customer: undefined, + organizationName: undefined, ciName: analytics["ciName"], system: analytics["systemConfig"], isCI: analytics["isCI"], @@ -519,8 +537,8 @@ describe("AnalyticsHandler", () => { }, }) }) - it("should include enterprise metadata", async () => { - scope.post(`/v1/batch`).reply(200) + it("should include enterprise metadata from config", async () => { + setupNock().post(`/v1/batch`).reply(200) const root = getDataDir("test-projects", "login", "has-domain-and-id") garden = await makeTestGarden(root) @@ -546,6 +564,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectIdV2: AnalyticsHandler.hashV2("dummy-id"), isLoggedIn: false, customer: undefined, + organizationName: undefined, ciName: analytics["ciName"], system: analytics["systemConfig"], isCI: analytics["isCI"], @@ -570,7 +589,7 @@ describe("AnalyticsHandler", () => { }) }) it("should override the parentSessionId", async () => { - scope.post(`/v1/batch`).reply(200) + setupNock().post(`/v1/batch`).reply(200) const root = getDataDir("test-projects", "login", "has-domain-and-id") garden = await makeTestGarden(root) @@ -596,6 +615,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectIdV2: AnalyticsHandler.hashV2("dummy-id"), isLoggedIn: false, customer: undefined, + organizationName: undefined, ciName: analytics["ciName"], system: analytics["systemConfig"], isCI: analytics["isCI"], @@ -620,7 +640,7 @@ describe("AnalyticsHandler", () => { }) }) it("should have counts for action kinds", async () => { - scope.post(`/v1/batch`).reply(200) + setupNock().post(`/v1/batch`).reply(200) const root = getDataDir("test-projects", "config-templates") garden = await makeTestGarden(root) @@ -640,12 +660,13 @@ describe("AnalyticsHandler", () => { projectIdV2: AnalyticsHandler.hashV2(apiRemoteOriginUrl), projectName: AnalyticsHandler.hash("config-templates"), projectNameV2: AnalyticsHandler.hashV2("config-templates"), - enterpriseDomain: AnalyticsHandler.hash(DEFAULT_GARDEN_CLOUD_DOMAIN), - enterpriseDomainV2: AnalyticsHandler.hashV2(DEFAULT_GARDEN_CLOUD_DOMAIN), + enterpriseDomain: undefined, + enterpriseDomainV2: undefined, enterpriseProjectId: undefined, enterpriseProjectIdV2: undefined, isLoggedIn: false, customer: undefined, + organizationName: undefined, ciName: analytics["ciName"], system: analytics["systemConfig"], isCI: analytics["isCI"], @@ -685,7 +706,7 @@ describe("AnalyticsHandler", () => { }) it("should return the event as a success", async () => { - scope.post(`/v1/batch`).reply(200) + setupNock().post(`/v1/batch`).reply(200) await garden.globalConfigStore.set("analytics", basicConfig) @@ -712,10 +733,11 @@ describe("AnalyticsHandler", () => { projectNameV2, enterpriseProjectId: undefined, enterpriseProjectIdV2: undefined, - enterpriseDomain: AnalyticsHandler.hash(DEFAULT_GARDEN_CLOUD_DOMAIN), - enterpriseDomainV2: AnalyticsHandler.hashV2(DEFAULT_GARDEN_CLOUD_DOMAIN), + enterpriseDomain: undefined, + enterpriseDomainV2: undefined, isLoggedIn: false, customer: undefined, + organizationName: undefined, ciName: analytics["ciName"], system: analytics["systemConfig"], isCI: analytics["isCI"], @@ -740,7 +762,7 @@ describe("AnalyticsHandler", () => { }) }) it("should return the event as a failure with nested error metadata", async () => { - scope.post(`/v1/batch`).reply(200) + setupNock().post(`/v1/batch`).reply(200) await garden.globalConfigStore.set("analytics", basicConfig) @@ -821,7 +843,7 @@ describe("AnalyticsHandler", () => { }) }) it("should return the event as a failure with multiple errors", async () => { - scope.post(`/v1/batch`).reply(200) + const scope = setupNock().post(`/v1/batch`).reply(200) await garden.globalConfigStore.set("analytics", basicConfig) @@ -877,11 +899,13 @@ describe("AnalyticsHandler", () => { // That's why there are usually two mock requests per test below. describe("flush", () => { const getEvents = (body: any) => - body.batch.map((event: any) => ({ - event: event.event, - type: event.type, - name: event.properties.name, - })) + body.batch + .filter((event: any) => event.type === "track") + .map((event: any) => ({ + event: event.event, + type: event.type, + name: event.properties.name, + })) beforeEach(async () => { garden = await makeTestGardenA() @@ -896,7 +920,7 @@ describe("AnalyticsHandler", () => { }) it("should wait for pending events on network delays", async () => { - scope + const scope = setupNock() .post(`/v1/batch`, (body) => { // Assert that the event batch contains a single "track" event return isEqual(getEvents(body), [ diff --git a/core/test/unit/src/cli/analytics.ts b/core/test/unit/src/cli/analytics.ts index c21cafbbf5..ad3b2ddfd0 100644 --- a/core/test/unit/src/cli/analytics.ts +++ b/core/test/unit/src/cli/analytics.ts @@ -51,7 +51,7 @@ describe("cli analytics", () => { } } - it.skip("should access the version check service", async () => { + it("should access the version check service", async () => { const scope = nock("https://get.garden.io") scope.get("/version").query(true).reply(200) @@ -63,7 +63,7 @@ describe("cli analytics", () => { expect(scope.done()).to.not.throw }) - it.skip("should wait for queued analytic events to flush", async () => { + it("should wait for queued analytic events to flush", async () => { const scope = nock("https://api.segment.io") // Each command run result in two events: @@ -99,7 +99,7 @@ describe("cli analytics", () => { expect(scope.done()).to.not.throw }) - it.skip("should not send analytics if disabled for command", async () => { + it("should not send analytics if disabled for command", async () => { const scope = nock("https://api.segment.io") scope.post(`/v1/batch`).reply(201) From 532bfbc0fe9449d92edc3c12d8cd98fff14e2325 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Tue, 29 Aug 2023 17:40:53 +0200 Subject: [PATCH 3/7] test(analytics): introduced tests for noProject commands --- core/src/analytics/analytics.ts | 30 +++- core/src/cli/cli.ts | 28 ++-- core/test/setup.ts | 1 + core/test/unit/src/cli/analytics.ts | 202 ++++++++++++++++++++++--- core/test/unit/src/cloud/auth-token.ts | 16 +- 5 files changed, 241 insertions(+), 36 deletions(-) diff --git a/core/src/analytics/analytics.ts b/core/src/analytics/analytics.ts index 5642e487f8..a5ffdba0dc 100644 --- a/core/src/analytics/analytics.ts +++ b/core/src/analytics/analytics.ts @@ -282,7 +282,7 @@ export class AnalyticsHandler { private systemConfig: SystemInfo private isCI: boolean private sessionId: string - private pendingEvents: Map + private pendingEvents: Map protected garden: Garden private projectMetadata: ProjectMetadata public isEnabled: boolean @@ -386,7 +386,7 @@ export class AnalyticsHandler { // A user can be logged in to the community tier if (cloudUser) { - this.cloudUserId = AnalyticsHandler.makeCloudUserId(cloudUser) + this.cloudUserId = AnalyticsHandler.makeUniqueCloudUserId(cloudUser) this.cloudOrganizationName = cloudUser.organizationName this.cloudDomain = cloudUser.domain } @@ -579,7 +579,7 @@ export class AnalyticsHandler { } } - static makeCloudUserId(cloudUser: CloudUserProfile) { + static makeUniqueCloudUserId(cloudUser: CloudUserProfile) { return `${cloudUser.organizationName}_${cloudUser.userId}` } @@ -597,6 +597,7 @@ export class AnalyticsHandler { enterpriseDomain: this.enterpriseDomain, enterpriseDomainV2: this.enterpriseDomainV2, isLoggedIn: this.isLoggedIn, + cloudUserId: this.cloudUserId, ciName: this.ciName, customer: this.cloudOrganizationName, organizationName: this.cloudOrganizationName, @@ -658,7 +659,20 @@ export class AnalyticsHandler { if (!this.segment || !this.isEnabled) { return false } - this.segment.identify(event) + + const eventUid = uuidv4() + this.pendingEvents.set(eventUid, event) + this.segment.identify(event, (err: any) => { + this.pendingEvents.delete(eventUid) + + this.log.silly(dedent`Tracking identify event. + Payload: + ${JSON.stringify(event)} + `) + if (err && this.log) { + this.log.debug(`Error sending identify tracking event: ${err}`) + } + }) return event } @@ -825,7 +839,13 @@ export class AnalyticsHandler { if (this.pendingEvents.size === 0 || retry >= 3) { if (this.pendingEvents.size > 0) { const pendingEvents = Array.from(this.pendingEvents.values()) - .map((event) => event.event) + .map((event: SegmentEvent | IdentifyEvent) => { + if ("event" in event) { + return event.event + } else { + return event + } + }) .join(", ") this.log.debug(`Timed out while waiting for events to flush: ${pendingEvents}`) } diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index c0ce68e042..314e894339 100644 --- a/core/src/cli/cli.ts +++ b/core/src/cli/cli.ts @@ -66,6 +66,7 @@ export interface GardenCliParams { plugins?: GardenPluginReference[] initLogger?: boolean cloudApiFactory?: CloudApiFactory + globalConfigStoreDir?: string } function hasHelpFlag(argv: minimist.ParsedArgs) { @@ -81,11 +82,18 @@ export class GardenCli { private initLogger: boolean public processRecord?: GardenProcess protected cloudApiFactory: CloudApiFactory - - constructor({ plugins, initLogger = false, cloudApiFactory = CloudApi.factory }: GardenCliParams = {}) { + private globalConfigStore: GlobalConfigStore + + constructor({ + plugins, + globalConfigStoreDir: globalConfigStorePath, + initLogger = false, + cloudApiFactory = CloudApi.factory, + }: GardenCliParams = {}) { this.plugins = plugins || [] this.initLogger = initLogger this.cloudApiFactory = cloudApiFactory + this.globalConfigStore = new GlobalConfigStore(globalConfigStorePath) const commands = sortBy(getBuiltinCommands(), (c) => c.name) commands.forEach((command) => this.addCommand(command)) @@ -222,9 +230,7 @@ ${renderCommands(commands)} const commandLoggerType = command.getTerminalWriterType({ opts: parsedOpts, args: parsedArgs }) getRootLogger().setTerminalWriter(getTerminalWriterType({ silent, output, loggerTypeOpt, commandLoggerType })) - const globalConfigStore = new GlobalConfigStore() - - await validateRuntimeRequirementsCached(log, globalConfigStore, checkRequirements) + await validateRuntimeRequirementsCached(log, this.globalConfigStore, checkRequirements) command.printHeader({ log, args: parsedArgs, opts: parsedOpts }) const sessionId = uuidv4() @@ -239,7 +245,7 @@ ${renderCommands(commands)} const distroName = getCloudDistributionName(cloudDomain) try { - cloudApi = await this.cloudApiFactory({ log, cloudDomain, globalConfigStore }) + cloudApi = await this.cloudApiFactory({ log, cloudDomain, globalConfigStore: this.globalConfigStore }) } catch (err) { if (err instanceof CloudApiTokenRefreshError) { log.warn(dedent` @@ -273,6 +279,7 @@ ${renderCommands(commands)} forceRefresh, variableOverrides: parsedCliVars, plugins: this.plugins, + globalConfigStore: this.globalConfigStore, cloudApi, } @@ -324,7 +331,7 @@ ${renderCommands(commands)} if (processRecord) { // Update the db record for the process - await globalConfigStore.update("activeProcesses", String(processRecord.pid), { + await this.globalConfigStore.update("activeProcesses", String(processRecord.pid), { command: command.name, sessionId, persistent, @@ -338,7 +345,9 @@ ${renderCommands(commands)} } } - analytics = await garden.getAnalyticsHandler() + if (command.enableAnalytics) { + analytics = await garden.getAnalyticsHandler() + } // Register log file writers. We need to do this after the Garden class is initialised because // the file writers depend on the project root. @@ -571,8 +580,7 @@ ${renderCommands(commands)} } if (!processRecord) { - const globalConfigStore = new GlobalConfigStore() - processRecord = await registerProcess(globalConfigStore, command.getFullName(), args) + processRecord = await registerProcess(this.globalConfigStore, command.getFullName(), args) } this.processRecord = processRecord! diff --git a/core/test/setup.ts b/core/test/setup.ts index cb5f342962..0c6a406c23 100644 --- a/core/test/setup.ts +++ b/core/test/setup.ts @@ -32,6 +32,7 @@ exports.mochaHooks = { getDefaultProfiler().setEnabled(true) gardenEnv.GARDEN_DISABLE_ANALYTICS = true + gardenEnv.GARDEN_DISABLE_VERSION_CHECK = true testFlags.expandErrors = true testFlags.disableShutdown = true }, diff --git a/core/test/unit/src/cli/analytics.ts b/core/test/unit/src/cli/analytics.ts index ad3b2ddfd0..90ec9684d0 100644 --- a/core/test/unit/src/cli/analytics.ts +++ b/core/test/unit/src/cli/analytics.ts @@ -10,35 +10,52 @@ import nock from "nock" import { expect } from "chai" import { GardenCli } from "../../../../src/cli/cli" -import { GlobalConfigStore } from "../../../../src/config-store/global" import { TestGarden, enableAnalytics, makeTestGardenA } from "../../../helpers" import { Command } from "../../../../src/commands/base" import { isEqual } from "lodash" import { TestGardenCli } from "../../../helpers/cli" +import { AnalyticsHandler } from "../../../../src/analytics/analytics" +import { DEFAULT_GARDEN_CLOUD_DOMAIN, gardenEnv } from "../../../../src/constants" +import { CloudApi, CloudUserProfile } from "../../../../src/cloud/api" +import { uuidv4 } from "../../../../src/util/random" +import { getRootLogger } from "../../../../src/logger/logger" +import path from "path" // TODO: These tests are skipped because they fail repeatedly in CI, but works fine locally describe("cli analytics", () => { let cli: GardenCli - const globalConfigStore = new GlobalConfigStore() + + let garden: TestGarden + let resetAnalyticsConfig: Function + + before(async () => { + nock.disableNetConnect() + }) + + after(async () => { + nock.enableNetConnect() + nock.cleanAll() + }) beforeEach(async () => { - cli = new TestGardenCli() garden = await makeTestGardenA() + const configStoreDir = path.dirname(garden.globalConfigStore.getConfigPath()) + // initialize based on the garden project temp dir to avoid using the default global config + cli = new TestGardenCli({ globalConfigStoreDir: configStoreDir }) resetAnalyticsConfig = await enableAnalytics(garden) }) afterEach(async () => { if (cli.processRecord && cli.processRecord.pid) { - await globalConfigStore.delete("activeProcesses", String(cli.processRecord.pid)) + await garden.globalConfigStore.delete("activeProcesses", String(cli.processRecord.pid)) } await resetAnalyticsConfig() + // make sure we get a new analytics instance after each run + AnalyticsHandler.clearInstance() nock.cleanAll() }) - let garden: TestGarden - let resetAnalyticsConfig: Function - class TestCommand extends Command { name = "test-command" help = "hilfe!" @@ -51,29 +68,49 @@ describe("cli analytics", () => { } } - it("should access the version check service", async () => { - const scope = nock("https://get.garden.io") - scope.get("/version").query(true).reply(200) + describe("version check service", () => { + beforeEach(async () => { + // the version check service is mocked here so its safe to enable the check in tests + gardenEnv.GARDEN_DISABLE_VERSION_CHECK = false + }) - const command = new TestCommand() - cli.addCommand(command) + afterEach(async () => { + gardenEnv.GARDEN_DISABLE_VERSION_CHECK = true + }) - await cli.run({ args: ["test-command"], exitOnError: false }) + it("should access the version check service", async () => { + const scope = nock("https://get.garden.io") + scope.get("/version").query(true).reply(200) - expect(scope.done()).to.not.throw + const command = new TestCommand() + cli.addCommand(command) + + await cli.run({ args: ["test-command"], exitOnError: false, cwd: garden.projectRoot }) + + expect(scope.done()).to.not.throw + }) }) it("should wait for queued analytic events to flush", async () => { const scope = nock("https://api.segment.io") + // Initially there is always an identify + scope + .post(`/v1/batch`, (body) => { + const identify = body.batch.filter((event: any) => event.type === "identify").map((event: any) => event) + return identify.length === 1 + }) + .reply(200) + // Each command run result in two events: // 'Run Command' and 'Command Result' + scope .post(`/v1/batch`, (body) => { const events = body.batch.map((event: any) => ({ - event: event.event, + event: event?.event, type: event.type, - name: event.properties.name, + name: event.properties?.name, })) return isEqual(events, [ @@ -94,7 +131,7 @@ describe("cli analytics", () => { const command = new TestCommand() cli.addCommand(command) - await cli.run({ args: ["test-command"], exitOnError: false }) + await cli.run({ args: ["test-command"], exitOnError: false, cwd: garden.projectRoot }) expect(scope.done()).to.not.throw }) @@ -102,15 +139,142 @@ describe("cli analytics", () => { it("should not send analytics if disabled for command", async () => { const scope = nock("https://api.segment.io") - scope.post(`/v1/batch`).reply(201) + scope.post(`/v1/batch`).reply(200) const command = new TestCommand() command.enableAnalytics = false cli.addCommand(command) - await cli.run({ args: ["test-command"], exitOnError: false }) + await cli.run({ args: ["test-command"], exitOnError: false, cwd: garden.projectRoot }) expect(scope.isDone()).to.equal(false) + expect(scope.pendingMocks().length).to.equal(1) + }) + + it("should include project name when noProject is set", async () => { + const scope = nock("https://api.segment.io") + + // Each command run result in two events: + // 'Run Command' and 'Command Result' + scope + .post(`/v1/batch`, (body) => { + const identify = body.batch.filter((event: any) => event.type === "identify").map((event: any) => event) + return identify.length === 1 + }) + .reply(200) + + scope + .post(`/v1/batch`, (body) => { + const events = body.batch.map((event: any) => ({ + event: event.event, + type: event.type, + name: event.properties.name, + projectName: event.properties.projectNameV2, + })) + + expect(events).to.eql([ + { + event: "Run Command", + type: "track", + name: "test-command", + projectName: AnalyticsHandler.hashV2("test-project-a"), + }, + { + event: "Command Result", + type: "track", + name: "test-command", + projectName: AnalyticsHandler.hashV2("test-project-a"), + }, + ]) + return true + }) + .reply(200) + + const command = new TestCommand() + cli.addCommand(command) + + await cli.run({ args: ["test-command"], exitOnError: false, cwd: garden.projectRoot }) + + expect(scope.done()).to.not.throw + }) + + describe("with logged in user", () => { + const log = getRootLogger().createLog() + const domain = DEFAULT_GARDEN_CLOUD_DOMAIN + const cloudUserId = "user-id" + const cloudOrganizationName = "organization-name" + const uniqueCloudUserId = `${cloudOrganizationName}_${cloudUserId}` + + beforeEach(async () => { + // Save the auth token + const testToken = { + token: uuidv4(), + refreshToken: uuidv4(), + tokenValidity: 9999, + } + const userProfile: CloudUserProfile = { + userId: cloudUserId, + organizationName: cloudOrganizationName, + domain, + } + + await CloudApi.saveAuthToken(log, garden.globalConfigStore, testToken, domain, userProfile) + }) + + afterEach(async () => { + await CloudApi.clearAuthToken(log, garden.globalConfigStore, domain) + }) + + it("should include userId and organization name when noProject is set and the user has previously logged in", async () => { + const scope = nock("https://api.segment.io") + + // Each command run result in two events: + // 'Run Command' and 'Command Result' + scope + .post(`/v1/batch`, (body) => { + const identify = body.batch.filter((event: any) => event.type === "identify").map((event: any) => event) + return identify.length === 1 + }) + .reply(200) + + scope + .post(`/v1/batch`, (body) => { + const events = body.batch.map((event: any) => ({ + event: event.event, + type: event.type, + name: event.properties.name, + cloudUserId: event.properties.cloudUserId, + organizationName: event.properties.organizationName, + })) + + expect(events).to.eql([ + { + event: "Run Command", + type: "track", + name: "test-command", + cloudUserId: uniqueCloudUserId, + organizationName: cloudOrganizationName, + }, + { + event: "Command Result", + type: "track", + name: "test-command", + cloudUserId: uniqueCloudUserId, + organizationName: cloudOrganizationName, + }, + ]) + + return true + }) + .reply(200) + + const command = new TestCommand() + cli.addCommand(command) + + await cli.run({ args: ["test-command"], exitOnError: false, cwd: garden.projectRoot }) + + expect(scope.done()).to.not.throw + }) }) }) diff --git a/core/test/unit/src/cloud/auth-token.ts b/core/test/unit/src/cloud/auth-token.ts index ea60aa87b3..1a2272dcb5 100644 --- a/core/test/unit/src/cloud/auth-token.ts +++ b/core/test/unit/src/cloud/auth-token.ts @@ -13,11 +13,23 @@ import { CloudApi, CloudUserProfile } from "../../../../src/cloud/api" import { uuidv4 } from "../../../../src/util/random" import { randomString } from "../../../../src/util/string" import { GlobalConfigStore } from "../../../../src/config-store/global" +import { makeTempDir, TempDirectory } from "../../../helpers" describe("AuthToken", () => { const log = getRootLogger().createLog() - const domain = "https://garden." + randomString() - const globalConfigStore = new GlobalConfigStore() + let domain: string + let globalConfigStore: GlobalConfigStore + let tmpDir: TempDirectory + + beforeEach(async () => { + domain = "https://garden." + randomString() + tmpDir = await makeTempDir({ git: false }) + globalConfigStore = new GlobalConfigStore(tmpDir.path) + }) + + afterEach(async () => { + await tmpDir.cleanup() + }) describe("getAuthToken", () => { it("should return null when no auth token is present", async () => { From 57d61dc718c22c2fb2f9e003d23de347ba526981 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Wed, 30 Aug 2023 10:22:24 +0200 Subject: [PATCH 4/7] test(analytics): added cloud user id field to analytics struct --- core/test/unit/src/analytics/analytics.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/test/unit/src/analytics/analytics.ts b/core/test/unit/src/analytics/analytics.ts index eceab8001d..40d7f58d25 100644 --- a/core/test/unit/src/analytics/analytics.ts +++ b/core/test/unit/src/analytics/analytics.ts @@ -403,6 +403,7 @@ describe("AnalyticsHandler", () => { enterpriseDomain: undefined, enterpriseDomainV2: undefined, isLoggedIn: false, + cloudUserId: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -449,6 +450,7 @@ describe("AnalyticsHandler", () => { enterpriseDomain: undefined, enterpriseDomainV2: undefined, isLoggedIn: false, + cloudUserId: undefined, customer: undefined, organizationName: undefined, system: analytics["systemConfig"], @@ -512,6 +514,7 @@ describe("AnalyticsHandler", () => { enterpriseDomain: undefined, enterpriseDomainV2: undefined, isLoggedIn: false, + cloudUserId: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -563,6 +566,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectId: AnalyticsHandler.hash("dummy-id"), enterpriseProjectIdV2: AnalyticsHandler.hashV2("dummy-id"), isLoggedIn: false, + cloudUserId: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -614,6 +618,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectId: AnalyticsHandler.hash("dummy-id"), enterpriseProjectIdV2: AnalyticsHandler.hashV2("dummy-id"), isLoggedIn: false, + cloudUserId: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -665,6 +670,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectId: undefined, enterpriseProjectIdV2: undefined, isLoggedIn: false, + cloudUserId: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -736,6 +742,7 @@ describe("AnalyticsHandler", () => { enterpriseDomain: undefined, enterpriseDomainV2: undefined, isLoggedIn: false, + cloudUserId: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], From ada15d8140e63a28b2807d3fde7c10dd9feca6d8 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Wed, 30 Aug 2023 14:40:20 +0200 Subject: [PATCH 5/7] test(analytics): adding a best-effort verification of auth token validity + minor cleanup --- core/src/analytics/analytics.ts | 5 +-- core/src/cloud/api.ts | 4 ++ core/test/unit/src/cli/analytics.ts | 56 +++++++++++++------------- core/test/unit/src/cloud/auth-token.ts | 20 +++++++++ 4 files changed, 53 insertions(+), 32 deletions(-) diff --git a/core/src/analytics/analytics.ts b/core/src/analytics/analytics.ts index a5ffdba0dc..f0f54c2969 100644 --- a/core/src/analytics/analytics.ts +++ b/core/src/analytics/analytics.ts @@ -299,7 +299,6 @@ export class AnalyticsHandler { isEnabled, ciInfo, projectName, - fallbackCloudDomain, }: { garden: Garden log: Log @@ -311,7 +310,6 @@ export class AnalyticsHandler { cloudUser?: CloudUserProfile ciInfo: CiInfo projectName: string - fallbackCloudDomain?: string }) { const segmentApiKey = gardenEnv.ANALYTICS_DEV ? SEGMENT_DEV_API_KEY : SEGMENT_PROD_API_KEY @@ -388,7 +386,7 @@ export class AnalyticsHandler { if (cloudUser) { this.cloudUserId = AnalyticsHandler.makeUniqueCloudUserId(cloudUser) this.cloudOrganizationName = cloudUser.organizationName - this.cloudDomain = cloudUser.domain + this.isLoggedIn = true } this.isRecurringUser = getIsRecurringUser(analyticsConfig.firstRunAt, analyticsConfig.latestRunAt) @@ -551,7 +549,6 @@ export class AnalyticsHandler { ciInfo, anonymousUserId, projectName, - fallbackCloudDomain, }) } diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index 8c97ec6c3b..51068e2e1a 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -351,6 +351,10 @@ export class CloudApi { return undefined } + if (authToken.validity < new Date()) { + return undefined + } + if (!authToken.userId || !authToken.organizationName) { return undefined } diff --git a/core/test/unit/src/cli/analytics.ts b/core/test/unit/src/cli/analytics.ts index 90ec9684d0..fd73d35157 100644 --- a/core/test/unit/src/cli/analytics.ts +++ b/core/test/unit/src/cli/analytics.ts @@ -68,29 +68,6 @@ describe("cli analytics", () => { } } - describe("version check service", () => { - beforeEach(async () => { - // the version check service is mocked here so its safe to enable the check in tests - gardenEnv.GARDEN_DISABLE_VERSION_CHECK = false - }) - - afterEach(async () => { - gardenEnv.GARDEN_DISABLE_VERSION_CHECK = true - }) - - it("should access the version check service", async () => { - const scope = nock("https://get.garden.io") - scope.get("/version").query(true).reply(200) - - const command = new TestCommand() - cli.addCommand(command) - - await cli.run({ args: ["test-command"], exitOnError: false, cwd: garden.projectRoot }) - - expect(scope.done()).to.not.throw - }) - }) - it("should wait for queued analytic events to flush", async () => { const scope = nock("https://api.segment.io") @@ -173,7 +150,7 @@ describe("cli analytics", () => { projectName: event.properties.projectNameV2, })) - expect(events).to.eql([ + return isEqual(events, [ { event: "Run Command", type: "track", @@ -187,7 +164,6 @@ describe("cli analytics", () => { projectName: AnalyticsHandler.hashV2("test-project-a"), }, ]) - return true }) .reply(200) @@ -246,15 +222,17 @@ describe("cli analytics", () => { name: event.properties.name, cloudUserId: event.properties.cloudUserId, organizationName: event.properties.organizationName, + isLoggedIn: event.properties.isLoggedIn, })) - expect(events).to.eql([ + return isEqual(events, [ { event: "Run Command", type: "track", name: "test-command", cloudUserId: uniqueCloudUserId, organizationName: cloudOrganizationName, + isLoggedIn: true, }, { event: "Command Result", @@ -262,10 +240,9 @@ describe("cli analytics", () => { name: "test-command", cloudUserId: uniqueCloudUserId, organizationName: cloudOrganizationName, + isLoggedIn: true, }, ]) - - return true }) .reply(200) @@ -277,4 +254,27 @@ describe("cli analytics", () => { expect(scope.done()).to.not.throw }) }) + + describe("version check service", () => { + beforeEach(async () => { + // the version check service is mocked here so its safe to enable the check in tests + gardenEnv.GARDEN_DISABLE_VERSION_CHECK = false + }) + + afterEach(async () => { + gardenEnv.GARDEN_DISABLE_VERSION_CHECK = true + }) + + it("should access the version check service", async () => { + const scope = nock("https://get.garden.io") + scope.get("/version").query(true).reply(200) + + const command = new TestCommand() + cli.addCommand(command) + + await cli.run({ args: ["test-command"], exitOnError: false, cwd: garden.projectRoot }) + + expect(scope.done()).to.not.throw + }) + }) }) diff --git a/core/test/unit/src/cloud/auth-token.ts b/core/test/unit/src/cloud/auth-token.ts index 1a2272dcb5..4cab50265a 100644 --- a/core/test/unit/src/cloud/auth-token.ts +++ b/core/test/unit/src/cloud/auth-token.ts @@ -68,6 +68,26 @@ describe("AuthToken", () => { expect(savedProfile).to.eql(userProfile) }) + it("should not return a user profile when the token has expired", async () => { + const testToken = { + token: uuidv4(), + refreshToken: uuidv4(), + tokenValidity: -9999, + } + const userProfile: CloudUserProfile = { + userId: "some-uuid", + organizationName: "some-org-name", + domain, + } + + await CloudApi.saveAuthToken(log, globalConfigStore, testToken, domain, userProfile) + const savedToken = await CloudApi.getAuthToken(log, globalConfigStore, domain) + expect(savedToken).to.eql(testToken.token) + + const savedProfile = await CloudApi.getAuthTokenUserProfile(log, globalConfigStore, domain) + expect(savedProfile).to.be.undefined + }) + it("should return the value of GARDEN_AUTH_TOKEN if it's present", async () => { const tokenBackup = gardenEnv.GARDEN_AUTH_TOKEN const testToken = "token-from-env" From ed4145c6365d096033c6abda5fb3e61e15d4b10c Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Sun, 17 Sep 2023 12:51:49 +0200 Subject: [PATCH 6/7] chore(cloud): refactored save token api to use named params --- core/src/cloud/api.ts | 30 ++++++++++++++++------ core/src/commands/login.ts | 4 +-- core/test/unit/src/cli/analytics.ts | 8 +++++- core/test/unit/src/cloud/auth-token.ts | 8 +++--- core/test/unit/src/commands/login.ts | 21 +++++++++++++--- core/test/unit/src/commands/logout.ts | 35 ++++++++++++++++++++++---- 6 files changed, 83 insertions(+), 23 deletions(-) diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index 51068e2e1a..f5d0f25021 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -173,6 +173,14 @@ export interface CloudApiFactoryParams { skipLogging?: boolean } +export interface SaveAuthTokenParams { + log: Log + globalConfigStore: GlobalConfigStore + tokenResponse: AuthTokenResponse + domain: string + userProfile?: CloudUserProfile +} + export type CloudApiFactory = (params: CloudApiFactoryParams) => Promise /** @@ -260,13 +268,13 @@ export class CloudApi { return api } - static async saveAuthToken( - log: Log, - globalConfigStore: GlobalConfigStore, - tokenResponse: AuthTokenResponse, - domain: string, - userProfile: CloudUserProfile | undefined = undefined - ) { + static async saveAuthToken({ + log, + globalConfigStore, + tokenResponse, + domain, + userProfile = undefined, + }: SaveAuthTokenParams) { const distroName = getCloudDistributionName(domain) if (!tokenResponse.token) { @@ -504,7 +512,13 @@ export class CloudApi { domain: this.domain, } } - await CloudApi.saveAuthToken(this.log, this.globalConfigStore, tokenObj, this.domain, userProfile) + await CloudApi.saveAuthToken({ + log: this.log, + globalConfigStore: this.globalConfigStore, + tokenResponse: tokenObj, + domain: this.domain, + userProfile, + }) } catch (err) { if (!(err instanceof GotHttpError)) { throw err diff --git a/core/src/commands/login.ts b/core/src/commands/login.ts index 43b2483524..0cdaaf84e6 100644 --- a/core/src/commands/login.ts +++ b/core/src/commands/login.ts @@ -114,7 +114,7 @@ export class LoginCommand extends Command<{}, Opts> { log.info({ msg: `Logging in to ${cloudDomain}...` }) const tokenResponse = await login(log, cloudDomain, garden.events) // Save the token, then try to create a cloud API instance and retrieve the profile - await CloudApi.saveAuthToken(log, globalConfigStore, tokenResponse, cloudDomain) + await CloudApi.saveAuthToken({ log, globalConfigStore, tokenResponse, domain: cloudDomain }) try { cloudApi = await CloudApi.factory({ log, cloudDomain, skipLogging: true, globalConfigStore }) @@ -137,7 +137,7 @@ export class LoginCommand extends Command<{}, Opts> { log.silly(`Failed to retreive the user profile after retrieving access token, ${err}`) } - await CloudApi.saveAuthToken(log, globalConfigStore, tokenResponse, cloudDomain, userProfile) + await CloudApi.saveAuthToken({ log, globalConfigStore, tokenResponse, domain: cloudDomain, userProfile }) log.info({ msg: `Successfully logged in to ${cloudDomain}.` }) } catch (err) { await CloudApi.clearAuthToken(log, globalConfigStore, cloudDomain) diff --git a/core/test/unit/src/cli/analytics.ts b/core/test/unit/src/cli/analytics.ts index fd73d35157..57eb683d7c 100644 --- a/core/test/unit/src/cli/analytics.ts +++ b/core/test/unit/src/cli/analytics.ts @@ -195,7 +195,13 @@ describe("cli analytics", () => { domain, } - await CloudApi.saveAuthToken(log, garden.globalConfigStore, testToken, domain, userProfile) + await CloudApi.saveAuthToken({ + log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain, + userProfile, + }) }) afterEach(async () => { diff --git a/core/test/unit/src/cloud/auth-token.ts b/core/test/unit/src/cloud/auth-token.ts index 4cab50265a..6f6d7956e3 100644 --- a/core/test/unit/src/cloud/auth-token.ts +++ b/core/test/unit/src/cloud/auth-token.ts @@ -43,7 +43,7 @@ describe("AuthToken", () => { refreshToken: uuidv4(), tokenValidity: 9999, } - await CloudApi.saveAuthToken(log, globalConfigStore, testToken, domain) + await CloudApi.saveAuthToken({ log, globalConfigStore, tokenResponse: testToken, domain }) const savedToken = await CloudApi.getAuthToken(log, globalConfigStore, domain) expect(savedToken).to.eql(testToken.token) }) @@ -60,7 +60,7 @@ describe("AuthToken", () => { domain, } - await CloudApi.saveAuthToken(log, globalConfigStore, testToken, domain, userProfile) + await CloudApi.saveAuthToken({ log, globalConfigStore, tokenResponse: testToken, domain, userProfile }) const savedToken = await CloudApi.getAuthToken(log, globalConfigStore, domain) expect(savedToken).to.eql(testToken.token) @@ -80,7 +80,7 @@ describe("AuthToken", () => { domain, } - await CloudApi.saveAuthToken(log, globalConfigStore, testToken, domain, userProfile) + await CloudApi.saveAuthToken({ log, globalConfigStore, tokenResponse: testToken, domain, userProfile }) const savedToken = await CloudApi.getAuthToken(log, globalConfigStore, domain) expect(savedToken).to.eql(testToken.token) @@ -108,7 +108,7 @@ describe("AuthToken", () => { refreshToken: uuidv4(), tokenValidity: 9999, } - await CloudApi.saveAuthToken(log, globalConfigStore, testToken, domain) + await CloudApi.saveAuthToken({ log, globalConfigStore, tokenResponse: testToken, domain }) await CloudApi.clearAuthToken(log, globalConfigStore, domain) const savedToken = await CloudApi.getAuthToken(log, globalConfigStore, domain) expect(savedToken).to.be.undefined diff --git a/core/test/unit/src/commands/login.ts b/core/test/unit/src/commands/login.ts index 7bb820ed6a..64c409f94b 100644 --- a/core/test/unit/src/commands/login.ts +++ b/core/test/unit/src/commands/login.ts @@ -178,7 +178,12 @@ describe("LoginCommand", () => { globalConfigStore, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) td.replace(CloudApi.prototype, "checkClientAuthToken", async () => true) td.replace(CloudApi.prototype, "startInterval", async () => {}) @@ -282,7 +287,12 @@ describe("LoginCommand", () => { globalConfigStore, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) td.replace(CloudApi.prototype, "checkClientAuthToken", async () => false) td.replace(CloudApi.prototype, "refreshToken", async () => { throw new Error("bummer") @@ -313,7 +323,12 @@ describe("LoginCommand", () => { globalConfigStore, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) td.replace(CloudApi.prototype, "checkClientAuthToken", async () => false) td.replace(CloudApi.prototype, "refreshToken", async () => { throw new CloudApiError({ message: "bummer", responseStatusCode: 401 }) diff --git a/core/test/unit/src/commands/logout.ts b/core/test/unit/src/commands/logout.ts index 9a799e6ce2..4723c52595 100644 --- a/core/test/unit/src/commands/logout.ts +++ b/core/test/unit/src/commands/logout.ts @@ -60,7 +60,12 @@ describe("LogoutCommand", () => { globalConfigStore, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) td.replace(CloudApi.prototype, "checkClientAuthToken", async () => true) td.replace(CloudApi.prototype, "startInterval", async () => {}) td.replace(CloudApi.prototype, "post", async () => {}) @@ -98,7 +103,12 @@ describe("LogoutCommand", () => { commandInfo: { name: "foo", args: {}, opts: {} }, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) td.replace(CloudApi.prototype, "checkClientAuthToken", async () => true) td.replace(CloudApi.prototype, "startInterval", async () => {}) td.replace(CloudApi.prototype, "post", async () => {}) @@ -151,7 +161,12 @@ describe("LogoutCommand", () => { globalConfigStore, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) // Throw when initializing Enterprise API td.replace(CloudApi.prototype, "factory", async () => { throw new Error("Not tonight") @@ -191,7 +206,12 @@ describe("LogoutCommand", () => { globalConfigStore, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) // Throw when using Enterprise API to call logout endpoint td.replace(CloudApi.prototype, "post", async () => { throw new Error("Not tonight") @@ -249,7 +269,12 @@ describe("LogoutCommand", () => { globalConfigStore, }) - await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) + await CloudApi.saveAuthToken({ + log: garden.log, + globalConfigStore: garden.globalConfigStore, + tokenResponse: testToken, + domain: garden.cloudDomain!, + }) td.replace(CloudApi.prototype, "checkClientAuthToken", async () => true) td.replace(CloudApi.prototype, "startInterval", async () => {}) td.replace(CloudApi.prototype, "post", async () => {}) From 1d826c5deb641dc3267c611a9346fe8f6d0992c2 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Mon, 18 Sep 2023 10:33:00 +0200 Subject: [PATCH 7/7] chore(analytics): make sure we use enterpriseDomain/projectId from config only --- core/src/analytics/analytics.ts | 65 ++++++++++++++--------- core/test/unit/src/analytics/analytics.ts | 7 +++ 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/core/src/analytics/analytics.ts b/core/src/analytics/analytics.ts index f0f54c2969..d26c38b2e0 100644 --- a/core/src/analytics/analytics.ts +++ b/core/src/analytics/analytics.ts @@ -115,6 +115,7 @@ interface PropertiesBase { enterpriseDomainV2?: string isLoggedIn: boolean cloudUserId?: string + cloudDomain?: string customer?: string organizationName?: string ciName: string | null @@ -299,6 +300,8 @@ export class AnalyticsHandler { isEnabled, ciInfo, projectName, + configuredCloudDomain, + configuredCloudProjectId, }: { garden: Garden log: Log @@ -310,6 +313,8 @@ export class AnalyticsHandler { cloudUser?: CloudUserProfile ciInfo: CiInfo projectName: string + configuredCloudDomain?: string + configuredCloudProjectId?: string }) { const segmentApiKey = gardenEnv.ANALYTICS_DEV ? SEGMENT_DEV_API_KEY : SEGMENT_PROD_API_KEY @@ -370,28 +375,25 @@ export class AnalyticsHandler { // The enterprise project ID is the UID for this project in Garden Cloud that the user puts // in the project level Garden configuration. Not to be confused with the anonymized project ID we generate from // the project name for the purpose of analytics. - const enterpriseProjectId = this.garden.projectId - const enterpriseDomain = this.garden.cloudDomain - - // we only set this when defined in the project config since it indicates - // that the project has been connected to a cloud project - if (enterpriseProjectId && enterpriseDomain) { - this.enterpriseProjectId = AnalyticsHandler.hash(enterpriseProjectId) - this.enterpriseProjectIdV2 = AnalyticsHandler.hashV2(enterpriseProjectId) - this.enterpriseDomain = AnalyticsHandler.hash(enterpriseDomain) - this.enterpriseDomainV2 = AnalyticsHandler.hashV2(enterpriseDomain) + if (configuredCloudProjectId && configuredCloudDomain) { + this.enterpriseProjectId = AnalyticsHandler.hash(configuredCloudProjectId) + this.enterpriseProjectIdV2 = AnalyticsHandler.hashV2(configuredCloudProjectId) + this.enterpriseDomain = AnalyticsHandler.hash(configuredCloudDomain) + this.enterpriseDomainV2 = AnalyticsHandler.hashV2(configuredCloudDomain) } // A user can be logged in to the community tier if (cloudUser) { this.cloudUserId = AnalyticsHandler.makeUniqueCloudUserId(cloudUser) this.cloudOrganizationName = cloudUser.organizationName + this.cloudDomain = this.garden.cloudDomain this.isLoggedIn = true } this.isRecurringUser = getIsRecurringUser(analyticsConfig.firstRunAt, analyticsConfig.latestRunAt) const userIdV2 = AnalyticsHandler.hashV2(anonymousUserId) + this.identify({ userId: this.cloudUserId, anonymousId: anonymousUserId, @@ -475,31 +477,39 @@ export class AnalyticsHandler { // best effort load the project if this is a dummy garden instance let projectName = garden.projectName - let fallbackCloudDomain: string | undefined - // Not logged in and this is a dummy instance, try to best effort retrieve - // the user and project metadata - if (!garden.cloudApi && garden instanceof DummyGarden) { - const config: ProjectConfig | undefined = await findProjectConfig({ log, path: garden.projectRoot }) + let projectConfig: ProjectConfig | undefined - if (config) { - fallbackCloudDomain = getGardenCloudDomain(config.domain) - // override the project name since it will default to no-project - projectName = config.name + if (garden instanceof DummyGarden) { + // Not logged in and this is a dummy instance, try to best effort retrieve the config + projectConfig = await findProjectConfig({ log, path: garden.projectRoot }) - // fallback to the stored user profile (this is done without verifying the token and the content) - const userProfile = await CloudApi.getAuthTokenUserProfile(log, garden.globalConfigStore, fallbackCloudDomain) + // override the project name since it will default to no-project + if (projectConfig) { + projectName = projectConfig.name - if (userProfile) { - cloudUser = { - userId: userProfile.userId, - organizationName: userProfile.organizationName, - domain: fallbackCloudDomain, + if (!garden.cloudApi) { + const fallbackCloudDomain = getGardenCloudDomain(projectConfig.domain) + + // fallback to the stored user profile (this is done without verifying the token and the content) + const userProfile = await CloudApi.getAuthTokenUserProfile(log, garden.globalConfigStore, fallbackCloudDomain) + + if (userProfile) { + cloudUser = { + userId: userProfile.userId, + organizationName: userProfile.organizationName, + domain: fallbackCloudDomain, + } } } } + } else { + projectConfig = garden.getProjectConfig() } + const configuredCloudDomain = projectConfig?.domain + const configuredCloudProjectId = projectConfig?.id + if (isFirstRun && !ciInfo.isCi) { const gitHubUrl = getGitHubUrl("docs/misc/telemetry.md") const msg = dedent` @@ -549,6 +559,8 @@ export class AnalyticsHandler { ciInfo, anonymousUserId, projectName, + configuredCloudDomain, + configuredCloudProjectId, }) } @@ -595,6 +607,7 @@ export class AnalyticsHandler { enterpriseDomainV2: this.enterpriseDomainV2, isLoggedIn: this.isLoggedIn, cloudUserId: this.cloudUserId, + cloudDomain: this.cloudDomain, ciName: this.ciName, customer: this.cloudOrganizationName, organizationName: this.cloudOrganizationName, diff --git a/core/test/unit/src/analytics/analytics.ts b/core/test/unit/src/analytics/analytics.ts index 40d7f58d25..9492a6b007 100644 --- a/core/test/unit/src/analytics/analytics.ts +++ b/core/test/unit/src/analytics/analytics.ts @@ -404,6 +404,7 @@ describe("AnalyticsHandler", () => { enterpriseDomainV2: undefined, isLoggedIn: false, cloudUserId: undefined, + cloudDomain: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -451,6 +452,7 @@ describe("AnalyticsHandler", () => { enterpriseDomainV2: undefined, isLoggedIn: false, cloudUserId: undefined, + cloudDomain: undefined, customer: undefined, organizationName: undefined, system: analytics["systemConfig"], @@ -515,6 +517,7 @@ describe("AnalyticsHandler", () => { enterpriseDomainV2: undefined, isLoggedIn: false, cloudUserId: undefined, + cloudDomain: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -567,6 +570,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectIdV2: AnalyticsHandler.hashV2("dummy-id"), isLoggedIn: false, cloudUserId: undefined, + cloudDomain: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -619,6 +623,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectIdV2: AnalyticsHandler.hashV2("dummy-id"), isLoggedIn: false, cloudUserId: undefined, + cloudDomain: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -671,6 +676,7 @@ describe("AnalyticsHandler", () => { enterpriseProjectIdV2: undefined, isLoggedIn: false, cloudUserId: undefined, + cloudDomain: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"], @@ -743,6 +749,7 @@ describe("AnalyticsHandler", () => { enterpriseDomainV2: undefined, isLoggedIn: false, cloudUserId: undefined, + cloudDomain: undefined, customer: undefined, organizationName: undefined, ciName: analytics["ciName"],