diff --git a/.github/actions/delete-dev-release/action.yml b/.github/actions/delete-dev-release/action.yml index b48c1fbae..d79d29f57 100644 --- a/.github/actions/delete-dev-release/action.yml +++ b/.github/actions/delete-dev-release/action.yml @@ -82,7 +82,6 @@ runs: if [[ ! -z "$found" ]] then - kubectl exec deploy/$release_name -it -- bundle exec rails app_store:unsubscribe helm delete $release_name kubectl delete pvc data-$release_name-postgresql-0 kubectl delete pvc redis-data-$release_name-redis-master-0 diff --git a/app/controllers/sync_controller.rb b/app/controllers/sync_controller.rb deleted file mode 100644 index b367a0561..000000000 --- a/app/controllers/sync_controller.rb +++ /dev/null @@ -1,18 +0,0 @@ -class SyncController < ApplicationController - skip_before_action :authenticate_user! - before_action :authenticate_app_store!, only: :sync_individual - skip_before_action :verify_authenticity_token, only: :sync_individual - before_action :skip_authorization - - def sync_all - head :ok - end - - def sync_individual - head :ok - end - - def authenticate_app_store! - head(:unauthorized) unless AppStoreTokenAuthenticator.call(request.headers) - end -end diff --git a/app/services/app_store_client.rb b/app/services/app_store_client.rb index a687f367d..0446157b3 100644 --- a/app/services/app_store_client.rb +++ b/app/services/app_store_client.rb @@ -2,17 +2,6 @@ class AppStoreClient include HTTParty headers 'Content-Type' => 'application/json' - def get_all_submissions(last_update) - url = "/v1/applications?since=#{last_update.to_i}" - response = self.class.get "#{host}#{url}", **options - - process_response( - response, - "Unexpected response from AppStore - status #{response.code} for '#{url}'", - 200 => ->(body) { JSON.parse(body) }, - ) - end - def get_submission(submission_id) url = "/v1/submissions/#{submission_id}" response = self.class.get "#{host}#{url}", **options @@ -49,17 +38,6 @@ def update_submission_metadata(submission, payload) ) end - def trigger_subscription(payload, action: :create) - method = action == :create ? :post : :delete - response = self.class.send(method, "#{host}/v1/subscriber", **options(payload)) - - process_response( - response, - "Unexpected response from AppStore - status #{response.code} on #{action} subscription", - 200..204 => :success, - ) - end - def create_events(submission_id, payload) response = self.class.post("#{host}/v1/submissions/#{submission_id}/events", **options(payload)) diff --git a/app/services/app_store_subscriber.rb b/app/services/app_store_subscriber.rb deleted file mode 100644 index 83a6299b4..000000000 --- a/app/services/app_store_subscriber.rb +++ /dev/null @@ -1,45 +0,0 @@ -class AppStoreSubscriber - include Rails.application.routes.url_helpers - - MAX_ATTEMPTS = 3 - SECONDS_BETWEEN_RETRIES = 3 - - def self.subscribe - attempts = 0 - begin - new.change_subscription(:create) - rescue StandardError => e - attempts += 1 - if attempts < 3 - # Sometimes a freshly minted pod fails to connect to Microsoft to generate a token, - # but the issue should resolve itself automatically. Therefore we retry after a short - # sleep - sleep SECONDS_BETWEEN_RETRIES - retry - else - Sentry.capture_exception(e) - end - end - end - - def self.unsubscribe - new.change_subscription(:destroy) - rescue StandardError => e - Sentry.capture_exception(e) - end - - def change_subscription(action) - hostname = ENV.fetch('INTERNAL_HOST_NAME', ENV.fetch('HOSTS', nil)&.split(',')&.first) - return if hostname.blank? - - url = app_store_webhook_url( - host: hostname, - protocol: 'http' - ) - - AppStoreClient.new.trigger_subscription( - { webhook_url: url, subscriber_type: :caseworker }, - action:, - ) - end -end diff --git a/app/services/app_store_token_authenticator.rb b/app/services/app_store_token_authenticator.rb deleted file mode 100644 index 46d77dd6e..000000000 --- a/app/services/app_store_token_authenticator.rb +++ /dev/null @@ -1,46 +0,0 @@ -class AppStoreTokenAuthenticator - class << self - def call(headers) - return true unless AppStoreTokenProvider.instance.authentication_configured? - - jwt = headers['Authorization'].gsub(/^Bearer /, '') - data = parse(jwt) - valid?(data) - rescue StandardError - false - end - - private - - def parse(jwt) - JWT.decode(jwt, nil, true, algorithms: 'RS256', jwks: jwks) - end - - def valid?(data) - data[0]['aud'] == app_store_client_id && - data[0]['iss'] == "https://login.microsoftonline.com/#{tenant_id}/v2.0" && - Time.zone.at(data[0]['exp']) > Time.zone.now - end - - def jwks - @jwks ||= JWT::JWK::Set.new(keys) - end - - def keys - HTTParty.get(jwks_uri)['keys'] - end - - def jwks_uri - config_url = "https://login.microsoftonline.com/#{tenant_id}/.well-known/openid-configuration" - HTTParty.get(config_url)['jwks_uri'] - end - - def tenant_id - ENV.fetch('APP_STORE_TENANT_ID', 'UNDEFINED_APP_STORE_TENANT_ID') - end - - def app_store_client_id - ENV.fetch('APP_STORE_CLIENT_ID', 'UNDEFINED_APP_STORE_CLIENT_ID') - end - end -end diff --git a/app/views/prior_authority/applications/show.html.erb b/app/views/prior_authority/applications/show.html.erb index 7f4bcf5c6..9fdd2a039 100644 --- a/app/views/prior_authority/applications/show.html.erb +++ b/app/views/prior_authority/applications/show.html.erb @@ -9,7 +9,7 @@

