From 9f376c43b5b782425a486ef62bf8884ea896a0ef Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 10 Mar 2026 16:35:09 +0530 Subject: [PATCH] fix(signup): normalize account signup config checks (#13745) This makes account signup enforcement consistent when signup is disabled at the installation level. Email signup and Google signup now stay blocked regardless of whether the config value is stored as a string or a boolean. This effectively covers the config-loader path, where `YAML.safe_load` reads `value: false` from `installation_config.yml` as a native boolean and persists it that way. - Normalized the account signup check so disabled signup is handled consistently across config value types. - Reused the same check across API signup and Google signup entry points. - Added regression coverage for the disabled-signup cases in the existing controller specs. --------- Co-authored-by: Vishnu Narayanan --- app/controllers/api/v1/accounts_controller.rb | 2 +- app/controllers/api/v2/accounts_controller.rb | 2 +- .../omniauth_callbacks_controller.rb | 3 +-- lib/global_config_service.rb | 4 ++++ .../api/v1/accounts_controller_spec.rb | 23 +++++++++++++++++++ .../api/v2/accounts_controller_spec.rb | 23 +++++++++++++++++++ .../omniauth_callbacks_controller_spec.rb | 20 ++++++++++++++++ 7 files changed, 73 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index bcbf80355..3e513a4b2 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -100,7 +100,7 @@ class Api::V1::AccountsController < Api::BaseController end def check_signup_enabled - raise ActionController::RoutingError, 'Not Found' if GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') == 'false' + raise ActionController::RoutingError, 'Not Found' unless GlobalConfigService.account_signup_enabled? end def validate_captcha diff --git a/app/controllers/api/v2/accounts_controller.rb b/app/controllers/api/v2/accounts_controller.rb index bed0a212a..5a19ddeed 100644 --- a/app/controllers/api/v2/accounts_controller.rb +++ b/app/controllers/api/v2/accounts_controller.rb @@ -58,7 +58,7 @@ class Api::V2::AccountsController < Api::BaseController end def check_signup_enabled - raise ActionController::RoutingError, 'Not Found' if GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') == 'false' + raise ActionController::RoutingError, 'Not Found' unless GlobalConfigService.account_signup_enabled? end def validate_captcha diff --git a/app/controllers/devise_overrides/omniauth_callbacks_controller.rb b/app/controllers/devise_overrides/omniauth_callbacks_controller.rb index 900125670..af759af54 100644 --- a/app/controllers/devise_overrides/omniauth_callbacks_controller.rb +++ b/app/controllers/devise_overrides/omniauth_callbacks_controller.rb @@ -51,8 +51,7 @@ class DeviseOverrides::OmniauthCallbacksController < DeviseTokenAuth::OmniauthCa end def account_signup_allowed? - # set it to true by default, this is the behaviour across the app - GlobalConfigService.load('ENABLE_ACCOUNT_SIGNUP', 'false') != 'false' + GlobalConfigService.account_signup_enabled? end def resource_class(_mapping = nil) diff --git a/lib/global_config_service.rb b/lib/global_config_service.rb index 0649c24af..31612a240 100644 --- a/lib/global_config_service.rb +++ b/lib/global_config_service.rb @@ -14,4 +14,8 @@ class GlobalConfigService GlobalConfig.clear_cache i.value end + + def self.account_signup_enabled? + load('ENABLE_ACCOUNT_SIGNUP', 'false').to_s != 'false' + end end diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index ec49ecd39..d773cafa7 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -81,6 +81,29 @@ RSpec.describe 'Accounts API', type: :request do end end + context 'when ENABLE_ACCOUNT_SIGNUP is stored as boolean false' do + before do + GlobalConfig.clear_cache + InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').delete_all + InstallationConfig.create!(name: 'ENABLE_ACCOUNT_SIGNUP', value: false, locked: false) + end + + after do + InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').delete_all + GlobalConfig.clear_cache + end + + it 'responds 404 on requests' do + params = { account_name: 'test', email: email, user_full_name: user_full_name, password: 'Password1!' } + + 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!' } diff --git a/spec/controllers/api/v2/accounts_controller_spec.rb b/spec/controllers/api/v2/accounts_controller_spec.rb index 182ebadac..a39e37a91 100644 --- a/spec/controllers/api/v2/accounts_controller_spec.rb +++ b/spec/controllers/api/v2/accounts_controller_spec.rb @@ -94,6 +94,29 @@ RSpec.describe 'Accounts API', type: :request do end end + context 'when ENABLE_ACCOUNT_SIGNUP is stored as boolean false' do + before do + GlobalConfig.clear_cache + InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').delete_all + InstallationConfig.create!(name: 'ENABLE_ACCOUNT_SIGNUP', value: false, locked: false) + end + + after do + InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').delete_all + GlobalConfig.clear_cache + end + + it 'responds 404 on requests' do + params = { email: email, password: 'Password1!' } + + post api_v2_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 let(:account_builder) { double } let(:account) { create(:account) } diff --git a/spec/controllers/devise/omniauth_callbacks_controller_spec.rb b/spec/controllers/devise/omniauth_callbacks_controller_spec.rb index 1a775f88f..603458a01 100644 --- a/spec/controllers/devise/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/devise/omniauth_callbacks_controller_spec.rb @@ -106,6 +106,26 @@ RSpec.describe 'DeviseOverrides::OmniauthCallbacksController', type: :request do end end + it 'blocks signup if config is stored as boolean false' do + GlobalConfig.clear_cache + InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').delete_all + InstallationConfig.create!(name: 'ENABLE_ACCOUNT_SIGNUP', value: false, locked: false) + + with_modified_env FRONTEND_URL: 'http://www.example.com' do + set_omniauth_config('does-not-exist-for-sure@example.com') + allow(email_validation_service).to receive(:perform).and_return(true) + + get '/omniauth/google_oauth2/callback' + + expect(response).to redirect_to('http://www.example.com/auth/google_oauth2/callback') + follow_redirect! + expect(response).to redirect_to(%r{/app/login\?error=no-account-found$}) + end + ensure + InstallationConfig.where(name: 'ENABLE_ACCOUNT_SIGNUP').delete_all + GlobalConfig.clear_cache + end + it 'allows login' do with_modified_env FRONTEND_URL: 'http://www.example.com' do create(:user, email: 'test@example.com')