scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-10-31 02:28:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

OpenAPI: Separate ListOperation from ReadOperation (#21723)

Browse Source 
* OpenAPI: Separate ListOperation from ReadOperation

Historically, since Vault's ReadOperation and ListOperation both map to
the HTTP GET method, their representation in the generated OpenAPI has
been a bit confusing.

This was partially mitigated some time ago, by making the `list` query
parameter express whether it was required or optional - but only in
a way useful to human readers - the human had to know, for example, that
the schema of the response body would change depending on whether `list`
was selected.

Now that there is an effort underway to automatically generate API
clients from the OpenAPI spec, we have a need to fix this more
comprehensively. Fortunately, we do have a means to do so - since Vault
has opinionated treatment of trailing slashes, linked to operations
being list or not, we can use an added trailing slash on the URL path to
separate list operations in the OpenAPI spec.

This PR implements that, and then fixes an operation ID which becomes
duplicated, with this change applied.

See also hashicorp/vault-client-go#174, a bug which will be fixed by
this work.

* Set further DisplayAttrs in auth/github plugin

To mask out more duplicate read/list functionality, now being separately
generated to OpenAPI client libraries as a result of this change.

* Apply requested changes to operation IDs

I'm not totally convinced its worth the extra lines of code, but
equally, I don't have strong feelings about it, so I'll just make the
change.

* Adjust logic to prevent any possibility of generating OpenAPI paths with doubled final slashes

Even in the edge case of improper use of regex patterns and operations.

* changelog

* Fix TestSudoPaths to pass again... which snowballed a bit...

Once I looked hard at it, I found it was missing several sudo paths,
which led to additional bug fixing elsewhere.

I might need to pull some parts of this change out into a separate PR
for ease of review...

* Fix other tests

* More test fixing

* Undo scope creep - back away from fixing sudo paths not shown as such in OpenAPI, at least within this PR

Just add TODO comments for now.
This commit is contained in:
Max Bowsher
2023-07-13 18:36:52 +01:00
committed by GitHub
parent c2cbd5a578
commit e47dd9df53
11 changed files with 228 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
api/plugin_helpers.go
View File

@@ -40,7 +40,8 @@ const (
// path matches that path or not (useful specifically for the paths that
// contain templated fields.)
var sudoPaths = map[string]*regexp.Regexp{
"/auth/token/accessors": regexp.MustCompile(`^/auth/token/accessors/?$`),
"/auth/token/accessors": regexp.MustCompile(`^/auth/token/accessors/?$`),
// TODO /auth/token/revoke-orphan requires sudo but isn't represented as such in the OpenAPI spec
"/pki/root": regexp.MustCompile(`^/pki/root$`),
"/pki/root/sign-self-issued": regexp.MustCompile(`^/pki/root/sign-self-issued$`),
"/sys/audit": regexp.MustCompile(`^/sys/audit$`),
@@ -52,28 +53,33 @@ var sudoPaths = map[string]*regexp.Regexp{
"/sys/config/cors": regexp.MustCompile(`^/sys/config/cors$`),
"/sys/config/ui/headers": regexp.MustCompile(`^/sys/config/ui/headers/?$`),
"/sys/config/ui/headers/{header}": regexp.MustCompile(`^/sys/config/ui/headers/.+$`),
"/sys/leases": regexp.MustCompile(`^/sys/leases$`),
"/sys/leases/lookup/": regexp.MustCompile(`^/sys/leases/lookup/?$`),
"/sys/leases/lookup/{prefix}": regexp.MustCompile(`^/sys/leases/lookup/.+$`),
"/sys/leases/revoke-force/{prefix}": regexp.MustCompile(`^/sys/leases/revoke-force/.+$`),
"/sys/leases/revoke-prefix/{prefix}": regexp.MustCompile(`^/sys/leases/revoke-prefix/.+$`),
"/sys/plugins/catalog/{name}": regexp.MustCompile(`^/sys/plugins/catalog/[^/]+$`),
"/sys/plugins/catalog/{type}": regexp.MustCompile(`^/sys/plugins/catalog/[\w-]+$`),
"/sys/plugins/catalog/{type}/{name}": regexp.MustCompile(`^/sys/plugins/catalog/[\w-]+/[^/]+$`),
"/sys/raw": regexp.MustCompile(`^/sys/raw$`),
"/sys/raw/{path}": regexp.MustCompile(`^/sys/raw/.+$`),
"/sys/remount": regexp.MustCompile(`^/sys/remount$`),
"/sys/revoke-force/{prefix}": regexp.MustCompile(`^/sys/revoke-force/.+$`),
"/sys/revoke-prefix/{prefix}": regexp.MustCompile(`^/sys/revoke-prefix/.+$`),
"/sys/rotate": regexp.MustCompile(`^/sys/rotate$`),
"/sys/internal/inspect/router/{tag}": regexp.MustCompile(`^/sys/internal/inspect/router/.+$`),
"/sys/leases": regexp.MustCompile(`^/sys/leases$`),
// This entry is a bit wrong... sys/leases/lookup does NOT require sudo. But sys/leases/lookup/ with a trailing
// slash DOES require sudo. But the part of the Vault CLI that uses this logic doesn't pass operation-appropriate
// trailing slashes, it always strips them off, so we end up giving the wrong answer for one of these.
"/sys/leases/lookup": regexp.MustCompile(`^/sys/leases/lookup/?$`),
"/sys/leases/lookup/{prefix}": regexp.MustCompile(`^/sys/leases/lookup/.+$`),
"/sys/leases/revoke-force/{prefix}": regexp.MustCompile(`^/sys/leases/revoke-force/.+$`),
"/sys/leases/revoke-prefix/{prefix}": regexp.MustCompile(`^/sys/leases/revoke-prefix/.+$`),
"/sys/plugins/catalog/{name}": regexp.MustCompile(`^/sys/plugins/catalog/[^/]+$`),
"/sys/plugins/catalog/{type}": regexp.MustCompile(`^/sys/plugins/catalog/[\w-]+$`),
"/sys/plugins/catalog/{type}/{name}": regexp.MustCompile(`^/sys/plugins/catalog/[\w-]+/[^/]+$`),
"/sys/raw": regexp.MustCompile(`^/sys/raw$`),
"/sys/raw/{path}": regexp.MustCompile(`^/sys/raw/.+$`),
"/sys/remount": regexp.MustCompile(`^/sys/remount$`),
"/sys/revoke-force/{prefix}": regexp.MustCompile(`^/sys/revoke-force/.+$`),
"/sys/revoke-prefix/{prefix}": regexp.MustCompile(`^/sys/revoke-prefix/.+$`),
"/sys/rotate": regexp.MustCompile(`^/sys/rotate$`),
// TODO /sys/seal requires sudo but isn't represented as such in the OpenAPI spec
// TODO /sys/step-down requires sudo but isn't represented as such in the OpenAPI spec
// enterprise-only paths
"/sys/replication/dr/primary/secondary-token": regexp.MustCompile(`^/sys/replication/dr/primary/secondary-token$`),
"/sys/replication/performance/primary/secondary-token": regexp.MustCompile(`^/sys/replication/performance/primary/secondary-token$`),
"/sys/replication/primary/secondary-token": regexp.MustCompile(`^/sys/replication/primary/secondary-token$`),
"/sys/replication/reindex": regexp.MustCompile(`^/sys/replication/reindex$`),
"/sys/storage/raft/snapshot-auto/config/": regexp.MustCompile(`^/sys/storage/raft/snapshot-auto/config/?$`),
"/sys/storage/raft/snapshot-auto/config": regexp.MustCompile(`^/sys/storage/raft/snapshot-auto/config/?$`),
"/sys/storage/raft/snapshot-auto/config/{name}": regexp.MustCompile(`^/sys/storage/raft/snapshot-auto/config/[^/]+$`),
}
@@ -252,6 +258,8 @@ func SudoPaths() map[string]*regexp.Regexp {
// Determine whether the given path requires the sudo capability.
// Note that this uses hardcoded static path information, so will return incorrect results for paths in namespaces,
// or for secret engines mounted at non-default paths.
// Expects to receive a path with an initial slash, but no trailing slashes, as the Vault CLI (the only known and
// expected user of this function) sanitizes its paths that way.
func IsSudoPath(path string) bool {
// Return early if the path is any of the non-templated sudo paths.
if _, ok := sudoPaths[path]; ok {