Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#sign_out method needs patching? #24

Open
stanislaw opened this issue May 16, 2011 · 2 comments
Open

#sign_out method needs patching? #24

stanislaw opened this issue May 16, 2011 · 2 comments

Comments

@stanislaw
Copy link

Hello.

Today, after I installed cream (before that I had only roles_generic and roles_active_record), request specs, beginning with visit 'users/sign_out' (very common for integration tests (rspec requests and cucumber)), started to fail with ArgumentError: wrong number of arguments (0 for 1).

Further exploration led me to
cream-0.9.2/lib/cream/controller/user_control.rb:71 --

sign_out method requires (resource_or_scope) argument. And if i just visit 'users/sign_out' during request test -- resource_or_scope is nil.

To reproduce -- simply begin any request test with visit 'users/sign_out' (assuming that Devise and it is default routes are used).

The same behaviour in real-user experience I can get, if i press sign_out button (or visit 'localhost:8080/users/sign_out') at the very first request after webserver started.

No pull request at this time. I'm not familliar with what warden does so not sure is it possible to simply make it like this:

def sign_out(resource_or_scope = nil)
  session[:user_id] = nil
  @current_user = nil      
  return unless resource_or_scope
  scope = Devise::Mapping.find_scope!(resource_or_scope)
  warden.user(scope) # Without loading user here, before_logout hook is not called
  warden.raw_session.inspect # Without this inspect here. The session does not clear.
  warden.logout(scope)
end  
@stanislaw
Copy link
Author

The patch is indeed needed to make webrat work.
But having the code done as I described above led me to stupid bugs in production mode (session and @current_user = nil are both should be after warden.logoout(scope) line for sign_out being made properly).
The key point is that cream's #sign_out method should have a possibility to run without argument.
I think changing "resource_or_scope" to "resource_or_scope = nil" in args brackets should be enough.

  def sign_out(resource_or_scope = nil)
    return unless resource_or_scope # nothing to do when calling sign_out without resource_or_scope
    scope = Devise::Mapping.find_scope!(resource_or_scope)
    warden.user(scope) # Without loading user here, before_logout hook is not called
    warden.raw_session.inspect # Without this inspect here. The session does not clear.
    warden.logout(scope)
    # user id
    session[:user_id] = nil
    @current_user = nil      
  end  

upd:
One more argument:
Devise has configuration option config.sign_out_all_scopes = false
Setting it to true will lead to the same error. #sign_out method in cream doesn't now handles this possibility (all_scope instead of default :user scope) and fails with similar error (0 for 1 arguments).

@kristianmandrup
Copy link
Owner

"Devise has configuration option config.sign_out_all_scopes = false. Setting it to true will lead to the same error."

Unfortunately I don't know enough about the Devise/Warden internals to know how to handle setting such an option. Would be better if we ask such questions on the Devise or Warden user group list IMO ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants