Merge pull request #162 from github-community-projects/ajhenry/middle…

…ware

fix: middleware race condition when checking auth validity

src/server/git/router.ts

@@ -1,9 +1,9 @@
-import { procedure, router } from '../../utils/trpc-server'
+import { gitProcedure, router } from '../../utils/trpc-server'
 import { syncReposHandler } from './controller'
 import { SyncReposSchema } from './schema'
 
 const gitRouter = router({
-  syncRepos: procedure
+  syncRepos: gitProcedure
     .input(SyncReposSchema)
     .mutation(({ input }) => syncReposHandler({ input })),
 })

src/utils/auth.ts

@@ -43,12 +43,19 @@ export const checkGitHubAppInstallationAuth = async (
 
   const octokit = personalOctokit(accessToken)
 
-  const data = await octokit.rest.repos.get({
-    owner: mirrorOrgOwner,
-    repo: mirrorRepo,
-  })
+  const data = await octokit.rest.repos
+    .get({
+      owner: mirrorOrgOwner,
+      repo: mirrorRepo,
+    })
+    .catch((error) => {
+      middlewareLogger.error('Error checking github app installation auth', {
+        error,
+      })
+      return null
+    })
 
-  if (!data.data) {
+  if (!data?.data) {
     middlewareLogger.error('App does not have access to mirror repo')
     throw new TRPCError({ code: 'UNAUTHORIZED' })
   }

src/utils/trpc-middleware.ts

@@ -1,24 +1,26 @@
 import { checkGitHubAppInstallationAuth, checkGitHubAuth } from './auth'
 import { Middleware } from './trpc-server'
 
-export const verifyAuth: Middleware = async (opts) => {
-  const { ctx, rawInput, path } = opts
+export const verifyGitHubAppAuth: Middleware = async (opts) => {
+  const { ctx, rawInput } = opts
+
+  // Check app authentication
+  await checkGitHubAppInstallationAuth(
+    (rawInput as Record<string, string>)?.accessToken,
+    (rawInput as Record<string, string>)?.mirrorOwner,
+    (rawInput as Record<string, string>)?.mirrorName,
+  )
 
-  if (path === 'git.syncRepos') {
-    // Check app authentication
-    checkGitHubAppInstallationAuth(
-      (rawInput as Record<string, string>)?.accessToken,
-      (rawInput as Record<string, string>)?.mirrorOwner,
-      (rawInput as Record<string, string>)?.mirrorName,
-    )
+  return opts.next({
+    ctx,
+  })
+}
 
-    return opts.next({
-      ctx,
-    })
-  }
+export const verifyAuth: Middleware = async (opts) => {
+  const { ctx, rawInput } = opts
 
   // Verify valid github session
-  checkGitHubAuth(
+  await checkGitHubAuth(
     ctx.session?.user?.accessToken,
     (rawInput as Record<string, string>)?.orgId, // Fetch orgId if there is one
   )

src/utils/trpc-server.ts

@@ -1,9 +1,9 @@
 import { initTRPC } from '@trpc/server'
 import { CreateNextContextOptions } from '@trpc/server/adapters/next'
 import { getServerSession } from 'next-auth'
-import { nextAuthOptions } from '../app/api/auth/lib/nextauth-options'
-import { verifyAuth } from '../utils/trpc-middleware'
 import SuperJSON from 'superjson'
+import { nextAuthOptions } from '../app/api/auth/lib/nextauth-options'
+import { verifyAuth, verifyGitHubAppAuth } from '../utils/trpc-middleware'
 
 export const createContext = async (opts: CreateNextContextOptions) => {
   const session = await getServerSession(opts.req, opts.res, nextAuthOptions)
@@ -23,5 +23,9 @@ export const t = initTRPC.context<typeof createContext>().create({
 
 // Base router and procedure helpers
 export const router = t.router
+const publicProcedure = t.procedure
 export type Middleware = Parameters<(typeof t.procedure)['use']>[0]
-export const procedure = t.procedure.use(verifyAuth)
+// Used for general user access token verification
+export const procedure = publicProcedure.use(verifyAuth)
+// Used for GitHub App authentication verification (non-user events like 'push')
+export const gitProcedure = publicProcedure.use(verifyGitHubAppAuth)