Skip to content

[server/fga] centralize lookup of findUserById #18382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 31, 2023
Merged

[server/fga] centralize lookup of findUserById #18382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 15 additions & 17 deletions components/server/src/auth/authenticator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,33 @@
* See License.AGPL.txt in the project root for license information.
*/

import * as express from "express";
import * as passport from "passport";
import { injectable, postConstruct, inject } from "inversify";
import { TeamDB } from "@gitpod/gitpod-db/lib";
import { User } from "@gitpod/gitpod-protocol";
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
import { TeamDB, UserDB } from "@gitpod/gitpod-db/lib";
import * as express from "express";
import { inject, injectable, postConstruct } from "inversify";
import * as passport from "passport";
import { Config } from "../config";
import { HostContextProvider } from "./host-context-provider";
import { AuthFlow, AuthProvider } from "./auth-provider";
import { reportLoginCompleted } from "../prometheus-metrics";
import { TokenProvider } from "../user/token-provider";
import { AuthProviderService } from "./auth-provider-service";
import { UserAuthentication } from "../user/user-authentication";
import { UserService } from "../user/user-service";
import { AuthFlow, AuthProvider } from "./auth-provider";
import { AuthProviderService } from "./auth-provider-service";
import { HostContextProvider } from "./host-context-provider";
import { SignInJWT } from "./jwt";
import { reportLoginCompleted } from "../prometheus-metrics";

