Skip to content

Commit 61db07c

Browse files
authored
Log invalid Freebuff login auth codes (#662)
1 parent ee1b878 commit 61db07c

4 files changed

Lines changed: 131 additions & 2 deletions

File tree

freebuff/web/src/app/api/auth/[...nextauth]/auth-options.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import GitHubProvider from 'next-auth/providers/github'
1515
import type { NextAuthOptions } from 'next-auth'
1616
import type { Adapter } from 'next-auth/adapters'
1717

18+
import {
19+
getCliAuthCodeHashPrefix,
20+
isCliAuthCodeCandidate,
21+
} from '@/app/onboard/_helpers'
1822
import { logger } from '@/util/logger'
1923

2024
async function createAndLinkStripeCustomer(params: {
@@ -104,6 +108,31 @@ export const authOptions: NextAuthOptions = {
104108
const authCode = potentialRedirectUrl.searchParams.get('auth_code')
105109

106110
if (authCode) {
111+
if (!isCliAuthCodeCandidate(authCode)) {
112+
const searchParamKeys = Array.from(
113+
potentialRedirectUrl.searchParams.keys(),
114+
).sort()
115+
logger.warn(
116+
{
117+
authCodeLength: authCode.length,
118+
authCodeTrimmedLength: authCode.trim().length,
119+
authCodeHashPrefix: getCliAuthCodeHashPrefix(authCode),
120+
authCodeParamCount:
121+
potentialRedirectUrl.searchParams.getAll('auth_code').length,
122+
searchParamKeys,
123+
searchParamCount: searchParamKeys.length,
124+
hasCallbackUrlParam: searchParamKeys.includes('callbackUrl'),
125+
hasCodeParam: searchParamKeys.includes('code'),
126+
hasRedirectParam: searchParamKeys.includes('redirect'),
127+
dotCount: authCode.match(/\./g)?.length ?? 0,
128+
hyphenCount: authCode.match(/-/g)?.length ?? 0,
129+
redirectUrlOrigin: potentialRedirectUrl.origin,
130+
baseUrl,
131+
},
132+
'Freebuff auth redirect received non-CLI-shaped auth_code',
133+
)
134+
}
135+
107136
const onboardUrl = new URL(`${baseUrl}/onboard`)
108137
potentialRedirectUrl.searchParams.forEach((value, key) => {
109138
onboardUrl.searchParams.set(key, value)

freebuff/web/src/app/login/page.tsx

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
'use server'
22

33
import { env } from '@codebuff/common/env'
4+
import { headers } from 'next/headers'
45

6+
import {
7+
getCliAuthCodeHashPrefix,
8+
isAuthCodeExpired,
9+
isCliAuthCodeCandidate,
10+
parseAuthCode,
11+
} from '@/app/onboard/_helpers'
512
import { BackgroundBeams } from '@/components/background-beams'
613
import { HeroGrid } from '@/components/hero-grid'
714
import { LoginCard } from '@/components/login/login-card'
@@ -12,17 +19,67 @@ import {
1219
CardDescription,
1320
CardContent,
1421
} from '@/components/ui/card'
15-
import { isAuthCodeExpired, parseAuthCode } from '@/app/onboard/_helpers'
22+
import { logger } from '@/util/logger'
1623

1724
export default async function LoginPage({
1825
searchParams,
1926
}: {
2027
searchParams?: Promise<{ [key: string]: string | string[] | undefined }>
2128
}) {
2229
const resolvedSearchParams = searchParams ? await searchParams : {}
23-
const authCode = resolvedSearchParams?.auth_code as string | undefined
30+
const rawAuthCode = resolvedSearchParams?.auth_code
31+
const authCode = Array.isArray(rawAuthCode) ? rawAuthCode[0] : rawAuthCode
32+
const searchParamKeys = Object.keys(resolvedSearchParams).sort()
2433

2534
if (authCode) {
35+
if (!isCliAuthCodeCandidate(authCode)) {
36+
const headerStore = await headers()
37+
logger.warn(
38+
{
39+
authCodeLength: authCode.length,
40+
authCodeTrimmedLength: authCode.trim().length,
41+
authCodeHashPrefix: getCliAuthCodeHashPrefix(authCode),
42+
authCodeParamCount: Array.isArray(rawAuthCode)
43+
? rawAuthCode.length
44+
: 1,
45+
searchParamKeys,
46+
searchParamCount: searchParamKeys.length,
47+
hasCallbackUrlParam: searchParamKeys.includes('callbackUrl'),
48+
hasCodeParam: searchParamKeys.includes('code'),
49+
hasRedirectParam: searchParamKeys.includes('redirect'),
50+
dotCount: authCode.match(/\./g)?.length ?? 0,
51+
hyphenCount: authCode.match(/-/g)?.length ?? 0,
52+
requestHost: headerStore.get('host') ?? '',
53+
forwardedHost: headerStore.get('x-forwarded-host') ?? '',
54+
forwardedProto: headerStore.get('x-forwarded-proto') ?? '',
55+
originHeader: headerStore.get('origin') ?? '',
56+
referer: headerStore.get('referer') ?? '',
57+
userAgent: headerStore.get('user-agent') ?? '',
58+
referrerParam:
59+
typeof resolvedSearchParams.referrer === 'string'
60+
? resolvedSearchParams.referrer
61+
: '',
62+
utmSource:
63+
typeof resolvedSearchParams.utm_source === 'string'
64+
? resolvedSearchParams.utm_source
65+
: '',
66+
utmMedium:
67+
typeof resolvedSearchParams.utm_medium === 'string'
68+
? resolvedSearchParams.utm_medium
69+
: '',
70+
utmCampaign:
71+
typeof resolvedSearchParams.utm_campaign === 'string'
72+
? resolvedSearchParams.utm_campaign
73+
: '',
74+
utmContent:
75+
typeof resolvedSearchParams.utm_content === 'string'
76+
? resolvedSearchParams.utm_content
77+
: '',
78+
},
79+
'Freebuff login received non-CLI-shaped auth_code',
80+
)
81+
}
82+
2683
const { expiresAt } = parseAuthCode(authCode)
2784

2885
if (expiresAt && isAuthCodeExpired(expiresAt)) {

freebuff/web/src/app/onboard/__tests__/helpers.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
getConsumedCliAuthCodeTokenIdentifier,
99
getConsumedCliAuthCodeTokenValue,
1010
isAuthCodeExpired,
11+
isCliAuthCodeCandidate,
1112
isOpaqueCliAuthCodeToken,
1213
parseAuthCode,
1314
resolveCliAuthCode,
@@ -114,6 +115,34 @@ describe('freebuff onboard/_helpers', () => {
114115
expect(isOpaqueCliAuthCodeToken(`${'A'.repeat(42)}.`)).toBe(false)
115116
})
116117

118+
test('identifies auth code candidates by supported shapes', () => {
119+
const opaqueToken = 'A'.repeat(41) + '-_'
120+
const signedAuthCode = buildCliAuthCode(
121+
testFingerprintId,
122+
'1704067200000',
123+
'a'.repeat(64),
124+
)
125+
const legacyAuthCode = `1234567890abcdef-1704067200000-${'b'.repeat(
126+
64,
127+
)}`
128+
129+
expect(isCliAuthCodeCandidate(opaqueToken)).toBe(true)
130+
expect(isCliAuthCodeCandidate(signedAuthCode)).toBe(true)
131+
expect(isCliAuthCodeCandidate(legacyAuthCode)).toBe(true)
132+
expect(isCliAuthCodeCandidate(crypto.randomUUID())).toBe(false)
133+
expect(isCliAuthCodeCandidate('F0xe_Mt2yA2az_LUXGxlBsGDIgJ')).toBe(false)
134+
expect(
135+
isCliAuthCodeCandidate(
136+
buildCliAuthCode(testFingerprintId, 'not-a-number', 'a'.repeat(64)),
137+
),
138+
).toBe(false)
139+
expect(
140+
isCliAuthCodeCandidate(
141+
buildCliAuthCode(testFingerprintId, '1704067200000', 'short-hash'),
142+
),
143+
).toBe(false)
144+
})
145+
117146
test('hashes auth codes for log correlation without logging the token', () => {
118147
expect(getCliAuthCodeHashPrefix('a'.repeat(43))).toBe('66d34fba71f8')
119148
expect(getCliAuthCodeHashPrefix(` ${'a'.repeat(43)}\n`)).toBe(

freebuff/web/src/app/onboard/_helpers.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createHash } from 'node:crypto'
33
import { genAuthCode } from '@codebuff/common/util/credentials'
44

55
const OPAQUE_CLI_AUTH_CODE_TOKEN_RE = /^[A-Za-z0-9_-]{43}$/
6+
const CLI_AUTH_CODE_HASH_RE = /^[a-f0-9]{64}$/i
67
const CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX = 'cli-login:'
78
const CONSUMED_CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX = 'cli-login-consumed:'
89
const CONSUMED_CLI_AUTH_CODE_TOKEN_VALUE = 'consumed'
@@ -23,6 +24,19 @@ export function isOpaqueCliAuthCodeToken(authCode: string): boolean {
2324
return OPAQUE_CLI_AUTH_CODE_TOKEN_RE.test(authCode.trim())
2425
}
2526

27+
export function isCliAuthCodeCandidate(authCode: string): boolean {
28+
if (isOpaqueCliAuthCodeToken(authCode)) {
29+
return true
30+
}
31+
32+
const { fingerprintId, expiresAt, receivedHash } = parseAuthCode(authCode)
33+
return (
34+
fingerprintId.length > 0 &&
35+
/^\d+$/.test(expiresAt) &&
36+
CLI_AUTH_CODE_HASH_RE.test(receivedHash)
37+
)
38+
}
39+
2640
export function getCliAuthCodeHashPrefix(authCode: string): string {
2741
return getCliAuthCodeHash(authCode).slice(0, 12)
2842
}

0 commit comments

Comments
 (0)