refactor: query planner service no longer returns graphql response

.changesets/breaking_bryn_router_1973_emptied_operations_shaped_data.md

@@ -0,0 +1,13 @@
+### Operations with only authorization errors return spec-compliant data ([PR #9911](https://github.com/apollographql/router/pull/9911))
+
+When every field in an operation fails authorization, the response now carries each requested root field as `null` alongside an error for each field, the same shape clients receive when some fields fail:
+
+```json
+{"data": {"orga": null}, "errors": [{"message": "Unauthorized field or type", "path": ["orga", "id"], "extensions": {"code": "UNAUTHORIZED_FIELD_OR_TYPE"}}]}
+```
+
+The router previously returned `"data": null` here, incorrectly reporting that execution never produced a result. Clients that detect this case by checking `data` for `null` should check for errors with the `UNAUTHORIZED_FIELD_OR_TYPE` code instead, which covers partial failures as well.
+
+`authorization.directives.reject_unauthorized` keeps returning `"data": null`; spec compliance for refused operations is tracked separately.
+
+By [@BrynCooke](https://github.com/BrynCooke) in https://github.com/apollographql/router/pull/9911

.changesets/breaking_bryn_router_1973_gate_ordering.md

@@ -0,0 +1,5 @@
+### Request validation errors answer before authorization errors ([PR #9911](https://github.com/apollographql/router/pull/9911))
+
+Authorization enforcement runs at execution, after the router validates the request. An operation that fails both checks now receives the validation error alone: a missing or invalid variable returns the 400 validation response, and a subscription or `@defer` operation sent without the matching `Accept` header returns the 406, where these previously received the authorization errors. Fixing the request then surfaces the authorization errors.
+
+By [@BrynCooke](https://github.com/BrynCooke) in https://github.com/apollographql/router/pull/9911

.changesets/fix_bryn_router_1973_report_rejected_operations.md

@@ -0,0 +1,9 @@
+### Report operations whose fields all fail authorization to Apollo Studio ([PR #9911](https://github.com/apollographql/router/pull/9911))
+
+When authorization raises errors for every field in an operation, or `authorization.directives.reject_unauthorized` refuses the operation outright, Apollo Studio now receives it as an operation, identified by the signature of the query the client sent and carrying the client name, version, and request count. Studio previously received the operation count alone and had nothing to attribute it to.
+
+Such an operation counts as one licensed operation.
+
+The `Authorization error` log event for these operations now appears under the `execution` span instead of inside `query_planning`. Update log or trace filters that match this event by span name.
+
+By [@BrynCooke](https://github.com/BrynCooke) in https://github.com/apollographql/router/pull/9911

apollo-router/src/batching/query_plan_analysis_layer.rs

@@ -215,7 +215,6 @@ mod tests {
     use crate::graphql;
     use crate::query_planner::QueryPlan;
     use crate::query_planner::QueryPlannerService;
-    use crate::services::QueryPlannerContent;
     use crate::services::QueryPlannerRequest;
     use crate::services::execution;
     use crate::spec::Query;
@@ -228,9 +227,7 @@ mod tests {
     ) -> Arc<QueryPlan> {
         let document = Query::parse_document(query, None, &schema, &configuration).unwrap();
 
-        let QueryPlannerContent::Plan {
-            plan: query_plan, ..
-        } = QueryPlannerService::for_test(schema, configuration)
+        QueryPlannerService::for_test(schema, configuration)
             .unwrap()
             .oneshot(
                 QueryPlannerRequest::builder()
@@ -245,11 +242,6 @@ mod tests {
             .unwrap()
             .content
             .unwrap()
-        else {
-            panic!("unexpected query planner output");
-        };
-
-        query_plan
     }
 
     #[tokio::test]

apollo-router/src/error.rs

@@ -271,10 +271,6 @@ pub(crate) enum QueryPlannerError {
     /// spec error: {0}
     SpecError(SpecError),
 
-    // Safe to cache because user scopes and policies are included in the cache key.
-    /// Unauthorized field or type
-    Unauthorized(Vec<Path>),
-
     /// Federation error: {0}
     FederationError(FederationErrorBridge),
 

apollo-router/src/plugins/authorization/mod.rs

@@ -34,7 +34,6 @@ use crate::layers::ServiceBuilderExt;
 use crate::plugin::Plugin;
 use crate::plugin::PluginInit;
 use crate::plugins::authentication;
-use crate::query_planner::FilteredQuery;
 use crate::query_planner::QueryKey;
 use crate::services::execution;
 use crate::services::supergraph;
@@ -138,6 +137,18 @@ pub(crate) struct UnauthorizedPaths {
     pub(crate) errors: ErrorConfig,
 }
 
+/// What [`AuthorizationPlugin::filter_query`] did to an operation.
+pub(crate) enum FilterResult {
+    /// The operation asks for nothing the request lacks authorization for.
+    Unchanged,
+    /// `document` is the operation with `paths` removed. Filtering can empty the
+    /// document entirely, leaving no definitions.
+    Filtered {
+        paths: Vec<Path>,
+        document: ast::Document,
+    },
+}
+
 impl UnauthorizedPaths {
     pub(crate) fn log_unauthorized_paths(&self) {
         // nothing to do if we have no paths or we're not supposed to log
@@ -194,6 +205,7 @@ fn default_enable_directives() -> bool {
 
 pub(crate) struct AuthorizationPlugin {
     require_authentication: bool,
+    reject_unauthorized: bool,
 }
 
 impl AuthorizationPlugin {
@@ -337,8 +349,7 @@ impl AuthorizationPlugin {
         configuration: &Conf,
         key: &QueryKey,
         schema: &Schema,
-    ) -> Result<Option<FilteredQuery>, QueryPlannerError> {
-        let reject_unauthorized = configuration.directives.reject_unauthorized;
+    ) -> Result<FilterResult, QueryPlannerError> {
         let dry_run = configuration.directives.dry_run;
 
         // The filtered query will then be used
@@ -366,7 +377,10 @@ impl AuthorizationPlugin {
 
                 // FIXME: consider only `filtered_doc.operations.get(key.operation_name)`?
                 if filtered_doc.definitions.is_empty() {
-                    return Err(QueryPlannerError::Unauthorized(unauthorized_paths));
+                    return Ok(FilterResult::Filtered {
+                        paths: unauthorized_paths,
+                        document: filtered_doc,
+                    });
                 }
 
                 is_filtered = true;
@@ -384,7 +398,10 @@ impl AuthorizationPlugin {
 
                 // FIXME: consider only `filtered_doc.operations.get(key.operation_name)`?
                 if filtered_doc.definitions.is_empty() {
-                    return Err(QueryPlannerError::Unauthorized(unauthorized_paths));
+                    return Ok(FilterResult::Filtered {
+                        paths: unauthorized_paths,
+                        document: filtered_doc,
+                    });
                 }
 
                 is_filtered = true;
@@ -402,7 +419,10 @@ impl AuthorizationPlugin {
 
                 // FIXME: consider only `filtered_doc.operations.get(key.operation_name)`?
                 if filtered_doc.definitions.is_empty() {
-                    return Err(QueryPlannerError::Unauthorized(unauthorized_paths));
+                    return Ok(FilterResult::Filtered {
+                        paths: unauthorized_paths,
+                        document: filtered_doc,
+                    });
                 }
 
                 is_filtered = true;
@@ -411,14 +431,13 @@ impl AuthorizationPlugin {
             }
         };
 
-        if reject_unauthorized && !unauthorized_paths.is_empty() {
-            return Err(QueryPlannerError::Unauthorized(unauthorized_paths));
-        }
-
         if is_filtered {
-            Ok(Some((unauthorized_paths, doc)))
+            Ok(FilterResult::Filtered {
+                paths: unauthorized_paths,
+                document: doc,
+            })
         } else {
-            Ok(None)
+            Ok(FilterResult::Unchanged)
         }
     }
 
@@ -542,6 +561,7 @@ impl Plugin for AuthorizationPlugin {
     async fn new(init: PluginInit<Self::Config>) -> Result<Self, BoxError> {
         Ok(AuthorizationPlugin {
             require_authentication: init.config.require_authentication,
+            reject_unauthorized: init.config.directives.reject_unauthorized,
         })
     }
 
@@ -580,7 +600,30 @@ impl Plugin for AuthorizationPlugin {
     }
 
     fn execution_service(&self, service: execution::BoxCloneService) -> execution::BoxCloneService {
+        let reject_unauthorized = self.reject_unauthorized;
+
         ServiceBuilder::new()
+            // Ahead of the counter below, so a refused operation stays uncounted.
+            .checkpoint_async(move |request: execution::Request| async move {
+                if reject_unauthorized && !request.query_plan.query.unauthorized.paths.is_empty() {
+                    let unauthorized = request.query_plan.query.unauthorized.clone();
+                    unauthorized.log_unauthorized_paths();
+
+                    // We knowingly build an invalid response here. Execution was prevented,
+                    // so we should respond with a request error and no data. Instead, we're
+                    // responding with execution errors and a fake/incorrect `data: null`. We
+                    // maintain backwards compatibility for the time being.
+                    // Tracked in ROUTER-2063.
+                    let mut response = graphql::Response::builder().data(Value::Null).build();
+                    unauthorized.update_response_with_unauthorized_path_errors(&mut response);
+
+                    return Ok(ControlFlow::Break(
+                        execution::Response::new_from_graphql_response(response, request.context),
+                    ));
+                }
+
+                Ok(ControlFlow::Continue(request))
+            })
             .map_request(|request: execution::Request| {
                 let filtered = !request.query_plan.query.unauthorized.paths.is_empty();
                 let needs_authenticated = request.context.contains_key(AUTHENTICATION_REQUIRED_KEY);

apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__tests__partial_filter_leaving_no_executable_work.snap

@@ -0,0 +1,21 @@
+---
+source: apollo-router/src/plugins/authorization/tests.rs
+expression: body
+---
+{
+  "data": {
+    "currentUser": null
+  },
+  "errors": [
+    {
+      "message": "Unauthorized field or type",
+      "path": [
+        "currentUser",
+        "phone"
+      ],
+      "extensions": {
+        "code": "UNAUTHORIZED_FIELD_OR_TYPE"
+      }
+    }
+  ]
+}