Skip to content

Commit d9b50f1

Browse files
icecrasher321claude
andcommitted
fix(desktop): compose the connect completion URL through the URL API
Concatenating `getBaseUrl()` with the completion path leaves the result dependent on how the deployment spelled `NEXT_PUBLIC_APP_URL`: the helper only adds a missing protocol, so a trailing slash produced `//desktop/connect/complete`, a pathname that matches no route. The completion page is what bounces the OAuth result to the desktop app's loopback, so that typo would have stranded the flow just past the callback it was meant to fix. Both callback URLs in the page — the launcher's and the workspace-scoped authorize redirect's — now go through one helper that resolves the path against the base with `new URL`, matching how the same function already builds the authorize URL, with coverage for a trailing-slash base. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent fbd094d commit d9b50f1

2 files changed

Lines changed: 44 additions & 7 deletions

File tree

apps/sim/app/desktop/connect/page.test.tsx

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockGetSession, mockRedirect } = vi.hoisted(() => ({
6+
const { mockGetSession, mockRedirect, baseUrl } = vi.hoisted(() => ({
77
mockGetSession: vi.fn(),
88
mockRedirect: vi.fn((url: string) => {
99
throw new Error(`NEXT_REDIRECT:${url}`)
1010
}),
11+
/** Mutable so a test can give the deployment a trailing-slash base URL. */
12+
baseUrl: { value: 'https://sim.test' },
1113
}))
1214

1315
vi.mock('@/lib/auth', () => ({
@@ -21,7 +23,7 @@ vi.mock('@/lib/auth/auth-client', () => ({
2123
}))
2224

2325
vi.mock('@/lib/core/utils/urls', () => ({
24-
getBaseUrl: () => 'https://sim.test',
26+
getBaseUrl: () => baseUrl.value,
2527
}))
2628

2729
/** Keeps the landing-page barrel the real shell pulls in out of this graph. */
@@ -57,6 +59,7 @@ async function renderPage(params: Record<string, string>) {
5759
describe('DesktopConnectPage', () => {
5860
beforeEach(() => {
5961
vi.clearAllMocks()
62+
baseUrl.value = 'https://sim.test'
6063
mockGetSession.mockResolvedValue({ user: { id: 'user-1', email: 'user@example.com' } })
6164
})
6265

@@ -93,6 +96,32 @@ describe('DesktopConnectPage', () => {
9396
expect(() => new URL(result.props.completeUrl as string)).not.toThrow()
9497
})
9598

99+
it('keeps the completion route intact when the deployment base URL has a trailing slash', async () => {
100+
// `//desktop/connect/complete` matches no route, so the provider result
101+
// would never reach the loopback and the connect would hang.
102+
baseUrl.value = 'https://sim.test/'
103+
104+
const launcher = await renderPage({
105+
provider: 'google-email',
106+
state: VALID_STATE,
107+
port: PORT,
108+
})
109+
expect(new URL(launcher.props.completeUrl as string).pathname).toBe('/desktop/connect/complete')
110+
111+
await expect(
112+
DesktopConnectPage(
113+
pageProps({
114+
provider: 'google-email',
115+
state: VALID_STATE,
116+
port: PORT,
117+
workspaceId: 'workspace-1',
118+
})
119+
)
120+
).rejects.toThrow('NEXT_REDIRECT:')
121+
const callbackUrl = new URL(mockRedirect.mock.calls[0][0]).searchParams.get('callbackURL')
122+
expect(new URL(callbackUrl as string).pathname).toBe('/desktop/connect/complete')
123+
})
124+
96125
it('sends a workspace-scoped connect to the authorize route with an absolute callback', async () => {
97126
await expect(
98127
DesktopConnectPage(

apps/sim/app/desktop/connect/page.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ function InvalidRequest() {
3434
)
3535
}
3636

37+
/**
38+
* Absolute URL better-auth returns the browser to once the OAuth callback is
39+
* done. Composed through the URL API rather than concatenated, so a trailing
40+
* slash on `NEXT_PUBLIC_APP_URL` cannot yield a `//desktop/...` pathname that
41+
* matches no route — this page is what bounces the result to the app's
42+
* loopback, so a base-URL typo would otherwise strand the whole flow.
43+
*/
44+
function buildConnectCompleteUrl(state: string, port: number, draftId?: string): string {
45+
return new URL(buildConnectCompletePath(state, port, draftId), getBaseUrl()).toString()
46+
}
47+
3748
/**
3849
* Desktop OAuth-connect landing. The desktop app opens this page in the
3950
* system browser with the provider to connect, a one-time state, and the port
@@ -112,10 +123,7 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
112123
const authorize = new URL('/api/auth/oauth2/authorize', getBaseUrl())
113124
authorize.searchParams.set('providerId', providerId)
114125
authorize.searchParams.set('workspaceId', workspaceId)
115-
authorize.searchParams.set(
116-
'callbackURL',
117-
`${getBaseUrl()}${buildConnectCompletePath(state, port)}`
118-
)
126+
authorize.searchParams.set('callbackURL', buildConnectCompleteUrl(state, port))
119127
if (credentialId) {
120128
authorize.searchParams.set('credentialId', credentialId)
121129
}
@@ -125,7 +133,7 @@ export default async function DesktopConnectPage({ searchParams }: DesktopConnec
125133
return (
126134
<ConnectLauncher
127135
providerId={providerId}
128-
completeUrl={`${getBaseUrl()}${buildConnectCompletePath(state, port, draftId)}`}
136+
completeUrl={buildConnectCompleteUrl(state, port, draftId)}
129137
/>
130138
)
131139
}

0 commit comments

Comments
 (0)