mirror of
https://github.com/discourse/discourse.git
synced 2025-06-04 11:11:13 +08:00
FEATURE: Implement 2factor login TOTP
implemented review items. Blocking previous codes - valid 2-factor auth tokens can only be authenticated once/30 seconds. I played with updating the “last used” any time the token was attempted but that seemed to be overkill, and frustrating as to why a token would fail. Translatable texts. Move second factor logic to a helper class. Move second factor specific controller endpoints to its own controller. Move serialization logic for 2-factor details in admin user views. Add a login ember component for de-duplication Fix up code formatting Change verbiage of google authenticator add controller tests: second factor controller tests change email tests change password tests admin login tests add qunit tests - password reset, preferences fix: check for 2factor on change email controller fix: email controller - only show second factor errors on attempt fix: check against 'true' to enable second factor. Add modal for explaining what 2fa with links to Google Authenticator/FreeOTP add two factor to email signin link rate limit if second factor token present add rate limiter test for second factor attempts
This commit is contained in:
@ -25,7 +25,8 @@ class Admin::UsersController < Admin::AdminController
|
||||
:generate_api_key,
|
||||
:revoke_api_key,
|
||||
:anonymize,
|
||||
:reset_bounce_score]
|
||||
:reset_bounce_score,
|
||||
:disable_second_factor]
|
||||
|
||||
def index
|
||||
users = ::AdminUserIndexQuery.new(params).find_users
|
||||
@ -340,6 +341,18 @@ class Admin::UsersController < Admin::AdminController
|
||||
}
|
||||
end
|
||||
|
||||
def disable_second_factor
|
||||
guardian.ensure_can_disable_second_factor! @user
|
||||
if @user.user_second_factor.try(:delete)
|
||||
StaffActionLogger.new(current_user).log_disable_second_factor_auth(@user)
|
||||
end
|
||||
Jobs.enqueue(
|
||||
:critical_user_email,
|
||||
type: :account_second_factor_disabled,
|
||||
user_id: @user.id
|
||||
)
|
||||
end
|
||||
|
||||
def destroy
|
||||
user = User.find_by(id: params[:id].to_i)
|
||||
guardian.ensure_can_delete_user!(user)
|
||||
|
51
app/controllers/second_factor_controller.rb
Normal file
51
app/controllers/second_factor_controller.rb
Normal file
@ -0,0 +1,51 @@
|
||||
class SecondFactorController < ApplicationController
|
||||
|
||||
def create
|
||||
RateLimiter.new(nil, "login-hr-#{request.remote_ip}", SiteSetting.max_logins_per_ip_per_hour, 1.hour).performed!
|
||||
RateLimiter.new(nil, "login-min-#{request.remote_ip}", SiteSetting.max_logins_per_ip_per_minute, 1.minute).performed!
|
||||
if user = User.find_by_username_or_email(params[:login])
|
||||
unless user.confirm_password?(params[:password])
|
||||
return invalid_credentials
|
||||
end
|
||||
qrcode = RQRCode::QRCode.new(SecondFactorHelper.provisioning_uri(user))
|
||||
qrcode_svg = qrcode.as_svg(
|
||||
offset: 0,
|
||||
color: '000',
|
||||
shape_rendering: 'crispEdges',
|
||||
module_size: 4
|
||||
)
|
||||
render json: { key: user.user_second_factor.data, qr: qrcode_svg }
|
||||
end
|
||||
end
|
||||
|
||||
def update
|
||||
params.require(:token)
|
||||
user = fetch_user_from_params
|
||||
unless SecondFactorHelper.authenticate(user, params[:token])
|
||||
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
|
||||
render json: { error: I18n.t("login.invalid_second_factor_code") }
|
||||
return
|
||||
end
|
||||
if params[:enable] == "true"
|
||||
SecondFactorHelper.create_totp(user)
|
||||
user.user_second_factor.enabled = true
|
||||
user.user_second_factor.save!
|
||||
return render json: { result: "ok", action: "enabled" }
|
||||
else
|
||||
user.user_second_factor.delete
|
||||
Jobs.enqueue(
|
||||
:critical_user_email,
|
||||
type: :account_second_factor_disabled,
|
||||
user_id: user.id
|
||||
)
|
||||
return render json: { result: "ok", action: "disabled" }
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def invalid_credentials
|
||||
render json: { error: I18n.t("login.incorrect_username_email_or_password") }
|
||||
end
|
||||
|
||||
end
|
@ -188,6 +188,10 @@ class SessionController < ApplicationController
|
||||
end
|
||||
|
||||
def create
|
||||
unless params[:second_factor_token].blank?
|
||||
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
|
||||
end
|
||||
|
||||
params.require(:login)
|
||||
params.require(:password)
|
||||
|
||||
@ -221,6 +225,12 @@ class SessionController < ApplicationController
|
||||
if payload = login_error_check(user)
|
||||
render json: payload
|
||||
else
|
||||
|
||||
if SecondFactorHelper.totp_enabled?(user)
|
||||
unless SecondFactorHelper.authenticate(user, params[:second_factor_token])
|
||||
return render json: { error: I18n.t("login.invalid_second_factor_code"), reason: "invalid_second_factor" }
|
||||
end
|
||||
end
|
||||
(user.active && user.email_confirmed?) ? login(user) : not_activated(user)
|
||||
end
|
||||
end
|
||||
@ -228,6 +238,14 @@ class SessionController < ApplicationController
|
||||
def email_login
|
||||
raise Discourse::NotFound if !SiteSetting.enable_local_logins_via_email
|
||||
|
||||
if params[:second_factor_token].present?
|
||||
@error = I18n.t("login.invalid_second_factor_code")
|
||||
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
|
||||
end
|
||||
unless EmailToken.second_factor_valid(params[:token], params[:second_factor_token])
|
||||
@second_factor_required = true
|
||||
return render layout: 'no_ember'
|
||||
end
|
||||
if EmailToken.valid_token_format?(params[:token]) && (user = EmailToken.confirm(params[:token]))
|
||||
if login_not_approved_for?(user)
|
||||
@error = login_not_approved[:error]
|
||||
|
@ -470,12 +470,21 @@ class UsersController < ApplicationController
|
||||
end
|
||||
end
|
||||
|
||||
if @user && (!SecondFactorHelper.totp_enabled?(@user) || SecondFactorHelper.authenticate(@user, params[:second_factor_token]))
|
||||
secure_session["second-factor-#{token}"] = "true"
|
||||
end
|
||||
@valid_second_factor = secure_session["second-factor-#{token}"] == "true"
|
||||
|
||||
if !@user
|
||||
@error = I18n.t('password_reset.no_token')
|
||||
elsif request.put?
|
||||
@invalid_password = params[:password].blank? || params[:password].length > User.max_password_length
|
||||
|
||||
if @invalid_password
|
||||
if !@valid_second_factor
|
||||
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
|
||||
@user.errors.add(:second_factor, :invalid)
|
||||
@error = I18n.t('login.invalid_second_factor_code')
|
||||
elsif @invalid_password
|
||||
@user.errors.add(:password, :invalid)
|
||||
else
|
||||
@user.password = params[:password]
|
||||
@ -484,6 +493,7 @@ class UsersController < ApplicationController
|
||||
if @user.save
|
||||
Invite.invalidate_for_email(@user.email) # invite link can't be used to log in anymore
|
||||
secure_session["password-#{token}"] = nil
|
||||
secure_session["second-factor-#{token}"] = nil
|
||||
logon_after_password_reset
|
||||
end
|
||||
end
|
||||
@ -496,7 +506,7 @@ class UsersController < ApplicationController
|
||||
else
|
||||
store_preloaded(
|
||||
"password_reset",
|
||||
MultiJson.dump(is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin?)
|
||||
MultiJson.dump(is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin?, second_factor_required: !@valid_second_factor)
|
||||
)
|
||||
end
|
||||
return redirect_to(wizard_path) if request.put? && Wizard.user_requires_completion?(@user)
|
||||
@ -521,7 +531,7 @@ class UsersController < ApplicationController
|
||||
}
|
||||
end
|
||||
else
|
||||
render json: { is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin? }
|
||||
render json: { is_developer: UsernameCheckerService.is_developer?(@user.email), admin: @user.admin?, second_factor_required: !@valid_second_factor }
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -550,7 +560,7 @@ class UsersController < ApplicationController
|
||||
def admin_login
|
||||
return redirect_to(path("/")) if current_user
|
||||
|
||||
if request.put?
|
||||
if request.put? && params[:email].present?
|
||||
RateLimiter.new(nil, "admin-login-hr-#{request.remote_ip}", 6, 1.hour).performed!
|
||||
RateLimiter.new(nil, "admin-login-min-#{request.remote_ip}", 3, 1.minute).performed!
|
||||
|
||||
@ -563,13 +573,20 @@ class UsersController < ApplicationController
|
||||
end
|
||||
elsif params[:token].present?
|
||||
if EmailToken.valid_token_format?(params[:token])
|
||||
@user = EmailToken.confirm(params[:token])
|
||||
|
||||
if @user&.admin?
|
||||
log_on_user(@user)
|
||||
return redirect_to path("/")
|
||||
if params[:second_factor_token].present?
|
||||
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
|
||||
end
|
||||
if EmailToken.second_factor_valid(params[:token], params[:second_factor_token])
|
||||
@user = EmailToken.confirm(params[:token])
|
||||
if @user && @user.admin?
|
||||
log_on_user(@user)
|
||||
return redirect_to path("/")
|
||||
else
|
||||
@message = I18n.t("admin_login.errors.unknown_email_address")
|
||||
end
|
||||
else
|
||||
@message = I18n.t("admin_login.errors.unknown_email_address")
|
||||
@second_factor_required = true
|
||||
@message = I18n.t("login.second_factor_title")
|
||||
end
|
||||
else
|
||||
@message = I18n.t("admin_login.errors.invalid_token")
|
||||
|
@ -33,6 +33,21 @@ class UsersEmailController < ApplicationController
|
||||
|
||||
def confirm
|
||||
expires_now
|
||||
token = EmailToken.confirmable params[:token]
|
||||
change_req = token&.user&.email_change_requests
|
||||
&.where('new_email_token_id = :token_id', token_id: token.id)
|
||||
&.first
|
||||
if change_req.try(:change_state) == EmailChangeRequest.states[:authorizing_new] &&
|
||||
!EmailToken.second_factor_valid(params[:token], params[:second_factor_token])
|
||||
@update_result = :invalid_second_factor
|
||||
if params[:second_factor_token].present?
|
||||
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
|
||||
@show_invalid_second_factor_error = true
|
||||
end
|
||||
render layout: 'no_ember'
|
||||
return
|
||||
end
|
||||
|
||||
updater = EmailUpdater.new
|
||||
@update_result = updater.confirm(params[:token])
|
||||
|
||||
|
Reference in New Issue
Block a user