[server] createProject should not query all repositories – EXP-459 (#18532)

AlexTugarev · web-flow · commit 95d14d3f66fc · 2023-08-17T16:16:43.000+02:00
* [server] createProject should not query all repositories

except for github.com when using the GitHub App.

* fixup: circular dependency problem

* extract webhook related functions to scm-service.ts

* ensure canCreateProject is case-insensitive
diff --git a/components/server/src/container-module.ts b/components/server/src/container-module.ts
@@ -131,6 +131,7 @@ import { WorkspaceService } from "./workspace/workspace-service";
 import { SSHKeyService } from "./user/sshkey-service";
 import { GitpodTokenService } from "./user/gitpod-token-service";
 import { EnvVarService } from "./user/env-var-service";
+import { ScmService } from "./projects/scm-service";
 
 export const productionContainerModule = new ContainerModule(
     (bind, unbind, isBound, rebind, unbindAsync, onActivation, onDeactivation) => {
@@ -258,6 +259,7 @@ export const productionContainerModule = new ContainerModule(
 
         bind(OrganizationService).toSelf().inSingletonScope();
         bind(ProjectsService).toSelf().inSingletonScope();
+        bind(ScmService).toSelf().inSingletonScope();
 
         bind(NewsletterSubscriptionController).toSelf().inSingletonScope();
 
diff --git a/components/server/src/projects/projects-service.ts b/components/server/src/projects/projects-service.ts
@@ -19,23 +19,23 @@ import { HostContextProvider } from "../auth/host-context-provider";
 import { RepoURL } from "../repohost";
 import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
 import { PartialProject } from "@gitpod/gitpod-protocol/lib/teams-projects-protocol";
-import { Config } from "../config";
 import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
 import { ErrorCodes, ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
 import { URL } from "url";
 import { Authorizer } from "../authorization/authorizer";
 import { TransactionalContext } from "@gitpod/gitpod-db/lib/typeorm/transactional-db-impl";
+import { ScmService } from "./scm-service";
 
 @injectable()
 export class ProjectsService {
     constructor(
         @inject(ProjectDB) private readonly projectDB: ProjectDB,
         @inject(TracedWorkspaceDB) private readonly workspaceDb: DBWithTracing<WorkspaceDB>,
         @inject(HostContextProvider) private readonly hostContextProvider: HostContextProvider,
-        @inject(Config) private readonly config: Config,
         @inject(IAnalyticsWriter) private readonly analytics: IAnalyticsWriter,
         @inject(WebhookEventDB) private readonly webhookEventDB: WebhookEventDB,
         @inject(Authorizer) private readonly auth: Authorizer,
+        @inject(ScmService) private readonly scmService: ScmService,
     ) {}
 
     async getProject(userId: string, projectId: string): Promise<Project> {
@@ -244,7 +244,12 @@ export class ProjectsService {
             await this.auth.removeProjectFromOrg(installer.id, teamId, project.id);
             throw err;
         }
-        await this.onDidCreateProject(project, installer);
+        await this.scmService.installWebhookForPrebuilds(project, installer);
+
+        // Pre-fetch project details in the background -- don't await
+        this.getProjectOverview(installer, project.id).catch((err) => {
+            log.error(`Error pre-fetching project details for project ${project.id}: ${err}`);
+        });
 
         this.analytics.track({
             userId: installer.id,
@@ -261,40 +266,6 @@ export class ProjectsService {
         return project;
     }
 
-    private async onDidCreateProject(project: Project, installer: User) {
-        // Pre-fetch project details in the background -- don't await
-        this.getProjectOverview(installer, project.id).catch((err) => {
-            log.error(`Error pre-fetching project details for project ${project.id}: ${err}`);
-        });
-
-        // Install the prebuilds webhook if possible
-        const { teamId, cloneUrl } = project;
-        const parsedUrl = RepoURL.parseRepoUrl(project.cloneUrl);
-        const hostContext = parsedUrl?.host ? this.hostContextProvider.get(parsedUrl?.host) : undefined;
-        const authProvider = hostContext && hostContext.authProvider.info;
-        const type = authProvider && authProvider.authProviderType;
-        if (
-            type === "GitLab" ||
-            type === "Bitbucket" ||
-            type === "BitbucketServer" ||
-            (type === "GitHub" && (authProvider?.host !== "github.com" || !this.config.githubApp?.enabled))
-        ) {
-            const repositoryService = hostContext?.services?.repositoryService;
-            if (repositoryService) {
-                // Note: For GitLab, we expect .canInstallAutomatedPrebuilds() to always return true, because earlier
-                // in the project creation flow, we only propose repositories where the user is actually allowed to
-                // install a webhook.
-                if (await repositoryService.canInstallAutomatedPrebuilds(installer, cloneUrl)) {
-                    log.info(
-                        { organizationId: teamId, userId: installer.id },
-                        "Update prebuild installation for project.",
-                    );
-                    await repositoryService.installAutomatedPrebuilds(installer, cloneUrl);
-                }
-            }
-        }
-    }
-
     async deleteProject(userId: string, projectId: string, transactionCtx?: TransactionalContext): Promise<void> {
         await this.auth.checkPermissionOnProject(userId, "delete", projectId);
 
diff --git a/components/server/src/projects/scm-service.ts b/components/server/src/projects/scm-service.ts
@@ -0,0 +1,77 @@
+/**
+ * Copyright (c) 2023 Gitpod GmbH. All rights reserved.
+ * Licensed under the GNU Affero General Public License (AGPL).
+ * See License.AGPL.txt in the project root for license information.
+ */
+
+import { Project, User } from "@gitpod/gitpod-protocol";
+import { RepoURL } from "../repohost";
+import { inject, injectable } from "inversify";
+import { HostContextProvider } from "../auth/host-context-provider";
+import { Config } from "../config";
+import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
+
+@injectable()
+export class ScmService {
+    constructor(
+        @inject(HostContextProvider) private readonly hostContextProvider: HostContextProvider,
+        @inject(Config) private readonly config: Config,
+    ) {}
+
+    async canInstallWebhook(currentUser: User, cloneURL: string) {
+        try {
+            const parsedUrl = RepoURL.parseRepoUrl(cloneURL);
+            const hostContext = parsedUrl?.host ? this.hostContextProvider.get(parsedUrl?.host) : undefined;
+            const authProvider = hostContext && hostContext.authProvider.info;
+            const type = authProvider && authProvider.authProviderType;
+            const host = authProvider?.host;
+            if (!type || !host) {
+                throw Error("Unknown host: " + parsedUrl?.host);
+            }
+            if (
+                type === "GitLab" ||
+                type === "Bitbucket" ||
+                type === "BitbucketServer" ||
+                (type === "GitHub" && (host !== "github.com" || !this.config.githubApp?.enabled))
+            ) {
+                const repositoryService = hostContext?.services?.repositoryService;
+                if (repositoryService) {
+                    return await repositoryService.canInstallAutomatedPrebuilds(currentUser, cloneURL);
+                }
+            }
+            // The GitHub App case isn't handled here due to a circular dependency problem.
+        } catch (error) {
+            log.error("Failed to check precondition for creating a project.");
+        }
+        return false;
+    }
+
+    async installWebhookForPrebuilds(project: Project, installer: User) {
+        // Install the prebuilds webhook if possible
+        const { teamId, cloneUrl } = project;
+        const parsedUrl = RepoURL.parseRepoUrl(project.cloneUrl);
+        const hostContext = parsedUrl?.host ? this.hostContextProvider.get(parsedUrl?.host) : undefined;
+        const authProvider = hostContext && hostContext.authProvider.info;
+        const type = authProvider && authProvider.authProviderType;
+        if (
+            type === "GitLab" ||
+            type === "Bitbucket" ||
+            type === "BitbucketServer" ||
+            (type === "GitHub" && (authProvider?.host !== "github.com" || !this.config.githubApp?.enabled))
+        ) {
+            const repositoryService = hostContext?.services?.repositoryService;
+            if (repositoryService) {
+                // Note: For GitLab, we expect .canInstallAutomatedPrebuilds() to always return true, because earlier
+                // in the project creation flow, we only propose repositories where the user is actually allowed to
+                // install a webhook.
+                if (await repositoryService.canInstallAutomatedPrebuilds(installer, cloneUrl)) {
+                    log.info(
+                        { organizationId: teamId, userId: installer.id },
+                        "Update prebuild installation for project.",
+                    );
+                    await repositoryService.installAutomatedPrebuilds(installer, cloneUrl);
+                }
+            }
+        }
+    }
+}
diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts
@@ -179,6 +179,7 @@ import { SSHKeyService } from "../user/sshkey-service";
 import { StartWorkspaceOptions, WorkspaceService, mapGrpcError } from "./workspace-service";
 import { GitpodTokenService } from "../user/gitpod-token-service";
 import { EnvVarService } from "../user/env-var-service";
+import { ScmService } from "../projects/scm-service";
 
 // shortcut
 export const traceWI = (ctx: TraceContext, wi: Omit<LogContext, "userId">) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager
@@ -238,6 +239,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         @inject(HeadlessLogService) private readonly headlessLogService: HeadlessLogService,
 
         @inject(ProjectsService) private readonly projectsService: ProjectsService,
+        @inject(ScmService) private readonly scmService: ScmService,
 
         @inject(IDEService) private readonly ideService: IDEService,
 
@@ -2580,16 +2582,13 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         await this.auth.checkPermissionOnOrganization(user.id, "create_project", params.teamId);
 
         // Check if provided clone URL is accessible for the current user, and user has admin permissions.
-        let url;
         try {
-            url = new URL(params.cloneUrl);
+            new URL(params.cloneUrl);
         } catch (err) {
             throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Clone URL must be a valid URL.");
         }
-        const availableRepositories = await this.getProviderRepositoriesForUser(ctx, { provider: url.host });
-        if (!availableRepositories.some((r) => r.cloneUrl === params.cloneUrl)) {
-            // The error message is derived from internals of `getProviderRepositoriesForUser` and
-            // `getRepositoriesForAutomatedPrebuilds`, which require admin permissions to be present.
+        const canCreateProject = await this.canCreateProject(user, params.cloneUrl);
+        if (!canCreateProject) {
             throw new ApplicationError(
                 ErrorCodes.BAD_REQUEST,
                 "Repository URL seems to be inaccessible, or admin permissions are missing.",
@@ -2618,6 +2617,37 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         return project;
     }
 
+    /**
+     * Checks if a project can be created, i.e. the current user has the required permissions
+     * to install webhooks for the given repository.
+     */
+    private async canCreateProject(currentUser: User, cloneURL: string) {
+        try {
+            const parsedUrl = RepoURL.parseRepoUrl(cloneURL);
+            const host = parsedUrl?.host;
+            if (!host) {
+                throw Error("Unknown host: " + parsedUrl?.host);
+            }
+            if (host === "github.com" && this.config.githubApp?.enabled) {
+                const availableRepositories = await this.githubAppSupport.getProviderRepositoriesForUser({
+                    user: currentUser,
+                    provider: "github.com",
+                });
+                return availableRepositories.some(
+                    (r) => r?.cloneUrl?.toLocaleLowerCase() === cloneURL?.toLocaleLowerCase(),
+                );
+            } else {
+                return await this.scmService.canInstallWebhook(currentUser, cloneURL);
+
+                // note: the GitHub App based check is not included in the ProjectService due
+                // to a circular dependency problem which would otherwise occur.
+            }
+        } catch (error) {
+            log.error("Failed to check precondition for creating a project.");
+        }
+        return false;
+    }
+
     public async updateProjectPartial(ctx: TraceContext, partialProject: PartialProject): Promise<void> {
         traceAPIParams(ctx, {
             // censor everything irrelevant
