feat: restore client-secret app credentials #2322
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
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,70 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package config | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/spf13/cobra" | ||
|
|
||
| "github.com/larksuite/cli/errs" | ||
| "github.com/larksuite/cli/internal/cmdutil" | ||
| "github.com/larksuite/cli/internal/core" | ||
| "github.com/larksuite/cli/internal/output" | ||
| ) | ||
|
|
||
| func validateRestoreFlags(cmd *cobra.Command, opts *ConfigInitOptions) error { | ||
| if !opts.Restore { | ||
| return nil | ||
| } | ||
| for _, name := range []string{"new", "app-id", "app-secret-stdin", "brand", "lang", "name"} { | ||
| if cmd.Flags().Changed(name) { | ||
| flag := "--" + name | ||
| return errs.NewValidationError(errs.SubtypeInvalidArgument, | ||
| "%s cannot be used with --restore", flag).WithParam(flag) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func runRestoreFlow( | ||
| opts *ConfigInitOptions, | ||
| existing *core.MultiAppConfig, | ||
| f *cmdutil.Factory, | ||
| msg *initMsg, | ||
| ) error { | ||
| app, err := existing.RequireAppConfig(f.Invocation.Profile, f.Invocation.ProfileSource) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if app.AppId == "" { | ||
| return errs.NewConfigError(errs.SubtypeInvalidConfig, | ||
| "app selected for restore has an empty app ID") | ||
| } | ||
|
|
||
| result, err := runCreateAppFlow(opts.Ctx, f, core.ParseBrand(string(app.Brand)), msg, app.AppId) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if result == nil || result.AppID != app.AppId || result.AppSecret == "" { | ||
| return errs.NewConfigError(errs.SubtypeInvalidClient, | ||
| "app restore returned invalid credentials for the configured app") | ||
| } | ||
|
|
||
| secret, err := core.ForStorage(app.AppId, core.PlainSecret(result.AppSecret), f.Keychain) | ||
| if err != nil { | ||
| return errs.NewInternalError(errs.SubtypeSDKError, "%v", err).WithCause(err) | ||
| } | ||
| app.AppSecret = secret | ||
| app.Brand = result.Brand | ||
| if err := core.SaveMultiAppConfig(existing); err != nil { | ||
| return wrapSaveConfigError(err) | ||
| } | ||
|
|
||
| output.PrintSuccess(f.IOStreams.ErrOut, fmt.Sprintf(msg.AppCreated, app.AppId)) | ||
| output.PrintJson(f.IOStreams.Out, map[string]interface{}{ | ||
| "appId": app.AppId, "appSecret": "****", "brand": app.Brand, | ||
| }) | ||
| return runProbe(opts.Ctx, f, app.AppId, result.AppSecret, app.Brand) | ||
| } | ||
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,166 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package config | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "io" | ||
| "net/http" | ||
| "reflect" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/larksuite/cli/errs" | ||
| "github.com/larksuite/cli/internal/cmdutil" | ||
| "github.com/larksuite/cli/internal/core" | ||
| ) | ||
|
|
||
| type restoreRoundTripFunc func(*http.Request) (*http.Response, error) | ||
|
|
||
| func (f restoreRoundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { | ||
| return f(req) | ||
| } | ||
|
|
||
| type recordingKeychain struct { | ||
| setCalls int | ||
| account string | ||
| value string | ||
| } | ||
|
|
||
| func (k *recordingKeychain) Get(_, _ string) (string, error) { return "", nil } | ||
| func (k *recordingKeychain) Remove(_, _ string) error { return nil } | ||
| func (k *recordingKeychain) Set(_, account, value string) error { | ||
| k.setCalls++ | ||
| k.account = account | ||
| k.value = value | ||
| return nil | ||
| } | ||
|
|
||
| func TestConfigInitRestoreUpdatesSelectedProfile(t *testing.T) { | ||
| clearAgentEnv(t) | ||
| t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) | ||
| t.Setenv("LARK_CLI_NO_PROXY", "") | ||
| original := &core.MultiAppConfig{ | ||
| CurrentApp: "other", | ||
| Apps: []core.AppConfig{ | ||
| {Name: "target", AppId: "cli_target", AppSecret: core.PlainSecret("old-secret"), Brand: core.BrandFeishu, | ||
| Users: []core.AppUser{{UserOpenId: "ou_target", UserName: "Target"}}}, | ||
| {Name: "other", AppId: "cli_other", AppSecret: core.PlainSecret("other-secret"), Brand: core.BrandLark}, | ||
| }, | ||
| } | ||
| if err := core.SaveMultiAppConfig(original); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| var beginAppID string | ||
| replaceRestoreDefaultTransport(t, restoreRoundTripFunc(func(req *http.Request) (*http.Response, error) { | ||
| if err := req.ParseForm(); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| switch req.Form.Get("action") { | ||
| case "begin": | ||
| beginAppID = req.Form.Get("app_id") | ||
| return restoreJSONResponse(`{"device_code":"device","user_code":"TEST-CODE","expire_in":30,"interval":0}`), nil | ||
| case "poll": | ||
| return restoreJSONResponse(`{"client_id":"cli_target","client_secret":"restored-secret"}`), nil | ||
| default: | ||
| t.Fatalf("unexpected action %q", req.Form.Get("action")) | ||
| return nil, nil | ||
| } | ||
| })) | ||
|
|
||
| kc := &recordingKeychain{} | ||
| rt := &fakeRT{} | ||
| f, stdout, _, _ := cmdutil.TestFactory(t, nil) | ||
| f.Keychain = kc | ||
| f.Invocation = cmdutil.InvocationContext{Profile: "target", ProfileSource: core.ProfileFromFlag} | ||
| f.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: rt}, nil } | ||
| cmd := NewCmdConfigInit(f, nil) | ||
| cmd.SetArgs([]string{"--restore"}) | ||
| if err := cmd.ExecuteContext(context.Background()); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| if beginAppID != "cli_target" { | ||
| t.Fatalf("begin app_id = %q, want cli_target", beginAppID) | ||
| } | ||
| if kc.setCalls != 1 || kc.account != "appsecret:cli_target" || kc.value != "restored-secret" { | ||
| t.Fatalf("keychain set = calls:%d account:%q value:%q", kc.setCalls, kc.account, kc.value) | ||
| } | ||
| after, err := core.LoadMultiAppConfig() | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if after.Apps[0].AppSecret.Ref == nil || after.Apps[0].AppSecret.Ref.ID != "appsecret:cli_target" { | ||
| t.Fatalf("target secret = %#v", after.Apps[0].AppSecret) | ||
| } | ||
| if !reflect.DeepEqual(after.Apps[0].Users, original.Apps[0].Users) || !reflect.DeepEqual(after.Apps[1], original.Apps[1]) { | ||
| t.Fatalf("restore changed unrelated config: %#v", after) | ||
| } | ||
| var output map[string]interface{} | ||
| if err := json.Unmarshal(stdout.Bytes(), &output); err != nil { | ||
| t.Fatalf("stdout is not JSON: %v", err) | ||
| } | ||
| if output["appSecret"] != "****" || strings.Contains(stdout.String(), "restored-secret") { | ||
| t.Fatalf("stdout leaked restored secret: %q", stdout.String()) | ||
| } | ||
| } | ||
|
|
||
| func TestConfigInitRestoreRejectsMismatchedAppIDWithoutPersisting(t *testing.T) { | ||
| clearAgentEnv(t) | ||
| t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) | ||
| t.Setenv("LARK_CLI_NO_PROXY", "") | ||
| original := &core.MultiAppConfig{Apps: []core.AppConfig{{ | ||
| AppId: "cli_target", AppSecret: core.PlainSecret("old-secret"), Brand: core.BrandFeishu, | ||
| }}} | ||
| if err := core.SaveMultiAppConfig(original); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| replaceRestoreDefaultTransport(t, restoreRoundTripFunc(func(req *http.Request) (*http.Response, error) { | ||
| if err := req.ParseForm(); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if req.Form.Get("action") == "begin" { | ||
| return restoreJSONResponse(`{"device_code":"device","user_code":"TEST-CODE","expire_in":30,"interval":0}`), nil | ||
| } | ||
| return restoreJSONResponse(`{"client_id":"cli_other","client_secret":"returned-secret"}`), nil | ||
| })) | ||
|
|
||
| kc := &recordingKeychain{} | ||
| f, stdout, _, _ := cmdutil.TestFactory(t, nil) | ||
| f.Keychain = kc | ||
| cmd := NewCmdConfigInit(f, nil) | ||
| cmd.SetArgs([]string{"--restore"}) | ||
| err := cmd.ExecuteContext(context.Background()) | ||
| problem, ok := errs.ProblemOf(err) | ||
| if !ok || problem.Category != errs.CategoryConfig || problem.Subtype != errs.SubtypeInvalidClient { | ||
| t.Fatalf("problem = %#v, err = %v", problem, err) | ||
| } | ||
| if kc.setCalls != 0 || stdout.Len() != 0 { | ||
| t.Fatalf("restore persisted mismatched credentials: keychain calls=%d stdout=%q", kc.setCalls, stdout.String()) | ||
| } | ||
| after, loadErr := core.LoadMultiAppConfig() | ||
| if loadErr != nil { | ||
| t.Fatal(loadErr) | ||
| } | ||
| if !reflect.DeepEqual(after, original) { | ||
| t.Fatalf("config changed: got %#v, want %#v", after, original) | ||
| } | ||
| } | ||
|
|
||
| func restoreJSONResponse(body string) *http.Response { | ||
| return &http.Response{ | ||
| StatusCode: http.StatusOK, | ||
| Body: io.NopCloser(strings.NewReader(body)), | ||
| Header: make(http.Header), | ||
| } | ||
| } | ||
|
|
||
| func replaceRestoreDefaultTransport(t *testing.T, rt http.RoundTripper) { | ||
| t.Helper() | ||
| original := http.DefaultTransport | ||
| http.DefaultTransport = rt | ||
| t.Cleanup(func() { http.DefaultTransport = original }) | ||
| } |
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
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Report restore success instead of app creation.
The restore path updates an existing application.
msg.AppCreatedreports the wrong operation. Usemsg.ConfigSavedor add a restore-specific message. Capture stderr in the success test and assert that it does not report application creation.cmd/config/init_restore.go#L65-L68: replacemsg.AppCreatedwith a restore-appropriate success message.cmd/config/init_restore_test.go#L102-L108: capture stderr fromcmdutil.TestFactoryand assert the restore success message.As per coding guidelines: “Preserve established CLI behavior, tests, lint, CI, output contracts, and public APIs unless a breaking change is explicitly requested.”
📍 Affects 2 files
cmd/config/init_restore.go#L65-L68(this comment)cmd/config/init_restore_test.go#L102-L108🤖 Prompt for AI Agents
Source: Coding guidelines