-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix(oauth): preflight Google integration config #4793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import type { NextRequest } from 'next/server' | ||
| import { NextResponse } from 'next/server' | ||
| import { getOAuthProviderConfigContract } from '@/lib/api/contracts/auth' | ||
| import { parseRequest } from '@/lib/api/server' | ||
| import { getSession } from '@/lib/auth' | ||
| import { withRouteHandler } from '@/lib/core/utils/with-route-handler' | ||
| import { getOAuthProviderConfigStatus } from '@/lib/oauth/provider-config' | ||
|
|
||
| export const dynamic = 'force-dynamic' | ||
|
|
||
| export const GET = withRouteHandler(async (request: NextRequest) => { | ||
| const session = await getSession() | ||
| if (!session?.user?.id) { | ||
| return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) | ||
| } | ||
|
|
||
| const parsed = await parseRequest(getOAuthProviderConfigContract, request, {}) | ||
| if (!parsed.success) return parsed.response | ||
|
|
||
| return NextResponse.json(getOAuthProviderConfigStatus(parsed.data.query.providerId)) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| /** | ||
| * @vitest-environment node | ||
| */ | ||
| import { afterEach, describe, expect, it } from 'vitest' | ||
| import { getOAuthProviderConfigStatus, getOAuthRedirectUri } from '@/lib/oauth/provider-config' | ||
|
|
||
| const ORIGINAL_ENV = { ...process.env } | ||
|
|
||
| afterEach(() => { | ||
| process.env = { ...ORIGINAL_ENV } | ||
| }) | ||
|
|
||
| describe('getOAuthProviderConfigStatus', () => { | ||
| it('reports missing Google OAuth env vars with the provider redirect URI', () => { | ||
| process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000' | ||
| process.env.GOOGLE_CLIENT_ID = '' | ||
| process.env.GOOGLE_CLIENT_SECRET = '' | ||
|
|
||
| const status = getOAuthProviderConfigStatus('google-sheets') | ||
|
|
||
| expect(status.available).toBe(false) | ||
| expect(status.status).toBe('missing_env') | ||
| expect(status.requiredEnv).toEqual(['GOOGLE_CLIENT_ID', 'GOOGLE_CLIENT_SECRET']) | ||
| expect(status.redirectUri).toBe('http://localhost:3000/api/auth/oauth2/callback/google-sheets') | ||
| expect(status.message).toContain('GOOGLE_CLIENT_ID') | ||
| expect(status.message).toContain('GOOGLE_CLIENT_SECRET') | ||
| }) | ||
|
|
||
| it('blocks placeholder Google OAuth credentials', () => { | ||
| process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000' | ||
| process.env.GOOGLE_CLIENT_ID = 'your-google-client-id' | ||
| process.env.GOOGLE_CLIENT_SECRET = 'your-google-client-secret' | ||
|
|
||
| const status = getOAuthProviderConfigStatus('google-drive') | ||
|
|
||
| expect(status.available).toBe(false) | ||
| expect(status.status).toBe('placeholder_env') | ||
| }) | ||
|
|
||
| it('rejects malformed Google OAuth client IDs before redirecting to Google', () => { | ||
| process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000' | ||
| process.env.GOOGLE_CLIENT_ID = 'not-a-google-client' | ||
| process.env.GOOGLE_CLIENT_SECRET = 'real-secret' | ||
|
|
||
| const status = getOAuthProviderConfigStatus('google-sheets') | ||
|
|
||
| expect(status.available).toBe(false) | ||
| expect(status.status).toBe('invalid_env') | ||
| expect(status.message).toContain('.apps.googleusercontent.com') | ||
| }) | ||
|
|
||
| it('accepts configured Google OAuth credentials', () => { | ||
| process.env.NEXT_PUBLIC_APP_URL = 'https://sim.example.com/' | ||
| process.env.GOOGLE_CLIENT_ID = '123.apps.googleusercontent.com' | ||
| process.env.GOOGLE_CLIENT_SECRET = 'real-secret' | ||
|
|
||
| const status = getOAuthProviderConfigStatus('google-sheets') | ||
|
|
||
| expect(status.available).toBe(true) | ||
| expect(status.status).toBe('ready') | ||
| expect(status.redirectUri).toBe( | ||
| 'https://sim.example.com/api/auth/oauth2/callback/google-sheets' | ||
| ) | ||
| }) | ||
|
|
||
| it('does not block non-Google providers', () => { | ||
| const status = getOAuthProviderConfigStatus('slack') | ||
|
|
||
| expect(status.available).toBe(true) | ||
| expect(status.requiredEnv).toEqual([]) | ||
| }) | ||
| }) | ||
|
|
||
| describe('getOAuthRedirectUri', () => { | ||
| it('normalizes a trailing slash on the base URL', () => { | ||
| expect(getOAuthRedirectUri('google-sheets', 'https://sim.example.com/')).toBe( | ||
| 'https://sim.example.com/api/auth/oauth2/callback/google-sheets' | ||
| ) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,97 @@ | ||||||
| import { getEnv } from '@/lib/core/config/env' | ||||||
| import { getBaseUrl } from '@/lib/core/utils/urls' | ||||||
| import type { OAuthProvider } from '@/lib/oauth/types' | ||||||
|
|
||||||
| export type OAuthProviderConfigStatus = { | ||||||
| providerId: OAuthProvider | string | ||||||
| available: boolean | ||||||
| status: 'ready' | 'missing_env' | 'placeholder_env' | 'invalid_env' | ||||||
| message: string | ||||||
| redirectUri?: string | ||||||
| requiredEnv: string[] | ||||||
| } | ||||||
|
|
||||||
| const GOOGLE_CLIENT_ID_SUFFIX = '.apps.googleusercontent.com' | ||||||
| const PLACEHOLDER_PATTERN = /^(|your-|change-me|changeme|example|<.*>)$/i | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||
|
|
||||||
| function hasPlaceholderValue(value: string | undefined): boolean { | ||||||
| if (!value) return false | ||||||
| const normalized = value.trim() | ||||||
| return PLACEHOLDER_PATTERN.test(normalized) || normalized.includes('your-google-client') | ||||||
| } | ||||||
|
|
||||||
| function isGoogleProvider(providerId: string): boolean { | ||||||
| return providerId === 'google' || providerId.startsWith('google-') || providerId === 'vertex-ai' | ||||||
| } | ||||||
|
|
||||||
| export function getOAuthRedirectUri(providerId: string, baseUrl = getBaseUrl()): string { | ||||||
| return `${baseUrl.replace(/\/$/, '')}/api/auth/oauth2/callback/${providerId}` | ||||||
| } | ||||||
|
|
||||||
| export function getOAuthProviderConfigStatus( | ||||||
| providerId: OAuthProvider | string | ||||||
| ): OAuthProviderConfigStatus { | ||||||
| const requiredEnv = isGoogleProvider(providerId) | ||||||
| ? ['GOOGLE_CLIENT_ID', 'GOOGLE_CLIENT_SECRET'] | ||||||
| : [] | ||||||
|
|
||||||
| if (!isGoogleProvider(providerId)) { | ||||||
| return { | ||||||
| providerId, | ||||||
| available: true, | ||||||
| status: 'ready', | ||||||
| message: 'OAuth provider configuration is ready.', | ||||||
| requiredEnv, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| const clientId = getEnv('GOOGLE_CLIENT_ID')?.trim() | ||||||
| const clientSecret = getEnv('GOOGLE_CLIENT_SECRET')?.trim() | ||||||
| const redirectUri = getOAuthRedirectUri(providerId) | ||||||
|
|
||||||
| if (!clientId || !clientSecret) { | ||||||
| const missing = [ | ||||||
| !clientId ? 'GOOGLE_CLIENT_ID' : null, | ||||||
| !clientSecret ? 'GOOGLE_CLIENT_SECRET' : null, | ||||||
| ].filter(Boolean) | ||||||
| return { | ||||||
| providerId, | ||||||
| available: false, | ||||||
| status: 'missing_env', | ||||||
| message: `Google OAuth is not configured. Set ${missing.join(' and ')} and add ${redirectUri} as an authorized redirect URI in Google Cloud.`, | ||||||
| redirectUri, | ||||||
| requiredEnv, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (hasPlaceholderValue(clientId) || hasPlaceholderValue(clientSecret)) { | ||||||
| return { | ||||||
| providerId, | ||||||
| available: false, | ||||||
| status: 'placeholder_env', | ||||||
| message: `Google OAuth still has placeholder credentials. Replace GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET, then add ${redirectUri} as an authorized redirect URI in Google Cloud.`, | ||||||
| redirectUri, | ||||||
| requiredEnv, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (!clientId.endsWith(GOOGLE_CLIENT_ID_SUFFIX)) { | ||||||
| return { | ||||||
| providerId, | ||||||
| available: false, | ||||||
| status: 'invalid_env', | ||||||
| message: `GOOGLE_CLIENT_ID does not look like a Google OAuth web client ID. It should end with ${GOOGLE_CLIENT_ID_SUFFIX}, and ${redirectUri} must be registered as an authorized redirect URI.`, | ||||||
| redirectUri, | ||||||
| requiredEnv, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| return { | ||||||
| providerId, | ||||||
| available: true, | ||||||
| status: 'ready', | ||||||
| message: 'Google OAuth provider configuration is ready.', | ||||||
| redirectUri, | ||||||
| requiredEnv, | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated Google provider predicate may diverge
Medium Severity
shouldPreflightOAuthProviderin the modal duplicates the exact logic ofisGoogleProviderinprovider-config.ts. The client-side gate decides whether to call the preflight API, while the server-side function decides what to validate — if a new Google-family provider is added to one but not the other, the preflight will silently be skipped or return a false positive. Extracting the predicate into a shared, dependency-free module (or exportingisGoogleProvider) would keep the two sides in sync.Additional Locations (1)
apps/sim/lib/oauth/provider-config.ts#L22-L25Reviewed by Cursor Bugbot for commit cd08394. Configure here.