THREESCALE-10153: Do not sync apps with Zync when not using OpenID Co…

…nnect (#3579) * PublishZyncEventSubscriber is not subscribed to CinstanceCancellationEvent * Skip Zync events for applications on non-OIDC services * Rename zync event prop indicating OIDC * Add a test * Refactor to reduce cognitive complexity * Update app/subscribers/publish_zync_event_subscriber.rb Co-authored-by: Aleksandar N. Kostadinov <[email protected]> * Some fixes after review * Put the feature under a setting * Fix 🤦 * Some fixes for tests * Pass service with ApplicationRelatedEvents instead of extra metadata * Fixes * Remove outdated log --------- Co-authored-by: Aleksandar N. Kostadinov <[email protected]>
3scale · Nov 14, 2023 · e59383f · e59383f
1 parent bce1ab8
commit e59383f
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 30 deletions.
diff --git a/app/events/applications/application_created_event.rb b/app/events/applications/application_created_event.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class Applications::ApplicationCreatedEvent < ApplicationRelatedEvent
   # @param [Cinstance] application
   # @param [User] user
@@ -8,13 +10,11 @@ def self.create(application, user)
       application: application,
       account:     account = application.user_account,
       provider:    provider,
-      # this really can't be application.service as it would break:
-      # $ rspec ./spec/acceptance/api/application_spec.rb -e 'Cinstance application json format GET /admin/api/applications/find.:format with app id Get Application'
-      service:     application.plan.issuer,
+      service:     application.service,
       plan:        application.plan,
-      user:        user || account.try!(:first_admin),
+      user:        user || account&.first_admin,
       metadata: {
-        provider_id: provider.try!(:id),
+        provider_id: provider&.id,
         zync: {
           service_id: application.service_id
         }

diff --git a/app/events/applications/application_enabled_changed_event.rb b/app/events/applications/application_enabled_changed_event.rb
@@ -10,9 +10,9 @@ def create(application)
         application: application,
         metadata: {
           provider_id: provider.id,
-            zync: {
-              service_id: application.service_id
-            }
+          zync: {
+            service_id: application.service_id
+          }
         }
       )
     end

diff --git a/app/subscribers/publish_zync_event_subscriber.rb b/app/subscribers/publish_zync_event_subscriber.rb
@@ -10,25 +10,49 @@ def initialize(publisher = DEFAULT_PUBLISHER)
 
   attr_reader :publisher
 
-  # @param [ZyncEvent] event
-  def call(event)
-    unless ThreeScale.config.onpremises
+  delegate :domain_event_in_saas?, :build_zync_event, to: :class
+
+  class << self
+    def domain_event_in_saas?(event)
+      ThreeScale.saas? && (event.is_a?(Domains::ProxyDomainsChangedEvent) || event.is_a?(Domains::ProviderDomainsChangedEvent))
+    end
+
+    def build_zync_event(event)
       case event
-      when Domains::ProxyDomainsChangedEvent, Domains::ProviderDomainsChangedEvent
-        return
+      when ApplicationRelatedEvent
+        service_oidc?(event) || sync_non_oidc_apps ? ZyncEvent.create(event, event.application) : nil
+      when OIDC::ProxyChangedEvent, Domains::ProxyDomainsChangedEvent
+        ZyncEvent.create(event, event.proxy)
+      when OIDC::ServiceChangedEvent
+        ZyncEvent.create(event, event.service)
+      when Domains::ProviderDomainsChangedEvent
+        ZyncEvent.create(event, event.provider)
+      else raise "Unknown event type #{event.class}"
       end
     end
 
-    zync = case event
-           # When the app is deleted the event caries all the needed information
-           when Cinstances::CinstanceCancellationEvent then ZyncEvent.create(event)
-           when ApplicationRelatedEvent then ZyncEvent.create(event, event.application)
-           when OIDC::ProxyChangedEvent, Domains::ProxyDomainsChangedEvent then ZyncEvent.create(event, event.proxy)
-           when OIDC::ServiceChangedEvent then ZyncEvent.create(event, event.service)
-           when Domains::ProviderDomainsChangedEvent then ZyncEvent.create(event, event.provider)
-           else raise "Unknown event type #{event.class}"
-           end
-
-    publisher.call(zync, 'zync')
+    private
+
+    # The default is 'true', if the configuration is missing
+    def sync_non_oidc_apps
+      !Rails.configuration.zync.skip_non_oidc_applications
+    end
+
+    # Returns 'true' if the service with id exists, and has OAuth authentication, and 'false' otherwise
+    def service_oidc?(event)
+      zync_metadata = event.metadata.fetch(:zync, {})
+      service = Service.find_by(id: zync_metadata[:service_id])
+      !!service&.oauth?
+    end
   end
+
+  # @param [ZyncEvent] event
+  def call(event)
+    # skip domain-related events in SaaS
+    return if domain_event_in_saas? event
+
+    zync_event = build_zync_event event
+    publisher.call(zync_event, 'zync') if zync_event
+  end
+
 end
diff --git a/config/examples/zync.yml b/config/examples/zync.yml
@@ -5,6 +5,7 @@ base: &default
   connect_timeout: 2
   send_timeout: 2
   receive_timeout: 10
+  skip_non_oidc_applications: false
 
 development:
   <<: *default

diff --git a/test/subscribers/publish_zync_event_subscriber_test.rb b/test/subscribers/publish_zync_event_subscriber_test.rb
@@ -1,14 +1,59 @@
+# frozen_string_literal: true
+
 require 'test_helper'
 
 class PublishZyncEventSubscriberTest < ActiveSupport::TestCase
-  def setup
-    @subscriber = PublishZyncEventSubscriber.new
+
+  class OIDCApplicationEventTest < ActiveSupport::TestCase
+    attr_reader :event, :subscriber, :publisher
+
+    setup do
+      service = FactoryBot.create(:simple_service, backend_version: 'oauth')
+      application = FactoryBot.create(:simple_cinstance, service: service)
+      @event = Applications::ApplicationCreatedEvent.create(application, nil)
+      @publisher = mock('publisher')
+      @subscriber = PublishZyncEventSubscriber.new(publisher)
+    end
+
+    test 'publish Zync Event for OIDC auth always' do
+      publisher.expects(:call).times(3).returns(:ok)
+
+      Rails.configuration.zync.stubs(skip_non_oidc_applications: false)
+      assert subscriber.call(event)
+
+      Rails.configuration.zync.stubs(skip_non_oidc_applications: true)
+      assert subscriber.call(event)
+
+      Rails.configuration.zync.stubs(skip_non_oidc_applications: nil)
+      assert subscriber.call(event)
+    end
   end
 
-  def test_create
-    application = FactoryBot.build_stubbed(:simple_cinstance, tenant_id: 1)
-    event = Applications::ApplicationCreatedEvent.new(application: application)
-    assert @subscriber.call(event)
+  class NonOIDCApplicationEventTest < ActiveSupport::TestCase
+    attr_reader :event, :subscriber, :publisher
+
+    setup do
+      service = FactoryBot.create(:simple_service)
+      application = FactoryBot.create(:simple_cinstance, service: service)
+      @event = Applications::ApplicationCreatedEvent.create(application, nil)
+      @publisher = mock('publisher')
+      @subscriber = PublishZyncEventSubscriber.new(publisher)
+    end
+
+    test 'publish Zync Event by if not skipped' do
+      publisher.expects(:call).times(2).returns(:ok)
+      Rails.configuration.zync.stubs(skip_non_oidc_applications: false)
+      assert subscriber.call(event)
+
+      Rails.configuration.zync.stubs(skip_non_oidc_applications: nil)
+      assert subscriber.call(event)
+    end
+
+    test 'do not publish Zync Event if skipped' do
+      publisher.expects(:call).never
+      Rails.configuration.zync.stubs(skip_non_oidc_applications: true)
+      assert_nil subscriber.call(event)
+    end
   end
 
   class DomainEventsTest < ActiveSupport::TestCase