From 35389cda285d2c923ee6a497e90bb44ab9c0cd81 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Fri, 27 Jul 2018 18:44:49 +0800
Subject: [PATCH 01/19] Add multifactor-auth prompt to push command.

---
 lib/rubygems/commands/push_command.rb         |  6 ++++
 lib/rubygems/gemcutter_utilities.rb           | 34 +++++++++++++++++++
 .../test_gem_commands_push_command.rb         | 19 +++++++++++
 3 files changed, 59 insertions(+)

diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index 672138c73186..4e3324538909 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -41,6 +41,10 @@ def initialize
       @user_defined_host = true
     end
 
+    add_option('--otp CODE', 'Digit code for multifactor authentication') do |value, options|
+      options[:otp] = value
+    end
+
     @host = nil
   end
 
@@ -68,6 +72,7 @@ def execute
             end
 
     sign_in @host
+    run_mfa_check
 
     send_gem(gem_name)
   end
@@ -118,6 +123,7 @@ def send_gem(name)
       request.add_field "Content-Length", request.body.size
       request.add_field "Content-Type",   "application/octet-stream"
       request.add_field "Authorization",  api_key
+      request.add_field "OTP", options[:otp] if need_otp?
     end
 
     with_response response
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 0f455689ec7d..5952d615bcc7 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -119,6 +119,40 @@ def sign_in sign_in_host = nil
     end
   end
 
+  ##
+  # Fetch user's multifactor authentication settings and return if an extra
+  # OTP code is needed.
+
+  def need_otp?
+    return false if options[:suppress_mfa]
+    unless instance_variable_defined? :@mfa_level
+      response = rubygems_api_request(:get, 'api/v1/multifactor_auth') do |request|
+        request.add_field 'Authorization', api_key
+      end
+
+      # For compatibility to Gemcutters without mfa support
+      @mfa_level = case response
+                   when Net::HTTPNotFound
+                     'no_mfa'
+                   else
+                     with_response(response) { |resp| resp.body }
+                   end
+    end
+
+    @mfa_level == 'mfa_login_and_write'
+  end
+
+  ##
+  # Require user for extra OTP code if multifactor authentication is enabled.
+
+  def run_mfa_check
+    return unless need_otp?
+    unless options[:otp]
+      say 'This command needs an extra OTP code for multifactor authentication.'
+      options[:otp] = ask 'Code: '
+    end
+  end
+
   ##
   # Retrieves the pre-configured API key +key+ or terminates interaction with
   # an error.
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index ccc46d4f4545..4f938310bf1c 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -51,6 +51,8 @@ def send_battery
       @cmd.send_gem(@path)
     end
 
+    @cmd.options[:suppress_mfa] = true
+
     assert_match %r{Pushing gem to #{@host}...}, @ui.output
 
     assert_equal Net::HTTP::Post, @fetcher.last_request.class
@@ -67,6 +69,7 @@ def test_execute
     @fetcher.data["#{Gem.host}/api/v1/gems"] = [@response, 200, 'OK']
 
     @cmd.options[:args] = [@path]
+    @cmd.options[:suppress_mfa] = true
 
     @cmd.execute
 
@@ -86,6 +89,7 @@ def test_execute_host
 
     @cmd.options[:host] = host
     @cmd.options[:args] = [@path]
+    @cmd.options[:suppress_mfa] = true
 
     @cmd.execute
 
@@ -119,6 +123,8 @@ def test_sending_when_default_host_disabled
     Gem.configuration.disable_default_gem_server = true
     response = "You must specify a gem server"
 
+    @cmd.options[:suppress_mfa] = true
+
     assert_raises Gem::MockGemUi::TermError do
       use_ui @ui do
         @cmd.send_gem(@path)
@@ -133,6 +139,7 @@ def test_sending_when_default_host_disabled_with_override
     Gem.configuration.disable_default_gem_server = true
     @response = "Successfully registered gem: freewill (1.0.0)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
+    @cmd.options[:suppress_mfa] = true
 
     send_battery
   end
@@ -145,6 +152,7 @@ def test_sending_gem_to_metadata_host
     end
 
     @api_key = "EYKEY"
+    @cmd.options[:suppress_mfa] = true
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -168,6 +176,8 @@ def test_sending_gem
     @response = "Successfully registered gem: freewill (1.0.0)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
 
+    @cmd.options[:suppress_mfa] = true
+
     send_battery
   end
 
@@ -179,6 +189,7 @@ def test_sending_gem_to_allowed_push_host
     end
 
     @api_key = "PRIVKEY"
+    @cmd.options[:suppress_mfa] = true
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -207,6 +218,7 @@ def test_sending_gem_to_allowed_push_host_with_basic_credentials
     end
 
     @api_key = "DOESNTMATTER"
+    @cmd.options[:suppress_mfa] = true
 
     keys = {
       :rubygems_api_key => @api_key,
@@ -230,6 +242,8 @@ def test_sending_gem_to_disallowed_default_host
       spec.metadata['allowed_push_host'] = "https://privategemserver.example"
     end
 
+    @cmd.options[:suppress_mfa] = true
+
     response = %{ERROR:  "#{@host}" is not allowed by the gemspec, which only allows "https://privategemserver.example"}
 
     assert_raises Gem::MockGemUi::TermError do
@@ -248,6 +262,7 @@ def test_sending_gem_to_disallowed_push_host
     end
 
     @api_key = "PRIVKEY"
+    @cmd.options[:suppress_mfa] = true
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -280,6 +295,7 @@ def test_sending_gem_defaulting_to_allowed_push_host
     end
 
     api_key = "PRIVKEY"
+    @cmd.options[:suppress_mfa] = true
 
     keys = {
       host => api_key
@@ -312,6 +328,7 @@ def test_sending_gem_defaulting_to_allowed_push_host
 
   def test_raises_error_with_no_arguments
     def @cmd.sign_in(*); end
+    @cmd.options[:suppress_mfa] = true
     assert_raises Gem::CommandLineError do
       @cmd.execute
     end
@@ -321,6 +338,7 @@ def test_sending_gem_denied
     response = "You don't have permission to push to this gem"
     @fetcher.data["#{@host}/api/v1/gems"] = [response, 403, 'Forbidden']
     @cmd.instance_variable_set :@host, @host
+    @cmd.options[:suppress_mfa] = true
 
     assert_raises Gem::MockGemUi::TermError do
       use_ui @ui do
@@ -340,6 +358,7 @@ def test_sending_gem_key
     Gem.configuration.load_api_keys
 
     @cmd.handle_options %w(-k other)
+    @cmd.options[:suppress_mfa] = true
     @cmd.instance_variable_set :@host, @host
     @cmd.send_gem(@path)
 

From 9d2235e1b713110b914be67803c0928f00328eb6 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Sat, 28 Jul 2018 16:21:57 +0800
Subject: [PATCH 02/19] Add OTP requirement to `owner_command`.

---
 lib/rubygems/commands/owner_command.rb           | 7 +++++++
 test/rubygems/test_gem_commands_owner_command.rb | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index 637b5bdc4dfe..8bedfc784c72 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -45,12 +45,18 @@ def initialize
                '  (e.g. https://rubygems.org)' do |value, options|
       options[:host] = value
     end
+
+    add_option '--otp CODE',
+               'Digit code for multifactor authentication' do |value, options|
+      options[:otp] = value
+    end
   end
 
   def execute
     @host = options[:host]
 
     sign_in
+    run_mfa_check
     name = get_one_gem_name
 
     add_owners    name, options[:add]
@@ -87,6 +93,7 @@ def manage_owners method, name, owners
         response = rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
           request.set_form_data 'email' => owner
           request.add_field "Authorization", api_key
+          request.add_field "OTP", options[:otp] if need_otp?
         end
 
         action = method == :delete ? "Removing" : "Adding"
diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index 1f9a2efbcad1..3caa4759986e 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -129,6 +129,7 @@ def test_show_owners_key
   def test_add_owners
     response = "Owner added successfully."
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 200, 'OK']
+    @cmd.options[:suppress_mfa] = true
 
     use_ui @stub_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
@@ -144,6 +145,7 @@ def test_add_owners
   def test_add_owners_denied
     response = "You don't have permission to push to this gem"
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 403, 'Forbidden']
+    @cmd.options[:suppress_mfa] = true
 
     use_ui @stub_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
@@ -160,6 +162,7 @@ def test_add_owner_with_host_option_through_execute
     @stub_fetcher.data["#{host}/api/v1/gems/freewill/owners.yaml"] = [show_owners_response, 200, 'OK']
 
     @cmd.handle_options %W[--host #{host} --add user-new1@example.com freewill]
+    @cmd.options[:suppress_mfa] = true
 
     use_ui @stub_ui do
       @cmd.execute
@@ -180,6 +183,7 @@ def test_add_owners_key
 
     @cmd.handle_options %w(-k other)
     @cmd.add_owners('freewill', ['user-new1@example.com'])
+    @cmd.options[:suppress_mfa] = true
 
     assert_equal '701229f217cdf23b1344c7b4b54ca97', @stub_fetcher.last_request['Authorization']
   end
@@ -187,6 +191,7 @@ def test_add_owners_key
   def test_remove_owners
     response = "Owner removed successfully."
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 200, 'OK']
+    @cmd.options[:suppress_mfa] = true
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["user-remove1@example.com"])
@@ -202,6 +207,7 @@ def test_remove_owners
   def test_remove_owners_denied
     response = "You don't have permission to push to this gem"
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 403, 'Forbidden']
+    @cmd.options[:suppress_mfa] = true
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["user-remove1@example.com"])
@@ -220,6 +226,7 @@ def test_remove_owners_key
 
     @cmd.handle_options %w(-k other)
     @cmd.remove_owners('freewill', ['user-remove1@example.com'])
