🔒 Enforce LOGINDISABLED requirement

This may be considered a "breaking change", but it should have no
negative effect on well behaved servers.  This should merely change a
NoResponseError into a LoginDisabledError.

However, some broken servers have been known to hang indefinitely when
issued a `CAPABILITY` command prior to authentication.  For those
servers, wo offer the `enforce_logindisabled` config option.

Fixes #32.

Author: nevans

lib/net/imap.rb

@@ -1378,20 +1378,19 @@ def authenticate(mechanism, *creds,
     # ===== Capabilities
     #
     # An IMAP client MUST NOT call #login when the server advertises the
-    # +LOGINDISABLED+ capability.
-    #
-    #    if imap.capability? "LOGINDISABLED"
-    #      raise "Remote server has disabled the login command"
-    #    else
-    #      imap.login username, password
-    #    end
+    # +LOGINDISABLED+ capability.  By default, Net::IMAP will raise a
+    # LoginDisabledError when that capability is present.  See
+    # Config#enforce_logindisabled.
     #
     # Server capabilities may change after #starttls, #login, and #authenticate.
     # Cached capabilities _must_ be invalidated after this method completes.
     # The TaggedResponse to #login may include updated capabilities in its
     # ResponseCode.
     #
     def login(user, password)
+      if enforce_logindisabled? && capability?("LOGINDISABLED")
+        raise LoginDisabledError
+      end
       send_command("LOGIN", user, password)
         .tap { @capabilities = capabilities_from_resp_code _1 }
     end
@@ -2869,6 +2868,14 @@ def put_string(str)
       end
     end
 
+    def enforce_logindisabled?
+      if config.enforce_logindisabled == :when_capabilities_cached
+        capabilities_cached?
+      else
+        config.enforce_logindisabled
+      end
+    end
+
     def search_internal(cmd, keys, charset)
       if keys.instance_of?(String)
         keys = [RawData.new(keys)]

lib/net/imap/config.rb

@@ -213,6 +213,32 @@ def self.[](config)
       # | v0.4                  | +true+  <em>(support added)</em>         |
       attr_accessor :sasl_ir, type: :boolean
 
+      # :markup: markdown
+      #
+      # Controls the behavior of Net::IMAP#login when the `LOGINDISABLED`
+      # capability is present.  When enforced, Net::IMAP will raise a
+      # LoginDisabledError when that capability is present.  Valid values are:
+      #
+      # [+false+]
+      #   Send the +LOGIN+ command without checking for +LOGINDISABLED+.
+      #
+      # [+:when_capabilities_cached+]
+      #   Enforce the requirement when Net::IMAP#capabilities_cached? is true,
+      #   but do not send a +CAPABILITY+ command to discover the capabilities.
+      #
+      # [+true+]
+      #   Only send the +LOGIN+ command if the +LOGINDISABLED+ capability is not
+      #   present.  When capabilities are unknown, Net::IMAP will automatically
+      #   send a +CAPABILITY+ command first before sending +LOGIN+.
+      #
+      # | Starting with version   | The default value is           |
+      # |-------------------------|--------------------------------|
+      # | _original_              | `false`                        |
+      # | v0.5                    | `true`                         |
+      attr_accessor :enforce_logindisabled, type: [
+        false, :when_capabilities_cached, true
+      ]
+
       # :markup: markdown
       #
       # Controls the behavior of Net::IMAP#responses when called without a
@@ -306,6 +332,7 @@ def defaults_hash
         open_timeout: 30,
         idle_response_timeout: 5,
         sasl_ir: true,
+        enforce_logindisabled: true,
         responses_without_block: :warn,
       ).freeze
 
@@ -317,6 +344,7 @@ def defaults_hash
       version_defaults[0] = Config[:current].dup.update(
         sasl_ir: false,
         responses_without_block: :silence_deprecation_warning,
+        enforce_logindisabled: false,
       ).freeze
       version_defaults[0.0] = Config[0]
       version_defaults[0.1] = Config[0]

lib/net/imap/errors.rb

@@ -7,6 +7,12 @@ class IMAP < Protocol
     class Error < StandardError
     end
 
