DXCDT-301: Storing client secret in keyring (2/2) (#578)

Store client secret in keyring

Co-authored-by: Sergiu Ghitea <[email protected]>

internal/cli/cli.go

@@ -54,7 +54,6 @@ type Tenant struct {
 	Apps         map[string]app `json:"apps,omitempty"`
 	DefaultAppID string         `json:"default_app_id,omitempty"`
 	ClientID     string         `json:"client_id"`
-	ClientSecret string         `json:"client_secret"`
 }
 
 type app struct {
@@ -97,11 +96,11 @@ type cli struct {
 }
 
 func (t *Tenant) authenticatedWithClientCredentials() bool {
-	return t.ClientID != "" && t.ClientSecret != ""
+	return t.ClientID != ""
 }
 
 func (t *Tenant) authenticatedWithDeviceCodeFlow() bool {
-	return t.ClientID == "" && t.ClientSecret == ""
+	return t.ClientID == ""
 }
 
 func (t *Tenant) hasExpiredToken() bool {
@@ -131,11 +130,16 @@ func (t *Tenant) additionalRequestedScopes() []string {
 
 func (t *Tenant) regenerateAccessToken(ctx context.Context, c *cli) error {
 	if t.authenticatedWithClientCredentials() {
+		clientSecret, err := keyring.GetClientSecret(t.Domain)
+		if err != nil {
+			return fmt.Errorf("failed to retrieve client secret from keyring: %w", err)
+		}
+
 		token, err := auth.GetAccessTokenFromClientCreds(
 			ctx,
 			auth.ClientCredentials{
 				ClientID:     t.ClientID,
-				ClientSecret: t.ClientSecret,
+				ClientSecret: clientSecret,
 				Domain:       t.Domain,
 			},
 		)
@@ -242,12 +246,16 @@ func (c *cli) prepareTenant(ctx context.Context) (Tenant, error) {
 
 	if err := t.regenerateAccessToken(ctx, c); err != nil {
 		if t.authenticatedWithClientCredentials() {
-			return t, fmt.Errorf(
-				"failed to fetch access token using client credentials.\n\n"+
-					"This may occur if the designated application has been deleted or the client secret has been rotated.\n\n"+
+			errorMessage := fmt.Errorf(
+				"failed to fetch access token using client credentials: %w\n\n"+
+					"This may occur if the designated Auth0 application has been deleted, "+
+					"the client secret has been rotated or previous failure to store client secret in the keyring.\n\n"+
 					"Please re-authenticate by running: %s",
+				err,
 				ansi.Bold("auth0 login --domain <tenant-domain --client-id <client-id> --client-secret <client-secret>"),
 			)
+
+			return t, errorMessage
 		}
 
 		c.renderer.Warnf("Failed to renew access token: %s", err)

internal/cli/login.go

@@ -266,15 +266,19 @@ func RunLoginAsMachine(ctx context.Context, inputs LoginInputs, cli *cli, cmd *c
 				"Ensure that the provided client-id, client-secret and domain are correct. \n\nerror: %w\n", err)
 	}
 
+	if err = keyring.StoreClientSecret(inputs.Domain, inputs.ClientSecret); err != nil {
+		cli.renderer.Warnf("Could not store the client secret to the keyring: %s", err)
+		cli.renderer.Warnf("Expect to login again when your access token expires.")
+	}
+
 	t := Tenant{
-		Domain:       inputs.Domain,
-		AccessToken:  token.AccessToken,
-		ExpiresAt:    token.ExpiresAt,
-		ClientID:     inputs.ClientID,
-		ClientSecret: inputs.ClientSecret,
+		Domain:      inputs.Domain,
+		AccessToken: token.AccessToken,
+		ExpiresAt:   token.ExpiresAt,
+		ClientID:    inputs.ClientID,
 	}
 
-	if err := cli.addTenant(t); err != nil {
+	if err = cli.addTenant(t); err != nil {
 		return fmt.Errorf("unexpected error when attempting to save tenant data: %w", err)
 	}
 

internal/keyring/keyring.go

@@ -2,11 +2,16 @@ package keyring
 
 import (
 	"errors"
+	"fmt"
+	"strings"
 
 	"github.com/zalando/go-keyring"
 )
 
-const secretRefreshToken = "Auth0 CLI Refresh Token"
+const (
+	secretRefreshToken = "Auth0 CLI Refresh Token"
+	secretClientSecret = "Auth0 CLI Client Secret"
+)
 
 // StoreRefreshToken stores a tenant's refresh token in the system keyring.
 func StoreRefreshToken(tenant, value string) error {
@@ -18,15 +23,35 @@ func GetRefreshToken(tenant string) (string, error) {
 	return keyring.Get(secretRefreshToken, tenant)
 }
 
+// StoreClientSecret stores a tenant's client secret in the system keyring.
+func StoreClientSecret(tenant, value string) error {
+	return keyring.Set(secretClientSecret, tenant, value)
+}
+
+// GetClientSecret retrieves a tenant's client secret from the system keyring.
+func GetClientSecret(tenant string) (string, error) {
+	return keyring.Get(secretClientSecret, tenant)
+}
+
 // DeleteSecretsForTenant deletes all secrets for a given tenant.
 func DeleteSecretsForTenant(tenant string) error {
+	var multiErrors []string
+
 	if err := keyring.Delete(secretRefreshToken, tenant); err != nil {
-		if errors.Is(err, keyring.ErrNotFound) {
-			return nil
+		if !errors.Is(err, keyring.ErrNotFound) {
+			multiErrors = append(multiErrors, fmt.Sprintf("failed to delete refresh token from keyring: %s", err))
 		}
+	}
+
+	if err := keyring.Delete(secretClientSecret, tenant); err != nil {
+		if !errors.Is(err, keyring.ErrNotFound) {
+			multiErrors = append(multiErrors, fmt.Sprintf("failed to delete client secret from keyring: %s", err))
+		}
+	}
 
-		return err
+	if len(multiErrors) == 0 {
+		return nil
 	}
 
-	return nil
+	return errors.New(strings.Join(multiErrors, ", "))
 }

internal/keyring/keyring_test.go

@@ -40,4 +40,35 @@ func TestSecrets(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, expectedRefreshToken, actualRefreshToken)
 	})
+
+	t.Run("it fails to retrieve an nonexistent client secret", func(t *testing.T) {
+		keyring.MockInit()
+
+		_, actualError := GetClientSecret(testTenantName)
+		assert.EqualError(t, actualError, keyring.ErrNotFound.Error())
+	})
+
+	t.Run("it successfully retrieves an existent client secret", func(t *testing.T) {
+		keyring.MockInit()
+
+		expectedRefreshToken := "fake-refresh-token"
+		err := keyring.Set(secretClientSecret, testTenantName, expectedRefreshToken)
+		assert.NoError(t, err)
+
+		actualRefreshToken, err := GetClientSecret(testTenantName)
+		assert.NoError(t, err)
+		assert.Equal(t, expectedRefreshToken, actualRefreshToken)
+	})
+
+	t.Run("it successfully stores a client secret", func(t *testing.T) {
+		keyring.MockInit()
+
+		expectedRefreshToken := "fake-refresh-token"
+		err := StoreClientSecret(testTenantName, expectedRefreshToken)
+		assert.NoError(t, err)
+
+		actualRefreshToken, err := GetClientSecret(testTenantName)
+		assert.NoError(t, err)
+		assert.Equal(t, expectedRefreshToken, actualRefreshToken)
+	})
 }