Skip to content

Commit c2e498b

Browse files
svenefftingesvenefftinge
authored
[server/fga] centralize lookup of findUserById (#18382)
1 parent 58da110 commit c2e498b

13 files changed

+171
-185
lines changed

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

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,33 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7-
import * as express from "express";
8-
import * as passport from "passport";
9-
import { injectable, postConstruct, inject } from "inversify";
7+
import { TeamDB } from "@gitpod/gitpod-db/lib";
108
import { User } from "@gitpod/gitpod-protocol";
119
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
12-
import { TeamDB, UserDB } from "@gitpod/gitpod-db/lib";
10+
import * as express from "express";
11+
import { inject, injectable, postConstruct } from "inversify";
12+
import * as passport from "passport";
1313
import { Config } from "../config";
14-
import { HostContextProvider } from "./host-context-provider";
15-
import { AuthFlow, AuthProvider } from "./auth-provider";
14+
import { reportLoginCompleted } from "../prometheus-metrics";
1615
import { TokenProvider } from "../user/token-provider";
17-
import { AuthProviderService } from "./auth-provider-service";
1816
import { UserAuthentication } from "../user/user-authentication";
17+
import { UserService } from "../user/user-service";
18+
import { AuthFlow, AuthProvider } from "./auth-provider";
19+
import { AuthProviderService } from "./auth-provider-service";
20+
import { HostContextProvider } from "./host-context-provider";
1921
import { SignInJWT } from "./jwt";
20-
import { reportLoginCompleted } from "../prometheus-metrics";
2122

2223
@injectable()
2324
export class Authenticator {
2425
protected passportInitialize: express.Handler;
2526

2627
@inject(Config) protected readonly config: Config;
27-
@inject(UserDB) protected userDb: UserDB;
28+
@inject(UserService) protected userService: UserService;
2829
@inject(TeamDB) protected teamDb: TeamDB;
2930
@inject(HostContextProvider) protected hostContextProvider: HostContextProvider;
3031
@inject(TokenProvider) protected readonly tokenProvider: TokenProvider;
3132
@inject(AuthProviderService) protected readonly authProviderService: AuthProviderService;
32-
@inject(UserAuthentication) protected readonly userService: UserAuthentication;
33+
@inject(UserAuthentication) protected readonly userAuthentication: UserAuthentication;
3334
@inject(SignInJWT) protected readonly signInJWT: SignInJWT;
3435

3536
@postConstruct()
@@ -45,12 +46,9 @@ export class Authenticator {
4546
});
4647
passport.deserializeUser(async (id, done) => {
4748
try {
48-
const user = await this.userDb.findUserById(id as string);
49-
if (user) {
50-
done(null, user);
51-
} else {
52-
done(new Error("User not found."));
53-
}
49+
const userId = id as string;
50+
const user = await this.userService.findUserById(userId, userId);
51+
done(null, user);
5452
} catch (err) {
5553
done(err);
5654
}
@@ -202,7 +200,7 @@ export class Authenticator {
202200
}
203201

204202
try {
205-
await this.userService.deauthorize(user, authProvider.authProviderId);
203+
await this.userAuthentication.deauthorize(user, authProvider.authProviderId);
206204
res.redirect(returnTo);
207205
} catch (error) {
208206
next(error);

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { inject, injectable } from "inversify";
1414
import { Config } from "../config";
1515
import { AllAccessFunctionGuard, ExplicitFunctionAccessGuard, WithFunctionAccessGuard } from "./function-access";
1616
import { TokenResourceGuard, WithResourceAccessGuard } from "./resource-access";
17+
import { UserService } from "../user/user-service";
1718

1819
export function getBearerToken(headers: IncomingHttpHeaders): string | undefined {
1920
const authorizationHeader = headers["authorization"];
@@ -40,9 +41,12 @@ function createBearerAuthError(message: string): BearerAuthError {
4041

4142
@injectable()
4243
export class BearerAuth {
43-
@inject(Config) protected readonly config: Config;
44-
@inject(UserDB) protected readonly userDB: UserDB;
45-
@inject(PersonalAccessTokenDB) protected readonly personalAccessTokenDB: PersonalAccessTokenDB;
44+
constructor(
45+
@inject(Config) private readonly config: Config,
46+
@inject(UserDB) private readonly userDB: UserDB,
47+
@inject(UserService) private readonly userService: UserService,
48+
@inject(PersonalAccessTokenDB) private readonly personalAccessTokenDB: PersonalAccessTokenDB,
49+
) {}
4650

4751
get restHandler(): express.RequestHandler {
4852
return async (req, res, next) => {
@@ -123,11 +127,7 @@ export class BearerAuth {
123127
throw new Error("Failed to find PAT by hash");
124128
}
125129

126-
const userByID = await this.userDB.findUserById(pat.userId);
127-
if (!userByID) {
128-
throw new Error("Failed to find user referenced by PAT");
129-
}
130-
130+
const userByID = await this.userService.findUserById(pat.userId, pat.userId);
131131
return { user: userByID, scopes: pat.scopes };
132132
} catch (e) {
133133
log.error("Failed to authenticate using PAT", e);
@@ -142,9 +142,8 @@ export class BearerAuth {
142142
throw createBearerAuthError("invalid Bearer token");
143143
}
144144

145-
// hack: load the user again to get ahold of all identities
146-
// TODO(cw): instead of re-loading the user, we should properly join the identities in findUserByGitpodToken
147-
const user = (await this.userDB.findUserById(userAndToken.user.id))!;
145+
// load the user through user-service again to get ahold of all identities
146+
const user = await this.userService.findUserById(userAndToken.user.id, userAndToken.user.id);
148147
return { user, scopes: userAndToken.token.scopes };
149148
}
150149
}

‎components/server/src/dev/authenticator-dev.ts

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,24 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7+
import { AuthProviderInfo } from "@gitpod/gitpod-protocol";
78
import * as express from "express";
8-
import { injectable, inject } from "inversify";
9-
import { UserDB } from "@gitpod/gitpod-db/lib";
9+
import { injectable } from "inversify";
1010
import { Strategy as DummyStrategy } from "passport-dummy";
11-
import { ErrorCodes, ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
12-
import { Authenticator } from "../auth/authenticator";
1311
import { AuthProvider } from "../auth/auth-provider";
14-
import { AuthProviderInfo } from "@gitpod/gitpod-protocol";
12+
import { Authenticator } from "../auth/authenticator";
13+
import { UserService } from "../user/user-service";
1514
import { DevData } from "./dev-data";
1615

1716
@injectable()
1817
export class AuthenticatorDevImpl extends Authenticator {
19-
@inject(UserDB) protected userDb: UserDB;
20-
2118
protected async getAuthProviderForHost(_host: string): Promise<AuthProvider | undefined> {
22-
return new DummyAuthProvider(this.userDb);
19+
return new DummyAuthProvider(this.userService);
2320
}
2421
}
2522

2623
class DummyAuthProvider implements AuthProvider {
27-
constructor(protected userDb: UserDB) {}
24+
constructor(protected userService: UserService) {}
2825
get info(): AuthProviderInfo {
2926
return {
3027
authProviderId: "Public-GitHub",
@@ -43,12 +40,10 @@ class DummyAuthProvider implements AuthProvider {
4340
throw new Error("Method not implemented.");
4441
};
4542
readonly strategy = new DummyStrategy(async (done) => {
46-
const maybeUser = await this.userDb.findUserById(DevData.createTestUser().id);
47-
if (!maybeUser) {
48-
done(new ApplicationError(ErrorCodes.NOT_AUTHENTICATED, "No dev user in DB."), undefined);
49-
}
43+
const testUser = DevData.createTestUser();
5044
try {
51-
done(undefined, maybeUser);
45+
const user = await this.userService.findUserById(testUser.id, testUser.id);
46+
done(undefined, user);
5247
} catch (err) {
5348
done(err, undefined);
5449
}

‎components/server/src/prebuilds/bitbucket-app.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import * as express from "express";
88
import { postConstruct, injectable, inject } from "inversify";
9-
import { ProjectDB, TeamDB, UserDB, WebhookEventDB } from "@gitpod/gitpod-db/lib";
9+
import { ProjectDB, TeamDB, WebhookEventDB } from "@gitpod/gitpod-db/lib";
1010
import { User, StartPrebuildResult, CommitContext, CommitInfo, Project, WebhookEvent } from "@gitpod/gitpod-protocol";
1111
import { PrebuildManager } from "./prebuild-manager";
1212
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
@@ -15,10 +15,12 @@ import { ContextParser } from "../workspace/context-parser-service";
1515
import { HostContextProvider } from "../auth/host-context-provider";
1616
import { RepoURL } from "../repohost";
1717
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
18+
import { UserService } from "../user/user-service";
19+
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1820

1921
@injectable()
2022
export class BitbucketApp {
21-
@inject(UserDB) protected readonly userDB: UserDB;
23+
@inject(UserService) protected readonly userService: UserService;
2224
@inject(PrebuildManager) protected readonly prebuildManager: PrebuildManager;
2325
@inject(TokenService) protected readonly tokenService: TokenService;
2426
@inject(ProjectDB) protected readonly projectDB: ProjectDB;
@@ -84,7 +86,7 @@ export class BitbucketApp {
8486
try {
8587
span.setTag("secret-token", secretToken);
8688
const [userid, tokenValue] = secretToken.split("|");
87-
const user = await this.userDB.findUserById(userid);
89+
const user = await this.userService.findUserById(userid, userid);
8890
if (!user) {
8991
throw new Error("No user found for " + secretToken + " found.");
9092
} else if (!!user.blocked) {
@@ -94,7 +96,7 @@ export class BitbucketApp {
9496
if (!identity) {
9597
throw new Error(`User ${user.id} has no identity for '${TokenService.GITPOD_AUTH_PROVIDER_ID}'.`);
9698
}
97-
const tokens = await this.userDB.findTokensForIdentity(identity);
99+
const tokens = await this.userService.findTokensForIdentity(userid, identity);
98100
const token = tokens.find((t) => t.token.value === tokenValue);
99101
if (!token) {
100102
throw new Error(`User ${user.id} has no token with given value.`);
@@ -193,25 +195,25 @@ export class BitbucketApp {
193195
cloneURL: string,
194196
webhookInstaller: User,
195197
): Promise<{ user: User; project?: Project }> {
196-
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
197-
if (project) {
198-
if (project.userId) {
199-
const user = await this.userDB.findUserById(project.userId);
200-
if (user) {
201-
return { user, project };
198+
try {
199+
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
200+
if (project) {
201+
if (!project.teamId) {
202+
throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "Project has no teamId");
202203
}
203-
} else if (project.teamId) {
204-
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId || "");
204+
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId);
205205
if (teamMembers.some((t) => t.userId === webhookInstaller.id)) {
206206
return { user: webhookInstaller, project };
207207
}
208208
for (const teamMember of teamMembers) {
209-
const user = await this.userDB.findUserById(teamMember.userId);
209+
const user = await this.userService.findUserById(teamMember.userId, teamMember.userId);
210210
if (user && user.identities.some((i) => i.authProviderId === "Public-Bitbucket")) {
211211
return { user, project };
212212
}
213213
}
214214
}
215+
} catch (err) {
216+
log.info({ userId: webhookInstaller.id }, "Failed to find project and owner", err);
215217
}
216218
return { user: webhookInstaller };
217219
}

‎components/server/src/prebuilds/bitbucket-server-app.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import * as express from "express";
88
import { postConstruct, injectable, inject } from "inversify";
9-
import { ProjectDB, TeamDB, UserDB, WebhookEventDB } from "@gitpod/gitpod-db/lib";
9+
import { ProjectDB, TeamDB, WebhookEventDB } from "@gitpod/gitpod-db/lib";
1010
import { PrebuildManager } from "./prebuild-manager";
1111
import { TokenService } from "../user/token-service";
1212
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
@@ -15,10 +15,12 @@ import { RepoURL } from "../repohost";
1515
import { HostContextProvider } from "../auth/host-context-provider";
1616
import { ContextParser } from "../workspace/context-parser-service";
1717
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
18+
import { UserService } from "../user/user-service";
19+
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1820

1921
@injectable()
2022
export class BitbucketServerApp {
21-
@inject(UserDB) protected readonly userDB: UserDB;
23+
@inject(UserService) protected readonly userService: UserService;
2224
@inject(PrebuildManager) protected readonly prebuildManager: PrebuildManager;
2325
@inject(TokenService) protected readonly tokenService: TokenService;
2426
@inject(ProjectDB) protected readonly projectDB: ProjectDB;
@@ -81,7 +83,7 @@ export class BitbucketServerApp {
8183
try {
8284
span.setTag("secret-token", secretToken);
8385
const [userid, tokenValue] = secretToken.split("|");
84-
const user = await this.userDB.findUserById(userid);
86+
const user = await this.userService.findUserById(userid, userid);
8587
if (!user) {
8688
throw new Error("No user found for " + secretToken + " found.");
8789
} else if (!!user.blocked) {
@@ -91,7 +93,7 @@ export class BitbucketServerApp {
9193
if (!identity) {
9294
throw new Error(`User ${user.id} has no identity for '${TokenService.GITPOD_AUTH_PROVIDER_ID}'.`);
9395
}
94-
const tokens = await this.userDB.findTokensForIdentity(identity);
96+
const tokens = await this.userService.findTokensForIdentity(userid, identity);
9597
const token = tokens.find((t) => t.token.value === tokenValue);
9698
if (!token) {
9799
throw new Error(`User ${user.id} has no token with given value.`);
@@ -205,25 +207,25 @@ export class BitbucketServerApp {
205207
cloneURL: string,
206208
webhookInstaller: User,
207209
): Promise<{ user: User; project?: Project }> {
208-
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
209-
if (project) {
210-
if (project.userId) {
211-
const user = await this.userDB.findUserById(project.userId);
212-
if (user) {
213-
return { user, project };
210+
try {
211+
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
212+
if (project) {
213+
if (!project.teamId) {
214+
throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "Project has no teamId.");
214215
}
215-
} else if (project.teamId) {
216-
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId || "");
216+
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId);
217217
if (teamMembers.some((t) => t.userId === webhookInstaller.id)) {
218218
return { user: webhookInstaller, project };
219219
}
220220
for (const teamMember of teamMembers) {
221-
const user = await this.userDB.findUserById(teamMember.userId);
221+
const user = await this.userService.findUserById(webhookInstaller.id, teamMember.userId);
222222
if (user && user.identities.some((i) => i.authProviderId === "Public-Bitbucket")) {
223223
return { user, project };
224224
}
225225
}
226226
}
227+
} catch (err) {
228+
log.info({ userId: webhookInstaller.id }, "Failed to find project and owner", err);
227229
}
228230
return { user: webhookInstaller };
229231
}

‎components/server/src/prebuilds/github-app.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { ContextParser } from "../workspace/context-parser-service";
3939
import { HostContextProvider } from "../auth/host-context-provider";
4040
import { RepoURL } from "../repohost";
4141
import { ApplicationError, ErrorCode } from "@gitpod/gitpod-protocol/lib/messaging/error";
42+
import { UserService } from "../user/user-service";
4243

4344
/**
4445
* GitHub app urls:
@@ -54,8 +55,9 @@ import { ApplicationError, ErrorCode } from "@gitpod/gitpod-protocol/lib/messagi
5455
export class GithubApp {
5556
@inject(ProjectDB) protected readonly projectDB: ProjectDB;
5657
@inject(TeamDB) protected readonly teamDB: TeamDB;
57-
@inject(AppInstallationDB) protected readonly appInstallationDB: AppInstallationDB;
5858
@inject(UserDB) protected readonly userDB: UserDB;
59+
@inject(AppInstallationDB) protected readonly appInstallationDB: AppInstallationDB;
60+
@inject(UserService) protected readonly userService: UserService;
5961
@inject(TracedWorkspaceDB) protected readonly workspaceDB: DBWithTracing<WorkspaceDB>;
6062
@inject(GithubAppRules) protected readonly appRules: GithubAppRules;
6163
@inject(PrebuildManager) protected readonly prebuildManager: PrebuildManager;
@@ -646,7 +648,7 @@ export class GithubApp {
646648
return installationOwner;
647649
}
648650
for (const teamMember of teamMembers) {
649-
const user = await this.userDB.findUserById(teamMember.userId);
651+
const user = await this.userService.findUserById(teamMember.userId, teamMember.userId);
650652
if (user && user.identities.some((i) => i.authProviderId === "Public-GitHub")) {
651653
return user;
652654
}
@@ -667,7 +669,7 @@ export class GithubApp {
667669
}
668670

669671
const ownerID = installation.ownerUserID || "this-should-never-happen";
670-
const user = await this.userDB.findUserById(ownerID);
672+
const user = await this.userService.findUserById(ownerID, ownerID);
671673
if (!user) {
672674
return;
673675
}

0 commit comments

Comments
 (0)