janko / rodauth-rails

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

Add support for Rails URL helpers #245

Closed janko closed 3 months ago

janko commented 7 months ago

So far, rodauth-rails has recommended using Rodauth's route helpers.

rodauth.login_path(type: "visitor") #=> "/login?type=visitor"
rodauth.create_account_url #=> "https://example.com/create-account"

This communicates that Rodauth routes are different from Rails routes, since Rodauth endpoints are not handled by the Rails router. However, this starts being less convenient in certain situations:

This adds the ability to define Rails URL helpers for Rodauth routes using the #rodauth router method.

Rails.application.routes.draw do
  rodauth # generate URL helpers for main configuration
  rodauth(:admin) # generate URL helpers for secondary configuration
end
$ rails routes -g rodauth
                Prefix Verb     URI Pattern             Controller#Action
                 login GET|POST /login                  rodauth#login
        create_account GET|POST /create-account         rodauth#create_account
 verify_account_resend GET|POST /verify-account-resend  rodauth#verify_account_resend
        verify_account GET|POST /verify-account         rodauth#verify_account
                logout GET|POST /logout                 rodauth#logout
              remember GET|POST /remember               rodauth#remember
reset_password_request GET|POST /reset-password-request rodauth#reset_password_request
        reset_password GET|POST /reset-password         rodauth#reset_password
       change_password GET|POST /change-password        rodauth#change_password
          change_login GET|POST /change-login           rodauth#change_login
   verify_login_change GET|POST /verify-login-change    rodauth#verify_login_change
         close_account GET|POST /close-account          rodauth#close_account

This also changes instrumentation to use RodauthController + <action> instead of RodauthApp + call, which should be more reliable for monitoring agents that expect :controller to reference an actual controller, and it should make it easier to differentiate between Rodauth endpoints in monitoring services.

34code commented 7 months ago

This is exactly what i need to have a before_action for rodauth controllers for captcha support..

e.g. before_action :check_captcha, only: %i[create_account]

janko commented 7 months ago

Wait, does that before_action actually work with these changes? 🤔 It wasn't intended, but it's an interesting corrolary, and seems like a nice to have 🙂

34code commented 7 months ago

It's not working with the current live latest rodauth-rails version.. but I can check again after this PR is merged 😃

janko commented 7 months ago

It required one additional line to make per-action controller callbacks work 🤘🏻

I'm not sure if this isn't giving too much of an impression that the controller is processing the request instead of the Roda middleware. But if we already support controller callbacks, then I suppose we can also support specifying the action while we're at it. One would just need to distinguish the HTTP verb, e.g:

before_action :check_captcha, only: %i[create_account], if: -> { request.post? }
34code commented 7 months ago

perfect, I will implement my captcha check and check for request.post on the before_action.. is this ready in the current latest version of the gem or will you need to make a new release?

janko commented 7 months ago

No, I just added it to this PR, so it hasn't yet been released.

janko commented 7 months ago

@34code BTW, in the meanwhile you can just use a Rodauth hook:

before_create_account_route do
  rails_controller_instance.send(:check_captcha) if request.post?
end

Another thing that should be solved before merging this PR is that fact that Rodauth routes without imported view templates appear in the rails routes --unused list (relevant code).

34code commented 7 months ago

@34code BTW, in the meanwhile you can just use a Rodauth hook:

before_create_account_route do
  rails_controller_instance.send(:check_captcha) if request.post?
end

Another thing that should be solved before merging this PR is that fact that Rodauth routes without imported view templates appear in the rails routes --unused list (relevant code).

Awesome, will use the Rodauth hook method in that case :) thank you!

janko commented 7 months ago

A big downside of these URL helpers are that it forces Rodauth to be loaded at boot time in development, since routes are loaded on boot. This means that people using URL helpers won't benefit from lazy loading provided by rodauth-rails. Since Rodauth also loads Sequel, this can have a noticeable impact on boot time.

pboling commented 7 months ago

It looks like this will make it much easier to write request specs, by making the ubuquitous line handle Rodauth as well as Rails routes:

RSpec.configure do |config|
  config.include Rails.application.routes.url_helpers, type: :request
end
34code commented 6 months ago

@34code BTW, in the meanwhile you can just use a Rodauth hook:


before_create_account_route do

  rails_controller_instance.send(:check_captcha) if request.post?

end

Another thing that should be solved before merging this PR is that fact that Rodauth routes without imported view templates appear in the rails routes --unused list (relevant code).

Awesome, will use the Rodauth hook method in that case :) thank you!

Confirmed that rodauth route hooks work perfectly with my captcha method!

Dainii commented 4 months ago

Hi @janko,Thank you for this work.

What is the status of this PR? Do you need some testing, or can I do something to help this be merged?

janko commented 4 months ago

@Dainii I'm actually almost ready to close this PR, now that Rodauth allows retrieving the current route. I mostly wanted this to make locale switching easier, but the current route will do that.

At the moment I still think native route helpers can cause confusion, making Rodauth appear more integrated in Rails than it actually is, and I didn't see compelling enough use cases to actually go ahead and merge this. What do you want the native URL helpers for?

Dainii commented 4 months ago

Thank you for the reply. I didn't need it for something specific, but I wanted to know if this was still planned or not.

I recently had to work with rodauth-omniauth and I was working with the SAML strategy. I wanted to update the require_login_redirect to automatically redirect to the SAML endpoint, so I tried to look up the name of the route, but neither rails rodauth:routes nor rails routes displayed it. And when I was investigating why, I saw this PR open and asked for the status.

I agree that since rodauth routes are not managed the same way as rails routes, it makes sense to have separate commands.

janko commented 4 months ago

Yeah, I don't think we can display OmniAuth routes in the rodauth:routes task, because these are technically not Rodauth routes. rodauth-omniauth does add some path helpers for OmniAuth endpoints, but internally they're not registered as Rodauth routes are.

janko commented 3 months ago

I decided not to pursue this pull request, because I think having Rails URL helpers alongside Rodauth URL helpers can add confusion, and suggest that requests are processed at the controller level rather than the middleware level.

Now that the rodauth.current_route method has been added to latest Rodauth version, it's much easier to implement things like locale switchers, which was one of the main motivators behind introducing Rails URL helpers. I also implemented most of the other changes from this PR.

I might still revisit this idea later, but I really want to be conservative and add only things that provide real value, to keep the scope of the gem as narrow as possible.