Skip to content

Commit 8a86de0

Browse files
committed
This is a combination of 3 commits.
[server] Make gRPC accept PAT tokens
1 parent 453392b commit 8a86de0

File tree

6 files changed

+86
-33
lines changed

6 files changed

+86
-33
lines changed

components/server/src/api/server.ts

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import { MethodKind, ServiceType } from "@bufbuild/protobuf";
88
import { Code, ConnectError, ConnectRouter, HandlerContext, ServiceImpl } from "@connectrpc/connect";
99
import { expressConnectMiddleware } from "@connectrpc/connect-express";
10-
import { User } from "@gitpod/gitpod-protocol";
1110
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1211
import { PublicAPIConverter } from "@gitpod/gitpod-protocol/lib/public-api-converter";
1312
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
@@ -49,6 +48,7 @@ import { ConfigurationServiceAPI } from "./configuration-service-api";
4948
import { AuthProviderServiceAPI } from "./auth-provider-service-api";
5049
import { Unauthenticated } from "./unauthenticated";
5150
import { SubjectId } from "../auth/subject-id";
51+
import { BearerAuth } from "../auth/bearer-authenticator";
5252

5353
decorate(injectable(), PublicAPIConverter);
5454

@@ -71,6 +71,7 @@ export class API {
7171
@inject(Redis) private readonly redis: Redis;
7272
@inject(Config) private readonly config: Config;
7373
@inject(UserService) private readonly userService: UserService;
74+
@inject(BearerAuth) private readonly bearerAuthenticator: BearerAuth;
7475

7576
listenPrivate(): http.Server {
7677
const app = express();
@@ -220,20 +221,20 @@ export class API {
220221
// actually call the RPC handler
221222
const auth = async () => {
222223
// Authenticate
223-
const userId = await self.verify(connectContext);
224-
const isAuthenticated = !!userId;
224+
const subjectId = await self.verify(connectContext);
225+
const isAuthenticated = !!subjectId;
225226
const requiresAuthentication = !Unauthenticated.get(target, prop);
226227
if (!isAuthenticated && requiresAuthentication) {
227228
throw new ConnectError("unauthenticated", Code.Unauthenticated);
228229
}
229230

230231
if (isAuthenticated) {
231-
await rateLimit(userId);
232-
await self.ensureFgaMigration(userId);
232+
await rateLimit(subjectId.toString());
233+
await self.ensureFgaMigration(subjectId);
233234
}
234235
// TODO(at) if unauthenticated, we still need to apply enforece a rate limit
235236

236-
return userId ? SubjectId.fromUserId(userId) : undefined;
237+
return subjectId;
237238
};
238239

239240
const apply = async <T>(): Promise<T> => {
@@ -293,30 +294,46 @@ export class API {
293294
};
294295
}
295296

296-
private async verify(context: HandlerContext): Promise<string | undefined> {
297+
private async verify(context: HandlerContext): Promise<SubjectId | undefined> {
298+
// 1. Try Bearer token first
299+
try {
300+
const subjectId = await this.bearerAuthenticator.tryAuthFromHeaders(context.requestHeader);
301+
if (subjectId) {
302+
return subjectId;
303+
}
304+
// fall-through to session JWT
305+
} catch (err) {
306+
log.warn("error authenticating subject by Bearer token", err);
307+
}
308+
309+
// 2. Try for session JWT in the "cookie" header
297310
const cookieHeader = (context.requestHeader.get("cookie") || "") as string;
298311
try {
299312
const claims = await this.sessionHandler.verifyJWTCookie(cookieHeader);
300313
const userId = claims?.sub;
301-
return userId;
302-
} catch (error) {
303-
log.warn("Failed to authenticate user with JWT Session", error);
314+
return !!userId ? SubjectId.fromUserId(userId) : undefined;
315+
} catch (err) {
316+
log.warn("Failed to authenticate user with JWT Session", err);
304317
return undefined;
305318
}
306319
}
307320

308-
private async ensureFgaMigration(userId: string): Promise<User> {
309-
const fgaChecksEnabled = await isFgaChecksEnabled(userId);
321+
private async ensureFgaMigration(subjectId: SubjectId): Promise<void> {
322+
const fgaChecksEnabled = await isFgaChecksEnabled(subjectId.userId());
310323
if (!fgaChecksEnabled) {
311324
throw new ConnectError("unauthorized", Code.PermissionDenied);
312325
}
313-
try {
314-
return await this.userService.findUserById(userId, userId);
315-
} catch (e) {
316-
if (e instanceof ApplicationError && e.code === ErrorCodes.NOT_FOUND) {
317-
throw new ConnectError("unauthorized", Code.PermissionDenied);
326+
327+
if (subjectId.kind === "user") {
328+
const userId = subjectId.userId()!;
329+
try {
330+
await this.userService.findUserById(userId, userId);
331+
} catch (e) {
332+
if (e instanceof ApplicationError && e.code === ErrorCodes.NOT_FOUND) {
333+
throw new ConnectError("unauthorized", Code.PermissionDenied);
334+
}
335+
throw e;
318336
}
319-
throw e;
320337
}
321338
}
322339

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { Config } from "../config";
2727
import { OrganizationService } from "../orgs/organization-service";
2828
import { ProjectsService } from "../projects/projects-service";
2929
import { AuthProviderService } from "../auth/auth-provider-service";
30+
import { BearerAuth } from "../auth/bearer-authenticator";
3031

3132
const expect = chai.expect;
3233

@@ -45,6 +46,7 @@ export class APITeamsServiceSpec {
4546
this.container.bind(WorkspaceService).toConstantValue({} as WorkspaceService);
4647
this.container.bind(OrganizationService).toConstantValue({} as OrganizationService);
4748
this.container.bind(UserAuthentication).toConstantValue({} as UserAuthentication);
49+
this.container.bind(BearerAuth).toConstantValue({} as BearerAuth);
4850
this.container.bind(SessionHandler).toConstantValue({} as SessionHandler);
4951
this.container.bind(Config).toConstantValue({} as Config);
5052
this.container.bind(Redis).toConstantValue({} as Redis);

components/server/src/auth/bearer-authenticator.ts

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,19 @@ import { GitpodTokenType, User } from "@gitpod/gitpod-protocol";
99
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1010
import * as crypto from "crypto";
1111
import express from "express";
12-
import { IncomingHttpHeaders } from "http";
1312
import { inject, injectable } from "inversify";
1413
import { Config } from "../config";
15-
import { AllAccessFunctionGuard, ExplicitFunctionAccessGuard, WithFunctionAccessGuard } from "./function-access";
14+
import {
15+
AllAccessFunctionGuard,
16+
ExplicitFunctionAccessGuard,
17+
FunctionAccessGuard,
18+
WithFunctionAccessGuard,
19+
} from "./function-access";
1620
import { TokenResourceGuard, WithResourceAccessGuard } from "./resource-access";
1721
import { UserService } from "../user/user-service";
22+
import { SubjectId } from "./subject-id";
1823

19-
export function getBearerToken(headers: IncomingHttpHeaders): string | undefined {
24+
export function getBearerToken(headers: { [key: string]: any }): string | undefined {
2025
const authorizationHeader = headers["authorization"];
2126
if (!authorizationHeader || !(typeof authorizationHeader === "string")) {
2227
return;
@@ -51,7 +56,7 @@ export class BearerAuth {
5156
get restHandler(): express.RequestHandler {
5257
return async (req, res, next) => {
5358
try {
54-
await this.auth(req);
59+
await this.authExpressRequest(req);
5560
} catch (e) {
5661
if (isBearerAuthError(e)) {
5762
// (AT) while investigating https://github.com/gitpod-io/gitpod/issues/8703 we
@@ -71,15 +76,15 @@ export class BearerAuth {
7176
get restHandlerOptionally(): express.RequestHandler {
7277
return async (req, res, next) => {
7378
try {
74-
await this.auth(req);
79+
await this.authExpressRequest(req);
7580
} catch (e) {
7681
// don't error the request, we just have not bearer authentication token
7782
}
7883
return next();
7984
};
8085
}
8186

82-
async auth(req: express.Request): Promise<void> {
87+
async authExpressRequest(req: express.Request): Promise<void> {
8388
const token = getBearerToken(req.headers);
8489
if (!token) {
8590
throw createBearerAuthError("missing Bearer token");
@@ -90,10 +95,8 @@ export class BearerAuth {
9095
const resourceGuard = new TokenResourceGuard(user.id, scopes);
9196
(req as WithResourceAccessGuard).resourceGuard = resourceGuard;
9297

93-
const functionScopes = scopes
94-
.filter((s) => s.startsWith("function:"))
95-
.map((s) => s.substring("function:".length));
96-
if (functionScopes.length === 1 && functionScopes[0] === "*") {
98+
const { isAllAccessFunctionGuard, functionScopes } = FunctionAccessGuard.extractFunctionScopes(scopes);
99+
if (isAllAccessFunctionGuard) {
97100
(req as WithFunctionAccessGuard).functionGuard = new AllAccessFunctionGuard();
98101
} else {
99102
// We always install a function access guard. If the token has no scopes, it's not allowed to do anything.
@@ -103,6 +106,22 @@ export class BearerAuth {
103106
req.user = user;
104107
}
105108

109+
async tryAuthFromHeaders(headers: { [key: string]: any }): Promise<SubjectId | undefined> {
110+
const token = getBearerToken(headers);
111+
if (!token) {
112+
return undefined;
113+
}
114+
const { user, scopes } = await this.userAndScopesFromToken(token);
115+
116+
// gpl: Once we move PAT to FGA-backed scopes, this special case will go away, and covered by a different SubjectIdKind.
117+
const { isAllAccessFunctionGuard } = FunctionAccessGuard.extractFunctionScopes(scopes);
118+
if (!isAllAccessFunctionGuard) {
119+
return undefined;
120+
}
121+
122+
return SubjectId.fromUserId(user.id);
123+
}
124+
106125
private async userAndScopesFromToken(token: string): Promise<{ user: User; scopes: string[] }> {
107126
// We handle two types of Bearer tokens:
108127
// 1. Personal Access Tokens which are prefixed with `gitpod_pat_`

components/server/src/auth/function-access.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,19 @@ export interface FunctionAccessGuard {
1010
canAccess(name: string): boolean;
1111
}
1212

13+
export namespace FunctionAccessGuard {
14+
export function extractFunctionScopes(scopes: string[]): {
15+
functionScopes: string[];
16+
isAllAccessFunctionGuard: boolean;
17+
} {
18+
const functionScopes = scopes
19+
.filter((s) => s.startsWith("function:"))
20+
.map((s) => s.substring("function:".length));
21+
const isAllAccessFunctionGuard = functionScopes.length === 1 && functionScopes[0] === "*";
22+
return { functionScopes, isAllAccessFunctionGuard };
23+
}
24+
}
25+
1326
export interface WithFunctionAccessGuard {
1427
functionGuard?: FunctionAccessGuard;
1528
}

components/server/src/authorization/authorizer.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -503,11 +503,13 @@ export class Authorizer {
503503
}
504504
}
505505

506-
export async function isFgaChecksEnabled(userId: string): Promise<boolean> {
506+
export async function isFgaChecksEnabled(userId: string | undefined): Promise<boolean> {
507507
return getExperimentsClientForBackend().getValueAsync("centralizedPermissions", false, {
508-
user: {
509-
id: userId,
510-
},
508+
user: userId
509+
? {
510+
id: userId,
511+
}
512+
: undefined,
511513
});
512514
}
513515

components/server/src/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ export class Server {
191191
}
192192

193193
try {
194-
await this.bearerAuth.auth(info.req as express.Request);
194+
await this.bearerAuth.authExpressRequest(info.req as express.Request);
195195
authenticatedUsingBearerToken = true;
196196
} catch (e) {
197197
if (isBearerAuthError(e)) {

0 commit comments

Comments
 (0)