diff --git a/app/controllers/email_smtp_settings_controller.rb b/app/controllers/email_smtp_settings_controller.rb index 766a9545..58c498be 100644 --- a/app/controllers/email_smtp_settings_controller.rb +++ b/app/controllers/email_smtp_settings_controller.rb @@ -8,19 +8,37 @@ class EmailSmtpSettingsController < ApplicationController def index; end def create - if @encrypted_config.update(email_configs) - unless Docuseal.multitenant? - SettingsMailer.smtp_successful_setup(@encrypted_config.value['from_email'] || current_user.email).deliver_now! + # Store the original values in case of error + original_value = @encrypted_config.value + Rails.logger.info "SMTP Update: Original config: #{original_value.inspect}" + + begin + if @encrypted_config.update(email_configs) + unless Docuseal.multitenant? + # Only attempt to send test email if SMTP settings are provided + if @encrypted_config.value.present? && @encrypted_config.value['address'].present? + SettingsMailer.smtp_successful_setup(@encrypted_config.value['from_email'] || current_user.email).deliver_now! + end + end + + redirect_to settings_email_index_path, notice: I18n.t('changes_have_been_saved') + return + else + Rails.logger.error "SMTP Update: Update failed with errors: #{@encrypted_config.errors.full_messages}" + render :index, status: :unprocessable_entity + return end - - redirect_to settings_email_index_path, notice: I18n.t('changes_have_been_saved') - else + rescue StandardError => e + Rails.logger.error "SMTP Update: Error occurred: #{e.message}" + Rails.logger.error e.backtrace.join("\n") + + # Restore the original values to prevent data loss + @encrypted_config.value = original_value + @encrypted_config.save if @encrypted_config.changed? + + flash[:alert] = "Error updating SMTP settings: #{e.message}" render :index, status: :unprocessable_entity end - rescue StandardError => e - flash[:alert] = e.message - - render :index, status: :unprocessable_entity end private diff --git a/app/models/user.rb b/app/models/user.rb index 2c589bca..8af6d976 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -122,53 +122,37 @@ class User < ApplicationRecord # Omniauth callback method def self.from_omniauth(auth) - # Find existing user by provider and uid + # Find an existing user by provider and uid user = User.find_by(provider: auth.provider, uid: auth.uid) - - if user - # Update user info from provider if needed - user.update( + return user if user + + # IMPORTANT: The following section has been removed to prevent account takeover vulnerabilities. + # Automatic linking of accounts based on email is not secure. + # A user should link their SSO provider only when they are already logged in. + + # If the user does not exist, create a new one. + # NOTE: This account provisioning logic may need to be customized based on your application's needs. + begin + account = if Docuseal.multitenant? + # This logic is potentially insecure and should be reviewed. + # It is recommended to have a more robust way of assigning accounts. + Account.find_or_create_by(name: auth.info.email&.split('@')&.last || 'SSO Account') + else + Account.first_or_create!(name: 'Default Account') + end + + User.create!( email: auth.info.email, first_name: auth.info.first_name || auth.info.name&.split&.first, - last_name: auth.info.last_name || auth.info.name&.split&.last - ) if auth.info.email.present? - return user - end - - # Try to find user by email if no provider/uid match - existing_user = User.find_by(email: auth.info.email) if auth.info.email.present? - - if existing_user - # Link existing account with omniauth provider - existing_user.update( + last_name: auth.info.last_name || auth.info.name&.split&.last, provider: auth.provider, - uid: auth.uid + uid: auth.uid, + account: account, + password: Devise.friendly_token[0, 20] ) - return existing_user + rescue ActiveRecord::RecordInvalid => e + Rails.logger.error "Failed to create user from omniauth: #{e.message}" + nil end - - # Create new user from omniauth data - # For multitenant setups, we need to determine the account - account = if Docuseal.multitenant? - # In multitenant mode, create account from domain or use default logic - Account.find_or_create_by(name: auth.info.email&.split('@')&.last || 'SSO Account') - else - # In single-tenant mode, use the first account or create one - Account.first || Account.create!(name: 'Default Account') - end - - User.create!( - email: auth.info.email, - first_name: auth.info.first_name || auth.info.name&.split&.first, - last_name: auth.info.last_name || auth.info.name&.split&.last, - provider: auth.provider, - uid: auth.uid, - account: account, - # Skip password validation for omniauth users - password: Devise.friendly_token[0, 20] - ) - rescue ActiveRecord::RecordInvalid => e - Rails.logger.error "Failed to create user from omniauth: #{e.message}" - nil end end diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index f5cae5d8..710c2b7a 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -37,18 +37,30 @@ <% # Check if SAML is configured (either in ENV or database) saml_configured = false - if ENV['SAML_IDP_SSO_SERVICE_URL'].present? && ENV['SAML_IDP_CERT_FINGERPRINT'].present? - saml_configured = true - else - saml_config_record = EncryptedConfig.find_by(key: 'saml_configs') - if saml_config_record&.value.present? - begin - config = JSON.parse(saml_config_record.value) - saml_configured = config['idp_sso_service_url'].present? && config['idp_cert_fingerprint'].present? - rescue JSON::ParserError - # Invalid JSON, treat as not configured + begin + if ENV['SAML_IDP_SSO_SERVICE_URL'].present? && ENV['SAML_IDP_CERT_FINGERPRINT'].present? + saml_configured = true + Rails.logger.info "SAML Login Page: Using ENV configuration" + else + # Try to find SAML config in any account (not just current_account) + saml_config_record = EncryptedConfig.where(key: 'saml_configs').first + Rails.logger.info "SAML Login Page: Config record found: #{saml_config_record.present?}" + + if saml_config_record&.value.present? + begin + config = JSON.parse(saml_config_record.value) + saml_configured = config['idp_sso_service_url'].present? && config['idp_cert_fingerprint'].present? + Rails.logger.info "SAML Login Page: Valid JSON: Yes, SSO URL: #{config['idp_sso_service_url'].present?}, Cert: #{config['idp_cert_fingerprint'].present?}" + rescue JSON::ParserError => e + Rails.logger.error "SAML Login Page: JSON parse error: #{e.message}" + # Invalid JSON, treat as not configured + end + else + Rails.logger.info "SAML Login Page: Config value is empty or nil" end end + rescue => e + Rails.logger.error "SAML Login Page: Error checking config: #{e.message}" end %> <% if User.omniauth_providers.include?(:saml) && saml_configured %>