diff --git a/CHANGELOG.md b/CHANGELOG.md index 3586ee0..043a9fd 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/README.md b/README.md index 8765791..0429cb5 100644 --- a/README.md +++ b/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 diff --git a/artifactory.go b/artifactory.go index 29fafb9..4c32daa 100644 --- a/artifactory.go +++ b/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) diff --git a/artifactory_test.go b/artifactory_test.go index 27d4b3f..e362721 100644 --- a/artifactory_test.go +++ b/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"]) } diff --git a/path_config.go b/path_config.go index 038737a..3a3566c 100644 --- a/path_config.go +++ b/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 diff --git a/path_config_rotate.go b/path_config_rotate.go index afa316c..8deedd0 100644 --- a/path_config_rotate.go +++ b/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 } diff --git a/path_config_test.go b/path_config_test.go index cdb2623..e17b3a8 100644 --- a/path_config_test.go +++ b/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) +} diff --git a/secret_access_token.go b/secret_access_token.go index 8e1b838..d949092 100644 --- a/secret_access_token.go +++ b/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 } diff --git a/test_utils.go b/test_utils.go index e15457d..72dcb20 100644 --- a/test_utils.go +++ b/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) }