janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
599 stars 40 forks source link

Issue when using a roda prefix when using the same prefix as a namespace in Rails #34

Closed nicolas-besnard closed 3 years ago

nicolas-besnard commented 3 years ago

In rails, I'm using the following routes to scope my Admins controller:

namespace :admins do
  resources :questions

  root to: 'dashboard#index'
end

Which creates the routes:

admins_root      GET  /admins(.:format)                      admins/dashboard#index
admins_questions GET  /admins/questions(.:format)            admins/questions#index

Then I'm configuration rodauth with:

configure(:admin) do
    enable :login, :logout, :remember
    rails_controller { Admins::RodauthController }
    prefix "/admins"
end

route do |r|
    rodauth.load_memory # autologin remembered users

    r.on "admins" do
      r.rodauth(:admin)

       if !rodauth(:admin).logged_in?
         rodauth(:admin).require_authentication
       end
    end
end

The login is working, but once I'm logged, I can not access the Rails routes and I got a blank page.

One solution would be to use an other Roda prefix (such as admin-authentication), but maybe there's a way to fallthrough Rails routes ?

I've a branch wich reproduce this issue https://github.com/nicolas-besnard/rodauth-rails/commit/ddf5da42d27c325f72324dec363c2b0d73448770

janko commented 3 years ago

Thanks again for the report. This is caused by how Roda routing works; if a request enters a r.on block, the request is considered as handled, even if it reaches the end of the block, and in this case Roda middleware will not call the Rails app.

One workaround is to load the pass Roda plugin and call r.pass at the end of the r.on block:

class RodauthApp < Rodauth::Rails::App
  # ...
  plugin :pass

  route do |r|
    # ...
    r.on "admins" do
      r.rodauth(:admin)

      if !rodauth(:admin).logged_in?
        rodauth(:admin).require_authentication
      end

      r.pass # this will break out of the block, and allow the route block to reach the end
    end
    # ...
  end
end

@jeremyevans do you know of a better way to have custom routes accessible that have the same prefix as the Rodauth prefix when the Roda app is used as a middleware?

jeremyevans commented 3 years ago

Using r.pass is fine. You can also use throw :next, true at any point to directly pass control to the next middleware. Maybe we could add a Roda method for that to the middleware plugin (e.g. r.next_middleware).

bjeanes commented 3 years ago

I'm not sure if there are any downsides to this, but I have been just using break which seems to accomplish it too:

admin = rodauth(:admin)
r.on(admin.prefix.delete_prefix("/")) do
  r.rodauth(:admin)
  admin.require_authentication
  break # IMPORTANT: allow routing to continue to Rails
end

Should I switch to using throw :next or r.pass?

janko commented 3 years ago

Wow, thank you for this discovery, break seems to work for me too 🙏🏻. It's definitely preferred, since it's something rubyists are already familiar with 👍🏻

pboling commented 10 months ago

So break > r.pass? If break works, what is the purpose of the pass plugin sugar @jeremyevans ? Are we missing out on something if we simply use break?

jeremyevans commented 10 months ago

The pass plugin was designed for users familiar with the same feature in Sinatra. Agreed that break seems like a better way to handle things in Roda. I'll have to do some testing to see if there are any corner cases (maybe around hash_routes/multi_route?).

janko commented 10 months ago

I found that break doesn't produce correct behavior, because it doesn't reset the matched segment from the block we're skipping. So, the following:

r.on "auth" do
  r.rodauth
  break
end

r.on "admin" do
  r.rodauth(:admin)
end

would in addition to /admin/login also route /auth/admin/login, assuming the :admin Rodauth configuration loads the login feature.

As of latest rodauth-rails version, you shouldn't need break or r.pass anymore (unless you're defining custom routes in the Roda app itself), because it activates the next_if_not_found: true on Roda's middleware plugin, which will automatically forward any unmatched requests to the Rails router.

jeremyevans commented 10 months ago

Since break inside routing blocks doesn't work by default for the reasons @janko mentioned, I added a break plugin to Roda to support it: https://github.com/jeremyevans/roda/commit/bc1fefea1d583883b9c3d4501f6b45eb19003751