-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Logout path should be configurable #1386
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
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: