diff --git a/api/dashboard/client.go b/api/dashboard/client.go index 46d77a5e..546473fb 100644 --- a/api/dashboard/client.go +++ b/api/dashboard/client.go @@ -539,39 +539,41 @@ var WriteACL = []string{ "settings", "editSettings", "recommendation", } -// CreateAPIKey creates a new API key with the given ACL for the specified application. +// CreateAPIKey creates a new API key with the given ACL for the specified +// application. It returns the API key value and its resource ID (UUID), the +// latter so callers can persist it for later rotation/revocation. func (c *Client) CreateAPIKey( accessToken, appID string, acl []string, description string, -) (string, error) { +) (key, uuid string, err error) { payload := CreateAPIKeyRequest{ACL: acl, Description: description} body, err := json.Marshal(payload) if err != nil { - return "", err + return "", "", err } endpoint := fmt.Sprintf("%s/1/applications/%s/api-keys", c.APIURL, url.PathEscape(appID)) req, err := http.NewRequest(http.MethodPost, endpoint, bytes.NewReader(body)) if err != nil { - return "", err + return "", "", err } c.setAPIHeaders(req, accessToken) req.Header.Set("Content-Type", "application/json") resp, err := c.client.Do(req) if err != nil { - return "", fmt.Errorf("create API key request failed: %w", err) + return "", "", fmt.Errorf("create API key request failed: %w", err) } defer resp.Body.Close() respBody, err := io.ReadAll(resp.Body) if err != nil { - return "", fmt.Errorf("failed to read API key response: %w", err) + return "", "", fmt.Errorf("failed to read API key response: %w", err) } if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - return "", fmt.Errorf( + return "", "", fmt.Errorf( "create API key failed with status %d: %s", resp.StatusCode, string(respBody), @@ -580,22 +582,22 @@ func (c *Client) CreateAPIKey( var keyResp CreateAPIKeyResponse if err := json.Unmarshal(respBody, &keyResp); err != nil { - return "", fmt.Errorf( + return "", "", fmt.Errorf( "failed to parse API key response: %w (body: %s)", err, string(respBody), ) } - key := keyResp.Data.Attributes.Value + key = keyResp.Data.Attributes.Value if key == "" { - return "", fmt.Errorf( + return "", "", fmt.Errorf( "API key creation succeeded but no key was returned in the response: %s", string(respBody), ) } - return key, nil + return key, keyResp.Data.ID, nil } // GetCrawlerUser gets the crawler API user data for the current authenticated user diff --git a/api/dashboard/types.go b/api/dashboard/types.go index da309f4d..fa681299 100644 --- a/api/dashboard/types.go +++ b/api/dashboard/types.go @@ -47,10 +47,11 @@ type ApplicationPlan struct { // Application is a flattened view of an Algolia application for CLI consumption. type Application struct { - ID string `json:"id"` - Name string `json:"name"` - APIKey string `json:"api_key,omitempty"` - PlanLabel string `json:"plan_label,omitempty"` // current plan label, e.g. "Grow Plus" + ID string `json:"id"` + Name string `json:"name"` + APIKey string `json:"api_key,omitempty"` + APIKeyUUID string `json:"-"` // resource ID of APIKey when freshly created; persisted as api_key_uuid + PlanLabel string `json:"plan_label,omitempty"` // current plan label, e.g. "Grow Plus" } // PaginationMeta contains page-based pagination metadata. diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index ad69d089..a770fa51 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -12,6 +12,7 @@ import ( "github.com/algolia/cli/pkg/cmd/shared/apputil" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/config" + "github.com/algolia/cli/pkg/config/state" "github.com/algolia/cli/pkg/iostreams" "github.com/algolia/cli/pkg/prompt" "github.com/algolia/cli/pkg/validators" @@ -122,12 +123,9 @@ func runSelectCmd(opts *SelectOptions) (*dashboard.Application, error) { return nil, err } - // If a profile already exists for this app, switch the default - // and ensure it has an API key. + // If a profile already exists for this app, switch the current + // application and ensure it has an API key. if exists, profileName := opts.Config.ApplicationIDExists(chosen.ID); exists { - // Read the profile BEFORE SetDefaultProfile, because viper.Set() calls - // inside SetDefaultProfile pollute the override map and cause - // UnmarshalKey to return empty fields (known viper issue). var existingProfile *config.Profile for _, p := range opts.Config.ConfiguredProfiles() { if p.Name == profileName { @@ -142,13 +140,15 @@ func runSelectCmd(opts *SelectOptions) (*dashboard.Application, error) { fmt.Fprintf(opts.IO.Out, "%s Switched to profile %q (application %s).\n", cs.SuccessIcon(), profileName, cs.Bold(chosen.ID)) - if existingProfile != nil && existingProfile.APIKey == "" { + storedKey, _ := state.GetSecret(chosen.ID, state.SecretAPIKey) + if existingProfile != nil && storedKey == "" { app := &dashboard.Application{ID: chosen.ID, Name: chosen.Name} if err := apputil.EnsureAPIKey(opts.IO, client, accessToken, app); err != nil { return nil, err } existingProfile.ApplicationID = chosen.ID existingProfile.APIKey = app.APIKey + existingProfile.APIKeyUUID = app.APIKeyUUID if err := existingProfile.Add(); err != nil { return nil, err } diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 7fdb1461..d44f5170 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -13,6 +13,7 @@ import ( "github.com/algolia/cli/pkg/cmd/shared/apputil" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/config" + "github.com/algolia/cli/pkg/config/state" "github.com/algolia/cli/pkg/iostreams" "github.com/algolia/cli/pkg/prompt" "github.com/algolia/cli/pkg/telemetry" @@ -169,13 +170,22 @@ func applyStoredIdentity(ctx context.Context) bool { } // reuseExistingAPIKey checks if a local profile already has an API key for -// the given application. If so, it sets app.APIKey and returns true. +// the given application. If so, it sets app.APIKey and returns true. The key +// itself lives in the OS keychain; the profile only carries metadata. func reuseExistingAPIKey(cfg config.IConfig, app *dashboard.Application) bool { for _, p := range cfg.ConfiguredProfiles() { - if p.ApplicationID == app.ID && p.APIKey != "" { + if p.ApplicationID != app.ID { + continue + } + // In-memory profiles (tests/legacy) may already hold the key. + if p.APIKey != "" { app.APIKey = p.APIKey return true } + if key, err := state.GetSecret(app.ID, state.SecretAPIKey); err == nil && key != "" { + app.APIKey = key + return true + } } return false } diff --git a/pkg/cmd/profile/add/add.go b/pkg/cmd/profile/add/add.go index bab975d3..e4262bfc 100644 --- a/pkg/cmd/profile/add/add.go +++ b/pkg/cmd/profile/add/add.go @@ -82,7 +82,7 @@ func NewAddCmd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command cmd := &cobra.Command{ Use: "add", Args: validators.NoArgs(), - Short: "Add a new profile configuration to the CLI", + Short: "(deprecated) Add a new profile configuration to the CLI", Example: heredoc.Doc(` # Add a new profile (interactive) $ algolia profile add diff --git a/pkg/cmd/profile/application.go b/pkg/cmd/profile/application.go index f3f44c85..24a920c9 100644 --- a/pkg/cmd/profile/application.go +++ b/pkg/cmd/profile/application.go @@ -1,6 +1,7 @@ package profile import ( + "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" "github.com/algolia/cli/pkg/auth" @@ -16,7 +17,21 @@ func NewProfileCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "profile", Aliases: []string{"profiles"}, - Short: "Manage your Algolia CLI profiles", + Short: "(deprecated) Manage your Algolia CLI profiles", + Long: heredoc.Doc(` + Manage your Algolia CLI profiles. + + These commands are deprecated. Credentials now live in state.toml + (non-secrets) and the OS keychain (secrets), managed by: + + - algolia auth login sign in and configure an application + - algolia application list list applications, marking configured ones + - algolia application select switch the active application + + Existing profiles keep working and remain resolvable as aliases via + the deprecated --profile flag until the next major version. + `), + Deprecated: "use `algolia auth login`, `algolia application list`, and `algolia application select` instead. Profiles still resolve as aliases until the next major version.", } auth.DisableAuthCheck(cmd) diff --git a/pkg/cmd/profile/list/list.go b/pkg/cmd/profile/list/list.go index c9e49c6a..4ef3683c 100644 --- a/pkg/cmd/profile/list/list.go +++ b/pkg/cmd/profile/list/list.go @@ -11,6 +11,7 @@ import ( "github.com/algolia/cli/pkg/cmd/factory" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/config" + "github.com/algolia/cli/pkg/config/state" "github.com/algolia/cli/pkg/iostreams" "github.com/algolia/cli/pkg/printers" "github.com/algolia/cli/pkg/validators" @@ -32,7 +33,7 @@ func NewListCmd(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Use: "list", Aliases: []string{"l"}, Args: validators.NoArgs(), - Short: "List the configured profile(s)", + Short: "(deprecated) List the configured profile(s)", Example: heredoc.Doc(` # List the configured profiles $ algolia profile list @@ -75,6 +76,9 @@ func runListCmd(opts *ListOptions) error { table.AddField(profile.ApplicationID, nil, nil) apiKey := profile.APIKey + if apiKey == "" { + apiKey, _ = state.GetSecret(profile.ApplicationID, state.SecretAPIKey) + } if apiKey == "" { apiKey = profile.AdminAPIKey // Legacy } diff --git a/pkg/cmd/profile/remove/remove.go b/pkg/cmd/profile/remove/remove.go index 094a03f0..73fb49e7 100644 --- a/pkg/cmd/profile/remove/remove.go +++ b/pkg/cmd/profile/remove/remove.go @@ -37,7 +37,7 @@ func NewRemoveCmd(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co Use: "remove ", Args: validators.ExactArgs(1), ValidArgsFunction: cmdutil.ConfiguredProfilesCompletionFunc(f), - Short: "Remove the specified profile", + Short: "(deprecated) Remove the specified profile", Long: `Remove the specified profile from the configuration.`, Example: heredoc.Doc(` # Remove the profile named "my-app" from the configuration diff --git a/pkg/cmd/profile/setdefault/setdefault.go b/pkg/cmd/profile/setdefault/setdefault.go index db27c597..9dc966b5 100644 --- a/pkg/cmd/profile/setdefault/setdefault.go +++ b/pkg/cmd/profile/setdefault/setdefault.go @@ -30,7 +30,7 @@ func NewSetDefaultCmd(f *cmdutil.Factory, runF func(*SetDefaultOptions) error) * Use: "setdefault ", Args: validators.ExactArgs(1), ValidArgsFunction: cmdutil.ConfiguredProfilesCompletionFunc(f), - Short: "Set the default profile", + Short: "(deprecated) Set the default profile", Example: heredoc.Doc(` # Set the default profile to "my-app" $ algolia profile setdefault my-app diff --git a/pkg/cmd/shared/apputil/create.go b/pkg/cmd/shared/apputil/create.go index a5d217ee..5fe82f56 100644 --- a/pkg/cmd/shared/apputil/create.go +++ b/pkg/cmd/shared/apputil/create.go @@ -129,13 +129,14 @@ func EnsureAPIKey( ) error { cs := io.ColorScheme() io.StartProgressIndicatorWithLabel("Generating API key") - apiKey, err := client.CreateAPIKey(accessToken, app.ID, dashboard.WriteACL, "Algolia CLI") + apiKey, apiKeyUUID, err := client.CreateAPIKey(accessToken, app.ID, dashboard.WriteACL, "Algolia CLI") io.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to generate API key: %w", err) } app.APIKey = apiKey + app.APIKeyUUID = apiKeyUUID fmt.Fprintf(io.Out, "%s API key generated for application %s\n", cs.SuccessIcon(), cs.Bold(app.ID)) return nil @@ -168,6 +169,7 @@ func ConfigureProfile( Name: profileName, ApplicationID: appDetails.ID, APIKey: appDetails.APIKey, + APIKeyUUID: appDetails.APIKeyUUID, Default: setDefault, } diff --git a/pkg/config/config.go b/pkg/config/config.go index 28dbe4ea..d2dbf3f7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,18 +1,16 @@ package config import ( - "bytes" "fmt" "os" "path/filepath" + "sort" - "github.com/BurntSushi/toml" "github.com/mitchellh/go-homedir" log "github.com/sirupsen/logrus" "github.com/spf13/viper" "github.com/algolia/cli/pkg/config/state" - "github.com/algolia/cli/pkg/utils" ) type IConfig interface { @@ -88,21 +86,73 @@ func (c *Config) GetConfigFolder(xdgPath string) string { return filepath.Join(configPath, "algolia") } -// ConfiguredProfiles return the profiles in the configuration file +// loadState loads state.toml for read operations. When no state path is +// configured (e.g. a bare Config in unit tests) it returns an empty state so +// reads stay deterministic and never touch a real home directory. +func (c *Config) loadState() *state.State { + if c.CurrentProfile.statePath == "" { + return state.New() + } + s, err := state.Load(c.CurrentProfile.statePath) + if err != nil { + return state.New() + } + return s +} + +// writeStatePath returns the path used for state writes, defaulting to the +// well-known location when not explicitly configured (always set in production +// by InitConfig). +func (c *Config) writeStatePath() string { + if c.CurrentProfile.statePath != "" { + return c.CurrentProfile.statePath + } + return state.DefaultPath() +} + +// profileName returns the alias for an application, falling back to its ID. +func profileName(app *state.ApplicationState) string { + if app.Alias != "" { + return app.Alias + } + return app.ApplicationID +} + +// resolveApp resolves a profile name to its application state, matching the +// stored alias first and then the application ID. +func (c *Config) resolveApp(st *state.State, name string) *state.ApplicationState { + if app := st.AppByAlias(name); app != nil { + return app + } + return st.App(name) +} + +// ConfiguredProfiles returns the configured applications from state.toml. Only +// non-secret metadata is populated; secrets stay in the keychain and are read +// lazily by callers that need them. func (c *Config) ConfiguredProfiles() []*Profile { - configs := viper.AllSettings() - applications := make([]*Profile, 0, len(configs)) - for appName := range configs { - app := &Profile{ - Name: appName, - } - if err := viper.UnmarshalKey(appName, app); err != nil { - log.Fatalf("%s", err) - } - applications = append(applications, app) + s := c.loadState() + + appIDs := make([]string, 0, len(s.Applications)) + for appID := range s.Applications { + appIDs = append(appIDs, appID) + } + sort.Strings(appIDs) + + profiles := make([]*Profile, 0, len(appIDs)) + for _, appID := range appIDs { + app := s.Applications[appID] + profiles = append(profiles, &Profile{ + Name: profileName(app), + ApplicationID: app.ApplicationID, + APIKeyUUID: app.APIKeyUUID, + SearchHosts: app.SearchHosts, + Default: appID == s.CurrentApplicationID, + statePath: c.CurrentProfile.statePath, + }) } - return applications + return profiles } // Profile returns the current profile @@ -110,7 +160,7 @@ func (c *Config) Profile() *Profile { return &c.CurrentProfile } -// Default returns the default profile +// Default returns the default (current) profile func (c *Config) Default() *Profile { for _, profile := range c.ConfiguredProfiles() { if profile.Default { @@ -120,136 +170,99 @@ func (c *Config) Default() *Profile { return nil } -// ProfileNames returns the list of name of the configured profiles +// ProfileNames returns the aliases of the configured profiles. func (c *Config) ProfileNames() []string { - return viper.AllKeys() + profiles := c.ConfiguredProfiles() + names := make([]string, 0, len(profiles)) + for _, profile := range profiles { + names = append(names, profile.Name) + } + return names } -// ProfileExists check if a profile with the given name exists -func (c *Config) ProfileExists(appName string) bool { - return viper.IsSet(appName) +// ProfileExists checks whether a profile with the given name (alias or +// application ID) exists. +func (c *Config) ProfileExists(name string) bool { + return c.resolveApp(c.loadState(), name) != nil } -// RemoveProfile remove a profile from the configuration +// RemoveProfile removes a profile from state.toml and deletes its secrets from +// the keychain. func (c *Config) RemoveProfile(name string) error { - runtimeViper := viper.GetViper() - configMap := runtimeViper.AllSettings() - delete(configMap, name) - - buf := new(bytes.Buffer) - - encodeErr := toml.NewEncoder(buf).Encode(configMap) - if encodeErr != nil { - return encodeErr + path := c.writeStatePath() + s, err := state.Load(path) + if err != nil { + return err } - nv := viper.New() - nv.SetConfigType("toml") // hint to viper that we've encoded the data as toml + app := c.resolveApp(s, name) + if app == nil { + return fmt.Errorf("profile '%s' not found", name) + } + appID := app.ApplicationID - err := nv.ReadConfig(buf) - if err != nil { + s.RemoveApp(appID) + if err := s.Save(path); err != nil { return err } - return c.write(nv) + _ = state.DeleteSecret(appID, state.SecretAPIKey) + _ = state.DeleteSecret(appID, state.SecretCrawlerAPIKey) + return nil } -// SetDefaultProfile set the default profile +// SetDefaultProfile marks the named profile's application as the current one. func (c *Config) SetDefaultProfile(name string) error { - configuration, err := c.read() + path := c.writeStatePath() + s, err := state.Load(path) if err != nil { return err } - configs := configuration.AllSettings() - - found := false - - for profileName := range configs { - runtimeViper := viper.GetViper() - runtimeViper.Set(profileName+".default", false) - - if profileName == name { - found = true - runtimeViper.Set(profileName+".default", true) - } - } - - if !found { + app := c.resolveApp(s, name) + if app == nil { return fmt.Errorf("profile '%s' not found", name) } - return c.write(configuration) + s.SetCurrent(app.ApplicationID) + return s.Save(path) } -// ApplicationIDExists check if an application ID exists in any profiles +// ApplicationIDExists checks whether an application ID is configured. func (c *Config) ApplicationIDExists(appID string) (bool, string) { - for _, profile := range c.ConfiguredProfiles() { - if profile.ApplicationID == appID { - return true, profile.Name - } + if app := c.loadState().App(appID); app != nil { + return true, profileName(app) } - return false, "" } // ApplicationIDForProfile returns the application ID for a given profile name. -func (c *Config) ApplicationIDForProfile(profileName string) (bool, string) { - for _, profile := range c.ConfiguredProfiles() { - if profile.Name == profileName { - return true, profile.ApplicationID - } +func (c *Config) ApplicationIDForProfile(name string) (bool, string) { + if app := c.resolveApp(c.loadState(), name); app != nil { + return true, app.ApplicationID } - return false, "" } -// SetCrawlerAuth sets the config properties for crawler public api +// SetCrawlerAuth stores the crawler user ID in state.toml and the crawler API +// key in the keychain for the named profile. func (c *Config) SetCrawlerAuth(profile, crawlerUserID, crawlerAPIKey string) error { - configuration, err := c.read() + path := c.writeStatePath() + s, err := state.Load(path) if err != nil { return err } - profiles := configuration.AllSettings() - - if _, exists := profiles[profile]; !exists { + app := c.resolveApp(s, profile) + if app == nil { return fmt.Errorf("profile '%s' not found", profile) } - configuration.Set(profile+".crawler_user_id", crawlerUserID) - configuration.Set(profile+".crawler_api_key", crawlerAPIKey) - - return c.write(configuration) -} - -// read reads the configuration file and returns its runtime -func (c *Config) read() (*viper.Viper, error) { - runtimeViper := viper.GetViper() - - runtimeViper.SetConfigType("toml") - err := runtimeViper.ReadInConfig() - if err != nil { - return nil, err - } - - return runtimeViper, nil -} - -// write writes the configuration file -func (c *Config) write(runtimeViper *viper.Viper) error { - configFile := viper.ConfigFileUsed() - err := utils.MakePath(configFile) - if err != nil { - return err - } - runtimeViper.SetConfigFile(configFile) - runtimeViper.SetConfigType(filepath.Ext(configFile)) - - err = runtimeViper.WriteConfig() - if err != nil { + app.CrawlerUserID = crawlerUserID + s.SetApp(app) + if err := s.Save(path); err != nil { return err } - return nil + return state.SetSecret(app.ApplicationID, state.SecretCrawlerAPIKey, crawlerAPIKey) } diff --git a/pkg/config/config_store_test.go b/pkg/config/config_store_test.go new file mode 100644 index 00000000..57296c2d --- /dev/null +++ b/pkg/config/config_store_test.go @@ -0,0 +1,169 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/pkg/config/state" +) + +// newStateConfig returns a Config whose current profile points at statePath, +// mirroring what InitConfig does in production. +func newStateConfig(statePath string) *Config { + c := &Config{} + c.CurrentProfile.statePath = statePath + return c +} + +func TestConfig_ConfiguredProfiles_FromState(t *testing.T) { + keyring.MockInit() + + statePath := seedState( + t, + "APP_B", + &state.ApplicationState{ + ApplicationID: "APP_A", + Alias: "alpha", + SearchHosts: []string{"h1"}, + }, + &state.ApplicationState{ApplicationID: "APP_B", Alias: "beta"}, + &state.ApplicationState{ApplicationID: "APP_C"}, // no alias -> name falls back to ID + ) + + cfg := newStateConfig(statePath) + profiles := cfg.ConfiguredProfiles() + require.Len(t, profiles, 3) + + // Deterministic order: sorted by application ID. + assert.Equal(t, "alpha", profiles[0].Name) + assert.Equal(t, "APP_A", profiles[0].ApplicationID) + assert.Equal(t, []string{"h1"}, profiles[0].SearchHosts) + assert.False(t, profiles[0].Default) + + assert.Equal(t, "beta", profiles[1].Name) + assert.True(t, profiles[1].Default, "current application is the default profile") + + assert.Equal(t, "APP_C", profiles[2].Name, "missing alias falls back to application ID") + + // Secrets are never populated by ConfiguredProfiles. + for _, p := range profiles { + assert.Empty(t, p.APIKey) + } +} + +func TestConfig_ConfiguredProfiles_EmptyWhenNoStatePath(t *testing.T) { + cfg := &Config{} // bare config, as in unit tests without InitConfig + assert.Empty(t, cfg.ConfiguredProfiles()) + assert.Nil(t, cfg.Default()) + assert.Empty(t, cfg.ProfileNames()) + assert.False(t, cfg.ProfileExists("anything")) +} + +func TestConfig_SetDefaultProfile_ByAliasAndID(t *testing.T) { + keyring.MockInit() + + statePath := seedState(t, "APP_A", + &state.ApplicationState{ApplicationID: "APP_A", Alias: "alpha"}, + &state.ApplicationState{ApplicationID: "APP_B", Alias: "beta"}, + ) + cfg := newStateConfig(statePath) + + // Resolve by alias. + require.NoError(t, cfg.SetDefaultProfile("beta")) + assert.Equal(t, "beta", cfg.Default().Name) + + // Resolve by application ID. + require.NoError(t, cfg.SetDefaultProfile("APP_A")) + assert.Equal(t, "alpha", cfg.Default().Name) + + // Unknown profile errors. + assert.Error(t, cfg.SetDefaultProfile("nope")) +} + +func TestConfig_RemoveProfile_DeletesStateAndSecrets(t *testing.T) { + keyring.MockInit() + + statePath := seedState(t, "APP_A", + &state.ApplicationState{ApplicationID: "APP_A", Alias: "alpha"}, + &state.ApplicationState{ApplicationID: "APP_B", Alias: "beta"}, + ) + require.NoError(t, state.SetSecret("APP_A", state.SecretAPIKey, "key-a")) + require.NoError(t, state.SetSecret("APP_A", state.SecretCrawlerAPIKey, "crawler-a")) + + cfg := newStateConfig(statePath) + + require.NoError(t, cfg.RemoveProfile("alpha")) + + // State entry removed and current pointer cleared (it referenced APP_A). + profiles := cfg.ConfiguredProfiles() + require.Len(t, profiles, 1) + assert.Equal(t, "beta", profiles[0].Name) + assert.Nil(t, cfg.Default(), "removing the current app clears the default") + + // Secrets are deleted from the keychain. + apiKey, err := state.GetSecret("APP_A", state.SecretAPIKey) + require.NoError(t, err) + assert.Empty(t, apiKey) + crawlerKey, err := state.GetSecret("APP_A", state.SecretCrawlerAPIKey) + require.NoError(t, err) + assert.Empty(t, crawlerKey) + + // Removing an unknown profile errors. + assert.Error(t, cfg.RemoveProfile("nope")) +} + +func TestConfig_ApplicationIDLookups(t *testing.T) { + keyring.MockInit() + + statePath := seedState(t, "APP_A", + &state.ApplicationState{ApplicationID: "APP_A", Alias: "alpha"}, + ) + cfg := newStateConfig(statePath) + + exists, name := cfg.ApplicationIDExists("APP_A") + assert.True(t, exists) + assert.Equal(t, "alpha", name) + + exists, _ = cfg.ApplicationIDExists("UNKNOWN") + assert.False(t, exists) + + exists, appID := cfg.ApplicationIDForProfile("alpha") + assert.True(t, exists) + assert.Equal(t, "APP_A", appID) + + // Resolvable by application ID too. + exists, appID = cfg.ApplicationIDForProfile("APP_A") + assert.True(t, exists) + assert.Equal(t, "APP_A", appID) + + assert.True(t, cfg.ProfileExists("alpha")) + assert.True(t, cfg.ProfileExists("APP_A")) + assert.False(t, cfg.ProfileExists("nope")) +} + +func TestConfig_SetCrawlerAuth_StateAndKeychain(t *testing.T) { + keyring.MockInit() + + statePath := seedState(t, "APP_A", + &state.ApplicationState{ApplicationID: "APP_A", Alias: "alpha"}, + ) + cfg := newStateConfig(statePath) + + require.NoError(t, cfg.SetCrawlerAuth("alpha", "crawler-user", "crawler-key")) + + // Non-secret crawler_user_id lands in state.toml. + st, err := state.Load(statePath) + require.NoError(t, err) + assert.Equal(t, "crawler-user", st.App("APP_A").CrawlerUserID) + + // The crawler API key lands in the keychain. + key, err := state.GetSecret("APP_A", state.SecretCrawlerAPIKey) + require.NoError(t, err) + assert.Equal(t, "crawler-key", key) + + // Unknown profile errors. + assert.Error(t, cfg.SetCrawlerAuth("nope", "u", "k")) +} diff --git a/pkg/config/profile.go b/pkg/config/profile.go index a0bd8d57..26140cfe 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -2,13 +2,11 @@ package config import ( "os" - "path/filepath" "strings" "github.com/spf13/viper" "github.com/algolia/cli/pkg/config/state" - "github.com/algolia/cli/pkg/utils" ) // DefaultSearchHosts can be set at build time via ldflags, e.g. @@ -23,11 +21,16 @@ type Profile struct { AdminAPIKey string `mapstructure:"admin_api_key"` SearchHosts []string `mapstructure:"search_hosts"` + // APIKeyUUID is the resource ID of the API key stored in the keychain. + // It is persisted to state.toml (not config.toml) so the key can later + // be rotated/revoked without re-reading the secret. + APIKeyUUID string `mapstructure:"-"` + Default bool `mapstructure:"default"` // statePath points at state.toml; it is set by Config.InitConfig. When - // empty (e.g. in unit tests) credential resolution falls back to the - // legacy config.toml behavior. + // empty (e.g. in unit tests) it falls back to state.DefaultPath() for + // writes and disables state-based reads. statePath string } @@ -234,29 +237,55 @@ func (p *Profile) GetCrawlerAPIKey() (string, error) { return "", ErrCrawlerAPIKeyNotConfigured } -// Add adds a profile to the configuration, preserving any existing profiles. +// Add persists the profile to the new store: non-secret metadata (application +// ID, alias, search hosts, API key UUID) goes to state.toml and the API key +// itself goes to the OS keychain. config.toml is no longer written. +// +// Existing non-secret fields for the application (e.g. crawler_user_id) are +// preserved. The profile becomes the current application when it is flagged as +// default, or when no application is current yet (so the first configured +// application is immediately usable). func (p *Profile) Add() error { - runtimeViper := viper.GetViper() - runtimeViper.Set(p.GetFieldName("application_id"), p.ApplicationID) - runtimeViper.Set(p.GetFieldName("api_key"), p.APIKey) + if p.ApplicationID == "" { + return ErrApplicationIDNotConfigured + } - return p.write(runtimeViper) -} + path := p.statePath + if path == "" { + path = state.DefaultPath() + } -// write writes the configuration file -func (p *Profile) write(runtimeViper *viper.Viper) error { - configFile := viper.ConfigFileUsed() - err := utils.MakePath(configFile) + s, err := state.Load(path) if err != nil { return err } - runtimeViper.SetConfigFile(configFile) - runtimeViper.SetConfigType(filepath.Ext(configFile)) - err = runtimeViper.WriteConfig() - if err != nil { + app := s.App(p.ApplicationID) + if app == nil { + app = &state.ApplicationState{ApplicationID: p.ApplicationID} + } + if p.Name != "" { + app.Alias = p.Name + } + if len(p.SearchHosts) > 0 { + app.SearchHosts = p.SearchHosts + } + if p.APIKeyUUID != "" { + app.APIKeyUUID = p.APIKeyUUID + } + s.SetApp(app) + + if p.Default || s.CurrentApplicationID == "" { + s.SetCurrent(p.ApplicationID) + } + + if err := s.Save(path); err != nil { return err } + if p.APIKey != "" { + return state.SetSecret(p.ApplicationID, state.SecretAPIKey, p.APIKey) + } + return nil } diff --git a/pkg/config/profile_test.go b/pkg/config/profile_test.go index 18cbaeb0..55f2b2b2 100644 --- a/pkg/config/profile_test.go +++ b/pkg/config/profile_test.go @@ -180,33 +180,35 @@ default = true assert.Equal(t, "key-cfg", apiKey) } -func TestAddProfile_PreservesExistingProfiles(t *testing.T) { - configFile := filepath.Join(t.TempDir(), "config.toml") +func TestAddProfile_WritesStateAndKeychain(t *testing.T) { + keyring.MockInit() + viper.Reset() - // First CLI invocation: add profile A. - initTestViper(configFile) + statePath := filepath.Join(t.TempDir(), "state.toml") + // First CLI invocation: add profile A. With no current app yet, it + // becomes the current application. profileA := &Profile{ + statePath: statePath, Name: "app-a", ApplicationID: "APP_A_ID", APIKey: "key-a", + APIKeyUUID: "uuid-a", } require.NoError(t, profileA.Add()) - // Second CLI invocation: add profile B. - initTestViper(configFile) - + // Second CLI invocation: add profile B (not default), preserving A. profileB := &Profile{ + statePath: statePath, Name: "app-b", ApplicationID: "APP_B_ID", APIKey: "key-b", } require.NoError(t, profileB.Add()) - // Third CLI invocation: read back and verify both profiles exist. - initTestViper(configFile) - + // Read back via Config pointed at the same state.toml. cfg := &Config{} + cfg.CurrentProfile.statePath = statePath profiles := cfg.ConfiguredProfiles() profilesByName := make(map[string]*Profile) @@ -217,4 +219,20 @@ func TestAddProfile_PreservesExistingProfiles(t *testing.T) { assert.Len(t, profiles, 2, "both profiles should be preserved on disk") assert.Equal(t, "APP_A_ID", profilesByName["app-a"].ApplicationID) assert.Equal(t, "APP_B_ID", profilesByName["app-b"].ApplicationID) + assert.Equal(t, "uuid-a", profilesByName["app-a"].APIKeyUUID) + + // The first application added becomes the current (default) one. + assert.True(t, profilesByName["app-a"].Default) + assert.False(t, profilesByName["app-b"].Default) + + // config.toml is never written; secrets live in the keychain only. + keyA, err := state.GetSecret("APP_A_ID", state.SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "key-a", keyA) + keyB, err := state.GetSecret("APP_B_ID", state.SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "key-b", keyB) + + // ConfiguredProfiles must not leak secrets into the profile metadata. + assert.Empty(t, profilesByName["app-a"].APIKey) } diff --git a/pkg/config/state/state.go b/pkg/config/state/state.go index ad812017..6a05f3b9 100644 --- a/pkg/config/state/state.go +++ b/pkg/config/state/state.go @@ -181,6 +181,18 @@ func (s *State) SetApp(app *ApplicationState) { s.Applications[app.ApplicationID] = app } +// RemoveApp deletes the application entry for appID and clears the current +// pointer when it referenced that application. +func (s *State) RemoveApp(appID string) { + if s == nil || appID == "" { + return + } + delete(s.Applications, appID) + if s.CurrentApplicationID == appID { + s.CurrentApplicationID = "" + } +} + // PutAPIKeyUUID records the keychain API key UUID for appID, creating the // entry when missing. func (s *State) PutAPIKeyUUID(appID, uuid string) {