Skip to content
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
6 changes: 6 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,9 @@ GITHUB_AUTHZ_ORG=flexion
SESSION_SECRET=generated_random_32_byte_secret
AWS_REGION=us-east-1
AWS_BEDROCK_REGION=us-west-2
# Sign-in allowlist. A user passes if their GitHub login is in
# ALLOWED_USERS OR any of their verified GitHub emails matches a
# domain in ALLOWED_EMAIL_DOMAINS. Either alone is sufficient.
# Leaving both unset defaults to ALLOWED_USERS=danielnaab.
ALLOWED_USERS=danielnaab
ALLOWED_EMAIL_DOMAINS=flexion.us
1 change: 1 addition & 0 deletions infrastructure/nixos/modules/deploy.nix
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ GITHUB_CLIENT_SECRET=$(cat /run/secrets/github-client-secret 2>/dev/null || echo
SESSION_SECRET=$(cat /run/secrets/session-secret 2>/dev/null || echo "")
GITHUB_AUTHZ_ORG=flexion
ALLOWED_USERS=danielnaab,FlexionCodeReview
ALLOWED_EMAIL_DOMAINS=flexion.us
AWS_REGION=us-east-1
AWS_BEDROCK_REGION=us-west-2
CACHE_DB_PATH=/srv/forms-lab/cache.sqlite
Expand Down
55 changes: 47 additions & 8 deletions src/entrypoints/app/routes/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
COOKIE_NAME,
encryptSession,
exchangeCodeForToken,
fetchUserEmails,
fetchUserProfile,
hasAllowedEmailDomain,
} from '../../../../services/auth'
import { resolveUrl } from '../../../../shared/base-path'

