From 8f7d76d78138540b883ac89e0e7a465ab2464f66 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira de Souza Date: Wed, 23 Jul 2025 11:33:25 -0300 Subject: [PATCH] VAULT-35087: add Open-API support for secret recovery operations (#31331) * support open-api secret recovery operations * add changelog * Update changelog/31331.txt Co-authored-by: miagilepner --------- Co-authored-by: miagilepner --- changelog/31331.txt | 3 + sdk/framework/openapi.go | 31 +++++++ sdk/framework/openapi_test.go | 116 +++++++++++++++++++------ sdk/framework/testdata/operations.json | 26 ++++++ vault/logical_cubbyhole.go | 8 +- 5 files changed, 153 insertions(+), 31 deletions(-) create mode 100644 changelog/31331.txt diff --git a/changelog/31331.txt b/changelog/31331.txt new file mode 100644 index 0000000000..be1e2e447c --- /dev/null +++ b/changelog/31331.txt @@ -0,0 +1,3 @@ +```release-note:improvement +openapi: Add OpenAPI support for secret recovery operations. +``` diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index c4e1d563a0..32fea12eda 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -245,10 +245,12 @@ func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocum func documentPath(p *Path, backend *Backend, requestResponsePrefix string, doc *OASDocument) error { var sudoPaths []string var unauthPaths []string + var allowSnapshotReadPaths []string if backend.PathsSpecial != nil { sudoPaths = backend.PathsSpecial.Root unauthPaths = backend.PathsSpecial.Unauthenticated + allowSnapshotReadPaths = backend.PathsSpecial.AllowSnapshotRead } // Convert optional parameters into distinct patterns to be processed independently. @@ -278,6 +280,7 @@ func documentPath(p *Path, backend *Backend, requestResponsePrefix string, doc * pi.Sudo = specialPathMatch(path, sudoPaths) pi.Unauthenticated = specialPathMatch(path, unauthPaths) pi.DisplayAttrs = withoutOperationHints(p.DisplayAttrs) + allowSnapshotRead := specialPathMatch(path, allowSnapshotReadPaths) // If the newer style Operations map isn't defined, create one from the legacy fields. operations := p.Operations @@ -329,6 +332,14 @@ func documentPath(p *Path, backend *Backend, requestResponsePrefix string, doc * continue } + // OpenAPI doesn't allow for multiple operations on the same path and with the same HTTP method, so both + // Create, Update and Recover which operate on either POST or PUT methods are folded into Update (under POST + // method on OpenAPI). Furthermore, so far there's no use case for a Recover operation on an endpoint that + // doesn't support either Create or Update, so we skip it here as well. + if opType == logical.RecoverOperation { + continue + } + if opType == logical.CreateOperation { pi.CreateSupported = true @@ -385,6 +396,17 @@ func documentPath(p *Path, backend *Backend, requestResponsePrefix string, doc * addFieldToOASSchema(s, name, field) } + // The recover operation is a special case, it's under the same POST/PUT method as Create and + // Update, but it's triggered via a query parameter, not a field in the body. + if operations[logical.RecoverOperation] != nil && opType != logical.PatchOperation { + op.Parameters = append(op.Parameters, OASParameter{ + Name: "recover_snapshot_id", + Description: "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", + In: "query", + Schema: &OASSchema{Type: "string"}, + }) + } + // Make the ordering deterministic, so that the generated OpenAPI spec document, observed over several // versions, doesn't contain spurious non-semantic changes. sort.Strings(s.Required) @@ -466,6 +488,15 @@ func documentPath(p *Path, backend *Backend, requestResponsePrefix string, doc * op.Parameters = append(op.Parameters, p) } + if allowSnapshotRead && opType != logical.DeleteOperation { + op.Parameters = append(op.Parameters, OASParameter{ + Name: "read_snapshot_id", + Description: "Targets the read operation to the provided loaded snapshot Id", + In: "query", + Schema: &OASSchema{Type: "string"}, + }) + } + // Sort parameters for a stable output sort.Slice(op.Parameters, func(i, j int) bool { return op.Parameters[i].Name < op.Parameters[j].Name diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 474d0d4f19..38fb5c56ac 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "reflect" "regexp" + "slices" "sort" "strings" "testing" @@ -246,18 +247,22 @@ func TestOpenAPI_SplitFields(t *testing.T) { func TestOpenAPI_SpecialPaths(t *testing.T) { tests := map[string]struct { - pattern string - rootPaths []string - rootExpected bool - unauthenticatedPaths []string - unauthenticatedExpected bool + pattern string + rootPaths []string + rootExpected bool + unauthenticatedPaths []string + unauthenticatedExpected bool + allowSnapshotRead []string + allowSnapshotReadExpected bool }{ "empty": { - pattern: "foo", - rootPaths: []string{}, - rootExpected: false, - unauthenticatedPaths: []string{}, - unauthenticatedExpected: false, + pattern: "foo", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + allowSnapshotRead: []string{}, + allowSnapshotReadExpected: false, }, "exact-match-unauthenticated": { pattern: "foo", @@ -336,17 +341,75 @@ func TestOpenAPI_SpecialPaths(t *testing.T) { unauthenticatedPaths: []string{"foo/bar"}, unauthenticatedExpected: false, }, + "exact-match-snapshot-read": { + pattern: "foo", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + allowSnapshotRead: []string{"foo"}, + allowSnapshotReadExpected: true, + }, + "asterisk-match-snapshot-read": { + pattern: "foo/bar", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + allowSnapshotRead: []string{"foo/*"}, + allowSnapshotReadExpected: true, + }, + "no-match-snapshot-read": { + pattern: "foo/bar", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + allowSnapshotRead: []string{"baz"}, + allowSnapshotReadExpected: false, + }, + "multiple-snapshot-read-paths": { + pattern: "foo/bar", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + allowSnapshotRead: []string{"foo/*", "baz"}, + allowSnapshotReadExpected: true, + }, + "plus-match-snapshot-read": { + pattern: "foo/bar/baz", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + allowSnapshotRead: []string{"foo/+/baz"}, + allowSnapshotReadExpected: true, + }, + "plus-and-asterisk-snapshot-read": { + pattern: "foo/bar/baz/something", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + allowSnapshotRead: []string{"foo/+/baz/*"}, + allowSnapshotReadExpected: true, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { doc := NewOASDocument("version") path := Path{ Pattern: test.pattern, + Operations: map[logical.Operation]OperationHandler{ + logical.ReadOperation: &PathOperation{}, + }, } backend := &Backend{ PathsSpecial: &logical.Paths{ - Root: test.rootPaths, - Unauthenticated: test.unauthenticatedPaths, + Root: test.rootPaths, + Unauthenticated: test.unauthenticatedPaths, + AllowSnapshotRead: test.allowSnapshotRead, }, BackendType: logical.TypeLogical, } @@ -364,6 +427,16 @@ func TestOpenAPI_SpecialPaths(t *testing.T) { if actual != test.unauthenticatedExpected { t.Fatalf("Test (unauth): expected: %v; got: %v", test.unauthenticatedExpected, actual) } + + var supportsSnapshotId bool + if doc.Paths["/"+test.pattern].Get != nil { + supportsSnapshotId = slices.ContainsFunc(doc.Paths["/"+test.pattern].Get.Parameters, func(p OASParameter) bool { + return p.Name == "read_snapshot_id" + }) + } + if supportsSnapshotId != test.allowSnapshotReadExpected { + t.Fatalf("Test (allowSnapshotRead): expected: %v; got: %v", test.allowSnapshotReadExpected, actual) + } }) } } @@ -475,6 +548,9 @@ func TestOpenAPI_Paths(t *testing.T) { Summary: "This shouldn't show up", Unpublished: true, }, + logical.RecoverOperation: &PathOperation{ + Summary: "Recover Summary shouldn't show up", + }, }, DisplayAttrs: &DisplayAttributes{ Navigation: true, @@ -482,7 +558,8 @@ func TestOpenAPI_Paths(t *testing.T) { } sp := &logical.Paths{ - Root: []string{"foo*"}, + Root: []string{"foo*"}, + AllowSnapshotRead: []string{"*"}, } testPath(t, p, sp, expected("operations")) }) @@ -930,19 +1007,6 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string) } } -func getPathOp(pi *OASPathItem, op string) *OASOperation { - switch op { - case "get": - return pi.Get - case "post": - return pi.Post - case "delete": - return pi.Delete - default: - panic("unexpected operation: " + op) - } -} - func expected(name string) string { data, err := ioutil.ReadFile(filepath.Join("testdata", name+".json")) if err != nil { diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index 43d7a46a08..be3188c2b4 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -43,6 +43,14 @@ "schema": { "type": "string" } + }, + { + "name": "read_snapshot_id", + "description": "Targets the read operation to the provided loaded snapshot Id", + "in": "query", + "schema": { + "type": "string" + } } ], "responses": { @@ -58,6 +66,16 @@ "tags": [ "secrets" ], + "parameters": [ + { + "name": "recover_snapshot_id", + "description": "Triggers a recover operation using the given snapshot ID. Request body is ignored when a recover operation is requested.", + "in": "query", + "schema": { + "type": "string" + } + } + ], "requestBody": { "required": true, "content": { @@ -142,6 +160,14 @@ ] }, "required": true + }, + { + "name": "read_snapshot_id", + "description": "Targets the read operation to the provided loaded snapshot Id", + "in": "query", + "schema": { + "type": "string" + } } ], "responses": { diff --git a/vault/logical_cubbyhole.go b/vault/logical_cubbyhole.go index 7445e7853d..8995089a8b 100644 --- a/vault/logical_cubbyhole.go +++ b/vault/logical_cubbyhole.go @@ -79,7 +79,7 @@ func (b *CubbyholeBackend) paths() []*framework.Path { DisplayAttrs: &framework.DisplayAttributes{ OperationVerb: "write", }, - Summary: "Store a secret at the specified location.", + Summary: "Store a secret at the specified location, or (enterprise-only) recover it from given snapshot Id.", }, logical.CreateOperation: &framework.PathOperation{ Callback: b.handleWrite, @@ -104,10 +104,8 @@ func (b *CubbyholeBackend) paths() []*framework.Path { }, logical.RecoverOperation: &framework.PathOperation{ Callback: b.handleWrite, - DisplayAttrs: &framework.DisplayAttributes{ - OperationVerb: "recover", - }, - Summary: "Recover a secret at the specified location.", + // just like CreateOperation, recover is folded into update for OpenAPI documentation purposes, so + // no operation verb or summary is set }, },