+    @cmd.options[:suppress_mfa] = true
 
     assert_equal '701229f217cdf23b1344c7b4b54ca97', @stub_fetcher.last_request['Authorization']
   end
@@ -227,6 +234,7 @@ def test_remove_owners_key
   def test_remove_owners_missing
     response = 'Owner could not be found.'
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 404, 'Not Found']
+    @cmd.options[:suppress_mfa] = true
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["missing@example"])

From 816278997cd7fd189da7826b6197ca8da987a196 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Tue, 31 Jul 2018 14:58:21 +0800
Subject: [PATCH 03/19] Change `Gemcutter` in comment to `server`.

---
 lib/rubygems/gemcutter_utilities.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 5952d615bcc7..46a2932fe1cc 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -130,7 +130,7 @@ def need_otp?
         request.add_field 'Authorization', api_key
       end
 
-      # For compatibility to Gemcutters without mfa support
+      # For compatibility to servers without mfa support
       @mfa_level = case response
                    when Net::HTTPNotFound
                      'no_mfa'

From 17824f6c48e0597fa327492ac5cd9eb4d85d0f7e Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Tue, 31 Jul 2018 15:06:40 +0800
Subject: [PATCH 04/19] Rename `run_mfa_check` to `check_mfa`.

---
 lib/rubygems/commands/owner_command.rb | 2 +-
 lib/rubygems/commands/push_command.rb  | 2 +-
 lib/rubygems/gemcutter_utilities.rb    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index 8bedfc784c72..40b2f2e86a3a 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -56,7 +56,7 @@ def execute
     @host = options[:host]
 
     sign_in
-    run_mfa_check
+    check_mfa
     name = get_one_gem_name
 
     add_owners    name, options[:add]
diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index 4e3324538909..0574fb3657bf 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -72,7 +72,7 @@ def execute
             end
 
     sign_in @host
-    run_mfa_check
+    check_mfa
 
     send_gem(gem_name)
   end
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 46a2932fe1cc..03f730535b54 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -145,7 +145,7 @@ def need_otp?
   ##
   # Require user for extra OTP code if multifactor authentication is enabled.
 
-  def run_mfa_check
+  def check_mfa
     return unless need_otp?
     unless options[:otp]
       say 'This command needs an extra OTP code for multifactor authentication.'

From ec1b87860a85978b4c1674bc7b4dc0540af33530 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Tue, 31 Jul 2018 15:19:20 +0800
Subject: [PATCH 05/19] Rename `otp` in code to `mfa`.

---
 lib/rubygems/commands/owner_command.rb |  6 +++---
 lib/rubygems/commands/push_command.rb  |  6 +++---
 lib/rubygems/gemcutter_utilities.rb    | 16 ++++++++--------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index 40b2f2e86a3a..9927e9f19ce1 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -46,9 +46,9 @@ def initialize
       options[:host] = value
     end
 
-    add_option '--otp CODE',
+    add_option '--mfa CODE',
                'Digit code for multifactor authentication' do |value, options|
-      options[:otp] = value
+      options[:mfa] = value
     end
   end
 
@@ -93,7 +93,7 @@ def manage_owners method, name, owners
         response = rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
           request.set_form_data 'email' => owner
           request.add_field "Authorization", api_key
-          request.add_field "OTP", options[:otp] if need_otp?
+          request.add_field "OTP", options[:mfa] if need_mfa?
         end
 
         action = method == :delete ? "Removing" : "Adding"
diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index 0574fb3657bf..c1ef5145cbfc 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -41,8 +41,8 @@ def initialize
       @user_defined_host = true
     end
 
-    add_option('--otp CODE', 'Digit code for multifactor authentication') do |value, options|
-      options[:otp] = value
+    add_option('--mfa CODE', 'Digit code for multifactor authentication') do |value, options|
+      options[:mfa] = value
     end
 
     @host = nil
@@ -123,7 +123,7 @@ def send_gem(name)
       request.add_field "Content-Length", request.body.size
       request.add_field "Content-Type",   "application/octet-stream"
       request.add_field "Authorization",  api_key
-      request.add_field "OTP", options[:otp] if need_otp?
+      request.add_field "OTP", options[:mfa] if need_mfa?
     end
 
     with_response response
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 03f730535b54..00b40ed1c596 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -120,10 +120,10 @@ def sign_in sign_in_host = nil
   end
 
   ##
-  # Fetch user's multifactor authentication settings and return if an extra
-  # OTP code is needed.
+  # Fetch user's multifactor authentication settings and return if extra digit
+  # code is needed.
 
-  def need_otp?
+  def need_mfa?
     return false if options[:suppress_mfa]
     unless instance_variable_defined? :@mfa_level
       response = rubygems_api_request(:get, 'api/v1/multifactor_auth') do |request|
@@ -143,13 +143,13 @@ def need_otp?
   end
 
   ##
-  # Require user for extra OTP code if multifactor authentication is enabled.
+  # Require user for digit code if multifactor authentication is enabled.
 
   def check_mfa
-    return unless need_otp?
-    unless options[:otp]
-      say 'This command needs an extra OTP code for multifactor authentication.'
-      options[:otp] = ask 'Code: '
+    return unless need_mfa?
+    unless options[:mfa]
+      say 'This command needs digit code for multifactor authentication.'
+      options[:mfa] = ask 'Code: '
     end
   end
 

From 5fcf46952750df111bcdf9751a083821737a5503 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Mon, 6 Aug 2018 22:01:49 +0800
Subject: [PATCH 06/19] Add a method for `--mfa` command options.

---
 lib/rubygems/commands/owner_command.rb |  6 +-----
 lib/rubygems/commands/push_command.rb  |  5 +----
 lib/rubygems/gemcutter_utilities.rb    | 10 ++++++++++
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index 9927e9f19ce1..d6abbcf6c943 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -30,6 +30,7 @@ def initialize
     super 'owner', 'Manage gem owners of a gem on the push server'
     add_proxy_option
     add_key_option
+    add_mfa_option
     defaults.merge! :add => [], :remove => []
 
     add_option '-a', '--add EMAIL', 'Add an owner' do |value, options|
@@ -45,11 +46,6 @@ def initialize
                '  (e.g. https://rubygems.org)' do |value, options|
       options[:host] = value
     end
-
-    add_option '--mfa CODE',
-               'Digit code for multifactor authentication' do |value, options|
-      options[:mfa] = value
-    end
   end
 
   def execute
diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index c1ef5145cbfc..63a678189ca0 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -33,6 +33,7 @@ def initialize
 
     add_proxy_option
     add_key_option
