From 6c8712ef523a7cbc4db8b315a5f5e3953babe388 Mon Sep 17 00:00:00 2001 From: svenefftinge Date: Fri, 28 Jul 2023 13:59:22 +0000 Subject: [PATCH] [server/fga] centralize lookup of findUserById --- components/server/src/auth/authenticator.ts | 32 ++++--- .../server/src/auth/bearer-authenticator.ts | 21 +++-- .../server/src/dev/authenticator-dev.ts | 23 ++--- .../server/src/prebuilds/bitbucket-app.ts | 28 +++--- .../src/prebuilds/bitbucket-server-app.ts | 28 +++--- components/server/src/prebuilds/github-app.ts | 8 +- .../src/prebuilds/github-enterprise-app.ts | 43 ++++----- components/server/src/prebuilds/gitlab-app.ts | 26 +++--- components/server/src/session-handler.ts | 15 ++-- .../server/src/user/user-authentication.ts | 22 +++-- components/server/src/user/user-controller.ts | 9 +- components/server/src/user/user-service.ts | 14 ++- .../src/workspace/gitpod-server-impl.ts | 87 ++++++------------- 13 files changed, 171 insertions(+), 185 deletions(-) diff --git a/components/server/src/auth/authenticator.ts b/components/server/src/auth/authenticator.ts index c9f94e810746d0..0818a87b93dc30 100644 --- a/components/server/src/auth/authenticator.ts +++ b/components/server/src/auth/authenticator.ts @@ -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() @@ -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); } @@ -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); diff --git a/components/server/src/auth/bearer-authenticator.ts b/components/server/src/auth/bearer-authenticator.ts index f9084f173947c3..79cb51cc5aeea2 100644 --- a/components/server/src/auth/bearer-authenticator.ts +++ b/components/server/src/auth/bearer-authenticator.ts @@ -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"]; @@ -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) => { @@ -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); @@ -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 }; } } diff --git a/components/server/src/dev/authenticator-dev.ts b/components/server/src/dev/authenticator-dev.ts index dc3c4fcbfb88fa..9accc14f22d722 100644 --- a/components/server/src/dev/authenticator-dev.ts +++ b/components/server/src/dev/authenticator-dev.ts @@ -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 { - 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", @@ -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); } diff --git a/components/server/src/prebuilds/bitbucket-app.ts b/components/server/src/prebuilds/bitbucket-app.ts index 06cfde51507339..d5ab6461f56d5d 100644 --- a/components/server/src/prebuilds/bitbucket-app.ts +++ b/components/server/src/prebuilds/bitbucket-app.ts @@ -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"; @@ -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; @@ -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) { @@ -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.`); @@ -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 }; } diff --git a/components/server/src/prebuilds/bitbucket-server-app.ts b/components/server/src/prebuilds/bitbucket-server-app.ts index 554a3d96967169..e821e2f2e1c794 100644 --- a/components/server/src/prebuilds/bitbucket-server-app.ts +++ b/components/server/src/prebuilds/bitbucket-server-app.ts @@ -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"; @@ -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; @@ -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) { @@ -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.`); @@ -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 }; } diff --git a/components/server/src/prebuilds/github-app.ts b/components/server/src/prebuilds/github-app.ts index ed15ab4eaa9d99..0fb9736007096a 100644 --- a/components/server/src/prebuilds/github-app.ts +++ b/components/server/src/prebuilds/github-app.ts @@ -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: @@ -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; @inject(GithubAppRules) protected readonly appRules: GithubAppRules; @inject(PrebuildManager) protected readonly prebuildManager: PrebuildManager; @@ -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; } @@ -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; } diff --git a/components/server/src/prebuilds/github-enterprise-app.ts b/components/server/src/prebuilds/github-enterprise-app.ts index 6e719841fd6c05..0cec439b8c7fda 100644 --- a/components/server/src/prebuilds/github-enterprise-app.ts +++ b/components/server/src/prebuilds/github-enterprise-app.ts @@ -8,7 +8,7 @@ import * as express from "express"; import { createHmac, timingSafeEqual } from "crypto"; import { Buffer } from "buffer"; 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 { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing"; import { TokenService } from "../user/token-service"; @@ -20,10 +20,12 @@ import { URL } from "url"; import { ContextParser } from "../workspace/context-parser-service"; import { RepoURL } from "../repohost"; import { GithubAppRules } from "./github-app-rules"; +import { UserService } from "../user/user-service"; +import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; @injectable() export class GitHubEnterpriseApp { - @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(HostContextProvider) protected readonly hostContextProvider: HostContextProvider; @@ -112,9 +114,11 @@ export class GitHubEnterpriseApp { // Verify the webhook signature const signature = req.header("X-Hub-Signature-256"); const body = (req as any).rawBody; - const tokenEntries = (await this.userDB.findTokensForIdentity(gitpodIdentity)).filter((tokenEntry) => { - return tokenEntry.token.scopes.includes(GitHubService.PREBUILD_TOKEN_SCOPE); - }); + const tokenEntries = (await this.userService.findTokensForIdentity(user.id, gitpodIdentity)).filter( + (tokenEntry) => { + return tokenEntry.token.scopes.includes(GitHubService.PREBUILD_TOKEN_SCOPE); + }, + ); const signatureMatched = tokenEntries.some((tokenEntry) => { const sig = "sha256=" + @@ -240,13 +244,11 @@ export class GitHubEnterpriseApp { 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 { + 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 || ""); if (teamMembers.some((t) => t.userId === webhookInstaller.id)) { return { user: webhookInstaller, project }; @@ -254,25 +256,22 @@ export class GitHubEnterpriseApp { const hostContext = this.hostContextProvider.get(new URL(cloneURL).host); const authProviderId = hostContext?.authProvider.authProviderId; 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 === authProviderId)) { return { user, project }; } } } + } catch (err) { + log.info({ userId: webhookInstaller.id }, "Failed to find project and owner", err); } return { user: webhookInstaller }; } protected async findProjectOwners(cloneURL: string): Promise<{ users: User[]; project: Project } | undefined> { - const project = await this.projectDB.findProjectByCloneUrl(cloneURL); - if (project) { - if (project.userId) { - const user = await this.userDB.findUserById(project.userId); - if (user) { - return { users: [user], project }; - } - } else if (project.teamId) { + try { + const project = await this.projectDB.findProjectByCloneUrl(cloneURL); + if (project) { const users = []; const owners = (await this.teamDB.findMembersByTeam(project.teamId || "")).filter( (m) => m.role === "owner", @@ -280,13 +279,15 @@ export class GitHubEnterpriseApp { const hostContext = this.hostContextProvider.get(new URL(cloneURL).host); const authProviderId = hostContext?.authProvider.authProviderId; for (const teamMember of owners) { - 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 === authProviderId)) { users.push(user); } } return { users, project }; } + } catch (err) { + log.info("Failed to find project and owner", err); } return undefined; } diff --git a/components/server/src/prebuilds/gitlab-app.ts b/components/server/src/prebuilds/gitlab-app.ts index 18590324baf771..5ae0771f9ee73a 100644 --- a/components/server/src/prebuilds/gitlab-app.ts +++ b/components/server/src/prebuilds/gitlab-app.ts @@ -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 { Project, User, StartPrebuildResult, CommitContext, CommitInfo, WebhookEvent } from "@gitpod/gitpod-protocol"; import { PrebuildManager } from "./prebuild-manager"; import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing"; @@ -16,10 +16,12 @@ import { GitlabService } from "./gitlab-service"; import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; import { ContextParser } from "../workspace/context-parser-service"; import { RepoURL } from "../repohost"; +import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; +import { UserService } from "../user/user-service"; @injectable() export class GitLabApp { - @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(HostContextProvider) protected readonly hostCtxProvider: HostContextProvider; @@ -99,7 +101,7 @@ export class GitLabApp { const span = TraceContext.startSpan("GitLapApp.findUser", ctx); try { 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) { @@ -109,7 +111,7 @@ export class GitLabApp { 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.`); @@ -228,25 +230,25 @@ export class GitLabApp { 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 || ""); 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-GitLab")) { return { user, project }; } } } + } catch (err) { + log.info({ userId: webhookInstaller.id }, "Failed to find project and owner", err); } return { user: webhookInstaller }; } diff --git a/components/server/src/session-handler.ts b/components/server/src/session-handler.ts index e04aa5968b2b0a..c0b25f8f990625 100644 --- a/components/server/src/session-handler.ts +++ b/components/server/src/session-handler.ts @@ -5,21 +5,21 @@ */ import * as express from "express"; +import { inject, injectable } from "inversify"; import * as websocket from "ws"; -import { injectable, inject } from "inversify"; import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; -import { Config } from "./config"; -import { reportJWTCookieIssued } from "./prometheus-metrics"; import { AuthJWT } from "./auth/jwt"; -import { UserDB } from "@gitpod/gitpod-db/lib"; +import { Config } from "./config"; import { WsNextFunction, WsRequestHandler } from "./express/ws-handler"; +import { reportJWTCookieIssued } from "./prometheus-metrics"; +import { UserService } from "./user/user-service"; @injectable() export class SessionHandler { @inject(Config) protected readonly config: Config; @inject(AuthJWT) protected readonly authJWT: AuthJWT; - @inject(UserDB) protected userDb: UserDB; + @inject(UserService) protected userService: UserService; public jwtSessionConvertor(): express.Handler { return async (req, res) => { @@ -109,10 +109,7 @@ export class SessionHandler { throw new Error("Subject is missing from JWT session claims"); } - const user = await this.userDb.findUserById(subject); - if (!user) { - throw new Error("No user exists."); - } + const user = await this.userService.findUserById(subject, subject); // We set the user object on the request to signal the user is authenticated. // Passport uses the `user` property on the request to determine if the session diff --git a/components/server/src/user/user-authentication.ts b/components/server/src/user/user-authentication.ts index f6ec436a55a531..8e1b82d921253e 100644 --- a/components/server/src/user/user-authentication.ts +++ b/components/server/src/user/user-authentication.ts @@ -15,6 +15,7 @@ import { TokenService } from "./token-service"; import { EmailAddressAlreadyTakenException, SelectAccountException } from "../auth/errors"; import { SelectAccountPayload } from "@gitpod/gitpod-protocol/lib/auth"; import { ErrorCodes, ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error"; +import { UserService } from "./user-service"; export interface CreateUserParams { organizationId?: string; @@ -34,11 +35,12 @@ export class UserAuthentication { @inject(Config) private readonly config: Config, @inject(EmailDomainFilterDB) private readonly domainFilterDb: EmailDomainFilterDB, @inject(UserDB) private readonly userDb: UserDB, + @inject(UserService) private readonly userService: UserService, @inject(HostContextProvider) private readonly hostContextProvider: HostContextProvider, ) {} async blockUser(targetUserId: string, block: boolean): Promise { - const target = await this.userDb.findUserById(targetUserId); + const target = await this.userService.findUserById(targetUserId, targetUserId); if (!target) { throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found"); } @@ -49,12 +51,20 @@ export class UserAuthentication { async findUserForLogin(params: { candidate: IdentityLookup }) { const user = await this.userDb.findUserByIdentity(params.candidate); - return user; + return this.loadClean(user); + } + + // make sure we load through user service to ensure any sideffects (e.g. spicedb migrations) are applied + private async loadClean(user?: User): Promise { + if (!user) { + return undefined; + } + return await this.userService.findUserById(user.id, user.id); } async findOrgOwnedUser(params: { organizationId: string; email: string }): Promise { const user = await this.userDb.findOrgOwnedUser(params.organizationId, params.email); - return user; + return this.loadClean(user); } async updateUserOnLogin(user: User, authUser: AuthUser, candidate: Identity, token: Token): Promise { @@ -64,11 +74,7 @@ export class UserAuthentication { await this.updateUserIdentity(user, candidate); await this.userDb.storeSingleToken(candidate, token); - const updated = await this.userDb.findUserById(user.id); - if (!updated) { - throw new Error("User does not exist"); - } - return updated; + return await this.userService.findUserById(user.id, user.id); } async updateUserIdentity(user: User, candidate: Identity) { diff --git a/components/server/src/user/user-controller.ts b/components/server/src/user/user-controller.ts index 96f695cd59c783..3956703209a7a4 100644 --- a/components/server/src/user/user-controller.ts +++ b/components/server/src/user/user-controller.ts @@ -29,6 +29,7 @@ import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messag import { GitpodServerImpl } from "../workspace/gitpod-server-impl"; import { WorkspaceStarter } from "../workspace/workspace-starter"; import { StopWorkspacePolicy } from "@gitpod/ws-manager/lib"; +import { UserService } from "./user-service"; export const ServerFactory = Symbol("ServerFactory"); export type ServerFactory = () => GitpodServerImpl; @@ -36,6 +37,7 @@ export type ServerFactory = () => GitpodServerImpl; @injectable() export class UserController { @inject(WorkspaceDB) protected readonly workspaceDB: WorkspaceDB; + @inject(UserService) protected readonly userService: UserService; @inject(UserDB) protected readonly userDb: UserDB; @inject(TeamDB) protected readonly teamDb: TeamDB; @inject(Authenticator) protected readonly authenticator: Authenticator; @@ -97,7 +99,7 @@ export class UserController { throw new ApplicationError(401, "Invalid OTS key"); } - const user = await this.userDb.findUserById(userId); + const user = await this.userService.findUserById(userId, userId); if (!user) { throw new ApplicationError(404, "User not found"); } @@ -136,7 +138,10 @@ export class UserController { // The user has supplied a valid token, we need to sign them in. // Login this user (sets cookie as side-effect) - const user = await this.userDb.findUserById(BUILTIN_INSTLLATION_ADMIN_USER_ID); + const user = await this.userService.findUserById( + BUILTIN_INSTLLATION_ADMIN_USER_ID, + BUILTIN_INSTLLATION_ADMIN_USER_ID, + ); if (!user) { // We respond with NOT_AUTHENTICATED to prevent gleaning whether the user, or token are invalid. throw new ApplicationError(ErrorCodes.NOT_AUTHENTICATED, "Admin user not found"); diff --git a/components/server/src/user/user-service.ts b/components/server/src/user/user-service.ts index 6f4d337fd264dc..9c70ebb9ec562e 100644 --- a/components/server/src/user/user-service.ts +++ b/components/server/src/user/user-service.ts @@ -8,7 +8,7 @@ import { inject, injectable } from "inversify"; import { Config } from "../config"; import { UserDB } from "@gitpod/gitpod-db/lib"; import { Authorizer } from "../authorization/authorizer"; -import { AdditionalUserData, User } from "@gitpod/gitpod-protocol"; +import { AdditionalUserData, Identity, TokenEntry, User } from "@gitpod/gitpod-protocol"; import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; import { CreateUserParams } from "./user-authentication"; @@ -76,7 +76,7 @@ export class UserService { } } - public async findUserById(userId: string, id: string): Promise { + async findUserById(userId: string, id: string): Promise { await this.authorizer.checkPermissionOnUser(userId, "read_info", id); const result = await this.userDb.findUserById(id); if (!result) { @@ -85,6 +85,16 @@ export class UserService { return result; } + async findTokensForIdentity(userId: string, identity: Identity): Promise { + const result = await this.userDb.findTokensForIdentity(identity); + for (const token of result) { + if (!(await this.authorizer.hasPermissionOnUser(userId, "read_info", token.uid))) { + throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found"); + } + } + return result; + } + async updateUser(userId: string, update: Partial & { id: string }): Promise { const user = await this.findUserById(userId, update.id); await this.authorizer.checkPermissionOnUser(userId, "write_info", user.id); diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts index e295a3902a0a5a..9d0275c03dd595 100644 --- a/components/server/src/workspace/gitpod-server-impl.ts +++ b/components/server/src/workspace/gitpod-server-impl.ts @@ -590,11 +590,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { throw new ApplicationError(ErrorCodes.NOT_AUTHENTICATED, "User is not authenticated. Please login."); } - const user = await this.userDB.findUserById(this.userID); - if (!user) { - throw new ApplicationError(ErrorCodes.BAD_REQUEST, "User does not exist."); - } - + const user = await this.userService.findUserById(this.userID, this.userID); if (user.markedDeleted === true) { throw new ApplicationError(ErrorCodes.USER_DELETED, "User has been deleted."); } @@ -1222,13 +1218,16 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const workspace = await this.internalGetWorkspace(user, workspaceId, this.workspaceDb.trace(ctx)); await this.guardAccess({ kind: "workspace", subject: workspace }, "get"); - const owner = await this.userDB.findUserById(workspace.ownerId); - if (!owner) { - return undefined; + try { + const owner = await this.userService.findUserById(user.id, workspace.ownerId); + await this.guardAccess({ kind: "user", subject: owner }, "get"); + return { name: owner.name }; + } catch (e) { + if (e.code === ErrorCodes.NOT_FOUND) { + return undefined; + } + throw e; } - - await this.guardAccess({ kind: "user", subject: owner }, "get"); - return { name: owner.name }; } public async getWorkspaceUsers(ctx: TraceContext, workspaceId: string): Promise { @@ -1578,22 +1577,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { for (const repo of repositories) { const p = cloneUrlToProject.get(repo.cloneUrl); - const repoProvider = new URL(repo.cloneUrl).host.split(".")[0]; if (p) { - if (p.userId) { - const owner = await this.userDB.findUserById(p.userId); - if (owner) { - const ownerProviderMatchingRepoProvider = owner.identities.find((identity, index) => - identity.authProviderId.toLowerCase().includes(repoProvider), - ); - if (ownerProviderMatchingRepoProvider) { - repo.inUse = { - userName: ownerProviderMatchingRepoProvider?.authName, - }; - } - } - } else if (p.teamOwners && p.teamOwners[0]) { + if (p.teamOwners && p.teamOwners[0]) { repo.inUse = { userName: p.teamOwners[0] || "somebody", }; @@ -3151,19 +3137,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { async adminGetUser(ctx: TraceContext, userId: string): Promise { traceAPIParams(ctx, { userId }); - await this.guardAdminAccess("adminGetUser", { id: userId }, Permission.ADMIN_USERS); + const admin = await this.guardAdminAccess("adminGetUser", { id: userId }, Permission.ADMIN_USERS); - let result: User | undefined; - try { - result = await this.userDB.findUserById(userId); - } catch (e) { - throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, e.toString()); - } - - if (!result) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found"); - } - return result; + return await this.userService.findUserById(admin.id, userId); } async adminGetUsers(ctx: TraceContext, req: AdminGetListRequest): Promise> { @@ -3262,30 +3238,20 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { } async adminVerifyUser(ctx: TraceContext, userId: string): Promise { - await this.guardAdminAccess("adminVerifyUser", { id: userId }, Permission.ADMIN_USERS); - try { - const user = await this.userDB.findUserById(userId); - if (!user) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, `No user with id ${userId} found.`); - } - this.verificationService.markVerified(user); - await this.userDB.updateUserPartial(user); - return user; - } catch (e) { - throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, e.toString()); - } + const admin = await this.guardAdminAccess("adminVerifyUser", { id: userId }, Permission.ADMIN_USERS); + const user = await this.userService.findUserById(admin.id, userId); + + this.verificationService.markVerified(user); + await this.userDB.updateUserPartial(user); + return user; } async adminModifyRoleOrPermission(ctx: TraceContext, req: AdminModifyRoleOrPermissionRequest): Promise { traceAPIParams(ctx, { req }); - await this.guardAdminAccess("adminModifyRoleOrPermission", { req }, Permission.ADMIN_PERMISSIONS); - - const target = await this.userDB.findUserById(req.id); - if (!target) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found"); - } + const admin = await this.guardAdminAccess("adminModifyRoleOrPermission", { req }, Permission.ADMIN_PERMISSIONS); + const target = await this.userService.findUserById(admin.id, req.id); const rolesOrPermissions = new Set((target.rolesOrPermissions || []) as string[]); req.rpp.forEach((e) => { if (e.add) { @@ -3305,11 +3271,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { ): Promise { traceAPIParams(ctx, { req }); - await this.guardAdminAccess("adminModifyPermanentWorkspaceFeatureFlag", { req }, Permission.ADMIN_USERS); - const target = await this.userDB.findUserById(req.id); - if (!target) { - throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found"); - } + const admin = await this.guardAdminAccess( + "adminModifyPermanentWorkspaceFeatureFlag", + { req }, + Permission.ADMIN_USERS, + ); + const target = await this.userService.findUserById(admin.id, req.id); const featureSettings: UserFeatureSettings = target.featureFlags || {}; const featureFlags = new Set(featureSettings.permanentWSFeatureFlags || []);