feat(auth): add auth set-token for non-interactive token import - #2293
feat(auth): add auth set-token for non-interactive token import#2293caorantaoyao wants to merge 1 commit into
auth set-token for non-interactive token import#2293Conversation
|
caorantaoyao seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds ChangesAuth token import
README formatting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant NewCmdAuthSetToken
participant CredentialStore
participant Profile
CLI->>NewCmdAuthSetToken: Provide token flags
NewCmdAuthSetToken->>CredentialStore: Store token expiry and scope
NewCmdAuthSetToken->>Profile: Synchronize user list
Profile-->>NewCmdAuthSetToken: Return synchronization result
NewCmdAuthSetToken->>CredentialStore: Restore or remove credentials if synchronization fails
NewCmdAuthSetToken-->>CLI: Return JSON or text result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@cmd/auth/set_token.go`:
- Around line 93-95: Update the refresh lifetime handling in the set-token flow:
configure the flag’s default value as 30 days, then validate the supplied
opts.RefreshExpiresIn and return a validation error when it is non-positive
instead of applying the fallback. Preserve the normal path for positive values.
- Around line 131-163: Resolve the target profile from the selected appID before
constructing or storing storedToken, rather than always using
config.ProfileName. If --app-id differs from the active profile, select the
profile associated with that app ID or reject the mismatch before calling
larkauth.SetStoredToken; pass the resolved profile name to
syncLoginUserToProfile and keep cleanup keyed to the same appID.
- Around line 156-163: Replace the unconditional RemoveStoredToken cleanup after
syncLoginUserToProfile with an internal/auth rollback helper used by the
SetStoredToken flow. Snapshot the prior token while distinguishing absent from
unreadable state, then restore it only if the imported generation is still
stored; otherwise remove only that imported generation. Propagate rollback
failures and return a typed storage error preserving both synchronization and
rollback causes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8698345e-5312-4d29-b4a4-b27454de4d46
📒 Files selected for processing (2)
cmd/auth/auth.gocmd/auth/set_token.go
| appID := opts.AppID | ||
| if appID == "" { | ||
| appID = config.AppID | ||
| } | ||
| if appID == "" { | ||
| return errs.NewConfigError(errs.SubtypeNotConfigured, | ||
| "no app ID configured; run `lark-cli config init` first or pass --app-id") | ||
| } | ||
|
|
||
| profileName := config.ProfileName | ||
|
|
||
| // Build StoredUAToken | ||
| now := time.Now().UnixMilli() | ||
| storedToken := &larkauth.StoredUAToken{ | ||
| UserOpenId: opts.OpenID, | ||
| AppId: appID, | ||
| AccessToken: opts.AccessToken, | ||
| RefreshToken: opts.RefreshToken, | ||
| ExpiresAt: now + int64(opts.ExpiresIn)*1000, | ||
| RefreshExpiresAt: now + int64(opts.RefreshExpiresIn)*1000, | ||
| Scope: opts.Scope, | ||
| GrantedAt: now, | ||
| } | ||
|
|
||
| // Write to keychain | ||
| if err := larkauth.SetStoredToken(storedToken); err != nil { | ||
| return errs.NewInternalError(errs.SubtypeStorage, "failed to save token: %v", err).WithCause(err) | ||
| } | ||
|
|
||
| // Update profile user list (same logic as login success) | ||
| if err := syncLoginUserToProfile(profileName, appID, opts.OpenID, opts.UserName); err != nil { | ||
| _ = larkauth.RemoveStoredToken(appID, opts.OpenID) | ||
| return errs.NewInternalError(errs.SubtypeStorage, "failed to update profile: %v", err).WithCause(err) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline cmd/auth/set_token.go --match authSetTokenRun --view expanded
rg -n -C 8 --type go \
'func\s+syncLoginUserToProfile\b|syncLoginUserToProfile\s*\(' cmd internal
rg -n -C 8 --type go \
'func\s+\(.*\)\s*(CurrentAppConfig|FindApp|RequireAppConfig)\b' internal/coreRepository: larksuite/cli
Length of output: 10125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- syncLoginUserToProfile ---'
sed -n '472,535p' cmd/auth/login.go
printf '%s\n' '--- set-token setup and command wiring ---'
sed -n '1,180p' cmd/auth/set_token.go
printf '%s\n' '--- token storage implementation and callers ---'
rg -n -C 12 --type go \
'func\s+(SetStoredToken|RemoveStoredToken|GetStoredToken|LoadStoredToken)\b|SetStoredToken\s*\(|RemoveStoredToken\s*\(' .
printf '%s\n' '--- profile and app-ID fixtures/tests ---'
rg -n -C 8 --type go \
'AppId:|AppID:|ProfileName:|--app-id|app-id|syncLoginUserToProfile' cmd/auth internal | head -n 500Repository: larksuite/cli
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- targeted set-token and profile tests ---'
rg -n -C 12 --type go \
'SetToken|set-token|RefreshExpiresIn|RefreshExpiresIn|AppID:.*app|ProfileName:.*' \
cmd/auth/*test.go
printf '%s\n' '--- token storage definitions ---'
rg -l --type go \
'func\s+(SetStoredToken|RemoveStoredToken|GetStoredToken|LoadStoredToken)\b' internal cmd | while read -r file; do
echo "FILE: $file"
rg -n -A 35 -B 5 --type go \
'func\s+(SetStoredToken|RemoveStoredToken|GetStoredToken|LoadStoredToken)\b' "$file"
done
printf '%s\n' '--- profile identity and config loading ---'
rg -n -A 25 -B 8 --type go \
'func\s+\(.*\)\s*ProfileName\b|type\s+CliConfig\b|func\s+\(.*\)\s*Config\b|ProfileName\s*=' \
internal cmd/auth | head -n 300Repository: larksuite/cli
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- token store ---'
sed -n '1,180p' internal/auth/token_store.go
printf '%s\n' '--- profile model ---'
rg -l --type go 'func\s+\(.*\)\s*ProfileName\b|type\s+AppConfig\b|type\s+CliConfig\b' internal/core | while read -r file; do
echo "FILE: $file"
rg -n -A 30 -B 5 --type go \
'func\s+\(.*\)\s*ProfileName\b|type\s+AppConfig\b|type\s+CliConfig\b' "$file"
done
printf '%s\n' '--- deterministic call-contract check ---'
python3 - <<'PY'
from pathlib import Path
source = Path("cmd/auth/login.go").read_text()
start = source.index("func syncLoginUserToProfile(")
end = source.index("\n}\n\n// findProfileByName", start) + 2
body = source[start:end]
checks = {
"profile lookup uses profileName": "findProfileByName(multi, profileName)" in body,
"appID is not used for profile lookup": "findProfileByName(multi, appID)" not in body,
"appID is used for token cleanup": "RemoveStoredToken(appID" in body,
}
for label, result in checks.items():
print(f"{label}: {result}")
if not all(checks.values()):
raise SystemExit("unexpected syncLoginUserToProfile contract")
print("contract: --app-id can differ from the profile selected by profileName")
PYRepository: larksuite/cli
Length of output: 9985
Resolve --app-id to the target profile before syncing.
syncLoginUserToProfile selects the profile by profileName but uses appID for token cleanup. When --app-id differs from the active profile, the command stores an orphaned token, updates the wrong profile, and can remove tokens under the mismatched app ID. Resolve the profile from appID or reject the mismatch before storing the token.
🤖 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/set_token.go` around lines 131 - 163, Resolve the target profile
from the selected appID before constructing or storing storedToken, rather than
always using config.ProfileName. If --app-id differs from the active profile,
select the profile associated with that app ID or reject the mismatch before
calling larkauth.SetStoredToken; pass the resolved profile name to
syncLoginUserToProfile and keep cleanup keyed to the same appID.
| if err := larkauth.SetStoredToken(storedToken); err != nil { | ||
| return errs.NewInternalError(errs.SubtypeStorage, "failed to save token: %v", err).WithCause(err) | ||
| } | ||
|
|
||
| // Update profile user list (same logic as login success) | ||
| if err := syncLoginUserToProfile(profileName, appID, opts.OpenID, opts.UserName); err != nil { | ||
| _ = larkauth.RemoveStoredToken(appID, opts.OpenID) | ||
| return errs.NewInternalError(errs.SubtypeStorage, "failed to update profile: %v", err).WithCause(err) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline internal/auth --items all --type function --match 'StoredToken|Token'
rg -n -C 8 --type go \
'func\s+(SetStoredToken|RemoveStoredToken|GetStoredToken)\b|SetStoredToken\s*\(|RemoveStoredToken\s*\(' \
internal/auth cmd/auth
rg -n -C 8 --type go \
'func\s+syncLoginUserToProfile\b|syncLoginUserToProfile\s*\(' cmd internalRepository: larksuite/cli
Length of output: 39776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- token_store.go ---'
cat -n internal/auth/token_store.go | sed -n '1,135p'
echo '--- set_token.go ---'
cat -n cmd/auth/set_token.go | sed -n '1,190p'
echo '--- syncLoginUserToProfile ---'
cat -n cmd/auth/login.go | sed -n '468,500p'
echo '--- set-token tests and config resolution ---'
rg -n -C 10 --type go \
'SetToken|setToken|AppID|ProfileName|GetStoredToken|RemoveStoredToken' \
cmd/auth/set_token* cmd/auth/login_config_test.goRepository: larksuite/cli
Length of output: 23713
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- generation-guard helpers ---'
cat -n internal/auth/token_store.go | sed -n '117,210p'
echo '--- storage error composition patterns ---'
rg -n -C 6 --type go \
'errors\.Join|WithCause\(errors\.Join|ProblemOf|compareAnd(Delete|Swap)StoredToken' \
internal/auth cmd errs
echo '--- token-storage locking ---'
cat -n internal/auth/token_lock.go | sed -n '1,180p'Repository: larksuite/cli
Length of output: 50369
Make token replacement reversible and generation-aware.
SetStoredToken overwrites the (appID, OpenID) key. If profile synchronization fails, line 162 can delete the previous token or a newer token from a concurrent refresh. An ignored removal error can leave the imported token after command failure. GetStoredToken cannot safely snapshot state because it discards read errors.
Add an internal/auth rollback helper that distinguishes absent and unreadable tokens, restores the previous token only when the imported generation is still stored, and otherwise deletes only the imported generation. Return a typed storage error that preserves the synchronization and rollback causes.
🤖 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/set_token.go` around lines 156 - 163, Replace the unconditional
RemoveStoredToken cleanup after syncLoginUserToProfile with an internal/auth
rollback helper used by the SetStoredToken flow. Snapshot the prior token while
distinguishing absent from unreadable state, then restore it only if the
imported generation is still stored; otherwise remove only that imported
generation. Propagate rollback failures and return a typed storage error
preserving both synchronization and rollback causes.
Source: Coding guidelines
fa4d8bc to
b0a179e
Compare
6edb9c0 to
d5a13d3
Compare
3666751 to
c98a5d0
Compare
c98a5d0 to
11b0cf6
Compare
Adds a new `auth set-token` subcommand that allows importing externally obtained OAuth access/refresh tokens into the CLI's credential store without going through the device-flow login. This is useful for CI/CD pipelines and automation where the interactive browser flow is not possible. The command accepts --access-token, --refresh-token, --open-id, --user-name, --expires-in, --refresh-expires-in (default 30 days), and --scope flags. It writes the token to the same keychain-backed store used by `auth login`, updates the current profile's user list via the shared syncLoginUserToProfile helper, and safely rolls back to the previous token if profile synchronization fails.
7dd5f64 to
ce4bce8
Compare
Summary
Adds a new subcommand
lark-cli auth set-tokenthat imports an already-obtained user OAuth token (access + refresh) into the CLI's credential store, bypassing the interactive device-flow handshake.Motivation
Currently,
lark-cli auth loginonly supports the Device Flow (verification_uri+ polling), which requires the user to open a browser and click "authorize". This works great for interactive terminal use, but is a blocker for non-interactive environments:These environments cannot block on a user visiting
verification_uri, so there is no way to bootstrap a valid lark-cli credential today short of reverse-engineering the keychain file format.Changes
cmd/auth/set_token.go: implementsNewCmdAuthSetTokenfollowing the same pattern as the existing subcommands (login.go,logout.go, etc.)cmd/auth/auth.goalongside the otherauthsubcommandslarkauth.SetStoredToken— writes to the platform keychain (macOS Keychain / Linux AES-GCM file / Windows DPAPI) in exactly the same format produced byauth loginsyncLoginUserToProfile— updates the profile's user list, consistent with the login success pathCommand design
Key design decisions:
auth login, so subsequent calls toauth status,whoami,auth check, and all shortcut commands work identically. No new code paths downstream.--access-token,--refresh-token,--open-id,--expires-in. This avoids silent misconfiguration.--open-idis required (rather than fetched from the API) to keep the command offline and because the caller performing the OAuth exchange already knows the open_id.--jsonfor structured output, matchingauth login's JSON mode for agent harness consumption.Verification
go build ./...passesgo vet ./cmd/auth/...passesgo test ./cmd/auth/...passes (all existing auth tests + new code compiles against existing test infrastructure)set-tokenwith fake credentials into an isolated HOME, confirmedauth statusreportsuser.status: "ready",tokenStatus: "valid", and the user is listed inconfig showExample usage
Notes
auth login(device flow) remains the recommended interactive path.errs.NewValidationErrorpattern used throughout the codebase."write"(matchingauth login), so it participates in the existing risk-control/confirm flows.Looking forward to your feedback! Happy to adjust flags, add tests, or refactor as needed.
Summary by CodeRabbit
auth set-tokencommand for importing externally obtained OAuth access and refresh tokens.