feedback

Author: brendan-kellam

docs/snippets/experimental-feature-warning.mdx

@@ -1,4 +1,4 @@
 
 <Warning>
-This is an experimental feature. Certain functionality may be buggy or incomplete, and breaking changes may ship in non-major releases. Have feedback? Submit a [issue](https://github.com/sourcebot-dev/sourcebot/issues) on GitHub.
+This is an experimental feature. Certain functionality may be incomplete and breaking changes may ship in non-major releases. Have feedback? Submit a [issue](https://github.com/sourcebot-dev/sourcebot/issues) on GitHub.
 </Warning>

packages/backend/src/connectionManager.ts

@@ -28,6 +28,7 @@ export class ConnectionManager {
     private worker: Worker;
     private queue: Queue<JobPayload>;
     private logger = createLogger('connection-manager');
+    private interval?: NodeJS.Timeout;
 
     constructor(
         private db: PrismaClient,
@@ -71,7 +72,7 @@ export class ConnectionManager {
 
     public startScheduler() {
         this.logger.debug('Starting scheduler');
-        return setInterval(async () => {
+        this.interval = setInterval(async () => {
             const thresholdDate = new Date(Date.now() - this.settings.resyncConnectionIntervalMs);
             const connections = await this.db.connection.findMany({
                 where: {
@@ -364,6 +365,9 @@ export class ConnectionManager {
     }
 
     public dispose() {
+        if (this.interval) {
+            clearInterval(this.interval);
+        }
         this.worker.close();
         this.queue.close();
     }

packages/backend/src/ee/repoPermissionSyncer.ts

@@ -1,17 +1,14 @@
 import * as Sentry from "@sentry/node";
 import { PrismaClient, Repo, RepoPermissionSyncJobStatus } from "@sourcebot/db";
 import { createLogger } from "@sourcebot/logger";
-import { BitbucketConnectionConfig } from "@sourcebot/schemas/v3/bitbucket.type";
-import { GiteaConnectionConfig } from "@sourcebot/schemas/v3/gitea.type";
-import { GithubConnectionConfig } from "@sourcebot/schemas/v3/github.type";
-import { GitlabConnectionConfig } from "@sourcebot/schemas/v3/gitlab.type";
+import { hasEntitlement } from "@sourcebot/shared";
 import { Job, Queue, Worker } from 'bullmq';
 import { Redis } from 'ioredis';
-import { env } from "../env.js";
-import { createOctokitFromConfig, getUserIdsWithReadAccessToRepo } from "../github.js";
-import { RepoWithConnections, Settings } from "../types.js";
 import { PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES } from "../constants.js";
-import { hasEntitlement } from "@sourcebot/shared";
+import { env } from "../env.js";
+import { createOctokitFromToken, getRepoCollaborators } from "../github.js";
+import { Settings } from "../types.js";
+import { getAuthCredentialsForRepo } from "../utils.js";
 
 type RepoPermissionSyncJob = {
     jobId: string;
@@ -25,6 +22,7 @@ const logger = createLogger('repo-permission-syncer');
 export class RepoPermissionSyncer {
     private queue: Queue<RepoPermissionSyncJob>;
     private worker: Worker<RepoPermissionSyncJob>;
+    private interval?: NodeJS.Timeout;
 
     constructor(
         private db: PrismaClient,
@@ -49,7 +47,7 @@ export class RepoPermissionSyncer {
 
         logger.debug('Starting scheduler');
 
-        return setInterval(async () => {
+        this.interval = setInterval(async () => {
             // @todo: make this configurable
             const thresholdDate = new Date(Date.now() - this.settings.experiment_repoDrivenPermissionSyncIntervalMs);
 
@@ -104,6 +102,9 @@ export class RepoPermissionSyncer {
     }
 
     public dispose() {
+        if (this.interval) {
+            clearInterval(this.interval);
+        }
         this.worker.close();
         this.queue.close();
     }
@@ -157,15 +158,17 @@ export class RepoPermissionSyncer {
 
         logger.info(`Syncing permissions for repo ${repo.displayName}...`);
 
-        const connection = getFirstConnectionWithToken(repo);
-        if (!connection) {
-            throw new Error(`No connection with token found for repo ${id}`);
+        const credentials = await getAuthCredentialsForRepo(repo, this.db, logger);
+        if (!credentials) {
+            throw new Error(`No credentials found for repo ${id}`);
         }
 
         const userIds = await (async () => {
-            if (connection.connectionType === 'github') {
-                const config = connection.config as unknown as GithubConnectionConfig;
-                const { octokit } = await createOctokitFromConfig(config, repo.orgId, this.db);
+            if (repo.external_codeHostType === 'github') {
+                const { octokit } = await createOctokitFromToken({
+                    token: credentials.token,
+                    url: credentials.hostUrl,
+                });
 
                 // @note: this is a bit of a hack since the displayName _might_ not be set..
                 // however, this property was introduced many versions ago and _should_ be set
@@ -176,7 +179,8 @@ export class RepoPermissionSyncer {
 
                 const [owner, repoName] = repo.displayName.split('/');
 
-                const githubUserIds = await getUserIdsWithReadAccessToRepo(owner, repoName, octokit);
+                const collaborators = await getRepoCollaborators(owner, repoName, octokit);
+                const githubUserIds = collaborators.map(collaborator => collaborator.id.toString());
 
                 const accounts = await this.db.account.findMany({
                     where: {
@@ -268,34 +272,3 @@ export class RepoPermissionSyncer {
         }
     }
 }
-
-const getFirstConnectionWithToken = (repo: RepoWithConnections) => {
-    for (const { connection } of repo.connections) {
-        if (connection.connectionType === 'github') {
-            const config = connection.config as unknown as GithubConnectionConfig;
-            if (config.token) {
-                return connection;
-            }
-        }
-        if (connection.connectionType === 'gitlab') {
-            const config = connection.config as unknown as GitlabConnectionConfig;
-            if (config.token) {
-                return connection;
-            }
-        }
-        if (connection.connectionType === 'gitea') {
-            const config = connection.config as unknown as GiteaConnectionConfig;
-            if (config.token) {
-                return connection;
-            }
-        }
-        if (connection.connectionType === 'bitbucket') {
-            const config = connection.config as unknown as BitbucketConnectionConfig;
-            if (config.token) {
-                return connection;
-            }
-        }
-    }
-
-    return undefined;
-}

packages/backend/src/ee/userPermissionSyncer.ts

@@ -1,12 +1,11 @@
-import { Octokit } from "@octokit/rest";
 import * as Sentry from "@sentry/node";
 import { PrismaClient, User, UserPermissionSyncJobStatus } from "@sourcebot/db";
 import { createLogger } from "@sourcebot/logger";
 import { Job, Queue, Worker } from "bullmq";
 import { Redis } from "ioredis";
 import { PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES } from "../constants.js";
 import { env } from "../env.js";
-import { createOctokitFromOAuthToken, getReposForAuthenticatedUser } from "../github.js";
+import { createOctokitFromToken, getReposForAuthenticatedUser } from "../github.js";
 import { hasEntitlement } from "@sourcebot/shared";
 import { Settings } from "../types.js";
 
@@ -22,6 +21,7 @@ type UserPermissionSyncJob = {
 export class UserPermissionSyncer {
     private queue: Queue<UserPermissionSyncJob>;
     private worker: Worker<UserPermissionSyncJob>;
+    private interval?: NodeJS.Timeout;
 
     constructor(
         private db: PrismaClient,
@@ -46,7 +46,7 @@ export class UserPermissionSyncer {
 
         logger.debug('Starting scheduler');
 
-        return setInterval(async () => {
+        this.interval = setInterval(async () => {
             const thresholdDate = new Date(Date.now() - this.settings.experiment_userDrivenPermissionSyncIntervalMs);
 
             const users = await this.db.user.findMany({
@@ -102,6 +102,9 @@ export class UserPermissionSyncer {
     }
 
     public dispose() {
+        if (this.interval) {
+            clearInterval(this.interval);
+        }
         this.worker.close();
         this.queue.close();
     }
@@ -151,50 +154,61 @@ export class UserPermissionSyncer {
 
         logger.info(`Syncing permissions for user ${user.email}...`);
 
-        for (const account of user.accounts) {
-            const repoIds = await (async () => {
+        // Get a list of all repos that the user has access to from all connected accounts.
+        const repoIds = await (async () => {
+            const aggregatedRepoIds: Set<number> = new Set();
+
+            for (const account of user.accounts) {
                 if (account.provider === 'github') {
-                    const octokit = await createOctokitFromOAuthToken(account.access_token);
+                    if (!account.access_token) {
+                        throw new Error(`User '${user.email}' does not have an GitHub OAuth access token associated with their GitHub account.`);
+                    }
+
+                    const { octokit } = await createOctokitFromToken({
+                        token: account.access_token,
+                        url: env.AUTH_EE_GITHUB_BASE_URL,
+                    });
                     // @note: we only care about the private repos since we don't need to build a mapping
                     // for public repos.
                     // @see: packages/web/src/prisma.ts
-                    const repoIds = await getReposForAuthenticatedUser(/* visibility = */ 'private', octokit);
+                    const githubRepos = await getReposForAuthenticatedUser(/* visibility = */ 'private', octokit);
+                    const gitHubRepoIds = githubRepos.map(repo => repo.id.toString());
 
                     const repos = await this.db.repo.findMany({
                         where: {
                             external_codeHostType: 'github',
                             external_id: {
-                                in: repoIds,
+                                in: gitHubRepoIds,
                             }
                         }
                     });
 
-                    return repos.map(repo => repo.id);
+                    repos.forEach(repo => aggregatedRepoIds.add(repo.id));
                 }
+            }
 
-                return [];
-            })();
-
+            return Array.from(aggregatedRepoIds);
+        })();
 
-            await this.db.$transaction([
-                this.db.user.update({
-                    where: {
-                        id: user.id,
-                    },
-                    data: {
-                        accessibleRepos: {
-                            deleteMany: {},
-                        }
+        await this.db.$transaction([
+            this.db.user.update({
+                where: {
+                    id: user.id,
+                },
+                data: {
+                    accessibleRepos: {
+                        deleteMany: {},
                     }
-                }),
-                this.db.userToRepoPermission.createMany({
-                    data: repoIds.map(repoId => ({
-                        userId: user.id,
-                        repoId,
-                    }))
-                })
-            ]);
-        }
+                }
+            }),
+            this.db.userToRepoPermission.createMany({
+                data: repoIds.map(repoId => ({
+                    userId: user.id,
+                    repoId,
+                })),
+                skipDuplicates: true,
+            })
+        ]);
     }
 
     private async onJobCompleted(job: Job<UserPermissionSyncJob>) {
@@ -226,7 +240,7 @@ export class UserPermissionSyncer {
                 queue: QUEUE_NAME,
             }
         });
-        
+
         const errorMessage = (email: string) => `User permission sync job failed for user ${email}: ${err.message}`;
 
         if (job) {

packages/backend/src/git.ts

@@ -5,9 +5,15 @@ import { env } from './env.js';
 type onProgressFn = (event: SimpleGitProgressEvent) => void;
 
 export const cloneRepository = async (
-    remoteUrl: URL,
-    path: string,
-    onProgress?: onProgressFn
+    {
+        cloneUrl,
+        path,
+        onProgress,
+    }: {
+        cloneUrl: string,
+        path: string,
+        onProgress?: onProgressFn
+    }
 ) => {
     try {
         await mkdir(path, { recursive: true });
@@ -19,7 +25,7 @@ export const cloneRepository = async (
         })
 
         await git.clone(
-            remoteUrl.toString(),
+            cloneUrl,
             path,
             [
                 "--bare",
@@ -42,9 +48,15 @@ export const cloneRepository = async (
 };
 
 export const fetchRepository = async (
-    remoteUrl: URL,
-    path: string,
-    onProgress?: onProgressFn
+    {
+        cloneUrl,
+        path,
+        onProgress,
+    }: {
+        cloneUrl: string,
+        path: string,
+        onProgress?: onProgressFn
+    }
 ) => {
     try {
         const git = simpleGit({
@@ -54,7 +66,7 @@ export const fetchRepository = async (
         })
 
         await git.fetch([
-            remoteUrl.toString(),
+            cloneUrl,
             "+refs/heads/*:refs/heads/*",
             "--prune",
             "--progress"