From 68d35b14f4b5193c1bae0f175b9e461152e13ac7 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 2 Oct 2019 12:08:41 +1000 Subject: [PATCH] FEATURE: Webauthn authenticator management with 2FA login (Security Keys) (#8099) Adds 2 factor authentication method via second factor security keys over [web authn](https://developer.mozilla.org/en-US/docs/Web/API/Web_Authentication_API). Allows a user to authenticate a second factor on login, login-via-email, admin-login, and change password routes. Adds registration area within existing user second factor preferences to register multiple security keys. Supports both external (yubikey) and built-in (macOS/android fingerprint readers). --- Gemfile | 2 + Gemfile.lock | 5 + .../admin-login/admin-login.js.es6 | 46 +++++ .../admin-login/admin-login.no-module.js.es6 | 1 + .../components/second-factor-form.js.es6 | 29 ++- .../components/security-key-form.js.es6 | 11 + .../discourse/controllers/email-login.js.es6 | 43 +++- .../discourse/controllers/login.js.es6 | 65 ++++-- .../controllers/password-reset.js.es6 | 39 +++- .../preferences/second-factor.js.es6 | 29 ++- .../second-factor-add-security-key.js.es6 | 123 ++++++++++++ .../second-factor-edit-security-key.js.es6 | 42 ++++ .../javascripts/discourse/lib/webauthn.js.es6 | 74 +++++++ .../javascripts/discourse/models/user.js.es6 | 30 ++- .../routes/preferences-second-factor.js.es6 | 4 +- .../components/second-factor-form.hbs | 7 +- .../components/security-key-form.hbs | 14 ++ .../discourse/templates/email-login.hbs | 32 ++- .../discourse/templates/modal/login.hbs | 35 +++- .../modal/second-factor-add-security-key.hbs | 31 +++ .../modal/second-factor-edit-security-key.hbs | 15 ++ .../discourse/templates/password-reset.hbs | 33 ++- .../templates/preferences-second-factor.hbs | 27 +++ app/assets/stylesheets/desktop/login.scss | 3 +- app/controllers/session_controller.rb | 80 +++++++- app/controllers/users_controller.rb | 181 ++++++++++++++--- app/models/concerns/second_factor_manager.rb | 6 + app/models/user.rb | 18 ++ app/models/user_security_key.rb | 38 ++++ .../_second_factor_form_script.html.erb | 18 -- app/views/users/admin_login.html.erb | 92 +++++---- config/application.rb | 4 + config/locales/client.en.yml | 27 ++- config/locales/server.en.yml | 23 +++ config/routes.rb | 4 + ...0190904104533_create_user_security_keys.rb | 26 +++ ...5_add_secure_identifier_column_to_users.rb | 8 + ..._add_enabled_index_to_user_security_key.rb | 7 + .../security_key_authentication_service.rb | 87 ++++++++ .../security_key_base_validation_service.rb | 67 +++++++ .../security_key_registration_service.rb | 150 ++++++++++++++ lib/webauthn/webauthn.rb | 26 +++ .../user_security_key_fabricator.rb | 22 ++ ...ecurity_key_authentication_service_spec.rb | 134 +++++++++++++ .../security_key_registration_service_spec.rb | 155 ++++++++++++++ spec/models/user_spec.rb | 43 ++++ spec/requests/session_controller_spec.rb | 17 ++ spec/requests/users_controller_spec.rb | 189 +++++++++++++++++- .../acceptance/sign-in-test.js.es6 | 26 +++ .../helpers/create-pretender.js.es6 | 14 ++ 50 files changed, 2041 insertions(+), 161 deletions(-) create mode 100644 app/assets/javascripts/admin-login/admin-login.js.es6 create mode 100644 app/assets/javascripts/admin-login/admin-login.no-module.js.es6 create mode 100644 app/assets/javascripts/discourse/components/security-key-form.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/second-factor-add-security-key.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/second-factor-edit-security-key.js.es6 create mode 100644 app/assets/javascripts/discourse/lib/webauthn.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/security-key-form.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/second-factor-add-security-key.hbs create mode 100644 app/assets/javascripts/discourse/templates/modal/second-factor-edit-security-key.hbs create mode 100644 app/models/user_security_key.rb delete mode 100644 app/views/common/_second_factor_form_script.html.erb create mode 100644 db/migrate/20190904104533_create_user_security_keys.rb create mode 100644 db/migrate/20190908233325_add_secure_identifier_column_to_users.rb create mode 100644 db/migrate/20190917100006_add_enabled_index_to_user_security_key.rb create mode 100644 lib/webauthn/security_key_authentication_service.rb create mode 100644 lib/webauthn/security_key_base_validation_service.rb create mode 100644 lib/webauthn/security_key_registration_service.rb create mode 100644 lib/webauthn/webauthn.rb create mode 100644 spec/fabricators/user_security_key_fabricator.rb create mode 100644 spec/lib/webauthn/security_key_authentication_service_spec.rb create mode 100644 spec/lib/webauthn/security_key_registration_service_spec.rb diff --git a/Gemfile b/Gemfile index 98e165f3ac9..8c54f882909 100644 --- a/Gemfile +++ b/Gemfile @@ -114,6 +114,8 @@ gem 'execjs', require: false gem 'mini_racer' gem 'highline', '~> 1.7.0', require: false gem 'rack-protection' # security +gem 'cbor', require: false +gem 'cose', require: false # Gems used only for assets and not required in production environments by default. # Allow everywhere for now cause we are allowing asset debugging in production diff --git a/Gemfile.lock b/Gemfile.lock index e859be90ef2..d7fcb79f8d5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -77,12 +77,15 @@ GEM activesupport (>= 3.0.0) uniform_notifier (~> 1.11) byebug (11.0.1) + cbor (0.5.9.6) certified (1.0.0) chunky_png (1.3.11) coderay (1.1.2) colored2 (3.1.2) concurrent-ruby (1.1.5) connection_pool (2.2.2) + cose (0.9.0) + cbor (~> 0.5.9) cppjieba_rb (0.3.3) crack (0.4.3) safe_yaml (~> 1.0.0) @@ -438,8 +441,10 @@ DEPENDENCIES bootsnap bullet byebug + cbor certified colored2 + cose cppjieba_rb css_parser diffy diff --git a/app/assets/javascripts/admin-login/admin-login.js.es6 b/app/assets/javascripts/admin-login/admin-login.js.es6 new file mode 100644 index 00000000000..9481f7328e1 --- /dev/null +++ b/app/assets/javascripts/admin-login/admin-login.js.es6 @@ -0,0 +1,46 @@ +import { getWebauthnCredential } from "discourse/lib/webauthn"; + +export default function() { + document.getElementById( + "activate-security-key-alternative" + ).onclick = function() { + document.getElementById("second-factor-forms").style.display = "block"; + document.getElementById("primary-security-key-form").style.display = "none"; + }; + + document.getElementById("submit-security-key").onclick = function(e) { + e.preventDefault(); + getWebauthnCredential( + document.getElementById("security-key-challenge").value, + document + .getElementById("security-key-allowed-credential-ids") + .value.split(","), + credentialData => { + document.getElementById( + "security-key-credential" + ).value = JSON.stringify(credentialData); + e.target.parentElement.submit(); + }, + errorMessage => { + document.getElementById("security-key-error").innerText = errorMessage; + } + ); + }; + + var useTotp = I18n.t("login.second_factor_toggle.totp"); + var useBackup = I18n.t("login.second_factor_toggle.backup_code"); + var backupForm = document.getElementById("backup-second-factor-form"); + var primaryForm = document.getElementById("primary-second-factor-form"); + document.getElementById("toggle-form").onclick = function(event) { + event.preventDefault(); + if (backupForm.style.display === "none") { + backupForm.style.display = "block"; + primaryForm.style.display = "none"; + document.getElementById("toggle-form").innerHTML = useTotp; + } else { + backupForm.style.display = "none"; + primaryForm.style.display = "block"; + document.getElementById("toggle-form").innerHTML = useBackup; + } + }; +} diff --git a/app/assets/javascripts/admin-login/admin-login.no-module.js.es6 b/app/assets/javascripts/admin-login/admin-login.no-module.js.es6 new file mode 100644 index 00000000000..4de268433dc --- /dev/null +++ b/app/assets/javascripts/admin-login/admin-login.no-module.js.es6 @@ -0,0 +1 @@ +require("admin-login/admin-login").default(); 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 f990437ce5c..572fd509f7f 100644 --- a/app/assets/javascripts/discourse/components/second-factor-form.js.es6 +++ b/app/assets/javascripts/discourse/components/second-factor-form.js.es6 @@ -4,16 +4,26 @@ import { SECOND_FACTOR_METHODS } from "discourse/models/user"; export default Ember.Component.extend({ @computed("secondFactorMethod") secondFactorTitle(secondFactorMethod) { - return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP - ? I18n.t("login.second_factor_title") - : I18n.t("login.second_factor_backup_title"); + switch (secondFactorMethod) { + case SECOND_FACTOR_METHODS.TOTP: + return I18n.t("login.second_factor_title"); + case SECOND_FACTOR_METHODS.SECURITY_KEY: + return I18n.t("login.second_factor_title"); + case SECOND_FACTOR_METHODS.BACKUP_CODE: + return I18n.t("login.second_factor_backup_title"); + } }, @computed("secondFactorMethod") secondFactorDescription(secondFactorMethod) { - return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP - ? I18n.t("login.second_factor_description") - : I18n.t("login.second_factor_backup_description"); + switch (secondFactorMethod) { + case SECOND_FACTOR_METHODS.TOTP: + return I18n.t("login.second_factor_description"); + case SECOND_FACTOR_METHODS.SECURITY_KEY: + return I18n.t("login.security_key_description"); + case SECOND_FACTOR_METHODS.BACKUP_CODE: + return I18n.t("login.second_factor_backup_description"); + } }, @computed("secondFactorMethod", "isLogin") @@ -29,6 +39,13 @@ export default Ember.Component.extend({ } }, + @computed("backupEnabled", "secondFactorMethod") + showToggleMethodLink(backupEnabled, secondFactorMethod) { + return ( + backupEnabled && secondFactorMethod !== SECOND_FACTOR_METHODS.SECURITY_KEY + ); + }, + actions: { toggleSecondFactorMethod() { const secondFactorMethod = this.secondFactorMethod; diff --git a/app/assets/javascripts/discourse/components/security-key-form.js.es6 b/app/assets/javascripts/discourse/components/security-key-form.js.es6 new file mode 100644 index 00000000000..3161831869c --- /dev/null +++ b/app/assets/javascripts/discourse/components/security-key-form.js.es6 @@ -0,0 +1,11 @@ +import { SECOND_FACTOR_METHODS } from "discourse/models/user"; + +export default Ember.Component.extend({ + actions: { + useAnotherMethod() { + this.set("showSecurityKey", false); + this.set("showSecondFactor", true); + this.set("secondFactorMethod", SECOND_FACTOR_METHODS.TOTP); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/email-login.js.es6 b/app/assets/javascripts/discourse/controllers/email-login.js.es6 index 5a025ce7a6e..d062144afef 100644 --- a/app/assets/javascripts/discourse/controllers/email-login.js.es6 +++ b/app/assets/javascripts/discourse/controllers/email-login.js.es6 @@ -1,20 +1,40 @@ +import computed from "ember-addons/ember-computed-decorators"; import { SECOND_FACTOR_METHODS } from "discourse/models/user"; import { ajax } from "discourse/lib/ajax"; import DiscourseURL from "discourse/lib/url"; import { popupAjaxError } from "discourse/lib/ajax-error"; +import { getWebauthnCredential } from "discourse/lib/webauthn"; export default Ember.Controller.extend({ - secondFactorMethod: SECOND_FACTOR_METHODS.TOTP, lockImageUrl: Discourse.getURL("/images/lock.svg"), + + @computed("model") + secondFactorRequired(model) { + return model.security_key_required || model.second_factor_required; + }, + + @computed("model") + secondFactorMethod(model) { + return model.security_key_required + ? SECOND_FACTOR_METHODS.SECURITY_KEY + : SECOND_FACTOR_METHODS.TOTP; + }, + actions: { finishLogin() { + let data = {}; + if (this.securityKeyCredential) { + data = { security_key_credential: this.securityKeyCredential }; + } else { + data = { + second_factor_token: this.secondFactorToken, + second_factor_method: this.secondFactorMethod + }; + } ajax({ url: `/session/email-login/${this.model.token}`, type: "POST", - data: { - second_factor_token: this.secondFactorToken, - second_factor_method: this.secondFactorMethod - } + data: data }) .then(result => { if (result.success) { @@ -24,6 +44,19 @@ export default Ember.Controller.extend({ } }) .catch(popupAjaxError); + }, + authenticateSecurityKey() { + getWebauthnCredential( + this.model.challenge, + this.model.allowed_credential_ids, + credentialData => { + this.set("securityKeyCredential", credentialData); + this.send("finishLogin"); + }, + errorMessage => { + this.set("model.error", errorMessage); + } + ); } } }); diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index dfe7eaf68c7..5dfc9227da7 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/app/assets/javascripts/discourse/controllers/login.js.es6 @@ -8,6 +8,7 @@ import { escapeExpression, areCookiesEnabled } from "discourse/lib/utilities"; import { extractError } from "discourse/lib/ajax-error"; import computed from "ember-addons/ember-computed-decorators"; import { SECOND_FACTOR_METHODS } from "discourse/models/user"; +import { getWebauthnCredential } from "discourse/lib/webauthn"; // This is happening outside of the app via popup const AuthErrors = [ @@ -43,19 +44,20 @@ export default Ember.Controller.extend(ModalFunctionality, { loggedIn: false, secondFactorRequired: false, showSecondFactor: false, + showSecurityKey: false, showLoginButtons: true, awaitingApproval: false }); }, - @computed("showSecondFactor") - credentialsClass(showSecondFactor) { - return showSecondFactor ? "hidden" : ""; + @computed("showSecondFactor", "showSecurityKey") + credentialsClass(showSecondFactor, showSecurityKey) { + return showSecondFactor || showSecurityKey ? "hidden" : ""; }, - @computed("showSecondFactor") - secondFactorClass(showSecondFactor) { - return showSecondFactor ? "" : "hidden"; + @computed("showSecondFactor", "showSecurityKey") + secondFactorClass(showSecondFactor, showSecurityKey) { + return showSecondFactor || showSecurityKey ? "" : "hidden"; }, @computed("awaitingApproval", "hasAtLeastOneLoginButton") @@ -66,6 +68,11 @@ export default Ember.Controller.extend(ModalFunctionality, { return classes.join(" "); }, + @computed("showSecondFactor", "showSecurityKey") + disableLoginFields(showSecondFactor, showSecurityKey) { + return showSecondFactor || showSecurityKey; + }, + @computed("canLoginLocalWithEmail") hasAtLeastOneLoginButton(canLoginLocalWithEmail) { return findAll().length > 0 || canLoginLocalWithEmail; @@ -109,15 +116,20 @@ export default Ember.Controller.extend(ModalFunctionality, { login: this.loginName, password: this.loginPassword, second_factor_token: this.secondFactorToken, - second_factor_method: this.secondFactorMethod + second_factor_method: this.secondFactorMethod, + security_key_credential: this.securityKeyCredential } }).then( result => { // Successful login if (result && result.error) { this.set("loggingIn", false); + const invalidSecurityKey = result.reason === "invalid_security_key"; + const invalidSecondFactor = + result.reason === "invalid_second_factor"; + if ( - result.reason === "invalid_second_factor" && + (invalidSecondFactor || invalidSecurityKey) && !this.secondFactorRequired ) { document.getElementById("modal-alert").style.display = "none"; @@ -126,15 +138,24 @@ export default Ember.Controller.extend(ModalFunctionality, { secondFactorRequired: true, showLoginButtons: false, backupEnabled: result.backup_enabled, - showSecondFactor: true + showSecondFactor: invalidSecondFactor, + showSecurityKey: invalidSecurityKey, + secondFactorMethod: invalidSecurityKey + ? SECOND_FACTOR_METHODS.SECURITY_KEY + : SECOND_FACTOR_METHODS.TOTP, + securityKeyChallenge: result.challenge, + securityKeyAllowedCredentialIds: result.allowed_credential_ids }); - Ember.run.schedule("afterRender", () => - document - .getElementById("second-factor") - .querySelector("input") - .focus() - ); + // only need to focus the 2FA input for TOTP + if (!this.showSecurityKey) { + Ember.run.scheduleOnce("afterRender", () => + document + .getElementById("second-factor") + .querySelector("input") + .focus() + ); + } return; } else if (result.reason === "not_activated") { @@ -286,6 +307,20 @@ export default Ember.Controller.extend(ModalFunctionality, { }) .catch(e => this.flash(extractError(e), "error")) .finally(() => this.set("processingEmailLink", false)); + }, + + authenticateSecurityKey() { + getWebauthnCredential( + this.securityKeyChallenge, + this.securityKeyAllowedCredentialIds, + credentialData => { + this.set("securityKeyCredential", credentialData); + this.send("login"); + }, + errorMessage => { + this.flash(errorMessage, "error"); + } + ); } }, diff --git a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 index 2d48d74e2ec..ae3c5949a5f 100644 --- a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 +++ b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 @@ -4,13 +4,21 @@ import { ajax } from "discourse/lib/ajax"; import PasswordValidation from "discourse/mixins/password-validation"; import { userPath } from "discourse/lib/url"; import { SECOND_FACTOR_METHODS } from "discourse/models/user"; +import { getWebauthnCredential } from "discourse/lib/webauthn"; 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"), + securityKeyRequired: Ember.computed.alias("model.security_key_required"), backupEnabled: Ember.computed.alias("model.backup_enabled"), - secondFactorMethod: SECOND_FACTOR_METHODS.TOTP, + securityKeyOrSecondFactorRequired: Ember.computed.or( + "model.second_factor_required", + "model.security_key_required" + ), + secondFactorMethod: Ember.computed.alias("model.security_key_required") + ? SECOND_FACTOR_METHODS.SECURITY_KEY + : SECOND_FACTOR_METHODS.TOTP, passwordRequired: true, errorMessage: null, successMessage: null, @@ -39,7 +47,8 @@ export default Ember.Controller.extend(PasswordValidation, { data: { password: this.accountPassword, second_factor_token: this.secondFactorToken, - second_factor_method: this.secondFactorMethod + second_factor_method: this.secondFactorMethod, + security_key_credential: this.securityKeyCredential } }) .then(result => { @@ -53,15 +62,17 @@ export default Ember.Controller.extend(PasswordValidation, { DiscourseURL.redirectTo(result.redirect_to || "/"); } } else { - if (result.errors && result.errors.user_second_factors) { + if (result.errors && !result.errors.password) { this.setProperties({ - secondFactorRequired: true, + secondFactorRequired: this.secondFactorRequired, + securityKeyRequired: this.securityKeyRequired, password: null, errorMessage: result.message }); - } else if (this.secondFactorRequired) { + } else if (this.secondFactorRequired || this.securityKeyRequired) { this.setProperties({ secondFactorRequired: false, + securityKeyRequired: false, errorMessage: null }); } else if ( @@ -90,6 +101,24 @@ export default Ember.Controller.extend(PasswordValidation, { }); }, + authenticateSecurityKey() { + getWebauthnCredential( + this.model.challenge, + this.model.allowed_credential_ids, + credentialData => { + this.set("securityKeyCredential", credentialData); + this.send("submit"); + }, + errorMessage => { + this.setProperties({ + securityKeyRequired: true, + password: null, + errorMessage: errorMessage + }); + } + ); + }, + done() { this.set("redirected", true); DiscourseURL.redirectTo(this.redirectTo || "/"); 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 85129d453eb..34eb99d0e18 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/second-factor.js.es6 @@ -68,12 +68,14 @@ export default Ember.Controller.extend(CanCheckEmails, { errorMessage: null, loaded: true, totps: response.totps, + security_keys: response.security_keys, password: null, dirty: false }); this.set( "model.second_factor_enabled", - response.totps && response.totps.length > 0 + (response.totps && response.totps.length > 0) || + (response.security_keys && response.security_keys.length > 0) ); }) .catch(e => this.handleError(e)) @@ -147,6 +149,31 @@ export default Ember.Controller.extend(CanCheckEmails, { }); }, + createSecurityKey() { + const controller = showModal("second-factor-add-security-key", { + model: this.model, + title: "user.second_factor.security_key.add" + }); + controller.setProperties({ + onClose: () => this.loadSecondFactors(), + markDirty: () => this.markDirty(), + onError: e => this.handleError(e) + }); + }, + + editSecurityKey(security_key) { + const controller = showModal("second-factor-edit-security-key", { + model: security_key, + title: "user.second_factor.security_key.edit" + }); + controller.setProperties({ + user: this.model, + onClose: () => this.loadSecondFactors(), + markDirty: () => this.markDirty(), + onError: e => this.handleError(e) + }); + }, + editSecondFactor(second_factor) { const controller = showModal("second-factor-edit", { model: second_factor, diff --git a/app/assets/javascripts/discourse/controllers/second-factor-add-security-key.js.es6 b/app/assets/javascripts/discourse/controllers/second-factor-add-security-key.js.es6 new file mode 100644 index 00000000000..6106d2602b4 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/second-factor-add-security-key.js.es6 @@ -0,0 +1,123 @@ +import ModalFunctionality from "discourse/mixins/modal-functionality"; +import { bufferToBase64, stringToBuffer } from "discourse/lib/webauthn"; + +// model for this controller is user.js.es6 +export default Ember.Controller.extend(ModalFunctionality, { + loading: false, + errorMessage: null, + + onShow() { + // clear properties every time because the controller is a singleton + this.setProperties({ + errorMessage: null, + loading: true, + securityKeyName: I18n.t("user.second_factor.security_key.default_name") + }); + + this.model + .requestSecurityKeyChallenge() + .then(response => { + if (response.error) { + this.set("errorMessage", response.error); + return; + } + + this.setProperties({ + errorMessage: null, + loading: false, + challenge: response.challenge, + relayingParty: { + id: response.rp_id, + name: response.rp_name + }, + supported_algoriths: response.supported_algoriths, + user_secure_id: response.user_secure_id, + existing_active_credential_ids: + response.existing_active_credential_ids + }); + }) + .catch(error => { + this.send("closeModal"); + this.onError(error); + }) + .finally(() => this.set("loading", false)); + }, + + actions: { + registerSecurityKey() { + const publicKeyCredentialCreationOptions = { + challenge: Uint8Array.from(this.challenge, c => c.charCodeAt(0)), + rp: { + name: this.relayingParty.name, + id: this.relayingParty.id + }, + user: { + id: Uint8Array.from(this.user_secure_id, c => c.charCodeAt(0)), + displayName: this.model.username_lower, + name: this.model.username_lower + }, + pubKeyCredParams: this.supported_algoriths.map(alg => { + return { type: "public-key", alg: alg }; + }), + excludeCredentials: this.existing_active_credential_ids.map( + credentialId => { + return { + type: "public-key", + id: stringToBuffer(atob(credentialId)) + }; + } + ), + timeout: 20000, + attestation: "none", + authenticatorSelection: { + // see https://chromium.googlesource.com/chromium/src/+/master/content/browser/webauth/uv_preferred.md for why + // default value of preferred is not necesarrily what we want, it limits webauthn to only devices that support + // user verification, which usually requires entering a PIN + userVerification: "discouraged" + } + }; + + navigator.credentials + .create({ + publicKey: publicKeyCredentialCreationOptions + }) + .then( + credential => { + let serverData = { + id: credential.id, + rawId: bufferToBase64(credential.rawId), + type: credential.type, + attestation: bufferToBase64( + credential.response.attestationObject + ), + clientData: bufferToBase64(credential.response.clientDataJSON), + name: this.securityKeyName + }; + + this.model + .registerSecurityKey(serverData) + .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)); + }, + err => { + if (err.name === "NotAllowedError") { + return this.set( + "errorMessage", + I18n.t("user.second_factor.security_key.not_allowed_error") + ); + } + this.set("errorMessage", err.message); + } + ); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/second-factor-edit-security-key.js.es6 b/app/assets/javascripts/discourse/controllers/second-factor-edit-security-key.js.es6 new file mode 100644 index 00000000000..90815cfbea8 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/second-factor-edit-security-key.js.es6 @@ -0,0 +1,42 @@ +import ModalFunctionality from "discourse/mixins/modal-functionality"; + +export default Ember.Controller.extend(ModalFunctionality, { + actions: { + disableSecurityKey() { + this.user + .updateSecurityKey(this.model.id, this.model.name, true) + .then(response => { + if (response.error) { + return; + } + this.markDirty(); + }) + .catch(error => { + this.send("closeModal"); + this.onError(error); + }) + .finally(() => { + this.set("loading", false); + this.send("closeModal"); + }); + }, + + editSecurityKey() { + this.user + .updateSecurityKey(this.model.id, this.model.name, false) + .then(response => { + if (response.error) { + return; + } + this.markDirty(); + }) + .catch(error => { + this.onError(error); + }) + .finally(() => { + this.set("loading", false); + this.send("closeModal"); + }); + } + } +}); diff --git a/app/assets/javascripts/discourse/lib/webauthn.js.es6 b/app/assets/javascripts/discourse/lib/webauthn.js.es6 new file mode 100644 index 00000000000..3ec74ba2f17 --- /dev/null +++ b/app/assets/javascripts/discourse/lib/webauthn.js.es6 @@ -0,0 +1,74 @@ +export function stringToBuffer(str) { + let buffer = new ArrayBuffer(str.length); + let byteView = new Uint8Array(buffer); + for (let i = 0; i < str.length; i++) { + byteView[i] = str.charCodeAt(i); + } + return buffer; +} + +export function bufferToBase64(buffer) { + return btoa(String.fromCharCode(...new Uint8Array(buffer))); +} + +export function getWebauthnCredential( + challenge, + allowedCredentialIds, + successCallback, + errorCallback +) { + if (typeof PublicKeyCredential === "undefined") { + return errorCallback(I18n.t("login.security_key_support_missing_error")); + } + + let challengeBuffer = stringToBuffer(challenge); + let allowCredentials = allowedCredentialIds.map(credentialId => { + return { + id: stringToBuffer(atob(credentialId)), + type: "public-key" + }; + }); + + navigator.credentials + .get({ + publicKey: { + challenge: challengeBuffer, + allowCredentials: allowCredentials, + timeout: 60000, + + // see https://chromium.googlesource.com/chromium/src/+/master/content/browser/webauth/uv_preferred.md for why + // default value of preferred is not necesarrily what we want, it limits webauthn to only devices that support + // user verification, which usually requires entering a PIN + userVerification: "discouraged" + } + }) + .then(credential => { + // 1. if there is a credential, check if the raw ID base64 matches + // any of the allowed credential ids + if ( + !allowedCredentialIds.some( + credentialId => bufferToBase64(credential.rawId) === credentialId + ) + ) { + return errorCallback( + I18n.t("login.security_key_no_matching_credential_error") + ); + } + + const credentialData = { + signature: bufferToBase64(credential.response.signature), + clientData: bufferToBase64(credential.response.clientDataJSON), + authenticatorData: bufferToBase64( + credential.response.authenticatorData + ), + credentialId: bufferToBase64(credential.rawId) + }; + successCallback(credentialData); + }) + .catch(err => { + if (err.name === "NotAllowedError") { + return errorCallback(I18n.t("login.security_key_not_allowed_error")); + } + errorCallback(err); + }); +} diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 84ac5efed42..a16b24f80ea 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -21,7 +21,11 @@ import { defaultHomepage } from "discourse/lib/utilities"; import { userPath } from "discourse/lib/url"; import Category from "discourse/models/category"; -export const SECOND_FACTOR_METHODS = { TOTP: 1, BACKUP_CODE: 2 }; +export const SECOND_FACTOR_METHODS = { + TOTP: 1, + BACKUP_CODE: 2, + SECURITY_KEY: 3 +}; const isForever = dt => moment().diff(dt, "years") < -500; @@ -375,6 +379,19 @@ const User = RestModel.extend({ }); }, + requestSecurityKeyChallenge() { + return ajax("/u/create_second_factor_security_key.json", { + type: "POST" + }); + }, + + registerSecurityKey(credential) { + return ajax("/u/register_second_factor_security_key.json", { + data: credential, + type: "POST" + }); + }, + createSecondFactorTotp() { return ajax("/u/create_second_factor_totp.json", { type: "POST" @@ -409,6 +426,17 @@ const User = RestModel.extend({ }); }, + updateSecurityKey(id, name, disable) { + return ajax("/u/security_key.json", { + data: { + name, + disable, + id + }, + type: "PUT" + }); + }, + toggleSecondFactor(authToken, authMethod, targetMethod, enable) { return ajax("/u/second_factor.json", { data: { 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 f3a7c2ea7bd..763460bc239 100644 --- a/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 +++ b/app/assets/javascripts/discourse/routes/preferences-second-factor.js.es6 @@ -14,6 +14,7 @@ export default RestrictedUserRoute.extend({ setupController(controller, model) { controller.setProperties({ model, newUsername: model.get("username") }); controller.set("loading", true); + model .loadSecondFactorCodes("") .then(response => { @@ -24,7 +25,8 @@ export default RestrictedUserRoute.extend({ errorMessage: null, loaded: !response.password_required, dirty: !!response.password_required, - totps: response.totps + totps: response.totps, + security_keys: response.security_keys }); } }) 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 f2ccf505dfe..4c5c717be35 100644 --- a/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs +++ b/app/assets/javascripts/discourse/templates/components/second-factor-form.hbs @@ -5,12 +5,9 @@ {{/if}}

{{secondFactorDescription}}

{{yield}} - {{#if backupEnabled}} + {{#if showToggleMethodLink}}

- {{discourse-linked-text - class="toggle-second-factor-method" - action=(action "toggleSecondFactorMethod") - text=linkText}} + {{ i18n linkText }}

{{/if}} diff --git a/app/assets/javascripts/discourse/templates/components/security-key-form.hbs b/app/assets/javascripts/discourse/templates/components/security-key-form.hbs new file mode 100644 index 00000000000..e5bc247ad3c --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/security-key-form.hbs @@ -0,0 +1,14 @@ +
+ {{d-button + action=action + icon="key" + id="security-key-authenticate-button" + label="login.security_key_authenticate" + type="button" + class='btn btn-large btn-primary'}} +

+ {{#if otherMethodAllowed}} + {{ i18n 'login.security_key_alternative' }} + {{/if}} +

+
diff --git a/app/assets/javascripts/discourse/templates/email-login.hbs b/app/assets/javascripts/discourse/templates/email-login.hbs index 1556a212a27..4541fa86898 100644 --- a/app/assets/javascripts/discourse/templates/email-login.hbs +++ b/app/assets/javascripts/discourse/templates/email-login.hbs @@ -12,20 +12,34 @@ {{/if}} {{#if model.can_login}} - {{#if model.second_factor_required}} - {{#second-factor-form - secondFactorMethod=secondFactorMethod - secondFactorToken=secondFactorToken - backupEnabled=model.backup_codes_enabled - isLogin=true}} - {{second-factor-input value=secondFactorToken secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} - {{/second-factor-form}} + {{#if secondFactorRequired }} + {{#if model.security_key_required }} + {{#security-key-form + allowedCredentialIds=model.allowed_credential_ids + challenge=model.security_key_challenge + showSecurityKey=model.security_key_required + showSecondFactor=false + secondFactorMethod=secondFactorMethod + otherMethodAllowed=secondFactorRequired + action=(action "authenticateSecurityKey")}} + {{/security-key-form}} + {{else}} + {{#second-factor-form + secondFactorMethod=secondFactorMethod + secondFactorToken=secondFactorToken + backupEnabled=model.backup_codes_enabled + isLogin=true}} + {{second-factor-input value=secondFactorToken secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} + {{/second-factor-form}} + {{/if}} {{else}}

{{i18n "email_login.confirm_title" site_name=siteSettings.title}}

{{i18n "email_login.logging_in_as" email=model.token_email}}

{{/if}} - {{d-button label="email_login.confirm_button" action=(action "finishLogin") class="btn-primary"}} + {{#unless model.security_key_required }} + {{d-button label="email_login.confirm_button" action=(action "finishLogin") class="btn-primary"}} + {{/unless}} {{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/login.hbs b/app/assets/javascripts/discourse/templates/modal/login.hbs index 326fdd4999e..ff3b7f40eca 100644 --- a/app/assets/javascripts/discourse/templates/modal/login.hbs +++ b/app/assets/javascripts/discourse/templates/modal/login.hbs @@ -8,11 +8,11 @@ - + - + @@ -28,7 +28,19 @@ class=secondFactorClass backupEnabled=backupEnabled isLogin=true}} - {{second-factor-input value=secondFactorToken inputId='login-second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} + {{#if showSecurityKey}} + {{#security-key-form + allowedCredentialIds=securityKeyAllowedCredentialIds + challenge=securityKeyChallenge + showSecurityKey=showSecurityKey + showSecondFactor=showSecondFactor + secondFactorMethod=secondFactorMethod + otherMethodAllowed=secondFactorRequired + action=(action "authenticateSecurityKey")}} + {{/security-key-form}} + {{else}} + {{second-factor-input value=secondFactorToken inputId='login-second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}} + {{/if}} {{/second-factor-form}} {{/if}} @@ -43,13 +55,16 @@
{{#if canLoginLocal}} - {{d-button - action=(action "login") - form="login-form" - icon="unlock" - label=loginButtonLabel - disabled=loginDisabled - class='btn btn-large btn-primary'}} + {{#unless showSecurityKey }} + {{d-button + action=(action "login") + id="login-button" + form="login-form" + icon="unlock" + label=loginButtonLabel + disabled=loginDisabled + class='btn btn-large btn-primary'}} + {{/unless}} {{#if showSignupLink}}
+
+
+

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

+ {{d-button action=(action "createSecurityKey") + class="btn-primary new-security-key" + disabled=loading + label="user.second_factor.security_key.add"}} + {{#each security_keys as |security_key|}} +
+ {{#if security_key.name}} + {{security_key.name}} + {{else}} + {{i18n "user.second_factor.security_key.default_name"}} + {{/if}} + + {{#if isCurrentUser}} + {{d-button action=(action "editSecurityKey" security_key) + 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"}}

diff --git a/app/assets/stylesheets/desktop/login.scss b/app/assets/stylesheets/desktop/login.scss index 2f28e06105c..40b59d5b68e 100644 --- a/app/assets/stylesheets/desktop/login.scss +++ b/app/assets/stylesheets/desktop/login.scss @@ -33,10 +33,11 @@ form { min-width: 300px; + max-width: 100%; } #modal-alert { - max-width: 500px; + max-width: 100%; padding: s(2 4); } diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 08a3d9f249e..c060fb7fafd 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -4,6 +4,7 @@ require_dependency 'rate_limiter' require_dependency 'single_sign_on' require_dependency 'single_sign_on_provider' require_dependency 'url_helper' +require_dependency 'webauthn/webauthn' class SessionController < ApplicationController class LocalLoginNotAllowed < StandardError; end @@ -298,7 +299,18 @@ class SessionController < ApplicationController if payload = login_error_check(user) render json: payload else - if user.totp_enabled? && !user.authenticate_second_factor(params[:second_factor_token], params[:second_factor_method].to_i) + if (params[:second_factor_token].blank?) + security_key_valid = ::Webauthn::SecurityKeyAuthenticationService.new(user, params[:security_key_credential], + challenge: secure_session["staged-webauthn-challenge-#{user.id}"], + rp_id: secure_session["staged-webauthn-rp-id-#{user.id}"], + origin: Discourse.base_url + ).authenticate_security_key + return invalid_security_key(user) if user.security_keys_enabled? && !security_key_valid + end + + if user.totp_enabled? && \ + !user.authenticate_second_factor(params[:second_factor_token], params[:second_factor_method].to_i) && + !params[:security_key_credential].present? return render json: failed_json.merge( error: I18n.t("login.invalid_second_factor_code"), reason: "invalid_second_factor", @@ -308,6 +320,17 @@ class SessionController < ApplicationController (user.active && user.email_confirmed?) ? login(user) : not_activated(user) end + rescue ::Webauthn::SecurityKeyError => err + invalid_security_key(user, err.message) + end + + def invalid_security_key(user, err_message = nil) + stage_webauthn_security_key_challenge(user) if !params[:security_key_credential] + return render json: failed_json.merge( + error: err_message || I18n.t("login.invalid_security_key"), + reason: "invalid_security_key", + backup_enabled: user.backup_codes_enabled? + ).merge(webauthn_security_key_challenge_and_allowed_credentials(user)) end def email_login_info @@ -323,10 +346,18 @@ class SessionController < ApplicationController token_email: matched_token.email } - if matched_token.user&.totp_enabled? + matched_user = matched_token.user + if matched_user&.totp_enabled? response.merge!( second_factor_required: true, - backup_codes_enabled: matched_token.user&.backup_codes_enabled? + backup_codes_enabled: matched_user&.backup_codes_enabled? + ) + end + + if matched_user&.security_keys_enabled? + stage_webauthn_security_key_challenge(matched_user) + response.merge!( + webauthn_security_key_challenge_and_allowed_credentials(matched_user).merge(security_key_required: true) ) end @@ -343,15 +374,27 @@ class SessionController < ApplicationController raise Discourse::NotFound if !SiteSetting.enable_local_logins_via_email second_factor_token = params[:second_factor_token] second_factor_method = params[:second_factor_method].to_i + security_key_credential = params[:security_key_credential] token = params[:token] matched_token = EmailToken.confirmable(token) - if matched_token&.user&.totp_enabled? - if !second_factor_token.present? - return render json: { error: I18n.t('login.invalid_second_factor_code') } - elsif !matched_token.user.authenticate_second_factor(second_factor_token, second_factor_method) - RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! - return render json: { error: I18n.t('login.invalid_second_factor_code') } + if security_key_credential.present? + if matched_token&.user&.security_keys_enabled? + security_key_valid = ::Webauthn::SecurityKeyAuthenticationService.new(matched_token&.user, params[:security_key_credential], + challenge: secure_session["staged-webauthn-challenge-#{matched_token&.user&.id}"], + rp_id: secure_session["staged-webauthn-rp-id-#{matched_token&.user&.id}"], + origin: Discourse.base_url + ).authenticate_security_key + return invalid_security_key(matched_token&.user) if !security_key_valid + end + else + if matched_token&.user&.totp_enabled? + if !second_factor_token.present? + return render json: { error: I18n.t('login.invalid_second_factor_code') } + elsif !matched_token.user.authenticate_second_factor(second_factor_token, second_factor_method) + RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed! + return render json: { error: I18n.t('login.invalid_second_factor_code') } + end end end @@ -367,6 +410,8 @@ class SessionController < ApplicationController end return render json: { error: I18n.t('email_login.invalid_token') } + rescue ::Webauthn::SecurityKeyError => err + invalid_security_key(user, err.message) end def one_time_password @@ -535,4 +580,21 @@ class SessionController < ApplicationController def sso_url(sso) sso.to_url end + + def stage_webauthn_security_key_challenge(user) + challenge = SecureRandom.hex(30) + secure_session["staged-webauthn-challenge-#{user.id}"] = challenge + secure_session["staged-webauthn-rp-id-#{user.id}"] = Discourse.current_hostname + end + + def webauthn_security_key_challenge_and_allowed_credentials(user) + return {} if !user.security_keys_enabled? + credential_ids = user.security_keys.select(:credential_id) + .where(factor_type: UserSecurityKey.factor_types[:second_factor]) + .pluck(:credential_id) + { + allowed_credential_ids: credential_ids, + challenge: secure_session["staged-webauthn-challenge-#{user.id}"] + } + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 7a6225a15ac..5557b8030e3 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -6,6 +6,7 @@ require_dependency 'rate_limiter' require_dependency 'wizard' require_dependency 'wizard/builder' require_dependency 'admin_confirmation' +require_dependency 'webauthn/webauthn' class UsersController < ApplicationController @@ -17,7 +18,8 @@ class UsersController < ApplicationController :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 + :notification_level, :revoke_auth_token, :register_second_factor_security_key, + :create_second_factor_security_key ] skip_before_action :check_xhr, only: [ @@ -28,7 +30,9 @@ class UsersController < ApplicationController 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] + :disable_second_factor, :update_second_factor, :create_second_factor_backup, + :register_second_factor_security_key, :create_second_factor_security_key + ] before_action :respond_to_suspicious_request, only: [:create] @@ -496,17 +500,33 @@ class UsersController < ApplicationController second_factor_token = params[:second_factor_token] second_factor_method = params[:second_factor_method].to_i + security_key_credential = params[:security_key_credential] 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) + elsif security_key_credential.present? + security_key_authenticated = ::Webauthn::SecurityKeyAuthenticationService.new( + @user, + security_key_credential, + challenge: secure_session["staged-webauthn-challenge-#{@user.id}"], + rp_id: secure_session["staged-webauthn-rp-id-#{@user.id}"], + origin: Discourse.base_url + ).authenticate_security_key end - if second_factor_authenticated || !@user&.totp_enabled? + second_factor_totp_disabled = !@user&.totp_enabled? + if second_factor_authenticated || second_factor_totp_disabled || security_key_authenticated secure_session["second-factor-#{token}"] = "true" end + security_key_disabled = !@user&.security_keys_enabled? + if security_key_authenticated || security_key_disabled + secure_session["security-key-#{token}"] = "true" + end + valid_second_factor = secure_session["second-factor-#{token}"] == "true" + valid_security_key = secure_session["security-key-#{token}"] == "true" if !@user @error = I18n.t('password_reset.no_token') @@ -539,13 +559,17 @@ class UsersController < ApplicationController if @error render layout: 'no_ember' else + stage_webauthn_security_key_challenge(@user) store_preloaded( "password_reset", MultiJson.dump( - is_developer: UsernameCheckerService.is_developer?(@user.email), - admin: @user.admin?, - second_factor_required: !valid_second_factor, - backup_enabled: @user.backup_codes_enabled? + { + is_developer: UsernameCheckerService.is_developer?(@user.email), + admin: @user.admin?, + second_factor_required: !valid_second_factor, + security_key_required: !valid_security_key, + backup_enabled: @user.backup_codes_enabled? + }.merge(webauthn_security_key_challenge_and_allowed_credentials(@user)) ) ) end @@ -578,18 +602,25 @@ class UsersController < ApplicationController errors: @user&.errors&.to_hash } else + stage_webauthn_security_key_challenge(@user) if !valid_security_key && !security_key_credential.present? render json: { is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin?, second_factor_required: !valid_second_factor, + security_key_required: !valid_security_key, backup_enabled: @user.backup_codes_enabled? - } + }.merge(webauthn_security_key_challenge_and_allowed_credentials(@user)) end end end end rescue RateLimiter::LimitExceeded => e render_rate_limit_error(e) + rescue ::Webauthn::SecurityKeyError => err + render json: { + message: err.message, + errors: [err.message] + } end def confirm_email_token @@ -636,27 +667,55 @@ class UsersController < ApplicationController email_token_user = EmailToken.confirmable(token)&.user totp_enabled = email_token_user&.totp_enabled? + security_keys_enabled = email_token_user&.security_keys_enabled? second_factor_token = params[:second_factor_token] second_factor_method = params[:second_factor_method].to_i + security_key_credential = params[:security_key_credential] confirm_email = false + @security_key_required = security_keys_enabled - confirm_email = + if security_keys_enabled && params[:security_key_credential].blank? + stage_webauthn_security_key_challenge(email_token_user) + challenge_and_credentials = webauthn_security_key_challenge_and_allowed_credentials(email_token_user) + @security_key_challenge = challenge_and_credentials[:challenge] + @security_key_allowed_credential_ids = challenge_and_credentials[:allowed_credential_ids].join(",") + end + + if security_keys_enabled && params[:security_key_credential].present? + credential = JSON.parse(params[:security_key_credential]).with_indifferent_access + + confirm_email = ::Webauthn::SecurityKeyAuthenticationService.new(email_token_user, credential, + challenge: secure_session["staged-webauthn-challenge-#{email_token_user&.id}"], + rp_id: secure_session["staged-webauthn-rp-id-#{email_token_user&.id}"], + origin: Discourse.base_url + ).authenticate_security_key + @message = I18n.t('login.security_key_invalid') if !confirm_email + elsif security_keys_enabled && second_factor_token.blank? + confirm_email = false + @message = I18n.t("login.second_factor_title") if totp_enabled @second_factor_required = true @backup_codes_enabled = true - @message = I18n.t("login.second_factor_title") - - if second_factor_token.present? - if email_token_user.authenticate_second_factor(second_factor_token, second_factor_method) - true - else - @error = I18n.t("login.invalid_second_factor_code") - false - end - end - else - true end + else + confirm_email = + if totp_enabled + @second_factor_required = true + @backup_codes_enabled = true + @message = I18n.t("login.second_factor_title") + + if second_factor_token.present? + if email_token_user.authenticate_second_factor(second_factor_token, second_factor_method) + true + else + @error = I18n.t("login.invalid_second_factor_code") + false + end + end + else + true + end + end if confirm_email @user = EmailToken.confirm(token) @@ -673,10 +732,13 @@ class UsersController < ApplicationController end end - render layout: false + render layout: 'no_ember' rescue RateLimiter::LimitExceeded @message = I18n.t("rate_limiter.slow_down") - render layout: false + render layout: 'no_ember' + rescue ::Webauthn::SecurityKeyError => err + @message = err.message + render layout: 'no_ember' end def email_login @@ -1110,8 +1172,15 @@ class UsersController < ApplicationController end if secure_session["confirmed-password-#{current_user.id}"] == "true" + totp_second_factors = current_user.totps + .select(:id, :name, :last_used, :created_at, :method) + .where(enabled: true).order(:created_at) + + security_keys = current_user.security_keys.where(factor_type: UserSecurityKey.factor_types[:second_factor]).order(:created_at) + render json: success_json.merge( - totps: current_user.totps.select(:id, :name, :last_used, :created_at, :method).order(:created_at) + totps: totp_second_factors, + security_keys: security_keys ) else render json: success_json.merge( @@ -1144,6 +1213,55 @@ class UsersController < ApplicationController ) end + def create_second_factor_security_key + challenge = SecureRandom.hex(30) + secure_session["staged-webauthn-challenge-#{current_user.id}"] = challenge + secure_session["staged-webauthn-rp-id-#{current_user.id}"] = Discourse.current_hostname + secure_session["staged-webauthn-rp-name-#{current_user.id}"] = SiteSetting.title + + render json: success_json.merge( + challenge: challenge, + rp_id: Discourse.current_hostname, + rp_name: SiteSetting.title, + supported_algoriths: ::Webauthn::SUPPORTED_ALGORITHMS, + user_secure_id: current_user.create_or_fetch_secure_identifier, + existing_active_credential_ids: current_user.second_factor_security_key_credential_ids + ) + end + + def register_second_factor_security_key + params.require(:name) + params.require(:attestation) + params.require(:clientData) + + ::Webauthn::SecurityKeyRegistrationService.new( + current_user, + params, + challenge: secure_session["staged-webauthn-challenge-#{current_user.id}"], + rp_id: secure_session["staged-webauthn-rp-id-#{current_user.id}"], + origin: Discourse.base_url + ).register_second_factor_security_key + render json: success_json + rescue ::Webauthn::SecurityKeyError => err + render json: failed_json.merge( + error: err.message + ) + end + + def update_security_key + user_security_key = current_user.security_keys.find_by(id: params[:id].to_i) + raise Discourse::InvalidParameters unless user_security_key + + if params[:name] && !params[:name].blank? + user_security_key.update!(name: params[:name]) + end + if params[:disable] == "true" + user_security_key.update!(enabled: false) + end + + render json: success_json + end + def enable_second_factor_totp params.require(:second_factor_token) params.require(:name) @@ -1373,4 +1491,19 @@ class UsersController < ApplicationController end end end + + def stage_webauthn_security_key_challenge(user) + challenge = SecureRandom.hex(30) + secure_session["staged-webauthn-challenge-#{user.id}"] = challenge + secure_session["staged-webauthn-rp-id-#{user.id}"] = Discourse.current_hostname + end + + def webauthn_security_key_challenge_and_allowed_credentials(user) + return {} if !user.security_keys_enabled? + credential_ids = user.second_factor_security_key_credential_ids + { + allowed_credential_ids: credential_ids, + challenge: secure_session["staged-webauthn-challenge-#{user.id}"] + } + end end diff --git a/app/models/concerns/second_factor_manager.rb b/app/models/concerns/second_factor_manager.rb index 67caf8adf0b..7c20cc7ce2c 100644 --- a/app/models/concerns/second_factor_manager.rb +++ b/app/models/concerns/second_factor_manager.rb @@ -51,6 +51,12 @@ module SecondFactorManager self&.user_second_factors.backup_codes.exists? end + def security_keys_enabled? + !SiteSetting.enable_sso && + SiteSetting.enable_local_logins && + self&.security_keys.where(factor_type: UserSecurityKey.factor_types[:second_factor], enabled: true).exists? + end + def remaining_backup_codes self&.user_second_factors&.backup_codes&.count end diff --git a/app/models/user.rb b/app/models/user.rb index c1a94949a62..0b8f18da700 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -79,6 +79,10 @@ class User < ActiveRecord::Base where(method: UserSecondFactor.methods[:totp], enabled: true) }, class_name: "UserSecondFactor" + has_many :security_keys, -> { + where(enabled: true) + }, class_name: "UserSecurityKey" + has_one :anonymous_user_master, class_name: 'AnonymousUser' has_one :anonymous_user_shadow, ->(record) { where(active: true) }, foreign_key: :master_user_id, class_name: 'AnonymousUser' @@ -1263,6 +1267,20 @@ class User < ActiveRecord::Base SQL end + def create_or_fetch_secure_identifier + return secure_identifier if secure_identifier.present? + new_secure_identifier = SecureRandom.hex(20) + self.update(secure_identifier: new_secure_identifier) + new_secure_identifier + end + + def second_factor_security_key_credential_ids + security_keys + .select(:credential_id) + .where(factor_type: UserSecurityKey.factor_types[:second_factor]) + .pluck(:credential_id) + end + protected def badge_grant diff --git a/app/models/user_security_key.rb b/app/models/user_security_key.rb new file mode 100644 index 00000000000..61f325bd0de --- /dev/null +++ b/app/models/user_security_key.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class UserSecurityKey < ActiveRecord::Base + belongs_to :user + + scope :second_factors, -> do + where(factor_type: UserSecurityKey.factor_types[:second_factor], enabled: true) + end + + def self.factor_types + @factor_types ||= Enum.new( + second_factor: 0, + first_factor: 1, + multi_factor: 2, + ) + end +end + +# == Schema Information +# +# Table name: user_security_keys +# +# id :bigint not null, primary key +# user_id :integer not null +# factor_type :integer not null +# credential_id :string not null, UNIQUE +# public_key :string not null +# enabled :boolean default(FALSE), not null +# last_used :datetime +# created_at :datetime not null +# updated_at :datetime not null +# name :string not null +# +# Indexes +# +# index_user_security_keys_on_credential_id (credential_id) (UNIQUE) +# index_user_security_keys_on_factor_type (factor_type) +# diff --git a/app/views/common/_second_factor_form_script.html.erb b/app/views/common/_second_factor_form_script.html.erb deleted file mode 100644 index 226136b7be1..00000000000 --- a/app/views/common/_second_factor_form_script.html.erb +++ /dev/null @@ -1,18 +0,0 @@ -<%= javascript_tag do %> - var useTotp = "<%= t("login.second_factor_toggle.totp") %>"; - var useBackup = "<%= t("login.second_factor_toggle.backup_code") %>"; - var backupForm = document.getElementById("backup-second-factor-form"); - var primaryForm = document.getElementById("primary-second-factor-form"); - document.getElementById("toggle-form").onclick = function(event) { - event.preventDefault(); - if (backupForm.style.display === "none") { - backupForm.style.display = "block"; - primaryForm.style.display = "none"; - document.getElementById("toggle-form").innerHTML = useTotp; - } else { - backupForm.style.display = "none"; - primaryForm.style.display = "block"; - document.getElementById("toggle-form").innerHTML = useBackup; - } - } -<% end %> diff --git a/app/views/users/admin_login.html.erb b/app/views/users/admin_login.html.erb index aa29bc0fa84..42d3a5e6370 100644 --- a/app/views/users/admin_login.html.erb +++ b/app/views/users/admin_login.html.erb @@ -1,39 +1,61 @@ - - - Admin Login - - - <% if @message %> - <%= @message %> - <% if @error %>

<%= @error %>

<% end %> +<% if @message %> + <%= @message %> + <% if @error %>

<%= @error %>

<% end %> - <% if @second_factor_required %> -
- <%=form_tag({}, method: :put) do %> - <%= label_tag(:second_factor_token, t('login.second_factor_description')) %> - <%= render 'common/second_factor_text_field' %>

- <%= submit_tag t('submit')%> - <% end %> -
+ <% if @security_key_required %> +
+
+ <%= hidden_field_tag 'security_key_challenge', @security_key_challenge, id: 'security-key-challenge' %> + <%= hidden_field_tag 'security_key_allowed_credential_ids', @security_key_allowed_credential_ids, id: 'security-key-allowed-credential-ids' %> - <%if @backup_codes_enabled%> - - <%=t "login.second_factor_backup" %> - <%= render 'common/second_factor_form_script' %> - <%end%> - <% end %> - <% else %> <%=form_tag({}, method: :put) do %> - <%= label_tag(:email, t('admin_login.email_input')) %> - <%= text_field_tag(:email, nil, autofocus: true) %>

- <%= submit_tag t('admin_login.submit_button') %> +

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

+

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

+ <%= hidden_field_tag 'second_factor_method', '3' %> + <%= hidden_field_tag 'security_key_credential', nil, id: 'security-key-credential' %> + + <% if @second_factor_required %> + <%= link_to t('login.security_key_alternative'), '#', id: 'activate-security-key-alternative' %>

+ <% end %> + <%= button_tag t('login.security_key_authenticate'), id: 'submit-security-key' %> <% end %> - <% end %> - - +
+ <% end %> + + <% if @second_factor_required %> +
+
+ <%=form_tag({}, method: :put) do %> +
+ <%= label_tag(:second_factor_token, t('login.second_factor_description')) %> + <%= render 'common/second_factor_text_field' %>

+ <%= submit_tag t('submit')%> + <% end %> +
+ + <%if @backup_codes_enabled%> + + <%=t "login.second_factor_backup" %> + <%end%> +
+ <% end %> +<% else %> + <%=form_tag({}, method: :put) do %> + <%= label_tag(:email, t('admin_login.email_input')) %> + <%= text_field_tag(:email, nil, autofocus: true) %>

+ <%= submit_tag t('admin_login.submit_button') %> + <% end %> +<% end %> + +<%= preload_script "ember_jquery" %> +<%= preload_script "locales/#{I18n.locale}" %> +<%= preload_script "locales/i18n" %> +<%= preload_script "discourse/lib/webauthn" %> +<%= preload_script "admin-login/admin-login" %> +<%= preload_script "admin-login/admin-login.no-module" %> diff --git a/config/application.rb b/config/application.rb index 932aed28061..7b58fcd96e9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -144,6 +144,10 @@ module Discourse activate-account.js auto-redirect.js wizard-start.js + locales/i18n.js + discourse/lib/webauthn.js + admin-login/admin-login.js + admin-login/admin-login.no-module.js onpopstate-handler.js embed-application.js } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4d73f53c06e..019325302b4 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -960,7 +960,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" + use: "Use a backup code" enable_prerequisites: "You must enable a primary second factor before generating backup codes." codes: title: "Backup Codes Generated" @@ -981,7 +981,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" + 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" @@ -989,10 +989,20 @@ en: edit: "Edit" edit_title: "Edit Second Factor" edit_description: "Second Factor Name" + enable_security_key_description: "When you have your physical security key prepared press the Register button below." totp: title: "Token-Based Authenticators" add: "New Authenticator" default_name: "My Authenticator" + security_key: + register: "Register" + title: 'Security Keys' + add: "Register Security Key" + default_name: "Main Security Key" + not_allowed_error: "The security key registration process either timed out or was cancelled." + edit: 'Edit Security Key' + edit_description: 'Security Key Name' + disable: 'Disable' change_about: title: "Change About Me" @@ -1440,10 +1450,16 @@ en: password: "Password" second_factor_title: "Two Factor Authentication" second_factor_description: "Please enter the authentication code from your app:" - second_factor_backup: "Log in using a backup code" + second_factor_backup: "Log in using a backup code" second_factor_backup_title: "Two Factor Backup" second_factor_backup_description: "Please enter one of your backup codes:" - second_factor: "Log in using Authenticator app" + second_factor: "Log in using Authenticator app" + security_key_description: "When you have your physical security key prepared press the Authenticate with Security Key button below." + security_key_alternative: "Can't find your security key or want to use another method?" + security_key_authenticate: "Authenticate with Security Key" + security_key_not_allowed_error: "The security key authentication process either timed out or was cancelled." + security_key_no_matching_credential_error: "No matching credentials could be found in the provided security key." + security_key_support_missing_error: "Your current device or browser does not support the use of security keys. Please use a different method." email_placeholder: "email or username" caps_lock_warning: "Caps Lock is on" error: "Unknown error" @@ -1499,6 +1515,9 @@ en: name: "Discord" title: "with Discord" message: "Authenticating with Discord" + second_factor_toggle: + totp: "Use an authenticator app instead" + backup_code: "Use a backup code instead" invites: accept_title: "Invitation" welcome_to: "Welcome to %{site_name}!" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f10deb014f4..dcf1d7c2e3c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -880,6 +880,21 @@ en: no_drafts: self: "You have no drafts; begin composing a reply in any topic and it will be auto-saved as a new draft." + webauthn: + validation: + invalid_type_error: 'The webauthn type provided was invalid. Valid types are webauthn.get and webauthn.create.' + challenge_mismatch_error: 'The provided challenge does not match the challenge generated by the authentication server.' + invalid_origin_error: 'The origin of the authentication request does not match the server origin.' + malformed_attestation_error: 'There was an error decoding the attestation data.' + invalid_relying_party_id_error: 'The Relying Party ID of the authentication request does not match the server Relying Party ID.' + user_verification_error: 'User verification is required.' + unsupported_public_key_algorithm_error: 'The provided public key algorithm is not supported by the server.' + unsupported_attestation_format_error: 'The attestation format is not supported by the server.' + credential_id_in_use_error: 'The credential ID provided is already in use.' + public_key_error: 'The public key verification for the credential failed.' + ownership_error: 'The security key is not owned by the user.' + not_found_error: 'A security key with the provided credential ID could not be found.' + topic_flag_types: spam: title: "Spam" @@ -2229,6 +2244,13 @@ en: auto_deleted_by_timer: "Automatically deleted by timer." login: + security_key_description: "When you have your physical security key prepared press the Authenticate with Security Key button below." + security_key_alternative: "Can't find your security key or want to use another method?" + security_key_authenticate: "Authenticate with Security Key" + security_key_not_allowed_error: "The security key authentication process either timed out or was cancelled." + security_key_no_matching_credential_error: "No matching credentials could be found in the provided security key." + security_key_support_missing_error: "Your current device or browser does not support the use of security keys. Please use a different method." + security_key_invalid: "There was an error validating the security key." not_approved: "Your account hasn't been approved yet. You will be notified by email when you are ready to log in." incorrect_username_email_or_password: "Incorrect username, email or password" incorrect_password: "Incorrect password" @@ -2265,6 +2287,7 @@ en: second_factor_backup_description: "Please enter one of your backup codes:" second_factor_backup_title: "Two Factor Backup Code" invalid_second_factor_code: "Invalid authentication code. Each code can only be used once." + invalid_security_key: "Invalid security key." second_factor_toggle: totp: "Use an authenticator app instead" backup_code: "Use a backup code instead" diff --git a/config/routes.rb b/config/routes.rb index 328ad0e8b94..44a66d3f603 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -379,6 +379,10 @@ Discourse::Application.routes.draw do post "#{root_path}/second_factors" => "users#list_second_factors" put "#{root_path}/second_factor" => "users#update_second_factor" + + post "#{root_path}/create_second_factor_security_key" => "users#create_second_factor_security_key" + post "#{root_path}/register_second_factor_security_key" => "users#register_second_factor_security_key" + put "#{root_path}/security_key" => "users#update_security_key" 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" diff --git a/db/migrate/20190904104533_create_user_security_keys.rb b/db/migrate/20190904104533_create_user_security_keys.rb new file mode 100644 index 00000000000..acccab96856 --- /dev/null +++ b/db/migrate/20190904104533_create_user_security_keys.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class CreateUserSecurityKeys < ActiveRecord::Migration[5.2] + def up + create_table :user_security_keys do |t| + t.references :user, null: false, index: true, foreign_key: true + t.string :credential_id, null: false + t.string :public_key, null: false, index: true + t.integer :factor_type, null: false, default: 0, index: true + t.boolean :enabled, null: false, default: true + t.string :name, null: false + t.datetime :last_used + + t.timestamps + end + + add_index :user_security_keys, :credential_id, unique: true + add_index :user_security_keys, :last_used + end + + def down + if table_exists?(:user_security_keys) + drop_table(:user_security_keys) + end + end +end diff --git a/db/migrate/20190908233325_add_secure_identifier_column_to_users.rb b/db/migrate/20190908233325_add_secure_identifier_column_to_users.rb new file mode 100644 index 00000000000..c7b048de4b8 --- /dev/null +++ b/db/migrate/20190908233325_add_secure_identifier_column_to_users.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddSecureIdentifierColumnToUsers < ActiveRecord::Migration[5.2] + def change + add_column :users, :secure_identifier, :string + add_index :users, :secure_identifier, unique: true + end +end diff --git a/db/migrate/20190917100006_add_enabled_index_to_user_security_key.rb b/db/migrate/20190917100006_add_enabled_index_to_user_security_key.rb new file mode 100644 index 00000000000..14b9c027c28 --- /dev/null +++ b/db/migrate/20190917100006_add_enabled_index_to_user_security_key.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddEnabledIndexToUserSecurityKey < ActiveRecord::Migration[6.0] + def change + add_index :user_security_keys, [:factor_type, :enabled] + end +end diff --git a/lib/webauthn/security_key_authentication_service.rb b/lib/webauthn/security_key_authentication_service.rb new file mode 100644 index 00000000000..34a840e167d --- /dev/null +++ b/lib/webauthn/security_key_authentication_service.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true +require 'cose' + +module Webauthn + class SecurityKeyAuthenticationService < SecurityKeyBaseValidationService + + ## + # See https://w3c.github.io/webauthn/#sctn-verifying-assertion for + # the steps followed here. Memoized methods are called in their + # place in the step flow to make the process clearer. + def authenticate_security_key + return false if @params.blank? + + # 3. Identify the user being authenticated and verify that this user is the + # owner of the public key credential source credentialSource identified by credential.id: + security_key = UserSecurityKey.find_by(credential_id: @params[:credentialId]) + raise(NotFoundError, I18n.t('webauthn.validation.not_found_error')) if security_key.blank? + raise(OwnershipError, I18n.t('webauthn.validation.ownership_error')) if security_key.user != @current_user + + # 4. Using credential.id (or credential.rawId, if base64url encoding is inappropriate for your use case), + # look up the corresponding credential public key and let credentialPublicKey be that credential public key. + public_key = security_key.public_key + + # 5. Let cData, authData and sig denote the value of credential’s response's clientDataJSON, authenticatorData, and signature respectively. + # 6. Let JSONtext be the result of running UTF-8 decode on the value of cData. + # 7. Let C, the client data claimed as used for the signature, be the result of running an implementation-specific JSON parser on JSONtext. + client_data + + # 8. Verify that the value of C.type is the string webauthn.get. + validate_webauthn_type(::Webauthn::ACCEPTABLE_AUTHENTICATION_TYPE) + + # 9. Verify that the value of C.challenge equals the base64url encoding of options.challenge. + validate_challenge + + # 10. Verify that the value of C.origin matches the Relying Party's origin. + validate_origin + + # 11. Verify that the value of C.tokenBinding.status matches the state of Token Binding for the TLS connection + # over which the attestation was obtained. If Token Binding was used on that TLS connection, also verify + # that C.tokenBinding.id matches the base64url encoding of the Token Binding ID for the connection. + # Not using this right now. + + # 12. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the Relying Party. + validate_rp_id_hash + + # 13. Verify that the User Present bit of the flags in authData is set. + # https://blog.bigbinary.com/2011/07/20/ruby-pack-unpack.html + # + # bit 0 is the least significant bit - LSB first + # + # 14. If user verification is required for this registration, verify that + # the User Verified bit of the flags in authData is set. + validate_user_verification + + # 15. Verify that the values of the client extension outputs in clientExtensionResults and the authenticator + # extension outputs in the extensions in authData are as expected, considering the client extension input + # values that were given in options.extensions and any specific policy of the Relying Party regarding + # unsolicited extensions, i.e., those that were not specified as part of options.extensions. In the + # general case, the meaning of "are as expected" is specific to the Relying Party and which extensions are in use. + # Not using this right now. + + # 16. Let hash be the result of computing a hash over response.clientDataJSON using SHA-256. + client_data_hash + + # 17. Using credentialPublicKey, verify that sig is a valid signature over the binary concatenation of authData and hash. + cose_key = COSE::Key.deserialize(Base64.decode64(security_key.public_key)) + if !cose_key.to_pkey.verify(COSE::Algorithm.find(cose_key.alg).hash_function, signature, auth_data + client_data_hash) + raise(PublicKeyError, I18n.t('webauthn.validation.public_key_error')) + end + + # Success! Update the last used at time for the key. + security_key.update(last_used: Time.zone.now) + rescue OpenSSL::PKey::PKeyError + raise(PublicKeyError, I18n.t('webauthn.validation.public_key_error')) + end + + private + + def auth_data + @auth_data ||= Base64.decode64(@params[:authenticatorData]) + end + + def signature + @signature ||= Base64.decode64(@params[:signature]) + end + end +end diff --git a/lib/webauthn/security_key_base_validation_service.rb b/lib/webauthn/security_key_base_validation_service.rb new file mode 100644 index 00000000000..e5900876f17 --- /dev/null +++ b/lib/webauthn/security_key_base_validation_service.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Webauthn + class SecurityKeyBaseValidationService + def initialize(current_user, params, challenge_params) + @current_user = current_user + @params = params + @challenge_params = challenge_params + end + + def validate_webauthn_type(type_to_check) + return if client_data['type'] == type_to_check + raise(InvalidTypeError, I18n.t('webauthn.validation.invalid_type_error')) + end + + def validate_challenge + return if challenge_match? + raise(ChallengeMismatchError, I18n.t('webauthn.validation.challenge_mismatch_error')) + end + + def validate_origin + return if origin_match? + raise(InvalidOriginError, I18n.t('webauthn.validation.invalid_origin_error')) + end + + def validate_rp_id_hash + return if rp_id_hash_match? + raise(InvalidRelyingPartyIdError, I18n.t('webauthn.validation.invalid_relying_party_id_error')) + end + + def validate_user_verification + flags = auth_data[32].unpack("b*")[0].split('') + return if flags[0] == '1' + raise(UserVerificationError, I18n.t('webauthn.validation.user_verification_error')) + end + + private + + # https://w3c.github.io/webauthn/#sctn-registering-a-new-credential + # Let JSONtext be the result of running UTF-8 decode on the value of response.clientDataJSON. + def client_data_json + @client_data_json ||= Base64.decode64(@params[:clientData]) + end + + # Let C, the client data claimed as collected during the credential creation, be the result of running + # an implementation-specific JSON parser on JSONtext. + def client_data + @client_data ||= JSON.parse(client_data_json) + end + + def challenge_match? + Base64.decode64(client_data['challenge']) == @challenge_params[:challenge] + end + + def origin_match? + client_data['origin'] == @challenge_params[:origin] + end + + def rp_id_hash_match? + auth_data[0..31] == OpenSSL::Digest::SHA256.digest(@challenge_params[:rp_id]) + end + + def client_data_hash + @client_data_hash ||= OpenSSL::Digest::SHA256.digest(client_data_json) + end + end +end diff --git a/lib/webauthn/security_key_registration_service.rb b/lib/webauthn/security_key_registration_service.rb new file mode 100644 index 00000000000..bea9e322f28 --- /dev/null +++ b/lib/webauthn/security_key_registration_service.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true +require 'cbor' +require 'cose' + +module Webauthn + class SecurityKeyRegistrationService < SecurityKeyBaseValidationService + + ## + # See https://w3c.github.io/webauthn/#sctn-registering-a-new-credential for + # the registration steps followed here. Memoized methods are called in their + # place in the step flow to make the process clearer. + def register_second_factor_security_key + # 4. Verify that the value of C.type is webauthn.create. + validate_webauthn_type(::Webauthn::ACCEPTABLE_REGISTRATION_TYPE) + + # 5. Verify that the value of C.challenge equals the base64url encoding of options.challenge. + validate_challenge + + # 6. Verify that the value of C.origin matches the Relying Party's origin. + validate_origin + + # 7. Verify that the value of C.tokenBinding.status matches the state of Token Binding for the TLS + # connection over which the assertion was obtained. If Token Binding was used on that TLS connection, + # also verify that C.tokenBinding.id matches the base64url encoding of the Token Binding ID for the connection. + # Not using this right now. + + # 8. Let hash be the result of computing a hash over response.clientDataJSON using SHA-256. + client_data_hash + + # 9. Perform CBOR decoding on the attestationObject field of the AuthenticatorAttestationResponse + # structure to obtain the attestation statement format fmt, the authenticator data authData, + # and the attestation statement attStmt. + attestation + + # 10. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the Relying Party. + # check the SHA256 hash of the rpId is the same as the authData bytes 0..31 + validate_rp_id_hash + + # 11. Verify that the User Present bit of the flags in authData is set. + # https://blog.bigbinary.com/2011/07/20/ruby-pack-unpack.html + # + # bit 0 is the least significant bit - LSB first + # + # 12. If user verification is required for this registration, verify that + # the User Verified bit of the flags in authData is set. + validate_user_verification + + # 13. Verify that the "alg" parameter in the credential public key in authData matches the alg + # attribute of one of the items in options.pubKeyCredParams. + # https://w3c.github.io/webauthn/#table-attestedCredentialData + # See https://www.iana.org/assignments/cose/cose.xhtml#algorithms for supported algorithm + # codes, -7 which Discourse uses is ECDSA w/ SHA-256 + credential_public_key, credential_public_key_bytes, credential_id = extract_public_key_and_credential_from_attestation(auth_data) + raise(UnsupportedPublicKeyAlgorithmError, I18n.t('webauthn.validation.unsupported_public_key_algorithm_error')) if ::Webauthn::SUPPORTED_ALGORITHMS.exclude?(credential_public_key.alg) + + # 14. Verify that the values of the client extension outputs in clientExtensionResults and the authenticator + # extension outputs in the extensions in authData are as expected, considering the client extension input + # values that were given in options.extensions. In particular, any extension identifier values in the + # clientExtensionResults and the extensions in authData MUST also be present as extension identifier values + # in options.extensions, i.e., no extensions are present that were not requested. In the general case, the + # meaning of "are as expected" is specific to the Relying Party and which extensions are in use. + # Not using this right now. + + # 15. Determine the attestation statement format by performing a USASCII case-sensitive match on fmt against the + # set of supported WebAuthn Attestation Statement Format Identifier values. An up-to-date list of registered + # WebAuthn Attestation Statement Format Identifier values is maintained in the IANA registry of the same + # name [WebAuthn-Registries]. + # 16. Verify that attStmt is a correct attestation statement, conveying a valid attestation signature, + # by using the attestation statement format fmt’s verification procedure given attStmt, authData and hash. + if ::Webauthn::VALID_ATTESTATION_FORMATS.exclude?(attestation['fmt']) || attestation['fmt'] != 'none' + raise(UnsupportedAttestationFormatError, I18n.t('webauthn.validation.unsupported_attestation_format_error')) + end + + #================================================== + # ONLY APPLIES IF fmt !== none, this is all to do with + # verifying attestation. May want to come back to this at + # some point for additional security. + #================================================== + # + # 17. If validation is successful, obtain a list of acceptable trust anchors (attestation root certificates or + # ECDAA-Issuer public keys) for that attestation type and attestation statement format fmt, from a trusted + # source or from policy. For example, the FIDO Metadata Service [FIDOMetadataService] provides one way + # to obtain such information, using the aaguid in the attestedCredentialData in authData. + # + # 18. Assess the attestation trustworthiness using the outputs of the verification procedure in step 16, as follows: + # If no attestation was provided, verify that None attestation is acceptable under Relying Party policy. + #================================================== + + # 19. Check that the credentialId is not yet registered to any other user. If registration + # is requested for a credential that is already registered to a different user, + # the Relying Party SHOULD fail this registration ceremony, or it MAY decide to accept + # the registration, e.g. while deleting the older registration. + encoded_credential_id = Base64.strict_encode64(credential_id) + endcoded_public_key = Base64.strict_encode64(credential_public_key_bytes) + raise(CredentialIdInUseError, I18n.t('webauthn.validation.credential_id_in_use_error')) if UserSecurityKey.exists?(credential_id: encoded_credential_id) + + # 20. If the attestation statement attStmt verified successfully and is found to be trustworthy, + # then register the new credential with the account that was denoted in options.user, by + # associating it with the credentialId and credentialPublicKey in the attestedCredentialData + # in authData, as appropriate for the Relying Party's system. + UserSecurityKey.create( + user: @current_user, + credential_id: encoded_credential_id, + public_key: endcoded_public_key, + name: @params[:name], + factor_type: UserSecurityKey.factor_types[:second_factor] + ) + rescue CBOR::UnpackError, CBOR::TypeError, CBOR::MalformedFormatError, CBOR::StackError + raise MalformedAttestationError, I18n.t('webauthn.validation.malformed_attestation_error') + end + + private + + def attestation + @attestation ||= CBOR.decode(Base64.decode64(@params[:attestation])) + end + + def auth_data + @auth_data ||= attestation['authData'] + end + + def extract_public_key_and_credential_from_attestation(auth_data) + # see https://w3c.github.io/webauthn/#authenticator-data for lengths + # of authdata for extraction + rp_id_length = 32 + flags_length = 1 + sign_count_length = 4 + + attested_credential_data_start_position = rp_id_length + flags_length + sign_count_length # 37 + attested_credential_data_length = auth_data.size - attested_credential_data_start_position + attested_credential_data = auth_data[ + attested_credential_data_start_position..(attested_credential_data_start_position + attested_credential_data_length - 1) + ] + + # see https://w3c.github.io/webauthn/#attested-credential-data for lengths + # of data for extraction + aa_guid = attested_credential_data[0..15] + credential_id_length = attested_credential_data[16..17].unpack("n*")[0] + credential_id = attested_credential_data[18..(18 + credential_id_length - 1)] + + public_key_start_position = 18 + credential_id_length + public_key_bytes = attested_credential_data[ + public_key_start_position..(public_key_start_position + attested_credential_data.size - 1) + ] + public_key = COSE::Key.deserialize(public_key_bytes) + + [public_key, public_key_bytes, credential_id] + end + end +end diff --git a/lib/webauthn/webauthn.rb b/lib/webauthn/webauthn.rb new file mode 100644 index 00000000000..d53eefe3635 --- /dev/null +++ b/lib/webauthn/webauthn.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true +require 'webauthn/security_key_base_validation_service' +require 'webauthn/security_key_registration_service' +require 'webauthn/security_key_authentication_service' + +module Webauthn + ACCEPTABLE_REGISTRATION_TYPE = "webauthn.create".freeze + ACCEPTABLE_AUTHENTICATION_TYPE = "webauthn.get".freeze + SUPPORTED_ALGORITHMS = [-7].freeze + VALID_ATTESTATION_FORMATS = ['none', 'packed', 'fido-u2f'].freeze + + class SecurityKeyError < StandardError; end + + class InvalidOriginError < SecurityKeyError; end + class InvalidRelyingPartyIdError < SecurityKeyError; end + class UserVerificationError < SecurityKeyError; end + class ChallengeMismatchError < SecurityKeyError; end + class InvalidTypeError < SecurityKeyError; end + class UnsupportedPublicKeyAlgorithmError < SecurityKeyError; end + class UnsupportedAttestationFormatError < SecurityKeyError; end + class CredentialIdInUseError < SecurityKeyError; end + class MalformedAttestationError < SecurityKeyError; end + class NotFoundError < SecurityKeyError; end + class OwnershipError < SecurityKeyError; end + class PublicKeyError < SecurityKeyError; end +end diff --git a/spec/fabricators/user_security_key_fabricator.rb b/spec/fabricators/user_security_key_fabricator.rb new file mode 100644 index 00000000000..827413a18de --- /dev/null +++ b/spec/fabricators/user_security_key_fabricator.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +Fabricator(:user_security_key) do + user + # Note: these values are valid and decode to a credential ID and COSE public key + # HOWEVER they are largely useless unless you have the device that created + # them. It is nice to have an approximation though. + credential_id { 'mJAJ4CznTO0SuLkJbYwpgK75ao4KMNIPlU5KWM92nq39kRbXzI9mSv6GxTcsMYoiPgaouNw7b7zBiS4vsQaO6A==' } + public_key { 'pQECAyYgASFYIMNgw4GCpwBUlR2SznJ1yY7B9yFvsuxhfo+C9kcA4IitIlggRdofrCezymy2B/YarX+gfB6gZKg648/cHIMjf6wWmmU=' } + enabled true + factor_type { UserSecurityKey.factor_types[:second_factor] } + name { sequence(:name) { |i| "Security Key #{i + 1}" } } +end + +## +# Useful for specs that just need a user security key model but not +# any of the related usefulness as a webauthn credential, because the +# credential_id has a UNIQUE index +Fabricator(:user_security_key_with_random_credential, from: :user_security_key) do + credential_id { SecureRandom.base64(40) } + public_key { SecureRandom.base64(40) } +end diff --git a/spec/lib/webauthn/security_key_authentication_service_spec.rb b/spec/lib/webauthn/security_key_authentication_service_spec.rb new file mode 100644 index 00000000000..e448bf32c57 --- /dev/null +++ b/spec/lib/webauthn/security_key_authentication_service_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true +require 'rails_helper' +require 'webauthn/webauthn' +require 'webauthn/security_key_registration_service' + +describe Webauthn::SecurityKeyAuthenticationService do + let(:security_key_user) { current_user } + let(:security_key) do + Fabricate( + :user_security_key, + credential_id: 'mJAJ4CznTO0SuLkJbYwpgK75ao4KMNIPlU5KWM92nq39kRbXzI9mSv6GxTcsMYoiPgaouNw7b7zBiS4vsQaO6A==', + public_key: 'pQECAyYgASFYIMNgw4GCpwBUlR2SznJ1yY7B9yFvsuxhfo+C9kcA4IitIlggRdofrCezymy2B/YarX+gfB6gZKg648/cHIMjf6wWmmU=', + user: security_key_user, + last_used: nil + ) + end + let(:credential_id) { security_key.credential_id } + let(:challenge) { '81d4acfbd69eafa8f02bc2ecbec5267be8c9b28c1e0ba306d52b79f0f13d' } + let(:client_data_challenge) { Base64.strict_encode64(challenge) } + let(:client_data_webauthn_type) { 'webauthn.get' } + let(:client_data_origin) { 'http://localhost:3000' } + + ## + # IMPORTANT: For the SHA256 hash to match the same one as was used to generate + # the values for this spec, the three keys and values must be in the same order + # (challenge, origin, type) + let(:client_data_param) { + { + challenge: client_data_challenge, + origin: client_data_origin, + type: client_data_webauthn_type + } + } + ## + # These are sourced from an actual login using the UserSecurityKey credential + # defined in this spec. + let(:signature) { "MEUCIBppPyK8blxBDoktU54mI1vWEY96r1V5H1rEBtPDxwcGAiEAoi7LCmMoEAuWYu0krZpflZlULsbURCGcqOwP06amXYE=" } + let(:authenticator_data) { "SZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzzuoMdl2MBAAAAVw==" } + let(:params) do + { + clientData: Base64.strict_encode64(client_data_param.to_json), + credentialId: credential_id, + authenticatorData: authenticator_data, + signature: signature + } + end + ## + # The original key was generated in localhost + let(:rp_id) { 'localhost' } + let(:challenge_params) do + { + challenge: challenge, + rp_id: rp_id, + origin: 'http://localhost:3000' + } + end + let(:current_user) { Fabricate(:user) } + let(:subject) { described_class.new(current_user, params, challenge_params) } + + it 'updates last_used when valid' do + subject.authenticate_security_key + expect(security_key.reload.last_used).not_to eq(nil) + end + + context 'when the credential ID does not match any user security key in the database' do + let(:credential_id) { 'badid' } + + it 'raises a NotFoundError' do + expect { subject.authenticate_security_key }.to raise_error( + Webauthn::NotFoundError, I18n.t('webauthn.validation.not_found_error') + ) + end + end + + context 'when the credential ID does exist but it is for a different user' do + let(:security_key_user) { Fabricate(:user) } + + it 'raises an OwnershipError' do + expect { subject.authenticate_security_key }.to raise_error( + Webauthn::OwnershipError, I18n.t('webauthn.validation.ownership_error') + ) + end + end + + context 'when the client data webauthn type is not webauthn.get' do + let(:client_data_webauthn_type) { 'webauthn.explode' } + + it 'raises an InvalidTypeError' do + expect { subject.authenticate_security_key }.to raise_error( + Webauthn::InvalidTypeError, I18n.t('webauthn.validation.invalid_type_error') + ) + end + end + + context 'when the decoded challenge does not match the original challenge provided by the server' do + let(:client_data_challenge) { Base64.strict_encode64('invalid challenge') } + + it 'raises a ChallengeMismatchError' do + expect { subject.authenticate_security_key }.to raise_error( + Webauthn::ChallengeMismatchError, I18n.t('webauthn.validation.challenge_mismatch_error') + ) + end + end + + context 'when the origin of the client data does not match the server origin' do + let(:client_data_origin) { 'https://someothersite.com' } + + it 'raises a InvalidOriginError' do + expect { subject.authenticate_security_key }.to raise_error( + Webauthn::InvalidOriginError, I18n.t('webauthn.validation.invalid_origin_error') + ) + end + end + + context 'when the sha256 hash of the relaying party ID does not match the one in attestation.authData' do + let(:rp_id) { 'bad_rp_id' } + + it 'raises a InvalidRelyingPartyIdError' do + expect { subject.authenticate_security_key }.to raise_error( + Webauthn::InvalidRelyingPartyIdError, I18n.t('webauthn.validation.invalid_relying_party_id_error') + ) + end + end + + context 'when there is a problem verifying the public key (e.g. invalid signature)' do + let(:signature) { Base64.strict_encode64('badsig') } + + it 'raises a PublicKeyError' do + expect { subject.authenticate_security_key }.to raise_error( + Webauthn::PublicKeyError, I18n.t('webauthn.validation.public_key_error') + ) + end + end +end diff --git a/spec/lib/webauthn/security_key_registration_service_spec.rb b/spec/lib/webauthn/security_key_registration_service_spec.rb new file mode 100644 index 00000000000..e412d2284bc --- /dev/null +++ b/spec/lib/webauthn/security_key_registration_service_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true +require 'rails_helper' +require 'webauthn/webauthn' +require 'webauthn/security_key_registration_service' + +describe Webauthn::SecurityKeyRegistrationService do + let(:client_data_challenge) { Base64.encode64(challenge) } + let(:client_data_webauthn_type) { 'webauthn.create' } + let(:client_data_origin) { 'http://localhost:3000' } + let(:client_data_param) { + { + challenge: client_data_challenge, + type: client_data_webauthn_type, + origin: client_data_origin + } + } + ## + # This attestation object was sourced by manually registering + # a key with `navigator.credentials.create` and capturing the + # results in localhost. + let(:attestation) do + "o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVjESZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzzuoMdl2NBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQFmvayWc8OPJ4jj4sevfxBmvUglDMZrFalyokYrdnqOVvudC0lQialaGQv72eBzJM2Qn1GfJI7lpBgFJMprisLSlAQIDJiABIVgg+23/BZux7LK0/KQgCiQGtdr51ar+vfTtHWpRtN17gOwiWCBstV918mugVBexg/rdZjTs0wN/upHFoyBiAJCaGVD8OA==" + end + let(:params) do + { + clientData: Base64.encode64(client_data_param.to_json), + attestation: attestation, + name: 'My Yubikey' + } + end + ## + # The above attestation was generated in localhost; Discourse.current_hostname + # returns test.localhost which we do not want + let(:rp_id) { 'localhost' } + let(:challenge_params) do + { + challenge: challenge, + rp_id: rp_id, + origin: 'http://localhost:3000' + } + end + let(:challenge) { 'f1e04530f34a1b6a08d032d8550e23eb8330be04e4166008f26c0e1b42ad' } + let(:current_user) { Fabricate(:user) } + let(:subject) { described_class.new(current_user, params, challenge_params) } + + context 'when the client data webauthn type is not webauthn.create' do + let(:client_data_webauthn_type) { 'webauthn.explode' } + + it 'raises an InvalidTypeError' do + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::InvalidTypeError, I18n.t('webauthn.validation.invalid_type_error') + ) + end + end + + context 'when the decoded challenge does not match the original challenge provided by the server' do + let(:client_data_challenge) { Base64.encode64('invalid challenge') } + + it 'raises a ChallengeMismatchError' do + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::ChallengeMismatchError, I18n.t('webauthn.validation.challenge_mismatch_error') + ) + end + end + + context 'when the origin of the client data does not match the server origin' do + let(:client_data_origin) { 'https://someothersite.com' } + + it 'raises a InvalidOriginError' do + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::InvalidOriginError, I18n.t('webauthn.validation.invalid_origin_error') + ) + end + end + + context 'when the sha256 hash of the relaying party ID does not match the one in attestation.authData' do + let(:rp_id) { 'bad_rp_id' } + + it 'raises a InvalidRelyingPartyIdError' do + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::InvalidRelyingPartyIdError, I18n.t('webauthn.validation.invalid_relying_party_id_error') + ) + end + end + + context 'when the public key algorithm is not supported by the server' do + before do + @original_supported_alg_value = Webauthn::SUPPORTED_ALGORITHMS + silence_warnings do + Webauthn::SUPPORTED_ALGORITHMS = [-257] + end + end + + it 'raises a UnsupportedPublicKeyAlgorithmError' do + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::UnsupportedPublicKeyAlgorithmError, I18n.t('webauthn.validation.unsupported_public_key_algorithm_error') + ) + end + + after do + silence_warnings do + Webauthn::SUPPORTED_ALGORITHMS = @original_supported_alg_value + end + end + end + + context 'when the attestation format is not supported' do + before do + @original_supported_alg_value = Webauthn::VALID_ATTESTATION_FORMATS + silence_warnings do + Webauthn::VALID_ATTESTATION_FORMATS = ['err'] + end + end + + it 'raises a UnsupportedAttestationFormatError' do + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::UnsupportedAttestationFormatError, I18n.t('webauthn.validation.unsupported_attestation_format_error') + ) + end + + after do + silence_warnings do + Webauthn::VALID_ATTESTATION_FORMATS = @original_supported_alg_value + end + end + end + + context 'when the credential id is already in use for any user' do + it 'raises a CredentialIdInUseError' do + # register the key to the current user + security_key = subject.register_second_factor_security_key + + # update the key to be on a different user + other_user = Fabricate(:user) + security_key.update(user: other_user) + + # error! + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::CredentialIdInUseError, I18n.t('webauthn.validation.credential_id_in_use_error') + ) + end + end + + context 'when the attestation data is malformed' do + let(:attestation) do + "blah/krrmihjLHmVzzuoMdl2NBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQFmvayWc8OPJ4jj4sevfxBmvUglDMZrFalyokYrdnqOVvudC0lQialaGQv72eBzJM2Qn1GfJI7lpBgFJMprisLSlAQIDJiABIVgg+23/BZux7LK0/KQgCiQGtdr51ar+vfTtHWpRtN17gOwiWCBstV918mugVBexg/rdZjTs0wN/upHFoyBiAJCaGVD8OA==" + end + + it 'raises a MalformedAttestationError' do + expect { subject.register_second_factor_security_key }.to raise_error( + Webauthn::MalformedAttestationError, I18n.t('webauthn.validation.malformed_attestation_error') + ) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cc50d88b2ea..431ca1dacc9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2170,4 +2170,47 @@ describe User do end end end + + describe "Second-factor authenticators" do + describe "#totps" do + it "only includes enabled totp 2FA" do + enabled_totp_2fa = Fabricate(:user_second_factor_totp, user: user, name: 'Enabled TOTP', enabled: true) + disabled_totp_2fa = Fabricate(:user_second_factor_totp, user: user, name: 'Disabled TOTP', enabled: false) + + expect(user.totps.map(&:id)).to eq([enabled_totp_2fa.id]) + end + end + + describe "#security_keys" do + it "only includes enabled security_key 2FA" do + enabled_security_key_2fa = Fabricate(:user_security_key_with_random_credential, user: user, name: 'Enabled YubiKey', enabled: true) + disabled_security_key_2fa = Fabricate(:user_security_key_with_random_credential, user: user, name: 'Disabled YubiKey', enabled: false) + + expect(user.security_keys.map(&:id)).to eq([enabled_security_key_2fa.id]) + end + end + end + + describe 'Secure identifier for a user which is a string other than the ID used to identify the user in some cases e.g. security keys' do + describe '#create_or_fetch_secure_identifier' do + context 'if the user already has a secure identifier' do + let(:sec_ident) { SecureRandom.hex(20) } + before do + user.update(secure_identifier: sec_ident) + end + + it 'returns the identifier' do + expect(user.create_or_fetch_secure_identifier).to eq(sec_ident) + end + end + + context 'if the user already does not have a secure identifier' do + it 'creates one' do + expect(user.secure_identifier).to eq(nil) + user.create_or_fetch_secure_identifier + expect(user.reload.secure_identifier).not_to eq(nil) + end + end + end + end end diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 8e5ec95c179..f554e0fe46d 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -65,6 +65,23 @@ RSpec.describe SessionController do expect(JSON.parse(response.body)["backup_codes_enabled"]).to eq(true) end end + + context 'user has security key enabled' do + let!(:user_security_key) { Fabricate(:user_security_key, user: user) } + + it "includes that information in the response" do + get "/session/email-login/#{email_token.token}.json" + + expect(JSON.parse(response.body)["can_login"]).to eq(true) + expect(JSON.parse(response.body)["security_key_required"]).to eq(true) + expect(JSON.parse(response.body)["second_factor_required"]).to eq(nil) + expect(JSON.parse(response.body)["backup_codes_enabled"]).to eq(nil) + expect(JSON.parse(response.body)["allowed_credential_ids"]).to eq([user_security_key.credential_id]) + secure_session = SecureSession.new(session["secure_session_id"]) + expect(JSON.parse(response.body)["challenge"]).to eq(secure_session["staged-webauthn-challenge-#{user.id}"]) + expect(secure_session["staged-webauthn-rp-id-#{user.id}"]).to eq(Discourse.current_hostname) + end + end end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index ae5e2795bf7..9d4ab992cca 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -200,7 +200,7 @@ describe UsersController do expect(response.status).to eq(200) expect(response.body).to have_tag("div#data-preloaded") do |element| json = JSON.parse(element.current_scope.attribute('data-preloaded').value) - expect(json['password_reset']).to include('{"is_developer":false,"admin":false,"second_factor_required":false,"backup_enabled":false}') + expect(json['password_reset']).to include('{"is_developer":false,"admin":false,"second_factor_required":false,"security_key_required":false,"backup_enabled":false}') end expect(session["password-#{token}"]).to be_blank @@ -313,7 +313,7 @@ describe UsersController do expect(response.body).to have_tag("div#data-preloaded") do |element| json = JSON.parse(element.current_scope.attribute('data-preloaded').value) - expect(json['password_reset']).to include('{"is_developer":false,"admin":false,"second_factor_required":true,"backup_enabled":false}') + expect(json['password_reset']).to include('{"is_developer":false,"admin":false,"second_factor_required":true,"security_key_required":false,"backup_enabled":false}') end put "/u/password-reset/#{token}", params: { @@ -346,6 +346,58 @@ describe UsersController do expect(user.user_auth_tokens.count).to eq(1) end end + + context 'security key authentication required' do + let!(:security_key) { Fabricate(:user_security_key, user: user, factor_type: UserSecurityKey.factor_types[:second_factor]) } + + it 'preloads with a security key challenge and allowed credential ids' do + token = user.email_tokens.create!(email: user.email).token + + get "/u/password-reset/#{token}" + + expect(response.body).to have_tag("div#data-preloaded") do |element| + json = JSON.parse(element.current_scope.attribute('data-preloaded').value) + password_reset = JSON.parse(json['password_reset']) + expect(password_reset['challenge']).not_to eq(nil) + expect(password_reset['allowed_credential_ids']).to eq([security_key.credential_id]) + expect(password_reset['security_key_required']).to eq(true) + end + end + + it 'stages a webauthn challenge and rp-id for the user' do + token = user.email_tokens.create!(email: user.email).token + + get "/u/password-reset/#{token}" + + secure_session = SecureSession.new(session["secure_session_id"]) + expect(secure_session["staged-webauthn-challenge-#{user.id}"]).not_to eq(nil) + expect(secure_session["staged-webauthn-rp-id-#{user.id}"]).to eq(Discourse.current_hostname) + end + + it 'changes password with valid security key challenge and authentication' do + token = user.email_tokens.create(email: user.email).token + + get "/u/password-reset/#{token}" + + ::Webauthn::SecurityKeyAuthenticationService.any_instance.stubs(:authenticate_security_key).returns(true) + + put "/u/password-reset/#{token}", params: { + password: 'hg9ow8yHG32O', + security_key_credential: { + signature: 'test', + clientData: 'test', + authenticatorData: 'test', + credentialId: 'test' + }, + second_factor_method: UserSecondFactor.methods[:security_key] + } + + user.reload + expect(response.status).to eq(200) + expect(user.confirm_password?('hg9ow8yHG32O')).to eq(true) + expect(user.user_auth_tokens.count).to eq(1) + end + end end context 'submit change' do @@ -500,6 +552,55 @@ describe UsersController do expect(session[:current_user_id]).to eq(admin.id) end end + + describe 'when security key authentication required' do + fab!(:security_key) { Fabricate(:user_security_key, user: admin) } + fab!(:email_token) { Fabricate(:email_token, user: admin) } + + it 'does not log in when token required' do + security_key + get "/u/admin-login/#{email_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.security_key_authenticate')) + end + + describe 'invalid security key' do + it 'should display the right error' do + ::Webauthn::SecurityKeyAuthenticationService.any_instance.stubs(:authenticate_security_key).returns(false) + + put "/u/admin-login/#{email_token.token}", params: { + security_key_credential: { + signature: 'test', + clientData: 'test', + authenticatorData: 'test', + credentialId: 'test' + }.to_json, + second_factor_method: UserSecondFactor.methods[:security_key] + } + + expect(response.status).to eq(200) + expect(response.body).to include(I18n.t('login.security_key_invalid')) + end + end + + it 'logs in when a valid security key is given' do + ::Webauthn::SecurityKeyAuthenticationService.any_instance.stubs(:authenticate_security_key).returns(true) + + put "/u/admin-login/#{email_token.token}", params: { + security_key_credential: { + signature: 'test', + clientData: 'test', + authenticatorData: 'test', + credentialId: 'test' + }.to_json, + second_factor_method: UserSecondFactor.methods[:security_key] + } + + expect(response).to redirect_to('/') + expect(session[:current_user_id]).to eq(admin.id) + end + end end end @@ -3540,4 +3641,88 @@ describe UsersController do end end + + describe '#list_second_factors' do + before do + sign_in(user) + end + + context 'when SSO is enabled' do + before do + SiteSetting.sso_url = 'https://discourse.test/sso' + SiteSetting.enable_sso = true + end + + it 'does not allow access' do + post "/u/second_factors.json" + expect(response.status).to eq(404) + end + end + + context 'when local logins are not enabled' do + before do + SiteSetting.enable_local_logins = false + end + + it 'does not allow access' do + post "/u/second_factors.json" + expect(response.status).to eq(404) + end + end + + context 'when the site settings allow second factors' do + before do + SiteSetting.enable_local_logins = true + SiteSetting.enable_sso = false + end + + context 'when the password parameter is not provided' do + let(:password) { '' } + + before do + post "/u/second_factors.json", params: { password: password } + end + + it 'returns password required response' do + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body['password_required']).to eq(true) + end + end + + context 'when the password is provided' do + let(:user) { Fabricate(:user, password: '8555039dd212cc66ec68') } + + context 'when the password is correct' do + let(:password) { '8555039dd212cc66ec68' } + + it 'returns a list of enabled totps and security_key second factors' do + totp_second_factor = Fabricate(:user_second_factor_totp, user: user) + security_key_second_factor = Fabricate(:user_security_key, user: user, factor_type: UserSecurityKey.factor_types[:second_factor]) + + post "/u/second_factors.json", params: { password: password } + + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body['totps'].map { |second_factor| second_factor['id'] }).to include(totp_second_factor.id) + expect(response_body['security_keys'].map { |second_factor| second_factor['id'] }).to include(security_key_second_factor.id) + end + end + + context 'when the password is not correct' do + let(:password) { 'wrongpassword' } + + it 'returns the incorrect password response' do + + post "/u/second_factors.json", params: { password: password } + + response_body = JSON.parse(response.body) + expect(response_body['error']).to eq( + I18n.t("login.incorrect_password") + ) + end + end + end + end + end end diff --git a/test/javascripts/acceptance/sign-in-test.js.es6 b/test/javascripts/acceptance/sign-in-test.js.es6 index c7916b38e76..62520e8a629 100644 --- a/test/javascripts/acceptance/sign-in-test.js.es6 +++ b/test/javascripts/acceptance/sign-in-test.js.es6 @@ -101,6 +101,32 @@ QUnit.test("second factor", async assert => { ); }); +QUnit.test("security key", async assert => { + await visit("/"); + await click("header .login-button"); + + assert.ok(exists(".login-modal"), "it shows the login modal"); + + await fillIn("#login-account-name", "eviltrout"); + await fillIn("#login-account-password", "need-security-key"); + await click(".modal-footer .btn-primary"); + + assert.not(exists("#modal-alert:visible"), "it hides the login error"); + assert.not( + exists("#credentials:visible"), + "it hides the username and password prompt" + ); + assert.not( + exists("#login-second-factor:visible"), + "it does not display the second factor prompt" + ); + assert.ok( + exists("#security-key:visible"), + "it shows the security key prompt" + ); + assert.not(exists("#login-button:visible"), "hides the login button"); +}); + QUnit.test("create account", async assert => { await visit("/"); await click("header .sign-up-button"); diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index 9d229a654c1..915f082c20d 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -330,6 +330,20 @@ export default function() { }); } + if (data.password === "need-security-key") { + if (data.securityKeyCredential) { + return response({ username: "eviltrout" }); + } + + return response({ + error: "Invalid Security Key", + reason: "invalid_security_key", + backup_enabled: true, + sent_to_email: "eviltrout@example.com", + current_email: "current@example.com" + }); + } + return response(400, { error: "invalid login" }); });
{{text-field value=loginName placeholderKey="login.email_placeholder" id="login-account-name" autocorrect="off" autocapitalize="off" autofocus="autofocus" disabled=showSecondFactor}}{{text-field value=loginName placeholderKey="login.email_placeholder" id="login-account-name" autocorrect="off" autocapitalize="off" autofocus="autofocus" disabled=disableLoginFields}}
{{password-field value=loginPassword type="password" id="login-account-password" maxlength="200" capsLockOn=capsLockOn disabled=showSecondFactor}}{{password-field value=loginPassword type="password" id="login-account-password" maxlength="200" capsLockOn=capsLockOn disabled=disableLoginFields}} {{i18n 'forgot_password.action'}}