Skip to content

Commit b427a4b

Browse files
ksarangmathclaude
andauthored
fix(auth): onboard users without an organization before CLI approval (#468)
The device approval page was the only authenticated page a new user could reach without meeting the organization gate, so `omnara login` handed out a token to accounts with no organization and every later command dead-ended. Route the organization gate through a helper that preserves the destination, apply it to /device as well as the app shell, and let /onboarding return to that destination after creating an organization or accepting an invitation. The CLI now names the onboarding page when an account still has no organization instead of stopping at "this account has no organizations". Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent b0bd819 commit b427a4b

11 files changed

Lines changed: 200 additions & 28 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import type { CurrentUser } from '@omnara/sdk'
2+
import { isRedirect } from '@tanstack/react-router'
3+
import { describe, expect, it } from 'vitest'
4+
5+
import { requireOrganization } from '@/lib/require-organization'
6+
7+
function user(orgCount: number): CurrentUser {
8+
return {
9+
orgs: Array.from({ length: orgCount }, (_, index) => ({
10+
id: `org_${index}`,
11+
name: `Org ${index}`,
12+
})),
13+
} as unknown as CurrentUser
14+
}
15+
16+
function redirectHref(run: () => void): string {
17+
try {
18+
run()
19+
} catch (error) {
20+
if (isRedirect(error)) return (error as { options: { href: string } }).options.href
21+
throw error
22+
}
23+
throw new Error('expected a redirect')
24+
}
25+
26+
describe('requireOrganization', () => {
27+
it('lets organization members through', () => {
28+
expect(() => {
29+
requireOrganization(user(1), '/device?user_code=ABCDE-F1234')
30+
}).not.toThrow()
31+
})
32+
33+
it('sends users without an organization to onboarding and preserves their destination', () => {
34+
expect(
35+
redirectHref(() => {
36+
requireOrganization(user(0), '/device?user_code=ABCDE-F1234')
37+
}),
38+
).toBe('/onboarding?return_to=%2Fdevice%3Fuser_code%3DABCDE-F1234')
39+
})
40+
})
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import type { CurrentUser } from '@omnara/sdk'
2+
import { redirect } from '@tanstack/react-router'
3+
4+
export function requireOrganization(me: CurrentUser, returnTo: string): void {
5+
if (me.orgs.length > 0) return
6+
const search = returnTo === '/' ? '' : `?return_to=${encodeURIComponent(returnTo)}`
7+
// eslint-disable-next-line @typescript-eslint/only-throw-error -- TanStack Router throws redirects.
8+
throw redirect({ href: `/onboarding${search}` })
9+
}

‎frontend/apps/web/src/router.tsx‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import {
1212
import { Suspense } from 'react'
1313

1414
import { FullPageSpinner } from '@/components/ui/spinner'
15+
import { safeReturnTo } from '@/lib/auth-return-to'
16+
import { requireOrganization } from '@/lib/require-organization'
1517
import { RootError } from '@/routes/RootError'
1618

1719
export interface RouterContext {
@@ -57,12 +59,8 @@ const authenticatedRoute = createRoute({
5759
const onboardedRoute = createRoute({
5860
getParentRoute: () => authenticatedRoute,
5961
id: 'onboarded',
60-
beforeLoad: ({ context }) => {
61-
const { me } = context
62-
if (me.orgs.length === 0) {
63-
// eslint-disable-next-line @typescript-eslint/only-throw-error -- TanStack Router throws redirects.
64-
throw redirect({ href: '/onboarding' })
65-
}
62+
beforeLoad: ({ context, location }) => {
63+
requireOrganization(context.me, location.href)
6664
},
6765
component: lazyRouteComponent(() => import('@/routes/AuthedLayout'), 'AuthedLayout'),
6866
})
@@ -172,17 +170,20 @@ const agentRoute = createRoute({
172170
const deviceAuthRoute = createRoute({
173171
getParentRoute: () => authenticatedRoute,
174172
path: '/device',
173+
beforeLoad: ({ context, location }) => {
174+
requireOrganization(context.me, location.href)
175+
},
175176
component: lazyRouteComponent(() => import('@/routes/DeviceAuth'), 'DeviceAuth'),
176177
})
177178

178179
const onboardingRoute = createRoute({
179180
getParentRoute: () => authenticatedRoute,
180181
path: '/onboarding',
181-
beforeLoad: ({ context }) => {
182-
const { me } = context
183-
if (me.orgs.length > 0) {
182+
beforeLoad: ({ context, location }) => {
183+
if (context.me.orgs.length > 0) {
184+
const returnTo = new URL(location.href, window.location.origin).searchParams.get('return_to')
184185
// eslint-disable-next-line @typescript-eslint/only-throw-error -- TanStack Router throws redirects.
185-
throw redirect({ href: '/' })
186+
throw redirect({ href: safeReturnTo(returnTo) })
186187
}
187188
},
188189
component: lazyRouteComponent(() => import('@/routes/Onboarding'), 'Onboarding'),

‎frontend/apps/web/src/routes/Onboarding.test.tsx‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,15 @@ const hooks = vi.hoisted(() => ({
1111
createOrganization: vi.fn(),
1212
declineInvitation: vi.fn(),
1313
refetchInvitations: vi.fn(),
14+
pendingInvitations: [] as unknown[],
1415
}))
1516

1617
vi.mock('@omnara/react', () => ({
1718
useAcceptInvitation: () => ({ mutateAsync: hooks.acceptInvitation }),
1819
useCreateOrganization: () => ({ mutateAsync: hooks.createOrganization }),
1920
useDeclineInvitation: () => ({ mutateAsync: hooks.declineInvitation }),
2021
usePendingInvitations: () => ({
21-
data: { data: [] },
22+
data: { data: hooks.pendingInvitations },
2223
refetch: hooks.refetchInvitations,
2324
}),
2425
}))
@@ -79,6 +80,7 @@ beforeEach(() => {
7980
hooks.createOrganization.mockReset()
8081
hooks.declineInvitation.mockReset()
8182
hooks.refetchInvitations.mockReset()
83+
hooks.pendingInvitations.length = 0
8284
hooks.createOrganization.mockImplementation(() => new Promise(() => undefined))
8385
window.history.replaceState(null, '', '/onboarding')
8486

@@ -106,6 +108,76 @@ describe('Onboarding organization creation', () => {
106108
expect(window.location.pathname).toBe('/')
107109
})
108110

111+
it('returns to the device approval page it was sent from after creating the organization', async () => {
112+
window.history.replaceState(
113+
null,
114+
'',
115+
'/onboarding?return_to=%2Fdevice%3Fuser_code%3DABCDE-F1234',
116+
)
117+
hooks.createOrganization.mockResolvedValueOnce(undefined)
118+
renderOnboarding()
119+
120+
expect(container.textContent).toContain(
121+
'Create your organization, then we will bring you back to approve the CLI login.',
122+
)
123+
124+
setOrganizationName('Acme Inc.')
125+
await submitOrganization()
126+
127+
expect(window.location.pathname).toBe('/device')
128+
expect(window.location.search).toBe('?user_code=ABCDE-F1234')
129+
})
130+
131+
it('returns to the device approval page after accepting an invitation', async () => {
132+
window.history.replaceState(
133+
null,
134+
'',
135+
'/onboarding?return_to=%2Fdevice%3Fuser_code%3DABCDE-F1234',
136+
)
137+
const invitation = {
138+
id: 'inv_1',
139+
org_id: 'org_1',
140+
org_name: 'Acme Inc.',
141+
email: 'person@example.com',
142+
org_role: 'member',
143+
created_at: '2026-01-01T00:00:00Z',
144+
}
145+
hooks.pendingInvitations.push(invitation)
146+
hooks.acceptInvitation.mockResolvedValueOnce(invitation)
147+
renderOnboarding()
148+
149+
const accept = Array.from(container.querySelectorAll('button')).find(
150+
(candidate) => candidate.textContent.trim() === 'Accept',
151+
)
152+
if (!accept) throw new Error('Missing Accept button')
153+
await act(async () => {
154+
accept.click()
155+
await Promise.resolve()
156+
await Promise.resolve()
157+
})
158+
159+
expect(hooks.acceptInvitation).toHaveBeenCalledWith('inv_1')
160+
expect(window.location.pathname).toBe('/device')
161+
expect(window.location.search).toBe('?user_code=ABCDE-F1234')
162+
})
163+
164+
it('ignores an unsafe return destination', async () => {
165+
const host = window.location.host
166+
window.history.replaceState(
167+
null,
168+
'',
169+
'/onboarding?return_to=https%3A%2F%2Fevil.example%2Fsteal',
170+
)
171+
hooks.createOrganization.mockResolvedValueOnce(undefined)
172+
renderOnboarding()
173+
setOrganizationName('Acme Inc.')
174+
175+
await submitOrganization()
176+
177+
expect(window.location.host).toBe(host)
178+
expect(window.location.pathname).toBe('/')
179+
})
180+
109181
it('does not throw when a translator wraps the label before the pending update', async () => {
110182
renderOnboarding()
111183
setOrganizationName('Acme Inc.')

‎frontend/apps/web/src/routes/Onboarding.tsx‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/com
99
import { Field, FieldGroup, FieldLabel } from '@/components/ui/field'
1010
import { Input } from '@/components/ui/input'
1111
import { ResourceNameFieldError } from '@/components/ui/resource-name-error'
12+
import { isDeviceApprovalReturnTo, safeReturnTo } from '@/lib/auth-return-to'
1213
import { resourceNameValid } from '@/lib/resource-name'
1314
import { errorMessage } from '@/lib/submit-status'
1415

@@ -28,6 +29,8 @@ export function Onboarding() {
2829
const { data } = usePendingInvitations()
2930
const pending = data.data
3031
const createOrganization = useCreateOrganization()
32+
const returnTo = safeReturnTo(new URLSearchParams(window.location.search).get('return_to'))
33+
const approvingDevice = isDeviceApprovalReturnTo(returnTo)
3134

3235
const [state, setState] = useState<OnboardingState>(initialOnboardingState)
3336
const creating = state.action.kind === 'creating'
@@ -41,7 +44,7 @@ export function Onboarding() {
4144
body: { name: state.name },
4245
idempotencyKey: state.idempotencyKey,
4346
})
44-
window.location.assign('/')
47+
window.location.assign(returnTo)
4548
} catch (err) {
4649
setState((prev) => ({
4750
...prev,
@@ -62,8 +65,11 @@ export function Onboarding() {
6265
<h1 className="text-2xl font-semibold tracking-tight">Welcome to Omnara</h1>
6366
<p className="text-muted-foreground text-sm">
6467
{pending.length > 0
65-
? 'Accept an invitation or create your own organization to get started.'
66-
: 'Create your organization to get started.'}
68+
? 'Accept an invitation or create your own organization'
69+
: 'Create your organization'}
70+
{approvingDevice
71+
? ', then we will bring you back to approve the CLI login.'
72+
: ' to get started.'}
6773
</p>
6874
</div>
6975

@@ -73,7 +79,7 @@ export function Onboarding() {
7379
<PendingInvitationList
7480
invitations={pending}
7581
onAccepted={() => {
76-
window.location.assign('/')
82+
window.location.assign(returnTo)
7783
}}
7884
/>
7985
)}

‎frontend/packages/cli/src/config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ export function registerConfigCommand(program: Command, cli: CliConfig): void {
183183
throw new CliInputError('interactive selection needs a terminal')
184184
}
185185
await cli.ensureLoggedIn()
186-
const orgId = await promptOrgSelection(cli.client)
186+
const orgId = await promptOrgSelection(cli.client, cli.issuerUrl)
187187
const projectId = await promptProjectSelection(cli.client, orgId)
188188
updateConfigFile({ org_id: orgId, project_id: projectId })
189189
printConfig()

‎frontend/packages/cli/src/device-login.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ export interface DeviceLoginResult {
131131
token: string
132132
orgId?: string
133133
projectId?: string
134+
hasOrganizations?: boolean
134135
}
135136

136137
export async function loginWithDevice(options: DeviceLoginOptions): Promise<DeviceLoginResult> {
@@ -169,11 +170,13 @@ export async function loginWithDevice(options: DeviceLoginOptions): Promise<Devi
169170
const client = createOmnaraClient({ baseUrl: options.apiUrl, auth: bearerToken(token) })
170171
let orgId = saved.org_id
171172
let projectId = saved.project_id
173+
let hasOrganizations: boolean | undefined
172174
let loginMessage = 'Logged in'
173175
let validationWarning: string | undefined
174176
try {
175177
const { data: me } = await sdk.getCurrentUser({ client })
176178
loginMessage = `Logged in as ${me.user.email || me.user.display_name}`
179+
hasOrganizations = me.orgs.length > 0
177180
if (orgId !== undefined && !me.orgs.some((org) => org.id === orgId)) {
178181
updateConfigFile({ org_id: undefined, project_id: undefined })
179182
orgId = undefined
@@ -199,5 +202,5 @@ export async function loginWithDevice(options: DeviceLoginOptions): Promise<Devi
199202
if (process.env.OMNARA_API_KEY !== undefined) {
200203
report.warn('OMNARA_API_KEY is set and takes precedence over the saved token')
201204
}
202-
return { token, orgId, projectId }
205+
return { token, orgId, projectId, hasOrganizations }
203206
}

‎frontend/packages/cli/src/factory.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ interface ConfigParam {
114114
configKey: 'org_id' | 'project_id'
115115
describe: string
116116
resolve: (config: CliConfig, path: Record<string, unknown>) => string | undefined
117-
prompt: (client: OmnaraClient, path: Record<string, unknown>) => Promise<string>
117+
prompt: (config: CliConfig, path: Record<string, unknown>) => Promise<string>
118118
}
119119

120120
const CONFIG_PARAMS: ConfigParam[] = [
@@ -125,7 +125,7 @@ const CONFIG_PARAMS: ConfigParam[] = [
125125
configKey: 'org_id',
126126
describe: 'pass --org or set OMNARA_ORG_ID',
127127
resolve: (config) => config.defaultOrgId,
128-
prompt: (client) => promptOrgSelection(client),
128+
prompt: (config) => promptOrgSelection(config.client, config.issuerUrl),
129129
},
130130
{
131131
key: 'projectID',
@@ -137,12 +137,12 @@ const CONFIG_PARAMS: ConfigParam[] = [
137137
path.orgID === undefined || path.orgID === config.defaultOrgId
138138
? config.defaultProjectId
139139
: undefined,
140-
prompt: (client, path) => {
140+
prompt: (config, path) => {
141141
const orgId = path.orgID
142142
if (typeof orgId !== 'string') {
143143
throw new CliInputError('cannot select a project before an organization is set')
144144
}
145-
return promptProjectSelection(client, orgId)
145+
return promptProjectSelection(config.client, orgId)
146146
},
147147
},
148148
]
@@ -315,7 +315,7 @@ async function resolvePathValues(
315315
usedExplicitOverride = usedExplicitOverride || explicit !== undefined
316316
let value = explicit ?? param.resolve(config, path)
317317
if (value === undefined && canPromptInteractively()) {
318-
value = await param.prompt(config.client, path)
318+
value = await param.prompt(config, path)
319319
if (!usedExplicitOverride) saveConfigDefault(param.configKey, value)
320320
}
321321
if (value === undefined) {

‎frontend/packages/cli/src/interactive.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ async function selectFrom(message: string, choices: Choice[]): Promise<string> {
2626
return picked
2727
}
2828

29-
export async function promptOrgSelection(client: OmnaraClient): Promise<string> {
29+
export async function promptOrgSelection(client: OmnaraClient, issuerUrl: string): Promise<string> {
3030
const { data } = await sdk.getCurrentUser({ client })
3131
if (data.orgs.length === 0) {
32-
throw new CliInputError('this account has no organizations')
32+
throw new CliInputError(
33+
`this account has no organizations; create one at ${new URL('/onboarding', issuerUrl).toString()}`,
34+
)
3335
}
3436
return selectFrom(
3537
'Select an organization',

0 commit comments

Comments
 (0)