Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Rails URL helpers #245

Closed
wants to merge 5 commits into from
Closed

Add support for Rails URL helpers #245

wants to merge 5 commits into from

Conversation

janko
Copy link
Owner

@janko janko commented Nov 20, 2023

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:

  • not available in tests (need to use something like path_class_methods)
  • not displayed in rails routes (need to use separate rails rodauth:routes command)
  • locale switching using url_for(locale: locale) doesn't work on Rodauth pages

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.

This way we get access to String#delete_suffix and String#delete_prefix
methods.
Format suffixes won't be routed by Rodauth, so we can communicate that
when declaring the routes.
@34code
Copy link

34code commented Nov 24, 2023

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
Copy link
Owner Author

janko commented Nov 25, 2023

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
Copy link

34code commented Nov 25, 2023

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

@janko
Copy link
Owner Author

janko commented Nov 26, 2023

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
Copy link

34code commented Nov 26, 2023

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
Copy link
Owner Author

janko commented Nov 27, 2023

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

@janko
Copy link
Owner Author

janko commented Nov 27, 2023

@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
Copy link

34code commented Dec 1, 2023

@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
Copy link
Owner Author

janko commented Dec 4, 2023

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
Copy link

pboling commented Dec 14, 2023

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

@pboling pboling mentioned this pull request Dec 22, 2023
@34code
Copy link

34code commented Dec 27, 2023

@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
Copy link

Dainii commented Mar 8, 2024

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
Copy link
Owner Author

janko commented Mar 8, 2024

@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
Copy link

Dainii commented Mar 11, 2024

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
Copy link
Owner Author

janko commented Mar 11, 2024

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
Copy link
Owner Author

janko commented Apr 8, 2024

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.

@janko janko closed this Apr 8, 2024
@janko janko deleted the url-helpers branch April 8, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants