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.
This commit is contained in:
David Taylor 2023-10-26 10:54:30 +01:00 committed by GitHub
parent 8438aed727
commit c88303bb27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 5 additions and 20 deletions

View File

@ -11,8 +11,6 @@ class Middleware::OmniauthBypassMiddleware
def initialize(app, options = {}) def initialize(app, options = {})
@app = app @app = app
Discourse.plugins.each(&:notify_before_auth)
OmniAuth.config.before_request_phase do |env| OmniAuth.config.before_request_phase do |env|
request = ActionDispatch::Request.new(env) request = ActionDispatch::Request.new(env)

View File

@ -50,7 +50,6 @@ class Plugin::Instance
%i[ %i[
assets assets
color_schemes color_schemes
before_auth_initializers
initializers initializers
javascripts javascripts
locales locales
@ -510,13 +509,6 @@ class Plugin::Instance
@git_repo ||= GitRepo.new(directory, name) @git_repo ||= GitRepo.new(directory, name)
end 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 # A proxy to `DiscourseEvent.on` which does nothing if the plugin is disabled
def on(event_name, &block) def on(event_name, &block)
DiscourseEvent.on(event_name) { |*args, **kwargs| block.call(*args, **kwargs) if enabled? } DiscourseEvent.on(event_name) { |*args, **kwargs| block.call(*args, **kwargs) if enabled? }
@ -541,11 +533,6 @@ class Plugin::Instance
end end
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? # Applies to all sites in a multisite environment. Ignores plugin.enabled?
def register_category_custom_field_type(name, type) def register_category_custom_field_type(name, type)
reloadable_patch { |plugin| Category.register_custom_field_type(name, type) } reloadable_patch { |plugin| Category.register_custom_field_type(name, type) }
@ -793,7 +780,7 @@ class Plugin::Instance
end end
def auth_provider(opts) def auth_provider(opts)
before_auth do after_initialize do
provider = Auth::AuthProvider.new provider = Auth::AuthProvider.new
Auth::AuthProvider.auth_attributes.each do |sym| Auth::AuthProvider.auth_attributes.each do |sym|

View File

@ -218,14 +218,14 @@ RSpec.describe Plugin::Instance do
# No enabled_site_setting # No enabled_site_setting
authenticator = SimpleAuthenticator.new authenticator = SimpleAuthenticator.new
plugin.auth_provider(authenticator: authenticator) plugin.auth_provider(authenticator: authenticator)
plugin.notify_before_auth plugin.notify_after_initialize
expect(authenticator.enabled?).to eq(true) expect(authenticator.enabled?).to eq(true)
# With enabled site setting # With enabled site setting
plugin = Plugin::Instance.new plugin = Plugin::Instance.new
authenticator = SimpleAuthenticator.new authenticator = SimpleAuthenticator.new
plugin.auth_provider(enabled_setting: "enable_badges", authenticator: authenticator) plugin.auth_provider(enabled_setting: "enable_badges", authenticator: authenticator)
plugin.notify_before_auth plugin.notify_after_initialize
expect(authenticator.enabled?).to eq(false) expect(authenticator.enabled?).to eq(false)
# Defines own method # Defines own method
@ -241,7 +241,7 @@ RSpec.describe Plugin::Instance do
end end
.new .new
plugin.auth_provider(enabled_setting: "enable_badges", authenticator: authenticator) plugin.auth_provider(enabled_setting: "enable_badges", authenticator: authenticator)
plugin.notify_before_auth plugin.notify_after_initialize
expect(authenticator.enabled?).to eq(false) expect(authenticator.enabled?).to eq(false)
end end
@ -271,7 +271,7 @@ RSpec.describe Plugin::Instance do
plugin = plugin_from_fixtures("my_plugin") plugin = plugin_from_fixtures("my_plugin")
plugin.activate! plugin.activate!
expect(DiscoursePluginRegistry.auth_providers.count).to eq(0) expect(DiscoursePluginRegistry.auth_providers.count).to eq(0)
plugin.notify_before_auth plugin.notify_after_initialize
expect(DiscoursePluginRegistry.auth_providers.count).to eq(1) expect(DiscoursePluginRegistry.auth_providers.count).to eq(1)
auth_provider = DiscoursePluginRegistry.auth_providers.to_a[0] auth_provider = DiscoursePluginRegistry.auth_providers.to_a[0]
expect(auth_provider.authenticator.name).to eq("facebook") expect(auth_provider.authenticator.name).to eq("facebook")