Merge pull request #1235 from SUSE/sccproxy-activations-error-handling

Move error handling to the proper method

engines/registry/app/models/access_scope.rb

@@ -94,8 +94,8 @@ def allowed_paths(system = nil)
         }
         allowed_non_free_product_classes.each do |non_free_prod_class|
           activation_state = SccProxy.scc_check_subscription_expiration(
-            auth_header, system.login, system.system_token, Rails.logger, system.proxy_byos_mode, non_free_prod_class
-            )
+            auth_header, system, non_free_prod_class
+          )
           unless activation_state[:is_active]
             Rails.logger.info(
               "Access to #{non_free_prod_class} from system #{system.login} denied: #{activation_state[:message]}"

engines/registry/spec/app/models/access_scope_spec.rb

@@ -215,22 +215,15 @@
             allow(SccProxy).to receive(:scc_check_subscription_expiration)
               .with(
                 header_expected,
-                system.login,
-                system.system_token,
-                Rails.logger,
-                system.proxy_byos_mode,
+                system,
                 'SLES15-SP4-LTSS-X86'
             ).and_return(scc_response)
           end
 
-          # rubocop:disable RSpec/ExampleLength
           it 'returns no actions allowed' do
             expect(SccProxy).to receive(:scc_check_subscription_expiration).with(
               header_expected,
-              system.login,
-              system.system_token,
-              Rails.logger,
-              system.proxy_byos_mode,
+              system,
               'SLES15-SP4-LTSS-X86'
             )
             yaml_string = access_policy_content
@@ -250,7 +243,6 @@
               }
             )
           end
-          # rubocop:enable RSpec/ExampleLength
         end
       end
 

engines/scc_proxy/lib/scc_proxy/engine.rb

@@ -201,14 +201,19 @@ def parse_error(error_message, token = nil, email = nil)
       error_message
     end
 
-    def get_scc_activations(headers, system_token, mode)
+    def get_scc_activations(headers, system)
       auth = headers['HTTP_AUTHORIZATION'] if headers && headers.include?('HTTP_AUTHORIZATION')
       uri = URI.parse(SYSTEMS_ACTIVATIONS_URL)
       http = Net::HTTP.new(uri.host, uri.port)
       http.use_ssl = true
-      uri.query = URI.encode_www_form({ byos_mode: mode })
-      scc_request = Net::HTTP::Get.new(uri.path, headers(auth, system_token))
-      http.request(scc_request)
+      uri.query = URI.encode_www_form({ byos_mode: system.proxy_byos_mode })
+      scc_request = Net::HTTP::Get.new(uri.path, headers(auth, system.system_token))
+      response = http.request(scc_request)
+      unless response.code_type == Net::HTTPOK
+        Rails.logger.info "Could not get the system (#{system.login}) activations, error: #{response.message} #{response.code}"
+        raise ActionController::TranslatedError.new(response.body)
+      end
+      JSON.parse(response.body)
     end
 
     def product_path_access(x_original_uri, products_ids)
@@ -273,14 +278,8 @@ def activations_fail_state(scc_systems_activations, headers, product = nil)
     end
     # rubocop:enable Metrics/PerceivedComplexity
 
-    def scc_check_subscription_expiration(headers, login, system_token, logger, mode, product = nil) # rubocop:disable Metrics/ParameterLists
-      response = SccProxy.get_scc_activations(headers, system_token, mode)
-      unless response.code_type == Net::HTTPOK
-        logger.info "Could not get the system (#{login}) activations, error: #{response.message} #{response.code}"
-        response.message = SccProxy.parse_error(response.message) if response.message.include? 'json'
-        return { is_active: false, message: response.message }
-      end
-      scc_systems_activations = JSON.parse(response.body)
+    def scc_check_subscription_expiration(headers, system, product = nil)
+      scc_systems_activations = SccProxy.get_scc_activations(headers, system)
       return { is_active: false, message: 'No activations.' } if scc_systems_activations.empty?
 
       no_status_products_ids = scc_systems_activations.map do |act|
