mikker / passwordless

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

Always redirect magic link requests back to the sign_in page and render generic flash #120

Closed nickhammond closed 1 year ago

nickhammond commented 2 years ago

Hey @mikker,

I realized that I couldn't get magic link requests to work at all without always doing a redirect for Turbo. Turbo is always going to complain about the response needing to be a redirect when it's a simple render call without a status: :unprocessable_entity.

If sessions/create.html.erb is removed though I was thinking it might be nice to be able to specify a redirect location instead of the default or some way to detect that a magic link was just requested after the redirect.

Related to: https://github.com/mikker/passwordless/pull/116 Root issue: https://github.com/hotwired/turbo-rails/issues/12

mikker commented 2 years ago

Nice debugging, thank you!

I think we'd need a special page with what used to be in create.html.erb, as redirecting to sign in is going to confuse people, I think. Something like completed.html.erb. Maybe completed is the wrong word. waiting.html.erb maybe. What do you think?

nickhammond commented 2 years ago

@mikker I have what used to be on the create.html.erb page as a flash now so the user still sees it if they have flash configured which is pretty standard. Since the message is a locale(passwordless.sessions.create.email_sent_if_record_found) this can easily be overridden at the app level too.

I suppose not everyone is going to need a full post request magic link flow but right now you can easily override create.html.erb which is what I was thinking might need to be catered to.

What about just another default redirect route?

# Default redirection paths
Passwordless.success_redirect_path = '/' # When a user succeeds in logging in.
Passwordless.failure_redirect_path = '/' # When a a login is failed for any reason.
Passwordless.sign_out_redirect_path = '/' # When a user logs out.
Passwordless.session_creation_redirect_path = '/' # When a user requests a magic link

sign-in-example

Which makes your setup paths:

  1. Do nothing and by default get the redirect to request a magic link with the flash showing
  2. Specify a redirect path where the user can expand on whatever additional information they need to explain
vhazbun commented 1 year ago

@mikker do you think this will be merged soon?

mikker commented 1 year ago

Sure, thank you for this.

henrikbjorn commented 1 year ago

@mikker This is a breaking change, and "broke" my application. Why can't we support the previous logic with turbo?

E.g someone could just create a create.turbo_stream.erb template if they wanted to replace the form inline, or just have it target a specific id via data: { turbo_frame: 'asdasd' }

henrikbjorn commented 1 year ago

Also turbo can just be turned off for the form.

mikker commented 1 year ago

Apologies, I didn't realize.

I think actually your suggestion to just disable Turbo on our forms is the best and easiest approach. I think the slight downgrade in app speed is acceptable as what the user sees before and after signing is probably not very alike anyway.

ecomba commented 1 year ago

I just saw this after bundle upgrading.

As @henrikbjorn commented, it broke my app flow and I decided to revert to the previous version as a flash message will just not do it for me.

henrikbjorn commented 1 year ago

@mikker Since we have an easy way of specifying the controller. We could maybe create some helper methods in the SessionController so that it is easier overwrite the standard create behavior ?

henrikbjorn commented 1 year ago

@ecomba

You can do something like this:

# frozen_string_literal: true

class SessionsController < Passwordless::SessionsController
  layout 'passwordless'

  unauthenticated

  def create
    @resource = find_authenticatable

    @session = build_passwordless_session(@resource)

    Passwordless.config.after_session_save.call(@session, request) if @session.save

    # Not sure this is needed..
    render 'passwordless/sessions/create'
  end
end

Just use the new controller option for the route helper.

henrikbjorn commented 1 year ago

Then just remember to disable turbo on the form, since it requires a redirect...