feat(auth): add QR code support for device auth flow#942
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds QR-code generation (ASCII and base64 PNG) to device-flow login output and embeds those QR fields into JSON events; updates text output to show ASCII and base64 QR for non-TTY, and extends localized login messages with QR/agent guidance. ChangesQR Code Support in Device-Flow Login
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@cba34c25d7e330829e5d47528d62276b1c603eaa🧩 Skill updatenpx skills add larksuite/cli#feat/auth_qrcode -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/auth/login.go (1)
311-327: ⚡ Quick winUse the non-TTY message keys in the non-TTY branch.
This branch still prints
ScanQRCode/ScanOrOpenLinkfor all callers, soOpenLinkNonTTYandWaitingForScanNonTTYnever appear for agent/non-TTY flows.♻️ Proposed refactor
- fmt.Fprintf(f.IOStreams.ErrOut, msg.ScanQRCode) + if f.IOStreams.IsTerminal { + fmt.Fprintf(f.IOStreams.ErrOut, msg.ScanQRCode) + } else { + fmt.Fprintf(f.IOStreams.ErrOut, msg.OpenLinkNonTTY) + } qrCodeASCII, qrCodeBase64 := generateQRCode(authResp.VerificationUriComplete) fmt.Fprint(f.IOStreams.ErrOut, qrCodeASCII) if !f.IOStreams.IsTerminal { if qrCodeBase64 != "" { fmt.Fprintf(f.IOStreams.ErrOut, "[BASE64 QR CODE START]\n") fmt.Fprintf(f.IOStreams.ErrOut, "%s\n", qrCodeBase64) fmt.Fprintf(f.IOStreams.ErrOut, "[BASE64 QR CODE END]\n") fmt.Fprintf(f.IOStreams.ErrOut, "%s\n", msg.QRCodeDisplayHint) } } fmt.Fprintln(f.IOStreams.ErrOut) - fmt.Fprintf(f.IOStreams.ErrOut, msg.ScanOrOpenLink) + if f.IOStreams.IsTerminal { + fmt.Fprintf(f.IOStreams.ErrOut, msg.ScanOrOpenLink) + } fmt.Fprintf(f.IOStreams.ErrOut, " %s\n\n", authResp.VerificationUriComplete) + if !f.IOStreams.IsTerminal { + fmt.Fprintln(f.IOStreams.ErrOut, msg.WaitingForScanNonTTY) + } fmt.Fprintln(f.IOStreams.ErrOut, msg.AgentTimeoutHint)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/auth/login.go` around lines 311 - 327, The non-TTY branch is still using the interactive message keys (msg.ScanQRCode and msg.ScanOrOpenLink) so agents never see the non-TTY messages; update the conditional around f.IOStreams.IsTerminal in the login output block (the area that calls generateQRCode(authResp.VerificationUriComplete) and prints qrCodeASCII/qrCodeBase64) to print msg.OpenLinkNonTTY and msg.WaitingForScanNonTTY instead of msg.ScanQRCode/msg.ScanOrOpenLink when !f.IOStreams.IsTerminal, while preserving the base64 QR output and the VerificationUriComplete; ensure the TTY path still uses msg.ScanQRCode/msg.ScanOrOpenLink and that the non-TTY branch uses the correct non-TTY message keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/auth/login.go`:
- Around line 311-327: The non-TTY branch is still using the interactive message
keys (msg.ScanQRCode and msg.ScanOrOpenLink) so agents never see the non-TTY
messages; update the conditional around f.IOStreams.IsTerminal in the login
output block (the area that calls
generateQRCode(authResp.VerificationUriComplete) and prints
qrCodeASCII/qrCodeBase64) to print msg.OpenLinkNonTTY and
msg.WaitingForScanNonTTY instead of msg.ScanQRCode/msg.ScanOrOpenLink when
!f.IOStreams.IsTerminal, while preserving the base64 QR output and the
VerificationUriComplete; ensure the TTY path still uses
msg.ScanQRCode/msg.ScanOrOpenLink and that the non-TTY branch uses the correct
non-TTY message keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a289737-2071-41e6-8965-23bd798e8551
📒 Files selected for processing (2)
cmd/auth/login.gocmd/auth/login_messages.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #942 +/- ##
==========================================
+ Coverage 65.92% 66.49% +0.56%
==========================================
Files 523 562 +39
Lines 49694 51895 +2201
==========================================
+ Hits 32760 34506 +1746
- Misses 14134 14499 +365
- Partials 2800 2890 +90 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* feat(auth): add QR code support for device auth flow * docs: update login QR code display hints for AI agent * feat(auth): add ASCII QR code support for auth flow * docs: add comments for login and auth helper functions * chore: remove unused qrCodeToBase64 helper function * fix(auth/login): clarify verification_url handling in login hint
…te#942)" (larksuite#950) This reverts commit 8e63713.
Summary
This PR adds QR code support for the device authentication flow, including both image and ASCII QR code display options, with improved login hints optimized for AI agent parsing.
Changes
Test Plan
lark auth logincommand works as expectedRelated Issues
Summary by CodeRabbit
New Features
Documentation / Messages