Skip to content

Commit

Permalink
Refactor identity/mfa/method/* endpoints to fix bad OpenAPI (#20879)
Browse files Browse the repository at this point in the history
* Refactor `identity/mfa/method/*` endpoints to fix bad OpenAPI

There is a problem with how the `identity/mfa/method/*` endpoints are
defined, resulting in incorrect OpenAPI generation.

I raised hashicorp/vault-client-go#180 to track a consequence, and
opened #20873 which explains the problem and adds a log message to
detect it.

This PR is now the fix.

It's actually quite an interesting problem, that has come about through
some particular implementation choices, in Vault's first/only case where
REST API objects are created by writing to the collection URL, and have
their ID allocated by the server, instead of the client.

The triggering cause of the malfunction was trying to have a single
framework.Path struct instance which optionally includes or excludes the
method_id path parameter, and also another framework.Path struct
instance handling list operations.

The fix is to simplify the path regexes, and have one framework.Path
which handles the method_id being present, and one that handles it being
absent.

The diff is somewhat large, because the affected code had been
copy/pasted four times (TOTP, Okta, Duo, PingID) - so I took the
opportunity to fix the duplication, creating appropriate helper methods
so that the quadruplicated code could be re-unified.

* Revise documentation

This update refactors how the documentation presents these endpoints to
users, both for clarity, and to align with the new structure of the
code.

From a user perspective, it clears up some unclear presentation of when
the `method_id` parameter should and should not be present, adds
a missing description of the response to create requests, and changes
the `method_id` parameter name to be used consistently (rather than `id`
in some cases, unlike the actual code/OpenAPI).

* Fix incorrect acronym (review fix)

* Accept suggestion of tweaked grammar in documentation

Co-authored-by: Anton Averchenkov <[email protected]>

* Add changelog

---------

Co-authored-by: Anton Averchenkov <[email protected]>
  • Loading branch information
maxb and averche authored Jun 23, 2023
1 parent a71cdb6 commit 43ae739
Show file tree
Hide file tree
Showing 7 changed files with 385 additions and 411 deletions.
3 changes: 3 additions & 0 deletions changelog/20879.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
identity/mfa: Fixes to OpenAPI representation and returned error codes for `identity/mfa/method/*` APIs
```
Loading

0 comments on commit 43ae739

Please sign in to comment.