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.

Sign up for GitHub

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

Jump to bottom

Rename user OTP methods to reference OTP instead of MFA [2/4] #3807

Merged
merged 3 commits into from
May 19, 2023
Merged

Rename user OTP methods to reference OTP instead of MFA [2/4] #3807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c6f4af
Rename disable_mfa to disable_otp
jenshenny May 18, 2023
7e87356
Rename verify_and_enable_mfa to verify_and_enable_otp
jenshenny May 18, 2023
976036e
Rename enable_mfa to enable_otp
jenshenny May 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/avo/actions/reset_user_2fa.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class ResetUser2fa < BaseAction

class ActionHandler < ActionHandler
def handle_model(user)
user.disable_mfa!
user.disable_otp!
user.password = SecureRandom.hex(20).encode("UTF-8")
user.save!
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/multifactor_auths_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def new
end

def create
current_user.verify_and_enable_mfa!(@seed, :ui_and_api, otp_param, @expire)
current_user.verify_and_enable_otp!(@seed, :ui_and_api, otp_param, @expire)
if current_user.errors.any?
flash[:error] = current_user.errors[:base].join
redirect_to edit_settings_url
Expand Down Expand Up @@ -76,7 +76,7 @@ def handle_new_level_param
case level_param
when "disabled"
flash[:success] = t("multifactor_auths.destroy.success")
current_user.disable_mfa!
current_user.disable_otp!
when "ui_only"
flash[:error] = t("multifactor_auths.ui_only_warning")
else
Expand Down
8 changes: 4 additions & 4 deletions app/models/concerns/user_otp_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
module UserOtpMethods
extend ActiveSupport::Concern

def disable_mfa!
def disable_otp!
mfa_disabled!
self.mfa_seed = ""
self.mfa_recovery_codes = []
save!(validate: false)
Mailer.mfa_disabled(id, Time.now.utc).deliver_later
end

def verify_and_enable_mfa!(seed, level, otp, expiry)
def verify_and_enable_otp!(seed, level, otp, expiry)
if expiry < Time.now.utc
errors.add(:base, I18n.t("multifactor_auths.create.qrcode_expired"))
elsif verify_digit_otp(seed, otp)
enable_mfa!(seed, level)
enable_otp!(seed, level)
else
errors.add(:base, I18n.t("multifactor_auths.incorrect_otp"))
end
end

