From 17516b9dc4152d8b5866bf52dee827f00b6416f6 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Aug 2023 13:39:52 +0200 Subject: [PATCH 01/21] enable authorization by default --- .../src/plugins/authorization/mod.rs | 6 +- apollo-router/src/spec/schema.rs | 19 +++ apollo-router/src/state_machine.rs | 9 +- .../src/uplink/license_enforcement.rs | 118 ++++++++++++++++-- 4 files changed, 136 insertions(+), 16 deletions(-) diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 20bfe91db4..d0c62d986f 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -80,10 +80,14 @@ pub(crate) struct Conf { #[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, } diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs index b34b180f37..1b938f0892 100644 --- a/apollo-router/src/spec/schema.rs +++ b/apollo-router/src/spec/schema.rs @@ -59,6 +59,25 @@ impl Schema { Ok(schema) } + pub(crate) fn make_compiler(sdl: &str) -> Result { + let mut compiler = ApolloCompiler::new(); + let id = compiler.add_type_system(sdl, "schema.graphql"); + + let ast = compiler.db.ast(id); + + // Trace log recursion limit data + let recursion_limit = ast.recursion_limit(); + tracing::trace!(?recursion_limit, "recursion limit data"); + + let mut parse_errors = ast.errors().peekable(); + if parse_errors.peek().is_some() { + let errors = parse_errors.cloned().collect::>(); + return Err(SchemaError::Parse(ParseErrors { errors })); + } + + Ok(compiler) + } + pub(crate) fn parse(sdl: &str, configuration: &Configuration) -> Result { let mut compiler = ApolloCompiler::new(); let id = compiler.add_type_system(sdl, "schema.graphql"); diff --git a/apollo-router/src/state_machine.rs b/apollo-router/src/state_machine.rs index 688cc16d83..a9ee846c6c 100644 --- 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 State { 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 index ab2ef2093f..07e45bcf94 100644 --- 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; @@ -10,6 +11,8 @@ use std::time::Duration; use std::time::SystemTime; use std::time::UNIX_EPOCH; +use apollo_compiler::ApolloCompiler; +use apollo_compiler::HirDatabase; use buildstructor::Builder; use displaydoc::Display; use itertools::Itertools; @@ -75,19 +78,24 @@ where #[derive(Debug)] pub(crate) struct LicenseEnforcementReport { restricted_config_in_use: Vec, + restricted_schema_in_use: Vec, } 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: &ApolloCompiler, + ) -> 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,31 @@ impl LicenseEnforcementReport { configuration_violations } + fn validate_schema( + schema: &ApolloCompiler, + schema_restrictions: &Vec, + ) -> Vec { + let feature_urls = schema + .db + .schema() + .directives_by_name("link") + .filter_map(|link| { + link.argument_by_name("url") + .and_then(|value| value.as_str().map(|s| s.to_string())) + }) + .collect::>(); + + 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 { vec![ ConfigurationRestriction::builder() @@ -183,17 +216,47 @@ impl LicenseEnforcementReport { .build(), ] } + + fn schema_restrictions() -> Vec { + 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() { + write!(f, "\n")?; + } + } + + 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, "Configuration yaml:\n{restricted_config}") + write!(f, "Schema features:\n{restricted_schema}")? + } + + Ok(()) } } @@ -277,6 +340,13 @@ pub(crate) struct ConfigurationRestriction { value: Option, } +/// 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,6 +367,7 @@ 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; @@ -304,15 +375,19 @@ mod test { 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 +397,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 +409,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"); From 13d63d0bf04d7e8b6529f440919e8167e61eada1 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Aug 2023 14:51:54 +0200 Subject: [PATCH 02/21] missing files --- ...nfiguration__tests__schema_generation.snap | 2 +- .../src/uplink/testdata/authorization.graphql | 76 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 apollo-router/src/uplink/testdata/authorization.graphql 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 e54c9226cd..34939e65af 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 @@ -519,7 +519,7 @@ expression: "&schema" "properties": { "enabled": { "description": "enables the `@authenticated` and `@requiresScopes` directives", - "default": false, + "default": true, "type": "boolean" } } diff --git a/apollo-router/src/uplink/testdata/authorization.graphql b/apollo-router/src/uplink/testdata/authorization.graphql new file mode 100644 index 0000000000..378b5f0572 --- /dev/null +++ b/apollo-router/src/uplink/testdata/authorization.graphql @@ -0,0 +1,76 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) +@link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) +{ + query: Query +} + +directive @authenticated on FIELD_DEFINITION | OBJECT | INTERFACE | SCALAR | ENUM + +scalar federation__Scope +directive @requiresScopes(scopes: [[federation__Scope!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + +scalar join__FieldSet + +enum join__Graph { + PRODUCTS @join__graph(name: "products", url: "http://localhost:4001/") + REVIEWS @join__graph(name: "reviews", url: "http://localhost:4002/") +} + +scalar link__Import + +enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +type Product + @join__type(graph: PRODUCTS, key: "upc") + @join__type(graph: REVIEWS, key: "upc") + @authenticated +{ + upc: String! + name: String! @join__field(graph: PRODUCTS) + price: String! @join__field(graph: PRODUCTS) @authenticated + related: Product! @join__field(graph: PRODUCTS) + price1: String @join__field(graph: PRODUCTS) + review: Review! @join__field(graph: REVIEWS) @requiresScopes(scopes: [["review"]]) +} + +type Query + @join__type(graph: PRODUCTS) + @join__type(graph: REVIEWS) +{ + products(limit: Int!, size: Int, random: String): [Product!]! @join__field(graph: PRODUCTS) +} + +type Review + @join__type(graph: REVIEWS) +{ + body: String! + product: Product! + body1: String +} From 67f3ca33c2e40cea58840c37895fb64459497935 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Aug 2023 14:52:41 +0200 Subject: [PATCH 03/21] extract the authenticated directive name if it changed --- .../plugins/authorization/authenticated.rs | 201 ++++++++++++++++-- .../src/plugins/authorization/mod.rs | 42 ++-- ...thenticated__tests__renamed_directive.snap | 24 +++ ...d_authorization_directives_via_schema.snap | 10 + 4 files changed, 237 insertions(+), 40 deletions(-) create mode 100644 apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap create mode 100644 apollo-router/src/uplink/snapshots/apollo_router__uplink__license_enforcement__test__restricted_authorization_directives_via_schema.snap diff --git a/apollo-router/src/plugins/authorization/authenticated.rs b/apollo-router/src/plugins/authorization/authenticated.rs index e487eee2fb..6c463d77db 100644 --- a/apollo-router/src/plugins/authorization/authenticated.rs +++ b/apollo-router/src/plugins/authorization/authenticated.rs @@ -20,20 +20,40 @@ pub(crate) struct AuthenticatedCheckVisitor<'a> { compiler: &'a ApolloCompiler, file_id: FileId, pub(crate) found: bool, + authenticated_directive_name: String, } impl<'a> AuthenticatedCheckVisitor<'a> { - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { - Self { + pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Option { + let authenticated_directive_name = if let Some(link) = compiler + .db + .schema() + .directives_by_name("link") + .filter(|link| { + link.argument_by_name("url") + .and_then(|value| value.as_str()) + == Some("https://specs.apollo.dev/authenticated/v0.1") + }) + .next() + { + link.argument_by_name("as") + .and_then(|value| value.as_str().map(|s| s.to_string())) + .unwrap_or_else(|| AUTHENTICATED_DIRECTIVE_NAME.to_string()) + } else { + return None; + }; + + Some(Self { compiler, file_id, found: false, - } + authenticated_directive_name, + }) } fn is_field_authenticated(&self, field: &FieldDefinition) -> bool { field - .directive_by_name(AUTHENTICATED_DIRECTIVE_NAME) + .directive_by_name(&self.authenticated_directive_name) .is_some() || field .ty() @@ -43,7 +63,8 @@ impl<'a> AuthenticatedCheckVisitor<'a> { } fn is_type_authenticated(&self, t: &TypeDefinition) -> bool { - t.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some() + t.directive_by_name(&self.authenticated_directive_name) + .is_some() } } @@ -135,22 +156,44 @@ pub(crate) struct AuthenticatedVisitor<'a> { pub(crate) query_requires_authentication: bool, pub(crate) unauthorized_paths: Vec, current_path: Path, + authenticated_directive_name: String, } impl<'a> AuthenticatedVisitor<'a> { - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { - Self { + pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Option { + let authenticated_directive_name = if let Some(link) = compiler + .db + .schema() + .directives_by_name("link") + .filter(|link| { + link.argument_by_name("url") + .and_then(|value| value.as_str()) + == Some("https://specs.apollo.dev/authenticated/v0.1") + }) + .next() + { + link.argument_by_name("as") + .and_then(|value| value.as_str().map(|s| s.to_string())) + .unwrap_or_else(|| AUTHENTICATED_DIRECTIVE_NAME.to_string()) + } else { + return None; + }; + + println!("AuthenticatedVisitor: auth directive name = {authenticated_directive_name}"); + + Some(Self { compiler, file_id, query_requires_authentication: false, unauthorized_paths: Vec::new(), current_path: Path::default(), - } + authenticated_directive_name, + }) } fn is_field_authenticated(&self, field: &FieldDefinition) -> bool { field - .directive_by_name(AUTHENTICATED_DIRECTIVE_NAME) + .directive_by_name(&self.authenticated_directive_name) .is_some() || field .ty() @@ -160,7 +203,8 @@ impl<'a> AuthenticatedVisitor<'a> { } fn is_type_authenticated(&self, t: &TypeDefinition) -> bool { - t.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some() + t.directive_by_name(&self.authenticated_directive_name) + .is_some() } fn implementors_with_different_requirements( @@ -198,8 +242,9 @@ impl<'a> AuthenticatedVisitor<'a> { .cloned() .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) { - let ty_is_authenticated = - ty.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some(); + let ty_is_authenticated = ty + .directive_by_name(&self.authenticated_directive_name) + .is_some(); match is_authenticated { None => is_authenticated = Some(ty_is_authenticated), Some(other_ty_is_authenticated) => { @@ -238,8 +283,9 @@ impl<'a> AuthenticatedVisitor<'a> { .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) { if let Some(f) = ty.field(&self.compiler.db, field.name()) { - let field_is_authenticated = - f.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some(); + let field_is_authenticated = f + .directive_by_name(&self.authenticated_directive_name) + .is_some(); match is_authenticated { Some(other) => { if field_is_authenticated != other { @@ -269,7 +315,10 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { ) -> Result, BoxError> { let operation_requires_authentication = node .object_type(&self.compiler.db) - .map(|ty| ty.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some()) + .map(|ty| { + ty.directive_by_name(&self.authenticated_directive_name) + .is_some() + }) .unwrap_or(false); if operation_requires_authentication { @@ -424,6 +473,7 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { #[cfg(test)] mod tests { + use apollo_compiler::ApolloCompiler; use multimap::MultiMap; use serde_json_bytes::json; @@ -442,6 +492,16 @@ mod tests { use crate::TestHarness; static BASIC_SCHEMA: &str = r#" + + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) + { + query: Query + mutation: Mutation + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -498,7 +558,7 @@ mod tests { } assert!(diagnostics.is_empty()); - let mut visitor = AuthenticatedVisitor::new(&compiler, file_id); + let mut visitor = AuthenticatedVisitor::new(&compiler, file_id).unwrap(); ( transform::document(&mut visitor, file_id).unwrap(), @@ -734,6 +794,14 @@ mod tests { } static INTERFACE_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) + { + query: Query + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -801,6 +869,14 @@ mod tests { } static INTERFACE_FIELD_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) + { + query: Query + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -876,6 +952,14 @@ mod tests { #[test] fn union() { static UNION_MEMBERS_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) + { + query: Query + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -918,19 +1002,94 @@ mod tests { }); } + static RENAMED_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", as: "auth", for: SECURITY) + { + query: Query + mutation: Mutation + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + directive @auth on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD + + type Query { + topProducts: Product + customer: User + me: User @auth + itf: I! + } + + type Mutation @auth { + ping: User + other: String + } + + interface I { + id: ID + } + + type Product { + type: String + price(setPrice: Int): Int + reviews: [Review] @auth + internal: Internal + publicReviews: [Review] + nonNullId: ID! @auth + } + + scalar Internal @auth @specifiedBy(url: "http///example.com/test") + + type Review { + body: String + author: User + } + + type User + implements I + @auth { + id: ID + name: String + } + "#; + + #[test] + fn renamed_directive() { + static QUERY: &str = r#" + query { + topProducts { + type + } + + me { + name + } + } + "#; + + let (doc, paths) = filter(RENAMED_SCHEMA, QUERY); + + insta::assert_display_snapshot!(TestResult { + query: QUERY, + result: doc, + paths + }); + } + const SCHEMA: &str = r#"schema - @core(feature: "https://specs.apollo.dev/core/v0.1") - @core(feature: "https://specs.apollo.dev/join/v0.1") - @core(feature: "https://specs.apollo.dev/inaccessible/v0.1") + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) { query: Query } - directive @core(feature: String!) repeatable on SCHEMA + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE directive @join__graph(name: String!, url: String!) on ENUM_VALUE - directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION scalar join__FieldSet enum join__Graph { USER @join__graph(name: "user", url: "http://localhost:4001/graphql") diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index d0c62d986f..351c105ede 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -140,13 +140,13 @@ impl AuthorizationPlugin { ) { let (compiler, file_id) = Query::make_compiler(query, schema, configuration); - let mut visitor = AuthenticatedCheckVisitor::new(&compiler, file_id); - - // if this fails, the query is invalid and will fail at the query planning phase. - // We do not return validation errors here for now because that would imply a huge - // refactoring of telemetry and tests - if traverse::document(&mut visitor, file_id).is_ok() && !visitor.found { - context.insert(AUTHENTICATED_KEY, true).unwrap(); + if let Some(mut visitor) = AuthenticatedCheckVisitor::new(&compiler, file_id) { + // if this fails, the query is invalid and will fail at the query planning phase. + // We do not return validation errors here for now because that would imply a huge + // refactoring of telemetry and tests + if traverse::document(&mut visitor, file_id).is_ok() && !visitor.found { + context.insert(AUTHENTICATED_KEY, true).unwrap(); + } } let mut visitor = ScopeExtractionVisitor::new(&compiler, file_id); @@ -342,26 +342,30 @@ impl AuthorizationPlugin { .pop() .expect("the query was added to the compiler earlier"); - let mut visitor = AuthenticatedVisitor::new(compiler, id); - let modified_query = transform::document(&mut visitor, id) - .map_err(|e| SpecError::ParsingError(e.to_string()))? - .to_string(); + if let Some(mut visitor) = AuthenticatedVisitor::new(compiler, id) { + let modified_query = transform::document(&mut visitor, id) + .map_err(|e| SpecError::ParsingError(e.to_string()))? + .to_string(); - if visitor.query_requires_authentication { - if is_authenticated { - tracing::debug!("the query contains @authenticated, the request is authenticated, keeping the query"); - Ok(None) - } else { - tracing::debug!("the query contains @authenticated, modified query:\n{modified_query}\nunauthorized paths: {:?}", visitor + if visitor.query_requires_authentication { + if is_authenticated { + tracing::debug!("the query contains @authenticated, the request is authenticated, keeping the query"); + Ok(None) + } else { + tracing::debug!("the query contains @authenticated, modified query:\n{modified_query}\nunauthorized paths: {:?}", visitor .unauthorized_paths .iter() .map(|path| path.to_string()) .collect::>()); - Ok(Some((modified_query, visitor.unauthorized_paths))) + Ok(Some((modified_query, visitor.unauthorized_paths))) + } + } else { + tracing::debug!("the query does not contain @authenticated"); + Ok(None) } } else { - tracing::debug!("the query does not contain @authenticated"); + tracing::debug!("the schema does not contain @authenticated"); Ok(None) } } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap new file mode 100644 index 0000000000..d4691a6c97 --- /dev/null +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap @@ -0,0 +1,24 @@ +--- +source: apollo-router/src/plugins/authorization/authenticated.rs +expression: "TestResult { query: QUERY, result: doc, paths }" +--- +query: + + query { + topProducts { + type + } + + me { + name + } + } + +filtered: +query { + topProducts { + type + } +} + +paths: ["/me"] diff --git a/apollo-router/src/uplink/snapshots/apollo_router__uplink__license_enforcement__test__restricted_authorization_directives_via_schema.snap b/apollo-router/src/uplink/snapshots/apollo_router__uplink__license_enforcement__test__restricted_authorization_directives_via_schema.snap new file mode 100644 index 0000000000..60a9f08ef0 --- /dev/null +++ b/apollo-router/src/uplink/snapshots/apollo_router__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 From dacae5fde064fa080413f9778b7b3dafc22af045 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Aug 2023 15:48:01 +0200 Subject: [PATCH 04/21] fix tests for the authenticated directive --- .../plugins/authorization/authenticated.rs | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/apollo-router/src/plugins/authorization/authenticated.rs b/apollo-router/src/plugins/authorization/authenticated.rs index 6c463d77db..febf032100 100644 --- a/apollo-router/src/plugins/authorization/authenticated.rs +++ b/apollo-router/src/plugins/authorization/authenticated.rs @@ -1086,10 +1086,28 @@ mod tests { query: Query } directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION - directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE - directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE + directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION directive @join__graph(name: String!, url: String!) on ENUM_VALUE + directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + + scalar link__Import + + enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION + } + + scalar join__FieldSet enum join__Graph { USER @join__graph(name: "user", url: "http://localhost:4001/graphql") @@ -1098,12 +1116,14 @@ mod tests { directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM - type Query { + type Query + @join__type(graph: ORGA) + @join__type(graph: USER) + { currentUser: User @join__field(graph: USER) @authenticated orga(id: ID): Organization @join__field(graph: ORGA) } type User - @join__owner(graph: USER) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id"){ id: ID! @@ -1112,7 +1132,6 @@ mod tests { activeOrganization: Organization } type Organization - @join__owner(graph: ORGA) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") { id: ID @@ -1148,6 +1167,9 @@ mod tests { ("orga", MockSubgraph::builder().with_json( serde_json::json!{{"query":"{orga(id:1){id creatorUser{__typename id}}}"}}, serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": { "__typename": "User", "id": 0 } }}}} + ).with_json( + serde_json::json!{{"query":"{orga(id:1){id creatorUser{id name phone}}}"}}, + serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": { "__typename": "User", "id": 0, "name":"Ada", "phone": "1234" } }}}} ).build()) ].into_iter().collect()); @@ -1219,10 +1241,16 @@ mod tests { ] } }}, + ).with_json( + serde_json::json!{{"query":"{orga(id:1){id creatorUser{id name}}}"}}, + serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": {"id": 0, "name":"Ada" } }}}} ).build()), ("orga", MockSubgraph::builder().with_json( serde_json::json!{{"query":"{orga(id:1){id creatorUser{__typename id}}}"}}, serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": { "__typename": "User", "id": 0 } }}}} + ).with_json( + serde_json::json!{{"query":"{orga(id:1){id creatorUser{id name}}}"}}, + serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": {"id": 0, "name":"Ada" } }}}} ).build()) ].into_iter().collect()); From 93e4ce09e5b63d2ad3a504b8c9d83b970b5881b2 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Aug 2023 16:29:37 +0200 Subject: [PATCH 05/21] support renaming the requiresScopes directive --- .../src/plugins/authorization/mod.rs | 42 ++-- .../src/plugins/authorization/scopes.rs | 186 ++++++++++++++++-- 2 files changed, 192 insertions(+), 36 deletions(-) diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 351c105ede..446c36416d 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -149,16 +149,16 @@ impl AuthorizationPlugin { } } - let mut visitor = ScopeExtractionVisitor::new(&compiler, file_id); - - // if this fails, the query is invalid and will fail at the query planning phase. - // We do not return validation errors here for now because that would imply a huge - // refactoring of telemetry and tests - if traverse::document(&mut visitor, file_id).is_ok() { - let scopes: Vec = visitor.extracted_scopes.into_iter().collect(); + if let Some(mut visitor) = ScopeExtractionVisitor::new(&compiler, file_id) { + // if this fails, the query is invalid and will fail at the query planning phase. + // We do not return validation errors here for now because that would imply a huge + // refactoring of telemetry and tests + if traverse::document(&mut visitor, file_id).is_ok() { + let scopes: Vec = visitor.extracted_scopes.into_iter().collect(); - if !scopes.is_empty() { - context.insert(REQUIRED_SCOPES_KEY, scopes).unwrap(); + if !scopes.is_empty() { + context.insert(REQUIRED_SCOPES_KEY, scopes).unwrap(); + } } } @@ -380,24 +380,28 @@ impl AuthorizationPlugin { .pop() .expect("the query was added to the compiler earlier"); - let mut visitor = - ScopeFilteringVisitor::new(compiler, id, scopes.iter().cloned().collect()); - - let modified_query = transform::document(&mut visitor, id) - .map_err(|e| SpecError::ParsingError(e.to_string()))? - .to_string(); + if let Some(mut visitor) = + ScopeFilteringVisitor::new(compiler, id, scopes.iter().cloned().collect()) + { + let modified_query = transform::document(&mut visitor, id) + .map_err(|e| SpecError::ParsingError(e.to_string()))? + .to_string(); - if visitor.query_requires_scopes { - tracing::debug!("the query required scopes, the requests present scopes: {scopes:?}, modified query:\n{modified_query}\nunauthorized paths: {:?}", + if visitor.query_requires_scopes { + tracing::debug!("the query required scopes, the requests present scopes: {scopes:?}, modified query:\n{modified_query}\nunauthorized paths: {:?}", visitor .unauthorized_paths .iter() .map(|path| path.to_string()) .collect::>() ); - Ok(Some((modified_query, visitor.unauthorized_paths))) + Ok(Some((modified_query, visitor.unauthorized_paths))) + } else { + tracing::debug!("the query does not require scopes"); + Ok(None) + } } else { - tracing::debug!("the query does not require scopes"); + tracing::debug!("the schema does not contain @requiresScopes"); Ok(None) } } diff --git a/apollo-router/src/plugins/authorization/scopes.rs b/apollo-router/src/plugins/authorization/scopes.rs index 9664876828..bde39cefb8 100644 --- a/apollo-router/src/plugins/authorization/scopes.rs +++ b/apollo-router/src/plugins/authorization/scopes.rs @@ -26,23 +26,43 @@ pub(crate) struct ScopeExtractionVisitor<'a> { compiler: &'a ApolloCompiler, file_id: FileId, pub(crate) extracted_scopes: HashSet, + requires_scopes_directive_name: String, } pub(crate) const REQUIRES_SCOPES_DIRECTIVE_NAME: &str = "requiresScopes"; impl<'a> ScopeExtractionVisitor<'a> { #[allow(dead_code)] - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { - Self { + pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Option { + let requires_scopes_directive_name = if let Some(link) = compiler + .db + .schema() + .directives_by_name("link") + .filter(|link| { + link.argument_by_name("url") + .and_then(|value| value.as_str()) + == Some("https://specs.apollo.dev/requiresScopes/v0.1") + }) + .next() + { + link.argument_by_name("as") + .and_then(|value| value.as_str().map(|s| s.to_string())) + .unwrap_or_else(|| REQUIRES_SCOPES_DIRECTIVE_NAME.to_string()) + } else { + return None; + }; + + Some(Self { compiler, file_id, extracted_scopes: HashSet::new(), - } + requires_scopes_directive_name, + }) } fn scopes_from_field(&mut self, field: &FieldDefinition) { self.extracted_scopes.extend( - scopes_argument(field.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned(), + scopes_argument(field.directive_by_name(&self.requires_scopes_directive_name)).cloned(), ); if let Some(ty) = field.ty().type_def(&self.compiler.db) { @@ -51,8 +71,9 @@ impl<'a> ScopeExtractionVisitor<'a> { } fn scopes_from_type(&mut self, ty: &TypeDefinition) { - self.extracted_scopes - .extend(scopes_argument(ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned()); + self.extracted_scopes.extend( + scopes_argument(ty.directive_by_name(&self.requires_scopes_directive_name)).cloned(), + ); } } @@ -86,7 +107,8 @@ impl<'a> traverse::Visitor for ScopeExtractionVisitor<'a> { fn operation(&mut self, node: &hir::OperationDefinition) -> Result<(), BoxError> { if let Some(ty) = node.object_type(&self.compiler.db) { self.extracted_scopes.extend( - scopes_argument(ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned(), + scopes_argument(ty.directive_by_name(&self.requires_scopes_directive_name)) + .cloned(), ); } @@ -191,6 +213,7 @@ pub(crate) struct ScopeFilteringVisitor<'a> { pub(crate) query_requires_scopes: bool, pub(crate) unauthorized_paths: Vec, current_path: Path, + requires_scopes_directive_name: String, } impl<'a> ScopeFilteringVisitor<'a> { @@ -198,19 +221,38 @@ impl<'a> ScopeFilteringVisitor<'a> { compiler: &'a ApolloCompiler, file_id: FileId, scopes: HashSet, - ) -> Self { - Self { + ) -> Option { + let requires_scopes_directive_name = if let Some(link) = compiler + .db + .schema() + .directives_by_name("link") + .filter(|link| { + link.argument_by_name("url") + .and_then(|value| value.as_str()) + == Some("https://specs.apollo.dev/requiresScopes/v0.1") + }) + .next() + { + link.argument_by_name("as") + .and_then(|value| value.as_str().map(|s| s.to_string())) + .unwrap_or_else(|| REQUIRES_SCOPES_DIRECTIVE_NAME.to_string()) + } else { + return None; + }; + + Some(Self { compiler, file_id, request_scopes: scopes, query_requires_scopes: false, unauthorized_paths: vec![], current_path: Path::default(), - } + requires_scopes_directive_name, + }) } fn is_field_authorized(&mut self, field: &FieldDefinition) -> bool { - if let Some(directive) = field.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { + if let Some(directive) = field.directive_by_name(&self.requires_scopes_directive_name) { let mut field_scopes_sets = scopes_sets_argument(directive); // The outer array acts like a logical OR: if any of the inner arrays of scopes matches, the field @@ -234,7 +276,7 @@ impl<'a> ScopeFilteringVisitor<'a> { } fn is_type_authorized(&self, ty: &TypeDefinition) -> bool { - match ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { + match ty.directive_by_name(&self.requires_scopes_directive_name) { None => true, Some(directive) => { let mut type_scopes_sets = scopes_sets_argument(directive); @@ -292,7 +334,7 @@ impl<'a> ScopeFilteringVisitor<'a> { // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let ty_scope_sets = ty - .directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) + .directive_by_name(&self.requires_scopes_directive_name) .map(|directive| { let mut v = scopes_sets_argument(directive) .map(|h| { @@ -348,7 +390,7 @@ impl<'a> ScopeFilteringVisitor<'a> { // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let field_scope_sets = f - .directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) + .directive_by_name(&self.requires_scopes_directive_name) .map(|directive| { let mut v = scopes_sets_argument(directive) .map(|h| { @@ -389,7 +431,7 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { node: &hir::OperationDefinition, ) -> Result, BoxError> { let is_authorized = if let Some(ty) = node.object_type(&self.compiler.db) { - match ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { + match ty.directive_by_name(&self.requires_scopes_directive_name) { None => true, Some(directive) => { let mut type_scopes_sets = scopes_sets_argument(directive); @@ -583,6 +625,15 @@ mod tests { use crate::spec::query::traverse; static BASIC_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) + { + query: Query + mutation: Mutation + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA scalar federation__Scope directive @requiresScopes(scopes: [[federation__Scope!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM @@ -639,7 +690,7 @@ mod tests { } assert!(diagnostics.is_empty()); - let mut visitor = ScopeExtractionVisitor::new(&compiler, id); + let mut visitor = ScopeExtractionVisitor::new(&compiler, id).unwrap(); traverse::document(&mut visitor, id).unwrap(); visitor.extracted_scopes.into_iter().collect() @@ -682,7 +733,7 @@ mod tests { } assert!(diagnostics.is_empty()); - let mut visitor = ScopeFilteringVisitor::new(&compiler, file_id, scopes); + let mut visitor = ScopeFilteringVisitor::new(&compiler, file_id, scopes).unwrap(); ( transform::document(&mut visitor, file_id).unwrap(), visitor.unauthorized_paths, @@ -1067,6 +1118,14 @@ mod tests { } static INTERFACE_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) + { + query: Query + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD type Query { @@ -1181,6 +1240,14 @@ mod tests { } static INTERFACE_FIELD_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) + { + query: Query + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD type Query { @@ -1259,6 +1326,14 @@ mod tests { #[test] fn union() { static UNION_MEMBERS_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) + { + query: Query + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -1305,4 +1380,81 @@ mod tests { paths }); } + + static RENAMED_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", as: "scopes" for: SECURITY) + { + query: Query + mutation: Mutation + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + scalar federation__Scope + directive @scopes(scopes: [[federation__Scope!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + + type Query { + topProducts: Product + customer: User + me: User @scopes(scopes: [["profile"]]) + itf: I + } + + type Mutation @scopes(scopes: [["mut"]]) { + ping: User @scopes(scopes: [["ping"]]) + other: String + } + + interface I { + id: ID + } + + type Product { + type: String + price(setPrice: Int): Int + reviews: [Review] + internal: Internal + publicReviews: [Review] + } + + scalar Internal @scopes(scopes: [["internal", "test"]]) @specifiedBy(url: "http///example.com/test") + + type Review @scopes(scopes: [["review"]]) { + body: String + author: User + } + + type User implements I @scopes(scopes: [["read:user"]]) { + id: ID + name: String @scopes(scopes: [["read:username"]]) + } + "#; + + #[test] + fn renamed_directive() { + static QUERY: &str = r#" + query { + topProducts { + type + } + + me { + name + } + } + "#; + + let extracted_scopes = extract(RENAMED_SCHEMA, QUERY); + + let (doc, paths) = filter(RENAMED_SCHEMA, QUERY, HashSet::new()); + + insta::assert_display_snapshot!(TestResult { + query: QUERY, + extracted_scopes: &extracted_scopes, + scopes: Vec::new(), + result: doc, + paths + }); + } } From cc87acc3330d47609fe7a0cc07a06068b5b1ae58 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Aug 2023 16:58:24 +0200 Subject: [PATCH 06/21] make sure that alternative directives are not considered --- .../plugins/authorization/authenticated.rs | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/apollo-router/src/plugins/authorization/authenticated.rs b/apollo-router/src/plugins/authorization/authenticated.rs index febf032100..b844f68a3c 100644 --- a/apollo-router/src/plugins/authorization/authenticated.rs +++ b/apollo-router/src/plugins/authorization/authenticated.rs @@ -1078,6 +1078,78 @@ mod tests { }); } + static ALTERNATIVE_DIRECTIVE_SCHEMA: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/OtherAuthenticated/v0.1", import: ["@authenticated"]) + { + query: Query + mutation: Mutation + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD + + type Query { + topProducts: Product + customer: User + me: User @authenticated + itf: I! + } + + type Mutation @authenticated { + ping: User + other: String + } + + interface I { + id: ID + } + + type Product { + type: String + price(setPrice: Int): Int + reviews: [Review] @authenticated + internal: Internal + publicReviews: [Review] + nonNullId: ID! @authenticated + } + + scalar Internal @authenticated @specifiedBy(url: "http///example.com/test") + + type Review { + body: String + author: User + } + + type User + implements I + @authenticated { + id: ID + name: String + } + "#; + + // a directive named `@authenticated` imported from a different spec should not be considered + #[test] + #[should_panic] + fn alternative_directive() { + static QUERY: &str = r#" + query { + topProducts { + type + } + + me { + name + } + } + "#; + + let _ = filter(ALTERNATIVE_DIRECTIVE_SCHEMA, QUERY); + } + const SCHEMA: &str = r#"schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) From 487e2174a58567ddcf09c15fd8aa9027da6857ac Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 31 Aug 2023 16:58:37 +0200 Subject: [PATCH 07/21] remove initial check --- .../src/plugins/authorization/mod.rs | 34 ++----------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 446c36416d..ff2dda751e 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -19,15 +19,11 @@ use tower::ServiceExt; use self::authenticated::AuthenticatedCheckVisitor; use self::authenticated::AuthenticatedVisitor; -use self::authenticated::AUTHENTICATED_DIRECTIVE_NAME; use self::policy::PolicyExtractionVisitor; use self::policy::PolicyFilteringVisitor; -use self::policy::POLICY_DIRECTIVE_NAME; use self::scopes::ScopeExtractionVisitor; use self::scopes::ScopeFilteringVisitor; -use self::scopes::REQUIRES_SCOPES_DIRECTIVE_NAME; use crate::error::QueryPlannerError; -use crate::error::SchemaError; use crate::error::ServiceBuildError; use crate::graphql; use crate::json_ext::Path; @@ -95,7 +91,7 @@ pub(crate) struct AuthorizationPlugin { impl AuthorizationPlugin { pub(crate) fn enable_directives( configuration: &Configuration, - schema: &Schema, + _schema: &Schema, ) -> Result { let has_config = configuration .apollo_plugins @@ -104,32 +100,8 @@ 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 - .type_system - .definitions - .directives - .contains_key(AUTHENTICATED_DIRECTIVE_NAME) - || schema - .type_system - .definitions - .directives - .contains_key(REQUIRES_SCOPES_DIRECTIVE_NAME) - || schema - .type_system - .definitions - .directives - .contains_key(POLICY_DIRECTIVE_NAME); - - 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)) } pub(crate) async fn query_analysis( From f3ce2d9ee708b4c4c96fde51badfae701953cea8 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 1 Sep 2023 09:49:10 +0200 Subject: [PATCH 08/21] update test schemas --- .../src/plugins/authorization/tests.rs | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/apollo-router/src/plugins/authorization/tests.rs b/apollo-router/src/plugins/authorization/tests.rs index 3a1f455e18..6842e9ed1f 100644 --- a/apollo-router/src/plugins/authorization/tests.rs +++ b/apollo-router/src/plugins/authorization/tests.rs @@ -195,18 +195,21 @@ async fn unauthenticated_request() { } const AUTHENTICATED_SCHEMA: &str = r#"schema - @core(feature: "https://specs.apollo.dev/core/v0.1") - @core(feature: "https://specs.apollo.dev/join/v0.1") - @core(feature: "https://specs.apollo.dev/inaccessible/v0.1") - { - query: Query + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) + { + query: Query } -directive @core(feature: String!) repeatable on SCHEMA -directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION -directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE -directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION directive @join__graph(name: String!, url: String!) on ENUM_VALUE -directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION +directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR +directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + +scalar link__Import directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM @@ -215,12 +218,11 @@ enum join__Graph { USER @join__graph(name: "user", url: "http://localhost:4001/graphql") ORGA @join__graph(name: "orga", url: "http://localhost:4002/graphql") } -type Query { +type Query{ currentUser: User @join__field(graph: USER) orga(id: ID): Organization @join__field(graph: ORGA) } type User -@join__owner(graph: USER) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id"){ id: ID! @@ -229,7 +231,6 @@ type User activeOrganization: Organization } type Organization -@join__owner(graph: ORGA) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") { id: ID @authenticated @@ -345,22 +346,27 @@ async fn authenticated_directive() { .unwrap() .unwrap(); + println!("req2"); + insta::assert_json_snapshot!(response); } const SCOPES_SCHEMA: &str = r#"schema - @core(feature: "https://specs.apollo.dev/core/v0.1") - @core(feature: "https://specs.apollo.dev/join/v0.1") - @core(feature: "https://specs.apollo.dev/inaccessible/v0.1") - { + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) + { query: Query } -directive @core(feature: String!) repeatable on SCHEMA -directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION -directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE -directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION directive @join__graph(name: String!, url: String!) on ENUM_VALUE -directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION +directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR +directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + +scalar link__Import directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM @@ -369,12 +375,13 @@ enum join__Graph { USER @join__graph(name: "user", url: "http://localhost:4001/graphql") ORGA @join__graph(name: "orga", url: "http://localhost:4002/graphql") } -type Query { +type Query +@join__type(graph: ORGA) +@join__type(graph: USER){ currentUser: User @join__field(graph: USER) orga(id: ID): Organization @join__field(graph: ORGA) } type User -@join__owner(graph: USER) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") @requiresScopes(scopes: [["user:read"], ["admin"]]) { @@ -384,7 +391,6 @@ type User activeOrganization: Organization } type Organization -@join__owner(graph: ORGA) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") { id: ID From b4f731ddf48688be7952581694fb56f426a95c9a Mon Sep 17 00:00:00 2001 From: Maria Elisabeth Schreiber Date: Wed, 6 Sep 2023 13:29:30 -0600 Subject: [PATCH 09/21] Add authorization directives support to configuration overview --- docs/source/configuration/overview.mdx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/source/configuration/overview.mdx b/docs/source/configuration/overview.mdx index 212f3fe467..deb26e9bab 100644 --- a/docs/source/configuration/overview.mdx +++ b/docs/source/configuration/overview.mdx @@ -507,6 +507,10 @@ See [Apollo Router support for `@defer`](../executing-operations/defer-support/# See [GraphQL subscriptions in the Apollo Router](../executing-operations/subscription-support/#router-setup). +### Authorization directives support + +See [Authorization in the Apollo Router](./authorization#authorization-directives). + ### External coprocessing See [External coprocessing in the Apollo Router](../customizations/coprocessor/). From 89140d62de9b279f2ccd03742de3b5b7198222ad Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 11:16:26 +0200 Subject: [PATCH 10/21] split off directive renaming in a separate PR --- .../plugins/authorization/authenticated.rs | 312 ++---------------- .../src/plugins/authorization/mod.rs | 92 +++--- .../src/plugins/authorization/scopes.rs | 186 +---------- .../src/plugins/authorization/tests.rs | 56 ++-- 4 files changed, 113 insertions(+), 533 deletions(-) diff --git a/apollo-router/src/plugins/authorization/authenticated.rs b/apollo-router/src/plugins/authorization/authenticated.rs index b844f68a3c..f48b24b129 100644 --- a/apollo-router/src/plugins/authorization/authenticated.rs +++ b/apollo-router/src/plugins/authorization/authenticated.rs @@ -20,40 +20,20 @@ pub(crate) struct AuthenticatedCheckVisitor<'a> { compiler: &'a ApolloCompiler, file_id: FileId, pub(crate) found: bool, - authenticated_directive_name: String, } impl<'a> AuthenticatedCheckVisitor<'a> { - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Option { - let authenticated_directive_name = if let Some(link) = compiler - .db - .schema() - .directives_by_name("link") - .filter(|link| { - link.argument_by_name("url") - .and_then(|value| value.as_str()) - == Some("https://specs.apollo.dev/authenticated/v0.1") - }) - .next() - { - link.argument_by_name("as") - .and_then(|value| value.as_str().map(|s| s.to_string())) - .unwrap_or_else(|| AUTHENTICATED_DIRECTIVE_NAME.to_string()) - } else { - return None; - }; - - Some(Self { + pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { + Self { compiler, file_id, found: false, - authenticated_directive_name, - }) + } } fn is_field_authenticated(&self, field: &FieldDefinition) -> bool { field - .directive_by_name(&self.authenticated_directive_name) + .directive_by_name(AUTHENTICATED_DIRECTIVE_NAME) .is_some() || field .ty() @@ -63,8 +43,7 @@ impl<'a> AuthenticatedCheckVisitor<'a> { } fn is_type_authenticated(&self, t: &TypeDefinition) -> bool { - t.directive_by_name(&self.authenticated_directive_name) - .is_some() + t.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some() } } @@ -156,44 +135,22 @@ pub(crate) struct AuthenticatedVisitor<'a> { pub(crate) query_requires_authentication: bool, pub(crate) unauthorized_paths: Vec, current_path: Path, - authenticated_directive_name: String, } impl<'a> AuthenticatedVisitor<'a> { - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Option { - let authenticated_directive_name = if let Some(link) = compiler - .db - .schema() - .directives_by_name("link") - .filter(|link| { - link.argument_by_name("url") - .and_then(|value| value.as_str()) - == Some("https://specs.apollo.dev/authenticated/v0.1") - }) - .next() - { - link.argument_by_name("as") - .and_then(|value| value.as_str().map(|s| s.to_string())) - .unwrap_or_else(|| AUTHENTICATED_DIRECTIVE_NAME.to_string()) - } else { - return None; - }; - - println!("AuthenticatedVisitor: auth directive name = {authenticated_directive_name}"); - - Some(Self { + pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { + Self { compiler, file_id, query_requires_authentication: false, unauthorized_paths: Vec::new(), current_path: Path::default(), - authenticated_directive_name, - }) + } } fn is_field_authenticated(&self, field: &FieldDefinition) -> bool { field - .directive_by_name(&self.authenticated_directive_name) + .directive_by_name(AUTHENTICATED_DIRECTIVE_NAME) .is_some() || field .ty() @@ -203,8 +160,7 @@ impl<'a> AuthenticatedVisitor<'a> { } fn is_type_authenticated(&self, t: &TypeDefinition) -> bool { - t.directive_by_name(&self.authenticated_directive_name) - .is_some() + t.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some() } fn implementors_with_different_requirements( @@ -242,9 +198,8 @@ impl<'a> AuthenticatedVisitor<'a> { .cloned() .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) { - let ty_is_authenticated = ty - .directive_by_name(&self.authenticated_directive_name) - .is_some(); + let ty_is_authenticated = + ty.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some(); match is_authenticated { None => is_authenticated = Some(ty_is_authenticated), Some(other_ty_is_authenticated) => { @@ -283,9 +238,8 @@ impl<'a> AuthenticatedVisitor<'a> { .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) { if let Some(f) = ty.field(&self.compiler.db, field.name()) { - let field_is_authenticated = f - .directive_by_name(&self.authenticated_directive_name) - .is_some(); + let field_is_authenticated = + f.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some(); match is_authenticated { Some(other) => { if field_is_authenticated != other { @@ -315,10 +269,7 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { ) -> Result, BoxError> { let operation_requires_authentication = node .object_type(&self.compiler.db) - .map(|ty| { - ty.directive_by_name(&self.authenticated_directive_name) - .is_some() - }) + .map(|ty| ty.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some()) .unwrap_or(false); if operation_requires_authentication { @@ -492,16 +443,6 @@ mod tests { use crate::TestHarness; static BASIC_SCHEMA: &str = r#" - - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) - { - query: Query - mutation: Mutation - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -558,7 +499,7 @@ mod tests { } assert!(diagnostics.is_empty()); - let mut visitor = AuthenticatedVisitor::new(&compiler, file_id).unwrap(); + let mut visitor = AuthenticatedVisitor::new(&compiler, file_id); ( transform::document(&mut visitor, file_id).unwrap(), @@ -794,14 +735,6 @@ mod tests { } static INTERFACE_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) - { - query: Query - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -869,14 +802,6 @@ mod tests { } static INTERFACE_FIELD_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) - { - query: Query - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -952,14 +877,6 @@ mod tests { #[test] fn union() { static UNION_MEMBERS_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) - { - query: Query - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -1002,184 +919,19 @@ mod tests { }); } - static RENAMED_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", as: "auth", for: SECURITY) - { - query: Query - mutation: Mutation - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - directive @auth on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM - directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD - - type Query { - topProducts: Product - customer: User - me: User @auth - itf: I! - } - - type Mutation @auth { - ping: User - other: String - } - - interface I { - id: ID - } - - type Product { - type: String - price(setPrice: Int): Int - reviews: [Review] @auth - internal: Internal - publicReviews: [Review] - nonNullId: ID! @auth - } - - scalar Internal @auth @specifiedBy(url: "http///example.com/test") - - type Review { - body: String - author: User - } - - type User - implements I - @auth { - id: ID - name: String - } - "#; - - #[test] - fn renamed_directive() { - static QUERY: &str = r#" - query { - topProducts { - type - } - - me { - name - } - } - "#; - - let (doc, paths) = filter(RENAMED_SCHEMA, QUERY); - - insta::assert_display_snapshot!(TestResult { - query: QUERY, - result: doc, - paths - }); - } - - static ALTERNATIVE_DIRECTIVE_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/OtherAuthenticated/v0.1", import: ["@authenticated"]) - { - query: Query - mutation: Mutation - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM - directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD - - type Query { - topProducts: Product - customer: User - me: User @authenticated - itf: I! - } - - type Mutation @authenticated { - ping: User - other: String - } - - interface I { - id: ID - } - - type Product { - type: String - price(setPrice: Int): Int - reviews: [Review] @authenticated - internal: Internal - publicReviews: [Review] - nonNullId: ID! @authenticated - } - - scalar Internal @authenticated @specifiedBy(url: "http///example.com/test") - - type Review { - body: String - author: User - } - - type User - implements I - @authenticated { - id: ID - name: String - } - "#; - - // a directive named `@authenticated` imported from a different spec should not be considered - #[test] - #[should_panic] - fn alternative_directive() { - static QUERY: &str = r#" - query { - topProducts { - type - } - - me { - name - } - } - "#; - - let _ = filter(ALTERNATIVE_DIRECTIVE_SCHEMA, QUERY); - } - const SCHEMA: &str = r#"schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) + @core(feature: "https://specs.apollo.dev/core/v0.1") + @core(feature: "https://specs.apollo.dev/join/v0.1") + @core(feature: "https://specs.apollo.dev/inaccessible/v0.1") { query: Query } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE - directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + directive @core(feature: String!) repeatable on SCHEMA + directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION + directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE + directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE directive @join__graph(name: String!, url: String!) on ENUM_VALUE - directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE - directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR - directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION - - scalar link__Import - - enum link__Purpose { - """ - `SECURITY` features provide metadata necessary to securely resolve fields. - """ - SECURITY - - """ - `EXECUTION` features provide metadata necessary for operation execution. - """ - EXECUTION - } - - + directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION scalar join__FieldSet enum join__Graph { USER @join__graph(name: "user", url: "http://localhost:4001/graphql") @@ -1188,14 +940,12 @@ mod tests { directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM - type Query - @join__type(graph: ORGA) - @join__type(graph: USER) - { + type Query { currentUser: User @join__field(graph: USER) @authenticated orga(id: ID): Organization @join__field(graph: ORGA) } type User + @join__owner(graph: USER) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id"){ id: ID! @@ -1204,6 +954,7 @@ mod tests { activeOrganization: Organization } type Organization + @join__owner(graph: ORGA) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") { id: ID @@ -1239,9 +990,6 @@ mod tests { ("orga", MockSubgraph::builder().with_json( serde_json::json!{{"query":"{orga(id:1){id creatorUser{__typename id}}}"}}, serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": { "__typename": "User", "id": 0 } }}}} - ).with_json( - serde_json::json!{{"query":"{orga(id:1){id creatorUser{id name phone}}}"}}, - serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": { "__typename": "User", "id": 0, "name":"Ada", "phone": "1234" } }}}} ).build()) ].into_iter().collect()); @@ -1313,16 +1061,10 @@ mod tests { ] } }}, - ).with_json( - serde_json::json!{{"query":"{orga(id:1){id creatorUser{id name}}}"}}, - serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": {"id": 0, "name":"Ada" } }}}} ).build()), ("orga", MockSubgraph::builder().with_json( serde_json::json!{{"query":"{orga(id:1){id creatorUser{__typename id}}}"}}, serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": { "__typename": "User", "id": 0 } }}}} - ).with_json( - serde_json::json!{{"query":"{orga(id:1){id creatorUser{id name}}}"}}, - serde_json::json!{{"data": {"orga": { "id": 1, "creatorUser": {"id": 0, "name":"Ada" } }}}} ).build()) ].into_iter().collect()); diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 1e79266ab3..f62e39cbfa 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -19,11 +19,15 @@ use tower::ServiceExt; use self::authenticated::AuthenticatedCheckVisitor; use self::authenticated::AuthenticatedVisitor; +use self::authenticated::AUTHENTICATED_DIRECTIVE_NAME; use self::policy::PolicyExtractionVisitor; use self::policy::PolicyFilteringVisitor; +use self::policy::POLICY_DIRECTIVE_NAME; use self::scopes::ScopeExtractionVisitor; use self::scopes::ScopeFilteringVisitor; +use self::scopes::REQUIRES_SCOPES_DIRECTIVE_NAME; use crate::error::QueryPlannerError; +use crate::error::SchemaError; use crate::error::ServiceBuildError; use crate::graphql; use crate::json_ext::Path; @@ -91,7 +95,7 @@ pub(crate) struct AuthorizationPlugin { impl AuthorizationPlugin { pub(crate) fn enable_directives( configuration: &Configuration, - _schema: &Schema, + schema: &Schema, ) -> Result { let has_config = configuration .apollo_plugins @@ -112,25 +116,25 @@ impl AuthorizationPlugin { ) { let (compiler, file_id) = Query::make_compiler(query, schema, configuration); - if let Some(mut visitor) = AuthenticatedCheckVisitor::new(&compiler, file_id) { - // if this fails, the query is invalid and will fail at the query planning phase. - // We do not return validation errors here for now because that would imply a huge - // refactoring of telemetry and tests - if traverse::document(&mut visitor, file_id).is_ok() && visitor.found { - context.insert(AUTHENTICATED_KEY, true).unwrap(); - } + let mut visitor = AuthenticatedCheckVisitor::new(&compiler, file_id); + + // if this fails, the query is invalid and will fail at the query planning phase. + // We do not return validation errors here for now because that would imply a huge + // refactoring of telemetry and tests + if traverse::document(&mut visitor, file_id).is_ok() && visitor.found { + context.insert(AUTHENTICATED_KEY, true).unwrap(); } - if let Some(mut visitor) = ScopeExtractionVisitor::new(&compiler, file_id) { - // if this fails, the query is invalid and will fail at the query planning phase. - // We do not return validation errors here for now because that would imply a huge - // refactoring of telemetry and tests - if traverse::document(&mut visitor, file_id).is_ok() { - let scopes: Vec = visitor.extracted_scopes.into_iter().collect(); + let mut visitor = ScopeExtractionVisitor::new(&compiler, file_id); - if !scopes.is_empty() { - context.insert(REQUIRED_SCOPES_KEY, scopes).unwrap(); - } + // if this fails, the query is invalid and will fail at the query planning phase. + // We do not return validation errors here for now because that would imply a huge + // refactoring of telemetry and tests + if traverse::document(&mut visitor, file_id).is_ok() { + let scopes: Vec = visitor.extracted_scopes.into_iter().collect(); + + if !scopes.is_empty() { + context.insert(REQUIRED_SCOPES_KEY, scopes).unwrap(); } } @@ -314,30 +318,26 @@ impl AuthorizationPlugin { .pop() .expect("the query was added to the compiler earlier"); - if let Some(mut visitor) = AuthenticatedVisitor::new(compiler, id) { - let modified_query = transform::document(&mut visitor, id) - .map_err(|e| SpecError::ParsingError(e.to_string()))? - .to_string(); - - if visitor.query_requires_authentication { - if is_authenticated { - tracing::debug!("the query contains @authenticated, the request is authenticated, keeping the query"); - Ok(None) - } else { - tracing::debug!("the query contains @authenticated, modified query:\n{modified_query}\nunauthorized paths: {:?}", visitor + let mut visitor = AuthenticatedVisitor::new(compiler, id); + let modified_query = transform::document(&mut visitor, id) + .map_err(|e| SpecError::ParsingError(e.to_string()))? + .to_string(); + + if visitor.query_requires_authentication { + if is_authenticated { + tracing::debug!("the query contains @authenticated, the request is authenticated, keeping the query"); + Ok(None) + } else { + tracing::debug!("the query contains @authenticated, modified query:\n{modified_query}\nunauthorized paths: {:?}", visitor .unauthorized_paths .iter() .map(|path| path.to_string()) .collect::>()); - Ok(Some((modified_query, visitor.unauthorized_paths))) - } - } else { - tracing::debug!("the query does not contain @authenticated"); - Ok(None) + Ok(Some((modified_query, visitor.unauthorized_paths))) } } else { - tracing::debug!("the schema does not contain @authenticated"); + tracing::debug!("the query does not contain @authenticated"); Ok(None) } } @@ -352,28 +352,24 @@ impl AuthorizationPlugin { .pop() .expect("the query was added to the compiler earlier"); - if let Some(mut visitor) = - ScopeFilteringVisitor::new(compiler, id, scopes.iter().cloned().collect()) - { - let modified_query = transform::document(&mut visitor, id) - .map_err(|e| SpecError::ParsingError(e.to_string()))? - .to_string(); + let mut visitor = + ScopeFilteringVisitor::new(compiler, id, scopes.iter().cloned().collect()); + + let modified_query = transform::document(&mut visitor, id) + .map_err(|e| SpecError::ParsingError(e.to_string()))? + .to_string(); - if visitor.query_requires_scopes { - tracing::debug!("the query required scopes, the requests present scopes: {scopes:?}, modified query:\n{modified_query}\nunauthorized paths: {:?}", + if visitor.query_requires_scopes { + tracing::debug!("the query required scopes, the requests present scopes: {scopes:?}, modified query:\n{modified_query}\nunauthorized paths: {:?}", visitor .unauthorized_paths .iter() .map(|path| path.to_string()) .collect::>() ); - Ok(Some((modified_query, visitor.unauthorized_paths))) - } else { - tracing::debug!("the query does not require scopes"); - Ok(None) - } + Ok(Some((modified_query, visitor.unauthorized_paths))) } else { - tracing::debug!("the schema does not contain @requiresScopes"); + tracing::debug!("the query does not require scopes"); Ok(None) } } diff --git a/apollo-router/src/plugins/authorization/scopes.rs b/apollo-router/src/plugins/authorization/scopes.rs index bde39cefb8..9664876828 100644 --- a/apollo-router/src/plugins/authorization/scopes.rs +++ b/apollo-router/src/plugins/authorization/scopes.rs @@ -26,43 +26,23 @@ pub(crate) struct ScopeExtractionVisitor<'a> { compiler: &'a ApolloCompiler, file_id: FileId, pub(crate) extracted_scopes: HashSet, - requires_scopes_directive_name: String, } pub(crate) const REQUIRES_SCOPES_DIRECTIVE_NAME: &str = "requiresScopes"; impl<'a> ScopeExtractionVisitor<'a> { #[allow(dead_code)] - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Option { - let requires_scopes_directive_name = if let Some(link) = compiler - .db - .schema() - .directives_by_name("link") - .filter(|link| { - link.argument_by_name("url") - .and_then(|value| value.as_str()) - == Some("https://specs.apollo.dev/requiresScopes/v0.1") - }) - .next() - { - link.argument_by_name("as") - .and_then(|value| value.as_str().map(|s| s.to_string())) - .unwrap_or_else(|| REQUIRES_SCOPES_DIRECTIVE_NAME.to_string()) - } else { - return None; - }; - - Some(Self { + pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { + Self { compiler, file_id, extracted_scopes: HashSet::new(), - requires_scopes_directive_name, - }) + } } fn scopes_from_field(&mut self, field: &FieldDefinition) { self.extracted_scopes.extend( - scopes_argument(field.directive_by_name(&self.requires_scopes_directive_name)).cloned(), + scopes_argument(field.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned(), ); if let Some(ty) = field.ty().type_def(&self.compiler.db) { @@ -71,9 +51,8 @@ impl<'a> ScopeExtractionVisitor<'a> { } fn scopes_from_type(&mut self, ty: &TypeDefinition) { - self.extracted_scopes.extend( - scopes_argument(ty.directive_by_name(&self.requires_scopes_directive_name)).cloned(), - ); + self.extracted_scopes + .extend(scopes_argument(ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned()); } } @@ -107,8 +86,7 @@ impl<'a> traverse::Visitor for ScopeExtractionVisitor<'a> { fn operation(&mut self, node: &hir::OperationDefinition) -> Result<(), BoxError> { if let Some(ty) = node.object_type(&self.compiler.db) { self.extracted_scopes.extend( - scopes_argument(ty.directive_by_name(&self.requires_scopes_directive_name)) - .cloned(), + scopes_argument(ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned(), ); } @@ -213,7 +191,6 @@ pub(crate) struct ScopeFilteringVisitor<'a> { pub(crate) query_requires_scopes: bool, pub(crate) unauthorized_paths: Vec, current_path: Path, - requires_scopes_directive_name: String, } impl<'a> ScopeFilteringVisitor<'a> { @@ -221,38 +198,19 @@ impl<'a> ScopeFilteringVisitor<'a> { compiler: &'a ApolloCompiler, file_id: FileId, scopes: HashSet, - ) -> Option { - let requires_scopes_directive_name = if let Some(link) = compiler - .db - .schema() - .directives_by_name("link") - .filter(|link| { - link.argument_by_name("url") - .and_then(|value| value.as_str()) - == Some("https://specs.apollo.dev/requiresScopes/v0.1") - }) - .next() - { - link.argument_by_name("as") - .and_then(|value| value.as_str().map(|s| s.to_string())) - .unwrap_or_else(|| REQUIRES_SCOPES_DIRECTIVE_NAME.to_string()) - } else { - return None; - }; - - Some(Self { + ) -> Self { + Self { compiler, file_id, request_scopes: scopes, query_requires_scopes: false, unauthorized_paths: vec![], current_path: Path::default(), - requires_scopes_directive_name, - }) + } } fn is_field_authorized(&mut self, field: &FieldDefinition) -> bool { - if let Some(directive) = field.directive_by_name(&self.requires_scopes_directive_name) { + if let Some(directive) = field.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { let mut field_scopes_sets = scopes_sets_argument(directive); // The outer array acts like a logical OR: if any of the inner arrays of scopes matches, the field @@ -276,7 +234,7 @@ impl<'a> ScopeFilteringVisitor<'a> { } fn is_type_authorized(&self, ty: &TypeDefinition) -> bool { - match ty.directive_by_name(&self.requires_scopes_directive_name) { + match ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { None => true, Some(directive) => { let mut type_scopes_sets = scopes_sets_argument(directive); @@ -334,7 +292,7 @@ impl<'a> ScopeFilteringVisitor<'a> { // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let ty_scope_sets = ty - .directive_by_name(&self.requires_scopes_directive_name) + .directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) .map(|directive| { let mut v = scopes_sets_argument(directive) .map(|h| { @@ -390,7 +348,7 @@ impl<'a> ScopeFilteringVisitor<'a> { // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let field_scope_sets = f - .directive_by_name(&self.requires_scopes_directive_name) + .directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) .map(|directive| { let mut v = scopes_sets_argument(directive) .map(|h| { @@ -431,7 +389,7 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { node: &hir::OperationDefinition, ) -> Result, BoxError> { let is_authorized = if let Some(ty) = node.object_type(&self.compiler.db) { - match ty.directive_by_name(&self.requires_scopes_directive_name) { + match ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { None => true, Some(directive) => { let mut type_scopes_sets = scopes_sets_argument(directive); @@ -625,15 +583,6 @@ mod tests { use crate::spec::query::traverse; static BASIC_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) - { - query: Query - mutation: Mutation - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA scalar federation__Scope directive @requiresScopes(scopes: [[federation__Scope!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM @@ -690,7 +639,7 @@ mod tests { } assert!(diagnostics.is_empty()); - let mut visitor = ScopeExtractionVisitor::new(&compiler, id).unwrap(); + let mut visitor = ScopeExtractionVisitor::new(&compiler, id); traverse::document(&mut visitor, id).unwrap(); visitor.extracted_scopes.into_iter().collect() @@ -733,7 +682,7 @@ mod tests { } assert!(diagnostics.is_empty()); - let mut visitor = ScopeFilteringVisitor::new(&compiler, file_id, scopes).unwrap(); + let mut visitor = ScopeFilteringVisitor::new(&compiler, file_id, scopes); ( transform::document(&mut visitor, file_id).unwrap(), visitor.unauthorized_paths, @@ -1118,14 +1067,6 @@ mod tests { } static INTERFACE_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) - { - query: Query - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD type Query { @@ -1240,14 +1181,6 @@ mod tests { } static INTERFACE_FIELD_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) - { - query: Query - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD type Query { @@ -1326,14 +1259,6 @@ mod tests { #[test] fn union() { static UNION_MEMBERS_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", for: SECURITY) - { - query: Query - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM directive @defer on INLINE_FRAGMENT | FRAGMENT_SPREAD @@ -1380,81 +1305,4 @@ mod tests { paths }); } - - static RENAMED_SCHEMA: &str = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/requiresScopes/v0.1", as: "scopes" for: SECURITY) - { - query: Query - mutation: Mutation - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - scalar federation__Scope - directive @scopes(scopes: [[federation__Scope!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM - - type Query { - topProducts: Product - customer: User - me: User @scopes(scopes: [["profile"]]) - itf: I - } - - type Mutation @scopes(scopes: [["mut"]]) { - ping: User @scopes(scopes: [["ping"]]) - other: String - } - - interface I { - id: ID - } - - type Product { - type: String - price(setPrice: Int): Int - reviews: [Review] - internal: Internal - publicReviews: [Review] - } - - scalar Internal @scopes(scopes: [["internal", "test"]]) @specifiedBy(url: "http///example.com/test") - - type Review @scopes(scopes: [["review"]]) { - body: String - author: User - } - - type User implements I @scopes(scopes: [["read:user"]]) { - id: ID - name: String @scopes(scopes: [["read:username"]]) - } - "#; - - #[test] - fn renamed_directive() { - static QUERY: &str = r#" - query { - topProducts { - type - } - - me { - name - } - } - "#; - - let extracted_scopes = extract(RENAMED_SCHEMA, QUERY); - - let (doc, paths) = filter(RENAMED_SCHEMA, QUERY, HashSet::new()); - - insta::assert_display_snapshot!(TestResult { - query: QUERY, - extracted_scopes: &extracted_scopes, - scopes: Vec::new(), - result: doc, - paths - }); - } } diff --git a/apollo-router/src/plugins/authorization/tests.rs b/apollo-router/src/plugins/authorization/tests.rs index 6842e9ed1f..3a1f455e18 100644 --- a/apollo-router/src/plugins/authorization/tests.rs +++ b/apollo-router/src/plugins/authorization/tests.rs @@ -195,21 +195,18 @@ async fn unauthenticated_request() { } const AUTHENTICATED_SCHEMA: &str = r#"schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) - { - query: Query + @core(feature: "https://specs.apollo.dev/core/v0.1") + @core(feature: "https://specs.apollo.dev/join/v0.1") + @core(feature: "https://specs.apollo.dev/inaccessible/v0.1") + { + query: Query } -directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA -directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE -directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION +directive @core(feature: String!) repeatable on SCHEMA +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION +directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE +directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE directive @join__graph(name: String!, url: String!) on ENUM_VALUE -directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE -directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR -directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION - -scalar link__Import +directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM @@ -218,11 +215,12 @@ enum join__Graph { USER @join__graph(name: "user", url: "http://localhost:4001/graphql") ORGA @join__graph(name: "orga", url: "http://localhost:4002/graphql") } -type Query{ +type Query { currentUser: User @join__field(graph: USER) orga(id: ID): Organization @join__field(graph: ORGA) } type User +@join__owner(graph: USER) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id"){ id: ID! @@ -231,6 +229,7 @@ type User activeOrganization: Organization } type Organization +@join__owner(graph: ORGA) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") { id: ID @authenticated @@ -346,27 +345,22 @@ async fn authenticated_directive() { .unwrap() .unwrap(); - println!("req2"); - insta::assert_json_snapshot!(response); } const SCOPES_SCHEMA: &str = r#"schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) - { + @core(feature: "https://specs.apollo.dev/core/v0.1") + @core(feature: "https://specs.apollo.dev/join/v0.1") + @core(feature: "https://specs.apollo.dev/inaccessible/v0.1") + { query: Query } -directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA -directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE -directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION +directive @core(feature: String!) repeatable on SCHEMA +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION +directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE +directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE directive @join__graph(name: String!, url: String!) on ENUM_VALUE -directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE -directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR -directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION - -scalar link__Import +directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM @@ -375,13 +369,12 @@ enum join__Graph { USER @join__graph(name: "user", url: "http://localhost:4001/graphql") ORGA @join__graph(name: "orga", url: "http://localhost:4002/graphql") } -type Query -@join__type(graph: ORGA) -@join__type(graph: USER){ +type Query { currentUser: User @join__field(graph: USER) orga(id: ID): Organization @join__field(graph: ORGA) } type User +@join__owner(graph: USER) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") @requiresScopes(scopes: [["user:read"], ["admin"]]) { @@ -391,6 +384,7 @@ type User activeOrganization: Organization } type Organization +@join__owner(graph: ORGA) @join__type(graph: ORGA, key: "id") @join__type(graph: USER, key: "id") { id: ID From 629d6088bcdec087aa5a836957f89bb84ce83bab Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 11:20:58 +0200 Subject: [PATCH 11/21] remove snapshot --- ...thenticated__tests__renamed_directive.snap | 24 ------------------- 1 file changed, 24 deletions(-) delete mode 100644 apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap deleted file mode 100644 index d4691a6c97..0000000000 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__renamed_directive.snap +++ /dev/null @@ -1,24 +0,0 @@ ---- -source: apollo-router/src/plugins/authorization/authenticated.rs -expression: "TestResult { query: QUERY, result: doc, paths }" ---- -query: - - query { - topProducts { - type - } - - me { - name - } - } - -filtered: -query { - topProducts { - type - } -} - -paths: ["/me"] From 04962fe264a2e4b67dcf2546a222c1e709443a4c Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 11:31:07 +0200 Subject: [PATCH 12/21] use const --- apollo-router/src/spec/mod.rs | 3 +++ apollo-router/src/uplink/license_enforcement.rs | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/spec/mod.rs b/apollo-router/src/spec/mod.rs index 4589813c91..ff2642fbf2 100644 --- 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/uplink/license_enforcement.rs b/apollo-router/src/uplink/license_enforcement.rs index 540bcc415f..56b445bd6a 100644 --- a/apollo-router/src/uplink/license_enforcement.rs +++ b/apollo-router/src/uplink/license_enforcement.rs @@ -28,6 +28,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"; @@ -134,9 +136,9 @@ impl LicenseEnforcementReport { let feature_urls = schema .db .schema() - .directives_by_name("link") + .directives_by_name(LINK_DIRECTIVE_NAME) .filter_map(|link| { - link.argument_by_name("url") + link.argument_by_name(LINK_URL_ARGUMENT) .and_then(|value| value.as_str().map(|s| s.to_string())) }) .collect::>(); From 740a1fa0c396e4e31bb5bc7597f315a4c928d578 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 11:32:02 +0200 Subject: [PATCH 13/21] lint --- apollo-router/src/plugins/authorization/mod.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index f62e39cbfa..0b9427a059 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -19,15 +19,11 @@ use tower::ServiceExt; use self::authenticated::AuthenticatedCheckVisitor; use self::authenticated::AuthenticatedVisitor; -use self::authenticated::AUTHENTICATED_DIRECTIVE_NAME; use self::policy::PolicyExtractionVisitor; use self::policy::PolicyFilteringVisitor; -use self::policy::POLICY_DIRECTIVE_NAME; use self::scopes::ScopeExtractionVisitor; use self::scopes::ScopeFilteringVisitor; -use self::scopes::REQUIRES_SCOPES_DIRECTIVE_NAME; use crate::error::QueryPlannerError; -use crate::error::SchemaError; use crate::error::ServiceBuildError; use crate::graphql; use crate::json_ext::Path; From 9daae23c470fa7f4178fe7560c90541a30a690b2 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 11:39:24 +0200 Subject: [PATCH 14/21] still check for the directive presence no need to activate the plugin if the directives are not in the schema --- .../src/plugins/authorization/mod.rs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 0b9427a059..a2eefa9029 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -19,10 +19,13 @@ use tower::ServiceExt; use self::authenticated::AuthenticatedCheckVisitor; use self::authenticated::AuthenticatedVisitor; +use self::authenticated::AUTHENTICATED_DIRECTIVE_NAME; use self::policy::PolicyExtractionVisitor; use self::policy::PolicyFilteringVisitor; +use self::policy::POLICY_DIRECTIVE_NAME; use self::scopes::ScopeExtractionVisitor; use self::scopes::ScopeFilteringVisitor; +use self::scopes::REQUIRES_SCOPES_DIRECTIVE_NAME; use crate::error::QueryPlannerError; use crate::error::ServiceBuildError; use crate::graphql; @@ -101,7 +104,23 @@ impl AuthorizationPlugin { .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())); - Ok(has_config.unwrap_or(true)) + let has_authorization_directives = schema + .type_system + .definitions + .directives + .contains_key(AUTHENTICATED_DIRECTIVE_NAME) + || schema + .type_system + .definitions + .directives + .contains_key(REQUIRES_SCOPES_DIRECTIVE_NAME) + || schema + .type_system + .definitions + .directives + .contains_key(POLICY_DIRECTIVE_NAME); + + Ok(has_config.unwrap_or(true) && has_authorization_directives) } pub(crate) async fn query_analysis( From 9c47a0239250b994db3c9ced34471f8665936595 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 12:28:59 +0200 Subject: [PATCH 15/21] lint --- apollo-router/src/plugins/authorization/authenticated.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/apollo-router/src/plugins/authorization/authenticated.rs b/apollo-router/src/plugins/authorization/authenticated.rs index f48b24b129..e487eee2fb 100644 --- a/apollo-router/src/plugins/authorization/authenticated.rs +++ b/apollo-router/src/plugins/authorization/authenticated.rs @@ -424,7 +424,6 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { #[cfg(test)] mod tests { - use apollo_compiler::ApolloCompiler; use multimap::MultiMap; use serde_json_bytes::json; From 75b4f6d2035c34c38e0e5045902c9a6928cbaf92 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 12:38:13 +0200 Subject: [PATCH 16/21] changeset --- .changesets/feat_geal_authz_optout.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changesets/feat_geal_authz_optout.md diff --git a/.changesets/feat_geal_authz_optout.md b/.changesets/feat_geal_authz_optout.md new file mode 100644 index 0000000000..bc4df3e7b4 --- /dev/null +++ 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 wxithout further configuration. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/3713 \ No newline at end of file From dd7df492bf27c7820a48ce48ed1e40c94bc9e7ab Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 2 Oct 2023 12:38:19 +0200 Subject: [PATCH 17/21] lint --- apollo-router/src/uplink/license_enforcement.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/uplink/license_enforcement.rs b/apollo-router/src/uplink/license_enforcement.rs index 56b445bd6a..a7ad1db886 100644 --- a/apollo-router/src/uplink/license_enforcement.rs +++ b/apollo-router/src/uplink/license_enforcement.rs @@ -244,7 +244,7 @@ impl Display for LicenseEnforcementReport { write!(f, "Configuration yaml:\n{restricted_config}")?; if !self.restricted_schema_in_use.is_empty() { - write!(f, "\n")?; + writeln!(f)?; } } From 8d338fc8a731ac687bee1e721f6fc6249cbcf8fc Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 20 Oct 2023 16:33:46 +0200 Subject: [PATCH 18/21] fix --- apollo-router/src/spec/schema.rs | 20 ++++++++----------- .../src/uplink/license_enforcement.rs | 13 ++++++------ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs index f700390222..66f352d789 100644 --- a/apollo-router/src/spec/schema.rs +++ b/apollo-router/src/spec/schema.rs @@ -59,23 +59,19 @@ impl Schema { Ok(schema) } - pub(crate) fn make_compiler(sdl: &str) -> Result { - let mut compiler = ApolloCompiler::new(); - let id = compiler.add_type_system(sdl, "schema.graphql"); - - let ast = compiler.db.ast(id); + pub(crate) fn make_compiler(sdl: &str) -> Result { + let mut parser = apollo_compiler::Parser::new(); + let ast = parser.parse_ast(sdl, "schema.graphql"); // Trace log recursion limit data - let recursion_limit = ast.recursion_limit(); + let recursion_limit = parser.recursion_reached(); tracing::trace!(?recursion_limit, "recursion limit data"); - let mut parse_errors = ast.errors().peekable(); - if parse_errors.peek().is_some() { - let errors = parse_errors.cloned().collect::>(); - return Err(SchemaError::Parse(ParseErrors { errors })); - } + ast.check_parse_errors() + .map_err(|errors| SchemaError::Parse(ParseErrors { errors }))?; - Ok(compiler) + let definitions = ast.to_schema(); + Ok(definitions) } pub(crate) fn parse(sdl: &str, configuration: &Configuration) -> Result { diff --git a/apollo-router/src/uplink/license_enforcement.rs b/apollo-router/src/uplink/license_enforcement.rs index a7ad1db886..e51433c9e6 100644 --- a/apollo-router/src/uplink/license_enforcement.rs +++ b/apollo-router/src/uplink/license_enforcement.rs @@ -11,8 +11,6 @@ use std::time::Duration; use std::time::SystemTime; use std::time::UNIX_EPOCH; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::HirDatabase; use buildstructor::Builder; use displaydoc::Display; use itertools::Itertools; @@ -90,7 +88,7 @@ impl LicenseEnforcementReport { pub(crate) fn build( configuration: &Configuration, - schema: &ApolloCompiler, + schema: &apollo_compiler::schema::Schema, ) -> LicenseEnforcementReport { LicenseEnforcementReport { restricted_config_in_use: Self::validate_configuration( @@ -130,13 +128,14 @@ impl LicenseEnforcementReport { } fn validate_schema( - schema: &ApolloCompiler, + schema: &apollo_compiler::schema::Schema, schema_restrictions: &Vec, ) -> Vec { let feature_urls = schema - .db - .schema() - .directives_by_name(LINK_DIRECTIVE_NAME) + .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())) From 3e250c25648e794a1fe355ce765d7ffd847296bd Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 20 Oct 2023 19:12:35 +0200 Subject: [PATCH 19/21] fix serde error we can't derive Default and declare serde(default) on fields at the same time --- apollo-router/src/plugins/authorization/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 29b022e32b..f30e8d59d5 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -60,7 +60,7 @@ pub(crate) struct CacheKeyMetadata { } /// Authorization plugin -#[derive(Clone, Debug, Default, Deserialize, JsonSchema)] +#[derive(Clone, Debug, Deserialize, JsonSchema)] #[allow(dead_code)] pub(crate) struct Conf { /// Reject unauthenticated requests From 8fa5a0d5ff99b2dd92251ee17c765af2e80509b5 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 20 Oct 2023 19:25:28 +0200 Subject: [PATCH 20/21] fix again --- apollo-router/src/plugins/authorization/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index f30e8d59d5..d2907db2e8 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -60,7 +60,7 @@ pub(crate) struct CacheKeyMetadata { } /// Authorization plugin -#[derive(Clone, Debug, Deserialize, JsonSchema)] +#[derive(Clone, Debug, serde_derive_default::Default, Deserialize, JsonSchema)] #[allow(dead_code)] pub(crate) struct Conf { /// Reject unauthenticated requests @@ -71,7 +71,7 @@ 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 From 6f2d1f8b7d67a17258c4554d4f905f98400595c2 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 23 Oct 2023 10:25:27 +0200 Subject: [PATCH 21/21] Update .changesets/feat_geal_authz_optout.md Co-authored-by: Jeremy Lempereur --- .changesets/feat_geal_authz_optout.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changesets/feat_geal_authz_optout.md b/.changesets/feat_geal_authz_optout.md index bc4df3e7b4..fcf9ffc3f4 100644 --- a/.changesets/feat_geal_authz_optout.md +++ b/.changesets/feat_geal_authz_optout.md @@ -1,5 +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 wxithout further configuration. +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 \ No newline at end of file