Skip to content

Commit 7f3b3c7

Browse files
authored
Ak/revert (#18959)
* Revert "fix JWT verification (#18957)" This reverts commit 90c3541. * Revert "[public-api] add rate limiting in server (#18953)" This reverts commit 01f100b.
1 parent 90c3541 commit 7f3b3c7

File tree

5 files changed

+17
-135
lines changed

5 files changed

+17
-135
lines changed

components/server/src/api/rate-limited.ts

Lines changed: 0 additions & 23 deletions
This file was deleted.

components/server/src/api/server.ts

Lines changed: 7 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,6 @@ import { APIStatsService as StatsServiceAPI } from "./stats";
3030
import { APITeamsService as TeamsServiceAPI } from "./teams";
3131
import { APIUserService as UserServiceAPI } from "./user";
3232
import { WorkspaceServiceAPI } from "./workspace-service-api";
33-
import { IRateLimiterOptions, RateLimiterMemory, RateLimiterRedis, RateLimiterRes } from "rate-limiter-flexible";
34-
import { Redis } from "ioredis";
35-
import { RateLimited } from "./rate-limited";
36-
import { Config } from "../config";
37-
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
38-
import { UserService } from "../user/user-service";
39-
import { User } from "@gitpod/gitpod-protocol";
4033

4134
decorate(injectable(), PublicAPIConverter);
4235

@@ -53,9 +46,6 @@ export class API {
5346
@inject(HelloServiceAPI) private readonly helloServiceApi: HelloServiceAPI;
5447
@inject(SessionHandler) private readonly sessionHandler: SessionHandler;
5548
@inject(PublicAPIConverter) private readonly apiConverter: PublicAPIConverter;
56-
@inject(Redis) private readonly redis: Redis;
57-
@inject(Config) private readonly config: Config;
58-
@inject(UserService) private readonly userService: UserService;
5949

6050
listenPrivate(): http.Server {
6151
const app = express();
@@ -184,30 +174,9 @@ export class API {
184174

185175
const context = args[1] as HandlerContext;
186176

187-
const rateLimit = async (subjectId: string) => {
188-
const key = `${grpc_service}/${grpc_method}`;
189-
const options = self.config.rateLimits?.[key] || RateLimited.getOptions(target, prop);
190-
try {
191-
await self.getRateLimitter(options).consume(`${subjectId}_${key}`);
192-
} catch (e) {
193-
if (e instanceof RateLimiterRes) {
194-
throw new ConnectError("rate limit exceeded", Code.ResourceExhausted, {
195-
// http compatibility, can be respected by gRPC clients as well
196-
// instead of doing an ad-hoc retry, the client can wait for the given amount of seconds
197-
"Retry-After": e.msBeforeNext / 1000,
198-
"X-RateLimit-Limit": options.points,
199-
"X-RateLimit-Remaining": e.remainingPoints,
200-
"X-RateLimit-Reset": new Date(Date.now() + e.msBeforeNext),
201-
});
202-
}
203-
throw e;
204-
}
205-
};
206-
207177
const apply = async <T>(): Promise<T> => {
208-
const subjectId = await self.verify(context);
209-
await rateLimit(subjectId);
210-
context.user = await self.ensureFgaMigration(subjectId);
178+
const user = await self.verify(context);
179+
context.user = user;
211180

212181
return Reflect.apply(target[prop as any], target, args);
213182
};
@@ -242,49 +211,16 @@ export class API {
242211
};
243212
}
244213

245-
private async verify(context: HandlerContext): Promise<string> {
246-
const claims = await this.sessionHandler.verifyJWTCookie(context.requestHeader.get("cookie") || "");
247-
const subjectId = claims?.sub;
248-
if (!subjectId) {
214+
private async verify(context: HandlerContext) {
215+
const user = await this.sessionHandler.verify(context.requestHeader.get("cookie") || "");
216+
if (!user) {
249217
throw new ConnectError("unauthenticated", Code.Unauthenticated);
250218
}
251-
return subjectId;
252-
}
253-
254-
private async ensureFgaMigration(userId: string): Promise<User> {
255-
const fgaChecksEnabled = await isFgaChecksEnabled(userId);
219+
const fgaChecksEnabled = await isFgaChecksEnabled(user.id);
256220
if (!fgaChecksEnabled) {
257221
throw new ConnectError("unauthorized", Code.PermissionDenied);
258222
}
259-
try {
260-
return await this.userService.findUserById(userId, userId);
261-
} catch (e) {
262-
if (e instanceof ApplicationError && e.code === ErrorCodes.NOT_FOUND) {
263-
throw new ConnectError("unauthorized", Code.PermissionDenied);
264-
}
265-
throw e;
266-
}
267-
}
268-
269-
private readonly rateLimiters = new Map<string, RateLimiterRedis>();
270-
private getRateLimitter(options: IRateLimiterOptions): RateLimiterRedis {
271-
const sortedKeys = Object.keys(options).sort();
272-
const sortedObject: { [key: string]: any } = {};
273-
for (const key of sortedKeys) {
274-
sortedObject[key] = options[key as keyof IRateLimiterOptions];
275-
}
276-
const key = JSON.stringify(sortedObject);
277-
278-
let rateLimiter = this.rateLimiters.get(key);
279-
if (!rateLimiter) {
280-
rateLimiter = new RateLimiterRedis({
281-
storeClient: this.redis,
282-
...options,
283-
insuranceLimiter: new RateLimiterMemory(options),
284-
});
285-
this.rateLimiters.set(key, rateLimiter);
286-
}
287-
return rateLimiter;
223+
return user;
288224
}
289225

290226
static bindAPI(bind: interfaces.Bind): void {

components/server/src/api/teams.spec.db.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@ import { UserAuthentication } from "../user/user-authentication";
2121
import { WorkspaceService } from "../workspace/workspace-service";
2222
import { API } from "./server";
2323
import { SessionHandler } from "../session-handler";
24-
import { Redis } from "ioredis";
25-
import { UserService } from "../user/user-service";
26-
import { Config } from "../config";
2724

2825
const expect = chai.expect;
2926

@@ -42,9 +39,6 @@ export class APITeamsServiceSpec {
4239
this.container.bind(WorkspaceService).toConstantValue({} as WorkspaceService);
4340
this.container.bind(UserAuthentication).toConstantValue({} as UserAuthentication);
4441
this.container.bind(SessionHandler).toConstantValue({} as SessionHandler);
45-
this.container.bind(Config).toConstantValue({} as Config);
46-
this.container.bind(Redis).toConstantValue({} as Redis);
47-
this.container.bind(UserService).toConstantValue({} as UserService);
4842

4943
// Clean-up database
5044
const typeorm = testContainer.get<TypeORM>(TypeORM);

components/server/src/config.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1717
import { filePathTelepresenceAware } from "@gitpod/gitpod-protocol/lib/env";
1818
import { WorkspaceClassesConfig } from "./workspace/workspace-classes";
1919
import { PrebuildRateLimiters } from "./workspace/prebuild-rate-limiter";
20-
import { IRateLimiterOptions } from "rate-limiter-flexible";
2120

2221
export const Config = Symbol("Config");
2322
export type Config = Omit<
@@ -173,20 +172,9 @@ export interface ConfigSerialized {
173172

174173
/**
175174
* The configuration for the rate limiter we (mainly) use for the websocket API
176-
* @deprecated used for JSON-RPC API, for gRPC use rateLimits
177175
*/
178176
rateLimiter: RateLimiterConfig;
179177

180-
/**
181-
* The configuration for the rate limiter we use for the gRPC API.
182-
* As a primary means use RateLimited decorator.
183-
* Only use this if you need to adjst in production, make sure to apply changes to the decorator as well.
184-
* Key is of the form `<grpc_service>/<grpc_method>`
185-
*/
186-
rateLimits?: {
187-
[key: string]: IRateLimiterOptions;
188-
};
189-
190178
/**
191179
* The address content service clients connect to
192180
* Example: content-service:8080

components/server/src/session-handler.ts

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import { Config } from "./config";
1515
import { WsNextFunction, WsRequestHandler } from "./express/ws-handler";
1616
import { reportJWTCookieIssued } from "./prometheus-metrics";
1717
import { UserService } from "./user/user-service";
18-
import { JwtPayload } from "jsonwebtoken";
1918

2019
@injectable()
2120
export class SessionHandler {
@@ -104,15 +103,21 @@ export class SessionHandler {
104103
}
105104

106105
async verify(cookie: string): Promise<User | undefined> {
106+
const cookies = parseCookieHeader(cookie);
107+
const jwtToken = cookies[this.getJWTCookieName(this.config)];
108+
if (!jwtToken) {
109+
log.debug("No JWT session present on request");
110+
return undefined;
111+
}
107112
try {
108-
const claims = await this.verifyJWTCookie(cookie);
109-
if (!claims) {
110-
return undefined;
111-
}
113+
const claims = await this.authJWT.verify(jwtToken);
114+
log.debug("JWT Session token verified", { claims });
115+
112116
const subject = claims.sub;
113117
if (!subject) {
114118
throw new Error("Subject is missing from JWT session claims");
115119
}
120+
116121
return await this.userService.findUserById(subject, subject);
117122
} catch (err) {
118123
log.warn("Failed to authenticate user with JWT Session", err);
@@ -121,24 +126,6 @@ export class SessionHandler {
121126
}
122127
}
123128

124-
/**
125-
* Verify the JWT session cookie.
126-
* @throws only in case of programming errors, if the cookie is invalid, undefined is returned
127-
* @param cookie - the cookie header value to verify
128-
* @returns returns the claims of the JWT session cookie or undefined if the cookie is not present or invalid
129-
*/
130-
async verifyJWTCookie(cookie: string): Promise<JwtPayload | undefined> {
131-
const cookies = parseCookieHeader(cookie);
132-
const jwtToken = cookies[this.getJWTCookieName(this.config)];
133-
if (!jwtToken) {
134-
log.debug("No JWT session present on request");
135-
return undefined;
136-
}
137-
const claims = await this.authJWT.verify(jwtToken);
138-
log.debug("JWT Session token verified", { claims });
139-
return claims;
140-
}
141-
142129
public async createJWTSessionCookie(
143130
userID: string,
144131
): Promise<{ name: string; value: string; opts: express.CookieOptions }> {

0 commit comments

Comments
 (0)