FIX: Do not allow `invite_only` and `enable_sso` at the same time

FIX: Do not allow invite_only and enable_sso at the same time

This functionality was never supported but before the new review queue it didn’t have any errors. Now the combination of settings is prevented and existing sites with sso enabled will be migrated to remove invite only.

diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 0376302..a248c8d 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2033,6 +2033,7 @@ en:
       staged_users_disabled: "You must first enable 'staged users' before enabling this setting."
       reply_by_email_disabled: "You must first enable 'reply by email' before enabling this setting."
       sso_url_is_empty: "You must set a 'sso url' before enabling this setting."
+      sso_invite_only: "You cannot enable sso and invite only at the same time."
       enable_local_logins_disabled: "You must first enable 'enable local logins' before enabling this setting."
       min_username_length_exists: "You cannot set the minimum username length above the shortest username (%{username})."
       min_username_length_range: "You cannot set the minimum above the maximum."
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 60d4f2e..cfe6844 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -304,6 +304,7 @@ login:
     refresh: true
     client: true
     default: false
+    validator: "EnableInviteOnlyValidator"
   login_required:
     refresh: true
     client: true
diff --git a/db/migrate/20190402142223_disable_invite_only_sso.rb b/db/migrate/20190402142223_disable_invite_only_sso.rb
new file mode 100644
index 0000000..abeab5c
--- /dev/null
+++ b/db/migrate/20190402142223_disable_invite_only_sso.rb
@@ -0,0 +1,9 @@
+class DisableInviteOnlySso < ActiveRecord::Migration[5.2]
+  def up
+    execute(<<~SQL)
+      UPDATE site_settings SET value = 'f'
+      WHERE name = 'invite_only'
+        AND EXISTS(SELECT 1 FROM site_settings WHERE name = 'enable_sso' AND value = 't')
+    SQL
+  end
+end
diff --git a/lib/validators/enable_invite_only_validator.rb b/lib/validators/enable_invite_only_validator.rb
new file mode 100644
index 0000000..d9101c3
--- /dev/null
+++ b/lib/validators/enable_invite_only_validator.rb
@@ -0,0 +1,14 @@
+class EnableInviteOnlyValidator
+  def initialize(opts = {})
+    @opts = opts
+  end
+
+  def valid_value?(val)
+    return true if val == 'f'
+    !SiteSetting.enable_sso?
+  end
+
+  def error_message
+    I18n.t('site_settings.errors.sso_invite_only')
+  end
+end
diff --git a/lib/validators/enable_sso_validator.rb b/lib/validators/enable_sso_validator.rb
index 0757986..5204bb5 100644
--- a/lib/validators/enable_sso_validator.rb
+++ b/lib/validators/enable_sso_validator.rb
@@ -5,10 +5,12 @@ class EnableSsoValidator
 
   def valid_value?(val)
     return true if val == 'f'
-    SiteSetting.sso_url.present?
+    return false if SiteSetting.sso_url.blank? || SiteSetting.invite_only?
+    true
   end
 
   def error_message
-    I18n.t('site_settings.errors.sso_url_is_empty')
+    return I18n.t('site_settings.errors.sso_url_is_empty') if SiteSetting.sso_url.blank?
+    return I18n.t('site_settings.errors.sso_invite_only') if SiteSetting.invite_only?
   end
 end
diff --git a/spec/components/validators/enable_invite_only_validator_spec.rb b/spec/components/validators/enable_invite_only_validator_spec.rb
new file mode 100644
index 0000000..39a8e01
--- /dev/null
+++ b/spec/components/validators/enable_invite_only_validator_spec.rb
@@ -0,0 +1,31 @@
+require 'rails_helper'
+
+RSpec.describe EnableInviteOnlyValidator do
+  subject { described_class.new }
+
+  context "when sso is enabled" do
+    before do
+      SiteSetting.sso_url = "https://example.com/sso"
+      SiteSetting.enable_sso = true
+    end
+
+    it "is valid when false" do
+      expect(subject.valid_value?('f')).to eq(true)
+    end
+
+    it "is isn't value for true" do
+      expect(subject.valid_value?('t')).to eq(false)
+      expect(subject.error_message).to eq(I18n.t(
+        'site_settings.errors.sso_invite_only'
+      ))
+    end
+  end
+
+  context "when sso isn't enabled" do
+    it "is valid when true or false" do
+      expect(subject.valid_value?('f')).to eq(true)
+      expect(subject.valid_value?('t')).to eq(true)
+    end
+  end
+
+end
diff --git a/spec/components/validators/enable_sso_validator_spec.rb b/spec/components/validators/enable_sso_validator_spec.rb
index d60c7b1..c78bbb9 100644
--- a/spec/components/validators/enable_sso_validator_spec.rb
+++ b/spec/components/validators/enable_sso_validator_spec.rb
@@ -26,6 +26,24 @@ RSpec.describe EnableSsoValidator do
       end
     end
 
+    describe "when invite_only is set" do
+      before do
+        SiteSetting.invite_only = true
+        SiteSetting.sso_url = 'https://example.com/sso'
+      end
+
+      it 'allows a false value' do
+        expect(subject.valid_value?('f')).to eq(true)
+      end
+
+      it "doesn't allow true" do
+        expect(subject.valid_value?('t')).to eq(false)
+        expect(subject.error_message).to eq(I18n.t(
+          'site_settings.errors.sso_invite_only'
+        ))
+      end
+    end
+
     describe "when 'sso url' is present" do
       before do
         SiteSetting.sso_url = "https://www.example.com/sso"

GitHub sha: 6ebadaed

This commit has been mentioned on Discourse Meta. There might be relevant details there:

This commit has been mentioned on Discourse Meta. There might be relevant details there: