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
97 changes: 97 additions & 0 deletions packages/worker/client/routes/connect-oauth.node.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { expect, test } from 'vitest'
import {
buildIntegrationValueName,
formatOAuthExchangeFailure,
getIntegrationValueCandidates,
isOAuthExchangeSessionExpired,
mergeConnectOauthConfig,
parseStoredIntegrationConfig,
summarizeStoredSetupState,
Expand Down Expand Up @@ -88,6 +90,7 @@ test('connect OAuth helpers parse stored integrations, merge reconnect configs,
apiBaseUrl: 'https://api.github.com',
scopes: ['repo', 'read:user'],
flow: 'confidential',
tokenExchangeStyle: 'form',
scopeSeparator: ' ',
extraAuthorizeParams: { prompt: 'consent' },
dashboardUrl: 'https://github.com/settings/developers',
Expand Down Expand Up @@ -227,6 +230,7 @@ test('connect OAuth helpers parse stored integrations, merge reconnect configs,
tokenHost: 'accounts.spotify.com',
tokenUrl: 'https://accounts.spotify.com/api/token',
flow: 'pkce',
tokenExchangeStyle: 'form',
clientIdValueName: 'spotify-client-id',
clientSecretSecretName: null,
accessTokenSecretName: 'spotifyAccessToken',
Expand All @@ -249,3 +253,96 @@ test('connect OAuth helpers parse stored integrations, merge reconnect configs,
expect(pkceSetup.isReady).toBe(true)
expect(pkceSetup.missingFields).toEqual([])
})

test('connect OAuth derives Notion basic-json exchange and surfaces provider failures instead of session expiry', () => {
const notionConfig = mergeConnectOauthConfig({
queryConfig: {
provider: 'notion',
providerKey: 'notion',
authorizeHost: 'api.notion.com',
authorizeUrl: 'https://api.notion.com/v1/oauth/authorize',
tokenUrl: 'https://api.notion.com/v1/oauth/token',
apiBaseUrl: 'https://api.notion.com/v1',
scopes: [],
flow: 'confidential',
scopeSeparator: ' ',
extraAuthorizeParams: { owner: 'user', response_type: 'code' },
providerSetupInstructions: null,
dashboardUrl: null,
allowedHosts: ['api.notion.com'],
},
storedIntegration: null,
})

expect(notionConfig).toMatchObject({
provider: 'notion',
tokenUrl: 'https://api.notion.com/v1/oauth/token',
flow: 'confidential',
tokenExchangeStyle: 'basic-json',
clientSecretSecretName: 'notionClientSecret',
accessTokenSecretName: 'notionAccessToken',
})

const storedNotion = parseStoredIntegrationConfig(
JSON.stringify({
name: 'notion',
tokenUrl: 'https://api.notion.com/v1/oauth/token',
apiBaseUrl: 'https://api.notion.com/v1',
flow: 'confidential',
clientIdValueName: 'notion-client-id',
clientSecretSecretName: 'notionClientSecret',
accessTokenSecretName: 'notionAccessToken',
refreshTokenSecretName: 'notionRefreshToken',
requiredHosts: ['api.notion.com'],
tokenExchangeStyle: 'basic-json',
authorization: {
authorizeUrl: 'https://api.notion.com/v1/oauth/authorize',
scopes: [],
scopeSeparator: null,
extraAuthorizeParams: { owner: 'user' },
},
}),
null,
)
expect(storedNotion?.tokenExchangeStyle).toBe('basic-json')

expect(
formatOAuthExchangeFailure({
status: 401,
data: { ok: false, error: 'Unauthorized.' },
}),
).toEqual({
treatAsSessionExpired: true,
error: 'Session expired.',
})
expect(
isOAuthExchangeSessionExpired({
status: 401,
data: { ok: false, error: 'Unauthorized.' },
}),
).toBe(true)

expect(
formatOAuthExchangeFailure({
status: 502,
data: {
ok: false,
error: 'invalid_client',
error_description: 'Client authentication failed',
providerStatus: 401,
},
}),
).toEqual({
treatAsSessionExpired: false,
error: 'Client authentication failed',
})
expect(
isOAuthExchangeSessionExpired({
status: 401,
data: {
error: 'invalid_client',
error_description: 'Client authentication failed',
},
}),
).toBe(false)
})
95 changes: 85 additions & 10 deletions packages/worker/client/routes/connect-oauth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
} from '#client/styles/style-primitives.ts'

type OAuthFlow = 'pkce' | 'confidential'
type TokenExchangeStyle = 'form' | 'basic-json'

type ConnectOauthQueryConfig = {
provider: string
Expand Down Expand Up @@ -65,6 +66,7 @@ type ConnectOauthConfig = {
apiBaseUrl: string | null
scopes: Array<string>
flow: OAuthFlow
tokenExchangeStyle: TokenExchangeStyle
scopeSeparator: string
extraAuthorizeParams: Record<string, string>
providerSetupInstructions: string | null
Expand All @@ -86,6 +88,7 @@ type StoredIntegrationConfig = {
accessTokenSecretName: string
refreshTokenSecretName: string | null
requiredHosts: Array<string>
tokenExchangeStyle?: TokenExchangeStyle | null
authorization?: StoredIntegrationAuthorization | null
}

Expand Down Expand Up @@ -601,31 +604,31 @@ export function ConnectOauthRoute(handle: Handle) {
tokenUrl: nextConfig.tokenUrl,
params: params.toString(),
flow: nextConfig.flow,
tokenExchangeStyle: nextConfig.tokenExchangeStyle,
clientSecretSecretName: nextConfig.clientSecretSecretName,
allowedHosts: nextConfig.allowedHosts,
}),
})
if (redirectToLoginOn401(response)) {
return { ok: false, status: 401, error: 'Session expired.' }
}
const text = await response.text()
let data: Record<string, unknown> | null = null
try {
data = JSON.parse(text)
} catch {
data = null
}
const failure = formatOAuthExchangeFailure({
status: response.status,
data,
})
if (failure.treatAsSessionExpired) {
window.location.assign('/login')
return { ok: false, status: 401, error: 'Session expired.' }
}
if (!response.ok || !data) {
const errorDescription =
typeof data?.error_description === 'string'
? data.error_description
: typeof data?.error === 'string'
? data.error
: null
return {
ok: false,
status: response.status,
error: errorDescription ?? 'Token exchange failed.',
error: failure.error,
}
}
return { ok: true, data, status: response.status }
Expand Down Expand Up @@ -753,6 +756,7 @@ export function ConnectOauthRoute(handle: Handle) {
scopeSeparator: config.scopeSeparator,
extraAuthorizeParams: config.extraAuthorizeParams,
flow: config.flow,
tokenExchangeStyle: config.tokenExchangeStyle,
clientIdValueName: config.clientIdValueName,
clientSecretSecretName: config.clientSecretSecretName,
allowedHosts: config.allowedHosts,
Expand Down Expand Up @@ -1304,6 +1308,9 @@ export function parseStoredIntegrationConfig(
parsed.clientSecretSecretName.trim()
? parsed.clientSecretSecretName.trim()
: null
const tokenExchangeStyle = parseTokenExchangeStyle(
parsed.tokenExchangeStyle,
)
const requiredHosts = Array.isArray(parsed.requiredHosts)
? parsed.requiredHosts.filter(
(value): value is string => typeof value === 'string',
Expand All @@ -1328,6 +1335,7 @@ export function parseStoredIntegrationConfig(
accessTokenSecretName,
refreshTokenSecretName,
requiredHosts: normalizeHosts(requiredHosts),
...(tokenExchangeStyle ? { tokenExchangeStyle } : {}),
authorization,
}
} catch {
Expand Down Expand Up @@ -1426,6 +1434,10 @@ export function mergeConnectOauthConfig(input: {
input.storedIntegration?.apiBaseUrl ?? input.queryConfig.apiBaseUrl,
scopes,
flow,
tokenExchangeStyle: resolveConnectOauthTokenExchangeStyle({
tokenUrl,
storedStyle: input.storedIntegration?.tokenExchangeStyle ?? null,
}),
scopeSeparator:
input.queryConfig.scopeSeparator ??
input.storedIntegration?.authorization?.scopeSeparator ??
Expand Down Expand Up @@ -1487,6 +1499,69 @@ export function summarizeStoredSetupState(input: {
}
}

/**
* Distinguish real Kody session expiry (401 Unauthorized) from provider token
* exchange failures that historically leaked through as HTTP 401.
*/
export function formatOAuthExchangeFailure(input: {
status: number
data: Record<string, unknown> | null
}): { treatAsSessionExpired: boolean; error: string } {
if (isOAuthExchangeSessionExpired(input)) {
return { treatAsSessionExpired: true, error: 'Session expired.' }
}
const errorDescription =
typeof input.data?.error_description === 'string' &&
input.data.error_description.trim()
? input.data.error_description.trim()
: typeof input.data?.error === 'string' && input.data.error.trim()
? input.data.error.trim()
: null
return {
treatAsSessionExpired: false,
error: errorDescription ?? 'Token exchange failed.',
}
}

export function isOAuthExchangeSessionExpired(input: {
status: number
data: Record<string, unknown> | null
}) {
if (input.status !== 401) return false
if (hasProviderOAuthExchangeError(input.data)) return false
return true
}

function hasProviderOAuthExchangeError(data: Record<string, unknown> | null) {
if (!data) return false
if (typeof data.providerStatus === 'number') return true
if (
typeof data.error_description === 'string' &&
data.error_description.trim()
) {
return true
}
return (
typeof data.error === 'string' &&
data.error.trim() !== '' &&
data.error !== 'Unauthorized.'
)
}

function resolveConnectOauthTokenExchangeStyle(input: {
tokenUrl: string
storedStyle: TokenExchangeStyle | null
}): TokenExchangeStyle {
if (input.storedStyle) return input.storedStyle
const host = safeParseHost(input.tokenUrl)
if (host === 'api.notion.com') return 'basic-json'
return 'form'
}

function parseTokenExchangeStyle(raw: unknown): TokenExchangeStyle | null {
return raw === 'form' || raw === 'basic-json' ? raw : null
}

function formatMissingSetupFields(missingFields: Array<string>) {
if (missingFields.length === 0) return 'Ready to connect.'
if (missingFields.length === 1) {
Expand Down
Loading
Loading