NoamB / sorcery

Magical authentication for Rails 3 & 4
MIT License
2.31k stars 386 forks source link

telling users they need to activate account when they try to login #232

Closed emiltin closed 7 years ago

emiltin commented 12 years ago

after a user signs up, but haven't yet activated, an attempt to login will simply tell the user that the email/password was invalid. this is because login() seems to return nil when the activation state is pending, even though the provided email/pw is correct.

it would be nice with an easy way to send uses to a page telling them they still need to activate their account.

panSarin commented 12 years ago

+1 from me . Small feature but quite important.

kentaroi commented 12 years ago

There are several ways to implement this feature.

Just an idea, but one of the ways is this: https://github.com/kentaroi/sorcery/commit/e237ca46fe3e3e3e0c2d560d5f6df7e748d63dba

It introduces reason_for_login_failure method, and we are able to know the reason from controller. What do you think?

kentaroi commented 12 years ago

It seems you do not like the above my code..

I know this is an evil code, but...

Write the following code at the end of config/initializers/sorcery.rb file

module Sorcery::Model::Submodules::UserActivation::InstanceMethods
  def prevent_non_active_login
    config = sorcery_config
    is_activated = (Thread.current[:activation_state] = self.send(config.activation_state_attribute_name)) == 'active'
    config.prevent_non_active_users_to_login ? is_activated : true
  end
end

class NilClass
  def activation_state
    Thread.current[:activation_state]
  end
end

then you can access to activation_state even if the return value is nil. app/controllers/user_sessions_controller.rb

class UserSessionsController < ApplicationController
  def create
    respond_to do |format|
      if @user = login(params[:username],params[:password])
        format.html { redirect_back_or_to(:users, :notice => 'Login successful.') }
        format.xml { render :xml => @user, :status => :created, :location => @user }
      else
        format.html { flash.now[:alert] = "Login failed. #{'You need to activate your account' if @user.activation_state =='pending'}"; render :action => "new" }
        format.xml { render :xml => @user.errors, :status => :unprocessable_entity }
      end
    end
  end
end
emiltin commented 12 years ago

that's a bit wild

emiltin commented 12 years ago

here's what i did:

  #sessions_controller.rb
  def create
    user = login(params[:email], params[:password], params[:remember_me])
    if user
      redirect_back_or_to root_url, :notice => "Logged in"
    else
      user = User.authenticate_regardless_of_activation_state params[:email], params[:password]
      if user && user.activation_state == 'pending'
        render :pending
      else
        flash.now.alert = "Email or password was invalid"
        render :new
      end
    end
  end

  #user.rb
  def self.authenticate_regardless_of_activation_state(*credentials)
    raise ArgumentError, "at least 2 arguments required" if credentials.size < 2
    credentials[0].downcase! if @sorcery_config.downcase_username_before_authenticating
    user = find_by_credentials(credentials)

    set_encryption_attributes()

    _salt = user.send(@sorcery_config.salt_attribute_name) if user && !@sorcery_config.salt_attribute_name.nil? && !@sorcery_config.encryption_provider.nil?
    user if user && credentials_match?(user.send(@sorcery_config.crypted_password_attribute_name),credentials[1],_salt)
  end
kentaroi commented 12 years ago

Yeah. I completely agree with you. Actually, I'm also want a clean way to get activation state...

kentaroi commented 12 years ago

Thanks

emiltin commented 12 years ago

not that i find my solution to be especially clean. it would be better if it was build into sorcery

kentaroi commented 12 years ago

I think it would be nice if login method would also take a user instance as an argument.

  # config/initializers/sorcery.rb
  user.prevent_non_active_users_to_login = false

  #sessions_controller.rb
  def create
    user = User.authenticate(params[:email], params[:password])
    if user
      if user.activation_state == 'active'
        login(user, params[:remember_me])
        redirect_back_or_to root_url, :notice => "Logged in"
      else
        render :pending
      end
    else
      flash.now.alert = "Email or password was invalid"
      render :new
    end
  end
      # lib/sorcery/controller.rb
      def login(*credentials)
        if credentials[0].is_a?(user_class)
          user = credentials.shift
          credentials.unshift(nil, nil)
        else
          user = user_class.authenticate(*credentials)
        end

        if user
          return_to_url = session[:return_to_url]
          reset_session # protect from session fixation attacks
          session[:return_to_url] = return_to_url
          auto_login(user)
          after_login!(user, credentials)
          current_user
        else
          after_failed_login!(credentials)
          nil
        end
      end
eric88 commented 12 years ago

I wrote a slightly cleaner (imo) version of emiltin's "hack"

def self.authenticate_without_active_check(*credentials)
  prevent_check = self.sorcery_config.before_authenticate.delete(:prevent_non_active_login)
  user = self.authenticate(*credentials)
  self.sorcery_config.before_authenticate << prevent_check if prevent_check
  return user
end
emiltin commented 7 years ago

outdated