OpenAPI `generic_mount_paths` follow-up (#18663)

* OpenAPI `generic_mount_paths` follow-up

An incremental improvement within larger context discussed in #18560.

* Following the revert in #18617, re-introduce the change from
  `{mountPath}` to `{<path-of-mount>_mount_path}`; this is needed, as
  otherwise paths from multiple plugins would clash - e.g. almost every
  auth method would provide a conflicting definition for
  `auth/{mountPath}/login`, and the last one written into the map would
  win.

* Move the half of the functionality that was in `sdk/framework/` to
  `vault/logical_system.go` with the rest; this is needed, as
  `sdk/framework/` gets compiled in to externally built plugins, and
  therefore there may be version skew between it and the Vault main
  code. Implementing the `generic_mount_paths` feature entirely on one
  side of this boundary frees us from problems caused by this.

* Update the special exception that recognizes `system` and `identity`
  as singleton mounts to also include the other two singleton mounts,
  `cubbyhole` and `auth/token`.

* Include a comment that documents to restricted circumstances in which
  the `generic_mount_paths` option makes sense to use:

	    // Note that for this to actually be useful, you have to be using it with
	    // a Vault instance in which you have mounted one of each secrets engine
	    // and auth method of types you are interested in, at paths which identify
	    // their type, and for the KV secrets engine you will probably want to
	    // mount separate kv-v1 and kv-v2 mounts to include the documentation for
	    // each of those APIs.

* Fix tests

Also remove comment "// TODO update after kv repo update" which was
added 4 years ago in #5687 - the implied update has not happened.

* Add changelog

* Update 18663.txt
This commit is contained in:
Max Bowsher 2023-01-18 04:07:11 +00:00 committed by GitHub
parent e742440686
commit 4c5f583f39
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 59 additions and 44 deletions

3
changelog/18663.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
openapi: generic_mount_paths: Move implementation fully into server, rather than partially in plugin framework; recognize all 4 singleton mounts (auth/token, cubbyhole, identity, system) rather than just 2; change parameter from `{mountPath}` to `{<type>_mount_path}`
```

View File

@ -539,13 +539,6 @@ 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
vaultVersion := "unknown"
if b.System() != nil {
@ -557,7 +550,7 @@ func (b *Backend) handleRootHelp(req *logical.Request) (*logical.Response, error
}
doc := NewOASDocument(vaultVersion)
if err := documentPaths(b, requestResponsePrefix, genericMountPaths, doc); err != nil {
if err := documentPaths(b, requestResponsePrefix, doc); err != nil {
b.Logger().Warn("error generating OpenAPI", "error", err)
}

View File

@ -215,9 +215,9 @@ var (
)
// documentPaths parses all paths in a framework.Backend into OpenAPI paths.
func documentPaths(backend *Backend, requestResponsePrefix string, genericMountPaths bool, doc *OASDocument) error {
func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocument) error {
for _, p := range backend.Paths {
if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, genericMountPaths, backend.BackendType, doc); err != nil {
if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, backend.BackendType, doc); err != nil {
return err
}
}
@ -226,7 +226,7 @@ func documentPaths(backend *Backend, requestResponsePrefix string, genericMountP
}
// documentPath parses a framework.Path into one or more OpenAPI paths.
func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, genericMountPaths bool, backendType logical.BackendType, doc *OASDocument) error {
func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, backendType logical.BackendType, doc *OASDocument) error {
var sudoPaths []string
var unauthPaths []string
@ -265,21 +265,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
// Body fields will be added to individual operations.
pathFields, bodyFields := splitFields(p.Fields, path)
if genericMountPaths && requestResponsePrefix != "system" && requestResponsePrefix != "identity" {
// Add mount path as a parameter
p := OASParameter{
Name: "mountPath",
Description: "Path that the backend was mounted at",
In: "path",
Schema: &OASSchema{
Type: "string",
},
Required: true,
}
pi.Parameters = append(pi.Parameters, p)
}
for name, field := range pathFields {
location := "path"
required := true

View File

@ -270,7 +270,7 @@ func TestOpenAPI_SpecialPaths(t *testing.T) {
Root: test.rootPaths,
Unauthenticated: test.unauthPaths,
}
err := documentPath(&path, sp, "kv", false, logical.TypeLogical, doc)
err := documentPath(&path, sp, "kv", logical.TypeLogical, doc)
if err != nil {
t.Fatal(err)
}
@ -528,11 +528,11 @@ func TestOpenAPI_OperationID(t *testing.T) {
for _, context := range []string{"", "bar"} {
doc := NewOASDocument("version")
err := documentPath(path1, nil, "kv", false, logical.TypeLogical, doc)
err := documentPath(path1, nil, "kv", logical.TypeLogical, doc)
if err != nil {
t.Fatal(err)
}
err = documentPath(path2, nil, "kv", false, logical.TypeLogical, doc)
err = documentPath(path2, nil, "kv", logical.TypeLogical, doc)
if err != nil {
t.Fatal(err)
}
@ -592,7 +592,7 @@ func TestOpenAPI_CustomDecoder(t *testing.T) {
}
docOrig := NewOASDocument("version")
err := documentPath(p, nil, "kv", false, logical.TypeLogical, docOrig)
err := documentPath(p, nil, "kv", logical.TypeLogical, docOrig)
if err != nil {
t.Fatal(err)
}
@ -655,7 +655,7 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string)
t.Helper()
doc := NewOASDocument("dummyversion")
if err := documentPath(path, sp, "kv", false, logical.TypeLogical, doc); err != nil {
if err := documentPath(path, sp, "kv", logical.TypeLogical, doc); err != nil {
t.Fatal(err)
}
doc.CreateOperationIDs("")

View File

@ -342,7 +342,7 @@ func (p *Path) helpCallback(b *Backend) OperationFunc {
}
}
doc := NewOASDocument(vaultVersion)
if err := documentPath(p, b.SpecialPaths(), requestResponsePrefix, false, b.BackendType, doc); err != nil {
if err := documentPath(p, b.SpecialPaths(), requestResponsePrefix, b.BackendType, doc); err != nil {
b.Logger().Warn("error generating OpenAPI", "error", err)
}

View File

@ -4496,10 +4496,20 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
context := d.Get("context").(string)
// Set up target document and convert to map[string]interface{} which is what will
// be received from plugin backends.
// Set up target document
doc := framework.NewOASDocument(version.Version)
// Generic mount paths will primarily be used for code generation purposes.
// This will result in parameterized mount paths being returned instead of
// hardcoded actual paths. For example /auth/my-auth-method/login would be
// replaced with /auth/{my-auth-method_mount_path}/login.
//
// Note that for this to actually be useful, you have to be using it with
// a Vault instance in which you have mounted one of each secrets engine
// and auth method of types you are interested in, at paths which identify
// their type, and for the KV secrets engine you will probably want to
// mount separate kv-v1 and kv-v2 mounts to include the documentation for
// each of those APIs.
genericMountPaths, _ := d.Get("generic_mount_paths").(bool)
procMountGroup := func(group, mountPrefix string) error {
@ -4519,7 +4529,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, "genericMountPaths": genericMountPaths},
Data: map[string]interface{}{"requestResponsePrefix": pluginType},
}
resp, err := backend.HandleRequest(ctx, req)
@ -4557,6 +4567,19 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
tag = "identity"
}
// When set to the empty string, mountPathParameterName means not to use a parameter at all;
// the one variable combines both boolean, and value-to-use-if-true semantics.
mountPathParameterName := ""
if genericMountPaths {
isSingletonMount := (group == "auth" && pluginType == "token") ||
(group == "secret" &&
(pluginType == "system" || pluginType == "identity" || pluginType == "cubbyhole"))
if !isSingletonMount {
mountPathParameterName = strings.TrimRight(mount, "/") + "_mount_path"
}
}
// Merge backend paths with existing document
for path, obj := range backendDoc.Paths {
path := strings.TrimPrefix(path, "/")
@ -4573,12 +4596,23 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
}
}
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
mountForOpenAPI := mount
if mountPathParameterName != "" {
mountForOpenAPI = "{" + mountPathParameterName + "}/"
obj.Parameters = append(obj.Parameters, framework.OASParameter{
Name: mountPathParameterName,
Description: "Path that the backend was mounted at",
In: "path",
Schema: &framework.OASSchema{
Type: "string",
},
Required: true,
})
}
doc.Paths["/"+mountPrefix+mountForOpenAPI+path] = obj
}
// Merge backend schema components

View File

@ -3631,10 +3631,10 @@ func TestSystemBackend_OASGenericMount(t *testing.T) {
path string
tag string
}{
{"/auth/{mountPath}/lookup", "auth"},
{"/{mountPath}/{path}", "secrets"},
{"/auth/token/lookup", "auth"},
{"/cubbyhole/{path}", "secrets"},
{"/identity/group/id", "identity"},
{"/{mountPath}/.*", "secrets"},
{"/{secret_mount_path}/.*", "secrets"},
{"/sys/policy", "system"},
}
@ -3719,7 +3719,7 @@ func TestSystemBackend_OpenAPI(t *testing.T) {
{"/auth/token/lookup", "auth"},
{"/cubbyhole/{path}", "secrets"},
{"/identity/group/id", "identity"},
{"/secret/.*", "secrets"}, // TODO update after kv repo update
{"/secret/.*", "secrets"},
{"/sys/policy", "system"},
}