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.
This commit is contained in:
Robin Ward 2019-04-02 10:26:27 -04:00
parent af04318aff
commit 6ebadaed2c
7 changed files with 78 additions and 2 deletions

@ -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."

@ -304,6 +304,7 @@ login:
refresh: true
client: true
default: false
validator: "EnableInviteOnlyValidator"
login_required:
refresh: true
client: true

@ -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

@ -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

@ -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

@ -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

@ -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"