Authenticating the engine mount with custom Rails auth

[maxim]

Hi, I could use some advice. Typically, when you roll your own Rails auth, you use before_actions to authenticate. However, an engine is mounted under routes.

• If I was using Devise, I'd be able to authenticate routes, but we don't want to use it on this project.
• If I was to write my own middleware (similar to Warden), I'd be able to supply current_user in a request's env, and write my own routes constraint, however I would lose out on Rails controller conveniences, and it would be a pretty significant feature.
• With simply using before_actions in Rails, there doesn't seem to be a good way to authenticate the GoodJob's engine.

The only thing we'd need to check is current_user&.admin?.

It seems that a solution for this would fit the philosophy of the project of being very Rails-centric. What are your thoughts? Perhaps there's an existing configuration I missed?

[bensheldon]

I'll sympathize that I think Rails Engines could make it simpler.

Most straightforwardly, my recommendation would be to copy your user session code into an initializer where you mix it into GoodJob::ApplicationController. To be DRYer, you could extract the user session code to a module, and then mix it into both your ApplicationController and GoodJob::ApplicationController. Something like this:

# config/initializers/good_job.rb

module GoodJobAuthentication
  extend ActiveSupport::Concern

  included do
    before_action do
      raise ActionController::RoutingError.new('Not Found') unless current_user&.admin?
    end
  end

  def current_user
    # load your user out of the session
  end
end

Rails.application.reloader.to_prepare do
  GoodJob::ApplicationController.include(GoodJobAuthentication)
end

That to_prepare isn't ideal though. Do you think it would be any better/clearer if that was like config.good_job.engine_controller_include = ["GoodJobAuthentication"]?

[maxim]

You know, this is surprisingly good for now, I didn't think of extending the engine's ApplicationController.

In my case, I was including an already existing concern into GoodJobAuth module, so had to wrap the whole thing into to_prepare, like this.

Rails.application.reloader.to_prepare do
  module GoodJobAuthorization
    extend ActiveSupport::Concern
    include SessionManagement # This doesn't work outside of to_prepare.

    included do
      before_action :set_session_from_cookie

      before_action do
        unless current_user&.admin?
          raise ActionController::RoutingError.new('Not Found')
        end
      end
    end
  end

  GoodJob::ApplicationController.include(GoodJobAuthorization)
end

This seems to work perfectly. 👍🏻

That said, it would probably be too presumptuous to assume a specific controller structure in the engine, and for you (gem author) to commit to it as a public API. Seems safer to do the config option you proposed. Maybe there's a more elegant way to allow this? 🤔

[maxim]

I ended up cleaning this up and generalizing this a little by adding this concern into app/controllers/concerns/mounted_engine_admin_authentication.rb:

# This module is not for our controllers, it's for including into mounted
# engines, to make them use our authentication strategy and require admin.
module MountedEngineAdminAuthentication
  extend ActiveSupport::Concern
  include SessionManagement

  included do
    before_action :set_session_from_cookie

    before_action do
      unless current_user&.admin?
        raise ActionController::RoutingError.new('Not Found')
      end
    end
  end
end

And config/initializers/good_job.rb now looks like this:

Rails.application.reloader.to_prepare do
  GoodJob::ApplicationController.include(MountedEngineAdminAuthentication)
end

[bensheldon]

That looks nice!

Maybe there's a more elegant way to allow this?

Also, this makes me think I should add an initializer hook into the GoodJob::ApplicationController, that would be the ideal "this is extensible, but still not wholly 'public'" (and an initializer hook is a little nicer on the autoloader than a to_prepare because an initializer hook will only be called when the constant is loaded, whereas the prepare hook will load the constant every autoloader reload).

That all said, I believe that GoodJob::ApplicationController is in practice very stable for the purpose of extending with auth.

btw, I think it is a little cleaner if you move the include SessionManagement into the concern's included do block. That will defer the constant lookup for the autoloaded concern:

module MountedEngineAdminAuthentication
  extend ActiveSupport::Concern
  
  included do
    include SessionManagement # <-- right here

    before_action :set_session_from_cookie

    before_action do
      unless current_user&.admin?
        raise ActionController::RoutingError.new('Not Found')
      end
    end
  end
end

[maxim]

I should add an initializer hook into the GoodJob::ApplicationController

I like it, it would cement intentional support for this use case.

I think it is a little cleaner if you move the include SessionManagement into the concern's included do block

If I understand correctly, in my version, Rails will still reload SessionManagement automatically on change (because it's located in app/controllers/concerns). Each reload will redefine the module, and changes will take effect without having to re-include it. If I move the include to included, I would be re-including SessionManagement on each reload, which seems like an unnecessary step. Is that correct? 🤔 I'm no expert on zeitwerk, so can't tell for sure.

If the idea is to avoid looking up this constant until later, I believe the lookup would happen early anyway, because to_prepare would run include on init, and include would run included right away.

[bensheldon]

I just released https://github.com/bensheldon/good_job/releases/tag/v3.19.1 with a load hook:

# config/initializers/good_job.rb

ActiveSupport.on_load(:good_job_application_controller) do
  # context here is GoodJob::ApplicationController

  include SessionManagement

  before_action :set_session_from_cookie

  before_action do
    unless current_user&.admin?
      raise ActionController::RoutingError.new('Not Found')
    end
  end
end