diff --git a/app/services/settings/authentication/upsert.rb b/app/services/settings/authentication/upsert.rb index 114947df3..31f84e49b 100644 --- a/app/services/settings/authentication/upsert.rb +++ b/app/services/settings/authentication/upsert.rb @@ -11,9 +11,7 @@ module Settings end def self.update_enabled_providers(value) - return if value.blank? - - enabled_providers = value.split(",").filter_map do |entry| + enabled_providers = value.to_s.split(",").filter_map do |entry| entry unless invalid_provider_entry(entry) end return if email_login_disabled_with_one_or_less_auth_providers(enabled_providers) diff --git a/spec/services/settings/authentication/upsert_spec.rb b/spec/services/settings/authentication/upsert_spec.rb new file mode 100644 index 000000000..458253fca --- /dev/null +++ b/spec/services/settings/authentication/upsert_spec.rb @@ -0,0 +1,77 @@ +require "rails_helper" + +RSpec.describe Settings::Authentication::Upsert, type: :service do + before { Settings::Authentication.providers = %w[github] } + + it "assigns enabled providers from parameters" do + expect do + described_class.call( + { + "auth_providers_to_enable" => "github,facebook,twitter", + "github_key" => "asdf", + "github_secret" => "asdf_secret", + "twitter_key" => "asdf", + "twitter_secret" => "asdf_secret", + "facebook_key" => "asdf", + "facebook_secret" => "asdf_secret" + }, + ) + end.to change { + Settings::Authentication.providers + }.from(%w[github]).to(%w[github facebook twitter]) + end + + it "disables providers that are not present" do + expect do + described_class.call( + { + "auth_providers_to_enable" => "twitter", + "twitter_key" => "asdf", + "twitter_secret" => "asdf_secret" + }, + ) + end.to change { + Settings::Authentication.providers + }.from(%w[github]).to(%w[twitter]) + end + + it "does not save 1 or fewer providers when email_password login is not allowed" do + expect do + Settings::Authentication.allow_email_password_login = false + described_class.call( + { + "auth_providers_to_enable" => "twitter", + "twitter_key" => "asdf", + "twitter_secret" => "asdf_secret" + }, + ) + end.not_to change { + Settings::Authentication.providers + } + end + + it "will save with 1 or providers providers when email_password login is not allowed" do + expect do + Settings::Authentication.allow_email_password_login = false + described_class.call( + { + "auth_providers_to_enable" => "twitter,github", + "twitter_key" => "asdf", + "twitter_secret" => "asdf_secret", + "github_key" => "asdf", + "github_secret" => "asdf_secret" + }, + ) + end.to change { + Settings::Authentication.providers + }.from(%w[github]).to(%w[twitter github]) + end + + it "disables providers even when provider parameter is blank" do + expect do + described_class.call({ "auth_providers_to_enable" => "" }) + end.to change { + Settings::Authentication.providers + }.from(%w[github]).to([]) + end +end