Expand Down Expand Up @@ -53,7 +55,10 @@ export function createAuthRoutes(userStore: UserStore): Hono {
const authUrl = new URL('https://github.com/login/oauth/authorize')
authUrl.searchParams.set('client_id', clientId)
authUrl.searchParams.set('redirect_uri', callbackUrl)
authUrl.searchParams.set('scope', 'read:user read:org')
// `user:email` is required to check the user's verified email
// domain against ALLOWED_EMAIL_DOMAINS; `read:org` remains for
// the legacy org-membership path.
authUrl.searchParams.set('scope', 'read:user read:org user:email')
authUrl.searchParams.set('state', state)

return c.redirect(authUrl.toString())
Expand Down Expand Up @@ -98,16 +103,50 @@ export function createAuthRoutes(userStore: UserStore): Hono {
// Fetch user profile
const ghUser = await fetchUserProfile(token)

// TODO: Replace with org membership check once OAuth app is approved
// Temporary allowlist for development
const allowedUsers = (process.env.ALLOWED_USERS ?? 'danielnaab').split(
',',
)
if (!allowedUsers.includes(ghUser.login)) {
console.log(`Authorization failed for user: ${ghUser.login}`)
// Authorization: user passes if their GitHub login is on the
// ALLOWED_USERS list OR any of their verified emails matches a
// domain on ALLOWED_EMAIL_DOMAINS. Either mechanism alone is
// sufficient; both are evaluated so a personal-login dev can
// still get in on an instance with a strict corporate domain.
const allowedUsers = (process.env.ALLOWED_USERS ?? 'danielnaab')
.split(',')
.map((u) => u.trim())
.filter(Boolean)
const allowedDomains = (process.env.ALLOWED_EMAIL_DOMAINS ?? '')
.split(',')
.map((d) => d.trim())
.filter(Boolean)

let authorised = allowedUsers.includes(ghUser.login)
let emailMatchDomain: string | null = null

if (!authorised && allowedDomains.length > 0) {
const emails = await fetchUserEmails(token)
if (hasAllowedEmailDomain(emails, allowedDomains)) {
authorised = true
emailMatchDomain =
emails.find((e) => {
const at = e.email.lastIndexOf('@')
if (at === -1 || !e.verified) return false
const domain = e.email.slice(at + 1).toLowerCase()
return allowedDomains.map((d) => d.toLowerCase()).includes(domain)
})?.email ?? null
}
}

if (!authorised) {
console.log(
`Authorization failed for user: ${ghUser.login} (allowlist=${allowedUsers.length} users, ${allowedDomains.length} domains)`,
)
return c.redirect(resolveUrl('/?error=unauthorized'))
}

if (emailMatchDomain) {
console.log(
`Authorized ${ghUser.login} via email domain match: ${emailMatchDomain}`,
)
}

// Persist user profile
userStore.upsert({
login: ghUser.login,
Expand Down
58 changes: 58 additions & 0 deletions src/services/auth/github-oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,64 @@ export async function fetchUserProfile(token: string): Promise<GitHubUser> {
return await response.json()
}

export interface GitHubEmail {
email: string
primary: boolean
verified: boolean
visibility: string | null
}

/**
* Fetch the authenticated user's email addresses. Requires the
* `user:email` OAuth scope. Returns all emails GitHub has on file
* for the user, including verified-ness and primary-ness.
*
* Note on trust: we only ever treat `verified: true` emails as
* identity signals. GitHub verifies by sending a confirmation email,
* which is a low bar but enough to prevent a user from claiming an
* arbitrary domain they don't control.
*/
export async function fetchUserEmails(token: string): Promise<GitHubEmail[]> {
const response = await fetch('https://api.github.com/user/emails', {
headers: {
Authorization: `Bearer ${token}`,
Accept: 'application/vnd.github.v3+json',
},
})

if (!response.ok) {
// 404 here means the token lacks user:email scope. Return empty
// so the caller can decide whether that is fatal (strict email
// policy) or tolerable (login-allowlist fallback).
console.warn(`Email fetch failed: HTTP ${response.status}`)
return []
}

return await response.json()
}

/**
* Returns true when the user has a verified email whose domain
* matches one of the allowed domains (case-insensitive exact match).
* Returns false if `allowedDomains` is empty — callers should treat
* an empty list as "no domain policy" rather than "everyone denied".
*/
export function hasAllowedEmailDomain(
emails: GitHubEmail[],
allowedDomains: string[],
): boolean {
if (allowedDomains.length === 0) return false
const normalised = allowedDomains.map((d) => d.toLowerCase().trim())
for (const email of emails) {
if (!email.verified) continue
const at = email.email.lastIndexOf('@')
if (at === -1) continue
const domain = email.email.slice(at + 1).toLowerCase()
if (normalised.includes(domain)) return true
}
return false
}

export async function checkOrgMembership(
token: string,
org: string,
Expand Down
3 changes: 3 additions & 0 deletions src/services/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ export {
checkOrgMembership,
checkRepoPermission,
exchangeCodeForToken,
fetchUserEmails,
fetchUserProfile,
type GitHubEmail,
type GitHubUser,
hasAllowedEmailDomain,
} from './github-oauth'

export {
Expand Down
4 changes: 3 additions & 1 deletion test/auth-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ describe('Auth Routes', () => {
expect(url.origin).toBe('https://github.com')
expect(url.pathname).toBe('/login/oauth/authorize')
expect(url.searchParams.get('client_id')).toBe('test_client_id')
expect(url.searchParams.get('scope')).toBe('read:user read:org')
expect(url.searchParams.get('scope')).toBe(
'read:user read:org user:email',
)
expect(url.searchParams.get('state')).toBeTruthy()
})

Expand Down
113 changes: 113 additions & 0 deletions test/github-oauth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ import {
checkOrgMembership,
checkRepoPermission,
exchangeCodeForToken,
fetchUserEmails,
fetchUserProfile,
type GitHubEmail,
type GitHubUser,
hasAllowedEmailDomain,
} from '../src/services/auth'

describe('GitHub OAuth', () => {
Expand Down Expand Up @@ -215,4 +218,114 @@ describe('GitHub OAuth', () => {
expect(hasPermission).toBe(false)
})
})

describe('fetchUserEmails', () => {
it('returns the email list on success', async () => {
const emails: GitHubEmail[] = [
{
email: 'daniel@flexion.us',
primary: true,
verified: true,
visibility: 'private',
},
{
email: 'daniel@example.com',
primary: false,
verified: false,
visibility: null,
},
]
global.fetch = mock(
() =>
Promise.resolve(
new Response(JSON.stringify(emails), { status: 200 }),
),
// biome-ignore lint/suspicious/noExplicitAny: mock signature
) as any

const result = await fetchUserEmails('gho_test_token')
expect(result).toHaveLength(2)
expect(result[0].email).toBe('daniel@flexion.us')
})

it('returns empty array when scope is missing', async () => {
global.fetch = mock(
() => Promise.resolve(new Response('', { status: 404 })),
// biome-ignore lint/suspicious/noExplicitAny: mock signature
) as any

const result = await fetchUserEmails('gho_test_token')
expect(result).toEqual([])
})
})

describe('hasAllowedEmailDomain', () => {
const emails: GitHubEmail[] = [
{
email: 'person@flexion.us',
primary: true,
verified: true,
visibility: null,
},
{
email: 'person@example.com',
primary: false,
verified: true,
visibility: null,
},
{
email: 'person@unverified-flexion.us',
primary: false,
verified: false,
visibility: null,
},
]

it('matches a verified email on the allowed domain', () => {
expect(hasAllowedEmailDomain(emails, ['flexion.us'])).toBe(true)
})

it('is case-insensitive on the domain', () => {
expect(hasAllowedEmailDomain(emails, ['FLEXION.US'])).toBe(true)
const cased: GitHubEmail[] = [
{ ...emails[0], email: 'Person@Flexion.US' },
]
expect(hasAllowedEmailDomain(cased, ['flexion.us'])).toBe(true)
})

it('ignores unverified emails even when the domain matches', () => {
const unverified: GitHubEmail[] = [
{ ...emails[2], email: 'person@flexion.us', verified: false },
]
expect(hasAllowedEmailDomain(unverified, ['flexion.us'])).toBe(false)
})

it('returns false when no domain matches', () => {
expect(hasAllowedEmailDomain(emails, ['other.org'])).toBe(false)
})

it('returns false when allowedDomains is empty', () => {
expect(hasAllowedEmailDomain(emails, [])).toBe(false)
})

it('supports multiple allowed domains', () => {
expect(hasAllowedEmailDomain(emails, ['other.org', 'flexion.us'])).toBe(
true,
)
})

it('does not match substrings of a domain', () => {
// "not-flexion.us" ends with "flexion.us" but is not equal; the
// check must not admit it.
const tricky: GitHubEmail[] = [
{
email: 'attacker@not-flexion.us',
primary: true,
verified: true,
visibility: null,
},
]
expect(hasAllowedEmailDomain(tricky, ['flexion.us'])).toBe(false)
})
})
})
Loading