diff --git a/.changesets/feat_geal_authz_log_errors.md b/.changesets/feat_geal_authz_log_errors.md new file mode 100644 index 0000000000..6ebbee139c --- /dev/null +++ b/.changesets/feat_geal_authz_log_errors.md @@ -0,0 +1,13 @@ +### add a flag to disable authorization error logs ([PR #4076](https://github.com/apollographql/router/pull/4076)) + +Authorization errors can be seen as common usage of the service when filtering fields from queries depending on the client's rights, so they might not warrant error logs to be analyzed by the router operators + +Those logs can be disabled in the configuration: + +```yaml +authorization: + preview_directives: + log_errors: true +``` + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/4076 \ No newline at end of file diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 1946df6985..c1091b3c79 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -528,6 +528,11 @@ expression: "&schema" "default": true, "type": "boolean" }, + "log_errors": { + "description": "Log authorization errors", + "default": true, + "type": "boolean" + }, "reject_unauthorized": { "description": "refuse a query entirely if any part would be filtered", "default": false, diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 2a43d2c82a..89e3fc1d6b 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -80,6 +80,19 @@ pub(crate) struct Directives { /// refuse a query entirely if any part would be filtered #[serde(default)] reject_unauthorized: bool, + /// Log authorization errors + #[serde(default = "enable_log_errors")] + log_errors: bool, +} + +fn enable_log_errors() -> bool { + true +} + +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub(crate) struct UnauthorizedPaths { + pub(crate) paths: Vec, + pub(crate) log_errors: bool, } fn default_enable_directives() -> bool { @@ -110,6 +123,17 @@ impl AuthorizationPlugin { Ok(has_config.unwrap_or(true) && has_authorization_directives) } + pub(crate) fn log_errors(configuration: &Configuration) -> bool { + let has_config = configuration + .apollo_plugins + .plugins + .iter() + .find(|(s, _)| s.as_str() == "authorization") + .and_then(|(_, v)| v.get("preview_directives").and_then(|v| v.as_object())) + .and_then(|v| v.get("log_errors").and_then(|v| v.as_bool())); + has_config.unwrap_or(true) + } + pub(crate) async fn query_analysis( query: &str, schema: &Schema, @@ -453,7 +477,7 @@ impl Plugin for AuthorizationPlugin { fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { ServiceBuilder::new() .map_request(|request: execution::Request| { - let filtered = !request.query_plan.query.unauthorized_paths.is_empty(); + let filtered = !request.query_plan.query.unauthorized.paths.is_empty(); let needs_authenticated = request.context.contains_key(AUTHENTICATED_KEY); let needs_requires_scopes = request.context.contains_key(REQUIRED_SCOPES_KEY); diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index a1db816e1a..d75e54437e 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -28,6 +28,7 @@ use crate::json_ext::Object; use crate::json_ext::Path; use crate::plugins::authorization::AuthorizationPlugin; use crate::plugins::authorization::CacheKeyMetadata; +use crate::plugins::authorization::UnauthorizedPaths; use crate::query_planner::labeler::add_defer_labels; use crate::services::layers::query_analysis::ParsedDocument; use crate::services::layers::query_analysis::ParsedDocumentInner; @@ -231,7 +232,10 @@ impl BridgeQueryPlanner { fragments, operations, filtered_query: None, - unauthorized_paths: vec![], + unauthorized: UnauthorizedPaths { + paths: vec![], + log_errors: AuthorizationPlugin::log_errors(&self.configuration), + }, subselections, defer_stats, is_original: true, @@ -547,7 +551,7 @@ impl BridgeQueryPlanner { executable: new_doc.to_executable(&self.schema.api_schema().definitions), ast: new_doc, }); - selections.unauthorized_paths = unauthorized_paths; + selections.unauthorized.paths = unauthorized_paths; } if selections.contains_introspection() { diff --git a/apollo-router/src/services/execution_service.rs b/apollo-router/src/services/execution_service.rs index 3de41b2891..00cf5629d9 100644 --- a/apollo-router/src/services/execution_service.rs +++ b/apollo-router/src/services/execution_service.rs @@ -237,8 +237,9 @@ impl ExecutionService { tracing::debug_span!("format_response").in_scope(|| { let mut paths = Vec::new(); if let Some(filtered_query) = query.filtered_query.as_ref() { - let unauthorized_paths = query.unauthorized_paths.iter().map(|path| path.to_string()).collect::>(); - if !unauthorized_paths.is_empty() { + if query.unauthorized.log_errors && !query.unauthorized.paths.is_empty() { + let unauthorized_paths = query.unauthorized.paths.iter().map(|path| path.to_string()).collect::>(); + event!(Level::ERROR, unauthorized_query_paths = ?unauthorized_paths, "Authorization error",); } @@ -250,7 +251,7 @@ impl ExecutionService { variables_set, ); - for path in &query.unauthorized_paths { + for path in &query.unauthorized.paths { response.errors.push(Error::builder() .message("Unauthorized field or type") .path(path.clone()) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 0151b925ea..113b56d073 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -30,6 +30,7 @@ use crate::json_ext::Object; use crate::json_ext::Path; use crate::json_ext::ResponsePathElement; use crate::json_ext::Value; +use crate::plugins::authorization::UnauthorizedPaths; use crate::query_planner::fetch::OperationKind; use crate::services::layers::query_analysis::ParsedDocument; use crate::services::layers::query_analysis::ParsedDocumentInner; @@ -59,7 +60,7 @@ pub(crate) struct Query { #[derivative(PartialEq = "ignore", Hash = "ignore")] pub(crate) subselections: HashMap, #[derivative(PartialEq = "ignore", Hash = "ignore")] - pub(crate) unauthorized_paths: Vec, + pub(crate) unauthorized: UnauthorizedPaths, #[derivative(PartialEq = "ignore", Hash = "ignore")] pub(crate) filtered_query: Option>, #[derivative(PartialEq = "ignore", Hash = "ignore")] @@ -97,7 +98,7 @@ impl Query { }, operations: Vec::new(), subselections: HashMap::new(), - unauthorized_paths: Vec::new(), + unauthorized: UnauthorizedPaths::default(), filtered_query: None, defer_stats: DeferStats { has_defer: false, @@ -296,7 +297,7 @@ impl Query { fragments, operations, subselections: HashMap::new(), - unauthorized_paths: Vec::new(), + unauthorized: UnauthorizedPaths::default(), filtered_query: None, defer_stats, is_original: true, diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index c63c86c0f0..eed72da13b 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -5660,7 +5660,7 @@ fn filtered_defer_fragment() { subselections, defer_stats, is_original: true, - unauthorized_paths: vec![], + unauthorized: UnauthorizedPaths::default(), validation_error: None, }; @@ -5688,7 +5688,7 @@ fn filtered_defer_fragment() { subselections, defer_stats, is_original: false, - unauthorized_paths: vec![], + unauthorized: UnauthorizedPaths::default(), validation_error: None, }; diff --git a/docs/source/configuration/authorization.mdx b/docs/source/configuration/authorization.mdx index f5dc7a9885..4a4b229490 100644 --- a/docs/source/configuration/authorization.mdx +++ b/docs/source/configuration/authorization.mdx @@ -637,4 +637,14 @@ authorization: reject_unauthorized: true # default: false ``` -This option will entirely reject queries that would have some parts filtered by authorization directives. The response will contain the list of paths that are affected. \ No newline at end of file +This option will entirely reject queries that would have some parts filtered by authorization directives. The response will contain the list of paths that are affected. + +### log_errors + +```yaml title="router.yaml" +authorization: + preview_directives: + log_errors: true +``` + +By default, when part of a query is filtered by authorization, the list of filtered paths is added to the response and logged by the router. Filtering parts of a query according to the client's rights may be seen as normal operation, and not warrant investigation by platform operators, so those logs can be disabled. \ No newline at end of file