GraphOS Enterprise: authorization directives #3397
Conversation
We introduce two new directives, `@authenticated` and `requiresScopes`, that define authorization policies for field and types in the supergraph schema. They are defined as follows: ```graphql directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @requiresScopes(scopes: [String!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM ``` They are implemented by hooking the request lifecycle at multiple steps: - in query analysis, we extract from the query the list of scopes that would be relevant to authorize the query - in a supergraph plugin, we calculate the authorization status and put it in the context: `is_authenticated` for `@authenticated`, and the intersection of the query's required scopes and the scopes provided in the token, for `@requiresScopes` - in the query planning phase, we filter the query to remove the fields that are not authorized, then the filtered query goes through query planning - at the subgraph level, if query deduplication is active, the authorization status is used to group queries together - at the execution service level, the response is formatted according to the filtered query first, which will remove any unauthorized information, then to the shape of the original query, which will propagate nulls as needed - at the execution service level, errors are added to the response indicating which fields were removed because they were not authorized
This comment has been minimized.
This comment has been minimized.
|
CI performance tests
|
| They are defined as follows: | ||
|
|
||
| ```graphql | ||
| directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM |
There was a problem hiding this comment.
Should these be imported from a @link?
There was a problem hiding this comment.
probably yes. We'll update following @trevor-scheer's recommendations
There was a problem hiding this comment.
Yes:
@link(url: "https://specs.apollo.dev/federation/v2.5", import: [..., "@authenticated", "@requiresScopes"])|
|
||
| type User | ||
| implements I | ||
| @authenticated { |
There was a problem hiding this comment.
this is the only example that applies the directive to a type. should we have a section on "inheritance rules" and how applying these directives to types affect fields across subgraphs? (this is a common footgun in contracts)
There was a problem hiding this comment.
I think so yes. I'm unsure about the balance in these docs between simple examples and nearly full spec describing edge cases. I think we'll end up documenting all the edge cases here anyway
Co-authored-by: Lenny Burdette <lenny@apollographql.com>
this uncovers an issue with type condition on fragments, fragment spreads and inline fragments: we should check if the type is authorized there
if we want rhai or a (future) coprocessor to modify the authorization status at the supergraph level, then the cache key metadata for authorization should be set up after those plugins have run
| }) | ||
| }) | ||
| }); | ||
| let query_scopes = context.get_json_value(REQUIRED_SCOPES_KEY).and_then(|v| { |
There was a problem hiding this comment.
this (and other new entries) should probably go in private entries instead
There was a problem hiding this comment.
this one cannot go in private entries because it is cached by query analysis, and private entries cannot be cloned
| paths = filtered_query.format_response( | ||
| &mut response, | ||
| operation_name, | ||
| variables.clone(), | ||
| schema.api_schema(), | ||
| variables_set, | ||
| ); | ||
|
|
There was a problem hiding this comment.
if a subgraph sets an error at a path of an unauthorized field (ex: that field is used for joins between subgraphs), that error should be removed from the response
| }), | ||
| Some((query, mut paths)) => { | ||
| if query.is_empty() { | ||
| return Err(QueryPlannerError::PlanningErrors(PlanErrors { |
There was a problem hiding this comment.
do we want to return a query planning error, or return an empty or null data field with an error?
| .map_err(|e| SpecError::ParsingError(e.to_string()))? | ||
| .to_string(); | ||
|
|
||
| if visitor.query_requires_authentication { |
There was a problem hiding this comment.
maybe make this logic easier to understand (we return Ok(None) if we don't need to modify the query, which means either the query does not contain fields requiring authentication, or we are authenticated and the query does not need to be modified)
| .get(condition) | ||
| .is_some_and(|ty| self.is_type_authorized(ty)); | ||
|
|
||
| // FIXME: if a field was removed inside a fragment definition, then we should add an unauthorized path |
There was a problem hiding this comment.
this comment should be under fragment_definition
There was a problem hiding this comment.
the transform visitor handles operations before fragments. It should instead check fragments first, then operations. We would then be able to store the error paths per fragment, and at their application point, generate an error path starting from the operation
If authorization directives are not set consistently on all types
implementing an interface, then a query on that interface should use
fragments.
In the same way, if they are not applied consistently on the fields of
the interface, the query should use fragments.
As an example, with this schema:
```graphql
type Query {
test: String
itf: I!
}
interface I {
id: ID
}
type A implements I {
id: ID
a: String
}
type B implements I @authenticated {
id: ID
b: String
}
```
The query:
```graphql
query {
test
itf {
id
}
}
```
should be filtered as:
```graphql
query {
test
}
```
While this one:
```graphql
query {
test
itf {
... on A {
id
}
... on B {
id
}
}
}
```
will be filtered as:
```graphql
query {
test
itf {
... on A {
id
}
}
}
```
now snapshots will display the query, filtered query and paths in one file
Co-authored-by: Bryn Cooke <BrynCooke@gmail.com>
We introduce two new directives, `@authenticated` and `requiresScopes`, that define authorization policies for field and types in the supergraph schema. They are defined as follows: ```graphql directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM scalar federation__Scope directive @requiresScopes(scopes: [[federation__Scope!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM ``` They are implemented by hooking the request lifecycle at multiple steps: - in query analysis, we extract from the query the list of scopes that would be relevant to authorize the query - in a supergraph plugin, we calculate the authorization status and put it in the context: `is_authenticated` for `@authenticated`, and the intersection of the query's required scopes and the scopes provided in the token, for `@requiresScopes` - in the query planning phase, we filter the query to remove the fields that are not authorized, then the filtered query goes through query planning - at the subgraph level, if query deduplication is active, the authorization status is used to group queries together - at the execution service level, the response is formatted according to the filtered query first, which will remove any unauthorized information, then to the shape of the original query, which will propagate nulls as needed - at the execution service level, errors are added to the response indicating which fields were removed because they were not authorized --------- Co-authored-by: Lenny Burdette <lenny@apollographql.com> Co-authored-by: Maria Elisabeth Schreiber <maria.schreiber@apollographql.com> Co-authored-by: Lucas Leadbetter <5595530+lleadbet@users.noreply.github.com> Co-authored-by: Simon Sapin <simon@apollographql.com> Co-authored-by: Chandrika Srinivasan <chandrikas@users.noreply.github.com> Co-authored-by: Bryn Cooke <BrynCooke@gmail.com>
We introduce two new directives,
@authenticatedandrequiresScopes, that define authorization policies for field and types in the supergraph schema.They are defined as follows:
They are implemented by hooking the request lifecycle at multiple steps:
is_authenticatedfor@authenticated, and the intersection of the query's required scopes and the scopes provided in the token, for@requiresScopesChecklist
Complete the checklist (and note appropriate exceptions) before a final PR is raised.
Exceptions
Note any exceptions here
Notes
[^1]. It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this.
[^2]. Configuration is an important part of many changes. Where applicable please try to document configuration examples.
[^3]. Tick whichever testing boxes are applicable. If you are adding Manual Tests:
- please document the manual testing (extensively) in the Exceptions.
- please raise a separate issue to automate the test and label it (or ask for it to be labeled) as
manual test