-
Notifications
You must be signed in to change notification settings - Fork 451
Upgrade react router and add auth middleware #1040
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
base: main
Are you sure you want to change the base?
Changes from all commits
9f9766a
05e00cf
1e86901
d0db7c6
ff7fc72
426b315
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import { createContext } from 'react-router' | ||
|
||
// Holds the authenticated user's ID for routes protected by middleware | ||
export const userIdContext = createContext<string | null>(null) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import { redirect, type MiddlewareFunction } from 'react-router' | ||
import { userIdContext } from '#app/context.ts' | ||
import { prisma } from '#app/utils/db.server.ts' | ||
import { authSessionStorage } from '#app/utils/session.server.ts' | ||
|
||
export const requireUserMiddleware: MiddlewareFunction = async ({ | ||
request, | ||
context, | ||
}) => { | ||
const cookie = request.headers.get('cookie') | ||
const session = await authSessionStorage.getSession(cookie) | ||
const sessionId = session.get('sessionId') as string | undefined | ||
if (!sessionId) | ||
throw redirect( | ||
`/login?redirectTo=${encodeURIComponent(new URL(request.url).pathname)}`, | ||
) | ||
|
||
const sessionRecord = await prisma.session.findUnique({ | ||
select: { userId: true, expirationDate: true }, | ||
where: { id: sessionId }, | ||
}) | ||
|
||
if (!sessionRecord || sessionRecord.expirationDate < new Date()) { | ||
throw redirect( | ||
`/login?redirectTo=${encodeURIComponent(new URL(request.url).pathname)}`, | ||
) | ||
} | ||
|
||
context.set(userIdContext, sessionRecord.userId) | ||
} | ||
|
||
export const requireAnonymousMiddleware: MiddlewareFunction = async ({ | ||
request, | ||
}) => { | ||
const cookie = request.headers.get('cookie') | ||
const session = await authSessionStorage.getSession(cookie) | ||
const sessionId = session.get('sessionId') as string | undefined | ||
if (sessionId) throw redirect('/') | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,3 @@ | ||
import { invariant } from '@epic-web/invariant' | ||
import { redirect } from 'react-router' | ||
import { verifySessionStorage } from '#app/utils/verification.server.ts' | ||
import { onboardingEmailSessionKey } from './onboarding.tsx' | ||
import { type VerifyFunctionArgs } from './verify.server.ts' | ||
import { requireAnonymousMiddleware } from '#app/middleware.server.ts' | ||
|
||
export async function handleVerification({ submission }: VerifyFunctionArgs) { | ||
invariant( | ||
submission.status === 'success', | ||
'Submission should be successful by now', | ||
) | ||
const verifySession = await verifySessionStorage.getSession() | ||
verifySession.set(onboardingEmailSessionKey, submission.value.target) | ||
return redirect('/onboarding', { | ||
headers: { | ||
'set-cookie': await verifySessionStorage.commitSession(verifySession), | ||
}, | ||
}) | ||
} | ||
export const middleware = [requireAnonymousMiddleware] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import { requireAnonymousMiddleware } from '#app/middleware.server.ts' | ||
|
||
export const middleware = [requireAnonymousMiddleware] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,21 @@ | ||
import { generateSitemap } from '@nasa-gcn/remix-seo' | ||
import { type ServerBuild } from 'react-router' | ||
import { | ||
type ServerBuild, | ||
type RouterContextProvider, | ||
createContext, | ||
} from 'react-router' | ||
import { getDomainUrl } from '#app/utils/misc.tsx' | ||
import { type Route } from './+types/sitemap[.]xml.ts' | ||
// recreate context key to match the one set in server getLoadContext | ||
export const serverBuildContext = createContext<Promise<{ | ||
error: unknown | ||
build: ServerBuild | ||
}> | null>(null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Sitemap Route Fails to Access Server Build ContextThe |
||
|
||
export async function loader({ request, context }: Route.LoaderArgs) { | ||
const serverBuild = (await context.serverBuild) as { build: ServerBuild } | ||
const serverBuild = (await (context as Readonly<RouterContextProvider>).get( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this actually work? I think the routes can accessed like this if I'm not mistaken:
The |
||
serverBuildContext, | ||
)) as { build: ServerBuild } | ||
|
||
// TODO: This is typeerror is coming up since of the remix-run/server-runtime package. We might need to remove/update that one. | ||
// @ts-expect-error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could use this modified version https://gist.github.com/hilja/5e6f6ec6a86a3e06113501f8d60e20a4 |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import { requireUserMiddleware } from '#app/middleware.server.ts' | ||
|
||
export const middleware = [requireUserMiddleware] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Inconsistent Session Key Causes Authentication Issues
The middleware functions in
app/middleware.server.ts
use a hardcoded'sessionId'
string for session retrieval. This is inconsistent with thesessionKey
constant used throughout the rest of the authentication system, causing authentication failures and incorrect route protection.