diff --git a/Gemfile b/Gemfile index 133d0b1e42a..01aef3667eb 100644 --- a/Gemfile +++ b/Gemfile @@ -175,6 +175,9 @@ gem 'logster' gem 'sassc', require: false +gem 'rotp' +gem 'rqrcode' + if ENV["IMPORT"] == "1" gem 'mysql2' gem 'redcarpet' diff --git a/Gemfile.lock b/Gemfile.lock index bdbc00a36db..48ca0c83c0d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -73,6 +73,7 @@ GEM uniform_notifier (~> 1.10.0) byebug (9.0.6) certified (1.0.0) + chunky_png (1.3.8) coderay (1.1.2) concurrent-ruby (1.0.5) connection_pool (2.2.1) @@ -298,6 +299,9 @@ GEM redis (~> 3.0, >= 3.0.4) request_store (1.3.2) rinku (2.0.2) + rotp (3.3.0) + rqrcode (0.10.1) + chunky_png (~> 1.0) rspec (3.6.0) rspec-core (~> 3.6.0) rspec-expectations (~> 3.6.0) @@ -479,6 +483,8 @@ DEPENDENCIES redis redis-namespace rinku + rotp + rqrcode rspec rspec-html-matchers rspec-rails diff --git a/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 b/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 index 41869d976e2..b232d5bc288 100644 --- a/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 @@ -41,6 +41,11 @@ export default Ember.Controller.extend(CanCheckEmails, { return userPath(`${username}/preferences`); }, + @computed('model.second_factor_enabled','model.can_disable_second_factor') + canDisableSecondFactor(secondFactorEnabled, canDisableSecondFactor) { + return secondFactorEnabled && canDisableSecondFactor; + }, + actions: { impersonate() { return this.get("model").impersonate(); }, @@ -63,6 +68,7 @@ export default Ember.Controller.extend(CanCheckEmails, { deleteAllPosts() { return this.get("model").deleteAllPosts(); }, anonymize() { return this.get('model').anonymize(); }, destroy() { return this.get('model').destroy(); }, + disableSecondFactor() { return this.get('model').disableSecondFactor(); }, viewActionLogs() { this.get('adminTools').showActionLogs(this, { diff --git a/app/assets/javascripts/admin/models/admin-user.js.es6 b/app/assets/javascripts/admin/models/admin-user.js.es6 index e3f1530badf..52348e4019e 100644 --- a/app/assets/javascripts/admin/models/admin-user.js.es6 +++ b/app/assets/javascripts/admin/models/admin-user.js.es6 @@ -168,6 +168,14 @@ const AdminUser = Discourse.User.extend({ }).catch(popupAjaxError); }, + disableSecondFactor() { + return ajax("/admin/users/" + this.get('id') + "/disable_second_factor", { + type: 'PUT' + }).then(() => { + this.set('second_factor_enabled', false); + }).catch(popupAjaxError); + }, + refreshBrowsers() { return ajax("/admin/users/" + this.get('id') + "/refresh_browsers", { type: 'POST' diff --git a/app/assets/javascripts/admin/templates/user-index.hbs b/app/assets/javascripts/admin/templates/user-index.hbs index 28b526da55e..11f96713650 100644 --- a/app/assets/javascripts/admin/templates/user-index.hbs +++ b/app/assets/javascripts/admin/templates/user-index.hbs @@ -156,6 +156,22 @@ {{/if}} + +
+
{{i18n 'user.second_factor.title'}}
+
+ {{#if model.second_factor_enabled}} + {{i18n "yes_value"}} + {{else}} + {{i18n "no_value"}} + {{/if}} +
+
+ {{#if canDisableSecondFactor}} + {{d-button action="disableSecondFactor" icon="unlock-alt" label="user.second_factor.disable"}} + {{/if}} +
+
{{#if userFields}} diff --git a/app/assets/javascripts/discourse/components/login-modal.js.es6 b/app/assets/javascripts/discourse/components/login-modal.js.es6 index e366392b342..c4d710966ac 100644 --- a/app/assets/javascripts/discourse/components/login-modal.js.es6 +++ b/app/assets/javascripts/discourse/components/login-modal.js.es6 @@ -11,7 +11,7 @@ export default Ember.Component.extend({ } Ember.run.schedule('afterRender', () => { - $('#login-account-password, #login-account-name').keydown(e => { + $('#login-account-password, #login-account-name, #login-second-factor').keydown(e => { if (e.keyCode === 13) { this.sendAction(); } diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 6eb266d98f2..51033172214 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/app/assets/javascripts/discourse/controllers/login.js.es6 @@ -31,6 +31,9 @@ export default Ember.Controller.extend(ModalFunctionality, { this.set('authenticate', null); this.set('loggingIn', false); this.set('loggedIn', false); + this.set('secondFactorRequired', false); + $("#credentials").show(); + $("#second-factor").hide(); }, // Determines whether at least one login button is enabled @@ -67,12 +70,19 @@ export default Ember.Controller.extend(ModalFunctionality, { this.set('loggingIn', true); ajax("/session", { - data: { login: this.get('loginName'), password: this.get('loginPassword') }, + data: { login: this.get('loginName'), password: this.get('loginPassword'), second_factor_token: this.get('loginSecondFactor') }, type: 'POST' }).then(function (result) { // Successful login if (result && result.error) { self.set('loggingIn', false); + if(result.reason === 'invalid_second_factor' && !self.get('secondFactorRequired')) { + $('#modal-alert').hide(); + self.set('secondFactorRequired', true); + $("#credentials").hide(); + $("#second-factor").show(); + return; + } if (result.reason === 'not_activated') { self.send('showNotActivated', { username: self.get('loginName'), diff --git a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 index 91c62436d6a..0c1a9e0faf5 100644 --- a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 +++ b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 @@ -8,6 +8,7 @@ import { userPath } from 'discourse/lib/url'; export default Ember.Controller.extend(PasswordValidation, { isDeveloper: Ember.computed.alias('model.is_developer'), admin: Ember.computed.alias('model.admin'), + secondFactorRequired: Ember.computed.alias('model.second_factor_required'), passwordRequired: true, errorMessage: null, successMessage: null, @@ -32,7 +33,8 @@ export default Ember.Controller.extend(PasswordValidation, { url: userPath(`password-reset/${this.get('model.token')}.json`), type: 'PUT', data: { - password: this.get('accountPassword') + password: this.get('accountPassword'), + second_factor_token: this.get('secondFactor') } }).then(result => { if (result.success) { @@ -45,7 +47,19 @@ export default Ember.Controller.extend(PasswordValidation, { DiscourseURL.redirectTo(result.redirect_to || '/'); } } else { - if (result.errors && result.errors.password && result.errors.password.length > 0) { + if (result.errors && result.errors.second_factor) { + this.setProperties({ + secondFactorRequired: true, + password: null, + errorMessage: result.message + }); + } + else if (this.get('secondFactorRequired')) { + //ok 2factor + this.set('secondFactorRequired',false); + this.set('errorMessage', null); + } + 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]); } diff --git a/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 new file mode 100644 index 00000000000..4365bdd1688 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 @@ -0,0 +1,72 @@ +import { default as computed } from 'ember-addons/ember-computed-decorators'; +import DiscourseURL from 'discourse/lib/url'; +import { userPath } from 'discourse/lib/url'; +import { popupAjaxError } from 'discourse/lib/ajax-error'; + +export default Ember.Controller.extend({ + + loading: false, + password: null, + secondFactorImage: null, + secondFactorKey: null, + showSecondFactorKey: false, + + errorMessage: null, + newUsername: null, + + @computed('secondFactorImage','secondFactorKey') + loaded(secondFactorImage, secondFactorKey) { + return secondFactorImage && secondFactorKey; + }, + + @computed('loading') + submitButtonText(loading) { + if (loading) return I18n.t('loading'); + return I18n.t('submit'); + }, + + toggleSecondFactor(enable) { + if(!this.get('second_factor_token')) { + return; + } + this.set('loading', true); + this.get('content').toggleSecondFactor(this.get('second_factor_token'), enable).then((resp) => { + if(resp.error) { + this.set('errorMessage',resp.error); + return; + } + this.set('errorMessage',null); + DiscourseURL.redirectTo(userPath(this.get('content').username.toLowerCase() + "/preferences")); + }) + .catch(popupAjaxError) + .finally(() => this.set('loading', false)); + }, + + actions: { + confirmPassword() { + if(!this.get('password')) { + return; + } + this.set('loading', true); + this.get('content').loadSecondFactorCodes(this.get('password')).then((resp) => { + if(resp.error) { + this.set('errorMessage',resp.error); + return; + } + this.set('errorMessage',null); + this.set('secondFactorKey', resp.key); + this.set('secondFactorImage', resp.qr); + }).catch(popupAjaxError) + .finally(() => this.set('loading', false)); + }, + showSecondFactorKey() { + this.set('showSecondFactorKey', true); + }, + enableSecondFactor() { + this.toggleSecondFactor(true); + }, + disableSecondFactor() { + this.toggleSecondFactor(false); + } + } +}); diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 9925d4798a0..ad06bb36e4d 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -304,6 +304,23 @@ const User = RestModel.extend({ }); }, + loadSecondFactorCodes(password) { + return ajax("/second_factor/create", { + dataType: 'json', + data: { login: this.get('username'), + password: password}, + type: 'POST' + }); + }, + + toggleSecondFactor(token, enable) { + return ajax(userPath(`${this.get('username_lower')}/preferences/second-factor`), { + dataType: 'json', + data: { token, enable }, + type: 'POST' + }); + }, + loadUserAction(id) { const stream = this.get('stream'); return ajax(`/user_actions/${id}.json`, { cache: 'false' }).then(result => { 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 72dca540a18..58f541d7365 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -110,6 +110,7 @@ export default function() { this.route('username'); this.route('email'); + this.route('second-factor'); this.route('about', { path: '/about-me' }); this.route('badgeTitle', { path: '/badge_title' }); this.route('card-badge', { path: '/card-badge' }); diff --git a/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 b/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 new file mode 100644 index 00000000000..fdc9a5fac75 --- /dev/null +++ b/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 @@ -0,0 +1,21 @@ +import RestrictedUserRoute from "discourse/routes/restricted-user"; + +export default RestrictedUserRoute.extend({ + model() { + return this.modelFor('user'); + }, + + renderTemplate() { + return this.render({ into: 'user' }); + }, + + // A bit odd, but if we leave to /preferences we need to re-render that outlet + deactivate() { + this._super(); + this.render('preferences', { into: 'user', controller: 'preferences' }); + }, + + setupController(controller, user) { + controller.setProperties({ model: user, newUsername: user.get('username') }); + } +}); diff --git a/app/assets/javascripts/discourse/routes/preferences.js.es6 b/app/assets/javascripts/discourse/routes/preferences.js.es6 index 1eb3d4b3e17..ef42132ddcf 100644 --- a/app/assets/javascripts/discourse/routes/preferences.js.es6 +++ b/app/assets/javascripts/discourse/routes/preferences.js.es6 @@ -15,6 +15,9 @@ export default RestrictedUserRoute.extend({ }, actions: { + showTwoFactorModal() { + showModal('second-factor-intro'); + }, showAvatarSelector() { showModal('avatar-selector'); diff --git a/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs b/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs new file mode 100644 index 00000000000..0535a9578cd --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs @@ -0,0 +1,15 @@ + diff --git a/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs b/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs index 0e191a6c8da..a9483afdd41 100644 --- a/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs +++ b/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs @@ -1,9 +1,9 @@ -{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword action="login"}} +{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword loginSecondFactor=loginSecondFactor action="login"}} {{#d-modal-body title="login.title" class="login-modal"}} {{login-buttons action="externalLogin"}} {{#if canLoginLocal}}
-
+
@@ -29,8 +29,9 @@
@@ -15,10 +15,10 @@
- + - {{text-field value=loginPassword type="password" id="login-account-password" maxlength="200"}}   + {{text-field value=loginPassword type="password" id="login-account-password" maxlength="200"}}  
- - + {{#second-factor-form}} + {{text-field value=loginSecondFactor id="login-second-factor" autocorrect="off" autocapitalize="off" autofocus="autofocus"}} + {{/second-factor-form}} {{/if}} {{authMessage}} @@ -44,9 +45,9 @@ {{#if canLoginLocal}} {{#if showSignupLink}} diff --git a/app/assets/javascripts/discourse/templates/modal/login.hbs b/app/assets/javascripts/discourse/templates/modal/login.hbs index 9b47c129588..0da7e8ad046 100644 --- a/app/assets/javascripts/discourse/templates/modal/login.hbs +++ b/app/assets/javascripts/discourse/templates/modal/login.hbs @@ -1,9 +1,9 @@ -{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword action="login"}} +{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword loginSecondFactor=loginSecondFactor action="login"}} {{#d-modal-body title="login.title" class="login-modal"}} {{login-buttons action="externalLogin"}} {{#if canLoginLocal}}
-
+
@@ -22,6 +22,9 @@
+ {{#second-factor-form}} + {{text-field value=loginSecondFactor id="login-second-factor" autocorrect="off" autocapitalize="off" autofocus="autofocus"}} + {{/second-factor-form}} {{/if}} {{authMessage}} diff --git a/app/assets/javascripts/discourse/templates/modal/second-factor-intro.hbs b/app/assets/javascripts/discourse/templates/modal/second-factor-intro.hbs new file mode 100644 index 00000000000..f573c45cc91 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/second-factor-intro.hbs @@ -0,0 +1,6 @@ +{{#d-modal-body title="user.second_factor.title"}} +
{{{i18n 'user.second_factor.extended_description'}}}
+{{/d-modal-body}} + +
+
diff --git a/app/assets/javascripts/discourse/templates/password-reset.hbs b/app/assets/javascripts/discourse/templates/password-reset.hbs index e999fedb1ef..01d1db9df3a 100644 --- a/app/assets/javascripts/discourse/templates/password-reset.hbs +++ b/app/assets/javascripts/discourse/templates/password-reset.hbs @@ -16,20 +16,28 @@ {{/if}} {{else}}
+ {{#if secondFactorRequired}} +

{{i18n 'login.second_factor_title'}}

+

{{i18n 'login.second_factor_description'}}

+
+ {{input value=secondFactor id="second-factor" autofocus="autofocus"}} +
+ {{d-button action="submit" class='btn-primary' label='submit'}} + {{else}} +

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

-

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

+
+ {{password-field value=accountPassword type="password" id="new-account-password" capsLockOn=capsLockOn autofocus="autofocus"}} +  {{input-tip validation=passwordValidation}} +
-
- {{password-field value=accountPassword type="password" id="new-account-password" capsLockOn=capsLockOn autofocus="autofocus"}} -  {{input-tip validation=passwordValidation}} -
+
+
+ {{d-icon "exclamation-triangle"}} {{i18n 'login.caps_lock_warning'}}
+
-
-
- {{d-icon "exclamation-triangle"}} {{i18n 'login.caps_lock_warning'}}
-
- - + {{d-button action="submit" class='btn-primary' label='user.change_password.set_password'}} + {{/if}} {{#if errorMessage}}

diff --git a/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs b/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs new file mode 100644 index 00000000000..dfafe0ed8e5 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs @@ -0,0 +1,69 @@ +
+ + +
+ +
+ {{#if model.second_factor_enabled}} +

{{i18n 'user.second_factor.disable_description'}}

+ + {{text-field value=second_factor_token id="second_factor_token" classNames="input-large" autofocus="autofocus"}} +

+ {{#if errorMessage}} + {{errorMessage}} + {{/if}} +

+ + {{else}} + {{#if loaded}} +

{{i18n 'user.second_factor.enable_description'}}

+
+ {{{ secondFactorImage }}} +

+ {{#if showSecondFactorKey}} + {{ secondFactorKey }} + {{else}} + {{i18n 'user.second_factor.show_key_description'}} + {{/if}} +

+
+
+ + {{text-field value=second_factor_token id="second_factor_token" classNames="input-large" autofocus="autofocus"}} +
+

+ {{#if errorMessage}} + {{errorMessage}} + {{/if}} +

+
+ +
+ {{else}} +
+

{{i18n 'user.second_factor.confirm_password_description'}}

+ +
+ {{text-field value=password id="password" type="password" classNames="input-xxlarge" autofocus="autofocus"}} +
+

+ {{#if errorMessage}} + {{errorMessage}} + {{/if}} +

+
+ +
+
+ + {{#if saved}}{{i18n 'saved'}}{{/if}} +
+
+ {{/if}} + {{/if}} +
+
+ + + +
diff --git a/app/assets/javascripts/discourse/templates/preferences/account.hbs b/app/assets/javascripts/discourse/templates/preferences/account.hbs index 38569dd9bdd..a37f525e9c6 100644 --- a/app/assets/javascripts/discourse/templates/preferences/account.hbs +++ b/app/assets/javascripts/discourse/templates/preferences/account.hbs @@ -14,6 +14,7 @@ {{/if}}
+ {{#if canEditName}}
@@ -66,6 +67,23 @@ {{passwordProgress}}
+
+ +
+ {{#link-to "preferences.second-factor" class="btn"}} + {{#if model.second_factor_enabled}} + {{d-icon "unlock-alt"}} + {{i18n 'user.second_factor.disable'}} + {{else}} + {{d-icon "lock"}} + {{i18n 'user.second_factor.enable'}} + {{/if}} + {{/link-to}} +
+
+ {{i18n 'user.second_factor.info_prompt'}} +
+
{{/if}}
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index d948d5f5f09..75615c23b08 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -25,7 +25,8 @@ class Admin::UsersController < Admin::AdminController :generate_api_key, :revoke_api_key, :anonymize, - :reset_bounce_score] + :reset_bounce_score, + :disable_second_factor] def index users = ::AdminUserIndexQuery.new(params).find_users @@ -340,6 +341,18 @@ class Admin::UsersController < Admin::AdminController } end + def disable_second_factor + guardian.ensure_can_disable_second_factor! @user + if @user.user_second_factor.try(:delete) + StaffActionLogger.new(current_user).log_disable_second_factor_auth(@user) + end + Jobs.enqueue( + :critical_user_email, + type: :account_second_factor_disabled, + user_id: @user.id + ) + end + def destroy user = User.find_by(id: params[:id].to_i) guardian.ensure_can_delete_user!(user) diff --git a/app/controllers/second_factor_controller.rb b/app/controllers/second_factor_controller.rb new file mode 100644 index 00000000000..ca22f0d25d5 --- /dev/null +++ b/app/controllers/second_factor_controller.rb @@ -0,0 +1,51 @@ +class SecondFactorController < ApplicationController + + 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! + if user = User.find_by_username_or_email(params[:login]) + unless user.confirm_password?(params[:password]) + return invalid_credentials + end + qrcode = RQRCode::QRCode.new(SecondFactorHelper.provisioning_uri(user)) + qrcode_svg = qrcode.as_svg( + offset: 0, + color: '000', + shape_rendering: 'crispEdges', + module_size: 4 + ) + render json: { key: user.user_second_factor.data, qr: qrcode_svg } + end + end + + def update + params.require(:token) + user = fetch_user_from_params + unless SecondFactorHelper.authenticate(user, params[:token]) + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + render json: { error: I18n.t("login.invalid_second_factor_code") } + return + end + if params[:enable] == "true" + SecondFactorHelper.create_totp(user) + user.user_second_factor.enabled = true + user.user_second_factor.save! + return render json: { result: "ok", action: "enabled" } + else + user.user_second_factor.delete + Jobs.enqueue( + :critical_user_email, + type: :account_second_factor_disabled, + user_id: user.id + ) + return render json: { result: "ok", action: "disabled" } + end + end + + private + + def invalid_credentials + render json: { error: I18n.t("login.incorrect_username_email_or_password") } + end + +end diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 8b0e0f243c0..61f93af2764 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -188,6 +188,10 @@ class SessionController < ApplicationController end def create + unless params[:second_factor_token].blank? + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + end + params.require(:login) params.require(:password) @@ -221,6 +225,12 @@ class SessionController < ApplicationController if payload = login_error_check(user) render json: payload else + + if SecondFactorHelper.totp_enabled?(user) + unless SecondFactorHelper.authenticate(user, params[:second_factor_token]) + return render json: { error: I18n.t("login.invalid_second_factor_code"), reason: "invalid_second_factor" } + end + end (user.active && user.email_confirmed?) ? login(user) : not_activated(user) end end @@ -228,6 +238,14 @@ class SessionController < ApplicationController def email_login raise Discourse::NotFound if !SiteSetting.enable_local_logins_via_email + if params[:second_factor_token].present? + @error = I18n.t("login.invalid_second_factor_code") + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + end + unless EmailToken.second_factor_valid(params[:token], params[:second_factor_token]) + @second_factor_required = true + return render layout: 'no_ember' + end if EmailToken.valid_token_format?(params[:token]) && (user = EmailToken.confirm(params[:token])) if login_not_approved_for?(user) @error = login_not_approved[:error] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0ccaa731029..4a9ccd31b00 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -470,12 +470,21 @@ class UsersController < ApplicationController end end + if @user && (!SecondFactorHelper.totp_enabled?(@user) || SecondFactorHelper.authenticate(@user, params[:second_factor_token])) + secure_session["second-factor-#{token}"] = "true" + end + @valid_second_factor = secure_session["second-factor-#{token}"] == "true" + if !@user @error = I18n.t('password_reset.no_token') elsif request.put? @invalid_password = params[:password].blank? || params[:password].length > User.max_password_length - if @invalid_password + if !@valid_second_factor + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + @user.errors.add(:second_factor, :invalid) + @error = I18n.t('login.invalid_second_factor_code') + elsif @invalid_password @user.errors.add(:password, :invalid) else @user.password = params[:password] @@ -484,6 +493,7 @@ class UsersController < ApplicationController if @user.save Invite.invalidate_for_email(@user.email) # invite link can't be used to log in anymore secure_session["password-#{token}"] = nil + secure_session["second-factor-#{token}"] = nil logon_after_password_reset end end @@ -496,7 +506,7 @@ class UsersController < ApplicationController else store_preloaded( "password_reset", - MultiJson.dump(is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin?) + MultiJson.dump(is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin?, second_factor_required: !@valid_second_factor) ) end return redirect_to(wizard_path) if request.put? && Wizard.user_requires_completion?(@user) @@ -521,7 +531,7 @@ class UsersController < ApplicationController } end else - render json: { is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin? } + render json: { is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin?, second_factor_required: !@valid_second_factor } end end end @@ -550,7 +560,7 @@ class UsersController < ApplicationController def admin_login return redirect_to(path("/")) if current_user - if request.put? + if request.put? && params[:email].present? RateLimiter.new(nil, "admin-login-hr-#{request.remote_ip}", 6, 1.hour).performed! RateLimiter.new(nil, "admin-login-min-#{request.remote_ip}", 3, 1.minute).performed! @@ -563,13 +573,20 @@ class UsersController < ApplicationController end 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("/") + if params[:second_factor_token].present? + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + end + if EmailToken.second_factor_valid(params[:token], params[:second_factor_token]) + @user = EmailToken.confirm(params[:token]) + if @user && @user.admin? + log_on_user(@user) + return redirect_to path("/") + else + @message = I18n.t("admin_login.errors.unknown_email_address") + end else - @message = I18n.t("admin_login.errors.unknown_email_address") + @second_factor_required = true + @message = I18n.t("login.second_factor_title") end else @message = I18n.t("admin_login.errors.invalid_token") diff --git a/app/controllers/users_email_controller.rb b/app/controllers/users_email_controller.rb index e408a84f2a8..45b7920a9a4 100644 --- a/app/controllers/users_email_controller.rb +++ b/app/controllers/users_email_controller.rb @@ -33,6 +33,21 @@ class UsersEmailController < ApplicationController def confirm expires_now + token = EmailToken.confirmable params[:token] + change_req = token&.user&.email_change_requests + &.where('new_email_token_id = :token_id', token_id: token.id) + &.first + if change_req.try(:change_state) == EmailChangeRequest.states[:authorizing_new] && + !EmailToken.second_factor_valid(params[:token], params[:second_factor_token]) + @update_result = :invalid_second_factor + if params[:second_factor_token].present? + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + @show_invalid_second_factor_error = true + end + render layout: 'no_ember' + return + end + updater = EmailUpdater.new @update_result = updater.confirm(params[:token]) diff --git a/app/helpers/second_factor_helper.rb b/app/helpers/second_factor_helper.rb new file mode 100644 index 00000000000..0d2feb304a1 --- /dev/null +++ b/app/helpers/second_factor_helper.rb @@ -0,0 +1,35 @@ +module SecondFactorHelper + + def self.totp(user) + self.create_totp user + ROTP::TOTP.new(user.user_second_factor.data, issuer: SiteSetting.title) + end + + def self.create_totp(user) + if !user.user_second_factor + user.user_second_factor = UserSecondFactor.create(user_id: user.id, method: "totp", data: ROTP::Base32.random_base32) + end + end + + def self.provisioning_uri(user) + self.totp(user).provisioning_uri(user.email) + end + + def self.authenticate(user, token) + totp = self.totp(user) + last_used = 0 + if user.user_second_factor.last_used + last_used = user.user_second_factor.last_used.to_i + end + authenticated = !token.blank? && totp.verify_with_drift_and_prior(token, 0, last_used) + if authenticated + user.user_second_factor.last_used = DateTime.now + user.user_second_factor.save + end + return authenticated + end + + def self.totp_enabled?(user) + !!user.user_second_factor && user.user_second_factor.enabled? + end +end diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 9d0df12796e..c9cd1a032d2 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -120,6 +120,15 @@ class UserNotifications < ActionMailer::Base ) end + def account_second_factor_disabled(user, opts = {}) + build_email( + user.email, + template: 'user_notifications.account_second_factor_disabled', + locale: user_locale(user), + email: user.email + ) + end + def short_date(dt) if dt.year == Time.now.year I18n.l(dt, format: :short_no_year) diff --git a/app/models/email_token.rb b/app/models/email_token.rb index 2a10dd4e234..7cfe74dab58 100644 --- a/app/models/email_token.rb +++ b/app/models/email_token.rb @@ -39,6 +39,15 @@ class EmailToken < ActiveRecord::Base token.present? && token =~ /\h{#{token.length / 2}}/i end + def self.second_factor_valid(token, second_factor_token) + # Fail only when token is valid, second factor token is required, and does NOT check out. + return true unless valid_token_format?(token) + email_token = confirmable(token) + return true if email_token.blank? + return true unless SecondFactorHelper.totp_enabled?(email_token.user) + return SecondFactorHelper.authenticate(email_token.user, second_factor_token) + end + def self.atomic_confirm(token) failure = { success: false } return failure unless valid_token_format?(token) diff --git a/app/models/user.rb b/app/models/user.rb index 48b328a06f2..0514b7038c6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -60,6 +60,7 @@ class User < ActiveRecord::Base has_one :github_user_info, dependent: :destroy has_one :google_user_info, dependent: :destroy has_one :oauth2_user_info, dependent: :destroy + has_one :user_second_factor, dependent: :destroy has_one :user_stat, dependent: :destroy has_one :user_profile, dependent: :destroy, inverse_of: :user has_one :single_sign_on_record, dependent: :destroy @@ -461,6 +462,10 @@ class User < ActiveRecord::Base '' # so that validator doesn't complain that a password attribute doesn't exist end + def second_factor + '' # so that validator doesn't complain that a password attribute doesn't exist + end + # Indicate that this is NOT a passwordless account for the purposes of validation def password_required! @password_required = true diff --git a/app/models/user_history.rb b/app/models/user_history.rb index bf2fe2b6e44..5e809af46e8 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -66,7 +66,8 @@ class UserHistory < ActiveRecord::Base change_name: 48, post_locked: 49, post_unlocked: 50, - check_personal_message: 51) + check_personal_message: 51, + disabled_second_factor: 52) end # Staff actions is a subset of all actions, used to audit actions taken by staff users. @@ -110,7 +111,8 @@ class UserHistory < ActiveRecord::Base :backup_destroy, :post_locked, :post_unlocked, - :check_personal_message] + :check_personal_message, + :disabled_second_factor] end def self.staff_action_ids diff --git a/app/models/user_second_factor.rb b/app/models/user_second_factor.rb new file mode 100644 index 00000000000..da48d2302f3 --- /dev/null +++ b/app/models/user_second_factor.rb @@ -0,0 +1,3 @@ +class UserSecondFactor < ActiveRecord::Base + belongs_to :user +end diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index 8cc9316cb5d..f1a0e1c93f5 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -25,7 +25,9 @@ class AdminDetailedUserSerializer < AdminUserSerializer :user_fields, :bounce_score, :reset_bounce_score_after, - :can_view_action_logs + :can_view_action_logs, + :second_factor_enabled, + :can_disable_second_factor has_one :approved_by, serializer: BasicUserSerializer, embed: :objects has_one :api_key, serializer: ApiKeySerializer, embed: :objects @@ -34,6 +36,19 @@ class AdminDetailedUserSerializer < AdminUserSerializer has_one :tl3_requirements, serializer: TrustLevel3RequirementsSerializer, embed: :objects has_many :groups, embed: :object, serializer: BasicGroupSerializer + def include_second_factor_enabled? + scope.is_staff? + end + + def can_disable_second_factor + (object.id && object.id != scope.user.try(:id)) && + scope.is_staff? + end + + def second_factor_enabled + SecondFactorHelper.totp_enabled?(object) + end + def can_revoke_admin scope.can_revoke_admin?(object) end diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 30bb1038c6f..63677173ce2 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -1,3 +1,5 @@ +require 'rqrcode' + class UserSerializer < BasicUserSerializer attr_accessor :omit_stats, @@ -72,7 +74,8 @@ class UserSerializer < BasicUserSerializer :primary_group_flair_url, :primary_group_flair_bg_color, :primary_group_flair_color, - :staged + :staged, + :second_factor_enabled has_one :invited_by, embed: :object, serializer: BasicUserSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer @@ -145,6 +148,15 @@ class UserSerializer < BasicUserSerializer (scope.is_staff? && object.staged?) end + def include_second_factor_enabled? + (object.id && object.id == scope.user.try(:id)) || + scope.is_staff? + end + + def second_factor_enabled + SecondFactorHelper.totp_enabled?(object) + end + def can_change_bio !(SiteSetting.enable_sso && SiteSetting.sso_overrides_bio) end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index b5a64c3f2f2..6058c1b5842 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -305,6 +305,12 @@ class StaffActionLogger target_user_id: user.id)) end + def log_disable_second_factor_auth(user, opts = {}) + raise Discourse::InvalidParameters.new(:user) unless user + UserHistory.create(params(opts).merge(action: UserHistory.actions[:disabled_second_factor], + target_user_id: user.id)) + end + def log_grant_admin(user, opts = {}) raise Discourse::InvalidParameters.new(:user) unless user UserHistory.create(params(opts).merge(action: UserHistory.actions[:grant_admin], diff --git a/app/views/session/email_login.html.erb b/app/views/session/email_login.html.erb index 7bd6cf03fd9..03704e0a441 100644 --- a/app/views/session/email_login.html.erb +++ b/app/views/session/email_login.html.erb @@ -3,6 +3,18 @@ <%= @error %>
<%end%> +<%if @second_factor_required%> +
+
+ <%= form_tag(method: "post") do%> +

<%=t "login.second_factor_title" %>

+ <%= label_tag(:second_factor_token, t("login.second_factor_description")) %> +
<%= text_field_tag(:second_factor_token) %>
+ <%= submit_tag(t("login.submit"), class: "btn btn-large btn-primary") %> + <%end%> +
+
+<%end%> <% content_for :title do %><%=t "email_login.title" %><% end %> diff --git a/app/views/users/admin_login.html.erb b/app/views/users/admin_login.html.erb index 9b40e951ec7..b6fc06f9725 100644 --- a/app/views/users/admin_login.html.erb +++ b/app/views/users/admin_login.html.erb @@ -5,6 +5,13 @@ <% if @message %> <%= @message %> + <% if @second_factor_required %> + <%=form_tag({}, method: :put) do %> + <%= label_tag(:second_factor_token, t('login.second_factor_description')) %> + <%= text_field_tag(:second_factor_token, nil, autofocus: true) %>

+ <%= submit_tag t('login.submit')%> + <% end %> + <% end %> <% else %> <%=form_tag({}, method: :put) do %> <%= label_tag(:email, t('admin_login.email_input')) %> diff --git a/app/views/users_email/confirm.html.erb b/app/views/users_email/confirm.html.erb index 0538ddfaac3..9411b3473ce 100644 --- a/app/views/users_email/confirm.html.erb +++ b/app/views/users_email/confirm.html.erb @@ -7,6 +7,17 @@

<%= t 'change_email.confirmed' %>


<%= t('change_email.please_continue', site_name: SiteSetting.title) %> + <% elsif @update_result == :invalid_second_factor%> +

<%= t('login.second_factor_title') %>

+
+ <%=form_tag({}, method: :put) do %> + <%= label_tag(:second_factor_token, t('login.second_factor_description')) %> + <%= text_field_tag(:second_factor_token, nil, autofocus: true) %>
+ <% if @show_invalid_second_factor_error %> +
<%= t('login.invalid_second_factor_code') %>
+ <% end %> + <%= submit_tag t('login.submit'), class: "btn btn-primary" %> + <% end %> <% else %>
<%=t 'change_email.already_done' %> diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 56d9beaefeb..835cbfbe4f4 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -207,6 +207,7 @@ en: not_implemented: "That feature hasn't been implemented yet, sorry!" no_value: "No" yes_value: "Yes" + submit: "Submit" generic_error: "Sorry, an error has occurred." generic_error_with_reason: "An error occurred: %{error}" sign_up: "Sign Up" @@ -707,6 +708,17 @@ en: choose_new: "Choose a new password" choose: "Choose a password" + second_factor: + title: "Two Factor Authentication" + enable: "Enable 2-Step Verification" + disable: "Disable 2-Step Verification" + confirm_password_description: "Confirm your password to continue enabling 2-Step Verification." + enable_description: "To complete 2-Step Verification setup, scan the following QR code and submit a 2-Step Verification code." + disable_description: "Enter a 2-Step Verification code to disable." + show_key_description: "Or enter the key manually." + info_prompt: "What is Two Factor authentication?" + extended_description: "Two-factor authentication adds an extra security step to logging in by requiring a one-time token in addition to your password. These tokens are generated by compatible apps for iPhone or Android such as Google Authenticator, Authy, and FreeOTP." + change_about: title: "Change About Me" error: "There was an error changing this value." @@ -1097,6 +1109,9 @@ en: title: "Log In" username: "User" password: "Password" + second_factor_title: "2-Step Verification Required" + second_factor_description: "Enter a generated verification code." + second_factor_label: "Code" email_placeholder: "email or username" caps_lock_warning: "Caps Lock is on" error: "Unknown error" @@ -3262,6 +3277,7 @@ en: post_locked: "post locked" post_unlocked: "post unlocked" check_personal_message: "check personal message" + disabled_second_factor: "disable 2-step auth" screened_emails: title: "Screened Emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c326faf39c8..42e61730f49 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1783,6 +1783,10 @@ en: auth_complete: "Authentication is complete." click_to_continue: "Click here to continue." already_logged_in: "Oops, looks like you are attempting to accept an invitation for another user. If you are not %{current_user}, please log out and try again." + second_factor_title: "2-Step Verification Required" + second_factor_description: "Enter a generated verification code." + invalid_second_factor_code: "Invalid 2-Step Verification Code" + submit: "Submit" user: no_accounts_associated: "No accounts associated" @@ -2730,6 +2734,15 @@ en: + account_second_factor_disabled: + title: "2-Step Verification disabled" + subject_template: "[%{email_prefix}] 2-Step Verification disabled" + text_body_template: | + Your account’s 2-Step verification at %{site_name} has been disabled. The account no longer needs a 2-Step Verification code to sign in. + + If you have any questions, [contact our friendly staff](%{base_url}/about). + + digest: why: "A brief summary of %{site_link} since your last visit on %{last_seen_at}" since_last_visit: "Since your last visit" diff --git a/config/routes.rb b/config/routes.rb index bb1b8c8fedb..bf838b8ff46 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -129,6 +129,7 @@ Discourse::Application.routes.draw do get "tl3_requirements" put "anonymize" post "reset_bounce_score" + put "disable_second_factor" end get "users/:id.json" => 'users#show', defaults: { format: 'json' } get 'users/:id/:username' => 'users#show', constraints: { username: RouteFormat.username } @@ -302,6 +303,8 @@ Discourse::Application.routes.draw do get "session/current" => "session#current" get "session/csrf" => "session#csrf" get "session/email-login/:token" => "session#email_login" + post "session/email-login/:token" => "session#email_login" + post "second_factor/create" => "second_factor#create" get "composer_messages" => "composer_messages#index" post "composer/parse_html" => "composer#parse_html" @@ -335,6 +338,7 @@ Discourse::Application.routes.draw do 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" + put "#{root_path}/admin-login/:token" => "users#admin_login" post "#{root_path}/toggle-anon" => "users#toggle_anon" post "#{root_path}/read-faq" => "users#read_faq" get "#{root_path}/search/users" => "users#search_users" @@ -349,6 +353,7 @@ Discourse::Application.routes.draw do get "#{root_path}/activate-account/:token" => "users#activate_account" put({ "#{root_path}/activate-account/:token" => "users#perform_account_activation" }.merge(index == 1 ? { as: 'perform_activate_account' } : {})) get "#{root_path}/authorize-email/:token" => "users_email#confirm" + put "#{root_path}/authorize-email/:token" => "users_email#confirm" get({ "#{root_path}/confirm-admin/:token" => "users#confirm_admin", constraints: { token: /[0-9a-f]+/ } @@ -380,6 +385,8 @@ Discourse::Application.routes.draw do put "#{root_path}/:username/preferences/badge_title" => "users#badge_title", constraints: { username: RouteFormat.username } get "#{root_path}/:username/preferences/username" => "users#preferences", constraints: { username: RouteFormat.username } put "#{root_path}/:username/preferences/username" => "users#username", constraints: { username: RouteFormat.username } + post "#{root_path}/:username/preferences/second-factor" => "second_factor#update", constraints: { username: RouteFormat.username } + get "#{root_path}/:username/preferences/second-factor" => "users#preferences", constraints: { username: RouteFormat.username } delete "#{root_path}/:username/preferences/user_image" => "users#destroy_user_image", constraints: { username: RouteFormat.username } put "#{root_path}/:username/preferences/avatar/pick" => "users#pick_avatar", constraints: { username: RouteFormat.username } get "#{root_path}/:username/preferences/card-badge" => "users#card_badge", constraints: { username: RouteFormat.username } diff --git a/db/migrate/20180109222722_create_user_second_factors.rb b/db/migrate/20180109222722_create_user_second_factors.rb new file mode 100644 index 00000000000..46abb216d19 --- /dev/null +++ b/db/migrate/20180109222722_create_user_second_factors.rb @@ -0,0 +1,12 @@ +class CreateUserSecondFactors < ActiveRecord::Migration[5.1] + def change + create_table :user_second_factors do |t| + t.integer :user_id, null: false + t.string :method + t.string :data + t.boolean :enabled, null: false, default: false + t.timestamp :last_used + t.timestamps + end + end +end diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb index 26a1056262f..c9f056b034c 100644 --- a/lib/guardian/user_guardian.rb +++ b/lib/guardian/user_guardian.rb @@ -72,4 +72,8 @@ module UserGuardian user == @user || is_staff? end + def can_disable_second_factor?(user) + user && can_administer_user?(user) + end + end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 703021890fc..0c81cde5969 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -265,6 +265,19 @@ describe Admin::UsersController do end end + context '#disable_second_factor' do + before do + @another_user = Fabricate(:user) + SecondFactorHelper.create_totp(@another_user) + end + + it 'disables the second factor' do + expect(User.find(@another_user.id).user_second_factor).not_to eq(nil) + put :disable_second_factor, params: { user_id: @another_user.id }, format: :json + expect(User.find(@another_user.id).user_second_factor).to eq(nil) + end + end + context '#add_group' do let(:user) { Fabricate(:user) } let(:group) { Fabricate(:group) } diff --git a/spec/controllers/second_factor_controller_spec.rb b/spec/controllers/second_factor_controller_spec.rb new file mode 100644 index 00000000000..d74ba44ce7a --- /dev/null +++ b/spec/controllers/second_factor_controller_spec.rb @@ -0,0 +1,69 @@ +require 'rails_helper' + +RSpec.describe SecondFactorController, type: :controller do + # featheredtoast-todo also write qunit tests. + describe '.create' do + + let(:user) { Fabricate(:user) } + + describe 'create 2fa request' do + it 'fails on incorrect password' do + post :create, params: { + login: user.username, password: 'wrongpassword' + }, format: :json + expect(JSON.parse(response.body)['error']).to eq(I18n.t("login.incorrect_username_email_or_password")) + end + + it 'succeeds on correct password' do + post :create, params: { + login: user.username, password: 'myawesomepassword' + }, format: :json + expect(JSON.parse(response.body).keys).to contain_exactly('key', 'qr') + end + end + end + + describe '.update' do + let(:user) { Fabricate(:user) } + + context 'when user has totp setup' do + second_factor_data = "rcyryaqage3jexfj" + before do + user.user_second_factor = UserSecondFactor.create(user_id: user.id, method: "totp", data: second_factor_data) + end + + it 'errors on incorrect code' do + post :update, params: { + username: user.username, + token: '000000', + enable: 'true' + }, format: :json + expect(JSON.parse(response.body)['error']).to eq(I18n.t("login.invalid_second_factor_code")) + user.reload + end + + it 'can be enabled' do + post :update, params: { + username: user.username, + token: ROTP::TOTP.new(second_factor_data).now, + enable: 'true' + }, format: :json + expect(JSON.parse(response.body)['result']).to eq('ok') + user.reload + expect(user.user_second_factor.enabled).to be true + end + + it 'can be disabled' do + post :update, params: { + username: user.username, + enable: 'false', + token: ROTP::TOTP.new(second_factor_data).now + }, format: :json + expect(JSON.parse(response.body)['result']).to eq('ok') + user.reload + expect(user.user_second_factor).to be_nil + end + end + end + +end diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb index 96b58c5483d..0b83ab8ab2c 100644 --- a/spec/controllers/session_controller_spec.rb +++ b/spec/controllers/session_controller_spec.rb @@ -584,6 +584,39 @@ describe SessionController do end end + context 'when user has 2-factor logins' do + second_factor_data = "rcyryaqage3jexfj" + before do + user.user_second_factor = UserSecondFactor.create(user_id: user.id, method: "totp", data: second_factor_data, enabled: true) + end + + describe 'failure no 2-factor' do + it 'should return an error' do + post :create, params: { + login: user.username, password: 'myawesomepassword' + }, format: :json + expect(JSON.parse(response.body)['error']).to eq(I18n.t('login.invalid_second_factor_code')) + end + end + describe 'successful 2-factor' do + it 'logs in correctly' do + events = DiscourseEvent.track_events do + post :create, params: { + login: user.username, password: 'myawesomepassword', second_factor_token: ROTP::TOTP.new(second_factor_data).now + }, format: :json + end + + expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in) + + user.reload + + expect(session[:current_user_id]).to eq(user.id) + expect(user.user_auth_tokens.count).to eq(1) + expect(UserAuthToken.hash_token(cookies[:_t])).to eq(user.user_auth_tokens.first.auth_token) + end + end + end + describe 'with a blocked IP' do before do screened_ip = Fabricate(:screened_ip_address) @@ -777,6 +810,26 @@ describe SessionController do login: user.username, password: 'myawesomepassword' }, format: :json + expect(response).not_to be_success + json = JSON.parse(response.body) + expect(json["error_type"]).to eq("rate_limit") + end + it 'rate limits second factor attempts' do + RateLimiter.enable + RateLimiter.clear_all! + + 3.times do + post :create, params: { + login: user.username, password: 'myawesomepassword', second_factor_token: '000000' + }, format: :json + + expect(response).to be_success + end + + post :create, params: { + login: user.username, password: 'myawesomepassword', second_factor_token: '000000' + }, format: :json + expect(response).not_to be_success json = JSON.parse(response.body) expect(json["error_type"]).to eq("rate_limit") diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index f9b802a02db..206fd6bcf59 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -343,7 +343,7 @@ describe UsersController do ) expect(response).to be_success - expect(response.body).to include('{"is_developer":false,"admin":false}') + expect(response.body).to include('{"is_developer":false,"admin":false,"second_factor_required":false}') user.reload @@ -406,6 +406,46 @@ describe UsersController do expect(email_token.confirmed).to eq(false) expect(UserAuthToken.where(id: user_token.id).count).to eq(1) end + + context '2-factor required' do + + second_factor_data = "rcyryaqage3jexfj" + let(:user) { Fabricate(:user) } + + before do + user.user_second_factor = UserSecondFactor.create(user_id: user.id, method: "totp", data: second_factor_data, enabled: true) + end + + it 'does not change with an invalid token' do + token = user.email_tokens.create(email: user.email).token + + get :password_reset, params: { token: token } + + expect(response.body).to include('{"is_developer":false,"admin":false,"second_factor_required":true}') + + put :password_reset, + params: { token: token, password: 'hg9ow8yHG32O', second_factor_token: '000000' } + + expect(response.body).to include(I18n.t("login.invalid_second_factor_code")) + + user.reload + expect(user.confirm_password?('hg9ow8yHG32O')).not_to eq(true) + expect(user.user_auth_tokens.count).not_to eq(1) + end + + it 'changes password with valid 2-factor tokens' do + token = user.email_tokens.create(email: user.email).token + + get :password_reset, params: { token: token } + + put :password_reset, + params: { token: token, password: 'hg9ow8yHG32O', second_factor_token: ROTP::TOTP.new(second_factor_data).now } + + user.reload + expect(user.confirm_password?('hg9ow8yHG32O')).to eq(true) + expect(user.user_auth_tokens.count).to eq(1) + end + end end context 'submit change' do @@ -514,6 +554,29 @@ describe UsersController do expect(session[:current_user_id]).to eq(admin.id) end end + + context 'needs 2-factor' do + render_views + second_factor_data = "rcyryaqage3jexfj" + before do + admin.user_second_factor = UserSecondFactor.create(user_id: admin.id, method: "totp", data: second_factor_data, enabled: true) + end + + it 'does not log in when token required' do + token = admin.email_tokens.create(email: admin.email).token + get :admin_login, params: { token: token } + expect(response).not_to redirect_to('/') + expect(session[:current_user_id]).not_to eq(admin.id) + expect(response.body).to include(I18n.t('login.second_factor_description')); + end + + it 'logs in when a valid 2-factor token is given' do + token = admin.email_tokens.create(email: admin.email).token + put :admin_login, params: { token: token, second_factor_token: ROTP::TOTP.new(second_factor_data).now } + expect(response).to redirect_to('/') + expect(session[:current_user_id]).to eq(admin.id) + end + end end end diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 951968984f2..65004efe145 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -136,6 +136,41 @@ RSpec.describe SessionController do date: I18n.l(user.suspended_till, format: :date_only) )) end + + context 'user has 2-factor logins' do + second_factor_data = "rcyryaqage3jexfj" + before do + user.user_second_factor = UserSecondFactor.create(user_id: user.id, method: "totp", data: second_factor_data, enabled: true) + end + + describe 'requires second factor' do + it 'should return a second factor prompt' do + get "/session/email-login/#{email_token.token}" + + expect(response.status).to eq(200) + + expect(CGI.unescapeHTML(response.body)).to include(I18n.t("login.second_factor_title")) + end + end + + describe 'errors on incorrect 2-factor' do + it 'does not log in with incorrect two factor' do + post "/session/email-login/#{email_token.token}", params: { second_factor_token: "0000" } + + expect(response.status).to eq(200) + + expect(CGI.unescapeHTML(response.body)).to include(I18n.t("login.invalid_second_factor_code")) + end + end + + describe 'allows successful 2-factor' do + it 'logs in correctly' do + post "/session/email-login/#{email_token.token}", params: { second_factor_token: ROTP::TOTP.new(second_factor_data).now } + + expect(response).to redirect_to("/") + end + end + end end end end diff --git a/spec/requests/users_email_controller_spec.rb b/spec/requests/users_email_controller_spec.rb index 04912f8f206..e7feb560dbd 100644 --- a/spec/requests/users_email_controller_spec.rb +++ b/spec/requests/users_email_controller_spec.rb @@ -60,6 +60,35 @@ describe UsersEmailController do expect(user.user_stat.bounce_score).to eq(0) expect(user.user_stat.reset_bounce_score_after).to eq(nil) end + + context 'second factor required' do + second_factor_data = "rcyryaqage3jexfj" + before do + user.user_second_factor = UserSecondFactor.create(user_id: user.id, method: "totp", data: second_factor_data, enabled: true) + end + + it 'requires a second factor token' do + get "/u/authorize-email/#{user.email_tokens.last.token}" + expect(response.body).to include(I18n.t("login.second_factor_title")) + expect(response.body).not_to include(I18n.t("login.invalid_second_factor_code")) + end + + it 'adds an error on a second factor attempt' do + get "/u/authorize-email/#{user.email_tokens.last.token}", params: { + second_factor_token: "000000" + } + expect(response.body).to include(I18n.t("login.invalid_second_factor_code")) + end + + it 'confirms with a correct second token' do + get "/u/authorize-email/#{user.email_tokens.last.token}", params: { + second_factor_token: ROTP::TOTP.new(second_factor_data).now + } + expect(response).to be_success + expect(response.body).not_to include(I18n.t("login.second_factor_title")) + expect(response.body).not_to include(I18n.t("login.invalid_second_factor_code")) + end + end end end diff --git a/test/javascripts/acceptance/password-reset-test.js.es6 b/test/javascripts/acceptance/password-reset-test.js.es6 index 4706b88c29b..211ef5f47b8 100644 --- a/test/javascripts/acceptance/password-reset-test.js.es6 +++ b/test/javascripts/acceptance/password-reset-test.js.es6 @@ -24,6 +24,20 @@ acceptance("Password Reset", { return response({success: "OK", message: I18n.t('password_reset.success')}); } }); + + server.get('/u/confirm-email-token/requiretwofactor.json', () => { //eslint-disable-line + return response({success: "OK"}); + }); + server.put('/u/password-reset/requiretwofactor.json', request => { //eslint-disable-line + const body = parsePostData(request.requestBody); + if (body.password === "perf3ctly5ecur3" && body.second_factor_token === "123123") { + return response({success: "OK", message: I18n.t('password_reset.success')}); + } else if (body.second_factor_token === "123123") { + return response({success: false, errors: {password: ["invalid"]}}); + } else { + return response({success: false, message: "invalid token", errors: {second_factor: ["invalid token"]}}); + } + }); } }); @@ -58,4 +72,35 @@ QUnit.test("Password Reset Page", assert => { andThen(() => { assert.ok(!exists(".password-reset form"), "form is gone"); }); -}); \ No newline at end of file +}); + +QUnit.test("Password Reset Page With Second Factor", assert => { + PreloadStore.store('password_reset', {is_developer: false, second_factor_required: true}); + + visit("/u/password-reset/requiretwofactor"); + andThen(() => { + assert.notOk(exists("#new-account-password"), "does not show the input"); + assert.ok(exists("#second-factor"), "shows the second factor prompt"); + }); + + fillIn('#second-factor', '0000'); + + click('.password-reset form button'); + andThen(() => { + assert.ok(exists(".alert-error"), "shows 2 factor error"); + assert.ok(find(".alert-error").html().indexOf("invalid token") > -1, "server validation error message shows"); + }); + + fillIn('#second-factor', '123123'); + click('.password-reset form button'); + andThen(() => { + assert.notOk(exists(".alert-error"), "hides error"); + assert.ok(exists("#new-account-password"), "shows the input"); + }); + + fillIn('.password-reset input', 'perf3ctly5ecur3'); + click('.password-reset form button'); + andThen(() => { + assert.ok(!exists(".password-reset form"), "form is gone"); + }); +}); diff --git a/test/javascripts/acceptance/preferences-test.js.es6 b/test/javascripts/acceptance/preferences-test.js.es6 index 0014cefd652..c8fbee7aae0 100644 --- a/test/javascripts/acceptance/preferences-test.js.es6 +++ b/test/javascripts/acceptance/preferences-test.js.es6 @@ -1,5 +1,20 @@ import { acceptance } from "helpers/qunit-helpers"; -acceptance("User Preferences", { loggedIn: true }); +acceptance("User Preferences", { + loggedIn: true, + beforeEach() { + const response = (object) => { + return [ + 200, + {"Content-Type": "application/json"}, + object + ]; + }; + + server.post('/second_factor/create', () => { //eslint-disable-line + return response({key: "rcyryaqage3jexfj", qr: '
qr-code
'}); + }); + } +}); QUnit.test("update some fields", assert => { visit("/u/eviltrout/preferences"); @@ -73,3 +88,16 @@ QUnit.test("email", assert => { assert.equal(find('.tip.bad').text().trim(), I18n.t('user.email.invalid'), 'it should display invalid email tip'); }); }); + +QUnit.test("second factor", assert => { + visit("/u/eviltrout/preferences/second-factor"); + andThen(() => { + assert.ok(exists("#password"), "it has a password input"); + }); + fillIn('#password', 'secrets'); + click(".user-content .btn-primary"); + andThen(() => { + assert.ok(exists("#test-qr"), "shows qr code"); + assert.notOk(exists("#password"), "it hides the password input"); + }); +}); diff --git a/test/javascripts/acceptance/sign-in-test.js.es6 b/test/javascripts/acceptance/sign-in-test.js.es6 index 12d77228a59..ffe8c9cab7a 100644 --- a/test/javascripts/acceptance/sign-in-test.js.es6 +++ b/test/javascripts/acceptance/sign-in-test.js.es6 @@ -76,6 +76,32 @@ QUnit.test("sign in - not activated - edit email", assert => { }); }); +QUnit.test("second factor", assert => { + visit("/"); + click("header .login-button"); + andThen(() => { + assert.ok(exists('.login-modal'), "it shows the login modal"); + }); + + // Login with username and password only + fillIn('#login-account-name', 'eviltrout'); + fillIn('#login-account-password', 'need-second-factor'); + click('.modal-footer .btn-primary'); + andThen(() => { + assert.not(exists('#modal-alert:visible'), 'it hides the login error'); + assert.not(exists('#credentials:visible'), 'it hides the username and password prompt'); + assert.ok(exists('#second-factor:visible'), 'it displays the second factor prompt'); + assert.not(exists('.modal-footer .btn-primary:disabled'), "enables the login button"); + }); + + // Login with username, password, and token + fillIn('#login-second-factor', '123456'); + click('.modal-footer .btn-primary'); + andThen(() => { + assert.ok(exists('.modal-footer .btn-primary:disabled'), "disables the login button"); + }); +}); + QUnit.test("create account", assert => { visit("/"); click("header .sign-up-button"); @@ -106,4 +132,4 @@ QUnit.test("create account", assert => { andThen(() => { assert.ok(exists('.modal-footer .btn-primary:disabled'), "create account is disabled"); }); -}); \ No newline at end of file +}); diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index b0bb47e3e8d..db8b8520a1c 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -227,6 +227,16 @@ export default function() { current_email: 'current@example.com' }); } + if (data.password === 'need-second-factor') { + if (data.second_factor_token) { + return response({username: 'eviltrout'}); + } + return response({ error: "Invalid Second Factor", + reason: "invalid_second_factor", + sent_to_email: 'eviltrout@example.com', + current_email: 'current@example.com' }); + } + return response(400, {error: 'invalid login'}); });