From 03b3e57a44da228bca7296bd752e26447956e1d6 Mon Sep 17 00:00:00 2001 From: Erick Guan <fantasticfears@gmail.com> Date: Thu, 20 Apr 2017 17:17:24 +0200 Subject: [PATCH] FEATURE: login by a link from email Co-authored-by: tgxworld <tgx@discourse.org> --- .../controllers/forgot-password.js.es6 | 80 +++++----- .../templates/modal/forgot-password.hbs | 14 +- app/controllers/session_controller.rb | 85 ++++++++--- app/controllers/users_controller.rb | 38 ++++- app/mailers/user_notifications.rb | 71 ++++++--- app/views/session/email_login.html.erb | 17 +++ config/locales/client.en.yml | 10 ++ config/locales/server.en.yml | 17 +++ config/routes.rb | 2 + config/site_settings.yml | 5 +- ...enable_local_logins_via_email_validator.rb | 14 ++ ...e_local_logins_via_email_validator_spec.rb | 47 ++++++ spec/controllers/session_controller_spec.rb | 27 ++-- spec/mailers/user_notifications_spec.rb | 22 +++ spec/requests/session_controller_spec.rb | 141 ++++++++++++++++++ spec/requests/users_controller_spec.rb | 61 ++++++++ .../acceptance/forgot-password-test.js.es6 | 90 +++++++++++ 17 files changed, 640 insertions(+), 101 deletions(-) create mode 100644 app/views/session/email_login.html.erb create mode 100644 lib/validators/enable_local_logins_via_email_validator.rb create mode 100644 spec/components/validators/enable_local_logins_via_email_validator_spec.rb create mode 100644 spec/requests/session_controller_spec.rb create mode 100644 test/javascripts/acceptance/forgot-password-test.js.es6 diff --git a/app/assets/javascripts/discourse/controllers/forgot-password.js.es6 b/app/assets/javascripts/discourse/controllers/forgot-password.js.es6 index 4311368186c..7e385fb3022 100644 --- a/app/assets/javascripts/discourse/controllers/forgot-password.js.es6 +++ b/app/assets/javascripts/discourse/controllers/forgot-password.js.es6 @@ -20,48 +20,54 @@ export default Ember.Controller.extend(ModalFunctionality, { }, actions: { - submit() { - if (this.get('submitDisabled')) return false; - - this.set('disabled', true); - - ajax('/session/forgot_password', { - data: { login: this.get('accountEmailOrUsername').trim() }, - type: 'POST' - }).then(data => { - const escaped = escapeExpression(this.get('accountEmailOrUsername')); - const isEmail = this.get('accountEmailOrUsername').match(/@/); - let key = 'forgot_password.complete_' + (isEmail ? 'email' : 'username'); - let extraClass; - - if (data.user_found === true) { - key += '_found'; - this.set('accountEmailOrUsername', ''); - this.set('offerHelp', I18n.t(key, {email: escaped, username: escaped})); - } else { - if (data.user_found === false) { - key += '_not_found'; - extraClass = 'error'; - } - - this.flash(I18n.t(key, {email: escaped, username: escaped}), extraClass); - } - }).catch(e => { - this.flash(extractError(e), 'error'); - }).finally(() => { - setTimeout(() => this.set('disabled', false), 1000); - }); - - return false; - }, - ok() { this.send('closeModal'); }, help() { this.setProperties({ offerHelp: I18n.t('forgot_password.help'), helpSeen: true }); - } - } + }, + resetPassword() { + return this._submit('/session/forgot_password', 'forgot_password.complete'); + }, + + emailLogin() { + return this._submit('/u/email-login', 'email_login.complete'); + } + }, + + _submit(route, translationKey) { + if (this.get('submitDisabled')) return false; + this.set('disabled', true); + + ajax(route, { + data: { login: this.get('accountEmailOrUsername').trim() }, + type: 'POST' + }).then(data => { + const escaped = escapeExpression(this.get('accountEmailOrUsername')); + const isEmail = this.get('accountEmailOrUsername').match(/@/); + let key = `${translationKey}_${isEmail ? 'email' : 'username'}`; + let extraClass; + + if (data.user_found === true) { + key += '_found'; + this.set('accountEmailOrUsername', ''); + this.set('offerHelp', I18n.t(key, { email: escaped, username: escaped })); + } else { + if (data.user_found === false) { + key += '_not_found'; + extraClass = 'error'; + } + + this.flash(I18n.t(key, { email: escaped, username: escaped }), extraClass); + } + }).catch(e => { + this.flash(extractError(e), 'error'); + }).finally(() => { + this.set('disabled', false); + }); + + return false; + }, }); diff --git a/app/assets/javascripts/discourse/templates/modal/forgot-password.hbs b/app/assets/javascripts/discourse/templates/modal/forgot-password.hbs index c739f771979..8b42f88739d 100644 --- a/app/assets/javascripts/discourse/templates/modal/forgot-password.hbs +++ b/app/assets/javascripts/discourse/templates/modal/forgot-password.hbs @@ -9,10 +9,16 @@ {{/d-modal-body}} <div class="modal-footer"> {{#unless offerHelp}} - {{d-button action="submit" - label="forgot_password.reset" - disabled=submitDisabled - class="btn-primary"}} + {{d-button action="resetPassword" + label="forgot_password.reset" + disabled=submitDisabled + class="btn-primary"}} + {{#if siteSettings.enable_local_logins_via_email}} + {{d-button action="emailLogin" + label="email_login.label" + disabled=submitDisabled + class="email-login"}} + {{/if}} {{else}} {{d-button class="btn-large btn-primary" label="forgot_password.button_ok" diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index e96566fd25c..079ba724efb 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -7,9 +7,10 @@ class SessionController < ApplicationController render body: nil, status: 500 end - before_action :check_local_login_allowed, only: %i(create forgot_password) + before_action :check_local_login_allowed, only: %i(create forgot_password email_login) + before_action :rate_limit_login, only: %i(create email_login) skip_before_action :redirect_to_login_if_required - skip_before_action :preload_json, :check_xhr, only: ['sso', 'sso_login', 'become', 'sso_provider', 'destroy'] + skip_before_action :preload_json, :check_xhr, only: %i(sso sso_login become sso_provider destroy email_login) ACTIVATE_USER_KEY = "activate_user" @@ -187,9 +188,6 @@ class SessionController < ApplicationController end def create - RateLimiter.new(nil, "login-hr-#{request.remote_ip}", SiteSetting.max_logins_per_ip_per_hour, 1.hour).performed! - RateLimiter.new(nil, "login-min-#{request.remote_ip}", SiteSetting.max_logins_per_ip_per_minute, 1.minute).performed! - params.require(:login) params.require(:password) @@ -208,7 +206,7 @@ class SessionController < ApplicationController # If the site requires user approval and the user is not approved yet if login_not_approved_for?(user) - login_not_approved + render json: login_not_approved return end @@ -220,20 +218,31 @@ class SessionController < ApplicationController return end - if user.suspended? - failed_to_login(user) - return + if payload = login_error_check(user) + render json: payload + else + (user.active && user.email_confirmed?) ? login(user) : not_activated(user) end + end - if ScreenedIpAddress.should_block?(request.remote_ip) - return not_allowed_from_ip_address(user) + def email_login + raise Discourse::NotFound if !SiteSetting.enable_local_logins_via_email + + if EmailToken.valid_token_format?(params[:token]) && (user = EmailToken.confirm(params[:token])) + if login_not_approved_for?(user) + @error = login_not_approved[:error] + return render layout: 'no_ember' + elsif payload = login_error_check(user) + @error = payload[:error] + return render layout: 'no_ember' + else + log_on_user(user) + redirect_to path("/") + end + else + @error = I18n.t('email_login.invalid_token') + return render layout: 'no_ember' end - - if ScreenedIpAddress.block_admin_login?(user, request.remote_ip) - return admin_not_allowed_from_ip_address(user) - end - - (user.active && user.email_confirmed?) ? login(user) : not_activated(user) end def forgot_password @@ -291,6 +300,18 @@ class SessionController < ApplicationController private + def login_error_check(user) + return failed_to_login(user) if user.suspended? + + if ScreenedIpAddress.should_block?(request.remote_ip) + return not_allowed_from_ip_address(user) + end + + if ScreenedIpAddress.block_admin_login?(user, request.remote_ip) + return admin_not_allowed_from_ip_address(user) + end + end + def login_not_approved_for?(user) SiteSetting.must_approve_users? && !user.approved? && !user.admin? end @@ -300,7 +321,7 @@ class SessionController < ApplicationController end def login_not_approved - render json: { error: I18n.t("login.not_approved") } + { error: I18n.t("login.not_approved") } end def not_activated(user) @@ -314,19 +335,21 @@ class SessionController < ApplicationController end def not_allowed_from_ip_address(user) - render json: { error: I18n.t("login.not_allowed_from_ip_address", username: user.username) } + { error: I18n.t("login.not_allowed_from_ip_address", username: user.username) } end def admin_not_allowed_from_ip_address(user) - render json: { error: I18n.t("login.admin_not_allowed_from_ip_address", username: user.username) } + { error: I18n.t("login.admin_not_allowed_from_ip_address", username: user.username) } end def failed_to_login(user) message = user.suspend_reason ? "login.suspended_with_reason" : "login.suspended" - render json: { - error: I18n.t(message, date: I18n.l(user.suspended_till, format: :date_only), - reason: Rack::Utils.escape_html(user.suspend_reason)), + { + error: I18n.t(message, + date: I18n.l(user.suspended_till, format: :date_only), + reason: Rack::Utils.escape_html(user.suspend_reason) + ), reason: 'suspended' } end @@ -342,6 +365,22 @@ class SessionController < ApplicationController end end + def rate_limit_login + RateLimiter.new( + nil, + "login-hr-#{request.remote_ip}", + SiteSetting.max_logins_per_ip_per_hour, + 1.hour + ).performed! + + RateLimiter.new( + nil, + "login-min-#{request.remote_ip}", + SiteSetting.max_logins_per_ip_per_minute, + 1.minute + ).performed! + end + def render_sso_error(status:, text:) @sso_error = text render status: status, layout: 'no_ember' diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b26f1a5312d..613215df520 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -18,7 +18,7 @@ class UsersController < ApplicationController skip_before_action :check_xhr, only: [ :show, :badges, :password_reset, :update, :account_created, :activate_account, :perform_account_activation, :user_preferences_redirect, :avatar, - :my_redirect, :toggle_anon, :admin_login, :confirm_admin + :my_redirect, :toggle_anon, :admin_login, :confirm_admin, :email_login ] before_action :respond_to_suspicious_request, only: [:create] @@ -37,6 +37,7 @@ class UsersController < ApplicationController :update_activation_email, :password_reset, :confirm_email_token, + :email_login, :admin_login, :confirm_admin] @@ -563,6 +564,7 @@ class UsersController < ApplicationController elsif params[:token].present? if EmailToken.valid_token_format?(params[:token]) @user = EmailToken.confirm(params[:token]) + if @user&.admin? log_on_user(@user) return redirect_to path("/") @@ -580,6 +582,40 @@ class UsersController < ApplicationController render layout: false end + def email_login + raise Discourse::NotFound if !SiteSetting.enable_local_logins_via_email + return redirect_to path("/") if current_user + + expires_now + params.require(:login) + + RateLimiter.new(nil, "email-login-hour-#{request.remote_ip}", 6, 1.hour).performed! + RateLimiter.new(nil, "email-login-min-#{request.remote_ip}", 3, 1.minute).performed! + user = User.human_users.find_by_username_or_email(params[:login]) + user_presence = user.present? && !user.staged + + if user + RateLimiter.new(nil, "email-login-hour-#{user.id}", 6, 1.hour).performed! + RateLimiter.new(nil, "email-login-min-#{user.id}", 3, 1.minute).performed! + + if user_presence + email_token = user.email_tokens.create!(email: user.email) + + Jobs.enqueue(:critical_user_email, + type: :email_login, + user_id: user.id, + email_token: email_token.token + ) + end + end + + json = { result: "ok" } + json[:user_found] = user_presence unless SiteSetting.hide_email_address_taken + render json: json + rescue RateLimiter::LimitExceeded + render_json_error(I18n.t("rate_limiter.slow_down")) + end + def toggle_anon user = AnonymousShadowCreator.get_master(current_user) || AnonymousShadowCreator.get(current_user) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 17260a5d8e1..f5d8e537ecb 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -12,10 +12,11 @@ class UserNotifications < ActionMailer::Base include Email::BuildEmailHelper def signup(user, opts = {}) - build_email(user.email, - template: "user_notifications.signup", - locale: user_locale(user), - email_token: opts[:email_token]) + build_user_email_token_by_template( + "user_notifications.signup", + user, + opts[:email_token] + ) end def signup_after_approval(user, opts = {}) @@ -33,38 +34,51 @@ class UserNotifications < ActionMailer::Base end def confirm_old_email(user, opts = {}) - build_email(user.email, - template: "user_notifications.confirm_old_email", - locale: user_locale(user), - email_token: opts[:email_token]) + build_user_email_token_by_template( + "user_notifications.confirm_old_email", + user, + opts[:email_token] + ) end def confirm_new_email(user, opts = {}) - build_email(user.email, - template: "user_notifications.confirm_new_email", - locale: user_locale(user), - email_token: opts[:email_token]) + build_user_email_token_by_template( + "user_notifications.confirm_new_email", + user, + opts[:email_token] + ) end def forgot_password(user, opts = {}) - build_email(user.email, - template: user.has_password? ? "user_notifications.forgot_password" : "user_notifications.set_password", - locale: user_locale(user), - email_token: opts[:email_token]) + build_user_email_token_by_template( + user.has_password? ? "user_notifications.forgot_password" : "user_notifications.set_password", + user, + opts[:email_token] + ) + end + + def email_login(user, opts = {}) + build_user_email_token_by_template( + "user_notifications.email_login", + user, + opts[:email_token] + ) end def admin_login(user, opts = {}) - build_email(user.email, - template: "user_notifications.admin_login", - locale: user_locale(user), - email_token: opts[:email_token]) + build_user_email_token_by_template( + "user_notifications.admin_login", + user, + opts[:email_token] + ) end def account_created(user, opts = {}) - build_email(user.email, - template: "user_notifications.account_created", - locale: user_locale(user), - email_token: opts[:email_token]) + build_user_email_token_by_template( + "user_notifications.account_created", + user, + opts[:email_token] + ) end def account_silenced(user, opts = nil) @@ -532,6 +546,15 @@ class UserNotifications < ActionMailer::Base private + def build_user_email_token_by_template(template, user, email_token) + build_email( + user.email, + template: template, + locale: user_locale(user), + email_token: email_token + ) + end + def build_summary_for(user) @site_name = SiteSetting.email_prefix.presence || SiteSetting.title # used by I18n @user = user diff --git a/app/views/session/email_login.html.erb b/app/views/session/email_login.html.erb new file mode 100644 index 00000000000..7bd6cf03fd9 --- /dev/null +++ b/app/views/session/email_login.html.erb @@ -0,0 +1,17 @@ +<%if @error%> + <div class='alert alert-error'> + <%= @error %> + </div> +<%end%> + +<% content_for :title do %><%=t "email_login.title" %><% end %> + +<%- content_for(:no_ember_head) do %> + <meta name="referrer" content="no-referrer"> + <%= preload_script "ember_jquery" %> + <%= render_google_universal_analytics_code %> +<%- end %> + +<%- content_for(:head) do %> + <meta name="referrer" content="no-referrer"> +<%- end %> diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 46f59e97716..49d0be51dc3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1083,6 +1083,16 @@ en: help: "Email not arriving? Be sure to check your spam folder first.<p>Not sure which email address you used? Enter an email address and we’ll let you know if it exists here.</p><p>If you no longer have access to the email address on your account, please contact <a href='/about'>our helpful staff.</a></p>" button_ok: "OK" button_help: "Help" + + email_login: + label: "Login With Email" + complete_username: "If an account matches the username <b>%{username}</b>, you should receive an email with a magic login link shortly." + complete_email: "If an account matches <b>%{email}</b>, you should receive an email with a magic login link shortly." + complete_username_found: "We found an account that matches the username <b>%{username}</b>, you should receive an email with a magic login link shortly." + complete_email_found: "We found an account that matches <b>%{email}</b>, you should receive an email with a magic login link shortly." + complete_username_not_found: "No account matches the username <b>%{username}</b>" + complete_email_not_found: "No account matches <b>%{email}</b>" + login: title: "Log In" username: "User" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4e2b817c0f8..d5ef33a877f 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -648,6 +648,10 @@ en: success: "You successfully changed your password and are now logged in." success_unapproved: "You successfully changed your password." + email_login: + invalid_token: "Sorry, that email login link is too old. Select the Log In button and use 'I forgot my password' to get a new link." + title: "Email login" + change_email: confirmed: "Your email has been updated." please_continue: "Continue to %{site_name}" @@ -1149,6 +1153,7 @@ en: sso_allows_all_return_paths: "Do not restrict the domain for return_paths provided by SSO (by default return path must be on current site)" enable_local_logins: "Enable local username and password login based accounts. (Note: this must be enabled for invites to work)" + enable_local_logins_via_email: "Email user logins via email." allow_new_registrations: "Allow new user registrations. Uncheck this to prevent anyone from creating a new account." enable_signup_cta: "Show a notice to returning anonymous users prompting them to sign up for an account." enable_yahoo_logins: "Enable Yahoo authentication" @@ -1640,6 +1645,7 @@ en: staged_users_disabled: "You must first enable 'staged users' before enabling this setting." reply_by_email_disabled: "You must first enable 'reply by email' before enabling this setting." sso_url_is_empty: "You must set a 'sso url' before enabling this setting." + enable_local_logins_disabled: "You must first enable 'enable local logins' before enabling this setting." search: within_post: "#%{post_number} by %{username}" @@ -2772,6 +2778,17 @@ en: Click the following link to choose a new password: %{base_url}/u/password-reset/%{email_token} + email_login: + title: "Email login link" + subject_template: "[%{email_prefix}] Email login link" + text_body_template: | + Somebody asked to login your account on [%{site_name}](%{base_url}). + + If it was not you, you can safely ignore this email. + + Click the following link to login: + %{base_url}/session/email-login/%{email_token} + set_password: title: "Set Password" subject_template: "[%{email_prefix}] Set Password" diff --git a/config/routes.rb b/config/routes.rb index 0f51c44a175..11edecf0f9c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -301,6 +301,7 @@ Discourse::Application.routes.draw do get "session/sso_provider" => "session#sso_provider" get "session/current" => "session#current" get "session/csrf" => "session#csrf" + get "session/email-login/:token" => "session#email_login" get "composer_messages" => "composer_messages#index" post "composer/parse_html" => "composer#parse_html" @@ -330,6 +331,7 @@ Discourse::Application.routes.draw do put "#{root_path}/update-activation-email" => "users#update_activation_email" get "#{root_path}/hp" => "users#get_honeypot_value" + post "#{root_path}/email-login" => "users#email_login" get "#{root_path}/admin-login" => "users#admin_login" put "#{root_path}/admin-login" => "users#admin_login" get "#{root_path}/admin-login/:token" => "users#admin_login" diff --git a/config/site_settings.yml b/config/site_settings.yml index 64b71a2b344..55163cd323b 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -240,6 +240,10 @@ login: enable_local_logins: client: true default: true + enable_local_logins_via_email: + client: true + default: false + validator: "EnableLocalLoginsViaEmailValidator" allow_new_registrations: client: true default: true @@ -331,7 +335,6 @@ login: default: 1440 min: 1 max: 175200 - users: min_username_length: client: true diff --git a/lib/validators/enable_local_logins_via_email_validator.rb b/lib/validators/enable_local_logins_via_email_validator.rb new file mode 100644 index 00000000000..0537f3697e7 --- /dev/null +++ b/lib/validators/enable_local_logins_via_email_validator.rb @@ -0,0 +1,14 @@ +class EnableLocalLoginsViaEmailValidator + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(val) + return true if val == 'f' + SiteSetting.enable_local_logins + end + + def error_message + I18n.t('site_settings.errors.enable_local_logins_disabled') + end +end diff --git a/spec/components/validators/enable_local_logins_via_email_validator_spec.rb b/spec/components/validators/enable_local_logins_via_email_validator_spec.rb new file mode 100644 index 00000000000..289ce4911d0 --- /dev/null +++ b/spec/components/validators/enable_local_logins_via_email_validator_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe EnableLocalLoginsViaEmailValidator do + subject { described_class.new } + + describe '#valid_value?' do + describe "when 'enable_local_logins' is false" do + before do + SiteSetting.enable_local_logins = false + end + + describe 'when val is false' do + it 'should be valid' do + expect(subject.valid_value?('f')).to eq(true) + end + end + + describe 'when value is true' do + it 'should not be valid' do + expect(subject.valid_value?('t')).to eq(false) + + expect(subject.error_message).to eq(I18n.t( + 'site_settings.errors.enable_local_logins_disabled' + )) + end + end + end + + describe "when 'enable_local_logins' is true" do + before do + SiteSetting.enable_local_logins = true + end + + describe 'when val is false' do + it 'should be valid' do + expect(subject.valid_value?('f')).to eq(true) + end + end + + describe 'when value is true' do + it 'should be valid' do + expect(subject.valid_value?('t')).to eq(true) + end + end + end + end +end diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb index cc535e72223..96b58c5483d 100644 --- a/spec/controllers/session_controller_spec.rb +++ b/spec/controllers/session_controller_spec.rb @@ -8,7 +8,7 @@ describe SessionController do end end - describe 'become' do + describe '#become' do let!(:user) { Fabricate(:user) } it "does not work when not in development mode" do @@ -26,7 +26,7 @@ describe SessionController do end end - describe '.sso_login' do + describe '#sso_login' do before do @sso_url = "http://somesite.com/discourse_sso" @@ -410,7 +410,7 @@ describe SessionController do end end - describe '.sso_provider' do + describe '#sso_provider' do before do SiteSetting.enable_sso_provider = true SiteSetting.enable_sso = false @@ -470,7 +470,7 @@ describe SessionController do end end - describe '.create' do + describe '#create' do let(:user) { Fabricate(:user) } @@ -515,7 +515,9 @@ describe SessionController do login: user.username, password: 'sssss' }, format: :json - expect(::JSON.parse(response.body)['error']).to be_present + expect(::JSON.parse(response.body)['error']).to eq( + I18n.t("login.incorrect_username_email_or_password") + ) end end @@ -526,7 +528,9 @@ describe SessionController do login: user.username, password: ('s' * (User.max_password_length + 1)) }, format: :json - expect(::JSON.parse(response.body)['error']).to be_present + expect(::JSON.parse(response.body)['error']).to eq( + I18n.t("login.incorrect_username_email_or_password") + ) end end @@ -536,14 +540,15 @@ describe SessionController do user.suspended_at = Time.now user.save! StaffActionLogger.new(user).log_user_suspend(user, "<strike>banned</strike>") + post :create, params: { login: user.username, password: 'myawesomepassword' }, format: :json - error = ::JSON.parse(response.body)['error'] - expect(error).to be_present - expect(error).to match(/banned/) - expect(error).not_to match(/<strike>/) + expect(JSON.parse(response.body)['error']).to eq(I18n.t('login.suspended_with_reason', + date: I18n.l(user.suspended_till, format: :date_only), + reason: Rack::Utils.escape_html(user.suspend_reason) + )) end end @@ -881,7 +886,7 @@ describe SessionController do end end - describe '.current' do + describe '#current' do context "when not logged in" do it "retuns 404" do get :current, format: :json diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 9e3327aa927..81cca747984 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -79,6 +79,28 @@ describe UserNotifications do end + describe '.email_login' do + let(:email_token) { user.email_tokens.create!(email: user.email).token } + subject { UserNotifications.email_login(user, email_token: email_token) } + + it "generates the right email" do + expect(subject.to).to eq([user.email]) + expect(subject.from).to eq([SiteSetting.notification_email]) + + expect(subject.subject).to eq(I18n.t( + 'user_notifications.email_login.subject_template', + email_prefix: SiteSetting.title + )) + + expect(subject.body.to_s).to match(I18n.t( + 'user_notifications.email_login.text_body_template', + site_name: SiteSetting.title, + base_url: Discourse.base_url, + email_token: email_token + )) + end + end + describe '.digest' do subject { UserNotifications.digest(user) } diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb new file mode 100644 index 00000000000..951968984f2 --- /dev/null +++ b/spec/requests/session_controller_spec.rb @@ -0,0 +1,141 @@ +require 'rails_helper' + +RSpec.describe SessionController do + let(:email_token) { Fabricate(:email_token) } + let(:user) { email_token.user } + + describe '#email_login' do + before do + SiteSetting.enable_local_logins_via_email = true + end + + context 'missing token' do + it 'returns the right response' do + get "/session/email-login" + expect(response.status).to eq(404) + end + end + + context 'invalid token' do + it 'returns the right response' do + get "/session/email-login/adasdad" + + expect(response).to be_success + + expect(CGI.unescapeHTML(response.body)).to match( + I18n.t('email_login.invalid_token') + ) + end + + context 'when token has expired' do + it 'should return the right response' do + email_token.update!(created_at: 999.years.ago) + + get "/session/email-login/#{email_token.token}" + + expect(response).to be_success + + expect(CGI.unescapeHTML(response.body)).to match( + I18n.t('email_login.invalid_token') + ) + end + end + end + + context 'valid token' do + it 'returns success' do + get "/session/email-login/#{email_token.token}" + + expect(response).to redirect_to("/") + end + + it 'fails when local logins via email is disabled' do + SiteSetting.enable_local_logins_via_email = false + + get "/session/email-login/#{email_token.token}" + + expect(response.status).to eq(404) + end + + it 'fails when local logins is disabled' do + SiteSetting.enable_local_logins = false + + get "/session/email-login/#{email_token.token}" + + expect(response.status).to eq(500) + end + + it "doesn't log in the user when not approved" do + SiteSetting.must_approve_users = true + + get "/session/email-login/#{email_token.token}" + + expect(response.status).to eq(200) + + expect(CGI.unescapeHTML(response.body)).to include( + I18n.t("login.not_approved") + ) + end + + context "when admin IP address is not valid" do + before do + Fabricate(:screened_ip_address, + ip_address: "111.111.11.11", + action_type: ScreenedIpAddress.actions[:allow_admin] + ) + + SiteSetting.use_admin_ip_whitelist = true + user.update!(admin: true) + end + + it 'returns the right response' do + get "/session/email-login/#{email_token.token}" + + expect(response.status).to eq(200) + + expect(CGI.unescapeHTML(response.body)).to include( + I18n.t("login.admin_not_allowed_from_ip_address", username: user.username) + ) + end + end + + context "when IP address is blocked" do + let(:permitted_ip_address) { '111.234.23.11' } + + before do + Fabricate(:screened_ip_address, + ip_address: permitted_ip_address, + action_type: ScreenedIpAddress.actions[:block] + ) + end + + it 'returns the right response' do + ActionDispatch::Request.any_instance.stubs(:remote_ip).returns(permitted_ip_address) + + get "/session/email-login/#{email_token.token}" + + expect(response.status).to eq(200) + + expect(CGI.unescapeHTML(response.body)).to include( + I18n.t("login.not_allowed_from_ip_address", username: user.username) + ) + end + end + + it "fails when user is suspended" do + user.update!( + suspended_till: 2.days.from_now, + suspended_at: Time.zone.now + ) + + get "/session/email-login/#{email_token.token}" + + expect(response.status).to eq(200) + + expect(CGI.unescapeHTML(response.body)).to include(I18n.t("login.suspended", + date: I18n.l(user.suspended_till, format: :date_only) + )) + end + end + end +end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index d3351e59e1d..ba41c482ecd 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -340,4 +340,65 @@ RSpec.describe UsersController do expect(response).to redirect_to("/u/#{user.username_lower}/preferences") end end + + describe '#email_login' do + before do + SiteSetting.queue_jobs = true + SiteSetting.enable_local_logins_via_email = true + end + + it "enqueues the right email" do + post "/u/email-login.json", params: { login: user.email } + + expect(response).to be_success + expect(JSON.parse(response.body)['user_found']).to eq(true) + + job_args = Jobs::CriticalUserEmail.jobs.last["args"].first + + expect(job_args["user_id"]).to eq(user.id) + expect(job_args["type"]).to eq("email_login") + expect(job_args["email_token"]).to eq(user.email_tokens.last.token) + end + + describe 'when enable_local_logins_via_email is disabled' do + before do + SiteSetting.enable_local_logins_via_email = false + end + + it 'should return the right response' do + post "/u/email-login.json", params: { login: user.email } + + expect(response.status).to eq(404) + end + end + + describe 'when username or email is not valid' do + it 'should not enqueue the email to login' do + post "/u/email-login.json", params: { login: '@random' } + + expect(response).to be_success + expect(JSON.parse(response.body)['user_found']).to eq(false) + expect(Jobs::CriticalUserEmail.jobs).to eq([]) + end + end + + describe 'when hide_email_address_taken is true' do + it 'should return the right response' do + SiteSetting.hide_email_address_taken = true + post "/u/email-login.json", params: { login: user.email } + + expect(response).to be_success + expect(JSON.parse(response.body).has_key?('user_found')).to eq(false) + end + end + + describe "when user is already logged in" do + it 'should redirect to the root path' do + sign_in(user) + post "/u/email-login.json", params: { login: user.email } + + expect(response).to redirect_to("/") + end + end + end end diff --git a/test/javascripts/acceptance/forgot-password-test.js.es6 b/test/javascripts/acceptance/forgot-password-test.js.es6 new file mode 100644 index 00000000000..d3a868cb1f2 --- /dev/null +++ b/test/javascripts/acceptance/forgot-password-test.js.es6 @@ -0,0 +1,90 @@ +import { acceptance } from "helpers/qunit-helpers"; + +let userFound = false; + +acceptance("Forgot password", { + settings: { + enable_local_logins_via_email: true + }, + beforeEach() { + const response = object => { + return [ + 200, + { "Content-Type": "application/json" }, + object + ]; + }; + + server.post('/u/email-login', () => { // eslint-disable-line no-undef + return response({ "user_found": userFound }); + }); + } +}); + +QUnit.test("logging in via email", assert => { + visit("/"); + click("header .login-button"); + + andThen(() => { + assert.ok(exists('.login-modal'), "it shows the login modal"); + }); + + click('#forgot-password-link'); + + fillIn("#username-or-email", 'someuser'); + click('.email-login'); + + andThen(() => { + assert.equal( + find(".alert-error").html(), + I18n.t('email_login.complete_username_not_found', { username: 'someuser' }), + 'it should display the right error message' + ); + }); + + fillIn("#username-or-email", 'someuser@gmail.com'); + click('.email-login'); + + andThen(() => { + assert.equal( + find(".alert-error").html(), + I18n.t('email_login.complete_email_not_found', { email: 'someuser@gmail.com' }), + 'it should display the right error message' + ); + }); + + fillIn("#username-or-email", 'someuser'); + + andThen(() => { + userFound = true; + }); + + click('.email-login'); + + andThen(() => { + assert.equal( + find(".modal-body").html().trim(), + I18n.t('email_login.complete_username_found', { username: 'someuser' }), + 'it should display the right message' + ); + }); + + visit("/"); + click("header .login-button"); + + andThen(() => { + assert.ok(exists('.login-modal'), "it shows the login modal"); + }); + + click('#forgot-password-link'); + fillIn("#username-or-email", 'someuser@gmail.com'); + click('.email-login'); + + andThen(() => { + assert.equal( + find(".modal-body").html().trim(), + I18n.t('email_login.complete_email_found', { email: 'someuser@gmail.com' }), + 'it should display the right message' + ); + }); +});