Fix consul token revocation with namespace specific policies #23010
Conversation
…n create tokens in another
github-actions
bot
added
the
hashicorp-contributed-pr
|
Build Results: |
|
CI Results: |
| if ok { | ||
| namespace := namespaceRaw.(string) | ||
| revokeWriteOptions = &api.WriteOptions{ | ||
| Namespace: namespace, | ||
| } |
There was a problem hiding this comment.
Hi David, thanks for catching this oversight!
Actually I believe this check is unnecessary. Unless I am mistaken, Consul never has an unnamed namespace to return because the default one is literally named "default". So this ok should always evaluate to true.
Also it turns out admin partitions have the exact same bug. I'm happy to make these changes myself if you don't mind me pushing to your branch?
There was a problem hiding this comment.
This check is to see if we had a consul namespace set in the secret itself in Vault. After some testing, it looks like in the case of not having a namespace set in the backend config, or the role config, namespace will return as an empty string so this can evaluate to false. I originally was just passing this WriteOptions struct along with the request even if there wasn't a namespace set as Consul seems to handle that case well, but I wanted to default to the original behavior here.
There was a problem hiding this comment.
@davidadeleon I'm not seeing this behavior, am I maybe testing this the wrong way? If I remember correctly, when a token is created the namespace and partition are returned from Consul and set on the secret here which should be the "default" value if never set in Vault. It seems to be behaving this way in my testing.
Can you add debug output to confirm the values of req.Data["consul_namespace"] and req.Data["partition"] are empty strings since I'm unable to recreate it?
There was a problem hiding this comment.
With Kyle's help I believe we discovered the discrepancy. When using Consul enterprise unset namespace and partition fields are indeed set to default, however free versions of Consul are set to empty strings.
In fact, due to this, the ok still always evaluates to true like I thought, but not exactly for the reason I stated. It is still "set" to empty string rather than being unset, and so the ok check passes. I've committed my changes for this reason.
robmonte
deleted the
davidadeleon/fix-consul-token-revoke-with-namespace
branch
This PR fixes a problem with the Consul Secrets engine where tokens generated in specific Consul namespaces were unable to be revoked if the token provided to Vault to be used for management had namespace specific policies.
Closes #22895