-
Notifications
You must be signed in to change notification settings - Fork 445
feat: agent-friendly login flow (--request / --check) #7971
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+323
−42
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ae3371e
feat: add --request and --check flags to netlify login for agent-frie…
biilmann 358955c
docs sync via npm run docs
DavidWells cfa5bbe
Merge branch 'main' into agent-login-flow
sean-roberts ce0f510
Merge branch 'main' into agent-login-flow
sean-roberts 63bf195
Merge branch 'main' into agent-login-flow
biilmann 4ecea64
Merge branch 'main' into agent-login-flow
biilmann 82f7a3b
feat: pass message to API when creating login ticket via --request
biilmann b8bffb4
fix: add type assertion for options.request to satisfy eslint
biilmann 9b28861
refactor: pass apiOpts and globalConfig to login helpers
biilmann e3675e3
fix: pass missing args in login-check rethrow tests
biilmann 90330d8
Merge branch 'main' into agent-login-flow
biilmann File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { NetlifyAPI } from '@netlify/api' | ||
| import { OptionValues } from 'commander' | ||
|
|
||
| import { log, logAndThrowError, logJson } from '../../utils/command-helpers.js' | ||
| import { storeToken } from '../base-command.js' | ||
| import type { NetlifyOptions } from '../types.js' | ||
|
|
||
| export const loginCheck = async ( | ||
| options: OptionValues, | ||
| apiOpts: NetlifyOptions['apiOpts'], | ||
| globalConfig: NetlifyOptions['globalConfig'], | ||
| ) => { | ||
| const ticketId = options.check as string | ||
|
|
||
| const api = new NetlifyAPI('', apiOpts) | ||
|
|
||
| let ticket: { authorized?: boolean } | ||
| try { | ||
| ticket = await api.showTicket({ ticketId }) | ||
| } catch (error) { | ||
| const status = (error as { status?: number }).status | ||
| if (status === 401 || status === 404) { | ||
| logJson({ status: 'denied' }) | ||
| log('Status: denied') | ||
| return | ||
| } | ||
| throw error | ||
| } | ||
|
|
||
| if (!ticket.authorized) { | ||
| logJson({ status: 'pending' }) | ||
| log('Status: pending') | ||
| return | ||
| } | ||
|
|
||
| const tokenResponse = await api.exchangeTicket({ ticketId }) | ||
| const accessToken = tokenResponse.access_token | ||
| if (!accessToken) { | ||
| return logAndThrowError('Could not retrieve access token') | ||
| } | ||
|
|
||
| api.accessToken = accessToken | ||
| const user = await api.getCurrentUser() | ||
| if (!user.id) { | ||
| return logAndThrowError('Could not retrieve user ID from Netlify API') | ||
| } | ||
|
|
||
| storeToken(globalConfig, { | ||
| userId: user.id, | ||
| name: user.full_name, | ||
| email: user.email, | ||
| accessToken, | ||
| }) | ||
|
|
||
| logJson({ | ||
| status: 'authorized', | ||
| user: { id: user.id, email: user.email, name: user.full_name }, | ||
| }) | ||
|
|
||
| log('Status: authorized') | ||
| log(`Name: ${user.full_name ?? ''}`) | ||
| log(`Email: ${user.email ?? ''}`) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { NetlifyAPI } from '@netlify/api' | ||
|
|
||
| import { log, logAndThrowError, logJson } from '../../utils/command-helpers.js' | ||
| import { CLIENT_ID } from '../base-command.js' | ||
| import type { NetlifyOptions } from '../types.js' | ||
|
|
||
| export const loginRequest = async (message: string, apiOpts: NetlifyOptions['apiOpts']) => { | ||
| const webUI = process.env.NETLIFY_WEB_UI || 'https://app.netlify.com' | ||
|
|
||
| const api = new NetlifyAPI('', apiOpts) | ||
|
|
||
| const ticket = await api.createTicket({ clientId: CLIENT_ID, body: { message } }) | ||
|
|
||
| if (!ticket.id) { | ||
| return logAndThrowError('Failed to create login ticket') | ||
| } | ||
| const ticketId = ticket.id | ||
| const url = `${webUI}/authorize?response_type=ticket&ticket=${ticketId}` | ||
|
|
||
| logJson({ ticket_id: ticketId, url, check_command: `netlify login --check ${ticketId}` }) | ||
|
|
||
| log(`Ticket ID: ${ticketId}`) | ||
| log(`Authorize URL: ${url}`) | ||
| log() | ||
| log(`After authorizing, run: netlify login --check ${ticketId}`) | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
lodash merge undefined values overwrite destination property💡 Result:
_.merge()is designed to skipundefinedvalues from the source when the destination already has a value: “Source properties that resolve toundefinedare skipped if a destination value exists.” [1]. So{ a: undefined }in the source will not overwrite an existingdest.a.Also,
_.mergeWith()won’t help you force “writeundefined”, because if the customizer returnsundefined, Lodash treats that as “fall back to normal merge behavior” [1]—which still skipsundefined.Workarounds (pick one):
null(or another sentinel) instead ofundefinedwhen you want to clear a value, so merge will overwrite.undefined), use a non-merge approach (e.g.,Object.assign/ spread) for those fields (note: shallow) [1].undefinedin the source onto the result (works for shallow keys; for deep paths you’d need to walk the object).Example (shallow keys):
Sources: Lodash docs for
_.merge/_.mergeWithbehavior [1]; discussion/example ofmerge()skippingundefined[2].[1] (lodash.com)
[2] (onlinecode.org)
Citations:
Reset token-reset semantics explicitly instead of relying on
mergewithundefined.lodash/mergeskips source properties that areundefinedwhen a destination value already exists. This means staleauth.githubcredentials will not be cleared by the current code, creating a security risk where old tokens can survive token-reset operations.Replace the merge with explicit property assignment:
Explicit overwrite approach
📝 Committable suggestion
🤖 Prompt for AI Agents