Merge pull request #580 from binarylogic/require_hash_for_credentials

Remove support for ActionController::Parameters

CHANGELOG.md

@@ -2,13 +2,17 @@
 
 ## 4.0.0 Unreleased
 
-* Breaking Changes
+* Breaking Changes, Major
   * Drop support for ruby < 2.2
   * Drop support for rails < 4.2
   * HTTP Basic Auth is now disabled by default (use allow_http_basic_auth to enable)
   * 'httponly' and 'secure' cookie options are enabled by default now
   * maintain_sessions config has been removed. It has been split into 2 new options:
     log_in_after_create & log_in_after_password_change (@lucasminissale)
+  * [#558](https://github.com/binarylogic/authlogic/pull/558) Passing an
+    ActionController::Parameters into authlogic will now raise an error
+
+* Breaking Changes, Minor
   * Methods in Authlogic::Random are now module methods, and are no longer
     instance methods. Previously, there were both. Do not use Authlogic::Random
     as a mixin.

lib/authlogic/session/foundation.rb

@@ -12,6 +12,37 @@ def self.included(klass)
       end
 
       module InstanceMethods
+        E_AC_PARAMETERS = <<-EOS.strip_heredoc.freeze
+          Passing an ActionController::Parameters to Authlogic is not allowed.
+
+          In Authlogic 3, especially during the transition of rails to Strong
+          Parameters, it was common for Authlogic users to forget to `permit`
+          their params. They would pass their params into Authlogic, we'd call
+          `to_h`, and they'd be surprised when authentication failed.
+
+          In 2018, people are still making this mistake. We'd like to help them
+          and make authlogic a little simpler at the same time, so in Authlogic
+          3.7.0, we deprecated the use of ActionController::Parameters. Instead,
+          pass a plain Hash. Please replace:
+
+              UserSession.new(user_session_params)
+              UserSession.create(user_session_params)
+
+          with
+
+              UserSession.new(user_session_params.to_h)
+              UserSession.create(user_session_params.to_h)
+
+          And don't forget to `permit`!
+
+          We discussed this issue thoroughly between late 2016 and early
+          2018. Notable discussions include:
+
+          - https://github.com/binarylogic/authlogic/issues/512
+          - https://github.com/binarylogic/authlogic/pull/558
+          - https://github.com/binarylogic/authlogic/pull/577
+        EOS
+
         def initialize(*args)
           self.credentials = args
         end
@@ -27,7 +58,10 @@ def credentials
         #
         #   session.credentials = {:login => "my login", :password => "my password", :remember_me => true}
         #
-        # or you can pass an array of objects:
+        # You must pass an actual Hash, `ActionController::Parameters` is
+        # specifically not allowed.
+        #
+        # You can pass an array of objects:
         #
         #   session.credentials = [my_user_object, true]
         #
@@ -38,6 +72,10 @@ def credentials
         #   session.credentials = [{:login => "my login", :password => "my password", :remember_me => true}, :my_id]
         #   session.credentials = [my_user_object, true, :my_id]
         def credentials=(values)
+          normalized = Array.wrap(values)
+          if normalized.first.class.name == "ActionController::Parameters"
+            raise TypeError.new(E_AC_PARAMETERS)
+          end
         end
 
         def inspect

lib/authlogic/session/password.rb

@@ -152,10 +152,15 @@ def credentials
 
         # Accepts the login_field / password_field credentials combination in
         # hash form.
+        #
+        # You must pass an actual Hash, `ActionController::Parameters` is
+        # specifically not allowed.
+        #
+        # See `Authlogic::Session::Foundation#credentials=` for an overview of
+        # all method signatures.
         def credentials=(value)
           super
-          values = parse_param_val(value) # add strong parameters check
-
+          values = Array.wrap(value)
           if values.first.is_a?(Hash)
             values.first.with_indifferent_access.slice(login_field, password_field).each do |field, val|
               next if val.blank?
@@ -270,17 +275,6 @@ def password_field
           def verify_password_method
             self.class.verify_password_method
           end
-
-          # In Rails 5 the ActionController::Parameters no longer inherits from
-          # HashWithIndifferentAccess. (http://bit.ly/2gnK08F) This method
-          # converts the ActionController::Parameters to a Hash.
-          def parse_param_val(value)
-            if value.first.class.name == "ActionController::Parameters"
-              [value.first.to_h]
-            else
-              Array.wrap(value)
-            end
-          end
       end
     end
   end

test/session_test/foundation_test.rb

@@ -1,6 +1,22 @@
 require 'test_helper'
 
+# We forbid the use of AC::Parameters, and we have a test to that effect, but we
+# do not want a development dependency on `actionpack`, so we define it here.
+module ActionController
+  class Parameters; end
+end
+
 module SessionTest
   class FoundationTest < ActiveSupport::TestCase
+    def test_credentials_raise_if_not_a_hash
+      session = UserSession.new
+      e = assert_raises(TypeError) {
+        session.credentials = ActionController::Parameters.new
+      }
+      assert_equal(
+        ::Authlogic::Session::Foundation::InstanceMethods::E_AC_PARAMETERS,
+        e.message
+      )
+    end
   end
 end