vault icon indicating copy to clipboard operation
vault copied to clipboard

Several Vault endpoints spuriously implement CreateOperation & advertise x-vault-createSupported in OpenAPI

Open maxb opened this issue 3 years ago • 5 comments

Several Vault endpoints spuriously implement CreateOperation and so advertise x-vault-createSupported in OpenAPI, when it is meaningless to do so.

This may go unnoticed by the vast majority of users, but if you are writing checks which consult valid operations in OpenAPI, to vet whether your users are making mistakes in writing their Vault policies, it is important.

It's a coding error in a Vault framework.Path object, to implement logical.CreateOperation in your Operations or Callbacks, if you do not also implement ExistenceCheck - yet there are a number of instances of doing so in Vault code.

Here are the ones I've found so far:

  • ad/rotate-role/{name}
  • ad/rotate-root
  • kv-v2/config
  • kv-v2/delete/{path}
  • kv-v2/destroy/{path}
  • kv-v2/undelete/{path}
  • transit/cache-config
  • auth/kubernetes/config

In each case, the logical.CreateOperation entry should be removed from the Operations or Callbacks map.

Ideally, Vault would log a warning on startup if such a coding error was present, when initialising plugins, to guide future codes to use the API correctly.

maxb avatar Aug 15 '21 08:08 maxb

Hi @maxb! I spoke with our engineers, and they agree your proposed solution is the one they would also choose. Please feel free to tackle this, and submit a PR and link it to this issue. (Anything with a label of good-first-issue is one where we welcome community contributions!) I apologize about the delay in getting back to you, and if you have any questions, please feel free to let me know. :)

heatherezell avatar Sep 10 '21 22:09 heatherezell

Thanks, I will raise with my employer whether I can sign the CLA as an individual contributor, or the company needs to sign it. This might take a little while - if anyone else wants to take the idea and run with it in the meantime, that's fine too!

maxb avatar Sep 11 '21 04:09 maxb

Hey @hsimon-hashicorp, I would like to pick this issue

mallikarjun-b-r avatar Oct 10 '21 21:10 mallikarjun-b-r

It has been a long while since I opened this issue, but since my CLA is now sorted and I have some time over Christmas, I'm now in a position to start making some PRs for it.

maxb avatar Dec 20 '22 07:12 maxb

I have created the 10 PRs to implement this change.

maxb avatar Dec 20 '22 14:12 maxb