diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 0cfc12445..a98da15da 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -55,7 +55,7 @@ class Api::V1::AccountsController < Api::BaseController end def check_signup_enabled - raise ActionController::RoutingError, 'Not Found' if ENV.fetch('ENABLE_ACCOUNT_SIGNUP', true) == 'false' + raise ActionController::RoutingError, 'Not Found' if GlobalConfig.get_value('ENABLE_ACCOUNT_SIGNUP') == 'false' end def pundit_user diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index d00aafc18..eb7a17caa 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -27,7 +27,8 @@ class DashboardController < ActionController::Base 'ANALYTICS_TOKEN', 'ANALYTICS_HOST' ).merge( - APP_VERSION: Chatwoot.config[:version] + APP_VERSION: Chatwoot.config[:version], + ENABLE_ACCOUNT_SIGNUP: GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') ) end diff --git a/app/views/layouts/vueapp.html.erb b/app/views/layouts/vueapp.html.erb index 808b9c15a..4e4a3d971 100644 --- a/app/views/layouts/vueapp.html.erb +++ b/app/views/layouts/vueapp.html.erb @@ -34,7 +34,7 @@ window.chatwootConfig = { hostURL: '<%= ENV.fetch('FRONTEND_URL', '') %>', fbAppId: '<%= ENV.fetch('FB_APP_ID', nil) %>', - signupEnabled: '<%= ENV.fetch('ENABLE_ACCOUNT_SIGNUP', true) %>', + signupEnabled: '<%= @global_config['ENABLE_ACCOUNT_SIGNUP'] %>', <% if ENV['VAPID_PUBLIC_KEY'] %> vapidPublicKey: new Uint8Array(<%= Base64.urlsafe_decode64(ENV['VAPID_PUBLIC_KEY']).bytes %>), <% end %> diff --git a/lib/global_config.rb b/lib/global_config.rb index 50bca4b13..5c94e4c0c 100644 --- a/lib/global_config.rb +++ b/lib/global_config.rb @@ -15,6 +15,10 @@ class GlobalConfig config.with_indifferent_access end + def get_value(arg) + load_from_cache(arg) + end + def clear_cache cached_keys = $alfred.keys("#{VERSION}:#{KEY_PREFIX}:*") (cached_keys || []).each do |cached_key| diff --git a/lib/global_config_service.rb b/lib/global_config_service.rb new file mode 100644 index 000000000..6eb700d58 --- /dev/null +++ b/lib/global_config_service.rb @@ -0,0 +1,17 @@ +class GlobalConfigService + def self.load(config_key, default_value) + config = GlobalConfig.get(config_key) + return config[config_key] if config[config_key].present? + + # To support migrating existing instance relying on env variables + # TODO: deprecate this later down the line + config_value = ENV[config_key] || default_value + + return if config_value.blank? + + i = InstallationConfig.where(name: config_key).first_or_create(value: config_value) + # To clear a nil value that might have been cached in the previous call + GlobalConfig.clear_cache + i.value + end +end diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 810515a03..687e868a7 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -5,6 +5,11 @@ RSpec.describe 'Accounts API', type: :request do let(:email) { Faker::Internet.email } let(:user_full_name) { Faker::Name.name_with_middle } + before do + # to clear redis cache + GlobalConfig.clear_cache + end + context 'when posting to accounts with correct parameters' do let(:account_builder) { double } let(:account) { create(:account) } @@ -15,62 +20,56 @@ RSpec.describe 'Accounts API', type: :request do end it 'calls account builder' do - with_modified_env ENABLE_ACCOUNT_SIGNUP: nil do - allow(account_builder).to receive(:perform).and_return([user, account]) + GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'nil') + allow(account_builder).to receive(:perform).and_return([user, account]) - params = { account_name: 'test', email: email, user: nil, user_full_name: user_full_name, password: 'Password1!' } + params = { account_name: 'test', email: email, user: nil, user_full_name: user_full_name, password: 'Password1!' } - post api_v1_accounts_url, - params: params, - as: :json + post api_v1_accounts_url, + params: params, + as: :json - expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password])) - expect(account_builder).to have_received(:perform) - expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid') - end + expect(AccountBuilder).to have_received(:new).with(params.except(:password).merge(user_password: params[:password])) + expect(account_builder).to have_received(:perform) + expect(response.headers.keys).to include('access-token', 'token-type', 'client', 'expiry', 'uid') end it 'renders error response on invalid params' do - with_modified_env ENABLE_ACCOUNT_SIGNUP: nil do - allow(account_builder).to receive(:perform).and_return(nil) + GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'nil') + allow(account_builder).to receive(:perform).and_return(nil) - params = { account_name: nil, email: nil, user: nil, user_full_name: nil } + params = { account_name: nil, email: nil, user: nil, user_full_name: nil } - post api_v1_accounts_url, - params: params, - as: :json + post api_v1_accounts_url, + params: params, + as: :json - expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password])) - expect(account_builder).to have_received(:perform) - expect(response).to have_http_status(:forbidden) - expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json) - end + expect(AccountBuilder).to have_received(:new).with(params.merge(user_password: params[:password])) + expect(account_builder).to have_received(:perform) + expect(response).to have_http_status(:forbidden) + expect(response.body).to eq({ message: I18n.t('errors.signup.failed') }.to_json) end end context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to false' do it 'responds 404 on requests' do - params = { account_name: 'test', email: email, user_full_name: user_full_name } - with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do - post api_v1_accounts_url, - params: params, - as: :json - - expect(response).to have_http_status(:not_found) - end + params = { account_name: 'test', email: email, user_full_name: user_full_name, password: 'Password1!' } + GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') + post api_v1_accounts_url, + params: params, + as: :json + expect(response).to have_http_status(:not_found) end end context 'when ENABLE_ACCOUNT_SIGNUP env variable is set to api_only' do it 'does not respond 404 on requests' do params = { account_name: 'test', email: email, user_full_name: user_full_name, password: 'Password1!' } - with_modified_env ENABLE_ACCOUNT_SIGNUP: 'api_only' do - post api_v1_accounts_url, - params: params, - as: :json - - expect(response).to have_http_status(:success) - end + GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'api_only') + post api_v1_accounts_url, + params: params, + as: :json + expect(response).to have_http_status(:success) end end end diff --git a/spec/lib/global_config_service_spec.rb b/spec/lib/global_config_service_spec.rb new file mode 100644 index 000000000..d7a6680c8 --- /dev/null +++ b/spec/lib/global_config_service_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +describe GlobalConfigService do + subject(:trigger) { described_class } + + describe 'execute' do + context 'when called with default options' do + before do + # to clear redis cache + GlobalConfig.clear_cache + end + + it 'set default value if not found on db nor env var' do + value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP') + expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq nil + + described_class.load('ENABLE_ACCOUNT_SIGNUP', 'true') + + value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP') + expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true' + expect(InstallationConfig.find_by(name: 'ENABLE_ACCOUNT_SIGNUP')&.value).to eq 'true' + end + + it 'get value from env variable if not found on DB' do + with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do + expect(InstallationConfig.find_by(name: 'ENABLE_ACCOUNT_SIGNUP')&.value).to eq nil + described_class.load('ENABLE_ACCOUNT_SIGNUP', 'true') + value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP') + expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'false' + end + end + + it 'get value from DB if found' do + # Set a value in db first and make sure this value + # is not respected even when load() method is called with + # another value. + InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').first_or_create(value: 'true') + described_class.load('ENABLE_ACCOUNT_SIGNUP', 'false') + value = GlobalConfig.get('ENABLE_ACCOUNT_SIGNUP') + expect(value['ENABLE_ACCOUNT_SIGNUP']).to eq 'true' + end + end + end +end