feat(auth): accept configured native Google ID token audiences (#2518)
This commit is contained in:
@@ -42,6 +42,32 @@ pnpm dev:backend
|
||||
stay on its private network. The internal `/internal/*` boundary has no
|
||||
application token, and Caddy rejects that path at the public edge.
|
||||
|
||||
## Native Google sign-in
|
||||
|
||||
Set `AUTH_GOOGLE_NATIVE_CLIENT_IDS` to a comma-separated list of additional Google OAuth client IDs.
|
||||
For Android Credential Manager, include the Web client ID passed as `serverClientId`.
|
||||
|
||||
- For the standalone Auth process, set the variable in `server/apps/auth/.env.local`.
|
||||
- For `pnpm dev:backend`, set it in `server/apps/api/.env.local`.
|
||||
Compose loads `server/apps/api/.env` and `.env.local` into the Auth container, in that order.
|
||||
It does not load `server/apps/auth/.env.local`.
|
||||
Run `pnpm dev:backend` again after edits so Compose recreates the container with the updated values.
|
||||
- For Railway, set it in the Auth service variables for the target environment.
|
||||
|
||||
```dotenv
|
||||
AUTH_GOOGLE_NATIVE_CLIENT_IDS=123456789-native.apps.googleusercontent.com
|
||||
```
|
||||
|
||||
The original `AUTH_GOOGLE_CLIENT_ID` stays first in the provider configuration.
|
||||
Browser authorization still uses that client and `AUTH_GOOGLE_CLIENT_SECRET`.
|
||||
Native ID tokens can use any configured audience. Better Auth checks the token signature, issuer, expiry, and supplied nonce.
|
||||
Omit the new variable to keep the existing configuration. No database migration is required.
|
||||
|
||||
Google ID token sign-in can create an account without a Google access or refresh token.
|
||||
Account deletion continues when neither token is stored, because AIRI has no Google API credential to revoke.
|
||||
This does not revoke consent in the user's Google Account.
|
||||
If either token is stored, Auth must complete its existing revocation policy before it deletes AIRI data.
|
||||
|
||||
## Railway
|
||||
|
||||
Deploy this as the Auth Railway service with Config File Path
|
||||
|
||||
@@ -23,6 +23,7 @@ import { eq } from 'drizzle-orm'
|
||||
import * as authSchema from '@proj-airi/auth-shared'
|
||||
|
||||
import { ApiError } from './error'
|
||||
import { googleClientIds } from './google-client-ids'
|
||||
import { getAuthTrustedOrigins, getTrustedOrigin } from './origin'
|
||||
import { banGuard } from './plugins/ban-guard'
|
||||
import { oidcJwtBearer } from './plugins/oidc-jwt-bearer'
|
||||
@@ -660,7 +661,7 @@ export function createAuth(
|
||||
|
||||
socialProviders: {
|
||||
google: {
|
||||
clientId: env.AUTH_GOOGLE_CLIENT_ID,
|
||||
clientId: googleClientIds(env.AUTH_GOOGLE_CLIENT_ID, env.AUTH_GOOGLE_NATIVE_CLIENT_IDS),
|
||||
clientSecret: env.AUTH_GOOGLE_CLIENT_SECRET,
|
||||
// Force the provider's authorization page to let users choose an
|
||||
// identity before linking. Without this, an existing provider session
|
||||
|
||||
@@ -5,6 +5,8 @@ import { exit } from 'node:process'
|
||||
import { useLogger } from '@guiiai/logg'
|
||||
import { array, integer, minValue, nonEmpty, object, optional, parse, picklist, pipe, string, transform, url } from 'valibot'
|
||||
|
||||
import { GoogleNativeClientIdsSchema } from './google-client-ids'
|
||||
|
||||
function optionalIntegerFromString(defaultValue: number, envKey: string, minimum: number) {
|
||||
return optional(
|
||||
pipe(
|
||||
@@ -41,6 +43,7 @@ const AuthEnvSchema = object({
|
||||
REDIS_URL: pipe(string(), nonEmpty('REDIS_URL is required')),
|
||||
BETTER_AUTH_SECRET: pipe(string(), nonEmpty('BETTER_AUTH_SECRET is required')),
|
||||
AUTH_GOOGLE_CLIENT_ID: pipe(string(), nonEmpty('AUTH_GOOGLE_CLIENT_ID is required')),
|
||||
AUTH_GOOGLE_NATIVE_CLIENT_IDS: optional(GoogleNativeClientIdsSchema),
|
||||
AUTH_GOOGLE_CLIENT_SECRET: pipe(string(), nonEmpty('AUTH_GOOGLE_CLIENT_SECRET is required')),
|
||||
AUTH_GITHUB_CLIENT_ID: pipe(string(), nonEmpty('AUTH_GITHUB_CLIENT_ID is required')),
|
||||
AUTH_GITHUB_CLIENT_SECRET: pipe(string(), nonEmpty('AUTH_GITHUB_CLIENT_SECRET is required')),
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import { array, pipe, regex, string, transform } from 'valibot'
|
||||
|
||||
/**
|
||||
* Parses explicit Google OAuth audiences without accepting arbitrary hosts or wildcard values.
|
||||
* Accepts legacy numeric IDs and IDs with a project suffix.
|
||||
*
|
||||
* @example
|
||||
* parse(GoogleNativeClientIdsSchema, ' 123.apps.googleusercontent.com,123.apps.googleusercontent.com, ')
|
||||
* // => ['123.apps.googleusercontent.com']
|
||||
*/
|
||||
export const GoogleNativeClientIdsSchema = pipe(
|
||||
string(),
|
||||
transform(raw => raw.split(',').map(value => value.trim()).filter(Boolean)),
|
||||
array(pipe(string(), regex(/^\d+(?:-[a-z0-9]+)?\.apps\.googleusercontent\.com$/, 'Invalid Google OAuth client ID'))),
|
||||
transform(values => [...new Set(values)]),
|
||||
)
|
||||
|
||||
/**
|
||||
* Keeps the browser client first so native audiences do not change browser OAuth credentials.
|
||||
*
|
||||
* @example
|
||||
* googleClientIds('123.apps.googleusercontent.com', ['456-native.apps.googleusercontent.com', '123.apps.googleusercontent.com'])
|
||||
* // => ['123.apps.googleusercontent.com', '456-native.apps.googleusercontent.com']
|
||||
*/
|
||||
export function googleClientIds(browserClientId: string, nativeClientIds: string[] = []): string | string[] {
|
||||
if (nativeClientIds.length === 0)
|
||||
return browserClientId
|
||||
return [...new Set([browserClientId, ...nativeClientIds])]
|
||||
}
|
||||
@@ -129,7 +129,14 @@ async function revokeGoogleToken(token: string, fetchRequest: typeof fetch): Pro
|
||||
})
|
||||
}
|
||||
|
||||
/** Revokes retained Google API credentials while allowing ID-token-only accounts to be deleted. */
|
||||
async function revokeGoogleAuthorization(account: SocialAccount, fetchRequest: typeof fetch): Promise<void> {
|
||||
// Native Google sign-in can retain only an identity, without OAuth API tokens.
|
||||
// There is no saved credential to revoke in that case. This does not imply
|
||||
// that Google consent was revoked, and must not block AIRI account deletion.
|
||||
if (!account.accessToken && !account.refreshToken)
|
||||
return
|
||||
|
||||
const { token, tokenType } = revocationToken(account)
|
||||
const result = await revokeGoogleToken(token, fetchRequest)
|
||||
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { google } from 'better-auth/social-providers'
|
||||
import { parse } from 'valibot'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { googleClientIds, GoogleNativeClientIdsSchema } from '../google-client-ids'
|
||||
|
||||
const nativeId = '123456789-native.apps.googleusercontent.com'
|
||||
|
||||
describe('native Google audiences', () => {
|
||||
it('preserves the existing browser config when no native clients are configured', () => {
|
||||
expect(googleClientIds('existing-browser-client')).toBe('existing-browser-client')
|
||||
})
|
||||
|
||||
it('keeps the original browser ID first and adds only explicit audiences', () => {
|
||||
const clients = parse(GoogleNativeClientIdsSchema, ` ${nativeId}, ${nativeId}, `)
|
||||
expect(googleClientIds('existing-browser-client', clients)).toEqual(['existing-browser-client', nativeId])
|
||||
})
|
||||
|
||||
// https://github.com/moeru-ai/airi/pull/2518#discussion_r3986076852
|
||||
// ROOT CAUSE:
|
||||
// The required dash rejected legacy Google client IDs during environment parsing.
|
||||
// An optional suffix accepts both forms without changing the audience hostname.
|
||||
it('accepts legacy Google client IDs alongside modern audiences', () => {
|
||||
const legacyId = '123456789.apps.googleusercontent.com'
|
||||
const clients = parse(GoogleNativeClientIdsSchema, ` ${legacyId}, ${nativeId}, ${legacyId} `)
|
||||
expect(googleClientIds('browser-client', clients)).toEqual(['browser-client', legacyId, nativeId])
|
||||
})
|
||||
|
||||
it('keeps browser authorization on the original client when native audiences are enabled', async () => {
|
||||
const provider = google({
|
||||
clientId: googleClientIds('browser-client', [nativeId]),
|
||||
clientSecret: 'browser-secret',
|
||||
})
|
||||
const url = await provider.createAuthorizationURL({
|
||||
state: 'state',
|
||||
codeVerifier: 'test-code-verifier-with-at-least-forty-three-characters',
|
||||
redirectURI: 'https://example.com/api/auth/callback/google',
|
||||
})
|
||||
expect(url.searchParams.get('client_id')).toBe('browser-client')
|
||||
expect(url.searchParams.get('redirect_uri')).toBe('https://example.com/api/auth/callback/google')
|
||||
})
|
||||
|
||||
it('does not duplicate a browser client also used by native apps', () => {
|
||||
expect(googleClientIds(nativeId, [nativeId])).toEqual([nativeId])
|
||||
})
|
||||
|
||||
it('treats an empty optional list as the original browser configuration', () => {
|
||||
expect(googleClientIds('browser', parse(GoogleNativeClientIdsSchema, ' , '))).toBe('browser')
|
||||
})
|
||||
|
||||
it.each([
|
||||
'*',
|
||||
'https://example.com',
|
||||
'123-abc.apps.googleusercontent.com.evil.test',
|
||||
'123.apps.googleusercontent.com.evil.test',
|
||||
'123-.apps.googleusercontent.com',
|
||||
'*.apps.googleusercontent.com',
|
||||
'not-a-client',
|
||||
])('rejects invalid audience %s', (value) => {
|
||||
expect(() => parse(GoogleNativeClientIdsSchema, value)).toThrow()
|
||||
})
|
||||
})
|
||||
@@ -7,6 +7,8 @@ import { generateKeyPairSync } from 'node:crypto'
|
||||
import { decodeJwt } from 'jose'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { createAuth } from '../auth'
|
||||
import { parseAuthEnv } from '../env'
|
||||
import { createSocialAuthorizationRevoker } from '../social-authorization'
|
||||
|
||||
interface SocialAccount {
|
||||
@@ -233,20 +235,78 @@ describe('social authorization revocation', () => {
|
||||
expect(fetchRequest).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('aborts deletion when a social account has no revocable token', async () => {
|
||||
// ROOT CAUSE:
|
||||
// Native Google ID token sign-in stores no access or refresh token.
|
||||
// Requiring either token blocked deletion before resource cleanup.
|
||||
it('continues deletion for Google ID token accounts without a revocation request', async () => {
|
||||
const fetchRequest = vi.fn<typeof fetch>()
|
||||
const revoker = createSocialAuthorizationRevoker(
|
||||
createAccountDb([{ providerId: 'google', accessToken: null, refreshToken: null }]),
|
||||
createCredentials(),
|
||||
fetchRequest,
|
||||
)
|
||||
|
||||
await expect(revoker.revokeForUser('user-1')).resolves.toBeUndefined()
|
||||
expect(fetchRequest).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each(['apple', 'github'])('still aborts deletion when %s has no revocable token', async (providerId) => {
|
||||
const revoker = createSocialAuthorizationRevoker(
|
||||
createAccountDb([{ providerId, accessToken: null, refreshToken: null }]),
|
||||
createCredentials(),
|
||||
vi.fn<typeof fetch>(),
|
||||
)
|
||||
|
||||
await expect(revoker.revokeForUser('user-1')).rejects.toMatchObject({
|
||||
statusCode: 503,
|
||||
errorCode: 'oauth/revocation_token_missing',
|
||||
details: { providerId: 'google' },
|
||||
details: { providerId },
|
||||
})
|
||||
})
|
||||
|
||||
it.each([200, 503])('applies linked Google revocation policy before resource cleanup (status %s)', async (status) => {
|
||||
const db = createAccountDb([
|
||||
{ providerId: 'google', accessToken: null, refreshToken: null },
|
||||
{ providerId: 'google', accessToken: 'saved-access-token', refreshToken: null },
|
||||
])
|
||||
const credentials = createCredentials()
|
||||
const fetchRequest = vi.fn<typeof fetch>(async () => new Response(null, { status }))
|
||||
const softDeleteUserData = vi.fn(async () => {})
|
||||
const auth = createAuth(db, parseAuthEnv({
|
||||
...credentials,
|
||||
DATABASE_URL: 'postgres://localhost/test',
|
||||
REDIS_URL: 'redis://localhost:6379',
|
||||
PUBLIC_URL: 'http://localhost:3000',
|
||||
BETTER_AUTH_SECRET: 'test-secret-test-secret-test-secret',
|
||||
}), undefined, undefined, {
|
||||
softDeleteUserData,
|
||||
trackAuthEvent: vi.fn(async () => {}),
|
||||
}, createSocialAuthorizationRevoker(db, credentials, fetchRequest))
|
||||
const beforeDelete = auth.options.user?.deleteUser?.beforeDelete
|
||||
if (!beforeDelete)
|
||||
throw new TypeError('Expected account-deletion hook')
|
||||
|
||||
const deletion = beforeDelete({
|
||||
id: 'user-1',
|
||||
name: 'User One',
|
||||
email: 'user@example.com',
|
||||
emailVerified: true,
|
||||
image: null,
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
}, new Request('http://localhost:3000/api/auth/delete-user'))
|
||||
if (status === 200) {
|
||||
await deletion
|
||||
expect(softDeleteUserData).toHaveBeenCalledWith({ userId: 'user-1', reason: 'user-requested' })
|
||||
}
|
||||
else {
|
||||
await expect(deletion).rejects.toMatchObject({ statusCode: 502 })
|
||||
expect(softDeleteUserData).not.toHaveBeenCalled()
|
||||
}
|
||||
expect(fetchRequest).toHaveBeenCalledTimes(1)
|
||||
expect(new URLSearchParams(fetchRequest.mock.calls[0][1]?.body?.toString()).get('token')).toBe('saved-access-token')
|
||||
})
|
||||
|
||||
it('aborts deletion for an external provider without a revocation policy', async () => {
|
||||
const revoker = createSocialAuthorizationRevoker(
|
||||
createAccountDb([{ providerId: 'future-provider', accessToken: 'token', refreshToken: null }]),
|
||||
|
||||
Reference in New Issue
Block a user