diff --git a/app/controllers/api/v1/accounts/integrations/shopify_controller.rb b/app/controllers/api/v1/accounts/integrations/shopify_controller.rb index 915f90471..8d21b5f5e 100644 --- a/app/controllers/api/v1/accounts/integrations/shopify_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/shopify_controller.rb @@ -106,13 +106,17 @@ class Api::V1::Accounts::Integrations::ShopifyController < Api::V1::Accounts::Ba end def claim_pending_install_token(token_key, account_id) - pending_data = ::Redis::Alfred.get(token_key) - return { error: 'Invalid or expired install token' } if pending_data.blank? + json_data = Redis::SecureStorage.get(token_key) + return { error: 'Invalid or expired install token' } if json_data.blank? - data = JSON.parse(pending_data) + begin + data = JSON.parse(json_data) + rescue JSON::ParserError + return { error: 'Invalid or corrupted install token' } + end if data['claimed'] - ::Redis::Alfred.delete(token_key) + Redis::SecureStorage.delete(token_key) return { error: 'Install token already used' } end @@ -123,7 +127,7 @@ class Api::V1::Accounts::Integrations::ShopifyController < Api::V1::Accounts::Ba data['claimed'] = true data['account_id'] = account_id ttl = ::Redis::Alfred.ttl(token_key) - ::Redis::Alfred.setex(token_key, data.to_json, [ttl, 60].max) if ttl.positive? + Redis::SecureStorage.set(token_key, data, [ttl, 60].max) if ttl.positive? data end diff --git a/app/controllers/shopify/callbacks_controller.rb b/app/controllers/shopify/callbacks_controller.rb index f4dc8db2f..63b32e71a 100644 --- a/app/controllers/shopify/callbacks_controller.rb +++ b/app/controllers/shopify/callbacks_controller.rb @@ -21,6 +21,7 @@ class Shopify::CallbacksController < ApplicationController def handle_chatwoot_initiated_flow @account_id = verify_shopify_token(params[:state]) raise StandardError, 'Invalid state parameter' if account.blank? + raise StandardError, 'Invalid HMAC signature' unless valid_hmac? @response = oauth_client.auth_code.get_token(params[:code], redirect_uri: redirect_callback_uri) create_hook @@ -29,7 +30,11 @@ class Shopify::CallbacksController < ApplicationController def handle_shopify_initiated_flow raise StandardError, 'Invalid shop domain' unless valid_shop_domain? + raise StandardError, 'Invalid HMAC signature' unless valid_hmac? + # Security: HMAC validation ensures params (including shop) haven't been tampered with. + # Additionally, the OAuth code is cryptographically bound to the shop that issued it. + # Shopify will reject any attempt to exchange a code at a different shop's endpoint. @response = oauth_client.auth_code.get_token(params[:code], redirect_uri: redirect_callback_uri) token_key = SecureRandom.hex(16) @@ -38,8 +43,9 @@ class Shopify::CallbacksController < ApplicationController shop: params[:shop], scope: parsed_body['scope'], claimed: false - }.to_json - ::Redis::Alfred.setex("shopify_pending_install:#{token_key}", pending_data, 10.minutes) + } + + Redis::SecureStorage.set("shopify_pending_install:#{token_key}", pending_data, 10.minutes) redirect_url = "settings/integrations/shopify?shopify_pending_install=#{CGI.escape(token_key)}" redirect_to "#{frontend_url}/app/login?redirect_url=#{CGI.escape(redirect_url)}", allow_other_host: true @@ -105,4 +111,15 @@ class Shopify::CallbacksController < ApplicationController # Shopify shop domains must match: *.myshopify.com or *.myshopify.io (for dev shops) params[:shop].match?(/\A[a-zA-Z0-9][a-zA-Z0-9\-]*\.myshopify\.(com|io)\z/) end + + def valid_hmac? + return false if params[:hmac].blank? + + # Shopify signs callback parameters with HMAC to prevent tampering + hmac = params[:hmac] + query_params = params.except(:hmac, :controller, :action).to_query + computed_hmac = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('SHA256'), client_secret, query_params) + + ActiveSupport::SecurityUtils.secure_compare(computed_hmac, hmac) + end end diff --git a/app/views/api/v1/models/_hook.json.jbuilder b/app/views/api/v1/models/_hook.json.jbuilder index 7beb86f6c..5df214ac8 100644 --- a/app/views/api/v1/models/_hook.json.jbuilder +++ b/app/views/api/v1/models/_hook.json.jbuilder @@ -4,7 +4,6 @@ json.status resource.enabled? json.inbox resource.inbox&.slice(:id, :name) json.account_id resource.account_id json.hook_type resource.hook_type -json.created_at resource.created_at json.settings resource.settings if Current.account_user&.administrator? json.reference_id resource.reference_id if Current.account_user&.administrator? diff --git a/lib/redis/secure_storage.rb b/lib/redis/secure_storage.rb new file mode 100644 index 000000000..ebd9c3445 --- /dev/null +++ b/lib/redis/secure_storage.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +# Redis::SecureStorage provides encrypted storage for sensitive temporary data in Redis. +# Uses ActiveSupport::MessageEncryptor with AES-256-GCM for authenticated encryption. +# +# Example: +# Redis::SecureStorage.set('session:token', { access_token: 'secret' }, 10.minutes) +# data = Redis::SecureStorage.get('session:token') +# +module Redis::SecureStorage + class << self + # Store data in Redis with encryption + # @param key [String] Redis key + # @param data [Hash, String] Data to store (will be converted to JSON) + # @param expiry [Integer, ActiveSupport::Duration] TTL in seconds + def set(key, data, expiry) + encrypted = encrypt(data) + Alfred.setex(key, encrypted, expiry) + end + + # Retrieve and decrypt data from Redis + # @param key [String] Redis key + # @return [Hash, nil] Decrypted data or nil if not found/invalid + def get(key) + encrypted = Alfred.get(key) + return nil if encrypted.blank? + + decrypt(encrypted) + rescue ActiveSupport::MessageEncryptor::InvalidMessage, JSON::ParserError + nil + end + + # Delete data from Redis + # @param key [String] Redis key + def delete(key) + Alfred.delete(key) + end + + private + + def encryptor + @encryptor ||= ActiveSupport::MessageEncryptor.new( + Rails.application.credentials.secret_key_base[0..31], + cipher: 'aes-256-gcm' + ) + end + + def encrypt(data) + json_data = data.is_a?(String) ? data : data.to_json + return json_data unless Chatwoot.encryption_configured? + + encryptor.encrypt_and_sign(json_data) + end + + def decrypt(encrypted_data) + return encrypted_data unless Chatwoot.encryption_configured? + + encryptor.decrypt_and_verify(encrypted_data) + end + end +end