mirror of
				https://github.com/lingble/chatwoot.git
				synced 2025-10-31 11:08:04 +00:00 
			
		
		
		
	feat: Secure external credentials with database encryption (#12648)
## Changelog
- Added conditional Active Record encryption to every external
credential we store (SMTP/IMAP passwords, Twilio tokens,
Slack/OpenAI hook tokens, Facebook/Instagram tokens, LINE/Telegram keys,
Twitter secrets) so new writes are encrypted
whenever Chatwoot.encryption_configured? is true; legacy installs still
receive plaintext until their secrets are
    updated.
- Tuned encryption settings in config/application.rb to allow legacy
reads (support_unencrypted_data) and to extend
deterministic queries so lookups continue to match plaintext rows during
the rollout; added TODOs to retire the
    fallback once encryption becomes mandatory.
- Introduced an MFA-pipeline test suite
(spec/models/external_credentials_encryption_spec.rb) plus shared
examples to
verify each attribute encrypts at rest and that plaintext records
re-encrypt on update, with a dedicated Telegram case.
The existing MFA GitHub workflow now runs these tests using the
preconfigured encryption keys.
fixes:
https://linear.app/chatwoot/issue/CW-5453/encrypt-sensitive-credentials-stored-in-plain-text-in-database
## Testing Instructions
 1. Instance without encryption keys
- Unset ACTIVE_RECORD_ENCRYPTION_* vars (or run in an environment where
they’re absent).
      - Create at least one credentialed channel (e.g., Email SMTP).
- Confirm workflows still function (send/receive mail or a similar
sanity check).
- In the DB you should still see plaintext values—this confirms the
guard prevents encryption when keys are missing.
  2. Instance with encryption keys
      - Configure the three encryption env vars and restart.
- Pick a couple of representative integrations (e.g., Email SMTP +
Twilio SMS).
      - Legacy channel check:
- Use existing records created before enabling keys. Trigger their
workflow (send an email / SMS, or hit the
            webhook) to ensure they still authenticate.
- Inspect the raw column—value remains plaintext until changed.
      - Update legacy channel:
- Edit one legacy channel’s credential (e.g., change SMTP password).
- Verify the operation still works and the stored value is now encrypted
(raw column differs, accessor returns
            original).
      - New channel creation:
- Create a new channel of the same type; confirm functionality and that
the stored credential is encrypted from
            the start.
---------
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
			
			
This commit is contained in:
		
							
								
								
									
										1
									
								
								.github/workflows/run_mfa_spec.yml
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										1
									
								
								.github/workflows/run_mfa_spec.yml
									
									
									
									
										vendored
									
									
								
							| @@ -70,6 +70,7 @@ jobs: | |||||||
|             spec/services/mfa/authentication_service_spec.rb \ |             spec/services/mfa/authentication_service_spec.rb \ | ||||||
|             spec/requests/api/v1/profile/mfa_controller_spec.rb \ |             spec/requests/api/v1/profile/mfa_controller_spec.rb \ | ||||||
|             spec/controllers/devise_overrides/sessions_controller_spec.rb \ |             spec/controllers/devise_overrides/sessions_controller_spec.rb \ | ||||||
|  |             spec/models/application_record_external_credentials_encryption_spec.rb \ | ||||||
|             --profile=10 \ |             --profile=10 \ | ||||||
|             --format documentation |             --format documentation | ||||||
|         env: |         env: | ||||||
|   | |||||||
| @@ -40,6 +40,12 @@ class Channel::Email < ApplicationRecord | |||||||
|  |  | ||||||
|   AUTHORIZATION_ERROR_THRESHOLD = 10 |   AUTHORIZATION_ERROR_THRESHOLD = 10 | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   if Chatwoot.encryption_configured? | ||||||
|  |     encrypts :imap_password | ||||||
|  |     encrypts :smtp_password | ||||||
|  |   end | ||||||
|  |  | ||||||
|   self.table_name = 'channel_email' |   self.table_name = 'channel_email' | ||||||
|   EDITABLE_ATTRS = [:email, :imap_enabled, :imap_login, :imap_password, :imap_address, :imap_port, :imap_enable_ssl, |   EDITABLE_ATTRS = [:email, :imap_enabled, :imap_login, :imap_password, :imap_address, :imap_port, :imap_enable_ssl, | ||||||
|                     :smtp_enabled, :smtp_login, :smtp_password, :smtp_address, :smtp_port, :smtp_domain, :smtp_enable_starttls_auto, |                     :smtp_enabled, :smtp_login, :smtp_password, :smtp_address, :smtp_port, :smtp_domain, :smtp_enable_starttls_auto, | ||||||
|   | |||||||
| @@ -21,6 +21,12 @@ class Channel::FacebookPage < ApplicationRecord | |||||||
|   include Channelable |   include Channelable | ||||||
|   include Reauthorizable |   include Reauthorizable | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   if Chatwoot.encryption_configured? | ||||||
|  |     encrypts :page_access_token | ||||||
|  |     encrypts :user_access_token | ||||||
|  |   end | ||||||
|  |  | ||||||
|   self.table_name = 'channel_facebook_pages' |   self.table_name = 'channel_facebook_pages' | ||||||
|  |  | ||||||
|   validates :page_id, uniqueness: { scope: :account_id } |   validates :page_id, uniqueness: { scope: :account_id } | ||||||
|   | |||||||
| @@ -19,6 +19,9 @@ class Channel::Instagram < ApplicationRecord | |||||||
|   include Reauthorizable |   include Reauthorizable | ||||||
|   self.table_name = 'channel_instagram' |   self.table_name = 'channel_instagram' | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   encrypts :access_token if Chatwoot.encryption_configured? | ||||||
|  |  | ||||||
|   AUTHORIZATION_ERROR_THRESHOLD = 1 |   AUTHORIZATION_ERROR_THRESHOLD = 1 | ||||||
|  |  | ||||||
|   validates :access_token, presence: true |   validates :access_token, presence: true | ||||||
|   | |||||||
| @@ -18,6 +18,12 @@ | |||||||
| class Channel::Line < ApplicationRecord | class Channel::Line < ApplicationRecord | ||||||
|   include Channelable |   include Channelable | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   if Chatwoot.encryption_configured? | ||||||
|  |     encrypts :line_channel_secret | ||||||
|  |     encrypts :line_channel_token | ||||||
|  |   end | ||||||
|  |  | ||||||
|   self.table_name = 'channel_line' |   self.table_name = 'channel_line' | ||||||
|   EDITABLE_ATTRS = [:line_channel_id, :line_channel_secret, :line_channel_token].freeze |   EDITABLE_ATTRS = [:line_channel_id, :line_channel_secret, :line_channel_token].freeze | ||||||
|  |  | ||||||
|   | |||||||
| @@ -17,6 +17,9 @@ | |||||||
| class Channel::Telegram < ApplicationRecord | class Channel::Telegram < ApplicationRecord | ||||||
|   include Channelable |   include Channelable | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   encrypts :bot_token, deterministic: true if Chatwoot.encryption_configured? | ||||||
|  |  | ||||||
|   self.table_name = 'channel_telegram' |   self.table_name = 'channel_telegram' | ||||||
|   EDITABLE_ATTRS = [:bot_token].freeze |   EDITABLE_ATTRS = [:bot_token].freeze | ||||||
|  |  | ||||||
|   | |||||||
| @@ -28,6 +28,9 @@ class Channel::TwilioSms < ApplicationRecord | |||||||
|  |  | ||||||
|   self.table_name = 'channel_twilio_sms' |   self.table_name = 'channel_twilio_sms' | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   encrypts :auth_token if Chatwoot.encryption_configured? | ||||||
|  |  | ||||||
|   validates :account_sid, presence: true |   validates :account_sid, presence: true | ||||||
|   # The same parameter is used to store api_key_secret if api_key authentication is opted |   # The same parameter is used to store api_key_secret if api_key authentication is opted | ||||||
|   validates :auth_token, presence: true |   validates :auth_token, presence: true | ||||||
|   | |||||||
| @@ -19,6 +19,12 @@ | |||||||
| class Channel::TwitterProfile < ApplicationRecord | class Channel::TwitterProfile < ApplicationRecord | ||||||
|   include Channelable |   include Channelable | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   if Chatwoot.encryption_configured? | ||||||
|  |     encrypts :twitter_access_token | ||||||
|  |     encrypts :twitter_access_token_secret | ||||||
|  |   end | ||||||
|  |  | ||||||
|   self.table_name = 'channel_twitter_profiles' |   self.table_name = 'channel_twitter_profiles' | ||||||
|  |  | ||||||
|   validates :profile_id, uniqueness: { scope: :account_id } |   validates :profile_id, uniqueness: { scope: :account_id } | ||||||
|   | |||||||
| @@ -21,6 +21,9 @@ class Integrations::Hook < ApplicationRecord | |||||||
|   before_validation :ensure_hook_type |   before_validation :ensure_hook_type | ||||||
|   after_create :trigger_setup_if_crm |   after_create :trigger_setup_if_crm | ||||||
|  |  | ||||||
|  |   # TODO: Remove guard once encryption keys become mandatory (target 3-4 releases out). | ||||||
|  |   encrypts :access_token, deterministic: true if Chatwoot.encryption_configured? | ||||||
|  |  | ||||||
|   validates :account_id, presence: true |   validates :account_id, presence: true | ||||||
|   validates :app_id, presence: true |   validates :app_id, presence: true | ||||||
|   validates :inbox_id, presence: true, if: -> { hook_type == 'inbox' } |   validates :inbox_id, presence: true, if: -> { hook_type == 'inbox' } | ||||||
|   | |||||||
| @@ -75,7 +75,11 @@ module Chatwoot | |||||||
|       config.active_record.encryption.primary_key = ENV['ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY'] |       config.active_record.encryption.primary_key = ENV['ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY'] | ||||||
|       config.active_record.encryption.deterministic_key = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY', nil) |       config.active_record.encryption.deterministic_key = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY', nil) | ||||||
|       config.active_record.encryption.key_derivation_salt = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT', nil) |       config.active_record.encryption.key_derivation_salt = ENV.fetch('ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT', nil) | ||||||
|  |       # TODO: Remove once encryption is mandatory and legacy plaintext is migrated. | ||||||
|       config.active_record.encryption.support_unencrypted_data = true |       config.active_record.encryption.support_unencrypted_data = true | ||||||
|  |       # Extend deterministic queries so they match both encrypted and plaintext rows | ||||||
|  |       config.active_record.encryption.extend_queries = true | ||||||
|  |       # Store a per-row key reference to support future key rotation | ||||||
|       config.active_record.encryption.store_key_references = true |       config.active_record.encryption.store_key_references = true | ||||||
|     end |     end | ||||||
|   end |   end | ||||||
| @@ -94,6 +98,8 @@ module Chatwoot | |||||||
|   end |   end | ||||||
|  |  | ||||||
|   def self.encryption_configured? |   def self.encryption_configured? | ||||||
|  |     # TODO: Once Active Record encryption keys are mandatory (target 3-4 releases out), | ||||||
|  |     # remove this guard and assume encryption is always enabled. | ||||||
|     # Check if proper encryption keys are configured |     # Check if proper encryption keys are configured | ||||||
|     # MFA/2FA features should only be enabled when proper keys are set |     # MFA/2FA features should only be enabled when proper keys are set | ||||||
|     ENV['ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY'].present? && |     ENV['ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY'].present? && | ||||||
|   | |||||||
| @@ -0,0 +1,113 @@ | |||||||
|  | # frozen_string_literal: true | ||||||
|  |  | ||||||
|  | require 'rails_helper' | ||||||
|  |  | ||||||
|  | RSpec.describe ApplicationRecord do | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_email, | ||||||
|  |                   attribute: :smtp_password, | ||||||
|  |                   value: 'smtp-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_email, | ||||||
|  |                   attribute: :imap_password, | ||||||
|  |                   value: 'imap-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_twilio_sms, | ||||||
|  |                   attribute: :auth_token, | ||||||
|  |                   value: 'twilio-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :integrations_hook, | ||||||
|  |                   attribute: :access_token, | ||||||
|  |                   value: 'hook-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_facebook_page, | ||||||
|  |                   attribute: :page_access_token, | ||||||
|  |                   value: 'fb-page-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_facebook_page, | ||||||
|  |                   attribute: :user_access_token, | ||||||
|  |                   value: 'fb-user-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_instagram, | ||||||
|  |                   attribute: :access_token, | ||||||
|  |                   value: 'ig-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_line, | ||||||
|  |                   attribute: :line_channel_secret, | ||||||
|  |                   value: 'line-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_line, | ||||||
|  |                   attribute: :line_channel_token, | ||||||
|  |                   value: 'line-token-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_telegram, | ||||||
|  |                   attribute: :bot_token, | ||||||
|  |                   value: 'telegram-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_twitter_profile, | ||||||
|  |                   attribute: :twitter_access_token, | ||||||
|  |                   value: 'twitter-access-secret' | ||||||
|  |  | ||||||
|  |   it_behaves_like 'encrypted external credential', | ||||||
|  |                   factory: :channel_twitter_profile, | ||||||
|  |                   attribute: :twitter_access_token_secret, | ||||||
|  |                   value: 'twitter-secret-secret' | ||||||
|  |  | ||||||
|  |   context 'when backfilling legacy plaintext' do | ||||||
|  |     before do | ||||||
|  |       skip('encryption keys missing; see run_mfa_spec workflow') unless Chatwoot.encryption_configured? | ||||||
|  |     end | ||||||
|  |  | ||||||
|  |     it 'reads existing plaintext and encrypts on update' do | ||||||
|  |       account = create(:account) | ||||||
|  |       channel = create(:channel_email, account: account, smtp_password: nil) | ||||||
|  |  | ||||||
|  |       # Simulate legacy plaintext by updating the DB directly | ||||||
|  |       sql = ActiveRecord::Base.send( | ||||||
|  |         :sanitize_sql_array, | ||||||
|  |         ['UPDATE channel_email SET smtp_password = ? WHERE id = ?', 'legacy-plain', channel.id] | ||||||
|  |       ) | ||||||
|  |       ActiveRecord::Base.connection.execute(sql) | ||||||
|  |  | ||||||
|  |       legacy_record = Channel::Email.find(channel.id) | ||||||
|  |       expect(legacy_record.smtp_password).to eq('legacy-plain') | ||||||
|  |  | ||||||
|  |       legacy_record.update!(smtp_password: 'encrypted-now') | ||||||
|  |  | ||||||
|  |       stored_value = legacy_record.reload.read_attribute_before_type_cast(:smtp_password) | ||||||
|  |       expect(stored_value).to be_present | ||||||
|  |       expect(stored_value).not_to include('encrypted-now') | ||||||
|  |       expect(legacy_record.smtp_password).to eq('encrypted-now') | ||||||
|  |     end | ||||||
|  |   end | ||||||
|  |  | ||||||
|  |   context 'when looking up telegram legacy records' do | ||||||
|  |     before do | ||||||
|  |       skip('encryption keys missing; see run_mfa_spec workflow') unless Chatwoot.encryption_configured? | ||||||
|  |     end | ||||||
|  |  | ||||||
|  |     it 'finds plaintext records via fallback lookup' do | ||||||
|  |       channel = create(:channel_telegram, bot_token: 'legacy-token') | ||||||
|  |  | ||||||
|  |       # Simulate legacy plaintext by updating the DB directly | ||||||
|  |       sql = ActiveRecord::Base.send( | ||||||
|  |         :sanitize_sql_array, | ||||||
|  |         ['UPDATE channel_telegram SET bot_token = ? WHERE id = ?', 'legacy-token', channel.id] | ||||||
|  |       ) | ||||||
|  |       ActiveRecord::Base.connection.execute(sql) | ||||||
|  |  | ||||||
|  |       found = Channel::Telegram.find_by(bot_token: 'legacy-token') | ||||||
|  |       expect(found).to eq(channel) | ||||||
|  |     end | ||||||
|  |   end | ||||||
|  | end | ||||||
| @@ -0,0 +1,21 @@ | |||||||
|  | # frozen_string_literal: true | ||||||
|  |  | ||||||
|  | RSpec.shared_examples 'encrypted external credential' do |factory:, attribute:, value: 'secret-token'| | ||||||
|  |   before do | ||||||
|  |     skip('encryption keys missing; see run_mfa_spec workflow') unless Chatwoot.encryption_configured? | ||||||
|  |     if defined?(Facebook::Messenger::Subscriptions) | ||||||
|  |       allow(Facebook::Messenger::Subscriptions).to receive(:subscribe).and_return(true) | ||||||
|  |       allow(Facebook::Messenger::Subscriptions).to receive(:unsubscribe).and_return(true) | ||||||
|  |     end | ||||||
|  |   end | ||||||
|  |  | ||||||
|  |   it "encrypts #{attribute} at rest" do | ||||||
|  |     record = create(factory, attribute => value) | ||||||
|  |  | ||||||
|  |     raw_stored_value = record.reload.read_attribute_before_type_cast(attribute).to_s | ||||||
|  |     expect(raw_stored_value).to be_present | ||||||
|  |     expect(raw_stored_value).not_to include(value) | ||||||
|  |     expect(record.public_send(attribute)).to eq(value) | ||||||
|  |     expect(record.encrypted_attribute?(attribute)).to be(true) | ||||||
|  |   end | ||||||
|  | end | ||||||
		Reference in New Issue
	
	Block a user
	 Sojan Jose
					Sojan Jose