@injectable()
export class Authenticator {
protected passportInitialize: express.Handler;

@inject(Config) protected readonly config: Config;
@inject(UserDB) protected userDb: UserDB;
@inject(UserService) protected userService: UserService;
@inject(TeamDB) protected teamDb: TeamDB;
@inject(HostContextProvider) protected hostContextProvider: HostContextProvider;
@inject(TokenProvider) protected readonly tokenProvider: TokenProvider;
@inject(AuthProviderService) protected readonly authProviderService: AuthProviderService;
@inject(UserAuthentication) protected readonly userService: UserAuthentication;
@inject(UserAuthentication) protected readonly userAuthentication: UserAuthentication;
@inject(SignInJWT) protected readonly signInJWT: SignInJWT;

@postConstruct()
Expand All @@ -45,12 +46,9 @@ export class Authenticator {
});
passport.deserializeUser(async (id, done) => {
try {
const user = await this.userDb.findUserById(id as string);
if (user) {
done(null, user);
} else {
done(new Error("User not found."));
}
const userId = id as string;
const user = await this.userService.findUserById(userId, userId);
done(null, user);
} catch (err) {
done(err);
}
Expand Down Expand Up @@ -202,7 +200,7 @@ export class Authenticator {
}

try {
await this.userService.deauthorize(user, authProvider.authProviderId);
await this.userAuthentication.deauthorize(user, authProvider.authProviderId);
res.redirect(returnTo);
} catch (error) {
next(error);
Expand Down
21 changes: 10 additions & 11 deletions components/server/src/auth/bearer-authenticator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { inject, injectable } from "inversify";
import { Config } from "../config";
import { AllAccessFunctionGuard, ExplicitFunctionAccessGuard, WithFunctionAccessGuard } from "./function-access";
import { TokenResourceGuard, WithResourceAccessGuard } from "./resource-access";
import { UserService } from "../user/user-service";

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

@injectable()
export class BearerAuth {
@inject(Config) protected readonly config: Config;
@inject(UserDB) protected readonly userDB: UserDB;
@inject(PersonalAccessTokenDB) protected readonly personalAccessTokenDB: PersonalAccessTokenDB;
constructor(
@inject(Config) private readonly config: Config,
@inject(UserDB) private readonly userDB: UserDB,
@inject(UserService) private readonly userService: UserService,
@inject(PersonalAccessTokenDB) private readonly personalAccessTokenDB: PersonalAccessTokenDB,
) {}

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

const userByID = await this.userDB.findUserById(pat.userId);
if (!userByID) {
throw new Error("Failed to find user referenced by PAT");
}

const userByID = await this.userService.findUserById(pat.userId, pat.userId);
return { user: userByID, scopes: pat.scopes };
} catch (e) {
log.error("Failed to authenticate using PAT", e);
Expand All @@ -142,9 +142,8 @@ export class BearerAuth {
throw createBearerAuthError("invalid Bearer token");
}

// hack: load the user again to get ahold of all identities
// TODO(cw): instead of re-loading the user, we should properly join the identities in findUserByGitpodToken
const user = (await this.userDB.findUserById(userAndToken.user.id))!;
// load the user through user-service again to get ahold of all identities
const user = await this.userService.findUserById(userAndToken.user.id, userAndToken.user.id);
return { user, scopes: userAndToken.token.scopes };
}
}
Expand Down
23 changes: 9 additions & 14 deletions components/server/src/dev/authenticator-dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,24 @@
* See License.AGPL.txt in the project root for license information.
*/

import { AuthProviderInfo } from "@gitpod/gitpod-protocol";
import * as express from "express";
import { injectable, inject } from "inversify";
import { UserDB } from "@gitpod/gitpod-db/lib";
import { injectable } from "inversify";
import { Strategy as DummyStrategy } from "passport-dummy";
import { ErrorCodes, ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { Authenticator } from "../auth/authenticator";
import { AuthProvider } from "../auth/auth-provider";
import { AuthProviderInfo } from "@gitpod/gitpod-protocol";
import { Authenticator } from "../auth/authenticator";
import { UserService } from "../user/user-service";
import { DevData } from "./dev-data";

@injectable()
export class AuthenticatorDevImpl extends Authenticator {
@inject(UserDB) protected userDb: UserDB;

protected async getAuthProviderForHost(_host: string): Promise<AuthProvider | undefined> {
return new DummyAuthProvider(this.userDb);
return new DummyAuthProvider(this.userService);
}
}

class DummyAuthProvider implements AuthProvider {
constructor(protected userDb: UserDB) {}
constructor(protected userService: UserService) {}
get info(): AuthProviderInfo {
return {
authProviderId: "Public-GitHub",
Expand All @@ -43,12 +40,10 @@ class DummyAuthProvider implements AuthProvider {
throw new Error("Method not implemented.");
};
readonly strategy = new DummyStrategy(async (done) => {
const maybeUser = await this.userDb.findUserById(DevData.createTestUser().id);
if (!maybeUser) {
done(new ApplicationError(ErrorCodes.NOT_AUTHENTICATED, "No dev user in DB."), undefined);
}
const testUser = DevData.createTestUser();
try {
done(undefined, maybeUser);
const user = await this.userService.findUserById(testUser.id, testUser.id);
done(undefined, user);
} catch (err) {
done(err, undefined);
}
Expand Down
28 changes: 15 additions & 13 deletions components/server/src/prebuilds/bitbucket-app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

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

@injectable()
export class BitbucketApp {
@inject(UserDB) protected readonly userDB: UserDB;
@inject(UserService) protected readonly userService: UserService;
@inject(PrebuildManager) protected readonly prebuildManager: PrebuildManager;
@inject(TokenService) protected readonly tokenService: TokenService;
@inject(ProjectDB) protected readonly projectDB: ProjectDB;
Expand Down Expand Up @@ -84,7 +86,7 @@ export class BitbucketApp {
try {
span.setTag("secret-token", secretToken);
const [userid, tokenValue] = secretToken.split("|");
const user = await this.userDB.findUserById(userid);
const user = await this.userService.findUserById(userid, userid);
if (!user) {
throw new Error("No user found for " + secretToken + " found.");
} else if (!!user.blocked) {
Expand All @@ -94,7 +96,7 @@ export class BitbucketApp {
if (!identity) {
throw new Error(`User ${user.id} has no identity for '${TokenService.GITPOD_AUTH_PROVIDER_ID}'.`);
}
const tokens = await this.userDB.findTokensForIdentity(identity);
const tokens = await this.userService.findTokensForIdentity(userid, identity);
const token = tokens.find((t) => t.token.value === tokenValue);
if (!token) {
throw new Error(`User ${user.id} has no token with given value.`);
Expand Down Expand Up @@ -193,25 +195,25 @@ export class BitbucketApp {
cloneURL: string,
webhookInstaller: User,
): Promise<{ user: User; project?: Project }> {
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
if (project) {
if (project.userId) {
const user = await this.userDB.findUserById(project.userId);
if (user) {
return { user, project };
try {
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
if (project) {
if (!project.teamId) {
throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "Project has no teamId");
}
} else if (project.teamId) {
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId || "");
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId);
if (teamMembers.some((t) => t.userId === webhookInstaller.id)) {
return { user: webhookInstaller, project };
}
for (const teamMember of teamMembers) {
const user = await this.userDB.findUserById(teamMember.userId);
const user = await this.userService.findUserById(teamMember.userId, teamMember.userId);
if (user && user.identities.some((i) => i.authProviderId === "Public-Bitbucket")) {
return { user, project };
}
}
}
} catch (err) {
log.info({ userId: webhookInstaller.id }, "Failed to find project and owner", err);
}
return { user: webhookInstaller };
}
Expand Down
28 changes: 15 additions & 13 deletions components/server/src/prebuilds/bitbucket-server-app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

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

@injectable()
export class BitbucketServerApp {
@inject(UserDB) protected readonly userDB: UserDB;
@inject(UserService) protected readonly userService: UserService;
@inject(PrebuildManager) protected readonly prebuildManager: PrebuildManager;
@inject(TokenService) protected readonly tokenService: TokenService;
@inject(ProjectDB) protected readonly projectDB: ProjectDB;
Expand Down Expand Up @@ -81,7 +83,7 @@ export class BitbucketServerApp {
try {
span.setTag("secret-token", secretToken);
const [userid, tokenValue] = secretToken.split("|");
const user = await this.userDB.findUserById(userid);
const user = await this.userService.findUserById(userid, userid);
if (!user) {
throw new Error("No user found for " + secretToken + " found.");
} else if (!!user.blocked) {
Expand All @@ -91,7 +93,7 @@ export class BitbucketServerApp {
if (!identity) {
throw new Error(`User ${user.id} has no identity for '${TokenService.GITPOD_AUTH_PROVIDER_ID}'.`);
}
const tokens = await this.userDB.findTokensForIdentity(identity);
const tokens = await this.userService.findTokensForIdentity(userid, identity);
const token = tokens.find((t) => t.token.value === tokenValue);
if (!token) {
throw new Error(`User ${user.id} has no token with given value.`);
Expand Down Expand Up @@ -205,25 +207,25 @@ export class BitbucketServerApp {
cloneURL: string,
webhookInstaller: User,
): Promise<{ user: User; project?: Project }> {
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
if (project) {
if (project.userId) {
const user = await this.userDB.findUserById(project.userId);
if (user) {
return { user, project };
try {
const project = await this.projectDB.findProjectByCloneUrl(cloneURL);
if (project) {
if (!project.teamId) {
throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "Project has no teamId.");
}
} else if (project.teamId) {
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId || "");
const teamMembers = await this.teamDB.findMembersByTeam(project.teamId);
if (teamMembers.some((t) => t.userId === webhookInstaller.id)) {
return { user: webhookInstaller, project };
}
for (const teamMember of teamMembers) {
const user = await this.userDB.findUserById(teamMember.userId);
const user = await this.userService.findUserById(webhookInstaller.id, teamMember.userId);
if (user && user.identities.some((i) => i.authProviderId === "Public-Bitbucket")) {
return { user, project };
}
}
}
} catch (err) {
log.info({ userId: webhookInstaller.id }, "Failed to find project and owner", err);
}
return { user: webhookInstaller };
}
Expand Down
8 changes: 5 additions & 3 deletions components/server/src/prebuilds/github-app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { ContextParser } from "../workspace/context-parser-service";
import { HostContextProvider } from "../auth/host-context-provider";
import { RepoURL } from "../repohost";
import { ApplicationError, ErrorCode } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { UserService } from "../user/user-service";

/**
* GitHub app urls:
Expand All @@ -54,8 +55,9 @@ import { ApplicationError, ErrorCode } from "@gitpod/gitpod-protocol/lib/messagi
export class GithubApp {
@inject(ProjectDB) protected readonly projectDB: ProjectDB;
@inject(TeamDB) protected readonly teamDB: TeamDB;
@inject(AppInstallationDB) protected readonly appInstallationDB: AppInstallationDB;
@inject(UserDB) protected readonly userDB: UserDB;
@inject(AppInstallationDB) protected readonly appInstallationDB: AppInstallationDB;
@inject(UserService) protected readonly userService: UserService;
@inject(TracedWorkspaceDB) protected readonly workspaceDB: DBWithTracing<WorkspaceDB>;
@inject(GithubAppRules) protected readonly appRules: GithubAppRules;
@inject(PrebuildManager) protected readonly prebuildManager: PrebuildManager;
Expand Down Expand Up @@ -646,7 +648,7 @@ export class GithubApp {
return installationOwner;
}
for (const teamMember of teamMembers) {
const user = await this.userDB.findUserById(teamMember.userId);
const user = await this.userService.findUserById(teamMember.userId, teamMember.userId);
if (user && user.identities.some((i) => i.authProviderId === "Public-GitHub")) {
return user;
}
Expand All @@ -667,7 +669,7 @@ export class GithubApp {
}

const ownerID = installation.ownerUserID || "this-should-never-happen";
const user = await this.userDB.findUserById(ownerID);
const user = await this.userService.findUserById(ownerID, ownerID);
if (!user) {
return;
}
Expand Down
Loading