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

Management API #58

Merged
merged 20 commits into from
Apr 5, 2023
Merged

Conversation

jnschaeffer
Copy link
Contributor

@jnschaeffer jnschaeffer commented Mar 29, 2023

This PR introduces the start of a management API for permissions-api based on the following resource types:

  • Role: A collection of actions scoped to a resource
  • Relationship: A named link between two resources
  • Assignment: A link between a role and a subject

POST and GET are implemented for these resources. Future PRs will implement other methods (PATCH, DELETE) as needed.

@jnschaeffer jnschaeffer changed the title Add management API create endpoints [WIP] Management API Mar 29, 2023
@JAORMX
Copy link
Contributor

JAORMX commented Mar 30, 2023

@jnschaeffer in the PR defining the terminology we had agreed to use "Role assignments", what do you think about renaming .../memberships to .../assignments instead? That way we stay consistent in our terminology throughout the repository.

I'm also open to renaming the concept to be a membership, but then lets rename that throughout the repo and docs.

@JAORMX
Copy link
Contributor

JAORMX commented Mar 30, 2023

@jnschaeffer can you be a little more explicit about the user flows that this is covering? Mostly I'm struggling to understand how this would look for an admin in terms of effectively creating and assigning a role. Do they also have to explicitly create a relationship? Or is that process meant to be automated elsewhere?

@jnschaeffer
Copy link
Contributor Author

@jnschaeffer in the PR defining the terminology we had agreed to use "Role assignments", what do you think about renaming .../memberships to .../assignments instead? That way we stay consistent in our terminology throughout the repository.

I'm also open to renaming the concept to be a membership, but then lets rename that throughout the repo and docs.

We should stick with the terms we already defined; I'll update the PR accordingly.

@jnschaeffer can you be a little more explicit about the user flows that this is covering? Mostly I'm struggling to understand how this would look for an admin in terms of effectively creating and assigning a role. Do they also have to explicitly create a relationship? Or is that process meant to be automated elsewhere?

Sure thing. Would you prefer that in the README or some other documentation in the repo?

@jnschaeffer jnschaeffer force-pushed the management-api branch 2 times, most recently from fea3f64 to 7d687c8 Compare March 30, 2023 19:47
README.md Outdated Show resolved Hide resolved
This commit introduces the start of a management API for
permissions-api based on the following resource types:

* Role: A collection of actions scoped to a resource
* Relationship: A named link between two resources
* Membership: A link between a role and a subject

Currently only POST is implemented for these resources. Future commits
will implement other methods (GET, PATCH, DELETE) as needed.

Signed-off-by: John Schaeffer <[email protected]>
SpiceDB's (and Zanzibar's) notion of resource and subject are the
inverse of what one would typically think of, especially when looking
at schema definitions. This commit updates the relationships API to
allow for the definition of relationships for multiple subjects on a
single resource, which matches schemas and resources as defined more
closely.

Signed-off-by: John Schaeffer <[email protected]>
Assignments is the term we use elsewhere when describing the
relationship between a subject and a role. This commit replaces usage
of the word "membership" with "assignment" accordingly.

Signed-off-by: John Schaeffer <[email protected]>
Signed-off-by: John Schaeffer <[email protected]>
Signed-off-by: John Schaeffer <[email protected]>
Signed-off-by: John Schaeffer <[email protected]>
@JAORMX
Copy link
Contributor

JAORMX commented Apr 4, 2023

@jnschaeffer thanks for addressing the comments, let me know when it's ready for review

For testing purposes, it's easier if schema updates don't delete
everything in SpiceDB. This commit sets the flag necessary to do
append-only schema updates in SpiceDB.

Signed-off-by: John Schaeffer <[email protected]>
@jnschaeffer jnschaeffer changed the title [WIP] Management API Management API Apr 5, 2023
@jnschaeffer jnschaeffer marked this pull request as ready for review April 5, 2023 13:18
Copy link
Contributor

@fishnix fishnix left a comment

Choose a reason for hiding this comment

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

just a couple minor questions - otherwise lgtm 👍


```
$ curl --oauth2-bearer "$AUTH_TOKEN" \
-d '{"actions": ["loadbalancer_create"]}' \
Copy link
Contributor

Choose a reason for hiding this comment

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

do roles have names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They should, but SpiceDB doesn't have a notion of a resource that we can give names, create/update times, etc to. That will have to come as part of a subsequent set of PRs that add a separate set of tables for tracking that information.

}

resp := createAssignmentResponse{
Success: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

is it useful to return the assignment id here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no assignment ID as such. Assignments are relationships, and the entire (resource, relation, subject) tuple effectively is the identifier for a relationship, where SpiceDB only really supports querying for sets of relationships by certain criteria.

In other words, rather than saying "delete assignment $UUID", you say "delete all relationships with a resource of x, a relation of y, and (maybe) a subject resource of z".

@jnschaeffer jnschaeffer merged commit 3d1092b into infratographer:main Apr 5, 2023
@jnschaeffer jnschaeffer deleted the management-api branch April 5, 2023 15:20
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.

3 participants