🔀 Merge pull request #184 from ruby/sasl/prerelease-api-changes

✨ Minor updates to SASL::Authenticators API

Author: nevans

lib/net/imap.rb

@@ -1251,21 +1251,19 @@ def authenticate(mechanism, *creds, sasl_ir: true, **props, &callback)
       authenticator = SASL.authenticator(mechanism, *creds, **props, &callback)
       cmdargs = ["AUTHENTICATE", mechanism]
       if sasl_ir && capable?("SASL-IR") && auth_capable?(mechanism) &&
-          SASL.initial_response?(authenticator)
+          authenticator.respond_to?(:initial_response?) &&
+          authenticator.initial_response?
         response = authenticator.process(nil)
         cmdargs << (response.empty? ? "=" : [response].pack("m0"))
       end
-      result = send_command(*cmdargs) do |resp|
-        if resp.instance_of?(ContinuationRequest)
-          challenge = resp.data.text.unpack1("m")
-          response  = authenticator.process(challenge)
-          response  = [response].pack("m0")
-          put_string(response + CRLF)
-        end
-      end
-      unless SASL.done?(authenticator)
+      result = send_command_with_continuations(*cmdargs) {|data|
+        challenge = data.unpack1("m")
+        response  = authenticator.process challenge
+        [response].pack("m0")
+      }
+      if authenticator.respond_to?(:done?) && !authenticator.done?
         logout!
-        raise SASL::AuthenticationFailed, "authentication ended prematurely"
+        raise SASL::AuthenticationIncomplete, result
       end
       @capabilities = capabilities_from_resp_code result
       result
@@ -2570,6 +2568,18 @@ def capabilities_from_resp_code(resp)
 
     #############################
 
+    # Calls send_command, yielding the text of each ContinuationRequest and
+    # responding with each block result.  Returns TaggedResponse.  Raises
+    # NoResponseError or BadResponseError.
+    def send_command_with_continuations(cmd, *args)
+      send_command(cmd, *args) do |server_response|
+        if server_response.instance_of?(ContinuationRequest)
+          client_response = yield server_response.data.text
+          put_string(client_response + CRLF)
+        end
+      end
+    end
+
     def send_command(cmd, *args, &block)
       synchronize do
         args.each do |i|

lib/net/imap/sasl.rb

@@ -114,6 +114,18 @@ module SASL
       # messages has not passed integrity checks.
       AuthenticationFailed = Class.new(Error)
 
+      # Indicates that authentication cannot proceed because one of the server's
+      # ended authentication prematurely.
+      class AuthenticationIncomplete < AuthenticationFailed
+        # The success response from the server
+        attr_reader :response
+
+        def initialize(response, message = "authentication ended prematurely")
+          super(message)
+          @response = response
+        end
+      end
+
       # autoloading to avoid loading all of the regexps when they aren't used.
       sasl_stringprep_rb = File.expand_path("sasl/stringprep", __dir__)
       autoload :StringPrep,          sasl_stringprep_rb
@@ -141,9 +153,7 @@ module SASL
       autoload :LoginAuthenticator,       "#{sasl_dir}/login_authenticator"
 
       # Returns the default global SASL::Authenticators instance.
-      def self.authenticators
-        @authenticators ||= Authenticators.new(use_defaults: true)
-      end
+      def self.authenticators; @authenticators ||= Authenticators.new end
 
       # Delegates to ::authenticators.  See Authenticators#authenticator.
       def self.authenticator(...)     authenticators.authenticator(...) end
@@ -158,24 +168,6 @@ def saslprep(string, **opts)
         Net::IMAP::StringPrep::SASLprep.saslprep(string, **opts)
       end
 
-      # Returns whether +authenticator+ is client-first and supports sending an
-      # "initial response".
-      def initial_response?(authenticator)
-        authenticator.respond_to?(:initial_response?) &&
-          authenticator.initial_response?
-      end
-
-      # Returns whether +authenticator+ considers the authentication exchange to
-      # be complete.
-      #
-      # The authentication should not succeed if this returns false, but
-      # returning true does *not* indicate success.  Authentication succeeds
-      # when this method returns true and the server responds with a
-      # protocol-specific success.
-      def done?(authenticator)
-        !authenticator.respond_to?(:done?) || authenticator.done?
-      end
-
     end
   end
 end

lib/net/imap/sasl/authenticators.rb

@@ -26,24 +26,23 @@ class Authenticators
     # This class is usually not instantiated directly.  Use SASL.authenticators
     # to reuse the default global registry.
     #
-    # By default, the registry will be empty--without any registrations.  When
-    # +add_defaults+ is +true+, authenticators for all standard mechanisms will
-    # be registered.
-    #
-    def initialize(use_defaults: false)
+    # When +use_defaults+ is +false+, the registry will start empty.  When
+    # +use_deprecated+ is +false+, deprecated authenticators will not be
+    # included with the defaults.
+    def initialize(use_defaults: true, use_deprecated: true)
       @authenticators = {}
