Skip to content

Commit

Permalink
Change blocking in GraphQL to use ActionHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
y9v committed Jan 17, 2025
1 parent 5152ba7 commit 5d7a7f9
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 46 deletions.
8 changes: 1 addition & 7 deletions lib/datadog/appsec/contrib/graphql/appsec_trace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,10 @@ def execute_multiplex(multiplex:)

gateway_multiplex = Gateway::Multiplex.new(multiplex)

multiplex_return, multiplex_response = Instrumentation.gateway.push('graphql.multiplex', gateway_multiplex) do
multiplex_return, = Instrumentation.gateway.push('graphql.multiplex', gateway_multiplex) do
super
end

# Returns an error * the number of queries so that the entire multiplex is blocked
if multiplex_response
blocked_event = multiplex_response.find { |action, _options| action == :block }
multiplex_return = AppSec::Response.graphql_response(gateway_multiplex) if blocked_event
end

multiplex_return
end
end
Expand Down
9 changes: 5 additions & 4 deletions lib/datadog/appsec/contrib/graphql/gateway/watcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def watch
# This time we don't throw but use next
def watch_multiplex(gateway = Instrumentation.gateway)
gateway.watch('graphql.multiplex', :appsec) do |stack, gateway_multiplex|
block = false
event = nil
context = AppSec::Context.active
engine = AppSec::Reactive::Engine.new
Expand All @@ -39,13 +38,15 @@ def watch_multiplex(gateway = Instrumentation.gateway)

Datadog::AppSec::Event.tag_and_keep!(context, result)
context.events << event

result.actions.each do |action_type, action_params|
Datadog::AppSec::ActionHandler.handle(action_type, action_params)
end
end

block = GraphQL::Reactive::Multiplex.publish(engine, gateway_multiplex)
GraphQL::Reactive::Multiplex.publish(engine, gateway_multiplex)
end

next [nil, [[:block, event]]] if block

stack.call(gateway_multiplex.arguments)
end
end
Expand Down
14 changes: 0 additions & 14 deletions lib/datadog/appsec/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,6 @@ def build(action_params, http_accept_header)
block_response(action_params, http_accept_header)
end

def graphql_response(gateway_multiplex)
multiplex_return = []
gateway_multiplex.queries.each do |query|
# This method is only called in places where GraphQL-Ruby is already required
query_result = ::GraphQL::Query::Result.new(
query: query,
values: JSON.parse(content('application/json'))
)
multiplex_return << query_result
end

multiplex_return
end

private

def block_response(action_params, http_accept_header)
Expand Down
1 change: 0 additions & 1 deletion sig/datadog/appsec/response.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module Datadog
def to_rack: () -> ::Array[untyped]

def self.build: (::Hash[::String, ::String] action_params, ::String http_accept_header) -> Response
def self.graphql_response: (Datadog::AppSec::Contrib::GraphQL::Gateway::Multiplex gateway_multiplex) -> Array[::GraphQL::Query::Result]

private

Expand Down
30 changes: 10 additions & 20 deletions spec/datadog/appsec/contrib/graphql/integration_test_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,8 @@
let(:query) { '{ userByName(name: "$testattack") { id } }' }

it do
expect(last_response.body).to eq(
{
'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }]
}.to_json
expect(JSON.parse(last_response.body)).to eq(
'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }]
)
expect(spans).to include(
an_object_having_attributes(
Expand All @@ -240,8 +238,7 @@
)
end

# GraphQL errors should have no impact on the HTTP layer
it_behaves_like 'a POST 200 span'
it_behaves_like 'a POST 403 span'
it_behaves_like 'a trace with AppSec tags'
it_behaves_like 'a trace with AppSec events'
end
Expand Down Expand Up @@ -297,10 +294,8 @@
let(:mutation) { 'mutation { createUser(name: "$testattack") { user { name, id } } }' }

it do
expect(last_response.body).to eq(
{
'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }]
}.to_json
expect(JSON.parse(last_response.body)).to eq(
'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }]
)
expect(spans).to include(
an_object_having_attributes(
Expand All @@ -317,7 +312,7 @@
)
end

it_behaves_like 'a POST 200 span'
it_behaves_like 'a POST 403 span'
it_behaves_like 'a trace with AppSec tags'
it_behaves_like 'a trace with AppSec events'

Expand Down Expand Up @@ -435,11 +430,8 @@
end

it do
expect(last_response.body).to eq(
[
{ 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] },
{ 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] }
].to_json
expect(JSON.parse(last_response.body)).to eq(
'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }]
)
expect(spans).to include(
an_object_having_attributes(
Expand Down Expand Up @@ -549,10 +541,8 @@
end

it do
expect(last_response.body).to eq(
[
{ 'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }] }
].to_json
expect(JSON.parse(last_response.body)).to eq(
'errors' => [{ 'title' => 'Blocked', 'detail' => 'Security provided by Datadog.' }]
)
expect(spans).to include(
an_object_having_attributes(
Expand Down

0 comments on commit 5d7a7f9

Please sign in to comment.