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

UI: Show correct nav items when in chroot namespace #24492

Merged
merged 9 commits into from
Dec 13, 2023

Conversation

hashishaw
Copy link
Collaborator

@hashishaw hashishaw commented Dec 12, 2023

This PR handles the necessary updates to enable a user logged in on a chrooted namespace listener to see their correct nav items. This depends on backend work in 1.16 (backported to 1.15.5) which adds chroot_namespace key to the resultant-acl endpoint.

@hashishaw hashishaw added the ui label Dec 12, 2023
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Dec 12, 2023
@@ -53,66 +53,11 @@ module('Unit | Service | permissions', function (hooks) {
assert.deepEqual(this.service.get('globPaths'), PERMISSIONS_RESPONSE.data.glob_paths);
});

test('returns true if a policy includes access to an exact path', function (assert) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reorganized these tests, so these are just moved to a module now

assert.strictEqual(this.service.pathNameWithNamespace('sys/auth'), 'marketing/sys/auth');
});

test('appends the chroot and namespace when both present', function (assert) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test and the following ones are new

if (this.canViewAll) {
return true;
}
const path = this.pathNameWithNamespace(pathName);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved this so we get a little better efficiency, skipping the method if canViewAll

@hashishaw hashishaw added this to the 1.15.5 milestone Dec 12, 2023
@hashishaw hashishaw marked this pull request as ready for review December 12, 2023 18:44
Copy link

Build Results:
All builds succeeded! ✅

@@ -15,4 +15,9 @@ export default class SidebarNavClusterComponent extends Component {
get cluster() {
return this.currentCluster.cluster;
}

get isRootNamespace() {
// should only return true if we're in the true root namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

ty for the comment!

Copy link
Contributor

@hellobontempo hellobontempo Dec 13, 2023

Choose a reason for hiding this comment

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

I'm still getting up to speed with the chroot situation - does true "root" mean root root?

Does this.currentCluster.hasChrootNamespace mean the user has a configured root namespace and so they will not have root as the base/parent namespace? So therefore configuring chroot and having root as a namespace are mutually exclusive?

Just looking for clarification so I understand 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hasChrootNamespace means the Vault operator has set chroot_namespace to some value on the config of the given listener, and so the user does not have access to "true root" (which yes, is root namespace). So, even if the UI is in its top-most namespace, if chroot_namespace is configured it is not the true root.

Does that answer your question?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes! I think so - I wanted to clarify that setting the chroot_namespace means that you are configuring the top namespace and so therefore root will NOT be a possible/accessible.

Thank you!

assert.strictEqual(this.service.pathNameWithNamespace('/sys/auth'), 'admin/marketing/sys/auth');
assert.strictEqual(
this.service.pathNameWithNamespace('/sys/policies/'),
'admin/marketing/sys/policies/'
Copy link
Contributor

Choose a reason for hiding this comment

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

is this expected to have the path end in a /?

Copy link
Collaborator Author

@hashishaw hashishaw Dec 13, 2023

Choose a reason for hiding this comment

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

In practice I haven't seen it, but I wanted to make sure it was preserved in case it's important in some undocumented case

Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

Mostly clarifying questions! 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

3 participants