Devise: allow_params_authentication! authenticates the user too

[abdelbk]

I'm facing the same issue described here https://github.com/plataformatec/devise/issues/4285.

I'm running the 4.5.0 of Devise.

Basically, the user is authenticated even when overriding the create method. I don't know if it's the intended behavior, but allow_params_authentication! prevents any authentication customizations.

I can skip this action, but I do believe that there is a deeper issue. The allow_params_authentication! does more than what its name describes.

[tegon]

Hi @abdelbk, thanks for your report.

I'm not sure I followed what the issue is though. This method is only setting a value in the Rack request environment that will later be used inside the Warden strategy to check whether the request is valid - i.e. to decide if the strategy should run.

If can share a piece of your code explaining what you're trying to achieve I think it would help me understand what's going on.

[abdelbk]

My code is roughly similar to this

class SessionsController < Devise::SessionsController
  def create
    # Rails.logger.info user_signed_in? returns true before running the create method
    # Custom authentication logic which gets the user by email and verify the password
  end
end

The issue is that the current_user is set before entering the create method. So to debug, I listed all the before_action callbacks which are ran before the create method. I found that the allow_params_authentication! causes the authentication of the user. I can't tell you why because, as you said, it just sets an env variable.

Adding skip_before_action allow_params_authentication! makes the issue disappear.

I hope I've been clear.

[tegon]

Thanks for clarifying that. I know what's happening now: the user is being authenticated when you call user_signed_in?. It turns out that internally, this method is delegating the call to Warden#authenticate? - which not only returns a boolean indicating whether the user is authenticated or not but also runs the Warden strategies if it isn't.
This results in the user being signed in when the sign in parameters are valid.

The correct Warden method that only checks if the user is authenticated or not is #authenticated? - with a d in the end, the difference is very subtle.

I've copied the output below from my terminal, I think it demonstrates better what I'm trying to say:

Started POST "/users/sign_in" for 127.0.0.1 at 2019-01-30 19:42:21 -0200
Processing by Users::SessionsController#create as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"rbEvXe73v7i+Mknb0c9vAd4vKKplLPPBJfR0WWeRcIoNRY7r1hoivaJERJl4wCZzu+/UoA59tebZ/gR5HaNwZw==", "user"=>{"email"=>"[email protected]", "password"=>"[FILTERED]", "remember_me"=>"0"}, "commit"=>"Log in"}
Return value is: nil

[9, 18] in /Users/tegon/src/public/allow_params_auth/app/controllers/users/sessions_controller.rb
    9:   # end
   10:
   11:   # POST /resource/sign_in
   12:   def create
   13:     byebug
=> 14:   end
   15:
   16:   # DELETE /resource/sign_out
   17:   # def destroy
   18:   #   super
(byebug) warden.authenticated?(:user)
false
(byebug) user_signed_in?
  User Load (0.6ms)  SELECT  "users".* FROM "users" WHERE "users"."email" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["email", "[email protected]"], ["LIMIT", 1]]
  ↳ (byebug):1
true
(byebug) warden.authenticated?(:user)
true

The reason why calling skip_before_action allow_params_authentication! works is because of what I mentioned in a previous comment: since the Warden strategy checks this value to know whether it should run or not, skipping the before action prevents the strategy from running.

That being said I think we should change the behavior of user_signed_in? to not authenticate the user. Unfortunately, since this is public API we'll have to release it in the next major version since there might be some code out there relying on this behavior already.

[abdelbk]

Thank you for your response. The issue is also discussed here https://github.com/plataformatec/devise/issues/4951. It's all clear now.

[JanBussieck]

Is someone already working on this? If not I'm happy to open an PR in the next couple of days.

[tegon]

@JanBussieck Not yet, feel free to grab it.

[vincentwoo]

yo @tegon are you gonna fix this

[mracos]

Closed via https://github.com/plataformatec/devise/commit/4be9389dcb791b063825b4996df8d04cbf7e0e89