fix: gracefully handle SAML-protected orgs during GitHub import (#217) (#218)

2026-03-09 07:13:48 -05:00 · 2026-03-07 06:57:28 +05:30
parent de28469210
commit c00d48199b
4 changed files with 149 additions and 41 deletions
--- a/src/components/config/ConfigTabs.tsx
+++ b/src/components/config/ConfigTabs.tsx
@@ -124,19 +124,31 @@ export function ConfigTabs() {
    if (!user?.id) return;
    setIsSyncing(true);
    try {
-      const result = await apiRequest<{ success: boolean; message?: string }>(
+      const result = await apiRequest<{ success: boolean; message?: string; failedOrgs?: string[]; recoveredOrgs?: number }>(
        `/sync?userId=${user.id}`,
        { method: 'POST' },
      );
-      result.success
-        ? toast.success(
-            'GitHub data imported successfully! Head to the Repositories page to start mirroring.',
-          )
-        : toast.error(
-            `Failed to import GitHub data: ${
-              result.message || 'Unknown error'
-            }`,
+      if (result.success) {
+        toast.success(
+          'GitHub data imported successfully! Head to the Repositories page to start mirroring.',
+        );
+        if (result.failedOrgs && result.failedOrgs.length > 0) {
+          toast.warning(
+            `${result.failedOrgs.length} org${result.failedOrgs.length > 1 ? 's' : ''} failed to import (${result.failedOrgs.join(', ')}). Check the Organizations tab for details.`,
          );
+        }
+        if (result.recoveredOrgs && result.recoveredOrgs > 0) {
+          toast.success(
+            `${result.recoveredOrgs} previously failed org${result.recoveredOrgs > 1 ? 's' : ''} recovered successfully.`,
+          );
+        }
+      } else {
+        toast.error(
+          `Failed to import GitHub data: ${
+            result.message || 'Unknown error'
+          }`,
+        );
+      }
    } catch (error) {
      toast.error(
        `Error importing GitHub data: ${
--- a/src/components/organizations/OrganizationsList.tsx
+++ b/src/components/organizations/OrganizationsList.tsx
@@ -248,6 +248,11 @@ export function OrganizationList({
                </div>
              </div>

+              {/* Error message for failed orgs */}
+              {org.status === "failed" && org.errorMessage && (
+                <p className="text-xs text-destructive line-clamp-2">{org.errorMessage}</p>
+              )}
+
              {/* Destination override section */}
              <div>
                <MirrorDestinationEditor
@@ -304,6 +309,13 @@ export function OrganizationList({
                />
              </div>

+              {/* Error message for failed orgs */}
+              {org.status === "failed" && org.errorMessage && (
+                <div className="mb-4 p-3 rounded-md bg-destructive/10 border border-destructive/20">
+                  <p className="text-sm text-destructive">{org.errorMessage}</p>
+                </div>
+              )}
+
              {/* Repository statistics */}
              <div className="mb-4">
                <div className="flex items-center gap-4 text-sm">
--- a/src/lib/github.ts
+++ b/src/lib/github.ts
@@ -369,7 +369,7 @@ export async function getGithubOrganizations({
 }: {
  octokit: Octokit;
  config: Partial<Config>;
-}): Promise<GitOrg[]> {
+}): Promise<{ organizations: GitOrg[]; failedOrgs: { name: string; avatarUrl: string; reason: string }[] }> {
  try {
    const { data: orgs } = await octokit.orgs.listForAuthenticatedUser({
      per_page: 100,
@@ -392,30 +392,47 @@ export async function getGithubOrganizations({
      return true;
    });

-    const organizations = await Promise.all(
+    const failedOrgs: { name: string; avatarUrl: string; reason: string }[] = [];
+    const results = await Promise.all(
      filteredOrgs.map(async (org) => {
-        const [{ data: orgDetails }, { data: membership }] = await Promise.all([
-          octokit.orgs.get({ org: org.login }),
-          octokit.orgs.getMembershipForAuthenticatedUser({ org: org.login }),
-        ]);
+        try {
+          const [{ data: orgDetails }, { data: membership }] = await Promise.all([
+            octokit.orgs.get({ org: org.login }),
+            octokit.orgs.getMembershipForAuthenticatedUser({ org: org.login }),
+          ]);

-        const totalRepos =
-          orgDetails.public_repos + (orgDetails.total_private_repos ?? 0);
+          const totalRepos =
+            orgDetails.public_repos + (orgDetails.total_private_repos ?? 0);

-        return {
-          name: org.login,
-          avatarUrl: org.avatar_url,
-          membershipRole: membership.role as MembershipRole,
-          isIncluded: false,
-          status: "imported" as RepoStatus,
-          repositoryCount: totalRepos,
-          createdAt: new Date(),
-          updatedAt: new Date(),
-        };
+          return {
+            name: org.login,
+            avatarUrl: org.avatar_url,
+            membershipRole: membership.role as MembershipRole,
+            isIncluded: false,
+            status: "imported" as RepoStatus,
+            repositoryCount: totalRepos,
+            createdAt: new Date(),
+            updatedAt: new Date(),
+          };
+        } catch (error: any) {
+          // Capture organizations that return 403 (SAML enforcement, insufficient token scope, etc.)
+          if (error?.status === 403) {
+            const reason = error?.message || "access denied";
+            console.warn(
+              `Failed to import organization ${org.login} - ${reason}`,
+            );
+            failedOrgs.push({ name: org.login, avatarUrl: org.avatar_url, reason });
+            return null;
+          }
+          throw error;
+        }
      }),
    );

-    return organizations;
+    return {
+      organizations: results.filter((org): org is NonNullable<typeof org> => org !== null),
+      failedOrgs,
+    };
  } catch (error) {
    throw new Error(
      `Error fetching organizations: ${
--- a/src/pages/api/sync/index.ts
+++ b/src/pages/api/sync/index.ts
@@ -1,6 +1,6 @@
 import type { APIRoute } from "astro";
 import { db, organizations, repositories, configs } from "@/lib/db";
-import { eq } from "drizzle-orm";
+import { eq, and } from "drizzle-orm";
 import { v4 as uuidv4 } from "uuid";
 import { createMirrorJob } from "@/lib/helpers";
 import {
@@ -47,13 +47,14 @@ export const POST: APIRoute = async ({ request, locals }) => {
    const octokit = createGitHubClient(decryptedToken, userId, githubUsername);

    // Fetch GitHub data in parallel
-    const [basicAndForkedRepos, starredRepos, gitOrgs] = await Promise.all([
+    const [basicAndForkedRepos, starredRepos, orgResult] = await Promise.all([
      getGithubRepositories({ octokit, config }),
      config.githubConfig?.includeStarred
        ? getGithubStarredRepositories({ octokit, config })
        : Promise.resolve([]),
      getGithubOrganizations({ octokit, config }),
    ]);
+    const { organizations: gitOrgs, failedOrgs } = orgResult;

    // Merge and de-duplicate by fullName, preferring starred variant when duplicated
    const allGithubRepos = mergeGitReposPreferStarred(basicAndForkedRepos, starredRepos);
@@ -108,8 +109,27 @@ export const POST: APIRoute = async ({ request, locals }) => {
      updatedAt: new Date(),
    }));

+    // Prepare failed org records for DB insertion
+    const failedOrgRecords = failedOrgs.map((org) => ({
+      id: uuidv4(),
+      userId,
+      configId: config.id,
+      name: org.name,
+      normalizedName: org.name.toLowerCase(),
+      avatarUrl: org.avatarUrl,
+      membershipRole: "member" as const,
+      isIncluded: false,
+      status: "failed" as const,
+      errorMessage: org.reason,
+      repositoryCount: 0,
+      createdAt: new Date(),
+      updatedAt: new Date(),
+    }));
+
    let insertedRepos: typeof newRepos = [];
    let insertedOrgs: typeof newOrgs = [];
+    let insertedFailedOrgs: typeof failedOrgRecords = [];
+    let recoveredOrgCount = 0;

    // Transaction to insert only new items
    await db.transaction(async (tx) => {
@@ -119,18 +139,62 @@ export const POST: APIRoute = async ({ request, locals }) => {
          .from(repositories)
          .where(eq(repositories.userId, userId)),
        tx
-          .select({ normalizedName: organizations.normalizedName })
+          .select({ normalizedName: organizations.normalizedName, status: organizations.status })
          .from(organizations)
          .where(eq(organizations.userId, userId)),
      ]);

      const existingRepoNames = new Set(existingRepos.map((r) => r.normalizedFullName));
-      const existingOrgNames = new Set(existingOrgs.map((o) => o.normalizedName));
+      const existingOrgMap = new Map(existingOrgs.map((o) => [o.normalizedName, o.status]));

      insertedRepos = newRepos.filter(
        (r) => !existingRepoNames.has(r.normalizedFullName)
      );
-      insertedOrgs = newOrgs.filter((o) => !existingOrgNames.has(o.normalizedName));
+      insertedOrgs = newOrgs.filter((o) => !existingOrgMap.has(o.normalizedName));
+
+      // Update previously failed orgs that now succeeded
+      const recoveredOrgs = newOrgs.filter(
+        (o) => existingOrgMap.get(o.normalizedName) === "failed"
+      );
+      for (const org of recoveredOrgs) {
+        await tx
+          .update(organizations)
+          .set({
+            status: "imported",
+            errorMessage: null,
+            repositoryCount: org.repositoryCount,
+            avatarUrl: org.avatarUrl,
+            membershipRole: org.membershipRole,
+            updatedAt: new Date(),
+          })
+          .where(
+            and(
+              eq(organizations.userId, userId),
+              eq(organizations.normalizedName, org.normalizedName),
+            )
+          );
+      }
+      recoveredOrgCount = recoveredOrgs.length;
+
+      // Insert or update failed orgs (only update orgs already in "failed" state — don't overwrite good state)
+      insertedFailedOrgs = failedOrgRecords.filter((o) => !existingOrgMap.has(o.normalizedName));
+      const stillFailedOrgs = failedOrgRecords.filter(
+        (o) => existingOrgMap.get(o.normalizedName) === "failed"
+      );
+      for (const org of stillFailedOrgs) {
+        await tx
+          .update(organizations)
+          .set({
+            errorMessage: org.errorMessage,
+            updatedAt: new Date(),
+          })
+          .where(
+            and(
+              eq(organizations.userId, userId),
+              eq(organizations.normalizedName, org.normalizedName),
+            )
+          );
+      }

      // Batch insert repositories to avoid SQLite parameter limit (dynamic by column count)
      const sample = newRepos[0];
@@ -148,9 +212,10 @@ export const POST: APIRoute = async ({ request, locals }) => {

      // Batch insert organizations (they have fewer fields, so we can use larger batches)
      const ORG_BATCH_SIZE = 100;
-      if (insertedOrgs.length > 0) {
-        for (let i = 0; i < insertedOrgs.length; i += ORG_BATCH_SIZE) {
-          const batch = insertedOrgs.slice(i, i + ORG_BATCH_SIZE);
+      const allNewOrgs = [...insertedOrgs, ...insertedFailedOrgs];
+      if (allNewOrgs.length > 0) {
+        for (let i = 0; i < allNewOrgs.length; i += ORG_BATCH_SIZE) {
+          const batch = allNewOrgs.slice(i, i + ORG_BATCH_SIZE);
          await tx.insert(organizations).values(batch);
        }
      }
@@ -189,6 +254,8 @@ export const POST: APIRoute = async ({ request, locals }) => {
        newRepositories: insertedRepos.length,
        newOrganizations: insertedOrgs.length,
        skippedDisabledRepositories: allGithubRepos.length - mirrorableGithubRepos.length,
+        failedOrgs: failedOrgs.map((o) => o.name),
+        recoveredOrgs: recoveredOrgCount,
      },
    });
  } catch (error) {