From 6a6cc6e2f8bca9c0092f6c674d438aab858e28af Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 2 Oct 2024 15:40:27 +1000 Subject: [PATCH] ConnectedApps controller, handle ConnectedApps::Vine Add logiv to connect and disconnect VINE API plus spec --- .env | 4 + .../admin/connected_apps_controller.rb | 58 +++++- .../initializers/filter_parameter_logging.rb | 4 +- config/locales/en.yml | 5 +- .../admin/connected_apps_controller_spec.rb | 174 ++++++++++++++++++ 5 files changed, 236 insertions(+), 9 deletions(-) create mode 100644 spec/requests/admin/connected_apps_controller_spec.rb diff --git a/.env b/.env index fe9b06d4ff1..b5bfa57c44c 100644 --- a/.env +++ b/.env @@ -61,3 +61,7 @@ SMTP_PASSWORD="f00d" # NEW_RELIC_AGENT_ENABLED=true # NEW_RELIC_APP_NAME="Open Food Network" # NEW_RELIC_LICENSE_KEY="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + +# VINE API settings +# VINE_API_URL="https://vine-staging.openfoodnetwork.org.au/api/v1" +# VINE_API_SECRET="XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" diff --git a/app/controllers/admin/connected_apps_controller.rb b/app/controllers/admin/connected_apps_controller.rb index 3531c7371ab..6cf7ff34ede 100644 --- a/app/controllers/admin/connected_apps_controller.rb +++ b/app/controllers/admin/connected_apps_controller.rb @@ -5,12 +5,7 @@ class ConnectedAppsController < ApplicationController def create authorize! :admin, enterprise - attributes = {} - attributes[:type] = connected_app_params[:type] if connected_app_params[:type] - - app = ConnectedApp.create!(enterprise_id: enterprise.id, **attributes) - app.connect(api_key: spree_current_user.spree_api_key, - channel: SessionChannel.for_request(request)) + connect render_panel end @@ -26,6 +21,45 @@ def destroy private + def create_connected_app + attributes = {} + attributes[:type] = connected_app_params[:type] if connected_app_params[:type] + + @app = ConnectedApp.create!(enterprise_id: enterprise.id, **attributes) + end + + def connect + return connect_vine if connected_app_params[:type] == "ConnectedApps::Vine" + + create_connected_app + @app.connect(api_key: spree_current_user.spree_api_key, + channel: SessionChannel.for_request(request)) + end + + def connect_vine + if connected_app_params[:vine_api_key].empty? + return flash[:error] = I18n.t("admin.enterprises.form.connected_apps.vine.api_key_empty") + end + + create_connected_app + + jwt_service = VineJwtService.new(secret: ENV.fetch("VINE_API_SECRET")) + vine_api = VineApiService.new(api_key: connected_app_params[:vine_api_key], + jwt_generator: jwt_service) + + if !@app.connect(api_key: connected_app_params[:vine_api_key], vine_api:) + error_message = "#{@app.errors.full_messages.to_sentence}. \ + #{I18n.t('admin.enterprises.form.connected_apps.vine.api_key_error')}".squish + handle_error(error_message) + end + rescue Faraday::Error => e + log_and_notify_exception(e) + handle_error(I18n.t("admin.enterprises.form.connected_apps.vine.connection_error")) + rescue KeyError => e + log_and_notify_exception(e) + handle_error(I18n.t("admin.enterprises.form.connected_apps.vine.setup_error")) + end + def enterprise @enterprise ||= Enterprise.find(params.require(:enterprise_id)) end @@ -34,8 +68,18 @@ def render_panel redirect_to "#{edit_admin_enterprise_path(enterprise)}#/connected_apps_panel" end + def handle_error(message) + flash[:error] = message + @app.destroy + end + + def log_and_notify_exception(exception) + Rails.logger.error exception.inspect + Bugsnag.notify(exception) + end + def connected_app_params - params.permit(:type) + params.permit(:type, :vine_api_key) end end end diff --git a/config/initializers/filter_parameter_logging.rb b/config/initializers/filter_parameter_logging.rb index e203fcee0a9..42c801cbbd9 100644 --- a/config/initializers/filter_parameter_logging.rb +++ b/config/initializers/filter_parameter_logging.rb @@ -1,2 +1,4 @@ +# frozen_string_literal: true + # Configure sensitive parameters which will be filtered from the log file. -Rails.application.config.filter_parameters += [:password] +Rails.application.config.filter_parameters += [:password, :vine_api_key] diff --git a/config/locales/en.yml b/config/locales/en.yml index d0136b68adc..fc1579deb83 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1437,7 +1437,10 @@ en: VINE