-      if use_defaults
-        add_authenticator "Anonymous"
-        add_authenticator "External"
-        add_authenticator "OAuthBearer"
-        add_authenticator "Plain"
-        add_authenticator "Scram-SHA-1"
-        add_authenticator "Scram-SHA-256"
-        add_authenticator "XOAuth2"
-        add_authenticator "Login"      # deprecated
-        add_authenticator "Cram-MD5"   # deprecated
-        add_authenticator "Digest-MD5" # deprecated
-      end
+      return unless use_defaults
+      add_authenticator "Anonymous"
+      add_authenticator "External"
+      add_authenticator "OAuthBearer"
+      add_authenticator "Plain"
+      add_authenticator "Scram-SHA-1"
+      add_authenticator "Scram-SHA-256"
+      add_authenticator "XOAuth2"
+      return unless use_deprecated
+      add_authenticator "Login"      # deprecated
+      add_authenticator "Cram-MD5"   # deprecated
+      add_authenticator "Digest-MD5" # deprecated
     end
 
     # Returns the names of all registered SASL mechanisms.
@@ -78,6 +77,12 @@ def add_authenticator(name, authenticator = nil)
       @authenticators[key] = authenticator
     end
 
+    # Removes the authenticator registered for +name+
+    def remove_authenticator(name)
+      key = name.upcase.to_sym
+      @authenticators.delete(key)
+    end
+
     # :call-seq:
     #   authenticator(mechanism, ...) -> auth_session
     #
@@ -100,6 +105,7 @@ def authenticator(mechanism, ...)
       end
       auth.respond_to?(:new) ? auth.new(...) : auth.call(...)
     end
+    alias new authenticator
 
   end
 

lib/net/imap/sasl/cram_md5_authenticator.rb

@@ -24,6 +24,8 @@ def initialize(user, password, warn_deprecation: true, **_ignored)
     @done = false
   end
 
+  def initial_response?; false end
+
   def process(challenge)
     digest = hmac_md5(challenge, @password)
     return @user + " " + digest

lib/net/imap/sasl/digest_md5_authenticator.rb

@@ -73,6 +73,8 @@ def initialize(user = nil, pass = nil, authz = nil,
     @nc, @stage = {}, STAGE_ONE
   end
 
+  def initial_response?; false end
+
   # Responds to server challenge in two stages.
   def process(challenge)
     case @stage

lib/net/imap/sasl/login_authenticator.rb

@@ -32,6 +32,8 @@ def initialize(user, password, warn_deprecation: true, **_ignored)
     @state = STATE_USER
   end
 
+  def initial_response?; false end
+
   def process(data)
     case @state
     when STATE_USER

test/net/imap/test_imap.rb

@@ -966,7 +966,7 @@ def test_id
         server.state.authenticate(server.config.user)
         cmd.done_ok
       end
-      assert_raise(Net::IMAP::SASL::AuthenticationFailed) do
+      assert_raise(Net::IMAP::SASL::AuthenticationIncomplete) do
         imap.authenticate("DIGEST-MD5", "test_user", "test-password",
                           warn_deprecation: false)
       end

test/net/imap/test_imap_authenticators.rb

@@ -41,7 +41,6 @@ def test_plain_authenticator_matches_mechanism
 
   def test_plain_supports_initial_response
     assert plain("foo", "bar").initial_response?
-    assert Net::IMAP::SASL.initial_response?(plain("foo", "bar"))
   end
 
   def test_plain_response
@@ -194,7 +193,6 @@ def test_xoauth2_kwargs
 
   def test_xoauth2_supports_initial_response
     assert xoauth2("foo", "bar").initial_response?
-    assert Net::IMAP::SASL.initial_response?(xoauth2("foo", "bar"))
   end
 
   # ----------------------
@@ -276,7 +274,7 @@ def test_login_authenticator_matches_mechanism
   end
 
   def test_login_does_not_support_initial_response
-    refute Net::IMAP::SASL.initial_response?(login("foo", "bar"))
+    refute login("foo", "bar").initial_response?
   end
 
   def test_login_authenticator_deprecated
@@ -306,7 +304,7 @@ def test_cram_md5_authenticator_matches_mechanism
   end
 
   def test_cram_md5_does_not_support_initial_response
-    refute Net::IMAP::SASL.initial_response?(cram_md5("foo", "bar"))
+    refute cram_md5("foo", "bar").initial_response?
   end
 
   def test_cram_md5_authenticator_deprecated
@@ -343,7 +341,7 @@ def test_digest_md5_authenticator_deprecated
   end
 
   def test_digest_md5_does_not_support_initial_response
-    refute Net::IMAP::SASL.initial_response?(digest_md5("foo", "bar"))
+    refute digest_md5("foo", "bar").initial_response?
   end
 
   def test_digest_md5_authenticator