-
Notifications
You must be signed in to change notification settings - Fork 124
Description
Would it be acceptable to make a change to allow read requests to pass parameters as query parameters?
In the Vault docs, it describes this as a valid way to query the API:
A few endpoints consume calls with GET query string parameters, but only if those parameters are not sensitive, especially since some load balancers will be able log these. Most endpoints that accept POST query string parameters expect those parameters in the request body.
You can list secrets as well. To do this, either issue a GET with the query string parameter list=true, or you use the LIST HTTP verb. For the kv secrets engine, listing is allowed on directories only, which returns the keys at the requested path:
...
The API documentation uses LIST as the HTTP verb, but you can still use GET with the ?list=true query string.
The use case here is that it is possible for a secret engines to get params from query params in GET requests, so making this change would allow dynamic secrets to query these endpoints. For example, the OIDC secret engine and OIDC auth engine.
The implication is that the code currently defaults all requests w params to PUT/POST, which would need to be changed:
vault-secrets-operator/controllers/vaultdynamicsecret_controller.go
Lines 360 to 367 in 8e13742
if params != nil { | |
if !(method == http.MethodPost || method == http.MethodPut) { | |
logger.V(consts.LogLevelWarning).Info( | |
"Params provided, ignoring specified method", | |
"requestHTTPMethod", o.Spec.RequestHTTPMethod) | |
} | |
method = http.MethodPut | |
} |
This impacts users who rely on this behavior. One way to mitigate this is to allow GET only if RequestHTTPMethod
is explicitly set to GET, instead of removing the default.
The code is mostly there already, just the parameters need to be passed:
vault-secrets-operator/vault/client.go
Line 704 in 8e13742
secret, err = c.client.Logical().ReadWithDataWithContext(ctx, path, request.Values()) resp, err = c.Read(ctx, vault.NewReadRequest(path, nil))
There is more context about query parameters in this issue: hashicorp/vault-client-go#155