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

Enhancement: Granular client and project permissions #480

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

smcgu
Copy link
Contributor

@smcgu smcgu commented Jul 15, 2024

Issue

Non-privileged accounts are unable to create clients and projects. In some organizations, there are no (project) managers that set up clients and assign projects; non-privileged users are expected to create or re-use existing clients for new projects.

Description of the Change

  • Added a "View All Clients" permission. This allows non-privileged users to view all clients in the rolodex. This does not provide the ability to add, edit, or delete clients.
  • Added create, edit, and delete permissions for clients and projects. These permissions are (currently) linked (e.g, providing the client creation privilege means that a non-privileged user has the ability to create projects on any client). These changes follow those made previously for observations and findings.
  • Modified the project list function to return only assigned project for non-privileged users, instead of all projects associated with related clients.

Alternate Designs

Separating client and project permissions would likely require implementing some form of object ownership tracking. Creating project add/edit/delete permissions could lead to unintended consequences where an admin/manager could expect that the non-privileged user would only touch assigned clients, but the API would actually allow modifications to any clients. Setting up an ownership model where a non-privileged could create a client or project, then only modify that client or project would likely necessitate tracking ownership (and allowing changing of ownership).

So, linking the two for now seemed the safest from an expectation standpoint.

Possible Drawbacks

These permissions are still somewhat broad but they are an improvement from the existing alternative of non-privileged vs privileged/admin.

Verification Process

This setup has been merged and operating on a v4.2.1 Ghostwriter install. It has not been tested against 4.2.2.

Release Notes

  • Added a "View All Clients" permission.
  • Added create, edit, and delete permissions for clients and projects.
  • Restricted the project list functionality.

@smcgu smcgu force-pushed the granular-client-permissions branch from a07097f to 7456837 Compare August 30, 2024 04:13
@chrismaddalena
Copy link
Collaborator

This PR looks neglected since conversations have happened in Slack, so I'm dropping a note here that we have changes to tighten up the RBAC in #500. I'll also review the changes in this PR, but based on our Slack conversations, it doesn't cover everything (e.g., project and client invites and a front-end for them).

PR #500 should be good but needs testing and review. It's been on the back burner for a minute because the RBAC changes are significant and will require a major release (e.g., v4.4) and an advanced warning so people can prepare. Nothing breaks, but it's likely to confuse people, so we will release it as something like v4.4-rc1. I didn't want to add the RBAC changes on top of the other significant changes in v4.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants