chore: fix security issues
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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?
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user