Skip to content

Commit a7dc0a6

Browse files
aaronArinderaaronArinder
committed
squash me
1 parent 8b5e13a commit a7dc0a6

File tree

3 files changed

+254
-11
lines changed

3 files changed

+254
-11
lines changed

‎apollo-router/src/plugins/authorization/policy.rs‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ impl traverse::Visitor for PolicyExtractionVisitor<'_> {
144144
node: &executable::Field,
145145
) -> Result<(), BoxError> {
146146
self.get_policies_from_field(field_def);
147-
148147
traverse::field(self, field_def, node)
149148
}
150149

@@ -363,9 +362,6 @@ impl<'a> PolicyFilteringVisitor<'a> {
363362
None => policies_sets = Some(ty_policies_sets),
364363
Some(other_policies) => {
365364
if ty_policies_sets != *other_policies {
366-
println!(
367-
"AWA :: different TYPE policies -> type policies: {ty_policies_sets:?}, other policies: {other_policies:?}"
368-
);
369365
return true;
370366
}
371367
}
@@ -411,9 +407,6 @@ impl<'a> PolicyFilteringVisitor<'a> {
411407
None => policies_sets = Some(field_policies),
412408
Some(other_policies) => {
413409
if field_policies != *other_policies {
414-
println!(
415-
"AWA :: different FIELD policies -> field policies: {field_policies:?}, other policies: {other_policies:?}"
416-
);
417410
return true;
418411
}
419412
}
@@ -1400,9 +1393,6 @@ mod tests {
14001393
// which should result in `successful_policies` to contain `read` as well
14011394
let read_policy: HashSet<String> = ["read".to_string()].into_iter().collect();
14021395
let (doc, paths) = filter(SCHEMA, QUERY, read_policy);
1403-
// NOTE: paths ==> unauthorized paths
1404-
//
1405-
println!("AWA :: unauthorized paths: {paths:?}");
14061396
insta::assert_snapshot!(TestResult {
14071397
query: QUERY,
14081398
// this should have `read` as the extracted policy
@@ -1470,7 +1460,6 @@ mod tests {
14701460
// which should result in `successful_policies` to contain `read` as well
14711461
let read_policy: HashSet<String> = ["read".to_string()].into_iter().collect();
14721462
let (doc, paths) = filter(SCHEMA, QUERY, read_policy);
1473-
println!("AWA :: unauthorized paths: {paths:?}");
14741463
insta::assert_snapshot!(TestResult {
14751464
query: QUERY,
14761465
// this should have `read` as the extracted policy

‎apollo-router/tests/fixtures/directives/policy/policy_schema_with_interfaces.graphql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ type Query
4545
private: Private @join__field(graph: SUBGRAPH_A) @policy(policies: [["admin"]])
4646
public: Public @join__field(graph: SUBGRAPH_A)
4747
opensecret: OpenSecret @join__field(graph: SUBGRAPH_A)
48+
secure: Secure @join__field(graph: SUBGRAPH_A)
4849
}
4950

5051
interface Secure

‎apollo-router/tests/integration/directives/policy.rs‎

Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,3 +367,256 @@ async fn policy_directive_interfaces_with_different_implementors_without_policy_
367367

368368
Ok(())
369369
}
370+
371+
#[tokio::test(flavor = "multi_thread")]
372+
async fn policy_directive_interfaces_with_different_implementors_disallowed() -> Result<(), BoxError>
373+
{
374+
// GIVEN
375+
// * a schema with @policy
376+
// * an interface using the @policy with implementors that have different policies
377+
// * see the fixture for notes
378+
// * requesting the interface directly, not one of its implementors
379+
// * a mock coprocessor that marks the admin policy as false (unused, see note above)
380+
// * a mock subgraph serving both public and private data
381+
// * a context object with the admin policy set to false
382+
// * the supergraph service layer
383+
// * a request for a policy-gated field
384+
385+
let mock_coprocessor = MockServer::start().await;
386+
let coprocessor_address = mock_coprocessor.uri();
387+
388+
Mock::given(method("POST"))
389+
.and(path("/"))
390+
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
391+
"version": 1,
392+
"stage": "SupergraphRequest",
393+
"control": "continue",
394+
"context": {
395+
"entries": {
396+
"apollo::authorization::required_policies": {
397+
// NOTE: see the note above, but this shouldn't govern how the test
398+
// behaves; it's the context object that does the dirt for the TestHarness.
399+
// Change this value if you use it with the IntegrationTest builder
400+
"admin": false
401+
}
402+
}
403+
}
404+
})))
405+
.mount(&mock_coprocessor)
406+
.await;
407+
408+
let mut subgraphs = MockedSubgraphs::default();
409+
subgraphs.insert(
410+
"subgraph_a",
411+
MockSubgraph::builder()
412+
.with_json(
413+
serde_json::json!({"query": "{private{id}}"}),
414+
serde_json::json!({"data": {"private": {"id": "123"}}}),
415+
)
416+
.with_json(
417+
serde_json::json!({"query": "{public{id}}"}),
418+
serde_json::json!({"data": {"public": {"id": "456"}}}),
419+
)
420+
.with_json(
421+
serde_json::json!({"query": "{opensecret{id}}"}),
422+
serde_json::json!({"data": {"opensecret": {"id": "789"}}}),
423+
)
424+
.with_json(
425+
serde_json::json!({"query": "{secure{id}}"}),
426+
serde_json::json!({"data": {"secure": {"id": "000"}}}),
427+
)
428+
.build(),
429+
);
430+
431+
let supergraph_harness = TestHarness::builder()
432+
.configuration_json(serde_json::json!({
433+
"coprocessor": {
434+
"url": coprocessor_address,
435+
"supergraph": {
436+
"request": {
437+
"context": "all"
438+
}
439+
}
440+
},
441+
"include_subgraph_errors": {
442+
"all": true
443+
}
444+
}))
445+
.unwrap()
446+
.schema(include_str!(
447+
"../../fixtures/directives/policy/policy_schema_with_interfaces.graphql"
448+
))
449+
.extra_plugin(subgraphs)
450+
.build_supergraph()
451+
.await
452+
.unwrap();
453+
454+
let context = Context::new();
455+
context
456+
// NOTE: there is no `admin` policy in the context
457+
.insert("apollo::authorization::required_policies", json! { [] })
458+
.unwrap();
459+
460+
// WHEN
461+
// * we make a request with the interface itself off of Query
462+
let request = supergraph::Request::fake_builder()
463+
.query(r#"{ secure { id } }"#)
464+
.context(context)
465+
.method(Method::POST)
466+
.build()
467+
.unwrap();
468+
469+
// THEN
470+
// * we don't get data
471+
472+
let response = supergraph_harness
473+
.oneshot(request)
474+
.await
475+
.unwrap()
476+
.next_response()
477+
.await
478+
.unwrap()
479+
.data
480+
.unwrap();
481+
482+
let response = response.as_object().unwrap();
483+
484+
assert!(response.is_empty());
485+
Ok(())
486+
}
487+
488+
// NOTE: this is the test to pay attention to for a recent customer question
489+
#[tokio::test(flavor = "multi_thread")]
490+
async fn policy_directive_interfaces_with_different_implementors_open_question()
491+
-> Result<(), BoxError> {
492+
// GIVEN
493+
// * a schema with @policy
494+
// * an interface using the @policy with implementors that have different policies
495+
// * see the fixture for notes
496+
// * requesting the interface directly, not one of its implementors
497+
// * a mock coprocessor that marks the admin policy as true (unused, see note above)
498+
// * a mock subgraph serving both public and private data
499+
// * a context object with the admin policy set to true
500+
// * the supergraph service layer
501+
// * a request for a policy-gated field
502+
503+
let mock_coprocessor = MockServer::start().await;
504+
let coprocessor_address = mock_coprocessor.uri();
505+
506+
Mock::given(method("POST"))
507+
.and(path("/"))
508+
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
509+
"version": 1,
510+
"stage": "SupergraphRequest",
511+
"control": "continue",
512+
"context": {
513+
"entries": {
514+
"apollo::authorization::required_policies": {
515+
// NOTE: see the note above, but this shouldn't govern how the test
516+
// behaves; it's the context object that does the dirt for the TestHarness.
517+
// Change this value if you use it with the IntegrationTest builder
518+
"admin": true
519+
}
520+
}
521+
}
522+
})))
523+
.mount(&mock_coprocessor)
524+
.await;
525+
526+
let mut subgraphs = MockedSubgraphs::default();
527+
subgraphs.insert(
528+
"subgraph_a",
529+
MockSubgraph::builder()
530+
.with_json(
531+
serde_json::json!({"query": "{private{id}}"}),
532+
serde_json::json!({"data": {"private": {"id": "123"}}}),
533+
)
534+
.with_json(
535+
serde_json::json!({"query": "{public{id}}"}),
536+
serde_json::json!({"data": {"public": {"id": "456"}}}),
537+
)
538+
.with_json(
539+
serde_json::json!({"query": "{opensecret{id}}"}),
540+
serde_json::json!({"data": {"opensecret": {"id": "789"}}}),
541+
)
542+
.with_json(
543+
serde_json::json!({"query": "{secure{id}}"}),
544+
serde_json::json!({"data": {"secure": {"id": "000"}}}),
545+
)
546+
.build(),
547+
);
548+
549+
let supergraph_harness = TestHarness::builder()
550+
.configuration_json(serde_json::json!({
551+
"coprocessor": {
552+
"url": coprocessor_address,
553+
"supergraph": {
554+
"request": {
555+
"context": "all"
556+
}
557+
}
558+
},
559+
"include_subgraph_errors": {
560+
"all": true
561+
}
562+
}))
563+
.unwrap()
564+
.schema(include_str!(
565+
"../../fixtures/directives/policy/policy_schema_with_interfaces.graphql"
566+
))
567+
.extra_plugin(subgraphs)
568+
.build_supergraph()
569+
.await
570+
.unwrap();
571+
572+
let context = Context::new();
573+
context
574+
.insert(
575+
"apollo::authorization::required_policies",
576+
json! {[ "admin" ]},
577+
)
578+
.unwrap();
579+
580+
// WHEN
581+
// * we make a request with the interface itself off of Query
582+
let request = supergraph::Request::fake_builder()
583+
.query(r#"{ secure { id } }"#)
584+
.context(context)
585+
.method(Method::POST)
586+
.build()
587+
.unwrap();
588+
589+
// THEN
590+
// * we... get the data? expected? not sure
591+
592+
let response = supergraph_harness
593+
.oneshot(request)
594+
.await
595+
.unwrap()
596+
.next_response()
597+
.await
598+
.unwrap()
599+
.data
600+
.unwrap();
601+
602+
// NOTE: we're failing here; the response is null, and the expectations for this test are
603+
// unclear: should there be data returned if an interface is queried that has a policy when
604+
// that policy is in the context and the coprocessor returns `true` for it?
605+
println!("AWA :: response: {response:?}");
606+
607+
let response = response
608+
.as_object()
609+
.unwrap()
610+
.get_key_value("secure")
611+
.unwrap()
612+
.1
613+
.as_object()
614+
.unwrap()
615+
.get_key_value("id")
616+
.unwrap()
617+
.1;
618+
619+
assert_eq!(response, "000");
620+
621+
Ok(())
622+
}

0 commit comments

Comments
 (0)