UI/fix renew token button#31807
Conversation
|
@lklivingstone is attempting to deploy a commit to the HashiCorp Team on Vercel. A member of the Team first needs to authorize it. |
| } catch (e) { | ||
| this.set('canRenewSelf', false); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Thanks for opening this PR! We actually have a capabilities service to use for these sorts of requests, if we could use that instead. Here's an example. One reason is if the capabilities request fails we actually want to default to true rather than potentially gate an action a user can perform because of a failed capability check.
Can this request also be moved to the TokenExpireWarning component? If canRenewSelf is only set once upon login via this auth service, it will not get the updated value if policy changes are made upstream during this session. Requesting the data on demand ensures the latest state.
There was a problem hiding this comment.
Thanks for the suggestion! I’ve updated the implementation to use the capabilities service and moved the check into the TokenExpireWarning component. The capability is now fetched on initialization via fetchRenewCapability() so it’s evaluated on demand rather than being set at login. It also defaults to true on failure (fail-open) to avoid unintentionally gating the action.
Let me know if you’d prefer the capability check to be triggered differently.
| async fetchRenewCapability() { | ||
| try { | ||
| const { canUpdate } = await this.capabilities.fetchPathCapabilities('auth/token/renew-self'); | ||
| this.canRenewSelf = canUpdate; | ||
| } catch { | ||
| this.canRenewSelf = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
The service actually has a try...catch that handles this for you! So this can be simplified to
| async fetchRenewCapability() { | |
| try { | |
| const { canUpdate } = await this.capabilities.fetchPathCapabilities('auth/token/renew-self'); | |
| this.canRenewSelf = canUpdate; | |
| } catch { | |
| this.canRenewSelf = true; | |
| } | |
| } | |
| async fetchRenewCapability() { | |
| const { canUpdate } = await this.capabilities.fetchPathCapabilities('auth/token/renew-self'); | |
| this.canRenewSelf = canUpdate; | |
| } |
| assert.dom('[data-test-renew-token-button]').doesNotExist(); | ||
| }); | ||
|
|
||
| test('it shows Renew button if capability check fails (fail-open behavior)', async function (assert) { |
There was a problem hiding this comment.
We can actually remove this test case since fetchPathCapabilities() will never return an error.
There was a problem hiding this comment.
Great test coverage! Thank you for adding 🎉
| } | ||
|
|
||
| get canShowRenew() { | ||
| return this.auth?.authData?.renewable === true && this.canRenewSelf === true; |
There was a problem hiding this comment.
Since these are both boolean types this could be
| return this.auth?.authData?.renewable === true && this.canRenewSelf === true; | |
| return this.auth?.authData?.renewable && this.canRenewSelf |
hellobontempo
left a comment
There was a problem hiding this comment.
Excellent work! Thanks for quick turnaround addressing the refactor requests. Just one more small cleanup item to remove the try...catch in the fetchRenewCapability request and then this is good to go. In the mean time, I will hand this off to my teammate who is on the sustaining rotation to assist with getting this merged.
|
Thanks @hellobontempo |
There was a problem hiding this comment.
Looks great, thank you! This will have to be copied and merged into our private repo first and then ported over to the public Vault repo. The git history will be preserved and still credit you for this change 😄
Edit: I've just approved the workflows to run on this PR to confirm tests pass and that this doesn't introduce failures.
Copy workflow completed!
|
Copy pull request failed!
|
Description
What does this PR do?
Fixes: #31315
Vault UI currently renders the “Renew token” button based solely on the token’s renewable property. However, this does not account for policy-level restrictions on auth/token/renew-self.
If a policy explicitly denies renew-self, the button is still displayed.
Root Cause
The UI checked only the token’s renewable flag and did not verify whether the token had update capability on auth/token/renew-self.
Fix
The UI now:
Additionally included tests for them
Screenshot:

Do let me know if this is not the desired UI behaviour.
TODO only if you're a HashiCorp employee
backport/label that matches the desired release branch.PCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.