+    add_mfa_option
 
     add_option('--host HOST',
                'Push to another gemcutter-compatible host',
@@ -41,10 +42,6 @@ def initialize
       @user_defined_host = true
     end
 
-    add_option('--mfa CODE', 'Digit code for multifactor authentication') do |value, options|
-      options[:mfa] = value
-    end
-
     @host = nil
   end
 
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 00b40ed1c596..2fda58aac518 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -24,6 +24,16 @@ def add_key_option
     end
   end
 
+  ##
+  # Add the --mfa option
+
+  def add_mfa_option
+    add_option('--mfa CODE',
+               'Digit code for multifactor authentication') do |value, options|
+      options[:mfa] = value
+    end
+  end
+
   ##
   # The API key from the command options or from the user's configuration.
 

From 09c8b9adeac2abaa4c5cf40b159c42b3a6959359 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Mon, 6 Aug 2018 22:09:54 +0800
Subject: [PATCH 07/19] Remove `suppress_mfa` option for testing.

---
 lib/rubygems/gemcutter_utilities.rb           |  1 -
 .../test_gem_commands_owner_command.rb        | 16 ++++-----
 .../test_gem_commands_push_command.rb         | 34 +++++++++++--------
 3 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 2fda58aac518..d507cf578db4 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -134,7 +134,6 @@ def sign_in sign_in_host = nil
   # code is needed.
 
   def need_mfa?
-    return false if options[:suppress_mfa]
     unless instance_variable_defined? :@mfa_level
       response = rubygems_api_request(:get, 'api/v1/multifactor_auth') do |request|
         request.add_field 'Authorization', api_key
diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index 3caa4759986e..72dde13363e8 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -129,7 +129,7 @@ def test_show_owners_key
   def test_add_owners
     response = "Owner added successfully."
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 200, 'OK']
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
@@ -145,7 +145,7 @@ def test_add_owners
   def test_add_owners_denied
     response = "You don't have permission to push to this gem"
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 403, 'Forbidden']
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
@@ -162,7 +162,7 @@ def test_add_owner_with_host_option_through_execute
     @stub_fetcher.data["#{host}/api/v1/gems/freewill/owners.yaml"] = [show_owners_response, 200, 'OK']
 
     @cmd.handle_options %W[--host #{host} --add user-new1@example.com freewill]
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.execute
@@ -183,7 +183,7 @@ def test_add_owners_key
 
     @cmd.handle_options %w(-k other)
     @cmd.add_owners('freewill', ['user-new1@example.com'])
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_equal '701229f217cdf23b1344c7b4b54ca97', @stub_fetcher.last_request['Authorization']
   end
@@ -191,7 +191,7 @@ def test_add_owners_key
   def test_remove_owners
     response = "Owner removed successfully."
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 200, 'OK']
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["user-remove1@example.com"])
@@ -207,7 +207,7 @@ def test_remove_owners
   def test_remove_owners_denied
     response = "You don't have permission to push to this gem"
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 403, 'Forbidden']
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["user-remove1@example.com"])
@@ -226,7 +226,7 @@ def test_remove_owners_key
 
     @cmd.handle_options %w(-k other)
     @cmd.remove_owners('freewill', ['user-remove1@example.com'])
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_equal '701229f217cdf23b1344c7b4b54ca97', @stub_fetcher.last_request['Authorization']
   end
