diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 852d09459..85c67b6bf 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/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']}", diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb new file mode 100644 index 000000000..550ccdede --- /dev/null +++ b/config/initializers/omniauth.rb @@ -0,0 +1 @@ +OmniAuth.config.logger = Rails.logger diff --git a/spec/system/user_logs_in_with_twitter_spec.rb b/spec/system/user_logs_in_with_twitter_spec.rb index 926bf008c..4ac4570a9 100644 --- a/spec/system/user_logs_in_with_twitter_spec.rb +++ b/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 + # 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