Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent changing MFA to ui_only #3084

Merged
merged 1 commit into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions app/controllers/multifactor_auths_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,7 @@ def create

def update
if current_user.otp_verified?(otp_param)
if level_param == "disabled"
flash[:success] = t("multifactor_auths.destroy.success")
current_user.disable_mfa!
else
flash[:error] = t(".success")
current_user.update!(mfa_level: level_param)
end
handle_new_level_param
else
flash[:error] = t("multifactor_auths.incorrect_otp")
end
Expand Down Expand Up @@ -72,4 +66,17 @@ def seed_and_expire
session.delete(key)
end
end

def handle_new_level_param
case level_param
when "disabled"
flash[:success] = t("multifactor_auths.destroy.success")
current_user.disable_mfa!
when "ui_only"
flash[:error] = t("multifactor_auths.ui_only_warning")
else
flash[:error] = t(".success")
current_user.update!(mfa_level: level_param)
end
end
end
6 changes: 6 additions & 0 deletions app/controllers/settings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,11 @@ class SettingsController < ApplicationController

def edit
@user = current_user
@mfa_options = [
[t(".mfa.level.ui_and_api"), "ui_and_api"],
[t(".mfa.level.ui_and_gem_signin"), "ui_and_gem_signin"],
[t(".mfa.level.disabled"), "disabled"]
]
@mfa_options.insert(2, [t(".mfa.level.ui_only"), "ui_only"]) if @user.mfa_ui_only?
end
end
9 changes: 4 additions & 5 deletions app/views/settings/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
<p><%= t(".mfa.enabled_html") %></p>
<%= form_tag multifactor_auth_path, method: :put, id: "mfa-edit" do %>
<%= label_tag :level, t(".mfa.level.title"), class: "form__label" %>
<%= select_tag :level, options_for_select([
[t(".mfa.level.ui_and_api"), "ui_and_api"],
[t(".mfa.level.ui_and_gem_signin"), "ui_and_gem_signin"],
[t(".mfa.level.ui_only"), "ui_only"],
[t(".mfa.level.disabled"), "disabled"]], @user.mfa_level), class: "form__input form__select" %>


<%= select_tag :level, options_for_select(@mfa_options, @user.mfa_level), class: "form__input form__select" %>

<div class="text_field">
<%= label_tag :otp, "OTP code", class: "form__label" %>
<%= text_field_tag :otp, "", class: "form__input", autocomplete: :off %>
Expand Down
1 change: 1 addition & 0 deletions config/locales/de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ de:
require_mfa_enabled:
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title:
scan_prompt:
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ en:
require_mfa_enabled: Your multi-factor authentication has not been enabled. You have to enable it first.
setup_recommended: For protection of your account and your gems, we encourage you to set up multi-factor authentication. Your account will be required to have MFA enabled in the future.
strong_mfa_level_recommended: For protection of your account and your gems, we encourage you to change your MFA level to "UI and gem signin" or "UI and API". Your account will be required to have MFA enabled on one of these levels in the future.
ui_only_warning: Updating multi-factor authentication to "UI Only" is no longer supported. Please use "UI and gem signin" or "UI and API".
new:
title: Enabling multi-factor auth
scan_prompt: Please scan the qr-code with your authenticator app. If you cannot scan the code, add information below into your app manually.
Expand Down
1 change: 1 addition & 0 deletions config/locales/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ es:
Primero tienes que activarla.
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title: Activando autenticación de múltiples factores
scan_prompt: Por favor escanea el código QR con tu aplicación de Autenticación.
Expand Down
1 change: 1 addition & 0 deletions config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ fr:
Vous devez d'abord l'activer.
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title: Activer l'authentification multifacteur
scan_prompt: Veuillez scanner le QR code avec votre app d'authentification.
Expand Down
1 change: 1 addition & 0 deletions config/locales/ja.yml
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ ja:
require_mfa_enabled:
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title:
scan_prompt:
Expand Down
1 change: 1 addition & 0 deletions config/locales/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ nl:
require_mfa_enabled:
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title:
scan_prompt:
Expand Down
1 change: 1 addition & 0 deletions config/locales/pt-BR.yml
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ pt-BR:
require_mfa_enabled:
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title:
scan_prompt:
Expand Down
1 change: 1 addition & 0 deletions config/locales/zh-CN.yml
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ zh-CN:
require_mfa_enabled: 你的多重要素验证已停用,请先启用。
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title: 启用多重要素验证
scan_prompt: 请用你的验证装置扫描 QR-code。如果你没办法扫描,手动输入下面的资料。
Expand Down
1 change: 1 addition & 0 deletions config/locales/zh-TW.yml
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ zh-TW:
require_mfa_enabled: 你的多重要素驗證已停用,請先啟用。
setup_recommended:
strong_mfa_level_recommended:
ui_only_warning:
new:
title: 啟用多重要素驗證
scan_prompt: 請用你的驗證裝置掃描 QR-code。如果你沒辦法掃描,手動輸入下面的資料。
Expand Down
11 changes: 8 additions & 3 deletions test/functional/multifactor_auths_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,21 @@ class MultifactorAuthsControllerTest < ActionController::TestCase
end
end

context "on updating to ui_only" do
kevinlinxc marked this conversation as resolved.
Show resolved Hide resolved
context "on updating to ui_only, flash banner is set and mfa level is unchanged" do
setup do
@user.mfa_ui_and_api!
put :update, params: { otp: ROTP::TOTP.new(@user.mfa_seed).now, level: "ui_only" }
end

should respond_with :redirect
should redirect_to("the settings page") { edit_settings_path }
should "update mfa level to mfa_ui_only now" do
assert_predicate @user.reload, :mfa_ui_only?
expected = "Updating multi-factor authentication to \"UI Only\" is no longer supported. Please use \"UI and gem signin\" or \"UI and API\"."
should "set flash" do
assert_equal(expected, flash[:error])
end

should "mfa level should be same as before" do
assert_predicate @user.reload, :mfa_ui_and_api?
end
end

Expand Down
21 changes: 21 additions & 0 deletions test/integration/settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,27 @@ def mfa_key
assert_equal page.current_path, root_path
end

test "shows 'ui only' if user's level is ui_only" do
sign_in
enable_mfa
visit edit_settings_path

assert page.has_selector?("#level > option:nth-child(4)")
assert page.has_content? "UI Only"
end

test "does not shows 'ui only' if user's level is not ui_only" do
sign_in
enable_mfa
visit edit_settings_path

page.fill_in "otp", with: ROTP::TOTP.new(@user.mfa_seed).now
change_auth_level "Disabled"

refute page.has_selector?("#level > option:nth-child(4)")
refute page.has_content? "UI Only"
end

teardown do
Capybara.reset_sessions!
Capybara.use_default_driver
Expand Down