@@ -234,7 +234,7 @@ def test_remove_owners_key
   def test_remove_owners_missing
     response = 'Owner could not be found.'
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 404, 'Not Found']
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["missing@example"])
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index 4f938310bf1c..a8bdd4eddf3e 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -51,7 +51,7 @@ def send_battery
       @cmd.send_gem(@path)
     end
 
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_match %r{Pushing gem to #{@host}...}, @ui.output
 
@@ -69,7 +69,7 @@ def test_execute
     @fetcher.data["#{Gem.host}/api/v1/gems"] = [@response, 200, 'OK']
 
     @cmd.options[:args] = [@path]
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     @cmd.execute
 
@@ -89,7 +89,7 @@ def test_execute_host
 
     @cmd.options[:host] = host
     @cmd.options[:args] = [@path]
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     @cmd.execute
 
@@ -110,6 +110,7 @@ def test_execute_allowed_push_host
       ['fail', 500, 'Internal Server Error']
 
     @cmd.options[:args] = [@path]
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     @cmd.execute
 
@@ -123,7 +124,7 @@ def test_sending_when_default_host_disabled
     Gem.configuration.disable_default_gem_server = true
     response = "You must specify a gem server"
 
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_raises Gem::MockGemUi::TermError do
       use_ui @ui do
@@ -139,7 +140,7 @@ def test_sending_when_default_host_disabled_with_override
     Gem.configuration.disable_default_gem_server = true
     @response = "Successfully registered gem: freewill (1.0.0)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     send_battery
   end
@@ -152,7 +153,7 @@ def test_sending_gem_to_metadata_host
     end
 
     @api_key = "EYKEY"
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -169,6 +170,8 @@ def test_sending_gem_to_metadata_host
 
     @response = "Successfully registered gem: freebird (1.0.1)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
+
     send_battery
   end
 
@@ -176,7 +179,7 @@ def test_sending_gem
     @response = "Successfully registered gem: freewill (1.0.0)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
 
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     send_battery
   end
@@ -189,7 +192,7 @@ def test_sending_gem_to_allowed_push_host
     end
 
     @api_key = "PRIVKEY"
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -218,7 +221,7 @@ def test_sending_gem_to_allowed_push_host_with_basic_credentials
     end
 
     @api_key = "DOESNTMATTER"
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => @api_key,
@@ -242,7 +245,7 @@ def test_sending_gem_to_disallowed_default_host
       spec.metadata['allowed_push_host'] = "https://privategemserver.example"
     end
 
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     response = %{ERROR:  "#{@host}" is not allowed by the gemspec, which only allows "https://privategemserver.example"}
 
@@ -262,7 +265,7 @@ def test_sending_gem_to_disallowed_push_host
     end
 
     @api_key = "PRIVKEY"
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -295,7 +298,7 @@ def test_sending_gem_defaulting_to_allowed_push_host
     end
 
     api_key = "PRIVKEY"
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       host => api_key
@@ -311,6 +314,7 @@ def test_sending_gem_defaulting_to_allowed_push_host
 
     @response = "Successfully registered gem: freebird (1.0.1)"
     @fetcher.data["#{host}/api/v1/gems"]  = [@response, 200, 'OK']
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     # do not set @host
     use_ui(@ui) { @cmd.send_gem(@path) }
@@ -328,7 +332,7 @@ def test_sending_gem_defaulting_to_allowed_push_host
 
   def test_raises_error_with_no_arguments
     def @cmd.sign_in(*); end
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
     assert_raises Gem::CommandLineError do
       @cmd.execute
     end
@@ -338,7 +342,7 @@ def test_sending_gem_denied
     response = "You don't have permission to push to this gem"
     @fetcher.data["#{@host}/api/v1/gems"] = [response, 403, 'Forbidden']
     @cmd.instance_variable_set :@host, @host
-    @cmd.options[:suppress_mfa] = true
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_raises Gem::MockGemUi::TermError do
       use_ui @ui do
@@ -358,8 +362,8 @@ def test_sending_gem_key
     Gem.configuration.load_api_keys
 
     @cmd.handle_options %w(-k other)
-    @cmd.options[:suppress_mfa] = true
     @cmd.instance_variable_set :@host, @host
+    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
     @cmd.send_gem(@path)
 
     assert_equal Gem.configuration.api_keys[:other],

From 2a85f4a5ce31e359e314eccdd676034cbf623491 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Wed, 8 Aug 2018 01:18:30 +0800
Subject: [PATCH 08/19] Check response text to determine if mfa needed.

---
 lib/rubygems/commands/owner_command.rb        | 12 +++++-
 lib/rubygems/commands/push_command.rb         | 14 ++++++-
 lib/rubygems/gemcutter_utilities.rb           | 40 ++++++++-----------
 .../test_gem_commands_owner_command.rb        |  8 ----
 .../test_gem_commands_push_command.rb         | 21 ----------
 5 files changed, 39 insertions(+), 56 deletions(-)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index d6abbcf6c943..c220419e4e2c 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -52,7 +52,6 @@ def execute
     @host = options[:host]
 
     sign_in
-    check_mfa
     name = get_one_gem_name
 
     add_owners    name, options[:add]
@@ -89,7 +88,16 @@ def manage_owners method, name, owners
         response = rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
           request.set_form_data 'email' => owner
           request.add_field "Authorization", api_key
-          request.add_field "OTP", options[:mfa] if need_mfa?
+          request.add_field "OTP", options[:mfa] if options[:mfa]
+        end
+
+        if need_mfa? response
+          check_mfa
+          response = rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
+            request.set_form_data 'email' => owner
+            request.add_field "Authorization", api_key
+            request.add_field "OTP", options[:mfa]
+          end
         end
 
         action = method == :delete ? "Removing" : "Adding"
diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index 63a678189ca0..b544a5054a7b 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -69,7 +69,6 @@ def execute
             end
 
     sign_in @host
-    check_mfa
 
     send_gem(gem_name)
   end
@@ -120,7 +119,18 @@ def send_gem(name)
       request.add_field "Content-Length", request.body.size
       request.add_field "Content-Type",   "application/octet-stream"
       request.add_field "Authorization",  api_key
-      request.add_field "OTP", options[:mfa] if need_mfa?
+      request.add_field "OTP", options[:mfa] if options[:mfa]
+    end
+
+    if need_mfa? response
+      check_mfa
+      response = rubygems_api_request(*args) do |request|
+        request.body = Gem.read_binary name
+        request.add_field "Content-Length", request.body.size
+        request.add_field "Content-Type",   "application/octet-stream"
+        request.add_field "Authorization",  api_key
+        request.add_field "OTP", options[:mfa]
+      end
     end
 
     with_response response
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index d507cf578db4..d9a5de9b2621 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -123,39 +123,25 @@ def sign_in sign_in_host = nil
       request.basic_auth email, password
     end
 
+    if need_mfa? response
+      check_mfa
+      response = rubygems_api_request(:get, "api/v1/api_key", sign_in_host) do |request|
+        request.basic_auth email, password
+        request.add_field "OTP", options[:mfa]
+      end
+    end
+
     with_response response do |resp|
       say "Signed in."
+      options.delete :mfa
       set_api_key host, resp.body
     end
   end
 
-  ##
-  # Fetch user's multifactor authentication settings and return if extra digit
-  # code is needed.
-
-  def need_mfa?
-    unless instance_variable_defined? :@mfa_level
-      response = rubygems_api_request(:get, 'api/v1/multifactor_auth') do |request|
-        request.add_field 'Authorization', api_key
-      end
-
-      # For compatibility to servers without mfa support
-      @mfa_level = case response
-                   when Net::HTTPNotFound
-                     'no_mfa'
-                   else
-                     with_response(response) { |resp| resp.body }
-                   end
-    end
-
-    @mfa_level == 'mfa_login_and_write'
-  end
-
   ##
   # Require user for digit code if multifactor authentication is enabled.
 
   def check_mfa
-    return unless need_mfa?
     unless options[:mfa]
       say 'This command needs digit code for multifactor authentication.'
       options[:mfa] = ask 'Code: '
@@ -199,6 +185,14 @@ def with_response response, error_prefix = nil
     end
   end
 
+  ##
+  # Returns whether the user has enabled multifactor authentication from
+  # +response+ text.
+
+  def need_mfa? response
+    response.kind_of?(Net::HTTPUnauthorized) && response.body.start_with?('You have enabled multifactor authentication')
+  end
+
   def set_api_key host, key
     if host == Gem::DEFAULT_HOST
       Gem.configuration.rubygems_api_key = key
diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index 72dde13363e8..1f9a2efbcad1 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -129,7 +129,6 @@ def test_show_owners_key
   def test_add_owners
     response = "Owner added successfully."
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 200, 'OK']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
@@ -145,7 +144,6 @@ def test_add_owners
   def test_add_owners_denied
     response = "You don't have permission to push to this gem"
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 403, 'Forbidden']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
@@ -162,7 +160,6 @@ def test_add_owner_with_host_option_through_execute
     @stub_fetcher.data["#{host}/api/v1/gems/freewill/owners.yaml"] = [show_owners_response, 200, 'OK']
 
     @cmd.handle_options %W[--host #{host} --add user-new1@example.com freewill]
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.execute
@@ -183,7 +180,6 @@ def test_add_owners_key
 
     @cmd.handle_options %w(-k other)
     @cmd.add_owners('freewill', ['user-new1@example.com'])
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_equal '701229f217cdf23b1344c7b4b54ca97', @stub_fetcher.last_request['Authorization']
   end
@@ -191,7 +187,6 @@ def test_add_owners_key
   def test_remove_owners
     response = "Owner removed successfully."
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 200, 'OK']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["user-remove1@example.com"])
@@ -207,7 +202,6 @@ def test_remove_owners
   def test_remove_owners_denied
     response = "You don't have permission to push to this gem"
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 403, 'Forbidden']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["user-remove1@example.com"])
@@ -226,7 +220,6 @@ def test_remove_owners_key
 
     @cmd.handle_options %w(-k other)
     @cmd.remove_owners('freewill', ['user-remove1@example.com'])
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_equal '701229f217cdf23b1344c7b4b54ca97', @stub_fetcher.last_request['Authorization']
   end