@@ -289,6 +288,8 @@ def scc_check_subscription_expiration(headers, login, system_token, logger, mode
       return { is_active: true } unless no_status_products_ids.all?(&:nil?)
 
       SccProxy.activations_fail_state(scc_systems_activations, headers, product)
+    rescue StandardError
+      { is_active: false, message: 'Could not check the activations from SCC' }
     end
 
     def scc_upgrade(auth, product, system_login, mode, logger)
@@ -306,7 +307,6 @@ def scc_upgrade(auth, product, system_login, mode, logger)
     end
   end
 
-  # rubocop:disable Metrics/ClassLength
   class Engine < ::Rails::Engine
     isolate_namespace SccProxy
     config.generators.api_only = true
@@ -447,26 +447,17 @@ def scc_deactivate_product
           elsif @system.hybrid? && @product.extension?
             # check if product is on SCC and
             # if it is -> de-activate it
-            scc_systems_activations = find_hybrid_activations_on_scc(request.headers)
-            if scc_systems_activations.map { |act| act['service']['product']['id'] == @product.id }.present?
+            scc_hybrid_system_activations = SccProxy.get_scc_activations(headers, @system)
+            if scc_hybrid_system_activations.map { |act| act['service']['product']['id'] == @product.id }.present?
               # if product is found on SCC, regardless of the state
               # it is OK to remove it from SCC
               SccProxy.deactivate_product_scc(auth, @product, @system.system_token, logger)
-              make_system_payg(auth) if scc_systems_activations.reject { |act| act['service']['product']['id'] == @product.id }.blank?
+              make_system_payg(auth) if scc_hybrid_system_activations.reject { |act| act['service']['product']['id'] == @product.id }.blank?
             end
           end
           logger.info "Product '#{@product.friendly_name}' successfully deactivated from SCC"
         end
 
-        def find_hybrid_activations_on_scc(headers)
-          response = SccProxy.get_scc_activations(headers, @system.system_token, @system.proxy_byos_mode)
-          unless response.code_type == Net::HTTPOK
-            logger.info "Could not get the system (#{@system.login}) activations, error: #{response.message} #{response.code}"
-            raise ActionController::TranslatedError.new(response.body)
-          end
-          JSON.parse(response.body)
-        end
-
         def make_system_payg(auth)
           # if the system does not have more products activated on SCC
           # switch it back to payg
@@ -549,6 +540,5 @@ def get_system(systems)
       end
     end
   end
-  # rubocop:enable Metrics/ClassLength
 end
 # rubocop:enable Metrics/ModuleLength

engines/zypper_auth/lib/zypper_auth/engine.rb

@@ -31,12 +31,13 @@ def verify_instance(request, logger, system, params_product_id = nil)
       )
 
       is_valid = verification_provider.instance_valid?
-      return false if is_valid && system.hybrid? && !handle_scc_subscription(request, system, verification_provider, logger, params_product_id)
+      return false if is_valid && system.hybrid? && !handle_scc_subscription(request, system, verification_provider, params_product_id)
+
       # update repository and registry cache
       InstanceVerification.update_cache(request.remote_ip, system.login, base_product.id)
       is_valid
     rescue InstanceVerification::Exception => e
-      return handle_scc_subscription(request, system, verification_provider, logger) if system.byos?
+      return handle_scc_subscription(request, system, verification_provider) if system.byos?
 
       ZypperAuth.zypper_auth_message(request, system, verification_provider, e.message)
       false
@@ -49,9 +50,9 @@ def verify_instance(request, logger, system, params_product_id = nil)
       false
     end
 
-    def handle_scc_subscription(request, system, verification_provider, logger, params_product_id = nil)
+    def handle_scc_subscription(request, system, verification_provider, params_product_id = nil)
       product_class = Product.find_by(id: params_product_id).product_class if params_product_id.present?
-      result = SccProxy.scc_check_subscription_expiration(request.headers, system.login, system.system_token, logger, system.proxy_byos_mode, product_class)
+      result = SccProxy.scc_check_subscription_expiration(request.headers, system, product_class)
       return true if result[:is_active]
 
       ZypperAuth.zypper_auth_message(request, system, verification_provider, result[:message])

engines/zypper_auth/spec/requests/strict_authentication/authentication_controller_spec.rb

@@ -245,21 +245,16 @@
 
           before do
             stub_request(:get, scc_systems_activations_url).to_return(status: 200, body: [body_active].to_json, headers: {})
-            # allow(SccProxy).to receive(:get_scc_activations).and_return(status: 200, body: [body_active].to_json, headers: {})
             expect(URI).to receive(:encode_www_form).with({ byos_mode: 'hybrid' })
             allow(File).to receive(:directory?).and_return(true)
             allow(Dir).to receive(:mkdir)
             allow(FileUtils).to receive(:touch)
-            # get '/api/auth/check', headers: headers
           end
 
           it 'returns true' do
             result = SccProxy.scc_check_subscription_expiration(
               headers,
-              system_hybrid.login,
-              system_hybrid.system_token,
-              nil,
-              system_hybrid.proxy_byos_mode,
+              system_hybrid,
               system_hybrid.activations.first.product.product_class + '-LTSS'
             )
             expect(result[:is_active]).to be(true)
@@ -298,10 +293,7 @@
           it 'returns false, expired' do
             result = SccProxy.scc_check_subscription_expiration(
               headers,
-              system_hybrid.login,
-              system_hybrid.system_token,
-              nil,
-              system_hybrid.proxy_byos_mode,
+              system_hybrid,
               system_hybrid.activations.first.product.product_class + '-LTSS'
             )
             expect(result[:is_active]).to eq(false)
@@ -341,10 +333,7 @@
           it 'returns product not activated' do
             result = SccProxy.scc_check_subscription_expiration(
               headers,
-              system_hybrid.login,
-              system_hybrid.system_token,
-              nil,
-              system_hybrid.proxy_byos_mode,
+              system_hybrid,
               system_hybrid.activations.first.product.product_class + '-LTSS'
             )
             expect(result[:is_active]).to eq(false)
@@ -384,10 +373,7 @@
           it 'returns unexpected error' do
             result = SccProxy.scc_check_subscription_expiration(
               headers,
-              system_hybrid.login,
-              system_hybrid.system_token,
-              nil,
-              system_hybrid.proxy_byos_mode,
+              system_hybrid,
               system_hybrid.activations.first.product.product_class + '-LTSS'
             )
             expect(result[:is_active]).to eq(false)