Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add directory paths to KV capabilities checks #24404

Merged
merged 8 commits into from
Dec 7, 2023

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented Dec 6, 2023

Fixes Issue #24130

Follow the instructions from the original issue to duplicate. The bug occurred because we were passing in path to the capabilities check but path does not include the full path name.

Ex: for secret beep/bop/boop
We would always pass in boop even if we were searching for the list return of beep/bop/
Now we pass in beep/ or beep/bop/ or beep/bop/bop depending on the path in the URL for the view.

Screenshots using the policy on the issue
Before:
image

After:
image

@Monkeychip Monkeychip added ui bug Used to indicate a potential bug labels Dec 6, 2023
@Monkeychip Monkeychip added this to the 1.15.5 milestone Dec 6, 2023
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Dec 6, 2023
@Monkeychip Monkeychip changed the title Add directory path names to KV capabilities checks Add directory paths to KV capabilities checks Dec 6, 2023
@Monkeychip Monkeychip marked this pull request as ready for review December 7, 2023 16:53
Copy link

github-actions bot commented Dec 7, 2023

Build Results:
All builds succeeded! ✅

Copy link
Contributor

@zofskeez zofskeez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of questions surrounding fullSecretPath.

ui/app/models/kv/data.js Outdated Show resolved Hide resolved
@@ -95,9 +95,14 @@ export default class KvSecretMetadataModel extends Model {
};
}

get permissionsPath() {
return this.fullSecretPath || this.path;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why fullSecretPath wouldn't always be the value to use if it represents the full path to the secret.

Copy link
Contributor Author

@Monkeychip Monkeychip Dec 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's generated by the serializer (not easy to see from the diff). And the payload does not always return data.keys. See here.

An example of when the serializer wouldn't return this.fullSecretPath is when we're looking at a secret's details.

@Monkeychip Monkeychip requested a review from zofskeez December 7, 2023 18:15
Copy link
Contributor

@zofskeez zofskeez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! 🎉

@Monkeychip Monkeychip merged commit 85acabb into main Dec 7, 2023
71 checks passed
@Monkeychip Monkeychip deleted the ui/VAULT-22496/capabilities-fix-kv branch December 7, 2023 19:48
Monkeychip added a commit that referenced this pull request Dec 7, 2023
* add getter to metadata model

* add changelog and data model fix

* add test coverage

* add nested create coverage

* Update 24404.txt

* remove from data model

* return to how it was
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants