-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Conversation
@@ -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) { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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
Build Results: |
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ty for the comment!
There was a problem hiding this comment.
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 😅
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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/' |
There was a problem hiding this comment.
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 /
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly clarifying questions! 🚢
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 theresultant-acl
endpoint.