From 5ef50ba4c8ce386e4ae2f71293af415a70f3c56d Mon Sep 17 00:00:00 2001
From: Yahav Itzhak <yahavi@users.noreply.github.com>
Date: Wed, 24 Apr 2024 12:48:15 +0300
Subject: [PATCH] Lock config during import (#1176)

---
 common/commands/config.go      | 51 ++++++++++++++++++++++------------
 common/commands/config_test.go | 29 ++++++++++++++++++-
 2 files changed, 61 insertions(+), 19 deletions(-)

diff --git a/common/commands/config.go b/common/commands/config.go
index 53ce635e4..c51ded1fb 100644
--- a/common/commands/config.go
+++ b/common/commands/config.go
@@ -3,6 +3,13 @@ package commands
 import (
 	"errors"
 	"fmt"
+	"net/url"
+	"os"
+	"reflect"
+	"strconv"
+	"strings"
+	"sync"
+
 	"github.com/jfrog/jfrog-cli-core/v2/artifactory/utils"
 	"github.com/jfrog/jfrog-cli-core/v2/utils/config"
 	"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
@@ -14,12 +21,6 @@ import (
 	"github.com/jfrog/jfrog-client-go/utils/errorutils"
 	"github.com/jfrog/jfrog-client-go/utils/io/fileutils"
 	"github.com/jfrog/jfrog-client-go/utils/log"
-	"net/url"
-	"os"
-	"reflect"
-	"strconv"
-	"strings"
-	"sync"
 )
 
 type ConfigAction string
@@ -105,17 +106,7 @@ func (cc *ConfigCommand) SetDetails(details *config.ServerDetails) *ConfigComman
 
 func (cc *ConfigCommand) Run() (err error) {
 	log.Debug("Locking config file to run config " + cc.cmdType + " command.")
-	mutex.Lock()
-	defer func() {
-		mutex.Unlock()
-		log.Debug("Config " + cc.cmdType + " command completed successfully. config file is released.")
-	}()
-
-	lockDirPath, err := coreutils.GetJfrogConfigLockDir()
-	if err != nil {
-		return
-	}
-	unlockFunc, err := lock.CreateLock(lockDirPath)
+	unlockFunc, err := lockConfig()
 	// Defer the lockFile.Unlock() function before throwing a possible error to avoid deadlock situations.
 	defer func() {
 		err = errors.Join(err, unlockFunc())
@@ -551,7 +542,21 @@ func ShowConfig(serverName string) error {
 	return nil
 }
 
-func Import(configTokenString string) error {
+func lockConfig() (unlockFunc func() error, err error) {
+	mutex.Lock()
+	defer func() {
+		mutex.Unlock()
+		log.Debug("config file is released.")
+	}()
+
+	lockDirPath, err := coreutils.GetJfrogConfigLockDir()
+	if err != nil {
+		return
+	}
+	return lock.CreateLock(lockDirPath)
+}
+
+func Import(configTokenString string) (err error) {
 	serverDetails, err := config.Import(configTokenString)
 	if err != nil {
 		return err
@@ -561,6 +566,16 @@ func Import(configTokenString string) error {
 		details:  serverDetails,
 		serverId: serverDetails.ServerId,
 	}
+
+	log.Debug("Locking config file to run config import command.")
+	unlockFunc, err := lockConfig()
+	// Defer the lockFile.Unlock() function before throwing a possible error to avoid deadlock situations.
+	defer func() {
+		err = errors.Join(err, unlockFunc())
+	}()
+	if err != nil {
+		return err
+	}
 	return configCommand.config()
 }
 
diff --git a/common/commands/config_test.go b/common/commands/config_test.go
index 0002abfd9..cef289187 100644
--- a/common/commands/config_test.go
+++ b/common/commands/config_test.go
@@ -15,7 +15,12 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-const testServerId = "test"
+const (
+	testServerId = "test"
+	// #nosec G101 -- False positive - no hardcoded credentials.
+	// jfrog-ignore - not a real token
+	acmeConfigToken = "eyJ2ZXJzaW9uIjoyLCJ1cmwiOiJodHRwczovL2FjbWUuamZyb2cuaW8vIiwiYXJ0aWZhY3RvcnlVcmwiOiJodHRwczovL2FjbWUuamZyb2cuaW8vYXJ0aWZhY3RvcnkvIiwiZGlzdHJpYnV0aW9uVXJsIjoiaHR0cHM6Ly9hY21lLmpmcm9nLmlvL2Rpc3RyaWJ1dGlvbi8iLCJ4cmF5VXJsIjoiaHR0cHM6Ly9hY21lLmpmcm9nLmlvL3hyYXkvIiwibWlzc2lvbkNvbnRyb2xVcmwiOiJodHRwczovL2FjbWUuamZyb2cuaW8vbWMvIiwicGlwZWxpbmVzVXJsIjoiaHR0cHM6Ly9hY21lLmpmcm9nLmlvL3BpcGVsaW5lcy8iLCJ1c2VyIjoiYWRtaW4iLCJwYXNzd29yZCI6InBhc3N3b3JkIiwidG9rZW5SZWZyZXNoSW50ZXJ2YWwiOjYwLCJzZXJ2ZXJJZCI6ImFjbWUifQ=="
+)
 
 func init() {
 	log.SetDefaultLogger()
@@ -317,6 +322,28 @@ func TestKeyDecryptionError(t *testing.T) {
 	assert.ErrorContains(t, err, "cannot decrypt config")
 }
 
+func TestImport(t *testing.T) {
+	// Create temp jfrog home
+	cleanUpJfrogHome, err := utilsTests.SetJfrogHome()
+	assert.NoError(t, err)
+	defer cleanUpJfrogHome()
+
+	// Import config token
+	assert.NoError(t, Import(acmeConfigToken))
+	serverDetails, err := GetConfig("acme", true)
+	assert.NoError(t, err)
+
+	// Verify that the configuration was imported correctly
+	assert.Equal(t, "https://acme.jfrog.io/", serverDetails.GetUrl())
+	assert.Equal(t, "https://acme.jfrog.io/artifactory/", serverDetails.GetArtifactoryUrl())
+	assert.Equal(t, "https://acme.jfrog.io/distribution/", serverDetails.GetDistributionUrl())
+	assert.Equal(t, "https://acme.jfrog.io/xray/", serverDetails.GetXrayUrl())
+	assert.Equal(t, "https://acme.jfrog.io/mc/", serverDetails.GetMissionControlUrl())
+	assert.Equal(t, "https://acme.jfrog.io/pipelines/", serverDetails.GetPipelinesUrl())
+	assert.Equal(t, "admin", serverDetails.GetUser())
+	assert.Equal(t, "password", serverDetails.GetPassword())
+}
+
 func testExportImport(t *testing.T, inputDetails *config.ServerDetails) {
 	configToken, err := config.Export(inputDetails)
 	assert.NoError(t, err)