Merge pull request #17389 from NobodysNightmare/fix-eager-autodiscovery

Fix auto-discovery of OIDC details to be too eager
opf · Dec 9, 2024 · c5ad07b · c5ad07b
2 parents 08278f3 + 1480ed7
commit c5ad07b
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 10 deletions.
diff --git a/modules/openid_connect/app/components/openid_connect/providers/sections/form_component.rb b/modules/openid_connect/app/components/openid_connect/providers/sections/form_component.rb
@@ -30,7 +30,7 @@
 
 module OpenIDConnect::Providers::Sections
   class FormComponent < ::Saml::Providers::Sections::SectionComponent
-    attr_reader :edit_state, :next_edit_state, :edit_mode
+    attr_reader :edit_state, :next_edit_state, :edit_mode, :fetch_metadata
 
     def initialize(provider,
                    edit_state:,
@@ -39,7 +39,8 @@ def initialize(provider,
                    banner: nil,
                    banner_scheme: :default,
                    next_edit_state: nil,
-                   edit_mode: nil)
+                   edit_mode: nil,
+                   fetch_metadata: false)
       super(provider)
 
       @edit_state = edit_state
@@ -49,6 +50,7 @@ def initialize(provider,
       @heading = heading
       @banner = banner
       @banner_scheme = banner_scheme
+      @fetch_metadata = fetch_metadata
     end
 
     def url
@@ -61,9 +63,9 @@ def url
 
     def form_url_params
       if edit_mode
-        { edit_state:, edit_mode:, next_edit_state: }
+        { edit_state:, fetch_metadata:, edit_mode:, next_edit_state: }
       else
-        { edit_state: }
+        { edit_state:, fetch_metadata: }
       end
     end
 

diff --git a/modules/openid_connect/app/components/openid_connect/providers/view_component.html.erb b/modules/openid_connect/app/components/openid_connect/providers/view_component.html.erb
@@ -22,7 +22,8 @@
                                        edit_state:,
                                        next_edit_state: :client_details,
                                        edit_mode:,
-                                       heading: nil
+                                       heading: nil,
+                                       fetch_metadata: true
                                      )
                                   else
                                     OpenIDConnect::Providers::Sections::ShowComponent.new(
@@ -44,6 +45,7 @@
             edit_state:,
             edit_mode:,
             heading: nil,
+            fetch_metadata: true
           ))
         else
           render(OpenIDConnect::Providers::Sections::ShowComponent.new(
@@ -66,7 +68,8 @@
                                        edit_state:,
                                        next_edit_state: :client_details,
                                        edit_mode:,
-                                       heading: nil
+                                       heading: nil,
+                                       fetch_metadata: true
                                      )
                                   else
                                     OpenIDConnect::Providers::Sections::ShowComponent.new(
@@ -87,7 +90,8 @@
             form_class: OpenIDConnect::Providers::ClientDetailsForm,
             edit_state:,
             edit_mode:,
-            heading: nil
+            heading: nil,
+            fetch_metadata: true
           ))
         else
           render(OpenIDConnect::Providers::Sections::ShowComponent.new(
@@ -137,7 +141,8 @@
             heading: nil,
             edit_state:,
             edit_mode:,
-            next_edit_state: :metadata_details
+            next_edit_state: :metadata_details,
+            fetch_metadata: true
           ))
         else
           render(OpenIDConnect::Providers::Sections::ShowComponent.new(

diff --git a/modules/openid_connect/app/controllers/openid_connect/providers_controller.rb b/modules/openid_connect/app/controllers/openid_connect/providers_controller.rb
@@ -67,7 +67,7 @@ def update
                         .permit(:display_name, :oidc_provider, :limit_self_registration,
                                 *OpenIDConnect::Provider.stored_attributes[:options])
       call = OpenIDConnect::Providers::UpdateService
-        .new(model: @provider, user: User.current)
+        .new(model: @provider, user: User.current, fetch_metadata: fetch_metadata?)
         .call(update_params)
 
       if call.success?
@@ -162,5 +162,9 @@ def set_edit_state
       @edit_mode = ActiveRecord::Type::Boolean.new.cast(params[:edit_mode])
       @next_edit_state = params[:next_edit_state].to_sym if params.key?(:next_edit_state)
     end
+
+    def fetch_metadata?
+      params[:fetch_metadata] == "true"
+    end
   end
 end
diff --git a/modules/openid_connect/app/services/openid_connect/providers/update_service.rb b/modules/openid_connect/app/services/openid_connect/providers/update_service.rb
@@ -40,10 +40,16 @@ class AttributesContract < Dry::Validation::Contract
         end
       end
 
+      def initialize(*, fetch_metadata: false, **)
+        super(*, **)
+
+        @fetch_metadata = fetch_metadata
+      end
+
       def after_validate(_params, call)
         model = call.result
         metadata_url = get_metadata_url(model)
-        return call if metadata_url.blank?
+        return call if metadata_url.blank? || !@fetch_metadata
 
         extract_metadata(call, metadata_url, model)
       end