From 2cbdd4730a27d83644c918b83d48774dfd823685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Tron=C3=AD=C4=8Dek?= Date: Tue, 28 Jan 2025 14:40:21 +0000 Subject: [PATCH 1/2] [server] Add organization image auth context to workspace image validation Tool: gitpod/catfood.gitpod.cloud --- components/server/src/container-module.ts | 4 ++-- .../orgs/default-workspace-image-validator.ts | 6 +++++- .../server/src/orgs/organization-service.ts | 2 +- .../server/src/workspace/workspace-service.ts | 16 ++++++++++++---- .../server/src/workspace/workspace-starter.ts | 16 +++++++++++++++- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/components/server/src/container-module.ts b/components/server/src/container-module.ts index 0c49448d2f90a2..58d09ef485b4c5 100644 --- a/components/server/src/container-module.ts +++ b/components/server/src/container-module.ts @@ -412,9 +412,9 @@ export const productionContainerModule = new ContainerModule( bind(DefaultWorkspaceImageValidator) .toDynamicValue((ctx) => // lazy load to avoid circular dependency - async (userId: string, imageRef: string) => { + async (userId: string, imageRef: string, organizationId?: string) => { const user = await ctx.container.get(UserService).findUserById(userId, userId); - await ctx.container.get(WorkspaceService).validateImageRef({}, user, imageRef); + await ctx.container.get(WorkspaceService).validateImageRef({}, user, imageRef, organizationId); }, ) .inSingletonScope(); diff --git a/components/server/src/orgs/default-workspace-image-validator.ts b/components/server/src/orgs/default-workspace-image-validator.ts index fda29e97c9ce03..721d71605e9f31 100644 --- a/components/server/src/orgs/default-workspace-image-validator.ts +++ b/components/server/src/orgs/default-workspace-image-validator.ts @@ -4,5 +4,9 @@ * See License.AGPL.txt in the project root for license information. */ -export type DefaultWorkspaceImageValidator = (userId: string, imageRef: string) => Promise; +export type DefaultWorkspaceImageValidator = ( + userId: string, + imageRef: string, + organizationId?: string, +) => Promise; export const DefaultWorkspaceImageValidator = Symbol("DefaultWorkspaceImageValidator"); diff --git a/components/server/src/orgs/organization-service.ts b/components/server/src/orgs/organization-service.ts index e83402ad0a986c..e3a7375a9ba4d1 100644 --- a/components/server/src/orgs/organization-service.ts +++ b/components/server/src/orgs/organization-service.ts @@ -493,7 +493,7 @@ export class OrganizationService { if (typeof settings.defaultWorkspaceImage === "string") { const defaultWorkspaceImage = settings.defaultWorkspaceImage.trim(); if (defaultWorkspaceImage) { - await this.validateDefaultWorkspaceImage(userId, defaultWorkspaceImage); + await this.validateDefaultWorkspaceImage(userId, defaultWorkspaceImage, orgId); settings = { ...settings, defaultWorkspaceImage }; } else { settings = { ...settings, defaultWorkspaceImage: null }; diff --git a/components/server/src/workspace/workspace-service.ts b/components/server/src/workspace/workspace-service.ts index 2ff2dd44820e0c..eeb972fa34c366 100644 --- a/components/server/src/workspace/workspace-service.ts +++ b/components/server/src/workspace/workspace-service.ts @@ -1407,9 +1407,17 @@ export class WorkspaceService { }); } - public async validateImageRef(ctx: TraceContext, user: User, imageRef: string) { + public async validateImageRef(ctx: TraceContext, user: User, imageRef: string, organizationId?: string) { try { - return await this.workspaceStarter.resolveBaseImage(ctx, user, imageRef); + return await this.workspaceStarter.resolveBaseImage( + ctx, + user, + imageRef, + undefined, + undefined, + undefined, + organizationId, + ); } catch (e) { // see https://github.com/gitpod-io/gitpod/blob/f3e41f8d86234e4101edff2199c54f50f8cbb656/components/image-builder-mk3/pkg/orchestrator/orchestrator.go#L561 // TODO(ak) ideally we won't check a message (subject to change) @@ -1423,8 +1431,8 @@ export class WorkspaceService { ) { let message = details; // strip confusing prefix - if (details.startsWith("cannt resolve base image ref: ")) { - message = details.substring("cannt resolve base image ref: ".length); + if (details.startsWith("can't resolve base image ref: ")) { + message = details.substring("can't resolve base image ref: ".length); } throw new ApplicationError(ErrorCodes.BAD_REQUEST, message); } diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index d9e32754d6f680..b5a8bccd5f0b48 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -42,6 +42,7 @@ import { ImageBuildLogInfo, ImageConfigFile, NamedWorkspaceFeatureFlag, + OrgEnvVarWithValue, Permission, Project, RefType, @@ -128,7 +129,7 @@ import { TokenProvider } from "../user/token-provider"; import { UserAuthentication } from "../user/user-authentication"; import { ImageSourceProvider } from "./image-source-provider"; import { WorkspaceClassesConfig } from "./workspace-classes"; -import { SYSTEM_USER, SYSTEM_USER_ID } from "../authorization/authorizer"; +import { SYSTEM_USER, SYSTEM_USER_ID, Authorizer } from "../authorization/authorizer"; import { EnvVarService, ResolvedEnvVars } from "../user/env-var-service"; import { RedlockAbortSignal } from "redlock"; import { ConfigProvider } from "./config-provider"; @@ -239,6 +240,7 @@ export class WorkspaceStarter { @inject(EnvVarService) private readonly envVarService: EnvVarService, @inject(OrganizationService) private readonly orgService: OrganizationService, @inject(ProjectsService) private readonly projectService: ProjectsService, + @inject(Authorizer) private readonly auth: Authorizer, ) {} public async startWorkspace( @@ -2033,6 +2035,7 @@ export class WorkspaceStarter { workspace?: Workspace, instance?: WorkspaceInstance, region?: WorkspaceRegion, + organizationId?: string, ) { const req = new ResolveBaseImageRequest(); req.setRef(imageRef); @@ -2041,6 +2044,17 @@ export class WorkspaceStarter { const auth = new BuildRegistryAuth(); auth.setTotal(allowAll); req.setAuth(auth); + + // if the image resolution is for an organization, we also include the organization's set up env vars + if (organizationId) { + await this.auth.checkPermissionOnOrganization(user.id, "read_env_var", organizationId); + const orgEnvVars = await this.orgDB.getOrgEnvironmentVariables(organizationId); + const orgEnvVarValues: OrgEnvVarWithValue[] = await this.orgDB.getOrgEnvironmentVariableValues(orgEnvVars); + + const additionalAuth = await this.getAdditionalImageAuth({ workspace: orgEnvVarValues }); + additionalAuth.forEach((val, key) => auth.getAdditionalMap().set(key, val)); + } + const client = await this.getImageBuilderClient(user, workspace, instance, region); return client.resolveBaseImage({ span: ctx.span }, req); } From 719452234e8c19913887ebac4d07e42841631eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Tron=C3=AD=C4=8Dek?= Date: Wed, 29 Jan 2025 09:14:16 +0000 Subject: [PATCH 2/2] Introduce `listOrgEnvVarsWithValues` Tool: gitpod/catfood.gitpod.cloud --- components/server/src/user/env-var-service.ts | 7 +++++++ components/server/src/workspace/workspace-starter.ts | 10 +++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/components/server/src/user/env-var-service.ts b/components/server/src/user/env-var-service.ts index a0195db81a6497..1e6c6c6a1fbdd5 100644 --- a/components/server/src/user/env-var-service.ts +++ b/components/server/src/user/env-var-service.ts @@ -234,6 +234,13 @@ export class EnvVarService { return this.orgDB.getOrgEnvironmentVariables(orgId); } + async listOrgEnvVarsWithValues(requestorId: string, orgId: string): Promise { + const envVars = (await ApplicationError.notFoundToUndefined(this.listOrgEnvVars(requestorId, orgId))) ?? []; + const envVarValues = await this.orgDB.getOrgEnvironmentVariableValues(envVars); + + return envVarValues; + } + async getOrgEnvVarById(requestorId: string, id: string): Promise { const result = await this.orgDB.getOrgEnvironmentVariableById(id); if (!result) { diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index b5a8bccd5f0b48..1cf0934526c1b4 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -42,7 +42,6 @@ import { ImageBuildLogInfo, ImageConfigFile, NamedWorkspaceFeatureFlag, - OrgEnvVarWithValue, Permission, Project, RefType, @@ -129,7 +128,7 @@ import { TokenProvider } from "../user/token-provider"; import { UserAuthentication } from "../user/user-authentication"; import { ImageSourceProvider } from "./image-source-provider"; import { WorkspaceClassesConfig } from "./workspace-classes"; -import { SYSTEM_USER, SYSTEM_USER_ID, Authorizer } from "../authorization/authorizer"; +import { SYSTEM_USER, SYSTEM_USER_ID } from "../authorization/authorizer"; import { EnvVarService, ResolvedEnvVars } from "../user/env-var-service"; import { RedlockAbortSignal } from "redlock"; import { ConfigProvider } from "./config-provider"; @@ -240,7 +239,6 @@ export class WorkspaceStarter { @inject(EnvVarService) private readonly envVarService: EnvVarService, @inject(OrganizationService) private readonly orgService: OrganizationService, @inject(ProjectsService) private readonly projectService: ProjectsService, - @inject(Authorizer) private readonly auth: Authorizer, ) {} public async startWorkspace( @@ -2047,11 +2045,9 @@ export class WorkspaceStarter { // if the image resolution is for an organization, we also include the organization's set up env vars if (organizationId) { - await this.auth.checkPermissionOnOrganization(user.id, "read_env_var", organizationId); - const orgEnvVars = await this.orgDB.getOrgEnvironmentVariables(organizationId); - const orgEnvVarValues: OrgEnvVarWithValue[] = await this.orgDB.getOrgEnvironmentVariableValues(orgEnvVars); + const envVars = await this.envVarService.listOrgEnvVarsWithValues(user.id, organizationId); - const additionalAuth = await this.getAdditionalImageAuth({ workspace: orgEnvVarValues }); + const additionalAuth = await this.getAdditionalImageAuth({ workspace: envVars }); additionalAuth.forEach((val, key) => auth.getAdditionalMap().set(key, val)); }