From 40ab2e56677625ee2d97ea7cce1607c521f38fbb Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 5 Apr 2017 16:14:22 -0400 Subject: [PATCH] FEATURE: Let users update their emails before confirming This allows users who entered a typo or invalid email address when signing up an opportunity to fix it and resending the confirmation email to that address. --- app/assets/javascripts/application.js | 2 +- .../controllers/activation-edit.js.es6 | 36 +++++++++ .../controllers/basic-modal-body.js.es6 | 5 ++ .../controllers/not-activated.js.es6 | 18 +++-- .../discourse/lib/show-modal.js.es6 | 24 +++--- .../mixins/modal-functionality.js.es6 | 12 +++ .../components/modal-footer-close.hbs | 3 + .../templates/modal/activation-edit.hbs | 12 +++ .../templates/modal/activation-resent.hbs | 5 ++ .../templates/modal/not-activated.hbs | 16 ++-- app/assets/stylesheets/common/base/modal.scss | 8 ++ app/controllers/users_controller.rb | 23 ++++++ config/locales/client.en.yml | 6 ++ config/locales/server.en.yml | 2 +- config/routes.rb | 1 + spec/controllers/users_controller_spec.rb | 73 +++++++++++++++++++ spec/fabricators/user_fabricator.rb | 1 + .../acceptance/sign-in-test.js.es6 | 30 +++++++- .../helpers/create-pretender.js.es6 | 8 ++ 19 files changed, 255 insertions(+), 30 deletions(-) create mode 100644 app/assets/javascripts/discourse/controllers/activation-edit.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/basic-modal-body.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/modal-footer-close.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/activation-edit.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/activation-resent.hbs diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index ce7addad876..29c8e300b10 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -25,6 +25,7 @@ //= require ./discourse/lib/computed //= require ./discourse/lib/formatter //= require ./discourse/lib/eyeline +//= require ./discourse/lib/show-modal //= require ./discourse/mixins/scrolling //= require ./discourse/models/model //= require ./discourse/models/rest @@ -68,7 +69,6 @@ //= require ./discourse/lib/emoji/groups //= require ./discourse/lib/emoji/toolbar //= require ./discourse/components/d-editor -//= require ./discourse/lib/show-modal //= require ./discourse/lib/screen-track //= require ./discourse/routes/discourse //= require ./discourse/routes/build-topic-route diff --git a/app/assets/javascripts/discourse/controllers/activation-edit.js.es6 b/app/assets/javascripts/discourse/controllers/activation-edit.js.es6 new file mode 100644 index 00000000000..ddd8a2a5620 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/activation-edit.js.es6 @@ -0,0 +1,36 @@ +import computed from 'ember-addons/ember-computed-decorators'; +import ModalFunctionality from 'discourse/mixins/modal-functionality'; +import { ajax } from 'discourse/lib/ajax'; +import { extractError } from 'discourse/lib/ajax-error'; +import { userPath } from 'discourse/lib/url'; + +export default Ember.Controller.extend(ModalFunctionality, { + login: Ember.inject.controller(), + + currentEmail: null, + newEmail: null, + password: null, + + @computed('newEmail', 'currentEmail') + submitDisabled(newEmail, currentEmail) { + return newEmail === currentEmail; + }, + + actions: { + changeEmail() { + const login = this.get('login'); + + ajax(userPath('update-activation-email'), { + data: { + username: login.get('loginName'), + password: login.get('loginPassword'), + email: this.get('newEmail') + }, + type: 'PUT' + }).then(() => { + const modal = this.showModal('activation-resent', {title: 'log_in'}); + modal.set('currentEmail', this.get('newEmail')); + }).catch(err => this.flash(extractError(err), 'error')); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/basic-modal-body.js.es6 b/app/assets/javascripts/discourse/controllers/basic-modal-body.js.es6 new file mode 100644 index 00000000000..f7001555a9d --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/basic-modal-body.js.es6 @@ -0,0 +1,5 @@ +import ModalFunctionality from 'discourse/mixins/modal-functionality'; + +export default Ember.Controller.extend(ModalFunctionality, { + modal: null +}); diff --git a/app/assets/javascripts/discourse/controllers/not-activated.js.es6 b/app/assets/javascripts/discourse/controllers/not-activated.js.es6 index cf0164b0e6a..976e1311bd9 100644 --- a/app/assets/javascripts/discourse/controllers/not-activated.js.es6 +++ b/app/assets/javascripts/discourse/controllers/not-activated.js.es6 @@ -4,21 +4,23 @@ import ModalFunctionality from 'discourse/mixins/modal-functionality'; import { userPath } from 'discourse/lib/url'; export default Ember.Controller.extend(ModalFunctionality, { - emailSent: false, - - onShow() { - this.set("emailSent", false); - }, - actions: { sendActivationEmail() { ajax(userPath('action/send_activation_email'), { data: { username: this.get('username') }, type: 'POST' }).then(() => { - this.set('emailSent', true); + const modal = this.showModal('activation-resent', {title: 'log_in'}); + modal.set('currentEmail', this.get('currentEmail')); }).catch(popupAjaxError); + }, + + editActivationEmail() { + const modal = this.showModal('activation-edit', {title: 'login.change_email'}); + + const currentEmail = this.get('currentEmail'); + modal.set('currentEmail', currentEmail); + modal.set('newEmail', currentEmail); } } - }); diff --git a/app/assets/javascripts/discourse/lib/show-modal.js.es6 b/app/assets/javascripts/discourse/lib/show-modal.js.es6 index ed2457e065f..739fdd17b32 100644 --- a/app/assets/javascripts/discourse/lib/show-modal.js.es6 +++ b/app/assets/javascripts/discourse/lib/show-modal.js.es6 @@ -11,17 +11,23 @@ export default function(name, opts) { const controllerName = opts.admin ? `modals/${name}` : name; - const controller = container.lookup('controller:' + controllerName); + let controller = container.lookup('controller:' + controllerName); const templateName = opts.templateName || Ember.String.dasherize(name); const renderArgs = { into: 'modal', outlet: 'modalBody'}; - if (controller) { renderArgs.controller = controllerName; } + if (controller) { + renderArgs.controller = controllerName; + } else { + // use a basic controller + renderArgs.controller = 'basic-modal-body'; + controller = container.lookup(`controller:${renderArgs.controller}`); + } + if (opts.addModalBodyView) { renderArgs.view = 'modal-body'; } - const modalName = `modal/${templateName}`; const fullName = opts.admin ? `admin/templates/${modalName}` : modalName; route.render(fullName, renderArgs); @@ -29,13 +35,11 @@ export default function(name, opts) { modalController.set('title', I18n.t(opts.title)); } - if (controller) { - controller.set('modal', modalController); - const model = opts.model; - if (model) { controller.set('model', model); } - if (controller.onShow) { controller.onShow(); } - controller.set('flashMessage', null); - } + controller.set('modal', modalController); + const model = opts.model; + if (model) { controller.set('model', model); } + if (controller.onShow) { controller.onShow(); } + controller.set('flashMessage', null); return controller; }; diff --git a/app/assets/javascripts/discourse/mixins/modal-functionality.js.es6 b/app/assets/javascripts/discourse/mixins/modal-functionality.js.es6 index 23bc790e153..d78478dae63 100644 --- a/app/assets/javascripts/discourse/mixins/modal-functionality.js.es6 +++ b/app/assets/javascripts/discourse/mixins/modal-functionality.js.es6 @@ -1,5 +1,17 @@ +import showModal from 'discourse/lib/show-modal'; + export default Ember.Mixin.create({ flash(text, messageClass) { this.appEvents.trigger('modal-body:flash', { text, messageClass }); + }, + + showModal(...args) { + return showModal(...args); + }, + + actions: { + closeModal() { + this.get('modal').send('closeModal'); + } } }); diff --git a/app/assets/javascripts/discourse/templates/components/modal-footer-close.hbs b/app/assets/javascripts/discourse/templates/components/modal-footer-close.hbs new file mode 100644 index 00000000000..a1e59ab16b5 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/modal-footer-close.hbs @@ -0,0 +1,3 @@ +
+ +
diff --git a/app/assets/javascripts/discourse/templates/modal/activation-edit.hbs b/app/assets/javascripts/discourse/templates/modal/activation-edit.hbs new file mode 100644 index 00000000000..735f4912a7e --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/activation-edit.hbs @@ -0,0 +1,12 @@ +{{#d-modal-body}} +

{{i18n "login.provide_new_email"}}

+ {{input value=newEmail class="activate-new-email"}} +{{/d-modal-body}} + +
+ {{d-button action="changeEmail" + label="login.submit_new_email" + disabled=submitDisabled + class="btn-primary"}} + {{d-button action="closeModal" label="close"}} +
diff --git a/app/assets/javascripts/discourse/templates/modal/activation-resent.hbs b/app/assets/javascripts/discourse/templates/modal/activation-resent.hbs new file mode 100644 index 00000000000..e694c838bfa --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/activation-resent.hbs @@ -0,0 +1,5 @@ +{{#d-modal-body}} + {{{i18n 'login.sent_activation_email_again' currentEmail=currentEmail}}} +{{/d-modal-body}} + +{{modal-footer-close closeModal=(action "closeModal")}} diff --git a/app/assets/javascripts/discourse/templates/modal/not-activated.hbs b/app/assets/javascripts/discourse/templates/modal/not-activated.hbs index f8e4aeee297..4ed586628bd 100644 --- a/app/assets/javascripts/discourse/templates/modal/not-activated.hbs +++ b/app/assets/javascripts/discourse/templates/modal/not-activated.hbs @@ -1,12 +1,14 @@ {{#d-modal-body}} - {{#if emailSent}} - {{{i18n 'login.sent_activation_email_again' currentEmail=currentEmail}}} - {{else}} - {{{i18n 'login.not_activated' sentTo=sentTo}}} - {{i18n 'login.resend_activation_email'}} - {{/if}} + {{{i18n 'login.not_activated' sentTo=sentTo}}} {{/d-modal-body}}
- + {{d-button action="sendActivationEmail" + label="login.resend_title" + icon="envelope" + class="btn-primary resend"}} + {{d-button action="editActivationEmail" + label="login.change_email" + icon="pencil" + class="edit-email"}}
diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index 8fd7eff32af..c69d4a40583 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -376,3 +376,11 @@ } } } + +.modal-button-bar { + margin-top: 1em; + + button { + margin-right: 0.5em; + } +} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 7ca6f0d3801..c548d232a21 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -26,6 +26,7 @@ class UsersController < ApplicationController :activate_account, :perform_account_activation, :send_activation_email, + :update_activation_email, :password_reset, :confirm_email_token, :admin_login, @@ -569,6 +570,28 @@ class UsersController < ApplicationController render layout: 'no_ember' end + def update_activation_email + RateLimiter.new(nil, "activate-edit-email-hr-#{request.remote_ip}", 5, 1.hour).performed! + + @user = User.find_by_username_or_email(params[:username]) + raise Discourse::InvalidAccess.new unless @user.present? + raise Discourse::InvalidAccess.new if @user.active? + raise Discourse::InvalidAccess.new if current_user.present? + + raise Discourse::InvalidAccess.new unless @user.confirm_password?(params[:password]) + + User.transaction do + @user.email = params[:email] + if @user.save + @user.email_tokens.create(email: @user.email) + enqueue_activation_email + render json: success_json + else + render_json_error(@user) + end + end + end + def send_activation_email if current_user.blank? || !current_user.staff? RateLimiter.new(nil, "activate-hr-#{request.remote_ip}", 30, 1.hour).performed! diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 29b0d2a1799..f3db3d5853a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1040,6 +1040,12 @@ en: not_allowed_from_ip_address: "You can't login from that IP address." admin_not_allowed_from_ip_address: "You can't log in as admin from that IP address." resend_activation_email: "Click here to send the activation email again." + + resend_title: "Resend Activation Email" + change_email: "Change Email Address" + provide_new_email: "Provide a new address and we'll resend your confirmation email." + submit_new_email: "Update Email Address" + sent_activation_email_again: "We sent another activation email to you at {{currentEmail}}. It might take a few minutes for it to arrive; be sure to check your spam folder." to_continue: "Please Log In" preferences: "You need to be logged in to change your user preferences." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e952de369d0..4fccc2ace2d 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1655,7 +1655,7 @@ en: incorrect_username_email_or_password: "Incorrect username, email or password" wait_approval: "Thanks for signing up. We will notify you when your account has been approved." active: "Your account is activated and ready to use." - activate_email: "

You're almost done! We sent an activation mail to %{email}. Please follow the instructions in the email to activate your account.

If it doesn't arrive, check your spam folder, or try to log in again to send another activation mail.

" + activate_email: "

You're almost done! We sent an activation mail to %{email}. Please follow the instructions in the email to activate your account.

If it doesn't arrive, check your spam folder, or try to log in again to send another activation mail or to input a new email address.

" 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." not_allowed_from_ip_address: "You can't log in as %{username} from that IP address." admin_not_allowed_from_ip_address: "You can't log in as admin from that IP address." diff --git a/config/routes.rb b/config/routes.rb index 8456bbb50b6..b8091c79397 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -308,6 +308,7 @@ Discourse::Application.routes.draw do end end + put "#{root_path}/update-activation-email" => "users#update_activation_email" get "#{root_path}/hp" => "users#get_honeypot_value" get "#{root_path}/admin-login" => "users#admin_login" put "#{root_path}/admin-login" => "users#admin_login" diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 5d6ed231056..3b2f5f0d5a7 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1872,4 +1872,77 @@ describe UsersController do end + + describe '.update_activation_email' do + + it "raises an error with an invalid username" do + xhr :put, :update_activation_email, { + username: 'eviltrout', + password: 'invalid-password', + email: 'updatedemail@example.com' + } + expect(response).to_not be_success + end + + it "raises an error with an invalid password" do + xhr :put, :update_activation_email, { + username: Fabricate(:inactive_user).username, + password: 'invalid-password', + email: 'updatedemail@example.com' + } + expect(response).to_not be_success + end + + it "raises an error for an active user" do + xhr :put, :update_activation_email, { + username: Fabricate(:walter_white).username, + password: 'letscook', + email: 'updatedemail@example.com' + } + expect(response).to_not be_success + end + + it "raises an error when logged in" do + log_in(:moderator) + + xhr :put, :update_activation_email, { + username: Fabricate(:inactive_user).username, + password: 'qwerqwer123', + email: 'updatedemail@example.com' + } + expect(response).to_not be_success + end + + it "raises an error when the new email is taken" do + user = Fabricate(:user) + + xhr :put, :update_activation_email, { + username: Fabricate(:inactive_user).username, + password: 'qwerqwer123', + email: user.email + } + expect(response).to_not be_success + end + + it "can be updated" do + user = Fabricate(:inactive_user) + token = user.email_tokens.first + + xhr :put, :update_activation_email, { + username: user.username, + password: 'qwerqwer123', + email: 'updatedemail@example.com' + } + + expect(response).to be_success + + user.reload + expect(user.email).to eq('updatedemail@example.com') + expect(user.email_tokens.where(email: 'updatedemail@example.com', expired: false)).to be_present + + token.reload + expect(token.expired?).to eq(true) + end + end + end diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 5cad8011f9f..a635af32896 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -36,6 +36,7 @@ Fabricator(:inactive_user, from: :user) do name 'Inactive User' username 'inactive_user' email 'inactive@idontexist.com' + password 'qwerqwer123' active false end diff --git a/test/javascripts/acceptance/sign-in-test.js.es6 b/test/javascripts/acceptance/sign-in-test.js.es6 index d8efb4be181..21403770ba6 100644 --- a/test/javascripts/acceptance/sign-in-test.js.es6 +++ b/test/javascripts/acceptance/sign-in-test.js.es6 @@ -41,16 +41,40 @@ test("sign in - not activated", () => { ok(!exists('.modal-body small'), 'it escapes the email address'); }); - click('.modal-body .resend-link'); + click('.modal-footer button.resend'); andThen(() => { equal(find('.modal-body b').text(), 'current@example.com'); ok(!exists('.modal-body small'), 'it escapes the email address'); }); - - }); }); +test("sign in - not activated - edit email", () => { + visit("/"); + andThen(() => { + click("header .login-button"); + andThen(() => { + ok(exists('.login-modal'), "it shows the login modal"); + }); + + fillIn('#login-account-name', 'eviltrout'); + fillIn('#login-account-password', 'not-activated-edit'); + click('.modal-footer .btn-primary'); + click('.modal-footer button.edit-email'); + andThen(() => { + equal(find('.activate-new-email').val(), 'current@example.com'); + equal(find('.modal-footer .btn-primary:disabled').length, 1, "must change email"); + }); + fillIn('.activate-new-email', 'different@example.com'); + andThen(() => { + equal(find('.modal-footer .btn-primary:disabled').length, 0); + }); + click(".modal-footer .btn-primary"); + andThen(() => { + equal(find('.modal-body b').text(), 'different@example.com'); + }); + }); +}); test("create account", () => { visit("/"); diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index a704196dce4..8ae140d9224 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -195,10 +195,18 @@ export default function() { current_email: 'current@example.com' }); } + if (data.password === 'not-activated-edit') { + return response({ error: "not active", + reason: "not_activated", + sent_to_email: 'eviltrout@example.com', + current_email: 'current@example.com' }); + } + return response(400, {error: 'invalid login'}); }); this.post('/u/action/send_activation_email', success); + this.put('/u/update-activation-email', success); this.get('/u/hp.json', function() { return response({"value":"32faff1b1ef1ac3","challenge":"61a3de0ccf086fb9604b76e884d75801"});