From 4326827a4ee7d05c4e70925311fede4b4a25a09f Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Fri, 9 Oct 2020 22:36:38 +0530 Subject: [PATCH] FIX: second factor cannot be enabled if SSO is enabled (#10880) * FIX: second factor cannot be enabled if SSO is enabled If `enable_sso` setting is enabled then admin should not be able to enable `enforce_second_factor` setting as that will lock users out. Co-authored-by: Robin Ward --- config/locales/server.en.yml | 1 + lib/site_settings/validations.rb | 3 +++ spec/lib/site_settings/validations_spec.rb | 12 ++++++++++++ 3 files changed, 16 insertions(+) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 6d0ccad7d54..1cbf744a1bb 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -196,6 +196,7 @@ en: share_quote_facebook_requirements: "You must set a Facebook app id to enable quote sharing for Facebook." second_factor_cannot_enforce_with_socials: "You cannot enforce 2FA with social logins enabled. You must first disable login via: %{auth_provider_names}" second_factor_cannot_be_enforced_with_disabled_local_login: "You cannot enforce 2FA if local logins are disabled." + second_factor_cannot_be_enforced_with_sso_enabled: "You cannot enforce 2FA if SSO is enabled." local_login_cannot_be_disabled_if_second_factor_enforced: "You cannot disable local login if 2FA is enforced. Disable enforced 2FA before disabling local logins." cannot_enable_s3_uploads_when_s3_enabled_globally: "You cannot enable S3 uploads because S3 uploads are already globally enabled, and enabling this site-level could cause critical issues with uploads" conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to
https://meta.discourse.org/t/76575' diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb index c49b83910b0..6a1f9958c0e 100644 --- a/lib/site_settings/validations.rb +++ b/lib/site_settings/validations.rb @@ -173,6 +173,9 @@ module SiteSettings::Validations end def validate_enforce_second_factor(new_val) + if SiteSetting.enable_sso? + return validate_error :second_factor_cannot_be_enforced_with_sso_enabled + end if new_val == "all" && Discourse.enabled_auth_providers.count > 0 auth_provider_names = Discourse.enabled_auth_providers.map(&:name).join(", ") return validate_error(:second_factor_cannot_enforce_with_socials, auth_provider_names: auth_provider_names) diff --git a/spec/lib/site_settings/validations_spec.rb b/spec/lib/site_settings/validations_spec.rb index 420e8f45cb9..63aeaa1252a 100644 --- a/spec/lib/site_settings/validations_spec.rb +++ b/spec/lib/site_settings/validations_spec.rb @@ -149,6 +149,18 @@ describe SiteSettings::Validations do expect { subject.validate_enforce_second_factor("all") }.to raise_error(Discourse::InvalidParameters, error_message) end end + + context "when SSO is enabled" do + let(:error_message) { I18n.t("errors.site_settings.second_factor_cannot_be_enforced_with_sso_enabled") } + before do + SiteSetting.sso_url = "https://www.example.com/sso" + SiteSetting.enable_sso = true + end + + it "should raise an error" do + expect { subject.validate_enforce_second_factor("t") }.to raise_error(Discourse::InvalidParameters, error_message) + end + end end describe "#validate_enable_local_logins" do