Skip to content

Commit 6fd10a5

Browse files
fix(security): use maxAge instead of expires for session cookies (#209)
Session cookies were expiring immediately because the expires date was calculated at server startup time. After 7+ days of server uptime, all new cookies would have an expiration date in the past. Also adds error feedback display on login page when email verification fails (e.g., burner email, invalid format). Closes rb-login-broken Amp-Thread-ID: https://ampcode.com/threads/T-019bed38-9b38-74b0-8aa7-70eaf424dcc6 Co-authored-by: Amp <amp@ampcode.com>
1 parent 1ee9658 commit 6fd10a5

3 files changed

Lines changed: 39 additions & 9 deletions

File tree

app/components/alerts.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CheckCircleIcon } from '@heroicons/react/solid'
1+
import { CheckCircleIcon, XCircleIcon } from '@heroicons/react/solid'
22
import { ReactNode } from 'react'
33

44
export function Alert({ children }: { children: ReactNode }) {
@@ -18,3 +18,18 @@ export function Alert({ children }: { children: ReactNode }) {
1818
</div>
1919
)
2020
}
21+
22+
export function ErrorAlert({ children }: { children: ReactNode }) {
23+
return (
24+
<div className="bg-red-50 border-l-4 border-red-400 py-3 px-4 mt-4">
25+
<div className="flex">
26+
<div className="flex-shrink-0">
27+
<XCircleIcon className="h-5 w-5 text-red-400" aria-hidden="true" />
28+
</div>
29+
<div className="ml-3">
30+
<p className="text-sm font-medium text-red-900">{children}</p>
31+
</div>
32+
</div>
33+
</div>
34+
)
35+
}

app/routes/login.tsx

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,32 @@
11
import type { ActionFunction, LoaderFunction } from '@remix-run/node'
22
import { json } from '@remix-run/node'
33
import { Form, useLoaderData, useNavigation } from '@remix-run/react'
4-
import { Alert } from '~/components/alerts'
4+
import { Alert, ErrorAlert } from '~/components/alerts'
55
import { Button } from '~/components/form-elements'
66
import { auth } from '~/services/auth.server'
7-
import { getUserSession } from '~/services/session.server'
7+
import { commitSession, getUserSession } from '~/services/session.server'
88

99
export const loader: LoaderFunction = async ({ request }) => {
1010
await auth.isAuthenticated(request, { successRedirect: '/dashboard' })
1111
const session = await getUserSession(request)
1212
// This session key `auth:magiclink` is the default one used by the EmailLinkStrategy
1313
// you can customize it passing a `sessionMagicLinkKey` when creating an
1414
// instance.
15-
return json({
16-
user: session.get('user'),
17-
magicLinkSent: session.has('zain:magiclink'),
18-
})
15+
const error = session.get(auth.sessionErrorKey) as
16+
| { message: string }
17+
| undefined
18+
return json(
19+
{
20+
user: session.get('user'),
21+
magicLinkSent: session.has('zain:magiclink'),
22+
error: error?.message,
23+
},
24+
{
25+
headers: {
26+
'Set-Cookie': await commitSession(session),
27+
},
28+
}
29+
)
1930
}
2031

2132
export const action: ActionFunction = async ({ request }) => {
@@ -31,7 +42,10 @@ export const action: ActionFunction = async ({ request }) => {
3142
}
3243

3344
export default function Login() {
34-
const { magicLinkSent } = useLoaderData<{ magicLinkSent: boolean }>()
45+
const { magicLinkSent, error } = useLoaderData<{
46+
magicLinkSent: boolean
47+
error?: string
48+
}>()
3549
const { state } = useNavigation()
3650

3751
return (
@@ -79,6 +93,7 @@ export default function Login() {
7993
</Button>
8094
</div>
8195
</Form>
96+
{error ? <ErrorAlert>{error}</ErrorAlert> : null}
8297
{magicLinkSent ? (
8398
<Form action="/logout" method="post">
8499
<input type="hidden" name="redirectTo" value="/login" />

app/services/session.server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export const sessionStorage = createCookieSessionStorage({
77
sameSite: 'lax',
88
path: '/',
99
httpOnly: true,
10-
expires: new Date(Date.now() + 7 * 24 * 60 * 60 * 1000), // 7 days
10+
maxAge: 7 * 24 * 60 * 60, // 7 days in seconds
1111
secrets: [getRequiredServerEnvVar('SESSION_SECRET')],
1212
// normally you want this to be `secure: true`
1313
// but that doesn't work on localhost for Safari

0 commit comments

Comments
 (0)