New auth flow#430
Conversation
vivek8690
left a comment
vivek8690
left a comment
There was a problem hiding this comment.
A few notes on keeping this consistent with how the rest of the CLI is structured. Main one is the URL building — see inline.
| return options?.region?.trim(); | ||
| } | ||
|
|
||
| function getPanelAuthBase(env: string, region?: string) { |
There was a problem hiding this comment.
We already keep all URLs in src/lib/api/services/url.ts, and this PR even adds the right entries there (OAUTH_CLIENT_CONFIG, OAUTH_DEVICE_AUTHORIZATION, OAUTH_DEVICE_TOKEN) with tests. But here in Auth.ts we hand-build the URLs with this helper instead of using them, so those new URLS.* entries end up unused (only the test references them), and we're duplicating AUTH_URL() and hardcoding v1.0 in a second place.
The reason for going manual looks like region support (/region/${region}/...), which getBaseURL() doesn't handle today. Can we instead add the region bit into the URLS helpers (or getBaseURL) and call URLS.OAUTH_CLIENT_CONFIG() / etc. from here? That keeps URL building in one place like the rest of the services. At the very least the non-region path should use the entries we just added.
|
|
||
| try { | ||
| await open(verificationLink); | ||
| console.log(`Opened link to start the auth process: ${OutputFormatter.link(verificationLink)}`); |
There was a problem hiding this comment.
These use console.log, but the success message a few lines down uses Logger.info, and the rest of the file sends informational output through Logger. Can we switch these two to Logger.info so it stays consistent (and goes through the same formatting/debug-log handling)?
| 'Content-Type': 'application/json', | ||
| }, | ||
| data: { | ||
| client_id: 'fdk-cli', |
There was a problem hiding this comment.
Minor: 'fdk-cli', the urn:ietf:...:device_code grant type, and ['organization/*'] are inlined here (and repeated below for the token call, and on the skywarp side). We usually keep shared literals in helper/constants. Pulling these into named constants would match convention and keep client/server from drifting. Not blocking.
4 participants
New auth flow