From 0cb991219355fc1ef37ee6536c5fbb05274854ec Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Sat, 2 Dec 2023 11:13:38 +0200 Subject: [PATCH 1/7] feat: add config field and override param for require login --- core/src/cli/cli.ts | 25 ++++++++++++++++++++++++- core/src/config/project.ts | 8 ++++++++ core/src/constants.ts | 1 + 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index 5e6ef7bfe7..e778141465 100644 --- a/core/src/cli/cli.ts +++ b/core/src/cli/cli.ts @@ -16,7 +16,7 @@ import { shutdown, getPackageVersion } from "../util/util.js" import type { Command, CommandResult, BuiltinArgs } from "../commands/base.js" import { CommandGroup } from "../commands/base.js" import type { GardenError } from "../exceptions.js" -import { PluginError, toGardenError } from "../exceptions.js" +import { PluginError, RuntimeError, toGardenError } from "../exceptions.js" import type { GardenOpts } from "../garden.js" import { Garden, makeDummyGarden } from "../garden.js" import { getRootLogger, getTerminalWriterType, LogLevel, parseLogLevel, RootLogger } from "../logger/logger.js" @@ -275,6 +275,29 @@ ${renderCommands(commands)} throw err } } + + // If there is an ID in the project config and the user is not logged in (no cloudApi) + // 0.13 => check if login is required based on the `requireLogin` config value + if (!cloudApi && config && config.id) { + log.info("") + + // fallback to false if no variables are set + // TODO-0.14: requireLogin should default to true + const isLoginRequired = (gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE ?? config.requireLogin) || false + if (isLoginRequired) { + log.warn(dedent` + You are running this in a project with a Garden Cloud ID and logging in is required. + Please log in via the ${styles.command("garden login")} command.`) + throw new RuntimeError({ message: "" }) + } else { + log.warn( + `Warning: You are not logged in into Garden Cloud. Please log in via the ${styles.command( + "garden login" + )} command.` + ) + } + log.info("") + } } const commandInfo = { diff --git a/core/src/config/project.ts b/core/src/config/project.ts index adaaef644e..f834731287 100644 --- a/core/src/config/project.ts +++ b/core/src/config/project.ts @@ -208,6 +208,7 @@ export interface ProjectConfig extends BaseGardenResource { path: string id?: string domain?: string + requireLogin?: boolean configPath?: string proxy?: ProxyConfig defaultEnvironment: string @@ -323,6 +324,13 @@ export const projectSchema = createSchema({ .uri() .meta({ internal: true }) .description("The domain to use for cloud features. Should be the full API/backend URL."), + // TODO: Refer to enterprise documentation for more details. + requireLogin: joi + .boolean() + .default(false) + .meta({ internal: true }) + .description("Whether the project requires login to Garden Cloud."), + // Note: We provide a different schema below for actual validation, but need to define it this way for docs // because joi.alternatives() isn't handled well in the doc generation. environments: joi diff --git a/core/src/constants.ts b/core/src/constants.ts index 0909f3100f..26cf1f3ea3 100644 --- a/core/src/constants.ts +++ b/core/src/constants.ts @@ -105,4 +105,5 @@ export const gardenEnv = { // GARDEN_CLOUD_BUILDER will always override the config; That's why it doesn't have a default. // FIXME: If the environment variable is not set, asBool returns undefined, unlike the type suggests. That's why we cast to `boolean | undefined`. GARDEN_CLOUD_BUILDER: env.get("GARDEN_CLOUD_BUILDER").required(false).asBool() as boolean | undefined, + GARDEN_REQUIRE_LOGIN_OVERRIDE: env.get("GARDEN_REQUIRE_LOGIN_OVERRIDE").required(false).asBool(), } From a42474827af63ae5d334781fbae559b6ccbd25a2 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Fri, 8 Dec 2023 20:50:16 +0100 Subject: [PATCH 2/7] test(cloud): testing of require login function --- core/src/cli/cli.ts | 25 +---- core/src/cloud/api.ts | 99 +++++++++++++----- core/test/unit/src/cloud/api.ts | 177 ++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 50 deletions(-) diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index e778141465..5e6ef7bfe7 100644 --- a/core/src/cli/cli.ts +++ b/core/src/cli/cli.ts @@ -16,7 +16,7 @@ import { shutdown, getPackageVersion } from "../util/util.js" import type { Command, CommandResult, BuiltinArgs } from "../commands/base.js" import { CommandGroup } from "../commands/base.js" import type { GardenError } from "../exceptions.js" -import { PluginError, RuntimeError, toGardenError } from "../exceptions.js" +import { PluginError, toGardenError } from "../exceptions.js" import type { GardenOpts } from "../garden.js" import { Garden, makeDummyGarden } from "../garden.js" import { getRootLogger, getTerminalWriterType, LogLevel, parseLogLevel, RootLogger } from "../logger/logger.js" @@ -275,29 +275,6 @@ ${renderCommands(commands)} throw err } } - - // If there is an ID in the project config and the user is not logged in (no cloudApi) - // 0.13 => check if login is required based on the `requireLogin` config value - if (!cloudApi && config && config.id) { - log.info("") - - // fallback to false if no variables are set - // TODO-0.14: requireLogin should default to true - const isLoginRequired = (gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE ?? config.requireLogin) || false - if (isLoginRequired) { - log.warn(dedent` - You are running this in a project with a Garden Cloud ID and logging in is required. - Please log in via the ${styles.command("garden login")} command.`) - throw new RuntimeError({ message: "" }) - } else { - log.warn( - `Warning: You are not logged in into Garden Cloud. Please log in via the ${styles.command( - "garden login" - )} command.` - ) - } - log.info("") - } } const commandInfo = { diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index 3a9fa81135..fdd614fddc 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -167,6 +167,8 @@ export interface CloudApiFactoryParams { cloudDomain: string globalConfigStore: GlobalConfigStore skipLogging?: boolean + projectId?: string + requireLogin?: boolean } export type CloudApiFactory = (params: CloudApiFactoryParams) => Promise @@ -210,51 +212,96 @@ export class CloudApi { * Optionally skip logging during initialization. Useful for noProject commands that need to use the class * without all the "flair". */ - static async factory({ log, cloudDomain, globalConfigStore, skipLogging = false }: CloudApiFactoryParams) { + static async factory({ + log, + cloudDomain, + globalConfigStore, + skipLogging = false, + projectId = undefined, + requireLogin = false, + }: CloudApiFactoryParams) { const distroName = getCloudDistributionName(cloudDomain) const fixLevel = skipLogging ? LogLevel.silly : undefined const cloudFactoryLog = log.createLog({ fixLevel, name: getCloudLogSectionName(distroName), showDuration: true }) - cloudFactoryLog.debug("Initializing Garden Cloud API client.") + cloudFactoryLog.debug(`Initializing ${distroName} API client.`) const token = await CloudApi.getStoredAuthToken(log, globalConfigStore, cloudDomain) - if (!token && !gardenEnv.GARDEN_AUTH_TOKEN) { - log.debug( + const hasNoToken = !token && !gardenEnv.GARDEN_AUTH_TOKEN + + // fallback to false if no variables are set + // TODO-0.14: requireLogin should default to true + const isLoginRequired: boolean = + gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE !== undefined + ? gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE + : projectId !== undefined && requireLogin + + // Base case when the user is not logged in to cloud and the + // criteria for cloud login is not required: + // - The config parameter requiredLogin is false + // - The user is not running a project scoped command (no projectId) + if (hasNoToken && !isLoginRequired) { + cloudFactoryLog.debug( `No auth token found, proceeding without access to ${distroName}. Command results for this command run will not be available in ${distroName}.` ) return } - const api = new CloudApi({ log, domain: cloudDomain, globalConfigStore }) - const tokenIsValid = await api.checkClientAuthToken() + // Try to auth towards cloud + try { + const api = new CloudApi({ log, domain: cloudDomain, globalConfigStore }) + const tokenIsValid = await api.checkClientAuthToken() + + cloudFactoryLog.debug("Authorizing...") + + if (gardenEnv.GARDEN_AUTH_TOKEN) { + // Throw if using an invalid "CI" access token + if (!tokenIsValid) { + throw new CloudApiError({ + message: deline` + The provided access token is expired or has been revoked, please create a new + one from the ${distroName} UI.`, + }) + } + } else { + // Refresh the token if it's invalid. + if (!tokenIsValid) { + cloudFactoryLog.debug({ msg: `Current auth token is invalid, refreshing` }) - cloudFactoryLog.debug("Authorizing...") + // We can assert the token exists since we're not using GARDEN_AUTH_TOKEN + await api.refreshToken(token!) + } - if (gardenEnv.GARDEN_AUTH_TOKEN) { - // Throw if using an invalid "CI" access token - if (!tokenIsValid) { - throw new CloudApiError({ - message: deline` - The provided access token is expired or has been revoked, please create a new - one from the ${distroName} UI.`, - }) + // Start refresh interval if using JWT + cloudFactoryLog.debug({ msg: `Starting refresh interval.` }) + api.startInterval() } - } else { - // Refresh the token if it's invalid. - if (!tokenIsValid) { - cloudFactoryLog.debug({ msg: `Current auth token is invalid, refreshing` }) - // We can assert the token exists since we're not using GARDEN_AUTH_TOKEN - await api.refreshToken(token!) + return api + } catch (err) { + if (err instanceof CloudApiError) { + // If there is an ID in the project config and the user is not logged in (no cloudApi) + // 0.13 => check if login is required based on the `requireLogin` config value + if (projectId && isLoginRequired) { + const message = dedent` + You are running this in a project with a Garden ID and logging in is required. + Please log in via the ${styles.command("garden login")} command.` + + throw new CloudApiError({ message }) + } else { + cloudFactoryLog.warn( + `Warning: You are not logged in into Garden Cloud. Please log in via the ${styles.command( + "garden login" + )} command.` + ) + + return + } } - // Start refresh interval if using JWT - cloudFactoryLog.debug({ msg: `Starting refresh interval.` }) - api.startInterval() + throw err } - - return api } static async saveAuthToken( diff --git a/core/test/unit/src/cloud/api.ts b/core/test/unit/src/cloud/api.ts index da67ae0b5f..ebfb882ebb 100644 --- a/core/test/unit/src/cloud/api.ts +++ b/core/test/unit/src/cloud/api.ts @@ -13,6 +13,8 @@ import { CloudApi } from "../../../../src/cloud/api.js" import { uuidv4 } from "../../../../src/util/random.js" import { randomString } from "../../../../src/util/string.js" import { GlobalConfigStore } from "../../../../src/config-store/global.js" +import nock from "nock" +import { expectError } from "../../../helpers.js" describe("CloudApi", () => { const log = getRootLogger().createLog() @@ -67,4 +69,179 @@ describe("CloudApi", () => { await CloudApi.clearAuthToken(log, globalConfigStore, domain) }) }) + + describe("factory", async () => { + let cloudDomain: string + + const storeToken = async () => { + const testToken = { + token: uuidv4(), + refreshToken: uuidv4(), + tokenValidity: 9999, + } + await CloudApi.saveAuthToken(log, globalConfigStore, testToken, cloudDomain) + } + + beforeEach(async () => { + cloudDomain = "https://garden." + randomString() + }) + + afterEach(async () => { + await CloudApi.clearAuthToken(log, globalConfigStore, cloudDomain) + + nock.cleanAll() + }) + + it("should not return a CloudApi instance without an auth token", async () => { + const scope = nock(cloudDomain) + + scope.get("/api/token/verify").reply(404) + + const api = await CloudApi.factory({ log, globalConfigStore, cloudDomain }) + + // we don't expect a request to verify the token + expect(scope.isDone()).to.be.false + + expect(api).to.be.undefined + }) + + it("should return a CloudApi instance if there is a valid token", async () => { + const scope = nock(cloudDomain) + + scope.get("/api/token/verify").reply(200, {}) + + await storeToken() + + const api = await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + }) + + expect(scope.isDone()).to.be.true + + expect(api).to.be.instanceOf(CloudApi) + }) + + it("should not return a CloudApi instance without a token even if there is a project id and require login is false", async () => { + const scope = nock(cloudDomain) + + scope.get("/api/token/verify").reply(200, {}) + + const api = await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: false, + }) + + expect(scope.isDone()).to.be.false + + expect(api).to.be.undefined + }) + + it("should not return a CloudApi instance with an invalid token when require login is false", async () => { + const scope = nock(cloudDomain) + + // store a token, but return that its invalid and fail the refresh + await storeToken() + scope.get("/api/token/verify").reply(401, {}) + scope.get("/api/token/refresh").reply(401, {}) + + const api = await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: false, + }) + + expect(scope.isDone()).to.be.true + + expect(api).to.be.undefined + }) + + it("should throw an error when the token is invalid and require login is true", async () => { + const scope = nock(cloudDomain) + + // store a token, but return that its invalid and fail the refresh + await storeToken() + scope.get("/api/token/verify").reply(401, {}) + scope.get("/api/token/refresh").reply(401, {}) + + await expectError( + async () => + CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: true, + }), + "cloud-api" + ) + + expect(scope.isDone()).to.be.true + }) + + it("should not return a CloudApi instance when GARDEN_REQUIRE_LOGIN_OVERRIDE is false", async () => { + const scope = nock(cloudDomain) + + // store a token, but return that its invalid and fail the refresh + await storeToken() + scope.get("/api/token/verify").reply(401, {}) + scope.get("/api/token/refresh").reply(401, {}) + + const overrideEnvBackup = gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE + + gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE = false + + try { + const api = await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: true, + }) + + expect(scope.isDone()).to.be.true + expect(api).to.be.undefined + } finally { + gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE = overrideEnvBackup + } + }) + + it("should throw an error when GARDEN_REQUIRE_LOGIN_OVERRIDE is true", async () => { + const scope = nock(cloudDomain) + + // store a token, but return that its invalid and fail the refresh + await storeToken() + scope.get("/api/token/verify").reply(401, {}) + scope.get("/api/token/refresh").reply(401, {}) + + const overrideEnvBackup = gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE + + gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE = true + + try { + await expectError( + async () => + CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: true, + }), + "cloud-api" + ) + + expect(scope.isDone()).to.be.true + } finally { + gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE = overrideEnvBackup + } + }) + }) }) From 45e3eeeaba9ad21e713e882d7fd3a3e85decbf2d Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Tue, 12 Dec 2023 11:43:42 +0100 Subject: [PATCH 3/7] chore: cloud factory with mandatory projectId and requiredLogin params --- core/src/cli/cli.ts | 8 +++++++- core/src/cloud/api.ts | 8 ++++---- core/src/commands/login.ts | 9 ++++++++- core/src/commands/logout.ts | 2 ++ core/src/commands/serve.ts | 8 +++++++- core/src/server/instance-manager.ts | 2 ++ core/src/server/server.ts | 2 ++ core/test/unit/src/cloud/api.ts | 10 +++++++++- core/test/unit/src/garden.ts | 8 +++++++- 9 files changed, 48 insertions(+), 9 deletions(-) diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index 5e6ef7bfe7..de87aadbfc 100644 --- a/core/src/cli/cli.ts +++ b/core/src/cli/cli.ts @@ -256,7 +256,13 @@ ${renderCommands(commands)} const distroName = getCloudDistributionName(cloudDomain) try { - cloudApi = await this.cloudApiFactory({ log, cloudDomain, globalConfigStore }) + cloudApi = await this.cloudApiFactory({ + log, + cloudDomain, + globalConfigStore, + projectId: config?.id, + requireLogin: config?.requireLogin, + }) } catch (err) { if (err instanceof CloudApiTokenRefreshError) { log.warn(dedent` diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index fdd614fddc..4bafa101aa 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -167,8 +167,8 @@ export interface CloudApiFactoryParams { cloudDomain: string globalConfigStore: GlobalConfigStore skipLogging?: boolean - projectId?: string - requireLogin?: boolean + projectId: string | undefined + requireLogin: boolean | undefined } export type CloudApiFactory = (params: CloudApiFactoryParams) => Promise @@ -285,13 +285,13 @@ export class CloudApi { // 0.13 => check if login is required based on the `requireLogin` config value if (projectId && isLoginRequired) { const message = dedent` - You are running this in a project with a Garden ID and logging in is required. + You are running this in a project with a Garden Cloud ID and logging in is required. Please log in via the ${styles.command("garden login")} command.` throw new CloudApiError({ message }) } else { cloudFactoryLog.warn( - `Warning: You are not logged in into Garden Cloud. Please log in via the ${styles.command( + `Warning: You are not logged in to Garden Cloud. Please log in via the ${styles.command( "garden login" )} command.` ) diff --git a/core/src/commands/login.ts b/core/src/commands/login.ts index 04bbc21710..5cf4902860 100644 --- a/core/src/commands/login.ts +++ b/core/src/commands/login.ts @@ -89,7 +89,14 @@ export class LoginCommand extends Command<{}, Opts> { const distroName = getCloudDistributionName(cloudDomain) try { - const cloudApi = await CloudApi.factory({ log, cloudDomain, skipLogging: true, globalConfigStore }) + const cloudApi = await CloudApi.factory({ + log, + cloudDomain, + skipLogging: true, + globalConfigStore, + projectId: undefined, + requireLogin: undefined, + }) if (cloudApi) { log.success({ msg: `You're already logged in to ${cloudDomain}.` }) diff --git a/core/src/commands/logout.ts b/core/src/commands/logout.ts index 89f7382e0a..bb8236a6c8 100644 --- a/core/src/commands/logout.ts +++ b/core/src/commands/logout.ts @@ -78,6 +78,8 @@ export class LogOutCommand extends Command<{}, Opts> { cloudDomain, skipLogging: true, globalConfigStore: garden.globalConfigStore, + projectId: undefined, + requireLogin: undefined, }) if (!cloudApi) { diff --git a/core/src/commands/serve.ts b/core/src/commands/serve.ts index 5c791426a9..08b8437168 100644 --- a/core/src/commands/serve.ts +++ b/core/src/commands/serve.ts @@ -157,7 +157,13 @@ export class ServeCommand< } try { - const cloudApi = await manager.getCloudApi({ log, cloudDomain, globalConfigStore: garden.globalConfigStore }) + const cloudApi = await manager.getCloudApi({ + log, + cloudDomain, + globalConfigStore: garden.globalConfigStore, + projectId: projectConfig?.id, + requireLogin: projectConfig?.requireLogin, + }) const isLoggedIn = !!cloudApi const isCommunityEdition = cloudDomain === DEFAULT_GARDEN_CLOUD_DOMAIN diff --git a/core/src/server/instance-manager.ts b/core/src/server/instance-manager.ts index 19f9999ea2..9c1bc1c636 100644 --- a/core/src/server/instance-manager.ts +++ b/core/src/server/instance-manager.ts @@ -366,6 +366,8 @@ export class GardenInstanceManager { log, cloudDomain: getGardenCloudDomain(projectConfig.domain), globalConfigStore, + projectId: projectConfig.id, + requireLogin: projectConfig.requireLogin, }) } diff --git a/core/src/server/server.ts b/core/src/server/server.ts index b643a3150f..8b5970c692 100644 --- a/core/src/server/server.ts +++ b/core/src/server/server.ts @@ -871,6 +871,8 @@ export class GardenServer extends EventEmitter { log, cloudDomain: getGardenCloudDomain(garden.cloudDomain), globalConfigStore: garden.globalConfigStore, + projectId: garden.projectId, + requireLogin: undefined, // the user should be logged in for this }) // Use the server session ID. That is, the "main" session ID that belongs to the parent serve command. diff --git a/core/test/unit/src/cloud/api.ts b/core/test/unit/src/cloud/api.ts index ebfb882ebb..7504b6eed9 100644 --- a/core/test/unit/src/cloud/api.ts +++ b/core/test/unit/src/cloud/api.ts @@ -97,7 +97,13 @@ describe("CloudApi", () => { scope.get("/api/token/verify").reply(404) - const api = await CloudApi.factory({ log, globalConfigStore, cloudDomain }) + const api = await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: undefined, + requireLogin: undefined, + }) // we don't expect a request to verify the token expect(scope.isDone()).to.be.false @@ -116,6 +122,8 @@ describe("CloudApi", () => { log, globalConfigStore, cloudDomain, + projectId: undefined, + requireLogin: undefined, }) expect(scope.isDone()).to.be.true diff --git a/core/test/unit/src/garden.ts b/core/test/unit/src/garden.ts index 54905cb508..d3b1c6f224 100644 --- a/core/test/unit/src/garden.ts +++ b/core/test/unit/src/garden.ts @@ -743,7 +743,13 @@ describe("Garden", () => { refreshToken: "fake-refresh-token", validity: add(new Date(), { seconds: validityMs / 1000 }), }) - return CloudApi.factory({ log, cloudDomain: domain, globalConfigStore }) + return CloudApi.factory({ + log, + cloudDomain: domain, + globalConfigStore, + projectId: undefined, + requireLogin: undefined, + }) } before(async () => { From 5f98b194c50f58bba9bfe88799d0cafe810be847 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Tue, 12 Dec 2023 12:10:38 +0100 Subject: [PATCH 4/7] chore: adding a line in front of error logs --- core/src/cli/cli.ts | 2 ++ core/src/cloud/api.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index de87aadbfc..f7139b2f0e 100644 --- a/core/src/cli/cli.ts +++ b/core/src/cli/cli.ts @@ -278,6 +278,8 @@ ${renderCommands(commands)} } } else { // unhandled error when creating the cloud api + log.info("") + throw err } } diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index 4bafa101aa..86a6ff9e27 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -218,7 +218,7 @@ export class CloudApi { globalConfigStore, skipLogging = false, projectId = undefined, - requireLogin = false, + requireLogin = undefined, }: CloudApiFactoryParams) { const distroName = getCloudDistributionName(cloudDomain) const fixLevel = skipLogging ? LogLevel.silly : undefined @@ -235,7 +235,7 @@ export class CloudApi { const isLoginRequired: boolean = gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE !== undefined ? gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE - : projectId !== undefined && requireLogin + : projectId !== undefined && requireLogin === true // Base case when the user is not logged in to cloud and the // criteria for cloud login is not required: From 09e15a4e28beb1416412780af366132561ec3b04 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Tue, 12 Dec 2023 12:58:20 +0100 Subject: [PATCH 5/7] chore: using another log instance after merge --- core/src/cli/cli.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index f7139b2f0e..f349596e15 100644 --- a/core/src/cli/cli.ts +++ b/core/src/cli/cli.ts @@ -278,7 +278,7 @@ ${renderCommands(commands)} } } else { // unhandled error when creating the cloud api - log.info("") + gardenInitLog?.info("") throw err } From a789934758f6a80ef62d4b732d732f6dc95969e5 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Wed, 13 Dec 2023 12:21:08 +0100 Subject: [PATCH 6/7] chore: cloud api factory throws specific errors --- core/src/cloud/api.ts | 67 ++++++++++-------- core/src/commands/login.ts | 25 +++---- core/src/config/project.ts | 1 - core/test/unit/src/cloud/api.ts | 101 +++++++++++++++++---------- core/test/unit/src/commands/login.ts | 9 ++- 5 files changed, 119 insertions(+), 84 deletions(-) diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index 86a6ff9e27..2c6120e33d 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -40,10 +40,21 @@ import { RequestError } from "got" const gardenClientName = "garden-core" const gardenClientVersion = getPackageVersion() +// Thrown when trying to create a project with a name that already exists export class CloudApiDuplicateProjectsError extends CloudApiError {} +// A user token can be refreshed, thrown when the refresh fails export class CloudApiTokenRefreshError extends CloudApiError {} +// The access token passed via GARDEN_AUTH_TOKEN was not valid +export class CloudApiAccessTokenInvalidError extends CloudApiError {} + +// Thrown when the user is not logged in with a cloud connected project +export class CloudApiLoginRequiredError extends CloudApiError {} + +// Thrown there is no auth or access token +export class CloudApiNoTokenError extends CloudApiError {} + function extractErrorMessageBodyFromGotError(error: any): error is GotHttpError { return error?.response?.body?.message } @@ -245,7 +256,7 @@ export class CloudApi { cloudFactoryLog.debug( `No auth token found, proceeding without access to ${distroName}. Command results for this command run will not be available in ${distroName}.` ) - return + throw new CloudApiNoTokenError({ message: `No auth token available for ${distroName} at ${cloudDomain}` }) } // Try to auth towards cloud @@ -255,29 +266,26 @@ export class CloudApi { cloudFactoryLog.debug("Authorizing...") - if (gardenEnv.GARDEN_AUTH_TOKEN) { - // Throw if using an invalid "CI" access token - if (!tokenIsValid) { - throw new CloudApiError({ - message: deline` + if (gardenEnv.GARDEN_AUTH_TOKEN && !tokenIsValid) { + throw new CloudApiAccessTokenInvalidError({ + message: deline` The provided access token is expired or has been revoked, please create a new one from the ${distroName} UI.`, - }) - } - } else { - // Refresh the token if it's invalid. - if (!tokenIsValid) { - cloudFactoryLog.debug({ msg: `Current auth token is invalid, refreshing` }) + }) + } - // We can assert the token exists since we're not using GARDEN_AUTH_TOKEN - await api.refreshToken(token!) - } + // Try to refresh the token if it's invalid. + if (!tokenIsValid) { + cloudFactoryLog.debug({ msg: `Current auth token is invalid, refreshing` }) - // Start refresh interval if using JWT - cloudFactoryLog.debug({ msg: `Starting refresh interval.` }) - api.startInterval() + // We can assert the token exists since we're not using GARDEN_AUTH_TOKEN + await api.refreshToken(token!) } + // Start refresh interval if using JWT + cloudFactoryLog.debug({ msg: `Starting refresh interval.` }) + api.startInterval() + return api } catch (err) { if (err instanceof CloudApiError) { @@ -288,15 +296,7 @@ export class CloudApi { You are running this in a project with a Garden Cloud ID and logging in is required. Please log in via the ${styles.command("garden login")} command.` - throw new CloudApiError({ message }) - } else { - cloudFactoryLog.warn( - `Warning: You are not logged in to Garden Cloud. Please log in via the ${styles.command( - "garden login" - )} command.` - ) - - return + throw new CloudApiLoginRequiredError({ message }) } } @@ -518,8 +518,17 @@ export class CloudApi { throw err } - this.log.debug({ msg: `Failed to refresh the token.` }) - throw new CloudApiTokenRefreshError({ + this.log.debug({ msg: `Failed to refresh the auth token, response status code: ${err.response.statusCode}` }) + + // The token was invalid and could not be refreshed + if (err.response.statusCode === 401) { + throw new CloudApiTokenRefreshError({ + message: `The auth token could not be refreshed for ${getCloudDistributionName(this.domain)}`, + }) + } + + // Unhandled cloud api error + throw new CloudApiError({ message: `An error occurred while verifying client auth token with ${getCloudDistributionName(this.domain)}: ${ err.message }. Response status code: ${err.response.statusCode}`, diff --git a/core/src/commands/login.ts b/core/src/commands/login.ts index 5cf4902860..d0ab2a9a10 100644 --- a/core/src/commands/login.ts +++ b/core/src/commands/login.ts @@ -11,7 +11,7 @@ import { Command } from "./base.js" import { printHeader } from "../logger/util.js" import dedent from "dedent" import type { AuthTokenResponse } from "../cloud/api.js" -import { CloudApi, getGardenCloudDomain } from "../cloud/api.js" +import { CloudApi, CloudApiNoTokenError, CloudApiTokenRefreshError, getGardenCloudDomain } from "../cloud/api.js" import type { Log } from "../logger/log-entry.js" import { ConfigurationError, TimeoutError, InternalError, CloudApiError } from "../exceptions.js" import { AuthRedirectServer } from "../cloud/auth.js" @@ -86,8 +86,6 @@ export class LoginCommand extends Command<{}, Opts> { // should use the default domain or not. The token lifecycle ends on logout. const cloudDomain: string = getGardenCloudDomain(projectConfig?.domain) - const distroName = getCloudDistributionName(cloudDomain) - try { const cloudApi = await CloudApi.factory({ log, @@ -98,24 +96,27 @@ export class LoginCommand extends Command<{}, Opts> { requireLogin: undefined, }) - if (cloudApi) { - log.success({ msg: `You're already logged in to ${cloudDomain}.` }) - cloudApi.close() - return {} - } + log.success({ msg: `You're already logged in to ${cloudDomain}.` }) + cloudApi.close() + return {} } catch (err) { if (!(err instanceof CloudApiError)) { throw err } - if (err.responseStatusCode === 401) { + + if (err instanceof CloudApiTokenRefreshError) { const msg = dedent` - Looks like your session token is invalid. If you were previously logged into a different instance - of ${distroName}, log out first before logging in. + Your login token for ${cloudDomain} has expired and could not be refreshed. + Try to log out first before logging in. ` log.warn(msg) log.info("") } - throw err + + // This is expected if the user has not logged in + if (!(err instanceof CloudApiNoTokenError)) { + throw err + } } log.info({ msg: `Logging in to ${cloudDomain}...` }) diff --git a/core/src/config/project.ts b/core/src/config/project.ts index f834731287..bf9be550c9 100644 --- a/core/src/config/project.ts +++ b/core/src/config/project.ts @@ -327,7 +327,6 @@ export const projectSchema = createSchema({ // TODO: Refer to enterprise documentation for more details. requireLogin: joi .boolean() - .default(false) .meta({ internal: true }) .description("Whether the project requires login to Garden Cloud."), diff --git a/core/test/unit/src/cloud/api.ts b/core/test/unit/src/cloud/api.ts index 7504b6eed9..e77f28c323 100644 --- a/core/test/unit/src/cloud/api.ts +++ b/core/test/unit/src/cloud/api.ts @@ -97,18 +97,23 @@ describe("CloudApi", () => { scope.get("/api/token/verify").reply(404) - const api = await CloudApi.factory({ - log, - globalConfigStore, - cloudDomain, - projectId: undefined, - requireLogin: undefined, - }) + await expectError( + async () => + await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: undefined, + requireLogin: undefined, + }), + { + type: "cloud-api", + contains: "No auth token available for", + } + ) // we don't expect a request to verify the token expect(scope.isDone()).to.be.false - - expect(api).to.be.undefined }) it("should return a CloudApi instance if there is a valid token", async () => { @@ -136,17 +141,22 @@ describe("CloudApi", () => { scope.get("/api/token/verify").reply(200, {}) - const api = await CloudApi.factory({ - log, - globalConfigStore, - cloudDomain, - projectId: "test", - requireLogin: false, - }) + await expectError( + async () => + await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: false, + }), + { + type: "cloud-api", + contains: "No auth token available for", + } + ) expect(scope.isDone()).to.be.false - - expect(api).to.be.undefined }) it("should not return a CloudApi instance with an invalid token when require login is false", async () => { @@ -157,17 +167,22 @@ describe("CloudApi", () => { scope.get("/api/token/verify").reply(401, {}) scope.get("/api/token/refresh").reply(401, {}) - const api = await CloudApi.factory({ - log, - globalConfigStore, - cloudDomain, - projectId: "test", - requireLogin: false, - }) + await expectError( + async () => + await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: false, + }), + { + type: "cloud-api", + contains: "The auth token could not be refreshed for", + } + ) expect(scope.isDone()).to.be.true - - expect(api).to.be.undefined }) it("should throw an error when the token is invalid and require login is true", async () => { @@ -187,7 +202,10 @@ describe("CloudApi", () => { projectId: "test", requireLogin: true, }), - "cloud-api" + { + type: "cloud-api", + contains: "You are running this in a project with a Garden Cloud ID and logging in is required.", + } ) expect(scope.isDone()).to.be.true @@ -206,16 +224,22 @@ describe("CloudApi", () => { gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE = false try { - const api = await CloudApi.factory({ - log, - globalConfigStore, - cloudDomain, - projectId: "test", - requireLogin: true, - }) + await expectError( + async () => + await CloudApi.factory({ + log, + globalConfigStore, + cloudDomain, + projectId: "test", + requireLogin: true, + }), + { + type: "cloud-api", + contains: "The auth token could not be refreshed for", + } + ) expect(scope.isDone()).to.be.true - expect(api).to.be.undefined } finally { gardenEnv.GARDEN_REQUIRE_LOGIN_OVERRIDE = overrideEnvBackup } @@ -243,7 +267,10 @@ describe("CloudApi", () => { projectId: "test", requireLogin: true, }), - "cloud-api" + { + type: "cloud-api", + contains: "You are running this in a project with a Garden Cloud ID and logging in is required.", + } ) expect(scope.isDone()).to.be.true diff --git a/core/test/unit/src/commands/login.ts b/core/test/unit/src/commands/login.ts index d3d418d386..8ab97e589a 100644 --- a/core/test/unit/src/commands/login.ts +++ b/core/test/unit/src/commands/login.ts @@ -14,10 +14,9 @@ import { AuthRedirectServer } from "../../../../src/cloud/auth.js" import { LoginCommand } from "../../../../src/commands/login.js" import { dedent, randomString } from "../../../../src/util/string.js" -import { CloudApi } from "../../../../src/cloud/api.js" +import { CloudApi, CloudApiTokenRefreshError } from "../../../../src/cloud/api.js" import { LogLevel } from "../../../../src/logger/logger.js" import { DEFAULT_GARDEN_CLOUD_DOMAIN, gardenEnv } from "../../../../src/constants.js" -import { CloudApiError } from "../../../../src/exceptions.js" import { getLogMessages } from "../../../../src/util/testing.js" import { GlobalConfigStore } from "../../../../src/config-store/global.js" import { makeDummyGarden } from "../../../../src/garden.js" @@ -244,7 +243,7 @@ describe("LoginCommand", () => { await CloudApi.saveAuthToken(garden.log, garden.globalConfigStore, testToken, garden.cloudDomain!) td.replace(CloudApi.prototype, "checkClientAuthToken", async () => false) td.replace(CloudApi.prototype, "refreshToken", async () => { - throw new CloudApiError({ message: "bummer", responseStatusCode: 401 }) + throw new CloudApiTokenRefreshError({ message: "bummer" }) }) const savedToken = await CloudApi.getStoredAuthToken(garden.log, garden.globalConfigStore, garden.cloudDomain!) @@ -259,8 +258,8 @@ describe("LoginCommand", () => { const logOutput = getLogMessages(garden.log, (entry) => entry.level <= LogLevel.info).join("\n") expect(logOutput).to.include(dedent` - Looks like your session token is invalid. If you were previously logged into a different instance - of Garden Enterprise, log out first before logging in. + Your login token for https://example.invalid has expired and could not be refreshed. + Try to log out first before logging in. `) }) From 799f9be4b365b46c6e8068503a971496bf5f3c84 Mon Sep 17 00:00:00 2001 From: Mikael Hoegqvist Tabor Date: Wed, 13 Dec 2023 12:59:05 +0100 Subject: [PATCH 7/7] chore: handle cases which relied on api to be undefined on no token error --- core/src/cli/cli.ts | 8 ++++++-- core/src/cloud/api.ts | 2 +- core/src/commands/logout.ts | 13 ++++++------- core/src/server/instance-manager.ts | 13 +++++++++++-- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index f349596e15..4c846b3f64 100644 --- a/core/src/cli/cli.ts +++ b/core/src/cli/cli.ts @@ -50,7 +50,7 @@ import { generateBasicDebugInfoReport } from "../commands/get/get-debug-info.js" import type { AnalyticsHandler } from "../analytics/analytics.js" import type { GardenPluginReference } from "../plugin/plugin.js" import type { CloudApiFactory } from "../cloud/api.js" -import { CloudApi, CloudApiTokenRefreshError, getGardenCloudDomain } from "../cloud/api.js" +import { CloudApi, CloudApiNoTokenError, CloudApiTokenRefreshError, getGardenCloudDomain } from "../cloud/api.js" import { findProjectConfig } from "../config/base.js" import { pMemoizeDecorator } from "../lib/p-memoize.js" import { getCustomCommands } from "../commands/custom.js" @@ -264,7 +264,11 @@ ${renderCommands(commands)} requireLogin: config?.requireLogin, }) } catch (err) { - if (err instanceof CloudApiTokenRefreshError) { + if (err instanceof CloudApiNoTokenError) { + // this means the user has no token stored for the domain and we should just continue + // without a cloud api instance + gardenInitLog?.debug(`Cloud not configured since no token found for ${distroName} at ${cloudDomain}`) + } else if (err instanceof CloudApiTokenRefreshError) { log.warn(dedent` Unable to authenticate against ${distroName} with the current session token. Command results for this command run will not be available in ${distroName}. If this not a diff --git a/core/src/cloud/api.ts b/core/src/cloud/api.ts index 2c6120e33d..ba5c0a1f7a 100644 --- a/core/src/cloud/api.ts +++ b/core/src/cloud/api.ts @@ -230,7 +230,7 @@ export class CloudApi { skipLogging = false, projectId = undefined, requireLogin = undefined, - }: CloudApiFactoryParams) { + }: CloudApiFactoryParams): Promise { const distroName = getCloudDistributionName(cloudDomain) const fixLevel = skipLogging ? LogLevel.silly : undefined const cloudFactoryLog = log.createLog({ fixLevel, name: getCloudLogSectionName(distroName), showDuration: true }) diff --git a/core/src/commands/logout.ts b/core/src/commands/logout.ts index bb8236a6c8..d53a78da37 100644 --- a/core/src/commands/logout.ts +++ b/core/src/commands/logout.ts @@ -9,7 +9,7 @@ import type { CommandParams, CommandResult } from "./base.js" import { Command } from "./base.js" import { printHeader } from "../logger/util.js" -import { CloudApi, getGardenCloudDomain } from "../cloud/api.js" +import { CloudApi, CloudApiNoTokenError, getGardenCloudDomain } from "../cloud/api.js" import { getCloudDistributionName } from "../util/cloud.js" import { dedent, deline } from "../util/string.js" import { ConfigurationError } from "../exceptions.js" @@ -82,17 +82,16 @@ export class LogOutCommand extends Command<{}, Opts> { requireLogin: undefined, }) - if (!cloudApi) { - return {} - } - await cloudApi.post("token/logout", { headers: { Cookie: `rt=${token?.refreshToken}` } }) cloudApi.close() } catch (err) { - const msg = dedent` + // This is expected if the user never logged in + if (!(err instanceof CloudApiNoTokenError)) { + const msg = dedent` The following issue occurred while logging out from ${distroName} (your session will be cleared regardless): ${err}\n ` - log.warn(msg) + log.warn(msg) + } } finally { await CloudApi.clearAuthToken(log, garden.globalConfigStore, cloudDomain) log.success(`Successfully logged out from ${cloudDomain}.`) diff --git a/core/src/server/instance-manager.ts b/core/src/server/instance-manager.ts index 9c1bc1c636..ffcd8a0fff 100644 --- a/core/src/server/instance-manager.ts +++ b/core/src/server/instance-manager.ts @@ -12,7 +12,7 @@ import { Autocompleter } from "../cli/autocomplete.js" import { parseCliVarFlags } from "../cli/helpers.js" import type { ParameterObject, ParameterValues } from "../cli/params.js" import type { CloudApiFactory, CloudApiFactoryParams } from "../cloud/api.js" -import { CloudApi, getGardenCloudDomain } from "../cloud/api.js" +import { CloudApi, CloudApiNoTokenError, getGardenCloudDomain } from "../cloud/api.js" import type { Command } from "../commands/base.js" import { getBuiltinCommands, flattenCommands } from "../commands/commands.js" import { getCustomCommands } from "../commands/custom.js" @@ -202,7 +202,16 @@ export class GardenInstanceManager { let api = this.cloudApis.get(cloudDomain) if (!api) { - api = await this.cloudApiFactory(params) + try { + api = await this.cloudApiFactory(params) + } catch (err) { + // handles the case when the user should not be logged in + // otherwise we throw any error that can occure while + // authenticating + if (!(err instanceof CloudApiNoTokenError)) { + throw err + } + } api && this.cloudApis.set(cloudDomain, api) }