Skip to content

Commit 54cbd0c

Browse files
rperonkentcdodds
andauthored
Chore/update remix auth GitHub (#944)
* chore: upgrade remix-auth and remix-auth-github dependencies Update authentication libraries to latest versions: - remix-auth from 3.7.0 to 4.1.0 - remix-auth-github from 1.7.0 to 3.0.2 Refactor authentication implementation to match new library requirements, including: - Removing connection session storage - Updating GitHub strategy configuration - Adjusting authentication callback handling * fix profile and email fetching * add depencies to @mjackson/headers * remove variable * Fix most test an linting issue * Fix GitHub OAuth callback test response handling --------- Co-authored-by: Kent C. Dodds <[email protected]>
1 parent 47ee651 commit 54cbd0c

File tree

10 files changed

+136
-110
lines changed

10 files changed

+136
-110
lines changed

app/routes/_auth+/auth.$provider.callback.test.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { invariant } from '@epic-web/invariant'
22
import { faker } from '@faker-js/faker'
3+
import { SetCookie } from '@mjackson/headers'
34
import { http } from 'msw'
45
import { afterEach, expect, test } from 'vitest'
56
import { twoFAVerificationType } from '#app/routes/settings+/profile.two-factor.tsx'
67
import { getSessionExpirationDate, sessionKey } from '#app/utils/auth.server.ts'
7-
import { connectionSessionStorage } from '#app/utils/connections.server.ts'
88
import { GITHUB_PROVIDER_NAME } from '#app/utils/connections.tsx'
99
import { prisma } from '#app/utils/db.server.ts'
1010
import { authSessionStorage } from '#app/utils/session.server.ts'
@@ -35,7 +35,7 @@ test('when auth fails, send the user to login with a toast', async () => {
3535
consoleError.mockImplementation(() => {})
3636
server.use(
3737
http.post('https://github.com/login/oauth/access_token', async () => {
38-
return new Response('error', { status: 400 })
38+
return new Response(null, { status: 400 })
3939
}),
4040
)
4141
const request = await setupRequest()
@@ -219,19 +219,25 @@ async function setupRequest({
219219
const state = faker.string.uuid()
220220
url.searchParams.set('state', state)
221221
url.searchParams.set('code', code)
222-
const connectionSession = await connectionSessionStorage.getSession()
223-
connectionSession.set('oauth2:state', state)
224222
const authSession = await authSessionStorage.getSession()
225223
if (sessionId) authSession.set(sessionKey, sessionId)
226224
const setSessionCookieHeader =
227225
await authSessionStorage.commitSession(authSession)
228-
const setConnectionSessionCookieHeader =
229-
await connectionSessionStorage.commitSession(connectionSession)
226+
const searchParams = new URLSearchParams({ code, state })
227+
let authCookie = new SetCookie({
228+
name: 'github',
229+
value: searchParams.toString(),
230+
path: '/',
231+
sameSite: 'Lax',
232+
httpOnly: true,
233+
maxAge: 60 * 10,
234+
secure: process.env.NODE_ENV === 'production' || undefined,
235+
})
230236
const request = new Request(url.toString(), {
231237
method: 'GET',
232238
headers: {
233239
cookie: [
234-
convertSetCookieToCookie(setConnectionSessionCookieHeader),
240+
authCookie.toString(),
235241
convertSetCookieToCookie(setSessionCookieHeader),
236242
].join('; '),
237243
},

app/routes/_auth+/auth.$provider.callback.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export async function loader({ request, params }: Route.LoaderArgs) {
3838
const label = providerLabels[providerName]
3939

4040
const authResult = await authenticator
41-
.authenticate(providerName, request, { throwOnError: true })
41+
.authenticate(providerName, request)
4242
.then(
4343
(data) =>
4444
({

app/routes/_auth+/onboarding_.$provider.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import {
2323
signupWithConnection,
2424
requireAnonymous,
2525
} from '#app/utils/auth.server.ts'
26-
import { connectionSessionStorage } from '#app/utils/connections.server'
2726
import { ProviderNameSchema } from '#app/utils/connections.tsx'
2827
import { prisma } from '#app/utils/db.server.ts'
2928
import { useIsPending } from '#app/utils/misc.tsx'
@@ -78,24 +77,17 @@ async function requireData({
7877

7978
export async function loader({ request, params }: Route.LoaderArgs) {
8079
const { email } = await requireData({ request, params })
81-
const connectionSession = await connectionSessionStorage.getSession(
82-
request.headers.get('cookie'),
83-
)
80+
8481
const verifySession = await verifySessionStorage.getSession(
8582
request.headers.get('cookie'),
8683
)
8784
const prefilledProfile = verifySession.get(prefilledProfileKey)
8885

89-
const formError = connectionSession.get(authenticator.sessionErrorKey)
90-
const hasError = typeof formError === 'string'
91-
9286
return {
9387
email,
9488
status: 'idle',
9589
submission: {
96-
status: hasError ? 'error' : undefined,
9790
initialValue: prefilledProfile ?? {},
98-
error: { '': hasError ? [formError] : [] },
9991
} as SubmissionResult,
10092
}
10193
}

app/utils/auth.server.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import bcrypt from 'bcryptjs'
33
import { redirect } from 'react-router'
44
import { Authenticator } from 'remix-auth'
55
import { safeRedirect } from 'remix-utils/safe-redirect'
6-
import { connectionSessionStorage, providers } from './connections.server.ts'
6+
import { providers } from './connections.server.ts'
77
import { prisma } from './db.server.ts'
88
import { combineHeaders, downloadFile } from './misc.tsx'
99
import { type ProviderUser } from './providers/provider.ts'
@@ -16,9 +16,7 @@ export const getSessionExpirationDate = () =>
1616

1717
export const sessionKey = 'sessionId'
1818

19-
export const authenticator = new Authenticator<ProviderUser>(
20-
connectionSessionStorage,
21-
)
19+
export const authenticator = new Authenticator<ProviderUser>()
2220

2321
for (const [providerName, provider] of Object.entries(providers)) {
2422
authenticator.use(provider.getAuthStrategy(), providerName)

app/utils/connections.server.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,9 @@
1-
import { createCookieSessionStorage } from 'react-router'
1+
// import { createCookieSessionStorage } from 'react-router'
22
import { type ProviderName } from './connections.tsx'
33
import { GitHubProvider } from './providers/github.server.ts'
44
import { type AuthProvider } from './providers/provider.ts'
55
import { type Timings } from './timing.server.ts'
66

7-
export const connectionSessionStorage = createCookieSessionStorage({
8-
cookie: {
9-
name: 'en_connection',
10-
sameSite: 'lax', // CSRF protection is advised if changing to 'none'
11-
path: '/',
12-
httpOnly: true,
13-
maxAge: 60 * 10, // 10 minutes
14-
secrets: process.env.SESSION_SECRET.split(','),
15-
secure: process.env.NODE_ENV === 'production',
16-
},
17-
})
18-
197
export const providers: Record<ProviderName, AuthProvider> = {
208
github: new GitHubProvider(),
219
}

app/utils/providers/github.server.ts

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1+
import { SetCookie } from '@mjackson/headers'
12
import { createId as cuid } from '@paralleldrive/cuid2'
23
import { redirect } from 'react-router'
34
import { GitHubStrategy } from 'remix-auth-github'
45
import { z } from 'zod'
56
import { cache, cachified } from '../cache.server.ts'
6-
import { connectionSessionStorage } from '../connections.server.ts'
77
import { type Timings } from '../timing.server.ts'
88
import { MOCK_CODE_GITHUB_HEADER, MOCK_CODE_GITHUB } from './constants.ts'
99
import { type AuthProvider } from './provider.ts'
@@ -24,27 +24,62 @@ const shouldMock =
2424
process.env.GITHUB_CLIENT_ID?.startsWith('MOCK_') ||
2525
process.env.NODE_ENV === 'test'
2626

27+
type GitHubEmailsResponse = {
28+
email: string
29+
verified: boolean
30+
primary: boolean
31+
visibility: string | null
32+
}[]
33+
34+
type GitHubUserResponse = {
35+
login: string
36+
id: string
37+
name: string | undefined
38+
avatar_url: string | undefined
39+
}
40+
2741
export class GitHubProvider implements AuthProvider {
2842
getAuthStrategy() {
2943
return new GitHubStrategy(
3044
{
31-
clientID: process.env.GITHUB_CLIENT_ID,
45+
clientId: process.env.GITHUB_CLIENT_ID,
3246
clientSecret: process.env.GITHUB_CLIENT_SECRET,
33-
callbackURL: '/auth/github/callback',
47+
redirectURI: '/auth/github/callback',
3448
},
35-
async ({ profile }) => {
36-
const email = profile.emails[0]?.value.trim().toLowerCase()
49+
async ({ tokens }) => {
50+
// we need to fetch the user and the emails separately, this is a change in remix-auth-github
51+
// from the previous version that supported fetching both in one call
52+
const userResponse = await fetch('https://api.github.com/user', {
53+
headers: {
54+
Accept: 'application/vnd.github+json',
55+
Authorization: `Bearer ${tokens.accessToken()}`,
56+
'X-GitHub-Api-Version': '2022-11-28',
57+
},
58+
})
59+
const user = (await userResponse.json()) as GitHubUserResponse
60+
61+
const emailsResponse = await fetch(
62+
'https://api.github.com/user/emails',
63+
{
64+
headers: {
65+
Accept: 'application/vnd.github+json',
66+
Authorization: `Bearer ${tokens.accessToken()}`,
67+
'X-GitHub-Api-Version': '2022-11-28',
68+
},
69+
},
70+
)
71+
const emails = (await emailsResponse.json()) as GitHubEmailsResponse
72+
const email = emails.find((e) => e.primary)?.email
3773
if (!email) {
3874
throw new Error('Email not found')
3975
}
40-
const username = profile.displayName
41-
const imageUrl = profile.photos[0]?.value
76+
4277
return {
78+
id: user.id,
4379
email,
44-
id: profile.id,
45-
username,
46-
name: profile.name.givenName,
47-
imageUrl,
80+
name: user.name,
81+
username: user.login,
82+
imageUrl: user.avatar_url,
4883
}
4984
},
5085
)
@@ -85,21 +120,24 @@ export class GitHubProvider implements AuthProvider {
85120
async handleMockAction(request: Request) {
86121
if (!shouldMock) return
87122

88-
const connectionSession = await connectionSessionStorage.getSession(
89-
request.headers.get('cookie'),
90-
)
91123
const state = cuid()
92-
connectionSession.set('oauth2:state', state)
93-
94124
// allows us to inject a code when running e2e tests,
95125
// but falls back to a pre-defined 🐨 constant
96126
const code =
97127
request.headers.get(MOCK_CODE_GITHUB_HEADER) || MOCK_CODE_GITHUB
98128
const searchParams = new URLSearchParams({ code, state })
129+
let cookie = new SetCookie({
130+
name: 'github',
131+
value: searchParams.toString(),
132+
path: '/',
133+
sameSite: 'Lax',
134+
httpOnly: true,
135+
maxAge: 60 * 10,
136+
secure: process.env.NODE_ENV === 'production' || undefined,
137+
})
99138
throw redirect(`/auth/github/callback?${searchParams}`, {
100139
headers: {
101-
'set-cookie':
102-
await connectionSessionStorage.commitSession(connectionSession),
140+
'Set-Cookie': cookie.toString(),
103141
},
104142
})
105143
}

app/utils/providers/provider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { type Strategy } from 'remix-auth'
1+
import { type Strategy } from 'remix-auth/strategy'
22
import { type Timings } from '../timing.server.ts'
33

44
// Define a user type for cleaner typing

0 commit comments

Comments
 (0)