Skip to content

Commit

Permalink
Ensure Current.user is always up-to-date w/ Devise
Browse files Browse the repository at this point in the history
Previously, if a user was signed in or out in the *middle* of an action,
the value of `Current.user` was not updated.

This makes use of `Warden::Manager.after_set_user` [1] and
`Warden::Manager.before_logout` [2] hooks to update `Current.user`. I'm
using `Devise::Controllers::SignInOut#sign_in` [3] &
`Devise::Controllers::SignInOut#sign_out` within the tests, because I
believe that's what you're supposed to use when using `Devise` and
indeed it looks like that's what we're using in e.g.
`ConfirmationsController`, `Devise::TwoStepVerificationController` &
`Devise::TwoStepVerificationSessionController`.

I've added the `Warden::Manager` hooks within
`Devise::Hooks::SetCurrentUser` and required it from the top of
`app/controllers/application_controller.rb` to be consistent what we're
already doing with `Devise::Hooks::TwoStepVerification`, although it
seems a bit odd/non-idiomatic to execute some code via a call to
`require`. Also it strikes me as slightly odd that the hooks are in a
top-level `Devise` module despite being associated with `Warden`,
although I suppose `Warden` is only there because it's a dependency of
`Devise`.

[1]: https://www.rubydoc.info/gems/warden/1.2.9/Warden/Hooks#after_set_user-instance_method
[2]: https://www.rubydoc.info/gems/warden/1.2.9/Warden/Hooks#before_logout-instance_method
[3]: https://www.rubydoc.info/github/plataformatec/devise/Devise%2FControllers%2FSignInOut:sign_in
[4]: https://www.rubydoc.info/github/plataformatec/devise/Devise%2FControllers%2FSignInOut:sign_out
  • Loading branch information
floehopper committed Feb 21, 2024
1 parent bbbd872 commit 7439bb7
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 23 deletions.
1 change: 1 addition & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "devise/hooks/two_step_verification"
require "devise/hooks/set_current_user"

class ApplicationController < ActionController::Base
include Pundit::Authorization
Expand Down
8 changes: 8 additions & 0 deletions lib/devise/hooks/set_current_user.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Devise::Hooks::SetCurrentUser
Warden::Manager.after_set_user(only: :set_user) do |user, _auth, _options|
Current.user = user
end
Warden::Manager.before_logout do |_user, _auth, _options|
Current.user = nil
end
end
97 changes: 74 additions & 23 deletions test/integration/set_current_attributes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,65 +2,116 @@

class SetCurrentAttributesTest < ActionDispatch::IntegrationTest
class TestsController < ApplicationController
cattr_accessor :current_user
cattr_accessor :user, :signed_in_outside_action, :users, :user_ip
skip_after_action :verify_authorized

def show
render json: { user_id: Current.user&.id, user_ip: Current.user_ip }
self.class.users[:within_action] = Current.user
self.user_ip = Current.user_ip

head :ok
end

def signing_in_within_action
self.class.users[:before_signing_in] = Current.user
sign_in(self.class.user)
self.class.users[:after_signing_in] = Current.user

head :ok
end

def signing_out_within_action
self.class.users[:before_signing_out] = Current.user
sign_out
self.class.users[:after_signing_out] = Current.user

head :ok
end

private

def current_user
self.class.current_user
self.class.signed_in_outside_action ? self.class.user : super
end
end

should "set Current.user if user is signed in" do
with_test_route do
setup do
TestsController.user = nil
TestsController.signed_in_outside_action = false
TestsController.users = {}
TestsController.user_ip = nil
end

should "set Current.user if user is signed in outside action" do
with_test_routes do
user = create(:user)
with_current_user(user) do
visit "/test"
TestsController.signed_in_outside_action = true
TestsController.user = user
visit "/show"

assert_equal user.id, JSON.parse(page.body)["user_id"]
end
assert_equal user, TestsController.users[:within_action]
end
end

should "not set Current.user if user is not signed in" do
with_test_route do
with_current_user(nil) do
visit "/test"
should "not set Current.user if user is not signed in outside action" do
with_test_routes do
TestsController.signed_in_outside_action = true
TestsController.user = nil
visit "/show"

assert_nil JSON.parse(page.body)["user_id"]
end
assert_nil TestsController.users[:within_action]
end
end

should "set Current.user_ip" do
with_test_route do
with_test_routes do
page.driver.options[:headers] = { "REMOTE_ADDR" => "4.5.6.7" }
visit "/test"
visit "/show"

assert_equal "4.5.6.7", TestsController.user_ip
end
end

should "set Current.user if user signs in within action" do
with_test_routes do
user = create(:user)
TestsController.user = user
visit "/signing_in_within_action"

assert_nil TestsController.users[:before_signing_in]
assert_equal user, TestsController.users[:after_signing_in]
end
end

should "unset Current.user if user signs out within action" do
with_test_routes do
user = create(:user)
TestsController.signed_in_outside_action = true
TestsController.user = user
visit "/signing_out_within_action"

assert_equal "4.5.6.7", JSON.parse(page.body)["user_ip"]
assert_equal user, TestsController.users[:before_signing_out]
assert_nil TestsController.users[:after_signing_out]
end
end

private

def with_test_route
def with_test_routes
Rails.application.routes.draw do
get "/test" => "set_current_attributes_test/tests#show"
get "/show" => "set_current_attributes_test/tests#show"
get "/signing_in_within_action" => "set_current_attributes_test/tests#signing_in_within_action"
get "/signing_out_within_action" => "set_current_attributes_test/tests#signing_out_within_action"
end
yield
ensure
Rails.application.reload_routes!
end

def with_current_user(user)
TestsController.current_user = user
def with_user(user)
TestsController.user = user
yield
ensure
TestsController.current_user = nil
TestsController.user = nil
end
end

0 comments on commit 7439bb7

Please sign in to comment.