Add forward-compatible API for SASL mechanisms

nevans · nevans · commit baf9035a0ff7 · 2023-10-10T12:15:30.000-04:00
The current #start and #authenticate API can't fully support every SASL
mechanism.  Some SASL mechanisms do not require a +username+
(OAUTHBEARER, EXTERNAL, ANONYMOUS) or a +secret+ (EXTERNAL, ANONYMOUS).
Many SASL mechanisms will need to take extra arguments (e.g: `authzid`
for many mechanisms, `warn_deprecations` for deprecated mechanisms,
`min_iterations` for SCRAM-*, `anonymous_message` for ANONYMOUS), and so
on.  And, although it is convenient to use +username+ as an alias for
+authcid+ or +authzid+ and +secret+ as an alias for +password+ or
+oauth2_token+, it can also be useful to have keyword parameters that
keep stable semantics across many different mechanisms.

A SASL-compatible API must first find the authenticator for the
mechanism and then delegate any arbitrary parameters to that
authenticator.  Practically, that means that the mechanism name must
either be the first positional parameter or a keyword parameter, and
then every other parameter can be forwarded.  `Net::SMTP#auth` does this.
Also, an `auth` keyword parameter has been added to `Net::SMTP#start`,
allowing `start` to pass any arbitrary keyword parameters into `#auth`.

For backward compatibility, when one of the existing Authenticator
classes is selected, it converts keyword args to positional args.  Also
for backward compatibility, `Net::SMTP#authenticate` keeps its v0.4.0
implementation.
diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb
@@ -179,13 +179,29 @@ class SMTPUnsupportedCommand < ProtocolError
   #     # PLAIN
   #     Net::SMTP.start('your.smtp.server', 25,
   #                     user: 'Your Account', secret: 'Your Password', authtype: :plain)
+  #     Net::SMTP.start("your.smtp.server", 25,
+  #                     auth: {type:     :plain,
+  #                            username: "authentication identity",
+  #                            password: password})
+  #
   #     # LOGIN
   #     Net::SMTP.start('your.smtp.server', 25,
   #                     user: 'Your Account', secret: 'Your Password', authtype: :login)
+  #     Net::SMTP.start("your.smtp.server", 25,
+  #                     auth: {type:     :login,
+  #                            username: "authentication identity",
+  #                            password: password})
   #
   #     # CRAM MD5
   #     Net::SMTP.start('your.smtp.server', 25,
   #                     user: 'Your Account', secret: 'Your Password', authtype: :cram_md5)
+  #     Net::SMTP.start("your.smtp.server", 25,
+  #                     auth: {type:     :cram_md5,
+  #                            username: 'Your Account',
+  #                            password: 'Your Password'})
+  #
+  # +LOGIN+, and +CRAM-MD5+ are still available for backwards compatibility, but
+  # are deprecated and should be avoided.
   #
   class SMTP < Protocol
     VERSION = "0.4.0"
@@ -452,6 +468,7 @@ def debug_output=(arg)
 
     #
     # :call-seq:
+    #  start(address, port = nil, helo: 'localhost', auth: nil, tls: false, starttls: :auto, tls_verify: true, tls_hostname: nil, ssl_context_params: nil) { |smtp| ... }
     #  start(address, port = nil, helo: 'localhost', user: nil, secret: nil, authtype: nil, tls: false, starttls: :auto, tls_verify: true, tls_hostname: nil, ssl_context_params: nil) { |smtp| ... }
     #  start(address, port = nil, helo = 'localhost', user = nil, secret = nil, authtype = nil) { |smtp| ... }
     #
@@ -517,16 +534,17 @@ def debug_output=(arg)
     # * IOError
     #
     def SMTP.start(address, port = nil, *args, helo: nil,
-                   user: nil, secret: nil, password: nil, authtype: nil,
+                   user: nil, username: nil, secret: nil, password: nil,
+                   authtype: nil, auth: nil,
                    tls: false, starttls: :auto,
                    tls_verify: true, tls_hostname: nil, ssl_context_params: nil,
                    &block)
       raise ArgumentError, "wrong number of arguments (given #{args.size + 2}, expected 1..6)" if args.size > 4
       helo ||= args[0] || 'localhost'
-      user ||= args[1]
+      user ||= username || args[1]
       secret ||= password || args[2]
       authtype ||= args[3]
-      new(address, port, tls: tls, starttls: starttls, tls_verify: tls_verify, tls_hostname: tls_hostname, ssl_context_params: ssl_context_params).start(helo: helo, user: user, secret: secret, authtype: authtype, &block)
+      new(address, port, tls: tls, starttls: starttls, tls_verify: tls_verify, tls_hostname: tls_hostname, ssl_context_params: ssl_context_params).start(helo: helo, user: user, secret: secret, authtype: authtype, auth: auth, &block)
     end
 
     # +true+ if the SMTP session has been started.
