From 2663a8495d02622c108ed438a0dc0254966515c6 Mon Sep 17 00:00:00 2001 From: Sugar Ray Ledesma Date: Thu, 11 Jun 2026 22:06:34 +0900 Subject: [PATCH 01/10] refactor(security): rename path_without_extensions in Rack::Attack (#14216) Fix misspelling 'extentions' and clarify comments. Behavior unchanged: same path-stripping logic for throttle matching (e.g. /auth and /auth.json). Also correct 'You may' in remote_ip comment. --- config/initializers/rack_attack.rb | 40 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 15e78af9b..0158e3284 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -19,7 +19,7 @@ class Rack::Attack Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(redis: $velma, pool: false) class Request < ::Rack::Request - # You many need to specify a method to fetch the correct remote IP address + # You may need to specify a method to fetch the correct remote IP address # if the web server is behind a load balancer. def remote_ip @remote_ip ||= (env['action_dispatch.remote_ip'] || ip).to_s @@ -31,9 +31,9 @@ class Rack::Attack (default_allowed_ips + env_allowed_ips).include?(remote_ip) end - # Rails would allow requests to paths with extentions, so lets compare against the path with extention stripped + # Rails would allow requests to paths with extensions, so lets compare against the path with extension stripped # example /auth & /auth.json would both work - def path_without_extentions + def path_without_extensions path[/^[^.]+/] end end @@ -75,11 +75,11 @@ class Rack::Attack ### Prevent Brute-Force Super Admin Login Attacks ### throttle('super_admin_login/ip', limit: 5, period: 5.minutes) do |req| - req.ip if req.path_without_extentions == '/super_admin/sign_in' && req.post? + req.ip if req.path_without_extensions == '/super_admin/sign_in' && req.post? end throttle('super_admin_login/email', limit: 5, period: 15.minutes) do |req| - if req.path_without_extentions == '/super_admin/sign_in' && req.post? + if req.path_without_extensions == '/super_admin/sign_in' && req.post? # NOTE: This line used to throw ArgumentError /rails/action_mailbox/sendgrid/inbound_emails : invalid byte sequence in UTF-8 # Hence placed in the if block # ref: https://github.com/rack/rack-attack/issues/399 @@ -91,7 +91,7 @@ class Rack::Attack # ### Prevent Brute-Force Login Attacks ### # Exclude MFA verification attempts from regular login throttling throttle('login/ip', limit: 5, period: 5.minutes) do |req| - if req.path_without_extentions == '/auth/sign_in' && req.post? && req.params['mfa_token'].blank? + if req.path_without_extensions == '/auth/sign_in' && req.post? && req.params['mfa_token'].blank? # Skip if this is an MFA verification request req.ip end @@ -99,7 +99,7 @@ class Rack::Attack throttle('login/email', limit: 10, period: 15.minutes) do |req| # Skip if this is an MFA verification request - if req.path_without_extentions == '/auth/sign_in' && req.post? && req.params['mfa_token'].blank? + if req.path_without_extensions == '/auth/sign_in' && req.post? && req.params['mfa_token'].blank? # ref: https://github.com/rack/rack-attack/issues/399 # NOTE: This line used to throw ArgumentError /rails/action_mailbox/sendgrid/inbound_emails : invalid byte sequence in UTF-8 # Hence placed in the if block @@ -110,11 +110,11 @@ class Rack::Attack ## Reset password throttling throttle('reset_password/ip', limit: 5, period: 30.minutes) do |req| - req.ip if req.path_without_extentions == '/auth/password' && req.post? + req.ip if req.path_without_extensions == '/auth/password' && req.post? end throttle('reset_password/email', limit: 5, period: 1.hour) do |req| - if req.path_without_extentions == '/auth/password' && req.post? + if req.path_without_extensions == '/auth/password' && req.post? email = req.params['email'].presence || ActionDispatch::Request.new(req.env).params['email'].presence email.to_s.downcase.gsub(/\s+/, '') end @@ -122,11 +122,11 @@ class Rack::Attack ## Resend confirmation throttling (unauthenticated) throttle('resend_confirmation/ip', limit: 5, period: 30.minutes) do |req| - req.ip if req.path_without_extentions == '/resend_confirmation' && req.post? + req.ip if req.path_without_extensions == '/resend_confirmation' && req.post? end throttle('resend_confirmation/email', limit: 5, period: 1.hour) do |req| - if req.path_without_extentions == '/resend_confirmation' && req.post? + if req.path_without_extensions == '/resend_confirmation' && req.post? email = req.params['email'].presence || ActionDispatch::Request.new(req.env).params['email'].presence email.to_s.downcase.gsub(/\s+/, '') end @@ -134,25 +134,25 @@ class Rack::Attack ## Resend confirmation throttling (authenticated) throttle('resend_confirmation_auth/ip', limit: 5, period: 30.minutes) do |req| - req.ip if req.path_without_extentions == '/api/v1/profile/resend_confirmation' && req.post? + req.ip if req.path_without_extensions == '/api/v1/profile/resend_confirmation' && req.post? end ## MFA throttling - prevent brute force attacks throttle('mfa_verification/ip', limit: 5, period: 1.minute) do |req| - if req.path_without_extentions == '/api/v1/profile/mfa' + if req.path_without_extensions == '/api/v1/profile/mfa' req.ip if req.delete? # Throttle disable attempts - elsif req.path_without_extentions.match?(%r{/api/v1/profile/mfa/(verify|backup_codes)}) + elsif req.path_without_extensions.match?(%r{/api/v1/profile/mfa/(verify|backup_codes)}) req.ip if req.post? # Throttle verify and backup_codes attempts end end # Separate rate limiting for MFA verification attempts throttle('mfa_login/ip', limit: 10, period: 1.minute) do |req| - req.ip if req.path_without_extentions == '/auth/sign_in' && req.post? && req.params['mfa_token'].present? + req.ip if req.path_without_extensions == '/auth/sign_in' && req.post? && req.params['mfa_token'].present? end throttle('mfa_login/token', limit: 10, period: 1.minute) do |req| - if req.path_without_extentions == '/auth/sign_in' && req.post? + if req.path_without_extensions == '/auth/sign_in' && req.post? # Track by MFA token to prevent brute force on a specific token mfa_token = req.params['mfa_token'].presence (mfa_token.presence) @@ -161,7 +161,7 @@ class Rack::Attack ## Prevent Brute-Force Signup Attacks ### throttle('accounts/ip', limit: 5, period: 30.minutes) do |req| - req.ip if req.path_without_extentions == '/api/v1/accounts' && req.post? + req.ip if req.path_without_extensions == '/api/v1/accounts' && req.post? end ##-----------------------------------------------## @@ -176,17 +176,17 @@ class Rack::Attack if ActiveModel::Type::Boolean.new.cast(ENV.fetch('ENABLE_RACK_ATTACK_WIDGET_API', true)) ## Prevent Conversation Bombing on Widget APIs ### throttle('api/v1/widget/conversations', limit: 6, period: 12.hours) do |req| - req.ip if req.path_without_extentions == '/api/v1/widget/conversations' && req.post? + req.ip if req.path_without_extensions == '/api/v1/widget/conversations' && req.post? end ## Prevent Contact update Bombing in Widget API ### throttle('api/v1/widget/contacts', limit: 60, period: 1.hour) do |req| - req.ip if req.path_without_extentions == '/api/v1/widget/contacts' && (req.patch? || req.put?) + req.ip if req.path_without_extensions == '/api/v1/widget/contacts' && (req.patch? || req.put?) end ## Prevent Conversation Bombing through multiple sessions throttle('widget?website_token={website_token}&cw_conversation={x-auth-token}', limit: 5, period: 1.hour) do |req| - req.ip if req.path_without_extentions == '/widget' && ActionDispatch::Request.new(req.env).params['cw_conversation'].blank? + req.ip if req.path_without_extensions == '/widget' && ActionDispatch::Request.new(req.env).params['cw_conversation'].blank? end end From c6a38e2fc61a05af85cc966fd35fcbcd12883295 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Fri, 12 Jun 2026 09:39:26 +0400 Subject: [PATCH 02/10] fix: Keep Instagram scopes out of new Messenger OAuth flows (#14695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes Instagram permissions from the Facebook Messenger inbox setup flow to avoid blocking Meta App Review for Messenger-only apps. Meta rejects or stalls Messenger-only app reviews when the OAuth flow requests unrelated Instagram permissions such as `instagram_basic` and `instagram_manage_messages`. This blocks approval for Messenger permissions like `human_agent`, even when the user only wants to configure a Facebook Messenger inbox. New Facebook Messenger inbox setup now requests only the Page and Messenger permissions required for Messenger. Existing Facebook Messenger inboxes that already have Instagram details continue to request Instagram permissions during reauthorization, preserving support for the legacy combined Facebook/Instagram inbox flow. Going forward, new Instagram setups should use the dedicated Instagram channel inbox instead of being bundled into the Messenger setup flow. Closes #13860 ## How to test 1. Go to Settings → Inboxes → Add Inbox → Facebook Messenger. 2. Click Login with Facebook. 3. Verify the OAuth scope does not include `instagram_basic` or `instagram_manage_messages`. 4. Reauthorize an existing Facebook Messenger inbox with `instagram_id` present. 5. Verify the reauth scope still includes `instagram_basic` and `instagram_manage_messages`. --------- Co-authored-by: Muhsin <12408980+muhsin-k@users.noreply.github.com> --- .../spec/useFacebookPageConnect.spec.js | 3 ++- .../composables/useFacebookPageConnect.js | 8 ++----- .../dashboard/helper/facebookScopes.js | 22 +++++++++++++++++++ .../settings/inbox/facebook/Reauthorize.vue | 9 ++++++-- 4 files changed, 33 insertions(+), 9 deletions(-) create mode 100644 app/javascript/dashboard/helper/facebookScopes.js diff --git a/app/javascript/dashboard/composables/spec/useFacebookPageConnect.spec.js b/app/javascript/dashboard/composables/spec/useFacebookPageConnect.spec.js index 5d9edb993..4ec17e753 100644 --- a/app/javascript/dashboard/composables/spec/useFacebookPageConnect.spec.js +++ b/app/javascript/dashboard/composables/spec/useFacebookPageConnect.spec.js @@ -70,7 +70,8 @@ describe('useFacebookPageConnect', () => { ACCOUNT_ID ); expect(window.FB.login).toHaveBeenCalledWith(expect.any(Function), { - scope: expect.stringContaining('pages_show_list'), + scope: + 'pages_manage_metadata,business_management,pages_messaging,pages_show_list,pages_read_engagement', }); }); diff --git a/app/javascript/dashboard/composables/useFacebookPageConnect.js b/app/javascript/dashboard/composables/useFacebookPageConnect.js index 46a177d58..dbe06dd0d 100644 --- a/app/javascript/dashboard/composables/useFacebookPageConnect.js +++ b/app/javascript/dashboard/composables/useFacebookPageConnect.js @@ -1,13 +1,9 @@ import { ref } from 'vue'; import { useMapGetter } from 'dashboard/composables/store'; import ChannelApi from 'dashboard/api/channels'; +import { buildFacebookLoginScopes } from 'dashboard/helper/facebookScopes'; import { setupFacebookSdk } from 'dashboard/routes/dashboard/settings/inbox/channels/whatsapp/utils'; -// Page-management + messaging scopes required to list pages and create a -// Channel::FacebookPage inbox (mirrors the standalone settings flow). -const FB_PAGE_SCOPES = - 'pages_manage_metadata,business_management,pages_messaging,instagram_basic,pages_show_list,pages_read_engagement,instagram_manage_messages'; - // Headless half of the Facebook Page connect flow: load the Meta SDK, run // FB.login for page scopes, and fetch the user's pages. The caller owns the // page-picker UI and the channel creation, because choosing a page is an @@ -51,7 +47,7 @@ export function useFacebookPageConnect() { : null ); }, - { scope: FB_PAGE_SCOPES } + { scope: buildFacebookLoginScopes() } ); }); diff --git a/app/javascript/dashboard/helper/facebookScopes.js b/app/javascript/dashboard/helper/facebookScopes.js new file mode 100644 index 000000000..755b3f465 --- /dev/null +++ b/app/javascript/dashboard/helper/facebookScopes.js @@ -0,0 +1,22 @@ +export const FACEBOOK_PAGE_SCOPES = [ + 'pages_manage_metadata', + 'business_management', + 'pages_messaging', + 'pages_show_list', + 'pages_read_engagement', +]; + +export const INSTAGRAM_SCOPES = [ + 'instagram_basic', + 'instagram_manage_messages', +]; + +export const buildFacebookLoginScopes = ({ + includeInstagramScopes = false, +} = {}) => { + const scopes = [...FACEBOOK_PAGE_SCOPES]; + if (includeInstagramScopes) { + scopes.push(...INSTAGRAM_SCOPES); + } + return scopes.join(','); +}; diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/facebook/Reauthorize.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/facebook/Reauthorize.vue index dd566f131..cf25177ea 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/inbox/facebook/Reauthorize.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/inbox/facebook/Reauthorize.vue @@ -4,6 +4,7 @@ import InboxReconnectionRequired from '../components/InboxReconnectionRequired.v import { useAlert } from 'dashboard/composables'; import { loadScript } from 'dashboard/helper/DOMHelpers'; +import { buildFacebookLoginScopes } from 'dashboard/helper/facebookScopes'; import * as Sentry from '@sentry/vue'; export default { @@ -20,6 +21,11 @@ export default { inboxId() { return this.inbox.id; }, + facebookLoginScopes() { + return buildFacebookLoginScopes({ + includeInstagramScopes: !!this.inbox.instagram_id, + }); + }, }, mounted() { window.fbAsyncInit = this.runFBInit; @@ -77,8 +83,7 @@ export default { } }, { - scope: - 'pages_manage_metadata,business_management,pages_messaging,instagram_basic,pages_show_list,pages_read_engagement,instagram_manage_messages', + scope: this.facebookLoginScopes, auth_type: 'reauthorize', } ); From c041fde3a2d745b9b69d73df8726c687cf351164 Mon Sep 17 00:00:00 2001 From: Rorrick <34399621+Rorrick@users.noreply.github.com> Date: Fri, 12 Jun 2026 08:21:42 +0200 Subject: [PATCH 03/10] fix: Preserve original filenames for WhatsApp cloud attachments (#14168) ## Summary Preserves the original filename provided in the WhatsApp Cloud attachment payload when downloading media files. ## Problem Files containing accented or special characters could be saved with malformed names or incorrect extensions due to relying on remote download metadata. ## Changes Made - Uses attachment_payload[:filename] when present - Creates a tempfile using the original filename and extension - Preserves content type metadata - Falls back to existing behavior when no filename is provided ## Notes This should improve attachment downloads for filenames containing accented characters. Related to #10973 --------- Co-authored-by: Rorrick Smith Co-authored-by: Muhsin Keloth Co-authored-by: Muhsin <12408980+muhsin-k@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ...incoming_message_whatsapp_cloud_service.rb | 11 +++++- ...ing_message_whatsapp_cloud_service_spec.rb | 35 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb b/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb index 164c3ac12..f836a63c3 100644 --- a/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb +++ b/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb @@ -13,8 +13,17 @@ class Whatsapp::IncomingMessageWhatsappCloudService < Whatsapp::IncomingMessageB inbox.channel.media_url(attachment_payload[:id]), headers: inbox.channel.api_headers ) + # This url response will be failure if the access token has expired. inbox.channel.authorization_error! if url_response.unauthorized? - Down.download(url_response.parsed_response['url'], headers: inbox.channel.api_headers) if url_response.success? + + return unless url_response.success? + + downloaded_file = Down.download(url_response.parsed_response['url'], headers: inbox.channel.api_headers) + # WhatsApp Cloud sends the original filename in the payload; preserve it so accented + # names keep their correct extension instead of relying on the mangled remote metadata. + filename = attachment_payload[:filename] + downloaded_file.define_singleton_method(:original_filename) { filename } if filename.present? + downloaded_file end end diff --git a/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb b/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb index 70c29c092..1bbd6d8f4 100644 --- a/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb +++ b/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb @@ -59,6 +59,41 @@ describe Whatsapp::IncomingMessageWhatsappCloudService do end end + context 'when document attachment includes an accented filename' do + let(:document_params) do + { + phone_number: whatsapp_channel.phone_number, + object: 'whatsapp_business_account', + entry: [{ + changes: [{ + value: { + contacts: [{ profile: { name: 'Sojan Jose' }, wa_id: '2423423243' }], + messages: [{ + from: '2423423243', + document: { + id: 'b1c68f38-8734-4ad3-b4a1-ef0c10d683', + mime_type: 'application/pdf', + filename: 'Currículum café.pdf', + caption: 'My résumé' + }, + timestamp: '1664799904', type: 'document' + }] + } + }] + }] + }.with_indifferent_access + end + + it 'preserves the original filename from the payload' do + stub_media_url_request + stub_sample_png_request + described_class.new(inbox: whatsapp_channel.inbox, params: document_params).perform + + attachment = whatsapp_channel.inbox.messages.first.attachments.first + expect(attachment.file.filename.to_s).to eq('Currículum café.pdf') + end + end + context 'when invalid attachment message params' do let(:error_params) do { From e055cead35fb8cfead9c7cd1ffc396c320be3c45 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:25:25 +0530 Subject: [PATCH 04/10] fix: only subscribe calls webhook field when voice calling enabled (#14718) ## Description Solves issue https://github.com/chatwoot/chatwoot/issues/14690 ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) ## How has this been tested? - UI flows ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Muhsin Keloth --- app/models/channel/whatsapp.rb | 16 +++++----- app/services/whatsapp/facebook_api_client.rb | 2 +- .../whatsapp/webhook_setup_service.rb | 20 +++++++------ .../whatsapp/facebook_api_client_spec.rb | 4 +-- .../whatsapp/webhook_setup_service_spec.rb | 29 +++++++++++-------- 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/app/models/channel/whatsapp.rb b/app/models/channel/whatsapp.rb index 46cf65f3b..2a558cd13 100644 --- a/app/models/channel/whatsapp.rb +++ b/app/models/channel/whatsapp.rb @@ -69,8 +69,10 @@ class Channel::Whatsapp < ApplicationRecord end end - # Enables voice: turns calling on at Meta (idempotent), subscribes the `calls` - # webhook field, and sets calling_enabled. Raises on Meta failure. + # Enables voice: turns calling on at Meta (idempotent), then re-registers webhooks + # with the in-memory calling_enabled flag so the `calls` field is subscribed. The + # flag is persisted only after registration succeeds, so a webhook failure can't + # leave the inbox reporting voice_enabled? while the WABA isn't subscribed to calls. # Saved with validate: false to skip validate_provider_config's remote credential # re-check, which could spuriously fail and desync the flag from Meta. def enable_voice_calling! @@ -78,21 +80,21 @@ class Channel::Whatsapp < ApplicationRecord raise 'WhatsApp calling requires the channel_voice feature' unless account.feature_enabled?('channel_voice') provider_service.update_calling_status('ENABLED') - webhook_setup_service.register_callback self.provider_config = provider_config.merge('calling_enabled' => true) + webhook_setup_service.register_callback save!(validate: false) end - # Disables voice: unsets calling_enabled (gates the call subsystem) and drops - # `calls` from the webhook subscription (best-effort, so a Meta outage can't - # trap admins). Leaves Meta's WABA calling.status untouched. + # Disables voice: unsets calling_enabled (gates the call subsystem) and re-registers + # webhooks, which drops `calls` from the subscription (best-effort, so a Meta outage + # can't trap admins). Leaves Meta's WABA calling.status untouched. def disable_voice_calling! raise 'WhatsApp calling requires a whatsapp_cloud inbox' unless voice_calling_supported? self.provider_config = provider_config.merge('calling_enabled' => false) save!(validate: false) begin - webhook_setup_service.register_callback(subscribed_fields: %w[messages smb_message_echoes]) + webhook_setup_service.register_callback rescue StandardError => e Rails.logger.warn "[WHATSAPP CALL] disable webhook re-subscribe failed: #{e.message}" end diff --git a/app/services/whatsapp/facebook_api_client.rb b/app/services/whatsapp/facebook_api_client.rb index eef84b022..22e75aac0 100644 --- a/app/services/whatsapp/facebook_api_client.rb +++ b/app/services/whatsapp/facebook_api_client.rb @@ -60,7 +60,7 @@ class Whatsapp::FacebookApiClient data['code_verification_status'] == 'VERIFIED' end - WEBHOOK_DEFAULT_FIELDS = %w[messages smb_message_echoes calls].freeze + WEBHOOK_DEFAULT_FIELDS = %w[messages smb_message_echoes].freeze def subscribe_waba_webhook(waba_id, callback_url, verify_token, subscribed_fields: WEBHOOK_DEFAULT_FIELDS) # Step 1: Subscribe app to WABA first (required before override) diff --git a/app/services/whatsapp/webhook_setup_service.rb b/app/services/whatsapp/webhook_setup_service.rb index a287b4977..2abf113da 100644 --- a/app/services/whatsapp/webhook_setup_service.rb +++ b/app/services/whatsapp/webhook_setup_service.rb @@ -17,9 +17,9 @@ class Whatsapp::WebhookSetupService setup_webhook end - def register_callback(subscribed_fields: nil) + def register_callback validate_parameters! - setup_webhook(subscribed_fields: subscribed_fields) + setup_webhook end private @@ -55,21 +55,23 @@ class Whatsapp::WebhookSetupService @channel.save! end - def setup_webhook(subscribed_fields: nil) + def setup_webhook callback_url = build_callback_url verify_token = @channel.provider_config['webhook_verify_token'] - args = [@waba_id, callback_url, verify_token] - if subscribed_fields - @api_client.subscribe_waba_webhook(*args, subscribed_fields: subscribed_fields) - else - @api_client.subscribe_waba_webhook(*args) - end + @api_client.subscribe_waba_webhook(@waba_id, callback_url, verify_token, subscribed_fields: subscribed_fields) rescue StandardError => e Rails.logger.error("[WHATSAPP] Webhook setup failed: #{e.message}") raise "Webhook setup failed: #{e.message}" end + # Subscribe to `calls` only when voice calling is enabled on the inbox + def subscribed_fields + fields = %w[messages smb_message_echoes] + fields << 'calls' if @channel.provider_config['calling_enabled'] + fields + end + def build_callback_url frontend_url = ENV.fetch('FRONTEND_URL', nil) phone_number = @channel.phone_number diff --git a/spec/services/whatsapp/facebook_api_client_spec.rb b/spec/services/whatsapp/facebook_api_client_spec.rb index a33999bee..74fb2f6e2 100644 --- a/spec/services/whatsapp/facebook_api_client_spec.rb +++ b/spec/services/whatsapp/facebook_api_client_spec.rb @@ -177,7 +177,7 @@ describe Whatsapp::FacebookApiClient do .with( headers: { 'Authorization' => "Bearer #{access_token}", 'Content-Type' => 'application/json' }, body: { override_callback_uri: callback_url, verify_token: verify_token, - subscribed_fields: %w[messages smb_message_echoes calls] }.to_json + subscribed_fields: %w[messages smb_message_echoes] }.to_json ) .to_return( status: 200, @@ -224,7 +224,7 @@ describe Whatsapp::FacebookApiClient do .with( headers: { 'Authorization' => "Bearer #{access_token}", 'Content-Type' => 'application/json' }, body: { override_callback_uri: callback_url, verify_token: verify_token, - subscribed_fields: %w[messages smb_message_echoes calls] }.to_json + subscribed_fields: %w[messages smb_message_echoes] }.to_json ) .to_return(status: 400, body: { error: 'Webhook callback override failed' }.to_json) end diff --git a/spec/services/whatsapp/webhook_setup_service_spec.rb b/spec/services/whatsapp/webhook_setup_service_spec.rb index d35d14cb9..e80036f32 100644 --- a/spec/services/whatsapp/webhook_setup_service_spec.rb +++ b/spec/services/whatsapp/webhook_setup_service_spec.rb @@ -43,7 +43,7 @@ describe Whatsapp::WebhookSetupService do allow(SecureRandom).to receive(:random_number).with(900_000).and_return(123_456) allow(api_client).to receive(:register_phone_number).with('123456789', 223_456) allow(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, anything, 'test_verify_token').and_return({ 'success' => true }) + .with(waba_id, anything, 'test_verify_token', subscribed_fields: %w[messages smb_message_echoes]).and_return({ 'success' => true }) allow(channel).to receive(:save!) end @@ -51,7 +51,8 @@ describe Whatsapp::WebhookSetupService do with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do expect(api_client).to receive(:register_phone_number).with('123456789', 223_456) expect(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token') + .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token', subscribed_fields: %w[messages + smb_message_echoes]) service.perform end end @@ -65,14 +66,15 @@ describe Whatsapp::WebhookSetupService do throughput: { level: 'APPLICABLE' } }) allow(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, anything, 'test_verify_token').and_return({ 'success' => true }) + .with(waba_id, anything, 'test_verify_token', subscribed_fields: %w[messages smb_message_echoes]).and_return({ 'success' => true }) end it 'does NOT register phone, but sets up webhook' do with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do expect(api_client).not_to receive(:register_phone_number) expect(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token') + .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token', subscribed_fields: %w[messages + smb_message_echoes]) service.perform end end @@ -88,7 +90,7 @@ describe Whatsapp::WebhookSetupService do allow(SecureRandom).to receive(:random_number).with(900_000).and_return(123_456) allow(api_client).to receive(:register_phone_number).with('123456789', 223_456) allow(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, anything, 'test_verify_token').and_return({ 'success' => true }) + .with(waba_id, anything, 'test_verify_token', subscribed_fields: %w[messages smb_message_echoes]).and_return({ 'success' => true }) allow(channel).to receive(:save!) end @@ -96,7 +98,8 @@ describe Whatsapp::WebhookSetupService do with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do expect(api_client).to receive(:register_phone_number).with('123456789', 223_456) expect(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token') + .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token', subscribed_fields: %w[messages + smb_message_echoes]) service.perform end end @@ -112,7 +115,7 @@ describe Whatsapp::WebhookSetupService do allow(SecureRandom).to receive(:random_number).with(900_000).and_return(123_456) allow(api_client).to receive(:register_phone_number).with('123456789', 223_456) allow(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, anything, 'test_verify_token').and_return({ 'success' => true }) + .with(waba_id, anything, 'test_verify_token', subscribed_fields: %w[messages smb_message_echoes]).and_return({ 'success' => true }) allow(channel).to receive(:save!) end @@ -120,7 +123,8 @@ describe Whatsapp::WebhookSetupService do with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do expect(api_client).to receive(:register_phone_number).with('123456789', 223_456) expect(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token') + .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'test_verify_token', subscribed_fields: %w[messages + smb_message_echoes]) service.perform end end @@ -279,14 +283,15 @@ describe Whatsapp::WebhookSetupService do throughput: { level: 'APPLICABLE' } }) allow(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, anything, 'existing_verify_token').and_return({ 'success' => true }) + .with(waba_id, anything, 'existing_verify_token', subscribed_fields: %w[messages smb_message_echoes]).and_return({ 'success' => true }) end it 'successfully reauthorizes with new access token' do with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do expect(api_client).not_to receive(:register_phone_number) expect(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'existing_verify_token') + .with(waba_id, 'https://app.chatwoot.com/webhooks/whatsapp/+1234567890', 'existing_verify_token', + subscribed_fields: %w[messages smb_message_echoes]) service_reauth.perform end end @@ -294,7 +299,7 @@ describe Whatsapp::WebhookSetupService do it 'uses the existing webhook verify token during reauthorization' do with_modified_env FRONTEND_URL: 'https://app.chatwoot.com' do expect(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, anything, 'existing_verify_token') + .with(waba_id, anything, 'existing_verify_token', subscribed_fields: %w[messages smb_message_echoes]) service_reauth.perform end end @@ -308,7 +313,7 @@ describe Whatsapp::WebhookSetupService do throughput: { level: 'APPLICABLE' } }) allow(api_client).to receive(:subscribe_waba_webhook) - .with(waba_id, anything, 'test_verify_token').and_return({ 'success' => true }) + .with(waba_id, anything, 'test_verify_token', subscribed_fields: %w[messages smb_message_echoes]).and_return({ 'success' => true }) end it 'completes successfully without errors' do From 92a1fb8ab7226cadf4dcdee822fac47abad45a36 Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Fri, 12 Jun 2026 16:11:07 +0530 Subject: [PATCH 05/10] feat: manage active user sessions from profile (CW-7169) (#14556) ## Description First PR of user_sessions feature - enforcement, impersonation and mfa will be handled separately. Adds an Active Sessions section under Profile where users can see every device currently logged in and revoke any session they don't recognize. Helps users lock down stale or unrecognized logins on their own without needing support. **Behavior at the limit, by client:** - **Browser:** returns 409 with a picker overlay; user picks a session to revoke or chooses "End all sessions" to clear them. - **Mobile / API client:** silently evicts the oldest session and proceeds with login (no picker UI to render). - **Pre-tracking users** (token rows without `user_sessions`, i.e. anyone already logged in before this ships): silent-evict any untracked token first, so freshly tracked sessions are never killed in favor of legacy ones. Sessions are stored in a new `user_sessions` table keyed on `(user_id, client_id)` with browser, platform, IP, last activity and (when configured) geo. Kept in sync with `user.tokens` via an after_save callback so revoking a token from any path cleans up the row. Fixes https://linear.app/chatwoot/issue/CW-7169 ## Type of change - [x] New feature (non-breaking change which adds functionality) ## How Has This Been Tested? - Added specs. - Manual local testing: browser picker fires at limit; pre-tracking user silent-evicts; mixed tracked/untracked correctly drops the untracked one first; profile page revoke succeeds; current session cannot be revoked from profile. ## Checklist - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules --- .../api/v1/profile/sessions_controller.rb | 36 +++++ app/controllers/application_controller.rb | 1 + .../concerns/track_session_activity.rb | 22 +++ .../devise_overrides/sessions_controller.rb | 14 ++ app/javascript/dashboard/api/auth.js | 6 + .../helper/AnalyticsHelper/events.js | 4 + .../dashboard/i18n/locale/en/settings.json | 11 ++ .../settings/profile/ActiveSessions.vue | 137 ++++++++++++++++++ .../dashboard/settings/profile/Index.vue | 9 ++ app/jobs/user_session_ip_lookup_job.rb | 18 +++ app/models/user.rb | 7 + app/models/user_session.rb | 46 ++++++ app/services/user_session_tracking_service.rb | 39 +++++ .../v1/profile/sessions/index.json.jbuilder | 15 ++ config/locales/en.yml | 4 + config/routes.rb | 1 + .../20260611184600_create_user_sessions.rb | 23 +++ db/schema.rb | 23 ++- .../sessions_controller_spec.rb | 17 +++ spec/jobs/user_session_ip_lookup_job_spec.rb | 45 ++++++ spec/models/user_session_spec.rb | 61 ++++++++ spec/models/user_spec.rb | 33 +++++ .../v1/profile/sessions_controller_spec.rb | 101 +++++++++++++ .../user_session_tracking_service_spec.rb | 82 +++++++++++ 24 files changed, 754 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/v1/profile/sessions_controller.rb create mode 100644 app/controllers/concerns/track_session_activity.rb create mode 100644 app/javascript/dashboard/routes/dashboard/settings/profile/ActiveSessions.vue create mode 100644 app/jobs/user_session_ip_lookup_job.rb create mode 100644 app/models/user_session.rb create mode 100644 app/services/user_session_tracking_service.rb create mode 100644 app/views/api/v1/profile/sessions/index.json.jbuilder create mode 100644 db/migrate/20260611184600_create_user_sessions.rb create mode 100644 spec/jobs/user_session_ip_lookup_job_spec.rb create mode 100644 spec/models/user_session_spec.rb create mode 100644 spec/requests/api/v1/profile/sessions_controller_spec.rb create mode 100644 spec/services/user_session_tracking_service_spec.rb diff --git a/app/controllers/api/v1/profile/sessions_controller.rb b/app/controllers/api/v1/profile/sessions_controller.rb new file mode 100644 index 000000000..72e9451eb --- /dev/null +++ b/app/controllers/api/v1/profile/sessions_controller.rb @@ -0,0 +1,36 @@ +class Api::V1::Profile::SessionsController < Api::BaseController + before_action :set_session, only: [:destroy] + + def index + @sessions = current_user.user_sessions.where(client_id: active_token_client_ids).order(last_activity_at: :desc) + @current_client_id = request.headers['client'] + end + + def destroy + if @session.current?(request.headers['client']) + render json: { error: I18n.t('profile_settings.sessions.cannot_revoke_current') }, status: :unprocessable_entity + return + end + + revoke_token!(@session.client_id) + @session.destroy! + head :ok + end + + private + + def set_session + @session = current_user.user_sessions.find(params[:id]) + end + + def revoke_token!(client_id) + tokens = current_user.tokens + tokens.delete(client_id) + current_user.update!(tokens: tokens) + end + + def active_token_client_ids + now = Time.current.to_i + (current_user.tokens || {}).select { |_, v| v['expiry'].to_i > now }.keys + end +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2f389049d..9dea4b4da 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,7 @@ class ApplicationController < ActionController::Base include RequestExceptionHandler include Pundit::Authorization include SwitchLocale + include TrackSessionActivity skip_before_action :verify_authenticity_token diff --git a/app/controllers/concerns/track_session_activity.rb b/app/controllers/concerns/track_session_activity.rb new file mode 100644 index 000000000..f6a512922 --- /dev/null +++ b/app/controllers/concerns/track_session_activity.rb @@ -0,0 +1,22 @@ +module TrackSessionActivity + extend ActiveSupport::Concern + + included do + after_action :update_session_activity + end + + private + + def update_session_activity + return unless current_user + return if request.headers['client'].blank? + + UserSessionTrackingService.new( + user: current_user, + request: request, + client_id: request.headers['client'] + ).update_activity! + rescue StandardError => e + Rails.logger.warn "Session activity update failed: #{e.message}" + end +end diff --git a/app/controllers/devise_overrides/sessions_controller.rb b/app/controllers/devise_overrides/sessions_controller.rb index bd7bb9b44..1ef6d9511 100644 --- a/app/controllers/devise_overrides/sessions_controller.rb +++ b/app/controllers/devise_overrides/sessions_controller.rb @@ -20,6 +20,7 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController end def render_create_success + track_user_session render partial: 'devise/auth', formats: [:json], locals: { resource: @resource } end @@ -114,6 +115,19 @@ class DeviseOverrides::SessionsController < DeviseTokenAuth::SessionsController def render_mfa_error(message_key, status = :bad_request) render json: { error: I18n.t(message_key) }, status: status end + + def track_user_session + client_id = @token&.try(:client) || response.headers['client'] + return unless client_id.present? && @resource.present? + + UserSessionTrackingService.new( + user: @resource, + request: request, + client_id: client_id + ).create_or_update! + rescue StandardError => e + Rails.logger.warn "Session tracking failed: #{e.message}" + end end DeviseOverrides::SessionsController.prepend_mod_with('DeviseOverrides::SessionsController') diff --git a/app/javascript/dashboard/api/auth.js b/app/javascript/dashboard/api/auth.js index a1b15ee79..b9dc59964 100644 --- a/app/javascript/dashboard/api/auth.js +++ b/app/javascript/dashboard/api/auth.js @@ -106,4 +106,10 @@ export default { const urlData = endPoints('resetAccessToken'); return axios.post(urlData.url); }, + getSessions() { + return axios.get('/api/v1/profile/sessions'); + }, + revokeSession(id) { + return axios.delete(`/api/v1/profile/sessions/${id}`); + }, }; diff --git a/app/javascript/dashboard/helper/AnalyticsHelper/events.js b/app/javascript/dashboard/helper/AnalyticsHelper/events.js index 6dc47cfbf..751229404 100644 --- a/app/javascript/dashboard/helper/AnalyticsHelper/events.js +++ b/app/javascript/dashboard/helper/AnalyticsHelper/events.js @@ -154,6 +154,10 @@ export const YEAR_IN_REVIEW_EVENTS = Object.freeze({ SHARE_CLICKED: 'Year in Review: Share clicked', }); +export const SESSION_EVENTS = Object.freeze({ + REVOKED_FROM_PROFILE: 'Revoked an active session', +}); + export const ONBOARDING_EVENTS = Object.freeze({ ACCOUNT_DETAILS_VISITED: 'Onboarding: Account details visited', ACCOUNT_DETAILS_COMPLETED: 'Onboarding: Account details completed', diff --git a/app/javascript/dashboard/i18n/locale/en/settings.json b/app/javascript/dashboard/i18n/locale/en/settings.json index bfbd920a7..d14175cb5 100644 --- a/app/javascript/dashboard/i18n/locale/en/settings.json +++ b/app/javascript/dashboard/i18n/locale/en/settings.json @@ -86,6 +86,17 @@ "NOTE": "Manage additional security features for your account.", "MFA_BUTTON": "Manage Two-Factor Authentication" }, + "SESSIONS_SECTION": { + "TITLE": "Active Sessions", + "NOTE": "These are the devices currently logged in to your account.", + "CURRENT": "Current session", + "REVOKE": "Revoke", + "REVOKE_SUCCESS": "Session revoked successfully", + "REVOKE_ERROR": "Unable to revoke session. Please try again.", + "FETCH_ERROR": "Unable to fetch sessions. Please try again.", + "LAST_ACTIVE": "Last active", + "UNKNOWN_DEVICE": "Unknown device" + }, "ACCESS_TOKEN": { "TITLE": "Access Token", "NOTE": "This token can be used if you are building an API based integration", diff --git a/app/javascript/dashboard/routes/dashboard/settings/profile/ActiveSessions.vue b/app/javascript/dashboard/routes/dashboard/settings/profile/ActiveSessions.vue new file mode 100644 index 000000000..ed4868462 --- /dev/null +++ b/app/javascript/dashboard/routes/dashboard/settings/profile/ActiveSessions.vue @@ -0,0 +1,137 @@ + + + diff --git a/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue b/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue index 75eb8a2f8..04de7bda2 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue @@ -20,6 +20,7 @@ import SectionLayout from '../account/components/SectionLayout.vue'; import BaseSettingsHeader from '../components/BaseSettingsHeader.vue'; import AccessToken from './AccessToken.vue'; import MfaSettingsCard from './MfaSettingsCard.vue'; +import ActiveSessions from './ActiveSessions.vue'; import Policy from 'dashboard/components/policy.vue'; import RadioCard from 'dashboard/components-next/radioCard/RadioCard.vue'; import { @@ -42,6 +43,7 @@ export default { AudioNotifications, AccessToken, MfaSettingsCard, + ActiveSessions, BaseSettingsHeader, }, setup() { @@ -307,6 +309,13 @@ export default { > + + + e + Rails.logger.warn "UserSessionIpLookupJob failed: #{e.message}" + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 4aa38bbcd..729f674d3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -101,6 +101,7 @@ class User < ApplicationRecord has_many :messages, as: :sender, dependent: :nullify has_many :invitees, through: :account_users, class_name: 'User', foreign_key: 'inviter_id', source: :inviter, dependent: :nullify + has_many :user_sessions, dependent: :destroy has_many :custom_filters, dependent: :destroy_async has_many :dashboard_apps, dependent: :nullify has_many :mentions, dependent: :destroy_async @@ -118,6 +119,7 @@ class User < ApplicationRecord before_validation :set_password_and_uid, on: :create after_destroy :remove_macros + after_save :sync_user_sessions, if: :saved_change_to_tokens? scope :order_by_full_name, -> { order('lower(name) ASC') } @@ -214,6 +216,11 @@ class User < ApplicationRecord private + def sync_user_sessions + active_client_ids = (tokens || {}).keys + user_sessions.where.not(client_id: active_client_ids).destroy_all + end + def remove_macros macros.personal.destroy_all end diff --git a/app/models/user_session.rb b/app/models/user_session.rb new file mode 100644 index 000000000..0f2503382 --- /dev/null +++ b/app/models/user_session.rb @@ -0,0 +1,46 @@ +# == Schema Information +# +# Table name: user_sessions +# +# id :bigint not null, primary key +# browser_name :string +# browser_version :string +# city :string +# country :string +# country_code :string +# device_name :string +# ip_address :string +# last_activity_at :datetime +# platform_name :string +# platform_version :string +# user_agent :string +# created_at :datetime not null +# updated_at :datetime not null +# client_id :string not null +# user_id :bigint not null +# +# Indexes +# +# index_user_sessions_on_user_id (user_id) +# index_user_sessions_on_user_id_and_client_id (user_id,client_id) UNIQUE +# +# Foreign Keys +# +# fk_rails_... (user_id => users.id) +# + +class UserSession < ApplicationRecord + ACTIVITY_THROTTLE = 5.minutes + + belongs_to :user + + validates :client_id, presence: true, uniqueness: { scope: :user_id } + + def current?(active_client_id) + client_id == active_client_id + end + + def should_update_activity? + last_activity_at.nil? || last_activity_at < ACTIVITY_THROTTLE.ago + end +end diff --git a/app/services/user_session_tracking_service.rb b/app/services/user_session_tracking_service.rb new file mode 100644 index 000000000..28f272a18 --- /dev/null +++ b/app/services/user_session_tracking_service.rb @@ -0,0 +1,39 @@ +class UserSessionTrackingService + def initialize(user:, request:, client_id:) + @user = user + @request = request + @client_id = client_id + end + + def create_or_update! + session = @user.user_sessions.find_or_initialize_by(client_id: @client_id) + session.assign_attributes(session_attributes) + session.last_activity_at = Time.current + session.save! + UserSessionIpLookupJob.perform_later(session) if session.ip_address.present? + session + end + + def update_activity! + session = @user.user_sessions.find_by(client_id: @client_id) + return unless session&.should_update_activity? + + session.update_columns(last_activity_at: Time.current) # rubocop:disable Rails/SkipsModelValidations + end + + private + + def session_attributes + browser = Browser.new(@request.user_agent) + + { + ip_address: @request.remote_ip, + user_agent: @request.user_agent, + browser_name: browser.name, + browser_version: browser.full_version, + device_name: browser.device.name, + platform_name: browser.platform.name, + platform_version: browser.platform.version + } + end +end diff --git a/app/views/api/v1/profile/sessions/index.json.jbuilder b/app/views/api/v1/profile/sessions/index.json.jbuilder new file mode 100644 index 000000000..b009271e0 --- /dev/null +++ b/app/views/api/v1/profile/sessions/index.json.jbuilder @@ -0,0 +1,15 @@ +json.array! @sessions do |session| + json.id session.id + json.browser_name session.browser_name + json.browser_version session.browser_version + json.device_name session.device_name + json.platform_name session.platform_name + json.platform_version session.platform_version + json.ip_address session.ip_address + json.city session.city + json.country session.country + json.country_code session.country_code + json.last_activity_at session.last_activity_at + json.created_at session.created_at + json.current session.current?(@current_client_id) +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 8b8202f1c..826894466 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -47,6 +47,10 @@ en: saml_not_available: SAML authentication is not available in this installation. inbox_deletetion_response: Your inbox deletion request will be processed in some time. + profile_settings: + sessions: + cannot_revoke_current: You cannot revoke the current session. + errors: account: reporting_timezone: diff --git a/config/routes.rb b/config/routes.rb index 885b69062..f86e3f2cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -437,6 +437,7 @@ Rails.application.routes.draw do post :verify post :backup_codes end + resources :sessions, only: [:index, :destroy] end end diff --git a/db/migrate/20260611184600_create_user_sessions.rb b/db/migrate/20260611184600_create_user_sessions.rb new file mode 100644 index 000000000..96ad2821e --- /dev/null +++ b/db/migrate/20260611184600_create_user_sessions.rb @@ -0,0 +1,23 @@ +class CreateUserSessions < ActiveRecord::Migration[7.0] + def change + create_table :user_sessions do |t| + t.references :user, null: false, foreign_key: true + t.string :client_id, null: false + t.string :ip_address + t.string :user_agent + t.string :browser_name + t.string :browser_version + t.string :device_name + t.string :platform_name + t.string :platform_version + t.string :city + t.string :country + t.string :country_code + t.datetime :last_activity_at + + t.timestamps + end + + add_index :user_sessions, [:user_id, :client_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index eb6b82508..2060bcfda 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2026_06_04_000000) do +ActiveRecord::Schema[7.1].define(version: 2026_06_11_184600) do # These extensions should be enabled to support this database enable_extension "pg_stat_statements" enable_extension "pg_trgm" @@ -1253,6 +1253,26 @@ ActiveRecord::Schema[7.1].define(version: 2026_06_04_000000) do t.index ["name", "account_id"], name: "index_teams_on_name_and_account_id", unique: true end + create_table "user_sessions", force: :cascade do |t| + t.bigint "user_id", null: false + t.string "client_id", null: false + t.string "ip_address" + t.string "user_agent" + t.string "browser_name" + t.string "browser_version" + t.string "device_name" + t.string "platform_name" + t.string "platform_version" + t.string "city" + t.string "country" + t.string "country_code" + t.datetime "last_activity_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["user_id", "client_id"], name: "index_user_sessions_on_user_id_and_client_id", unique: true + t.index ["user_id"], name: "index_user_sessions_on_user_id" + end + create_table "users", id: :serial, force: :cascade do |t| t.string "provider", default: "email", null: false t.string "uid", default: "", null: false @@ -1325,6 +1345,7 @@ ActiveRecord::Schema[7.1].define(version: 2026_06_04_000000) do add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" add_foreign_key "inboxes", "portals" + add_foreign_key "user_sessions", "users" create_trigger("accounts_after_insert_row_tr", :generated => true, :compatibility => 1). on("accounts"). after(:insert). diff --git a/spec/controllers/devise_overrides/sessions_controller_spec.rb b/spec/controllers/devise_overrides/sessions_controller_spec.rb index 8ee012670..7e12011b2 100644 --- a/spec/controllers/devise_overrides/sessions_controller_spec.rb +++ b/spec/controllers/devise_overrides/sessions_controller_spec.rb @@ -163,4 +163,21 @@ RSpec.describe DeviseOverrides::SessionsController, type: :controller do expect(response).to redirect_to('/frontend/app/login?error=access-denied') end end + + describe 'session tracking' do + let(:user) { create(:user, password: 'Test@123456') } + let(:browser_ua) { 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.2.1 Safari/605.1.15' } + + context 'with a successful login' do + before { request.env['HTTP_USER_AGENT'] = browser_ua } + + it 'creates a UserSession row for the new client_id' do + expect { post :create, params: { email: user.email, password: 'Test@123456' } }.to change(user.user_sessions, :count).by(1) + + session = user.user_sessions.last + expect(session.browser_name).to eq('Safari') + expect(session.platform_name).to eq('macOS') + end + end + end end diff --git a/spec/jobs/user_session_ip_lookup_job_spec.rb b/spec/jobs/user_session_ip_lookup_job_spec.rb new file mode 100644 index 000000000..8d7d08a33 --- /dev/null +++ b/spec/jobs/user_session_ip_lookup_job_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +RSpec.describe UserSessionIpLookupJob do + let(:user) { create(:user) } + let(:session) { user.user_sessions.create!(client_id: 'c', ip_address: '8.8.8.8', last_activity_at: Time.current) } + let(:geo_result) { OpenStruct.new(city: 'Mountain View', country: 'United States', country_code: 'US') } + let(:ip_lookup) { instance_double(IpLookupService) } + + before { allow(IpLookupService).to receive(:new).and_return(ip_lookup) } + + it 'backfills geo data on the session' do + allow(ip_lookup).to receive(:perform).with('8.8.8.8').and_return(geo_result) + + described_class.perform_now(session) + + session.reload + expect(session.city).to eq('Mountain View') + expect(session.country).to eq('United States') + expect(session.country_code).to eq('US') + end + + it 'is a no-op when ip_address is blank' do + session.update_columns(ip_address: nil) # rubocop:disable Rails/SkipsModelValidations + + described_class.perform_now(session) + + expect(IpLookupService).not_to have_received(:new) + end + + it 'leaves the session untouched when lookup returns nil' do + allow(ip_lookup).to receive(:perform).and_return(nil) + + described_class.perform_now(session) + + session.reload + expect(session.city).to be_nil + expect(session.country).to be_nil + end + + it 'swallows lookup errors so a flaky geocoder does not poison the queue' do + allow(ip_lookup).to receive(:perform).and_raise(StandardError.new('boom')) + + expect { described_class.perform_now(session) }.not_to raise_error + end +end diff --git a/spec/models/user_session_spec.rb b/spec/models/user_session_spec.rb new file mode 100644 index 000000000..0c8fa6cf1 --- /dev/null +++ b/spec/models/user_session_spec.rb @@ -0,0 +1,61 @@ +require 'rails_helper' + +RSpec.describe UserSession do + let(:user) { create(:user) } + + describe 'associations' do + it { is_expected.to belong_to(:user) } + end + + describe 'validations' do + subject { described_class.new(user: user, client_id: 'abc') } + + it { is_expected.to validate_presence_of(:client_id) } + + it 'validates uniqueness of client_id scoped to user_id' do + described_class.create!(user: user, client_id: 'abc', last_activity_at: Time.current) + + duplicate = described_class.new(user: user, client_id: 'abc') + expect(duplicate).not_to be_valid + expect(duplicate.errors[:client_id]).to be_present + end + + it 'allows the same client_id for different users' do + other = create(:user) + described_class.create!(user: user, client_id: 'abc', last_activity_at: Time.current) + + expect(described_class.new(user: other, client_id: 'abc', last_activity_at: Time.current)).to be_valid + end + end + + describe '#current?' do + let(:session) { described_class.create!(user: user, client_id: 'abc', last_activity_at: Time.current) } + + it 'returns true when client_id matches' do + expect(session.current?('abc')).to be true + end + + it 'returns false when client_id differs' do + expect(session.current?('xyz')).to be false + end + end + + describe '#should_update_activity?' do + let(:session) { described_class.new(user: user, client_id: 'abc') } + + it 'returns true when last_activity_at is nil' do + session.last_activity_at = nil + expect(session.should_update_activity?).to be true + end + + it 'returns true when last_activity_at is older than the throttle window' do + session.last_activity_at = 10.minutes.ago + expect(session.should_update_activity?).to be true + end + + it 'returns false when last_activity_at is within the throttle window' do + session.last_activity_at = 1.minute.ago + expect(session.should_update_activity?).to be false + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d263708af..fc7a8953b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -254,4 +254,37 @@ RSpec.describe User do end end end + + describe 'sync_user_sessions callback' do + let(:user_with_tokens) do + u = create(:user) + u.tokens = { + 'client-a' => { 'token' => 'x', 'expiry' => 1.month.from_now.to_i }, + 'client-b' => { 'token' => 'x', 'expiry' => 1.month.from_now.to_i } + } + u.save! + u.user_sessions.create!(client_id: 'client-a', last_activity_at: Time.current) + u.user_sessions.create!(client_id: 'client-b', last_activity_at: Time.current) + u + end + + it 'destroys user_sessions whose client_id is no longer in tokens' do + user_with_tokens.tokens = user_with_tokens.tokens.except('client-a') + + expect { user_with_tokens.save! }.to change(user_with_tokens.user_sessions, :count).by(-1) + expect(user_with_tokens.user_sessions.pluck(:client_id)).to eq(['client-b']) + end + + it 'leaves user_sessions alone when tokens did not change' do + user_with_tokens.update!(name: 'New Name') + + expect(user_with_tokens.user_sessions.count).to eq(2) + end + + it 'destroys all user_sessions when tokens is cleared' do + user_with_tokens.tokens = {} + + expect { user_with_tokens.save! }.to change(user_with_tokens.user_sessions, :count).by(-2) + end + end end diff --git a/spec/requests/api/v1/profile/sessions_controller_spec.rb b/spec/requests/api/v1/profile/sessions_controller_spec.rb new file mode 100644 index 000000000..239fa8e83 --- /dev/null +++ b/spec/requests/api/v1/profile/sessions_controller_spec.rb @@ -0,0 +1,101 @@ +require 'rails_helper' + +RSpec.describe 'Profile Sessions API', type: :request do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:auth_headers) { user.create_new_auth_token } + let(:current_client_id) { auth_headers['client'] } + + describe 'GET /api/v1/profile/sessions' do + it 'returns 401 without auth' do + get '/api/v1/profile/sessions', as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'returns the current user sessions ordered by last_activity_at desc' do + older = user.user_sessions.create!(client_id: current_client_id, browser_name: 'Chrome', last_activity_at: 2.days.ago) + newer = user.user_sessions.create!(client_id: 'other-client', browser_name: 'Firefox', last_activity_at: 1.hour.ago) + user.update!(tokens: user.tokens.merge('other-client' => { 'token' => 'x', 'expiry' => 1.month.from_now.to_i })) + + get '/api/v1/profile/sessions', headers: auth_headers, as: :json + + expect(response).to have_http_status(:success) + sessions = response.parsed_body + expect(sessions.map { |s| s['id'] }).to eq([newer.id, older.id]) + expect(sessions.find { |s| s['id'] == older.id }['current']).to be true + expect(sessions.find { |s| s['id'] == newer.id }['current']).to be false + end + + it 'excludes sessions whose token has expired' do + live = user.user_sessions.create!(client_id: current_client_id, last_activity_at: 1.hour.ago) + expired = user.user_sessions.create!(client_id: 'expired-client', last_activity_at: 1.day.ago) + user.update!(tokens: user.tokens.merge('expired-client' => { 'token' => 'x', 'expiry' => 1.day.ago.to_i })) + + get '/api/v1/profile/sessions', headers: auth_headers, as: :json + + expect(response).to have_http_status(:success) + ids = response.parsed_body.map { |s| s['id'] } + expect(ids).to include(live.id) + expect(ids).not_to include(expired.id) + end + + it 'returns an empty array when no sessions exist' do + get '/api/v1/profile/sessions', headers: auth_headers, as: :json + + expect(response).to have_http_status(:success) + expect(response.parsed_body).to eq([]) + end + end + + describe 'DELETE /api/v1/profile/sessions/:id' do + let!(:other_session) { user.user_sessions.create!(client_id: 'other-client', last_activity_at: 1.hour.ago) } + + before do + # Seed tokens hash so revoke can clean it up + user.tokens = user.tokens.merge('other-client' => { 'token' => 'x', 'expiry' => 1.month.from_now.to_i }) + user.save! + end + + it 'destroys the session and removes its token entry' do + expect do + delete "/api/v1/profile/sessions/#{other_session.id}", headers: auth_headers, as: :json + end.to change(user.user_sessions, :count).by(-1) + + expect(response).to have_http_status(:ok) + expect(user.reload.tokens.keys).not_to include('other-client') + end + + it 'returns 422 when trying to revoke the current session' do + current = user.user_sessions.create!(client_id: current_client_id, last_activity_at: Time.current) + + delete "/api/v1/profile/sessions/#{current.id}", headers: auth_headers, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body['error']).to be_present + expect(user.user_sessions.exists?(id: current.id)).to be true + end + + it 'returns 404 for a nonexistent session id' do + delete '/api/v1/profile/sessions/9999999', headers: auth_headers, as: :json + + expect(response).to have_http_status(:not_found) + end + + it 'does not allow revoking another user' do + other_user = create(:user, account: account) + foreign = other_user.user_sessions.create!(client_id: 'foreign', last_activity_at: 1.hour.ago) + + delete "/api/v1/profile/sessions/#{foreign.id}", headers: auth_headers, as: :json + + expect(response).to have_http_status(:not_found) + expect(other_user.user_sessions.exists?(id: foreign.id)).to be true + end + + it 'returns 401 without auth' do + delete "/api/v1/profile/sessions/#{other_session.id}", as: :json + + expect(response).to have_http_status(:unauthorized) + end + end +end diff --git a/spec/services/user_session_tracking_service_spec.rb b/spec/services/user_session_tracking_service_spec.rb new file mode 100644 index 000000000..71b6d5924 --- /dev/null +++ b/spec/services/user_session_tracking_service_spec.rb @@ -0,0 +1,82 @@ +require 'rails_helper' + +RSpec.describe UserSessionTrackingService do + let(:user) { create(:user) } + let(:client_id) { 'client-abc' } + let(:request) do + instance_double( + ActionDispatch::Request, + user_agent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.2.1 Safari/605.1.15', + remote_ip: '8.8.8.8' + ) + end + let(:service) { described_class.new(user: user, request: request, client_id: client_id) } + + describe '#create_or_update!' do + it 'creates a new UserSession with the right client_id and timestamps' do + expect { service.create_or_update! }.to change(user.user_sessions, :count).by(1) + + session = user.user_sessions.last + expect(session.client_id).to eq(client_id) + expect(session.last_activity_at).to be_within(1.second).of(Time.current) + end + + it 'populates request and browser metadata synchronously', :aggregate_failures do + service.create_or_update! + + session = user.user_sessions.last + expect(session.ip_address).to eq('8.8.8.8') + expect(session.browser_name).to eq('Safari') + expect(session.platform_name).to eq('macOS') + end + + it 'does not call IpLookupService synchronously' do + expect(IpLookupService).not_to receive(:new) + + service.create_or_update! + end + + it 'enqueues UserSessionIpLookupJob to backfill geo data' do + expect { service.create_or_update! }.to have_enqueued_job(UserSessionIpLookupJob) + end + + it 'updates an existing session when client_id matches' do + existing = user.user_sessions.create!(client_id: client_id, ip_address: '1.1.1.1', last_activity_at: 1.day.ago) + + expect { service.create_or_update! }.not_to change(user.user_sessions, :count) + expect(existing.reload.ip_address).to eq('8.8.8.8') + expect(existing.last_activity_at).to be_within(1.second).of(Time.current) + end + end + + describe '#update_activity!' do + it 'does nothing when no session exists for the client_id' do + expect { service.update_activity! }.not_to change(user.user_sessions, :count) + end + + it 'does nothing when the session was recently active' do + session = user.user_sessions.create!(client_id: client_id, last_activity_at: 1.minute.ago) + before_ts = session.last_activity_at + + service.update_activity! + + expect(session.reload.last_activity_at).to be_within(1.second).of(before_ts) + end + + it 'bumps last_activity_at when the session is stale' do + session = user.user_sessions.create!(client_id: client_id, last_activity_at: 10.minutes.ago) + + service.update_activity! + + expect(session.reload.last_activity_at).to be_within(1.second).of(Time.current) + end + + it 'bumps last_activity_at when last_activity_at is nil' do + session = user.user_sessions.create!(client_id: client_id, last_activity_at: nil) + + service.update_activity! + + expect(session.reload.last_activity_at).to be_within(1.second).of(Time.current) + end + end +end From b1c2db5435433a03744e1cbea9a993ff3ec6dfcf Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Sat, 13 Jun 2026 19:57:42 +0530 Subject: [PATCH 06/10] fix: Stabilize help center builder error spec (#14725) ## Description Stabilizes the enterprise help center article builder source URL validation spec by asserting the custom exception via its class name and message text. This keeps the spec focused on the intended behavior while avoiding brittle custom exception constant identity checks in CI/reloading environments. A bunch of builds on different PRs have been failing because of this error, sample traces are below: * https://app.circleci.com/pipelines/github/chatwoot/chatwoot/114064/workflows/bdb6eca9-3b65-4c38-b8cf-f2f8564476f8/jobs/158777 * https://app.circleci.com/pipelines/github/chatwoot/chatwoot/114064/workflows/bdb6eca9-3b65-4c38-b8cf-f2f8564476f8/jobs/158777 Fixes # N/A ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - `/Users/sonymathew/.rbenv/shims/bundle exec rspec spec/enterprise/services/onboarding/help_center_article_builder_spec.rb` - `/Users/sonymathew/.rbenv/shims/bundle exec rubocop spec/enterprise/services/onboarding/help_center_article_builder_spec.rb` - `git diff --check` ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] My changes generate no new warnings - [x] New and existing unit tests pass locally with my changes - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] Any dependent changes have been merged and published in downstream modules --- .../services/onboarding/help_center_article_builder_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/enterprise/services/onboarding/help_center_article_builder_spec.rb b/spec/enterprise/services/onboarding/help_center_article_builder_spec.rb index a0aba2f91..c731f9624 100644 --- a/spec/enterprise/services/onboarding/help_center_article_builder_spec.rb +++ b/spec/enterprise/services/onboarding/help_center_article_builder_spec.rb @@ -12,7 +12,10 @@ RSpec.describe Onboarding::HelpCenterArticleBuilder do expect(Firecrawl::Configuration).not_to receive(:client) expect { builder.perform } - .to raise_error(Onboarding::HelpCenterErrors::ArticleBuildFailed, /no source urls/) + .to raise_error(StandardError) { |error| + expect(error.class.name).to eq('Onboarding::HelpCenterErrors::ArticleBuildFailed') + expect(error.message).to include('no source urls') + } end end end From 006b529918bc57d864f8db2e86b0a858273b1166 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 13 Jun 2026 22:49:03 +0530 Subject: [PATCH 07/10] chore(deps): bump net-imap from 0.4.24 to 0.6.4.1 (#14688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [net-imap](https://github.com/ruby/net-imap) from 0.4.24 to 0.6.4.1.
Release notes

Sourced from net-imap's releases.

v0.5.15

What's Changed

🔒 Security

This release fixes several more security vulnerabilities which are related to the fixes in v0.5.14. Please see the linked security advisories for more information.

  • (moderate) Command Injection via non-synchronizing literal in "raw" argument (CVE-2026-47240, GHSA-8p34-64r3-mwg8) This vulnerability depends how the server interprets non-synchronizing literals. The connection is not vulnerable if the server supports non-synchronizing literals.
  • (moderate) Command Injection via unvalidated ID and ENABLE arguments (CVE-2026-47242, GHSA-46q3-7gv7-qmgg)
  • (low) Denial of Service via incomplete "raw" argument validation (CVE-2026-47241, GHSA-c4fp-cxrr-mj66) This results in the affected command hanging until the connection is closed. If another thread attempts to send a concurrent pipelined command, the first thread will return with a syntax error and the second thread will hang until the connection closes.

Fixed

Documentation

Other Changes

Miscellaneous

Full Changelog: https://github.com/ruby/net-imap/compare/v0.5.14...v0.5.15

v0.5.14

What's Changed

🔒 Security

This release contains fixes for multiple vulnerabilities concerning STARTTLS stripping, argument validation, and denial of service attacks.

[!WARNING] ruby/net-imap#665 fixes a STARTTLS stripping vulnerability (GHSA-vcgp-9326-pqcp). Without this fix, a man-in-the-middle attacker can cause Net::IMAP#starttls to return "successfully", without starting TLS.

[!IMPORTANT] Argument validation is significantly improved. Several command injection vulnerabilities have been fixed: ruby/net-imap#662 fixes CRLF/command/argument injection via Symbol arguments (GHSA-75xq-5h9v-w6px). ruby/net-imap#662 fixes CRLF/command/argument injection via the attr argument to #store/#uid_store (GHSA-hm49-wcqc-g2xg)

... (truncated)

Commits
  • ce20fc8 🔖 Bump version to 0.5.15
  • 0b7b83c 🔀 Merge pull request #703 from ruby/backport/v0.5/security-patches
  • f22fd6c 🍒 pick 0ea9eba3 (#701): ✅ Fix flaky tests for MacOS, TruffleRuby
  • 1246074 🍒 pick ae9f83b5 (#701): ♻️ Extract str.bytesize lvar in send_literal
  • a2f61af 🍒 pick 62a0da6d (#701): 🥅 Validate non-synchronizing literals support
  • e33348c 🍒 pick d6ddd294 (#700): 🐛 Prevent trailing {0} in RawData validation
  • 4f81b69 🍒 pick 1f97168b (#699): 🥅 Validate #enable arguments are all atoms
  • 69da4a4 🍒 pick 8d9397ab (#698): 🥅 Validate QuotedString contains only valid bytes
  • 7aab580 🍒 pick e3c50fad (#698): ♻️ Refactor RawText, add improve test coverage
  • fac1733 🍒 pick aab64f92 (#686): 🧵 Fix deadlock in #disconnect
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=net-imap&package-manager=bundler&previous-version=0.4.24&new-version=0.5.15)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/chatwoot/chatwoot/network/alerts).
--------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sony Mathew Co-authored-by: Sony Mathew <2040199+sony-mathew@users.noreply.github.com> --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 141afc122..8d6132849 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -582,7 +582,7 @@ GEM uri (>= 0.11.1) net-http-persistent (4.0.2) connection_pool (~> 2.2) - net-imap (0.4.24) + net-imap (0.6.4.1) date net-protocol net-pop (0.1.2) From 27404b4a27f26ff745b21de6e529e3ac9050bfab Mon Sep 17 00:00:00 2001 From: Botshxlo <46230844+Botshxlo@users.noreply.github.com> Date: Sun, 14 Jun 2026 03:23:13 +0200 Subject: [PATCH 08/10] fix: redact sensitive integration secrets from API responses (#14147) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary The `GET /api/v1/accounts/:id/integrations/apps` endpoint returns raw secret values (OpenAI API keys, Google service account private keys, Linear refresh tokens, etc.) in hook settings within the JSON response. Although gated behind an administrator check, these secrets are visible in the browser network tab. This PR filters hook settings through the existing `visible_properties` whitelist defined in `config/integration/apps.yml`, and adds explicit whitelists to integrations that were missing them. Closes #14042 ## Bug reproduction **Setup:** Created an OpenAI integration hook with a fake API key (`sk-test-secret-12345`). **Step 1 — Browser Network tab shows raw secrets:** Screenshot 2026-04-24 at 14 32 28 Navigate to Settings > Integrations as an admin. Open DevTools Network tab and observe the response from `GET /api/v1/accounts/:id/integrations/apps`. The hook settings contain the full API key in plaintext: ```json "hooks": [ { "id": 1, "app_id": "openai", "settings": { "api_key": "sk-test-secret-12345", "label_suggestion": false } } ] ``` **Step 2 — API call confirms the leak:** ```bash curl -s "http://localhost:3000/api/v1/accounts/2/integrations/apps" \ -H "access-token: " \ -H "client: " \ -H "uid: user@test.com" \ | jq '.payload[] | select(.id == "openai") | {id, name, hooks: [.hooks[] | {id, app_id, settings}]}' ``` Response: ```json { "id": "openai", "name": "OpenAI", "hooks": [ { "id": 1, "app_id": "openai", "settings": { "api_key": "sk-test-secret-12345", "label_suggestion": false } } ] } ``` Any admin user can extract the raw API key from the response. The same applies to other integrations — Dialogflow exposes full Google service account credentials, Linear exposes refresh tokens, etc. ## After fix After applying this change, the GET /api/v1/accounts/:id/integrations/apps endpoint no longer returns sensitive secret values in hook settings. Instead, the response is filtered using each integration’s visible_properties whitelist, ensuring only safe, user-facing fields are exposed. For example, OpenAI integrations return non-sensitive fields like label_suggestion while excluding raw API keys. This prevents secrets from being exposed in the browser network tab or API responses, even for authenticated admin users. ```bash curl -X GET "http://localhost:3000/api/v1/accounts/2/integrations/apps" \ -H "Accept: application/json" \ -H "Authorization: Bearer eyJhY2Nlc3MtdG9rZW4iOiJqZG5jOWg4TnljaWFJa3JlZkxGQzRnIiwidG9rZW4tdHlwZSI6IkJlYXJlciIsImNsaWVudCI6Ik81bjVnTDFEOVVhbGpwbWxjaHZNanciLCJleHBpcnkiOiIxNzgyMzMyNTA4IiwidWlkIjoidXNlckB0ZXN0LmNvbSJ9" \ -H "access-token: jdnc9h8NyciaIkrefLFC4g" \ -H "client: O5n5gL1D9UaljpmlchvMjw" \ -H "uid: user@test.com" \ | jq '.payload[] | select(.id == "openai") | {id, name, hooks: [.hooks[] | {id, app_id, settings}]}' % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 10174 100 10174 0 0 33232 0 --:--:-- --:--:-- --:--:-- 33357 { "id": "openai", "name": "OpenAI", "hooks": [ { "id": 1, "app_id": "openai", "settings": { "label_suggestion": false } } ] } ``` ## What changed - Added `visible_properties` accessor to `Integrations::App` model to expose the whitelist from config - Updated `_hook.json.jbuilder` to filter `resource.settings` through the associated app's `visible_properties` instead of returning the full hash - Added `visible_properties` to integrations that were missing it: - Linear: `[]` (settings contain refresh_token) - Notion: `[]` (OAuth-based, no user-facing settings) - Slack: `['channel_name']` (UI needs this to display connected channel) - Shopify: `[]` (no settings) App config metadata (`_app.json.jbuilder`) is left unchanged — hook_type, settings_form_schema, etc. are not secrets and the frontend depends on them. ## How to test 1. Create an OpenAI integration hook with an API key 2. As an admin, call `GET /api/v1/accounts/:id/integrations/apps` 3. Verify hook settings include `api_key` (whitelisted) but not raw credential objects 4. For Dialogflow hooks, verify `credentials` (private key JSON) is excluded while `project_id` is included 5. For Slack hooks, verify `channel_name` is still returned 6. For Linear hooks, verify `refresh_token` is not returned --------- Co-authored-by: Botshelo Nokoane (Konstruktors) Co-authored-by: Sony Mathew Co-authored-by: Sony Mathew <2040199+sony-mathew@users.noreply.github.com> --- app/models/integrations/app.rb | 4 + app/views/api/v1/models/_hook.json.jbuilder | 9 +- config/integration/apps.yml | 8 +- .../integrations/apps_controller_spec.rb | 101 +++++++++++++++--- spec/models/integrations/app_spec.rb | 37 +++++-- 5 files changed, 134 insertions(+), 25 deletions(-) diff --git a/app/models/integrations/app.rb b/app/models/integrations/app.rb index 5e4d28c06..b5b0123a2 100644 --- a/app/models/integrations/app.rb +++ b/app/models/integrations/app.rb @@ -30,6 +30,10 @@ class Integrations::App params[:fields] end + def visible_properties + Array(params[:visible_properties]).map(&:to_s) + end + # There is no way to get the account_id from the linear callback # so we are using the generate_linear_token method to generate a token and encode it in the state parameter def encode_state diff --git a/app/views/api/v1/models/_hook.json.jbuilder b/app/views/api/v1/models/_hook.json.jbuilder index 5df214ac8..3b9b14029 100644 --- a/app/views/api/v1/models/_hook.json.jbuilder +++ b/app/views/api/v1/models/_hook.json.jbuilder @@ -5,5 +5,10 @@ json.inbox resource.inbox&.slice(:id, :name) json.account_id resource.account_id json.hook_type resource.hook_type -json.settings resource.settings if Current.account_user&.administrator? -json.reference_id resource.reference_id if Current.account_user&.administrator? +if Current.account_user&.administrator? + visible_properties = resource.app&.visible_properties || [] + settings = (resource.settings || {}).select { |key, _| visible_properties.include?(key.to_s) } + + json.settings settings + json.reference_id resource.reference_id +end diff --git a/config/integration/apps.yml b/config/integration/apps.yml index 1a45cc098..9ef01ed30 100644 --- a/config/integration/apps.yml +++ b/config/integration/apps.yml @@ -6,6 +6,7 @@ # hook_type: ( account / inbox ) # feature_flag: (string) feature flag to enable/disable the integration # allow_multiple_hooks: whether multiple hooks can be created for the integration +# visible_properties: hook setting keys safe to return in API responses and show in the UI # settings_json_schema: the json schema used to validate the settings hash (https://json-schema.org/) # settings_form_schema: the formulate schema used in frontend to render settings form (https://vueformulate.com/) ######################################################## @@ -55,7 +56,7 @@ openai: 'validation': '', }, ] - visible_properties: ['api_key', 'label_suggestion'] + visible_properties: ['label_suggestion'] linear: id: linear logo: linear.png @@ -63,12 +64,14 @@ linear: action: https://linear.app/oauth/authorize hook_type: account allow_multiple_hooks: false + visible_properties: [] notion: id: notion logo: notion.png i18n_key: notion hook_type: account allow_multiple_hooks: false + visible_properties: [] slack: id: slack logo: slack.png @@ -76,6 +79,7 @@ slack: action: https://slack.com/oauth/v2/authorize?scope=commands,chat:write,channels:read,channels:manage,channels:join,groups:read,groups:write,im:write,mpim:write,users:read,users:read.email,chat:write.customize,channels:history,groups:history,mpim:history,im:history,files:read,files:write hook_type: account allow_multiple_hooks: false + visible_properties: ['channel_name'] dialogflow: id: dialogflow logo: dialogflow.png @@ -240,6 +244,7 @@ shopify: i18n_key: shopify hook_type: account allow_multiple_hooks: false + visible_properties: [] leadsquared: id: leadsquared @@ -310,7 +315,6 @@ leadsquared: ] visible_properties: [ - 'access_key', 'endpoint_url', 'enable_conversation_activity', 'enable_transcript_activity', diff --git a/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb index bf95c9826..db8cd12f1 100644 --- a/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/apps_controller_spec.rb @@ -42,7 +42,7 @@ RSpec.describe 'Integration Apps API', type: :request do expect(app['hooks'].first['settings']).to be_nil end - it 'returns all active apps with sensitive information if user is an admin' do + it 'returns all active apps with admin metadata if user is an admin' do first_app = Integrations::App.all.find { |app| app.active?(account) } get api_v1_account_integrations_apps_url(account), headers: admin.create_new_auth_token, @@ -56,19 +56,21 @@ RSpec.describe 'Integration Apps API', type: :request do end it 'returns slack app with appropriate redirect url when configured' do - with_modified_env SLACK_CLIENT_ID: 'client_id', SLACK_CLIENT_SECRET: 'client_secret' do - get api_v1_account_integrations_apps_url(account), - headers: admin.create_new_auth_token, - as: :json + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('SLACK_CLIENT_ID', nil).and_return('client_id') + allow(GlobalConfigService).to receive(:load).with('SLACK_CLIENT_SECRET', nil).and_return('client_secret') - expect(response).to have_http_status(:success) - apps = response.parsed_body['payload'] - slack_app = apps.find { |app| app['id'] == 'slack' } - expect(slack_app['action']).to include('client_id=client_id') - end + get api_v1_account_integrations_apps_url(account), + headers: admin.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + apps = response.parsed_body['payload'] + slack_app = apps.find { |app| app['id'] == 'slack' } + expect(slack_app['action']).to include('client_id=client_id') end - it 'will return sensitive information for openai app for admins' do + it 'returns visible hook settings for openai app for admins' do openai = create(:integrations_hook, :openai, account: account) get api_v1_account_integrations_apps_url(account), headers: admin.create_new_auth_token, @@ -79,6 +81,34 @@ RSpec.describe 'Integration Apps API', type: :request do app = response.parsed_body['payload'].find { |int_app| int_app['id'] == openai.app.id } expect(app['hooks'].first['settings']).not_to be_nil end + + it 'redacts secrets and only returns visible settings for openai hooks' do + openai = create( + :integrations_hook, + :openai, + account: account, + settings: { api_key: 'sk-secret', label_suggestion: true } + ) + get api_v1_account_integrations_apps_url(account), + headers: admin.create_new_auth_token, + as: :json + + app = response.parsed_body['payload'].find { |int_app| int_app['id'] == openai.app.id } + expect(app['hooks'].first['settings']).to eq('label_suggestion' => true) + end + + it 'keeps slack channel display settings while redacting unspecified settings' do + create(:integrations_hook, account: account, settings: { channel_name: 'support', signing_secret: 'secret' }) + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('SLACK_CLIENT_SECRET', nil).and_return('client_secret') + + get api_v1_account_integrations_apps_url(account), + headers: admin.create_new_auth_token, + as: :json + + app = response.parsed_body['payload'].find { |int_app| int_app['id'] == 'slack' } + expect(app['hooks'].first['settings']).to eq('channel_name' => 'support') + end end end @@ -117,7 +147,7 @@ RSpec.describe 'Integration Apps API', type: :request do expect(app['hooks'].first['settings']).to be_nil end - it 'will return sensitive information for openai app for admins' do + it 'returns visible hook settings for openai app for admins' do openai = create(:integrations_hook, :openai, account: account) get api_v1_account_integrations_app_url(account_id: account.id, id: openai.app.id), headers: admin.create_new_auth_token, @@ -128,6 +158,53 @@ RSpec.describe 'Integration Apps API', type: :request do app = response.parsed_body expect(app['hooks'].first['settings']).not_to be_nil end + + it 'hides credentials and keeps visible settings for google credential integrations' do + hook = create(:integrations_hook, :google_translate, account: account, + settings: { project_id: 'project-1', + credentials: { private_key: 'secret' } }) + get api_v1_account_integrations_app_url(account_id: account.id, id: hook.app.id), + headers: admin.create_new_auth_token, + as: :json + + app = response.parsed_body + expect(app['hooks'].first['settings']).to eq('project_id' => 'project-1') + end + + it 'returns empty settings for oauth integrations with no visible properties' do + hook = create( + :integrations_hook, + :linear, + account: account, + settings: { token_type: 'Bearer', refresh_token: 'refresh-secret', expires_in: 7200 } + ) + get api_v1_account_integrations_app_url(account_id: account.id, id: hook.app.id), + headers: admin.create_new_auth_token, + as: :json + + app = response.parsed_body + expect(app['hooks'].first['settings']).to eq({}) + end + + it 'does not expose leadsquared credential keys in visible settings' do + account.enable_features('crm_integration') + hook = create(:integrations_hook, :leadsquared, account: account, + settings: { + 'access_key' => 'access-secret', + 'secret_key' => 'secret', + 'endpoint_url' => 'https://api.leadsquared.com/', + 'enable_conversation_activity' => true + }) + get api_v1_account_integrations_app_url(account_id: account.id, id: hook.app.id), + headers: admin.create_new_auth_token, + as: :json + + settings = response.parsed_body['hooks'].first['settings'] + expect(settings).to eq( + 'endpoint_url' => 'https://api.leadsquared.com/', + 'enable_conversation_activity' => true + ) + end end end end diff --git a/spec/models/integrations/app_spec.rb b/spec/models/integrations/app_spec.rb index f9e7c7532..46ae56632 100644 --- a/spec/models/integrations/app_spec.rb +++ b/spec/models/integrations/app_spec.rb @@ -23,6 +23,24 @@ RSpec.describe Integrations::App do end end + describe '#visible_properties' do + context 'when the app has visible properties' do + let(:app_name) { 'dialogflow' } + + it 'returns the configured property names as strings' do + expect(app.visible_properties).to contain_exactly('project_id', 'region', 'language_code') + end + end + + context 'when the app has no visible properties configured' do + let(:app_name) { 'webhook' } + + it 'defaults to an empty list' do + expect(app.visible_properties).to eq([]) + end + end + end + describe '#action' do let(:app_name) { 'slack' } @@ -32,12 +50,13 @@ RSpec.describe Integrations::App do context 'when the app is slack' do it 'returns the action URL with client_id and redirect_uri' do - with_modified_env SLACK_CLIENT_ID: 'dummy_client_id' do - expect(app.action).to include('client_id=dummy_client_id') - expect(app.action).to include( - "/app/accounts/#{account.id}/settings/integrations/slack" - ) - end + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('SLACK_CLIENT_ID', nil).and_return('dummy_client_id') + + expect(app.action).to include('client_id=dummy_client_id') + expect(app.action).to include( + "/app/accounts/#{account.id}/settings/integrations/slack" + ) end end end @@ -47,9 +66,9 @@ RSpec.describe Integrations::App do context 'when the app is slack' do it 'returns true if SLACK_CLIENT_SECRET is present' do - with_modified_env SLACK_CLIENT_SECRET: 'random_secret' do - expect(app.active?(account)).to be true - end + allow(GlobalConfigService).to receive(:load).with('SLACK_CLIENT_SECRET', nil).and_return('random_secret') + + expect(app.active?(account)).to be true end end From 274e92e0e421fe4afd193ba1e1d7b68e6faa0c5f Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Sun, 14 Jun 2026 06:54:18 +0530 Subject: [PATCH 09/10] chore: collapse conversation sidebar sections (folders, teams, inboxes and labels) - CW-7059 (#14509) ## Description Added ability to collapse conversation sidebar sections (folders, teams, inboxes and labels) Fixes #CW-7059 ## Type of change Please delete options that are not relevant. - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? Tested locally. Added specs. Attaching the loom for them same. https://github.com/user-attachments/assets/40d613e7-6c82-4078-abf4-79739a00f718 ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Co-authored-by: iamsivin --- .../components-next/sidebar/Sidebar.vue | 16 ++ .../components-next/sidebar/SidebarGroup.vue | 95 +++------ .../sidebar/SidebarGroupLeaf.vue | 17 +- .../sidebar/SidebarGroupSeparator.vue | 61 +++++- .../sidebar/SidebarSubGroup.vue | 198 ++++++++++++------ .../sidebar/specs/SidebarSubGroup.spec.js | 172 +++++++++++++++ .../dashboard/constants/localStorage.js | 1 + theme/icons.js | 5 + 8 files changed, 423 insertions(+), 142 deletions(-) create mode 100644 app/javascript/dashboard/components-next/sidebar/specs/SidebarSubGroup.spec.js diff --git a/app/javascript/dashboard/components-next/sidebar/Sidebar.vue b/app/javascript/dashboard/components-next/sidebar/Sidebar.vue index f71652ca0..ab037618c 100644 --- a/app/javascript/dashboard/components-next/sidebar/Sidebar.vue +++ b/app/javascript/dashboard/components-next/sidebar/Sidebar.vue @@ -300,6 +300,7 @@ const menuItems = computed(() => { { name: 'All', label: t('SIDEBAR.ALL_CONVERSATIONS'), + icon: 'i-lucide-inbox', badgeCount: allUnreadCount.value, activeOn: ['inbox_conversation'], to: accountScopedRoute('home'), @@ -307,12 +308,14 @@ const menuItems = computed(() => { { name: 'Mentions', label: t('SIDEBAR.MENTIONED_CONVERSATIONS'), + icon: 'i-lucide-at-sign', activeOn: ['conversation_through_mentions'], to: accountScopedRoute('conversation_mentions'), }, { name: 'Participating', label: t('SIDEBAR.PARTICIPATING_CONVERSATIONS'), + icon: 'i-lucide-user-round-check', activeOn: ['conversation_through_participating'], to: accountScopedRoute('conversation_participating'), }, @@ -320,6 +323,7 @@ const menuItems = computed(() => { name: 'Unattended', activeOn: ['conversation_through_unattended'], label: t('SIDEBAR.UNATTENDED_CONVERSATIONS'), + icon: 'i-lucide-clock-alert', to: accountScopedRoute('conversation_unattended'), }, { @@ -327,6 +331,8 @@ const menuItems = computed(() => { label: t('SIDEBAR.CUSTOM_VIEWS_FOLDER'), icon: 'i-lucide-folder', activeOn: ['conversations_through_folders'], + collapsible: true, + showTreeLine: true, children: conversationCustomViews.value.map(view => ({ name: `${view.name}-${view.id}`, label: view.name, @@ -338,6 +344,8 @@ const menuItems = computed(() => { label: t('SIDEBAR.TEAMS'), icon: 'i-lucide-users', activeOn: ['conversations_through_team'], + collapsible: true, + showTreeLine: true, children: sortedTeams.value.map(team => ({ name: `${team.name}-${team.id}`, label: team.name, @@ -350,6 +358,8 @@ const menuItems = computed(() => { label: t('SIDEBAR.CHANNELS'), icon: 'i-lucide-mailbox', activeOn: ['conversation_through_inbox'], + collapsible: true, + showTreeLine: true, children: sortedInboxes.value.map(inbox => ({ name: `${inbox.name}-${inbox.id}`, label: inbox.name, @@ -370,6 +380,8 @@ const menuItems = computed(() => { label: t('SIDEBAR.LABELS'), icon: 'i-lucide-tag', activeOn: ['conversations_through_label'], + collapsible: true, + showTreeLine: true, children: sortedLabels.value.map(label => ({ name: `${label.title}-${label.id}`, label: label.title, @@ -481,6 +493,8 @@ const menuItems = computed(() => { name: 'Segments', icon: 'i-lucide-group', label: t('SIDEBAR.CUSTOM_VIEWS_SEGMENTS'), + collapsible: true, + showTreeLine: true, children: contactCustomViews.value.map(view => ({ name: `${view.name}-${view.id}`, label: view.name, @@ -499,6 +513,8 @@ const menuItems = computed(() => { name: 'Tagged With', icon: 'i-lucide-tag', label: t('SIDEBAR.TAGGED_WITH'), + collapsible: true, + showTreeLine: true, children: labels.value.map(label => ({ name: `${label.title}-${label.id}`, label: label.title, diff --git a/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue b/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue index 048a99cf8..f618ad9ba 100644 --- a/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue +++ b/app/javascript/dashboard/components-next/sidebar/SidebarGroup.vue @@ -99,20 +99,39 @@ const handleWindowBlur = () => { closeActivePopover(); }; -const accessibleItems = computed(() => { +const hasAccessibleSubChildren = child => { + return child.children?.some( + subChild => subChild.to && isAllowed(subChild.to) + ); +}; + +const visibleChildren = computed(() => { if (!hasChildren.value) return []; + return props.children.filter(child => { - // If a item has no link, it means it's just a subgroup header - // So we don't need to check for permissions here, because there's nothing to - // access here anyway + if (child.children) return hasAccessibleSubChildren(child); + return child.to && isAllowed(child.to); }); }); -const hasAccessibleChildren = computed(() => { - return accessibleItems.value.length > 0; +const accessibleItems = computed(() => { + if (!hasChildren.value) return []; + + return visibleChildren.value + .flatMap(child => child.children || child) + .filter(child => child.to && isAllowed(child.to)); }); +const hasAccessibleChildren = computed(() => { + return visibleChildren.value.length > 0; +}); + +const isLastVisibleChild = child => { + const lastChild = visibleChildren.value[visibleChildren.value.length - 1]; + return lastChild === child; +}; + const isActive = computed(() => { if (props.to) { if (route.path === resolvePath(props.to)) return true; @@ -274,14 +293,18 @@ watch(