def enable_mfa!(seed, level)
def enable_otp!(seed, level)
self.mfa_level = level
self.mfa_seed = seed
self.mfa_recovery_codes = Array.new(10).map { SecureRandom.hex(6) }
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def block!
transaction do
update_attribute(:email, "security+locked-#{SecureRandom.hex(4)}-#{display_handle.downcase}@rubygems.org")
confirm_email!
disable_mfa!
disable_otp!
update_attribute(:password, SecureRandom.alphanumeric)
update!(
remember_token: nil,
Expand Down
2 changes: 1 addition & 1 deletion script/disable_mfa
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require_relative "../config/environment"
begin
user = User.find_by_name(name)
puts "Found user: #{user.handle} #{user.email}"
user.disable_mfa!
user.disable_otp!
user.password = SecureRandom.hex(20).encode("UTF-8")
user.save!
puts "Done."
Expand Down
20 changes: 10 additions & 10 deletions test/functional/api/v1/api_keys_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def self.should_expect_otp_for_update
context "when user has enabled MFA for UI and API" do
setup do
@user = create(:user)
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
authorize_with("#{@user.email}:#{@user.password}")
end

Expand All @@ -234,7 +234,7 @@ def self.should_expect_otp_for_update
context "when user has enabled MFA for UI and gem signin" do
setup do
@user = create(:user)
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
authorize_with("#{@user.email}:#{@user.password}")
end

Expand Down Expand Up @@ -383,7 +383,7 @@ def self.should_expect_otp_for_update
context "when a user provides an OTP code" do
setup do
@user = create(:user)
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
authorize_with("#{@user.email}:#{@user.password}")
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
post :create, params: { name: "test-key", index_rubygems: "true" }, format: "text"
Expand Down Expand Up @@ -431,7 +431,7 @@ def self.should_expect_otp_for_update

context "when user has enabled MFA for UI and API" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
authorize_with("#{@user.email}:#{@user.password}")
end

Expand All @@ -440,7 +440,7 @@ def self.should_expect_otp_for_update

context "when user has enabled MFA for UI and gem signin" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
authorize_with("#{@user.email}:#{@user.password}")
end

Expand Down Expand Up @@ -473,7 +473,7 @@ def self.should_expect_otp_for_update

context "by user on `ui_only` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
post :create, params: { name: "test-key", index_rubygems: "true" }, format: "text"
end

Expand All @@ -492,7 +492,7 @@ def self.should_expect_otp_for_update

context "by user on `ui_and_gem_signin` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
post :create, params: { name: "test-key", index_rubygems: "true" }, format: "text"
end

Expand All @@ -505,7 +505,7 @@ def self.should_expect_otp_for_update

context "by user on `ui_and_api` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
post :create, params: { name: "test-key", index_rubygems: "true" }, format: "text"
end

Expand Down Expand Up @@ -579,7 +579,7 @@ def self.should_expect_otp_for_update

context "when user has enabled MFA for UI and API" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
authorize_with("#{@user.email}:#{@user.password}")
end

Expand All @@ -588,7 +588,7 @@ def self.should_expect_otp_for_update

context "when user has enabled MFA for UI and gem signin" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
authorize_with("#{@user.email}:#{@user.password}")
end

Expand Down
18 changes: 9 additions & 9 deletions test/functional/api/v1/deletions_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "when mfa for UI and API is enabled" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
end

context "ON DELETE to create for existing gem version without OTP" do
Expand Down Expand Up @@ -82,7 +82,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "when mfa for UI only is enabled" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
end

context "api key has mfa enabled" do
Expand Down Expand Up @@ -110,7 +110,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "when user has mfa enabled" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
delete :create, params: { gem_name: @rubygem.to_param, version: @v1.number }
end
Expand Down Expand Up @@ -202,7 +202,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "by user on `ui_only` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
delete :create, params: { gem_name: @rubygem.name, version: @v1.number }
end

Expand All @@ -222,7 +222,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "by user on `ui_and_gem_signin` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
delete :create, params: { gem_name: @rubygem.name, version: @v1.number }
end

Expand All @@ -235,7 +235,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "by user on `ui_and_api` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
delete :create, params: { gem_name: @rubygem.name, version: @v1.number }
end
Expand Down Expand Up @@ -304,7 +304,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "by user on `ui_only` mfa level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
end

should "include change mfa level warning" do
Expand All @@ -327,7 +327,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "by user on `ui_and_gem_signin` mfa level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
end

should "not include mfa warnings" do
Expand All @@ -345,7 +345,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase

context "by user on `ui_and_api` mfa level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
end

should "not include mfa warnings" do
Expand Down
34 changes: 17 additions & 17 deletions test/functional/api/v1/owners_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def self.should_respond_to(format)

context "when mfa for UI and API is enabled" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
end

context "array of emails" do
Expand Down Expand Up @@ -174,7 +174,7 @@ def self.should_respond_to(format)

context "when mfa for UI only is enabled" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
end

context "api key has mfa enabled" do
Expand Down Expand Up @@ -253,7 +253,7 @@ def self.should_respond_to(format)

context "api user has enabled mfa" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
end

Expand Down Expand Up @@ -376,7 +376,7 @@ def self.should_respond_to(format)

context "by user on `ui_only` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
end

should "block adding the owner" do
Expand All @@ -397,7 +397,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_gem_signin` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
end

should "not show error message" do
Expand All @@ -411,7 +411,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_api` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
end

Expand Down Expand Up @@ -449,7 +449,7 @@ def self.should_respond_to(format)

context "by user on `ui_only` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
end

should "include change mfa level warning" do
Expand All @@ -470,7 +470,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_gem_signin` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
end

should "not include MFA warnings" do
Expand All @@ -485,7 +485,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_api` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
end

Expand Down Expand Up @@ -542,7 +542,7 @@ def self.should_respond_to(format)

context "when mfa for UI and API is enabled" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
end

context "removing gem owner without OTP" do
Expand Down Expand Up @@ -639,7 +639,7 @@ def self.should_respond_to(format)

context "api user has enabled mfa" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
end

Expand Down Expand Up @@ -742,7 +742,7 @@ def self.should_respond_to(format)

context "by user on `ui_only` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
end

should "block adding the owner" do
Expand All @@ -763,7 +763,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_gem_signin` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
end

should "not show error message" do
Expand All @@ -777,7 +777,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_api` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
end

Expand Down Expand Up @@ -815,7 +815,7 @@ def self.should_respond_to(format)

context "by user on `ui_only` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_only)
end

should "include change mfa level warning" do
Expand All @@ -836,7 +836,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_gem_signin` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_gem_signin)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
end

should "not include mfa warnings" do
Expand All @@ -851,7 +851,7 @@ def self.should_respond_to(format)

context "by user on `ui_and_api` level" do
setup do
@user.enable_mfa!(ROTP::Base32.random_base32, :ui_and_api)
@user.enable_otp!(ROTP::Base32.random_base32, :ui_and_api)
@request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.mfa_seed).now
end

Expand Down