Open WozniakMac opened 1 year ago
I'm having this issue as well. I wanted to prevent users that have a certain flag from signing in and just redirected them instead. I'm noticing two things:
The ApplicationController
is hit before the User::SessionsController
is. When I place a debugger in a before action inside of application controller, the user is already signed in.
The same issue you're seeing where the user is signed in prior to hitting the create
action.
I would love to hear from other people or the team as to why they might think this is happening.
+1
https://github.com/heartcombo/devise/issues/4951
Figured this out @WozniakMac and @JoshingYou1. If you put this code in your application_controller or sessions_controller, it won't sign in. The Devise team decided that calling current_user
needs to sign in the user circumventing anything you'd do in the create method. Caused a vulnerability for me, hopefully you caught it before anything like that.
def current_user
super if warden.authenticated?(scope: :user)
end
This is still hacky IMO, I'd love to hear from the team on something better for this. Or at least some updated docs...
You're correct on the implementation of current_#{mapping}
- ref https://github.com/heartcombo/devise/blob/main/lib/devise/controllers/helpers.rb#L127
However, it's been that way for 10 years, so it may be a conscious design decision.
I haven't tested this, but if you do this in your devise controller:
prepend_before_action -> { warden.lock! }, only: %i[create]
it will hopefully do what you're expecting, as long as it's ending up early enough in the request chain. warden#lock!
disables any further authentication strategies for the current request, but doesn't touch existing sessions.
If you need to access warden
outside of a subclass of Devise::Controller
then request.env['warden']
will return the same value
I mean saying "its been that way for 10 years" isn't exactly the mindset I'd want for my authentication code....
Things should be updated to make sense to the conventions and what the developers are using today. Additionally, they never changed the documentation from that issue. For example, the docs still say you can overwrite the create method in the sessions controller to create a custom sign in, which is not true at all: https://github.com/heartcombo/devise#configuring-controllers
Yeah, that example in the README has also been around awhile. My recommendation would be to put together a PR with a test that fails with the current code, and change the implementation to something such as
def current_#{mapping}
@current_#{mapping} ||= warden.authenticated?(scope: :#{mapping}) ? nil : warden.authenticate(scope: :#{mapping})
end
haven't tested but my guess is that this should retain the existing behaviour while also satisfying your needs - and arguably being a slightly better design due to the lack of unintended side effect.
Environment
Current behavior
If I overwrite Devise::SessionsController#create in custom controller to not sign in user and just redirect it still signs in the user if login and password are correct. Confirmed on 2 different apps. I think it could be related to implementation of
current_user
that callswarden.authenticate
Expected behavior
It should not sign in the user