@@ -234,7 +227,6 @@ def test_remove_owners_key
   def test_remove_owners_missing
     response = 'Owner could not be found.'
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 404, 'Not Found']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     use_ui @stub_ui do
       @cmd.remove_owners("freewill", ["missing@example"])
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index a8bdd4eddf3e..c24e26b8e945 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -51,8 +51,6 @@ def send_battery
       @cmd.send_gem(@path)
     end
 
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
-
     assert_match %r{Pushing gem to #{@host}...}, @ui.output
 
     assert_equal Net::HTTP::Post, @fetcher.last_request.class
@@ -69,7 +67,6 @@ def test_execute
     @fetcher.data["#{Gem.host}/api/v1/gems"] = [@response, 200, 'OK']
 
     @cmd.options[:args] = [@path]
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     @cmd.execute
 
@@ -89,7 +86,6 @@ def test_execute_host
 
     @cmd.options[:host] = host
     @cmd.options[:args] = [@path]
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     @cmd.execute
 
@@ -110,7 +106,6 @@ def test_execute_allowed_push_host
       ['fail', 500, 'Internal Server Error']
 
     @cmd.options[:args] = [@path]
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     @cmd.execute
 
@@ -124,8 +119,6 @@ def test_sending_when_default_host_disabled
     Gem.configuration.disable_default_gem_server = true
     response = "You must specify a gem server"
 
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
-
     assert_raises Gem::MockGemUi::TermError do
       use_ui @ui do
         @cmd.send_gem(@path)
@@ -140,7 +133,6 @@ def test_sending_when_default_host_disabled_with_override
     Gem.configuration.disable_default_gem_server = true
     @response = "Successfully registered gem: freewill (1.0.0)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     send_battery
   end
@@ -153,7 +145,6 @@ def test_sending_gem_to_metadata_host
     end
 
     @api_key = "EYKEY"
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -170,7 +161,6 @@ def test_sending_gem_to_metadata_host
 
     @response = "Successfully registered gem: freebird (1.0.1)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     send_battery
   end
@@ -179,8 +169,6 @@ def test_sending_gem
     @response = "Successfully registered gem: freewill (1.0.0)"
     @fetcher.data["#{@host}/api/v1/gems"]  = [@response, 200, 'OK']
 
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
-
     send_battery
   end
 
@@ -192,7 +180,6 @@ def test_sending_gem_to_allowed_push_host
     end
 
     @api_key = "PRIVKEY"
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -221,7 +208,6 @@ def test_sending_gem_to_allowed_push_host_with_basic_credentials
     end
 
     @api_key = "DOESNTMATTER"
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => @api_key,
@@ -245,7 +231,6 @@ def test_sending_gem_to_disallowed_default_host
       spec.metadata['allowed_push_host'] = "https://privategemserver.example"
     end
 
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     response = %{ERROR:  "#{@host}" is not allowed by the gemspec, which only allows "https://privategemserver.example"}
 
@@ -265,7 +250,6 @@ def test_sending_gem_to_disallowed_push_host
     end
 
     @api_key = "PRIVKEY"
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       :rubygems_api_key => 'KEY',
@@ -298,7 +282,6 @@ def test_sending_gem_defaulting_to_allowed_push_host
     end
 
     api_key = "PRIVKEY"
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     keys = {
       host => api_key
@@ -314,7 +297,6 @@ def test_sending_gem_defaulting_to_allowed_push_host
 
     @response = "Successfully registered gem: freebird (1.0.1)"
     @fetcher.data["#{host}/api/v1/gems"]  = [@response, 200, 'OK']
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     # do not set @host
     use_ui(@ui) { @cmd.send_gem(@path) }
@@ -332,7 +314,6 @@ def test_sending_gem_defaulting_to_allowed_push_host
 
   def test_raises_error_with_no_arguments
     def @cmd.sign_in(*); end
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
     assert_raises Gem::CommandLineError do
       @cmd.execute
     end
@@ -342,7 +323,6 @@ def test_sending_gem_denied
     response = "You don't have permission to push to this gem"
     @fetcher.data["#{@host}/api/v1/gems"] = [response, 403, 'Forbidden']
     @cmd.instance_variable_set :@host, @host
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
 
     assert_raises Gem::MockGemUi::TermError do
       use_ui @ui do
@@ -363,7 +343,6 @@ def test_sending_gem_key
 
     @cmd.handle_options %w(-k other)
     @cmd.instance_variable_set :@host, @host
-    @cmd.instance_variable_set :@mfa_level, 'no_mfa'
     @cmd.send_gem(@path)
 
     assert_equal Gem.configuration.api_keys[:other],

From f606f78d042b4227aadacb8cd5b4753d414c96c5 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Fri, 10 Aug 2018 20:31:26 +0800
Subject: [PATCH 09/19] Add support to callable objects to data of
 `FakeFetcher#request`.

---
 lib/rubygems/test_utilities.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rubygems/test_utilities.rb b/lib/rubygems/test_utilities.rb
index 7a72f9924794..2a4f8a012e1e 100644
--- a/lib/rubygems/test_utilities.rb
+++ b/lib/rubygems/test_utilities.rb
@@ -87,7 +87,7 @@ def open_uri_or_path(path)
 
   def request(uri, request_class, last_modified = nil)
     data = find_data(uri)
-    body, code, msg = data
+    body, code, msg = (data.respond_to?(:call) ? data.call : data)
 
     @last_request = request_class.new uri.request_uri
     yield @last_request if block_given?

From c5c4cc2902a252eecf7b0cf1d48bb4793c848104 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Fri, 10 Aug 2018 20:33:14 +0800
Subject: [PATCH 10/19] Add multifactor auth related test to gemcutter
 utilities.

---
 test/rubygems/test_gem_gemcutter_utilities.rb | 30 +++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/test/rubygems/test_gem_gemcutter_utilities.rb b/test/rubygems/test_gem_gemcutter_utilities.rb
index 90f9142171fb..09a4d35885f3 100644
--- a/test/rubygems/test_gem_gemcutter_utilities.rb
+++ b/test/rubygems/test_gem_gemcutter_utilities.rb
@@ -187,7 +187,33 @@ def test_sign_in_with_bad_credentials
     assert_match %r{Access Denied.}, @sign_in_ui.output
   end
 
-  def util_sign_in response, host = nil, args = []
+  def test_sign_in_with_correct_mfa_code
+    api_key       = 'a5fdbb6ba150cbb83aad2bb2fede64cf040453903'
+    response_fail = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
+
+    util_sign_in(proc do
+      @call_count ||= 0
+      (@call_count += 1).odd? ? [response_fail, 401, 'Unauthorized'] : [api_key, 200, 'OK']
+    end, nil, [], "111111\n")
+
+    assert_match 'This command needs digit code for multifactor authentication.', @sign_in_ui.output
+    assert_match 'Code: ', @sign_in_ui.output
+    assert_match 'Signed in.', @sign_in_ui.output
+  end
+
+  def test_sign_in_with_incorrect_mfa_code
+    response = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
+
+    assert_raises Gem::MockGemUi::TermError do
+      util_sign_in [response, 401, 'Unauthorized'], nil, [], "111111\n"
+    end
+
+    assert_match 'This command needs digit code for multifactor authentication.', @sign_in_ui.output
+    assert_match 'Code: ', @sign_in_ui.output
+    assert_match response, @sign_in_ui.output
+  end
+
+  def util_sign_in response, host = nil, args = [], extra_input = ''
     email    = 'you@example.com'
     password = 'secret'
 
@@ -201,7 +227,7 @@ def util_sign_in response, host = nil, args = []
     @fetcher.data["#{host}/api/v1/api_key"] = response
     Gem::RemoteFetcher.fetcher = @fetcher
 
-    @sign_in_ui = Gem::MockGemUi.new "#{email}\n#{password}\n"
+    @sign_in_ui = Gem::MockGemUi.new("#{email}\n#{password}\n" + extra_input)
 
     use_ui @sign_in_ui do
       if args.length > 0 then

From 03636c3eb6bf6d93a7558872b64bb304dc168040 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Fri, 10 Aug 2018 20:35:14 +0800
Subject: [PATCH 11/19] Add multifactor auth related test to `push` and `owner`
 command.

---
 .../test_gem_commands_owner_command.rb        | 33 +++++++++++++++++
 .../test_gem_commands_push_command.rb         | 35 +++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index 1f9a2efbcad1..b81b20405ed1 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -235,4 +235,37 @@ def test_remove_owners_missing
     assert_equal "Removing missing@example: #{response}\n", @stub_ui.output
   end
 
+  def test_mfa_success
+    response_fail = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
+    response_success = "Owner added successfully."
+
+    @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = proc do
+      @call_count ||= 0
+      (@call_count += 1).odd? ? [response_fail, 401, 'Unauthorized'] : [response_success, 200, 'OK']
+    end
+
+    @mfa_ui = Gem::MockGemUi.new "111111\n"
+    use_ui @mfa_ui do
+      @cmd.add_owners("freewill", ["user-new1@example.com"])
+    end
+
+    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
+    assert_match 'Code: ', @mfa_ui.output
+    assert_match response_success, @mfa_ui.output
+  end
+
+  def test_mfa_failure
+    response = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
+    @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 401, 'Unauthorized']
+
+    @mfa_ui = Gem::MockGemUi.new "111111\n"
+    use_ui @mfa_ui do
+      @cmd.add_owners("freewill", ["user-new1@example.com"])
+    end
+
+    assert_match response, @mfa_ui.output
+    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
+    assert_match 'Code: ', @mfa_ui.output
+  end
+
 end
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index c24e26b8e945..5e5ae73cf022 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -349,4 +349,39 @@ def test_sending_gem_key
                  @fetcher.last_request["Authorization"]
   end
 
+  def test_mfa_success
+    response_fail = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
+    response_success = 'Successfully registered gem: freewill (1.0.0)'
+
+    @fetcher.data["#{Gem.host}/api/v1/gems"] = proc do
+      @call_count ||= 0
+      (@call_count += 1).odd? ? [response_fail, 401, 'Unauthorized'] : [response_success, 200, 'OK']
+    end
+
+    @mfa_ui = Gem::MockGemUi.new "111111\n"
+    use_ui @mfa_ui do
+      @cmd.send_gem(@path)
+    end
+
+    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
+    assert_match 'Code: ', @mfa_ui.output
+    assert_match response_success, @mfa_ui.output
+  end
+
+  def test_mfa_failure
+    response = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
+    @fetcher.data["#{Gem.host}/api/v1/gems"] = [response, 401, 'Unauthorized']
+
+    @mfa_ui = Gem::MockGemUi.new "111111\n"
+    assert_raises Gem::MockGemUi::TermError do
+      use_ui @mfa_ui do
+        @cmd.send_gem(@path)
+      end
+    end
+
+    assert_match response, @mfa_ui.output
+    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
+    assert_match 'Code: ', @mfa_ui.output
+  end
+
 end

From bba2a3f6cff5f9854bf5be304888d02337289f49 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Sat, 11 Aug 2018 11:11:06 +0800
Subject: [PATCH 12/19] Add `--mfa` option to `signin` command.

---
 lib/rubygems/commands/signin_command.rb | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/rubygems/commands/signin_command.rb b/lib/rubygems/commands/signin_command.rb
index 64b187f26f23..d83bcb03eac7 100644
--- a/lib/rubygems/commands/signin_command.rb
+++ b/lib/rubygems/commands/signin_command.rb
@@ -12,6 +12,9 @@ def initialize
     add_option('--host HOST', 'Push to another gemcutter-compatible host') do |value, options|
       options[:host] = value
     end
+
+    add_mfa_option
+
   end
 
   def description # :nodoc:

From 73b6ad23aceabde88e4aa1a467943f54bce691ad Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Sun, 12 Aug 2018 00:34:21 +0800
Subject: [PATCH 13/19] Wrap api request needing mfa into single methods.

---
 lib/rubygems/commands/owner_command.rb | 22 ++++++++++++----------
 lib/rubygems/commands/push_command.rb  | 26 ++++++++++++--------------
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index c220419e4e2c..b44b3354faa9 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -85,19 +85,11 @@ def remove_owners name, owners
   def manage_owners method, name, owners
     owners.each do |owner|
       begin
-        response = rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
-          request.set_form_data 'email' => owner
-          request.add_field "Authorization", api_key
-          request.add_field "OTP", options[:mfa] if options[:mfa]
-        end
+        response = send_owner_request(method, name, owner)
 
         if need_mfa? response
           check_mfa
-          response = rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
-            request.set_form_data 'email' => owner
-            request.add_field "Authorization", api_key
-            request.add_field "OTP", options[:mfa]
-          end
+          response = send_owner_request(method, name, owner, true)
         end
 
         action = method == :delete ? "Removing" : "Adding"
@@ -109,4 +101,14 @@ def manage_owners method, name, owners
     end
   end
 
+  private
+
+  def send_owner_request(method, name, owner, use_mfa = false)
+    rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
+      request.set_form_data 'email' => owner
+      request.add_field "Authorization", api_key
+      request.add_field "OTP", options[:mfa] if use_mfa
+    end
+  end
+
 end
diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index b544a5054a7b..e9374c549bb9 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -114,23 +114,11 @@ def send_gem(name)
 
     say "Pushing gem to #{@host || Gem.host}..."
 
-    response = rubygems_api_request(*args) do |request|
-      request.body = Gem.read_binary name
-      request.add_field "Content-Length", request.body.size
-      request.add_field "Content-Type",   "application/octet-stream"
-      request.add_field "Authorization",  api_key
-      request.add_field "OTP", options[:mfa] if options[:mfa]
-    end
+    response = send_push_request(name, args)
 
     if need_mfa? response
       check_mfa
-      response = rubygems_api_request(*args) do |request|
-        request.body = Gem.read_binary name
-        request.add_field "Content-Length", request.body.size
-        request.add_field "Content-Type",   "application/octet-stream"
-        request.add_field "Authorization",  api_key
-        request.add_field "OTP", options[:mfa]
-      end
+      response = send_push_request(name, args, true)
     end
 
     with_response response
@@ -138,6 +126,16 @@ def send_gem(name)
 
   private
 
+  def send_push_request(name, args, use_mfa = false)
+    rubygems_api_request(*args) do |request|
+      request.body = Gem.read_binary name
+      request.add_field "Content-Length", request.body.size
+      request.add_field "Content-Type",   "application/octet-stream"
+      request.add_field "Authorization",  api_key
+      request.add_field "OTP", options[:mfa] if use_mfa
+    end
+  end
+
   def get_hosts_for(name)
     gem_metadata = Gem::Package.new(name).spec.metadata
 

From 3b789c4c84dc192a043d9786897f736f8e0fa677 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Sun, 12 Aug 2018 00:46:21 +0800
Subject: [PATCH 14/19] Unify all multi-factor auth related names to `otp`.

---
 lib/rubygems/commands/owner_command.rb        | 10 ++++----
 lib/rubygems/commands/push_command.rb         | 10 ++++----
 lib/rubygems/commands/signin_command.rb       |  2 +-
 lib/rubygems/gemcutter_utilities.rb           | 24 +++++++++----------
 .../test_gem_commands_owner_command.rb        | 24 +++++++++----------
 .../test_gem_commands_push_command.rb         | 24 +++++++++----------
 test/rubygems/test_gem_gemcutter_utilities.rb |  4 ++--
 7 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index b44b3354faa9..242b3b37b9d8 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -30,7 +30,7 @@ def initialize
     super 'owner', 'Manage gem owners of a gem on the push server'
     add_proxy_option
     add_key_option
-    add_mfa_option
+    add_otp_option
     defaults.merge! :add => [], :remove => []
 
     add_option '-a', '--add EMAIL', 'Add an owner' do |value, options|
@@ -87,8 +87,8 @@ def manage_owners method, name, owners
       begin
         response = send_owner_request(method, name, owner)
 
-        if need_mfa? response
-          check_mfa
+        if need_otp? response
+          check_otp
           response = send_owner_request(method, name, owner, true)
         end
 
@@ -103,11 +103,11 @@ def manage_owners method, name, owners
 
   private
 
-  def send_owner_request(method, name, owner, use_mfa = false)
+  def send_owner_request(method, name, owner, use_otp = false)
     rubygems_api_request method, "api/v1/gems/#{name}/owners" do |request|
       request.set_form_data 'email' => owner
       request.add_field "Authorization", api_key
-      request.add_field "OTP", options[:mfa] if use_mfa
+      request.add_field "OTP", options[:otp] if use_otp
     end
   end
 
diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index e9374c549bb9..08aa5804b391 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -33,7 +33,7 @@ def initialize
 
     add_proxy_option
     add_key_option
-    add_mfa_option
+    add_otp_option
 
     add_option('--host HOST',
                'Push to another gemcutter-compatible host',
@@ -116,8 +116,8 @@ def send_gem(name)
 
     response = send_push_request(name, args)
 
-    if need_mfa? response
-      check_mfa
+    if need_otp? response
+      check_otp
       response = send_push_request(name, args, true)
     end
 
@@ -126,13 +126,13 @@ def send_gem(name)
 
   private
 
-  def send_push_request(name, args, use_mfa = false)
+  def send_push_request(name, args, use_otp = false)
     rubygems_api_request(*args) do |request|
       request.body = Gem.read_binary name
       request.add_field "Content-Length", request.body.size
       request.add_field "Content-Type",   "application/octet-stream"
       request.add_field "Authorization",  api_key
-      request.add_field "OTP", options[:mfa] if use_mfa
+      request.add_field "OTP", options[:otp] if use_otp
     end
   end
 
diff --git a/lib/rubygems/commands/signin_command.rb b/lib/rubygems/commands/signin_command.rb
index d83bcb03eac7..b616695733ad 100644
--- a/lib/rubygems/commands/signin_command.rb
+++ b/lib/rubygems/commands/signin_command.rb
@@ -13,7 +13,7 @@ def initialize
       options[:host] = value
     end
 
-    add_mfa_option
+    add_otp_option
 
   end
 
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index d9a5de9b2621..92ad6014d837 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -25,12 +25,12 @@ def add_key_option
   end
 
   ##
-  # Add the --mfa option
+  # Add the --otp option
 
-  def add_mfa_option
-    add_option('--mfa CODE',
+  def add_otp_option
+    add_option('--otp CODE',
                'Digit code for multifactor authentication') do |value, options|
-      options[:mfa] = value
+      options[:otp] = value
     end
   end
 
@@ -123,17 +123,17 @@ def sign_in sign_in_host = nil
       request.basic_auth email, password
     end
 
-    if need_mfa? response
-      check_mfa
+    if need_otp? response
+      check_otp
       response = rubygems_api_request(:get, "api/v1/api_key", sign_in_host) do |request|
         request.basic_auth email, password
-        request.add_field "OTP", options[:mfa]
+        request.add_field "OTP", options[:otp]
       end
     end
 
     with_response response do |resp|
       say "Signed in."
-      options.delete :mfa
+      options.delete :otp
       set_api_key host, resp.body
     end
   end
@@ -141,10 +141,10 @@ def sign_in sign_in_host = nil
   ##
   # Require user for digit code if multifactor authentication is enabled.
 
-  def check_mfa
-    unless options[:mfa]
+  def check_otp
+    unless options[:otp]
       say 'This command needs digit code for multifactor authentication.'
-      options[:mfa] = ask 'Code: '
+      options[:otp] = ask 'Code: '
     end
   end
 
@@ -189,7 +189,7 @@ def with_response response, error_prefix = nil
   # Returns whether the user has enabled multifactor authentication from
   # +response+ text.
 
-  def need_mfa? response
+  def need_otp? response
     response.kind_of?(Net::HTTPUnauthorized) && response.body.start_with?('You have enabled multifactor authentication')
   end
 
diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index b81b20405ed1..a752aae25b1f 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -235,7 +235,7 @@ def test_remove_owners_missing
     assert_equal "Removing missing@example: #{response}\n", @stub_ui.output
   end
 
-  def test_mfa_success
+  def test_otp_verified_success
     response_fail = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
     response_success = "Owner added successfully."
 
@@ -244,28 +244,28 @@ def test_mfa_success
       (@call_count += 1).odd? ? [response_fail, 401, 'Unauthorized'] : [response_success, 200, 'OK']
     end
 
-    @mfa_ui = Gem::MockGemUi.new "111111\n"
-    use_ui @mfa_ui do
+    @otp_ui = Gem::MockGemUi.new "111111\n"
+    use_ui @otp_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
     end
 
-    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
-    assert_match 'Code: ', @mfa_ui.output
-    assert_match response_success, @mfa_ui.output
+    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'Code: ', @otp_ui.output
+    assert_match response_success, @otp_ui.output
   end
 
-  def test_mfa_failure
+  def test_otp_verified_failure
     response = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
     @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [response, 401, 'Unauthorized']
 
-    @mfa_ui = Gem::MockGemUi.new "111111\n"
-    use_ui @mfa_ui do
+    @otp_ui = Gem::MockGemUi.new "111111\n"
+    use_ui @otp_ui do
       @cmd.add_owners("freewill", ["user-new1@example.com"])
     end
 
-    assert_match response, @mfa_ui.output
-    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
-    assert_match 'Code: ', @mfa_ui.output
+    assert_match response, @otp_ui.output
+    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'Code: ', @otp_ui.output
   end
 
 end
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index 5e5ae73cf022..77b434552f14 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -349,7 +349,7 @@ def test_sending_gem_key
                  @fetcher.last_request["Authorization"]
   end
 
-  def test_mfa_success
+  def test_otp_verified_success
     response_fail = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
     response_success = 'Successfully registered gem: freewill (1.0.0)'
 
@@ -358,30 +358,30 @@ def test_mfa_success
       (@call_count += 1).odd? ? [response_fail, 401, 'Unauthorized'] : [response_success, 200, 'OK']
     end
 
-    @mfa_ui = Gem::MockGemUi.new "111111\n"
-    use_ui @mfa_ui do
+    @otp_ui = Gem::MockGemUi.new "111111\n"
+    use_ui @otp_ui do
       @cmd.send_gem(@path)
     end
 
-    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
-    assert_match 'Code: ', @mfa_ui.output
-    assert_match response_success, @mfa_ui.output
+    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'Code: ', @otp_ui.output
+    assert_match response_success, @otp_ui.output
   end
 
-  def test_mfa_failure
+  def test_otp_verified_failure
     response = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
     @fetcher.data["#{Gem.host}/api/v1/gems"] = [response, 401, 'Unauthorized']
 
-    @mfa_ui = Gem::MockGemUi.new "111111\n"
+    @otp_ui = Gem::MockGemUi.new "111111\n"
     assert_raises Gem::MockGemUi::TermError do
-      use_ui @mfa_ui do
+      use_ui @otp_ui do
         @cmd.send_gem(@path)
       end
     end
 
-    assert_match response, @mfa_ui.output
-    assert_match 'This command needs digit code for multifactor authentication.', @mfa_ui.output
-    assert_match 'Code: ', @mfa_ui.output
+    assert_match response, @otp_ui.output
+    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'Code: ', @otp_ui.output
   end
 
 end
diff --git a/test/rubygems/test_gem_gemcutter_utilities.rb b/test/rubygems/test_gem_gemcutter_utilities.rb
index 09a4d35885f3..9bef1e8964e4 100644
--- a/test/rubygems/test_gem_gemcutter_utilities.rb
+++ b/test/rubygems/test_gem_gemcutter_utilities.rb
@@ -187,7 +187,7 @@ def test_sign_in_with_bad_credentials
     assert_match %r{Access Denied.}, @sign_in_ui.output
   end
 
-  def test_sign_in_with_correct_mfa_code
+  def test_sign_in_with_correct_otp_code
     api_key       = 'a5fdbb6ba150cbb83aad2bb2fede64cf040453903'
     response_fail = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
 
@@ -201,7 +201,7 @@ def test_sign_in_with_correct_mfa_code
     assert_match 'Signed in.', @sign_in_ui.output
   end
 
-  def test_sign_in_with_incorrect_mfa_code
+  def test_sign_in_with_incorrect_otp_code
     response = "You have enabled multifactor authentication but your request doesn't have the correct OTP code. Please check it and retry."
 
     assert_raises Gem::MockGemUi::TermError do

From 1c232f87c8f6f8b55fb07accb2be4569961030f3 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Sun, 12 Aug 2018 01:02:12 +0800
Subject: [PATCH 15/19] Move otp asking logic into `need_otp?`.

---
 lib/rubygems/commands/owner_command.rb |  1 -
 lib/rubygems/commands/push_command.rb  |  1 -
 lib/rubygems/gemcutter_utilities.rb    | 19 +++++++------------
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb
index 242b3b37b9d8..12284e00c99e 100644
--- a/lib/rubygems/commands/owner_command.rb
+++ b/lib/rubygems/commands/owner_command.rb
@@ -88,7 +88,6 @@ def manage_owners method, name, owners
         response = send_owner_request(method, name, owner)
 
         if need_otp? response
-          check_otp
           response = send_owner_request(method, name, owner, true)
         end
 
diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb
index 08aa5804b391..24cc98c2ded4 100644
--- a/lib/rubygems/commands/push_command.rb
+++ b/lib/rubygems/commands/push_command.rb
@@ -117,7 +117,6 @@ def send_gem(name)
     response = send_push_request(name, args)
 
     if need_otp? response
-      check_otp
       response = send_push_request(name, args, true)
     end
 
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 92ad6014d837..8189dab69030 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -124,7 +124,6 @@ def sign_in sign_in_host = nil
     end
 
     if need_otp? response
-      check_otp
       response = rubygems_api_request(:get, "api/v1/api_key", sign_in_host) do |request|
         request.basic_auth email, password
         request.add_field "OTP", options[:otp]
@@ -138,16 +137,6 @@ def sign_in sign_in_host = nil
     end
   end
 
-  ##
-  # Require user for digit code if multifactor authentication is enabled.
-
-  def check_otp
-    unless options[:otp]
-      say 'This command needs digit code for multifactor authentication.'
-      options[:otp] = ask 'Code: '
-    end
-  end
-
   ##
   # Retrieves the pre-configured API key +key+ or terminates interaction with
   # an error.
@@ -190,7 +179,13 @@ def with_response response, error_prefix = nil
   # +response+ text.
 
   def need_otp? response
-    response.kind_of?(Net::HTTPUnauthorized) && response.body.start_with?('You have enabled multifactor authentication')
+    return unless response.kind_of?(Net::HTTPUnauthorized) &&
+        response.body.start_with?('You have enabled multifactor authentication')
+    return true if options[:otp]
+
+    say 'This command needs digit code for multifactor authentication.'
+    options[:otp] = ask 'Code: '
+    true
   end
 
   def set_api_key host, key

From 89f6c2b7bb63cca3b03504f330a6ae7ea04b7770 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Sun, 12 Aug 2018 15:23:44 +0800
Subject: [PATCH 16/19] Add test for OTP fields in request header.

---
 test/rubygems/test_gem_commands_owner_command.rb | 2 ++
 test/rubygems/test_gem_commands_push_command.rb  | 2 ++
 test/rubygems/test_gem_gemcutter_utilities.rb    | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index a752aae25b1f..d53c668c33ab 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -252,6 +252,7 @@ def test_otp_verified_success
     assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
     assert_match response_success, @otp_ui.output
+    assert_equal '111111', @stub_fetcher.last_request['OTP']
   end
 
   def test_otp_verified_failure
@@ -266,6 +267,7 @@ def test_otp_verified_failure
     assert_match response, @otp_ui.output
     assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
+    assert_equal '111111', @stub_fetcher.last_request['OTP']
   end
 
 end
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index 77b434552f14..662a24f2549f 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -366,6 +366,7 @@ def test_otp_verified_success
     assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
     assert_match response_success, @otp_ui.output
+    assert_equal '111111', @fetcher.last_request['OTP']
   end
 
   def test_otp_verified_failure
@@ -382,6 +383,7 @@ def test_otp_verified_failure
     assert_match response, @otp_ui.output
     assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
+    assert_equal '111111', @fetcher.last_request['OTP']
   end
 
 end
diff --git a/test/rubygems/test_gem_gemcutter_utilities.rb b/test/rubygems/test_gem_gemcutter_utilities.rb
index 9bef1e8964e4..af836f14a148 100644
--- a/test/rubygems/test_gem_gemcutter_utilities.rb
+++ b/test/rubygems/test_gem_gemcutter_utilities.rb
@@ -199,6 +199,7 @@ def test_sign_in_with_correct_otp_code
     assert_match 'This command needs digit code for multifactor authentication.', @sign_in_ui.output
     assert_match 'Code: ', @sign_in_ui.output
     assert_match 'Signed in.', @sign_in_ui.output
+    assert_equal '111111', @fetcher.last_request['OTP']
   end
 
   def test_sign_in_with_incorrect_otp_code
@@ -211,6 +212,7 @@ def test_sign_in_with_incorrect_otp_code
     assert_match 'This command needs digit code for multifactor authentication.', @sign_in_ui.output
     assert_match 'Code: ', @sign_in_ui.output
     assert_match response, @sign_in_ui.output
+    assert_equal '111111', @fetcher.last_request['OTP']
   end
 
   def util_sign_in response, host = nil, args = [], extra_input = ''

From c45f786b42b657bdff7c5d611f26e10fa134c87c Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Wed, 24 Oct 2018 15:10:07 +0800
Subject: [PATCH 17/19] Keep OTP after signed in.

---
 lib/rubygems/gemcutter_utilities.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 8189dab69030..cd97146406f9 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -132,7 +132,6 @@ def sign_in sign_in_host = nil
 
     with_response response do |resp|
       say "Signed in."
-      options.delete :otp
       set_api_key host, resp.body
     end
   end

From f24d666046e00919db489ef92e2407ee772e5664 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <fwage73@gmail.com>
Date: Mon, 12 Nov 2018 19:55:20 +0800
Subject: [PATCH 18/19] Tweak MFA prompt text.

---
 lib/rubygems/gemcutter_utilities.rb              | 4 ++--
 test/rubygems/test_gem_commands_owner_command.rb | 4 ++--
 test/rubygems/test_gem_commands_push_command.rb  | 4 ++--
 test/rubygems/test_gem_gemcutter_utilities.rb    | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index cd97146406f9..a56eccd04eb8 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -174,7 +174,7 @@ def with_response response, error_prefix = nil
   end
 
   ##
-  # Returns whether the user has enabled multifactor authentication from
+  # Returns true when the user has enabled multifactor authentication from
   # +response+ text.
 
   def need_otp? response
@@ -182,7 +182,7 @@ def need_otp? response
         response.body.start_with?('You have enabled multifactor authentication')
     return true if options[:otp]
 
-    say 'This command needs digit code for multifactor authentication.'
+    say 'You have enabled multi-factor authentication. Please enter OTP code.'
     options[:otp] = ask 'Code: '
     true
   end
diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index d53c668c33ab..071079c0dd7c 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -249,7 +249,7 @@ def test_otp_verified_success
       @cmd.add_owners("freewill", ["user-new1@example.com"])
     end
 
-    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'You have enabled multi-factor authentication. Please enter OTP code.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
     assert_match response_success, @otp_ui.output
     assert_equal '111111', @stub_fetcher.last_request['OTP']
@@ -265,7 +265,7 @@ def test_otp_verified_failure
     end
 
     assert_match response, @otp_ui.output
-    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'You have enabled multi-factor authentication. Please enter OTP code.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
     assert_equal '111111', @stub_fetcher.last_request['OTP']
   end
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index 662a24f2549f..9d2185dcd9fc 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -363,7 +363,7 @@ def test_otp_verified_success
       @cmd.send_gem(@path)
     end
 
-    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'You have enabled multi-factor authentication. Please enter OTP code.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
     assert_match response_success, @otp_ui.output
     assert_equal '111111', @fetcher.last_request['OTP']
@@ -381,7 +381,7 @@ def test_otp_verified_failure
     end
 
     assert_match response, @otp_ui.output
-    assert_match 'This command needs digit code for multifactor authentication.', @otp_ui.output
+    assert_match 'You have enabled multi-factor authentication. Please enter OTP code.', @otp_ui.output
     assert_match 'Code: ', @otp_ui.output
     assert_equal '111111', @fetcher.last_request['OTP']
   end
diff --git a/test/rubygems/test_gem_gemcutter_utilities.rb b/test/rubygems/test_gem_gemcutter_utilities.rb
index af836f14a148..c4d43ed343e7 100644
--- a/test/rubygems/test_gem_gemcutter_utilities.rb
+++ b/test/rubygems/test_gem_gemcutter_utilities.rb
@@ -196,7 +196,7 @@ def test_sign_in_with_correct_otp_code
       (@call_count += 1).odd? ? [response_fail, 401, 'Unauthorized'] : [api_key, 200, 'OK']
     end, nil, [], "111111\n")
 
-    assert_match 'This command needs digit code for multifactor authentication.', @sign_in_ui.output
+    assert_match 'You have enabled multi-factor authentication. Please enter OTP code.', @sign_in_ui.output
     assert_match 'Code: ', @sign_in_ui.output
     assert_match 'Signed in.', @sign_in_ui.output
     assert_equal '111111', @fetcher.last_request['OTP']
@@ -209,7 +209,7 @@ def test_sign_in_with_incorrect_otp_code
       util_sign_in [response, 401, 'Unauthorized'], nil, [], "111111\n"
     end
 
-    assert_match 'This command needs digit code for multifactor authentication.', @sign_in_ui.output
+    assert_match 'You have enabled multi-factor authentication. Please enter OTP code.', @sign_in_ui.output
     assert_match 'Code: ', @sign_in_ui.output
     assert_match response, @sign_in_ui.output
     assert_equal '111111', @fetcher.last_request['OTP']

From ee0cbc9ecc822ae8e4356aba1ac8a2b1d4e545a5 Mon Sep 17 00:00:00 2001
From: SHIBATA Hiroshi <hsbt@ruby-lang.org>
Date: Sat, 1 Dec 2018 10:07:59 +0700
Subject: [PATCH 19/19] rubocop -a

---
 lib/rubygems/commands/signin_command.rb | 1 -
 lib/rubygems/gemcutter_utilities.rb     | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/rubygems/commands/signin_command.rb b/lib/rubygems/commands/signin_command.rb
index b616695733ad..0d527fc33980 100644
--- a/lib/rubygems/commands/signin_command.rb
+++ b/lib/rubygems/commands/signin_command.rb
@@ -14,7 +14,6 @@ def initialize
     end
 
     add_otp_option
-
   end
 
   def description # :nodoc:
diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index c26eed54626b..3607b61529f9 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -177,7 +177,7 @@ def with_response(response, error_prefix = nil)
   # Returns true when the user has enabled multifactor authentication from
   # +response+ text.
 
-  def need_otp? response
+  def need_otp?(response)
     return unless response.kind_of?(Net::HTTPUnauthorized) &&
         response.body.start_with?('You have enabled multifactor authentication')
     return true if options[:otp]