REFACTOR: Initialize auth providers after `plugin.activate!`

Also added some helpful functionality for plugin developers:
- Raises RuntimeException if the auth provider has been registered too late
- Logs use of deprecated parameters

lib/auth/auth_provider.rb

@@ -8,11 +8,18 @@ class Auth::AuthProvider
   def self.auth_attributes
     [:pretty_name, :title, :message, :frame_width, :frame_height, :authenticator,
      :pretty_name_setting, :title_setting, :enabled_setting, :full_screen_login, :full_screen_login_setting,
-     :custom_url]
+     :custom_url, :background_color]
   end
 
   attr_accessor(*auth_attributes)
 
+  def enabled_setting=(val)
+    Discourse.deprecate("enabled_setting is deprecated. Please define authenticator.enabled? instead")
+    @enabled_setting = val
+  end
+
+  def background_color=(val) Discourse.deprecate("background_color is no longer functional. Please define authenticator.enabled? instead") end;
+
   def name
     authenticator.name
   end

lib/middleware/omniauth_bypass_middleware.rb

@@ -7,6 +7,8 @@ class Middleware::OmniauthBypassMiddleware
   def initialize(app, options = {})
     @app = app
 
+    Discourse.plugins.each(&:notify_before_auth)
+
     # if you need to test this and are having ssl issues see:
     #  http://stackoverflow.com/questions/6756460/openssl-error-using-omniauth-specified-ssl-path-but-didnt-work
     # OpenSSL::SSL::VERIFY_PEER = OpenSSL::SSL::VERIFY_NONE if Rails.env.development?

lib/plugin/instance.rb

@@ -25,8 +25,8 @@ class Plugin::Instance
 
   # Memoized array readers
   [:assets,
-   :auth_providers,
    :color_schemes,
+   :before_auth_initializers,
    :initializers,
    :javascripts,
    :locales,
@@ -287,6 +287,11 @@ class Plugin::Instance
     initializers << block
   end
 
+  def before_auth(&block)
+    raise "Auth providers must be registered before omniauth middleware. after_initialize is too late!" if @before_auth_complete
+    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) do |*args|
@@ -313,6 +318,13 @@ class Plugin::Instance
     end
   end
 
+  def notify_before_auth
+    before_auth_initializers.each do |callback|
+      callback.call(self)
+    end
+    @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 do |plugin|
@@ -458,7 +470,6 @@ class Plugin::Instance
     register_assets! unless assets.blank?
     register_locales!
     register_service_workers!
-    register_auth_providers!
 
     seed_data.each do |key, value|
       DiscoursePluginRegistry.register_seed_data(key, value)
@@ -496,13 +507,13 @@ class Plugin::Instance
   end
 
   def auth_provider(opts)
-    provider = Auth::AuthProvider.new
+    before_auth do
+      provider = Auth::AuthProvider.new
 
-    Auth::AuthProvider.auth_attributes.each do |sym|
-      provider.send "#{sym}=", opts.delete(sym)
-    end
+      Auth::AuthProvider.auth_attributes.each do |sym|
+        provider.send "#{sym}=", opts.delete(sym)
+      end
 
-    after_initialize do
       begin
         provider.authenticator.enabled?
       rescue NotImplementedError
@@ -513,9 +524,9 @@ class Plugin::Instance
           true
         end
       end
-    end
 
-    auth_providers << provider
+      DiscoursePluginRegistry.register_auth_provider(provider)
+    end
   end
 
   # shotgun approach to gem loading, in future we need to hack bundler
@@ -594,12 +605,6 @@ class Plugin::Instance
     end
   end
 
-  def register_auth_providers!
-    auth_providers.each do |auth_provider|
-      DiscoursePluginRegistry.register_auth_provider(auth_provider)
-    end
-  end
-
   def register_locales!
     root_path = File.dirname(@path)
 

spec/components/plugin/instance_spec.rb

@@ -133,16 +133,18 @@ describe Plugin::Instance do
     # No enabled_site_setting
     authenticator = Auth::Authenticator.new
     plugin.auth_provider(authenticator: authenticator)
-    plugin.notify_after_initialize
+    plugin.notify_before_auth
     expect(authenticator.enabled?).to eq(true)
 
     # With enabled site setting
+    plugin = Plugin::Instance.new
     authenticator = Auth::Authenticator.new
     plugin.auth_provider(enabled_setting: 'ubuntu_login_enabled', authenticator: authenticator)
-    plugin.notify_after_initialize
+    plugin.notify_before_auth
     expect(authenticator.enabled?).to eq(false)
 
     # Defines own method
+    plugin = Plugin::Instance.new
     SiteSetting.stubs(:ubuntu_login_enabled).returns(true)
     authenticator = Class.new(Auth::Authenticator) do
       def enabled?
@@ -150,7 +152,7 @@ describe Plugin::Instance do
       end
     end.new
     plugin.auth_provider(enabled_setting: 'ubuntu_login_enabled', authenticator: authenticator)
-    plugin.notify_after_initialize
+    plugin.notify_before_auth
     expect(authenticator.enabled?).to eq(false)
   end
 
@@ -182,11 +184,11 @@ describe Plugin::Instance do
       plugin = Plugin::Instance.new
       plugin.path = "#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb"
       plugin.activate!
-
-      expect(plugin.auth_providers.count).to eq(1)
-      auth_provider = plugin.auth_providers[0]
-      expect(auth_provider.authenticator.name).to eq('ubuntu')
+      expect(DiscoursePluginRegistry.auth_providers.count).to eq(0)
+      plugin.notify_before_auth
       expect(DiscoursePluginRegistry.auth_providers.count).to eq(1)
+      auth_provider = DiscoursePluginRegistry.auth_providers.to_a[0]
+      expect(auth_provider.authenticator.name).to eq('ubuntu')
     end
 
     it "finds all the custom assets" do