<%= t('.overview') %>

- <% if @details.assessment_comment.present? %> + <% if @summary.assessed? && @details.assessment_comment.present? %>

<%= t(".assessed_statuses.#{@details.submission.state}") %>

<%= simple_format @details.assessment_comment %>

diff --git a/config/application.rb b/config/application.rb index 4a2b71d02..c2fab2023 100644 --- a/config/application.rb +++ b/config/application.rb @@ -79,9 +79,5 @@ class Application < Rails::Application end config.x.contact.feedback_url = 'tbc' - config.after_initialize do - Rails.application.reload_routes! - AppStoreSubscriber.subscribe - end end end diff --git a/config/routes.rb b/config/routes.rb index b2e3fb144..534407b29 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -132,12 +132,7 @@ resource :search, only: %i[new show] end - if ENV.fetch("ENABLE_SYNC_TRIGGER_ENDPOINT", false) == "true" - get "sync", to: "sync#sync_all" - end get "robots.txt", to: "robots#index" - post :app_store_webhook, to: 'sync#sync_individual' - resource :dashboard, only: %i[new show] end diff --git a/lib/tasks/app_store.rake b/lib/tasks/app_store.rake deleted file mode 100644 index 46e5adc1b..000000000 --- a/lib/tasks/app_store.rake +++ /dev/null @@ -1,7 +0,0 @@ -namespace :app_store do - desc 'Unsubscribe this branch deployment from app store notifications' - - task unsubscribe: :environment do - AppStoreSubscriber.unsubscribe - end -end diff --git a/spec/lib/tasks/app_store_unsubscribe_spec.rb b/spec/lib/tasks/app_store_unsubscribe_spec.rb deleted file mode 100644 index 6fc430876..000000000 --- a/spec/lib/tasks/app_store_unsubscribe_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'rails_helper' - -describe 'app_store:', type: :task do - describe 'unsubscribe' do - subject { Rake::Task['app_store:unsubscribe'] } - - before do - Rails.application.load_tasks if Rake::Task.tasks.empty? - allow(AppStoreSubscriber).to receive(:unsubscribe) - end - - after do - Rake::Task['app_store:unsubscribe'] - end - - it 'calls the service' do - subject.invoke - expect(AppStoreSubscriber).to have_received(:unsubscribe) - end - end -end diff --git a/spec/requests/sync_spec.rb b/spec/requests/sync_spec.rb deleted file mode 100644 index 4dfe0e773..000000000 --- a/spec/requests/sync_spec.rb +++ /dev/null @@ -1,76 +0,0 @@ -require 'rails_helper' - -RSpec.describe 'Syncs' do - describe 'GET /sync' do - it 'triggers a sync job' do - get '/sync' - - expect(response).to have_http_status(:ok) - end - end - - describe 'POST /app_store_webhook' do - let(:record) { { 'foo' => 'bar' } } - - context 'when no auth token is provided' do - it 'rejects all requests' do - post '/app_store_webhook' - expect(response).to have_http_status :unauthorized - end - - context 'when authentication can be bypassed' do - let(:token_provider) { instance_double(AppStoreTokenProvider) } - - before do - allow(AppStoreTokenProvider).to receive(:instance).and_return(token_provider) - allow(token_provider).to receive(:authentication_configured?).and_return false - end - - it 'triggers a sync' do - post '/app_store_webhook', params: { submission_id: '123', data: record }, headers: { 'Authorization' => 'Bearer ABC' } - expect(response).to have_http_status(:ok) - end - end - end - - context 'when an auth token is provided' do - before do - AppStoreTokenAuthenticator.instance_variable_set(:@jwks, nil) - stub_request(:get, 'https://login.microsoftonline.com/123/.well-known/openid-configuration') - .to_return(status: 200, - body: { jwks_uri: 'https://example.com/jwks' }.to_json, - headers: { 'Content-type' => 'application/json' }) - stub_request(:get, 'https://example.com/jwks') - .to_return(status: 200, - body: { keys: 'keys' }.to_json, - headers: { 'Content-type' => 'application/json' }) - end - - context 'when the token is invalid' do - it 'rejects the request' do - post '/app_store_webhook', headers: { 'Authorization' => 'Bearer ABC' } - expect(response).to have_http_status(:unauthorized) - end - end - - context 'when the token is valid' do - let(:jwks) { instance_double(JWT::JWK::Set) } - let(:decoded) do - [{ 'aud' => 'UNDEFINED_APP_STORE_CLIENT_ID', - 'iss' => 'https://login.microsoftonline.com/123/v2.0', - 'exp' => 1.hour.from_now.to_i }] - end - - before do - allow(JWT::JWK::Set).to receive(:new).with('keys').and_return(jwks) - allow(JWT).to receive(:decode).with('ABC', nil, true, { algorithms: 'RS256', jwks: jwks }).and_return(decoded) - end - - it 'triggers a sync' do - post '/app_store_webhook', params: { submission_id: '123', data: record }, headers: { 'Authorization' => 'Bearer ABC' } - expect(response).to have_http_status(:ok) - end - end - end - end -end diff --git a/spec/services/app_store_client_spec.rb b/spec/services/app_store_client_spec.rb index 5a5db2fed..bea8744dd 100644 --- a/spec/services/app_store_client_spec.rb +++ b/spec/services/app_store_client_spec.rb @@ -13,47 +13,6 @@ .and_return(response) end - describe '#get_all_submissions' do - context 'when APP_STORE_URL is present' do - before do - allow(ENV).to receive(:fetch).with('APP_STORE_URL', 'http://localhost:8000') - .and_return('http://some.url') - end - - it 'get the claims to the specified URL' do - expect(described_class).to receive(:get).with('http://some.url/v1/applications?since=1', - headers: { authorization: 'Bearer test-bearer-token' }) - - subject.get_all_submissions(1) - end - end - - context 'when APP_STORE_URL is not present' do - it 'get the claims to default localhost url' do - expect(described_class).to receive(:get).with('https://appstore.example.com/v1/applications?since=1', - headers: { authorization: 'Bearer test-bearer-token' }) - - subject.get_all_submissions(1) - end - end - - context 'when response code is 200 - ok' do - it 'returns the parsed json' do - expect(subject.get_all_submissions(1)).to eq('some' => 'data') - end - end - - context 'when response code is unexpected (neither 201 or 209)' do - let(:code) { 501 } - - it 'raises and error' do - expect { subject.get_all_submissions(1) }.to raise_error( - "Unexpected response from AppStore - status 501 for '/v1/applications?since=1'" - ) - end - end - end - describe '#update_submission' do let(:application_id) { SecureRandom.uuid } let(:message) { { application_id: } } @@ -138,50 +97,6 @@ end end - describe '#trigger_subscription' do - let(:application_id) { SecureRandom.uuid } - let(:message) { { application_id: } } - let(:response) { double(:response, code:) } - let(:code) { 201 } - let(:username) { nil } - - before do - allow(described_class).to receive_messages(post: response, delete: response) - end - - it 'posts the message to host url' do - expect(described_class).to receive(:post).with('https://appstore.example.com/v1/subscriber', - body: message.to_json, - headers: { authorization: 'Bearer test-bearer-token' }) - - subject.trigger_subscription(message) - end - - it 'uses delete if a destroy action is specified' do - expect(described_class).to receive(:delete).with('https://appstore.example.com/v1/subscriber', - body: message.to_json, - headers: { authorization: 'Bearer test-bearer-token' }) - - subject.trigger_subscription(message, action: :destroy) - end - - context 'when response code is 201 - created' do - it 'returns a created status' do - expect(subject.trigger_subscription(message)).to eq(:success) - end - end - - context 'when response code is unexpected (neither 200 or 201)' do - let(:code) { 501 } - - it 'raises an error' do - expect { subject.trigger_subscription(message) }.to raise_error( - 'Unexpected response from AppStore - status 501 on create subscription' - ) - end - end - end - describe '#create_events' do let(:application_id) { SecureRandom.uuid } let(:message) { [{ event: :data }] } diff --git a/spec/services/app_store_subscriber_spec.rb b/spec/services/app_store_subscriber_spec.rb deleted file mode 100644 index 6d27ae7dd..000000000 --- a/spec/services/app_store_subscriber_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -require 'rails_helper' - -RSpec.describe AppStoreSubscriber do - describe '.subscribe' do - let(:client) { instance_double(AppStoreClient) } - - before do - allow(AppStoreClient).to receive(:new).and_return(client) - allow(client).to receive(:trigger_subscription) - end - - context 'when there is no defined host' do - it 'does not make a request' do - expect(AppStoreClient).not_to receive(:new) - described_class.subscribe - end - end - - context 'when there is a host' do - around do |example| - ENV['HOSTS'] = 'example.com' - example.run - ENV['HOSTS'] = nil - end - - it 'makes a request' do - described_class.subscribe - expect(client).to have_received(:trigger_subscription).with( - { webhook_url: 'http://example.com/app_store_webhook', subscriber_type: :caseworker }, - action: :create - ) - end - - context 'when the app store request errors out' do - before do - allow(client).to receive(:trigger_subscription).and_raise(StandardError) - allow(Sentry).to receive(:capture_exception) - allow(described_class).to receive(:sleep) - end - - it 'retries 3 times then passes the error to Sentry' do - expect { described_class.subscribe }.not_to raise_error - expect(client).to have_received(:trigger_subscription).exactly(3).times - expect(described_class).to have_received(:sleep).with(3).exactly(2).times - expect(Sentry).to have_received(:capture_exception).exactly(1).times - end - end - end - - context 'when there are multiple hosts' do - around do |example| - ENV['HOSTS'] = 'other.com,example.com' - example.run - ENV['HOSTS'] = nil - end - - it 'picks the first one' do - described_class.subscribe - expect(client).to have_received(:trigger_subscription).with( - { webhook_url: 'http://other.com/app_store_webhook', subscriber_type: :caseworker }, - action: :create - ) - end - end - - context 'when there is an internal host' do - around do |example| - ENV['INTERNAL_HOST_NAME'] = 'internal.svc.local' - example.run - ENV['INTERNAL_HOST_NAME'] = nil - end - - it 'uses that' do - described_class.subscribe - expect(client).to have_received(:trigger_subscription).with( - { webhook_url: 'http://internal.svc.local/app_store_webhook', subscriber_type: :caseworker }, - action: :create - ) - end - end - end - - describe '.unsubscribe' do - let(:client) { instance_double(AppStoreClient) } - - before do - allow(AppStoreClient).to receive(:new).and_return(client) - allow(client).to receive(:trigger_subscription) - end - - around do |example| - ENV['HOSTS'] = 'example.com' - example.run - ENV['HOSTS'] = nil - end - - it 'uses the right action' do - described_class.unsubscribe - expect(client).to have_received(:trigger_subscription).with( - { webhook_url: 'http://example.com/app_store_webhook', subscriber_type: :caseworker }, - action: :destroy - ) - end - - context 'when the app store request errors out' do - before do - allow(client).to receive(:trigger_subscription).and_raise(StandardError) - allow(Sentry).to receive(:capture_exception) - end - - it 'passes the error to Sentry' do - expect { described_class.unsubscribe }.not_to raise_error - expect(Sentry).to have_received(:capture_exception) - end - end - end -end