fix(composition): restrict usage of auth directives on interfaces

apollo-federation/src/connectors/spec/connect.rs

@@ -487,7 +487,6 @@ mod tests {
         let fields = schema
             .referencers()
             .get_directive(CONNECT_DIRECTIVE_NAME_IN_SPEC.as_str())
-            .unwrap()
             .object_fields
             .iter()
             .map(|f| f.get(schema.schema()).unwrap().to_string())

apollo-federation/src/connectors/spec/source.rs

@@ -285,8 +285,7 @@ mod tests {
         insta::assert_debug_snapshot!(
             subgraph.schema
                 .referencers()
-                .get_directive(SOURCE_DIRECTIVE_NAME_IN_SPEC.as_str())
-                .unwrap(),
+                .get_directive(SOURCE_DIRECTIVE_NAME_IN_SPEC.as_str()),
             @r###"
                 DirectiveReferencers {
                     schema: Some(
@@ -406,8 +405,7 @@ mod tests {
         // Extract the sources from the schema definition and map them to their `Source` equivalent
         let sources = schema
             .referencers()
-            .get_directive(&SOURCE_DIRECTIVE_NAME_IN_SPEC)
-            .unwrap();
+            .get_directive(&SOURCE_DIRECTIVE_NAME_IN_SPEC);
 
         let schema_directive_refs = sources.schema.as_ref().unwrap();
         let sources: Result<Vec<_>, _> = schema_directive_refs

apollo-federation/src/error/mod.rs

@@ -828,6 +828,14 @@ pub enum SingleFederationError {
     InvalidTagName { message: String },
     #[error("{message}")]
     QueryRootMissing { message: String },
+    #[error(
+        "Invalid use of @{directive_name} on {kind} \"{coordinate}\": @{directive_name} cannot be applied on interfaces, interface fields and interface objects"
+    )]
+    AuthRequirementsAppliedOnInterface {
+        directive_name: String,
+        kind: String,
+        coordinate: String,
+    },
 }
 
 impl SingleFederationError {
@@ -1053,6 +1061,9 @@ impl SingleFederationError {
             SingleFederationError::InvalidFieldSharing { .. } => ErrorCode::InvalidFieldSharing,
             SingleFederationError::InvalidTagName { .. } => ErrorCode::InvalidTagName,
             SingleFederationError::QueryRootMissing { .. } => ErrorCode::QueryRootMissing,
+            SingleFederationError::AuthRequirementsAppliedOnInterface { .. } => {
+                ErrorCode::AuthRequirementsAppliedOnInterface
+            }
         }
     }
 
@@ -2372,6 +2383,19 @@ static QUERY_ROOT_MISSING: LazyLock<ErrorCodeDefinition> = LazyLock::new(|| {
     )
 });
 
+static AUTH_REQUIREMENTS_APPLIED_ON_INTERFACE: LazyLock<ErrorCodeDefinition> = LazyLock::new(
+    || {
+        ErrorCodeDefinition::new(
+        "AUTH_REQUIREMENTS_APPLIED_ON_INTERFACE".to_owned(),
+        "The @authenticated, @requiresScopes and @policy directive cannot be applied on interface, interface object or their fields.".to_owned(),
+        Some(ErrorCodeMetadata {
+            added_in: "2.9.4",
+            replaces: &[],
+        }),
+    )
+    },
+);
+
 #[derive(Debug, PartialEq, strum_macros::EnumIter)]
 pub enum ErrorCode {
     ErrorCodeMissing,
@@ -2474,6 +2498,7 @@ pub enum ErrorCode {
     OverrideLabelInvalid,
     ContextualArgumentNotContextualInAllSubgraphs,
     QueryRootMissing,
+    AuthRequirementsAppliedOnInterface,
 }
 
 impl ErrorCode {
@@ -2597,6 +2622,9 @@ impl ErrorCode {
                 &CONTEXTUAL_ARGUMENT_NOT_CONTEXTUAL_IN_ALL_SUBGRAPHS
             }
             ErrorCode::QueryRootMissing => &QUERY_ROOT_MISSING,
+            ErrorCode::AuthRequirementsAppliedOnInterface => {
+                &AUTH_REQUIREMENTS_APPLIED_ON_INTERFACE
+            }
         }
     }
 }

apollo-federation/src/link/inaccessible_spec_definition.rs

@@ -1070,7 +1070,7 @@ fn remove_inaccessible_elements(
         })?;
 
     // Find all elements that use @inaccessible. Clone so there's no live borrow.
-    let inaccessible_referencers = schema.referencers().get_directive(&directive_name)?.clone();
+    let inaccessible_referencers = schema.referencers().get_directive(&directive_name).clone();
 
     // Remove fields and arguments from inaccessible types first. If any inaccessible type has a field
     // that references another inaccessible type, it would prevent the other type from being

apollo-federation/src/merger/merge_links.rs

@@ -65,7 +65,7 @@ impl Merger {
                 let Some(linked_elem) = features.source_link_of_directive(directive) else {
                     continue;
                 };
-                if referencers.len() == 0 {
+                if referencers.is_empty() {
                     continue;
                 }
                 let source = linked_elem.link;

apollo-federation/src/merger/merger.rs

@@ -293,13 +293,14 @@ impl Merger {
         subgraphs
             .iter()
             .fold(Default::default(), |mut acc, subgraph| {
-                if let Ok(Some(directive_name)) = subgraph.from_context_directive_name()
-                    && let Ok(referencers) = subgraph
+                if let Ok(Some(directive_name)) = subgraph.from_context_directive_name() {
+                    let referencers = subgraph
                         .schema()
                         .referencers()
-                        .get_directive(&directive_name)
-                {
-                    acc.extend(referencers);
+                        .get_directive(&directive_name);
+                    if !referencers.is_empty() {
+                        acc.extend(referencers);
+                    }
                 }
                 acc
             })
@@ -311,13 +312,14 @@ impl Merger {
         subgraphs
             .iter()
             .fold(Default::default(), |mut acc, subgraph| {
-                if let Ok(Some(directive_name)) = subgraph.override_directive_name()
-                    && let Ok(referencers) = subgraph
+                if let Ok(Some(directive_name)) = subgraph.override_directive_name() {
+                    let referencers = subgraph
                         .schema()
                         .referencers()
-                        .get_directive(&directive_name)
-                {
-                    acc.extend(referencers);
+                        .get_directive(&directive_name);
+                    if !referencers.is_empty() {
+                        acc.extend(referencers);
+                    }
                 }
                 acc
             })
@@ -2489,7 +2491,7 @@ format!("Field \"{field}\" of {} type \"{}\" is defined in some but not all subg
         let graph_enum_values: Vec<Name> = graph_enum.values.keys().cloned().collect();
 
         let referencers = self.merged.referencers();
-        let field_positions = referencers.get_directive(&join_field_directive_name)?;
+        let field_positions = referencers.get_directive(&join_field_directive_name);
         let positions_to_process: Vec<DirectiveTargetPosition> = field_positions.iter().collect();
 
         for pos in positions_to_process {

apollo-federation/src/query_graph/build_query_graph.rs

@@ -116,7 +116,7 @@ pub fn build_supergraph_api_query_graph(
     let join_field_definition = join_spec.field_directive_definition(&supergraph_schema)?;
     let join_field_applications = supergraph_schema
         .referencers()
-        .get_directive_applications(&supergraph_schema, &join_field_definition.name)?;
+        .get_directive_applications(&supergraph_schema, &join_field_definition.name);
 
     let mut override_labels_by_field = IndexMap::default();
     for (pos, node) in join_field_applications {
@@ -2251,7 +2251,7 @@ impl FederatedQueryGraphBuilder {
             let subgraph_data = self.subgraphs.get(source)?;
             for type_pos in &schema
                 .referencers()
-                .get_directive(&subgraph_data.interface_object_directive_definition_name)?
+                .get_directive(&subgraph_data.interface_object_directive_definition_name)
                 .object_types
             {
                 // This method is run after handling @provides, so there may be multiple nodes here

apollo-federation/src/schema/blueprint.rs

@@ -28,6 +28,7 @@ use crate::schema::ValidFederationSchema;
 use crate::schema::compute_subgraph_metadata;
 use crate::schema::position::DirectiveDefinitionPosition;
 use crate::schema::subgraph_metadata::SubgraphMetadata;
+use crate::schema::validators::access_control::validate_no_access_control_on_interfaces;
 use crate::schema::validators::context::validate_context_directives;
 use crate::schema::validators::cost::validate_cost_directives;
 use crate::schema::validators::external::validate_external_directives;
@@ -139,6 +140,7 @@ impl FederationBlueprint {
         validate_cost_directives(schema, &mut error_collector)?;
         validate_list_size_directives(schema, &mut error_collector)?;
         validate_tag_directives(schema, &mut error_collector)?;
+        validate_no_access_control_on_interfaces(schema, meta, &mut error_collector)?;
 
         error_collector.into_result()
     }
@@ -305,7 +307,11 @@ impl FederationBlueprint {
                 // definition to re-add the "correct" version, we'd have to re-attach existing applications (doable but not
                 // done). This assert is so we notice it quickly if that ever happens (again, unlikely, because fed1 schema
                 // is a backward compatibility thing and there is no reason to expand that too much in the future).
-                if schema.referencers().get_directive(directive_name)?.len() > 0 {
+                if !schema
+                    .referencers()
+                    .get_directive(directive_name)
+                    .is_empty()
+                {
                     bail!(
                         "Subgraph has applications of @{directive_name} but we are trying to remove the definition."
                     );

apollo-federation/src/schema/mod.rs

@@ -413,7 +413,7 @@ impl FederationSchema {
         let context_directive_definition = federation_spec.context_directive_definition(self)?;
         let context_directive_referencers = self
             .referencers()
-            .get_directive(&context_directive_definition.name)?;
+            .get_directive(&context_directive_definition.name);
 
         let mut applications = Vec::new();
         for interface_type_position in &context_directive_referencers.interface_types {
@@ -472,7 +472,7 @@ impl FederationSchema {
         let context_directive_definition = context_spec.context_directive_definition(self)?;
         let context_directive_referencers = self
             .referencers()
-            .get_directive(&context_directive_definition.name)?;
+            .get_directive(&context_directive_definition.name);
         let mut applications = Vec::new();
         for type_pos in context_directive_referencers.composite_type_positions() {
             let directive_apps =
@@ -499,7 +499,7 @@ impl FederationSchema {
             federation_spec.from_context_directive_definition(self)?;
         let from_context_directive_referencers = self
             .referencers()
-            .get_directive(&from_context_directive_definition.name)?;
+            .get_directive(&from_context_directive_definition.name);
 
         let mut applications = Vec::new();
 
@@ -555,7 +555,7 @@ impl FederationSchema {
         let key_directive_definition = federation_spec.key_directive_definition(self)?;
         let key_directive_referencers = self
             .referencers()
-            .get_directive(&key_directive_definition.name)?;
+            .get_directive(&key_directive_definition.name);
 
         let mut applications: Vec<Result<KeyDirective, FederationError>> = Vec::new();
         for object_type_position in &key_directive_referencers.object_types {
@@ -617,7 +617,7 @@ impl FederationSchema {
         let provides_directive_definition = federation_spec.provides_directive_definition(self)?;
         let provides_directive_referencers = self
             .referencers()
-            .get_directive(&provides_directive_definition.name)?;
+            .get_directive(&provides_directive_definition.name);
 
         let mut applications: Vec<Result<ProvidesDirective, FederationError>> = Vec::new();
         for field_definition_position in provides_directive_referencers.object_or_interface_fields()
@@ -668,7 +668,7 @@ impl FederationSchema {
         let requires_directive_definition = federation_spec.requires_directive_definition(self)?;
         let requires_directive_referencers = self
             .referencers()
-            .get_directive(&requires_directive_definition.name)?;
+            .get_directive(&requires_directive_definition.name);
 
         let mut applications = Vec::new();
         for field_definition_position in requires_directive_referencers.object_or_interface_fields()
@@ -716,7 +716,7 @@ impl FederationSchema {
         let tag_directive_definition = federation_spec.tag_directive_definition(self)?;
         let tag_directive_referencers = self
             .referencers()
-            .get_directive(&tag_directive_definition.name)?;
+            .get_directive(&tag_directive_definition.name);
 
         let mut applications = Vec::new();
         // Schema
@@ -1026,12 +1026,9 @@ impl FederationSchema {
         else {
             return Ok(Vec::new());
         };
-        let Ok(list_size_directive_referencers) = self
+        let list_size_directive_referencers = self
             .referencers()
-            .get_directive(list_size_directive_name.as_str())
-        else {
-            return Ok(Vec::new());
-        };
+            .get_directive(list_size_directive_name.as_str());
 
         let mut applications = Vec::new();
         for field_definition_position in
@@ -1073,7 +1070,7 @@ impl FederationSchema {
 
         let result = self
             .referencers()
-            .get_directive_applications(self, &cache_tag_directive_definition.name)?
+            .get_directive_applications(self, &cache_tag_directive_definition.name)
             .map(|(pos, application)| {
                 let arguments = federation_spec.cache_tag_directive_arguments(application);
                 arguments.map(|args| CacheTagDirective {

apollo-federation/src/schema/referencer.rs

@@ -1,4 +1,5 @@
 use std::fmt;
+use std::sync::LazyLock;
 
 use apollo_compiler::Name;
 use apollo_compiler::Node;
@@ -10,7 +11,6 @@ use itertools::Itertools;
 use super::FederationSchema;
 use crate::error::FederationError;
 use crate::error::SingleFederationError;
-use crate::internal_error;
 use crate::schema::position::CompositeTypeDefinitionPosition;
 use crate::schema::position::DirectiveArgumentDefinitionPosition;
 use crate::schema::position::DirectiveTargetPosition;
@@ -181,29 +181,23 @@ impl Referencers {
         })
     }
 
-    pub(crate) fn get_directive(
-        &self,
-        name: &str,
-    ) -> Result<&DirectiveReferencers, FederationError> {
-        self.directives.get(name).ok_or_else(|| {
-            internal_error!("Directive referencers unexpectedly missing directive `{name}`")
-        })
+    pub(crate) fn get_directive(&self, name: &str) -> &DirectiveReferencers {
+        self.directives
+            .get(name)
+            .unwrap_or_else(|| &EMPTY_REFERENCERS)
     }
 
     pub(crate) fn get_directive_applications<'schema>(
         &self,
         schema: &'schema FederationSchema,
         name: &Name,
-    ) -> Result<
-        impl Iterator<Item = (DirectiveTargetPosition, &'schema Node<ast::Directive>)>,
-        FederationError,
-    > {
-        let directive_referencers = self.get_directive(name)?;
-        Ok(directive_referencers.iter().flat_map(|pos| {
+    ) -> impl Iterator<Item = (DirectiveTargetPosition, &'schema Node<ast::Directive>)> {
+        let directive_referencers = self.get_directive(name);
+        directive_referencers.iter().flat_map(|pos| {
             pos.get_applied_directives(schema, name)
                 .into_iter()
                 .map(move |directive_application| (pos.clone(), directive_application))
-        }))
+        })
     }
 
     pub(crate) fn rename_object_type(&mut self, old_name: &Name, new_name: &Name) {
@@ -525,6 +519,9 @@ impl InputObjectTypeReferencers {
     }
 }
 
+static EMPTY_REFERENCERS: LazyLock<DirectiveReferencers> =
+    LazyLock::new(DirectiveReferencers::default);
+
 #[derive(Debug, Clone, Default)]
 pub(crate) struct DirectiveReferencers {
     pub(crate) schema: Option<SchemaDefinitionPosition>,

apollo-federation/src/schema/schema_upgrader.rs

@@ -313,7 +313,7 @@ impl SchemaUpgrader {
     ) -> Result<(), FederationError> {
         // both @provides and @requires will only have an object_fields referencer
         for directive_name in ["requires", "provides"] {
-            let referencers = schema.referencers().get_directive(directive_name)?;
+            let referencers = schema.referencers().get_directive(directive_name);
             for field in &referencers.object_fields.clone() {
                 let field_type = field.make_mut(&mut schema.schema)?.make_mut();
 
@@ -331,7 +331,7 @@ impl SchemaUpgrader {
 
         // now do the exact same thing for @key. The difference is that the directive location will be object_types
         // rather than object_fields
-        let referencers = schema.referencers().get_directive("key")?;
+        let referencers = schema.referencers().get_directive("key");
         for field in &referencers.object_types.clone() {
             let field_type = field.make_mut(&mut schema.schema)?.make_mut();
 
@@ -689,7 +689,7 @@ impl SchemaUpgrader {
         if let Some(key) = &upgrade_metadata.key_directive_name {
             for pos in schema
                 .referencers()
-                .get_directive(key)?
+                .get_directive(key)
                 .interface_types
                 .clone()
             {
@@ -1098,7 +1098,7 @@ fn is_interface_object_used(subgraph: &Subgraph<Expanded>) -> Result<bool, Feder
         let referencers = subgraph
             .schema()
             .referencers()
-            .get_directive(interface_object_def.name.as_str())?;
+            .get_directive(interface_object_def.name.as_str());
         if !referencers.object_types.is_empty() {
             return Ok(true);
         }