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

Docs: Note permissions distinction between global policy automations and software install (#19551) and script execution (#17129) policy automations #23447

Merged
merged 5 commits into from
Nov 4, 2024

Conversation

iansltx
Copy link
Member

@iansltx iansltx commented Nov 1, 2024

No description provided.

…ftware install (#19551) and script execution (#17129) policy automations
@iansltx
Copy link
Member Author

iansltx commented Nov 1, 2024

Preview:
image

@noahtalerman
Copy link
Member

noahtalerman commented Nov 1, 2024

UPDATE: Learned from @iansltx, that calendar events are only available to admins and GitOps (global and team).


Hey @iansltx I made some tweaks to the docs given my latest understanding of permissions (see screenshots below):

Screenshot 2024-11-01 at 9 51 48 AM

Screenshot 2024-11-01 at 9 51 56 AM

Is my understanding accurate?

@iansltx
Copy link
Member Author

iansltx commented Nov 1, 2024

@noahtalerman ...sort of, but not quite, due to some API intricacies.

tl;dr: The current UI workflow (enable/disable + set URL + toggle policies) requires an admin user, though if we dropped its scope to just toggling on/off whether a policy has calendar events (so, just the checkboxes, not the webhook URL) that would be permitted for Maintainers. Toggling is available to Maintainers via the API because it's its own per-policy endpoint, so if we want the RBAC matrix to reflect what's available in the API then yes, Maintainers can turn on/off calendar events for a given policy failure.

Detailed explanation + potential UI fixes below:


Updating calendar event automations via the UI makes two API calls:

  1. Editing the team to enable/disable calendar integrations team-wide and specify the calendar events webhook - requires team.write permissions, which are Admin/GitOps-only
  2. Editing the individual policies to enable/disable the calendar automation on that policy - requires policy.write permissions, which are Maintainer-or-above

In addition to the above, setting up calendar events at all requires setting up the Google Calendar integration globally, and that's admin-only.

If we want to allow maintainers to toggle on/off calendar events for policies (which is all the API would let them do), we'd need to do one of the following in the UI (in addition to making the drop-down visible for Maintainers):

  1. Implement Move policy-specific automations into individual policy detail UI for read/write, policy list for reads #22511 (in which case the same permission filter for all other policy edits would apply to policy automations, including the calendar events toggle)
  2. Hide team-wide config for calendar events from the dialog when Maintainers see it
  3. Move calendar events enable/disable + webhook setting from the dialog to team settings, and link to the settings page in the dialog (potentially open in new tab) for Admins

On options 2 and 3, we likely want to disable the menu option for calendar events (with help text of why it's disabled) for Maintainers unless calendar events are enabled team-wide, as they won't be able to do anything if they click through.

Option 1 can (and probably should) be combined with Option 3, so it feels like Option 3 is the most maintainable/cleanest way to allow Maintainers to flip calendar automation toggles for policies while allowing admins to configure those policies. This also nudges us in the direction of putting system/team configuration in a centralized location, versus there being various different places in the UI to throw webhook URLs (I have the same opinion on the vulnerabilities webhook URL linked from the Software page).

All of the above are frontend-only fixes, just with varying levels of effort.

articles/role-based-access.md Show resolved Hide resolved
articles/role-based-access.md Show resolved Hide resolved
@noahtalerman
Copy link
Member

The current UI workflow (enable/disable + set URL + toggle policies) requires an admin user, though if we dropped its scope to just toggling on/off whether a policy has calendar events (so, just the checkboxes, not the webhook URL) that would be permitted for Maintainers

@iansltx thanks for the summary! My current understanding is that policies for calendar events can be edited (turned on/off) by maintainers, admins, and GitOps (global and team) via the API. Admins only via the UI.

Calendar events can be turned on/off and webhook URL can be set by admins and GitOps via the API. Admins only via the UI.

I updated the docs to reflect the scope of the API permissions. That way the docs reflect the widest scope.

If we want to allow maintainers to toggle on/off calendar events for policies

I think we can come back to updating the UI's permissions to reflect the API. Can you please track a feature request for that so we don't forget?

@iansltx
Copy link
Member Author

iansltx commented Nov 4, 2024

@iansltx thanks for the summary! My current understanding is that policies for calendar events can be edited (turned on/off) by maintainers, admins, and GitOps (global and team) via the API. Admins only via the UI.

Correct. To be clear, this is specific to the check box (or YAML equivalent) per-policy.

Calendar events can be turned on/off and webhook URL can be set by admins and GitOps via the API. Admins only via the UI.

Correct.

I think we can come back to updating the UI's permissions to reflect the API. Can you please track a feature request for that so we don't forget?

Done; #23483.

What else is needed prior to merging this change? Of note, this is all existing functionality, so main is the correct branch for this.

@iansltx iansltx merged commit 1d0ab56 into main Nov 4, 2024
5 checks passed
@iansltx iansltx deleted the 17129-rbac-docs branch November 4, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants