mikker / passwordless

🗝 Authentication for your Rails app without the icky-ness of passwords
MIT License
1.26k stars 85 forks source link

Passwordless route namespace hijacks Rails root_url and other routes #133

Closed cmdrkeene closed 1 year ago

cmdrkeene commented 1 year ago

The passwordless route helpers clobber root_url and other route helpers inside the /users namespace.

For example, if you have a 'WelcomeController#index' action, configured in routes.rb with passwordless:

Rails.application.routes.draw do
  passwordless_for :users
  root 'welcome#index'
end

If you are inside app/views/welcome/index.html.erb and you reference root_url from your shared application.html.erb layout, you get what you expect:

<%= root_url %> <!-- outputs "/" as expected -->

But if you use that same application.html.erb layout to render the bundled passwordless views, and try to link back home, you will get /users prefixed onto ALL OF YOUR ROUTES!

For example, if you call root_url inside application.html.erb rendered from app/views/passwordless/sessions.new.html.erb you will get AN INCORRECT URL!

<%= root_url %> <!-- outputs "/users" incorrectly! -->

The only fix I have found is to NOT use the default passwordless_for and instead pass / to it:

Rails.application.routes.draw do
  passwordless_for :users, at: '/', as: :users
  root 'welcome#index'
end

Now visiting /sign_in does NOT prefix all your routes with /users including root_url and other helpers.

Happy to be corrected if wrong, but I think your routing is clobbering a namespace and should be implemented differently.

Thanks.

mikker commented 1 year ago

This is due to Passwordless using isolate_namespace (https://guides.rubyonrails.org/engines.html#routes).

Your routes should use main_app.projects_path and users.sign_in_path for the link helpers. It's cumbersome and frequently missed so I'm looking to removing it in 1.0 (#89).

Feel free to reopen if I'm misunderstanding something.

cmdrkeene commented 1 year ago

Thanks for clarifying! A note in the routes readme section as a reminder would work too. Thanks also for the gem!