Merge pull request #240 from jfrog/GH-237-fix-secret-revoke-error-wit…

…h-user-token

Fix secret revoke error with user_token

.github/workflows/acceptance-tests.yml

@@ -31,7 +31,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: 1.22
+          go-version: 1.23
       - name: Install Helm
         uses: azure/[email protected]
       - name: Install GoReleaser

.github/workflows/release.yml

@@ -19,7 +19,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: 1.22
+          go-version: 1.23
       - name: Import GPG key
         id: import_gpg
         uses: crazy-max/ghaction-import-gpg@v6

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 1.8.5 (January 14, 2025). Tested on Artifactory 7.98.13 with Vault v1.18.3 and OpenBao v2.0.0
+
+BUG FIXES:
+
+* Fix error when access token is not set for `config/admin` path when using user token. Issue: [#236](https://github.com/jfrog/artifactory-secrets-plugin/issues/236) PR: [#240](https://github.com/jfrog/artifactory-secrets-plugin/pull/240)
+* Fix token lease revoke error when access token is not set for `config/admin` path when using user token. Issue: [#237](https://github.com/jfrog/artifactory-secrets-plugin/issues/237) PR: [#240](https://github.com/jfrog/artifactory-secrets-plugin/pull/240)
+
 ## 1.8.4 (December 9, 2024). Tested on Artifactory 7.98.10 with Vault v1.18.2 and OpenBao v2.0.0
 
 BUG FIXES:

README.md

@@ -852,7 +852,7 @@ See the [contribution guide](./CONTRIBUTING.md).
 
 ## License
 
-Copyright (c) 2024 JFrog.
+Copyright (c) 2025 JFrog.
 
 Apache 2.0 licensed, see [LICENSE][LICENSE] file.
 

artifactory.go

@@ -350,7 +350,7 @@ func (b *backend) refreshExpiredAccessToken(ctx context.Context, req *logical.Re
 	logger.Debug("check if access token is expired by getting token itself")
 	err := b.getTokenByID(*config)
 	if err != nil {
-		logger.Debug("failed to get Viewer role", "err", err)
+		logger.Debug("failed to get token by ID", "err", err)
 
 		if _, ok := err.(*TokenExpiredError); ok {
 			logger.Info("access token expired. Attempt to refresh using the refresh token.", "err", err)
@@ -417,7 +417,7 @@ func (b *backend) getVersion(config baseConfiguration) (version string, err erro
 func (b *backend) getTokenByID(config baseConfiguration) error {
 	logger := b.Logger().With("func", "getTokenByID")
 
-	logger.Debug("fetching Viewer role")
+	logger.Debug("fetching token by ID")
 
 	// '/me' is special value to get info about token itself
 	// https://jfrog.com/help/r/jfrog-rest-apis/get-token-by-id

backend.go

@@ -36,7 +36,7 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend,
 		return nil, fmt.Errorf("configuration passed into backend is nil")
 	}
 
-	b, err := Backend(conf)
+	b, err := Backend()
 	if err != nil {
 		return nil, err
 	}
@@ -48,7 +48,7 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend,
 	return b, nil
 }
 
-func Backend(_ *logical.BackendConfig) (*backend, error) {
+func Backend() (*backend, error) {
 	b := &backend{}
 
 	up, err := testUsernameTemplate(defaultUserNameTemplate)

go.mod

@@ -1,7 +1,6 @@
 module github.com/jfrog/vault-plugin-secrets-artifactory
 
-go 1.22
-toolchain go1.23.4
+go 1.23.4
 
 require (
 	github.com/golang-jwt/jwt/v4 v4.5.1

path_config.go

@@ -187,9 +187,11 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da
 
 	b.InitializeHttpClient(config)
 
-	go b.sendUsage(config.baseConfiguration, "pathConfigRotateUpdate")
+	if config.AccessToken != "" {
+		go b.sendUsage(config.baseConfiguration, "pathConfigRotateUpdate")
 
-	config.UseNewAccessAPI = b.useNewAccessAPI(config.baseConfiguration)
+		config.UseNewAccessAPI = b.useNewAccessAPI(config.baseConfiguration)
+	}
 
 	entry, err := logical.StorageEntryJSON(configAdminPath, config)
 	if err != nil {
@@ -217,12 +219,16 @@ func (b *backend) pathConfigDelete(ctx context.Context, req *logical.Request, _
 		return logical.ErrorResponse("backend not configured"), nil
 	}
 
-	go b.sendUsage(config.baseConfiguration, "pathConfigDelete")
-
 	if err := req.Storage.Delete(ctx, configAdminPath); err != nil {
 		return nil, err
 	}
 
+	if config.AccessToken == "" {
+		return nil, nil
+	}
+
+	go b.sendUsage(config.baseConfiguration, "pathConfigDelete")
+
 	if config.RevokeOnDelete {
 		b.Logger().Info("config.RevokeOnDelete is 'true'. Attempt to revoke access token.")
 
@@ -257,28 +263,35 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f
 		return logical.ErrorResponse("backend not configured"), nil
 	}
 
-	go b.sendUsage(config.baseConfiguration, "pathConfigRead")
-
-	version, err := b.getVersion(config.baseConfiguration)
-	if err != nil {
-		logger.Error("failed to get system version", "err", err)
-		return nil, err
-	}
-
-	// I'm not sure if I should be returning the access token, so I'll hash it.
-	accessTokenHash := sha256.Sum256([]byte(config.AccessToken))
-
 	configMap := map[string]interface{}{
-		"access_token_sha256":                 fmt.Sprintf("%x", accessTokenHash[:]),
 		"url":                                 config.ArtifactoryURL,
-		"version":                             version,
 		"use_expiring_tokens":                 config.UseExpiringTokens,
 		"force_revocable":                     config.ForceRevocable,
 		"bypass_artifactory_tls_verification": config.BypassArtifactoryTLSVerification,
 		"allow_scope_override":                config.AllowScopeOverride,
 		"revoke_on_delete":                    config.RevokeOnDelete,
 	}
 
+	if config.AccessToken == "" {
+		return &logical.Response{
+			Warnings: []string{"access_token is not set"},
+			Data:     configMap,
+		}, nil
+	}
+
+	// I'm not sure if I should be returning the access token, so I'll hash it.
+	accessTokenHash := sha256.Sum256([]byte(config.AccessToken))
+	configMap["access_token_sha256"] = fmt.Sprintf("%x", accessTokenHash[:])
+
+	go b.sendUsage(config.baseConfiguration, "pathConfigRead")
+
+	version, err := b.getVersion(config.baseConfiguration)
+	if err != nil {
+		logger.Error("failed to get system version", "err", err)
+		return nil, err
+	}
+	configMap["version"] = version
+
 	// Optionally include username_template
 	if len(config.UsernameTemplate) > 0 {
 		configMap["username_template"] = config.UsernameTemplate

path_config_test.go

@@ -225,6 +225,7 @@ func TestBackend_AccessTokenAsSHA256(t *testing.T) {
 	assert.NotNil(t, resp)
 	assert.EqualValues(t, correctSHA256, resp.Data["access_token_sha256"])
 }
+
 func TestBackend_RevokeOnDelete(t *testing.T) {
 
 	httpmock.Activate()

path_config_user_token.go

@@ -214,8 +214,6 @@ func (b *backend) pathConfigUserTokenUpdate(ctx context.Context, req *logical.Re
 		userTokenConfig.AccessToken = adminConfig.AccessToken
 	}
 
-	go b.sendUsage(userTokenConfig.baseConfiguration, "pathConfigUserTokenUpdate")
-
 	if val, ok := data.GetOk("refresh_token"); ok {
 		userTokenConfig.RefreshToken = val.(string)
 	}
@@ -257,7 +255,11 @@ func (b *backend) pathConfigUserTokenUpdate(ctx context.Context, req *logical.Re
 		userTokenConfig.DefaultDescription = val.(string)
 	}
 
-	userTokenConfig.UseNewAccessAPI = b.useNewAccessAPI(userTokenConfig.baseConfiguration)
+	if userTokenConfig.AccessToken != "" {
+		go b.sendUsage(userTokenConfig.baseConfiguration, "pathConfigUserTokenUpdate")
+
+		userTokenConfig.UseNewAccessAPI = b.useNewAccessAPI(userTokenConfig.baseConfiguration)
+	}
 
 	err = b.storeUserTokenConfiguration(ctx, req, username, userTokenConfig)
 	if err != nil {

path_token_create.go

@@ -111,7 +111,7 @@ func (b *backend) pathTokenCreatePerform(ctx context.Context, req *logical.Reque
 	logger := b.Logger().With("func", "pathTokenCreatePerform")
 
 	maxLeaseTTL := b.Backend.System().MaxLeaseTTL()
-	logger.Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL)
+	logger.Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL.Seconds())
 
 	if value, ok := data.GetOk("max_ttl"); ok && value.(int) > 0 {
 		logger.Debug("max_ttl is set", "max_ttl", value)
@@ -122,26 +122,26 @@ func (b *backend) pathTokenCreatePerform(ctx context.Context, req *logical.Reque
 			maxLeaseTTL = maxTTL
 		}
 	} else if role.MaxTTL > 0 && role.MaxTTL < maxLeaseTTL {
-		logger.Debug("using role MaxTTL", "role.MaxTTL", role.MaxTTL)
+		logger.Debug("using role MaxTTL", "role.MaxTTL", role.MaxTTL.Seconds())
 		maxLeaseTTL = role.MaxTTL
 	}
-	logger.Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL)
+	logger.Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL.Seconds())
 
 	ttl := b.Backend.System().DefaultLeaseTTL()
 	if value, ok := data.GetOk("ttl"); ok && value.(int) > 0 {
 		logger.Debug("ttl is set", "ttl", value)
 		ttl = time.Second * time.Duration(value.(int))
 	} else if role.DefaultTTL != 0 {
-		logger.Debug("using role DefaultTTL", "role.DefaultTTL", role.DefaultTTL)
+		logger.Debug("using role DefaultTTL", "role.DefaultTTL", role.DefaultTTL.Seconds())
 		ttl = role.DefaultTTL
 	}
 
 	// cap ttl to maxLeaseTTL
 	if ttl > maxLeaseTTL {
-		logger.Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL)
+		logger.Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL.Seconds())
 		ttl = maxLeaseTTL
 	}
-	logger.Debug("TTL (sec)", "ttl", ttl)
+	logger.Debug("TTL (sec)", "ttl", ttl.Seconds())
 
 	// Set the role.ExpiresIn based on maxLeaseTTL if use_expiring_tokens is set to tru in config
 	// - This value will be passed to createToken and used as expires_in for versions of Artifactory 7.50.3 or higher

path_user_token_create.go

@@ -93,8 +93,8 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R
 		return nil, err
 	}
 
-	if userTokenConfig.baseConfiguration.AccessToken != "" {
-		baseConfig.AccessToken = userTokenConfig.baseConfiguration.AccessToken
+	if userTokenConfig.AccessToken != "" {
+		baseConfig.AccessToken = userTokenConfig.AccessToken
 	}
 
 	if baseConfig.AccessToken == "" {
@@ -129,7 +129,7 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R
 	}
 
 	maxLeaseTTL := b.Backend.System().MaxLeaseTTL()
-	logger.Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL)
+	logger.Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL.Seconds())
 
 	if value, ok := data.GetOk("max_ttl"); ok && value.(int) > 0 {
 		logger.Debug("max_ttl is set", "max_ttl", value)
@@ -140,27 +140,27 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R
 			maxLeaseTTL = maxTTL
 		}
 	} else if userTokenConfig.MaxTTL > 0 && userTokenConfig.MaxTTL < maxLeaseTTL {
-		logger.Debug("using user token config MaxTTL", "userTokenConfig.MaxTTL", userTokenConfig.MaxTTL)
+		logger.Debug("using user token config MaxTTL", "userTokenConfig.MaxTTL", userTokenConfig.MaxTTL.Seconds())
 		// use max TTL from user config if set and is less than system max lease TTL
 		maxLeaseTTL = userTokenConfig.MaxTTL
 	}
-	logger.Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL)
+	logger.Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL.Seconds())
 
 	ttl := b.Backend.System().DefaultLeaseTTL()
 	if value, ok := data.GetOk("ttl"); ok && value.(int) > 0 {
 		logger.Debug("ttl is set", "ttl", value)
 		ttl = time.Second * time.Duration(value.(int))
 	} else if userTokenConfig.DefaultTTL != 0 {
-		logger.Debug("using user config DefaultTTL", "userTokenConfig.DefaultTTL", userTokenConfig.DefaultTTL)
+		logger.Debug("using user config DefaultTTL", "userTokenConfig.DefaultTTL", userTokenConfig.DefaultTTL.Seconds())
 		ttl = userTokenConfig.DefaultTTL
 	}
 
 	// cap ttl to maxLeaseTTL
 	if maxLeaseTTL > 0 && ttl > maxLeaseTTL {
-		logger.Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL)
+		logger.Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL.Seconds())
 		ttl = maxLeaseTTL
 	}
