Add HTTP PATCH support to KV (#12687)

* handle HTTP PATCH requests as logical.PatchOperation

* update go.mod, go.sum

* a nil response for logical.PatchOperation should result in 404

* respond with 415 for incorrect MIME type in PATCH Content-Type header

* add abstraction to handle PatchOperation requests

* add ACLs for patch

* Adding JSON Merge support to the API client

* add HTTP PATCH tests to check high level response logic

* add permission-based 'kv patch' tests in prep to add HTTP PATCH

* adding more 'kv patch' CLI command tests

* fix TestHandler_Patch_NotFound

* Fix TestKvPatchCommand_StdinValue

* add audit log test for HTTP PATCH

* patch CLI changes

* add patch CLI tests

* change JSONMergePatch func to accept a ctx

* fix TestKVPatchCommand_RWMethodNotExists and TestKVPatchCommand_RWMethodSucceeds to specify -method flag

* go fmt

* add a test to verify patching works by default with the root token

* add changelog entry

* get vault-plugin-secrets-kv@add-patch-support

* PR feedback

* reorder some imports; go fmt

* add doc comment for HandlePatchOperation

* add json-patch@v5.5.0 to go.mod

* remove unnecessary cancelFunc for WriteBytes

* remove default for -method

* use stable version of json-patch; go mod tidy

* more PR feedback

* temp go get vault-plugin-secrets-kv@master until official release

Co-authored-by: Josh Black <raskchanky@users.noreply.github.com>

api/api_test.go

@@ -9,8 +9,7 @@ import (
 
 // testHTTPServer creates a test HTTP server that handles requests until
 // the listener returned is closed.
-func testHTTPServer(
-	t *testing.T, handler http.Handler) (*Config, net.Listener) {
+func testHTTPServer(t *testing.T, handler http.Handler) (*Config, net.Listener) {
 	ln, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatalf("err: %s", err)

api/logical.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net/http"
 	"net/url"
 	"os"
 
@@ -130,24 +131,37 @@ func (c *Logical) List(path string) (*Secret, error) {
 }
 
 func (c *Logical) Write(path string, data map[string]interface{}) (*Secret, error) {
+	ctx, cancelFunc := context.WithCancel(context.Background())
+	defer cancelFunc()
+
 	r := c.c.NewRequest("PUT", "/v1/"+path)
 	if err := r.SetJSONBody(data); err != nil {
 		return nil, err
 	}
 
-	return c.write(path, r)
+	return c.write(ctx, path, r)
+}
+
+func (c *Logical) JSONMergePatch(ctx context.Context, path string, data map[string]interface{}) (*Secret, error) {
+	r := c.c.NewRequest("PATCH", "/v1/"+path)
+	r.Headers = http.Header{
+		"Content-Type": []string{"application/merge-patch+json"},
+	}
+	if err := r.SetJSONBody(data); err != nil {
+		return nil, err
+	}
+
+	return c.write(ctx, path, r)
 }
 
 func (c *Logical) WriteBytes(path string, data []byte) (*Secret, error) {
 	r := c.c.NewRequest("PUT", "/v1/"+path)
 	r.BodyBytes = data
 
-	return c.write(path, r)
+	return c.write(context.Background(), path, r)
 }
 
-func (c *Logical) write(path string, request *Request) (*Secret, error) {
-	ctx, cancelFunc := context.WithCancel(context.Background())
-	defer cancelFunc()
+func (c *Logical) write(ctx context.Context, path string, request *Request) (*Secret, error) {
 	resp, err := c.c.RawRequestWithContext(ctx, request)
 	if resp != nil {
 		defer resp.Body.Close()

changelog/12687.txt

@@ -0,0 +1,5 @@
+```release-note:feature
+**KV patch**: Add partial update support the for the `/<mount>/data/:path` kv-v2
+endpoint through HTTP `PATCH`.  A new `patch` ACL capability has been added and
+is required to make such requests.
+```

command/kv_patch.go

@@ -1,11 +1,13 @@
 package command
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"os"
 	"strings"
 
+	"github.com/hashicorp/vault/api"
 	"github.com/mitchellh/cli"
 	"github.com/posener/complete"
 )
@@ -18,7 +20,9 @@ var (
 type KVPatchCommand struct {
 	*BaseCommand
 
-	testStdin io.Reader // for tests
+	flagCAS    int
+	flagMethod string
+	testStdin  io.Reader // for tests
 }
 
 func (c *KVPatchCommand) Synopsis() string {
@@ -45,6 +49,25 @@ Usage: vault kv patch [options] KEY [DATA]
 
       $ echo "abcd1234" | vault kv patch secret/foo bar=-
 
+  To perform a Check-And-Set operation, specify the -cas flag with the
+  appropriate version number corresponding to the key you want to perform
+  the CAS operation on:
+
+      $ vault kv patch -cas=1 secret/foo bar=baz
+
+  By default, this operation will attempt an HTTP PATCH operation. If your
+  policy does not allow that, it will fall back to a read/local update/write approach.
+  If you wish to specify which method this command should use, you may do so
+  with the -method flag. When -method=patch is specified, only an HTTP PATCH
+  operation will be tried. If it fails, the entire command will fail.
+
+      $ vault kv patch -method=patch secret/foo bar=baz
+
+  When -method=rw is specified, only a read/local update/write approach will be tried.
+  This was the default behavior previous to Vault 1.9.
+
+      $ vault kv patch -method=rw secret/foo bar=baz
+
   Additional flags and more advanced use cases are detailed below.
 
 ` + c.Flags().Help()
@@ -54,6 +77,27 @@ Usage: vault kv patch [options] KEY [DATA]
 func (c *KVPatchCommand) Flags() *FlagSets {
 	set := c.flagSet(FlagSetHTTP | FlagSetOutputField | FlagSetOutputFormat)
 
+	// Patch specific options
+	f := set.NewFlagSet("Common Options")
+
+	f.IntVar(&IntVar{
+		Name:    "cas",
+		Target:  &c.flagCAS,
+		Default: 0,
+		Usage: `Specifies to use a Check-And-Set operation. If set to 0 or not
+		set, the patch will be allowed. If the index is non-zero the patch will
+		only be allowed if the key’s current version matches the version
+		specified in the cas parameter.`,
+	})
+
+	f.StringVar(&StringVar{
+		Name:   "method",
+		Target: &c.flagMethod,
+		Usage: `Specifies which method of patching to use. If set to "patch", then
+		an HTTP PATCH request will be issued. If set to "rw", then a read will be
+		performed, then a local update, followed by a remote update.`,
+	})
+
 	return set
 }
 
@@ -121,6 +165,30 @@ func (c *KVPatchCommand) Run(args []string) int {
 		return 2
 	}
 
+	// Check the method and behave accordingly
+	var secret *api.Secret
+	var code int
+
+	switch c.flagMethod {
+	case "rw":
+		secret, code = c.readThenWrite(client, path, newData)
+	case "patch":
+		secret, code = c.mergePatch(client, path, newData, false)
+	case "":
+		secret, code = c.mergePatch(client, path, newData, true)
+	default:
+		c.UI.Error(fmt.Sprintf("Unsupported method provided to -method flag: %s", c.flagMethod))
+		return 2
+	}
+
+	if code != 0 {
+		return code
+	}
+
+	return OutputSecret(c.UI, secret)
+}
+
+func (c *KVPatchCommand) readThenWrite(client *api.Client, path string, newData map[string]interface{}) (*api.Secret, int) {
 	// First, do a read.
 	// Note that we don't want to see curl output for the read request.
 	curOutputCurl := client.OutputCurlString()
@@ -129,45 +197,45 @@ func (c *KVPatchCommand) Run(args []string) int {
 	client.SetOutputCurlString(curOutputCurl)
 	if err != nil {
 		c.UI.Error(fmt.Sprintf("Error doing pre-read at %s: %s", path, err))
-		return 2
+		return nil, 2
 	}
 
 	// Make sure a value already exists
 	if secret == nil || secret.Data == nil {
 		c.UI.Error(fmt.Sprintf("No value found at %s", path))
-		return 2
+		return nil, 2
 	}
 
 	// Verify metadata found
 	rawMeta, ok := secret.Data["metadata"]
 	if !ok || rawMeta == nil {
 		c.UI.Error(fmt.Sprintf("No metadata found at %s; patch only works on existing data", path))
-		return 2
+		return nil, 2
 	}
 	meta, ok := rawMeta.(map[string]interface{})
 	if !ok {
 		c.UI.Error(fmt.Sprintf("Metadata found at %s is not the expected type (JSON object)", path))
-		return 2
+		return nil, 2
 	}
 	if meta == nil {
 		c.UI.Error(fmt.Sprintf("No metadata found at %s; patch only works on existing data", path))
-		return 2
+		return nil, 2
 	}
 
 	// Verify old data found
 	rawData, ok := secret.Data["data"]
 	if !ok || rawData == nil {
 		c.UI.Error(fmt.Sprintf("No data found at %s; patch only works on existing data", path))
-		return 2
+		return nil, 2
 	}
 	data, ok := rawData.(map[string]interface{})
 	if !ok {
 		c.UI.Error(fmt.Sprintf("Data found at %s is not the expected type (JSON object)", path))
-		return 2
+		return nil, 2
 	}
 	if data == nil {
 		c.UI.Error(fmt.Sprintf("No data found at %s; patch only works on existing data", path))
-		return 2
+		return nil, 2
 	}
 
 	// Copy new data over
@@ -183,19 +251,58 @@ func (c *KVPatchCommand) Run(args []string) int {
 	})
 	if err != nil {
 		c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", path, err))
-		return 2
+		return nil, 2
 	}
+
 	if secret == nil {
 		// Don't output anything unless using the "table" format
 		if Format(c.UI) == "table" {
 			c.UI.Info(fmt.Sprintf("Success! Data written to: %s", path))
 		}
-		return 0
+		return nil, 0
 	}
 
 	if c.flagField != "" {
-		return PrintRawField(c.UI, secret, c.flagField)
+		return nil, PrintRawField(c.UI, secret, c.flagField)
 	}
 
-	return OutputSecret(c.UI, secret)
+	return secret, 0
+}
+
+func (c *KVPatchCommand) mergePatch(client *api.Client, path string, newData map[string]interface{}, rwFallback bool) (*api.Secret, int) {
+	data := map[string]interface{}{
+		"data":    newData,
+		"options": map[string]interface{}{},
+	}
+
+	if c.flagCAS > 0 {
+		data["options"].(map[string]interface{})["cas"] = c.flagCAS
+	}
+
+	secret, err := client.Logical().JSONMergePatch(context.Background(), path, data)
+	if err != nil {
+		// If it's a 403, that probably means they don't have the patch capability in their policy. Fall back to
+		// the old way of doing it if the user didn't specify a -method. If they did, and it was "patch", then just error.
+		if re, ok := err.(*api.ResponseError); ok && re.StatusCode == 403 && rwFallback {
+			c.UI.Warn(fmt.Sprintf("Data was written to %s but we recommend that you add the \"patch\" capability to your ACL policy in order to use HTTP PATCH in the future.", path))
+			return c.readThenWrite(client, path, newData)
+		}
+
+		c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", path, err))
+		return nil, 2
+	}
+
+	if secret == nil {
+		// Don't output anything unless using the "table" format
+		if Format(c.UI) == "table" {
+			c.UI.Info(fmt.Sprintf("Success! Data written to: %s", path))
+		}
+		return nil, 0
+	}
+
+	if c.flagField != "" {
+		return nil, PrintRawField(c.UI, secret, c.flagField)
+	}
+
+	return secret, 0
 }