Enable authorization directives by default (#3713)

Fix #3842 This requires a few steps, because authorization directives should not be usable without a valid license, so the license enforcement code has to check for their presence in the graph. Co-authored-by: Maria Elisabeth Schreiber <[email protected]> Co-authored-by: Jeremy Lempereur <[email protected]>
apollographql · Oct 23, 2023 · 0e51563 · 0e51563
1 parent 1103bf8
commit 0e51563
Show file tree

Hide file tree

Showing 10 changed files with 236 additions and 30 deletions.
diff --git a/.changesets/feat_geal_authz_optout.md b/.changesets/feat_geal_authz_optout.md
@@ -0,0 +1,5 @@
+### Authorization directives are enabled by default ([Issue #3842](https://github.com/apollographql/router/issues/3842))
+
+If the router starts with an API key from an Enterprise account, and the schema contains the authorization directives, then they will be usable directly without further configuration.
+
+By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/3713
diff --git a/...r/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/...r/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
@@ -525,7 +525,7 @@ expression: "&schema"
           "properties": {
             "enabled": {
               "description": "enables the `@authenticated` and `@requiresScopes` directives",
-              "default": false,
+              "default": true,
               "type": "boolean"
             }
           }

diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs
@@ -24,7 +24,6 @@ use self::scopes::ScopeExtractionVisitor;
 use self::scopes::ScopeFilteringVisitor;
 use self::scopes::REQUIRES_SCOPES_SPEC_URL;
 use crate::error::QueryPlannerError;
-use crate::error::SchemaError;
 use crate::error::ServiceBuildError;
 use crate::graphql;
 use crate::json_ext::Path;
@@ -61,7 +60,7 @@ pub(crate) struct CacheKeyMetadata {
 }
 
 /// Authorization plugin
-#[derive(Clone, Debug, Default, Deserialize, JsonSchema)]
+#[derive(Clone, Debug, serde_derive_default::Default, Deserialize, JsonSchema)]
 #[allow(dead_code)]
 pub(crate) struct Conf {
     /// Reject unauthenticated requests
@@ -72,14 +71,18 @@ pub(crate) struct Conf {
     preview_directives: Directives,
 }
 
-#[derive(Clone, Debug, Default, Deserialize, JsonSchema)]
+#[derive(Clone, Debug, serde_derive_default::Default, Deserialize, JsonSchema)]
 #[allow(dead_code)]
 pub(crate) struct Directives {
     /// enables the `@authenticated` and `@requiresScopes` directives
-    #[serde(default)]
+    #[serde(default = "default_enable_directives")]
     enabled: bool,
 }
 
+fn default_enable_directives() -> bool {
+    true
+}
+
 pub(crate) struct AuthorizationPlugin {
     require_authentication: bool,
 }
@@ -96,20 +99,12 @@ impl AuthorizationPlugin {
             .find(|(s, _)| s.as_str() == "authorization")
             .and_then(|(_, v)| v.get("preview_directives").and_then(|v| v.as_object()))
             .and_then(|v| v.get("enabled").and_then(|v| v.as_bool()));
+
         let has_authorization_directives = schema.has_spec(AUTHENTICATED_SPEC_URL)
             || schema.has_spec(REQUIRES_SCOPES_SPEC_URL)
             || schema.has_spec(POLICY_SPEC_URL);
 
-        match has_config {
-            Some(b) => Ok(b),
-            None => {
-                if has_authorization_directives {
-                    Err(ServiceBuildError::Schema(SchemaError::Api("cannot start the router on a schema with authorization directives without configuring the authorization plugin".to_string())))
-                } else {
-                    Ok(false)
-                }
-            }
-        }
+        Ok(has_config.unwrap_or(true) && has_authorization_directives)
     }
 
     pub(crate) async fn query_analysis(

diff --git a/apollo-router/src/spec/mod.rs b/apollo-router/src/spec/mod.rs
@@ -23,6 +23,9 @@ use thiserror::Error;
 use crate::graphql::ErrorExtension;
 use crate::json_ext::Object;
 
+pub(crate) const LINK_DIRECTIVE_NAME: &str = "link";
+pub(crate) const LINK_URL_ARGUMENT: &str = "url";
+
 /// GraphQL parsing errors.
 #[derive(Error, Debug, Display, Clone, Serialize, Deserialize)]
 #[non_exhaustive]

diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs
@@ -59,6 +59,21 @@ impl Schema {
         Ok(schema)
     }
 
+    pub(crate) fn make_compiler(sdl: &str) -> Result<apollo_compiler::schema::Schema, SchemaError> {
+        let mut parser = apollo_compiler::Parser::new();
+        let ast = parser.parse_ast(sdl, "schema.graphql");
+
+        // Trace log recursion limit data
+        let recursion_limit = parser.recursion_reached();
+        tracing::trace!(?recursion_limit, "recursion limit data");
+
+        ast.check_parse_errors()
+            .map_err(|errors| SchemaError::Parse(ParseErrors { errors }))?;
+
+        let definitions = ast.to_schema();
+        Ok(definitions)
+    }
+
     pub(crate) fn parse(sdl: &str, configuration: &Configuration) -> Result<Self, SchemaError> {
         let start = Instant::now();
         let mut parser = apollo_compiler::Parser::new();

diff --git a/apollo-router/src/state_machine.rs b/apollo-router/src/state_machine.rs
@@ -35,6 +35,7 @@ use crate::configuration::ListenAddr;
 use crate::router::Event::UpdateLicense;
 use crate::router_factory::RouterFactory;
 use crate::router_factory::RouterSuperServiceFactory;
+use crate::spec::Schema;
 use crate::uplink::license_enforcement::LicenseEnforcementReport;
 use crate::uplink::license_enforcement::LicenseState;
 use crate::uplink::license_enforcement::LICENSE_EXPIRED_URL;
@@ -316,8 +317,12 @@ impl<FA: RouterSuperServiceFactory> State<FA> {
         S: HttpServerFactory,
         FA: RouterSuperServiceFactory,
     {
-        // Check the license
-        let report = LicenseEnforcementReport::build(&configuration);
+        let report = {
+            let parsed_schema = Schema::make_compiler(&schema)
+                .map_err(|e| ServiceCreationError(e.to_string().into()))?;
+            // Check the license
+            LicenseEnforcementReport::build(&configuration, &parsed_schema)
+        };
 
         match license {
             LicenseState::Licensed => {

diff --git a/apollo-router/src/uplink/license_enforcement.rs b/apollo-router/src/uplink/license_enforcement.rs
@@ -3,6 +3,7 @@
 // Read more: https://github.com/hyperium/tonic/issues/1056
 #![allow(clippy::derive_partial_eq_without_eq)]
 
+use std::collections::HashSet;
 use std::fmt::Display;
 use std::fmt::Formatter;
 use std::str::FromStr;
@@ -25,6 +26,8 @@ use serde::Serialize;
 use serde_json::Value;
 use thiserror::Error;
 
+use crate::spec::LINK_DIRECTIVE_NAME;
+use crate::spec::LINK_URL_ARGUMENT;
 use crate::Configuration;
 
 pub(crate) const LICENSE_EXPIRED_URL: &str = "https://go.apollo.dev/o/elp";
@@ -75,19 +78,24 @@ where
 #[derive(Debug)]
 pub(crate) struct LicenseEnforcementReport {
     restricted_config_in_use: Vec<ConfigurationRestriction>,
+    restricted_schema_in_use: Vec<SchemaRestriction>,
 }
 
 impl LicenseEnforcementReport {
     pub(crate) fn uses_restricted_features(&self) -> bool {
-        !self.restricted_config_in_use.is_empty()
+        !self.restricted_config_in_use.is_empty() || !self.restricted_schema_in_use.is_empty()
     }
 
-    pub(crate) fn build(configuration: &Configuration) -> LicenseEnforcementReport {
+    pub(crate) fn build(
+        configuration: &Configuration,
+        schema: &apollo_compiler::schema::Schema,
+    ) -> LicenseEnforcementReport {
         LicenseEnforcementReport {
             restricted_config_in_use: Self::validate_configuration(
                 configuration,
                 &Self::configuration_restrictions(),
             ),
+            restricted_schema_in_use: Self::validate_schema(schema, &Self::schema_restrictions()),
         }
     }
 
@@ -119,6 +127,32 @@ impl LicenseEnforcementReport {
         configuration_violations
     }
 
+    fn validate_schema(
+        schema: &apollo_compiler::schema::Schema,
+        schema_restrictions: &Vec<SchemaRestriction>,
+    ) -> Vec<SchemaRestriction> {
+        let feature_urls = schema
+            .schema_definition
+            .directives
+            .iter()
+            .filter(|dir| dir.name.as_str() == LINK_DIRECTIVE_NAME)
+            .filter_map(|link| {
+                link.argument_by_name(LINK_URL_ARGUMENT)
+                    .and_then(|value| value.as_str().map(|s| s.to_string()))
+            })
+            .collect::<HashSet<_>>();
+
+        let mut schema_violations = Vec::new();
+
+        for restriction in schema_restrictions {
+            if feature_urls.contains(&restriction.url) {
+                schema_violations.push(restriction.clone());
+            }
+        }
+
+        schema_violations
+    }
+
     fn configuration_restrictions() -> Vec<ConfigurationRestriction> {
         vec![
             ConfigurationRestriction::builder()
@@ -183,17 +217,47 @@ impl LicenseEnforcementReport {
                 .build(),
         ]
     }
+
+    fn schema_restrictions() -> Vec<SchemaRestriction> {
+        vec![
+            SchemaRestriction::builder()
+                .name("@authenticated")
+                .url("https://specs.apollo.dev/authenticated/v0.1")
+                .build(),
+            SchemaRestriction::builder()
+                .name("@requiresScopes")
+                .url("https://specs.apollo.dev/requiresScopes/v0.1")
+                .build(),
+        ]
+    }
 }
 
 impl Display for LicenseEnforcementReport {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        let restricted_config = self
-            .restricted_config_in_use
-            .iter()
-            .map(|v| format!("* {}\n  {}", v.name, v.path.replace("$.", ".")))
-            .join("\n\n");
+        if !self.restricted_config_in_use.is_empty() {
+            let restricted_config = self
+                .restricted_config_in_use
+                .iter()
+                .map(|v| format!("* {}\n  {}", v.name, v.path.replace("$.", ".")))
+                .join("\n\n");
+            write!(f, "Configuration yaml:\n{restricted_config}")?;
+
+            if !self.restricted_schema_in_use.is_empty() {
+                writeln!(f)?;
+            }
+        }
 
-        write!(f, "Configuration yaml:\n{restricted_config}")
+        if !self.restricted_schema_in_use.is_empty() {
+            let restricted_schema = self
+                .restricted_schema_in_use
+                .iter()
+                .map(|v| format!("* {}\n  {}", v.name, v.url))
+                .join("\n\n");
+
+            write!(f, "Schema features:\n{restricted_schema}")?
+        }
+
+        Ok(())
     }
 }
 
@@ -277,6 +341,13 @@ pub(crate) struct ConfigurationRestriction {
     value: Option<Value>,
 }
 
+/// An individual check for the supergraph schema
+#[derive(Builder, Clone, Debug, Serialize, Deserialize)]
+pub(crate) struct SchemaRestriction {
+    name: String,
+    url: String,
+}
+
 impl License {
     pub(crate) fn jwks() -> &'static JwkSet {
         JWKS.get_or_init(|| {
@@ -297,22 +368,27 @@ mod test {
     use insta::assert_snapshot;
     use serde_json::json;
 
+    use crate::spec::Schema;
     use crate::uplink::license_enforcement::Audience;
     use crate::uplink::license_enforcement::Claims;
     use crate::uplink::license_enforcement::License;
     use crate::uplink::license_enforcement::LicenseEnforcementReport;
     use crate::uplink::license_enforcement::OneOrMany;
     use crate::Configuration;
 
-    fn check(router_yaml: &str) -> LicenseEnforcementReport {
+    fn check(router_yaml: &str, supergraph_schema: &str) -> LicenseEnforcementReport {
         let config = Configuration::from_str(router_yaml).expect("router config must be valid");
-
-        LicenseEnforcementReport::build(&config)
+        let schema =
+            Schema::make_compiler(supergraph_schema).expect("supergraph schema must be valid");
+        LicenseEnforcementReport::build(&config, &schema)
     }
 
     #[test]
     fn test_oss() {
-        let report = check(include_str!("testdata/oss.router.yaml"));
+        let report = check(
+            include_str!("testdata/oss.router.yaml"),
+            include_str!("testdata/oss.graphql"),
+        );
 
         assert!(
             report.restricted_config_in_use.is_empty(),
@@ -322,7 +398,10 @@ mod test {
 
     #[test]
     fn test_restricted_features_via_config() {
-        let report = check(include_str!("testdata/restricted.router.yaml"));
+        let report = check(
+            include_str!("testdata/restricted.router.yaml"),
+            include_str!("testdata/oss.graphql"),
+        );
 
         assert!(
             !report.restricted_config_in_use.is_empty(),
@@ -331,6 +410,20 @@ mod test {
         assert_snapshot!(report.to_string());
     }
 
+    #[test]
+    fn test_restricted_authorization_directives_via_schema() {
+        let report = check(
+            include_str!("testdata/oss.router.yaml"),
+            include_str!("testdata/authorization.graphql"),
+        );
+
+        assert!(
+            !report.restricted_schema_in_use.is_empty(),
+            "should have found restricted features"
+        );
+        assert_snapshot!(report.to_string());
+    }
+
     #[test]
     fn test_license_parse() {
         let license = License::from_str("eyJhbGciOiJFZERTQSJ9.eyJpc3MiOiJodHRwczovL3d3dy5hcG9sbG9ncmFwaHFsLmNvbS8iLCJzdWIiOiJhcG9sbG8iLCJhdWQiOiJTRUxGX0hPU1RFRCIsIndhcm5BdCI6MTY3NjgwODAwMCwiaGFsdEF0IjoxNjc4MDE3NjAwfQ.tXexfjZ2SQeqSwkWQ7zD4XBoxS_Hc5x7tSNJ3ln-BCL_GH7i3U9hsIgdRQTczCAjA_jjk34w39DeSV0nTc5WBw").expect("must be able to decode JWT");

diff --git a/...r__uplink__license_enforcement__test__restricted_authorization_directives_via_schema.snap b/...r__uplink__license_enforcement__test__restricted_authorization_directives_via_schema.snap
@@ -0,0 +1,10 @@
+---
+source: apollo-router/src/uplink/license_enforcement.rs
+expression: report.to_string()
+---
+Schema features:
+* @authenticated
+  https://specs.apollo.dev/authenticated/v0.1
+
+* @requiresScopes
+  https://specs.apollo.dev/requiresScopes/v0.1