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
58 changes: 53 additions & 5 deletions packages/worker/client/routes/connect-oauth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ export async function connectOauthRouteLoader(
if (!providerKey) {
return { connectOauth: emptyConnectOauthLoaderData }
}
const preferPlatform = params.get('platform') === '1'
const response = await fetch(
`/account/integrations.json?name=${encodeURIComponent(providerKey)}`,
`/account/integrations.json?name=${encodeURIComponent(providerKey)}${preferPlatform ? '&platform=1' : ''}`,
{
headers: { Accept: 'application/json' },
credentials: 'include',
Expand All @@ -139,6 +140,10 @@ export async function connectOauthRouteLoader(
provider: providerKey,
integration:
response.ok && payload?.ok ? (payload.integration ?? null) : null,
builtInAvailable:
response.ok && payload?.ok
? (payload.builtInAvailable ?? false)
: false,
},
}
}
Expand Down Expand Up @@ -173,6 +178,8 @@ export function ConnectOauthRoute(handle: Handle) {
let hasConfigError = false
let connectOauthHandled = false
let hostApprovalLinks: Array<ConnectOauthHostApprovalLink> = []
/** An enabled built-in exists that this user-lane connection is not using. */
let builtInAvailable = false
let nextSteps: ConnectOauthNextSteps | null = null
let approvingAllHosts = false
let submitting = false
Expand Down Expand Up @@ -454,8 +461,11 @@ export function ConnectOauthRoute(handle: Handle) {
const readExistingIntegrationConfig = async (
queryConfig: ConnectOauthQueryConfig,
): Promise<StoredIntegrationConfig | null> => {
const preferPlatform =
typeof window !== 'undefined' &&
new URLSearchParams(window.location.search).get('platform') === '1'
const response = await fetch(
`/account/integrations.json?name=${encodeURIComponent(queryConfig.providerKey)}`,
`/account/integrations.json?name=${encodeURIComponent(queryConfig.providerKey)}${preferPlatform ? '&platform=1' : ''}`,
{
method: 'GET',
headers: { Accept: 'application/json' },
Expand All @@ -466,9 +476,9 @@ export function ConnectOauthRoute(handle: Handle) {
const payload = (await response
.json()
.catch(() => null)) as AccountIntegrationDetailLoaderData | null
if (!response.ok || payload?.ok !== true || !payload.integration) {
return null
}
if (!response.ok || payload?.ok !== true) return null
builtInAvailable = payload.builtInAvailable ?? false
if (!payload.integration) return null
return toStoredIntegrationConfig(payload.integration)
}

Expand Down Expand Up @@ -718,6 +728,39 @@ export function ConnectOauthRoute(handle: Handle) {
}
}

/**
* Shown when the current (or prospective) connection runs on the user's
* own OAuth app while an enabled built-in exists for the same name. A
* complete bring-your-own record wins the lookup by design, so without
* this the built-in lane is invisible from the connect page.
*/
const renderBuiltInAlternative = () => {
if (!builtInAvailable || !config || config.platformAppSlug) return null
return (
<p mix={css(descriptionCss)}>
This uses your own OAuth app for {config.provider}. Prefer the hosted
one?{' '}
<a
href={`/connect/oauth?provider=${encodeURIComponent(config.providerKey)}&platform=1`}
mix={[
css(primaryLinkCss),
// Full document load on purpose: this page's init
// (config resolution, built-in auto-start) runs per
// document load, and a same-route SPA swap reuses the
// mounted instance without re-running it.
on('click', (event) => {
event.preventDefault()
window.location.assign(event.currentTarget.href)
}),
]}
>
Use the built-in {config.provider} integration instead
</a>{' '}
— connecting it replaces this connection&apos;s tokens and scopes.
</p>
)
}

const handleConnect = async () => {
if (!config || submitting) return
submitting = true
Expand Down Expand Up @@ -1007,6 +1050,9 @@ export function ConnectOauthRoute(handle: Handle) {
'connectOauth',
readCurrentRouterHref(handle),
)
if (routeData) {
builtInAvailable = routeData.builtInAvailable ?? false
}
const preloadedIntegrationFor = (providerKey: string) =>
routeData && routeData.provider === providerKey
? routeData.integration
Expand Down Expand Up @@ -1187,6 +1233,7 @@ export function ConnectOauthRoute(handle: Handle) {
1. {existingIntegrationConfig ? 'Review' : 'Save'} OAuth client
configuration
</h2>
{renderBuiltInAlternative()}
{renderProviderInstructions()}
{renderAllowedHosts()}
<form
Expand Down Expand Up @@ -1293,6 +1340,7 @@ export function ConnectOauthRoute(handle: Handle) {
<p mix={css({ margin: 0, color: colors.text })}>
Start the OAuth flow. You will be redirected to the provider.
</p>
{renderBuiltInAlternative()}
{existingIntegrationConfig && hasStoredClientId ? (
<p mix={css(descriptionCss)}>
Using stored client ID
Expand Down
15 changes: 15 additions & 0 deletions packages/worker/src/app/account-integrations-data.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '#worker/integrations/service.ts'
import { upsertPlatformOauthApp } from '#worker/integrations/platform-apps.ts'
import {
hasAlternativeBuiltInApp,
loadAccountIntegrationByName,
loadAccountIntegrationsData,
loadAccountOauthAppBySlug,
Expand Down Expand Up @@ -328,6 +329,20 @@ test('endpoint-incomplete user records defer to an enabled built-in of the same
)
expect(byoWins?.clientId).toBe('user-github-client')
expect(byoWins?.platform ?? false).toBe(false)
// The winning BYO record shadows an enabled built-in — the connect page
// surfaces the alternative lane instead of hiding it.
expect(await hasAlternativeBuiltInApp(env, 'github', byoWins)).toBe(true)

// Explicit built-in intent overrides the BYO win.
const forced = await loadAccountIntegrationByName(
env,
fakeUser(userId),
'github',
{ preferPlatform: true },
)
expect(forced?.platform).toBe(true)
expect(forced?.clientId).toBe('platform-github-client')
expect(await hasAlternativeBuiltInApp(env, 'github', forced)).toBe(false)

// No built-in for the slug: the incomplete record still returns so the
// setup form can prefill what exists.
Expand Down
28 changes: 28 additions & 0 deletions packages/worker/src/app/account-integrations-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,15 @@ export async function loadAccountIntegrationByName(
env: Env,
user: AuthenticatedUser,
name: string,
options?: {
/**
* Explicit built-in intent (`platform=1` on /connect/oauth): resolve
* the enabled platform app directly instead of letting a complete
* bring-your-own record win. Falls back to the normal priority when
* no built-in exists for the name.
*/
preferPlatform?: boolean
},
): Promise<AccountIntegrationRecord | null> {
// A user-lane record still wins when it can actually drive the flow (the
// bring-your-own override); an endpoint-incomplete one defers to an
Expand All @@ -239,6 +248,11 @@ export async function loadAccountIntegrationByName(
return platformApp ? toPlatformAppPrefillRecord(platformApp, name) : null
}

if (options?.preferPlatform) {
const platformRecord = await platformFallback()
if (platformRecord) return platformRecord
}

// 1. Existing connection (reconnect) — connection name, not app slug.
const joined = await getJoinedIntegration({
env,
Expand Down Expand Up @@ -268,3 +282,17 @@ export async function loadAccountIntegrationByName(
// the connect flow can skip client-credential setup entirely.
return platformFallback()
}

/**
* True when an enabled built-in exists for `name` that the resolved record
* is not already using — the connect page offers it as an alternative lane.
*/
export async function hasAlternativeBuiltInApp(
env: Env,
name: string,
record: AccountIntegrationRecord | null,
): Promise<boolean> {
if (record?.platform) return false
const platformApp = await getAvailablePlatformApp({ env, slug: name })
return platformApp != null
}
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ test('integrations API returns one connection by name for the connect OAuth flow
})
await expect(response.json()).resolves.toEqual({
ok: true,
builtInAvailable: false,
integration: {
name: 'github',
appSlug: 'github',
Expand Down Expand Up @@ -528,6 +529,7 @@ test('integrations API returns null when a named connection is missing', async (
expect(response.status).toBe(200)
await expect(response.json()).resolves.toEqual({
ok: true,
builtInAvailable: false,
integration: null,
})
})
Expand Down
15 changes: 14 additions & 1 deletion packages/worker/src/app/handlers/account-integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { jsonResponse } from '#worker/json-response.ts'
import { type Action } from 'remix/router'
import { safeParseHost } from '@kody-internal/shared/url-hosts.ts'
import {
hasAlternativeBuiltInApp,
loadAccountIntegrationByName,
loadAccountIntegrationsData,
loadAccountOauthAppBySlug,
Expand Down Expand Up @@ -75,9 +76,21 @@ export function createAccountIntegrationsApiHandler(env: Env) {
const searchParams = new URL(request.url).searchParams
const name = searchParams.get('name')?.trim()
if (name) {
const preferPlatform = searchParams.get('platform') === '1'
const integration = await loadAccountIntegrationByName(
env,
user,
name,
{ preferPlatform },
)
return jsonResponse({
ok: true,
integration: await loadAccountIntegrationByName(env, user, name),
integration,
builtInAvailable: await hasAlternativeBuiltInApp(
env,
name,
integration,
),
})
}
const appSlug = searchParams.get('appSlug')?.trim()
Expand Down
41 changes: 40 additions & 1 deletion packages/worker/src/app/handlers/connect-oauth.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const mockModule = vi.hoisted(() => ({
readAuthenticatedAppUser: vi.fn<() => Promise<unknown>>(),
requirePageSession: vi.fn<() => Promise<Response | null>>(),
loadAccountIntegrationByName: vi.fn<() => Promise<unknown>>(),
hasAlternativeBuiltInApp: vi.fn<() => Promise<boolean>>(),
renderAppPage: vi.fn<(input: unknown) => Promise<Response>>(),
}))

Expand All @@ -25,6 +26,8 @@ vi.mock('#app/page-auth.ts', () => ({
vi.mock('#app/account-integrations-data.ts', () => ({
loadAccountIntegrationByName: (...args: Array<unknown>) =>
mockModule.loadAccountIntegrationByName(...args),
hasAlternativeBuiltInApp: (...args: Array<unknown>) =>
mockModule.hasAlternativeBuiltInApp(...args),
}))

vi.mock('#app/ssr-render.tsx', () => ({
Expand Down Expand Up @@ -82,6 +85,8 @@ test('provider visits embed the integration record as SSR loader data', async ()
mockModule.loadAccountIntegrationByName.mockResolvedValue(record)
mockModule.renderAppPage.mockResolvedValue(new Response('ok'))

mockModule.hasAlternativeBuiltInApp.mockResolvedValue(false)

await createConnectOauthHandler(env).handler(
new RequestContext(
new Request('https://example.com/connect/oauth?provider=GitHub'),
Expand All @@ -92,16 +97,50 @@ test('provider visits embed the integration record as SSR loader data', async ()
env,
expect.anything(),
'github',
{ preferPlatform: false },
)
expect(mockModule.renderAppPage).toHaveBeenCalledWith(
expect.objectContaining({
loaderData: {
connectOauth: { ok: true, provider: 'github', integration: record },
connectOauth: {
ok: true,
provider: 'github',
integration: record,
builtInAvailable: false,
},
},
}),
)
})

test('platform=1 forces the built-in lookup', async () => {
const env = {} as Env
mockModule.requirePageSession.mockResolvedValue(null)
mockModule.readAuthenticatedAppUser.mockResolvedValue({
mcpUser: { userId: 'user-1' },
})
mockModule.loadAccountIntegrationByName.mockResolvedValue({
name: 'google',
platform: true,
})
mockModule.hasAlternativeBuiltInApp.mockResolvedValue(false)
mockModule.renderAppPage.mockResolvedValue(new Response('ok'))

await createConnectOauthHandler(env).handler(
new RequestContext(
new Request(
'https://example.com/connect/oauth?provider=google&platform=1',
),
),
)
expect(mockModule.loadAccountIntegrationByName).toHaveBeenLastCalledWith(
env,
expect.anything(),
'google',
{ preferPlatform: true },
)
})

test('callback returns render without loader data', async () => {
const env = {} as Env
mockModule.requirePageSession.mockResolvedValue(null)
Expand Down
19 changes: 17 additions & 2 deletions packages/worker/src/app/handlers/connect-oauth.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { type Action } from 'remix/router'
import { normalizeProviderKey } from '@kody-internal/shared/url-hosts.ts'
import { loadAccountIntegrationByName } from '#app/account-integrations-data.ts'
import {
hasAlternativeBuiltInApp,
loadAccountIntegrationByName,
} from '#app/account-integrations-data.ts'
import { readAuthenticatedAppUser } from '#app/authenticated-user.ts'
import { requirePageSession } from '#app/page-auth.ts'
import { renderAppPage } from '#app/ssr-render.tsx'
Expand Down Expand Up @@ -39,10 +42,22 @@ async function loadConnectOauthLoaderData(
if (!providerKey) return null
const user = await readAuthenticatedAppUser(request, env)
if (!user) return null
const preferPlatform = requestUrl.searchParams.get('platform') === '1'
const integration = await loadAccountIntegrationByName(
env,
user,
providerKey,
{ preferPlatform },
)
return {
ok: true,
provider: providerKey,
integration: await loadAccountIntegrationByName(env, user, providerKey),
integration,
builtInAvailable: await hasAlternativeBuiltInApp(
env,
providerKey,
integration,
),
}
}

Expand Down
30 changes: 30 additions & 0 deletions packages/worker/src/mcp/fetch-gateway.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,36 @@ test('fetch gateway resolves path-only URLs against baseUrl', async () => {
expect(nested.url).toBe('https://example.com/core/log')
})

test('outbound requests get a default User-Agent; caller values win', async () => {
// GitHub rejects UA-less requests with an opaque 403, and workerd sends
// no UA by default.
const bare = await expandSecretPlaceholders({
request: new Request('https://api.github.com/user'),
props,
env,
})
expect(bare.headers.get('user-agent')).toBe('kody-agent/1.0')

const custom = await expandSecretPlaceholders({
request: new Request('https://api.github.com/user', {
headers: { 'User-Agent': 'my-package/2.0' },
}),
props,
env,
})
expect(custom.headers.get('user-agent')).toBe('my-package/2.0')

// The resolution-off fast path applies the same default.
const modeOff = await expandSecretPlaceholders({
request: new Request('https://api.github.com/user', {
headers: { 'x-kody-secret-resolution': 'off' },
}),
props,
env,
})
expect(modeOff.headers.get('user-agent')).toBe('kody-agent/1.0')
})

test('gateway fetch records outbound_fetch usage metering', async () => {
const usageModule = await import('#worker/usage/record-usage.ts')
const recordUsageSpy = vi
Expand Down
Loading
Loading