From 88ef5e55fe67bf526a92f9ca85faf5257c83b087 Mon Sep 17 00:00:00 2001 From: Jeff Wong Date: Wed, 26 Jun 2019 16:58:06 -0700 Subject: [PATCH] FEATURE: add ability to have multiple totp factors (#7626) Adds a second factor landing page that centralizes a user's second factor configuration. This contains both TOTP and Backup, and also allows multiple TOTP tokens to be registered and organized by a name. Access to this page is authenticated via password, and cached for 30 minutes via a secure session. --- .../preferences/second-factor.js.es6 | 159 +++++++++----- .../controllers/second-factor-add-totp.js.es6 | 67 ++++++ ...s.es6 => second-factor-backup-edit.js.es6} | 69 +++--- .../controllers/second-factor-edit.js.es6 | 53 +++++ .../javascripts/discourse/models/user.js.es6 | 52 +++-- .../preferences-second-factor-backup.js.es6 | 21 -- .../routes/preferences-second-factor.js.es6 | 17 ++ .../modal/second-factor-add-totp.hbs | 51 +++++ .../modal/second-factor-backup-edit.hbs | 50 +++++ .../templates/modal/second-factor-edit.hbs | 15 ++ .../preferences-second-factor-backup.hbs | 71 ------- .../templates/preferences-second-factor.hbs | 199 +++++++++--------- .../templates/preferences/account.hbs | 29 +-- app/assets/stylesheets/common/base/user.scss | 15 ++ app/controllers/users_controller.rb | 141 ++++++++----- app/models/concerns/second_factor_manager.rb | 46 ++-- app/models/user_second_factor.rb | 12 +- config/locales/client.en.yml | 16 +- config/routes.rb | 5 +- ...7211829_add_name_to_user_second_factors.rb | 7 + .../concern/second_factor_manager_spec.rb | 26 +-- spec/requests/admin/users_controller_spec.rb | 4 +- spec/requests/users_controller_spec.rb | 140 ++++-------- .../enforce-second-factor-test.js.es6 | 10 +- .../acceptance/preferences-test.js.es6 | 67 ++++-- 25 files changed, 793 insertions(+), 549 deletions(-) create mode 100644 app/assets/javascripts/discourse/controllers/second-factor-add-totp.js.es6 rename app/assets/javascripts/discourse/controllers/{preferences/second-factor-backup.js.es6 => second-factor-backup-edit.js.es6} (57%) create mode 100644 app/assets/javascripts/discourse/controllers/second-factor-edit.js.es6 delete mode 100644 app/assets/javascripts/discourse/routes/preferences-second-factor-backup.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/modal/second-factor-add-totp.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/second-factor-backup-edit.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/second-factor-edit.hbs delete mode 100644 app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs create mode 100644 db/migrate/20190427211829_add_name_to_user_second_factors.rb 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 82be216d0d9..85129d453eb 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 @@ -1,37 +1,28 @@ import { default as computed } from "ember-addons/ember-computed-decorators"; +import CanCheckEmails from "discourse/mixins/can-check-emails"; 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"; +import showModal from "discourse/lib/show-modal"; -export default Ember.Controller.extend({ +export default Ember.Controller.extend(CanCheckEmails, { loading: false, + dirty: false, resetPasswordLoading: false, resetPasswordProgress: "", password: null, - secondFactorImage: null, - secondFactorKey: null, - showSecondFactorKey: false, errorMessage: null, newUsername: null, backupEnabled: Ember.computed.alias("model.second_factor_backup_enabled"), secondFactorMethod: SECOND_FACTOR_METHODS.TOTP, + totps: null, loaded: Ember.computed.and("secondFactorImage", "secondFactorKey"), - @computed("loading") - submitButtonText(loading) { - return loading ? "loading" : "continue"; - }, - - @computed("loading") - enableButtonText(loading) { - return loading ? "loading" : "enable"; - }, - - @computed("loading") - disableButtonText(loading) { - return loading ? "loading" : "disable"; + init() { + this._super(...arguments); + this.set("totps", []); }, @computed @@ -41,58 +32,64 @@ export default Ember.Controller.extend({ @computed("currentUser") showEnforcedNotice(user) { - return user && user.get("enforcedSecondFactor"); + return user && user.enforcedSecondFactor; }, - toggleSecondFactor(enable) { - if (!this.secondFactorToken) return; + handleError(error) { + if (error.jqXHR) { + error = error.jqXHR; + } + let parsedJSON = error.responseJSON; + if (parsedJSON.error_type === "invalid_access") { + const usernameLower = this.model.username.toLowerCase(); + DiscourseURL.redirectTo( + userPath(`${usernameLower}/preferences/second-factor`) + ); + } else { + popupAjaxError(error); + } + }, + + loadSecondFactors() { + if (this.dirty === false) { + return; + } this.set("loading", true); this.model - .toggleSecondFactor( - this.secondFactorToken, - this.secondFactorMethod, - SECOND_FACTOR_METHODS.TOTP, - enable - ) + .loadSecondFactorCodes(this.password) .then(response => { if (response.error) { this.set("errorMessage", response.error); return; } - this.set("errorMessage", null); - DiscourseURL.redirectTo( - userPath(`${this.model.username.toLowerCase()}/preferences`) + this.setProperties({ + errorMessage: null, + loaded: true, + totps: response.totps, + password: null, + dirty: false + }); + this.set( + "model.second_factor_enabled", + response.totps && response.totps.length > 0 ); }) - .catch(error => { - popupAjaxError(error); - }) + .catch(e => this.handleError(e)) .finally(() => this.set("loading", false)); }, + markDirty() { + this.set("dirty", true); + }, + actions: { confirmPassword() { if (!this.password) return; - this.set("loading", true); - - this.model - .loadSecondFactorCodes(this.password) - .then(response => { - if (response.error) { - this.set("errorMessage", response.error); - return; - } - - this.setProperties({ - errorMessage: null, - secondFactorKey: response.key, - secondFactorImage: response.qr - }); - }) - .catch(popupAjaxError) - .finally(() => this.set("loading", false)); + this.markDirty(); + this.loadSecondFactors(); + this.set("password", null); }, resetPassword() { @@ -113,16 +110,66 @@ export default Ember.Controller.extend({ .finally(() => this.set("resetPasswordLoading", false)); }, - showSecondFactorKey() { - this.set("showSecondFactorKey", true); + disableAllSecondFactors() { + if (this.loading) { + return; + } + bootbox.confirm( + I18n.t("user.second_factor.disable_confirm"), + I18n.t("cancel"), + I18n.t("user.second_factor.disable"), + result => { + if (result) { + this.model + .disableAllSecondFactors() + .then(() => { + const usernameLower = this.model.username.toLowerCase(); + DiscourseURL.redirectTo( + userPath(`${usernameLower}/preferences`) + ); + }) + .catch(e => this.handleError(e)) + .finally(() => this.set("loading", false)); + } + } + ); }, - enableSecondFactor() { - this.toggleSecondFactor(true); + createTotp() { + const controller = showModal("second-factor-add-totp", { + model: this.model, + title: "user.second_factor.totp.add" + }); + controller.setProperties({ + onClose: () => this.loadSecondFactors(), + markDirty: () => this.markDirty(), + onError: e => this.handleError(e) + }); }, - disableSecondFactor() { - this.toggleSecondFactor(false); + editSecondFactor(second_factor) { + const controller = showModal("second-factor-edit", { + model: second_factor, + title: "user.second_factor.edit_title" + }); + controller.setProperties({ + user: this.model, + onClose: () => this.loadSecondFactors(), + markDirty: () => this.markDirty(), + onError: e => this.handleError(e) + }); + }, + + editSecondFactorBackup() { + const controller = showModal("second-factor-backup-edit", { + model: this.model, + title: "user.second_factor_backup.title" + }); + controller.setProperties({ + onClose: () => this.loadSecondFactors(), + markDirty: () => this.markDirty(), + onError: e => this.handleError(e) + }); } } }); diff --git a/app/assets/javascripts/discourse/controllers/second-factor-add-totp.js.es6 b/app/assets/javascripts/discourse/controllers/second-factor-add-totp.js.es6 new file mode 100644 index 00000000000..e826f934cbf --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/second-factor-add-totp.js.es6 @@ -0,0 +1,67 @@ +import ModalFunctionality from "discourse/mixins/modal-functionality"; + +export default Ember.Controller.extend(ModalFunctionality, { + loading: false, + secondFactorImage: null, + secondFactorKey: null, + showSecondFactorKey: false, + errorMessage: null, + + onShow() { + this.setProperties({ + errorMessage: null, + secondFactorKey: null, + secondFactorToken: null, + showSecondFactorKey: false, + secondFactorImage: null, + loading: true + }); + this.model + .createSecondFactorTotp() + .then(response => { + if (response.error) { + this.set("errorMessage", response.error); + return; + } + + this.setProperties({ + errorMessage: null, + secondFactorKey: response.key, + secondFactorImage: response.qr + }); + }) + .catch(error => { + this.send("closeModal"); + this.onError(error); + }) + .finally(() => this.set("loading", false)); + }, + + actions: { + showSecondFactorKey() { + this.set("showSecondFactorKey", true); + }, + + enableSecondFactor() { + if (!this.secondFactorToken) return; + this.set("loading", true); + + this.model + .enableSecondFactorTotp( + this.secondFactorToken, + I18n.t("user.second_factor.totp.default_name") + ) + .then(response => { + if (response.error) { + this.set("errorMessage", response.error); + return; + } + this.markDirty(); + this.set("errorMessage", null); + this.send("closeModal"); + }) + .catch(error => this.onError(error)) + .finally(() => this.set("loading", false)); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/preferences/second-factor-backup.js.es6 b/app/assets/javascripts/discourse/controllers/second-factor-backup-edit.js.es6 similarity index 57% rename from app/assets/javascripts/discourse/controllers/preferences/second-factor-backup.js.es6 rename to app/assets/javascripts/discourse/controllers/second-factor-backup-edit.js.es6 index e5817ac9fa3..e11f484fbc5 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/second-factor-backup.js.es6 +++ b/app/assets/javascripts/discourse/controllers/second-factor-backup-edit.js.es6 @@ -1,9 +1,8 @@ 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"; +import ModalFunctionality from "discourse/mixins/modal-functionality"; -export default Ember.Controller.extend({ +export default Ember.Controller.extend(ModalFunctionality, { loading: false, errorMessage: null, successMessage: null, @@ -14,25 +13,6 @@ export default Ember.Controller.extend({ backupCodes: null, secondFactorMethod: SECOND_FACTOR_METHODS.TOTP, - @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") - isDisabledGenerateBackupCodeBtn(isValid, backupEnabled, loading) { - return !isValid || loading; - }, - - @computed("isValidSecondFactorToken", "backupEnabled", "loading") - isDisabledDisableBackupCodeBtn(isValid, backupEnabled, loading) { - return !isValid || !backupEnabled || loading; - }, - @computed("backupEnabled") generateBackupCodeBtnLabel(backupEnabled) { return backupEnabled @@ -40,6 +20,15 @@ export default Ember.Controller.extend({ : "user.second_factor_backup.enable"; }, + onShow() { + this.setProperties({ + loading: false, + errorMessage: null, + successMessage: null, + backupCodes: null + }); + }, + actions: { copyBackupCode(successful) { if (successful) { @@ -59,18 +48,10 @@ export default Ember.Controller.extend({ disableSecondFactorBackup() { this.set("backupCodes", []); - - if (!this.secondFactorToken) return; - this.set("loading", true); this.model - .toggleSecondFactor( - this.secondFactorToken, - this.secondFactorMethod, - SECOND_FACTOR_METHODS.BACKUP_CODE, - false - ) + .updateSecondFactor(0, "", true, SECOND_FACTOR_METHODS.BACKUP_CODE) .then(response => { if (response.error) { this.set("errorMessage", response.error); @@ -78,28 +59,28 @@ export default Ember.Controller.extend({ } this.set("errorMessage", null); - - const usernameLower = this.model.username.toLowerCase(); - DiscourseURL.redirectTo(userPath(`${usernameLower}/preferences`)); + this.model.set("second_factor_backup_enabled", false); + this.markDirty(); + this.send("closeModal"); + }) + .catch(error => { + this.send("closeModal"); + this.onError(error); }) - .catch(popupAjaxError) .finally(() => this.set("loading", false)); }, generateSecondFactorCodes() { - if (!this.secondFactorToken) return; this.set("loading", true); this.model - .generateSecondFactorCodes( - this.secondFactorToken, - this.secondFactorMethod - ) + .generateSecondFactorCodes() .then(response => { if (response.error) { this.set("errorMessage", response.error); return; } + this.markDirty(); this.setProperties({ errorMessage: null, backupCodes: response.backup_codes, @@ -107,11 +88,13 @@ export default Ember.Controller.extend({ remainingCodes: response.backup_codes.length }); }) - .catch(popupAjaxError) + .catch(error => { + this.send("closeModal"); + this.onError(error); + }) .finally(() => { this.setProperties({ - loading: false, - secondFactorToken: null + loading: false }); }); } diff --git a/app/assets/javascripts/discourse/controllers/second-factor-edit.js.es6 b/app/assets/javascripts/discourse/controllers/second-factor-edit.js.es6 new file mode 100644 index 00000000000..e39f0f2b659 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/second-factor-edit.js.es6 @@ -0,0 +1,53 @@ +import ModalFunctionality from "discourse/mixins/modal-functionality"; + +export default Ember.Controller.extend(ModalFunctionality, { + actions: { + disableSecondFactor() { + this.user + .updateSecondFactor( + this.model.id, + this.model.name, + true, + this.model.method + ) + .then(response => { + if (response.error) { + return; + } + this.markDirty(); + }) + .catch(error => { + this.send("closeModal"); + this.onError(error); + }) + .finally(() => { + this.set("loading", false); + this.send("closeModal"); + }); + }, + + editSecondFactor() { + this.user + .updateSecondFactor( + this.model.id, + this.model.name, + false, + this.model.method + ) + .then(response => { + if (response.error) { + return; + } + this.markDirty(); + }) + .catch(error => { + this.send("closeModal"); + this.onError(error); + }) + .finally(() => { + this.set("loading", false); + this.send("closeModal"); + }); + } + } +}); diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index b9064d13f62..21603e8ed96 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -205,17 +205,13 @@ const User = RestModel.extend({ return suspendedTill && moment(suspendedTill).isAfter(); }, - @computed("suspended_till") - suspendedForever: isForever, + @computed("suspended_till") suspendedForever: isForever, - @computed("silenced_till") - silencedForever: isForever, + @computed("silenced_till") silencedForever: isForever, - @computed("suspended_till") - suspendedTillDate: longDate, + @computed("suspended_till") suspendedTillDate: longDate, - @computed("silenced_till") - silencedTillDate: longDate, + @computed("silenced_till") silencedTillDate: longDate, changeUsername(new_username) { return ajax(userPath(`${this.username_lower}/preferences/username`), { @@ -366,6 +362,40 @@ const User = RestModel.extend({ }); }, + createSecondFactorTotp() { + return ajax("/u/create_second_factor_totp.json", { + type: "POST" + }); + }, + + enableSecondFactorTotp(authToken, name) { + return ajax("/u/enable_second_factor_totp.json", { + data: { + second_factor_token: authToken, + name + }, + type: "POST" + }); + }, + + disableAllSecondFactors() { + return ajax("/u/disable_second_factor.json", { + type: "PUT" + }); + }, + + updateSecondFactor(id, name, disable, targetMethod) { + return ajax("/u/second_factor.json", { + data: { + second_factor_target: targetMethod, + name, + disable, + id + }, + type: "PUT" + }); + }, + toggleSecondFactor(authToken, authMethod, targetMethod, enable) { return ajax("/u/second_factor.json", { data: { @@ -378,12 +408,8 @@ const User = RestModel.extend({ }); }, - generateSecondFactorCodes(authToken, authMethod) { + generateSecondFactorCodes() { return ajax("/u/second_factors_backup.json", { - data: { - second_factor_token: authToken, - second_factor_method: authMethod - }, type: "PUT" }); }, diff --git a/app/assets/javascripts/discourse/routes/preferences-second-factor-backup.js.es6 b/app/assets/javascripts/discourse/routes/preferences-second-factor-backup.js.es6 deleted file mode 100644 index afe26e906a8..00000000000 --- a/app/assets/javascripts/discourse/routes/preferences-second-factor-backup.js.es6 +++ /dev/null @@ -1,21 +0,0 @@ -import RestrictedUserRoute from "discourse/routes/restricted-user"; - -export default RestrictedUserRoute.extend({ - showFooter: true, - - model() { - return this.modelFor("user"); - }, - - renderTemplate() { - return this.render({ into: "user" }); - }, - - setupController(controller, model) { - controller.setProperties({ model, newUsername: model.get("username") }); - }, - - deactivate() { - this.controller.setProperties({ backupCodes: null }); - } -}); diff --git a/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 b/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 index 703bcf32c37..c9f2bc8250d 100644 --- a/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 +++ b/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 @@ -13,6 +13,23 @@ export default RestrictedUserRoute.extend({ setupController(controller, model) { controller.setProperties({ model, newUsername: model.get("username") }); + controller.set("loading", true); + model + .loadSecondFactorCodes("") + .then(response => { + if (response.error) { + controller.set("errorMessage", response.error); + } else { + controller.setProperties({ + errorMessage: null, + loaded: !response.password_required, + dirty: !!response.password_required, + totps: response.totps + }); + } + }) + .catch(controller.popupAjaxError) + .finally(() => controller.set("loading", false)); }, actions: { diff --git a/app/assets/javascripts/discourse/templates/modal/second-factor-add-totp.hbs b/app/assets/javascripts/discourse/templates/modal/second-factor-add-totp.hbs new file mode 100644 index 00000000000..4f44cd0678e --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/second-factor-add-totp.hbs @@ -0,0 +1,51 @@ +{{#d-modal-body}} + {{#conditional-loading-spinner condition=loading}} + {{#if errorMessage}} +
+
+
{{errorMessage}}
+
+
+ {{/if}} + +
+
+ {{{i18n 'user.second_factor.enable_description'}}} +
+
+ +
+
+
+
+ {{{secondFactorImage}}} +
+
+ +

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

+
+
+ +
+ + +
+ {{second-factor-input maxlength=6 value=secondFactorToken inputId='second-factor-token'}} +
+
+ +
+
+ {{d-button action=(action "enableSecondFactor") + class="btn btn-primary add-totp" + label="enable"}} +
+
+ {{/conditional-loading-spinner}} +{{/d-modal-body}} diff --git a/app/assets/javascripts/discourse/templates/modal/second-factor-backup-edit.hbs b/app/assets/javascripts/discourse/templates/modal/second-factor-backup-edit.hbs new file mode 100644 index 00000000000..491f13e8f94 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/second-factor-backup-edit.hbs @@ -0,0 +1,50 @@ +{{#d-modal-body}} +
+
+ {{#if successMessage}} +
+ {{successMessage}} +
+ {{/if}} + + {{#if errorMessage}} +
+ {{errorMessage}} +
+ {{/if}} + + {{#if backupEnabled}} + {{{i18n "user.second_factor_backup.remaining_codes" count=remainingCodes}}} + {{/if}} + +
+ {{d-button + action=(action "generateSecondFactorCodes") + class="btn btn-primary" + disabled=loading + label=generateBackupCodeBtnLabel}} + {{#if backupEnabled}} + {{d-button + action=(action "disableSecondFactorBackup") + class="btn btn-danger" + disabled=loading + label="user.second_factor_backup.disable"}} + {{/if}} +
+ + {{#conditional-loading-section isLoading=loading}} + {{#if backupCodes}} +

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

+ +

+ {{i18n "user.second_factor_backup.codes.description"}} +

+ + {{backup-codes + copyBackupCode=(action "copyBackupCode") + backupCodes=backupCodes}} + {{/if}} + {{/conditional-loading-section}} +
+
+{{/d-modal-body}} diff --git a/app/assets/javascripts/discourse/templates/modal/second-factor-edit.hbs b/app/assets/javascripts/discourse/templates/modal/second-factor-edit.hbs new file mode 100644 index 00000000000..48a4670230f --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/second-factor-edit.hbs @@ -0,0 +1,15 @@ +{{#d-modal-body}} +
+ {{input type="text" value=model.name}} +
+
+ {{i18n 'user.second_factor.edit_description'}} +
+ {{d-button action=(action "editSecondFactor") + class="btn-primary" + label="user.second_factor.edit"}} + + {{d-button action=(action "disableSecondFactor") + class="btn-danger" + label="user.second_factor.disable"}} +{{/d-modal-body}} diff --git a/app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs b/app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs deleted file mode 100644 index 20201f18117..00000000000 --- a/app/assets/javascripts/discourse/templates/preferences-second-factor-backup.hbs +++ /dev/null @@ -1,71 +0,0 @@ -
-
-
- -
- {{#if successMessage}} -
- {{successMessage}} -
- {{/if}} - - {{#if errorMessage}} -
- {{errorMessage}} -
- {{/if}} -
-
- -
-
- {{#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}} -
-
- -
-
-
- {{d-button - action=(action "generateSecondFactorCodes") - class="btn btn-primary" - disabled=isDisabledGenerateBackupCodeBtn - label=generateBackupCodeBtnLabel}} - {{#if backupEnabled}} - {{d-button - action=(action "disableSecondFactorBackup") - class="btn btn-danger" - disabled=isDisabledDisableBackupCodeBtn - label="user.second_factor_backup.disable"}} - {{/if}} -
- - {{#conditional-loading-section isLoading=loading}} - {{#if backupCodes}} -

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

- -

- {{i18n "user.second_factor_backup.codes.description"}} -

- - {{backup-codes - copyBackupCode=(action "copyBackupCode") - backupCodes=backupCodes}} - - {{#link-to "preferences.account" model.username}} - {{i18n "go_back"}} - {{/link-to}} - {{/if}} - {{/conditional-loading-section}} -
-
-
-
diff --git a/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs b/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs index 199e93c4197..37f89417c37 100644 --- a/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs +++ b/app/assets/javascripts/discourse/templates/preferences-second-factor.hbs @@ -1,4 +1,5 @@ -
+
+ {{#conditional-loading-spinner condition=loading}}
{{#if showEnforcedNotice}} @@ -9,6 +10,14 @@ {{/if}} + {{#if displayOAuthWarning}} +
+
+ {{i18n 'user.second_factor.oauth_enabled_warning'}} +
+
+ {{/if}} + {{#if errorMessage}}
@@ -17,121 +26,105 @@
{{/if}} - {{#if model.second_factor_enabled}} + {{#if loaded}}
- {{#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}} +

{{i18n "user.second_factor.totp.title"}}

+ {{d-button action=(action "createTotp") + class="btn-primary new-totp" + disabled=loading + label="user.second_factor.totp.add"}} + {{#each totps as |totp|}} +
+ {{totp.name}} + + {{#if isCurrentUser}} + {{d-button action=(action "editSecondFactor" totp) + class="btn-default btn-small btn-icon pad-left no-text edit" + disabled=loading + icon="pencil-alt" + }} + {{/if}} +
+ {{/each}} +
+
+ +
+
+

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

+ {{#if model.second_factor_enabled}} + {{#if model.second_factor_backup_enabled}} + {{{i18n 'user.second_factor_backup.manage' count=model.second_factor_remaining_backup_codes}}} + {{else}} + {{i18n 'user.second_factor_backup.enable_long'}} + {{/if}} + + {{#if isCurrentUser}} + {{d-button action=(action "editSecondFactorBackup") + class="btn-default btn-small btn-icon pad-left no-text edit edit-2fa-backup" + disabled=loading + icon="pencil-alt" + }} + {{/if}} + {{else}} + {{i18n "user.second_factor_backup.enable_prerequisites"}} + {{/if}} +
+
+ + {{#if model.second_factor_enabled}} + {{#unless showEnforcedNotice}} +
+
+

{{i18n "user.second_factor.disable_title"}}

+ {{d-button action=(action "disableAllSecondFactors") + class="btn btn-danger" + disabled=loading + label="disable"}} +
+
+ {{/unless}} + {{/if}} + {{else}} +
+ + +
+
+ {{text-field value=password + id="password" + type="password" + classNames="input-xxlarge" + autofocus="autofocus"}} +
+
+ {{i18n 'user.second_factor.confirm_password_description'}} +
- {{d-button action=(action "disableSecondFactor") - class="btn btn-primary" - disabled=loading - label=disableButtonText}} + {{d-button action=(action "confirmPassword") + class="btn-primary" + disabled=loading + label="continue"}} + + {{d-button action=(action "resetPassword") + class="btn" + disabled=resetPasswordLoading + icon="envelope" + label='user.change_password.action'}} + + {{resetPasswordProgress}} {{#unless showEnforcedNotice}} {{cancel-link route="preferences.account" args= model.username}} {{/unless}}
- {{else}} - {{#if loaded}} -
-
- {{{i18n 'user.second_factor.enable_description'}}} - - {{#if displayOAuthWarning}} - {{i18n 'user.second_factor.oauth_enabled_warning'}} - {{/if}} -
-
- -
-
-
-
- {{{secondFactorImage}}} -
-
- -

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

-
-
- -
- - -
- {{second-factor-input maxlength=6 value=secondFactorToken inputId='second-factor-token'}} -
-
- -
-
- {{d-button action=(action "enableSecondFactor") - class="btn btn-primary" - disabled=loading - label=enableButtonText}} - - {{#unless showEnforcedNotice}} - {{cancel-link route="preferences.account" args= model.username}} - {{/unless}} -
-
- {{else}} -
- - -
-
- {{text-field value=password - id="password" - type="password" - classNames="input-xxlarge" - autofocus="autofocus"}} -
-
- {{i18n 'user.second_factor.confirm_password_description'}} -
-
-
- -
-
- {{d-button action=(action "confirmPassword") - class="btn btn-primary" - disabled=loading - label=submitButtonText}} - - {{d-button action=(action "resetPassword") - class="btn" - disabled=resetPasswordLoading - icon="envelope" - label='user.change_password.action'}} - - {{resetPasswordProgress}} - - {{#unless showEnforcedNotice}} - {{cancel-link route="preferences.account" args= model.username}} - {{/unless}} -
-
- {{/if}} {{/if}} + {{/conditional-loading-spinner}}
diff --git a/app/assets/javascripts/discourse/templates/preferences/account.hbs b/app/assets/javascripts/discourse/templates/preferences/account.hbs index 7a1aef4dccb..666e147fcae 100644 --- a/app/assets/javascripts/discourse/templates/preferences/account.hbs +++ b/app/assets/javascripts/discourse/templates/preferences/account.hbs @@ -90,32 +90,11 @@ {{/unless}}
+ {{i18n 'user.second_factor.enable'}} {{#if isCurrentUser}} - {{#if model.second_factor_enabled}} - {{#link-to "preferences.second-factor" class="btn btn-default"}} - {{d-icon "unlock"}} {{i18n 'user.second_factor.disable'}} - {{/link-to}} - {{else}} - {{#link-to "preferences.second-factor" class="btn btn-default"}} - {{d-icon "lock"}} {{i18n 'user.second_factor.enable'}} - {{/link-to}} - {{/if}} - {{/if}} -
- -
- {{#if model.second_factor_enabled}} - {{#if isCurrentUser}} - {{#link-to "preferences.second-factor-backup"}} - - {{#if model.second_factor_backup_enabled}} - {{i18n 'user.second_factor_backup.manage'}} - {{else}} - {{i18n 'user.second_factor_backup.enable_long'}} - {{/if}} - - {{/link-to}} - {{/if}} + {{#link-to "preferences.second-factor" class="btn btn-default"}} + {{d-icon "lock"}} {{i18n 'user.second_factor.enable'}} + {{/link-to}} {{/if}}
diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss index 676baa76324..7fb618214e9 100644 --- a/app/assets/stylesheets/common/base/user.scss +++ b/app/assets/stylesheets/common/base/user.scss @@ -695,6 +695,21 @@ } } +.second-factor { + &.instructions { + color: $primary-medium; + margin-top: 5px; + margin-bottom: 10px; + font-size: $font-down-1; + } + .second-factor-item { + margin-top: 0.75em; + } + .btn.edit { + min-height: auto; + } +} + .primary-textual .staged, #user-card .staged { font-style: italic; diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3b1ba66f8e9..bd0bf7ae62d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -14,7 +14,8 @@ class UsersController < ApplicationController requires_login only: [ :username, :update, :user_preferences_redirect, :upload_user_image, :pick_avatar, :destroy_user_image, :destroy, :check_emails, - :topic_tracking_state, :preferences, :create_second_factor, + :topic_tracking_state, :preferences, :create_second_factor_totp, + :enable_second_factor_totp, :disable_second_factor, :list_second_factors, :update_second_factor, :create_second_factor_backup, :select_avatar, :notification_level, :revoke_auth_token ] @@ -25,6 +26,10 @@ class UsersController < ApplicationController :my_redirect, :toggle_anon, :admin_login, :confirm_admin, :email_login, :summary ] + before_action :second_factor_check_confirmed_password, only: [ + :create_second_factor_totp, :enable_second_factor_totp, + :disable_second_factor, :update_second_factor, :create_second_factor_backup] + before_action :respond_to_suspicious_request, only: [:create] # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the @@ -1063,39 +1068,32 @@ class UsersController < ApplicationController render layout: 'no_ember' end - def create_second_factor + def list_second_factors raise Discourse::NotFound if SiteSetting.enable_sso || !SiteSetting.enable_local_logins - 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! - unless current_user.confirm_password?(params[:password]) - return render json: failed_json.merge( - error: I18n.t("login.incorrect_password") - ) + unless params[:password].empty? + 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! + unless current_user.confirm_password?(params[:password]) + return render json: failed_json.merge( + error: I18n.t("login.incorrect_password") + ) + end + secure_session["confirmed-password-#{current_user.id}"] = "true" end - qrcode_svg = RQRCode::QRCode.new(current_user.totp_provisioning_uri).as_svg( - offset: 0, - color: '000', - shape_rendering: 'crispEdges', - module_size: 4 - ) - - render json: success_json.merge( - key: current_user.user_second_factors.totp.data.scan(/.{4}/).join(" "), - qr: qrcode_svg - ) + if secure_session["confirmed-password-#{current_user.id}"] == "true" + render json: success_json.merge( + totps: current_user.totps.select(:id, :name, :last_used, :created_at, :method).order(:created_at) + ) + else + render json: success_json.merge( + password_required: true + ) + end end def create_second_factor_backup - raise Discourse::NotFound if SiteSetting.enable_sso || !SiteSetting.enable_local_logins - - 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") - ) - end - backup_codes = current_user.generate_backup_codes render json: success_json.merge( @@ -1103,54 +1101,93 @@ class UsersController < ApplicationController ) end - def update_second_factor - params.require(:second_factor_token) - params.require(:second_factor_method) - params.require(:second_factor_target) + def create_second_factor_totp + totp_data = ROTP::Base32.random_base32 + secure_session["staged-totp-#{current_user.id}"] = totp_data + qrcode_svg = RQRCode::QRCode.new(current_user.totp_provisioning_uri(totp_data)).as_svg( + offset: 0, + color: '000', + shape_rendering: 'crispEdges', + module_size: 4 + ) - auth_method = params[:second_factor_method].to_i + render json: success_json.merge( + key: totp_data.scan(/.{4}/).join(" "), + qr: qrcode_svg + ) + end + + def enable_second_factor_totp + params.require(:second_factor_token) + params.require(:name) auth_token = params[:second_factor_token] - update_second_factor_method = params[:second_factor_target].to_i + totp_data = secure_session["staged-totp-#{current_user.id}"] + totp_object = current_user.get_totp_object(totp_data) [request.remote_ip, current_user.id].each do |key| RateLimiter.new(nil, "second-factor-min-#{key}", 3, 1.minute).performed! end + authenticated = !auth_token.blank? && totp_object.verify_with_drift(auth_token, 30) + unless authenticated + return render json: failed_json.merge( + error: I18n.t("login.invalid_second_factor_code") + ) + end + current_user.create_totp(data: totp_data, name: params[:name], enabled: true) + render json: success_json + end + + def disable_second_factor + # delete all second factors for a user + current_user.user_second_factors.destroy_all + + Jobs.enqueue( + :critical_user_email, + type: :account_second_factor_disabled, + user_id: current_user.id + ) + + render json: success_json + end + + def update_second_factor + params.require(:second_factor_target) + update_second_factor_method = params[:second_factor_target].to_i + if update_second_factor_method == UserSecondFactor.methods[:totp] - user_second_factor = current_user.user_second_factors.totp + params.require(:id) + second_factor_id = params[:id].to_i + user_second_factor = current_user.user_second_factors.totps.find_by(id: second_factor_id) 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_second_factor(auth_token, auth_method) - return render json: failed_json.merge( - error: I18n.t("login.invalid_second_factor_code") - ) + if params[:name] && !params[:name].blank? + user_second_factor.update!(name: params[:name]) end - - if params[:enable] == "true" - user_second_factor.update!(enabled: true) - else - # when disabling totp, backup is disabled too - if update_second_factor_method == UserSecondFactor.methods[:totp] - current_user.user_second_factors.destroy_all - - Jobs.enqueue( - :critical_user_email, - type: :account_second_factor_disabled, - user_id: current_user.id - ) - elsif update_second_factor_method == UserSecondFactor.methods[:backup_codes] + if params[:disable] == "true" + # Disabling backup codes deletes *all* backup codes + if update_second_factor_method == UserSecondFactor.methods[:backup_codes] current_user.user_second_factors.where(method: UserSecondFactor.methods[:backup_codes]).destroy_all + else + user_second_factor.update!(enabled: false) end + end render json: success_json end + def second_factor_check_confirmed_password + raise Discourse::NotFound if SiteSetting.enable_sso || !SiteSetting.enable_local_logins + + raise Discourse::InvalidAccess.new unless current_user && secure_session["confirmed-password-#{current_user.id}"] == "true" + end + def revoke_account user = fetch_user_from_params guardian.ensure_can_edit!(user) diff --git a/app/models/concerns/second_factor_manager.rb b/app/models/concerns/second_factor_manager.rb index 1d88fa9b2fe..67caf8adf0b 100644 --- a/app/models/concerns/second_factor_manager.rb +++ b/app/models/concerns/second_factor_manager.rb @@ -3,35 +3,39 @@ module SecondFactorManager extend ActiveSupport::Concern - def totp - self.create_totp - ROTP::TOTP.new(self.user_second_factors.totp.data, issuer: SiteSetting.title) - end - def create_totp(opts = {}) - if !self.user_second_factors.totp - UserSecondFactor.create!({ - user_id: self.id, - method: UserSecondFactor.methods[:totp], - data: ROTP::Base32.random_base32 - }.merge(opts)) - end + UserSecondFactor.create!({ + user_id: self.id, + method: UserSecondFactor.methods[:totp], + data: ROTP::Base32.random_base32 + }.merge(opts)) end - def totp_provisioning_uri - self.totp.provisioning_uri(self.email) + def get_totp_object(data) + ROTP::TOTP.new(data, issuer: SiteSetting.title) + end + + def totp_provisioning_uri(data) + get_totp_object(data).provisioning_uri(self.email) end def authenticate_totp(token) - totp = self.totp - last_used = 0 + totps = self&.user_second_factors.totps + authenticated = false + totps.each do |totp| - if self.user_second_factors.totp.last_used - last_used = self.user_second_factors.totp.last_used.to_i + last_used = 0 + + if totp.last_used + last_used = totp.last_used.to_i + end + + authenticated = !token.blank? && totp.get_totp_object.verify_with_drift_and_prior(token, 30, last_used) + if authenticated + totp.update!(last_used: DateTime.now) + break + end end - - authenticated = !token.blank? && totp.verify_with_drift_and_prior(token, 30, last_used) - self.user_second_factors.totp.update!(last_used: DateTime.now) if authenticated !!authenticated end diff --git a/app/models/user_second_factor.rb b/app/models/user_second_factor.rb index 7388751976b..0d3d8ee3512 100644 --- a/app/models/user_second_factor.rb +++ b/app/models/user_second_factor.rb @@ -11,6 +11,10 @@ class UserSecondFactor < ActiveRecord::Base where(method: UserSecondFactor.methods[:totp], enabled: true) end + scope :all_totps, -> do + where(method: UserSecondFactor.methods[:totp]) + end + def self.methods @methods ||= Enum.new( totp: 1, @@ -18,8 +22,12 @@ class UserSecondFactor < ActiveRecord::Base ) end - def self.totp - where(method: self.methods[:totp]).first + def get_totp_object + ROTP::TOTP.new(self.data, issuer: SiteSetting.title) + end + + def totp_provisioning_uri + get_totp_object.provisioning_uri(user.email) end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 710d614a51d..06da3d7c24f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -927,19 +927,19 @@ en: disable: "Disable" enable: "Enable" enable_long: "Enable backup codes" - manage: "Manage backup codes" + manage: "Manage backup codes. You have {{count}} backup codes remaining." 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" + enable_prerequisites: "You must enable a primary second factor before generating backup codes." codes: title: "Backup Codes Generated" description: "Each of these backup codes can only be used once. Keep them somewhere safe but accessible." second_factor: title: "Two Factor Authentication" - disable: "Disable Two Factor Authentication" - enable: "Enable Two Factor Authentication" + enable: "Manage Two Factor Authentication" confirm_password_description: "Please confirm your password to continue" label: "Code" rate_limit: "Please wait before trying another authentication code." @@ -954,6 +954,16 @@ en: 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" enforced_notice: "You are required to enable two factor authentication before accessing this site." + disable: "disable" + disable_title: "Disable Second Factor" + disable_confirm: "Are you sure you want to disable all second factors?" + edit: "Edit" + edit_title: "Edit Second Factor" + edit_description: "Second Factor Name" + totp: + title: "Token-Based Authenticators" + add: "New Authenticator" + default_name: "My Authenticator" change_about: title: "Change About Me" diff --git a/config/routes.rb b/config/routes.rb index 5c395de1790..1cfc99dc3fe 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -373,8 +373,11 @@ Discourse::Application.routes.draw do end end - post "#{root_path}/second_factors" => "users#create_second_factor" + post "#{root_path}/second_factors" => "users#list_second_factors" put "#{root_path}/second_factor" => "users#update_second_factor" + post "#{root_path}/create_second_factor_totp" => "users#create_second_factor_totp" + post "#{root_path}/enable_second_factor_totp" => "users#enable_second_factor_totp" + put "#{root_path}/disable_second_factor" => "users#disable_second_factor" put "#{root_path}/second_factors_backup" => "users#create_second_factor_backup" diff --git a/db/migrate/20190427211829_add_name_to_user_second_factors.rb b/db/migrate/20190427211829_add_name_to_user_second_factors.rb new file mode 100644 index 00000000000..33e5e05f543 --- /dev/null +++ b/db/migrate/20190427211829_add_name_to_user_second_factors.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddNameToUserSecondFactors < ActiveRecord::Migration[5.2] + def change + add_column :user_second_factors, :name, :string + end +end diff --git a/spec/components/concern/second_factor_manager_spec.rb b/spec/components/concern/second_factor_manager_spec.rb index 2c92ec393a8..11edba1a303 100644 --- a/spec/components/concern/second_factor_manager_spec.rb +++ b/spec/components/concern/second_factor_manager_spec.rb @@ -15,11 +15,11 @@ RSpec.describe SecondFactorManager do totp = nil expect do - totp = another_user.totp + totp = another_user.create_totp(enabled: true) end.to change { UserSecondFactor.count }.by(1) - expect(totp.issuer).to eq(SiteSetting.title) - expect(totp.secret).to eq(another_user.reload.user_second_factors.totp.data) + expect(totp.get_totp_object.issuer).to eq(SiteSetting.title) + expect(totp.get_totp_object.secret).to eq(another_user.reload.user_second_factors.totps.first.data) end end @@ -31,17 +31,11 @@ RSpec.describe SecondFactorManager do expect(second_factor.data).to be_present expect(second_factor.enabled).to eq(true) end - - describe 'when user has a second factor' do - it 'should return nil' do - expect(user.create_totp).to eq(nil) - end - end end describe '#totp_provisioning_uri' do it 'should return the right uri' do - expect(user.totp_provisioning_uri).to eq( + expect(user.user_second_factors.totps.first.totp_provisioning_uri).to eq( "otpauth://totp/#{SiteSetting.title}:#{user.email}?secret=#{user_second_factor_totp.data}&issuer=#{SiteSetting.title}" ) end @@ -50,12 +44,12 @@ RSpec.describe SecondFactorManager do describe '#authenticate_totp' do it 'should be able to authenticate a token' do freeze_time do - expect(user.user_second_factors.totp.last_used).to eq(nil) + expect(user.user_second_factors.totps.first.last_used).to eq(nil) - token = user.totp.now + token = user.user_second_factors.totps.first.get_totp_object.now expect(user.authenticate_totp(token)).to eq(true) - expect(user.user_second_factors.totp.last_used).to eq_time(DateTime.now) + expect(user.user_second_factors.totps.first.last_used).to eq_time(DateTime.now) expect(user.authenticate_totp(token)).to eq(false) end end @@ -63,14 +57,14 @@ RSpec.describe SecondFactorManager do describe 'when token is blank' do it 'should be false' do expect(user.authenticate_totp(nil)).to eq(false) - expect(user.user_second_factors.totp.last_used).to eq(nil) + expect(user.user_second_factors.totps.first.last_used).to eq(nil) end end describe 'when token is invalid' do it 'should be false' do expect(user.authenticate_totp('111111')).to eq(false) - expect(user.user_second_factors.totp.last_used).to eq(nil) + expect(user.user_second_factors.totps.first.last_used).to eq(nil) end end end @@ -84,7 +78,7 @@ RSpec.describe SecondFactorManager do describe "when user's second factor record is disabled" do it 'should return false' do - user.user_second_factors.totp.update!(enabled: false) + user.user_second_factors.totps.first.update!(enabled: false) expect(user.totp_enabled?).to eq(false) end end diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 93af224a49b..22869f1388b 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -937,7 +937,7 @@ RSpec.describe Admin::UsersController do end describe '#disable_second_factor' do - let(:second_factor) { user.create_totp } + let(:second_factor) { user.create_totp(enabled: true) } let(:second_factor_backup) { user.generate_backup_codes } describe 'as an admin' do @@ -945,7 +945,7 @@ RSpec.describe Admin::UsersController do sign_in(admin) second_factor second_factor_backup - expect(user.reload.user_second_factors.totp).to eq(second_factor) + expect(user.reload.user_second_factors.totps.first).to eq(second_factor) end it 'should able to disable the second factor for another user' do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 542b81b1e97..ed177c2fff7 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -3163,7 +3163,7 @@ describe UsersController do end end - describe '#create_second_factor' do + describe '#create_second_factor_totp' do context 'when not logged in' do it 'should return the right response' do post "/users/second_factors.json", params: { @@ -3181,24 +3181,17 @@ describe UsersController do describe 'create 2fa request' do it 'fails on incorrect password' do - post "/users/second_factors.json", params: { - password: 'wrongpassword' - } + ApplicationController.any_instance.expects(:secure_session).returns("confirmed-password-#{user.id}" => "false") + post "/users/create_second_factor_totp.json" - expect(response.status).to eq(200) - - expect(JSON.parse(response.body)['error']).to eq(I18n.t( - "login.incorrect_password") - ) + expect(response.status).to eq(403) end describe 'when local logins are disabled' do it 'should return the right response' do SiteSetting.enable_local_logins = false - post "/users/second_factors.json", params: { - password: 'myawesomepassword' - } + post "/users/create_second_factor_totp.json" expect(response.status).to eq(404) end @@ -3209,30 +3202,22 @@ describe UsersController do SiteSetting.sso_url = 'http://someurl.com' SiteSetting.enable_sso = true - post "/users/second_factors.json", params: { - password: 'myawesomepassword' - } + post "/users/create_second_factor_totp.json" expect(response.status).to eq(404) end end it 'succeeds on correct password' do - user.create_totp - user.user_second_factors.totp.update!(data: "abcdefghijklmnop") - - post "/users/second_factors.json", params: { - password: 'myawesomepassword' - } + session = {} + ApplicationController.any_instance.stubs(:secure_session).returns("confirmed-password-#{user.id}" => "true") + post "/users/create_second_factor_totp.json" expect(response.status).to eq(200) response_body = JSON.parse(response.body) - expect(response_body['key']).to eq( - "abcd efgh ijkl mnop" - ) - + expect(response_body['key']).to be_present expect(response_body['qr']).to be_present end end @@ -3244,10 +3229,7 @@ describe UsersController do context 'when not logged in' do it 'should return the right response' do - put "/users/second_factor.json", params: { - second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, - second_factor_method: UserSecondFactor.methods[:totp] - } + put "/users/second_factor.json" expect(response.status).to eq(403) end @@ -3263,52 +3245,39 @@ describe UsersController do context 'when token is missing' do it 'returns the right response' do put "/users/second_factor.json", params: { - enable: 'true', - } - - expect(response.status).to eq(400) - end - end - - context 'when token is invalid' do - it 'returns the right response' do - put "/users/second_factor.json", params: { - second_factor_token: '000000', - second_factor_method: UserSecondFactor.methods[:totp], + disable: 'true', second_factor_target: UserSecondFactor.methods[:totp], - enable: 'true', + id: user_second_factor.id } - expect(response.status).to eq(200) - - expect(JSON.parse(response.body)['error']).to eq(I18n.t( - "login.invalid_second_factor_code" - )) + expect(response.status).to eq(403) end end context 'when token is valid' do - it 'should allow second factor for the user to be enabled' do + before do + ApplicationController.any_instance.stubs(:secure_session).returns("confirmed-password-#{user.id}" => "true") + end + it 'should allow second factor for the user to be renamed' 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_target: UserSecondFactor.methods[:totp], - enable: 'true' - } + name: 'renamed', + second_factor_target: UserSecondFactor.methods[:totp], + id: user_second_factor.id + } expect(response.status).to eq(200) - expect(user.reload.user_second_factors.totp.enabled).to be true + expect(user.reload.user_second_factors.totps.first.name).to eq("renamed") end 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_target: UserSecondFactor.methods[:totp] + disable: 'true', + second_factor_target: UserSecondFactor.methods[:totp], + id: user_second_factor.id } expect(response.status).to eq(200) - expect(user.reload.user_second_factors.totp).to eq(nil) + expect(user.reload.user_second_factors.totps.first).to eq(nil) end end end @@ -3320,32 +3289,18 @@ describe UsersController do second_factor_target: UserSecondFactor.methods[:backup_codes] } - expect(response.status).to eq(400) - end - end - - context 'when token is invalid' do - it 'returns the right response' do - put "/users/second_factor.json", params: { - second_factor_token: '000000', - second_factor_method: UserSecondFactor.methods[:totp], - second_factor_target: UserSecondFactor.methods[:backup_codes] - } - - expect(response.status).to eq(200) - - expect(JSON.parse(response.body)['error']).to eq(I18n.t( - "login.invalid_second_factor_code" - )) + expect(response.status).to eq(403) end end context 'when token is valid' do + before do + ApplicationController.any_instance.stubs(:secure_session).returns("confirmed-password-#{user.id}" => "true") + end 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[:totp], - second_factor_target: UserSecondFactor.methods[:backup_codes] + second_factor_target: UserSecondFactor.methods[:backup_codes], + disable: 'true' } expect(response.status).to eq(200) @@ -3377,26 +3332,17 @@ 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_method: UserSecondFactor.methods[:totp] - } + ApplicationController.any_instance.expects(:secure_session).returns("confirmed-password-#{user.id}" => "false") + put "/users/second_factors_backup.json" - expect(response.status).to eq(200) - - expect(JSON.parse(response.body)['error']).to eq(I18n.t( - "login.invalid_second_factor_code") - ) + expect(response.status).to eq(403) end describe 'when local logins are disabled' do it 'should return the right response' 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_method: UserSecondFactor.methods[:totp] - } + put "/users/second_factors_backup.json" expect(response.status).to eq(404) end @@ -3407,22 +3353,16 @@ describe UsersController do SiteSetting.sso_url = 'http://someurl.com' SiteSetting.enable_sso = true - put "/users/second_factors_backup.json", params: { - second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, - second_factor_method: UserSecondFactor.methods[:totp] - } + put "/users/second_factors_backup.json" expect(response.status).to eq(404) end end it 'succeeds on correct password' do - user_second_factor + ApplicationController.any_instance.expects(:secure_session).returns("confirmed-password-#{user.id}" => "true") - put "/users/second_factors_backup.json", params: { - second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, - second_factor_method: UserSecondFactor.methods[:totp] - } + put "/users/second_factors_backup.json" expect(response.status).to eq(200) diff --git a/test/javascripts/acceptance/enforce-second-factor-test.js.es6 b/test/javascripts/acceptance/enforce-second-factor-test.js.es6 index 3b56755f92e..3f7ad666f15 100644 --- a/test/javascripts/acceptance/enforce-second-factor-test.js.es6 +++ b/test/javascripts/acceptance/enforce-second-factor-test.js.es6 @@ -1,7 +1,15 @@ import { acceptance, updateCurrentUser } from "helpers/qunit-helpers"; acceptance("Enforce Second Factor", { - loggedIn: true + loggedIn: true, + pretend(server, helper) { + server.post("/u/second_factors.json", () => { + return helper.response({ + success: "OK", + password_required: "true" + }); + }); + } }); QUnit.test("as an admin", async assert => { diff --git a/test/javascripts/acceptance/preferences-test.js.es6 b/test/javascripts/acceptance/preferences-test.js.es6 index ff4319e2c6e..56c171afc84 100644 --- a/test/javascripts/acceptance/preferences-test.js.es6 +++ b/test/javascripts/acceptance/preferences-test.js.es6 @@ -1,18 +1,26 @@ +import { acceptance, replaceCurrentUser } from "helpers/qunit-helpers"; import selectKit from "helpers/select-kit-helper"; -import { acceptance } from "helpers/qunit-helpers"; + import User from "discourse/models/user"; acceptance("User Preferences", { loggedIn: true, pretend(server, helper) { server.post("/u/second_factors.json", () => { + return helper.response({ + success: "OK", + password_required: "true" + }); + }); + + server.post("/u/create_second_factor_totp.json", () => { return helper.response({ key: "rcyryaqage3jexfj", qr: '
qr-code
' }); }); - server.put("/u/second_factor.json", () => { + server.post("/u/enable_second_factor_totp.json", () => { return helper.response({ error: "invalid token" }); }); @@ -215,12 +223,13 @@ QUnit.test("second factor", async assert => { await fillIn("#password", "secrets"); await click(".user-preferences .btn-primary"); - - assert.ok(exists("#test-qr"), "shows qr code"); assert.notOk(exists("#password"), "it hides the password input"); + await click(".new-totp"); + assert.ok(exists("#test-qr"), "shows qr code"); + await fillIn("#second-factor-token", "111111"); - await click(".btn-primary"); + await click(".add-totp"); assert.ok( find(".alert-error") @@ -230,20 +239,6 @@ QUnit.test("second factor", async assert => { ); }); -QUnit.test("second factor backup", async assert => { - await visit("/u/eviltrout/preferences/second-factor-backup"); - - assert.ok( - exists("#second-factor-token"), - "it has a authentication token input" - ); - - await fillIn("#second-factor-token", "111111"); - await click(".user-preferences .btn-primary"); - - assert.ok(exists(".backup-codes-area"), "shows backup codes"); -}); - QUnit.test("default avatar selector", async assert => { await visit("/u/eviltrout/preferences"); @@ -259,6 +254,40 @@ QUnit.test("default avatar selector", async assert => { ); }); +acceptance("Second Factor Backups", { + loggedIn: true, + pretend(server, helper) { + server.post("/u/second_factors.json", () => { + return helper.response({ + success: "OK", + totps: [{ id: 1, name: "one of them" }] + }); + }); + + server.put("/u/second_factors_backup.json", () => { + return helper.response({ + backup_codes: ["dsffdsd", "fdfdfdsf", "fddsds"] + }); + }); + + server.get("/u/eviltrout/activity.json", () => { + return helper.response({}); + }); + } +}); +QUnit.test("second factor backup", async assert => { + replaceCurrentUser({ second_factor_enabled: true }); + await visit("/u/eviltrout/preferences/second-factor"); + await click(".edit-2fa-backup"); + assert.ok( + exists(".second-factor-backup-preferences"), + "shows the 2fa backup panel" + ); + await click(".second-factor-backup-preferences .btn-primary"); + + assert.ok(exists(".backup-codes-area"), "shows backup codes"); +}); + acceptance("Avatar selector when selectable avatars is enabled", { loggedIn: true, settings: { selectable_avatars_enabled: true },