From c18b85873f321451303b249d9f6328ff7d780904 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 11 Feb 2013 11:18:26 -0500 Subject: [PATCH] Prevent login until email is confirmed --- app/controllers/session_controller.rb | 11 +- app/models/user.rb | 4 + config/locales/en.yml | 1 + spec/controllers/session_controller_spec.rb | 114 +++++++++++--------- spec/models/user_spec.rb | 26 +++++ 5 files changed, 104 insertions(+), 52 deletions(-) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 6aef4abba2a..b8f4c7ff972 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -22,9 +22,14 @@ class SessionController < ApplicationController # If their password is correct if @user.confirm_password?(params[:password]) - log_on_user(@user) - render_serialized(@user, UserSerializer) - return + if @user.email_confirmed? + log_on_user(@user) + render_serialized(@user, UserSerializer) + return + else + render :json => {error: I18n.t("login.not_activated")} + return + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 2f5593f1877..41dac47d71c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -380,6 +380,10 @@ class User < ActiveRecord::Base end end + def email_confirmed? + email_tokens.where(email: self.email, confirmed: true).present? + end + protected diff --git a/config/locales/en.yml b/config/locales/en.yml index 26dae763371..114e3241cee 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1039,6 +1039,7 @@ en: wait_approval: "Thanks for signing up. We will notify you when your account has been approved." active: "Your account is active and ready." activate_email: "You're almost done! We sent an activation email to %{email}. Please follow the instructions in the email to activate your account." + not_activated: "You can't log in yet. We sent an activation email to you. Please follow the instructions in the email to activate your account." errors: "Failed to create account: %{errors}" not_available: "Not available. Try %{suggestion}?" diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb index 45f312e1e52..59be7306925 100644 --- a/spec/controllers/session_controller_spec.rb +++ b/spec/controllers/session_controller_spec.rb @@ -6,77 +6,93 @@ describe SessionController do let(:user) { Fabricate(:user) } - it "raises an error when the login isn't present" do - lambda { xhr :post, :create }.should raise_error(Discourse::InvalidParameters) - end - - describe 'invalid password' do - - it "should return an error with an invalid password" do - xhr :post, :create, login: user.username, password: 'sssss' - ::JSON.parse(response.body)['error'].should be_present - end - - end - - describe 'success by username' do + context 'when email is confirmed' do before do - xhr :post, :create, login: user.username, password: 'myawesomepassword' - user.reload + token = user.email_tokens.where(email: user.email).first + EmailToken.confirm(token.token) end - it 'sets a session id' do - session[:current_user_id].should == user.id + it "raises an error when the login isn't present" do + lambda { xhr :post, :create }.should raise_error(Discourse::InvalidParameters) end - it 'gives the user an auth token' do - user.auth_token.should be_present + describe 'invalid password' do + it "should return an error with an invalid password" do + xhr :post, :create, login: user.username, password: 'sssss' + ::JSON.parse(response.body)['error'].should be_present + end end - it 'sets a cookie with the auth token' do - cookies[:_t].should == user.auth_token - end - end + describe 'success by username' do + before do + xhr :post, :create, login: user.username, password: 'myawesomepassword' + user.reload + end - describe 'strips leading @ symbol' do - before do - xhr :post, :create, login: "@" + user.username, password: 'myawesomepassword' - user.reload + it 'sets a session id' do + session[:current_user_id].should == user.id + end + + it 'gives the user an auth token' do + user.auth_token.should be_present + end + + it 'sets a cookie with the auth token' do + cookies[:_t].should == user.auth_token + end end - it 'sets a session id' do - session[:current_user_id].should == user.id - end - end + describe 'strips leading @ symbol' do + before do + xhr :post, :create, login: "@" + user.username, password: 'myawesomepassword' + user.reload + end - describe 'also allow login by email' do - before do - xhr :post, :create, login: user.email, password: 'myawesomepassword' + it 'sets a session id' do + session[:current_user_id].should == user.id + end end - it 'sets a session id' do - session[:current_user_id].should == user.id - end - end - - describe "when the site requires approval of users" do - before do - SiteSetting.expects(:must_approve_users?).returns(true) - end - - context 'with an unapproved user' do + describe 'also allow login by email' do before do xhr :post, :create, login: user.email, password: 'myawesomepassword' end - it "doesn't log in the user" do - session[:current_user_id].should be_blank + it 'sets a session id' do + session[:current_user_id].should == user.id end - end + describe "when the site requires approval of users" do + before do + SiteSetting.expects(:must_approve_users?).returns(true) + end + + context 'with an unapproved user' do + before do + xhr :post, :create, login: user.email, password: 'myawesomepassword' + end + + it "doesn't log in the user" do + session[:current_user_id].should be_blank + end + end + end end + context 'when email has not been confirmed' do + before do + xhr :post, :create, login: user.email, password: 'myawesomepassword' + end + + it "doesn't log in the user" do + session[:current_user_id].should be_blank + end + + it 'returns an error message' do + ::JSON.parse(response.body)['error'].should be_present + end + end end describe '.destroy' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9f04c9e0217..3e518ce38ef 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -617,4 +617,30 @@ describe User do it { should_not be_active } end + describe 'email_confirmed?' do + let(:user) { Fabricate(:user) } + + context 'when email has not been confirmed yet' do + it 'should return false' do + user.email_confirmed?.should be_false + end + end + + context 'when email has been confirmed' do + it 'should return true' do + token = user.email_tokens.where(email: user.email).first + EmailToken.confirm(token.token) + user.email_confirmed?.should be_true + end + end + + context 'when user has no email tokens for some reason' do + it 'should return false' do + user.email_tokens.each {|t| t.destroy} + user.reload + user.email_confirmed?.should be_false + end + end + end + end