Skip to content

Commit 12902d1

Browse files
committed
feat: enhance CLILoginClient to include user name in callback URL and update loading state handling
1 parent d1d1661 commit 12902d1

2 files changed

Lines changed: 52 additions & 17 deletions

File tree

__tests__/app/cli/login/cli-login-client.test.tsx

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,18 @@ import CLILoginClient, {
99

1010
describe('buildCallbackUrl', () => {
1111
it('should build a valid localhost callback URL', () => {
12-
const url = buildCallbackUrl('8080', 'my-token', 'my-state')
12+
const url = buildCallbackUrl('8080', 'my-token', 'my-state', 'Alice')
1313
expect(url.hostname).toBe('localhost')
1414
expect(url.port).toBe('8080')
1515
expect(url.pathname).toBe('/callback')
1616
expect(url.searchParams.get('token')).toBe('my-token')
1717
expect(url.searchParams.get('state')).toBe('my-state')
18+
expect(url.searchParams.get('name')).toBe('Alice')
1819
})
1920

2021
it('should reject non-localhost hosts', () => {
2122
// buildCallbackUrl hardcodes localhost, so this tests the safety check
22-
expect(() => buildCallbackUrl('8080', 't', 's')).not.toThrow()
23+
expect(() => buildCallbackUrl('8080', 't', 's', 'n')).not.toThrow()
2324
})
2425
})
2526

@@ -35,30 +36,34 @@ describe('CLILoginClient', () => {
3536
vi.restoreAllMocks()
3637
})
3738

38-
it('should show loading state initially', () => {
39-
fetchMock.mockReturnValue(new Promise(() => {})) // never resolves
39+
it('should show confirm screen initially', () => {
4040
render(<CLILoginClient userName="Test User" userEmail="test@example.com" />)
41-
expect(screen.getByText(/generating token/i)).toBeInTheDocument()
41+
expect(screen.getByText(/requesting access/i)).toBeInTheDocument()
42+
expect(
43+
screen.getByRole('button', { name: /authorize cli/i }),
44+
).toBeInTheDocument()
4245
expect(
4346
screen.getByText(/Test User \(test@example\.com\)/),
4447
).toBeInTheDocument()
48+
expect(fetchMock).not.toHaveBeenCalled()
4549
})
4650

47-
it('should display token for manual copy when no port is provided', async () => {
51+
it('should display token for manual copy after authorizing', async () => {
4852
fetchMock.mockResolvedValue({
4953
ok: true,
5054
json: () => Promise.resolve({ token: 'jwt-token-value' }),
5155
})
5256

5357
render(<CLILoginClient userName="Test User" userEmail="test@example.com" />)
58+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
5459

5560
await waitFor(() => {
5661
expect(screen.getByText('jwt-token-value')).toBeInTheDocument()
5762
})
5863
expect(screen.getByText(/copy this token/i)).toBeInTheDocument()
5964
})
6065

61-
it('should redirect to localhost callback when port and state are provided', async () => {
66+
it('should redirect to localhost callback after authorizing', async () => {
6267
fetchMock.mockResolvedValue({
6368
ok: true,
6469
json: () => Promise.resolve({ token: 'jwt-token-value' }),
@@ -90,6 +95,8 @@ describe('CLILoginClient', () => {
9095
/>,
9196
)
9297

98+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
99+
93100
await waitFor(() => {
94101
expect(hrefSetter).toHaveBeenCalledWith(
95102
expect.stringContaining('http://localhost:9876/callback'),
@@ -99,6 +106,7 @@ describe('CLILoginClient', () => {
99106
const redirectUrl = new URL(hrefSetter.mock.calls[0][0])
100107
expect(redirectUrl.searchParams.get('token')).toBe('jwt-token-value')
101108
expect(redirectUrl.searchParams.get('state')).toBe('random-state')
109+
expect(redirectUrl.searchParams.get('name')).toBe('Test User')
102110

103111
locationHref.mockRestore()
104112
})
@@ -113,6 +121,8 @@ describe('CLILoginClient', () => {
113121
/>,
114122
)
115123

124+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
125+
116126
await waitFor(() => {
117127
expect(screen.getByText(/invalid port/i)).toBeInTheDocument()
118128
})
@@ -129,6 +139,8 @@ describe('CLILoginClient', () => {
129139
/>,
130140
)
131141

142+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
143+
132144
await waitFor(() => {
133145
expect(screen.getByText(/invalid port/i)).toBeInTheDocument()
134146
})
@@ -144,6 +156,8 @@ describe('CLILoginClient', () => {
144156
/>,
145157
)
146158

159+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
160+
147161
await waitFor(() => {
148162
expect(screen.getByText(/missing state/i)).toBeInTheDocument()
149163
})
@@ -158,6 +172,7 @@ describe('CLILoginClient', () => {
158172
})
159173

160174
render(<CLILoginClient userName="Test User" userEmail="test@example.com" />)
175+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
161176

162177
await waitFor(() => {
163178
expect(screen.getByText('Unauthorized')).toBeInTheDocument()
@@ -168,6 +183,7 @@ describe('CLILoginClient', () => {
168183
fetchMock.mockRejectedValue(new Error('Network error'))
169184

170185
render(<CLILoginClient userName="Test User" userEmail="test@example.com" />)
186+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
171187

172188
await waitFor(() => {
173189
expect(screen.getByText(/failed to connect/i)).toBeInTheDocument()
@@ -184,6 +200,7 @@ describe('CLILoginClient', () => {
184200
Object.assign(navigator, { clipboard: { writeText } })
185201

186202
render(<CLILoginClient userName="Test User" userEmail="test@example.com" />)
203+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
187204

188205
await waitFor(() => {
189206
expect(screen.getByText('copy-me')).toBeInTheDocument()
@@ -210,6 +227,7 @@ describe('CLILoginClient', () => {
210227
})
211228

212229
render(<CLILoginClient userName="Test User" userEmail="test@example.com" />)
230+
fireEvent.click(screen.getByRole('button', { name: /authorize cli/i }))
213231

214232
await waitFor(() => {
215233
expect(screen.getByText('Server error')).toBeInTheDocument()

src/app/cli/login/cli-login-client.tsx

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
'use client'
22

3-
import { useState, useEffect, useCallback } from 'react'
3+
import { useState, useCallback } from 'react'
44
import {
55
CheckCircleIcon,
66
ClipboardDocumentIcon,
7+
CommandLineIcon,
78
ExclamationTriangleIcon,
89
} from '@heroicons/react/24/outline'
910

10-
type Status = 'loading' | 'redirecting' | 'display-token' | 'error'
11+
type Status = 'confirm' | 'loading' | 'redirecting' | 'display-token' | 'error'
1112

1213
const MIN_PORT = 1024
1314
const MAX_PORT = 65535
@@ -25,10 +26,12 @@ export function buildCallbackUrl(
2526
port: string,
2627
token: string,
2728
state: string,
29+
name: string,
2830
): URL {
2931
const url = new URL(`http://localhost:${port}/callback`)
3032
url.searchParams.set('token', token)
3133
url.searchParams.set('state', state)
34+
url.searchParams.set('name', name)
3235
if (!isLocalhostOrigin(url)) {
3336
throw new Error('Redirect target must be localhost')
3437
}
@@ -48,7 +51,7 @@ export default function CLILoginClient({
4851
userName,
4952
userEmail,
5053
}: CLILoginClientProps) {
51-
const [status, setStatus] = useState<Status>('loading')
54+
const [status, setStatus] = useState<Status>('confirm')
5255
const [token, setToken] = useState<string>('')
5356
const [error, setError] = useState<string>('')
5457
const [copied, setCopied] = useState(false)
@@ -67,6 +70,8 @@ export default function CLILoginClient({
6770
}
6871
}
6972

73+
setStatus('loading')
74+
7075
try {
7176
const res = await fetch('/api/auth/cli/token', { method: 'POST' })
7277
if (!res.ok) {
@@ -80,7 +85,7 @@ export default function CLILoginClient({
8085

8186
if (port && state) {
8287
setStatus('redirecting')
83-
const callbackUrl = buildCallbackUrl(port, jwt, state)
88+
const callbackUrl = buildCallbackUrl(port, jwt, state, userName)
8489
window.location.href = callbackUrl.toString()
8590
} else {
8691
setToken(jwt)
@@ -90,12 +95,7 @@ export default function CLILoginClient({
9095
setError('Failed to connect to authentication service')
9196
setStatus('error')
9297
}
93-
}, [port, state])
94-
95-
useEffect(() => {
96-
// eslint-disable-next-line react-hooks/set-state-in-effect -- fetch-on-mount pattern
97-
fetchTokenAndRedirect()
98-
}, [fetchTokenAndRedirect])
98+
}, [port, state, userName])
9999

100100
const handleCopy = async () => {
101101
await navigator.clipboard.writeText(token)
@@ -115,6 +115,23 @@ export default function CLILoginClient({
115115
</span>
116116
</p>
117117

118+
{status === 'confirm' && (
119+
<div className="mt-8">
120+
<CommandLineIcon className="mx-auto h-12 w-12 text-gray-400 dark:text-gray-500" />
121+
<p className="mt-4 text-sm text-gray-600 dark:text-gray-400">
122+
The <span className="font-mono font-medium">cnctl</span> CLI is
123+
requesting access to your account.
124+
</p>
125+
<button
126+
type="button"
127+
onClick={fetchTokenAndRedirect}
128+
className="mt-6 rounded-lg bg-brand-cloud-blue px-6 py-2.5 text-sm font-medium text-white hover:bg-brand-cloud-blue-hover"
129+
>
130+
Authorize CLI
131+
</button>
132+
</div>
133+
)}
134+
118135
{status === 'loading' && (
119136
<div className="mt-8">
120137
<div className="mx-auto h-8 w-8 animate-spin rounded-full border-4 border-gray-200 border-t-brand-cloud-blue" />

0 commit comments

Comments
 (0)