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: Update resultant-acl banner #25256

Merged
merged 10 commits into from
Feb 7, 2024
Merged

Conversation

hashishaw
Copy link
Collaborator

This PR updates when the resultant-acl banner is shown to a user.

Before, we were always calling resultant-acl at the current namespace. This typically would fail when the user is accessing a child namespace that they have access to, unless the administrator specifically gave them read access to the resultant-acl path within that namespace. This is a subpar experience for both users and administrators.

With this change:

  • resultant-acl will always be called at the user's root namespace instead of the current namespace
    • eg. if I'm user bob enabled at an auth mount in ns1, my resultant-acl call will always be made to namespace ns1 regardless of the namespace I'm currently accessing
  • As a result of the above, the call should never fail unless the user does not have the default policy attached. This means we can differentiate between no namespace access and a failed call.
    • If the call does fail, a banner with "Resultant ACL check failed" will show
  • Once the call succeeds, we then check if the current namespace the user is accessing is present in any of the paths returned from the call. This allows the behavior to work consistently regardless of the group_policy_application_mode set.
    • When the call succeeds but the namespace is not present in the paths, a banner with "
  • For CE, the namespace link will not show and it will in practice only show the "Resultant ACL check failed" message

When accessing a namespace that is not in your policy:
no namespace access example

When logged in with a user that does not have the default policy attached
no default policy example

@hashishaw hashishaw added the ui label Feb 7, 2024
@hashishaw hashishaw added this to the 1.16.0-rc1 milestone Feb 7, 2024
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Feb 7, 2024
@hashishaw hashishaw marked this pull request as ready for review February 7, 2024 17:06
Copy link

github-actions bot commented Feb 7, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Feb 7, 2024

Build Results:
All builds succeeded! ✅

}
const namespace = this.baseNs;
const allowed =
Object.keys(this.globPaths).any((k) => k.startsWith(namespace)) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

For someone not super familiar with this response, could you maybe add a comment with an example of glob vs exact paths? Especially since the API doc are 🙃 in information for this endpoint. Found this stackoverflow with someones examples to help me visual it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the best example is in the test file

I'd love to make this service glimmer + typescript, but since this change will be backported that will have to be a follow on

Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

Nice work. And a really thoughtful approach to the issue. One request, but non-blocking.

@hashishaw hashishaw enabled auto-merge (squash) February 7, 2024 18:47
@hashishaw hashishaw merged commit 30aa1b4 into main Feb 7, 2024
26 checks passed
@hashishaw hashishaw deleted the ui/VAULT-23578/resultant-acl-fix branch February 7, 2024 18:57
Monkeychip pushed a commit that referenced this pull request Feb 12, 2024
* Request resultant-acl only from users root namespace

* Update permissions adapter to always call resultant-acl at users root, with test

* Update resultant-acl to accept failType

* Update permissions service to set permissionsBanner based on resultant-acl contents

* wire it up

* add changelog

* cleanup unused adapter changes

* use getter for shared namespace logic
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

2 participants