From 6ebadaed2c120e2bd57fe46d693cdb42eeef6e98 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Tue, 2 Apr 2019 10:26:27 -0400
Subject: [PATCH] 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.
---
config/locales/server.en.yml | 1 +
config/site_settings.yml | 1 +
.../20190402142223_disable_invite_only_sso.rb | 9 ++++++
.../enable_invite_only_validator.rb | 14 +++++++++
lib/validators/enable_sso_validator.rb | 6 ++--
.../enable_invite_only_validator_spec.rb | 31 +++++++++++++++++++
.../validators/enable_sso_validator_spec.rb | 18 +++++++++++
7 files changed, 78 insertions(+), 2 deletions(-)
create mode 100644 db/migrate/20190402142223_disable_invite_only_sso.rb
create mode 100644 lib/validators/enable_invite_only_validator.rb
create mode 100644 spec/components/validators/enable_invite_only_validator_spec.rb
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 0376302e944..a248c8dff20 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 60d4f2eff1c..cfe68449084 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 00000000000..abeab5cb4bd
--- /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 00000000000..d9101c309a8
--- /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 075798654f7..5204bb5b863 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 00000000000..39a8e017861
--- /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 d60c7b1b756..c78bbb976cf 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"