jfrog / vault-plugin-secrets-artifactory

HashiCorp Vault Secrets Plugin for Artifactory
https://jfrog.com
Apache License 2.0
42 stars 21 forks source link

DELETE artifactory/config/admin should revoke its own access token #86

Closed TJM closed 8 months ago

TJM commented 1 year ago

While looking at the tests (specifically the cleanup), and even our own terraform code, it occurs to me that the DELETE request to /config/admin should revoke its own access token before deleting, especially if it has been rotated.

On a positive note, revoking an already revoked token will return 204, so even if someone (like us) has cleanups written into the terraform code, they will not start failing.

alexhung commented 1 year ago

Agree, I think the delete request should attempt to revoke the token. And move on regardless of the outcome. It should log anything that isn't success as warning.

TJM commented 1 year ago

Maybe we only cleanup if the token has been rotated? Once it has been rotated, it is under vault's control, so it should be revoked on deletion. If the token has not been rotated, there is a small possibility, that they got a single admin token and used it for more than one thing (bad practice), or they could be planning to maintain the admin token using another system (KVv2, or cloud secret management).

TJM commented 8 months ago

This one might have just bubbled up enough to get my attention... we have thousands of vault-admin tokens...

alexhung commented 8 months ago

@TJM Man, has it been nearly an year since this was original created? 🤯 Let me find the JIRA ticket and bump this up the priority!

alexhung commented 8 months ago

Maybe we only cleanup if the token has been rotated?

I don't think there's an out-of-the-box Vault meta data to keep track of this. I may have to store it in private meta data.

alexhung commented 8 months ago

What about adding a new field revoke_on_delete in the config/admin path? Then plugin will attempt to revoke the token if that field is set to true, regardless of it's been rotated or not?

TJM commented 8 months ago

I suppose we could look and see what other secret engines do, gcp maybe, but I like the idea of a parameter, and maybe default to true on rotate? 🤷