Fix Datadog auth failure payload and add tests (#7150)

* Fix Datadog auth failure payload and add tests

* Use functional Ruby properly

app/controllers/omniauth_callbacks_controller.rb

@@ -14,15 +14,16 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
 
   def failure
     error = request.env["omniauth.error"]
+    class_name = error.present? ? error.class.name : ""
 
     DatadogStatsClient.increment(
       "omniauth.failure",
       tags: [
-        "class:#{error}",
+        "class:#{class_name}",
         "message:#{error&.message}",
-        "reason:#{error&.error_reason}",
-        "type:#{error&.error}",
-        "uri:#{error&.error_uri}",
+        "reason:#{error.try(:error_reason)}",
+        "type:#{error.try(:error)}",
+        "uri:#{error.try(:error_uri)}",
         "provider:#{request.env['omniauth.strategy'].name}",
         "origin:#{request.env['omniauth.strategy.origin']}",
         "params:#{request.env['omniauth.params']}",

config/initializers/omniauth.rb

@@ -0,0 +1 @@
+OmniAuth.config.logger = Rails.logger

spec/system/user_logs_in_with_twitter_spec.rb

@@ -1,14 +1,51 @@
 require "rails_helper"
 
-def user_grants_authorization_on_twitter_popup(twitter_callback_hash)
+global_failure_handler = OmniAuth.config.on_failure
+
+def user_grants_authorization_on_twitter(twitter_callback_hash)
   OmniAuth.config.add_mock(:twitter, twitter_callback_hash)
 end
 
-def user_do_not_grants_authorization_on_twitter_popup
+def user_does_not_grant_authorization_on_twitter
   OmniAuth.config.mock_auth[:twitter] = :invalid_credentials
 end
 
-RSpec.describe "Authenticating with twitter" do
+def setup_omniauth_error(error)
+  # this hack is needed due to a limitation in how OmniAuth handles
+  # failures in mocked/testing environments,
+  # see <https://github.com/omniauth/omniauth/issues/654#issuecomment-610851884>
+  # for more details
+  global_failure_handler = OmniAuth.config.on_failure
+
+  local_failure_handler = lambda do |env|
+    env["omniauth.error"] = error
+    env
+  end
+  # here we compose the two handlers into a single function,
+  # the result will be global_failure_handler(local_failure_handler(env))
+  failure_handler = local_failure_handler >> global_failure_handler
+
+  OmniAuth.config.on_failure = failure_handler
+end
+
+def get_failure_args(error)
+  class_name = error.present? ? error.class.name : ""
+
+  [
+    tags: [
+      "class:#{class_name}",
+      "message:#{error&.message}",
+      "reason:#{error.try(:error_reason)}",
+      "type:#{error.try(:error)}",
+      "uri:#{error.try(:error_uri)}",
+      "provider:twitter",
+      "origin:",
+      "params:{}",
+    ],
+  ]
+end
+
+RSpec.describe "Authenticating with Twitter" do
   let(:twitter_callback_hash) do
     {
       provider: "twitter",
@@ -32,52 +69,105 @@ RSpec.describe "Authenticating with twitter" do
     }
   end
 
-  default_logger = Rails.logger
+  context "when a user is new" do
+    let(:twitter_link) { "Sign In With Twitter" }
 
-  # Override the default Rails logger as these tests require the Timber logger.
-  before do
-    timber_logger = Timber::Logger.new(nil)
-    Rails.logger = ActiveSupport::TaggedLogging.new(timber_logger)
-  end
+    context "when using valid credentials" do
+      before do
+        user_grants_authorization_on_twitter(twitter_callback_hash)
+      end
 
-  after { Rails.logger = default_logger }
+      it "logs in and redirects to the onboarding" do
+        visit root_path
+        click_link twitter_link
 
-  context "when user is new on dev.to" do
-    it "logging in with twitter using valid credentials" do
-      user_grants_authorization_on_twitter_popup(twitter_callback_hash)
-
-      visit root_path
-      click_link "Sign In With Twitter"
-
-      expect(page.html).to include("onboarding-container")
+        expect(page).to have_current_path("/onboarding?referrer=none")
+        expect(page.html).to include("onboarding-container")
+      end
     end
 
-    it "logging in with twitter using invalid credentials" do
-      allow(DatadogStatsClient).to receive(:increment)
+    context "when using invalid credentials" do
+      let(:callback_failure) do
+        [
+          "omniauth.failure",
+          tags: [
+            "class:",
+            "message:",
+            "reason:",
+            "type:",
+            "uri:",
+            "provider:twitter",
+            "origin:",
+            "params:{}",
+          ],
+        ]
+      end
 
-      user_do_not_grants_authorization_on_twitter_popup
+      before do
+        user_does_not_grant_authorization_on_twitter
 
-      visit root_path
-      click_link "Sign In With Twitter"
+        allow(DatadogStatsClient).to receive(:increment)
+      end
 
-      expect(page).to have_link "Sign In/Up"
-      expect(page).to have_link "Via Twitter"
-      expect(page).to have_link "Via GitHub"
-      expect(page).to have_link "All about #{ApplicationConfig['COMMUNITY_NAME']}"
+      after do
+        OmniAuth.config.on_failure = global_failure_handler
+      end
 
-      expect(DatadogStatsClient).to have_received(:increment).with(
-        "omniauth.failure",
-        tags: [
-          "class:",
-          "message:",
-          "reason:",
-          "type:",
-          "uri:",
-          "provider:twitter",
-          "origin:",
-          "params:{}",
-        ],
-      )
+      it "does not log in" do
+        visit root_path
+        click_link twitter_link
+
+        expect(page).to have_current_path("/users/sign_in")
+        expect(page).to have_link("Sign In/Up")
+        expect(page).to have_link("Via Twitter")
+        expect(page).to have_link("Via GitHub")
+        expect(page).to have_link("All about #{ApplicationConfig['COMMUNITY_NAME']}")
+      end
+
+      it "notifies Datadog about a callback error" do
+        error = OmniAuth::Strategies::OAuth2::CallbackError.new(
+          "Callback error", "Error reason", "https://example.com/error"
+        )
+
+        setup_omniauth_error(error)
+
+        visit root_path
+        click_link twitter_link
+
+        args = get_failure_args(error)
+        expect(DatadogStatsClient).to have_received(:increment).with(
+          "omniauth.failure", *args
+        )
+      end
+
+      it "notifies Datadog about an OAuth unauthorized error" do
+        request = double
+        allow(request).to receive(:code).and_return(401)
+        allow(request).to receive(:message).and_return("unauthorized")
+        error = OAuth::Unauthorized.new(request)
+        setup_omniauth_error(error)
+
+        visit root_path
+        click_link twitter_link
+
+        args = get_failure_args(error)
+        expect(DatadogStatsClient).to have_received(:increment).with(
+          "omniauth.failure", *args
+        )
+      end
+
+      it "notifies Datadog even with no OmniAuth error present" do
+        error = nil
+        setup_omniauth_error(error)
+
+        visit root_path
+        click_link twitter_link
+
+        args = get_failure_args(error)
+        expect(DatadogStatsClient).to have_received(:increment).with(
+          "omniauth.failure", *args
+        )
+      end
     end
   end
 end