-
Notifications
You must be signed in to change notification settings - Fork 267
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
integrates caveat context in PermissionService API methods #886
Conversation
8f3d3b8
to
c920bf4
Compare
d8c3a1f
to
a109547
Compare
a109547
to
72facd0
Compare
@@ -112,7 +111,7 @@ func (cl *ConcurrentLookup) LookupViaReachability(ctx context.Context, req Valid | |||
Metadata: req.Metadata, | |||
}, stream) | |||
if err != nil { | |||
resp := lookupResultError(NewErrInvalidArgument(fmt.Errorf("error in reachablility: %w", err)), emptyMetadata) | |||
resp := lookupResultError(err, emptyMetadata) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was turning every error received by reachable resources into an invalid argument, and prevented downstream code to provide a more specific error code. It also made the assumption everything fell in that category, whereas other error handling in this method did not make such assumption. Therefore it seemed uncontroversial to remove this.
@@ -111,6 +111,9 @@ func (cl *ConcurrentLookupSubjects) lookupDirectSubjects( | |||
if it.Err() != nil { | |||
return it.Err() | |||
} | |||
if tpl.Caveat != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
choke point to prevent evaluation of the graph if relationship is caveated. This is a temporal measure until caveat support is introduced in Lookup*
methods
72facd0
to
e417f4a
Compare
45f88f5
to
72b3cff
Compare
ba79cbd
to
3168732
Compare
3168732
to
1a16604
Compare
- fully implemented in Check API, new permissionship value "conditional" returned if fields were missing in context - LookupResources and LookupSubjects return errors when a caveated relationship is detected in the working set - a limit of 4096 bytes in the caveat context was introduced. Eventually we can turn this into a configurable setting
1a16604
to
97069de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes #881
Closes #882
Closes #880
What
integrates caveat context in PermissionService API methods
How
4096
bytes in the caveat context was introduced. Eventually we can turn that into a configurable parameterPERMISSIONSHIP_CONDITIONAL_PERMISSION
response, and the response will contain missing fieldsLookupSubjects
andLookupResources
are adjusted to return a gRPC unimplemented error. Various choke points where added to prevent evaluation of caveated relationships in the graph