From 178d188787c0fe89a6ba30497d1757f0a6e8283f Mon Sep 17 00:00:00 2001 From: Arnstein Ressem Date: Wed, 20 Apr 2022 00:11:50 +0200 Subject: Revert "Allow reading API key from default path when Auth0 is not configured" --- client/go/cmd/config.go | 24 +++++++++++++++++------- client/go/cmd/config_test.go | 39 ++++++++++++--------------------------- client/go/cmd/root.go | 8 +++++--- 3 files changed, 34 insertions(+), 37 deletions(-) diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go index 02477aecf28..726676ce476 100644 --- a/client/go/cmd/config.go +++ b/client/go/cmd/config.go @@ -20,6 +20,7 @@ import ( "github.com/spf13/pflag" "github.com/vespa-engine/vespa/client/go/auth/auth0" "github.com/vespa-engine/vespa/client/go/config" + "github.com/vespa-engine/vespa/client/go/util" "github.com/vespa-engine/vespa/client/go/vespa" ) @@ -432,21 +433,30 @@ func (c *Config) authConfigPath() string { return filepath.Join(c.homeDir, "auth.json") } -func (c *Config) readAPIKey(cli *CLI, system vespa.System, tenantName string) ([]byte, error) { +func (c *Config) readAPIKey(tenantName string) ([]byte, error) { if override, ok := c.apiKeyFromEnv(); ok { return override, nil } - if path, ok := c.apiKeyFileFromEnv(); ok { - return os.ReadFile(path) + return os.ReadFile(c.apiKeyPath(tenantName)) +} + +// useAPIKey returns true if an API key should be used when authenticating with system. +func (c *Config) useAPIKey(cli *CLI, system vespa.System, tenantName string) bool { + if _, ok := c.apiKeyFromEnv(); ok { + return true + } + if _, ok := c.apiKeyFileFromEnv(); ok { + return true } if !cli.isCI() { + // Fall back to API key, if present and Auth0 has not been configured client, err := auth0.New(c.authConfigPath(), system.Name, system.URL) - if err == nil && client.HasCredentials() { - return nil, nil // use Auth0 + if err != nil || !client.HasCredentials() { + cli.printWarning("Regular authentication is preferred over API key in a non-CI context", "Authenticate with 'vespa auth login'") + return util.PathExists(c.apiKeyPath(tenantName)) } - cli.printWarning("Authenticating with API key. This is discouraged in non-CI environments", "Authenticate with 'vespa auth login'") } - return os.ReadFile(c.apiKeyPath(tenantName)) + return false } func (c *Config) readSessionID(app vespa.ApplicationID) (int64, error) { diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go index 9f41ef46914..f89e752f82d 100644 --- a/client/go/cmd/config_test.go +++ b/client/go/cmd/config_test.go @@ -145,33 +145,17 @@ func assertConfigCommandErr(t *testing.T, configHome, expected string, args ...s assert.NotNil(t, assertConfigCommandStdErr(t, configHome, expected, args...)) } -func TestReadAPIKey(t *testing.T) { +func TestUseAPIKey(t *testing.T) { cli, _, _ := newTestCLI(t) - key, err := cli.config.readAPIKey(cli, vespa.PublicSystem, "t1") - assert.Nil(t, key) - require.NotNil(t, err) + assert.False(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t1")) - // From default path when it exists - require.Nil(t, os.WriteFile(filepath.Join(cli.config.homeDir, "t1.api-key.pem"), []byte("foo"), 0600)) - key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1") - require.Nil(t, err) - assert.Equal(t, []byte("foo"), key) - - // From file specified in environment - keyFile := filepath.Join(t.TempDir(), "key") - require.Nil(t, os.WriteFile(keyFile, []byte("bar"), 0600)) - cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY_FILE="+keyFile) - key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1") - require.Nil(t, err) - assert.Equal(t, []byte("bar"), key) + cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY_FILE=/tmp/foo") + assert.True(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t1")) - // From key specified in environment - cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY=baz") - key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1") - require.Nil(t, err) - assert.Equal(t, []byte("baz"), key) + cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY=foo") + assert.True(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t1")) - // Auth0 is preferred when configured + // Prefer Auth0, if configured authContent := ` { "version": 1, @@ -188,9 +172,10 @@ func TestReadAPIKey(t *testing.T) { } } }` - cli, _, _ = newTestCLI(t) - require.Nil(t, os.WriteFile(filepath.Join(cli.config.homeDir, "auth.json"), []byte(authContent), 0600)) - key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1") + cli, _, _ = newTestCLI(t, "VESPA_CLI_CLOUD_SYSTEM=public") + _, err := os.Create(filepath.Join(cli.config.homeDir, "t2.api-key.pem")) require.Nil(t, err) - assert.Equal(t, []byte(nil), key) + assert.True(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t2")) + require.Nil(t, os.WriteFile(filepath.Join(cli.config.homeDir, "auth.json"), []byte(authContent), 0600)) + assert.False(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t2")) } diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go index 0557248cedf..e88398a7fde 100644 --- a/client/go/cmd/root.go +++ b/client/go/cmd/root.go @@ -324,9 +324,11 @@ func (c *CLI) createCloudTarget(targetType string, opts targetOptions) (vespa.Ta ) switch targetType { case vespa.TargetCloud: - apiKey, err = c.config.readAPIKey(c, system, deployment.Application.Tenant) - if err != nil { - return nil, err + if c.config.useAPIKey(c, system, deployment.Application.Tenant) { + apiKey, err = c.config.readAPIKey(deployment.Application.Tenant) + if err != nil { + return nil, err + } } authConfigPath = c.config.authConfigPath() deploymentTLSOptions = vespa.TLSOptions{} -- cgit v1.2.3