+    class LoginDisabledError < Error
+      def initialize(msg = "Remote server has disabled the LOGIN command", ...)
+        super
+      end
+    end
+
     # Error raised when data is in the incorrect format.
     class DataFormatError < Error
     end

test/net/imap/test_imap_login.rb

@@ -0,0 +1,108 @@
+# frozen_string_literal: true
+
+require "net/imap"
+require "test/unit"
+require_relative "fake_server"
+
+class IMAPLoginTest < Test::Unit::TestCase
+  include Net::IMAP::FakeServer::TestHelper
+
+  def setup
+    Net::IMAP.config.reset
+    @do_not_reverse_lookup = Socket.do_not_reverse_lookup
+    Socket.do_not_reverse_lookup = true
+    @threads = []
+  end
+
+  def teardown
+    if !@threads.empty?
+      assert_join_threads(@threads)
+    end
+  ensure
+    Socket.do_not_reverse_lookup = @do_not_reverse_lookup
+  end
+
+  test "#login raises LoginDisabledError when LOGINDISABLED" do
+    with_fake_server(preauth: false, cleartext_login: false) do |server, imap|
+      assert imap.capabilities_cached?
+      assert_raise(Net::IMAP::LoginDisabledError) do
+        imap.login("test_user", "test-password")
+      end
+      assert_empty server.commands
+    end
+  end
+
+  test "#login first checks capabilities for LOGINDISABLED (success)" do
+    with_fake_server(
+      preauth: false, cleartext_login: true, greeting_capabilities: false
+    ) do |server, imap|
+      imap.login("test_user", "test-password")
+      cmd = server.commands.pop
+      assert_equal "CAPABILITY", cmd.name
+      cmd = server.commands.pop
+      assert_equal "LOGIN", cmd.name
+      assert_empty server.commands
+    end
+  end
+
+  test "#login first checks capabilities for LOGINDISABLED (failure)" do
+    with_fake_server(
+      preauth: false, cleartext_login: false, greeting_capabilities: false
+    ) do |server, imap|
+      assert_raise(Net::IMAP::LoginDisabledError) do
+        imap.login("test_user", "test-password")
+      end
+      cmd = server.commands.pop
+      assert_equal "CAPABILITY", cmd.name
+      assert_empty server.commands
+    end
+  end
+
+  test("#login sends LOGIN without asking CAPABILITY " \
+       "when config.enforce_logindisabled is false") do
+    with_fake_server(
+      preauth: false, cleartext_login: false, greeting_capabilities: false
+    ) do |server, imap|
+      imap.config.enforce_logindisabled = false
+      imap.login("test_user", "test-password")
+      cmd = server.commands.pop
+      assert_equal "LOGIN", cmd.name
+    end
+  end
+
+  test("#login raises LoginDisabledError without sending CAPABILITY " \
+       "when config.enforce_logindisabled is :when_capabilities_cached") do
+    with_fake_server(
+      preauth: false, cleartext_login: false, greeting_capabilities: true
+    ) do |server, imap|
+      imap.config.enforce_logindisabled = :when_capabilities_cached
+      assert_raise(Net::IMAP::LoginDisabledError) do
+        imap.login("test_user", "test-password")
+      end
+      assert_empty server.commands
+    end
+  end
+
+  test("#login sends LOGIN without asking CAPABILITY " \
+       "when config.enforce_logindisabled is :when_capabilities_cached") do
+    with_fake_server(
+      preauth: false, cleartext_login: false, greeting_capabilities: false
+    ) do |server, imap|
+      imap.config.enforce_logindisabled = :when_capabilities_cached
+      imap.login("test_user", "test-password")
+      cmd = server.commands.pop
+      assert_equal "LOGIN", cmd.name
+      assert_empty server.commands
+    end
+    with_fake_server(
+      preauth: false, cleartext_login: true, greeting_capabilities: true
+    ) do |server, imap|
+      imap.config.enforce_logindisabled = :when_capabilities_cached
+      imap.login("test_user", "test-password")
+      cmd = server.commands.pop
+      assert_equal "LOGIN", cmd.name
+      assert_empty server.commands
+    end
+  end
+
+end