From 995f56f0ad39ec085388fff04ff6ddd9305d4da7 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 28 Aug 2025 15:40:26 +0530 Subject: [PATCH] feat: move SAML code to enterprise --- .../omniauth_callbacks_controller.rb | 67 +------------------ .../devise_overrides/passwords_controller.rb | 12 +--- .../devise_overrides/sessions_controller.rb | 19 ------ .../omniauth_callbacks_controller.rb | 66 ++++++++++++++++++ .../devise_overrides/passwords_controller.rb | 19 ++++++ .../devise_overrides/sessions_controller.rb | 23 +++++++ 6 files changed, 113 insertions(+), 93 deletions(-) create mode 100644 enterprise/app/controllers/enterprise/devise_overrides/omniauth_callbacks_controller.rb create mode 100644 enterprise/app/controllers/enterprise/devise_overrides/passwords_controller.rb diff --git a/app/controllers/devise_overrides/omniauth_callbacks_controller.rb b/app/controllers/devise_overrides/omniauth_callbacks_controller.rb index fd69d10cf..6885ae22a 100644 --- a/app/controllers/devise_overrides/omniauth_callbacks_controller.rb +++ b/app/controllers/devise_overrides/omniauth_callbacks_controller.rb @@ -1,75 +1,12 @@ class DeviseOverrides::OmniauthCallbacksController < DeviseTokenAuth::OmniauthCallbacksController include EmailHelper - def saml - # Call parent's omniauth_success which handles the auth - omniauth_success - end - - def redirect_callbacks - # derive target redirect route from 'resource_class' param, which was set - # before authentication. - devise_mapping = get_devise_mapping - redirect_route = get_redirect_route(devise_mapping) - - # preserve omniauth info for success route. ignore 'extra' in twitter - # auth response to avoid CookieOverflow. - session['dta.omniauth.auth'] = request.env['omniauth.auth'].except('extra') - session['dta.omniauth.params'] = request.env['omniauth.params'] - - # For SAML, use 303 See Other to convert POST to GET and preserve session - if params[:provider] == 'saml' - redirect_to redirect_route, { status: 303 }.merge(redirect_options) - else - redirect_to redirect_route, { status: 307 }.merge(redirect_options) - end - end - def omniauth_success - case auth_hash&.dig('provider') - when 'saml' - handle_saml_auth - else - handle_standard_auth - end - end - - private - - def handle_saml_auth - # Check if enterprise edition and SAML feature are available - return redirect_to login_page_url(error: 'saml-not-available') unless ChatwootApp.enterprise? - - account_id = extract_saml_account_id - return redirect_to login_page_url(error: 'saml-not-enabled') unless saml_enabled_for_account?(account_id) - - @resource = SamlUserBuilder.new(auth_hash, account_id: account_id).perform - - if @resource.persisted? - sign_in_user - else - redirect_to login_page_url(error: 'saml-authentication-failed') - end - end - - def handle_standard_auth get_resource_from_auth_hash @resource.present? ? sign_in_user : sign_up_user end - def extract_saml_account_id - params[:account_id] || session[:saml_account_id] || request.env['omniauth.params']&.dig('account_id') - end - - def saml_enabled_for_account?(account_id) - return false unless ChatwootApp.enterprise? - return false if account_id.blank? - - account = Account.find_by(id: account_id) - return false unless account.feature_enabled?('saml') - - AccountSamlSettings.find_by(account_id: account_id, enabled: true).present? - end + private def sign_in_user @resource.skip_confirmation! if confirmable_enabled? @@ -135,3 +72,5 @@ class DeviseOverrides::OmniauthCallbacksController < DeviseTokenAuth::OmniauthCa 'user' end end + +DeviseOverrides::OmniauthCallbacksController.prepend_mod_with('DeviseOverrides::OmniauthCallbacksController') diff --git a/app/controllers/devise_overrides/passwords_controller.rb b/app/controllers/devise_overrides/passwords_controller.rb index a17ca1d98..00976c3cd 100644 --- a/app/controllers/devise_overrides/passwords_controller.rb +++ b/app/controllers/devise_overrides/passwords_controller.rb @@ -3,7 +3,6 @@ class DeviseOverrides::PasswordsController < Devise::PasswordsController skip_before_action :require_no_authentication, raise: false skip_before_action :authenticate_user!, raise: false - before_action :check_saml_user, only: [:create] def create @user = User.from_email(params[:email]) @@ -44,13 +43,6 @@ class DeviseOverrides::PasswordsController < Devise::PasswordsController message: message }, status: status end - - def check_saml_user - return if params[:email].blank? - - user = User.from_email(params[:email]) - return unless user&.saml_user? - - build_response(I18n.t('messages.reset_password_saml_user'), 403) - end end + +DeviseOverrides::PasswordsController.prepend_mod_with('DeviseOverrides::PasswordsController') diff --git a/app/controllers/devise_overrides/sessions_controller.rb b/app/controllers/devise_overrides/sessions_controller.rb index b81688c1b..fc7b12767 100644 --- a/app/controllers/devise_overrides/sessions_controller.rb +++ b/app/controllers/devise_overrides/sessions_controller.rb @@ -3,7 +3,6 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController # Unpermitted parameter: session wrap_parameters format: [] before_action :process_sso_auth_token, only: [:create] - before_action :check_saml_user, only: [:create] def new redirect_to login_page_url(error: 'access-denied') @@ -47,24 +46,6 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController user = User.from_email(params[:email]) @resource = user if user&.valid_sso_auth_token?(params[:sso_auth_token]) end - - def check_saml_user - # Skip if using SSO token (SAML users can use SSO tokens) - return if params[:sso_auth_token].present? - return if params[:email].blank? - - user = User.from_email(params[:email]) - return unless user&.saml_user? - - render_saml_user_error - end - - def render_saml_user_error - render json: { - success: false, - errors: [I18n.t('messages.login_saml_user')] - }, status: :unauthorized - end end DeviseOverrides::SessionsController.prepend_mod_with('DeviseOverrides::SessionsController') diff --git a/enterprise/app/controllers/enterprise/devise_overrides/omniauth_callbacks_controller.rb b/enterprise/app/controllers/enterprise/devise_overrides/omniauth_callbacks_controller.rb new file mode 100644 index 000000000..438a516fa --- /dev/null +++ b/enterprise/app/controllers/enterprise/devise_overrides/omniauth_callbacks_controller.rb @@ -0,0 +1,66 @@ +module Enterprise::DeviseOverrides::OmniauthCallbacksController + def saml + # Call parent's omniauth_success which handles the auth + omniauth_success + end + + def redirect_callbacks + # derive target redirect route from 'resource_class' param, which was set + # before authentication. + devise_mapping = get_devise_mapping + redirect_route = get_redirect_route(devise_mapping) + + # preserve omniauth info for success route. ignore 'extra' in twitter + # auth response to avoid CookieOverflow. + session['dta.omniauth.auth'] = request.env['omniauth.auth'].except('extra') + session['dta.omniauth.params'] = request.env['omniauth.params'] + + # For SAML, use 303 See Other to convert POST to GET and preserve session + if params[:provider] == 'saml' + redirect_to redirect_route, { status: 303 }.merge(redirect_options) + else + super + end + end + + def omniauth_success + case auth_hash&.dig('provider') + when 'saml' + handle_saml_auth + else + super + end + end + + private + + def handle_saml_auth + # Check if enterprise edition and SAML feature are available + return redirect_to login_page_url(error: 'saml-not-available') unless ChatwootApp.enterprise? + + account_id = extract_saml_account_id + return redirect_to login_page_url(error: 'saml-not-enabled') unless saml_enabled_for_account?(account_id) + + @resource = SamlUserBuilder.new(auth_hash, account_id: account_id).perform + + if @resource.persisted? + sign_in_user + else + redirect_to login_page_url(error: 'saml-authentication-failed') + end + end + + def extract_saml_account_id + params[:account_id] || session[:saml_account_id] || request.env['omniauth.params']&.dig('account_id') + end + + def saml_enabled_for_account?(account_id) + return false unless ChatwootApp.enterprise? + return false if account_id.blank? + + account = Account.find_by(id: account_id) + return false unless account.feature_enabled?('saml') + + AccountSamlSettings.find_by(account_id: account_id, enabled: true).present? + end +end diff --git a/enterprise/app/controllers/enterprise/devise_overrides/passwords_controller.rb b/enterprise/app/controllers/enterprise/devise_overrides/passwords_controller.rb new file mode 100644 index 000000000..345ce3c6c --- /dev/null +++ b/enterprise/app/controllers/enterprise/devise_overrides/passwords_controller.rb @@ -0,0 +1,19 @@ +module Enterprise::DeviseOverrides::PasswordsController + def create + check_saml_user + super + rescue CustomExceptions::Base => e + build_response(e.message, e.http_status_code) + end + + private + + def check_saml_user + return if params[:email].blank? + + user = User.from_email(params[:email]) + return unless user&.saml_user? + + raise CustomExceptions::Base.new(I18n.t('messages.reset_password_saml_user'), :forbidden) + end +end diff --git a/enterprise/app/controllers/enterprise/devise_overrides/sessions_controller.rb b/enterprise/app/controllers/enterprise/devise_overrides/sessions_controller.rb index e11e3fff9..99fde8c3c 100644 --- a/enterprise/app/controllers/enterprise/devise_overrides/sessions_controller.rb +++ b/enterprise/app/controllers/enterprise/devise_overrides/sessions_controller.rb @@ -1,4 +1,14 @@ module Enterprise::DeviseOverrides::SessionsController + def create + check_saml_user + super + rescue CustomExceptions::Base => e + render json: { + success: false, + errors: [e.message] + }, status: e.http_status_code + end + def render_create_success create_audit_event('sign_in') super @@ -22,4 +32,17 @@ module Enterprise::DeviseOverrides::SessionsController ) end end + + private + + def check_saml_user + # Skip if using SSO token (SAML users can use SSO tokens) + return if params[:sso_auth_token].present? + return if params[:email].blank? + + user = User.from_email(params[:email]) + return unless user&.saml_user? + + raise CustomExceptions::Base.new(I18n.t('messages.login_saml_user'), :unauthorized) + end end