diff --git a/api/plugin_helpers.go b/api/plugin_helpers.go index c4985107da..6602a044bd 100644 --- a/api/plugin_helpers.go +++ b/api/plugin_helpers.go @@ -37,9 +37,9 @@ const ( // path matches that path or not (useful specifically for the paths that // contain templated fields.) var sudoPaths = map[string]*regexp.Regexp{ - "/auth/{mount_path}/accessors/": regexp.MustCompile(`^/auth/.+/accessors/$`), - "/{mount_path}/root": regexp.MustCompile(`^/.+/root$`), - "/{mount_path}/root/sign-self-issued": regexp.MustCompile(`^/.+/root/sign-self-issued$`), + "/auth/token/accessors/": regexp.MustCompile(`^/auth/token/accessors/$`), + "/pki/root": regexp.MustCompile(`^/pki/root$`), + "/pki/root/sign-self-issued": regexp.MustCompile(`^/pki/root/sign-self-issued$`), "/sys/audit": regexp.MustCompile(`^/sys/audit$`), "/sys/audit/{path}": regexp.MustCompile(`^/sys/audit/.+$`), "/sys/auth/{path}": regexp.MustCompile(`^/sys/auth/.+$`), diff --git a/changelog/17839.txt b/changelog/17839.txt deleted file mode 100644 index 48d5ce6e33..0000000000 --- a/changelog/17839.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:improvement -openapi: Add {mount_path} parameter to secret & auth paths in the generated openapi.json spec. -``` diff --git a/sdk/framework/backend.go b/sdk/framework/backend.go index 7809a7e6e1..73d8f14e60 100644 --- a/sdk/framework/backend.go +++ b/sdk/framework/backend.go @@ -539,9 +539,16 @@ func (b *Backend) handleRootHelp(req *logical.Request) (*logical.Response, error // names in the OAS document. requestResponsePrefix := req.GetString("requestResponsePrefix") + // Generic mount paths will primarily be used for code generation purposes. + // This will result in dynamic mount paths being placed instead of + // hardcoded default paths. For example /auth/approle/login would be replaced + // with /auth/{mountPath}/login. This will be replaced for all secrets + // engines and auth methods that are enabled. + genericMountPaths, _ := req.Get("genericMountPaths").(bool) + // Build OpenAPI response for the entire backend doc := NewOASDocument() - if err := documentPaths(b, requestResponsePrefix, doc); err != nil { + if err := documentPaths(b, requestResponsePrefix, genericMountPaths, doc); err != nil { b.Logger().Warn("error generating OpenAPI", "error", err) } diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index abd06c5360..4659f7ae2f 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -13,8 +13,6 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/version" "github.com/mitchellh/mapstructure" - "golang.org/x/text/cases" - "golang.org/x/text/language" ) // OpenAPI specification (OAS): https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md @@ -208,16 +206,16 @@ var ( altRootsRe = regexp.MustCompile(`^\(([\w\-_]+(?:\|[\w\-_]+)+)\)(/.*)$`) // Pattern starting with alts, e.g. "(root1|root2)/(?Pregex)" cleanCharsRe = regexp.MustCompile("[()^$?]") // Set of regex characters that will be stripped during cleaning cleanSuffixRe = regexp.MustCompile(`/\?\$?$`) // Path suffix patterns that will be stripped during cleaning - nonWordRe = regexp.MustCompile(`[^a-zA-Z0-9]+`) // Match a sequence of non-word characters + nonWordRe = regexp.MustCompile(`[^\w]+`) // Match a sequence of non-word characters pathFieldsRe = regexp.MustCompile(`{(\w+)}`) // Capture OpenAPI-style named parameters, e.g. "lookup/{urltoken}", reqdRe = regexp.MustCompile(`\(?\?P<(\w+)>[^)]*\)?`) // Capture required parameters, e.g. "(?Pregex)" wsRe = regexp.MustCompile(`\s+`) // Match whitespace, to be compressed during cleaning ) // documentPaths parses all paths in a framework.Backend into OpenAPI paths. -func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocument) error { +func documentPaths(backend *Backend, requestResponsePrefix string, genericMountPaths bool, doc *OASDocument) error { for _, p := range backend.Paths { - if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, backend.BackendType, doc); err != nil { + if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, genericMountPaths, backend.BackendType, doc); err != nil { return err } } @@ -226,7 +224,7 @@ func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocum } // documentPath parses a framework.Path into one or more OpenAPI paths. -func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, backendType logical.BackendType, doc *OASDocument) error { +func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, genericMountPaths bool, backendType logical.BackendType, doc *OASDocument) error { var sudoPaths []string var unauthPaths []string @@ -235,11 +233,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st unauthPaths = specialPaths.Unauthenticated } - defaultMountPath := requestResponsePrefix - if requestResponsePrefix == "kv" { - defaultMountPath = "secret" - } - // Convert optional parameters into distinct patterns to be processed independently. paths := expandPattern(p.Pattern) @@ -270,17 +263,16 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st // Body fields will be added to individual operations. pathFields, bodyFields := splitFields(p.Fields, path) - // Add mount path as a parameter - if defaultMountPath != "system" && defaultMountPath != "identity" { + if genericMountPaths && requestResponsePrefix != "system" && requestResponsePrefix != "identity" { + // Add mount path as a parameter p := OASParameter{ - Name: "mount_path", - Description: "Path where the backend was mounted; the endpoint path will be offset by the mount path", + Name: "mountPath", + Description: "Path that the backend was mounted at", In: "path", Schema: &OASSchema{ - Type: "string", - Default: defaultMountPath, + Type: "string", }, - Required: false, + Required: true, } pi.Parameters = append(pi.Parameters, p) @@ -349,7 +341,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st op.Summary = props.Summary op.Description = props.Description op.Deprecated = props.Deprecated - op.OperationID = constructOperationID(string(opType), path, defaultMountPath) // Add any fields not present in the path as body parameters for POST. if opType == logical.CreateOperation || opType == logical.UpdateOperation { @@ -524,19 +515,6 @@ func constructRequestName(requestResponsePrefix string, path string) string { return b.String() } -func constructOperationID(method, path, defaultMountPath string) string { - // title caser - title := cases.Title(language.English) - - // Space-split on non-words, title case everything, recombine - id := nonWordRe.ReplaceAllLiteralString(strings.ToLower(path), " ") - id = fmt.Sprintf("%s %s", defaultMountPath, id) - id = title.String(id) - id = strings.ReplaceAll(id, " ", "") - - return method + id -} - func specialPathMatch(path string, specialPaths []string) bool { // Test for exact or prefix match of special paths. for _, sp := range specialPaths { @@ -749,3 +727,61 @@ func cleanResponse(resp *logical.Response) *cleanedResponse { Headers: resp.Headers, } } + +// CreateOperationIDs generates unique operationIds for all paths/methods. +// The transform will convert path/method into camelcase. e.g.: +// +// /sys/tools/random/{urlbytes} -> postSysToolsRandomUrlbytes +// +// In the unlikely case of a duplicate ids, a numeric suffix is added: +// +// postSysToolsRandomUrlbytes_2 +// +// An optional user-provided suffix ("context") may also be appended. +func (d *OASDocument) CreateOperationIDs(context string) { + opIDCount := make(map[string]int) + var paths []string + + // traverse paths in a stable order to ensure stable output + for path := range d.Paths { + paths = append(paths, path) + } + sort.Strings(paths) + + for _, path := range paths { + pi := d.Paths[path] + for _, method := range []string{"get", "post", "delete"} { + var oasOperation *OASOperation + switch method { + case "get": + oasOperation = pi.Get + case "post": + oasOperation = pi.Post + case "delete": + oasOperation = pi.Delete + } + + if oasOperation == nil { + continue + } + + // Space-split on non-words, title case everything, recombine + opID := nonWordRe.ReplaceAllString(strings.ToLower(path), " ") + opID = strings.Title(opID) + opID = method + strings.ReplaceAll(opID, " ", "") + + // deduplicate operationIds. This is a safeguard, since generated IDs should + // already be unique given our current path naming conventions. + opIDCount[opID]++ + if opIDCount[opID] > 1 { + opID = fmt.Sprintf("%s_%d", opID, opIDCount[opID]) + } + + if context != "" { + opID += "_" + context + } + + oasOperation.OperationID = opID + } + } +} diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index b2cf263240..8d3ecfea01 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -271,7 +271,7 @@ func TestOpenAPI_SpecialPaths(t *testing.T) { Root: test.rootPaths, Unauthenticated: test.unauthPaths, } - err := documentPath(&path, sp, "kv", logical.TypeLogical, doc) + err := documentPath(&path, sp, "kv", false, logical.TypeLogical, doc) if err != nil { t.Fatal(err) } @@ -517,32 +517,39 @@ func TestOpenAPI_OperationID(t *testing.T) { }, } - doc := NewOASDocument() - err := documentPath(path1, nil, "kv", logical.TypeLogical, doc) - if err != nil { - t.Fatal(err) - } - err = documentPath(path2, nil, "kv", logical.TypeLogical, doc) - if err != nil { - t.Fatal(err) - } + for _, context := range []string{"", "bar"} { + doc := NewOASDocument() + err := documentPath(path1, nil, "kv", false, logical.TypeLogical, doc) + if err != nil { + t.Fatal(err) + } + err = documentPath(path2, nil, "kv", false, logical.TypeLogical, doc) + if err != nil { + t.Fatal(err) + } + doc.CreateOperationIDs(context) - tests := []struct { - path string - op string - opID string - }{ - {"/Foo/{id}", "get", "readSecretFooId"}, - {"/foo/{id}", "post", "updateSecretFooId"}, - {"/foo/{id}", "delete", "deleteSecretFooId"}, - } + tests := []struct { + path string + op string + opID string + }{ + {"/Foo/{id}", "get", "getFooId"}, + {"/foo/{id}", "get", "getFooId_2"}, + {"/foo/{id}", "post", "postFooId"}, + {"/foo/{id}", "delete", "deleteFooId"}, + } - for _, test := range tests { - actual := getPathOp(doc.Paths[test.path], test.op).OperationID - expected := test.opID + for _, test := range tests { + actual := getPathOp(doc.Paths[test.path], test.op).OperationID + expected := test.opID + if context != "" { + expected += "_" + context + } - if actual != expected { - t.Fatalf("expected %v, got %v", expected, actual) + if actual != expected { + t.Fatalf("expected %v, got %v", expected, actual) + } } } } @@ -576,7 +583,7 @@ func TestOpenAPI_CustomDecoder(t *testing.T) { } docOrig := NewOASDocument() - err := documentPath(p, nil, "kv", logical.TypeLogical, docOrig) + err := documentPath(p, nil, "kv", false, logical.TypeLogical, docOrig) if err != nil { t.Fatal(err) } @@ -639,9 +646,10 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string) t.Helper() doc := NewOASDocument() - if err := documentPath(path, sp, "kv", logical.TypeLogical, doc); err != nil { + if err := documentPath(path, sp, "kv", false, logical.TypeLogical, doc); err != nil { t.Fatal(err) } + doc.CreateOperationIDs("") docJSON, err := json.MarshalIndent(doc, "", " ") if err != nil { diff --git a/sdk/framework/path.go b/sdk/framework/path.go index fe29a40089..8a8b1c7587 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -317,7 +317,7 @@ func (p *Path) helpCallback(b *Backend) OperationFunc { // Build OpenAPI response for this path doc := NewOASDocument() - if err := documentPath(p, b.SpecialPaths(), requestResponsePrefix, b.BackendType, doc); err != nil { + if err := documentPath(p, b.SpecialPaths(), requestResponsePrefix, false, b.BackendType, doc); err != nil { b.Logger().Warn("error generating OpenAPI", "error", err) } diff --git a/sdk/framework/testdata/legacy.json b/sdk/framework/testdata/legacy.json index 81850f5e0c..cb1f7ebd3f 100644 --- a/sdk/framework/testdata/legacy.json +++ b/sdk/framework/testdata/legacy.json @@ -21,23 +21,12 @@ "type": "string" }, "required": true - }, - { - "name": "mount_path", - "description": "Path where the backend was mounted; the endpoint path will be offset by the mount path", - "in": "path", - "schema": { - "type": "string", - "default": "secret" - } } ], "get": { + "operationId": "getLookupId", "summary": "Synopsis", - "operationId": "readSecretLookupId", - "tags": [ - "secrets" - ], + "tags": ["secrets"], "responses": { "200": { "description": "OK" @@ -45,11 +34,9 @@ } }, "post": { + "operationId": "postLookupId", "summary": "Synopsis", - "operationId": "updateSecretLookupId", - "tags": [ - "secrets" - ], + "tags": ["secrets"], "requestBody": { "content": { "application/json": { diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index 44f9b46186..097399c02e 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -34,19 +34,10 @@ "type": "string" }, "required": true - }, - { - "name": "mount_path", - "description": "Path where the backend was mounted; the endpoint path will be offset by the mount path", - "in": "path", - "schema": { - "type": "string", - "default": "secret" - } } ], "get": { - "operationId": "readSecretFooId", + "operationId": "getFooId", "tags": ["secrets"], "summary": "My Summary", "description": "My Description", @@ -67,7 +58,7 @@ ] }, "post": { - "operationId": "updateSecretFooId", + "operationId": "postFooId", "tags": ["secrets"], "summary": "Update Summary", "description": "Update Description", diff --git a/sdk/framework/testdata/operations_list.json b/sdk/framework/testdata/operations_list.json index 9c6b13f821..e89622a3c4 100644 --- a/sdk/framework/testdata/operations_list.json +++ b/sdk/framework/testdata/operations_list.json @@ -33,19 +33,10 @@ "type": "string" }, "required": true - }, - { - "name": "mount_path", - "description": "Path where the backend was mounted; the endpoint path will be offset by the mount path", - "in": "path", - "schema": { - "type": "string", - "default": "secret" - } } ], "get": { - "operationId": "listSecretFooId", + "operationId": "getFooId", "tags": ["secrets"], "summary": "List Summary", "description": "List Description", diff --git a/sdk/framework/testdata/responses.json b/sdk/framework/testdata/responses.json index fac2c1ce17..b0e197babe 100644 --- a/sdk/framework/testdata/responses.json +++ b/sdk/framework/testdata/responses.json @@ -12,20 +12,9 @@ "paths": { "/foo": { "description": "Synopsis", - "parameters": [ - { - "name": "mount_path", - "description": "Path where the backend was mounted; the endpoint path will be offset by the mount path", - "in": "path", - "schema": { - "type": "string", - "default": "secret" - } - } - ], "x-vault-unauthenticated": true, "delete": { - "operationId": "deleteSecretFoo", + "operationId": "deleteFoo", "tags": ["secrets"], "summary": "Delete stuff", "responses": { @@ -35,7 +24,7 @@ } }, "get": { - "operationId": "readSecretFoo", + "operationId": "getFoo", "tags": ["secrets"], "summary": "My Summary", "description": "My Description", diff --git a/vault/logical_system.go b/vault/logical_system.go index 1df776cd42..118762712e 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -4408,10 +4408,14 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re return nil, err } + context := d.Get("context").(string) + // Set up target document and convert to map[string]interface{} which is what will // be received from plugin backends. doc := framework.NewOASDocument() + genericMountPaths, _ := d.Get("generic_mount_paths").(bool) + procMountGroup := func(group, mountPrefix string) error { for mount, entry := range resp.Data[group].(map[string]interface{}) { @@ -4429,7 +4433,7 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re req := &logical.Request{ Operation: logical.HelpOperation, Storage: req.Storage, - Data: map[string]interface{}{"requestResponsePrefix": pluginType}, + Data: map[string]interface{}{"requestResponsePrefix": pluginType, "genericMountPaths": genericMountPaths}, } resp, err := backend.HandleRequest(ctx, req) @@ -4483,8 +4487,8 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re } } - if mount != "sys/" && mount != "identity/" { - s := fmt.Sprintf("/%s{mount_path}/%s", mountPrefix, path) + if genericMountPaths && mount != "sys/" && mount != "identity/" { + s := fmt.Sprintf("/%s{mountPath}/%s", mountPrefix, path) doc.Paths[s] = obj } else { doc.Paths["/"+mountPrefix+mount+path] = obj @@ -4506,6 +4510,8 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re return nil, err } + doc.CreateOperationIDs(context) + buf, err := json.Marshal(doc) if err != nil { return nil, err diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 2aa12d76e8..a890e1c17e 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -3598,10 +3598,10 @@ func TestSystemBackend_OASGenericMount(t *testing.T) { path string tag string }{ - {"/auth/{mount_path}/lookup", "auth"}, - {"/{mount_path}/{path}", "secrets"}, + {"/auth/{mountPath}/lookup", "auth"}, + {"/{mountPath}/{path}", "secrets"}, {"/identity/group/id", "identity"}, - {"/{mount_path}/.*", "secrets"}, + {"/{mountPath}/.*", "secrets"}, {"/sys/policy", "system"}, } @@ -3683,10 +3683,10 @@ func TestSystemBackend_OpenAPI(t *testing.T) { path string tag string }{ - {"/auth/{mount_path}/lookup", "auth"}, - {"/{mount_path}/{path}", "secrets"}, + {"/auth/token/lookup", "auth"}, + {"/cubbyhole/{path}", "secrets"}, {"/identity/group/id", "identity"}, - {"/{mount_path}/.*", "secrets"}, // TODO update after kv repo update + {"/secret/.*", "secrets"}, // TODO update after kv repo update {"/sys/policy", "system"}, } diff --git a/website/content/api-docs/system/internal-specs-openapi.mdx b/website/content/api-docs/system/internal-specs-openapi.mdx index d51d1f40b6..68545600d3 100644 --- a/website/content/api-docs/system/internal-specs-openapi.mdx +++ b/website/content/api-docs/system/internal-specs-openapi.mdx @@ -31,6 +31,10 @@ This endpoint returns a single OpenAPI document describing all paths visible to | :----- | :---------------------------- | | `GET` | `/sys/internal/specs/openapi` | +### Parameters + +- `generic_mount_paths` `(bool: false)` – Used to specify whether to use generic mount paths. If set, the mount paths will be replaced with a dynamic parameter: `{mountPath}` + ### Sample Request