Add 'revoke_on_delete' field to 'config/admin' path

CHANGELOG.md

@@ -1,3 +1,11 @@
+## 1.4.0 (March 8, 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.
+
+ Issue: [#86](https://github.com/jfrog/artifactory-secrets-plugin/issues/86) PR: [#161](https://github.com/jfrog/artifactory-secrets-plugin/pull/161)
+
 ## 1.3.0 (Feburary 27, 2023)
 
 IMPROVEMENTS:

README.md

@@ -419,6 +419,7 @@ No renewals or new tokens will be issued if the backend configuration (config/ad
 * `username_template` (string) - Optional. Vault Username Template for dynamically generating usernames.
 * `use_expiring_tokens` (boolean) - Optional. If Artifactory version >= 7.50.3, set `expires_in` to `max_ttl` (admin token) or `ttl` (user token) and `force_revocable = true`. Default to `false`.
 * `bypass_artifactory_tls_verification` (boolean) - Optional. Bypass certification verification for TLS connection with Artifactory. Default to `false`.
+* `revoke_on_delete` (boolean) - Optional. Revoke Administrator access token when this configuration is deleted. Default to `false`. Will be set to `true` if token is rotated.
 
 #### Example
 
@@ -427,7 +428,8 @@ vault write artifactory/config/admin url=$JFROG_URL \
   access_token=$JFROG_ACCESS_TOKEN \
   username_template="v_{{.DisplayName}}_{{.RoleName}}_{{random 10}}_{{unix_time}}" \
   use_expiring_tokens=true \
-  bypass_artifactory_tls_verification=true
+  bypass_artifactory_tls_verification=true \
+  revoke_on_delete=true
 ```
 
 ### User Token Config

artifactory.go

@@ -18,7 +18,6 @@ import (
 	jwt "github.com/golang-jwt/jwt/v4"
 	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/vault/sdk/helper/template"
-	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/samber/lo"
 )
 
@@ -31,9 +30,9 @@ const (
 var ErrIncompatibleVersion = errors.New("incompatible version")
 
 type baseConfiguration struct {
-	AccessToken                      string `json:"access_token"`
-	ArtifactoryURL                   string `json:"artifactory_url"`
-	UseExpiringTokens                bool   `json:"use_expiring_tokens,omitempty"`
+	AccessToken       string `json:"access_token"`
+	ArtifactoryURL    string `json:"artifactory_url"`
+	UseExpiringTokens bool   `json:"use_expiring_tokens,omitempty"`
 }
 
 type errorResponse struct {
@@ -42,8 +41,7 @@ type errorResponse struct {
 	Detail  string `json:"detail"`
 }
 
-func (b *backend) RevokeToken(config baseConfiguration, secret logical.Secret) error {
-	tokenId := secret.InternalData["token_id"].(string)
+func (b *backend) RevokeToken(config baseConfiguration, tokenId, accessToken string) error {
 	u, err := url.Parse(config.ArtifactoryURL)
 	if err != nil {
 		b.Logger().Error("could not parse artifactory url", "url", u, "err", err)
@@ -59,7 +57,6 @@ func (b *backend) RevokeToken(config baseConfiguration, secret logical.Secret) e
 			return err
 		}
 	} else {
-		accessToken := secret.InternalData["access_token"].(string)
 		values := url.Values{}
 		values.Set("token", accessToken)
 

artifactory_test.go

@@ -325,4 +325,14 @@ func TestBackend_RotateAdminToken(t *testing.T) {
 	})
 	assert.NoError(t, err)
 	assert.Nil(t, resp)
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.ReadOperation,
+		Path:      configAdminPath,
+		Storage:   config.StorageView,
+	})
+
+	assert.NoError(t, err)
+	assert.NotNil(t, resp)
+	assert.EqualValues(t, true, resp.Data["revoke_on_delete"])
 }

path_config.go

@@ -40,6 +40,11 @@ func (b *backend) pathConfig() *framework.Path {
 				Default:     false,
 				Description: "Optional. Bypass certification verification for TLS connection with Artifactory. Default to `false`.",
 			},
+			"revoke_on_delete": {
+				Type:        framework.TypeBool,
+				Default:     false,
+				Description: "Optional. Revoke Administrator access token when this configuration is deleted. Default to `false`. Will be set to `true` if token is rotated.",
+			},
 		},
 		Operations: map[logical.Operation]framework.OperationHandler{
 			logical.UpdateOperation: &framework.PathOperation{
@@ -81,6 +86,7 @@ type adminConfiguration struct {
 	baseConfiguration
 	UsernameTemplate                 string `json:"username_template,omitempty"`
 	BypassArtifactoryTLSVerification bool   `json:"bypass_artifactory_tls_verification,omitempty"`
+	RevokeOnDelete                   bool   `json:"revoke_on_delete,omitempty"`
 }
 
 // fetchAdminConfiguration will return nil,nil if there's no configuration
@@ -143,6 +149,10 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da
 		config.BypassArtifactoryTLSVerification = val.(bool)
 	}
 
+	if val, ok := data.GetOk("revoke_on_delete"); ok {
+		config.RevokeOnDelete = val.(bool)
+	}
+
 	if config.AccessToken == "" {
 		return logical.ErrorResponse("access_token is required"), nil
 	}
@@ -192,6 +202,22 @@ func (b *backend) pathConfigDelete(ctx context.Context, req *logical.Request, _
 		return nil, err
 	}
 
+	if config.RevokeOnDelete {
+		b.Logger().Info("config.RevokeOnDelete is 'true'. Attempt to revoke access token.")
+
+		token, err := b.getTokenInfo(config.baseConfiguration, config.AccessToken)
+		if err != nil {
+			b.Logger().Warn("error parsing existing access token", "err", err)
+			return nil, nil
+		}
+
+		err = b.RevokeToken(config.baseConfiguration, token.TokenID, config.AccessToken)
+		if err != nil {
+			b.Logger().Warn("error revoking existing access token", "tokenId", token.TokenID, "err", err)
+			return nil, nil
+		}
+	}
+
 	return nil, nil
 }
 
@@ -219,6 +245,7 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f
 		"version":                             b.version,
 		"use_expiring_tokens":                 config.UseExpiringTokens,
 		"bypass_artifactory_tls_verification": config.BypassArtifactoryTLSVerification,
+		"revoke_on_delete":                    config.RevokeOnDelete,
 	}
 
 	// Optionally include username_template

path_config_rotate.go

@@ -82,8 +82,10 @@ func (b *backend) pathConfigRotateWrite(ctx context.Context, req *logical.Reques
 		return logical.ErrorResponse("error creating new access token"), err
 	}
 
-	// Set new token
+	// Set new token and set revoke_on_delete to true
 	config.AccessToken = resp.AccessToken
+	b.Logger().Info("set config.RevokeOnDelete to 'true'")
+	config.RevokeOnDelete = true
 
 	// Save new config
 	entry, err := logical.StorageEntryJSON(configAdminPath, config)
@@ -97,13 +99,7 @@ func (b *backend) pathConfigRotateWrite(ctx context.Context, req *logical.Reques
 	}
 
 	// Invalidate Old Token
-	oldSecret := logical.Secret{
-		InternalData: map[string]interface{}{
-			"access_token": oldAccessToken,
-			"token_id":     token.TokenID,
-		},
-	}
-	err = b.RevokeToken(config.baseConfiguration, oldSecret)
+	err = b.RevokeToken(config.baseConfiguration, token.TokenID, oldAccessToken)
 	if err != nil {
 		return logical.ErrorResponse("error revoking existing access token %s", token.TokenID), err
 	}

path_config_test.go

@@ -2,6 +2,7 @@ package artifactory
 
 import (
 	"context"
+	"net/http"
 	"regexp"
 	"testing"
 
@@ -197,7 +198,7 @@ func TestBackend_AccessTokenAsSHA256(t *testing.T) {
 	mockArtifactoryUsageVersionRequests("")
 
 	httpmock.RegisterResponder(
-		"GET",
+		http.MethodGet,
 		"http://myserver.com:80/access/api/v1/cert/root",
 		httpmock.NewStringResponder(200, rootCert))
 
@@ -216,3 +217,56 @@ 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()
+	defer httpmock.DeactivateAndReset()
+
+	mockArtifactoryUsageVersionRequests(`{"version" : "7.33.8", "revision" : "73308900"}`)
+
+	httpmock.RegisterResponder(
+		http.MethodGet,
+		"http://myserver.com:80/access/api/v1/cert/root",
+		httpmock.NewStringResponder(200, rootCert))
+
+	b, config := configuredBackend(t, map[string]interface{}{
+		"access_token": `eyJ2ZXIiOiIyIiwidHlwIjoiSldUIiwiYWxnIjoiUlMyNTYiLCJraW` +
+			`QiOiJkMUxJUFRHbmY0RHZzQ2k0MzhodU9KdWN3bi1lSTBHc0lVR2g0bGhhdE53In0.eyJ` +
+			`zdWIiOiJqZmFjQDAxaDQyNGh2d3B5dHprMWF6eGg2azgwN2U1L3VzZXJzL2FkbWluIiwi` +
+			`c2NwIjoiYXBwbGllZC1wZXJtaXNzaW9ucy9hZG1pbiIsImF1ZCI6IipAKiIsImlzcyI6I` +
+			`mpmZmVAMDFoNDI0aHZ3cHl0emsxYXp4aDZrODA3ZTUiLCJleHAiOjE3NTMyOTM5OTMsIm` +
+			`lhdCI6MTY5MDIyMTk5MywianRpIjoiODRjMDYyNmItNzk3My00MGM5LTlkMzctNzAxYWF` +
+			`mNzNjZmI0In0.VXoZR--oQLRTqTLx3Ogz1UUrzT9hlihWQ8m_JgOucZEYwIjGa2P58wUW` +
+			`vUAxonkiqyvmFfEk8H1vyiaBQ0F9vQ7v16d3D3nfEDW71g09M3NnsKu065-pbjPRGUmSi` +
+			`SvV0WC3Gla5Ui31IA_vVhyc-kPDzoWpHwBWgOMWkJwP0ZrvQ5bwzKrwNQi6YB0SIX2eSH` +
+			`RpReef19W_4BpOUrqMrcDamB3mskwxcYFUMA45FRoV_JVxZsIMOyNNfDlNy01r5bA6ZcY` +
+			`EaseaQpU7skMCW07rUiWq4Z6U0xZEduKPlowJm9xbrBM13FEQTG4b4mW7yyOD4gqQ49wD` +
+			`GGXvhLVFoQ`,
+		"url":              "http://myserver.com:80",
+		"revoke_on_delete": true,
+	})
+
+	resp, err := b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.ReadOperation,
+		Path:      configAdminPath,
+		Storage:   config.StorageView,
+	})
+
+	assert.NoError(t, err)
+	assert.NotNil(t, resp)
+	assert.EqualValues(t, true, resp.Data["revoke_on_delete"])
+
+	httpmock.RegisterResponder(
+		http.MethodDelete,
+		"http://myserver.com:80/access/api/v1/tokens/84c0626b-7973-40c9-9d37-701aaf73cfb4",
+		httpmock.NewStringResponder(200, ""))
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.DeleteOperation,
+		Path:      configAdminPath,
+		Storage:   config.StorageView,
+	})
+
+	assert.NoError(t, err)
+	assert.Nil(t, resp)
+}

secret_access_token.go

@@ -84,7 +84,9 @@ func (b *backend) secretAccessTokenRevoke(ctx context.Context, req *logical.Requ
 		return logical.ErrorResponse("backend not configured"), nil
 	}
 
-	if err := b.RevokeToken(config.baseConfiguration, *req.Secret); err != nil {
+	tokenId := req.Secret.InternalData["token_id"].(string)
+	accessToken := req.Secret.InternalData["access_token"].(string)
+	if err := b.RevokeToken(config.baseConfiguration, tokenId, accessToken); err != nil {
 		return nil, err
 	}
 

test_utils.go

@@ -102,14 +102,7 @@ func (e *accTestEnv) revokeTestToken(t *testing.T, accessToken string, tokenID s
 		t.Fatal(err)
 	}
 
-	secret := logical.Secret{
-		InternalData: map[string]interface{}{
-			"access_token": accessToken,
-			"token_id":     tokenID,
-		},
-	}
-
-	err = e.Backend.(*backend).RevokeToken(config, secret)
+	err = e.Backend.(*backend).RevokeToken(config, tokenID, accessToken)
 	if err != nil {
 		t.Fatal(err)
 	}