hashicorp / vault-client-go

HashiCorp Vault Go Client Library generated from OpenAPI spec.
Mozilla Public License 2.0
77 stars 17 forks source link

Unable to list secrets at the root of a KV2 secrets engine #255

Open RyanW8 opened 7 months ago

RyanW8 commented 7 months ago

Expected Behavior

The KvV2List function should return secrets & folders at the root of the secret engine when path is empty.

Current Behavior

Throws a 404 error.

Failure Information

We're running Vault enterprises and are utilizing enterprise namespaces.

vault-client-go: v1.19.0 vault: v1.15.4 Vault Enterprises with namespaces enabled/being used

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. Setup Vault client & try and list secrets at the root
    
    client, err := vault.New(
        vault.WithAddress(config.VAULT_ADDR),
        vault.WithRequestTimeout(30*time.Second))
    if err != nil {
        return nil, err
    }
    err = client.SetNamespace("<your_namespace>")
    if err != nil {
        return nil, err
    }
    resp, err := client.Auth.JwtLogin(
        context.TODO(), schema.JwtLoginRequest{
                        Jwt: "<your_jwt_here>",
            Role: "<your_role>",
        }, vault.WithMountPath("<mount_path>"))
    if err := client.SetToken(resp.Auth.ClientToken); err != nil {
        return nil, err
    }
       resp, err := client.Secrets.KvV2List(ctx, "", vault.WithMountPath("<your_mount_path>")) // This throws an error
...

# Additional Information

Error thrown: 

{"level":"fatal","ts":1710409697.041736,"caller":"vault-secret-expiry-notifier/main.go:36","msg":"404 Not Found: no handler for route \"//metadata/\". route entry not found.","stacktrace":"main.main\n\t/Users/R713887/Documents/vault-secret-expiry-notifier/main.go:36\nruntime.main\n\t/usr/local/jpmc/go-1.21.5/src/runtime/proc.go:267"}

averche commented 7 months ago

Thanks for reporting this issue, @RyanW8!

I was able to reproduce it locally printing out the request using:

    client.SetRequestCallbacks(func(req *http.Request) {
        log.Println("REQUEST:", *req)
    })
2024/03/17 20:46:05 REQUEST: {GET http://127.0.0.1:8200/v1/my-mount-path/metadata//?list=true HTTP/1.1 1 1 map[User-Agent:[vault-client-go/0.4.3 (Darwin arm64; Go go1.22.0)] X-Vault-Namespace:[my-ns/] X-Vault-Token:[my-token]] <nil> <nil> 0 [] false 127.0.0.1:8200 map[] map[] <nil> map[]   <nil> <nil> <nil> 0x14000142a10 <nil> [] map[]}
2024/03/17 20:46:05 404 Not Found: no handler for route "my-ns/my-mount-path/metadata/". route entry not found.

Interestingly, if I don't set the namespace, it works correctly:

2024/03/17 20:48:29 REQUEST: {GET http://127.0.0.1:8200/v1/my-mount-path/metadata//?list=true HTTP/1.1 1 1 map[User-Agent:[vault-client-go/0.4.3 (Darwin arm64; Go go1.22.0)] X-Vault-Token:[my-token]] <nil> <nil> 0 [] false 127.0.0.1:8200 map[] map[] <nil> map[]   <nil> <nil> <nil> 0x14000220070 <nil> [] map[]}
2024/03/17 20:48:29 list: {[my-secret]}

The issue seems to be related to the closing slash (/) at the end of the path here:

https://github.com/hashicorp/vault-client-go/blob/cfdcbeda2e34770a045cc9f71ad25db195e97181/api_secrets.go#L3029

I believe this pattern was introduced in https://github.com/hashicorp/vault-client-go/pull/197 and, if I recall correctly, there was a special handling for the root path. @maxb, do you remember what it could have been?

maxb commented 7 months ago

Well, if it's working in the root namespace, but not in child namespaces, that's pretty clear that some Enterprise-only code is doing something wrong, or at least implementing some unspecified behaviour differently to source-available Vault.

The validity, or not, of a doubled slash character at the end of the URL path is something one could debate. No documentation that I am aware of promises that it will be accepted - but a great deal of common practice surrounding URLs suggests it probably will, and Vault's behaviour in most cases backs this up.

The reason the doubled slash is present in the API call generated by the library is as a result of awkward compromises mapping the Vault API to an OpenAPI spec, and then trying to generate a client library from that in an automated fashion.

Fundamentally, OpenAPI is opinionated about how APIs should work, and does not give you a toolkit to retroactively specify the behaviour of any pre-existing API - and the Vault API is not fully in accordance with OpenAPI's beliefs. (Although there are preliminary indications that a future major version of OpenAPI might address this.)

Regardless, with OpenAPI v3, we're stuck with producing something that is partially correct, and hoping that we can skirt around the differences between OpenAPI and Vault. There are two key issues:

1) ListOperation vs. GetOperation

Vault has two entirely different APIs (list/get) which can be invoked on the same URL-path with the same HTTP method (GET). As far as Vault is concerned, they're technically distinguished by the presence/absence of a list=true query parameter, but that doesn't fit into the OpenAPI world.

Instead, we've been able to work around this by generating URL-paths in the OpenAPI spec that are either with or without a trailing slash. That's quite a good logical fit, as Vault canonicalizes the presence/absence of the trailing slash in this way depending on the operation type, regardless of what was actually in the request.

2) APIs with a path parameter that represents part of the Vault path hierarchy

Vault has lots of APIs where part of the URL (usually but not exclusively the end) is a variable number of path segments from the Vault path hierarchy. OpenAPI really doesn't support this at all. In the OpenAPI world, path parameters do not contain unescaped slashes, ever.

Since the spec doesn't define such a thing, it's not surprising that code-gen tools for OpenAPI also don't implement it. The current generated client library actually encodes slashes as %2F in places where no human writing a client library would, for example. I was actually a bit surprised Vault is willing to accept that.

A possible fix client side

Since the Vault API doesn't actually require the trailing slash anyway, and the only reason we have both:

is to differentiate get vs. list operations in the OpenAPI model, it ought to work to remove the trailing slash from every operation path in the code generation template.

An aside

if I recall correctly, there was a special handling for the root path.

What I think you're thinking of there, is the truly bizarre generated API the library had for a while, where there was an entirely separate client library method for listing a root path vs a non-root path. We never had that for listing KVs, because it depended on exactly how the code author had written the path regexes in the Vault codebase, but an example of this may be seen in the two methods:

- (*System).RawList: added
- (*System).RawListPath: added

in the PR #197 that you mentioned.

I subsequently fixed that madness in #203