mikker / passwordless

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

get sign_in - Redirects users to root page if already signed in #46

Closed ghost closed 5 years ago

ghost commented 5 years ago

Description

When the user is signed in, still users will be able to go to /users/sign_in and create a new session. Since /users/sign_in is controlled by passwordless gem this is essential.

Types of changes

mikker commented 5 years ago

Hi @imadityang. I'm not sure why this is a problem? Is this something other auth libs do too? Devise/Clearance/whatever?

ghost commented 5 years ago

@mikker, Yes, devise does this. Since passwordless is managing the sign_in and sign_out for users, this feature should come out of the box.

Steps to reproduce:

  1. User goes to /users/sign_in -> login with a@example.com -> magic link is sent -> user clicks on magic link and thus authenticated.
  2. BUT, if the user(still authenticated) then tries to go to /users/sign_in -> he is again shown the login page, and he can log in same as described in point 1.

If this gem doesn't provide this functionality out of the box, then the developers should create a new controller -> Inherit from sessions controller -> override an existing method, which is kind of overhead.

I think the problem is in code coverage. I didn't write the test for this feature since I'm only familiar with rspec library, so I'm leaving the test part for you to implement. Also, before pushing the code, I ran the rake test, and there were no failing tests.

mikker commented 5 years ago

I get what you're describing, but I'm not sure: Why is it a problem that users can visit that endpoint?

Is it confusing to the user? Is there a security issue? Is it just your preference? Not saying we'll never do this, just trying to understand 😄

mikker commented 5 years ago

Closing this as there hasn't been any action. Thanks for contributing this @ghost – feel free to come back if you still feel we should do this 👋

drewda commented 5 years ago

This functionality would be very useful for our project. (We have some users who bookmark /users/sign_in and return to it everytime they visit the site, even if they've already authenticated.)

@mikker would you be able to share an example of how we can override the session controller new action to get different behavior when there is already a current_user?

(In any case, thanks for a very useful project.)

mikker commented 5 years ago

That's a valid argument for actually implementing this feature.

For now, Ruby being Ruby, I guess you can just patch the controller in an initializer or similar, some time after the gem has been loaded been loaded, like:

module Passwordless
  class SessionsController < ApplicationController
    def new
      if current_user # or whatever your user-checking method is called
        redirect_to somerwhere_else_path
        return
      end

      @email_field = email_field
      @session = Session.new
    end
  end
end

(Untested)

drewda commented 5 years ago

Thanks for such a prompt response, @mikker !

This code sample looks great and squares with what I was thinking as well. I'm just having trouble getting it to load at the right time. Neither an initializer or the /lib folder seem to be the right place to do this. If you have any suggestions, let me know.

drewda commented 5 years ago

Figured it out. This is what I added to an initializer:

Rails.application.config.to_prepare do
  Passwordless::SessionsController.class_eval do
    def new
      if current_user
        redirect_to '/'
      else
        @email_field = email_field
        @session = Passwordless::Session.new
      end
    end
  end
end