fix: account email validation during signup (#11307)

- Refactor email validation logic to be a service
- Use the service for both email/pass signup and Google SSO
- fix account email validation during signup
- Use `blocked_domain` setting for both email/pass signup and Google
Sign In [`BLOCKED_DOMAIN` via GlobalConfig]
- add specs for `account_builder`
- add specs for the new service

---------

Co-authored-by: Sojan Jose <sojan@pepalo.com>

app/builders/account_builder.rb

@@ -32,14 +32,7 @@ class AccountBuilder
   end
 
   def validate_email
-    raise InvalidEmail.new({ domain_blocked: domain_blocked }) if domain_blocked?
-
-    address = ValidEmail2::Address.new(@email)
-    if address.valid? && !address.disposable?
-      true
-    else
-      raise InvalidEmail.new({ valid: address.valid?, disposable: address.disposable? })
-    end
+    Account::SignUpEmailValidationService.new(@email).perform
   end
 
   def validate_user
@@ -81,21 +74,4 @@ class AccountBuilder
     @user.confirm if @confirmed
     @user.save!
   end
-
-  def domain_blocked?
-    domain = @email.split('@').last
-
-    blocked_domains.each do |blocked_domain|
-      return true if domain.match?(blocked_domain)
-    end
-
-    false
-  end
-
-  def blocked_domains
-    domains = GlobalConfigService.load('BLOCKED_EMAIL_DOMAINS', '')
-    return [] if domains.blank?
-
-    domains.split("\n").map(&:strip)
-  end
 end

app/controllers/devise_overrides/omniauth_callbacks_controller.rb

@@ -21,7 +21,7 @@ class DeviseOverrides::OmniauthCallbacksController < DeviseTokenAuth::OmniauthCa
 
   def sign_up_user
     return redirect_to login_page_url(error: 'no-account-found') unless account_signup_allowed?
-    return redirect_to login_page_url(error: 'business-account-only') unless validate_business_account?
+    return redirect_to login_page_url(error: 'business-account-only') unless validate_signup_email_is_business_domain?
 
     create_account_for_user
     token = @resource.send(:set_reset_password_token)
@@ -53,9 +53,11 @@ class DeviseOverrides::OmniauthCallbacksController < DeviseTokenAuth::OmniauthCa
     ).first
   end
 
-  def validate_business_account?
-    # return true if the user is a business account, false if it is a gmail account
-    auth_hash['info']['email'].downcase.exclude?('@gmail.com')
+  def validate_signup_email_is_business_domain?
+    # return true if the user is a business account, false if it is a blocked domain account
+    Account::SignUpEmailValidationService.new(auth_hash['info']['email']).perform
+  rescue CustomExceptions::Account::InvalidEmail
+    false
   end
 
   def create_account_for_user

app/services/account/sign_up_email_validation_service.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+class Account::SignUpEmailValidationService
+  include CustomExceptions::Account
+  attr_reader :email
+
+  def initialize(email)
+    @email = email
+  end
+
+  def perform
+    address = ValidEmail2::Address.new(email)
+
+    raise InvalidEmail.new({ valid: false, disposable: nil }) unless address.valid?
+
+    raise InvalidEmail.new({ domain_blocked: true }) if domain_blocked?
+
+    raise InvalidEmail.new({ valid: true, disposable: true }) if address.disposable?
+
+    true
+  end
+
+  private
+
+  def domain_blocked?
+    domain = email.split('@').last&.downcase
+    blocked_domains.any? { |blocked_domain| domain.match?(blocked_domain.downcase) }
+  end
+
+  def blocked_domains
+    domains = GlobalConfigService.load('BLOCKED_EMAIL_DOMAINS', '')
+    return [] if domains.blank?
+
+    domains.split("\n").map(&:strip)
+  end
+end

spec/builders/account_builder_spec.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AccountBuilder do
+  let(:email) { 'user@example.com' }
+  let(:user_password) { 'Password123!' }
+  let(:account_name) { 'Test Account' }
+  let(:user_full_name) { 'Test User' }
+  let(:validation_service) { instance_double(Account::SignUpEmailValidationService, perform: true) }
+  let(:account_builder) do
+    described_class.new(
+      account_name: account_name,
+      email: email,
+      user_full_name: user_full_name,
+      user_password: user_password,
+      confirmed: true
+    )
+  end
+
+  # Mock the email validation service
+  before do
+    allow(Account::SignUpEmailValidationService).to receive(:new).with(email).and_return(validation_service)
+  end
+
+  describe '#perform' do
+    context 'when valid params are passed' do
+      it 'creates a new account with correct name' do
+        _user, account = account_builder.perform
+        expect(account).to be_an(Account)
+        expect(account.name).to eq(account_name)
+      end
+
+      it 'creates a new confirmed user with correct details' do
+        user, _account = account_builder.perform
+        expect(user).to be_a(User)
+        expect(user.email).to eq(email)
+        expect(user.name).to eq(user_full_name)
+        expect(user.confirmed?).to be(true)
+      end
+
+      it 'links user to account as administrator' do
+        user, account = account_builder.perform
+        expect(user.account_users.first.role).to eq('administrator')
+        expect(user.accounts.first).to eq(account)
+      end
+
+      it 'increments the counts of models' do
+        expect do
+          account_builder.perform
+        end.to change(Account, :count).by(1)
+           .and change(User, :count).by(1)
+           .and change(AccountUser, :count).by(1)
+      end
+    end
+  end
+end

