From fcc70e3755329959ecd9c856a2bf4d413b152b19 Mon Sep 17 00:00:00 2001 From: Jan Keromnes Date: Tue, 1 Mar 2022 11:55:23 +0000 Subject: [PATCH 1/2] [server] Rate-limit 'triggerPrebuild' method to 50/min --- components/server/src/auth/rate-limiter.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/components/server/src/auth/rate-limiter.ts b/components/server/src/auth/rate-limiter.ts index e7084188b7d9f4..879c11eea59151 100644 --- a/components/server/src/auth/rate-limiter.ts +++ b/components/server/src/auth/rate-limiter.ts @@ -40,6 +40,10 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig { points: 1, // 1 workspace start per user per 10s durationsSec: 10 }, + startPrebuild: { + points: 50, // 50 prebuild starts per user per minute + durationsSec: 60 + }, } const defaultFunctions: FunctionsConfig = { "getLoggedInUser": { group: "default", points: 1 }, @@ -105,7 +109,7 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig { "deleteProject": { group: "default", points: 1 }, "findPrebuilds": { group: "default", points: 1 }, "getProjectOverview": { group: "default", points: 1 }, - "triggerPrebuild": { group: "default", points: 1 }, + "triggerPrebuild": { group: "startPrebuild", points: 1 }, "cancelPrebuild": { group: "default", points: 1 }, "fetchProjectRepositoryConfiguration": { group: "default", points: 1 }, "guessProjectConfiguration": { group: "default", points: 1 }, From 35e97c10643734396c4368240d453ad734c82bdf Mon Sep 17 00:00:00 2001 From: Jan Keromnes Date: Tue, 1 Mar 2022 12:46:24 +0000 Subject: [PATCH 2/2] [server] Also rate-limit 'PrebuildManager.startPrebuild' (used by apps & webhooks) --- .../ee/src/prebuilds/prebuild-manager.ts | 23 +++++++++++++------ components/server/src/auth/rate-limiter.ts | 14 ++++++----- .../websocket/websocket-connection-manager.ts | 2 +- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/components/server/ee/src/prebuilds/prebuild-manager.ts b/components/server/ee/src/prebuilds/prebuild-manager.ts index 2ef2b4a67c5831..dd05a5904d6a39 100644 --- a/components/server/ee/src/prebuilds/prebuild-manager.ts +++ b/components/server/ee/src/prebuilds/prebuild-manager.ts @@ -5,7 +5,7 @@ */ import { DBWithTracing, TracedWorkspaceDB, WorkspaceDB } from '@gitpod/gitpod-db/lib'; -import { CommitContext, Project, ProjectEnvVar, StartPrebuildContext, StartPrebuildResult, TaskConfig, User, WorkspaceConfig, WorkspaceInstance } from '@gitpod/gitpod-protocol'; +import { CommitContext, Project, ProjectEnvVar, RateLimiterError, StartPrebuildContext, StartPrebuildResult, TaskConfig, User, WorkspaceConfig, WorkspaceInstance } from '@gitpod/gitpod-protocol'; import { log } from '@gitpod/gitpod-protocol/lib/util/logging'; import { TraceContext } from '@gitpod/gitpod-protocol/lib/util/tracing'; import { inject, injectable } from 'inversify'; @@ -16,6 +16,9 @@ import { ConfigProvider } from '../../../src/workspace/config-provider'; import { WorkspaceStarter } from '../../../src/workspace/workspace-starter'; import { Config } from '../../../src/config'; import { ProjectsService } from '../../../src/projects/projects-service'; +import { startPrebuild, UserRateLimiter } from '../../../src/auth/rate-limiter'; +import { ErrorCodes } from '@gitpod/gitpod-protocol/lib/messaging/error'; +import { ResponseError } from 'vscode-ws-jsonrpc'; export class WorkspaceRunningError extends Error { constructor(msg: string, public instance: WorkspaceInstance) { @@ -119,12 +122,18 @@ export class PrebuildManager { const workspace = await this.workspaceFactory.createForContext({span}, user, prebuildContext, contextURL); const prebuildPromise = this.workspaceDB.trace({span}).findPrebuildByWorkspaceID(workspace.id)!; - // const canBuildNow = await this.prebuildRateLimiter.canBuildNow({ span }, user, cloneURL); - // if (!canBuildNow) { - // // we cannot start building now because the rate limiter prevents it. - // span.setTag("starting", false); - // return { wsid: workspace.id, done: false };; - // } + // rate limiting + try { + await UserRateLimiter.instance(this.config.rateLimiter).consume(user.id, startPrebuild); + } catch (error) { + span.setTag("starting", false); + if (error instanceof Error) { + log.error({ userId: user.id }, "Unexpected error in the rate limiter", error); + throw error; + } + log.warn({ userId: user.id }, "Rate limiter prevents accessing method due to too many requests.", error, { method: "startPrebuild" }); + throw new ResponseError(ErrorCodes.TOO_MANY_REQUESTS, "too many requests", { method: "startPrebuild", retryAfter: Math.ceil(error.msBeforeNext / 1000) || 1 }); + } span.setTag("starting", true); const projectEnvVars = await projectEnvVarsPromise; diff --git a/components/server/src/auth/rate-limiter.ts b/components/server/src/auth/rate-limiter.ts index 879c11eea59151..6896499c7351a5 100644 --- a/components/server/src/auth/rate-limiter.ts +++ b/components/server/src/auth/rate-limiter.ts @@ -11,8 +11,9 @@ import { RateLimiterMemory, RateLimiterRes } from "rate-limiter-flexible"; export const accessCodeSyncStorage = 'accessCodeSyncStorage'; export const accessHeadlessLogs = 'accessHeadlessLogs'; -type GitpodServerMethodType = keyof Omit | typeof accessCodeSyncStorage | typeof accessHeadlessLogs; -type GroupKey = "default" | "startWorkspace"; +export const startPrebuild = 'startPrebuild'; +type GitpodServerMethodType = keyof Omit | typeof accessCodeSyncStorage | typeof accessHeadlessLogs | typeof startPrebuild; +type GroupKey = "default" | "startWorkspace" | "startPrebuild"; type GroupsConfig = { [key: string]: { points: number, @@ -161,10 +162,6 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig { "getLicenseInfo": { group: "default", points: 1 }, "licenseIncludesFeature": { group: "default", points: 1 }, - "accessCodeSyncStorage": { group: "default", points: 1 }, - - accessHeadlessLogs: { group: "default", points: 1 }, - "adminAddStudentEmailDomain": { group: "default", points: 1 }, "adminGetAccountStatement": { group: "default", points: 1 }, "adminIsStudent": { group: "default", points: 1 }, @@ -197,6 +194,11 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig { "trackLocation": { group: "default", points: 1}, "identifyUser": { group: "default", points: 1}, "getIDEOptions": { group: "default", points: 1 }, + + // Non-server methods + "accessCodeSyncStorage": { group: "default", points: 1 }, + "accessHeadlessLogs": { group: "default", points: 1 }, + "startPrebuild": { group: "startPrebuild", points: 1 }, }; return { diff --git a/components/server/src/websocket/websocket-connection-manager.ts b/components/server/src/websocket/websocket-connection-manager.ts index 4a0b8cbe191642..4430920a45c662 100644 --- a/components/server/src/websocket/websocket-connection-manager.ts +++ b/components/server/src/websocket/websocket-connection-manager.ts @@ -358,7 +358,7 @@ class GitpodJsonRpcProxyFactory extends JsonRpcProxyFactory throw rlRejected; } log.warn({ userId }, "Rate limiter prevents accessing method due to too many requests.", rlRejected, { method }); - throw new ResponseError(ErrorCodes.TOO_MANY_REQUESTS, "too many requests", { method, retryAfter: Math.round(rlRejected.msBeforeNext / 1000) || 1 }); + throw new ResponseError(ErrorCodes.TOO_MANY_REQUESTS, "too many requests", { method, retryAfter: Math.ceil(rlRejected.msBeforeNext / 1000) || 1 }); } // access guard