From bdd76a511f4dbe4f98253815bdb49fe7b0f6a5ea Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Thu, 5 Oct 2023 12:56:35 +0000 Subject: [PATCH] temp --- .../server/src/authorization/authorizer.ts | 101 ++++++++--- .../src/authorization/spicedb-authorizer.ts | 164 ++++++++++++++++-- components/server/src/container-module.ts | 11 +- .../server/src/projects/projects-service.ts | 32 ++-- components/server/src/user/env-var-service.ts | 40 +++-- .../server/src/user/gitpod-token-service.ts | 12 +- components/server/src/user/sshkey-service.ts | 14 +- .../server/src/user/user-authentication.ts | 3 +- components/server/src/user/user-controller.ts | 2 +- .../server/src/user/user-deletion-service.ts | 2 +- .../server/src/user/user-service.spec.db.ts | 18 +- components/server/src/user/user-service.ts | 51 +++--- .../src/workspace/gitpod-server-impl.ts | 91 +++++----- .../server/src/workspace/workspace-factory.ts | 2 +- .../server/src/workspace/workspace-service.ts | 127 +++++++------- 15 files changed, 460 insertions(+), 210 deletions(-) diff --git a/components/server/src/authorization/authorizer.ts b/components/server/src/authorization/authorizer.ts index 3ec1bc6d3de3c9..da683454bb38b8 100644 --- a/components/server/src/authorization/authorizer.ts +++ b/components/server/src/authorization/authorizer.ts @@ -7,7 +7,7 @@ import { v1 } from "@authzed/authzed-node"; import { BUILTIN_INSTLLATION_ADMIN_USER_ID } from "@gitpod/gitpod-db/lib"; -import { Project, TeamMemberRole } from "@gitpod/gitpod-protocol"; +import { Project, TeamMemberRole, User, Workspace } from "@gitpod/gitpod-protocol"; import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; import { AllResourceTypes, @@ -69,7 +69,8 @@ export class Authorizer { consistency, }); - return this.authorizer.check(req, { userId }); + const result = await this.authorizer.check(undefined, req, { userId }); + return result.permitted; } async checkPermissionOnInstallation(userId: string, permission: InstallationPermission): Promise { @@ -98,7 +99,8 @@ export class Authorizer { consistency, }); - return this.authorizer.check(req, { userId }); + const result = await this.authorizer.check(orgId, req, { userId }); + return result.permitted; } async checkPermissionOnOrganization(userId: string, permission: OrganizationPermission, orgId: string) { @@ -116,40 +118,61 @@ export class Authorizer { ); } - async hasPermissionOnProject(userId: string, permission: ProjectPermission, projectId: string): Promise { + async hasPermissionOnProject( + userId: string, + permission: ProjectPermission, + project: { id: string; teamId?: string }, + ): Promise { if (userId === SYSTEM_USER) { return true; } + if (!project.teamId) { + return false; + } const req = v1.CheckPermissionRequest.create({ subject: subject("user", userId), permission, - resource: object("project", projectId), + resource: object("project", project.id), consistency, }); - return this.authorizer.check(req, { userId }); + const result = await this.authorizer.check(project.teamId, req, { userId }); + return result.permitted; } - async checkPermissionOnProject(userId: string, permission: ProjectPermission, projectId: string) { - if (await this.hasPermissionOnProject(userId, permission, projectId)) { + async checkPermissionOnProject( + userId: string, + permission: ProjectPermission, + project: { id: string; teamId?: string }, + ) { + if (await this.hasPermissionOnProject(userId, permission, project)) { return; } // check if the user has read permission - if ("read_info" === permission || !(await this.hasPermissionOnProject(userId, "read_info", projectId))) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, `Project ${projectId} not found.`); + if ("read_info" === permission || !(await this.hasPermissionOnProject(userId, "read_info", project))) { + throw new ApplicationError(ErrorCodes.NOT_FOUND, `Project ${project.id} not found.`); } throw new ApplicationError( ErrorCodes.PERMISSION_DENIED, - `You do not have ${permission} on project ${projectId}`, + `You do not have ${permission} on project ${project.id}`, ); } - async hasPermissionOnUser(userId: string, permission: UserPermission, resourceUserId: string): Promise { + async hasPermissionOnUser( + userId: string, + permission: UserPermission, + resourceUserId: string, + resourceUser: Pick | undefined, + ): Promise { if (userId === SYSTEM_USER) { return true; } + if (!resourceUser) { + return false; + } + // resourceUser.organizationId may very well be undefined here, if this is an installation-owned user const req = v1.CheckPermissionRequest.create({ subject: subject("user", userId), @@ -158,14 +181,23 @@ export class Authorizer { consistency, }); - return this.authorizer.check(req, { userId }); + const result = await this.authorizer.check(resourceUser.organizationId, req, { userId }); + return result.permitted; } - async checkPermissionOnUser(userId: string, permission: UserPermission, resourceUserId: string) { - if (await this.hasPermissionOnUser(userId, permission, resourceUserId)) { + async checkPermissionOnUser( + userId: string, + permission: UserPermission, + resourceUserId: string, + resourceUser: Pick | undefined, + ) { + if (await this.hasPermissionOnUser(userId, permission, resourceUserId, resourceUser)) { return; } - if ("read_info" === permission || !(await this.hasPermissionOnUser(userId, "read_info", resourceUserId))) { + if ( + "read_info" === permission || + !(await this.hasPermissionOnUser(userId, "read_info", resourceUserId, resourceUser)) + ) { throw new ApplicationError(ErrorCodes.NOT_FOUND, `User ${resourceUserId} not found.`); } @@ -178,34 +210,42 @@ export class Authorizer { async hasPermissionOnWorkspace( userId: string, permission: WorkspacePermission, - workspaceId: string, + workspace: { id: string; organizationId: string | undefined }, forceEnablement?: boolean, // temporary to find an issue with workspace sharing ): Promise { if (userId === SYSTEM_USER) { return true; } + if (!workspace.organizationId) { + return false; + } const req = v1.CheckPermissionRequest.create({ subject: subject("user", userId), permission, - resource: object("workspace", workspaceId), + resource: object("workspace", workspace.id), consistency, }); - return this.authorizer.check(req, { userId }, forceEnablement); + const result = await this.authorizer.check(workspace.organizationId, req, { userId }, forceEnablement); + return result.permitted; } - async checkPermissionOnWorkspace(userId: string, permission: WorkspacePermission, workspaceId: string) { - if (await this.hasPermissionOnWorkspace(userId, permission, workspaceId)) { + async checkPermissionOnWorkspace( + userId: string, + permission: WorkspacePermission, + workspace: { id: string; organizationId: string | undefined }, + ) { + if (await this.hasPermissionOnWorkspace(userId, permission, workspace)) { return; } - if ("read_info" === permission || !(await this.hasPermissionOnWorkspace(userId, "read_info", workspaceId))) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, `Workspace ${workspaceId} not found.`); + if ("read_info" === permission || !(await this.hasPermissionOnWorkspace(userId, "read_info", workspace))) { + throw new ApplicationError(ErrorCodes.NOT_FOUND, `Workspace ${workspace.id} not found.`); } throw new ApplicationError( ErrorCodes.PERMISSION_DENIED, - `You do not have ${permission} on workspace ${workspaceId}`, + `You do not have ${permission} on workspace ${workspace.id}`, ); } @@ -598,3 +638,16 @@ function asSet(array: (T | undefined)[]): Set { } return result; } + +export function projectIdFrom( + workspace: Pick, +): Pick { + return { id: workspace.projectId!, teamId: workspace.organizationId }; +} + +// function fromResource(resource: v1.ObjectReference): ObjectId { +// if (!ObjectId.isObjectKind(resource.objectType)) { +// throw new Error("Unknown object kind: " + resource.objectType); +// } +// return ObjectId.create(resource.objectType, resource.objectId); +// } diff --git a/components/server/src/authorization/spicedb-authorizer.ts b/components/server/src/authorization/spicedb-authorizer.ts index 2dfbd81f2ca8f0..d860192e550232 100644 --- a/components/server/src/authorization/spicedb-authorizer.ts +++ b/components/server/src/authorization/spicedb-authorizer.ts @@ -38,8 +38,33 @@ async function tryThree(errMessage: string, code: (attempt: number) => Promis throw new Error("unreachable"); } +export const SpiceDBAuthorizer = Symbol("SpiceDBAuthorizer"); +export interface SpiceDBAuthorizer { + check( + resourceOrgId: string | undefined, + req: v1.CheckPermissionRequest, + experimentsFields: { + userId: string; + }, + forceEnablement?: boolean, + ): Promise; + writeRelationships(...updates: v1.RelationshipUpdate[]): Promise; + deleteRelationships(req: v1.DeleteRelationshipsRequest): Promise; + readRelationships(req: v1.ReadRelationshipsRequest): Promise; +} + +export interface CheckResult { + permitted: boolean; + checkedAt?: string; +} + +export interface DeletionResult { + relationships: v1.ReadRelationshipsResponse[]; + deletedAt?: string; +} + @injectable() -export class SpiceDBAuthorizer { +export class SpiceDBAuthorizerImpl implements SpiceDBAuthorizer { constructor( @inject(SpiceDBClientProvider) private readonly clientProvider: SpiceDBClientProvider, @@ -50,14 +75,15 @@ export class SpiceDBAuthorizer { } async check( + resourceOrgId: string | undefined, req: v1.CheckPermissionRequest, experimentsFields: { userId: string; }, forceEnablement?: boolean, - ): Promise { + ): Promise { if (!(await isFgaWritesEnabled(experimentsFields.userId))) { - return true; + return { permitted: true }; } const featureEnabled = !!forceEnablement || (await isFgaChecksEnabled(experimentsFields.userId)); const result = (async () => { @@ -73,23 +99,23 @@ export class SpiceDBAuthorizer { response: new TrustedValue(response), request: new TrustedValue(req), }); - return true; + return { permitted: true, checkedAt: response.checkedAt?.token }; } - return permitted; + return { permitted, checkedAt: response.checkedAt?.token }; } catch (err) { error = err; log.error("[spicedb] Failed to perform authorization check.", err, { request: new TrustedValue(req), }); - return !featureEnabled; + return { permitted: !featureEnabled }; } finally { observeSpicedbClientLatency("check", error, timer()); } })(); // if the feature is not enabld, we don't await if (!featureEnabled) { - return true; + return { permitted: true }; } return result; } @@ -114,10 +140,11 @@ export class SpiceDBAuthorizer { } } - async deleteRelationships(req: v1.DeleteRelationshipsRequest): Promise { + async deleteRelationships(req: v1.DeleteRelationshipsRequest): Promise { const timer = spicedbClientLatency.startTimer(); let error: Error | undefined; try { + let deletedAt: string | undefined = undefined; const existing = await tryThree("readRelationships before deleteRelationships failed.", () => this.client.readRelationships(v1.ReadRelationshipsRequest.create(req), this.callOptions), ); @@ -125,6 +152,7 @@ export class SpiceDBAuthorizer { const response = await tryThree("deleteRelationships failed.", () => this.client.deleteRelationships(req, this.callOptions), ); + deletedAt = response.deletedAt?.token; const after = await tryThree("readRelationships failed.", () => this.client.readRelationships(v1.ReadRelationshipsRequest.create(req), this.callOptions), ); @@ -137,13 +165,16 @@ export class SpiceDBAuthorizer { existing, }); } - return existing; + return { + relationships: existing, + deletedAt, + }; } catch (err) { error = err; // While in we're running two authorization systems in parallel, we do not hard fail on writes. //TODO throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "Failed to delete relationships."); log.error("[spicedb] Failed to delete relationships.", err, { request: new TrustedValue(req) }); - return []; + return { relationships: [] }; } finally { observeSpicedbClientLatency("delete", error, timer()); } @@ -164,3 +195,116 @@ export class SpiceDBAuthorizer { }) as any as grpc.Metadata; } } + +@injectable() +export class CachingSpiceDBAuthorizer implements SpiceDBAuthorizer { + constructor( + @inject(SpiceDBAuthorizerImpl) + private readonly impl: SpiceDBAuthorizerImpl, + @inject(HierachicalZedTokenCache) + private readonly tokenCache: HierachicalZedTokenCache, + ) {} + + async check( + resourceOrgId: string | undefined, + req: v1.CheckPermissionRequest, + experimentsFields: { userId: string }, + forceEnablement?: boolean | undefined, + ): Promise { + req.consistency = await this.consistency(resourceOrgId, req.resource); + return this.impl.check(resourceOrgId, req, experimentsFields, forceEnablement); + } + + async writeRelationships(...updates: v1.RelationshipUpdate[]): Promise { + const result = await this.impl.writeRelationships(...updates); + const writtenAt = result?.writtenAt?.token; + await this.tokenCache.set( + ...updates.map((u) => [ + u.relationship?.resource, + writtenAt, // Make sure that in case we don't get a writtenAt token here, we at least invalidate the cache! + ]), + ); + return result; + } + + async deleteRelationships(req: v1.DeleteRelationshipsRequest): Promise { + const result = await this.impl.deleteRelationships(req); + log.info(`[spicedb] Deletion result`, { result }); + const deletedAt = result?.deletedAt; + if (deletedAt) { + // Only if we really deleted something we can actually update the cache. + await this.tokenCache.set( + ...result.relationships.map((r) => [r.relationship?.resource, deletedAt]), + ); + } + return result; + } + + async readRelationships(req: v1.ReadRelationshipsRequest): Promise { + // pass through with given consistency/caching for now + return this.impl.readRelationships(req); + } + + private async consistency( + orgId: string | undefined, + resourceRef: v1.ObjectReference | undefined, + ): Promise { + function fullyConsistent() { + return v1.Consistency.create({ + requirement: { + oneofKind: "fullyConsistent", + fullyConsistent: true, + }, + }); + } + + if (!resourceRef) { + return fullyConsistent(); + } + + const zedToken = await this.tokenCache.get(orgId, resourceRef); + if (!zedToken) { + return fullyConsistent(); + } + return v1.Consistency.create({ + requirement: { + oneofKind: "atLeastAsFresh", + atLeastAsFresh: v1.ZedToken.create({ + token: zedToken, + }), + }, + }); + } +} + +type ZedTokenCacheKV = [objectRef: v1.ObjectReference | undefined, token: string | undefined]; + +/** + * The entities in SpiceDB form a hierarchy, with "installation" at the top, and "workspace" and "user" at the bottom, for instance. + * This cache guarantees that for every requested entity, it always returns the _most recent ZedToken along the path to the bottom_. + */ +export class HierachicalZedTokenCache { + // private readonly cache = new Map(); + + async get(orgId: string | undefined, objectRef: v1.ObjectReference): Promise { + return "token"; + } + + async set(...kvs: ZedTokenCacheKV[]) { + // TODO + return undefined; + } + + async setIfUndefined(orgId: string | undefined, objectRef: v1.ObjectReference) {} + + // private buildPath(objectRef: v1.ObjectReference): string { + // const theInstallation = v1.ObjectReference.create({ objectType: "installation", objectId: InstallationID }); + // switch (objectRef.objectType) { + // case "installation": + // return `installation:${objectRef.objectId}`; + // case "user": + // // TODO(gpl): depends on orgId! + // return `${this.buildPath(theInstallation)}/user:${objectRef.objectId}`; + // } + // } +} diff --git a/components/server/src/container-module.ts b/components/server/src/container-module.ts index 05f2f2c3183672..25165a3303ce9d 100644 --- a/components/server/src/container-module.ts +++ b/components/server/src/container-module.ts @@ -51,7 +51,12 @@ import { Authorizer, createInitializingAuthorizer } from "./authorization/author import { RelationshipUpdater } from "./authorization/relationship-updater"; import { RelationshipUpdateJob } from "./authorization/relationship-updater-job"; import { SpiceDBClientProvider, spiceDBConfigFromEnv } from "./authorization/spicedb"; -import { SpiceDBAuthorizer } from "./authorization/spicedb-authorizer"; +import { + CachingSpiceDBAuthorizer, + HierachicalZedTokenCache, + SpiceDBAuthorizer, + SpiceDBAuthorizerImpl, +} from "./authorization/spicedb-authorizer"; import { BillingModes } from "./billing/billing-mode"; import { EntitlementService, EntitlementServiceImpl } from "./billing/entitlement-service"; import { EntitlementServiceUBP } from "./billing/entitlement-service-ubp"; @@ -317,7 +322,9 @@ export const productionContainerModule = new ContainerModule( ); }) .inSingletonScope(); - bind(SpiceDBAuthorizer).toSelf().inSingletonScope(); + bind(SpiceDBAuthorizerImpl).toSelf().inSingletonScope(); + bind(HierachicalZedTokenCache).toSelf().inSingletonScope(); + bind(SpiceDBAuthorizer).to(CachingSpiceDBAuthorizer).inSingletonScope(); bind(Authorizer) .toDynamicValue((ctx) => { const authorizer = ctx.container.get(SpiceDBAuthorizer); diff --git a/components/server/src/projects/projects-service.ts b/components/server/src/projects/projects-service.ts index 660a1c25a58cff..318ee4cbc5c68e 100644 --- a/components/server/src/projects/projects-service.ts +++ b/components/server/src/projects/projects-service.ts @@ -44,11 +44,12 @@ export class ProjectsService { ) {} async getProject(userId: string, projectId: string): Promise { - await this.auth.checkPermissionOnProject(userId, "read_info", projectId); const project = await this.projectDB.findProjectById(projectId); if (!project) { throw new ApplicationError(ErrorCodes.NOT_FOUND, `Project ${projectId} not found.`); } + await this.auth.checkPermissionOnProject(userId, "read_info", project); + return project; } @@ -86,7 +87,7 @@ export class ProjectsService { private async filterByReadAccess(userId: string, projects: Project[]) { const filteredProjects: Project[] = []; const filter = async (project: Project) => { - if (await this.auth.hasPermissionOnProject(userId, "read_info", project.id)) { + if (await this.auth.hasPermissionOnProject(userId, "read_info", project)) { return project; } return undefined; @@ -105,7 +106,7 @@ export class ProjectsService { const projects = await this.projectDB.findProjectsByCloneUrl(cloneUrl); const result: Project[] = []; for (const project of projects) { - if (await this.auth.hasPermissionOnProject(userId, "read_info", project.id)) { + if (await this.auth.hasPermissionOnProject(userId, "read_info", project)) { result.push(project); } } @@ -117,7 +118,9 @@ export class ProjectsService { projectId: string, kind: keyof ProjectUsage = "lastWorkspaceStart", ): Promise { - await this.auth.checkPermissionOnProject(userId, "read_info", projectId); + const project = await this.projectDB.findProjectById(projectId); + await this.auth.checkPermissionOnProject(userId, "read_info", { id: projectId, teamId: project?.teamId }); + await this.projectDB.updateProjectUsage(projectId, { [kind]: new Date().toISOString(), }); @@ -125,7 +128,8 @@ export class ProjectsService { async getProjectOverview(user: User, projectId: string): Promise { const project = await this.getProject(user.id, projectId); - await this.auth.checkPermissionOnProject(user.id, "read_info", project.id); + await this.auth.checkPermissionOnProject(user.id, "read_info", project); + // Check for a cached project overview (fast!) const cachedPromise = this.projectDB.findCachedProjectOverview(project.id); @@ -154,7 +158,7 @@ export class ProjectsService { } async getBranchDetails(user: User, project: Project, branchName?: string): Promise { - await this.auth.checkPermissionOnProject(user.id, "read_info", project.id); + await this.auth.checkPermissionOnProject(user.id, "read_info", project); const parsedUrl = RepoURL.parseRepoUrl(project.cloneUrl); if (!parsedUrl) { @@ -250,15 +254,14 @@ export class ProjectsService { } public async setVisibility(userId: string, projectId: string, visibility: Project.Visibility): Promise { - await this.auth.checkPermissionOnProject(userId, "write_info", projectId); const project = await this.getProject(userId, projectId); + await this.auth.checkPermissionOnProject(userId, "write_info", { id: projectId, teamId: project?.teamId }); + //TODO store this information in the DB await this.auth.setProjectVisibility(userId, projectId, project.teamId, visibility); } async deleteProject(userId: string, projectId: string, transactionCtx?: TransactionalContext): Promise { - await this.auth.checkPermissionOnProject(userId, "delete", projectId); - let orgId: string | undefined = undefined; try { await this.projectDB.transaction(transactionCtx, async (db) => { @@ -266,6 +269,8 @@ export class ProjectsService { if (!project) { throw new Error("Project does not exist"); } + await this.auth.checkPermissionOnProject(userId, "delete", project); + orgId = project.teamId; await db.markDeleted(projectId); @@ -288,11 +293,12 @@ export class ProjectsService { async findPrebuilds(userId: string, params: FindPrebuildsParams): Promise { const { projectId, prebuildId } = params; - await this.auth.checkPermissionOnProject(userId, "read_prebuild", projectId); const project = await this.projectDB.findProjectById(projectId); if (!project) { throw new ApplicationError(ErrorCodes.NOT_FOUND, `Project ${projectId} not found.`); } + await this.auth.checkPermissionOnProject(userId, "read_prebuild", project); + const parsedUrl = RepoURL.parseRepoUrl(project.cloneUrl); if (!parsedUrl) { throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, `Invalid clone URL on project ${projectId}.`); @@ -334,7 +340,7 @@ export class ProjectsService { } async updateProject(user: User, partialProject: PartialProject): Promise { - await this.auth.checkPermissionOnProject(user.id, "write_info", partialProject.id); + await this.auth.checkPermissionOnProject(user.id, "write_info", partialProject); const partial: PartialProject = { id: partialProject.id }; if (partialProject.name) { @@ -378,7 +384,9 @@ export class ProjectsService { } async isProjectConsideredInactive(userId: string, projectId: string): Promise { - await this.auth.checkPermissionOnProject(userId, "read_info", projectId); + const project = await this.getProject(userId, projectId); + await this.auth.checkPermissionOnProject(userId, "read_info", project); + const usage = await this.projectDB.getProjectUsage(projectId); if (!usage?.lastWorkspaceStart) { return false; diff --git a/components/server/src/user/env-var-service.ts b/components/server/src/user/env-var-service.ts index 0d865617b2568e..a449078a924678 100644 --- a/components/server/src/user/env-var-service.ts +++ b/components/server/src/user/env-var-service.ts @@ -8,6 +8,7 @@ import { ProjectDB, UserDB } from "@gitpod/gitpod-db/lib"; import { CommitContext, EnvVar, + Project, ProjectEnvVar, ProjectEnvVarWithValue, UserEnvVar, @@ -21,6 +22,7 @@ import { Authorizer } from "../authorization/authorizer"; import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics"; import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; import { Config } from "../config"; +import { ProjectPermission } from "../authorization/definitions"; export interface ResolvedEnvVars { // all project env vars, censored included always @@ -44,7 +46,8 @@ export class EnvVarService { userId: string, oldPermissionCheck?: (envvar: UserEnvVar) => Promise, // @deprecated ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "read_env_var", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "read_env_var", userId, user); const result: UserEnvVarValue[] = []; for (const value of await this.userDB.getEnvVars(userId)) { if (oldPermissionCheck && !(await oldPermissionCheck(value))) { @@ -66,7 +69,8 @@ export class EnvVarService { variable: UserEnvVarValue, oldPermissionCheck?: (envvar: UserEnvVar) => Promise, // @deprecated ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "write_env_var", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "write_env_var", userId, user); const validationError = UserEnvVar.validate(variable); if (validationError) { throw new ApplicationError(ErrorCodes.BAD_REQUEST, validationError); @@ -103,7 +107,8 @@ export class EnvVarService { variable: UserEnvVarValue, oldPermissionCheck?: (envvar: UserEnvVar) => Promise, // @deprecated ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "write_env_var", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "write_env_var", userId, user); const validationError = UserEnvVar.validate(variable); if (validationError) { throw new ApplicationError(ErrorCodes.BAD_REQUEST, validationError); @@ -131,7 +136,8 @@ export class EnvVarService { variable: UserEnvVarValue, oldPermissionCheck?: (envvar: UserEnvVar) => Promise, // @deprecated ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "write_env_var", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "write_env_var", userId, user); let envVarId = variable.id; if (!variable.id && variable.name && variable.repositoryPattern) { variable.repositoryPattern = UserEnvVar.normalizeRepoPattern(variable.repositoryPattern); @@ -160,7 +166,8 @@ export class EnvVarService { } async listProjectEnvVars(requestorId: string, projectId: string): Promise { - await this.auth.checkPermissionOnProject(requestorId, "read_env_var", projectId); + await this.checkPermissionOnProject(requestorId, "read_env_var", projectId); + return this.projectDB.getProjectEnvironmentVariables(projectId); } @@ -170,7 +177,7 @@ export class EnvVarService { throw new ApplicationError(ErrorCodes.NOT_FOUND, `Environment Variable ${variableId} not found.`); } try { - await this.auth.checkPermissionOnProject(requestorId, "read_env_var", result.projectId); + await this.checkPermissionOnProject(requestorId, "read_env_var", result.projectId); } catch (err) { throw new ApplicationError(ErrorCodes.NOT_FOUND, `Environment Variable ${variableId} not found.`); } @@ -178,7 +185,7 @@ export class EnvVarService { } async addProjectEnvVar(requestorId: string, projectId: string, envVar: ProjectEnvVarWithValue): Promise { - await this.auth.checkPermissionOnProject(requestorId, "write_env_var", projectId); + await this.checkPermissionOnProject(requestorId, "write_env_var", projectId); if (!envVar.name) { throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Variable name cannot be empty"); @@ -199,7 +206,7 @@ export class EnvVarService { } async updateProjectEnvVar(requestorId: string, projectId: string, envVar: ProjectEnvVarWithValue): Promise { - await this.auth.checkPermissionOnProject(requestorId, "write_env_var", projectId); + await this.checkPermissionOnProject(requestorId, "write_env_var", projectId); if (!envVar.name) { throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Variable name cannot be empty"); @@ -221,7 +228,7 @@ export class EnvVarService { async deleteProjectEnvVar(requestorId: string, variableId: string): Promise { const variable = await this.getProjectEnvVarById(requestorId, variableId); - await this.auth.checkPermissionOnProject(requestorId, "write_env_var", variable.projectId); + await this.checkPermissionOnProject(requestorId, "write_env_var", variable.projectId); return this.projectDB.deleteProjectEnvironmentVariable(variableId); } @@ -231,9 +238,10 @@ export class EnvVarService { wsType: WorkspaceType, wsContext: WorkspaceContext, ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "read_env_var", requestorId); + const requestor = await this.userDB.findUserById(requestorId); + await this.auth.checkPermissionOnUser(requestorId, "read_env_var", requestorId, requestor); if (projectId) { - await this.auth.checkPermissionOnProject(requestorId, "read_env_var", projectId); + await this.checkPermissionOnProject(requestorId, "read_env_var", projectId); } const workspaceEnvVars = new Map(); @@ -285,4 +293,14 @@ export class EnvVarService { workspace: [...workspaceEnvVars.values()], }; } + + private async checkPermissionOnProject( + subjectId: string, + permission: ProjectPermission, + projectId: string, + ): Promise { + const project = await this.projectDB.findProjectById(projectId); + await this.auth.checkPermissionOnProject(subjectId, permission, { id: projectId, teamId: project?.teamId }); + return project!; + } } diff --git a/components/server/src/user/gitpod-token-service.ts b/components/server/src/user/gitpod-token-service.ts index aea23b1965383b..22548638a8577a 100644 --- a/components/server/src/user/gitpod-token-service.ts +++ b/components/server/src/user/gitpod-token-service.ts @@ -19,7 +19,8 @@ export class GitpodTokenService { ) {} async getGitpodTokens(requestorId: string, userId: string): Promise { - await this.auth.checkPermissionOnUser(requestorId, "read_tokens", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "read_tokens", userId, user); const gitpodTokens = await this.userDB.findAllGitpodTokensOfUser(userId); return gitpodTokens.filter((v) => !v.deleted); } @@ -30,7 +31,8 @@ export class GitpodTokenService { options: { name?: string; type: GitpodTokenType; scopes?: string[] }, oldPermissionCheck?: (dbToken: DBGitpodToken) => Promise, // @deprecated ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "write_tokens", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "write_tokens", userId, user); const token = crypto.randomBytes(30).toString("hex"); const tokenHash = crypto.createHash("sha256").update(token, "utf8").digest("hex"); const dbToken: DBGitpodToken = { @@ -49,7 +51,8 @@ export class GitpodTokenService { } async findGitpodToken(requestorId: string, userId: string, tokenHash: string): Promise { - await this.auth.checkPermissionOnUser(requestorId, "read_tokens", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "read_tokens", userId, user); let token: GitpodToken | undefined; try { token = await this.userDB.findGitpodTokensOfUser(userId, tokenHash); @@ -68,7 +71,8 @@ export class GitpodTokenService { tokenHash: string, oldPermissionCheck?: (token: GitpodToken) => Promise, // @deprecated ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "write_tokens", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "write_tokens", userId, user); const existingTokens = await this.getGitpodTokens(requestorId, userId); const tkn = existingTokens.find((token) => token.tokenHash === tokenHash); if (!tkn) { diff --git a/components/server/src/user/sshkey-service.ts b/components/server/src/user/sshkey-service.ts index 48c8bd06abacf5..5e56fa01d84f21 100644 --- a/components/server/src/user/sshkey-service.ts +++ b/components/server/src/user/sshkey-service.ts @@ -23,12 +23,14 @@ export class SSHKeyService { ) {} async hasSSHPublicKey(requestorId: string, userId: string): Promise { - await this.auth.checkPermissionOnUser(requestorId, "read_ssh", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "read_ssh", userId, user); return this.userDB.hasSSHPublicKey(userId); } async getSSHPublicKeys(requestorId: string, userId: string): Promise { - await this.auth.checkPermissionOnUser(requestorId, "read_ssh", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "read_ssh", userId, user); const list = await this.userDB.getSSHPublicKeys(userId); return list.map((e) => ({ id: e.id, @@ -45,7 +47,9 @@ export class SSHKeyService { userId: string, value: SSHPublicKeyValue, ): Promise { - await this.auth.checkPermissionOnUser(requestorId, "write_ssh", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "write_ssh", userId, user); + const data = await this.userDB.addSSHPublicKey(userId, value); this.updateSSHKeysForRegularRunningInstances(userId).catch((err) => { log.error("Failed to update ssh keys on running instances.", err); @@ -61,7 +65,9 @@ export class SSHKeyService { } async deleteSSHPublicKey(requestorId: string, userId: string, id: string): Promise { - await this.auth.checkPermissionOnUser(requestorId, "write_ssh", userId); + const user = await this.userDB.findUserById(userId); + await this.auth.checkPermissionOnUser(requestorId, "write_ssh", userId, user); + await this.userDB.deleteSSHPublicKey(userId, id); this.updateSSHKeysForRegularRunningInstances(userId).catch((err) => { log.error("Failed to update ssh keys on running instances.", err); diff --git a/components/server/src/user/user-authentication.ts b/components/server/src/user/user-authentication.ts index b55261af3fcb15..92287a2481c9ce 100644 --- a/components/server/src/user/user-authentication.ts +++ b/components/server/src/user/user-authentication.ts @@ -41,8 +41,9 @@ export class UserAuthentication { ) {} async blockUser(userId: string, targetUserId: string, block: boolean): Promise { - await this.authorizer.checkPermissionOnUser(userId, "admin_control", targetUserId); const target = await this.userService.findUserById(userId, targetUserId); + await this.authorizer.checkPermissionOnUser(userId, "admin_control", targetUserId, target); + target.blocked = !!block; return await this.userDb.storeUser(target); } diff --git a/components/server/src/user/user-controller.ts b/components/server/src/user/user-controller.ts index 49c58bc2e26275..f7396abe2623ab 100644 --- a/components/server/src/user/user-controller.ts +++ b/components/server/src/user/user-controller.ts @@ -254,7 +254,7 @@ export class UserController { } // reset the FGA state - await this.userService.resetFgaVersion(user.id, user.id); + await this.userService.resetFgaVersion(user.id, user); const redirectToUrl = this.getSafeReturnToParam(req) || this.config.hostUrl.toString(); diff --git a/components/server/src/user/user-deletion-service.ts b/components/server/src/user/user-deletion-service.ts index ca678d78e3b5c0..bdd8401b46ff96 100644 --- a/components/server/src/user/user-deletion-service.ts +++ b/components/server/src/user/user-deletion-service.ts @@ -36,8 +36,8 @@ export class UserDeletionService { * we anonymize data that might contain user related/relatable data and keep the entities itself (incl. ids). */ async deleteUser(userId: string, targetUserId: string): Promise { - await this.authorizer.checkPermissionOnUser(userId, "delete", targetUserId); const user = await this.db.findUserById(targetUserId); + await this.authorizer.checkPermissionOnUser(userId, "delete", targetUserId, user); if (!user) { throw new Error(`No user with id ${targetUserId} found!`); } diff --git a/components/server/src/user/user-service.spec.db.ts b/components/server/src/user/user-service.spec.db.ts index 9f6925aa659724..482e87afd26f95 100644 --- a/components/server/src/user/user-service.spec.db.ts +++ b/components/server/src/user/user-service.spec.db.ts @@ -77,17 +77,19 @@ describe("UserService", async () => { }); it("createUser", async () => { - expect(await auth.hasPermissionOnUser(user.id, "read_info", user.id)).to.be.true; - expect(await auth.hasPermissionOnUser(user.id, "write_info", user.id)).to.be.true; + expect(await auth.hasPermissionOnUser(user.id, "read_info", user.id, user)).to.be.true; + expect(await auth.hasPermissionOnUser(user.id, "write_info", user.id, user)).to.be.true; - expect(await auth.hasPermissionOnUser(user2.id, "read_info", user.id)).to.be.true; - expect(await auth.hasPermissionOnUser(user2.id, "write_info", user.id)).to.be.false; + expect(await auth.hasPermissionOnUser(user2.id, "read_info", user.id, user)).to.be.true; + expect(await auth.hasPermissionOnUser(user2.id, "write_info", user.id, user)).to.be.false; - expect(await auth.hasPermissionOnUser(nonOrgUser.id, "read_info", user.id)).to.be.false; - expect(await auth.hasPermissionOnUser(nonOrgUser.id, "write_info", user.id)).to.be.false; + expect(await auth.hasPermissionOnUser(nonOrgUser.id, "read_info", user.id, user)).to.be.false; + expect(await auth.hasPermissionOnUser(nonOrgUser.id, "write_info", user.id, user)).to.be.false; - expect(await auth.hasPermissionOnUser(BUILTIN_INSTLLATION_ADMIN_USER_ID, "read_info", user.id)).to.be.true; - expect(await auth.hasPermissionOnUser(BUILTIN_INSTLLATION_ADMIN_USER_ID, "write_info", user.id)).to.be.false; + expect(await auth.hasPermissionOnUser(BUILTIN_INSTLLATION_ADMIN_USER_ID, "read_info", user.id, user)).to.be + .true; + expect(await auth.hasPermissionOnUser(BUILTIN_INSTLLATION_ADMIN_USER_ID, "write_info", user.id, user)).to.be + .false; }); it("updateLoggedInUser_avatarUrlNotUpdatable", async () => { diff --git a/components/server/src/user/user-service.ts b/components/server/src/user/user-service.ts index 1c0baccce86dc0..8d7b4d5117bb37 100644 --- a/components/server/src/user/user-service.ts +++ b/components/server/src/user/user-service.ts @@ -74,18 +74,19 @@ export class UserService { } } - async findUserById(userId: string, id: string): Promise { - if (userId !== id) { - await this.authorizer.checkPermissionOnUser(userId, "read_info", id); + async findUserById(userId: string, targetId: string): Promise { + const result = await this.userDb.findUserById(targetId); + if (userId !== targetId) { + await this.authorizer.checkPermissionOnUser(userId, "read_info", targetId, result); } - const result = await this.userDb.findUserById(id); if (!result) { throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found"); } + try { return await this.relationshipUpdater.migrate(result); } catch (error) { - log.error({ userId: id }, "Failed to migrate user", error); + log.error({ userId: targetId }, "Failed to migrate user", error); return result; } } @@ -96,35 +97,34 @@ export class UserService { } async updateUser(userId: string, update: Partial & { id: string }): Promise { - const user = await this.findUserById(userId, update.id); - await this.authorizer.checkPermissionOnUser(userId, "write_info", user.id); + const targetUser = await this.findUserById(userId, update.id); + await this.authorizer.checkPermissionOnUser(userId, "write_info", update.id, targetUser); //hang on to user profile before it's overwritten for analytics below - const oldProfile = User.getProfile(user); - + const oldProfile = User.getProfile(targetUser); const allowedFields: (keyof User)[] = ["fullName", "additionalData"]; for (const p of allowedFields) { if (p in update) { - (user[p] as any) = update[p]; + (targetUser[p] as any) = update[p]; } } - await this.userDb.updateUserPartial(user); + await this.userDb.updateUserPartial(targetUser); //track event and user profile if profile of partialUser changed - const newProfile = User.getProfile(user); + const newProfile = User.getProfile(targetUser); if (User.Profile.hasChanges(oldProfile, newProfile)) { this.analytics.track({ - userId: user.id, + userId: targetUser.id, event: "profile_changed", properties: { new: newProfile, old: oldProfile }, }); this.analytics.identify({ - userId: user.id, + userId: targetUser.id, traits: { email: newProfile.email, company: newProfile.company, name: newProfile.name }, }); } - return user; + return targetUser; } async updateWorkspaceTimeoutSetting( @@ -132,8 +132,6 @@ export class UserService { targetUserId: string, setting: Partial, ): Promise { - await this.authorizer.checkPermissionOnUser(userId, "write_info", targetUserId); - if (setting.workspaceTimeout) { try { WorkspaceTimeoutDuration.validate(setting.workspaceTimeout); @@ -142,9 +140,11 @@ export class UserService { } } - const user = await this.findUserById(userId, targetUserId); - AdditionalUserData.set(user, setting); - await this.userDb.updateUserPartial(user); + const target = await this.findUserById(userId, targetUserId); + await this.authorizer.checkPermissionOnUser(userId, "write_info", targetUserId, target); + + AdditionalUserData.set(target, setting); + await this.userDb.updateUserPartial(target); } async listUsers( @@ -168,7 +168,7 @@ export class UserService { ); const result = { total: res.total, rows: [] as User[] }; for (const user of res.rows) { - if (await this.authorizer.hasPermissionOnUser(userId, "read_info", user.id)) { + if (await this.authorizer.hasPermissionOnUser(userId, "read_info", user.id, user)) { result.rows.push(user); } else { result.total--; @@ -185,8 +185,9 @@ export class UserService { targetUserId: string, modifications: { role: RoleOrPermission; add?: boolean }[], ): Promise { - await this.authorizer.checkPermissionOnUser(userId, "make_admin", targetUserId); const target = await this.findUserById(userId, targetUserId); + await this.authorizer.checkPermissionOnUser(userId, "make_admin", targetUserId, target); + const rolesOrPermissions = new Set((target.rolesOrPermissions || []) as string[]); const adminBefore = rolesOrPermissions.has("admin"); modifications.forEach((e) => { @@ -221,9 +222,9 @@ export class UserService { } } - async resetFgaVersion(subjectId: string, userId: string) { - await this.authorizer.checkPermissionOnUser(subjectId, "write_info", userId); + async resetFgaVersion(subjectId: string, user: User) { + await this.authorizer.checkPermissionOnUser(subjectId, "write_info", user.id, user); - await this.userDb.updateUserPartial({ id: userId, fgaRelationshipsVersion: undefined }); + await this.userDb.updateUserPartial({ id: user.id, fgaRelationshipsVersion: undefined }); } } diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts index 0befe04fe5f8ee..e80d1bccdac1dd 100644 --- a/components/server/src/workspace/gitpod-server-impl.ts +++ b/components/server/src/workspace/gitpod-server-impl.ts @@ -160,13 +160,13 @@ import { } from "@gitpod/usage-api/lib/usage/v1/billing.pb"; import { ClientError } from "nice-grpc-common"; import { BillingModes } from "../billing/billing-mode"; -import { Authorizer, SYSTEM_USER, isFgaChecksEnabled } from "../authorization/authorizer"; +import { Authorizer, SYSTEM_USER, isFgaChecksEnabled, projectIdFrom } from "../authorization/authorizer"; import { OrganizationService } from "../orgs/organization-service"; import { RedisSubscriber } from "../messaging/redis-subscriber"; import { UsageService } from "../orgs/usage-service"; import { UserService } from "../user/user-service"; import { SSHKeyService } from "../user/sshkey-service"; -import { StartWorkspaceOptions, WorkspaceService } from "./workspace-service"; +import { StartWorkspaceOptions, WorkspaceService, instanceNotFound } from "./workspace-service"; import { GitpodTokenService } from "../user/gitpod-token-service"; import { EnvVarService } from "../user/env-var-service"; import { ScmService } from "../projects/scm-service"; @@ -177,8 +177,6 @@ import { suggestionFromRecentWorkspace, suggestionFromUserRepo, } from "./suggested-repos-sorter"; -import { TrustedValue } from "@gitpod/gitpod-protocol/lib/util/scrubbing"; -import { rel } from "../authorization/definitions"; // shortcut export const traceWI = (ctx: TraceContext, wi: Omit) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager @@ -392,7 +390,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { } // check if the user has access to the project - if (!(await this.auth.hasPermissionOnProject(user.id, "read_prebuild", prebuiltWorkspace.projectId))) { + const project = await this.projectsService.getProject(user.id, prebuiltWorkspace.projectId); + if (!(await this.auth.hasPermissionOnProject(user.id, "read_prebuild", project))) { return undefined; } if (prebuiltWorkspace.state === "available") { @@ -565,23 +564,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { `operation not permitted: missing ${op} permission on ${resource.kind}`, ); } - if (resource.kind === "workspace" && op === "get") { - // access to workspaces is granted. Let's verify that this is also thecase with FGA - const result = await this.auth.hasPermissionOnWorkspace( - this.userID!, - "read_info", - resource.subject.id, - true, - ); - if (!result) { - const isShared = await this.auth.find(rel.workspace(resource.subject.id).shared.anyUser); - log.error("user has access to workspace, but not through FGA", { - userId: this.userID, - workspace: new TrustedValue(resource.subject), - sharedRelationship: isShared && new TrustedValue(isShared), - }); - } - } } /** @@ -893,11 +875,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const user = await this.checkAndBlockUser("getOwnerToken"); //TODO this requests are only here to populate the resource guard check - const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId); + const { workspace, latestInstance } = await this.workspaceService.getWorkspace(user.id, workspaceId); await this.guardAccess({ kind: "workspace", subject: workspace }, "get"); - - const latestInstance = await this.workspaceService.getCurrentInstance(user.id, workspaceId); - await this.guardAccess({ kind: "workspaceInstance", subject: latestInstance, workspace }, "get"); + const instance = instanceNotFound(workspaceId, latestInstance); + await this.guardAccess({ kind: "workspaceInstance", subject: instance, workspace }, "get"); return await this.workspaceService.getOwnerToken(user.id, workspaceId); } @@ -1229,7 +1210,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { return true; } - await this.auth.checkPermissionOnProject(user.id, "read_prebuild", pws.projectId); + const project = await this.projectsService.getProject(user.id, pws.projectId); + await this.auth.checkPermissionOnProject(user.id, "read_prebuild", project); return PrebuiltWorkspace.isDone(pws); } @@ -1515,7 +1497,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const project = await this.projectsService.getProject(user.id, projectId); await this.guardProjectOperation(user, projectId, "get"); - await this.auth.checkPermissionOnProject(user.id, "read_prebuild", projectId); + await this.auth.checkPermissionOnProject(user.id, "read_prebuild", project); const events = await this.projectsService.getPrebuildEvents(user.id, project.id); return events; @@ -1532,7 +1514,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const project = await this.projectsService.getProject(user.id, projectId); await this.guardProjectOperation(user, projectId, "update"); - await this.auth.checkPermissionOnProject(user.id, "write_prebuild", projectId); + await this.auth.checkPermissionOnProject(user.id, "write_prebuild", project); const branchDetails = !!branchName ? await this.projectsService.getBranchDetails(user, project, branchName) @@ -1932,8 +1914,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const user = await this.checkAndBlockUser("updateGitStatus"); - const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId); - const instance = await this.workspaceService.getCurrentInstance(user.id, workspaceId); + const { workspace, latestInstance } = await this.workspaceService.getWorkspace(user.id, workspaceId); + const instance = instanceNotFound(workspaceId, latestInstance); traceWI(ctx, { instanceId: instance.id }); await this.guardAccess({ kind: "workspaceInstance", subject: instance, workspace }, "update"); @@ -2061,7 +2043,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const user = await this.checkAndBlockUser("takeSnapshot"); const workspace = await this.guardSnaphotAccess(ctx, user.id, workspaceId); - await this.auth.checkPermissionOnWorkspace(user.id, "create_snapshot", workspaceId); + await this.auth.checkPermissionOnWorkspace(user.id, "create_snapshot", workspace); const instance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId); if (!instance) { @@ -2567,7 +2549,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId); await this.guardAccess({ kind: "prebuild", subject: pbws, workspace, teamMembers }, "get"); - await this.auth.checkPermissionOnProject(user.id, "read_prebuild", workspace.projectId!); + await this.auth.checkPermissionOnProject(user.id, "read_prebuild", projectIdFrom(workspace)); const result: PrebuildWithStatus = { info, status: pbws.state }; if (pbws.error) { result.error = pbws.error; @@ -2586,13 +2568,13 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { this.workspaceDb.trace(ctx).findPrebuildByWorkspaceID(workspaceId), this.workspaceDb.trace(ctx).findById(workspaceId), ]); - if (!pbws || !workspace) { + if (!pbws || !workspace || !workspace.projectId) { return undefined; } const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId); await this.guardAccess({ kind: "prebuild", subject: pbws, workspace, teamMembers }, "get"); - await this.auth.checkPermissionOnProject(user.id, "read_prebuild", workspace.projectId!); + await this.auth.checkPermissionOnProject(user.id, "read_prebuild", projectIdFrom(workspace)); return pbws; } @@ -2630,9 +2612,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const user = await this.checkAndBlockUser("cancelPrebuild"); - await this.projectsService.getProject(user.id, projectId); + const project = await this.projectsService.getProject(user.id, projectId); await this.guardProjectOperation(user, projectId, "update"); - await this.auth.checkPermissionOnProject(user.id, "write_prebuild", projectId); + await this.auth.checkPermissionOnProject(user.id, "write_prebuild", project); const prebuild = await this.workspaceDb.trace(ctx).findPrebuildByID(prebuildId); if (!prebuild) { @@ -2886,8 +2868,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { async adminVerifyUser(ctx: TraceContext, userId: string): Promise { const admin = await this.guardAdminAccess("adminVerifyUser", { id: userId }, Permission.ADMIN_USERS); - await this.auth.checkPermissionOnUser(admin.id, "admin_control", userId); const user = await this.userService.findUserById(admin.id, userId); + await this.auth.checkPermissionOnUser(admin.id, "admin_control", userId, user); this.verificationService.markVerified(user); await this.userDB.updateUserPartial(user); @@ -2927,9 +2909,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { { req }, Permission.ADMIN_USERS, ); - await this.auth.checkPermissionOnUser(admin.id, "admin_control", req.id); const target = await this.userService.findUserById(admin.id, req.id); + await this.auth.checkPermissionOnUser(admin.id, "admin_control", target.id, target); const featureSettings: UserFeatureSettings = target.featureFlags || {}; const featureFlags = new Set(featureSettings.permanentWSFeatureFlags || []); @@ -2967,7 +2949,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { await Promise.all( wss.rows.map(async (row) => { - if (!(await this.auth.hasPermissionOnWorkspace(admin.id, "access", row.workspaceId))) { + if ( + !(await this.auth.hasPermissionOnWorkspace(admin.id, "access", { + id: row.workspaceId, + organizationId: row.organizationId, + })) + ) { wss.total--; wss.rows = wss.rows.filter((ws) => ws.workspaceId !== row.workspaceId); } @@ -2984,13 +2971,20 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { { id: workspaceId }, Permission.ADMIN_WORKSPACES, ); - await this.auth.checkPermissionOnWorkspace(admin.id, "access", workspaceId); - const result = await this.workspaceDb.trace(ctx).findWorkspaceAndInstance(workspaceId); - if (!result) { + const { workspace, latestInstance } = await this.workspaceService.getWorkspace(admin.id, workspaceId); + if (!latestInstance) { throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found"); } - return result; + return { + ...workspace, + ...latestInstance, + workspaceId: workspace.id, + workspaceCreationTime: workspace.creationTime, + instanceId: latestInstance.id, + instanceCreationTime: latestInstance.creationTime, + phase: latestInstance.status.phase, + }; } async adminGetWorkspaceInstances(ctx: TraceContext, workspaceId: string): Promise { @@ -3001,7 +2995,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { { id: workspaceId }, Permission.ADMIN_WORKSPACES, ); - await this.auth.checkPermissionOnWorkspace(admin.id, "access", workspaceId); + const { workspace } = await this.workspaceService.getWorkspace(admin.id, workspaceId); + await this.auth.checkPermissionOnWorkspace(admin.id, "access", workspace); const result = await this.workspaceDb.trace(ctx).findInstances(workspaceId); return result || []; @@ -3015,7 +3010,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { { id: workspaceId }, Permission.ADMIN_WORKSPACES, ); - await this.auth.checkPermissionOnWorkspace(admin.id, "admin_control", workspaceId); const workspace = await this.workspaceDb.trace(ctx).findById(workspaceId); if (workspace) { @@ -3038,9 +3032,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { { id: workspaceId }, Permission.ADMIN_WORKSPACES, ); - await this.auth.checkPermissionOnWorkspace(admin.id, "admin_control", workspaceId); const ws = await this.workspaceDb.trace(ctx).findById(workspaceId); + await this.auth.checkPermissionOnWorkspace(admin.id, "admin_control", { + id: workspaceId, + organizationId: ws?.organizationId, + }); if (!ws) { throw new ApplicationError(ErrorCodes.NOT_FOUND, `No workspace with id '${workspaceId}' found.`); } diff --git a/components/server/src/workspace/workspace-factory.ts b/components/server/src/workspace/workspace-factory.ts index 7dfc5c011e70db..8f462c53d69d5c 100644 --- a/components/server/src/workspace/workspace-factory.ts +++ b/components/server/src/workspace/workspace-factory.ts @@ -248,7 +248,7 @@ export class WorkspaceFactory { const teams = await this.teamDB.findTeamsByUser(user.id); if (teams.some((t) => t.id === project.teamId)) { projectId = project.id; - await this.authorizer.checkPermissionOnProject(user.id, "read_prebuild", projectId); + await this.authorizer.checkPermissionOnProject(user.id, "read_prebuild", project); } } diff --git a/components/server/src/workspace/workspace-service.ts b/components/server/src/workspace/workspace-service.ts index 39f55f7364b888..da06bd038b5b11 100644 --- a/components/server/src/workspace/workspace-service.ts +++ b/components/server/src/workspace/workspace-service.ts @@ -30,7 +30,7 @@ import { WorkspaceTimeoutDuration, } from "@gitpod/gitpod-protocol"; import { ErrorCodes, ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error"; -import { Authorizer } from "../authorization/authorizer"; +import { Authorizer, projectIdFrom } from "../authorization/authorizer"; import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing"; import { WorkspaceFactory } from "./workspace-factory"; import { @@ -123,9 +123,7 @@ export class WorkspaceService { async getWorkspace(userId: string, workspaceId: string): Promise { const workspace = await this.doGetWorkspace(userId, workspaceId); - - const latestInstancePromise = this.db.findCurrentInstance(workspaceId); - const latestInstance = await latestInstancePromise; + const latestInstance = await this.db.findCurrentInstance(workspaceId); return { workspace, @@ -144,30 +142,24 @@ export class WorkspaceService { const filtered = ( await Promise.all( res.map(async (info) => - (await this.auth.hasPermissionOnWorkspace(userId, "access", info.workspace.id)) ? info : undefined, + (await this.auth.hasPermissionOnWorkspace(userId, "access", info.workspace)) ? info : undefined, ), ) ).filter((info) => !!info) as WorkspaceInfo[]; return filtered; } - async getCurrentInstance(userId: string, workspaceId: string): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); - const result = await this.db.findCurrentInstance(workspaceId); - if (!result) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, "No workspace instance found.", { workspaceId }); - } - return result; - } - // Internal method for allowing for additional DBs to be passed in private async doGetWorkspace(userId: string, workspaceId: string, db: WorkspaceDB = this.db): Promise { const workspace = await db.findById(workspaceId); if (workspace?.type === "prebuild" && workspace.projectId) { - await this.auth.checkPermissionOnProject(userId, "read_prebuild", workspace.projectId); + await this.auth.checkPermissionOnProject(userId, "read_prebuild", projectIdFrom(workspace)); } else { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", { + id: workspaceId, + organizationId: workspace?.organizationId, + }); } // TODO(gpl) We might want to add || !!workspace.softDeleted here in the future, but we were unsure how that would affect existing clients @@ -179,12 +171,9 @@ export class WorkspaceService { } async getOwnerToken(userId: string, workspaceId: string): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); - - // Check: is deleted? - await this.getWorkspace(userId, workspaceId); + const { workspace, latestInstance } = await this.getWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); - const latestInstance = await this.db.findCurrentInstance(workspaceId); const ownerToken = latestInstance?.status.ownerToken; if (!ownerToken) { throw new ApplicationError(ErrorCodes.NOT_FOUND, "owner token not found"); @@ -193,9 +182,9 @@ export class WorkspaceService { } async getIDECredentials(userId: string, workspaceId: string): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); - const ws = await this.doGetWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", ws); + if (ws.config.ideCredentials) { return ws.config.ideCredentials; } @@ -218,9 +207,9 @@ export class WorkspaceService { reason: string, policy?: StopWorkspacePolicy, ): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "stop", workspaceId); - const workspace = await this.doGetWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "stop", workspace); + const instance = await this.db.findRunningInstance(workspace.id); if (!instance) { // there's no instance running - we're done @@ -239,7 +228,7 @@ export class WorkspaceService { const infos = await this.db.findRunningInstancesWithWorkspaces(undefined, targetUserId); await Promise.all( infos.map(async (info) => { - await this.auth.checkPermissionOnWorkspace(userId, "stop", info.workspace.id); + await this.auth.checkPermissionOnWorkspace(userId, "stop", info.workspace); await this.workspaceStarter.stopWorkspaceInstance( ctx, info.latestInstance.id, @@ -264,7 +253,8 @@ export class WorkspaceService { workspaceId: string, softDeleted: WorkspaceSoftDeletion = "user", ): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "delete", workspaceId); + const workspace = await this.doGetWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "delete", workspace); await this.stopWorkspace(userId, workspaceId, "deleted via WorkspaceService"); await this.db.updatePartial(workspaceId, { @@ -282,12 +272,16 @@ export class WorkspaceService { * @param workspaceId */ public async hardDeleteWorkspace(userId: string, workspaceId: string): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "delete", workspaceId); - const workspace = await this.db.findById(workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "delete", { + id: workspaceId, + organizationId: workspace?.organizationId, + }); + if (!workspace) { throw new ApplicationError(ErrorCodes.NOT_FOUND, "Workspace not found."); } + const orgId = workspace.organizationId; const ownerId = workspace.ownerId; try { @@ -307,9 +301,10 @@ export class WorkspaceService { } public async getOpenPorts(userId: string, workspaceId: string): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const { workspace, latestInstance } = await this.getWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); + const instance = instanceNotFound(workspaceId, latestInstance); - const instance = await this.getCurrentInstance(userId, workspaceId); const req = new DescribeWorkspaceRequest(); req.setId(instance.id); const client = await this.clientProvider.get(instance.region); @@ -342,9 +337,10 @@ export class WorkspaceService { workspaceId: string, port: WorkspaceInstancePort, ): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const { workspace, latestInstance } = await this.getWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); + const instance = instanceNotFound(workspaceId, latestInstance); - const instance = await this.getCurrentInstance(userId, workspaceId); if (instance.status.phase !== "running") { log.debug({ userId, workspaceId }, "Cannot open port for workspace with no running instance", { port, @@ -370,9 +366,10 @@ export class WorkspaceService { } public async closePort(userId: string, workspaceId: string, port: number) { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const { workspace, latestInstance } = await this.getWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); + const instance = instanceNotFound(workspaceId, latestInstance); - const instance = await this.getCurrentInstance(userId, workspaceId); if (instance.status.phase !== "running") { log.debug({ userId, workspaceId }, "Cannot close a port for workspace with no running instance", { port, @@ -438,9 +435,9 @@ export class WorkspaceService { options: StartWorkspaceOptions = {}, restrictToRegular = true, ): Promise { - await this.auth.checkPermissionOnWorkspace(user.id, "start", workspaceId); - const { workspace, latestInstance } = await this.getWorkspace(user.id, workspaceId); + await this.auth.checkPermissionOnWorkspace(user.id, "start", workspace); + if (latestInstance) { if (latestInstance.status.phase !== "stopped") { // We already have a running workspace instance @@ -583,12 +580,15 @@ export class WorkspaceService { } public async setPinned(userId: string, workspaceId: string, pinned: boolean): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const workspace = await this.doGetWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); + await this.db.updatePartial(workspaceId, { pinned }); } public async setDescription(userId: string, workspaceId: string, description: string) { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const workspace = await this.doGetWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); await this.db.updatePartial(workspaceId, { description }); } @@ -597,17 +597,17 @@ export class WorkspaceService { workspaceId: string, gitStatus: Required | undefined, ) { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const { workspace, latestInstance } = await this.getWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); + const instance = instanceNotFound(workspaceId, latestInstance); - let instance = await this.getCurrentInstance(userId, workspaceId); if (WorkspaceInstanceRepoStatus.equals(instance.gitStatus, gitStatus)) { return; } - const workspace = await this.doGetWorkspace(userId, workspaceId); - instance = await this.db.updateInstancePartial(instance.id, { gitStatus }); + const updatedInstance = await this.db.updateInstancePartial(instance.id, { gitStatus }); await this.publisher.publishInstanceUpdate({ - instanceID: instance.id, + instanceID: updatedInstance.id, ownerID: workspace.ownerId, workspaceID: workspace.id, }); @@ -638,9 +638,9 @@ export class WorkspaceService { workspaceId: string, check: (instance: WorkspaceInstance, workspace: Workspace) => Promise = async () => {}, ): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); - const workspace = await this.doGetWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); + const canChange = await this.entitlementService.maySetTimeout(userId, workspace.organizationId); const instance = await this.db.findCurrentInstance(workspaceId); @@ -666,7 +666,8 @@ export class WorkspaceService { duration: WorkspaceTimeoutDuration, check: (instance: WorkspaceInstance, workspace: Workspace) => Promise = async () => {}, ): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const { workspace, latestInstance } = await this.getWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); let validatedDuration; try { @@ -675,12 +676,11 @@ export class WorkspaceService { throw new ApplicationError(ErrorCodes.INVALID_VALUE, "Invalid duration : " + err.message); } - const workspace = await this.doGetWorkspace(userId, workspaceId); if (!(await this.entitlementService.maySetTimeout(userId, workspace.organizationId))) { throw new ApplicationError(ErrorCodes.PLAN_PROFESSIONAL_REQUIRED, "Plan upgrade is required"); } - const instance = await this.getCurrentInstance(userId, workspaceId); + const instance = instanceNotFound(workspaceId, latestInstance); if (instance.status.phase !== "running" || workspace.type !== "regular") { throw new ApplicationError(ErrorCodes.NOT_FOUND, "Can only set keep-alive for regular, running workspaces"); } @@ -712,7 +712,7 @@ export class WorkspaceService { throw new ApplicationError(ErrorCodes.CONFLICT, `Workspace is not a prebuild`); } - await this.auth.checkPermissionOnProject(userId, "read_prebuild", workspace.projectId); + await this.auth.checkPermissionOnProject(userId, "read_prebuild", projectIdFrom(workspace)); const wsiPromise = this.db.findInstanceById(instanceId); await check(workspace); @@ -821,15 +821,11 @@ export class WorkspaceService { check: (instance: WorkspaceInstance, workspace: Workspace) => Promise = async () => {}, ): Promise { const instanceId = options.instanceId; - const instance = await this.db.findInstanceById(instanceId); - if (!instance) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, "workspace does not exist"); - } - const workspaceId = instance.workspaceId; - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); + const { workspace, latestInstance } = await this.getWorkspace(userId, instanceId); + const instance = instanceNotFound(workspace.id, latestInstance); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); try { - const workspace = await this.doGetWorkspace(userId, workspaceId); await check(instance, workspace); const wasClosed = !!(options && options.wasClosed); @@ -858,8 +854,6 @@ export class WorkspaceService { level: "owner" | "everyone", check: (workspace: Workspace, instance?: WorkspaceInstance) => Promise = async () => {}, ): Promise { - await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId); - const lvlmap = new Map(); lvlmap.set("owner", AdmissionLevel.ADMIT_OWNER_ONLY); lvlmap.set("everyone", AdmissionLevel.ADMIT_EVERYONE); @@ -868,6 +862,7 @@ export class WorkspaceService { } const workspace = await this.doGetWorkspace(userId, workspaceId); + await this.auth.checkPermissionOnWorkspace(userId, "access", workspace); await check(workspace); if (level !== "owner" && workspace.organizationId) { @@ -910,6 +905,20 @@ export class WorkspaceService { } } +/** + * Throws NOT_FOUND error in case WorkspaceInstance is undefined. + * TODO(gpl) make private after FGA rollout + * @param workspaceId + * @param instance + * @returns instance + */ +export function instanceNotFound(workspaceId: string, instance: WorkspaceInstance | undefined): WorkspaceInstance { + if (!instance) { + throw new ApplicationError(ErrorCodes.NOT_FOUND, "No workspace instance found.", { workspaceId }); + } + return instance; +} + // TODO(gpl) Make private after FGA rollout export function mapGrpcError(err: Error): Error { if (!isGrpcError(err)) {