Add HTTP PATCH support for key metadata (#57)

* add PatchOperation to metadata endpoint

* add custom_metadata validation to patch handler

* return 404 in metadata patch handler when entry does not exist

* add cas_required warning in metadata patch handler

* run HandlePatchOperation for metadata patch handler

* metadata patch validation tests

* convert custom_metadata from TypeKVPairs to TypeMap

TypeKVPairs results in using a map[string]string
whereas TypeMap results in using a map[string]interface{}.
Being able to accept null values for custom_metadata
fields is important for HTTP PATCH operations as it
signals to the handler to remove the field. A shared
parser has been added to ensure that the provided
non-nil values are indeed parsable as strings.

* adding custom_metadata validation and patch tests

* go get vault/sdk@patch-field-data-error-handling

* fix delete_version_after handling for metadata patch

* go fmt

* add versions and custom_metadata checks

* check for simpler substr in metadata validation test

* go get vault/sdk@patch-field-data-error-handling

* fix custom metadata parsing

* tests for ignored unknown metadata fields

* move lock prior to metadata read

* add explicit check for cas_required in test despite warning

* add godoc for kv metadata patch

* remove filter of nils as sdk will do so

* add metadata test for nils unsetting values

* go get vault/sdk@main; go mod tidy

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/hashicorp/go-secure-stdlib/parseutil v0.1.2
 	github.com/hashicorp/go-secure-stdlib/strutil v0.1.1
 	github.com/hashicorp/vault/api v1.3.0
-	github.com/hashicorp/vault/sdk v0.3.0
+	github.com/hashicorp/vault/sdk v0.3.1-0.20220112143259-b48602fdb885
 	github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect
 	github.com/mitchellh/mapstructure v1.4.2
 	google.golang.org/protobuf v1.27.1

go.sum

@@ -91,8 +91,9 @@ github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brv
 github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
 github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
-github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
+github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
+github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
 github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
 github.com/hashicorp/go-hclog v0.14.1/go.mod h1:whpDNt7SSdeAju8AWKIWsul05p54N/39EeqMAyrmvFQ=
 github.com/hashicorp/go-hclog v0.16.2/go.mod h1:whpDNt7SSdeAju8AWKIWsul05p54N/39EeqMAyrmvFQ=
@@ -137,8 +138,9 @@ github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
 github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
 github.com/hashicorp/vault/api v1.3.0 h1:uDy39PLSvy6gtKyjOCRPizy2QdFiIYSWBR2pxCEzYL8=
 github.com/hashicorp/vault/api v1.3.0/go.mod h1:EabNQLI0VWbWoGlA+oBLC8PXmR9D60aUVgQGvangFWQ=
-github.com/hashicorp/vault/sdk v0.3.0 h1:kR3dpxNkhh/wr6ycaJYqp6AFT/i2xaftbfnwZduTKEY=
 github.com/hashicorp/vault/sdk v0.3.0/go.mod h1:aZ3fNuL5VNydQk8GcLJ2TV8YCRVvyaakYkhZRoVuhj0=
+github.com/hashicorp/vault/sdk v0.3.1-0.20220112143259-b48602fdb885 h1:nMbBmZC9p9Mr/xUObzIakaNlO/Q044LZnuSASE6CMjs=
+github.com/hashicorp/vault/sdk v0.3.1-0.20220112143259-b48602fdb885/go.mod h1:LzlzYEnxJWaC5mlt2nOVJimp5R15ecXAucsdjRc7KC0=
 github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM=
 github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ=
 github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM=
@@ -148,6 +150,7 @@ github.com/jhump/protoreflect v1.6.0/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyX
 github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
 github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
 github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
+github.com/keybase/go-crypto v0.0.0-20190403132359-d65b6b94177f/go.mod h1:ghbZscTyKdM07+Fw3KSi0hcJm+AlEUWj8QLlPtijN/M=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=

path_data.go

@@ -365,7 +365,7 @@ func (b *versionedKVBackend) pathDataWrite() framework.OperationFunc {
 // expects only the resource data to be provided. The "data" key must be lifted
 // from the request data to the pathDataPatch handler since it also accepts an
 // options map.
-func patchPreprocessor() framework.PatchPreprocessorFunc {
+func dataPatchPreprocessor() framework.PatchPreprocessorFunc {
 	return func(input map[string]interface{}) (map[string]interface{}, error) {
 		data, ok := input["data"]
 
@@ -390,13 +390,17 @@ func (b *versionedKVBackend) pathDataPatch() framework.OperationFunc {
 		key := data.Get("path").(string)
 
 		// Only validate that data is present to provide error response since
-		// HandlePatchOperation and patchPreprocessor will ultimately
+		// HandlePatchOperation and dataPatchPreprocessor will ultimately
 		// properly parse the field
 		_, ok := data.GetOk("data")
 		if !ok {
 			return logical.ErrorResponse("no data provided"), logical.ErrInvalidRequest
 		}
 
+		lock := locksutil.LockForKey(b.locks, key)
+		lock.Lock()
+		defer lock.Unlock()
+
 		meta, err := b.getKeyMetadata(ctx, req.Storage, key)
 		if err != nil {
 			return nil, err
@@ -416,10 +420,6 @@ func (b *versionedKVBackend) pathDataPatch() framework.OperationFunc {
 			return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
 		}
 
-		lock := locksutil.LockForKey(b.locks, key)
-		lock.Lock()
-		defer lock.Unlock()
-
 		currentVersion := meta.CurrentVersion
 
 		versionMetadata := meta.Versions[currentVersion]
@@ -480,7 +480,7 @@ func (b *versionedKVBackend) pathDataPatch() framework.OperationFunc {
 			return nil, err
 		}
 
-		patchedBytes, err := framework.HandlePatchOperation(data, versionData, patchPreprocessor())
+		patchedBytes, err := framework.HandlePatchOperation(data, versionData, dataPatchPreprocessor())
 		if err != nil {
 			return nil, err
 		}

path_metadata.go

@@ -2,9 +2,12 @@ package kv
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/go-secure-stdlib/strutil"
+	"github.com/mitchellh/mapstructure"
+	"net/http"
 	"strings"
 	"time"
 
@@ -46,7 +49,7 @@ A negative duration will cause an error.
 `,
 			},
 			"custom_metadata": {
-				Type: framework.TypeKVPairs,
+				Type: framework.TypeMap,
 				Description: `
 User-provided key-value pairs that are used to describe arbitrary and
 version-agnostic information about a secret.
@@ -59,6 +62,7 @@ version-agnostic information about a secret.
 			logical.ReadOperation:   b.upgradeCheck(b.pathMetadataRead()),
 			logical.DeleteOperation: b.upgradeCheck(b.pathMetadataDelete()),
 			logical.ListOperation:   b.upgradeCheck(b.pathMetadataList()),
+			logical.PatchOperation:  b.upgradeCheck(b.pathMetadataPatch()),
 		},
 
 		ExistenceCheck: b.metadataExistenceCheck(),
@@ -213,6 +217,31 @@ func validateCustomMetadata(customMetadata map[string]string) error {
 	return errs.ErrorOrNil()
 }
 
+// parseCustomMetadata is used to effectively convert the TypeMap
+// (map[string]interface{}) into a TypeKVPairs (map[string]string)
+// which is how custom_metadata is stored. Defining custom_metadata
+// as a TypeKVPairs will convert nulls into empty strings. A null,
+// however, is essential for a PATCH operation in that it signals
+// the handler to remove the field. The filterNils flag should
+// only be used during a patch operation.
+func parseCustomMetadata(raw map[string]interface{}, filterNils bool) (map[string]string, error) {
+	customMetadata := map[string]string{}
+	for k, v := range raw {
+		if filterNils && v == nil {
+			continue
+		}
+
+		var s string
+		if err := mapstructure.WeakDecode(v, &s); err != nil {
+			return nil, err
+		}
+
+		customMetadata[k] = s
+	}
+
+	return customMetadata, nil
+}
+
 func (b *versionedKVBackend) pathMetadataWrite() framework.OperationFunc {
 	return func(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 		key := data.Get("path").(string)
@@ -238,7 +267,11 @@ func (b *versionedKVBackend) pathMetadataWrite() framework.OperationFunc {
 		customMetadataMap := map[string]string{}
 
 		if cmOk {
-			customMetadataMap = customMetadataRaw.(map[string]string)
+			customMetadataMap, err = parseCustomMetadata(customMetadataRaw.(map[string]interface{}), false)
+			if err != nil {
+				return logical.ErrorResponse(fmt.Sprintf("%s: %s", customMetadataValidationErrorPrefix, err.Error())), nil
+			}
+
 			customMetadataErrs := validateCustomMetadata(customMetadataMap)
 
 			if customMetadataErrs != nil {
@@ -288,6 +321,108 @@ func (b *versionedKVBackend) pathMetadataWrite() framework.OperationFunc {
 	}
 }
 
+// metadataPatchPreprocessor returns a framework.PatchPreprocessorFunc meant to
+// be provided to framework.HandlePatchOperation. The returned
+// framework.PatchPreprocessorFunc handles filtering out Vault-managed fields,
+// and ensuring appropriate handling of data types not supported directly by FieldType.
+func metadataPatchPreprocessor() framework.PatchPreprocessorFunc {
+	return func(input map[string]interface{}) (map[string]interface{}, error) {
+		patchableKeys := []string{"max_versions", "cas_required", "delete_version_after", "custom_metadata"}
+		patchData := map[string]interface{}{}
+
+		for _, k := range patchableKeys {
+			if v, ok := input[k]; ok {
+				if k == "delete_version_after" {
+					patchData[k] = ptypes.DurationProto(time.Duration(v.(int)) * time.Second)
+				} else {
+					patchData[k] = v
+				}
+			}
+		}
+
+		return patchData, nil
+	}
+}
+
+// pathMetadataPatch handles a PatchOperation request for a secret's key metadata
+// The key metadata entry must exist to apply the provided patch data.
+func (b *versionedKVBackend) pathMetadataPatch() framework.OperationFunc {
+	return func(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
+		key := data.Get("path").(string)
+
+		if key == "" {
+			return logical.ErrorResponse("missing path"), nil
+		}
+
+		if cmRaw, cmOk := data.GetOk("custom_metadata"); cmOk {
+			customMetadataMap, err := parseCustomMetadata(cmRaw.(map[string]interface{}), true)
+			if err != nil {
+				return logical.ErrorResponse(fmt.Sprintf("%s: %s", customMetadataValidationErrorPrefix, err.Error())), nil
+			}
+
+			customMetadataErrs := validateCustomMetadata(customMetadataMap)
+
+			if customMetadataErrs != nil {
+				return logical.ErrorResponse(customMetadataErrs.Error()), nil
+			}
+		}
+
+		config, err := b.config(ctx, req.Storage)
+		if err != nil {
+			return nil, err
+		}
+
+		lock := locksutil.LockForKey(b.locks, key)
+		lock.Lock()
+		defer lock.Unlock()
+
+		meta, err := b.getKeyMetadata(ctx, req.Storage, key)
+		if err != nil {
+			return nil, err
+		}
+
+		if meta == nil {
+			return logical.RespondWithStatusCode(nil, req, http.StatusNotFound)
+		}
+
+		var resp *logical.Response
+		casRaw, cOk := data.GetOk("cas_required")
+
+		if cOk && config.CasRequired && !casRaw.(bool) {
+			resp = &logical.Response{}
+			resp.AddWarning("\"cas_required\" set to false, but is mandated by backend config. This value will be ignored.")
+		}
+
+		// proto-generated structs do not have mapstructure tags so marshal
+		// metadata here so that map keys are consistent with request data
+		metadataJSON, err := json.Marshal(meta)
+		if err != nil {
+			return nil, err
+		}
+
+		var metaMap map[string]interface{}
+		if err = json.Unmarshal(metadataJSON, &metaMap); err != nil {
+			return nil, err
+		}
+
+		patchedBytes, err := framework.HandlePatchOperation(data, metaMap, metadataPatchPreprocessor())
+		if err != nil {
+			return nil, err
+		}
+
+		var patchedMetadata *KeyMetadata
+		if err = json.Unmarshal(patchedBytes, &patchedMetadata); err != nil {
+			return nil, err
+		}
+
+		if err = b.writeKeyMetadata(ctx, req.Storage, patchedMetadata); err != nil {
+			return nil, err
+		}
+
+		return resp, nil
+	}
+}
+
 func (b *versionedKVBackend) pathMetadataDelete() framework.OperationFunc {
 	return func(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 		key := data.Get("path").(string)