spec/controllers/devise/omniauth_callbacks_controller_spec.rb

@@ -3,6 +3,7 @@ require 'rails_helper'
 RSpec.describe 'DeviseOverrides::OmniauthCallbacksController', type: :request do
   let(:account_builder) { double }
   let(:user_double) { object_double(:user) }
+  let(:email_validation_service) { instance_double(Account::SignUpEmailValidationService) }
 
   def set_omniauth_config(for_email = 'test@example.com')
     OmniAuth.config.test_mode = true
@@ -17,13 +18,18 @@ RSpec.describe 'DeviseOverrides::OmniauthCallbacksController', type: :request do
     )
   end
 
+  before do
+    allow(Account::SignUpEmailValidationService).to receive(:new).and_return(email_validation_service)
+  end
+
   describe '#omniauth_sucess' do
     it 'allows signup' do
-      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
+      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true', FRONTEND_URL: 'http://www.example.com' do
         set_omniauth_config('test_not_preset@example.com')
         allow(AccountBuilder).to receive(:new).and_return(account_builder)
         allow(account_builder).to receive(:perform).and_return(user_double)
         allow(Avatar::AvatarFromUrlJob).to receive(:perform_later).and_return(true)
+        allow(email_validation_service).to receive(:perform).and_return(true)
 
         get '/omniauth/google_oauth2/callback'
 
@@ -43,8 +49,10 @@ RSpec.describe 'DeviseOverrides::OmniauthCallbacksController', type: :request do
     end
 
     it 'blocks personal accounts signup' do
-      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
+      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true', FRONTEND_URL: 'http://www.example.com' do
         set_omniauth_config('personal@gmail.com')
+        allow(email_validation_service).to receive(:perform).and_raise(CustomExceptions::Account::InvalidEmail.new({ valid: false, disposable: nil }))
+
         get '/omniauth/google_oauth2/callback'
 
         # expect a 302 redirect to auth/google_oauth2/callback
@@ -57,10 +65,13 @@ RSpec.describe 'DeviseOverrides::OmniauthCallbacksController', type: :request do
     end
 
     it 'blocks personal accounts signup with different Gmail case variations' do
-      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true' do
+      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'true', FRONTEND_URL: 'http://www.example.com' do
         # Test different case variations of Gmail
         ['personal@Gmail.com', 'personal@GMAIL.com', 'personal@Gmail.COM'].each do |email|
           set_omniauth_config(email)
+          allow(email_validation_service).to receive(:perform).and_raise(CustomExceptions::Account::InvalidEmail.new({ valid: false,
+                                                                                                                       disposable: nil }))
+
           get '/omniauth/google_oauth2/callback'
 
           # expect a 302 redirect to auth/google_oauth2/callback
@@ -76,8 +87,10 @@ RSpec.describe 'DeviseOverrides::OmniauthCallbacksController', type: :request do
     # This test does not affect line coverage, but it is important to ensure that the logic
     # does not allow any signup if the ENV explicitly disables it
     it 'blocks signup if ENV disabled' do
-      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false' do
+      with_modified_env ENABLE_ACCOUNT_SIGNUP: 'false', 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 a 302 redirect to auth/google_oauth2/callback
@@ -90,38 +103,42 @@ RSpec.describe 'DeviseOverrides::OmniauthCallbacksController', type: :request do
     end
 
     it 'allows login' do
-      create(:user, email: 'test@example.com')
-      set_omniauth_config('test@example.com')
+      with_modified_env FRONTEND_URL: 'http://www.example.com' do
+        create(:user, email: 'test@example.com')
+        set_omniauth_config('test@example.com')
 
-      get '/omniauth/google_oauth2/callback'
-      # expect a 302 redirect to auth/google_oauth2/callback
-      expect(response).to redirect_to('http://www.example.com/auth/google_oauth2/callback')
+        get '/omniauth/google_oauth2/callback'
+        # expect a 302 redirect to auth/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\?email=.+&sso_auth_token=.+$})
+        follow_redirect!
+        expect(response).to redirect_to(%r{/app/login\?email=.+&sso_auth_token=.+$})
 