@@ -538,6 +556,7 @@ def started?
     # :call-seq:
     #  start(helo: 'localhost', user: nil, secret: nil, authtype: nil) { |smtp| ... }
     #  start(helo = 'localhost', user = nil, secret = nil, authtype = nil) { |smtp| ... }
+    #  start(helo = 'localhost', auth: {type: nil, **auth_kwargs}) { |smtp| ... }
     #
     # Opens a TCP connection and starts the SMTP session.
     #
@@ -546,11 +565,10 @@ def started?
     # +helo+ is the _HELO_ _domain_ that you'll dispatch mails from; see
     # the discussion in the overview notes.
     #
-    # If both of +user+ and +secret+ are given, SMTP authentication
-    # will be attempted using the AUTH command.  +authtype+ specifies
-    # the type of authentication to attempt; it must be one of
-    # :login, :plain, and :cram_md5.  See the notes on SMTP Authentication
-    # in the overview.
+    # If either +auth+ or +user+ are given, SMTP authentication will be
+    # attempted using the AUTH command.  +authtype+ specifies the type of
+    # authentication to attempt; it must be one of :login, :plain, and
+    # :cram_md5.  See the notes on SMTP Authentication in the overview.
     #
     # === Block Usage
     #
@@ -589,12 +607,15 @@ def started?
     # * Net::ReadTimeout
     # * IOError
     #
-    def start(*args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil)
+    def start(*args, helo: nil,
+              user: nil, username: nil, secret: nil, password: nil,
+              authtype: nil, auth: nil)
       raise ArgumentError, "wrong number of arguments (given #{args.size}, expected 0..4)" if args.size > 4
       helo ||= args[0] || 'localhost'
-      user ||= args[1]
-      secret ||= password || args[2]
-      authtype ||= args[3]
+      auth = merge_auth_params(user     || username || args[1],
+                               secret   || password || args[2],
+                               authtype ||             args[3],
+                               auth)
       if defined?(OpenSSL::VERSION)
         ssl_context_params = @ssl_context_params || {}
         unless ssl_context_params.has_key?(:verify_mode)
@@ -609,13 +630,13 @@ def start(*args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil
       end
       if block_given?
         begin
-          do_start helo, user, secret, authtype
+          do_start helo, **auth
           return yield(self)
         ensure
           do_finish
         end
       else
-        do_start helo, user, secret, authtype
+        do_start helo, **auth
         return self
       end
     end
@@ -633,12 +654,8 @@ def tcp_socket(address, port)
       TCPSocket.open address, port
     end
 
-    def do_start(helo_domain, user, secret, authtype)
+    def do_start(helo_domain, **authopts)
       raise IOError, 'SMTP session already started' if @started
-      if user or secret
-        check_auth_method(authtype || DEFAULT_AUTH_TYPE)
-        check_auth_args user, secret
-      end
       s = Timeout.timeout(@open_timeout, Net::OpenTimeout) do
         tcp_socket(@address, @port)
       end
@@ -655,7 +672,7 @@ def do_start(helo_domain, user, secret, authtype)
         # helo response may be different after STARTTLS
         do_helo helo_domain
       end
-      authenticate user, secret, (authtype || DEFAULT_AUTH_TYPE) if user
+      auth(**authopts) if authopts.any?
       @started = true
     ensure
       unless @started
@@ -833,13 +850,39 @@ def open_message_stream(from_addr, *to_addrs, &block)   # :yield: stream
 
     DEFAULT_AUTH_TYPE = :plain
 
+    # Deprecated: use #auth instead.
     def authenticate(user, secret, authtype = DEFAULT_AUTH_TYPE)
+      # warn "DEPRECATED: use Net::SMTP#auth instead"
       check_auth_method authtype
       check_auth_args user, secret
       authenticator = Authenticator.auth_class(authtype).new(self)
       critical { authenticator.auth(user, secret) }
     end
 
+    # call-seq:
+    #   auth(mechanism, ...)
+    #   auth(type: mechanism, **kwargs, &block)
+    #
+    # All arguments besides +mechanism+ are forwarded directly to the
+    # authenticator.  Alternatively, +mechanism+ can be provided by the +type+
+    # keyword parameter.  Positional parameters cannot be used with +type+.
+    #
+    # Different authenticators take different options, but common options
+    # include +authcid+ for authentication identity, +authzid+ for authorization
+    # identity, +username+ for either "authentication identity" or
+    # "authorization identity" depending on the +mechanism+, and +password+.
+    def auth(*args, **kwargs, &blk)
+      args, kwargs = backward_compatible_auth_args(*args, **kwargs)
+      authtype, *args = args
+      authenticator = Authenticator.auth_class(authtype).new(self)
+      if kwargs.empty?
+        # TODO: remove this, unless it is needed for 2.6/2.7/3.0 compatibility
+        critical { authenticator.auth(*args, &blk) }
+      else
+        critical { authenticator.auth(*args, **kwargs, &blk) }
+      end
+    end
+
     private
 
     def check_auth_method(type)
@@ -857,6 +900,45 @@ def check_auth_args(user, secret, authtype = DEFAULT_AUTH_TYPE)
       end
     end
 
+    # Convert the original +user+, +secret+, +authtype+ with +auth+, and checks
+    # the arguments.
+    def merge_auth_params(user, secret, authtype, auth)
+      auth = Hash.try_convert(auth) || {}
+      if user || secret || authtype
+        args = { type: authtype || DEFAULT_AUTH_TYPE,
+                 username: user, secret: secret }
+        auth = args.merge(auth)
+        check_auth_method(auth[:type])
+        check_auth_args(auth[:authcid]  || auth[:username],
+                        auth[:password] || auth[:secret],
+                        auth[:type])
+      elsif auth.any?
+        check_auth_method(auth[:type] || DEFAULT_AUTH_TYPE)
+        # check_auth_args may not be valid, depending on the authtype.
+      end
+      auth
+    end
+
+    # Convert +type+, +username+, +secret+ (etc) kwargs to positional args, for
+    # compatibility with existing authenticators.
+    def backward_compatible_auth_args(_type = nil, *args, type: nil,
+                                      username: nil, authcid: nil,
+                                      secret: nil, password: nil,
+                                      **kwargs)
+      type && _type and
+        raise ArgumentError, 'conflict between "type" keyword argument ' \
+                             'and positional argument'
+      type ||= _type || DEFAULT_AUTH_TYPE
+      check_auth_method(type)
+      auth_class = Authenticator.auth_class(type)
+      if auth_class.is_a?(Class) && auth_class <= Authenticator
+        args[0] ||= authcid  || username
+        args[1] ||= password || secret
+        check_auth_args(args[0], args[1], type)
+      end
+      [[type, *args], kwargs]
+    end
+
     #
     # SMTP command dispatcher
     #
diff --git a/test/net/smtp/test_smtp.rb b/test/net/smtp/test_smtp.rb
@@ -110,6 +110,21 @@ def test_auth_plain
       smtp = Net::SMTP.start 'localhost', server.port
       assert smtp.authenticate("account", "password", :plain).success?
       assert_equal "AUTH PLAIN AGFjY291bnQAcGFzc3dvcmQ=\r\n", server.commands.last
+
+      server = FakeServer.start(auth: 'plain')
+      smtp = Net::SMTP.start 'localhost', server.port
+      assert smtp.auth("PLAIN", "account", "password").success?
+      assert_equal "AUTH PLAIN AGFjY291bnQAcGFzc3dvcmQ=\r\n", server.commands.last
+
+      server = FakeServer.start(auth: 'plain')
+      smtp = Net::SMTP.start 'localhost', server.port
+      assert smtp.auth(type: "PLAIN", username: "account", secret: "password").success?
+      assert_equal "AUTH PLAIN AGFjY291bnQAcGFzc3dvcmQ=\r\n", server.commands.last
+
+      server = FakeServer.start(auth: 'plain')
+      smtp = Net::SMTP.start 'localhost', server.port
+      assert smtp.auth("PLAIN", username: "account", password: "password").success?
+      assert_equal "AUTH PLAIN AGFjY291bnQAcGFzc3dvcmQ=\r\n", server.commands.last
     end
 
     def test_unsucessful_auth_plain
@@ -120,10 +135,20 @@ def test_unsucessful_auth_plain
       assert_equal "535", err.response.status
     end
 
+    def test_auth_cram_md5
+      server = FakeServer.start(auth: 'CRAM-MD5')
+      smtp = Net::SMTP.start 'localhost', server.port
+      assert smtp.auth(:cram_md5, "account", password: "password").success?
+    end
+
     def test_auth_login
       server = FakeServer.start(auth: 'login')
       smtp = Net::SMTP.start 'localhost', server.port
       assert smtp.authenticate("account", "password", :login).success?
+
+      server = FakeServer.start(auth: 'login')
+      smtp = Net::SMTP.start 'localhost', server.port
+      assert smtp.auth("LOGIN", username: "account", secret: "password").success?
     end
 
     def test_unsucessful_auth_login
@@ -455,6 +480,15 @@ def test_start_auth_plain
       port = fake_server_start(auth: 'plain')
       Net::SMTP.start('localhost', port, user: 'account', password: 'password', authtype: :plain){}
 
+      port = fake_server_start(auth: 'plain')
+      Net::SMTP.start('localhost', port, authtype: "PLAIN",
+                      auth: {username: 'account', password: 'password'}){}
+
+      port = fake_server_start(auth: 'plain')
+      Net::SMTP.start('localhost', port, auth: {username: 'account',
+                                                password: 'password',
+                                                type: :plain}){}
+
       port = fake_server_start(auth: 'plain')
       assert_raise Net::SMTPAuthenticationError do
         Net::SMTP.start('localhost', port, user: 'account', password: 'invalid', authtype: :plain){}
