[rubygems/rubygems] Fix commands with 2 MFA requests when webauthn is enabled

If a command requires two MFA authenticated requests, and webauthn is
enabled, then first one will succeed but the second one will fail
because it tries to reuse the OTP code from the first request and that
does not work.

This happens when you have not yet logged in to rubygems.org, or when
you have an API key with invalid scopes for the current operation. In
that case, we need:

* An API request to get a token or change scopes for the one that you
  have.
* Another API request to perform the actual operation.

Instead of trying to reuse the token, make sure it's cleared so we are
asked to authenticate again. We only do this when webauthn is enabled
because reusing TOPT tokens otherwise is allowed and I don't want to
break that.

ruby/rubygems@669e343935

Author: deivid-rodriguez

lib/rubygems/gemcutter_utilities.rb

@@ -62,6 +62,10 @@ def otp
     options[:otp] || ENV["GEM_HOST_OTP_CODE"]
   end
 
+  def webauthn_enabled?
+    options[:webauthn]
+  end
+
   ##
   # The host to connect to either from the RUBYGEMS_HOST environment variable
   # or from the user's configuration
@@ -249,6 +253,8 @@ def request_with_otp(method, uri, &block)
       req["OTP"] = otp if otp
       block.call(req)
     end
+  ensure
+    options[:otp] = nil if webauthn_enabled?
   end
 
   def fetch_otp(credentials)
@@ -269,6 +275,8 @@ def fetch_otp(credentials)
         terminate_interaction(1)
       end
 
+      options[:webauthn] = true
+
       say "You are verified with a security device. You may close the browser window."
       otp_thread[:otp]
     else

test/rubygems/test_gem_commands_owner_command.rb

@@ -496,6 +496,47 @@ def test_remove_owners_unathorized_api_key
     assert_match response_success, @stub_ui.output
   end
 
+  def test_add_owners_no_api_key_webauthn_enabled_does_not_reuse_otp_codes
+    response_profile = "mfa: ui_and_api\n"
+    response_mfa_enabled = "You have enabled multifactor authentication but no OTP code provided. Please fill it and retry."
+    response_not_found = "Owner could not be found."
+    Gem.configuration.rubygems_api_key = nil
+
+    path_token = "odow34b93t6aPCdY"
+    webauthn_url = "#{Gem.host}/webauthn_verification/#{path_token}"
+
+    @stub_fetcher.data["#{Gem.host}/api/v1/profile/me.yaml"] = HTTPResponseFactory.create(body: response_profile, code: 200, msg: "OK")
+    @stub_fetcher.data["#{Gem.host}/api/v1/api_key"] = [
+      HTTPResponseFactory.create(body: response_mfa_enabled, code: 401, msg: "Unauthorized"),
+      HTTPResponseFactory.create(body: "", code: 200, msg: "OK"),
+    ]
+    @stub_fetcher.data["#{Gem.host}/api/v1/webauthn_verification"] = Gem::HTTPResponseFactory.create(body: webauthn_url, code: 200, msg: "OK")
+    @stub_fetcher.data["#{Gem.host}/api/v1/webauthn_verification/#{path_token}/status.json"] = [
+      Gem::HTTPResponseFactory.create(body: { status: "success", code: "Uvh6T57tkWuUnWYo" }.to_json, code: 200, msg: "OK"),
+      Gem::HTTPResponseFactory.create(body: { status: "success", code: "Uvh6T57tkWuUnWYz" }.to_json, code: 200, msg: "OK"),
+    ]
+    @stub_fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners"] = [
+      HTTPResponseFactory.create(body: response_mfa_enabled, code: 401, msg: "Unauthorized"),
+      HTTPResponseFactory.create(body: response_not_found, code: 404, msg: "Not Found"),
+    ]
+    @cmd.handle_options %W[--add some@example freewill]
+
+    @stub_ui = Gem::MockGemUi.new "[email protected]\npass\n"
+
+    server = Gem::MockTCPServer.new
+
+    assert_raise Gem::MockGemUi::TermError do
+      TCPServer.stub(:new, server) do
+        use_ui @stub_ui do
+          @cmd.execute
+        end
+      end
+    end
+
+    reused_otp_codes = @stub_fetcher.requests.filter_map {|req| req["OTP"] }.tally.filter_map {|el, count| el if count > 1 }
+    assert_empty reused_otp_codes
+  end
+
   def test_add_owners_unathorized_api_key
     response_forbidden = "The API key doesn't have access"
     response_success   = "Owner added successfully."

test/rubygems/utilities.rb

@@ -30,13 +30,13 @@
 # See RubyGems' tests for more examples of FakeFetcher.
 
 class Gem::FakeFetcher
-  attr_reader :data
-  attr_reader :last_request
+  attr_reader :data, :requests
   attr_accessor :paths
 
   def initialize
     @data = {}
     @paths = []
+    @requests = []
   end
 
   def find_data(path)
@@ -99,9 +99,13 @@ def open_uri_or_path(path)
     create_response(uri)
   end
 
+  def last_request
+    @requests.last
+  end
+
   def request(uri, request_class, last_modified = nil)
-    @last_request = request_class.new uri.request_uri
-    yield @last_request if block_given?
+    @requests << request_class.new(uri.request_uri)
+    yield last_request if block_given?
 
     create_response(uri)
   end