diff --git a/components/server/ee/src/workspace/gitpod-server-impl.ts b/components/server/ee/src/workspace/gitpod-server-impl.ts index 0a1e4a8da778b6..417a0c99f092bc 100644 --- a/components/server/ee/src/workspace/gitpod-server-impl.ts +++ b/components/server/ee/src/workspace/gitpod-server-impl.ts @@ -1596,7 +1596,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { // Team Subscriptions 2 async getTeamSubscription(ctx: TraceContext, teamId: string): Promise { this.checkUser("getTeamSubscription"); - await this.guardTeamOperation(teamId, "get", ["not_implemented"]); + await this.guardTeamOperation(teamId, "get", "not_implemented"); return this.teamSubscription2DB.findForTeam(teamId, new Date().toISOString()); } @@ -2098,7 +2098,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { try { if (attrId.kind == "team") { - await this.guardTeamOperation(attrId.teamId, "get", ["not_implemented"]); + await this.guardTeamOperation(attrId.teamId, "get", "not_implemented"); } const subscriptionId = await this.stripeService.findUncancelledSubscriptionByAttributionId(attributionId); return subscriptionId; @@ -2119,7 +2119,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { } let team: Team | undefined; if (attrId.kind === "team") { - team = (await this.guardTeamOperation(attrId.teamId, "update", ["not_implemented"])).team; + team = (await this.guardTeamOperation(attrId.teamId, "update", "not_implemented")).team; await this.ensureStripeApiIsAllowed({ team }); } else { if (attrId.userId !== user.id) { @@ -2141,7 +2141,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { } let team: Team | undefined; if (attrId.kind === "team") { - team = (await this.guardTeamOperation(attrId.teamId, "update", ["not_implemented"])).team; + team = (await this.guardTeamOperation(attrId.teamId, "update", "not_implemented")).team; await this.ensureStripeApiIsAllowed({ team }); } else { if (attrId.userId !== user.id) { @@ -2211,7 +2211,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { let team: Team | undefined; try { if (attrId.kind === "team") { - team = (await this.guardTeamOperation(attrId.teamId, "update", ["not_implemented"])).team; + team = (await this.guardTeamOperation(attrId.teamId, "update", "not_implemented")).team; await this.ensureStripeApiIsAllowed({ team }); } else { await this.ensureStripeApiIsAllowed({ user }); @@ -2260,7 +2260,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { await this.ensureStripeApiIsAllowed({ user }); returnUrl = this.config.hostUrl.with(() => ({ pathname: `/user/billing`, search: `org=0` })).toString(); } else if (attrId.kind === "team") { - const team = (await this.guardTeamOperation(attrId.teamId, "update", ["not_implemented"])).team; + const team = (await this.guardTeamOperation(attrId.teamId, "update", "not_implemented")).team; await this.ensureStripeApiIsAllowed({ team }); } let url: string; @@ -2493,7 +2493,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl { traceAPIParams(ctx, { teamId }); this.checkAndBlockUser("getBillingModeForTeam"); - const { team } = await this.guardTeamOperation(teamId, "get", ["not_implemented"]); + const { team } = await this.guardTeamOperation(teamId, "get", "not_implemented"); return this.billingModes.getBillingModeForTeam(team, new Date()); } diff --git a/components/server/src/authorization/checks.ts b/components/server/src/authorization/checks.ts new file mode 100644 index 00000000000000..0bf0abf19b0ad8 --- /dev/null +++ b/components/server/src/authorization/checks.ts @@ -0,0 +1,41 @@ +/** + * 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 { v1 } from "@authzed/authzed-node"; +import { ResourceType, SubjectType } from "./perms"; + +const FULLY_CONSISTENT = v1.Consistency.create({ + requirement: { + oneofKind: "fullyConsistent", + fullyConsistent: true, + }, +}); + +type SubjectResourceCheckFn = (subjectID: string, resourceID: string) => v1.CheckPermissionRequest; + +function check(subjectT: SubjectType, op: string, resourceT: ResourceType): SubjectResourceCheckFn { + return (subjectID, resourceID) => + v1.CheckPermissionRequest.create({ + subject: v1.SubjectReference.create({ + object: v1.ObjectReference.create({ + objectId: subjectID, + objectType: subjectT, + }), + }), + permission: op, + resource: v1.ObjectReference.create({ + objectId: resourceID, + objectType: resourceT, + }), + consistency: FULLY_CONSISTENT, + }); +} + +export const ReadOrganizationMetadata = check("user", "organization_metadata_read", "organization"); +export const WriteOrganizationMetadata = check("user", "organization_metadata_write", "organization"); + +export const ReadOrganizationMembers = check("user", "organization_members_read", "organization"); +export const WriteOrganizationMembers = check("user", "organization_members_write", "organization"); diff --git a/components/server/src/authorization/perms.ts b/components/server/src/authorization/perms.ts index c4a4c069997a7b..568b0dec2725bf 100644 --- a/components/server/src/authorization/perms.ts +++ b/components/server/src/authorization/perms.ts @@ -4,6 +4,17 @@ * See License.AGPL.txt in the project root for license information. */ +import { v1 } from "@authzed/authzed-node"; +import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; +import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; +import { inject, injectable } from "inversify"; +import { ResponseError } from "vscode-ws-jsonrpc"; +import { + observespicedbClientLatency as observeSpicedbClientLatency, + spicedbClientLatency, +} from "../prometheus-metrics"; +import { SpiceDBClient } from "./spicedb"; + export type OrganizationOperation = // A not yet implemented operation at this time. This exists such that we can be explicit about what // we have not yet migrated to fine-grained-permissions. @@ -31,3 +42,66 @@ export type OrganizationOperation = | "org_authprovider_write" // Ability to read Organization Auth Providers. | "org_authprovider_read"; + +export type ResourceType = "organization"; + +export type SubjectType = "user"; + +export type CheckResult = { + permitted: boolean; + err?: Error; + response?: v1.CheckPermissionResponse; +}; + +export const NotPermitted = { permitted: false }; + +export const PermissionChecker = Symbol("PermissionChecker"); + +export interface PermissionChecker { + check(req: v1.CheckPermissionRequest): Promise; +} + +@injectable() +export class Authorizer implements PermissionChecker { + @inject(SpiceDBClient) + private client: SpiceDBClient; + + async check(req: v1.CheckPermissionRequest): Promise { + if (!this.client) { + return { + permitted: false, + err: new Error("Authorization client not available."), + response: v1.CheckPermissionResponse.create({}), + }; + } + + const timer = spicedbClientLatency.startTimer(); + try { + const response = await this.client.checkPermission(req); + const permitted = response.permissionship === v1.CheckPermissionResponse_Permissionship.HAS_PERMISSION; + const err = !permitted ? newUnathorizedError(req.resource!, req.permission, req.subject!) : undefined; + + observeSpicedbClientLatency("check", req.permission, undefined, timer()); + + return { permitted, response, err }; + } catch (err) { + log.error("[spicedb] Failed to perform authorization check.", err, { req }); + observeSpicedbClientLatency("check", req.permission, err, timer()); + + throw err; + } + } +} + +function newUnathorizedError(resource: v1.ObjectReference, relation: string, subject: v1.SubjectReference) { + return new ResponseError( + ErrorCodes.PERMISSION_DENIED, + `Subject (${objString(subject.object)}) is not permitted to perform ${relation} on resource ${objString( + resource, + )}.`, + ); +} + +function objString(obj?: v1.ObjectReference): string { + return `${obj?.objectType}:${obj?.objectId}`; +} diff --git a/components/server/src/container-module.ts b/components/server/src/container-module.ts index ceebabdbc5d37a..1937f6ebd599ea 100644 --- a/components/server/src/container-module.ts +++ b/components/server/src/container-module.ts @@ -113,6 +113,7 @@ import { UbpResetOnCancel } from "@gitpod/gitpod-payment-endpoint/lib/chargebee/ import { retryMiddleware } from "nice-grpc-client-middleware-retry"; import { IamSessionApp } from "./iam/iam-session-app"; import { spicedbClientFromEnv, SpiceDBClient } from "./authorization/spicedb"; +import { Authorizer, PermissionChecker } from "./authorization/perms"; export const productionContainerModule = new ContainerModule((bind, unbind, isBound, rebind) => { bind(Config).toConstantValue(ConfigFile.fromFile()); @@ -313,4 +314,5 @@ export const productionContainerModule = new ContainerModule((bind, unbind, isBo bind(SpiceDBClient) .toDynamicValue(() => spicedbClientFromEnv()) .inSingletonScope(); + bind(PermissionChecker).to(Authorizer).inSingletonScope(); }); diff --git a/components/server/src/prometheus-metrics.ts b/components/server/src/prometheus-metrics.ts index c7873d66c64420..8591c7aa93d1bc 100644 --- a/components/server/src/prometheus-metrics.ts +++ b/components/server/src/prometheus-metrics.ts @@ -22,6 +22,8 @@ export function registerServerMetrics(registry: prometheusClient.Registry) { registry.registerMetric(stripeClientRequestsCompletedDurationSeconds); registry.registerMetric(imageBuildsStartedTotal); registry.registerMetric(imageBuildsCompletedTotal); + registry.registerMetric(centralizedPermissionsValidationsTotal); + registry.registerMetric(spicedbClientLatency); } const loginCounter = new prometheusClient.Counter({ @@ -195,3 +197,31 @@ export const imageBuildsCompletedTotal = new prometheusClient.Counter({ export function increaseImageBuildsCompletedTotal(outcome: "succeeded" | "failed") { imageBuildsCompletedTotal.inc({ outcome }); } + +const centralizedPermissionsValidationsTotal = new prometheusClient.Counter({ + name: "gitpod_perms_centralized_validations_total", + help: "counter of centralized permission checks validations against existing system", + labelNames: ["operation", "matches_expectation"], +}); + +export function reportCentralizedPermsValidation(operation: string, matches: boolean) { + centralizedPermissionsValidationsTotal.inc({ operation, matches_expectation: String(matches) }); +} + +export const spicedbClientLatency = new prometheusClient.Histogram({ + name: "gitpod_spicedb_client_requests_completed_seconds", + help: "Histogram of completed spicedb client requests", + labelNames: ["operation", "permission", "outcome"], +}); + +export function observespicedbClientLatency( + operation: string, + permission: string, + outcome: Error | undefined, + durationInSeconds: number, +) { + spicedbClientLatency.observe( + { operation, permission, outcome: outcome === undefined ? "success" : "error" }, + durationInSeconds, + ); +} diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts index 502919a940bd40..c8d7fd2db2cc0e 100644 --- a/components/server/src/workspace/gitpod-server-impl.ts +++ b/components/server/src/workspace/gitpod-server-impl.ts @@ -184,7 +184,20 @@ import { ConfigCatClientFactory, getExperimentsClientForBackend, } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server"; -import { OrganizationOperation } from "../authorization/perms"; +import { + Authorizer, + CheckResult, + OrganizationOperation, + NotPermitted, + PermissionChecker, +} from "../authorization/perms"; +import { + ReadOrganizationMembers, + ReadOrganizationMetadata, + WriteOrganizationMembers, + WriteOrganizationMetadata, +} from "../authorization/checks"; +import { reportCentralizedPermsValidation } from "../prometheus-metrics"; // shortcut export const traceWI = (ctx: TraceContext, wi: Omit) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager @@ -257,6 +270,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { @inject(ConfigCatClientFactory) protected readonly configCatClientFactory: ConfigCatClientFactory; + @inject(PermissionChecker) protected readonly authorizer: Authorizer; + /** Id the uniquely identifies this server instance */ public readonly uuid: string = uuidv4(); public readonly clientMetadata: ClientMetadata; @@ -2037,7 +2052,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { protected async guardTeamOperation( teamId: string, op: ResourceAccessOp, - fineGrainedOps: OrganizationOperation[], + fineGrainedOp: OrganizationOperation, ): Promise<{ team: Team; members: TeamMemberInfo[] }> { if (!uuidValidate(teamId)) { throw new ResponseError(ErrorCodes.BAD_REQUEST, "organization ID must be a valid UUID"); @@ -2053,19 +2068,92 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { }, ); - if (centralizedPermissionsEnabled) { - log.info("[perms] Checking team operations.", { org: teamId, operations: fineGrainedOps, user: user.id }); - } + const checkAgainstDB = async (): Promise<{ team: Team; members: TeamMemberInfo[] }> => { + // We deliberately wrap the entiry check in try-catch, because we're using Promise.all, which rejects if any of the promises reject. + const team = await this.teamDB.findTeamById(teamId); + if (!team) { + // We return Permission Denied because we don't want to leak the existence, or not of the Organization. + throw new ResponseError(ErrorCodes.PERMISSION_DENIED, `No access to Organization ID: ${teamId}`); + } - const team = await this.teamDB.findTeamById(teamId); - if (!team) { - // We return Permission Denied because we don't want to leak the existence, or not of the Organization. - throw new ResponseError(ErrorCodes.PERMISSION_DENIED, `No access to Organization ID: ${teamId}`); + const members = await this.teamDB.findMembersByTeam(team.id); + await this.guardAccess({ kind: "team", subject: team, members }, op); + + return { team, members }; + }; + + const checkWithCentralizedPerms = async (): Promise => { + if (centralizedPermissionsEnabled) { + log.info("[perms] Checking team operations.", { + org: teamId, + operations: fineGrainedOp, + user: user.id, + }); + + return await this.guardOrganizationOperationWithCentralizedPerms(teamId, fineGrainedOp); + } + + throw new Error("Centralized permissions feature not enabled."); + }; + + const [fromDB, fromCentralizedPerms] = await Promise.allSettled([ + // Permission checks against the DB will throw, if the user is not permitted to perform the action, or if iteraction with + // dependencies (DB) fail. + checkAgainstDB(), + + // Centralized perms checks only throw, when an interaction error occurs - connection not available or similar. + // When the user is not permitted to perform the action, the call will resolve, encoding the result in the response. + checkWithCentralizedPerms(), + ]); + + // check against DB resolved, which means the user is permitted to perform the action + if (fromDB.status === "fulfilled") { + if (fromCentralizedPerms.status === "fulfilled") { + // we got a result from centralized perms, but we still need to check if the outcome was such that the user is permitted + reportCentralizedPermsValidation(fineGrainedOp, fromCentralizedPerms.value.permitted === true); + } else { + // centralized perms promise rejected, we do not have an agreement + reportCentralizedPermsValidation(fineGrainedOp, false); + } + + // Always return the result from the DB check + return fromDB.value; + } else { + // The check agains the DB failed. This means the user does not have access. + + if (fromCentralizedPerms.status === "fulfilled") { + // we got a result from centralized perms, but we still need to check if the outcome was such that the user is NOT permitted + reportCentralizedPermsValidation(fineGrainedOp, fromCentralizedPerms.value.permitted === false); + } else { + // centralized perms promise rejected, we do not have an agreement + reportCentralizedPermsValidation(fineGrainedOp, false); + } + + // We re-throw the error from the DB permission check, to propagate it upstream. + throw fromDB.reason; } + } + + protected async guardOrganizationOperationWithCentralizedPerms( + orgId: string, + op: OrganizationOperation, + ): Promise { + const user = this.checkUser(); - const members = await this.teamDB.findMembersByTeam(team.id); - await this.guardAccess({ kind: "team", subject: team, members }, op); - return { team, members }; + switch (op) { + case "org_metadata_read": + return await this.authorizer.check(ReadOrganizationMetadata(user.id, orgId)); + case "org_metadata_write": + return await this.authorizer.check(WriteOrganizationMetadata(user.id, orgId)); + + case "org_members_read": + return await this.authorizer.check(ReadOrganizationMembers(user.id, orgId)); + case "org_members_write": + return await this.authorizer.check(WriteOrganizationMembers(user.id, orgId)); + + default: + return NotPermitted; + } } public async getTeams(ctx: TraceContext): Promise { @@ -2079,7 +2167,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { this.checkAndBlockUser("getTeam"); - const { team } = await this.guardTeamOperation(teamId, "get", ["org_members_read"]); + const { team } = await this.guardTeamOperation(teamId, "get", "org_members_read"); return team; } @@ -2087,7 +2175,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { traceAPIParams(ctx, { teamId }); this.checkUser("updateTeam"); - await this.guardTeamOperation(teamId, "update", ["org_metadata_write"]); + await this.guardTeamOperation(teamId, "update", "org_metadata_write"); const updatedTeam = await this.teamDB.updateTeam(teamId, team); return updatedTeam; @@ -2097,7 +2185,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { traceAPIParams(ctx, { teamId }); this.checkUser("getTeamMembers"); - const { members } = await this.guardTeamOperation(teamId, "get", ["org_members_read"]); + const { members } = await this.guardTeamOperation(teamId, "get", "org_members_read"); return members; } @@ -2168,7 +2256,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { } this.checkAndBlockUser("setTeamMemberRole"); - await this.guardTeamOperation(teamId, "update", ["org_members_write"]); + await this.guardTeamOperation(teamId, "update", "org_members_write"); await this.teamDB.setTeamMemberRole(userId, teamId, role); } @@ -2185,9 +2273,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const userLeavingTeam = user.id === userId; if (userLeavingTeam) { - await this.guardTeamOperation(teamId, "update", ["not_implemented"]); + await this.guardTeamOperation(teamId, "update", "not_implemented"); } else { - await this.guardTeamOperation(teamId, "get", ["org_members_write"]); + await this.guardTeamOperation(teamId, "get", "org_members_write"); } const membership = await this.teamDB.findTeamMembership(userId, teamId); @@ -2210,7 +2298,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { traceAPIParams(ctx, { teamId }); this.checkUser("getGenericInvite"); - await this.guardTeamOperation(teamId, "get", ["org_members_write"]); + await this.guardTeamOperation(teamId, "get", "org_members_write"); const invite = await this.teamDB.findGenericInviteByTeamId(teamId); if (invite) { @@ -2223,7 +2311,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { traceAPIParams(ctx, { teamId }); this.checkAndBlockUser("resetGenericInvite"); - await this.guardTeamOperation(teamId, "update", ["org_members_write"]); + await this.guardTeamOperation(teamId, "update", "org_members_write"); return this.teamDB.resetGenericInvite(teamId); } @@ -2240,7 +2328,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { } } else { // Anyone who can read a team's information (i.e. any team member) can manage team projects - await this.guardTeamOperation(project.teamId || "", "get", ["not_implemented"]); + await this.guardTeamOperation(project.teamId || "", "get", "not_implemented"); } } @@ -2267,7 +2355,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { } } else { // Anyone who can read a team's information (i.e. any team member) can create a new project. - await this.guardTeamOperation(params.teamId || "", "get", ["not_implemented"]); + await this.guardTeamOperation(params.teamId || "", "get", "not_implemented"); } return this.projectsService.createProject(params, user); @@ -2292,7 +2380,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const user = this.checkAndBlockUser("deleteTeam"); traceAPIParams(ctx, { teamId, userId: user.id }); - await this.guardTeamOperation(teamId, "delete", ["org_write"]); + await this.guardTeamOperation(teamId, "delete", "org_write"); const teamProjects = await this.projectsService.getTeamProjects(teamId); teamProjects.forEach((project) => { @@ -2325,7 +2413,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { this.checkUser("getTeamProjects"); - await this.guardTeamOperation(teamId, "get", ["not_implemented"]); + await this.guardTeamOperation(teamId, "get", "not_implemented"); return this.projectsService.getTeamProjects(teamId); } @@ -2948,7 +3036,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { } // Ensure user can perform this operation on this organization - await this.guardTeamOperation(newProvider.organizationId, "create", ["org_authprovider_write"]); + await this.guardTeamOperation(newProvider.organizationId, "create", "org_authprovider_write"); try { // on creating we're are checking for already existing runtime providers @@ -2996,7 +3084,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { await this.guardWithFeatureFlag("orgGitAuthProviders", providerUpdate.organizationId); - await this.guardTeamOperation(providerUpdate.organizationId, "update", ["org_authprovider_write"]); + await this.guardTeamOperation(providerUpdate.organizationId, "update", "org_authprovider_write"); try { const result = await this.authProviderService.updateOrgAuthProvider(providerUpdate); @@ -3017,7 +3105,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { await this.guardWithFeatureFlag("orgGitAuthProviders", params.organizationId); - await this.guardTeamOperation(params.organizationId, "get", ["org_authprovider_read"]); + await this.guardTeamOperation(params.organizationId, "get", "org_authprovider_read"); try { const result = await this.authProviderService.getAuthProvidersOfOrg(params.organizationId); @@ -3048,7 +3136,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { throw new ResponseError(ErrorCodes.NOT_FOUND, "Provider resource not found."); } - await this.guardTeamOperation(authProvider.organizationId || "", "delete", ["org_authprovider_write"]); + await this.guardTeamOperation(authProvider.organizationId || "", "delete", "org_authprovider_write"); try { await this.authProviderService.deleteAuthProvider(authProvider);