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

Role permissions checks #170

Merged

Conversation

jnschaeffer
Copy link
Contributor

This PR adds permissions checks for role management CRUD operations in the permissions-api API.

Because permissions checks are baked into the API (i.e., they cannot and should not be disabled), this PR also adds a command, create-role, which can be used to bootstrap the creation of roles that can allow a subject to create other roles. The conditions for actions like role_create are deliberately left out of permissions-api to allow operators to define their own policies.

Some functions, like currentSubject and checkAction, have logic that
is either duplicated or could be reused elsewhere for things like
checking access to actions in permissions-api itself. This commit
moves some of that logic around so it is easier for other
handlers (such as the handlers for roles) to use.

Signed-off-by: John Schaeffer <jschaeffer@equinix.com>
This commit adds permissions checks for role creation, as well as the
action and bindings to the example policy.

Signed-off-by: John Schaeffer <jschaeffer@equinix.com>
This commit adds a command to create roles directly in SpiceDB,
bypassing permissions checks. The intent of this command is to
bootstrap a new permissions-api deployment with enough access to start
provisioning roles using some subject.

Signed-off-by: John Schaeffer <jschaeffer@equinix.com>
This commit adds permissions checks for getting, listing, updating,
and deleting roles.

Signed-off-by: John Schaeffer <jschaeffer@equinix.com>
Signed-off-by: John Schaeffer <jschaeffer@equinix.com>
@jnschaeffer jnschaeffer requested review from a team as code owners September 6, 2023 19:09
mikemrm
mikemrm previously approved these changes Sep 6, 2023
@jnschaeffer jnschaeffer added the question Further information is requested label Sep 6, 2023
@jnschaeffer
Copy link
Contributor Author

One issue with this as written right now is that with the example policy, no relationships exist that convey that a role belongs to a resource. That actually seems necessary here, looking closer at the logic.

One of the quirks of our current role model is that roles don't belong
to a resource outright - instead, their binding to a resource is
inferred by the actions that can be performed. This means that we
can't use the role itself to make authorization decisions. This commit
updates permissions checks for roles to use the role's resource rather
than the role itself for checking permissions.

Signed-off-by: John Schaeffer <jschaeffer@equinix.com>
@jnschaeffer
Copy link
Contributor Author

Updated with a change that doesn't break everything.

@jnschaeffer jnschaeffer merged commit 5a66391 into infratographer:main Sep 6, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants