Merge pull request #162 from jfrog/GH-159-fix-user-config-ttl

Fix user token config fall back logic

CHANGELOG.md

@@ -1,10 +1,12 @@
-## 1.4.0 (March 8, 2023)
+## 1.4.0 (March 11, 2023)
 
 IMPROVEMENTS:
 
-* Add `revoke_on_delete` field to `config/admin` path. This enable automatic revocation of admin access token when set to `true`. This field will also be set to `true` if admin access token is rotated.
+* Add `revoke_on_delete` field to `config/admin` path. This enable automatic revocation of admin access token when set to `true`. This field will also be set to `true` if admin access token is rotated. Issue: [#86](https://github.com/jfrog/artifactory-secrets-plugin/issues/86) PR: [#161](https://github.com/jfrog/artifactory-secrets-plugin/pull/161)
 
- Issue: [#86](https://github.com/jfrog/artifactory-secrets-plugin/issues/86) PR: [#161](https://github.com/jfrog/artifactory-secrets-plugin/pull/161)
+BUG FIXES:
+
+* Fix `default_ttl` and `max_ttl` for `config/user_token` path fall back logic. Issue: [#159](https://github.com/jfrog/artifactory-secrets-plugin/issues/159) PR: [#162](https://github.com/jfrog/artifactory-secrets-plugin/pull/162)
 
 ## 1.3.0 (Feburary 27, 2023)
 

path_config_user_token.go

@@ -102,8 +102,24 @@ func (b *backend) fetchUserTokenConfiguration(ctx context.Context, storage logic
 		return nil, err
 	}
 
+	if entry == nil && len(username) > 0 {
+		b.Logger().Info(fmt.Sprintf("no configuration for username %s. Fetching default user token configuration", username), "path", configUserTokenPath)
+		e, err := storage.Get(ctx, configUserTokenPath)
+		if err != nil {
+			return nil, err
+		}
+
+		if e != nil {
+			entry = e
+		}
+	}
+
 	if entry == nil {
-		return &userTokenConfiguration{}, nil
+		b.Logger().Warn("no configuration found. Using system default configuration.")
+		return &userTokenConfiguration{
+			DefaultTTL: b.Backend.System().DefaultLeaseTTL(),
+			MaxTTL:     b.Backend.System().MaxLeaseTTL(),
+		}, nil
 	}
 
 	var config userTokenConfiguration

path_config_user_token_test.go

@@ -2,6 +2,7 @@ package artifactory
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -23,17 +24,17 @@ func TestAcceptanceBackend_PathConfigUserToken(t *testing.T) {
 }
 
 func (e *accTestEnv) PathConfigAccessTokenUpdate(t *testing.T) {
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		"access_token": "test123",
 	})
-	data := e.ReadConfigUserToken(t)
+	data := e.ReadConfigUserToken(t, "")
 	accessTokenHash := data["access_token_sha256"]
 	assert.NotEmpty(t, "access_token_sha256")
 
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		"access_token": "test456",
 	})
-	data = e.ReadConfigUserToken(t)
+	data = e.ReadConfigUserToken(t, "")
 	assert.NotEqual(t, data["access_token_sha256"], accessTokenHash)
 }
 
@@ -66,43 +67,78 @@ func (e *accTestEnv) PathConfigMaxTTLUpdate(t *testing.T) {
 }
 
 func (e *accTestEnv) pathConfigUserTokenUpdateStringField(t *testing.T, fieldName string) {
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		fieldName: "test123",
 	})
-	data := e.ReadConfigUserToken(t)
+	data := e.ReadConfigUserToken(t, "")
 	assert.Equal(t, "test123", data[fieldName])
 
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		fieldName: "test456",
 	})
-	data = e.ReadConfigUserToken(t)
+	data = e.ReadConfigUserToken(t, "")
 	assert.Equal(t, "test456", data[fieldName])
 }
 
 func (e *accTestEnv) pathConfigUserTokenUpdateBoolField(t *testing.T, fieldName string) {
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		fieldName: true,
 	})
-	data := e.ReadConfigUserToken(t)
+	data := e.ReadConfigUserToken(t, "")
 	assert.Equal(t, true, data[fieldName])
 
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		fieldName: false,
 	})
-	data = e.ReadConfigUserToken(t)
+	data = e.ReadConfigUserToken(t, "")
 	assert.Equal(t, false, data[fieldName])
 }
 
 func (e *accTestEnv) pathConfigUserTokenUpdateDurationField(t *testing.T, fieldName string) {
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		fieldName: 1.0,
 	})
-	data := e.ReadConfigUserToken(t)
+	data := e.ReadConfigUserToken(t, "")
 	assert.Equal(t, 1.0, data[fieldName])
 
-	e.UpdateConfigUserToken(t, testData{
+	e.UpdateConfigUserToken(t, "", testData{
 		fieldName: 4.0,
 	})
-	data = e.ReadConfigUserToken(t)
+	data = e.ReadConfigUserToken(t, "")
 	assert.Equal(t, 4.0, data[fieldName])
 }
+
+func TestAcceptanceBackend_PathConfigUserToken_UseUserDefault(t *testing.T) {
+	if !runAcceptanceTests {
+		t.SkipNow()
+	}
+	accTestEnv := NewConfiguredAcceptanceTestEnv(t)
+
+	accTestEnv.UpdateConfigUserToken(t, "", testData{
+		"default_ttl": time.Duration(2) * time.Hour,
+		"max_ttl":     time.Duration(4) * time.Hour,
+	})
+
+	data := accTestEnv.ReadConfigUserToken(t, "")
+	assert.Equal(t, (time.Duration(2) * time.Hour).Seconds(), data["default_ttl"])
+	assert.Equal(t, (time.Duration(4) * time.Hour).Seconds(), data["max_ttl"])
+
+	data = accTestEnv.ReadConfigUserToken(t, "test-user")
+	assert.Equal(t, (time.Duration(2) * time.Hour).Seconds(), data["default_ttl"])
+	assert.Equal(t, (time.Duration(4) * time.Hour).Seconds(), data["max_ttl"])
+}
+
+func TestAcceptanceBackend_PathConfigUserToken_UseSystemDefault(t *testing.T) {
+	if !runAcceptanceTests {
+		t.SkipNow()
+	}
+	accTestEnv := NewConfiguredAcceptanceTestEnv(t)
+
+	data := accTestEnv.ReadConfigUserToken(t, "")
+	assert.Equal(t, accTestEnv.Backend.System().DefaultLeaseTTL().Seconds(), data["default_ttl"])
+	assert.Equal(t, accTestEnv.Backend.System().MaxLeaseTTL().Seconds(), data["max_ttl"])
+
+	data = accTestEnv.ReadConfigUserToken(t, "test-user")
+	assert.Equal(t, accTestEnv.Backend.System().DefaultLeaseTTL().Seconds(), data["default_ttl"])
+	assert.Equal(t, accTestEnv.Backend.System().MaxLeaseTTL().Seconds(), data["max_ttl"])
+}

test_utils.go

@@ -2,8 +2,10 @@ package artifactory
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -123,8 +125,13 @@ func (e *accTestEnv) UpdateConfigAdmin(t *testing.T, data testData) {
 }
 
 // UpdateConfigAdmin will send a POST/PUT to the /config/user_token endpoint with testData (vault write artifactory/config/user_token)
-func (e *accTestEnv) UpdateConfigUserToken(t *testing.T, data testData) {
-	resp, err := e.update(configUserTokenPath, data)
+func (e *accTestEnv) UpdateConfigUserToken(t *testing.T, username string, data testData) {
+	path := configUserTokenPath
+	if len(username) > 0 && !strings.HasSuffix(path, username) {
+		path = fmt.Sprintf("%s/%s", path, username)
+	}
+
+	resp, err := e.update(path, data)
 	assert.NoError(t, err)
 	assert.Nil(t, resp)
 }
@@ -144,8 +151,13 @@ func (e *accTestEnv) ReadConfigAdmin(t *testing.T) testData {
 }
 
 // ReadConfigUserToken will send a GET to the /config/user_token endpoint (vault read artifactory/config/user_token)
-func (e *accTestEnv) ReadConfigUserToken(t *testing.T) testData {
-	resp, err := e.read(configUserTokenPath)
+func (e *accTestEnv) ReadConfigUserToken(t *testing.T, username string) testData {
+	path := configUserTokenPath
+	if len(username) > 0 && !strings.HasSuffix(path, username) {
+		path = fmt.Sprintf("%s/%s", path, username)
+	}
+
+	resp, err := e.read(path)
 
 	assert.NoError(t, err)
 	assert.NotNil(t, resp)