-	logger.Debug("TTL (sec)", "ttl", ttl)
+	logger.Debug("TTL (sec)", "ttl", ttl.Seconds())
 
 	// now ttl is determined, we set role.ExpiresIn so this value so expirable token has the correct expiration
 	if baseConfig.UseExpiringTokens {

secret_access_token.go

@@ -3,6 +3,7 @@ package artifactory
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/logical"
@@ -18,6 +19,14 @@ func (b *backend) secretAccessToken() *framework.Secret {
 				Type:        framework.TypeString,
 				Description: `Artifactory Access Token`,
 			},
+			"refresh_token": {
+				Type:        framework.TypeString,
+				Description: `Artifactory Refresh Token`,
+			},
+			"reference_token": {
+				Type:        framework.TypeString,
+				Description: `Artifactory Reference Token`,
+			},
 			"token_id": {
 				Type:        framework.TypeString,
 				Description: `Artifactory Access Token Id`,
@@ -34,8 +43,6 @@ func (b *backend) secretAccessToken() *framework.Secret {
 }
 
 func (b *backend) secretAccessTokenRenew(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
-	resp := &logical.Response{Secret: req.Secret}
-
 	config, err := b.fetchAdminConfiguration(ctx, req.Storage)
 	if err != nil {
 		return nil, err
@@ -62,6 +69,9 @@ func (b *backend) secretAccessTokenRenew(ctx context.Context, req *logical.Reque
 	if err != nil {
 		return nil, err
 	}
+
+	resp := &logical.Response{Secret: req.Secret}
+
 	if len(warnings) > 0 {
 		for _, warning := range warnings {
 			resp.AddWarning(warning)
@@ -74,19 +84,48 @@ func (b *backend) secretAccessTokenRenew(ctx context.Context, req *logical.Reque
 }
 
 func (b *backend) secretAccessTokenRevoke(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
-	config, err := b.fetchAdminConfiguration(ctx, req.Storage)
+	logger := b.Logger().With("func", "secretAccessTokenRevoke")
 
+	config, err := b.fetchAdminConfiguration(ctx, req.Storage)
 	if err != nil {
+		logger.Debug("failed to fetch admin config", "err", err)
 		return nil, err
 	}
 
 	if config == nil {
 		return logical.ErrorResponse("backend not configured"), nil
 	}
 
+	// logger.Debug("req", "Path", req.Path, "Secret.InternalData", req.Secret.InternalData)
+
+	if config.AccessToken == "" {
+		// check if this is admin token
+		if strings.HasPrefix(req.Path, "token/") {
+			return logical.ErrorResponse("admin access_token is not configured"), nil
+		}
+
+		// try to use user token
+		if strings.HasPrefix(req.Path, "user_token/") {
+			logger.Debug("admin access token is empty and request path is user_token")
+			username := req.Secret.InternalData["username"].(string)
+			userTokenConfig, err := b.fetchUserTokenConfiguration(ctx, req.Storage, username)
+			if err != nil {
+				logger.Debug("failed to fetch user config", "err", err)
+				return nil, err
+			}
+
+			if userTokenConfig.AccessToken == "" {
+				return logical.ErrorResponse("user access_token is not configured"), nil
+			}
+
+			config.AccessToken = userTokenConfig.AccessToken
+		}
+	}
+
 	tokenId := req.Secret.InternalData["token_id"].(string)
+
 	if err := b.RevokeToken(config.baseConfiguration, tokenId); err != nil {
-		return nil, err
+		return logical.ErrorResponse("failed to revoke access token"), err
 	}
 
 	return nil, nil

test_utils.go

@@ -608,7 +608,7 @@ func makeBackend(t *testing.T) (*backend, *logical.BackendConfig) {
 	config := logical.TestBackendConfig()
 	config.StorageView = &logical.InmemStorage{}
 
-	b, err := Backend(config)
+	b, err := Backend()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -623,7 +623,6 @@ func makeBackend(t *testing.T) (*backend, *logical.BackendConfig) {
 func configuredBackend(t *testing.T, adminConfig map[string]interface{}) (*backend, *logical.BackendConfig) {
 
 	b, config := makeBackend(t)
-	t.Logf("b.System().MaxLeaseTTL(): %v\n", b.System().MaxLeaseTTL())
 	b.InitializeHttpClient(&adminConfiguration{})
 
 	_, err := b.HandleRequest(context.Background(), &logical.Request{