Merge pull request #88 from TJM/feat/more-rotate-tests

test: improve test coverage

.gitignore

@@ -4,4 +4,4 @@
 
 .DS_Store
 dist/
-coverage.txt
+/coverage.*

Makefile

@@ -57,6 +57,11 @@ acceptance:
 	export JFROG_ACCESS_TOKEN=$(JFROG_ACCESS_TOKEN) && \
 		go test -run TestAcceptance -cover -coverprofile=coverage.txt -v -p 1 -timeout 5m ./...
 
+alltests:
+	export VAULT_ACC=true && \
+	export JFROG_ACCESS_TOKEN=$(JFROG_ACCESS_TOKEN) && \
+		go test -cover -coverprofile=coverage.out -v -p 1 -timeout 5m ./...
+
 clean:
 	rm -f $(PLUGIN_DIR)/$(PLUGIN_FILE)
 

path_config.go

@@ -104,7 +104,6 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da
 	}
 
 	if val, ok := data.GetOk("use_expiring_tokens"); ok {
-		b.Logger().Warn("config update use_expiring_tokens", "use_expiring_tokens", val)
 		switch exp := val.(type) {
 		case bool:
 			config.UseExpiringTokens = exp

path_config_rotate_test.go

@@ -3,6 +3,7 @@ package artifactory
 import (
 	"testing"
 
+	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -11,10 +12,30 @@ func TestAcceptanceBackend_PathRotate(t *testing.T) {
 		t.SkipNow()
 	}
 
+	// Unconfigured Test
+	unconfigured, err := newAcceptanceTestEnv()
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Run("unconfigured", unconfigured.PathConfigRotateUnconfigured)
+
+	//Configured Tests
 	e := NewConfiguredAcceptanceTestEnv(t)
+	t.Run("zeroLengthUsername", e.PathConfigRotateZeroLengthUsername)
 	t.Run("empty", e.PathConfigRotateEmpty)
 	t.Run("withDetails", e.PathConfigRotateWithDetails)
+	// Cleanup Token
 	e.Cleanup(t)
+
+	// Failure Tests
+	t.Run("CreateTokenErr", e.PathConfigRotateCreateTokenErr)
+	t.Run("badAccessToken", e.PathConfigRotateBadAccessToken)
+}
+
+func (e *accTestEnv) PathConfigRotateUnconfigured(t *testing.T) {
+	resp, err := e.update("config/rotate", testData{})
+	assert.Contains(t, resp.Data["error"], "backend not configured")
+	assert.NoError(t, err)
 }
 
 func (e *accTestEnv) PathConfigRotateEmpty(t *testing.T) {
@@ -24,6 +45,14 @@ func (e *accTestEnv) PathConfigRotateEmpty(t *testing.T) {
 	assert.NotEqual(t, before["access_token_sha256sum"], after["access_token_sha256"])
 }
 
+func (e *accTestEnv) PathConfigRotateZeroLengthUsername(t *testing.T) {
+	e.UpdateConfigRotate(t, testData{
+		"username": "",
+	}) // empty write
+	after := e.ReadConfigAdmin(t)
+	assert.Equal(t, "admin-vault-secrets-artifactory", after["username"])
+}
+
 func (e *accTestEnv) PathConfigRotateWithDetails(t *testing.T) {
 	newUsername := "vault-acceptance-test-changed"
 	description := "Artifactory Secrets Engine Accceptance Test"
@@ -37,3 +66,31 @@ func (e *accTestEnv) PathConfigRotateWithDetails(t *testing.T) {
 	assert.Equal(t, newUsername, after["username"])
 	// Not testing Description, because it is not returned in the token (yet)
 }
+
+func (e *accTestEnv) PathConfigRotateCreateTokenErr(t *testing.T) {
+	tokenId, accessToken := e.createNewNonAdminTestToken(t)
+	e.UpdateConfigAdmin(t, testData{
+		"access_token": accessToken,
+		"url":          e.URL,
+	})
+	resp, err := e.update("config/rotate", testData{})
+	assert.NotNil(t, resp)
+	assert.Contains(t, resp.Data["error"], "error creating new token")
+	assert.ErrorContains(t, err, "could not create access token")
+	e.revokeTestToken(t, e.AccessToken, tokenId)
+}
+
+func (e *accTestEnv) PathConfigRotateBadAccessToken(t *testing.T) {
+	// Forcibly set a bad token
+	entry, err := logical.StorageEntryJSON("config/admin", adminConfiguration{
+		AccessToken:    "bogus.token",
+		ArtifactoryURL: e.URL,
+	})
+	assert.NoError(t, err)
+	err = e.Storage.Put(e.Context, entry)
+	assert.NoError(t, err)
+	resp, err := e.update("config/rotate", testData{})
+	// assert.Contains(t, resp.Data["error"], "error parsing existing AccessToken")
+	assert.Contains(t, resp.Data["error"], "could not get the certificate")
+	assert.Error(t, err)
+}

path_config_test.go

@@ -19,9 +19,115 @@ func TestAcceptanceBackend_PathConfig(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	t.Run("notConfigured", accTestEnv.PathConfigReadUnconfigured)
 	t.Run("update", accTestEnv.UpdatePathConfig)
 	t.Run("read", accTestEnv.ReadPathConfig)
+	t.Run("expiringTokens", accTestEnv.PathConfigUpdateExpiringTokens)
+	t.Run("usernameTemplate", accTestEnv.PathConfigUpdateUsernameTemplate)
 	t.Run("delete", accTestEnv.DeletePathConfig)
+	t.Run("errors", accTestEnv.PathConfigUpdateErrors)
+	t.Run("badAccessToken", accTestEnv.PathConfigReadBadAccessToken)
+
+}
+
+func (e *accTestEnv) PathConfigReadUnconfigured(t *testing.T) {
+	resp, err := e.read("config/admin")
+	assert.Contains(t, resp.Data["error"], "backend not configured")
+	assert.NoError(t, err)
+}
+
+func (e *accTestEnv) PathConfigUpdateExpiringTokens(t *testing.T) {
+	// Boolean (not working as expected)
+	e.UpdateConfigAdmin(t, testData{
+		"use_expiring_tokens": true,
+	})
+	data := e.ReadConfigAdmin(t)
+	assert.Equal(t, data["use_expiring_tokens"], true)
+	e.UpdateConfigAdmin(t, testData{
+		"use_expiring_tokens": false,
+	})
+	data = e.ReadConfigAdmin(t)
+	assert.Equal(t, data["use_expiring_tokens"], false)
+	// String
+	e.UpdateConfigAdmin(t, testData{
+		"use_expiring_tokens": "true",
+	})
+	data = e.ReadConfigAdmin(t)
+	assert.Equal(t, data["use_expiring_tokens"], true)
+	e.UpdateConfigAdmin(t, testData{
+		"use_expiring_tokens": "false",
+	})
+	data = e.ReadConfigAdmin(t)
+	assert.Equal(t, data["use_expiring_tokens"], false)
+	// Fail Tests
+	resp, err := e.update("config/admin", testData{
+		"use_expiring_tokens": "Sure, why not",
+	})
+	assert.NotNil(t, resp)
+	assert.Contains(t, resp.Data["error"], "error parsing use_expired_tokens string to bool")
+	assert.ErrorContains(t, err, "strconv.ParseBool")
+}
+
+func (e *accTestEnv) PathConfigUpdateUsernameTemplate(t *testing.T) {
+	usernameTemplate := "v_{{.DisplayName}}_{{.RoleName}}_{{random 10}}_{{unix_time}}"
+	e.UpdateConfigAdmin(t, testData{
+		"username_template": usernameTemplate,
+	})
+	data := e.ReadConfigAdmin(t)
+	assert.Equal(t, data["username_template"], usernameTemplate)
+
+	// Bad Template
+	resp, err := e.update("config/admin", testData{
+		"username_template": "bad_{{ .somethingInvalid }}_testing {{",
+	})
+	assert.NotNil(t, resp)
+	assert.Contains(t, resp.Data["error"], "username_template error")
+	assert.ErrorContains(t, err, "username_template")
+}
+
+// most of these were covered by unit tests, but we want test coverage for acceptance
+func (e *accTestEnv) PathConfigUpdateErrors(t *testing.T) {
+	// Access Token Required
+	resp, err := e.update("config/admin", testData{
+		"url": e.URL,
+	})
+	assert.NoError(t, err)
+	assert.NotNil(t, resp)
+	assert.True(t, resp.IsError())
+	assert.Contains(t, resp.Error().Error(), "access_token")
+	// URL Required
+	resp, err = e.update("config/admin", testData{
+		"access_token": "test-access-token",
+	})
+	assert.NoError(t, err)
+	assert.NotNil(t, resp)
+	assert.True(t, resp.IsError())
+	assert.Contains(t, resp.Error().Error(), "url")
+	// Bad Token
+	resp, err = e.update("config/admin", testData{
+		"access_token": "test-access-token",
+		"url":          e.URL,
+	})
+	assert.NotNil(t, resp)
+	assert.True(t, resp.IsError())
+	assert.Contains(t, resp.Error().Error(), "Unable to get Artifactory Version")
+	assert.ErrorContains(t, err, "could not get the system version")
+}
+
+func (e *accTestEnv) PathConfigReadBadAccessToken(t *testing.T) {
+	// Forcibly set a bad token
+	entry, err := logical.StorageEntryJSON("config/admin", adminConfiguration{
+		AccessToken:    "bogus.token",
+		ArtifactoryURL: e.URL,
+	})
+	assert.NoError(t, err)
+	err = e.Storage.Put(e.Context, entry)
+	assert.NoError(t, err)
+	resp, err := e.read("config/admin")
+
+	assert.NoError(t, err)
+	assert.NotNil(t, resp)
+	// Otherwise success, we don't need to re-test for this
 }
 
 func TestBackend_AccessTokenRequired(t *testing.T) {

test_utils.go

@@ -54,6 +54,33 @@ func (e *accTestEnv) createNewTestToken(t *testing.T) (string, string) {
 	return resp.TokenId, resp.AccessToken
 }
 
+// createNewNonAdminTestToken creates a new "user" token using the one from test environment
+// primarily used to fail tests
+func (e *accTestEnv) createNewNonAdminTestToken(t *testing.T) (string, string) {
+	config := adminConfiguration{
+		AccessToken:    e.AccessToken,
+		ArtifactoryURL: e.URL,
+	}
+
+	role := artifactoryRole{
+		GrantType: "client_credentials",
+		Username:  "notTheAdmin",
+		Scope:     "applied-permissions/groups:readers",
+	}
+
+	err := e.Backend.(*backend).getVersion(config)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	resp, err := e.Backend.(*backend).CreateToken(config, role)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return resp.TokenId, resp.AccessToken
+}
+
 func (e *accTestEnv) revokeTestToken(t *testing.T, accessToken string, tokenID string) {
 	config := adminConfiguration{
 		AccessToken:    e.AccessToken,
@@ -87,13 +114,7 @@ func (e *accTestEnv) UpdatePathConfig(t *testing.T) {
 
 // UpdateConfigAdmin will send a POST/PUT to the /config/admin endpoint with testData (vault write artifactory/config/admin)
 func (e *accTestEnv) UpdateConfigAdmin(t *testing.T, data testData) {
-	resp, err := e.Backend.HandleRequest(e.Context, &logical.Request{
-		Operation: logical.UpdateOperation,
-		Path:      "config/admin",
-		Storage:   e.Storage,
-		Data:      data,
-	})
-
+	resp, err := e.update("config/admin", data)
 	assert.NoError(t, err)
 	assert.Nil(t, resp)
 }
@@ -104,11 +125,7 @@ func (e *accTestEnv) ReadPathConfig(t *testing.T) {
 
 // ReadConfigAdmin will send a GET to the /config/admin endpoint (vault read artifactory/config/admin)
 func (e *accTestEnv) ReadConfigAdmin(t *testing.T) testData {
-	resp, err := e.Backend.HandleRequest(e.Context, &logical.Request{
-		Operation: logical.ReadOperation,
-		Path:      "config/admin",
-		Storage:   e.Storage,
-	})
+	resp, err := e.read("config/admin")
 
 	assert.NoError(t, err)
 	assert.NotNil(t, resp)
@@ -132,17 +149,30 @@ func (e *accTestEnv) DeleteConfigAdmin(t *testing.T) {
 	assert.Nil(t, resp)
 }
 
-// UpdateConfigRotate will send a POST/PUT to the /config/rotate endpoint with testData (vault write artifactory/config/rotate)
+// UpdateConfigRotate will send a POST/PUT to the /config/rotate endpoint with testData (vault write artifactory/config/rotate) and test for errors
 func (e *accTestEnv) UpdateConfigRotate(t *testing.T, data testData) {
-	resp, err := e.Backend.HandleRequest(e.Context, &logical.Request{
+	resp, err := e.update("config/rotate", data)
+	assert.NoError(t, err)
+	assert.Nil(t, resp)
+}
+
+// read will send a GET  to "path"
+func (e *accTestEnv) read(path string) (*logical.Response, error) {
+	return e.Backend.HandleRequest(e.Context, &logical.Request{
+		Operation: logical.ReadOperation,
+		Path:      "config/admin",
+		Storage:   e.Storage,
+	})
+}
+
+// update will send a POST/PUT to "path" with testData
+func (e *accTestEnv) update(path string, data testData) (*logical.Response, error) {
+	return e.Backend.HandleRequest(e.Context, &logical.Request{
 		Operation: logical.UpdateOperation,
-		Path:      "config/rotate",
+		Path:      path,
 		Storage:   e.Storage,
 		Data:      data,
 	})
-
-	assert.NoError(t, err)
-	assert.Nil(t, resp)
 }
 
 func (e *accTestEnv) CreatePathRole(t *testing.T) {