Use Vault KV2 (#1214)

* Use vault kv2 store

* Fix tests and replicate previous behavior

* Further test fixes

Makefile

@@ -43,7 +43,7 @@ dev-env-setup: sysdeps certs
 	@touch ./local/vault/approle.json && touch ./local/vault/roleid.txt && touch ./local/vault/vault.env
 	@make version/commit.txt && make version/version.txt
 
-dev-env:
+dev-env: dev-env-setup
 	@docker compose --profile dev up --force-recreate
 
 dev-env-cleanup:

internal/jimmtest/vault.go

@@ -49,5 +49,5 @@ func VaultClient(tb fatalF, prefix string) (*api.Client, string, string, string,
 	if !ok {
 		panic("failed to convert role secret ID to string")
 	}
-	return vaultClient, "/jimm-kv/", roleIDString, roleSecretIDString, true
+	return vaultClient, "jimm-kv", roleIDString, roleSecretIDString, true
 }

internal/vault/vault.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	goerr "errors"
 	"net/http"
 	"path"
 	"sync"
@@ -25,6 +26,13 @@ const (
 	passwordKey = "password"
 )
 
+const (
+	jwksKey        = "jwks"
+	jwksExpiryKey  = "jwks-expiry"
+	jwksPrivateKey = "jwks-private"
+	oAuthSecretKey = "oauth-secret"
+)
+
 // A VaultStore stores cloud credential attributes and
 // controller credentials in vault.
 type VaultStore struct {
@@ -58,11 +66,11 @@ func (s *VaultStore) Get(ctx context.Context, tag names.CloudCredentialTag) (map
 		return nil, errors.E(op, err)
 	}
 
-	secret, err := client.Logical().ReadWithContext(ctx, s.path(tag))
-	if err != nil {
+	secret, err := client.KVv2(s.KVPath).Get(ctx, s.path(tag))
+	if err != nil && goerr.Unwrap(err) != api.ErrSecretNotFound {
 		return nil, errors.E(op, err)
 	}
-	if secret == nil {
+	if secret == nil || secret.Data == nil {
 		return nil, nil
 	}
 	attr := make(map[string]string, len(secret.Data))
@@ -95,7 +103,7 @@ func (s *VaultStore) Put(ctx context.Context, tag names.CloudCredentialTag, attr
 	for k, v := range attr {
 		data[k] = v
 	}
-	_, err = client.Logical().WriteWithContext(ctx, s.path(tag), data)
+	_, err = client.KVv2(s.KVPath).Put(ctx, s.path(tag), data)
 	if err != nil {
 		return errors.E(op, err)
 	}
@@ -111,7 +119,7 @@ func (s *VaultStore) delete(ctx context.Context, tag names.CloudCredentialTag) e
 	if err != nil {
 		return errors.E(op, err)
 	}
-	_, err = client.Logical().DeleteWithContext(ctx, s.path(tag))
+	err = client.KVv2(s.KVPath).Delete(ctx, s.path(tag))
 	if rerr, ok := err.(*api.ResponseError); ok && rerr.StatusCode == http.StatusNotFound {
 		// Ignore the error if attempting to delete something that isn't there.
 		err = nil
@@ -132,11 +140,11 @@ func (s *VaultStore) GetControllerCredentials(ctx context.Context, controllerNam
 		return "", "", errors.E(op, err)
 	}
 
-	secret, err := client.Logical().ReadWithContext(ctx, s.controllerCredentialsPath(controllerName))
-	if err != nil {
+	secret, err := client.KVv2(s.KVPath).Get(ctx, s.controllerCredentialsPath(controllerName))
+	if err != nil && goerr.Unwrap(err) != api.ErrSecretNotFound {
 		return "", "", errors.E(op, err)
 	}
-	if secret == nil {
+	if secret == nil || secret.Data == nil {
 		return "", "", nil
 	}
 	var username, password string
@@ -168,7 +176,7 @@ func (s *VaultStore) PutControllerCredentials(ctx context.Context, controllerNam
 		usernameKey: username,
 		passwordKey: password,
 	}
-	_, err = client.Logical().WriteWithContext(ctx, s.controllerCredentialsPath(controllerName), data)
+	_, err = client.KVv2(s.KVPath).Put(ctx, s.controllerCredentialsPath(controllerName), data)
 	if err != nil {
 		return errors.E(op, err)
 	}
@@ -185,9 +193,9 @@ func (s *VaultStore) CleanupJWKS(ctx context.Context) error {
 	}
 	// Vault does not return errors on deletion requests where
 	// the secret does not exist. As such we just return the last known error.
-	client.Logical().DeleteWithContext(ctx, s.getJWKSExpiryPath())
-	client.Logical().DeleteWithContext(ctx, s.getJWKSPath())
-	if _, err = client.Logical().DeleteWithContext(ctx, s.getJWKSPrivateKeyPath()); err != nil {
+	client.KVv2(s.KVPath).Delete(ctx, s.getJWKSExpiryPath())
+	client.KVv2(s.KVPath).Delete(ctx, s.getJWKSPath())
+	if err = client.KVv2(s.KVPath).Delete(ctx, s.getJWKSPrivateKeyPath()); err != nil {
 		return errors.E(op, err)
 	}
 	return nil
@@ -202,26 +210,26 @@ func (s *VaultStore) GetJWKS(ctx context.Context) (jwk.Set, error) {
 		return nil, errors.E(op, err)
 	}
 
-	secret, err := client.Logical().ReadWithContext(ctx, s.getJWKSPath())
-	if err != nil {
+	secret, err := client.KVv2(s.KVPath).Get(ctx, s.getJWKSPath())
+	if err != nil && goerr.Unwrap(err) != api.ErrSecretNotFound {
 		return nil, errors.E(op, err)
 	}
 
 	// This is how the current version of vaults API Read works,
 	// if the secret is not present on the path, it will instead return
 	// nil for the secret and a nil error. So we must check for this.
-	if secret == nil {
+	if secret == nil || secret.Data == nil {
 		msg := "no JWKS exists yet."
 		zapctx.Debug(ctx, msg)
 		return nil, errors.E(op, errors.CodeNotFound, msg)
 	}
 
-	b, err := json.Marshal(secret.Data)
-	if err != nil {
-		return nil, errors.E(op, err)
+	jsonString, ok := secret.Data[jwksKey].(string)
+	if !ok {
+		return nil, errors.E(op, "invalid type for jwks")
 	}
 
-	ks, err := jwk.ParseString(string(b))
+	ks, err := jwk.ParseString(jsonString)
 	if err != nil {
 		return nil, errors.E(op, err)
 	}
@@ -238,18 +246,18 @@ func (s *VaultStore) GetJWKSPrivateKey(ctx context.Context) ([]byte, error) {
 		return nil, errors.E(op, err)
 	}
 
-	secret, err := client.Logical().ReadWithContext(ctx, s.getJWKSPrivateKeyPath())
-	if err != nil {
+	secret, err := client.KVv2(s.KVPath).Get(ctx, s.getJWKSPrivateKeyPath())
+	if err != nil && goerr.Unwrap(err) != api.ErrSecretNotFound {
 		return nil, errors.E(op, err)
 	}
 
-	if secret == nil {
+	if secret == nil || secret.Data == nil {
 		msg := "no JWKS private key exists yet."
 		zapctx.Debug(ctx, msg)
 		return nil, errors.E(op, errors.CodeNotFound, msg)
 	}
 
-	keyPemB64 := secret.Data["key"].(string)
+	keyPemB64 := secret.Data[jwksPrivateKey].(string)
 
 	keyPem, err := base64.StdEncoding.DecodeString(keyPemB64)
 	if err != nil {
@@ -268,18 +276,18 @@ func (s *VaultStore) GetJWKSExpiry(ctx context.Context) (time.Time, error) {
 		return now, errors.E(op, err)
 	}
 
-	secret, err := client.Logical().ReadWithContext(ctx, s.getJWKSExpiryPath())
-	if err != nil {
+	secret, err := client.KVv2(s.KVPath).Get(ctx, s.getJWKSExpiryPath())
+	if err != nil && goerr.Unwrap(err) != api.ErrSecretNotFound {
 		return now, errors.E(op, err)
 	}
 
-	if secret == nil {
+	if secret == nil || secret.Data == nil {
 		msg := "no JWKS expiry exists yet."
 		zapctx.Debug(ctx, msg)
 		return now, errors.E(op, errors.CodeNotFound, msg)
 	}
 
-	expiry, ok := secret.Data["jwks-expiry"].(string)
+	expiry, ok := secret.Data[jwksExpiryKey].(string)
 	if !ok {
 		return now, errors.E(op, "failed to retrieve expiry")
 	}
@@ -309,14 +317,8 @@ func (s *VaultStore) PutJWKS(ctx context.Context, jwks jwk.Set) error {
 		return errors.E(op, err)
 	}
 
-	_, err = client.Logical().WriteBytesWithContext(
-		ctx,
-		// We persist in a similar folder to the controller credentials, but sub-route
-		// to .well-known for further extensions and mental clarity within our vault.
-		s.getJWKSPath(),
-		jwksJson,
-	)
-	if err != nil {
+	jwksData := map[string]any{jwksKey: string(jwksJson)}
+	if _, err = client.KVv2(s.KVPath).Put(ctx, s.getJWKSPath(), jwksData); err != nil {
 		return errors.E(op, err)
 	}
 
@@ -332,13 +334,8 @@ func (s *VaultStore) PutJWKSPrivateKey(ctx context.Context, pem []byte) error {
 		return errors.E(op, err)
 	}
 
-	if _, err := client.Logical().WriteWithContext(
-		ctx,
-		// We persist in a similar folder to the controller credentials, but sub-route
-		// to .well-known for further extensions and mental clarity within our vault.
-		s.getJWKSPrivateKeyPath(),
-		map[string]interface{}{"key": pem},
-	); err != nil {
+	privateKeyData := map[string]interface{}{jwksPrivateKey: pem}
+	if _, err := client.KVv2(s.KVPath).Put(ctx, s.getJWKSPrivateKeyPath(), privateKeyData); err != nil {
 		return errors.E(op, err)
 	}
 	return nil
@@ -352,22 +349,16 @@ func (s *VaultStore) PutJWKSExpiry(ctx context.Context, expiry time.Time) error
 	if err != nil {
 		return errors.E(op, err)
 	}
-
-	if _, err := client.Logical().WriteWithContext(
-		ctx,
-		s.getJWKSExpiryPath(),
-		map[string]interface{}{
-			"jwks-expiry": expiry,
-		},
-	); err != nil {
+	expiryData := map[string]interface{}{jwksExpiryKey: expiry}
+	if _, err := client.KVv2(s.KVPath).Put(ctx, s.getJWKSExpiryPath(), expiryData); err != nil {
 		return errors.E(op, err)
 	}
 	return nil
 }
 
 // getWellKnownPath returns a hard coded path to the .well-known credentials.
 func (s *VaultStore) getWellKnownPath() string {
-	return path.Join(s.KVPath, "creds", ".well-known")
+	return path.Join("creds", ".well-known")
 }
 
 // getJWKSPath returns a hardcoded suffixed vault path (dependent on
@@ -398,7 +389,7 @@ func (s *VaultStore) CleanupOAuthSecrets(ctx context.Context) error {
 
 	// Vault does not return errors on deletion requests where
 	// the secret does not exist.
-	if _, err := client.Logical().DeleteWithContext(ctx, s.GetOAuthSecretPath()); err != nil {
+	if err := client.KVv2(s.KVPath).Delete(ctx, s.GetOAuthSecretPath()); err != nil {
 		return errors.E(op, err)
 	}
 	return nil
@@ -413,25 +404,29 @@ func (s *VaultStore) GetOAuthSecret(ctx context.Context) ([]byte, error) {
 		return nil, errors.E(op, err)
 	}
 
-	secret, err := client.Logical().ReadWithContext(ctx, s.GetOAuthSecretPath())
-	if err != nil {
+	secret, err := client.KVv2(s.KVPath).Get(ctx, s.GetOAuthSecretPath())
+	if err != nil && goerr.Unwrap(err) != api.ErrSecretNotFound {
 		return nil, errors.E(op, err)
 	}
 
-	if secret == nil {
+	if secret == nil || secret.Data == nil {
 		msg := "no OAuth key exists"
 		zapctx.Debug(ctx, msg)
 		return nil, errors.E(op, errors.CodeNotFound, msg)
 	}
 
-	raw := secret.Data["key"]
-	if secret.Data["key"] == nil {
+	raw, ok := secret.Data[oAuthSecretKey]
+	if !ok {
 		msg := "nil OAuth key data"
 		zapctx.Debug(ctx, msg)
 		return nil, errors.E(op, errors.CodeNotFound, msg)
 	}
 
-	keyPemB64 := raw.(string)
+	keyPemB64, ok := raw.(string)
+	if !ok {
+		zapctx.Debug(ctx, "oauth secret is not a string")
+		return nil, errors.E(op, errors.CodeNotFound, "oauth secret not found")
+	}
 
 	keyPem, err := base64.StdEncoding.DecodeString(keyPemB64)
 	if err != nil {
@@ -450,11 +445,8 @@ func (s *VaultStore) PutOAuthSecret(ctx context.Context, raw []byte) error {
 		return errors.E(op, err)
 	}
 
-	if _, err := client.Logical().WriteWithContext(
-		ctx,
-		s.GetOAuthSecretPath(),
-		map[string]interface{}{"key": raw},
-	); err != nil {
+	oAuthSecretData := map[string]interface{}{oAuthSecretKey: raw}
+	if _, err := client.KVv2(s.KVPath).Put(ctx, s.GetOAuthSecretPath(), oAuthSecretData); err != nil {
 		return errors.E(op, err)
 	}
 	return nil
@@ -463,7 +455,7 @@ func (s *VaultStore) PutOAuthSecret(ctx context.Context, raw []byte) error {
 // GetOAuthSecretPath returns a hardcoded suffixed vault path (dependent on
 // the initial KVPath) to the OAuth JWK location.
 func (s *VaultStore) GetOAuthSecretPath() string {
-	return path.Join(s.KVPath, "creds", "oauth", "key")
+	return path.Join("creds", "oauth", "key")
 }
 
 // deleteControllerCredentials removes the credentials associated with the controller in
@@ -475,7 +467,7 @@ func (s *VaultStore) deleteControllerCredentials(ctx context.Context, controller
 	if err != nil {
 		return errors.E(op, err)
 	}
-	_, err = client.Logical().DeleteWithContext(ctx, s.controllerCredentialsPath(controllerName))
+	err = client.KVv2(s.KVPath).Delete(ctx, s.controllerCredentialsPath(controllerName))
 	if rerr, ok := err.(*api.ResponseError); ok && rerr.StatusCode == http.StatusNotFound {
 		// Ignore the error if attempting to delete something that isn't there.
 		err = nil
@@ -535,9 +527,9 @@ func (s *VaultStore) client(ctx context.Context) (*api.Client, error) {
 }
 
 func (s *VaultStore) path(tag names.CloudCredentialTag) string {
-	return path.Join(s.KVPath, "creds", tag.Cloud().Id(), tag.Owner().Id(), tag.Name())
+	return path.Join("creds", tag.Cloud().Id(), tag.Owner().Id(), tag.Name())
 }
 
 func (s *VaultStore) controllerCredentialsPath(controllerName string) string {
-	return path.Join(s.KVPath, "creds", controllerName)
+	return path.Join("creds", controllerName)
 }

internal/vault/vault_test.go

@@ -209,7 +209,7 @@ func TestGetOAuthSecretFailsIfDataIsNil(t *testing.T) {
 	c.Assert(err, qt.IsNil)
 
 	retrieved, err := store.GetOAuthSecret(ctx)
-	c.Assert(err, qt.ErrorMatches, "nil OAuth key data")
+	c.Assert(err, qt.ErrorMatches, "oauth secret not found")
 	c.Assert(retrieved, qt.IsNil)
 }
 

internal/wellknownapi/api_test.go

@@ -60,7 +60,7 @@ func setupHandlerAndRecorder(c *qt.C, path string, store *vault.VaultStore) *htt
 	return rr
 }
 
-// 404: In the event the JWKS cannot be found expliciticly from
+// 404: In the event the JWKS cannot be found explicitly from
 // the credential store.
 func TestWellknownAPIJWKSJSONHandles404(t *testing.T) {
 	c := qt.New(t)

local/vault/init.sh

@@ -46,7 +46,7 @@ echo "SecretID applied & wrapped in cubbyhole for 10h, token is: $JIMM_SECRET_WR
 # Enable the KV at the defined policy path
 echo "Enabling KV at policy path /jimm-kv"
 echo "/jimm-kv accessible by policy jimm-app"
-vault secrets enable -path /jimm-kv kv
+vault secrets enable -version=2 -path /jimm-kv kv
 echo "Creating approle auth file."
 VAULT_TOKEN=$JIMM_SECRET_WRAPPED vault unwrap > /vault/approle_tmp.yaml
 echo "$JIMM_ROLE_ID" > /vault/roleid.txt

service.go

@@ -488,7 +488,7 @@ func newVaultStore(ctx context.Context, p Params) (jimmcreds.CredentialStore, er
 		Client:       client,
 		RoleID:       p.VaultRoleID,
 		RoleSecretID: p.VaultRoleSecretID,
-		KVPath:       p.VaultPath,
+		KVPath:       strings.ReplaceAll(p.VaultPath, "/", ""),
 	}, nil
 }