Merge pull request #43 from Venafi/split-fuctions-refatoring

Split fuctions refatoring
Venafi · Mar 13, 2020 · 7ae91bf · 7ae91bf
2 parents 76b3714 + 8d89176
commit 7ae91bf
Show file tree

Hide file tree

Showing 8 changed files with 384 additions and 136 deletions.
diff --git a/go.mod b/go.mod
@@ -56,7 +56,7 @@ require (
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
 	github.com/hashicorp/consul/api v1.1.0 // indirect
 	github.com/hashicorp/go-gcp-common v0.5.0 // indirect
-	github.com/hashicorp/go-hclog v0.0.0-20181001195459-61d530d6c27f // indirect
+	github.com/hashicorp/go-hclog v0.0.0-20181001195459-61d530d6c27f
 	github.com/hashicorp/go-memdb v0.0.0-20181108192425-032f93b25bec // indirect
 	github.com/hashicorp/go-plugin v1.0.1-0.20190509212451-a1756f37cec6 // indirect
 	github.com/hashicorp/go-retryablehttp v0.5.0 // indirect

diff --git a/plugin/pki/backend_test.go b/plugin/pki/backend_test.go
@@ -61,7 +61,6 @@ func TestFakeStoreByOptions(t *testing.T) {
 	t.Run("read certificate by cn", integrationTestEnv.FakeReadCertificateByCN)
 	t.Run("delete role", integrationTestEnv.DeleteRole)
 
-
 	//test store_by default
 	t.Run("create role store_by serial", integrationTestEnv.FakeCreateRoleStoreBySerial)
 	t.Run("issue", integrationTestEnv.FakeIssueCertificateAndSaveSerial)
@@ -81,7 +80,6 @@ func TestFakeStoreByOptions(t *testing.T) {
 	t.Run("delete role", integrationTestEnv.DeleteRole)
 }
 
-
 //Testing Venafi Platform integration
 func TestTPPIntegration(t *testing.T) {
 

diff --git a/plugin/pki/env_test.go b/plugin/pki/env_test.go
@@ -197,6 +197,12 @@ func (e *testEnv) failToWriteRoleToBackend(t *testing.T, configString venafiConf
 	if resp != nil && !resp.IsError() {
 		t.Fatal("Role with mixed cloud api key and tpp url should fail to write")
 	}
+
+	errText := resp.Data["error"].(string)
+
+	if errText != errorTextTPPandCloudMixedCredentials {
+		t.Fatalf("Expecting error with text %s but got %s", errorTextTPPandCloudMixedCredentials, errText)
+	}
 }
 
 func (e *testEnv) listRolesInBackend(t *testing.T) {
@@ -460,7 +466,7 @@ func (e *testEnv) CheckThatThereIsNoCertificate(t *testing.T, certId string) {
 	}
 
 	const noCertError = "no entry found in path"
-	certContain := strings.Contains(err.Error(),noCertError)
+	certContain := strings.Contains(err.Error(), noCertError)
 	if !certContain {
 		t.Fatalf("error should contain %s substring but it is %s", noCertError, err)
 	}

diff --git a/plugin/pki/path_roles.go b/plugin/pki/path_roles.go
@@ -165,8 +165,15 @@ attached to them. Defaults to "false".`,
 }
 
 const (
-	storeByCNString     string = "cn"
-	storeBySerialString string = "serial"
+	storeByCNString                              = "cn"
+	storeBySerialString                          = "serial"
+	errorTextInvalidMode                         = "Invalid mode. fakemode or apikey or tpp credentials required"
+	errorTextValueMustBeLess                     = `"ttl" value must be less than "max_ttl" value`
+	errorTextTPPandCloudMixedCredentials         = `TPP credentials and Cloud API key can't be specified in one role`
+	errorTextStoreByAndStoreByCNOrSerialConflict = `Can't specify both story_by and store_by_cn or store_by_serial options '`
+	errorTextNoStoreAndStoreByCNOrSerialConflict = `Can't specify both no_store and store_by_cn or store_by_serial options '`
+	errorTextNoStoreAndStoreByConflict           = `Can't specify both no_store and store_by options '`
+	errTextStoreByWrongOption                    = "Option store_by can be %s or %s, not %s"
 )
 
 func (b *backend) getRole(ctx context.Context, s logical.Storage, n string) (*roleEntry, error) {
@@ -252,44 +259,60 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 		GenerateLease:    data.Get("generate_lease").(bool),
 		ServerTimeout:    time.Duration(data.Get("server_timeout").(int)) * time.Second,
 	}
+
+	err = validateEntry(entry)
+	if err != nil {
+		return logical.ErrorResponse(err.Error()), nil
+	}
+
+	// Store it
+	jsonEntry, err := logical.StorageEntryJSON("role/"+name, entry)
+	if err != nil {
+		return nil, err
+	}
+	if err := req.Storage.Put(ctx, jsonEntry); err != nil {
+		return nil, err
+	}
+
+	return nil, nil
+}
+
+func validateEntry(entry *roleEntry) (err error) {
 	if !entry.Fakemode && entry.Apikey == "" && (entry.TPPURL == "" || entry.TPPUser == "" || entry.TPPPassword == "") {
-		return logical.ErrorResponse("Invalid mode. fakemode or apikey or tpp credentials required"), nil
+		return fmt.Errorf(errorTextInvalidMode)
 	}
+
 	if entry.MaxTTL > 0 && entry.TTL > entry.MaxTTL {
-		return logical.ErrorResponse(
-			`"ttl" value must be less than "max_ttl" value`,
-		), nil
+		return fmt.Errorf(
+			errorTextValueMustBeLess,
+		)
 	}
 
 	if entry.TPPURL != "" && entry.Apikey != "" {
-		return logical.ErrorResponse(
-			`TPP url and Cloud API key can't be specified in one role`,
-		), nil
+		return fmt.Errorf(errorTextTPPandCloudMixedCredentials)
+	}
+
+	if entry.TPPUser != "" && entry.Apikey != "" {
+		return fmt.Errorf(errorTextTPPandCloudMixedCredentials)
 	}
 
 	if (entry.StoreByCN || entry.StoreBySerial) && entry.StoreBy != "" {
-		return logical.ErrorResponse(
-			`Can't specify both story_by and store_by_cn or store_by_serial options '`,
-		), nil
+		return fmt.Errorf(errorTextStoreByAndStoreByCNOrSerialConflict)
 	}
 
 	if (entry.StoreByCN || entry.StoreBySerial) && entry.NoStore {
-		return logical.ErrorResponse(
-			`Can't specify both no_store and store_by_cn or store_by_serial options '`,
-		), nil
+		return fmt.Errorf(errorTextNoStoreAndStoreByCNOrSerialConflict)
 	}
 
 	if entry.StoreBy != "" && entry.NoStore {
-		return logical.ErrorResponse(
-			`Can't specify both no_store and store_by options '`,
-		), nil
+		return fmt.Errorf(errorTextNoStoreAndStoreByConflict)
 	}
 
 	if entry.StoreBy != "" {
 		if (entry.StoreBy != storeBySerialString) && (entry.StoreBy != storeByCNString) {
-			return logical.ErrorResponse(
-				fmt.Sprintf("Option store_by can be %s or %s, not %s", storeBySerialString, storeByCNString, entry.StoreBy),
-			), nil
+			return fmt.Errorf(
+				fmt.Sprintf(errTextStoreByWrongOption, storeBySerialString, storeByCNString, entry.StoreBy),
+			)
 		}
 	}
 
@@ -302,16 +325,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 		entry.StoreBy = storeByCNString
 	}
 
-	// Store it
-	jsonEntry, err := logical.StorageEntryJSON("role/"+name, entry)
-	if err != nil {
-		return nil, err
-	}
-	if err := req.Storage.Put(ctx, jsonEntry); err != nil {
-		return nil, err
-	}
-
-	return nil, nil
+	return nil
 }
 
 type roleEntry struct {

diff --git a/plugin/pki/path_roles_test.go b/plugin/pki/path_roles_test.go
@@ -0,0 +1,161 @@
+package pki
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestRoleValidate(t *testing.T) {
+
+	entry := &roleEntry{
+		TPPURL: "https://ha-tpp12.sqlha.com:5008/vedsdk",
+	}
+
+	err := validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextInvalidMode {
+		t.Fatalf("Expecting error %s but got %s", errorTextInvalidMode, err)
+	}
+
+	entry = &roleEntry{
+		TPPURL:      "https://qa-tpp.exmple.com/vedsdk",
+		TPPUser:     "admin",
+		TPPPassword: "xxxx",
+		TTL:         120,
+		MaxTTL:      100,
+	}
+
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextValueMustBeLess {
+		t.Fatalf("Expecting error %s but got %s", errorTextValueMustBeLess, err)
+	}
+
+	entry = &roleEntry{
+		TPPURL:      "https://qa-tpp.exmple.com/vedsdk",
+		Apikey:      "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		TPPUser:     "admin",
+		TPPPassword: "xxxx",
+	}
+
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextTPPandCloudMixedCredentials {
+		t.Fatalf("Expecting error %s but got %s", errorTextTPPandCloudMixedCredentials, err)
+	}
+
+	entry = &roleEntry{
+		Apikey:    "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreByCN: true,
+		StoreBy:   "cn",
+	}
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextStoreByAndStoreByCNOrSerialConflict {
+		t.Fatalf("Expecting error %s but got %s", errorTextStoreByAndStoreByCNOrSerialConflict, err)
+	}
+
+	entry = &roleEntry{
+		Apikey:        "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreBySerial: true,
+		StoreBy:       "cn",
+	}
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextStoreByAndStoreByCNOrSerialConflict {
+		t.Fatalf("Expecting error %s but got %s", errorTextStoreByAndStoreByCNOrSerialConflict, err)
+	}
+
+	entry = &roleEntry{
+		Apikey:        "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreBySerial: true,
+		StoreByCN:     true,
+		StoreBy:       "cn",
+	}
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextStoreByAndStoreByCNOrSerialConflict {
+		t.Fatalf("Expecting error %s but got %s", errorTextStoreByAndStoreByCNOrSerialConflict, err)
+	}
+
+	entry = &roleEntry{
+		Apikey:        "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreBySerial: true,
+		StoreByCN:     true,
+		NoStore:       true,
+	}
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextNoStoreAndStoreByCNOrSerialConflict {
+		t.Fatalf("Expecting error %s but got %s", errorTextNoStoreAndStoreByCNOrSerialConflict, err)
+	}
+
+	entry = &roleEntry{
+		Apikey:  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreBy: "serial",
+		NoStore: true,
+	}
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	if err.Error() != errorTextNoStoreAndStoreByConflict {
+		t.Fatalf("Expecting error %s but got %s", errorTextNoStoreAndStoreByConflict, err)
+	}
+
+	entry = &roleEntry{
+		Apikey:  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreBy: "sebial",
+	}
+	err = validateEntry(entry)
+	if err == nil {
+		t.Fatalf("Expecting error")
+	}
+	expectingError := fmt.Sprintf(errTextStoreByWrongOption, storeBySerialString, storeByCNString, "sebial")
+	if err.Error() != expectingError {
+		t.Fatalf("Expecting error %s but got %s", expectingError, err)
+	}
+
+	entry = &roleEntry{
+		Apikey:        "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreBySerial: true,
+		StoreByCN:     true,
+
+	}
+	err = validateEntry(entry)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if entry.StoreBy != storeBySerialString {
+		t.Fatalf("Expecting store_by parameter will be set to %s", storeBySerialString)
+	}
+
+	entry = &roleEntry{
+		Apikey:        "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+		StoreByCN:     true,
+
+	}
+	err = validateEntry(entry)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if entry.StoreBy != storeByCNString {
+		t.Fatalf("Expecting store_by parameter will be set to %s", storeBySerialString)
+	}
+}