Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: middleware race condition when checking auth validity #162

Merged
merged 2 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/server/git/router.ts
Original file line number Diff line number Diff line change
@@ -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 })),
})
Expand Down
17 changes: 12 additions & 5 deletions src/utils/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
}
Expand Down
30 changes: 16 additions & 14 deletions src/utils/trpc-middleware.ts
Original file line number Diff line number Diff line change
@@ -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
)
Expand Down
10 changes: 7 additions & 3 deletions src/utils/trpc-server.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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)
Loading