diff --git a/server/apps/auth/README.md b/server/apps/auth/README.md index 5573cba3f..904c2b4c2 100644 --- a/server/apps/auth/README.md +++ b/server/apps/auth/README.md @@ -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 diff --git a/server/apps/auth/src/auth.ts b/server/apps/auth/src/auth.ts index 3c8506b99..2ff43f38b 100644 --- a/server/apps/auth/src/auth.ts +++ b/server/apps/auth/src/auth.ts @@ -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 diff --git a/server/apps/auth/src/env.ts b/server/apps/auth/src/env.ts index 8ace139c5..618c83918 100644 --- a/server/apps/auth/src/env.ts +++ b/server/apps/auth/src/env.ts @@ -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')), diff --git a/server/apps/auth/src/google-client-ids.ts b/server/apps/auth/src/google-client-ids.ts new file mode 100644 index 000000000..c819babb5 --- /dev/null +++ b/server/apps/auth/src/google-client-ids.ts @@ -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])] +} diff --git a/server/apps/auth/src/social-authorization.ts b/server/apps/auth/src/social-authorization.ts index 55d02812b..c75872e94 100644 --- a/server/apps/auth/src/social-authorization.ts +++ b/server/apps/auth/src/social-authorization.ts @@ -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 { + // 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) diff --git a/server/apps/auth/src/tests/google-client-ids.test.ts b/server/apps/auth/src/tests/google-client-ids.test.ts new file mode 100644 index 000000000..19ba1e147 --- /dev/null +++ b/server/apps/auth/src/tests/google-client-ids.test.ts @@ -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() + }) +}) diff --git a/server/apps/auth/src/tests/social-authorization.test.ts b/server/apps/auth/src/tests/social-authorization.test.ts index 989ba2d4e..7673ed122 100644 --- a/server/apps/auth/src/tests/social-authorization.test.ts +++ b/server/apps/auth/src/tests/social-authorization.test.ts @@ -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() 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(), ) 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(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 }]),