From 305d22955fd9a679ff4dc72b8ccb631a80558980 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 14 Oct 2023 16:01:29 -0400 Subject: [PATCH 1/9] Forward keyword args and block from #authenticate --- lib/net/smtp.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index cca06e6..44ebfb5 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -869,10 +869,10 @@ def open_message_stream(from_addr, *to_addrs, &block) # :yield: stream # All arguments-other than +authtype+-are forwarded to the authenticator. # Different authenticators may interpret the +user+ and +secret+ # arguments differently. - def authenticate(user, secret, authtype = DEFAULT_AUTH_TYPE) - check_auth_args authtype, user, secret + def authenticate(user, secret, authtype = DEFAULT_AUTH_TYPE, **kwargs, &block) + check_auth_args authtype, user, secret, **kwargs authenticator = Authenticator.auth_class(authtype).new(self) - authenticator.auth(user, secret) + authenticator.auth(user, secret, **kwargs, &block) end private From 2dd30e58e37cdf0a71acf41c3f3cf10e358090fd Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 7 Nov 2023 18:10:29 -0500 Subject: [PATCH 2/9] Add `auth` keyword arg to `start` methods This adds a new `auth` keyword param to `Net::SMTP.start` and `#start` that can be used to pass any arbitrary keyword parameters to `#authenticate`. The pre-existing `username`, `secret`, etc keyword params will retain their existing behavior as positional arguments to `#authenticate`. --- lib/net/smtp.rb | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index 44ebfb5..dbed2c4 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -459,6 +459,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| ... } # @@ -521,6 +522,8 @@ def debug_output=(arg) # These will be sent to #authenticate as positional arguments-the exact # semantics are dependent on the +authtype+. # + # +auth+ is an optional hash of keyword arguments for #authenticate. + # # See the discussion of Net::SMTP@SMTP+Authentication in the overview notes. # # === Errors @@ -538,6 +541,7 @@ def debug_output=(arg) # def SMTP.start(address, port = nil, *args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil, + auth: nil, tls: false, starttls: :auto, tls_verify: true, tls_hostname: nil, ssl_context_params: nil, &block) @@ -546,7 +550,8 @@ def SMTP.start(address, port = nil, *args, helo: nil, user ||= 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. @@ -558,6 +563,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. # @@ -578,6 +584,8 @@ def started? # These will be sent to #authenticate as positional arguments-the exact # semantics are dependent on the +authtype+. # + # +auth+ is an optional hash of keyword arguments for #authenticate. + # # See the discussion of Net::SMTP@SMTP+Authentication in the overview notes. # # See also: Net::SMTP.start @@ -619,12 +627,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, 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 ||= {} if defined?(OpenSSL::VERSION) ssl_context_params = @ssl_context_params || {} unless ssl_context_params.has_key?(:verify_mode) @@ -639,13 +650,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, user, secret, authtype, **auth return yield(self) ensure do_finish end else - do_start helo, user, secret, authtype + do_start helo, user, secret, authtype, **auth return self end end @@ -663,10 +674,10 @@ def tcp_socket(address, port) TCPSocket.open address, port end - def do_start(helo_domain, user, secret, authtype) + def do_start(helo_domain, user, secret, authtype, **auth) raise IOError, 'SMTP session already started' if @started - if user || secret || authtype - check_auth_args authtype, user, secret + if user || secret || authtype || auth.any? + check_auth_args(authtype, user, secret, **auth) end s = Timeout.timeout(@open_timeout, Net::OpenTimeout) do tcp_socket(@address, @port) @@ -684,7 +695,11 @@ 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 + if user or secret + authenticate(user, secret, authtype, **auth) + elsif authtype or auth.any? + authenticate(authtype, **auth) + end @started = true ensure unless @started From 7771a0f6c361ac54fb80bc16236b48ade0ef1a58 Mon Sep 17 00:00:00 2001 From: nicholas evans Date: Wed, 11 Oct 2023 08:46:25 -0400 Subject: [PATCH 3/9] Add `username` keyword param to `start` Although "user" is a reasonable abbreviation, the parameter is more accurately described as a "username" or an "authentication identity". They are synonomous here, with "username" winning when both are present. --- lib/net/smtp.rb | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index dbed2c4..a1f5612 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -183,7 +183,7 @@ class SMTPUnsupportedCommand < ProtocolError # # # PLAIN # Net::SMTP.start('your.smtp.server', 25, - # user: 'Your Account', secret: 'Your Password', authtype: :plain) + # username: 'Your Account', secret: 'Your Password', authtype: :plain) # # Support for other SASL mechanisms-such as +EXTERNAL+, +OAUTHBEARER+, # +SCRAM-SHA-256+, and +XOAUTH2+-will be added in a future release. @@ -460,15 +460,15 @@ 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| ... } + # start(address, port = nil, helo: 'localhost', username: 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', username = nil, secret = nil, authtype = nil) { |smtp| ... } # # Creates a new Net::SMTP object and connects to the server. # # This method is equivalent to: # # Net::SMTP.new(address, port, tls_verify: flag, tls_hostname: hostname, ssl_context_params: nil) - # .start(helo: helo_domain, user: account, secret: password, authtype: authtype) + # .start(helo: helo_domain, username: account, secret: password, authtype: authtype) # # See also: Net::SMTP.new, #start # @@ -515,7 +515,7 @@ def debug_output=(arg) # # +authtype+ is the SASL authentication mechanism. # - # +user+ is the authentication or authorization identity. + # +username+ or +user+ is the authentication or authorization identity. # # +secret+ or +password+ is your password or other authentication token. # @@ -541,17 +541,18 @@ def debug_output=(arg) # def SMTP.start(address, port = nil, *args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil, + username: 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] + username ||= user || 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, auth: auth, &block) + .start(helo: helo, username: username, secret: secret, authtype: authtype, auth: auth, &block) end # +true+ if the \SMTP session has been started. @@ -561,8 +562,8 @@ 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', username: nil, secret: nil, authtype: nil) { |smtp| ... } + # start(helo = 'localhost', username = nil, secret = nil, authtype = nil) { |smtp| ... } # start(helo = 'localhost', auth: {type: nil, **auth_kwargs}) { |smtp| ... } # # Opens a TCP connection and starts the SMTP session. @@ -577,7 +578,7 @@ def started? # # +authtype+ is the SASL authentication mechanism. # - # +user+ is the authentication or authorization identity. + # +username+ or +user+ is the authentication or authorization identity. # # +secret+ or +password+ is your password or other authentication token. # @@ -603,7 +604,7 @@ def started? # # require 'net/smtp' # smtp = Net::SMTP.new('smtp.mail.server', 25) - # smtp.start(helo: helo_domain, user: account, secret: password, authtype: authtype) do |smtp| + # smtp.start(helo: helo_domain, username: account, secret: password, authtype: authtype) do |smtp| # smtp.send_message msgstr, 'from@example.com', ['dest@example.com'] # end # @@ -628,11 +629,11 @@ def started? # * IOError # def start(*args, helo: nil, - user: nil, secret: nil, password: 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] + username ||= user || args[1] secret ||= password || args[2] authtype ||= args[3] auth ||= {} @@ -650,13 +651,13 @@ def start(*args, helo: nil, end if block_given? begin - do_start helo, user, secret, authtype, **auth + do_start helo, username, secret, authtype, **auth return yield(self) ensure do_finish end else - do_start helo, user, secret, authtype, **auth + do_start helo, username, secret, authtype, **auth return self end end @@ -882,12 +883,12 @@ def open_message_stream(from_addr, *to_addrs, &block) # :yield: stream # +authtype+ is the name of a SASL authentication mechanism. # # All arguments-other than +authtype+-are forwarded to the authenticator. - # Different authenticators may interpret the +user+ and +secret+ + # Different authenticators may interpret the +username+ and +secret+ # arguments differently. - def authenticate(user, secret, authtype = DEFAULT_AUTH_TYPE, **kwargs, &block) - check_auth_args authtype, user, secret, **kwargs + def authenticate(username, secret, authtype = DEFAULT_AUTH_TYPE, **kwargs, &block) + check_auth_args authtype, username, secret, **kwargs authenticator = Authenticator.auth_class(authtype).new(self) - authenticator.auth(user, secret, **kwargs, &block) + authenticator.auth(username, secret, **kwargs, &block) end private From 02408b1c380ee6e725e785fbca30c5266806774b Mon Sep 17 00:00:00 2001 From: nicholas evans Date: Wed, 11 Oct 2023 00:54:30 -0400 Subject: [PATCH 4/9] Add keyword parameters to authenticators Username can be set by args[0], authcid, username, or user. Secret can be set by args[1], secret, or password. Since all of the existing authenticators have the same API, it is sufficient to update `check_args` in the base class. --- lib/net/smtp/auth_cram_md5.rb | 7 ++++++- lib/net/smtp/auth_login.rb | 7 ++++++- lib/net/smtp/auth_plain.rb | 7 ++++++- lib/net/smtp/authenticator.rb | 15 ++++++++++++--- test/net/smtp/test_smtp.rb | 9 +++++++++ 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/net/smtp/auth_cram_md5.rb b/lib/net/smtp/auth_cram_md5.rb index 0490cd6..50994ee 100644 --- a/lib/net/smtp/auth_cram_md5.rb +++ b/lib/net/smtp/auth_cram_md5.rb @@ -9,7 +9,12 @@ class Net::SMTP class AuthCramMD5 < Net::SMTP::Authenticator auth_type :cram_md5 - def auth(user, secret) + def auth(user_arg = nil, secret_arg = nil, + authcid: nil, username: nil, user: nil, + secret: nil, password: nil, + **) + user = req_param authcid, username, user, user_arg, "username (authcid)" + secret = req_param password, secret, secret_arg, "secret (password)" challenge = continue('AUTH CRAM-MD5') crammed = cram_md5_response(secret, challenge.unpack1('m')) finish(base64_encode("#{user} #{crammed}")) diff --git a/lib/net/smtp/auth_login.rb b/lib/net/smtp/auth_login.rb index 545c1f9..174ab09 100644 --- a/lib/net/smtp/auth_login.rb +++ b/lib/net/smtp/auth_login.rb @@ -2,7 +2,12 @@ class Net::SMTP class AuthLogin < Net::SMTP::Authenticator auth_type :login - def auth(user, secret) + def auth(user_arg = nil, secret_arg = nil, + authcid: nil, username: nil, user: nil, + secret: nil, password: nil, + **) + user = req_param authcid, username, user, user_arg, "username (authcid)" + secret = req_param password, secret, secret_arg, "secret (password)" continue('AUTH LOGIN') continue(base64_encode(user)) finish(base64_encode(secret)) diff --git a/lib/net/smtp/auth_plain.rb b/lib/net/smtp/auth_plain.rb index 7fa1198..e778bad 100644 --- a/lib/net/smtp/auth_plain.rb +++ b/lib/net/smtp/auth_plain.rb @@ -2,7 +2,12 @@ class Net::SMTP class AuthPlain < Net::SMTP::Authenticator auth_type :plain - def auth(user, secret) + def auth(user_arg = nil, secret_arg = nil, + authcid: nil, username: nil, user: nil, + secret: nil, password: nil, + **) + user = req_param authcid, username, user, user_arg, "username (authcid)" + secret = req_param password, secret, secret_arg, "secret (password)" finish('AUTH PLAIN ' + base64_encode("\0#{user}\0#{secret}")) end end diff --git a/lib/net/smtp/authenticator.rb b/lib/net/smtp/authenticator.rb index 4e91228..6e381af 100644 --- a/lib/net/smtp/authenticator.rb +++ b/lib/net/smtp/authenticator.rb @@ -15,11 +15,14 @@ def self.auth_class(type) Authenticator.auth_classes[type] end - def self.check_args(user_arg = nil, secret_arg = nil, *, **) - unless user_arg + def self.check_args(user_arg = nil, secret_arg = nil, *, + authcid: nil, username: nil, user: nil, + secret: nil, password: nil, + **) + unless authcid || username || user || user_arg raise ArgumentError, 'SMTP-AUTH requested but missing user name' end - unless secret_arg + unless password || secret || secret_arg raise ArgumentError, 'SMTP-AUTH requested but missing secret phrase' end end @@ -52,6 +55,12 @@ def base64_encode(str) # expects "str" may not become too long [str].pack('m0') end + + def req_param(*args, name) + args.compact.first or + raise ArgumentError, "SMTP-AUTH requested but missing #{name}" + end + end end end diff --git a/test/net/smtp/test_smtp.rb b/test/net/smtp/test_smtp.rb index 3b9e245..dbe8980 100644 --- a/test/net/smtp/test_smtp.rb +++ b/test/net/smtp/test_smtp.rb @@ -470,6 +470,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){} From 0dacb93bbf611f38875cc969d8af679a54c399ab Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 11 Oct 2023 13:46:20 -0400 Subject: [PATCH 5/9] Do not require positional args for `#authenticate` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This API is a little bit confusing, IMO. But it does preserve backward compatibility, while allowing authenticators that don't allow positional parameters to work without crashing. But, authenticators that require only one parameter—or more than three—will still be inaccessible. --- lib/net/smtp.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index a1f5612..bff939a 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -878,6 +878,10 @@ def open_message_stream(from_addr, *to_addrs, &block) # :yield: stream DEFAULT_AUTH_TYPE = :plain + # call-seq: + # authenticate(authtype = DEFAULT_AUTH_TYPE, **, &) + # authenticate(username, secret, authtype = DEFAULT_AUTH_TYPE, **, &) + # # Authenticates with the server, using the "AUTH" command. # # +authtype+ is the name of a SASL authentication mechanism. @@ -885,10 +889,17 @@ def open_message_stream(from_addr, *to_addrs, &block) # :yield: stream # All arguments-other than +authtype+-are forwarded to the authenticator. # Different authenticators may interpret the +username+ and +secret+ # arguments differently. - def authenticate(username, secret, authtype = DEFAULT_AUTH_TYPE, **kwargs, &block) - check_auth_args authtype, username, secret, **kwargs + def authenticate(*args, **kwargs, &block) + case args.length + when 1, 3 then authtype = args.pop + when (4..) + raise ArgumentError, "wrong number of arguments " \ + "(given %d, expected 0..3)" % [args.length] + end + authtype ||= DEFAULT_AUTH_TYPE + check_auth_args authtype, *args, **kwargs authenticator = Authenticator.auth_class(authtype).new(self) - authenticator.auth(username, secret, **kwargs, &block) + authenticator.auth(*args, **kwargs, &block) end private From 00d50b24783e37cf585bcb76727c5de8b772c11d Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 14 Oct 2023 16:13:58 -0400 Subject: [PATCH 6/9] Add `type` keyword arg to `#authenticate` This is convenient for `smtp.start auth: {type:, **etc}`. --- lib/net/smtp.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index bff939a..178b949 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -879,12 +879,14 @@ def open_message_stream(from_addr, *to_addrs, &block) # :yield: stream DEFAULT_AUTH_TYPE = :plain # call-seq: - # authenticate(authtype = DEFAULT_AUTH_TYPE, **, &) - # authenticate(username, secret, authtype = DEFAULT_AUTH_TYPE, **, &) + # authenticate(type: DEFAULT_AUTH_TYPE, **, &) + # authenticate(type = DEFAULT_AUTH_TYPE, **, &) + # authenticate(username, secret, type: DEFAULT_AUTH_TYPE, **, &) + # authenticate(username, secret, type = DEFAULT_AUTH_TYPE, **, &) # # Authenticates with the server, using the "AUTH" command. # - # +authtype+ is the name of a SASL authentication mechanism. + # +type+ is the name of a SASL authentication mechanism. # # All arguments-other than +authtype+-are forwarded to the authenticator. # Different authenticators may interpret the +username+ and +secret+ @@ -896,19 +898,19 @@ def authenticate(*args, **kwargs, &block) raise ArgumentError, "wrong number of arguments " \ "(given %d, expected 0..3)" % [args.length] end - authtype ||= DEFAULT_AUTH_TYPE - check_auth_args authtype, *args, **kwargs + authtype, args, kwargs = check_auth_args authtype, *args, **kwargs authenticator = Authenticator.auth_class(authtype).new(self) authenticator.auth(*args, **kwargs, &block) end private - def check_auth_args(type, *args, **kwargs) - type ||= DEFAULT_AUTH_TYPE + def check_auth_args(type_arg = nil, *args, type: nil, **kwargs) + type ||= type_arg || DEFAULT_AUTH_TYPE klass = Authenticator.auth_class(type) or raise ArgumentError, "wrong authentication type #{type}" klass.check_args(*args, **kwargs) + [type, args, kwargs] end # From 1920c9ef62a919af49d1b22e91b3a227e0ff0a7a Mon Sep 17 00:00:00 2001 From: nicholas evans Date: Wed, 11 Oct 2023 08:56:08 -0400 Subject: [PATCH 7/9] Add `#auth` method for SASL authentication Although `#authenticate` can be updated to make username and secret _both_ optional, by placing the mechanism last and making it optional, it's not possible to use an authenticator with a _single_ positional parameter or with more than two positional parameters. By placing `type` first among positional parameters or as a keyword argument, we avoid this problem. --- lib/net/smtp.rb | 16 ++++++++++++++++ test/net/smtp/test_smtp.rb | 25 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index 178b949..a1d398f 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -898,6 +898,22 @@ def authenticate(*args, **kwargs, &block) raise ArgumentError, "wrong number of arguments " \ "(given %d, expected 0..3)" % [args.length] end + auth(authtype, *args, **kwargs, &block) + end + + # call-seq: + # auth(type = DEFAULT_AUTH_TYPE, ...) + # auth(type: DEFAULT_AUTH_TYPE, **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(authtype = DEFAULT_AUTH_TYPE, *args, **kwargs, &block) authtype, args, kwargs = check_auth_args authtype, *args, **kwargs authenticator = Authenticator.auth_class(authtype).new(self) authenticator.auth(*args, **kwargs, &block) diff --git a/test/net/smtp/test_smtp.rb b/test/net/smtp/test_smtp.rb index dbe8980..69d249a 100644 --- 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_unsuccessful_auth_plain @@ -120,10 +135,20 @@ def test_unsuccessful_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_unsuccessful_auth_login From dcc6e154cae90c53d686cb4b7832c6115a783ff7 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 9 Oct 2023 18:13:02 -0400 Subject: [PATCH 8/9] Drop support for ruby 2.6 The net-imap dependency requires 2.7.3, to deal with kwargs. --- .github/workflows/test.yml | 2 +- net-smtp.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 98f8f4c..97ff6fc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,7 +7,7 @@ jobs: uses: ruby/actions/.github/workflows/ruby_versions.yml@master with: engine: cruby - min_version: 2.6 + min_version: 2.7 build: needs: ruby-versions diff --git a/net-smtp.gemspec b/net-smtp.gemspec index 7bdb112..8bebe8e 100644 --- a/net-smtp.gemspec +++ b/net-smtp.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |spec| spec.description = %q{Simple Mail Transfer Protocol client library for Ruby.} spec.homepage = "https://github.com/ruby/net-smtp" spec.licenses = ["Ruby", "BSD-2-Clause"] - spec.required_ruby_version = ">= 2.6.0" + spec.required_ruby_version = ">= 2.7.3" spec.metadata["homepage_uri"] = spec.homepage spec.metadata["source_code_uri"] = spec.homepage From 0d4d05ef2e4d1472d0387e1c1cfc44ec03d08586 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 9 Oct 2023 15:39:21 -0400 Subject: [PATCH 9/9] =?UTF-8?q?Use=20net-imap's=20SASL=20implementation=20?= =?UTF-8?q?=F0=9F=9A=A7[WIP]=F0=9F=9A=A7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds the `net-imap` as a default fallback for mechanisms that haven't otherwise been added. In this commit, the original implementation is still used by `#authenticate` for the `PLAIN`, `XOAUTH2`, `LOGIN`, and `CRAM-MD5` mechanisms. Every other mechanism supported by `net-imap` v0.4.0 is added here: * `ANONYMOUS` * `DIGEST-MD5` _(deprecated)_ * `EXTERNAL` * `OAUTHBEARER` * `SCRAM-SHA-1` and `SCRAM-SHA-256` **TODO:** Ideally, `net-smtp` and `net-imap` should both depend on a shared `sasl` or `net-sasl` gem, rather than keep the SASL implementation inside one or the other. See https://github.com/ruby/net-imap/issues/23. --- .github/workflows/test.yml | 2 +- lib/net/smtp.rb | 56 +++++++++++++++++-- lib/net/smtp/auth_sasl_client_adapter.rb | 42 ++++++++++++++ .../smtp/auth_sasl_compatibility_adapter.rb | 19 +++++++ net-smtp.gemspec | 2 + test/net/smtp/test_smtp.rb | 10 ---- 6 files changed, 114 insertions(+), 17 deletions(-) create mode 100644 lib/net/smtp/auth_sasl_client_adapter.rb create mode 100644 lib/net/smtp/auth_sasl_compatibility_adapter.rb diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 97ff6fc..508cfb4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,4 +31,4 @@ jobs: - name: Install dependencies run: bundle install - name: Run test - run: rake test + run: bundle exec rake test diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index a1d398f..1d1a4e3 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -175,8 +175,8 @@ class SMTPUnsupportedCommand < ProtocolError # # The Net::SMTP class supports the \SMTP extension for SASL Authentication # [RFC4954[https://www.rfc-editor.org/rfc/rfc4954.html]] and the following - # SASL mechanisms: +PLAIN+, +LOGIN+ _(deprecated)_, and +CRAM-MD5+ - # _(deprecated)_. + # SASL mechanisms: +ANONYMOUS+, +EXTERNAL+, +OAUTHBEARER+, +PLAIN+, + # +SCRAM-SHA-1+, +SCRAM-SHA-256+, and +XOAUTH2+. # # To use \SMTP authentication, pass extra arguments to # SMTP.start or SMTP#start. @@ -185,10 +185,38 @@ class SMTPUnsupportedCommand < ProtocolError # Net::SMTP.start('your.smtp.server', 25, # username: 'Your Account', secret: 'Your Password', authtype: :plain) # - # Support for other SASL mechanisms-such as +EXTERNAL+, +OAUTHBEARER+, - # +SCRAM-SHA-256+, and +XOAUTH2+-will be added in a future release. + # # SCRAM-SHA-256 + # Net::SMTP.start("your.smtp.server", 25, + # user: "authentication identity", secret: password, + # authtype: :scram_sha_256) + # Net::SMTP.start("your.smtp.server", 25, + # auth: {type: :scram_sha_256, + # username: "authentication identity", + # password: password, + # authzid: "authorization identity"}) # optional # - # The +LOGIN+ and +CRAM-MD5+ mechanisms are still available for backwards + # # OAUTHBEARER + # Net::SMTP.start("your.smtp.server", 25, + # auth: {type: :oauthbearer, + # oauth2_token: oauth2_access_token, + # authzid: "authorization identity", # optional + # host: "your.smtp.server", # optional + # port: 25}) # optional + # + # # XOAUTH2 + # Net::SMTP.start("your.smtp.server", 25, + # user: "username", secret: oauth2_access_token, authtype: :xoauth2) + # Net::SMTP.start("your.smtp.server", 25, + # auth: {type: :xoauth2, + # username: "username", + # oauth2_token: oauth2_token}) + # + # # EXTERNAL + # Net::SMTP.start("your.smtp.server", 587, + # starttls: :always, ssl_context_params: ssl_ctx_params, + # authtype: "external") + # + # +DIGEST-MD5+, +LOGIN+, and +CRAM-MD5+ are still available for backwards # compatibility, but are deprecated and should be avoided. # class SMTP < Protocol @@ -921,8 +949,9 @@ def auth(authtype = DEFAULT_AUTH_TYPE, *args, **kwargs, &block) private - def check_auth_args(type_arg = nil, *args, type: nil, **kwargs) + def check_auth_args(type_arg = nil, *args, type: nil, user: nil, **kwargs) type ||= type_arg || DEFAULT_AUTH_TYPE + kwargs[:username] ||= user if user klass = Authenticator.auth_class(type) or raise ArgumentError, "wrong authentication type #{type}" klass.check_args(*args, **kwargs) @@ -1037,6 +1066,21 @@ def get_response(reqline) recv_response() end + # Returns a successful Response. + # + # Yields continuation data and replies to the server using the block result. + # + # Raises an exception for any non-successful, non-continuation response. + def send_command_with_continuations(*args) + server_resp = get_response args.join(" ") + while server_resp.continue? + client_resp = yield server_resp.string.strip.split(nil, 2).last + server_resp = get_response client_resp + end + server_resp.success? or raise server_resp.exception_class.new(server_resp) + server_resp + end + private def validate_line(line) diff --git a/lib/net/smtp/auth_sasl_client_adapter.rb b/lib/net/smtp/auth_sasl_client_adapter.rb new file mode 100644 index 0000000..e61d951 --- /dev/null +++ b/lib/net/smtp/auth_sasl_client_adapter.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require "net/imap" + +module Net + class SMTP + SASL = Net::IMAP::SASL + + # Experimental + # + # Initialize with a block that runs a command, yielding for continuations. + class SASLClientAdapter < SASL::ClientAdapter + include SASL::ProtocolAdapters::SMTP + + RESPONSE_ERRORS = [ + SMTPAuthenticationError, + SMTPServerBusy, + SMTPSyntaxError, + SMTPFatalError, + ].freeze + + def initialize(...) + super + @command_proc ||= client.method(:send_command_with_continuations) + end + + def authenticate(...) + super + rescue SMTPServerBusy, SMTPSyntaxError, SMTPFatalError => error + raise SMTPAuthenticationError.new(error.response) + rescue SASL::AuthenticationIncomplete => error + raise error.response.exception_class.new(error.response) + end + + def host; client.address end + def response_errors; RESPONSE_ERRORS end + def sasl_ir_capable?; true end + def drop_connection; client.finish end + def drop_connection!; client.finish end + end + end +end diff --git a/lib/net/smtp/auth_sasl_compatibility_adapter.rb b/lib/net/smtp/auth_sasl_compatibility_adapter.rb new file mode 100644 index 0000000..e6b5642 --- /dev/null +++ b/lib/net/smtp/auth_sasl_compatibility_adapter.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Net + class SMTP + + # Curries arguments to SASLAdapter.authenticate. + class AuthSASLCompatibilityAdapter + def initialize(mechanism) @mechanism = mechanism end + def check_args(...) SASL.authenticator(@mechanism, ...) end + def new(smtp) @sasl_adapter = SASLClientAdapter.new(smtp); self end + def auth(...) @sasl_adapter.authenticate(@mechanism, ...) end + end + + Authenticator.auth_classes.default_proc = ->hash, mechanism { + hash[mechanism] = AuthSASLCompatibilityAdapter.new(mechanism) + } + + end +end diff --git a/net-smtp.gemspec b/net-smtp.gemspec index 8bebe8e..ea7840a 100644 --- a/net-smtp.gemspec +++ b/net-smtp.gemspec @@ -28,4 +28,6 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] spec.add_dependency "net-protocol" + + spec.add_dependency "net-imap", ">= 0.4.2" # experimental SASL support end diff --git a/test/net/smtp/test_smtp.rb b/test/net/smtp/test_smtp.rb index 69d249a..d14e7bb 100644 --- a/test/net/smtp/test_smtp.rb +++ b/test/net/smtp/test_smtp.rb @@ -545,16 +545,6 @@ def test_start_auth_cram_md5 assert_raise Net::SMTPAuthenticationError do Net::SMTP.start('localhost', port, user: 'account', password: 'password', authtype: :cram_md5){} end - - port = fake_server_start(auth: 'CRAM-MD5') - smtp = Net::SMTP.new('localhost', port) - auth_cram_md5 = Net::SMTP::AuthCramMD5.new(smtp) - auth_cram_md5.define_singleton_method(:digest_class) { raise '"openssl" or "digest" library is required' } - Net::SMTP::AuthCramMD5.define_singleton_method(:new) { |_| auth_cram_md5 } - e = assert_raise RuntimeError do - smtp.start(user: 'account', password: 'password', authtype: :cram_md5){} - end - assert_equal('"openssl" or "digest" library is required', e.message) end def test_start_instance