From b1546ccd2e2adffdfe0879ada3ca66190a1bdd8d Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Wed, 8 Nov 2023 13:17:37 +0000 Subject: [PATCH] [server] Introduce RequestContext --- components/gitpod-protocol/src/analytics.ts | 4 +- .../gitpod-protocol/src/util/logging.ts | 1 + .../src/api/handler-context-augmentation.d.ts | 13 -- .../server/src/api/hello-service-api.ts | 18 +-- .../src/api/organization-service-api.ts | 63 ++++---- components/server/src/api/server.ts | 59 +++++--- .../server/src/api/workspace-service-api.ts | 5 +- components/server/src/auth/subject-id.ts | 96 ++++++++++++ .../caching-spicedb-authorizer.spec.db.ts | 77 ++++++---- .../src/authorization/spicedb-authorizer.ts | 5 +- components/server/src/container-module.ts | 8 +- components/server/src/jobs/runner.ts | 12 +- .../server/src/messaging/redis-subscriber.ts | 12 +- components/server/src/prebuilds/github-app.ts | 17 ++- .../src/prebuilds/github-enterprise-app.ts | 6 +- components/server/src/server.ts | 23 +-- components/server/src/util/log-context.ts | 29 ++-- components/server/src/util/request-context.ts | 138 +++++++++++++++--- .../websocket/websocket-connection-manager.ts | 27 ++-- .../src/workspace/gitpod-server-impl.ts | 9 +- .../server/src/workspace/workspace-starter.ts | 6 +- 21 files changed, 454 insertions(+), 174 deletions(-) delete mode 100644 components/server/src/api/handler-context-augmentation.d.ts create mode 100644 components/server/src/auth/subject-id.ts diff --git a/components/gitpod-protocol/src/analytics.ts b/components/gitpod-protocol/src/analytics.ts index 89cf67038f8814..12a6b9fc48426d 100644 --- a/components/gitpod-protocol/src/analytics.ts +++ b/components/gitpod-protocol/src/analytics.ts @@ -6,9 +6,7 @@ export const IAnalyticsWriter = Symbol("IAnalyticsWriter"); -type Identity = - | { userId: string | number; anonymousId?: string | number } - | { userId?: string | number; anonymousId: string | number }; +type Identity = { userId?: string | number; anonymousId?: string | number; subjectId?: string }; interface Message { messageId?: string; diff --git a/components/gitpod-protocol/src/util/logging.ts b/components/gitpod-protocol/src/util/logging.ts index 4542b703782d71..3ed477f1bc3a92 100644 --- a/components/gitpod-protocol/src/util/logging.ts +++ b/components/gitpod-protocol/src/util/logging.ts @@ -17,6 +17,7 @@ export interface LogContext { organizationId?: string; sessionId?: string; userId?: string; + subjectId?: string; workspaceId?: string; instanceId?: string; } diff --git a/components/server/src/api/handler-context-augmentation.d.ts b/components/server/src/api/handler-context-augmentation.d.ts deleted file mode 100644 index 8e3ea3532b98a5..00000000000000 --- a/components/server/src/api/handler-context-augmentation.d.ts +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Copyright (c) 2023 Gitpod GmbH. All rights reserved. - * Licensed under the GNU Affero General Public License (AGPL). - * See License.AGPL.txt in the project root for license information. - */ - -import { User } from "@gitpod/gitpod-protocol"; - -declare module "@connectrpc/connect" { - interface HandlerContext { - user: User; - } -} diff --git a/components/server/src/api/hello-service-api.ts b/components/server/src/api/hello-service-api.ts index c79623f702c9b6..fc419647298b77 100644 --- a/components/server/src/api/hello-service-api.ts +++ b/components/server/src/api/hello-service-api.ts @@ -5,7 +5,6 @@ */ import { HandlerContext, ServiceImpl } from "@connectrpc/connect"; -import { User } from "@gitpod/gitpod-protocol"; import { HelloService } from "@gitpod/public-api/lib/gitpod/experimental/v1/dummy_connect"; import { LotsOfRepliesRequest, @@ -14,27 +13,28 @@ import { SayHelloResponse, } from "@gitpod/public-api/lib/gitpod/experimental/v1/dummy_pb"; import { injectable } from "inversify"; +import { ctx } from "../util/request-context"; @injectable() export class HelloServiceAPI implements ServiceImpl { - async sayHello(req: SayHelloRequest, context: HandlerContext): Promise { + async sayHello(req: SayHelloRequest, _: HandlerContext): Promise { const response = new SayHelloResponse(); - response.reply = "Hello " + this.getSubject(context); + response.reply = "Hello " + getSubject(); return response; } - async *lotsOfReplies(req: LotsOfRepliesRequest, context: HandlerContext): AsyncGenerator { + async *lotsOfReplies(req: LotsOfRepliesRequest, _: HandlerContext): AsyncGenerator { let count = req.previousCount || 0; - while (!context.signal.aborted) { + while (!ctx().signal.aborted) { const response = new LotsOfRepliesResponse(); - response.reply = `Hello ${this.getSubject(context)} ${count}`; + response.reply = `Hello ${getSubject()} ${count}`; response.count = count; yield response; count++; await new Promise((resolve) => setTimeout(resolve, 30000)); } } +} - private getSubject(context: HandlerContext): string { - return User.getName(context.user) || "World"; - } +function getSubject(): string { + return ctx().subjectId?.toString() || "World"; } diff --git a/components/server/src/api/organization-service-api.ts b/components/server/src/api/organization-service-api.ts index 35ecb1d7743da3..263ab891d086eb 100644 --- a/components/server/src/api/organization-service-api.ts +++ b/components/server/src/api/organization-service-api.ts @@ -39,6 +39,8 @@ import { import { PublicAPIConverter } from "@gitpod/gitpod-protocol/lib/public-api-converter"; import { OrganizationService } from "../orgs/organization-service"; import { PaginationResponse } from "@gitpod/public-api/lib/gitpod/v1/pagination_pb"; +import { ctx, userId } from "../util/request-context"; +import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; @injectable() export class OrganizationServiceAPI implements ServiceImpl { @@ -49,18 +51,20 @@ export class OrganizationServiceAPI implements ServiceImpl { - const org = await this.orgService.createOrganization(context.user.id, req.name); + async createOrganization(req: CreateOrganizationRequest, _: HandlerContext): Promise { + // TODO(gpl) This mimicks the current behavior of adding the subjectId as owner + const ownerId = ctx().subjectId?.userId(); + if (!ownerId) { + throw new ApplicationError(ErrorCodes.BAD_REQUEST, "No userId available"); + } + const org = await this.orgService.createOrganization(ownerId, req.name); const response = new CreateOrganizationResponse(); response.organization = this.apiConverter.toOrganization(org); return response; } - async getOrganization(req: GetOrganizationRequest, context: HandlerContext): Promise { - const org = await this.orgService.getOrganization(context.user.id, req.organizationId); + async getOrganization(req: GetOrganizationRequest, _: HandlerContext): Promise { + const org = await this.orgService.getOrganization(userId(), req.organizationId); const response = new GetOrganizationResponse(); response.organization = this.apiConverter.toOrganization(org); return response; @@ -70,7 +74,7 @@ export class OrganizationServiceAPI implements ServiceImpl { - const org = await this.orgService.updateOrganization(context.user.id, req.organizationId, { + const org = await this.orgService.updateOrganization(userId(), req.organizationId, { name: req.name, }); return new UpdateOrganizationResponse({ @@ -83,7 +87,7 @@ export class OrganizationServiceAPI implements ServiceImpl { const orgs = await this.orgService.listOrganizations( - context.user.id, + userId(), { limit: req.pagination?.pageSize || 100, offset: (req.pagination?.page || 0) * (req.pagination?.pageSize || 0), @@ -97,26 +101,23 @@ export class OrganizationServiceAPI implements ServiceImpl { - await this.orgService.deleteOrganization(context.user.id, req.organizationId); + async deleteOrganization(req: DeleteOrganizationRequest, _: HandlerContext): Promise { + await this.orgService.deleteOrganization(userId(), req.organizationId); return new DeleteOrganizationResponse(); } async getOrganizationInvitation( req: GetOrganizationInvitationRequest, - context: HandlerContext, + _: HandlerContext, ): Promise { - const invitation = await this.orgService.getOrCreateInvite(context.user.id, req.organizationId); + const invitation = await this.orgService.getOrCreateInvite(userId(), req.organizationId); const response = new GetOrganizationInvitationResponse(); response.invitationId = invitation.id; return response; } - async joinOrganization(req: JoinOrganizationRequest, context: HandlerContext): Promise { - const orgId = await this.orgService.joinOrganization(context.user.id, req.invitationId); + async joinOrganization(req: JoinOrganizationRequest, _: HandlerContext): Promise { + const orgId = await this.orgService.joinOrganization(userId(), req.invitationId); const result = new JoinOrganizationResponse(); result.organizationId = orgId; return result; @@ -124,9 +125,9 @@ export class OrganizationServiceAPI implements ServiceImpl { - const inviteId = await this.orgService.resetInvite(context.user.id, req.organizationId); + const inviteId = await this.orgService.resetInvite(userId(), req.organizationId); const result = new ResetOrganizationInvitationResponse(); result.invitationId = inviteId.id; return result; @@ -134,9 +135,9 @@ export class OrganizationServiceAPI implements ServiceImpl { - const members = await this.orgService.listMembers(context.user.id, req.organizationId); + const members = await this.orgService.listMembers(userId(), req.organizationId); //TODO pagination const response = new ListOrganizationMembersResponse(); response.members = members.map((member) => this.apiConverter.toOrganizationMember(member)); @@ -147,16 +148,16 @@ export class OrganizationServiceAPI implements ServiceImpl { await this.orgService.addOrUpdateMember( - context.user.id, + userId(), req.organizationId, req.userId, this.apiConverter.fromOrgMemberRole(req.role), ); const member = await this.orgService - .listMembers(context.user.id, req.organizationId) + .listMembers(userId(), req.organizationId) .then((members) => members.find((member) => member.userId === req.userId)); return new UpdateOrganizationMemberResponse({ member: member && this.apiConverter.toOrganizationMember(member), @@ -165,17 +166,17 @@ export class OrganizationServiceAPI implements ServiceImpl { - await this.orgService.removeOrganizationMember(context.user.id, req.organizationId, req.userId); + await this.orgService.removeOrganizationMember(userId(), req.organizationId, req.userId); return new DeleteOrganizationMemberResponse(); } async getOrganizationSettings( req: GetOrganizationSettingsRequest, - context: HandlerContext, + _: HandlerContext, ): Promise { - const settings = await this.orgService.getSettings(context.user.id, req.organizationId); + const settings = await this.orgService.getSettings(userId(), req.organizationId); const response = new GetOrganizationSettingsResponse(); response.settings = this.apiConverter.toOrganizationSettings(settings); return response; @@ -183,9 +184,9 @@ export class OrganizationServiceAPI implements ServiceImpl { - const settings = await this.orgService.updateSettings(context.user.id, req.organizationId, { + const settings = await this.orgService.updateSettings(userId(), req.organizationId, { workspaceSharingDisabled: req.settings?.workspaceSharingDisabled, defaultWorkspaceImage: req.settings?.defaultWorkspaceImage, }); diff --git a/components/server/src/api/server.ts b/components/server/src/api/server.ts index a155039ade3179..7e216f30f6561d 100644 --- a/components/server/src/api/server.ts +++ b/components/server/src/api/server.ts @@ -30,8 +30,12 @@ import { Config } from "../config"; import { grpcServerHandled, grpcServerHandling, grpcServerStarted } from "../prometheus-metrics"; import { SessionHandler } from "../session-handler"; import { UserService } from "../user/user-service"; -import { LogContextOptions, runWithLogContext } from "../util/log-context"; -import { wrapAsyncGenerator } from "../util/request-context"; +import { + RequestContext, + runWithChildContext, + runWithRequestContext, + wrapAsyncGenerator, +} from "../util/request-context"; import { HelloServiceAPI } from "./hello-service-api"; import { OrganizationServiceAPI } from "./organization-service-api"; import { RateLimited } from "./rate-limited"; @@ -39,6 +43,7 @@ import { APIStatsService as StatsServiceAPI } from "./stats"; import { APITeamsService as TeamsServiceAPI } from "./teams"; import { APIUserService as UserServiceAPI } from "./user"; import { WorkspaceServiceAPI } from "./workspace-service-api"; +import { SubjectId } from "../auth/subject-id"; decorate(injectable(), PublicAPIConverter); @@ -127,17 +132,16 @@ export class API { return { get(target, prop) { return (...args: any[]) => { - const logContext: LogContextOptions & { - requestId?: string; - contextTimeMs: number; - grpc_service: string; - grpc_method: string; - } = { - contextTimeMs: performance.now(), - grpc_service, - grpc_method: prop as string, + const connectContext = args[1] as HandlerContext; + const requestContext: RequestContext = { + requestId: v4(), + requestKind: "public-api", + requestMethod: `${grpc_service}/${prop as string}`, + startTime: performance.now(), + signal: connectContext.signal, }; - const withRequestContext = (fn: () => T): T => runWithLogContext("public-api", logContext, fn); + + const withRequestContext = (fn: () => T): T => runWithRequestContext(requestContext, fn); const method = type.methods[prop as string]; if (!method) { @@ -161,8 +165,6 @@ export class API { grpc_type = "bidi_stream"; } - logContext.requestId = v4(); - grpcServerStarted.labels(grpc_service, grpc_method, grpc_type).inc(); const stopTimer = grpcServerHandling.startTimer({ grpc_service, grpc_method, grpc_type }); const done = (err?: ConnectError) => { @@ -176,7 +178,7 @@ export class API { if (reason != err && err.code === Code.Internal) { log.error("public api: unexpected internal error", reason); err = new ConnectError( - `Oops! Something went wrong. Please quote the request ID ${logContext.requestId} when reaching out to Gitpod Support.`, + `Oops! Something went wrong. Please quote the request ID ${requestContext.requestId} when reaching out to Gitpod Support.`, Code.Internal, // pass metadata to preserve the application error err.metadata, @@ -186,8 +188,6 @@ export class API { throw err; }; - const context = args[1] as HandlerContext; - const rateLimit = async (subjectId: string) => { const key = `${grpc_service}/${grpc_method}`; const options = self.config.rateLimits?.[key] || RateLimited.getOptions(target, prop); @@ -208,18 +208,25 @@ export class API { } }; - const apply = async (): Promise => { - const subjectId = await self.verify(context); - await rateLimit(subjectId); - context.user = await self.ensureFgaMigration(subjectId); + const auth = async () => { + const userId = await self.verify(connectContext); + await rateLimit(userId); + await self.ensureFgaMigration(userId); + + return SubjectId.fromUserId(userId); + }; + const apply = async (): Promise => { return Reflect.apply(target[prop as any], target, args); }; if (grpc_type === "unary" || grpc_type === "client_stream") { return withRequestContext(async () => { try { - const promise = await apply>(); - const result = await promise; + const subjectId = await auth(); + const result = await runWithChildContext({ subjectId }, async () => { + const promise = await apply>(); + return await promise; + }); done(); return result; } catch (e) { @@ -227,9 +234,13 @@ export class API { } }); } + + let subjectId: SubjectId | undefined = undefined; return wrapAsyncGenerator( (async function* () { try { + subjectId = await auth(); + const generator = await apply>(); for await (const item of generator) { yield item; @@ -239,7 +250,7 @@ export class API { handleError(e); } })(), - withRequestContext, + (f) => runWithChildContext({ subjectId }, f), ); }; }, diff --git a/components/server/src/api/workspace-service-api.ts b/components/server/src/api/workspace-service-api.ts index dbfed1576cdaf7..6729e830dac4d1 100644 --- a/components/server/src/api/workspace-service-api.ts +++ b/components/server/src/api/workspace-service-api.ts @@ -10,6 +10,7 @@ import { GetWorkspaceRequest, GetWorkspaceResponse } from "@gitpod/public-api/li import { inject, injectable } from "inversify"; import { WorkspaceService } from "../workspace/workspace-service"; import { PublicAPIConverter } from "@gitpod/gitpod-protocol/lib/public-api-converter"; +import { userId } from "../util/request-context"; @injectable() export class WorkspaceServiceAPI implements ServiceImpl { @@ -19,8 +20,8 @@ export class WorkspaceServiceAPI implements ServiceImpl { - const info = await this.workspaceService.getWorkspace(context.user.id, req.id); + async getWorkspace(req: GetWorkspaceRequest, _: HandlerContext): Promise { + const info = await this.workspaceService.getWorkspace(userId(), req.id); const response = new GetWorkspaceResponse(); response.item = this.apiConverter.toWorkspace(info); return response; diff --git a/components/server/src/auth/subject-id.ts b/components/server/src/auth/subject-id.ts new file mode 100644 index 00000000000000..8b32d29b430130 --- /dev/null +++ b/components/server/src/auth/subject-id.ts @@ -0,0 +1,96 @@ +/** + * Copyright (c) 2023 Gitpod GmbH. All rights reserved. + * Licensed under the GNU Affero General Public License (AGPL). + * See License.AGPL.txt in the project root for license information. + */ + +type ISubjectId = { + kind: SubjectKind; + /** + * The value of the subject id, _without_ the prefix. + */ + value: string; +}; +export type SubjectKind = keyof typeof SubjectKindNames; +const SubjectKindNames = { + user: "user", +}; +const SubjectKindByShortName: ReadonlyMap = new Map( + Object.keys(SubjectKindNames).map((k) => { + return [SubjectKindNames[k as SubjectKind], k as SubjectKind]; + }), +); + +export class SubjectId implements ISubjectId { + private static readonly SEPARATOR = "_"; + + constructor(public readonly kind: SubjectKind, public readonly value: string) {} + + public static fromUserId(userId: string): SubjectId { + return new SubjectId("user", userId); + } + + public static is(obj: any): obj is SubjectId { + return !!obj && obj instanceof SubjectId; + } + + public static isSubjectKind(str: string): str is SubjectKind { + return !!SubjectKindNames[str as SubjectKind]; + } + + public toString(): string { + const prefix = SubjectKindNames[this.kind]; + return prefix + SubjectId.SEPARATOR + this.value; + } + + public static parse(str: string): SubjectId | undefined { + try { + return SubjectId.tryParse(str); + } catch (err) { + return undefined; + } + } + + public static tryParse(str: string): SubjectId { + const parts = str.split(SubjectId.SEPARATOR); + if (parts.length < 2) { + throw new Error(`Unable to parse SubjectId`); + } + const kind = SubjectKindByShortName.get(parts[0]); + if (!kind) { + throw new Error(`Unable to parse SubjectId: unknown SubjectKind!`); + } + const value = parts.slice(1).join(); + return new SubjectId(kind, value); + } + + public userId(): string | undefined { + if (this.kind === "user") { + return this.value; + } + return undefined; + } + + public equals(other: SubjectId): boolean { + return this.kind === other.kind && this.value === other.value; + } +} + +// The following codes is meant for backwards-compatibility with the existing express types, or other code, that relies on `userId: string` or `User` +/** + * Interface type meant for backwards compatibility + */ +export type Subject = string | SubjectId; +export namespace Subject { + export function toId(subject: Subject): SubjectId { + if (SubjectId.is(subject)) { + return subject; + } + if (typeof subject === "string") { + // either a subjectId string or a userId string + const parsed = SubjectId.parse(subject); + return parsed || SubjectId.fromUserId(subject); + } + throw new Error("Invalid Subject"); + } +} diff --git a/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts b/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts index 30f363a5eda208..24ac3571058b94 100644 --- a/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts +++ b/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts @@ -17,12 +17,27 @@ import { WorkspaceService } from "../workspace/workspace-service"; import { UserService } from "../user/user-service"; import { ConfigProvider } from "../workspace/config-provider"; import { v1 } from "@authzed/authzed-node"; -import { runWithContext } from "../util/request-context"; +import { runWithRequestContext } from "../util/request-context"; import { RequestLocalZedTokenCache } from "./spicedb-authorizer"; +import { Subject } from "../auth/subject-id"; const expect = chai.expect; -const withCtx = (p: Promise) => runWithContext("test", {}, () => p); +const withCtx = (subject: Subject | User, p: Promise | (() => Promise)) => + runWithRequestContext( + { + requestKind: "testContext", + requestMethod: "testMethod", + signal: new AbortController().signal, + subjectId: Subject.toId(User.is(subject) ? subject.id : subject), + }, + () => { + if (typeof p === "function") { + return p(); + } + return p; + }, + ); describe("CachingSpiceDBAuthorizer", async () => { let container: Container; @@ -60,8 +75,8 @@ describe("CachingSpiceDBAuthorizer", async () => { it("should avoid new-enemy after removal", async () => { // userB and userC are members of org1, userA is owner. // All users are installation owned. - const org1 = await withCtx(orgSvc.createOrganization(SYSTEM_USER, "org1")); const userA = await withCtx( + SYSTEM_USER, userSvc.createUser({ organizationId: undefined, identity: { @@ -71,8 +86,10 @@ describe("CachingSpiceDBAuthorizer", async () => { }, }), ); - await withCtx(orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userA.id, "owner")); + const org1 = await withCtx(userA, orgSvc.createOrganization(userA.id, "org1")); + await withCtx(SYSTEM_USER, orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userA.id, "owner")); const userB = await withCtx( + SYSTEM_USER, userSvc.createUser({ organizationId: undefined, identity: { @@ -82,8 +99,9 @@ describe("CachingSpiceDBAuthorizer", async () => { }, }), ); - await withCtx(orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userB.id, "member")); + await withCtx(SYSTEM_USER, orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userB.id, "member")); const userC = await withCtx( + SYSTEM_USER, userSvc.createUser({ organizationId: undefined, identity: { @@ -93,38 +111,38 @@ describe("CachingSpiceDBAuthorizer", async () => { }, }), ); - await withCtx(orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userC.id, "member")); + await withCtx(SYSTEM_USER, orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userC.id, "member")); // userA creates a workspace when userB is still member of the org // All members have "read_info" (derived from membership) - const ws1 = await withCtx(createTestWorkspace(org1, userA)); + const ws1 = await withCtx(userA, createTestWorkspace(org1, userA)); expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id)), + await withCtx(userB, authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id)), "userB should have read_info after removal", ).to.be.true; expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), + await withCtx(userA, authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), "userA should have read_info after removal of userB", ).to.be.true; expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), + await withCtx(userC, authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), "userC should have read_info after removal of userB", ).to.be.true; // userB is removed from the org - await withCtx(orgSvc.removeOrganizationMember(SYSTEM_USER, org1.id, userB.id)); + await withCtx(SYSTEM_USER, orgSvc.removeOrganizationMember(SYSTEM_USER, org1.id, userB.id)); expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id)), + await withCtx(userB, authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id)), "userB should have read_info after removal", ).to.be.false; expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), + await withCtx(userA, authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), "userA should have read_info after removal of userB", ).to.be.true; expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), + await withCtx(userC, authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), "userC should have read_info after removal of userB", ).to.be.true; }); @@ -153,8 +171,8 @@ describe("CachingSpiceDBAuthorizer", async () => { it("should avoid read-your-writes problem when adding a new user", async () => { // userB and userC are members of org1, userA is owner. // All users are installation owned. - const org1 = await withCtx(orgSvc.createOrganization(SYSTEM_USER, "org1")); const userA = await withCtx( + SYSTEM_USER, userSvc.createUser({ organizationId: undefined, identity: { @@ -164,8 +182,10 @@ describe("CachingSpiceDBAuthorizer", async () => { }, }), ); - await withCtx(orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userA.id, "owner")); + const org1 = await withCtx(userA, orgSvc.createOrganization(userA.id, "org1")); + await withCtx(SYSTEM_USER, orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userA.id, "owner")); const userC = await withCtx( + SYSTEM_USER, userSvc.createUser({ organizationId: undefined, identity: { @@ -175,22 +195,23 @@ describe("CachingSpiceDBAuthorizer", async () => { }, }), ); - await withCtx(orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userC.id, "member")); + await withCtx(SYSTEM_USER, orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userC.id, "member")); // userA creates a workspace before userB is member of the org - const ws1 = await withCtx(createTestWorkspace(org1, userA)); + const ws1 = await withCtx(userA, createTestWorkspace(org1, userA)); expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), + await withCtx(SYSTEM_USER, authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), "userA should have read_info after removal of userB", ).to.be.true; expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), + await withCtx(userC, authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), "userC should have read_info after removal of userB", ).to.be.true; // userB is added to the org const userB = await withCtx( + SYSTEM_USER, userSvc.createUser({ organizationId: undefined, identity: { @@ -200,18 +221,18 @@ describe("CachingSpiceDBAuthorizer", async () => { }, }), ); - await withCtx(orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userB.id, "member")); + await withCtx(SYSTEM_USER, orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userB.id, "member")); expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id)), + await withCtx(userB, authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id)), "userB should have read_info after removal", ).to.be.true; expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), + await withCtx(userA, authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id)), "userA should have read_info after removal of userB", ).to.be.true; expect( - await withCtx(authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), + await withCtx(userC, authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id)), "userC should have read_info after removal of userB", ).to.be.true; }); @@ -253,7 +274,7 @@ describe("RequestLocalZedTokenCache", async () => { }); it("should store token", async () => { - await runWithContext("test", {}, async () => { + await withCtx(SYSTEM_USER, async () => { expect(await cache.get(ws1)).to.be.undefined; await cache.set([ws1, rawToken1]); expect(await cache.get(ws1)).to.equal(rawToken1); @@ -261,7 +282,7 @@ describe("RequestLocalZedTokenCache", async () => { }); it("should return newest token", async () => { - await runWithContext("test", {}, async () => { + await withCtx(SYSTEM_USER, async () => { await cache.set([ws1, rawToken1]); await cache.set([ws1, rawToken2]); expect(await cache.get(ws1)).to.equal(rawToken2); @@ -271,7 +292,7 @@ describe("RequestLocalZedTokenCache", async () => { }); it("should return proper consistency", async () => { - await runWithContext("test", {}, async () => { + await withCtx(SYSTEM_USER, async () => { expect(await cache.consistency(ws1)).to.deep.equal(fullyConsistent()); await cache.set([ws1, rawToken1]); expect(await cache.consistency(ws1)).to.deep.equal(atLeastAsFreshAs(rawToken1)); @@ -279,7 +300,7 @@ describe("RequestLocalZedTokenCache", async () => { }); it("should clear cache", async () => { - await runWithContext("test", {}, async () => { + await withCtx(SYSTEM_USER, async () => { await cache.set([ws1, rawToken1]); expect(await cache.get(ws1)).to.equal(rawToken1); await cache.set([ws1, undefined]); // this should trigger a clear diff --git a/components/server/src/authorization/spicedb-authorizer.ts b/components/server/src/authorization/spicedb-authorizer.ts index 6c45d9efa7ac76..4612dc1e600f91 100644 --- a/components/server/src/authorization/spicedb-authorizer.ts +++ b/components/server/src/authorization/spicedb-authorizer.ts @@ -14,8 +14,7 @@ import * as grpc from "@grpc/grpc-js"; import { isFgaChecksEnabled, isFgaWritesEnabled } from "./authorizer"; import { base64decode } from "@jmondi/oauth2-server"; import { DecodedZedToken } from "@gitpod/spicedb-impl/lib/impl/v1/impl.pb"; -import { RequestContext } from "node-fetch"; -import { getRequestContext } from "../util/request-context"; +import { RequestContext, tryCtx } from "../util/request-context"; import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; async function tryThree(errMessage: string, code: (attempt: number) => Promise): Promise { @@ -242,7 +241,7 @@ interface ZedTokenCache { type ContextWithZedToken = RequestContext & { zedToken?: StoredZedToken }; function getContext(): ContextWithZedToken { - return getRequestContext() as ContextWithZedToken; + return (tryCtx() || {}) as ContextWithZedToken; } /** diff --git a/components/server/src/container-module.ts b/components/server/src/container-module.ts index 230c5a35aa5c0d..67f7e37cea11a4 100644 --- a/components/server/src/container-module.ts +++ b/components/server/src/container-module.ts @@ -129,6 +129,7 @@ import { WorkspaceFactory } from "./workspace/workspace-factory"; import { WorkspaceService } from "./workspace/workspace-service"; import { WorkspaceStartController } from "./workspace/workspace-start-controller"; import { WorkspaceStarter } from "./workspace/workspace-starter"; +import { ContextAwareAnalyticsWriter } from "./util/request-context"; export const productionContainerModule = new ContainerModule( (bind, unbind, isBound, rebind, unbindAsync, onActivation, onDeactivation) => { @@ -248,7 +249,12 @@ export const productionContainerModule = new ContainerModule( bind(CodeSyncService).toSelf().inSingletonScope(); - bind(IAnalyticsWriter).toDynamicValue(newAnalyticsWriterFromEnv).inSingletonScope(); + bind(IAnalyticsWriter) + .toDynamicValue((ctx) => { + const writer = newAnalyticsWriterFromEnv(); + return new ContextAwareAnalyticsWriter(writer); + }) + .inSingletonScope(); bind(OAuthController).toSelf().inSingletonScope(); diff --git a/components/server/src/jobs/runner.ts b/components/server/src/jobs/runner.ts index c1b08614c1839e..cc6c80577f6c57 100644 --- a/components/server/src/jobs/runner.ts +++ b/components/server/src/jobs/runner.ts @@ -18,7 +18,9 @@ import { WorkspaceGarbageCollector } from "./workspace-gc"; import { SnapshotsJob } from "./snapshots"; import { RelationshipUpdateJob } from "../authorization/relationship-updater-job"; import { WorkspaceStartController } from "../workspace/workspace-start-controller"; -import { runWithContext } from "../util/request-context"; +import { runWithRequestContext } from "../util/request-context"; +import { SYSTEM_USER } from "../authorization/authorizer"; +import { SubjectId } from "../auth/subject-id"; export const Job = Symbol("Job"); @@ -81,7 +83,13 @@ export class JobRunner { try { await this.mutex.using([job.name, ...(job.lockedResources || [])], job.frequencyMs, async (signal) => { - await runWithContext(job.name, {}, async () => { + const ctx = { + signal, + requestKind: "job", + requestMethod: job.name, + subjectId: SubjectId.fromUserId(SYSTEM_USER), + }; + await runWithRequestContext(ctx, async () => { log.info(`Acquired lock for job ${job.name}.`, logCtx); // we want to hold the lock for the entire duration of the job, so we return earliest after frequencyMs const timeout = new Promise((resolve) => setTimeout(resolve, job.frequencyMs)); diff --git a/components/server/src/messaging/redis-subscriber.ts b/components/server/src/messaging/redis-subscriber.ts index 580e3286b99f7b..d2a780aa9f2fd6 100644 --- a/components/server/src/messaging/redis-subscriber.ts +++ b/components/server/src/messaging/redis-subscriber.ts @@ -29,7 +29,9 @@ import { } from "../prometheus-metrics"; import { Redis } from "ioredis"; import { WorkspaceDB } from "@gitpod/gitpod-db/lib"; -import { runWithContext } from "../util/request-context"; +import { runWithRequestContext } from "../util/request-context"; +import { SYSTEM_USER } from "../authorization/authorizer"; +import { SubjectId } from "../auth/subject-id"; const UNDEFINED_KEY = "undefined"; @@ -55,7 +57,13 @@ export class RedisSubscriber { } this.redis.on("message", async (channel: string, message: string) => { - await runWithContext("redis-subscriber", {}, async () => { + const ctx = { + signal: new AbortSignal(), + requestKind: "redis-subscriber", + requestMethod: channel, + subjectId: SubjectId.fromUserId(SYSTEM_USER), + }; + await runWithRequestContext(ctx, async () => { reportRedisUpdateReceived(channel); let err: Error | undefined; diff --git a/components/server/src/prebuilds/github-app.ts b/components/server/src/prebuilds/github-app.ts index ecdee252bfdea1..72421fafb6191c 100644 --- a/components/server/src/prebuilds/github-app.ts +++ b/components/server/src/prebuilds/github-app.ts @@ -41,7 +41,7 @@ import { RepoURL } from "../repohost"; import { ApplicationError, ErrorCode } from "@gitpod/gitpod-protocol/lib/messaging/error"; import { UserService } from "../user/user-service"; import { ProjectsService } from "../projects/projects-service"; -import { SYSTEM_USER } from "../authorization/authorizer"; +import { runWithRequestContext } from "../util/request-context"; /** * GitHub app urls: @@ -112,6 +112,21 @@ export class GithubApp { res.redirect(301, this.getBadgeImageURL()); }); + // Use RequestContext + options.getRouter && + options.getRouter().use((req, res, next) => { + runWithRequestContext( + { + requestKind: "probot", + requestMethod: req.path, + signal: new AbortController().signal, + subjectId: undefined, // these endpoints are guarded by a webhooksecret; still I don't think it's a good idea to use SYSTEM_USER here + }, + () => next(), + ); + next(); + }); + app.on("installation.created", (ctx: Context<"installation.created">) => { catchError( (async () => { diff --git a/components/server/src/prebuilds/github-enterprise-app.ts b/components/server/src/prebuilds/github-enterprise-app.ts index 9b185b0d224e4f..e34a51b2f900af 100644 --- a/components/server/src/prebuilds/github-enterprise-app.ts +++ b/components/server/src/prebuilds/github-enterprise-app.ts @@ -23,6 +23,8 @@ import { UserService } from "../user/user-service"; import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; import { ProjectsService } from "../projects/projects-service"; import { SYSTEM_USER } from "../authorization/authorizer"; +import { runWithChildContext } from "../util/request-context"; +import { SubjectId } from "../auth/subject-id"; @injectable() export class GitHubEnterpriseApp { @@ -258,7 +260,9 @@ export class GitHubEnterpriseApp { private async findProjectOwners(cloneURL: string): Promise<{ users: User[]; project: Project } | undefined> { try { - const projects = await this.projectService.findProjectsByCloneUrl(SYSTEM_USER, cloneURL); + const projects = await runWithChildContext({ subjectId: SubjectId.fromUserId(SYSTEM_USER) }, async () => + this.projectService.findProjectsByCloneUrl(SYSTEM_USER, cloneURL), + ); const project = projects[0]; if (project) { const users = []; diff --git a/components/server/src/server.ts b/components/server/src/server.ts index 1e360a3777ab16..bd5f5e4d543a73 100644 --- a/components/server/src/server.ts +++ b/components/server/src/server.ts @@ -47,7 +47,8 @@ import { GitHubEnterpriseApp } from "./prebuilds/github-enterprise-app"; import { JobRunner } from "./jobs/runner"; import { RedisSubscriber } from "./messaging/redis-subscriber"; import { HEADLESS_LOGS_PATH_PREFIX, HEADLESS_LOG_DOWNLOAD_PATH_PREFIX } from "./workspace/headless-log-service"; -import { runWithLogContext } from "./util/log-context"; +import { runWithRequestContext } from "./util/request-context"; +import { SubjectId } from "./auth/subject-id"; @injectable() export class Server { @@ -140,14 +141,18 @@ export class Server { // Install passport await this.authenticator.init(app); - // log context - app.use(async (req: express.Request, res: express.Response, next: express.NextFunction) => { - try { - const userId = req.user ? req.user.id : undefined; - runWithLogContext("http", { userId, requestPath: req.path }, () => next()); - } catch (err) { - next(err); - } + // Use RequestContext for authorization + app.use((req: express.Request, res: express.Response, next: express.NextFunction) => { + const userId = req.user ? req.user.id : undefined; + runWithRequestContext( + { + requestKind: "http", + requestMethod: req.path, + signal: new AbortController().signal, + subjectId: userId ? SubjectId.fromUserId(userId) : undefined, // TODO(gpl) Can we assume this? E.g., has this been verified? It should: It means we could decode the cookie, right? + }, + () => next(), + ); }); // Ensure that host contexts of dynamic auth providers are initialized. diff --git a/components/server/src/util/log-context.ts b/components/server/src/util/log-context.ts index 1c5b3fbf709ced..73eb3f1d27e314 100644 --- a/components/server/src/util/log-context.ts +++ b/components/server/src/util/log-context.ts @@ -6,28 +6,37 @@ import { LogContext } from "@gitpod/gitpod-protocol/lib/util/logging"; import { performance } from "node:perf_hooks"; -import { RequestContext, getGlobalContext, runWithContext } from "./request-context"; +import { RequestContext, tryCtx } from "./request-context"; export type LogContextOptions = LogContext & { [p: string]: any; }; +function mapToLogContext(ctx: RequestContext): LogContextOptions { + return { + requestId: ctx.requestId, + requestKind: ctx.requestKind, + requestMethod: ctx.requestMethod, + traceId: ctx.traceId, + subjectId: ctx.subjectId?.toString(), + userId: ctx.subjectId?.userId(), + contextTimeMs: ctx?.startTime ? performance.now() - ctx.startTime : undefined, + }; +} + // we are installing a special augmenter that enhances the log context if executed within `runWithContext` // with a contextId and a contextTimeMs, which denotes the amount of milliseconds since the context was created. -export type EnhancedLogContext = RequestContext & LogContextOptions; const augmenter: LogContext.Augmenter = (ctx) => { - const globalContext = getGlobalContext(); - const contextTimeMs = globalContext?.contextTimeMs ? performance.now() - globalContext.contextTimeMs : undefined; + const requestContext = tryCtx(); + let derivedContext: LogContextOptions = {}; + if (requestContext) { + derivedContext = mapToLogContext(requestContext); + } const result = { - ...globalContext, - contextTimeMs, + ...derivedContext, ...ctx, }; // if its an empty object return undefined return Object.keys(result).length === 0 ? undefined : result; }; LogContext.setAugmenter(augmenter); - -export function runWithLogContext(contextKind: string, context: EnhancedLogContext, fun: () => T): T { - return runWithContext(contextKind, context, fun); -} diff --git a/components/server/src/util/request-context.ts b/components/server/src/util/request-context.ts index e762890f9ffbb7..6eae1172772437 100644 --- a/components/server/src/util/request-context.ts +++ b/components/server/src/util/request-context.ts @@ -7,32 +7,98 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { performance } from "node:perf_hooks"; import { v4 } from "uuid"; +import { SubjectId } from "../auth/subject-id"; +import { IAnalyticsWriter, IdentifyMessage, PageMessage, TrackMessage } from "@gitpod/gitpod-protocol/lib/analytics"; +import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; export interface RequestContext { - contextId?: string; - contextKind?: string; - contextTimeMs?: number; + /** + * Unique, artificial ID for this request. + */ + readonly requestId: string; + + /** + * A request kind e.g. "job", "http" or "grpc". + */ + readonly requestKind: string; + + /** + * A name. Specific values depends on requestKind. + */ + readonly requestMethod: string; + + /** + * Propagate cancellation through the handler chain. + */ + readonly signal: AbortSignal; + + /** + * The UNIX timestamp in milliseconds when request processing started. + */ + readonly startTime: number; + + /** + * The SubjectId this request is authenticated with. + */ + readonly subjectId?: SubjectId; + + /** + * The trace ID for this request. This is used to correlate log messages and other events. + */ + readonly traceId?: string; } const asyncLocalStorage = new AsyncLocalStorage(); + +export function ctx(): RequestContext { + const ctx = asyncLocalStorage.getStore(); + if (!ctx) { + throw new Error("getRequestContext: No request context available"); + } + return ctx; +} + +export function tryCtx(): RequestContext | undefined { + return asyncLocalStorage.getStore() || undefined; +} + /** - * !!! Only to be used by selected internal code !!! + * @deprecated Only used during the rollout period. Use `getSubjectId` instead + */ +export function tryGetSubjectId(): SubjectId | undefined { + const ctx = tryCtx(); + return ctx?.subjectId; +} + +/** + * The context all our request-handling code should run in. + * By default, all fields are inhereted from the parent context. Only exceptions: `requestId` and `contextId`. + * @param subjectId If this undefined, the request is considered unauthorized + * @param contextKind + * @param context + * @param fun * @returns */ -export function getGlobalContext(): RequestContext | undefined { - return asyncLocalStorage.getStore(); +export function runWithRequestContext( + context: Omit & { requestId?: string; startTime?: number }, + fun: () => T, +): T { + const requestId = context.requestId || v4(); + const startTime = context.startTime || performance.now(); + return runWithContext({ ...context, requestId, startTime }, fun); } -export function runWithContext(contextKind: string, context: C, fun: () => T): T { - return asyncLocalStorage.run( - { - ...context, - contextKind, - contextId: context.contextId || v4(), - contextTimeMs: context.contextTimeMs || performance.now(), - }, - fun, - ); +export function runWithChildContext(child: Pick, fun: () => T): T { + const parent = ctx(); + if (!parent) { + throw new Error("runWithChildContext: No parent context available"); + } + // TODO(gpl) Here we'll want to create a new spanID for tracing + return runWithContext({ ...child, ...parent }, fun); +} + +function runWithContext(context: C, fun: () => T): T { + return asyncLocalStorage.run(context, fun); } export type AsyncGeneratorDecorator = (f: () => T) => T; @@ -51,6 +117,42 @@ export function wrapAsyncGenerator( }; } -export function getRequestContext(): RequestContext { - return asyncLocalStorage.getStore() || {}; // to ease usage we hand out an empty shape here +export class ContextAwareAnalyticsWriter implements IAnalyticsWriter { + constructor(readonly writer: IAnalyticsWriter) {} + + identify(msg: IdentifyMessage): void {} + + track(msg: TrackMessage): void {} + + page(msg: PageMessage): void { + const traceIds = this.getTraceIds(); + this.writer.page({ + ...msg, + userId: msg.userId || traceIds.userId, + subjectId: msg.subjectId || traceIds.subjectId, + }); + } + + private getTraceIds(): { userId?: string; subjectId?: string } { + const subjectId = ctx().subjectId; + if (!subjectId) { + return {}; + } + return { + userId: subjectId.userId(), + subjectId: subjectId.toString(), + }; + } +} + +/** + * @deprecated This function is meant to be used during the transition from the current style, first argument-based authorization, to authorization based on the SubjectId passed in the RequestContext. it's purpose is primarily to a) make sure we can split up work into smaller pieces, and b) make it easier to remove afterwards. + * @returns + */ +export function userId(): string { + const userId = ctx().subjectId?.userId(); + if (!userId) { + throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "No userId available"); + } + return userId; } diff --git a/components/server/src/websocket/websocket-connection-manager.ts b/components/server/src/websocket/websocket-connection-manager.ts index 85f2d841d87e45..79d3e4ed8a9453 100644 --- a/components/server/src/websocket/websocket-connection-manager.ts +++ b/components/server/src/websocket/websocket-connection-manager.ts @@ -49,7 +49,8 @@ import * as opentracing from "opentracing"; import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing"; import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url"; import { maskIp } from "../analytics"; -import { runWithLogContext } from "../util/log-context"; +import { runWithRequestContext, ctx as _ctx } from "../util/request-context"; +import { SubjectId } from "../auth/subject-id"; export type GitpodServiceFactory = () => GitpodServerImpl; @@ -376,18 +377,20 @@ class GitpodJsonRpcProxyFactory extends JsonRpcProxyFactory protected async onRequest(method: string, ...args: any[]): Promise { const span = TraceContext.startSpan(method, undefined); const userId = this.clientMetadata.userId; - const requestId = span.context().toTraceId(); - return runWithLogContext( - "request", + const rpcSignal = args[args.length - 1]; + const signal = rpcSignal ? (rpcSignal as AbortSignal) : new AbortController().signal; + return runWithRequestContext( { - userId, - contextId: requestId, - method, + requestKind: "jsonrpc", + requestMethod: method, + signal, + subjectId: userId ? SubjectId.fromUserId(userId) : undefined, + traceId: span.context().toTraceId(), }, () => { try { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - return this.internalOnRequest(span, requestId, method, ...args); + return this.internalOnRequest(span, method, ...args); } finally { span.finish(); } @@ -395,12 +398,7 @@ class GitpodJsonRpcProxyFactory extends JsonRpcProxyFactory ); } - private async internalOnRequest( - span: opentracing.Span, - requestId: string, - method: string, - ...args: any[] - ): Promise { + private async internalOnRequest(span: opentracing.Span, method: string, ...args: any[]): Promise { const userId = this.clientMetadata.userId; const ctx = { span }; const timer = apiCallDurationHistogram.startTimer(); @@ -446,6 +444,7 @@ class GitpodJsonRpcProxyFactory extends JsonRpcProxyFactory observeAPICallsDuration(method, 200, timer()); return result; } catch (e) { + const requestId = _ctx().requestId; const requestIdMessage = ` If this error is unexpected, please quote the request ID '${requestId}' when reaching out to Gitpod Support.`; if (ApplicationError.hasErrorCode(e)) { increaseApiCallCounter(method, e.code); diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts index 2bffb0cac6361d..785e00c820aac0 100644 --- a/components/server/src/workspace/gitpod-server-impl.ts +++ b/components/server/src/workspace/gitpod-server-impl.ts @@ -175,6 +175,8 @@ import { suggestionFromRecentWorkspace, suggestionFromUserRepo, } from "./suggested-repos-sorter"; +import { runWithChildContext } from "../util/request-context"; +import { SubjectId } from "../auth/subject-id"; // shortcut export const traceWI = (ctx: TraceContext, wi: Omit) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager @@ -467,11 +469,14 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { private async checkUser(methodName?: string, logPayload?: {}, ctx?: LogContext): Promise { // Generally, a user session is required. - if (!this.userID) { + const userId = this.userID; + if (!userId) { throw new ApplicationError(ErrorCodes.NOT_AUTHENTICATED, "User is not authenticated. Please login."); } - const user = await this.userService.findUserById(SYSTEM_USER, this.userID); + const user = await runWithChildContext({ subjectId: SubjectId.fromUserId(SYSTEM_USER) }, async () => + this.userService.findUserById(SYSTEM_USER, userId), + ); if (user.markedDeleted === true) { throw new ApplicationError(ErrorCodes.USER_DELETED, "User has been deleted."); } diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index a6b02ea690085f..8c1d12387e61bd 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -131,6 +131,8 @@ import { EnvVarService, ResolvedEnvVars } from "../user/env-var-service"; import { RedlockAbortSignal } from "redlock"; import { ConfigProvider } from "./config-provider"; import { isGrpcError } from "@gitpod/gitpod-protocol/lib/util/grpc"; +import { runWithChildContext } from "../util/request-context"; +import { SubjectId } from "../auth/subject-id"; export interface StartWorkspaceOptions extends GitpodServer.StartWorkspaceOptions { excludeFeatureFlags?: NamedWorkspaceFeatureFlag[]; @@ -487,7 +489,9 @@ export class WorkspaceStarter { if (blockedRepository.blockUser) { try { - await this.userService.blockUser(SYSTEM_USER, user.id, true); + await runWithChildContext({ subjectId: SubjectId.fromUserId(SYSTEM_USER) }, async () => + this.userService.blockUser(SYSTEM_USER, user.id, true), + ); log.info({ userId: user.id }, "Blocked user.", { contextURL }); } catch (error) { log.error({ userId: user.id }, "Failed to block user.", error, { contextURL });