-      # expect app/login page to respond with 200 and render
-      follow_redirect!
-      expect(response).to have_http_status(:ok)
+        # expect app/login page to respond with 200 and render
+        follow_redirect!
+        expect(response).to have_http_status(:ok)
+      end
     end
 
     # from a line coverage point of view this may seem redundant
     # but to ensure that the logic allows for existing users even if they have a gmail account
     # we need to test this explicitly
     it 'allows personal account login' do
-      create(:user, email: 'personal-existing@gmail.com')
-      set_omniauth_config('personal-existing@gmail.com')
+      with_modified_env FRONTEND_URL: 'http://www.example.com' do
+        create(:user, email: 'personal-existing@gmail.com')
+        set_omniauth_config('personal-existing@gmail.com')
 
-      get '/omniauth/google_oauth2/callback'
-      # expect a 302 redirect to auth/google_oauth2/callback
-      expect(response).to redirect_to('http://www.example.com/auth/google_oauth2/callback')
+        get '/omniauth/google_oauth2/callback'
+        # expect a 302 redirect to auth/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\?email=.+&sso_auth_token=.+$})
+        follow_redirect!
+        expect(response).to redirect_to(%r{/app/login\?email=.+&sso_auth_token=.+$})
 
-      # expect app/login page to respond with 200 and render
-      follow_redirect!
-      expect(response).to have_http_status(:ok)
+        # expect app/login page to respond with 200 and render
+        follow_redirect!
+        expect(response).to have_http_status(:ok)
+      end
     end
   end
 end

spec/services/account/sign_up_email_validation_service_spec.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Account::SignUpEmailValidationService, type: :service do
+  let(:service) { described_class.new(email) }
+  let(:blocked_domains) { "gmail.com\noutlook.com" }
+  let(:valid_email_address) { instance_double(ValidEmail2::Address, valid?: true, disposable?: false) }
+  let(:disposable_email_address) { instance_double(ValidEmail2::Address, valid?: true, disposable?: true) }
+  let(:invalid_email_address) { instance_double(ValidEmail2::Address, valid?: false) }
+
+  before do
+    allow(GlobalConfigService).to receive(:load).with('BLOCKED_EMAIL_DOMAINS', '').and_return(blocked_domains)
+  end
+
+  describe '#perform' do
+    context 'when email is invalid format' do
+      let(:email) { 'invalid-email' }
+
+      it 'raises InvalidEmail with invalid message' do
+        allow(ValidEmail2::Address).to receive(:new).with(email).and_return(invalid_email_address)
+        expect { service.perform }.to raise_error do |error|
+          expect(error).to be_a(CustomExceptions::Account::InvalidEmail)
+          expect(error.message).to eq(I18n.t('errors.signup.invalid_email'))
+        end
+      end
+    end
+
+    context 'when domain is blocked' do
+      let(:email) { 'test@gmail.com' }
+
+      it 'raises InvalidEmail with blocked domain message' do
+        allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address)
+        expect { service.perform }.to raise_error do |error|
+          expect(error).to be_a(CustomExceptions::Account::InvalidEmail)
+          expect(error.message).to eq(I18n.t('errors.signup.blocked_domain'))
+        end
+      end
+    end
+
+    context 'when domain is blocked (case insensitive)' do
+      let(:email) { 'test@GMAIL.COM' }
+
+      it 'raises InvalidEmail with blocked domain message' do
+        allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address)
+        expect { service.perform }.to raise_error do |error|
+          expect(error).to be_a(CustomExceptions::Account::InvalidEmail)
+          expect(error.message).to eq(I18n.t('errors.signup.blocked_domain'))
+        end
+      end
+    end
+
+    context 'when email is from disposable provider' do
+      let(:email) { 'test@mailinator.com' }
+
+      it 'raises InvalidEmail with disposable message' do
+        allow(ValidEmail2::Address).to receive(:new).with(email).and_return(disposable_email_address)
+        expect { service.perform }.to raise_error do |error|
+          expect(error).to be_a(CustomExceptions::Account::InvalidEmail)
+          expect(error.message).to eq(I18n.t('errors.signup.disposable_email'))
+        end
+      end
+    end
+
+    context 'when email is valid business email' do
+      let(:email) { 'test@example.com' }
+
+      it 'returns true' do
+        allow(ValidEmail2::Address).to receive(:new).with(email).and_return(valid_email_address)
+        expect(service.perform).to be(true)
+      end
+    end
+  end
+end