Merge pull request #1838 from 3scale/2.8/fix/THREESCALE-5103-oidc_con…

…f_attrs-revert-to-default-values

[2.8] Do not recreate OIDCConf object from UI IntegrationsController

app/controllers/api/integrations_controller.rb

@@ -240,7 +240,7 @@ def proxy_params
       :error_headers_auth_missing, :error_auth_missing, :error_status_no_match,
       :error_headers_no_match, :error_no_match, :error_status_limits_exceeded, :error_headers_limits_exceeded, :error_limits_exceeded,
       :api_test_path, :policies_config, proxy_rules_attributes: %i[_destroy id http_method pattern delta metric_id
-                                                                   redirect_url position last], oidc_configuration_attributes: OIDCConfiguration::Config::ATTRIBUTES
+                                                                   redirect_url position last], oidc_configuration_attributes: OIDCConfiguration::Config::ATTRIBUTES + [:id]
     ]
 
     if Rails.application.config.three_scale.apicast_custom_url || @proxy.saas_configuration_driven_apicast_self_managed?

app/controllers/api/services_controller.rb

@@ -101,7 +101,7 @@ def service_params
   end
 
   def proxy_params
-    oidc_params = %i[oidc_issuer_type oidc_issuer_endpoint jwt_claim_with_client_id jwt_claim_with_client_id_type] + [{oidc_configuration_attributes: OIDCConfiguration::Config::FLOWS}]
+    oidc_params = %i[oidc_issuer_type oidc_issuer_endpoint jwt_claim_with_client_id jwt_claim_with_client_id_type] + [{oidc_configuration_attributes: OIDCConfiguration::Config::FLOWS + [:id]}]
     permitted_params = oidc_params + %i[
       auth_user_key auth_app_id auth_app_key credentials_location hostname_rewrite secret_token
       error_status_auth_failed error_headers_auth_failed error_auth_failed

test/integration/api/integrations_controller_test.rb

@@ -154,14 +154,30 @@ def test_edit
   test 'update OIDC Authorization flows' do
     service = FactoryBot.create(:simple_service, account: @provider)
     ProxyTestService.any_instance.stubs(disabled?: true)
-    put admin_service_integration_path(service_id: service.id, proxy: {oidc_configuration_attributes: {standard_flow_enabled: false, direct_access_grants_enabled: true}})
+    service.proxy.oidc_configuration.save!
+    oidc_params = {oidc_configuration_attributes: {standard_flow_enabled: false, direct_access_grants_enabled: true, id: service.proxy.oidc_configuration.id}}
+    assert_no_change of: -> { service.proxy.reload.oidc_configuration.id } do
+      put admin_service_integration_path(service_id: service.id, proxy: oidc_params)
+    end
     assert_response :redirect
 
     service.reload
     refute service.proxy.oidc_configuration.standard_flow_enabled
     assert service.proxy.oidc_configuration.direct_access_grants_enabled
   end
 
+  test 'cannot update OIDC of another proxy' do
+    service = FactoryBot.create(:simple_service, account: @provider)
+    ProxyTestService.any_instance.stubs(disabled?: true)
+    service.proxy.oidc_configuration.save!
+    another_oidc_config = FactoryBot.create(:oidc_configuration)
+    oidc_params = {oidc_configuration_attributes: {standard_flow_enabled: false, direct_access_grants_enabled: true, id: another_oidc_config.id}}
+    assert_no_change of: -> { service.proxy.reload.oidc_configuration.id } do
+      put admin_service_integration_path(service_id: service.id, proxy: oidc_params)
+    end
+    assert_response :not_found
+  end
+
   test 'edit not found for apiap' do
     rolling_updates_on
     Account.any_instance.expects(:provider_can_use?).with(:api_as_product).returns(true)

test/integration/api/services_controller_test.rb

@@ -31,9 +31,12 @@ class SettingsTest < Api::ServicesControllerTest
     test 'update the settings' do
       Account.any_instance.stubs(:provider_can_use?).returns(true)
       rolling_update(:api_as_product, enabled: false)
-
       service.update!(deployment_option: 'self_managed')
-      put admin_service_path(service), update_params
+      service.proxy.oidc_configuration.save!
+      previous_oidc_config_id = service.proxy.reload.oidc_configuration.id
+
+      put admin_service_path(service), update_params(oidc_id: previous_oidc_config_id)
+
       assert_equal 'Service information updated.', flash[:notice]
 
       update_service_params = update_params[:service]
@@ -57,11 +60,21 @@ class SettingsTest < Api::ServicesControllerTest
       end
 
       oidc_configuration = proxy.oidc_configuration
-      oidc_configuration_params.each do |field_name, param_value|
+      oidc_configuration_params.except(:id).each do |field_name, param_value|
         expected_value = param_value == '1'
         assert_equal expected_value, oidc_configuration.public_send(field_name)
       end
+      assert_equal previous_oidc_config_id, proxy.reload.oidc_configuration.id
+    end
 
+    test 'cannot update OIDC of another proxy' do
+      service.proxy.oidc_configuration.save!
+      another_oidc_config = FactoryBot.create(:oidc_configuration)
+      oidc_params = {oidc_configuration_attributes: {direct_access_grants_enabled: true, id: another_oidc_config.id}}
+      assert_no_change of: -> { service.proxy.reload.oidc_configuration.id } do
+        put admin_service_path(service), {service: {proxy_attributes: oidc_params}}
+      end
+      assert_response :not_found
     end
 
     # This test can be removed once used deprecated attributes have been removed from the schema
@@ -159,7 +172,7 @@ class SettingsTest < Api::ServicesControllerTest
 
     private
 
-    def update_params
+    def update_params(oidc_id: nil)
       @update_params ||= { service:
         { intentions_required: '0',
           buyers_manage_apps: '0',
@@ -182,7 +195,8 @@ def update_params
               standard_flow_enabled: '1',
               implicit_flow_enabled: '1',
               service_accounts_enabled: '0',
-              direct_access_grants_enabled: '0'
+              direct_access_grants_enabled: '0',
+              id: oidc_id
             }
           }
         }