diff --git a/api/api_test.go b/api/api_test.go index b2b851df6..e4ba31532 100644 --- a/api/api_test.go +++ b/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) diff --git a/api/logical.go b/api/logical.go index cd950a2b7..f8f8bc537 100644 --- a/api/logical.go +++ b/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() diff --git a/changelog/12687.txt b/changelog/12687.txt new file mode 100644 index 000000000..f5998deed --- /dev/null +++ b/changelog/12687.txt @@ -0,0 +1,5 @@ +```release-note:feature +**KV patch**: Add partial update support the for the `//data/:path` kv-v2 +endpoint through HTTP `PATCH`. A new `patch` ACL capability has been added and +is required to make such requests. +``` diff --git a/command/kv_patch.go b/command/kv_patch.go index 05e759793..d05ff5eed 100644 --- a/command/kv_patch.go +++ b/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 } diff --git a/command/kv_test.go b/command/kv_test.go index 7e93a4cb8..a73fc0135 100644 --- a/command/kv_test.go +++ b/command/kv_test.go @@ -1,6 +1,7 @@ package command import ( + "fmt" "io" "strings" "testing" @@ -552,3 +553,606 @@ func TestKVMetadataGetCommand(t *testing.T) { assertNoTabs(t, cmd) }) } + +func testKVPatchCommand(tb testing.TB) (*cli.MockUi, *KVPatchCommand) { + tb.Helper() + + ui := cli.NewMockUi() + return ui, &KVPatchCommand{ + BaseCommand: &BaseCommand{ + UI: ui, + }, + } +} + +func TestKVPatchCommand_ArgValidation(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + args []string + out string + code int + }{ + { + "not_enough_args", + []string{}, + "Not enough arguments", + 1, + }, + { + "empty_kvs", + []string{"kv/patch/foo"}, + "Must supply data", + 1, + }, + { + "kvs_no_value", + []string{"kv/patch/foo", "foo"}, + "Failed to parse K=V data", + 1, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + ui, cmd := testKVPatchCommand(t) + cmd.client = client + + code := cmd.Run(tc.args) + + if code != tc.code { + t.Fatalf("expected code to be %d but was %d for cmd %#v with args %#v\n", tc.code, code, cmd, tc.args) + } + + combined := ui.OutputWriter.String() + ui.ErrorWriter.String() + + if !strings.Contains(combined, tc.out) { + t.Fatalf("expected output to be %q but was %q for cmd %#v with args %#v\n", tc.out, combined, cmd, tc.args) + } + }) + } +} + +func TestKvPatchCommand_StdinFull(t *testing.T) { + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + if _, err := client.Logical().Write("kv/data/patch/foo", map[string]interface{}{ + "data": map[string]interface{}{ + "foo": "a", + }, + }); err != nil { + t.Fatalf("write failed, err: %#v\n", err) + } + + stdinR, stdinW := io.Pipe() + go func() { + stdinW.Write([]byte(`{"foo":"bar"}`)) + stdinW.Close() + }() + + _, cmd := testKVPatchCommand(t) + cmd.client = client + + cmd.testStdin = stdinR + + args := []string{"kv/patch/foo", "-"} + code := cmd.Run(args) + if code != 0 { + t.Fatalf("expected code to be 0 but was %d for cmd %#v with args %#v\n", code, cmd, args) + } + + secret, err := client.Logical().Read("kv/data/patch/foo") + if err != nil { + t.Fatalf("read failed, err: %#v\n", err) + } + + if secret == nil || secret.Data == nil { + t.Fatal("expected secret to have data") + } + + secretDataRaw, ok := secret.Data["data"] + + if !ok { + t.Fatalf("expected secret to have nested data key, data: %#v", secret.Data) + } + + secretData := secretDataRaw.(map[string]interface{}) + foo, ok := secretData["foo"].(string) + if !ok { + t.Fatal("expected foo to be a string but it wasn't") + } + + if exp, act := "bar", foo; exp != act { + t.Fatalf("expected %q to be %q, data: %#v\n", act, exp, secret.Data) + } +} + +func TestKvPatchCommand_StdinValue(t *testing.T) { + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + if _, err := client.Logical().Write("kv/data/patch/foo", map[string]interface{}{ + "data": map[string]interface{}{ + "foo": "a", + }, + }); err != nil { + t.Fatalf("write failed, err: %#v\n", err) + } + + stdinR, stdinW := io.Pipe() + go func() { + stdinW.Write([]byte("bar")) + stdinW.Close() + }() + + _, cmd := testKVPatchCommand(t) + cmd.client = client + + cmd.testStdin = stdinR + + args := []string{"kv/patch/foo", "foo=-"} + code := cmd.Run(args) + if code != 0 { + t.Fatalf("expected code to be 0 but was %d for cmd %#v with args %#v\n", code, cmd, args) + } + + secret, err := client.Logical().Read("kv/data/patch/foo") + if err != nil { + t.Fatalf("read failed, err: %#v\n", err) + } + + if secret == nil || secret.Data == nil { + t.Fatal("expected secret to have data") + } + + secretDataRaw, ok := secret.Data["data"] + + if !ok { + t.Fatalf("expected secret to have nested data key, data: %#v\n", secret.Data) + } + + secretData := secretDataRaw.(map[string]interface{}) + + if exp, act := "bar", secretData["foo"].(string); exp != act { + t.Fatalf("expected %q to be %q, data: %#v\n", act, exp, secret.Data) + } +} + +func TestKVPatchCommand_RWMethodNotExists(t *testing.T) { + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + ui, cmd := testKVPatchCommand(t) + cmd.client = client + + args := []string{"-method", "rw", "kv/patch/foo", "foo=a"} + code := cmd.Run(args) + if code != 2 { + t.Fatalf("expected code to be 2 but was %d for cmd %#v with args %#v\n", code, cmd, args) + } + + combined := ui.OutputWriter.String() + ui.ErrorWriter.String() + + expectedOutputSubstr := "No value found" + if !strings.Contains(combined, expectedOutputSubstr) { + t.Fatalf("expected output %q to contain %q for cmd %#v with args %#v\n", combined, expectedOutputSubstr, cmd, args) + } +} + +func TestKVPatchCommand_RWMethodSucceeds(t *testing.T) { + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + if _, err := client.Logical().Write("kv/data/patch/foo", map[string]interface{}{ + "data": map[string]interface{}{ + "foo": "a", + "bar": "b", + }, + }); err != nil { + t.Fatalf("write failed, err: %#v\n", err) + } + + ui, cmd := testKVPatchCommand(t) + cmd.client = client + + // Test single value + args := []string{"-method", "rw", "kv/patch/foo", "foo=aa"} + code := cmd.Run(args) + if code != 0 { + t.Fatalf("expected code to be 0 but was %d for cmd %#v with args %#v\n", code, cmd, args) + } + + combined := ui.OutputWriter.String() + ui.ErrorWriter.String() + + expectedOutputSubstr := "created_time" + if !strings.Contains(combined, expectedOutputSubstr) { + t.Fatalf("expected output %q to contain %q for cmd %#v with args %#v\n", combined, expectedOutputSubstr, cmd, args) + } + + // Test multi value + ui, cmd = testKVPatchCommand(t) + cmd.client = client + + args = []string{"-method", "rw", "kv/patch/foo", "foo=aaa", "bar=bbb"} + code = cmd.Run(args) + if code != 0 { + t.Fatalf("expected code to be 0 but was %d for cmd %#v with args %#v\n", code, cmd, args) + } + + combined = ui.OutputWriter.String() + ui.ErrorWriter.String() + + if !strings.Contains(combined, expectedOutputSubstr) { + t.Fatalf("expected output %q to contain %q for cmd %#v with args %#v\n", combined, expectedOutputSubstr, cmd, args) + } +} + +func TestKVPatchCommand_CAS(t *testing.T) { + cases := []struct { + name string + args []string + expected string + out string + code int + }{ + { + "right version", + []string{"-cas", "1", "kv/foo", "bar=quux"}, + "quux", + "", + 0, + }, + { + "wrong version", + []string{"-cas", "2", "kv/foo", "bar=wibble"}, + "baz", + "check-and-set parameter did not match the current version", + 2, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + // create a policy with patch capability + policy := `path "kv/*" { capabilities = ["create", "update", "read", "patch"] }` + secretAuth, err := createTokenForPolicy(t, client, policy) + if err != nil { + t.Fatalf("policy/token creation failed for policy %s, err: %#v\n", policy, err) + } + + kvClient, err := client.Clone() + if err != nil { + t.Fatal(err) + } + + kvClient.SetToken(secretAuth.ClientToken) + + _, err = kvClient.Logical().Write("kv/data/foo", map[string]interface{}{"data": map[string]interface{}{"bar": "baz"}}) + if err != nil { + t.Fatal(err) + } + + ui, cmd := testKVPatchCommand(t) + cmd.client = kvClient + + code := cmd.Run(tc.args) + + if code != tc.code { + t.Fatalf("expected code to be %d but was %d", tc.code, code) + } + + if tc.out != "" { + combined := ui.OutputWriter.String() + ui.ErrorWriter.String() + if !strings.Contains(combined, tc.out) { + t.Errorf("expected %q to contain %q", combined, tc.out) + } + } + + secret, err := kvClient.Logical().Read("kv/data/foo") + bar := secret.Data["data"].(map[string]interface{})["bar"] + if bar != tc.expected { + t.Fatalf("expected bar to be %q but it was %q", tc.expected, bar) + } + }) + } +} + +func TestKVPatchCommand_Methods(t *testing.T) { + cases := []struct { + name string + args []string + expected string + code int + }{ + { + "rw", + []string{"-method", "rw", "kv/foo", "bar=quux"}, + "quux", + 0, + }, + { + "patch", + []string{"-method", "patch", "kv/foo", "bar=wibble"}, + "wibble", + 0, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + // create a policy with patch capability + policy := `path "kv/*" { capabilities = ["create", "update", "read", "patch"] }` + secretAuth, err := createTokenForPolicy(t, client, policy) + if err != nil { + t.Fatalf("policy/token creation failed for policy %s, err: %#v\n", policy, err) + } + + kvClient, err := client.Clone() + if err != nil { + t.Fatal(err) + } + + kvClient.SetToken(secretAuth.ClientToken) + + _, err = kvClient.Logical().Write("kv/data/foo", map[string]interface{}{"data": map[string]interface{}{"bar": "baz"}}) + if err != nil { + t.Fatal(err) + } + + _, cmd := testKVPatchCommand(t) + cmd.client = kvClient + + code := cmd.Run(tc.args) + + if code != tc.code { + t.Fatalf("expected code to be %d but was %d", tc.code, code) + } + + secret, err := kvClient.Logical().Read("kv/data/foo") + bar := secret.Data["data"].(map[string]interface{})["bar"] + if bar != tc.expected { + t.Fatalf("expected bar to be %q but it was %q", tc.expected, bar) + } + }) + } +} + +func TestKVPatchCommand_403Fallback(t *testing.T) { + cases := []struct { + name string + args []string + expected string + code int + }{ + // if no -method is specified, and patch fails, it should fall back to rw and succeed + { + "unspecified", + []string{"kv/foo", "bar=quux"}, + `add the "patch" capability to your ACL policy`, + 0, + }, + // if -method=patch is specified, and patch fails, it should not fall back, and just error + { + "specifying patch", + []string{"-method", "patch", "kv/foo", "bar=quux"}, + "permission denied", + 2, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + // create a policy without patch capability + policy := `path "kv/*" { capabilities = ["create", "update", "read"] }` + secretAuth, err := createTokenForPolicy(t, client, policy) + if err != nil { + t.Fatalf("policy/token creation failed for policy %s, err: %#v\n", policy, err) + } + + kvClient, err := client.Clone() + if err != nil { + t.Fatal(err) + } + + kvClient.SetToken(secretAuth.ClientToken) + + // Write a value then attempt to patch it + _, err = kvClient.Logical().Write("kv/data/foo", map[string]interface{}{"data": map[string]interface{}{"bar": "baz"}}) + if err != nil { + t.Fatal(err) + } + + ui, cmd := testKVPatchCommand(t) + cmd.client = kvClient + code := cmd.Run(tc.args) + + if code != tc.code { + t.Fatalf("expected code to be %d but was %d", tc.code, code) + } + + combined := ui.OutputWriter.String() + ui.ErrorWriter.String() + if !strings.Contains(combined, tc.expected) { + t.Errorf("expected %q to contain %q", combined, tc.expected) + } + }) + } +} + +func createTokenForPolicy(t *testing.T, client *api.Client, policy string) (*api.SecretAuth, error) { + t.Helper() + + if err := client.Sys().PutPolicy("policy", policy); err != nil { + return nil, err + } + + secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{ + Policies: []string{"policy"}, + TTL: "30m", + }) + if err != nil { + return nil, err + } + + if secret == nil || secret.Auth == nil || secret.Auth.ClientToken == "" { + return nil, fmt.Errorf("missing auth data: %#v", secret) + } + + return secret.Auth, err +} + +func TestKVPatchCommand_RWMethodPolicyVariations(t *testing.T) { + cases := []struct { + name string + args []string + policy string + expected string + code int + }{ + // if the policy doesn't have read capability and -method=rw is specified, it fails + { + "no read", + []string{"-method", "rw", "kv/foo", "bar=quux"}, + `path "kv/*" { capabilities = ["create", "update"] }`, + "permission denied", + 2, + }, + // if the policy doesn't have update capability and -method=rw is specified, it fails + { + "no update", + []string{"-method", "rw", "kv/foo", "bar=quux"}, + `path "kv/*" { capabilities = ["create", "read"] }`, + "permission denied", + 2, + }, + // if the policy has both read and update and -method=rw is specified, it succeeds + { + "read and update", + []string{"-method", "rw", "kv/foo", "bar=quux"}, + `path "kv/*" { capabilities = ["create", "read", "update"] }`, + "", + 0, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + client, closer := testVaultServer(t) + defer closer() + + if err := client.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + secretAuth, err := createTokenForPolicy(t, client, tc.policy) + if err != nil { + t.Fatalf("policy/token creation failed for policy %s, err: %#v\n", tc.policy, err) + } + + client.SetToken(secretAuth.ClientToken) + + if _, err := client.Logical().Write("kv/data/foo", map[string]interface{}{ + "data": map[string]interface{}{ + "foo": "bar", + "bar": "baz", + }, + }); err != nil { + t.Fatalf("write failed, err: %#v\n", err) + } + + ui, cmd := testKVPatchCommand(t) + cmd.client = client + + code := cmd.Run(tc.args) + + if code != tc.code { + t.Fatalf("expected code to be %d but was %d for cmd %#v with args %#v\n", tc.code, code, cmd, tc.args) + } + + if code != 0 { + combined := ui.OutputWriter.String() + ui.ErrorWriter.String() + if !strings.Contains(combined, tc.expected) { + t.Fatalf("expected output %q to contain %q for cmd %#v with args %#v\n", combined, tc.expected, cmd, tc.args) + } + } + }) + } +} diff --git a/go.mod b/go.mod index affcd85cc..b5ab8ff8b 100644 --- a/go.mod +++ b/go.mod @@ -113,13 +113,13 @@ require ( github.com/hashicorp/vault-plugin-secrets-azure v0.6.3-0.20210924190759-58a034528e35 github.com/hashicorp/vault-plugin-secrets-gcp v0.10.2 github.com/hashicorp/vault-plugin-secrets-gcpkms v0.9.0 - github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20210811133805-e060c2307b24 + github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20211013154503-eec8a1c892fb github.com/hashicorp/vault-plugin-secrets-mongodbatlas v0.4.0 github.com/hashicorp/vault-plugin-secrets-openldap v0.4.1-0.20210921171411-e86105e4986d github.com/hashicorp/vault-plugin-secrets-terraform v0.1.1-0.20210715043003-e02ca8f6408e github.com/hashicorp/vault-testing-stepwise v0.1.1 github.com/hashicorp/vault/api v1.1.1 - github.com/hashicorp/vault/sdk v0.2.1 + github.com/hashicorp/vault/sdk v0.2.2-0.20211004171540-a8c7e135dd6a github.com/influxdata/influxdb v0.0.0-20190411212539-d24b7ba8c4c4 github.com/jcmturner/gokrb5/v8 v8.0.0 github.com/jefferai/isbadcipher v0.0.0-20190226160619-51d2077c035f @@ -199,6 +199,7 @@ require ( google.golang.org/grpc v1.41.0 google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0 google.golang.org/protobuf v1.27.1 + gopkg.in/evanphx/json-patch.v4 v4.11.0 // indirect gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce gopkg.in/ory-am/dockertest.v3 v3.3.4 gopkg.in/square/go-jose.v2 v2.5.1 diff --git a/go.sum b/go.sum index 38b8e2148..b2490092b 100644 --- a/go.sum +++ b/go.sum @@ -350,6 +350,8 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.m github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/evanphx/json-patch v0.0.0-20190203023257-5858425f7550/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= +github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= +github.com/evanphx/json-patch v4.2.0+incompatible h1:fUDGZCv/7iAN7u0puUVhvKCcsR6vRfwrJatElLBEf0I= github.com/evanphx/json-patch v4.2.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU= @@ -748,8 +750,10 @@ github.com/hashicorp/vault-plugin-secrets-gcp v0.10.2 h1:+DtlYJTsrFRInQpAo09KkYN github.com/hashicorp/vault-plugin-secrets-gcp v0.10.2/go.mod h1:psRQ/dm5XatoUKLDUeWrpP9icMJNtu/jmscUr37YGK4= github.com/hashicorp/vault-plugin-secrets-gcpkms v0.9.0 h1:7a0iWuFA/YNinQ1xXogyZHStolxMVtLV+sy1LpEHaZs= github.com/hashicorp/vault-plugin-secrets-gcpkms v0.9.0/go.mod h1:hhwps56f2ATeC4Smgghrc5JH9dXR31b4ehSf1HblP5Q= -github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20210811133805-e060c2307b24 h1:uqPKQzkmO5vybOqk2aOdviXXi5088bcl2MrE0D1MhjM= -github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20210811133805-e060c2307b24/go.mod h1:4j2pZrSynPuUAAYrZQVgSSHD0A9xj7GK9Ji1sWtnO4s= +github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20211007143158-2d15a6fec12b h1:1GJj7AjgI0Td95haW8EK5on3Usuox78wmzLj+J9vcm4= +github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20211007143158-2d15a6fec12b/go.mod h1:iEKCVaKBQzzYxzb778O6VGLdd+8gA40ZI14bo+8tQjs= +github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20211013154503-eec8a1c892fb h1:nZ2a4a1G0ALLAzKOWQbLzD5oljKo+pjMarbq3BwU0pM= +github.com/hashicorp/vault-plugin-secrets-kv v0.5.7-0.20211013154503-eec8a1c892fb/go.mod h1:D/FQJ7zU5pD6FNJVUwaVtxr75ZsxIIqaG/Nh6RHt/xo= github.com/hashicorp/vault-plugin-secrets-mongodbatlas v0.4.0 h1:6ve+7hZmGn7OpML81iZUxYj2AaJptwys323S5XsvVas= github.com/hashicorp/vault-plugin-secrets-mongodbatlas v0.4.0/go.mod h1:4mdgPqlkO+vfFX1cFAWcxkeqz6JAtZgKxL/67q/58Oo= github.com/hashicorp/vault-plugin-secrets-openldap v0.4.1-0.20210921171411-e86105e4986d h1:o5Z9B1FztTYSnTQNzFr+iZJHPM8ZD23uV5A8gMxm2g0= @@ -803,6 +807,7 @@ github.com/jefferai/isbadcipher v0.0.0-20190226160619-51d2077c035f h1:E87tDTVS5W github.com/jefferai/isbadcipher v0.0.0-20190226160619-51d2077c035f/go.mod h1:3J2qVK16Lq8V+wfiL2lPeDZ7UWMxk5LemerHa1p6N00= github.com/jefferai/jsonx v1.0.0 h1:Xoz0ZbmkpBvED5W9W1B5B/zc3Oiq7oXqiW7iRV3B6EI= github.com/jefferai/jsonx v1.0.0/go.mod h1:OGmqmi2tTeI/PS+qQfBDToLHHJIy/RMp24fPo8vFvoQ= +github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.3.0/go.mod h1:9QtRXoHjLGCJ5IBSaohpXITPlowMeeYCZ7fLUTSywik= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= @@ -1672,6 +1677,8 @@ gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8X gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/errgo.v2 v2.1.0 h1:0vLT13EuvQ0hNvakwLuFZ/jYrLp5F3kcWHXdRggjCE8= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= +gopkg.in/evanphx/json-patch.v4 v4.11.0 h1:+kbwxm5IBGIiNYVhss+hM3Nv4ck+HnPSNscCNbD1cT0= +gopkg.in/evanphx/json-patch.v4 v4.11.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKWaSkCsqBpgog8nAV2xsGOxlo= diff --git a/http/handler_test.go b/http/handler_test.go index c228629ea..87a79d314 100644 --- a/http/handler_test.go +++ b/http/handler_test.go @@ -16,6 +16,10 @@ import ( "github.com/go-test/deep" "github.com/hashicorp/go-cleanhttp" + kv "github.com/hashicorp/vault-plugin-secrets-kv" + "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/audit" + auditFile "github.com/hashicorp/vault/builtin/audit/file" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/logical" @@ -818,3 +822,198 @@ func TestHandler_Parse_Form(t *testing.T) { t.Fatal(diff) } } + +func TestHandler_Patch_BadContentTypeHeader(t *testing.T) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "kv": kv.VersionedKVFactory, + }, + } + + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: Handler, + }) + + cluster.Start() + defer cluster.Cleanup() + + cores := cluster.Cores + + core := cores[0].Core + c := cluster.Cores[0].Client + vault.TestWaitActive(t, core) + + // Mount a KVv2 backend + err := c.Sys().Mount("kv", &api.MountInput{ + Type: "kv-v2", + }) + if err != nil { + t.Fatal(err) + } + + kvData := map[string]interface{}{ + "data": map[string]interface{}{ + "bar": "a", + }, + } + + resp, err := c.Logical().Write("kv/data/foo", kvData) + if err != nil { + t.Fatalf("write failed - err :%#v, resp: %#v\n", err, resp) + } + + resp, err = c.Logical().Read("kv/data/foo") + if err != nil { + t.Fatalf("read failed - err :%#v, resp: %#v\n", err, resp) + } + + req := c.NewRequest("PATCH", "/v1/kv/data/foo") + req.Headers = http.Header{ + "Content-Type": []string{"application/json"}, + } + + if err := req.SetJSONBody(kvData); err != nil { + t.Fatal(err) + } + + apiResp, err := c.RawRequestWithContext(context.Background(), req) + if err == nil || apiResp.StatusCode != http.StatusUnsupportedMediaType { + t.Fatalf("expected PATCH request to fail with %d status code - err :%#v, resp: %#v\n", http.StatusUnsupportedMediaType, err, apiResp) + } +} + +func TestHandler_Patch_NotFound(t *testing.T) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "kv": kv.VersionedKVFactory, + }, + } + + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: Handler, + }) + + cluster.Start() + defer cluster.Cleanup() + + cores := cluster.Cores + + core := cores[0].Core + c := cluster.Cores[0].Client + vault.TestWaitActive(t, core) + + // Mount a KVv2 backend + err := c.Sys().Mount("kv", &api.MountInput{ + Type: "kv-v2", + }) + if err != nil { + t.Fatal(err) + } + + kvData := map[string]interface{}{ + "data": map[string]interface{}{ + "bar": "a", + }, + } + + resp, err := c.Logical().JSONMergePatch(context.Background(), "kv/data/foo", kvData) + if err == nil { + t.Fatalf("expected PATCH request to fail, resp: %#v\n", resp) + } + + responseError := err.(*api.ResponseError) + if responseError.StatusCode != http.StatusNotFound { + t.Fatalf("expected PATCH request to fail with %d status code - err: %#v, resp: %#v\n", http.StatusNotFound, responseError, resp) + } +} + +func TestHandler_Patch_Audit(t *testing.T) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "kv": kv.VersionedKVFactory, + }, + AuditBackends: map[string]audit.Factory{ + "file": auditFile.Factory, + }, + } + + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: Handler, + }) + + cluster.Start() + defer cluster.Cleanup() + + cores := cluster.Cores + + core := cores[0].Core + c := cluster.Cores[0].Client + vault.TestWaitActive(t, core) + + if err := c.Sys().Mount("kv/", &api.MountInput{ + Type: "kv-v2", + }); err != nil { + t.Fatalf("kv-v2 mount attempt failed - err: %#v\n", err) + } + + auditLogFile, err := ioutil.TempFile("", "httppatch") + if err != nil { + t.Fatal(err) + } + + err = c.Sys().EnableAuditWithOptions("file", &api.EnableAuditOptions{ + Type: "file", + Options: map[string]string{ + "file_path": auditLogFile.Name(), + }, + }) + + writeData := map[string]interface{}{ + "data": map[string]interface{}{ + "bar": "a", + }, + } + + resp, err := c.Logical().Write("kv/data/foo", writeData) + if err != nil { + t.Fatalf("write request failed, err: %#v, resp: %#v\n", err, resp) + } + + patchData := map[string]interface{}{ + "data": map[string]interface{}{ + "baz": "b", + }, + } + + resp, err = c.Logical().JSONMergePatch(context.Background(), "kv/data/foo", patchData) + if err != nil { + t.Fatalf("patch request failed, err: %#v, resp: %#v\n", err, resp) + } + + patchRequestLogCount := 0 + patchResponseLogCount := 0 + decoder := json.NewDecoder(auditLogFile) + + var auditRecord map[string]interface{} + for decoder.Decode(&auditRecord) == nil { + auditRequest := map[string]interface{}{} + + if req, ok := auditRecord["request"]; ok { + auditRequest = req.(map[string]interface{}) + } + + if auditRequest["operation"] == "patch" && auditRecord["type"] == "request" { + patchRequestLogCount += 1 + } else if auditRequest["operation"] == "patch" && auditRecord["type"] == "response" { + patchResponseLogCount += 1 + } + } + + if patchRequestLogCount != 1 { + t.Fatalf("expected 1 patch request audit log record, saw %d\n", patchRequestLogCount) + } + + if patchResponseLogCount != 1 { + t.Fatalf("expected 1 patch response audit log record, saw %d\n", patchResponseLogCount) + } +} diff --git a/http/logical.go b/http/logical.go index dd9abce34..7984f8ac0 100644 --- a/http/logical.go +++ b/http/logical.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "io" + "mime" "net" "net/http" "strconv" @@ -38,6 +39,8 @@ func (b *bufferedReader) Close() error { return b.rOrig.Close() } +const MergePatchContentTypeHeader = "application/merge-patch+json" + func buildLogicalRequestNoAuth(perfStandby bool, w http.ResponseWriter, r *http.Request) (*logical.Request, io.ReadCloser, int, error) { ns, err := namespace.FromContext(r.Context()) if err != nil { @@ -139,6 +142,34 @@ func buildLogicalRequestNoAuth(perfStandby bool, w http.ResponseWriter, r *http. } } + case "PATCH": + op = logical.PatchOperation + + contentTypeHeader := r.Header.Get("Content-Type") + contentType, _, err := mime.ParseMediaType(contentTypeHeader) + if err != nil { + status := http.StatusBadRequest + logical.AdjustErrorStatusCode(&status, err) + return nil, nil, status, err + } + + if contentType != MergePatchContentTypeHeader { + return nil, nil, http.StatusUnsupportedMediaType, fmt.Errorf("PATCH requires Content-Type of %s, provided %s", MergePatchContentTypeHeader, contentType) + } + + origBody, err = parseJSONRequest(perfStandby, r, w, &data) + + if err == io.EOF { + data = nil + err = nil + } + + if err != nil { + status := http.StatusBadRequest + logical.AdjustErrorStatusCode(&status, err) + return nil, nil, status, fmt.Errorf("error parsing JSON") + } + case "LIST": op = logical.ListOperation if !strings.HasSuffix(path, "/") { diff --git a/sdk/framework/backend.go b/sdk/framework/backend.go index d498dd4ee..673351a47 100644 --- a/sdk/framework/backend.go +++ b/sdk/framework/backend.go @@ -3,6 +3,7 @@ package framework import ( "context" "crypto/rand" + "encoding/json" "fmt" "io" "io/ioutil" @@ -13,6 +14,7 @@ import ( "sync" "time" + jsonpatch "github.com/evanphx/json-patch" "github.com/hashicorp/errwrap" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-kms-wrapping/entropy" @@ -118,6 +120,10 @@ type InvalidateFunc func(context.Context, string) // Initialize() just after a plugin has been mounted. type InitializeFunc func(context.Context, *logical.InitializationRequest) error +// PatchPreprocessorFunc is used by HandlePatchOperation in order to shape +// the input as defined by request handler prior to JSON marshaling +type PatchPreprocessorFunc func(map[string]interface{}) (map[string]interface{}, error) + // Initialize is the logical.Backend implementation. func (b *Backend) Initialize(ctx context.Context, req *logical.InitializationRequest) error { if b.InitializeFunc != nil { @@ -272,6 +278,57 @@ func (b *Backend) HandleRequest(ctx context.Context, req *logical.Request) (*log return callback(ctx, req, &fd) } +// HandlePatchOperation acts as an abstraction for performing JSON merge patch +// operations (see https://datatracker.ietf.org/doc/html/rfc7396) for HTTP +// PATCH requests. It is responsible for properly processing and marshalling +// the input and existing resource prior to performing the JSON merge operation +// using the MergePatch function from the json-patch library. The preprocessor +// is an arbitrary func that can be provided to further process the input. The +// MergePatch function accepts and returns byte arrays. +func HandlePatchOperation(input *FieldData, resource map[string]interface{}, preprocessor PatchPreprocessorFunc) ([]byte, error) { + var err error + + if resource == nil { + return nil, fmt.Errorf("resource does not exist") + } + + inputMap := map[string]interface{}{} + + // Parse all fields to ensure data types are handled properly according to the FieldSchema + for key := range input.Raw { + val, ok := input.GetOk(key) + + // Only accept fields in the schema + if ok { + inputMap[key] = val + } + } + + if preprocessor != nil { + inputMap, err = preprocessor(inputMap) + if err != nil { + return nil, err + } + } + + marshaledResource, err := json.Marshal(resource) + if err != nil { + return nil, err + } + + marshaledInput, err := json.Marshal(inputMap) + if err != nil { + return nil, err + } + + modified, err := jsonpatch.MergePatch(marshaledResource, marshaledInput) + if err != nil { + return nil, err + } + + return modified, nil +} + // SpecialPaths is the logical.Backend implementation. func (b *Backend) SpecialPaths() *logical.Paths { return b.PathsSpecial diff --git a/sdk/logical/request.go b/sdk/logical/request.go index b88aabce2..e683217a6 100644 --- a/sdk/logical/request.go +++ b/sdk/logical/request.go @@ -350,6 +350,7 @@ const ( CreateOperation Operation = "create" ReadOperation = "read" UpdateOperation = "update" + PatchOperation = "patch" DeleteOperation = "delete" ListOperation = "list" HelpOperation = "help" diff --git a/sdk/logical/response_util.go b/sdk/logical/response_util.go index 6ae3005b7..524406937 100644 --- a/sdk/logical/response_util.go +++ b/sdk/logical/response_util.go @@ -17,7 +17,7 @@ import ( func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) { if err == nil && (resp == nil || !resp.IsError()) { switch { - case req.Operation == ReadOperation: + case req.Operation == ReadOperation, req.Operation == PatchOperation: if resp == nil { return http.StatusNotFound, nil } diff --git a/vault/acl.go b/vault/acl.go index 3d07c4089..fc9f353aa 100644 --- a/vault/acl.go +++ b/vault/acl.go @@ -305,6 +305,9 @@ func (a *ACL) Capabilities(ctx context.Context, path string) (pathCapabilities [ if capabilities&CreateCapabilityInt > 0 { pathCapabilities = append(pathCapabilities, CreateCapability) } + if capabilities&PatchCapabilityInt > 0 { + pathCapabilities = append(pathCapabilities, PatchCapability) + } // If "deny" is explicitly set or if the path has no capabilities at all, // set the path capabilities to "deny" @@ -406,6 +409,8 @@ CHECK: operationAllowed = capabilities&DeleteCapabilityInt > 0 case logical.CreateOperation: operationAllowed = capabilities&CreateCapabilityInt > 0 + case logical.PatchOperation: + operationAllowed = capabilities&PatchCapabilityInt > 0 // These three re-use UpdateCapabilityInt since that's the most appropriate // capability/operation mapping @@ -440,7 +445,7 @@ CHECK: // Only check parameter permissions for operations that can modify // parameters. - if op == logical.ReadOperation || op == logical.UpdateOperation || op == logical.CreateOperation { + if op == logical.ReadOperation || op == logical.UpdateOperation || op == logical.CreateOperation || op == logical.PatchOperation { for _, parameter := range permissions.RequiredParameters { if _, ok := req.Data[strings.ToLower(parameter)]; !ok { return diff --git a/vault/acl_test.go b/vault/acl_test.go index 29c690e01..fe3a33e8a 100644 --- a/vault/acl_test.go +++ b/vault/acl_test.go @@ -238,6 +238,12 @@ func testACLSingle(t *testing.T, ns *namespace.Namespace) { {logical.UpdateOperation, "foo/bar", false, true}, {logical.CreateOperation, "foo/bar", true, true}, + {logical.ReadOperation, "baz/quux", true, false}, + {logical.CreateOperation, "baz/quux", true, false}, + {logical.PatchOperation, "baz/quux", true, false}, + {logical.ListOperation, "baz/quux", false, false}, + {logical.UpdateOperation, "baz/quux", false, false}, + // Path segment wildcards {logical.ReadOperation, "test/foo/bar/segment", false, false}, {logical.ReadOperation, "test/foo/segment", true, false}, @@ -341,6 +347,12 @@ func testLayeredACL(t *testing.T, acl *ACL, ns *namespace.Namespace) { {logical.ListOperation, "foo/bar", false, false}, {logical.UpdateOperation, "foo/bar", false, false}, {logical.CreateOperation, "foo/bar", false, false}, + + {logical.ReadOperation, "baz/quux", false, false}, + {logical.ListOperation, "baz/quux", false, false}, + {logical.UpdateOperation, "baz/quux", false, false}, + {logical.CreateOperation, "baz/quux", false, false}, + {logical.PatchOperation, "baz/quux", false, false}, } for _, tc := range tcases { @@ -864,6 +876,9 @@ path "sys/*" { path "foo/bar" { capabilities = ["read", "create", "sudo"] } +path "baz/quux" { + capabilities = ["read", "create", "patch"] +} path "test/+/segment" { capabilities = ["read"] } @@ -912,6 +927,9 @@ path "sys/seal" { path "foo/bar" { capabilities = ["deny"] } +path "baz/quux" { + capabilities = ["deny"] +} ` // test merging diff --git a/vault/external_tests/kv/kv_patch_test.go b/vault/external_tests/kv/kv_patch_test.go new file mode 100644 index 000000000..33d36f4f0 --- /dev/null +++ b/vault/external_tests/kv/kv_patch_test.go @@ -0,0 +1,57 @@ +package kv + +import ( + "context" + "testing" + + logicalKv "github.com/hashicorp/vault-plugin-secrets-kv" + "github.com/hashicorp/vault/api" + vaulthttp "github.com/hashicorp/vault/http" + "github.com/hashicorp/vault/sdk/logical" + "github.com/hashicorp/vault/vault" +) + +// Verifies that patching works by default with the root token +func TestKV_Patch_RootToken(t *testing.T) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "kv": logicalKv.Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + core := cluster.Cores[0] + client := core.Client + + // make sure this client is using the root token + client.SetToken(cluster.RootToken) + + // Enable KVv2 + err := client.Sys().Mount("kv", &api.MountInput{ + Type: "kv-v2", + }) + if err != nil { + t.Fatal(err) + } + + // Write a kv value and patch it + _, err = client.Logical().Write("kv/data/foo", map[string]interface{}{"data": map[string]interface{}{"bar": "baz"}}) + if err != nil { + t.Fatal(err) + } + + _, err = client.Logical().JSONMergePatch(context.Background(), "kv/data/foo", map[string]interface{}{"data": map[string]interface{}{"bar": "quux"}}) + if err != nil { + t.Fatal(err) + } + + secret, err := client.Logical().Read("kv/data/foo") + bar := secret.Data["data"].(map[string]interface{})["bar"] + if bar != "quux" { + t.Fatalf("expected bar to be quux but it was %q", bar) + } +} diff --git a/vault/external_tests/misc/kvv2_upgrade_test.go b/vault/external_tests/kv/kvv2_upgrade_test.go similarity index 99% rename from vault/external_tests/misc/kvv2_upgrade_test.go rename to vault/external_tests/kv/kvv2_upgrade_test.go index 39d747e8a..3d3eb486f 100644 --- a/vault/external_tests/misc/kvv2_upgrade_test.go +++ b/vault/external_tests/kv/kvv2_upgrade_test.go @@ -1,4 +1,4 @@ -package misc +package kv import ( "bytes" diff --git a/vault/logical_system.go b/vault/logical_system.go index 1675475a4..c41d3cb82 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3398,7 +3398,8 @@ func hasMountAccess(ctx context.Context, acl *ACL, path string) bool { perms.CapabilitiesBitmap&ListCapabilityInt > 0, perms.CapabilitiesBitmap&ReadCapabilityInt > 0, perms.CapabilitiesBitmap&SudoCapabilityInt > 0, - perms.CapabilitiesBitmap&UpdateCapabilityInt > 0: + perms.CapabilitiesBitmap&UpdateCapabilityInt > 0, + perms.CapabilitiesBitmap&PatchCapabilityInt > 0: aclCapabilitiesGiven = true @@ -3684,6 +3685,9 @@ func (b *SystemBackend) pathInternalUIResultantACL(ctx context.Context, req *log if perms.CapabilitiesBitmap&UpdateCapabilityInt > 0 { capabilities = append(capabilities, UpdateCapability) } + if perms.CapabilitiesBitmap&PatchCapabilityInt > 0 { + capabilities = append(capabilities, PatchCapability) + } // If "deny" is explicitly set or if the path has no capabilities at all, // set the path capabilities to "deny" diff --git a/vault/policy.go b/vault/policy.go index a4686b1d8..e80d1657e 100644 --- a/vault/policy.go +++ b/vault/policy.go @@ -26,6 +26,7 @@ const ( ListCapability = "list" SudoCapability = "sudo" RootCapability = "root" + PatchCapability = "patch" // Backwards compatibility OldDenyPathPolicy = "deny" @@ -42,6 +43,7 @@ const ( DeleteCapabilityInt ListCapabilityInt SudoCapabilityInt + PatchCapabilityInt ) // Error constants for testing @@ -83,6 +85,7 @@ var cap2Int = map[string]uint32{ DeleteCapability: DeleteCapabilityInt, ListCapability: ListCapabilityInt, SudoCapability: SudoCapabilityInt, + PatchCapability: PatchCapabilityInt, } type egpPath struct { @@ -390,7 +393,7 @@ func parsePaths(result *Policy, list *ast.ObjectList, performTemplating bool, en pc.Capabilities = []string{DenyCapability} pc.Permissions.CapabilitiesBitmap = DenyCapabilityInt goto PathFinished - case CreateCapability, ReadCapability, UpdateCapability, DeleteCapability, ListCapability, SudoCapability: + case CreateCapability, ReadCapability, UpdateCapability, DeleteCapability, ListCapability, SudoCapability, PatchCapability: pc.Permissions.CapabilitiesBitmap |= cap2Int[cap] default: return fmt.Errorf("path %q: invalid capability %q", key, cap) diff --git a/vault/policy_test.go b/vault/policy_test.go index 045fae865..7f09d7cb2 100644 --- a/vault/policy_test.go +++ b/vault/policy_test.go @@ -83,6 +83,9 @@ path "test/req" { capabilities = ["create", "sudo"] required_parameters = ["foo"] } +path "test/patch" { + capabilities = ["patch"] +} path "test/mfa" { capabilities = ["create", "sudo"] mfa_methods = ["my_totp", "my_totp2"] @@ -244,6 +247,13 @@ func TestPolicy_Parse(t *testing.T) { RequiredParameters: []string{"foo"}, }, }, + { + Path: "test/patch", + Capabilities: []string{"patch"}, + Permissions: &ACLPermissions{ + CapabilitiesBitmap: (PatchCapabilityInt), + }, + }, { Path: "test/mfa", Capabilities: []string{