From ed2496c59daad4cc9f2087765132a108bbf64ec2 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Mon, 26 Feb 2024 12:08:12 +0800 Subject: [PATCH] FEATURE: Add scheduled Twitter login problem check - Part 1 (#25830) This PR adds a new scheduled problem check that simply tries to connect to Twitter OAuth endpoint to check that it's working. It is using the default retry strategy of 2 retries 30 seconds apart. --- config/locales/server.en.yml | 1 + lib/auth/twitter_authenticator.rb | 10 +++++ lib/problem_check.rb | 23 ++++++++++ lib/problem_check/twitter_login.rb | 25 +++++++++++ spec/jobs/problem_checks_spec.rb | 9 ++++ spec/lib/auth/twitter_authenticator_spec.rb | 33 ++++++++++++++ spec/lib/problem_check/twitter_login_spec.rb | 45 ++++++++++++++++++++ spec/lib/problem_check_spec.rb | 5 ++- 8 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 lib/problem_check/twitter_login.rb create mode 100644 spec/lib/problem_check/twitter_login_spec.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index da18d7136e1..2f3a9828e62 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1506,6 +1506,7 @@ en: description: "Top 10 users who have had likes from a wide range of people." dashboard: + twitter_login_warning: 'Twitter login appears to not be working at the moment. Check the credentials in the Site Settings.' group_email_credentials_warning: 'There was an issue with the email credentials for the group %{group_full_name}. No emails will be sent from the group inbox until this problem is addressed. %{error}' rails_env_warning: "Your server is running in %{env} mode." host_names_warning: "Your config/database.yml file is using the default localhost hostname. Update it to use your site's hostname." diff --git a/lib/auth/twitter_authenticator.rb b/lib/auth/twitter_authenticator.rb index 5817f617e35..b381209f38a 100644 --- a/lib/auth/twitter_authenticator.rb +++ b/lib/auth/twitter_authenticator.rb @@ -9,6 +9,16 @@ class Auth::TwitterAuthenticator < Auth::ManagedAuthenticator SiteSetting.enable_twitter_logins end + def healthy? + connection = + Faraday.new(url: "https://api.twitter.com") do |config| + config.basic_auth(SiteSetting.twitter_consumer_key, SiteSetting.twitter_consumer_secret) + end + connection.post("/oauth2/token").status == 200 + rescue Faraday::Error + false + end + def after_authenticate(auth_token, existing_account: nil) # Twitter sends a huge amount of data which we don't need, so ignore it auth_token[:extra] = {} diff --git a/lib/problem_check.rb b/lib/problem_check.rb index 8970240e375..fd053564ea4 100644 --- a/lib/problem_check.rb +++ b/lib/problem_check.rb @@ -3,6 +3,8 @@ class ProblemCheck include ActiveSupport::Configurable + config_accessor :priority, default: "low", instance_writer: false + # Determines if the check should be performed at a regular interval, and if # so how often. If left blank, the check will be performed every time the # admin dashboard is loaded, or the data is otherwise requested. @@ -50,4 +52,25 @@ class ProblemCheck def call raise NotImplementedError end + + private + + def problem + [ + Problem.new( + I18n.t(translation_key, base_path: Discourse.base_path), + priority: self.config.priority, + identifier:, + ), + ] + end + + def no_problem + [] + end + + def translation_key + # TODO: Infer a default based on class name, then move translations in locale file. + raise NotImplementedError + end end diff --git a/lib/problem_check/twitter_login.rb b/lib/problem_check/twitter_login.rb new file mode 100644 index 00000000000..c8df68c9e3b --- /dev/null +++ b/lib/problem_check/twitter_login.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class ProblemCheck::TwitterLogin < ProblemCheck + self.priority = "high" + + # TODO: Implement. + self.perform_every = 24.hours + + def call + return no_problem if !authenticator.enabled? + return no_problem if authenticator.healthy? + + problem + end + + private + + def authenticator + @authenticator ||= Auth::TwitterAuthenticator.new + end + + def translation_key + "dashboard.twitter_login_warning" + end +end diff --git a/spec/jobs/problem_checks_spec.rb b/spec/jobs/problem_checks_spec.rb index 2e1c30b6213..314e1968292 100644 --- a/spec/jobs/problem_checks_spec.rb +++ b/spec/jobs/problem_checks_spec.rb @@ -34,4 +34,13 @@ RSpec.describe Jobs::ProblemChecks do }, ) { described_class.new.execute([]) } end + + it "does not schedule non-scheduled checks" do + expect_not_enqueued_with( + job: :problem_check, + args: { + check_identifier: "non_scheduled_check", + }, + ) { described_class.new.execute([]) } + end end diff --git a/spec/lib/auth/twitter_authenticator_spec.rb b/spec/lib/auth/twitter_authenticator_spec.rb index 5052f4c831f..3473f8df8e3 100644 --- a/spec/lib/auth/twitter_authenticator_spec.rb +++ b/spec/lib/auth/twitter_authenticator_spec.rb @@ -65,4 +65,37 @@ RSpec.describe Auth::TwitterAuthenticator do expect(authenticator.description_for_user(user)).to eq("") end end + + describe "#healthy?" do + let(:authenticator) { described_class.new } + + let(:connection) { mock("Faraday::Connection") } + let(:response) { mock("Faraday::Response") } + + before do + Faraday.stubs(:new).returns(connection) + connection.stubs(:post).returns(response) + response.stubs(:status).returns(status) + end + + context "when endpoint is reachable" do + let(:status) { 200 } + + it { expect(authenticator).to be_healthy } + end + + context "when credentials aren't recognized" do + let(:status) { 403 } + + it { expect(authenticator).not_to be_healthy } + end + + context "when an unexpected error happens" do + let(:status) { anything } + + before { connection.stubs(:post).raises(Faraday::ServerError) } + + it { expect(authenticator).not_to be_healthy } + end + end end diff --git a/spec/lib/problem_check/twitter_login_spec.rb b/spec/lib/problem_check/twitter_login_spec.rb new file mode 100644 index 00000000000..0a94b6af620 --- /dev/null +++ b/spec/lib/problem_check/twitter_login_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +RSpec.describe ProblemCheck::TwitterLogin do + let(:problem_check) { described_class.new } + + let(:authenticator) { mock("Auth::TwitterAuthenticator") } + + before { Auth::TwitterAuthenticator.stubs(:new).returns(authenticator) } + + describe "#call" do + context "when Twitter authentication isn't enabled" do + before { authenticator.stubs(:enabled?).returns(false) } + + it { expect(problem_check.call).to be_empty } + end + + context "when Twitter authentication appears to work" do + before do + authenticator.stubs(:enabled?).returns(true) + authenticator.stubs(:healthy?).returns(true) + end + + it { expect(problem_check.call).to be_empty } + end + + context "when Twitter authentication appears not to work" do + before do + authenticator.stubs(:enabled?).returns(true) + authenticator.stubs(:healthy?).returns(false) + Discourse.stubs(:base_path).returns("foo.bar") + end + + it do + expect(problem_check.call).to contain_exactly( + have_attributes( + identifier: :twitter_login, + priority: "high", + message: + 'Twitter login appears to not be working at the moment. Check the credentials in the Site Settings.', + ), + ) + end + end + end +end diff --git a/spec/lib/problem_check_spec.rb b/spec/lib/problem_check_spec.rb index 9df52fb3a62..b608d09cdc2 100644 --- a/spec/lib/problem_check_spec.rb +++ b/spec/lib/problem_check_spec.rb @@ -26,11 +26,12 @@ RSpec.describe ProblemCheck do end describe ".checks" do - it { expect(described_class.checks).to contain_exactly(scheduled_check, unscheduled_check) } + it { expect(described_class.checks).to include(scheduled_check, unscheduled_check) } end describe ".scheduled" do - it { expect(described_class.scheduled).to contain_exactly(scheduled_check) } + it { expect(described_class.scheduled).to include(scheduled_check) } + it { expect(described_class.scheduled).not_to include(unscheduled_check) } end describe ".scheduled?" do