feat(graphql): granular permissions #1022
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1021
This implements granular permissions for each
fetcher
/mutator
within the GraphQL endpoint implementation. This is achieved by removing the endpoint-level resource actions set union - by default, only a basic "token is valid" check will be performed - and delegating further permissions checks to each subsequent or nestedfetcher
/mutator
that is executed afterward. In this implementation, each such subquery that is performed will result in a permissions check being performed, checking that the contextual user has the required permissions for the subquery's action. For complex GraphQL queries with deep nesting, this might add noticeable overall request latency since there is an authentication server request round-trip-time at each level of nested query. This might be something to optimize in the future by pre-computing the set union by recursing through the nested queries up front and performing a single permissions check all at once before actually executing the queries, but this needs either some deeper knowledge of GraphQL to pull off or a more complex implementation ofAbstractPermissionedDataFetcher
that encodes the nested query relationships.Overall this ends up implementing something very similar to how our HTTP API permissions checks work with the
AbstractAuthenticatedRequestHandler
/AbstractV2RequestHandler
, but on theDataFetcher
s.Unit tests are still overall sorely lacking - see #947. There is some basic integration test coverage, at least, which caught an error during implementation where the
DataFetchingEnvironment
passed to subqueries contained anull
GraphQLContext
.