- cleanup fixmes

- tidy via linting
jfrog · Jun 7, 2020 · f873920 · f873920
1 parent 4e83c86
commit f873920
Show file tree

Hide file tree

Showing 13 changed files with 137 additions and 91 deletions.
diff --git a/README.md b/README.md
@@ -1,36 +1,41 @@
-# Vault Mock Secrets Plugin
+# Vault Artifactory Secrets Plugin
 
-Mock is an example secrets engine plugin for [HashiCorp Vault](https://www.vaultproject.io/). It is meant for demonstration purposes only and should never be used in production.
+This is a [HashiCorp Vault](https://www.vaultproject.io/) plugin which talks to JFrog Artifactory server (5.0.0 or later) and will
+dynamically provision access tokens with specified scopes. This backend can be mounted multiple times
+to provide access to multiple Artifactory servers.
 
-## Usage
-
-All commands can be run using the provided [Makefile](./Makefile). However, it may be instructive to look at the commands to gain a greater understanding of how Vault registers plugins. Using the Makefile will result in running the Vault server in `dev` mode. Do not run Vault in `dev` mode in production. The `dev` server allows you to configure the plugin directory as a flag, and automatically registers plugin binaries in that directory. In production, plugin binaries must be manually registered.
-
-This will build the plugin binary and start the Vault dev server:
-```
-# Build Mock plugin and start Vault dev server with plugin automatically registered
-$ make
-```
+Using this plugin, you limit the accidental exposure window of Artifactory tokens; useful for continuous
+integration servers.
 
-Now open a new terminal window and run the following commands:
-```
-# Open a new terminal window and export Vault dev server http address
-$ export VAULT_ADDR='http://127.0.0.1:8200'
-
-# Enable the Mock plugin
-$ make enable
-
-# Write a secret to the Mock secrets engine
-$ vault write mock/test hello="world"
-Success! Data written to: mock/test
+## Usage
 
-# Retrieve secret from Mock secrets engine
-$ vault read mock/test
-Key      Value
----      -----
-hello    world
+```bash
+$ vault secrets enable artifactory
+
+# Also supports max_ttl= and default_ttl=
+$ vault write artifactory/config/admin \
+               url=https://artifactory.example.org \
+               access_token=0ab31978246345871028973fbcdeabcfadecbadef
+
+# Also supports grant_type=, and audience= (see JFrog documentation)
+$ vault write artifactory/roles/jenkins \
+               username="example-service-jenkins" \
+               scope="api:* member-of-groups:ci-server" \
+               refreshable=true \
+               default_ttl=1h max_ttl=3h 
+
+$ vault list artifactory/roles
+Keys
+----
+jenkins
+
+$ vault write -force artifactory/token/jenkins 
+Key                Value
+---                -----
+lease_id           artifactory/token/jenkins/25jYH8DjUU548323zPWiSakh
+access_token       adsdgbtybbeeyh...
+refreshable        true
+role               jenkins
+scope              api:* member-of-groups:ci-server
 ```
 
-## License
-
-Mock was contributed to the HashiCorp community by [hasheddan](https://github.com/hasheddan/vault-plugin-secrets-covert). In doing so, the original license has been removed.
diff --git a/artifactory.go b/artifactory.go
@@ -3,11 +3,12 @@ package artifactory
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/hashicorp/vault/sdk/logical"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
+
+	"github.com/hashicorp/vault/sdk/logical"
 )
 
 func (b *backend) revokeToken(config adminConfiguration, secret logical.Secret) error {
@@ -48,7 +49,7 @@ func (b *backend) refreshToken(config adminConfiguration, accessToken, refreshTo
 
 	if resp.StatusCode != http.StatusOK {
 		b.Backend.Logger().Warn("got status code", "statusCode", resp.StatusCode, "response", resp)
-		return nil, fmt.Errorf("could not create access token: HTTP response %v", resp.StatusCode)
+		return nil, fmt.Errorf("could not refresh access token: HTTP response %v", resp.StatusCode)
 	}
 
 	var createdToken createTokenResponse
@@ -60,17 +61,18 @@ func (b *backend) refreshToken(config adminConfiguration, accessToken, refreshTo
 	return &createdToken, nil
 }
 
-func (b *backend) createToken(config adminConfiguration, role artifactoryRole, TTL, maxTTL time.Duration) (*createTokenResponse, error) {
+func (b *backend) createToken(config adminConfiguration, role artifactoryRole, ttl, maxTTL time.Duration) (*createTokenResponse, error) {
 	values := url.Values{}
+
 	if role.GrantType != "" {
 		values.Set("grant_type", role.GrantType)
 	}
 
 	values.Set("username", role.Username)
 	values.Set("scope", role.Scope)
 
-	if TTL > 0 {
-		values.Set("expires_in", fmt.Sprintf("%d", int64(TTL.Seconds())))
+	if ttl > 0 {
+		values.Set("expires_in", fmt.Sprintf("%d", int64(ttl.Seconds())))
 	} else if maxTTL > 0 {
 		values.Set("expires_in", fmt.Sprintf("%d", int64(maxTTL.Seconds())))
 	}
@@ -115,6 +117,7 @@ func (b *backend) performArtifactoryRequest(config adminConfiguration, path stri
 	if err != nil {
 		return nil, err
 	}
+
 	req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", config.AccessToken))
 	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
 

diff --git a/artifactory_test.go b/artifactory_test.go
@@ -3,19 +3,19 @@ package artifactory
 import (
 	"bytes"
 	"context"
-	"github.com/hashicorp/vault/sdk/logical"
-	"github.com/stretchr/testify/assert"
 	"io/ioutil"
 	"net/http"
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/stretchr/testify/assert"
 )
 
 // Test that the HTTP request sent to Artifactory matches what the docs say, and that
 // handling the response translates into a proper response.
 func TestBackend_CreateTokenSuccess(t *testing.T) {
-
 	b, config := configuredBackend(t, map[string]interface{}{
 		"access_token": "test-access-token",
 		"url":          "https://127.0.0.1",
@@ -88,7 +88,6 @@ func TestBackend_CreateTokenSuccess(t *testing.T) {
 
 // Test that an error is returned if Artifactory is unavailable.
 func TestBackend_CreateTokenArtifactoryUnavailable(t *testing.T) {
-
 	b, config := configuredBackend(t, map[string]interface{}{
 		"access_token": "test-access-token",
 		"url":          "https://127.0.0.1",
@@ -130,7 +129,6 @@ func TestBackend_CreateTokenArtifactoryUnavailable(t *testing.T) {
 
 // Test that an error is returned if the access token is invalid for the operation being performed.
 func TestBackend_CreateTokenUnauthorized(t *testing.T) {
-
 	b, config := configuredBackend(t, map[string]interface{}{
 		"access_token": "test-access-token",
 		"url":          "https://127.0.0.1",
@@ -177,7 +175,6 @@ func TestBackend_CreateTokenUnauthorized(t *testing.T) {
 // Test that an error is returned when the nginx in front of Artifactory can't reach Artifactory.
 // It happens.
 func TestBackend_CreateTokenArtifactoryMisconfigured(t *testing.T) {
-
 	b, config := configuredBackend(t, map[string]interface{}{
 		"access_token": "test-access-token",
 		"url":          "https://127.0.0.1",

diff --git a/backend.go b/backend.go
@@ -18,7 +18,7 @@ type backend struct {
 	httpClient  *http.Client
 }
 
-// Factory configures and returns Artifactory secrets backends
+// Factory configures and returns Artifactory secrets backends.
 func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) {
 	if conf == nil {
 		return nil, fmt.Errorf("configuration passed into backend is nil")
@@ -56,6 +56,7 @@ func Backend(_ *logical.BackendConfig) (*backend, error) {
 		b.pathRoles(),
 		b.pathTokenCreate(),
 		b.pathConfig())
+
 	return b, nil
 }
 

diff --git a/cmd/artifactory/main.go b/cmd/artifactory/main.go
@@ -1,9 +1,10 @@
 package main
 
 import (
-	artifactory "github.com/idcmp/artifactory-secrets-plugin"
 	"os"
 
+	artifactory "github.com/idcmp/artifactory-secrets-plugin"
+
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/sdk/plugin"
@@ -12,10 +13,9 @@ import (
 func main() {
 	logger := hclog.New(&hclog.LoggerOptions{})
 
-	//TODO
-	logger.Error("Hello")
 	apiClientMeta := &api.PluginAPIClientMeta{}
 	flags := apiClientMeta.FlagSet()
+
 	if err := flags.Parse(os.Args[1:]); err != nil {
 		logger.Error("could not parse flags", "error", err)
 		os.Exit(1)

diff --git a/go.mod b/go.mod
@@ -26,6 +26,7 @@ require (
 	golang.org/x/net v0.0.0-20200602114024-627f9648deb9 // indirect
 	golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 // indirect
 	golang.org/x/text v0.3.2 // indirect
+	golang.org/x/tools v0.0.0-20200606014950-c42cb6316fb6 // indirect
 	google.golang.org/genproto v0.0.0-20200603110839-e855014d5736 // indirect
 	google.golang.org/grpc v1.29.1 // indirect
 )
diff --git a/go.sum b/go.sum
@@ -159,21 +159,27 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200602180216-279210d13fed h1:g4KENRiCMEx58Q7/ecwfT0N2o8z35Fnbsjig/Alf2T4=
 golang.org/x/crypto v0.0.0-20200602180216-279210d13fed/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
 golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
 golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
+golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
+golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200602114024-627f9648deb9 h1:pNX+40auqi2JqRfOP1akLGtYcn15TUbkhwuCO3foqqM=
 golang.org/x/net v0.0.0-20200602114024-627f9648deb9/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
@@ -182,6 +188,7 @@ golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190129075346-302c3dd5f1cc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
@@ -209,7 +216,14 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm
 golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
 golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
+golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 h1:5Beo0mZN8dRzgrMMkDp0jc8YXQKx9DiJ2k1dkvGsn5A=
 golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
+golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/tools v0.0.0-20200606014950-c42cb6316fb6 h1:5Y8c5HBW6hBYnGEE3AbJPV0R8RsQmg1/eaJrpvasns0=
+golang.org/x/tools v0.0.0-20200606014950-c42cb6316fb6/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=

diff --git a/path_config.go b/path_config.go
@@ -3,9 +3,10 @@ package artifactory
 import (
 	"context"
 	"crypto/sha256"
+	"time"
+
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/logical"
-	"time"
 )
 
 func (b *backend) pathConfig() *framework.Path {
@@ -25,30 +26,47 @@ func (b *backend) pathConfig() *framework.Path {
 			"max_ttl": {
 				Type:        framework.TypeDurationSecond,
 				Description: "Maximum duration any lease issued by this backend can be.",
-				Default:     time.Duration(1 * time.Hour),
+				Default:     1 * time.Hour,
 			},
 			"default_ttl": {
 				Type:        framework.TypeDurationSecond,
 				Description: "Default TTL when no other TTL is specified",
-				Default:     time.Duration(1 * time.Hour),
+				Default:     1 * time.Hour,
 			},
 		},
 		Operations: map[logical.Operation]framework.OperationHandler{
 			logical.UpdateOperation: &framework.PathOperation{
 				Callback: b.pathConfigUpdate,
-				Summary:  "FIXME",
+				Summary:  "Configure the Artifactory secrets backend.",
 			},
 			logical.DeleteOperation: &framework.PathOperation{
 				Callback: b.pathConfigDelete,
-				Summary:  "FIXME",
+				Summary:  "Delete the Artifactory secrets configuration.",
 			},
 			logical.ReadOperation: &framework.PathOperation{
 				Callback: b.pathConfigRead,
-				Summary:  "FIXME",
+				Summary:  "Examine the Artifactory secrets configuration.",
 			},
 		},
-		HelpSynopsis:    `FIXME`,
-		HelpDescription: `FIXME`,
+		HelpSynopsis: `Interact with the Artifactory secrets configuration.`,
+		HelpDescription: `
+Configure the parameters used to connect to the Artifactory server integrated with this backend. The two main
+parameters are "url" which is the absolute URL to the Artifactory server. Note that "/api" is prepended by the
+individual calls, so do not include it in the URL here.
+
+The second is "access_token" which must be an access token powerful enough to generate the other access tokens you'll
+be using. This value is stored seal wrapped when available. Once set, the access token cannot be retrieved, but the backend
+will send a sha256 hash of the token so you can compare it to your notes. 
+
+There are two TTL related parameters. The ultimate maximum lifetime that a token can live is controlled by "max_ttl", the
+backend will refuse to renew/refresh tokens beyond max_ttl. The second is "default_ttl" which is used when a role does
+not also specify a TTL. Default TTL must not be greater than Max TTL, and Max TTL must not be greater than the system-wide
+Max TTL defined in Vault's configuration. 
+
+Note that ultimately the maximum TTL is calculated as the time of issuing the access token.
+
+No renewals or new tokens will be issued if the backend configuration (config/admin) is deleted.
+`,
 	}
 }
 
@@ -60,7 +78,6 @@ type adminConfiguration struct {
 }
 
 func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
-
 	b.configMutex.Lock()
 	defer b.configMutex.Unlock()
 
@@ -86,6 +103,7 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da
 		if config.MaxTTL > b.Backend.System().MaxLeaseTTL() {
 			return logical.ErrorResponse("max_ttl exceeds system max_ttl"), nil
 		}
+
 		if config.DefaultTTL > b.Backend.System().MaxLeaseTTL() {
 			return logical.ErrorResponse("default_ttl exceeds system max_ttl"), nil
 		}
@@ -112,7 +130,7 @@ func (b *backend) pathConfigDelete(ctx context.Context, req *logical.Request, _
 		return nil, err
 	}
 
-	return logical.ErrorResponse("FIXME"), nil
+	return nil, nil
 }
 
 func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) {