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