Do not recreate OIDCConf object from UI ServicesController when ID pa…

…ram is sent
3scale · Apr 29, 2020 · 9565777 · 9565777
1 parent 456fd60
commit 9565777
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 6 deletions.
diff --git a/app/controllers/api/services_controller.rb b/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

diff --git a/test/integration/api/services_controller_test.rb b/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
             }
           }
         }