Skip to content

Commit c279fc4

Browse files
geroplgeropl
authored
[gRPC] Enable PATs to be used for authorizing with the gRPC API (#19081)
* [server] Make gRPC accept PAT tokens (incl. tests) * [gpctl] Switch "gpctl api ws ls/get" to use v1/v1connect
1 parent c00f28a commit c279fc4

14 files changed

+311
-69
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.spec.db.ts

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/**
2+
* Copyright (c) 2022 Gitpod GmbH. All rights reserved.
3+
* Licensed under the GNU Affero General Public License (AGPL).
4+
* See License.AGPL.txt in the project root for license information.
5+
*/
6+
7+
import { TypeORM, resetDB } from "@gitpod/gitpod-db/lib";
8+
import { BearerAuth, PersonalAccessToken } from "./bearer-authenticator";
9+
import { expect } from "chai";
10+
import { describe } from "mocha";
11+
import { Container } from "inversify";
12+
import { createTestContainer } from "../test/service-testing-container-module";
13+
import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
14+
import { UserService } from "../user/user-service";
15+
import { User } from "@gitpod/gitpod-protocol";
16+
import { Config } from "../config";
17+
import { Request } from "express";
18+
import { WithResourceAccessGuard } from "./resource-access";
19+
import { WithFunctionAccessGuard } from "./function-access";
20+
import { fail } from "assert";
21+
import { SubjectId } from "./subject-id";
22+
23+
function toDateTime(date: Date): string {
24+
return date.toISOString().replace("T", " ").replace("Z", "");
25+
}
26+
27+
describe("BearerAuth", () => {
28+
let container: Container;
29+
let bearerAuth: BearerAuth;
30+
let userService: UserService;
31+
let typeORM: TypeORM;
32+
let testUser: User;
33+
34+
async function insertPat(userId: string, patId: string, scopes: string[] = ["function:*"]): Promise<string> {
35+
const patValue = "someValue";
36+
const signature = "V7BsZVjpMQRaWxS5XJE9r-Ovpxk2xT_bfFSmic4yW6g"; // depends on the value
37+
const pat = new PersonalAccessToken("doesnotmatter", patValue);
38+
39+
const conn = await typeORM.getConnection();
40+
await conn.query(
41+
"INSERT d_b_personal_access_token (id, userId, hash, name, scopes, expirationTime, createdAt) VALUES (?, ?, ?, ?, ?, ?, ?)",
42+
[patId, userId, pat.hash(), patId, scopes, toDateTime(new Date("2030")), toDateTime(new Date())],
43+
);
44+
return `gitpod_pat_${signature}.${patValue}`;
45+
}
46+
47+
beforeEach(async () => {
48+
container = createTestContainer();
49+
Experiments.configureTestingClient({
50+
centralizedPermissions: true,
51+
});
52+
const oldConfig = container.get<Config>(Config);
53+
container.rebind(Config).toDynamicValue((ctx) => {
54+
return {
55+
...oldConfig,
56+
patSigningKey: "super-duper-secret-pat-signing-key",
57+
};
58+
});
59+
bearerAuth = container.get(BearerAuth);
60+
userService = container.get<UserService>(UserService);
61+
typeORM = container.get<TypeORM>(TypeORM);
62+
63+
testUser = await userService.createUser({
64+
identity: {
65+
authId: "gh-user-1",
66+
authName: "testUser",
67+
authProviderId: "public-github",
68+
},
69+
});
70+
});
71+
72+
afterEach(async () => {
73+
// Clean-up database
74+
await resetDB(container.get(TypeORM));
75+
});
76+
77+
it("authExpressRequest should successfully authenticate BearerToken (PAT)", async () => {
78+
const pat1 = await insertPat(testUser.id, "pat-1");
79+
80+
const req = {
81+
headers: {
82+
authorization: `Bearer ${pat1}`,
83+
},
84+
} as Request;
85+
await bearerAuth.authExpressRequest(req);
86+
87+
expect(req.user?.id).to.equal(testUser.id);
88+
expect((req as WithResourceAccessGuard).resourceGuard).to.not.be.undefined;
89+
expect((req as WithFunctionAccessGuard).functionGuard).to.not.be.undefined;
90+
});
91+
92+
it("authExpressRequest should fail to authenticate with missing BearerToken in header", async () => {
93+
await insertPat(testUser.id, "pat-1");
94+
95+
const req = {
96+
headers: {
97+
authorization: `Bearer `, // missing
98+
},
99+
} as Request;
100+
await expectError(async () => bearerAuth.authExpressRequest(req), "missing bearer token header");
101+
});
102+
103+
it("authExpressRequest should fail to authenticate with missing BearerToken from DB (PAT)", async () => {
104+
const patNotStored = "gitpod_pat_GrvGthczSRf3ypqFhNtcRiN5fK6CV7rdCkkPLfpbc_4";
105+
106+
const req = {
107+
headers: {
108+
authorization: `Bearer ${patNotStored}`,
109+
},
110+
} as Request;
111+
await expectError(async () => bearerAuth.authExpressRequest(req), "cannot find token");
112+
});
113+
114+
it("tryAuthFromHeaders should successfully authenticate BearerToken (PAT)", async () => {
115+
const pat1 = await insertPat(testUser.id, "pat-1");
116+
117+
const headers = new Headers();
118+
headers.set("authorization", `Bearer ${pat1}`);
119+
const subjectId = await bearerAuth.tryAuthFromHeaders(headers);
120+
121+
expect(subjectId?.toString()).to.equal(SubjectId.fromUserId(testUser.id).toString());
122+
});
123+
124+
it("tryAuthFromHeaders should return undefined with missing BearerToken in header", async () => {
125+
await insertPat(testUser.id, "pat-1");
126+
127+
const headers = new Headers();
128+
headers.set("authorization", `Bearer `); // missing
129+
expect(await bearerAuth.tryAuthFromHeaders(headers)).to.be.undefined;
130+
});
131+
132+
it("tryAuthFromHeaders should fail to authenticate with missing BearerToken from DB (PAT)", async () => {
133+
const patNotStored = "gitpod_pat_GrvGthczSRf3ypqFhNtcRiN5fK6CV7rdCkkPLfpbc_4";
134+
135+
const headers = new Headers();
136+
headers.set("authorization", `Bearer ${patNotStored}`);
137+
await expectError(async () => bearerAuth.tryAuthFromHeaders(headers), "cannot find token");
138+
});
139+
140+
async function expectError(fun: () => Promise<any>, message: string) {
141+
try {
142+
await fun();
143+
fail(`Expected error: ${message}`);
144+
} catch (err) {}
145+
}
146+
});

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

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,24 @@ 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 {
20-
const authorizationHeader = headers["authorization"];
24+
export function getBearerToken(authorizationHeader: string | undefined | null): string | undefined {
2125
if (!authorizationHeader || !(typeof authorizationHeader === "string")) {
22-
return;
26+
return undefined;
2327
}
2428
if (!authorizationHeader.startsWith("Bearer ")) {
25-
return;
29+
return undefined;
2630
}
2731

2832
return authorizationHeader.substring("Bearer ".length);
@@ -51,7 +55,7 @@ export class BearerAuth {
5155
get restHandler(): express.RequestHandler {
5256
return async (req, res, next) => {
5357
try {
54-
await this.auth(req);
58+
await this.authExpressRequest(req);
5559
} catch (e) {
5660
if (isBearerAuthError(e)) {
5761
// (AT) while investigating https://github.com/gitpod-io/gitpod/issues/8703 we
@@ -71,16 +75,16 @@ export class BearerAuth {
7175
get restHandlerOptionally(): express.RequestHandler {
7276
return async (req, res, next) => {
7377
try {
74-
await this.auth(req);
78+
await this.authExpressRequest(req);
7579
} catch (e) {
7680
// don't error the request, we just have not bearer authentication token
7781
}
7882
return next();
7983
};
8084
}
8185

82-
async auth(req: express.Request): Promise<void> {
83-
const token = getBearerToken(req.headers);
86+
async authExpressRequest(req: express.Request): Promise<void> {
87+
const token = getBearerToken(req.headers["authorization"]);
8488
if (!token) {
8589
throw createBearerAuthError("missing Bearer token");
8690
}
@@ -90,10 +94,8 @@ export class BearerAuth {
9094
const resourceGuard = new TokenResourceGuard(user.id, scopes);
9195
(req as WithResourceAccessGuard).resourceGuard = resourceGuard;
9296

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

108+
async tryAuthFromHeaders(headers: Headers): Promise<SubjectId | undefined> {
109+
const token = getBearerToken(headers.get("authorization"));
110+
if (!token) {
111+
return undefined;
112+
}
113+
const { user, scopes } = await this.userAndScopesFromToken(token);
114+
115+
// gpl: Once we move PAT to FGA-backed scopes, this special case will go away, and covered by a different SubjectIdKind.
116+
const { isAllAccessFunctionGuard } = FunctionAccessGuard.extractFunctionScopes(scopes);
117+
if (!isAllAccessFunctionGuard) {
118+
return undefined;
119+
}
120+
121+
return SubjectId.fromUserId(user.id);
122+
}
123+
106124
private async userAndScopesFromToken(token: string): Promise<{ user: User; scopes: string[] }> {
107125
// We handle two types of Bearer tokens:
108126
// 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
}

0 commit comments

Comments
 (0)