diff --git a/app/assets/javascripts/discourse/components/second-factor-form.js.es6 b/app/assets/javascripts/discourse/components/second-factor-form.js.es6 index f930a4cb39b..7c75b0edc1a 100644 --- a/app/assets/javascripts/discourse/components/second-factor-form.js.es6 +++ b/app/assets/javascripts/discourse/components/second-factor-form.js.es6 @@ -16,17 +16,23 @@ export default Ember.Component.extend({ : I18n.t("login.second_factor_backup_description"); }, - @computed("secondFactorMethod") - linkText(secondFactorMethod) { - return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP - ? "login.second_factor_backup" - : "login.second_factor"; + @computed("secondFactorMethod", "isLogin") + linkText(secondFactorMethod, isLogin) { + if (isLogin) { + return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP + ? "login.second_factor_backup" + : "login.second_factor"; + } else { + return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP + ? "user.second_factor_backup.use" + : "user.second_factor.use"; + } }, actions: { toggleSecondFactorMethod() { const secondFactorMethod = this.get("secondFactorMethod"); - this.set("loginSecondFactor", ""); + this.set("secondFactorToken", ""); if (secondFactorMethod === SECOND_FACTOR_METHODS.TOTP) { this.set("secondFactorMethod", SECOND_FACTOR_METHODS.BACKUP_CODE); } else { diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 34ca1434443..d5e6042c7b4 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/app/assets/javascripts/discourse/controllers/login.js.es6 @@ -106,7 +106,7 @@ export default Ember.Controller.extend(ModalFunctionality, { data: { login: this.get("loginName"), password: this.get("loginPassword"), - second_factor_token: this.get("loginSecondFactor"), + second_factor_token: this.get("secondFactorToken"), second_factor_method: this.get("secondFactorMethod") } }).then( diff --git a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 index 7e2ed248b52..2c74c8f2086 100644 --- a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 +++ b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 @@ -9,7 +9,7 @@ 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"), - backupEnabled: Ember.computed.alias("model.second_factor_backup_enabled"), + backupEnabled: Ember.computed.alias("model.backup_enabled"), secondFactorMethod: SECOND_FACTOR_METHODS.TOTP, passwordRequired: true, errorMessage: null, @@ -38,7 +38,7 @@ export default Ember.Controller.extend(PasswordValidation, { type: "PUT", data: { password: this.get("accountPassword"), - second_factor_token: this.get("secondFactor"), + second_factor_token: this.get("secondFactorToken"), second_factor_method: this.get("secondFactorMethod") } }) diff --git a/app/assets/javascripts/discourse/controllers/preferences/second-factor-backup.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/second-factor-backup.js.es6 index e0b703b52dc..5f41061f97d 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/second-factor-backup.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/second-factor-backup.js.es6 @@ -1,6 +1,7 @@ import { default as computed } from "ember-addons/ember-computed-decorators"; import { default as DiscourseURL, userPath } from "discourse/lib/url"; import { popupAjaxError } from "discourse/lib/ajax-error"; +import { SECOND_FACTOR_METHODS } from "discourse/models/user"; export default Ember.Controller.extend({ loading: false, @@ -11,10 +12,15 @@ export default Ember.Controller.extend({ "model.second_factor_remaining_backup_codes" ), backupCodes: null, + secondFactorMethod: SECOND_FACTOR_METHODS.TOTP, - @computed("secondFactorToken") - isValidSecondFactorToken(secondFactorToken) { - return secondFactorToken && secondFactorToken.length === 6; + @computed("secondFactorToken", "secondFactorMethod") + isValidSecondFactorToken(secondFactorToken, secondFactorMethod) { + if (secondFactorMethod === SECOND_FACTOR_METHODS.TOTP) { + return secondFactorToken && secondFactorToken.length === 6; + } else if (secondFactorMethod === SECOND_FACTOR_METHODS.BACKUP_CODE) { + return secondFactorToken && secondFactorToken.length === 16; + } }, @computed("isValidSecondFactorToken", "backupEnabled", "loading") @@ -59,7 +65,12 @@ export default Ember.Controller.extend({ this.set("loading", true); this.get("model") - .toggleSecondFactor(this.get("secondFactorToken"), false, 2) + .toggleSecondFactor( + this.get("secondFactorToken"), + this.get("secondFactorMethod"), + SECOND_FACTOR_METHODS.BACKUP_CODE, + false + ) .then(response => { if (response.error) { this.set("errorMessage", response.error); @@ -79,7 +90,10 @@ export default Ember.Controller.extend({ if (!this.get("secondFactorToken")) return; this.set("loading", true); this.get("model") - .generateSecondFactorCodes(this.get("secondFactorToken")) + .generateSecondFactorCodes( + this.get("secondFactorToken"), + this.get("secondFactorMethod") + ) .then(response => { if (response.error) { this.set("errorMessage", response.error); diff --git a/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 index af6e78eb6d6..662eb31adc1 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 @@ -2,6 +2,7 @@ import { default as computed } from "ember-addons/ember-computed-decorators"; import { default as DiscourseURL, userPath } from "discourse/lib/url"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { findAll } from "discourse/models/login-method"; +import { SECOND_FACTOR_METHODS } from "discourse/models/user"; export default Ember.Controller.extend({ loading: false, @@ -13,6 +14,8 @@ export default Ember.Controller.extend({ showSecondFactorKey: false, errorMessage: null, newUsername: null, + backupEnabled: Ember.computed.alias("model.second_factor_backup_enabled"), + secondFactorMethod: SECOND_FACTOR_METHODS.TOTP, loaded: Ember.computed.and("secondFactorImage", "secondFactorKey"), @@ -41,7 +44,12 @@ export default Ember.Controller.extend({ this.set("loading", true); this.get("model") - .toggleSecondFactor(this.get("secondFactorToken"), enable, 1) + .toggleSecondFactor( + this.get("secondFactorToken"), + this.get("secondFactorMethod"), + SECOND_FACTOR_METHODS.TOTP, + enable + ) .then(response => { if (response.error) { this.set("errorMessage", response.error); diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 80572feeb5e..428f08ba5bf 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -367,20 +367,24 @@ const User = RestModel.extend({ }); }, - toggleSecondFactor(token, enable, method) { + toggleSecondFactor(authToken, authMethod, targetMethod, enable) { return ajax("/u/second_factor.json", { data: { - second_factor_token: token, - second_factor_method: method, + second_factor_token: authToken, + second_factor_method: authMethod, + second_factor_target: targetMethod, enable }, type: "PUT" }); }, - generateSecondFactorCodes(token) { + generateSecondFactorCodes(authToken, authMethod) { return ajax("/u/second_factors_backup.json", { - data: { second_factor_token: token }, + data: { + second_factor_token: authToken, + second_factor_method: authMethod + }, type: "PUT" }); }, diff --git a/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs b/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs index d0d7beb42b7..f2ccf505dfe 100644 --- a/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs +++ b/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs @@ -1,5 +1,8 @@

{{secondFactorTitle}}

+ {{#if optionalText}} +

{{{optionalText}}}

+ {{/if}}

{{secondFactorDescription}}

{{yield}} {{#if backupEnabled}} diff --git a/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs b/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs index 907d36744e7..ad6101bea8d 100644 --- a/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs +++ b/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs @@ -1,4 +1,4 @@ -{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword loginSecondFactor=loginSecondFactor action=(action "login")}} +{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword secondFactorToken=secondFactorToken action=(action "login")}} {{#d-modal-body title="login.title" class="login-modal"}} {{#if showLoginButtons}} {{login-buttons @@ -36,8 +36,12 @@
- {{#second-factor-form secondFactorMethod=secondFactorMethod loginSecondFactor=loginSecondFactor backupEnabled=backupEnabled class=secondFactorClass}} - {{second-factor-input value=loginSecondFactor inputId='login-second-factor' secondFactorMethod=secondFactorMethod}} + {{#second-factor-form + secondFactorMethod=secondFactorMethod + secondFactorToken=secondFactorToken + class=secondFactorClass + isLogin=true}} + {{second-factor-input value=secondFactorToken inputId='login-second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} {{/second-factor-form}} {{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/login.hbs b/app/assets/javascripts/discourse/templates/modal/login.hbs index ae8d27a42f7..e8272134a9a 100644 --- a/app/assets/javascripts/discourse/templates/modal/login.hbs +++ b/app/assets/javascripts/discourse/templates/modal/login.hbs @@ -1,4 +1,4 @@ -{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword loginSecondFactor=loginSecondFactor action=(action "login")}} +{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword secondFactorToken=secondFactorToken action=(action "login")}} {{#d-modal-body title="login.title" class=(concat "login-modal" " " (if hasAtLeastOneLoginButton "has-alt-auth"))}} {{#if canLoginLocal}} @@ -21,8 +21,13 @@ - {{#second-factor-form secondFactorMethod=secondFactorMethod loginSecondFactor=loginSecondFactor backupEnabled=backupEnabled class=secondFactorClass}} - {{second-factor-input value=loginSecondFactor inputId='login-second-factor' secondFactorMethod=secondFactorMethod}} + {{#second-factor-form + secondFactorMethod=secondFactorMethod + secondFactorToken=secondFactorToken + class=secondFactorClass + backupEnabled=backupEnabled + isLogin=true}} + {{second-factor-input value=secondFactorToken inputId='login-second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} {{/second-factor-form}} {{/if}} diff --git a/app/assets/javascripts/discourse/templates/password-reset.hbs b/app/assets/javascripts/discourse/templates/password-reset.hbs index e1c49121e03..b145500da75 100644 --- a/app/assets/javascripts/discourse/templates/password-reset.hbs +++ b/app/assets/javascripts/discourse/templates/password-reset.hbs @@ -17,16 +17,17 @@ {{else}}
{{#if secondFactorRequired}} - {{#second-factor-form secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} - {{text-field - id="second-factor" - value=secondFactor - autocorrect="off" - autocapitalize="off" - autofocus="autofocus" - maxlength="6" - secondFactorMethod=secondFactorMethod - }} + {{#if errorMessage}} +
{{errorMessage}}
+
+ {{/if}} + + {{#second-factor-form + secondFactorMethod=secondFactorMethod + secondFactorToken=secondFactorToken + backupEnabled=backupEnabled + isLogin=false}} + {{second-factor-input value=secondFactorToken inputId='second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} {{/second-factor-form}} {{d-button action=(action "submit") class='btn-primary' label='submit'}} {{else}} @@ -44,12 +45,6 @@ {{d-button action=(action "submit") class='btn-primary' label='user.change_password.set_password'}} {{/if}} - - {{#if errorMessage}} -

-
{{errorMessage}}
- {{/if}} -
{{/if}} diff --git a/app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs b/app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs index 3511363614f..eb65b54a468 100644 --- a/app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs +++ b/app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs @@ -1,12 +1,7 @@
-
-

{{i18n "user.second_factor_backup.title"}}

- {{#if backupEnabled}} -

{{{i18n "user.second_factor_backup.remaining_codes" count=remainingCodes}}}

- {{/if}} -
+
{{#if successMessage}}
@@ -22,17 +17,17 @@
-
- {{second-factor-input - value=secondFactorToken - maxlength=6 - inputId="second-factor-token"}} - -
- {{i18n "user.second_factor.disable_description"}} -
+ {{#second-factor-form + secondFactorMethod=secondFactorMethod + backupEnabled=backupEnabled + secondFactorToken=secondFactorToken + secondFactorTitle=(i18n 'user.second_factor_backup.title') + optionalText=(if backupEnabled (i18n "user.second_factor_backup.remaining_codes" count=remainingCodes)) + isLogin=false}} + {{second-factor-input value=secondFactorToken inputId='second-factor-token' secondFactorMethod=secondFactorMethod}} + {{/second-factor-form}}
diff --git a/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs b/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs index 797135fcdcf..ad50e770957 100644 --- a/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs +++ b/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs @@ -1,12 +1,6 @@
-
-
-

{{i18n 'user.second_factor.title'}}

-
-
- {{#if errorMessage}}
@@ -16,15 +10,16 @@ {{/if}} {{#if model.second_factor_enabled}} - -
- {{second-factor-input maxlength=6 value=secondFactorToken inputId='second-factor-token'}} -
- -
- {{i18n 'user.second_factor.disable_description'}} + {{#second-factor-form + secondFactorMethod=secondFactorMethod + backupEnabled=backupEnabled + secondFactorToken=secondFactorToken + secondFactorTitle=(i18n 'user.second_factor.title') + isLogin=false}} + {{second-factor-input value=secondFactorToken inputId='second-factor-token' secondFactorMethod=secondFactorMethod}} + {{/second-factor-form}}
@@ -91,15 +86,16 @@
- {{text-field value=password +
+ {{text-field value=password id="password" type="password" classNames="input-xxlarge" autofocus="autofocus"}} -
- -
- {{i18n 'user.second_factor.confirm_password_description'}} +
+
+ {{i18n 'user.second_factor.confirm_password_description'}} +
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index efe426fb5f2..be2ba574f5f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -477,7 +477,7 @@ class UsersController < ApplicationController second_factor_token = params[:second_factor_token] second_factor_method = params[:second_factor_method].to_i - if second_factor_token.present? && second_factor_token[/\d{6}/] && UserSecondFactor.methods[second_factor_method] + if second_factor_token.present? && UserSecondFactor.methods[second_factor_method] RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! second_factor_authenticated = @user&.authenticate_second_factor(second_factor_token, second_factor_method) end @@ -1059,7 +1059,7 @@ class UsersController < ApplicationController def create_second_factor_backup raise Discourse::NotFound if SiteSetting.enable_sso || !SiteSetting.enable_local_logins - unless current_user.authenticate_totp(params[:second_factor_token]) + unless current_user.authenticate_second_factor(params[:second_factor_token], params[:second_factor_method].to_i) return render json: failed_json.merge( error: I18n.t("login.invalid_second_factor_code") ) @@ -1075,22 +1075,26 @@ class UsersController < ApplicationController def update_second_factor params.require(:second_factor_token) params.require(:second_factor_method) + params.require(:second_factor_target) - second_factor_method = params[:second_factor_method].to_i + auth_method = params[:second_factor_method].to_i + auth_token = params[:second_factor_token] + + update_second_factor_method = params[:second_factor_target].to_i [request.remote_ip, current_user.id].each do |key| RateLimiter.new(nil, "second-factor-min-#{key}", 3, 1.minute).performed! end - if second_factor_method == UserSecondFactor.methods[:totp] + if update_second_factor_method == UserSecondFactor.methods[:totp] user_second_factor = current_user.user_second_factors.totp - elsif second_factor_method == UserSecondFactor.methods[:backup_codes] + elsif update_second_factor_method == UserSecondFactor.methods[:backup_codes] user_second_factor = current_user.user_second_factors.backup_codes end raise Discourse::InvalidParameters unless user_second_factor - unless current_user.authenticate_totp(params[:second_factor_token]) + unless current_user.authenticate_second_factor(auth_token, auth_method) return render json: failed_json.merge( error: I18n.t("login.invalid_second_factor_code") ) @@ -1100,7 +1104,7 @@ class UsersController < ApplicationController user_second_factor.update!(enabled: true) else # when disabling totp, backup is disabled too - if second_factor_method == UserSecondFactor.methods[:totp] + if update_second_factor_method == UserSecondFactor.methods[:totp] current_user.user_second_factors.destroy_all Jobs.enqueue( @@ -1108,7 +1112,7 @@ class UsersController < ApplicationController type: :account_second_factor_disabled, user_id: current_user.id ) - elsif second_factor_method == UserSecondFactor.methods[:backup_codes] + elsif update_second_factor_method == UserSecondFactor.methods[:backup_codes] current_user.user_second_factors.where(method: UserSecondFactor.methods[:backup_codes]).destroy_all end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 9ee306acad0..65d4c0d891c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -782,6 +782,7 @@ en: copied_to_clipboard: "Copied to Clipboard" copy_to_clipboard_error: "Error copying data to Clipboard" remaining_codes: "You have {{count}} backup codes remaining." + use: "Use a backup code" codes: title: "Backup Codes Generated" description: "Each of these backup codes can only be used once. Keep them somewhere safe but accessible." @@ -800,6 +801,7 @@ en: extended_description: | Two factor authentication adds extra security to your account by requiring a one-time token in addition to your password. Tokens can be generated on Android and iOS devices. oauth_enabled_warning: "Please note that social logins will be disabled once two factor authentication has been enabled on your account." + use: "Use Authenticator app" change_about: title: "Change About Me" diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 3d278abee76..6465bc3bb97 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -3107,6 +3107,7 @@ describe UsersController do put "/users/second_factor.json", params: { second_factor_token: '000000', second_factor_method: UserSecondFactor.methods[:totp], + second_factor_target: UserSecondFactor.methods[:totp], enable: 'true', } @@ -3122,8 +3123,9 @@ describe UsersController do it 'should allow second factor for the user to be enabled' do put "/users/second_factor.json", params: { second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, - enable: 'true', - second_factor_method: UserSecondFactor.methods[:totp] + second_factor_method: UserSecondFactor.methods[:totp], + second_factor_target: UserSecondFactor.methods[:totp], + enable: 'true' } expect(response.status).to eq(200) @@ -3133,7 +3135,8 @@ describe UsersController do it 'should allow second factor for the user to be disabled' do put "/users/second_factor.json", params: { second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, - second_factor_method: UserSecondFactor.methods[:totp] + second_factor_method: UserSecondFactor.methods[:totp], + second_factor_target: UserSecondFactor.methods[:totp] } expect(response.status).to eq(200) @@ -3146,7 +3149,7 @@ describe UsersController do context 'when token is missing' do it 'returns the right response' do put "/users/second_factor.json", params: { - second_factor_method: UserSecondFactor.methods[:backup_codes], + second_factor_target: UserSecondFactor.methods[:backup_codes] } expect(response.status).to eq(400) @@ -3157,7 +3160,8 @@ describe UsersController do it 'returns the right response' do put "/users/second_factor.json", params: { second_factor_token: '000000', - second_factor_method: UserSecondFactor.methods[:backup_codes], + second_factor_method: UserSecondFactor.methods[:totp], + second_factor_target: UserSecondFactor.methods[:backup_codes] } expect(response.status).to eq(200) @@ -3172,7 +3176,8 @@ describe UsersController do it 'should allow second factor backup for the user to be disabled' do put "/users/second_factor.json", params: { second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, - second_factor_method: UserSecondFactor.methods[:backup_codes] + second_factor_method: UserSecondFactor.methods[:totp], + second_factor_target: UserSecondFactor.methods[:backup_codes] } expect(response.status).to eq(200) @@ -3189,7 +3194,8 @@ describe UsersController do context 'when not logged in' do it 'should return the right response' do put "/users/second_factors_backup.json", params: { - second_factor_token: 'wrongtoken' + second_factor_token: 'wrongtoken', + second_factor_method: UserSecondFactor.methods[:totp] } expect(response.status).to eq(403) @@ -3204,7 +3210,8 @@ describe UsersController do describe 'create 2fa request' do it 'fails on incorrect password' do put "/users/second_factors_backup.json", params: { - second_factor_token: 'wrongtoken' + second_factor_token: 'wrongtoken', + second_factor_method: UserSecondFactor.methods[:totp] } expect(response.status).to eq(200) @@ -3219,7 +3226,8 @@ describe UsersController do SiteSetting.enable_local_logins = false put "/users/second_factors_backup.json", params: { - second_factor_token: ROTP::TOTP.new(user_second_factor.data).now + second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, + second_factor_method: UserSecondFactor.methods[:totp] } expect(response.status).to eq(404) @@ -3232,7 +3240,8 @@ describe UsersController do SiteSetting.enable_sso = true put "/users/second_factors_backup.json", params: { - second_factor_token: ROTP::TOTP.new(user_second_factor.data).now + second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, + second_factor_method: UserSecondFactor.methods[:totp] } expect(response.status).to eq(404) @@ -3243,7 +3252,8 @@ describe UsersController do user_second_factor put "/users/second_factors_backup.json", params: { - second_factor_token: ROTP::TOTP.new(user_second_factor.data).now + second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, + second_factor_method: UserSecondFactor.methods[:totp] } expect(response.status).to eq(200)