diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 96a2066c414..d8cc43eb254 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -33,6 +33,7 @@ //= require ./discourse/models/permission-type //= require ./discourse/models/user-action-group //= require ./discourse/models/category +//= require ./discourse/models/input-validation //= require ./discourse/lib/ajax-error //= require ./discourse/lib/search //= require ./discourse/lib/user-search diff --git a/app/assets/javascripts/discourse/controllers/create-account.js.es6 b/app/assets/javascripts/discourse/controllers/create-account.js.es6 index 50bd0188021..3f0d180c292 100644 --- a/app/assets/javascripts/discourse/controllers/create-account.js.es6 +++ b/app/assets/javascripts/discourse/controllers/create-account.js.es6 @@ -5,8 +5,9 @@ import { setting } from 'discourse/lib/computed'; import { on } from 'ember-addons/ember-computed-decorators'; import { emailValid } from 'discourse/lib/utilities'; import InputValidation from 'discourse/models/input-validation'; +import PasswordValidation from "discourse/mixins/password-validation"; -export default Ember.Controller.extend(ModalFunctionality, { +export default Ember.Controller.extend(ModalFunctionality, PasswordValidation, { login: Ember.inject.controller(), uniqueUsernameValidation: null, @@ -16,7 +17,6 @@ export default Ember.Controller.extend(ModalFunctionality, { accountChallenge: 0, formSubmitted: false, rejectedEmails: Em.A([]), - rejectedPasswords: Em.A([]), prefilledUsername: null, userFields: null, isDeveloper: false, @@ -85,10 +85,6 @@ export default Ember.Controller.extend(ModalFunctionality, { }); }.property(), - passwordInstructions: function() { - return this.get('isDeveloper') ? I18n.t('user.password.instructions', {count: Discourse.SiteSettings.min_admin_password_length}) : I18n.t('user.password.instructions', {count: Discourse.SiteSettings.min_password_length}); - }.property('isDeveloper'), - nameInstructions: function() { return I18n.t(Discourse.SiteSettings.full_name_required ? 'user.name.instructions_required' : 'user.name.instructions'); }.property(), @@ -293,55 +289,6 @@ export default Ember.Controller.extend(ModalFunctionality, { return( this.get('globalNicknameExists') || false ); }, - // Validate the password - passwordValidation: function() { - if (!this.get('passwordRequired')) { - return InputValidation.create({ ok: true }); - } - - // If blank, fail without a reason - const password = this.get("accountPassword"); - if (Ember.isEmpty(this.get('accountPassword'))) { - return InputValidation.create({ failed: true }); - } - - // If too short - const passwordLength = this.get('isDeveloper') ? Discourse.SiteSettings.min_admin_password_length : Discourse.SiteSettings.min_password_length; - if (password.length < passwordLength) { - return InputValidation.create({ - failed: true, - reason: I18n.t('user.password.too_short') - }); - } - - if (this.get('rejectedPasswords').includes(password)) { - return InputValidation.create({ - failed: true, - reason: I18n.t('user.password.common') - }); - } - - if (!Ember.isEmpty(this.get('accountUsername')) && this.get('accountPassword') === this.get('accountUsername')) { - return InputValidation.create({ - failed: true, - reason: I18n.t('user.password.same_as_username') - }); - } - - if (!Ember.isEmpty(this.get('accountEmail')) && this.get('accountPassword') === this.get('accountEmail')) { - return InputValidation.create({ - failed: true, - reason: I18n.t('user.password.same_as_email') - }); - } - - // Looks good! - return InputValidation.create({ - ok: true, - reason: I18n.t('user.password.ok') - }); - }.property('accountPassword', 'rejectedPasswords.[]', 'accountUsername', 'accountEmail', 'isDeveloper'), - @on('init') fetchConfirmationValue() { return ajax('/users/hp.json').then(json => { diff --git a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 new file mode 100644 index 00000000000..7efa5db185c --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 @@ -0,0 +1,53 @@ +import { default as computed } from 'ember-addons/ember-computed-decorators'; +import getUrl from 'discourse-common/lib/get-url'; +import { ajax } from 'discourse/lib/ajax'; +import PasswordValidation from "discourse/mixins/password-validation"; + +export default Ember.Controller.extend(PasswordValidation, { + isDeveloper: Ember.computed.alias('model.is_developer'), + passwordRequired: true, + errorMessage: null, + successMessage: null, + requiresApproval: false, + + @computed() + continueButtonText() { + return I18n.t('password_reset.continue', {site_name: this.siteSettings.title}); + }, + + lockImageUrl: getUrl('/images/lock.svg'), + + actions: { + submit() { + ajax({ + url: `/users/password-reset/${this.get('model.token')}.json`, + type: 'PUT', + data: { + password: this.get('accountPassword') + } + }).then(result => { + if (result.success) { + this.set('successMessage', result.message); + this.set('redirectTo', result.redirect_to); + if (result.requires_approval) { + this.set('requiresApproval', true); + } + } else { + if (result.errors && result.errors.password && result.errors.password.length > 0) { + this.get('rejectedPasswords').pushObject(this.get('accountPassword')); + this.get('rejectedPasswordsMessages').set(this.get('accountPassword'), result.errors.password[0]); + } + if (result.message) { + this.set('errorMessage', result.message); + } + } + }).catch(response => { + throw response; + }); + }, + + done() { + window.location.pathname = this.get('redirectTo') || Discourse.getURL("/"); + } + } +}); diff --git a/app/assets/javascripts/discourse/mixins/password-validation.js.es6 b/app/assets/javascripts/discourse/mixins/password-validation.js.es6 new file mode 100644 index 00000000000..7e9caf62dcc --- /dev/null +++ b/app/assets/javascripts/discourse/mixins/password-validation.js.es6 @@ -0,0 +1,71 @@ +import InputValidation from 'discourse/models/input-validation'; +import { default as computed } from 'ember-addons/ember-computed-decorators'; + +export default Ember.Mixin.create({ + + rejectedPasswords: null, + + init() { + this._super(); + this.set('rejectedPasswords', []); + this.set('rejectedPasswordsMessages', Ember.Map.create()); + }, + + @computed('passwordMinLength') + passwordInstructions() { + return I18n.t('user.password.instructions', {count: this.get('passwordMinLength')}); + }, + + @computed('isDeveloper') + passwordMinLength() { + return this.get('isDeveloper') ? this.siteSettings.min_admin_password_length : this.siteSettings.min_password_length; + }, + + @computed('accountPassword', 'passwordRequired', 'rejectedPasswords.[]', 'accountUsername', 'accountEmail', 'isDeveloper') + passwordValidation(password, passwordRequired, rejectedPasswords, accountUsername, accountEmail, isDeveloper) { + if (!passwordRequired) { + return InputValidation.create({ ok: true }); + } + + if (rejectedPasswords.includes(password)) { + return InputValidation.create({ + failed: true, + reason: this.get('rejectedPasswordsMessages').get(password) || I18n.t('user.password.common') + }); + } + + // If blank, fail without a reason + if (Ember.isEmpty(password)) { + return InputValidation.create({ failed: true }); + } + + // If too short + const passwordLength = isDeveloper ? this.siteSettings.min_admin_password_length : this.siteSettings.min_password_length; + if (password.length < passwordLength) { + return InputValidation.create({ + failed: true, + reason: I18n.t('user.password.too_short') + }); + } + + if (!Ember.isEmpty(accountUsername) && password === accountUsername) { + return InputValidation.create({ + failed: true, + reason: I18n.t('user.password.same_as_username') + }); + } + + if (!Ember.isEmpty(accountEmail) && password === accountEmail) { + return InputValidation.create({ + failed: true, + reason: I18n.t('user.password.same_as_email') + }); + } + + // Looks good! + return InputValidation.create({ + ok: true, + reason: I18n.t('user.password.ok') + }); + } +}); diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 index 7dc4502cf75..6ea5b9bd5bd 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -63,6 +63,7 @@ export default function() { // User routes this.route('users', { resetNamespace: true }); + this.route('password-reset', { path: '/users/password-reset/:token' }); this.route('user', { path: '/users/:username', resetNamespace: true }, function() { this.route('summary'); this.route('userActivity', { path: '/activity', resetNamespace: true }, function() { diff --git a/app/assets/javascripts/discourse/routes/password-reset.js.es6 b/app/assets/javascripts/discourse/routes/password-reset.js.es6 new file mode 100644 index 00000000000..1f4cf2102c7 --- /dev/null +++ b/app/assets/javascripts/discourse/routes/password-reset.js.es6 @@ -0,0 +1,21 @@ +import PreloadStore from 'preload-store'; +import { ajax } from 'discourse/lib/ajax'; + +export default Discourse.Route.extend({ + titleToken() { + return I18n.t('login.reset_password'); + }, + + model(params) { + if (PreloadStore.get("password_reset")) { + return PreloadStore.getAndRemove("password_reset").then(json => _.merge(params, json)); + } + }, + + afterModel(model) { + // confirm token here so email clients who crawl URLs don't invalidate the link + if (model) { + return ajax({ url: `/users/confirm-email-token/${model.token}.json`, dataType: 'json' }); + } + } +}); diff --git a/app/assets/javascripts/discourse/templates/password-reset.hbs b/app/assets/javascripts/discourse/templates/password-reset.hbs new file mode 100644 index 00000000000..76d79e404c1 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/password-reset.hbs @@ -0,0 +1,40 @@ +
+
+ +
+ +
+ {{#if successMessage}} +

{{successMessage}}

+ + {{#if requiresApproval}} +

{{i18n 'login.not_approved'}}

+ {{else}} + {{continueButtonText}} + {{/if}} + {{else}} +
+ +

{{i18n 'user.change_password.choose'}}

+ +
+ {{password-field value=accountPassword type="password" id="new-account-password" capsLockOn=capsLockOn}} +  {{input-tip validation=passwordValidation}} +
+ +
+ +
{{i18n 'login.caps_lock_warning'}}
+
+ + + + {{#if errorMessage}} +

+
{{errorMessage}}
+ {{/if}} + +
+ {{/if}} +
+
diff --git a/app/assets/stylesheets/common/base/login.scss b/app/assets/stylesheets/common/base/login.scss index 7024567baec..e8eb255b2a4 100644 --- a/app/assets/stylesheets/common/base/login.scss +++ b/app/assets/stylesheets/common/base/login.scss @@ -64,6 +64,15 @@ $input-width: 220px; } } +.password-reset { + .instructions { + label { + color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); + } + } +} + + // alternate login / create new account buttons should be de-emphasized button#login-link, button#new-account-link diff --git a/app/assets/stylesheets/desktop/login.scss b/app/assets/stylesheets/desktop/login.scss index 69be797a69f..0e349367ca5 100644 --- a/app/assets/stylesheets/desktop/login.scss +++ b/app/assets/stylesheets/desktop/login.scss @@ -43,12 +43,6 @@ } } - tr.instructions { - label { - color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); - } - } - .tos-agree { margin-bottom: 12px; } @@ -57,4 +51,24 @@ margin-top: 15px; } + .instructions { + label { + color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); + } + } + +} + +.password-reset { + .col-form { + padding-top: 40px; + padding-left: 20px; + } + h2 { + margin-bottom: 12px; + } + .password-reset-img { + width: 200px; + height: 200px; + } } diff --git a/app/assets/stylesheets/mobile/login.scss b/app/assets/stylesheets/mobile/login.scss index 9359b1c47f8..aaf5cee0dff 100644 --- a/app/assets/stylesheets/mobile/login.scss +++ b/app/assets/stylesheets/mobile/login.scss @@ -84,3 +84,32 @@ $input-width: 184px; } } } + + +.password-reset { + margin-top: 30px; + .col-image { + padding-top: 12px; + } + .password-reset-img { + width: 50px; + height: 50px; + } + .col-form { + padding-left: 8px; + } + h2 { + margin-bottom: 12px; + } + .tip { + display: block; + margin: 6px 0; + max-width: 180px; + } +} + +.discourse-touch .password-reset { + .instructions { + margin-bottom: 16px; + } +} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a61541aa1ab..4c06f112777 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -405,8 +405,6 @@ class UsersController < ApplicationController user_id = secure_session["password-#{token}"].to_i @user = User.find(user_id) if user_id > 0 end - else - @invalid_token = true end if !@user @@ -424,12 +422,42 @@ class UsersController < ApplicationController Invite.invalidate_for_email(@user.email) # invite link can't be used to log in anymore secure_session["password-#{token}"] = nil logon_after_password_reset - - return redirect_to(wizard_path) if Wizard.user_requires_completion?(@user) end end end - render layout: 'no_ember' + + respond_to do |format| + format.html do + if @error + render layout: 'no_ember' + else + store_preloaded("password_reset", MultiJson.dump({ is_developer: UsernameCheckerService.is_developer?(@user.email) })) + end + return redirect_to(wizard_path) if Wizard.user_requires_completion?(@user) + end + + format.json do + if request.put? + if @error || @user&.errors&.any? + render json: { + success: false, + message: @error, + errors: @user&.errors&.to_hash, + is_developer: UsernameCheckerService.is_developer?(@user.email) + } + else + render json: { + success: true, + message: @success, + requires_approval: !Guardian.new(@user).can_access_forum?, + redirect_to: Wizard.user_requires_completion?(@user) ? wizard_path : nil + } + end + else + render json: {is_developer: UsernameCheckerService.is_developer?(@user.email)} + end + end + end end def confirm_email_token diff --git a/app/views/layouts/no_ember.html.erb b/app/views/layouts/no_ember.html.erb index 70ea347560f..2c6b3f74beb 100644 --- a/app/views/layouts/no_ember.html.erb +++ b/app/views/layouts/no_ember.html.erb @@ -2,7 +2,7 @@ - <%=SiteSetting.title%> + <%= content_for?(:title) ? yield(:title) + ' - ' + SiteSetting.title : SiteSetting.title %> <%= render partial: "layouts/head" %> <%= render partial: "common/special_font_face" %> diff --git a/app/views/users/password_reset.html.erb b/app/views/users/password_reset.html.erb index 61a61feb645..e64d9d81976 100644 --- a/app/views/users/password_reset.html.erb +++ b/app/views/users/password_reset.html.erb @@ -11,59 +11,18 @@ <% end %> <% end %> - - <%if @success%> -

- <%= @success %> - <%- if @requires_approval %> - <%= t 'login.not_approved' %> - <% else %> -
-
- "><%= t('password_reset.continue', site_name: SiteSetting.title) %> - <% end %> -

- <% else %> - <%if @user.present? %> -

- <% if @user.has_password? %> - <%= t 'password_reset.choose_new' %> - <% else %> - <%= t 'password_reset.choose' %> - <% end %> -

- - <%=form_tag({}, method: :put) do %> -

- - - -

- -

- <%=submit_tag( @user.has_password? ? t('password_reset.update') : t('password_reset.save'), class: 'btn')%> -

- <%end%> - <%end%> - <%end%> +<% content_for :title do %><%=t "password_reset.title" %><% end %> + <%- content_for(:no_ember_head) do %> <%= script "ember_jquery" %> <%= render_google_universal_analytics_code %> <%- end %> - +<%- content_for(:head) do %> + +<%- end %> <%= render_google_analytics_code %> diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6729aea20c4..e878e41d66b 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -655,6 +655,8 @@ en: error: "(error)" action: "Send Password Reset Email" set_password: "Set Password" + choose_new: "Choose a new password" + choose: "Choose a password" change_about: title: "Change About Me" @@ -1041,6 +1043,7 @@ en: to_continue: "Please Log In" preferences: "You need to be logged in to change your user preferences." forgot: "I don't recall my account details" + not_approved: "Your account hasn't been approved yet. You will be notified by email when you are ready to log in." google: title: "with Google" message: "Authenticating with Google (make sure pop up blockers are not enabled)" @@ -1063,6 +1066,9 @@ en: title: "with GitHub" message: "Authenticating with GitHub (make sure pop up blockers are not enabled)" + password_reset: + continue: "Continue to %{site_name}" + emoji_set: apple_international: "Apple/International" google: "Google" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index eb2e54eed1d..b416e043a2c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -580,7 +580,6 @@ en: title: 'Reset Password' success: "You successfully changed your password and are now logged in." success_unapproved: "You successfully changed your password." - continue: "Continue to %{site_name}" change_email: confirmed: "Your email has been updated." diff --git a/public/images/lock.svg b/public/images/lock.svg new file mode 100644 index 00000000000..05de608a7d1 --- /dev/null +++ b/public/images/lock.svg @@ -0,0 +1 @@ +image/svg+xml diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 5d4f5cb80bf..354f9d55f79 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -218,8 +218,8 @@ describe UsersController do it 'disallows login' do expect(assigns[:error]).to be_present expect(session[:current_user_id]).to be_blank - expect(assigns[:invalid_token]).to eq(nil) expect(response).to be_success + expect(response).to render_template(layout: 'no_ember') end end @@ -231,8 +231,8 @@ describe UsersController do it 'disallows login' do expect(assigns[:error]).to be_present expect(session[:current_user_id]).to be_blank - expect(assigns[:invalid_token]).to eq(true) expect(response).to be_success + expect(response).to render_template(layout: 'no_ember') end end diff --git a/test/javascripts/acceptance/password-reset-test.js.es6 b/test/javascripts/acceptance/password-reset-test.js.es6 new file mode 100644 index 00000000000..7b2b95dc824 --- /dev/null +++ b/test/javascripts/acceptance/password-reset-test.js.es6 @@ -0,0 +1,64 @@ +import { acceptance } from "helpers/qunit-helpers"; +import PreloadStore from 'preload-store'; +import { parsePostData } from "helpers/create-pretender"; + +acceptance("Password Reset", { + setup() { + const response = (object) => { + return [ + 200, + {"Content-Type": "application/json"}, + object + ]; + }; + + server.get('/users/confirm-email-token/myvalidtoken.json', () => { //eslint-disable-line + return response({success: "OK"}); + }); + + server.put('/users/password-reset/myvalidtoken.json', request => { //eslint-disable-line + const body = parsePostData(request.requestBody); + if (body.password === "jonesyAlienSlayer") { + return response({success: false, errors: {password: ["is the name of your cat"]}}); + } else { + return response({success: "OK", message: I18n.t('password_reset.success')}); + } + }); + } +}); + +test("Password Reset Page", () => { + PreloadStore.store('password_reset', {is_developer: false}); + + visit("/users/password-reset/myvalidtoken"); + andThen(() => { + ok(exists(".password-reset input"), "shows the input"); + ok(find('.password-reset .instructions').html().trim().includes(`${Discourse.SiteSettings.min_password_length} char`), "shows correct min length"); + }); + + fillIn('.password-reset input', 'perf3ctly5ecur3'); + andThen(() => { + ok(exists(".password-reset .tip.good"), "input looks good"); + }); + + fillIn('.password-reset input', '123'); + andThen(() => { + ok(exists(".password-reset .tip.bad"), "input is not valid"); + ok(find(".password-reset .tip.bad").html().trim().includes(I18n.t('user.password.too_short')), "password too short"); + }); + + fillIn('.password-reset input', 'jonesyAlienSlayer'); + click('.password-reset form button'); + andThen(() => { + ok(exists(".password-reset .tip.bad"), "input is not valid"); + ok(find(".password-reset .tip.bad").html().trim().includes("is the name of your cat"), "server validation error message shows"); + }); + + fillIn('.password-reset input', 'perf3ctly5ecur3'); + click('.password-reset form button'); + andThen(() => { + ok(!exists(".password-reset form"), "form is gone"); + ok(exists(".password-reset .btn"), "button is shown"); + }); +}); + diff --git a/test/javascripts/controllers/create-account-test.js.es6 b/test/javascripts/controllers/create-account-test.js.es6 index 91156d4e529..11a9045a136 100644 --- a/test/javascripts/controllers/create-account-test.js.es6 +++ b/test/javascripts/controllers/create-account-test.js.es6 @@ -11,7 +11,7 @@ test('basicUsernameValidation', function() { var subject = this.subject; var testInvalidUsername = function(username, expectedReason) { - var controller = subject(); + var controller = subject({ siteSettings: Discourse.SiteSettings }); controller.set('accountUsername', username); equal(controller.get('basicUsernameValidation.failed'), true, 'username should be invalid: ' + username); equal(controller.get('basicUsernameValidation.reason'), expectedReason, 'username validation reason: ' + username + ', ' + expectedReason); @@ -21,7 +21,7 @@ test('basicUsernameValidation', function() { testInvalidUsername('x', I18n.t('user.username.too_short')); testInvalidUsername('123456789012345678901', I18n.t('user.username.too_long')); - var controller = subject(); + var controller = subject({ siteSettings: Discourse.SiteSettings }); controller.set('accountUsername', 'porkchops'); controller.set('prefilledUsername', 'porkchops'); equal(controller.get('basicUsernameValidation.ok'), true, 'Prefilled username is valid'); @@ -31,7 +31,7 @@ test('basicUsernameValidation', function() { test('passwordValidation', function() { var subject = this.subject; - var controller = subject(); + var controller = subject({ siteSettings: Discourse.SiteSettings }); controller.set('passwordRequired', true); controller.set('accountEmail', 'pork@chops.com'); controller.set('accountUsername', 'porkchops'); @@ -42,7 +42,7 @@ test('passwordValidation', function() { equal(controller.get('passwordValidation.reason'), I18n.t('user.password.ok'), 'Password is valid'); var testInvalidPassword = function(password, expectedReason) { - var c = subject(); + var c = subject({ siteSettings: Discourse.SiteSettings }); c.set('accountPassword', password); equal(c.get('passwordValidation.failed'), true, 'password should be invalid: ' + password); equal(c.get('passwordValidation.reason'), expectedReason, 'password validation reason: ' + password + ', ' + expectedReason); diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index 9609685dfb5..92e37105b00 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -1,7 +1,7 @@ import storePretender from 'helpers/store-pretender'; import fixturePretender from 'helpers/fixture-pretender'; -function parsePostData(query) { +export function parsePostData(query) { const result = {}; query.split("&").forEach(function(part) { const item = part.split("="); @@ -18,7 +18,7 @@ function parsePostData(query) { }); return result; -} +}; function response(code, obj) { if (typeof code === "object") {