mikker / passwordless

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

Bug: default_url_options[:host] is required for mailer #184

Closed arrowcircle closed 1 year ago

arrowcircle commented 1 year ago

Hey! I found a bug, when using sidekiq to send emails. To reproduce, uncomment this in passwordless initializer

config.after_session_save = lambda do |session, request|
    Passwordless::Mailer.sign_in(session, session.token).deliver_later
  end

Set sidekiq as active_job adapter config.active_job.queue_adapter = :sidekiq

Then with working mailer, it will fail to deliver email saying about missing host. This is fixed adding line to production.rb after config.action_mailer.default_url_options

routes.default_url_options[:host] ||= "www.example.com"
mikker commented 1 year ago

Not a bug per se and also already mentioned in the docs. https://github.com/mikker/passwordless#urls-and-links

I agree that it could be more visible though

arrowcircle commented 1 year ago

Hey! Thanks for the answer. I think I didn't communicated problem properly. Even with set action_mailer routes host it does not work without adding routes.default_url_options. So it works only if both mailer and routes options are set

config.action_mailer.default_url_options = {host: "www.example.com"}
routes.default_url_options[:host] ||= "www.example.com"
mikker commented 1 year ago

I see. What do you suggest? Should we make this more clear in the setup instructions?

arrowcircle commented 1 year ago

I think we could at least mention this in the readme, so if anyone else struggle with this problem will not spend time trying to understand what is going wrong.

alexebird commented 1 year ago

(Responding re: #186)

I think just updating the docs would be good enough.

mikker commented 1 year ago

I've updated the README to include your example config @arrowcircle. Thank you for reporting this and providing a fix. https://github.com/mikker/passwordless/blob/master/README.md#urls-and-links