A bunch of little fixes (#23)

hashicorp · Feb 17, 2021 · 6449f92 · 6449f92
1 parent 65eba2f
commit 6449f92
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 8 deletions.
diff --git a/path_dynamic_creds.go b/path_dynamic_creds.go
@@ -68,7 +68,7 @@ func (b *backend) pathDynamicCredsRead(ctx context.Context, req *logical.Request
 		merr := multierror.Append(fmt.Errorf("failed to create user: %w", err))
 		_, err = b.executeLDIF(config.LDAP, dRole.RollbackLDIF, templateData, true)
 		if err != nil {
-			merr = multierror.Append(fmt.Errorf("failed to roll back user creation: %w", err))
+			merr = multierror.Append(merr, fmt.Errorf("failed to roll back user creation: %w", err))
 		}
 		return nil, merr
 	}

diff --git a/path_dynamic_roles.go b/path_dynamic_roles.go
@@ -19,7 +19,7 @@ const (
 	secretCredsType = "creds"
 
 	dynamicRolePath = "role/"
-	dynamicCredPath = "cred/"
+	dynamicCredPath = "creds/"
 )
 
 func (b *backend) pathDynamicRoles() []*framework.Path {

diff --git a/path_static_roles.go b/path_static_roles.go
@@ -3,7 +3,6 @@ package openldap
 import (
 	"context"
 	"fmt"
-	"path"
 	"time"
 
 	"github.com/hashicorp/vault/sdk/framework"
@@ -19,7 +18,7 @@ const (
 func (b *backend) pathListStaticRoles() []*framework.Path {
 	return []*framework.Path{
 		{
-			Pattern: path.Join(staticRolePath, framework.OptionalParamRegex("prefix")),
+			Pattern: staticRolePath + "?$",
 			Operations: map[logical.Operation]framework.OperationHandler{
 				logical.ListOperation: &framework.PathOperation{
 					Callback: b.pathStaticRoleList,

diff --git a/scripts/acceptance-tests.bats b/scripts/acceptance-tests.bats
@@ -221,7 +221,7 @@ teardown() {
   run vault read openldap/role/testrole
   [ ${status} -ne 0 ]
 
-  run vault read openldap/cred/testrole
+  run vault read openldap/creds/testrole
   [ ${status} -ne 0 ]
 }
 
@@ -275,11 +275,11 @@ teardown() {
   max_ttl=20
 
   # Create role
-  run vault write openldap/role/testrole creation_ldif="${creation_ldif}" deletion_ldif="${deletion_ldif}" default_ttl="${default_ttl}s" max_ttl="${max_ttl}s"
+  run vault write openldap/role/testrole creation_ldif="${creation_ldif}" deletion_ldif="${deletion_ldif}" rollback_ldif="${deletion_ldif}" default_ttl="${default_ttl}s" max_ttl="${max_ttl}s"
   [ ${status} -eq 0 ]
 
   # Get credentials
-  run vault read -format=json openldap/cred/testrole
+  run vault read -format=json openldap/creds/testrole
   [ ${status} -eq 0 ]
 
 
@@ -331,7 +331,7 @@ teardown() {
 
   # Get credentials
   log "Generating credentials..."
-  run vault read -format=json openldap/cred/testrole
+  run vault read -format=json openldap/creds/testrole
   [ ${status} -eq 0 ]
 
   lease_id=$(echo "${output}" | jq -r .lease_id)
@@ -404,3 +404,32 @@ teardown() {
   run ldapsearch -b "${dn}" -D "${dn}" -w "${password}"
   [ ${status} -ne 0 ]
 }
+
+@test "Dynamic Secrets - Useful error on creation failure" {
+  default_ttl=10
+  max_ttl=20
+
+  bad_creation_ldif='dn: cn={{.Username}},ou=thisgroupdoesnotexist,dc=learn,dc=example
+objectClass: person
+objectClass: top
+cn: learn
+sn: learn-{{.Username | utf16le | base64}}
+memberOf: cn=dev,ou=groups,dc=learn,dc=example
+userPassword: {{.Password}}'
+
+  # Create role
+  run vault write openldap/role/testrole creation_ldif="${bad_creation_ldif}" deletion_ldif="${deletion_ldif}" rollback_ldif="${deletion_ldif}" default_ttl="${default_ttl}s" max_ttl="${max_ttl}s"
+  [ ${status} -eq 0 ]
+
+  # Get credentials
+  run vault read -format=json openldap/creds/testrole
+  [ ${status} -ne 0 ]
+  [[ "${output}" == *"failed to create user" ]]
+
+  # Optional assertion that makes sure both errors are included but if this becomes flaky it isn't the important error and can be removed
+  [[ "${output}" == *"failed to roll back user" ]]
+
+  ## Assert the credentials do *not* work in OpenLDAP
+  run ldapsearch -b "${dn}" -D "${dn}" -w "${password}"
+  [ ${status} -ne 0 ]
+}