- + api_key_empty: "Please enter an API key" + api_key_error: "Check you entered your API key correctly, contact your instance manager if the error persists" + connection_error: "API connection error, please try again" + setup_error: "VINE API is not configured, please contact your instance manager" actions: edit_profile: Settings properties: Properties diff --git a/spec/requests/admin/connected_apps_controller_spec.rb b/spec/requests/admin/connected_apps_controller_spec.rb new file mode 100644 index 00000000000..9b4caedca1c --- /dev/null +++ b/spec/requests/admin/connected_apps_controller_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Admin::ConnectedAppsController do + let(:user) { create(:admin_user) } + let(:enterprise) { create(:enterprise, owner: user) } + let(:edit_enterprise_url) { "#{edit_admin_enterprise_url(enterprise)}#/connected_apps_panel" } + + before do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("VINE_API_SECRET").and_return("my_secret") + + sign_in user + end + + describe "POST /admin/enterprises/:enterprise_id/connected_apps" do + context "with type ConnectedApps::Vine" do + let(:vine_api) { instance_double(VineApiService) } + + before do + allow(VineJwtService).to receive(:new).and_return(instance_double(VineJwtService)) + allow(VineApiService).to receive(:new).and_return(vine_api) + end + + it "creates a new connected app" do + allow(vine_api).to receive(:my_team).and_return(mock_api_response(true)) + + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + + expect(ConnectedApps::Vine.find_by(enterprise_id: enterprise.id)).not_to be_nil + end + + it "redirects to enterprise edit page" do + allow(vine_api).to receive(:my_team).and_return(mock_api_response(true)) + + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + + expect(response).to redirect_to(edit_enterprise_url) + end + + context "when api key is empty" do + it "redirects to enterprise edit page, with an error" do + params = { + type: ConnectedApps::Vine, + vine_api_key: "" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + + expect(response).to redirect_to(edit_enterprise_url) + expect(flash[:error]).to eq("Please enter an API key") + expect(ConnectedApps::Vine.find_by(enterprise_id: enterprise.id)).to be_nil + end + end + + context "when api key is not valid" do + before do + allow(vine_api).to receive(:my_team).and_return(mock_api_response(false)) + end + + it "doesn't create a new connected app" do + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + + expect(ConnectedApps::Vine.find_by(enterprise_id: enterprise.id)).to be_nil + end + + it "redirects to enterprise edit page, with an error" do + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + + expect(response).to redirect_to(edit_enterprise_url) + expect(flash[:error]).to eq( + "An error occured when connecting to Vine API. Check you entered your API key \ + correctly, contact your instance manager if the error persists".squish + ) + end + end + + context "when there is a connection error" do + before do + allow(vine_api).to receive(:my_team).and_raise(Faraday::Error) + end + + it "redirects to enterprise edit page, with an error" do + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + + expect(response).to redirect_to(edit_enterprise_url) + expect(flash[:error]).to eq("API connection error, please try again") + end + + it "notifies Bugsnag" do + expect(Bugsnag).to receive(:notify) + + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + end + end + + context "when VINE API is not set up properly" do + before do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("VINE_API_SECRET").and_raise(KeyError) + end + + it "redirects to enterprise edit page, with an error" do + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + + expect(response).to redirect_to(edit_enterprise_url) + expect(flash[:error]).to eq( + "VINE API is not configured, please contact your instance manager" + ) + end + + it "notifies Bugsnag" do + expect(Bugsnag).to receive(:notify) + + params = { + type: ConnectedApps::Vine, + vine_api_key: "12345678" + } + post("/admin/enterprises/#{enterprise.id}/connected_apps", params: ) + end + end + end + + describe "DELETE /admin/enterprises/:enterprise_id/connected_apps/:id" do + it "deletes the connected app" do + app = ConnectedApps::Vine.create!(enterprise:) + delete("/admin/enterprises/#{enterprise.id}/connected_apps/#{app.id}") + + expect(ConnectedApps::Vine.find_by(enterprise_id: enterprise.id)).to be_nil + end + + it "redirect to enterprise edit page" do + app = ConnectedApps::Vine.create!(enterprise:, data: { api_key: "12345" }) + delete("/admin/enterprises/#{enterprise.id}/connected_apps/#{app.id}") + + expect(response).to redirect_to(edit_enterprise_url) + end + end + end + + def mock_api_response(success) + mock_response = instance_double(Faraday::Response) + allow(mock_response).to receive(:success?).and_return(success) + mock_response + end +end