diff --git a/app/graphql/mutations/concerns/mutations/can_mutate_spammable.rb b/app/graphql/mutations/concerns/mutations/can_mutate_spammable.rb
index 2d4983f0d6e4f43e85d52dec9eee56dc058d65e0..ba644eff36cdc03ff5523fa35d9f422a7fc0e625 100644
--- a/app/graphql/mutations/concerns/mutations/can_mutate_spammable.rb
+++ b/app/graphql/mutations/concerns/mutations/can_mutate_spammable.rb
@@ -5,6 +5,7 @@ module Mutations
# and optionally support the workflow to allow clients to display and solve CAPTCHAs.
module CanMutateSpammable
extend ActiveSupport::Concern
+ include Spam::Concerns::HasSpamActionResponseFields
# NOTE: The arguments and fields are intentionally named with 'captcha' instead of 'recaptcha',
# so that they can be applied to future alternative CAPTCHA implementations other than
@@ -59,25 +60,5 @@ def additional_spam_params
request: context[:request]
}
end
-
- # with_spam_action_fields(spammable) { {other_fields: true} } -> hash
- #
- # Takes a Spammable and a block as arguments.
- #
- # The block passed should be a hash, which the spam action fields will be merged into.
- def with_spam_action_fields(spammable)
- spam_action_fields = {
- spam: spammable.spam?,
- # NOTE: These fields are intentionally named with 'captcha' instead of 'recaptcha', so
- # that they can be applied to future alternative CAPTCHA implementations other than
- # reCAPTCHA (such as FriendlyCaptcha) without having to change the response field name
- # in the API.
- needs_captcha_response: spammable.render_recaptcha?,
- spam_log_id: spammable.spam_log&.id,
- captcha_site_key: Gitlab::CurrentSettings.recaptcha_site_key
- }
-
- yield.merge(spam_action_fields)
- end
end
end
diff --git a/app/graphql/mutations/snippets/create.rb b/app/graphql/mutations/snippets/create.rb
index 73eac9f0f3baf1fbacf5486a3348bc32c72ad7a8..7f2dd448b8b14c45fd94d97503cb0a213f91ce5c 100644
--- a/app/graphql/mutations/snippets/create.rb
+++ b/app/graphql/mutations/snippets/create.rb
@@ -56,7 +56,7 @@ def resolve(project_path: nil, **args)
end
snippet = service_response.payload[:snippet]
- with_spam_action_fields(snippet) do
+ with_spam_action_response_fields(snippet) do
{
snippet: service_response.success? ? snippet : nil,
errors: errors_on_object(snippet)
diff --git a/app/graphql/mutations/snippets/update.rb b/app/graphql/mutations/snippets/update.rb
index af8e6f384b70b5acd5a3c596217d162451d2c689..9f9f8bca8485b965bd41ab34216b54532da778c5 100644
--- a/app/graphql/mutations/snippets/update.rb
+++ b/app/graphql/mutations/snippets/update.rb
@@ -45,7 +45,7 @@ def resolve(id:, **args)
end
snippet = service_response.payload[:snippet]
- with_spam_action_fields(snippet) do
+ with_spam_action_response_fields(snippet) do
{
snippet: service_response.success? ? snippet : snippet.reset,
errors: errors_on_object(snippet)
diff --git a/app/services/spam/spam_action_service.rb b/app/services/spam/spam_action_service.rb
index ff32bc32d939f589841adb49fedcc0e03edf8b5a..185b9e39070a448a4ed3b28b77c6accd63c2c9a4 100644
--- a/app/services/spam/spam_action_service.rb
+++ b/app/services/spam/spam_action_service.rb
@@ -9,7 +9,9 @@ class SpamActionService
# after the spammable is created/updated based on the remaining parameters.
#
# Takes a hash of parameters from an incoming request to modify a model (via a controller,
- # service, or GraphQL mutation).
+ # service, or GraphQL mutation). The parameters will either be camelCase (if they are
+ # received directly via controller params) or underscore_case (if they have come from
+ # a GraphQL mutation which has converted them to underscore)
#
# Deletes the parameters which are related to spam and captcha processing, and returns
# them in a SpamParams parameters object. See:
@@ -18,12 +20,12 @@ def self.filter_spam_params!(params)
# NOTE: The 'captcha_response' field can be expanded to multiple fields when we move to future
# alternative captcha implementations such as FriendlyCaptcha. See
# https://gitlab.com/gitlab-org/gitlab/-/issues/273480
- captcha_response = params.delete(:captcha_response)
+ captcha_response = params.delete(:captcha_response) || params.delete(:captchaResponse)
SpamParams.new(
api: params.delete(:api),
captcha_response: captcha_response,
- spam_log_id: params.delete(:spam_log_id)
+ spam_log_id: params.delete(:spam_log_id) || params.delete(:spamLogId)
)
end
diff --git a/lib/quality/test_level.rb b/lib/quality/test_level.rb
index 45cfa9b373db249ceeabfbe0162db63d18e90af7..ad9de067375654aa11bfaf3411fdb4c976c60b31 100644
--- a/lib/quality/test_level.rb
+++ b/lib/quality/test_level.rb
@@ -43,6 +43,7 @@ class TestLevel
serializers
services
sidekiq
+ spam
support_specs
tasks
uploaders
diff --git a/lib/spam/concerns/has_spam_action_response_fields.rb b/lib/spam/concerns/has_spam_action_response_fields.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d49f5cd6454c11ae40d9057198ee94e13e344c9f
--- /dev/null
+++ b/lib/spam/concerns/has_spam_action_response_fields.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module Spam
+ module Concerns
+ # This concern is shared by the controller and GraphQL layer to handle
+ # addition of spam/CAPTCHA related fields in the response.
+ module HasSpamActionResponseFields
+ extend ActiveSupport::Concern
+
+ # spam_action_response_fields(spammable) -> hash
+ #
+ # Takes a Spammable as an argument and returns response fields necessary to display a CAPTCHA on
+ # the client.
+ def spam_action_response_fields(spammable)
+ {
+ spam: spammable.spam?,
+ # NOTE: These fields are intentionally named with 'captcha' instead of 'recaptcha', so
+ # that they can be applied to future alternative CAPTCHA implementations other than
+ # reCAPTCHA (such as FriendlyCaptcha) without having to change the response field name
+ # in the API.
+ needs_captcha_response: spammable.render_recaptcha?,
+ spam_log_id: spammable.spam_log&.id,
+ captcha_site_key: Gitlab::CurrentSettings.recaptcha_site_key
+ }
+ end
+
+ # with_spam_action_response_fields(spammable) { {other_fields: true} } -> hash
+ #
+ # Takes a Spammable and a block as arguments.
+ #
+ # The block passed should be a hash, which the spam_action_fields will be merged into.
+ def with_spam_action_response_fields(spammable)
+ yield.merge(spam_action_response_fields(spammable))
+ end
+ end
+ end
+end
diff --git a/spec/graphql/mutations/concerns/mutations/can_mutate_spammable_spec.rb b/spec/graphql/mutations/concerns/mutations/can_mutate_spammable_spec.rb
index ee8db7a1f31c7e0bc05f22ad2c5402c88e939ce9..8d1fce406fa6664d02a740d7a0d4440678cce5ea 100644
--- a/spec/graphql/mutations/concerns/mutations/can_mutate_spammable_spec.rb
+++ b/spec/graphql/mutations/concerns/mutations/can_mutate_spammable_spec.rb
@@ -30,7 +30,7 @@
end
it 'merges in spam action fields from spammable' do
- result = subject.send(:with_spam_action_fields, spammable) do
+ result = subject.send(:with_spam_action_response_fields, spammable) do
{ other_field: true }
end
expect(result)
diff --git a/spec/lib/quality/test_level_spec.rb b/spec/lib/quality/test_level_spec.rb
index 2232d47234f49f8d91305164c96924d11962b36a..32960cd571b0d5c0c5a13788983f8d5505dcc99d 100644
--- a/spec/lib/quality/test_level_spec.rb
+++ b/spec/lib/quality/test_level_spec.rb
@@ -28,7 +28,7 @@
context 'when level is unit' do
it 'returns a pattern' do
expect(subject.pattern(:unit))
- .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,models,policies,presenters,rack_servers,replicators,routing,rubocop,serializers,services,sidekiq,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb")
+ .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,models,policies,presenters,rack_servers,replicators,routing,rubocop,serializers,services,sidekiq,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb")
end
end
@@ -103,7 +103,7 @@
context 'when level is unit' do
it 'returns a regexp' do
expect(subject.regexp(:unit))
- .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|models|policies|presenters|rack_servers|replicators|routing|rubocop|serializers|services|sidekiq|support_specs|tasks|uploaders|validators|views|workers|tooling)})
+ .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|models|policies|presenters|rack_servers|replicators|routing|rubocop|serializers|services|sidekiq|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)})
end
end
diff --git a/spec/spam/concerns/has_spam_action_response_fields_spec.rb b/spec/spam/concerns/has_spam_action_response_fields_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..4d5f8d9d4311d7a7c07d65027d72713c946bee8a
--- /dev/null
+++ b/spec/spam/concerns/has_spam_action_response_fields_spec.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Spam::Concerns::HasSpamActionResponseFields do
+ subject do
+ klazz = Class.new
+ klazz.include described_class
+ klazz.new
+ end
+
+ describe '#with_spam_action_response_fields' do
+ let(:spam_log) { double(:spam_log, id: 1) }
+ let(:spammable) { double(:spammable, spam?: true, render_recaptcha?: true, spam_log: spam_log) }
+ let(:recaptcha_site_key) { 'abc123' }
+
+ before do
+ allow(Gitlab::CurrentSettings).to receive(:recaptcha_site_key) { recaptcha_site_key }
+ end
+
+ it 'merges in spam action fields from spammable' do
+ result = subject.send(:with_spam_action_response_fields, spammable) do
+ { other_field: true }
+ end
+ expect(result)
+ .to eq({
+ spam: true,
+ needs_captcha_response: true,
+ spam_log_id: 1,
+ captcha_site_key: recaptcha_site_key,
+ other_field: true
+ })
+ end
+ end
+end
diff --git a/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb b/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb
index d294f034d2eb6a09a760a52722526dc919ecfe2f..bb4270d7db6559722500582e0d867fbc7815041e 100644
--- a/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb
+++ b/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb
@@ -21,14 +21,14 @@
end
end
- describe "#with_spam_action_fields" do
+ describe "#with_spam_action_response_fields" do
it 'resolves with spam action fields' do
subject
# NOTE: We do not need to assert on the specific values of spam action fields here, we only need
- # to verify that #with_spam_action_fields was invoked and that the fields are present in the
- # response. The specific behavior of #with_spam_action_fields is covered in the
- # CanMutateSpammable unit tests.
+ # to verify that #with_spam_action_response_fields was invoked and that the fields are present in the
+ # response. The specific behavior of #with_spam_action_response_fields is covered in the
+ # HasSpamActionResponseFields unit tests.
expect(mutation_response.keys)
.to include('spam', 'spamLogId', 'needsCaptchaResponse', 'captchaSiteKey')
end