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

feat: batch check relations #1521

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

patrickduffy95
Copy link

@patrickduffy95 patrickduffy95 commented Apr 15, 2024

Related issue(s)

#812

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

NOTE: This is a "draft" PR that isn't complete and is meant to get early feedback on the proposed set of changes.

This change adds REST and gRPC endpoints for batch checking relations. The endpoint accepts a list of relation tuples to check, iterates through them, and returns a list of allowed responses.

REST API
New endpoint:
POST /relation-tuples/batchCheck?max-depth=<depth>
Request body:

{
    "tuples": [
        {
            "namespace": <namespace>,
            "object": <object>,
            "relation": <relation>,
            "subject_id": <subject_id>,
            "subject_set": <subject_set>
        }
    ]
}

Response:

{
    "results": [
        {
            "allowed": true
        }
    ]
}

gRPC
New RPC:
CheckService/BatchCheck
Request:

// The request for a CheckService.BatchCheck RPC.
// Checks a batch of relations.
message BatchCheckRequest {
  repeated RelationTuple tuples = 1;

  // This field is not implemented yet and has no effect.
  bool latest = 2;
  // This field is not implemented yet and has no effect.
  string snaptoken = 3;
  int32 max_depth = 4;
}

Response

// The response for a CheckService.Check rpc.
message BatchCheckResponse {
  //
  repeated CheckResponse results = 1;
}

Some specific questions I'd like feedback on:

  • Should we limit the number of tuples this batch API accepts?
  • Should we parallelize the check requests?
    • A more optimized implementation that reduces the number of calls to the DB rather than iterating through the request tuples is probably more than I'd want to tackle
  • Is implementing only the POST okay for the REST API?

@CLAassistant
Copy link

CLAassistant commented Apr 15, 2024

CLA assistant check
All committers have signed the CLA.

@patrickduffy95 patrickduffy95 changed the title Batch check relations feat: Batch check relations Apr 15, 2024
@patrickduffy95 patrickduffy95 changed the title feat: Batch check relations feat: batch check relations Apr 15, 2024
@patrickduffy95
Copy link
Author

@alnr would you be able to let me know if this PR is on the right track?

@aeneasr
Copy link
Member

aeneasr commented May 13, 2024

Hello, sorry for kot responding here. I think this feature is grand! @zepatrik and @hperl are finishing up some work on our end and can probably check out this PR in a week or two.

@PrimeDominus
Copy link

Thanks for implementing this. We've hit a roadblock and without this feature we have to write a bunch of custom code in our server which is getting cumbersome to maintain. Really looking forward to trying this.

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.

None yet

4 participants