Fixed LDAP crash and improved tests

lib/portus/ldap/authenticatable.rb

@@ -30,6 +30,7 @@ module LDAP
     # file. Take a look at this file in order to read more on the different
     # configurable values.
     class Authenticatable < Devise::Strategies::Authenticatable
+      include ::Portus::LDAP::Adapter
       include ::Portus::LDAP::Connection
       include ::Portus::LDAP::Errors
       include ::Portus::LDAP::Login
@@ -38,8 +39,11 @@ class Authenticatable < Devise::Strategies::Authenticatable
       # the user.
       def authenticate!
         fill_user_params!
-        cfg = ::Portus::LDAP::Configuration.new(params)
-        portus_login!(cfg) if bind_as(cfg)
+
+        cfg        = ::Portus::LDAP::Configuration.new(params)
+        connection = initialized_adapter
+
+        portus_login!(connection, cfg) if bind_as(connection, cfg)
       rescue ::Portus::LDAP::Error, Net::LDAP::Error => e
         logged_failure!(e.message)
       end

lib/portus/ldap/connection.rb

@@ -7,23 +7,20 @@ module LDAP
     # Connection holds a set of methods which are responsible for binding to the
     # LDAP server by considering the given configuration.
     module Connection
-      include ::Portus::LDAP::Adapter
-
-      # Bind to the LDAP server with the given configuration. Returns true if
-      # everything went alright, false otherwise. On failure it will also log
-      # the error and call `fail!`.
+      # Bind to the LDAP server with the given configuration by using the given
+      # connection. Returns true if everything went alright, false otherwise. On
+      # failure it will also log the error and call `fail!`.
       #
       # It will raise a ::Portus::LDAP::Error exception if the given
       # configuration is incomplete (e.g. missing parameters) or LDAP is
       # disabled. It might also raise a Net::LDAP::Error since in the end this
       # method calls `#bind_as` from Net::LDAP.
-      def bind_as(cfg)
+      def bind_as(connection, cfg)
         raise ::Portus::LDAP::Error, "LDAP is disabled" unless cfg.enabled?
         raise ::Portus::LDAP::Error, "Some parameters are missing" unless cfg.initialized?
 
-        cn  = initialized_adapter
-        res = cn.bind_as(bind_options(cfg))
-        logged_error_message!(cn, cfg.username) unless res
+        res = connection.bind_as(bind_options(cfg))
+        logged_error_message!(connection, cfg.username) unless res
         res
       end
 

lib/portus/ldap/login.rb

@@ -9,8 +9,8 @@ module Login
       # Fetch the user assumed from `cfg` and log it. If the user does not
       # exist yet, it will be created and the `session[:first_login]` value will
       # be set to true, so the sessions controller can act accordingly.
-      def portus_login!(cfg)
-        user, created = find_or_create_user!(cfg)
+      def portus_login!(connection, cfg)
+        user, created = find_or_create_user!(connection, cfg)
         if user.valid?
           session[:first_login] = true if created
           success!(user)
@@ -24,14 +24,14 @@ def portus_login!(cfg)
       # Retrieve the given user as an LDAP user. If it doesn't exist, create it
       # with the parameters given in `cfg`. Returns two objects: the user object
       # and a boolean set to true if the returned user was just created.
-      def find_or_create_user!(cfg)
+      def find_or_create_user!(connection, cfg)
         user = User.find_by(username: cfg.username)
         created = false
 
         # The user does not exist in Portus yet, let's create it.
         unless user
-          em = guess_email(cfg)
-          em = nil if User.exists?(email: em)
+          em = guess_email(connection, cfg)
+          em = nil if em && User.exists?(email: em)
 
           user = User.create(
             username: cfg.username,
@@ -47,12 +47,12 @@ def find_or_create_user!(cfg)
       # If the "ldap.guess_email" option is enabled, try to guess the email for
       # the user as specified in the configuration. Returns an nil if nothing
       # could be guessed.
-      def guess_email(configuration)
+      def guess_email(connection, configuration)
         cfg = APP_CONFIG["ldap"]["guess_email"]
-        return nil if cfg.nil? || !cfg["enabled"]
+        return if cfg.nil? || !cfg["enabled"]
 
-        record = @ldap.search(search_options(configuration))
-        return nil if record.size != 1
+        record = connection.search(search_options(configuration))
+        return if record&.size != 1
         record = record.first
 
         if cfg["attr"].empty?