GraphQL: improve authorization of lazy loaded items
For default authorize items when the connection field is executed - https://gitlab.com/gitlab-org/gitlab/blob/12-4-stable-ee/lib/gitlab/graphql/authorize/authorize_field_service.rb#L76-80, in short what we do is that we take the page of items which are supposed to be returned and filter out items user is not authorized to see.
For example if we load epic issues for multiple epics, then we run this authorization for each epic (to authorize issues for each single epic):
epics(first: 100) {
nodes {
issues(first: 100) {
nodes {
iid
}
}
}
}
This is a problem for lazy loaded items because issues
authorization will be done when executing/evaluating the connection field per each epic
. And it's too early - before all epics are processed -> so lazy loading will not work as expected because we want to access epic issues after all of them are lazy-loaded. Ideally we want to avoid accessing lazy-loaded value until it's rendered.
!19141 (merged) added support for epic issues lazy loading, but for now worked around the authorization system just by disabling authorization and calling epic issues authorization when rendering items. The benefit of this approach is that we filter/authorize only nodes which are going to be rendered. But to be able to filter items in this phase it introduces new type of connection - FilterableArrayConnection - !19141 (diffs) which extends default GraphQL Array connection but assures that items are filtered (by calling a proc) before being rendered (calling paged_nodes
method).
One direction I tried was using .then
method (https://www.rubydoc.info/github/rmosolgo/graphql-ruby/GraphQL/Execution/Lazy#then-instance_method) which in theory has a potential to avoid defining FilterableArray. IOW we would just wrap evaluation of lazy method with additional proc
which would be clean. There are couple of obstacles though:
- it's a private API method
- we want to filter all issues at once instead of each of them separately
I think this is an area where we could iterate on and eventually propose an upstream change which would allow to do filtering on lazy evaluation for only rendered items
Related to !19141 (comment 244440561)