FIX: make it possible to use backup code everywhere where 2FA required (#7010)

This commit is contained in:
Maja Komel 2019-02-27 10:37:33 +01:00 committed by Régis Hanol
parent e1d1073273
commit 6f427589b2
15 changed files with 140 additions and 94 deletions

View File

@ -16,17 +16,23 @@ export default Ember.Component.extend({
: I18n.t("login.second_factor_backup_description");
},
@computed("secondFactorMethod")
linkText(secondFactorMethod) {
return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP
? "login.second_factor_backup"
: "login.second_factor";
@computed("secondFactorMethod", "isLogin")
linkText(secondFactorMethod, isLogin) {
if (isLogin) {
return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP
? "login.second_factor_backup"
: "login.second_factor";
} else {
return secondFactorMethod === SECOND_FACTOR_METHODS.TOTP
? "user.second_factor_backup.use"
: "user.second_factor.use";
}
},
actions: {
toggleSecondFactorMethod() {
const secondFactorMethod = this.get("secondFactorMethod");
this.set("loginSecondFactor", "");
this.set("secondFactorToken", "");
if (secondFactorMethod === SECOND_FACTOR_METHODS.TOTP) {
this.set("secondFactorMethod", SECOND_FACTOR_METHODS.BACKUP_CODE);
} else {

View File

@ -106,7 +106,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
data: {
login: this.get("loginName"),
password: this.get("loginPassword"),
second_factor_token: this.get("loginSecondFactor"),
second_factor_token: this.get("secondFactorToken"),
second_factor_method: this.get("secondFactorMethod")
}
}).then(

View File

@ -9,7 +9,7 @@ export default Ember.Controller.extend(PasswordValidation, {
isDeveloper: Ember.computed.alias("model.is_developer"),
admin: Ember.computed.alias("model.admin"),
secondFactorRequired: Ember.computed.alias("model.second_factor_required"),
backupEnabled: Ember.computed.alias("model.second_factor_backup_enabled"),
backupEnabled: Ember.computed.alias("model.backup_enabled"),
secondFactorMethod: SECOND_FACTOR_METHODS.TOTP,
passwordRequired: true,
errorMessage: null,
@ -38,7 +38,7 @@ export default Ember.Controller.extend(PasswordValidation, {
type: "PUT",
data: {
password: this.get("accountPassword"),
second_factor_token: this.get("secondFactor"),
second_factor_token: this.get("secondFactorToken"),
second_factor_method: this.get("secondFactorMethod")
}
})

View File

@ -1,6 +1,7 @@
import { default as computed } from "ember-addons/ember-computed-decorators";
import { default as DiscourseURL, userPath } from "discourse/lib/url";
import { popupAjaxError } from "discourse/lib/ajax-error";
import { SECOND_FACTOR_METHODS } from "discourse/models/user";
export default Ember.Controller.extend({
loading: false,
@ -11,10 +12,15 @@ export default Ember.Controller.extend({
"model.second_factor_remaining_backup_codes"
),
backupCodes: null,
secondFactorMethod: SECOND_FACTOR_METHODS.TOTP,
@computed("secondFactorToken")
isValidSecondFactorToken(secondFactorToken) {
return secondFactorToken && secondFactorToken.length === 6;
@computed("secondFactorToken", "secondFactorMethod")
isValidSecondFactorToken(secondFactorToken, secondFactorMethod) {
if (secondFactorMethod === SECOND_FACTOR_METHODS.TOTP) {
return secondFactorToken && secondFactorToken.length === 6;
} else if (secondFactorMethod === SECOND_FACTOR_METHODS.BACKUP_CODE) {
return secondFactorToken && secondFactorToken.length === 16;
}
},
@computed("isValidSecondFactorToken", "backupEnabled", "loading")
@ -59,7 +65,12 @@ export default Ember.Controller.extend({
this.set("loading", true);
this.get("model")
.toggleSecondFactor(this.get("secondFactorToken"), false, 2)
.toggleSecondFactor(
this.get("secondFactorToken"),
this.get("secondFactorMethod"),
SECOND_FACTOR_METHODS.BACKUP_CODE,
false
)
.then(response => {
if (response.error) {
this.set("errorMessage", response.error);
@ -79,7 +90,10 @@ export default Ember.Controller.extend({
if (!this.get("secondFactorToken")) return;
this.set("loading", true);
this.get("model")
.generateSecondFactorCodes(this.get("secondFactorToken"))
.generateSecondFactorCodes(
this.get("secondFactorToken"),
this.get("secondFactorMethod")
)
.then(response => {
if (response.error) {
this.set("errorMessage", response.error);

View File

@ -2,6 +2,7 @@ import { default as computed } from "ember-addons/ember-computed-decorators";
import { default as DiscourseURL, userPath } from "discourse/lib/url";
import { popupAjaxError } from "discourse/lib/ajax-error";
import { findAll } from "discourse/models/login-method";
import { SECOND_FACTOR_METHODS } from "discourse/models/user";
export default Ember.Controller.extend({
loading: false,
@ -13,6 +14,8 @@ export default Ember.Controller.extend({
showSecondFactorKey: false,
errorMessage: null,
newUsername: null,
backupEnabled: Ember.computed.alias("model.second_factor_backup_enabled"),
secondFactorMethod: SECOND_FACTOR_METHODS.TOTP,
loaded: Ember.computed.and("secondFactorImage", "secondFactorKey"),
@ -41,7 +44,12 @@ export default Ember.Controller.extend({
this.set("loading", true);
this.get("model")
.toggleSecondFactor(this.get("secondFactorToken"), enable, 1)
.toggleSecondFactor(
this.get("secondFactorToken"),
this.get("secondFactorMethod"),
SECOND_FACTOR_METHODS.TOTP,
enable
)
.then(response => {
if (response.error) {
this.set("errorMessage", response.error);

View File

@ -367,20 +367,24 @@ const User = RestModel.extend({
});
},
toggleSecondFactor(token, enable, method) {
toggleSecondFactor(authToken, authMethod, targetMethod, enable) {
return ajax("/u/second_factor.json", {
data: {
second_factor_token: token,
second_factor_method: method,
second_factor_token: authToken,
second_factor_method: authMethod,
second_factor_target: targetMethod,
enable
},
type: "PUT"
});
},
generateSecondFactorCodes(token) {
generateSecondFactorCodes(authToken, authMethod) {
return ajax("/u/second_factors_backup.json", {
data: { second_factor_token: token },
data: {
second_factor_token: authToken,
second_factor_method: authMethod
},
type: "PUT"
});
},

View File

@ -1,5 +1,8 @@
<div id="second-factor">
<h3>{{secondFactorTitle}}</h3>
{{#if optionalText}}
<p>{{{optionalText}}}</p>
{{/if}}
<p>{{secondFactorDescription}}</p>
{{yield}}
{{#if backupEnabled}}

View File

@ -1,4 +1,4 @@
{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword loginSecondFactor=loginSecondFactor action=(action "login")}}
{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword secondFactorToken=secondFactorToken action=(action "login")}}
{{#d-modal-body title="login.title" class="login-modal"}}
{{#if showLoginButtons}}
{{login-buttons
@ -36,8 +36,12 @@
</tr>
</table>
</div>
{{#second-factor-form secondFactorMethod=secondFactorMethod loginSecondFactor=loginSecondFactor backupEnabled=backupEnabled class=secondFactorClass}}
{{second-factor-input value=loginSecondFactor inputId='login-second-factor' secondFactorMethod=secondFactorMethod}}
{{#second-factor-form
secondFactorMethod=secondFactorMethod
secondFactorToken=secondFactorToken
class=secondFactorClass
isLogin=true}}
{{second-factor-input value=secondFactorToken inputId='login-second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}}
{{/second-factor-form}}
</form>
{{/if}}

View File

@ -1,4 +1,4 @@
{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword loginSecondFactor=loginSecondFactor action=(action "login")}}
{{#login-modal screenX=lastX screenY=lastY loginName=loginName loginPassword=loginPassword secondFactorToken=secondFactorToken action=(action "login")}}
{{#d-modal-body title="login.title" class=(concat "login-modal" " " (if hasAtLeastOneLoginButton "has-alt-auth"))}}
{{#if canLoginLocal}}
@ -21,8 +21,13 @@
</tr>
</table>
</div>
{{#second-factor-form secondFactorMethod=secondFactorMethod loginSecondFactor=loginSecondFactor backupEnabled=backupEnabled class=secondFactorClass}}
{{second-factor-input value=loginSecondFactor inputId='login-second-factor' secondFactorMethod=secondFactorMethod}}
{{#second-factor-form
secondFactorMethod=secondFactorMethod
secondFactorToken=secondFactorToken
class=secondFactorClass
backupEnabled=backupEnabled
isLogin=true}}
{{second-factor-input value=secondFactorToken inputId='login-second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}}
{{/second-factor-form}}
</form>
{{/if}}

View File

@ -17,16 +17,17 @@
{{else}}
<form>
{{#if secondFactorRequired}}
{{#second-factor-form secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}}
{{text-field
id="second-factor"
value=secondFactor
autocorrect="off"
autocapitalize="off"
autofocus="autofocus"
maxlength="6"
secondFactorMethod=secondFactorMethod
}}
{{#if errorMessage}}
<div class='alert alert-error'>{{errorMessage}}</div>
<br/>
{{/if}}
{{#second-factor-form
secondFactorMethod=secondFactorMethod
secondFactorToken=secondFactorToken
backupEnabled=backupEnabled
isLogin=false}}
{{second-factor-input value=secondFactorToken inputId='second-factor' secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}}
{{/second-factor-form}}
{{d-button action=(action "submit") class='btn-primary' label='submit'}}
{{else}}
@ -44,12 +45,6 @@
{{d-button action=(action "submit") class='btn-primary' label='user.change_password.set_password'}}
{{/if}}
{{#if errorMessage}}
<br/><br/>
<div class='alert alert-error'>{{errorMessage}}</div>
{{/if}}
</form>
{{/if}}
</div>

View File

@ -1,12 +1,7 @@
<section class="user-preferences second-factor-backup-preferences">
<form class="form-horizontal">
<div class="control-group">
<div class="controls">
<h3>{{i18n "user.second_factor_backup.title"}}</h3>
{{#if backupEnabled}}
<p>{{{i18n "user.second_factor_backup.remaining_codes" count=remainingCodes}}}</p>
{{/if}}
</div>
<div class="controls">
{{#if successMessage}}
<div class="alert alert-success">
@ -22,17 +17,17 @@
</div>
</div>
<label class='control-label'>{{i18n 'user.second_factor.label'}}</label>
<div class="control-group">
<div class="controls">
{{second-factor-input
value=secondFactorToken
maxlength=6
inputId="second-factor-token"}}
<div class='instructions'>
{{i18n "user.second_factor.disable_description"}}
</div>
{{#second-factor-form
secondFactorMethod=secondFactorMethod
backupEnabled=backupEnabled
secondFactorToken=secondFactorToken
secondFactorTitle=(i18n 'user.second_factor_backup.title')
optionalText=(if backupEnabled (i18n "user.second_factor_backup.remaining_codes" count=remainingCodes))
isLogin=false}}
{{second-factor-input value=secondFactorToken inputId='second-factor-token' secondFactorMethod=secondFactorMethod}}
{{/second-factor-form}}
</div>
</div>

View File

@ -1,12 +1,6 @@
<section class='user-preferences'>
<form class="form-horizontal">
<div class="control-group">
<div class="controls">
<h3>{{i18n 'user.second_factor.title'}}</h3>
</div>
</div>
{{#if errorMessage}}
<div class="control-group">
<div class="controls">
@ -16,15 +10,16 @@
{{/if}}
{{#if model.second_factor_enabled}}
<label class='control-label'>{{i18n 'user.second_factor.label'}}</label>
<div class="control-group">
<div class="controls">
{{second-factor-input maxlength=6 value=secondFactorToken inputId='second-factor-token'}}
</div>
<div class='instructions controls'>
{{i18n 'user.second_factor.disable_description'}}
{{#second-factor-form
secondFactorMethod=secondFactorMethod
backupEnabled=backupEnabled
secondFactorToken=secondFactorToken
secondFactorTitle=(i18n 'user.second_factor.title')
isLogin=false}}
{{second-factor-input value=secondFactorToken inputId='second-factor-token' secondFactorMethod=secondFactorMethod}}
{{/second-factor-form}}
</div>
</div>
@ -91,15 +86,16 @@
<label class='control-label'>{{i18n 'user.password.title'}}</label>
<div class="controls">
{{text-field value=password
<div>
{{text-field value=password
id="password"
type="password"
classNames="input-xxlarge"
autofocus="autofocus"}}
</div>
<div class='instructions controls'>
{{i18n 'user.second_factor.confirm_password_description'}}
</div>
<div class='instructions'>
{{i18n 'user.second_factor.confirm_password_description'}}
</div>
</div>
</div>

View File

@ -477,7 +477,7 @@ class UsersController < ApplicationController
second_factor_token = params[:second_factor_token]
second_factor_method = params[:second_factor_method].to_i
if second_factor_token.present? && second_factor_token[/\d{6}/] && UserSecondFactor.methods[second_factor_method]
if second_factor_token.present? && UserSecondFactor.methods[second_factor_method]
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
second_factor_authenticated = @user&.authenticate_second_factor(second_factor_token, second_factor_method)
end
@ -1059,7 +1059,7 @@ class UsersController < ApplicationController
def create_second_factor_backup
raise Discourse::NotFound if SiteSetting.enable_sso || !SiteSetting.enable_local_logins
unless current_user.authenticate_totp(params[:second_factor_token])
unless current_user.authenticate_second_factor(params[:second_factor_token], params[:second_factor_method].to_i)
return render json: failed_json.merge(
error: I18n.t("login.invalid_second_factor_code")
)
@ -1075,22 +1075,26 @@ class UsersController < ApplicationController
def update_second_factor
params.require(:second_factor_token)
params.require(:second_factor_method)
params.require(:second_factor_target)
second_factor_method = params[:second_factor_method].to_i
auth_method = params[:second_factor_method].to_i
auth_token = params[:second_factor_token]
update_second_factor_method = params[:second_factor_target].to_i
[request.remote_ip, current_user.id].each do |key|
RateLimiter.new(nil, "second-factor-min-#{key}", 3, 1.minute).performed!
end
if second_factor_method == UserSecondFactor.methods[:totp]
if update_second_factor_method == UserSecondFactor.methods[:totp]
user_second_factor = current_user.user_second_factors.totp
elsif second_factor_method == UserSecondFactor.methods[:backup_codes]
elsif update_second_factor_method == UserSecondFactor.methods[:backup_codes]
user_second_factor = current_user.user_second_factors.backup_codes
end
raise Discourse::InvalidParameters unless user_second_factor
unless current_user.authenticate_totp(params[:second_factor_token])
unless current_user.authenticate_second_factor(auth_token, auth_method)
return render json: failed_json.merge(
error: I18n.t("login.invalid_second_factor_code")
)
@ -1100,7 +1104,7 @@ class UsersController < ApplicationController
user_second_factor.update!(enabled: true)
else
# when disabling totp, backup is disabled too
if second_factor_method == UserSecondFactor.methods[:totp]
if update_second_factor_method == UserSecondFactor.methods[:totp]
current_user.user_second_factors.destroy_all
Jobs.enqueue(
@ -1108,7 +1112,7 @@ class UsersController < ApplicationController
type: :account_second_factor_disabled,
user_id: current_user.id
)
elsif second_factor_method == UserSecondFactor.methods[:backup_codes]
elsif update_second_factor_method == UserSecondFactor.methods[:backup_codes]
current_user.user_second_factors.where(method: UserSecondFactor.methods[:backup_codes]).destroy_all
end
end

View File

@ -782,6 +782,7 @@ en:
copied_to_clipboard: "Copied to Clipboard"
copy_to_clipboard_error: "Error copying data to Clipboard"
remaining_codes: "You have <strong>{{count}}</strong> backup codes remaining."
use: "<a href>Use a backup code</a>"
codes:
title: "Backup Codes Generated"
description: "Each of these backup codes can only be used once. Keep them somewhere safe but accessible."
@ -800,6 +801,7 @@ en:
extended_description: |
Two factor authentication adds extra security to your account by requiring a one-time token in addition to your password. Tokens can be generated on <a href="https://www.google.com/search?q=authenticator+apps+for+android" target='_blank'>Android</a> and <a href="https://www.google.com/search?q=authenticator+apps+for+ios">iOS</a> devices.
oauth_enabled_warning: "Please note that social logins will be disabled once two factor authentication has been enabled on your account."
use: "<a href>Use Authenticator app</a>"
change_about:
title: "Change About Me"

View File

@ -3107,6 +3107,7 @@ describe UsersController do
put "/users/second_factor.json", params: {
second_factor_token: '000000',
second_factor_method: UserSecondFactor.methods[:totp],
second_factor_target: UserSecondFactor.methods[:totp],
enable: 'true',
}
@ -3122,8 +3123,9 @@ describe UsersController do
it 'should allow second factor for the user to be enabled' do
put "/users/second_factor.json", params: {
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
enable: 'true',
second_factor_method: UserSecondFactor.methods[:totp]
second_factor_method: UserSecondFactor.methods[:totp],
second_factor_target: UserSecondFactor.methods[:totp],
enable: 'true'
}
expect(response.status).to eq(200)
@ -3133,7 +3135,8 @@ describe UsersController do
it 'should allow second factor for the user to be disabled' do
put "/users/second_factor.json", params: {
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
second_factor_method: UserSecondFactor.methods[:totp]
second_factor_method: UserSecondFactor.methods[:totp],
second_factor_target: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(200)
@ -3146,7 +3149,7 @@ describe UsersController do
context 'when token is missing' do
it 'returns the right response' do
put "/users/second_factor.json", params: {
second_factor_method: UserSecondFactor.methods[:backup_codes],
second_factor_target: UserSecondFactor.methods[:backup_codes]
}
expect(response.status).to eq(400)
@ -3157,7 +3160,8 @@ describe UsersController do
it 'returns the right response' do
put "/users/second_factor.json", params: {
second_factor_token: '000000',
second_factor_method: UserSecondFactor.methods[:backup_codes],
second_factor_method: UserSecondFactor.methods[:totp],
second_factor_target: UserSecondFactor.methods[:backup_codes]
}
expect(response.status).to eq(200)
@ -3172,7 +3176,8 @@ describe UsersController do
it 'should allow second factor backup for the user to be disabled' do
put "/users/second_factor.json", params: {
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
second_factor_method: UserSecondFactor.methods[:backup_codes]
second_factor_method: UserSecondFactor.methods[:totp],
second_factor_target: UserSecondFactor.methods[:backup_codes]
}
expect(response.status).to eq(200)
@ -3189,7 +3194,8 @@ describe UsersController do
context 'when not logged in' do
it 'should return the right response' do
put "/users/second_factors_backup.json", params: {
second_factor_token: 'wrongtoken'
second_factor_token: 'wrongtoken',
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(403)
@ -3204,7 +3210,8 @@ describe UsersController do
describe 'create 2fa request' do
it 'fails on incorrect password' do
put "/users/second_factors_backup.json", params: {
second_factor_token: 'wrongtoken'
second_factor_token: 'wrongtoken',
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(200)
@ -3219,7 +3226,8 @@ describe UsersController do
SiteSetting.enable_local_logins = false
put "/users/second_factors_backup.json", params: {
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(404)
@ -3232,7 +3240,8 @@ describe UsersController do
SiteSetting.enable_sso = true
put "/users/second_factors_backup.json", params: {
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(404)
@ -3243,7 +3252,8 @@ describe UsersController do
user_second_factor
put "/users/second_factors_backup.json", params: {
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.status).to eq(200)