-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Logout path should be configurable #1386
Comments
Sure, I would add 2 options:
And use them in a view helper method. Just add a couple of unit tests in spec/helpers/application_helper_spec and you will be done. |
I use RailsAdmin with plain OmniAuth and it will be very good, if I can show Exit button wothout devise. Maybe some |
Deduce Devise scope from _current user. Fixes #1386, partly.
Logout still need Devise, because |
@pehlert @bbenezech @ai I just made PR #2062 with a fix for the logout_method. It is not configurable yet, but there is a fallback to a default :delete if devise is not defined. Also, logout_path is also kind of configurable (not by a rails_admin config though). If you have a route that defines logout_path for instance: delete 'sign_out', :to => 'sessions#destroy', as: 'logout' I will just work. |
This should be reopened, as def logout_method
return [Devise.sign_out_via].flatten.first if defined?(Devise)
:delete
end |
Currently, the logout button uses the default scope from warden.
While this may work in most of the setups, I am currently using rails_admin together with devise in an application that has the "sign_out_all_scopes" configuration option of devise set to false.
Thus in a setup where the scope used by rails_admin (e.g. "admin" in my case) and where this configuration option is set to false, the logout link will not work, but instead sign out a wrong scope.
While I could surely modify it, I wanted to provide the chance for some discussion in advance as it involves adding a new configuration option.
Would it be preferable to have something like "devise_scope" that could be reused in other places, or do you prefer "destroy_session_path" which is more specific to this issue (and has been proposed before, although for another reason: #788)?
The text was updated successfully, but these errors were encountered: