From c88303bb2788b1f9cdf129fffe4b9e54aa5006d2 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Thu, 26 Oct 2023 10:54:30 +0100
Subject: [PATCH] DEV: Relax auth provider registration restrictions for
 plugins (#24095)

In the past we would build the stack of Omniauth providers at boot, which meant that plugins had to register any authenticators in the root of their plugin.rb (i.e. not in an `after_initialize` block). This could be frustrating because many features are not available that early in boot (e.g. Zeitwerk autoloading).

Now that we build the omniauth strategy stack 'just in time', it is safe for plugins to register their auth methods in an `after_initialize` block. This commit relaxes the old restrictions so that plugin authors have the option to move things around.
---
 lib/middleware/omniauth_bypass_middleware.rb |  2 --
 lib/plugin/instance.rb                       | 15 +--------------
 spec/lib/plugin/instance_spec.rb             |  8 ++++----
 3 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/lib/middleware/omniauth_bypass_middleware.rb b/lib/middleware/omniauth_bypass_middleware.rb
index c9b6b93d565..95a2f5d1d18 100644
--- a/lib/middleware/omniauth_bypass_middleware.rb
+++ b/lib/middleware/omniauth_bypass_middleware.rb
@@ -11,8 +11,6 @@ class Middleware::OmniauthBypassMiddleware
   def initialize(app, options = {})
     @app = app
 
-    Discourse.plugins.each(&:notify_before_auth)
-
     OmniAuth.config.before_request_phase do |env|
       request = ActionDispatch::Request.new(env)
 
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index 17241f75005..e912cc31109 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -50,7 +50,6 @@ class Plugin::Instance
   %i[
     assets
     color_schemes
-    before_auth_initializers
     initializers
     javascripts
     locales
@@ -510,13 +509,6 @@ class Plugin::Instance
     @git_repo ||= GitRepo.new(directory, name)
   end
 
-  def before_auth(&block)
-    if @before_auth_complete
-      raise "Auth providers must be registered before omniauth middleware. after_initialize is too late!"
-    end
-    before_auth_initializers << block
-  end
-
   # A proxy to `DiscourseEvent.on` which does nothing if the plugin is disabled
   def on(event_name, &block)
     DiscourseEvent.on(event_name) { |*args, **kwargs| block.call(*args, **kwargs) if enabled? }
@@ -541,11 +533,6 @@ class Plugin::Instance
     end
   end
 
-  def notify_before_auth
-    before_auth_initializers.each { |callback| callback.call(self) }
-    @before_auth_complete = true
-  end
-
   # Applies to all sites in a multisite environment. Ignores plugin.enabled?
   def register_category_custom_field_type(name, type)
     reloadable_patch { |plugin| Category.register_custom_field_type(name, type) }
@@ -793,7 +780,7 @@ class Plugin::Instance
   end
 
   def auth_provider(opts)
-    before_auth do
+    after_initialize do
       provider = Auth::AuthProvider.new
 
       Auth::AuthProvider.auth_attributes.each do |sym|
diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb
index cc07ac964ab..57840d2d092 100644
--- a/spec/lib/plugin/instance_spec.rb
+++ b/spec/lib/plugin/instance_spec.rb
@@ -218,14 +218,14 @@ RSpec.describe Plugin::Instance do
     # No enabled_site_setting
     authenticator = SimpleAuthenticator.new
     plugin.auth_provider(authenticator: authenticator)
-    plugin.notify_before_auth
+    plugin.notify_after_initialize
     expect(authenticator.enabled?).to eq(true)
 
     # With enabled site setting
     plugin = Plugin::Instance.new
     authenticator = SimpleAuthenticator.new
     plugin.auth_provider(enabled_setting: "enable_badges", authenticator: authenticator)
-    plugin.notify_before_auth
+    plugin.notify_after_initialize
     expect(authenticator.enabled?).to eq(false)
 
     # Defines own method
@@ -241,7 +241,7 @@ RSpec.describe Plugin::Instance do
         end
         .new
     plugin.auth_provider(enabled_setting: "enable_badges", authenticator: authenticator)
-    plugin.notify_before_auth
+    plugin.notify_after_initialize
     expect(authenticator.enabled?).to eq(false)
   end
 
@@ -271,7 +271,7 @@ RSpec.describe Plugin::Instance do
       plugin = plugin_from_fixtures("my_plugin")
       plugin.activate!
       expect(DiscoursePluginRegistry.auth_providers.count).to eq(0)
-      plugin.notify_before_auth
+      plugin.notify_after_initialize
       expect(DiscoursePluginRegistry.auth_providers.count).to eq(1)
       auth_provider = DiscoursePluginRegistry.auth_providers.to_a[0]
       expect(auth_provider.authenticator.name).to eq("facebook")