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

Rails 7: logout redirect not working #5458

Closed
collimarco opened this issue Jan 17, 2022 · 32 comments · Fixed by #5548
Closed

Rails 7: logout redirect not working #5458

collimarco opened this issue Jan 17, 2022 · 32 comments · Fixed by #5548

Comments

@collimarco
Copy link

Set this in application_controller.rb:

  def after_sign_out_path_for scope
    root_path
  end

You expect the logout to redirect the user to root.

However Devise returns a redirect with status 302, which does not work with Turbo.

All redirects must return 303 (i.e. status: :see_other) in order to work with Turbo.

This should be fixed in Devise in order to work with Rails 7.

Related: rails/rails#44170

@collimarco
Copy link
Author

Not only the redirect for logout, but also the other redirects should use the status 303 in order to work with Turbo.

(The only other alternative would be to make Turbo accept the 302, but from the related issue linked above it seems that Rails team won't fix that)

@collimarco
Copy link
Author

Also note that by using 302, after logout response, you also get an "extra" DELETE request to root path, which results in a 404.

This is quite scary, since you get an unexpected DELETE request on random path!

@marcelolx
Copy link

I think this is related to #5453

While the PR isn't merged I would just disable turbo on devise views

@designium
Copy link

You have to change from:

<%= link_to "Sign out", destroy_user_session_path, method: delete %>

to

<%= link_to "Sign out", destroy_user_session_path, data: { turbo_method: :delete" } %>

@collimarco
Copy link
Author

@designium Yes, sure, I already use the correct code for Turbo... However there is the bug described in the original post.

@rzepetor
Copy link

rzepetor commented Jan 20, 2022

@collimarco I have found simple hotfix

# app/controllers/sessions_controller.rb
class SessionsController < Devise::SessionsController
  def respond_to_on_destroy
    respond_to do |format|
      format.all { head :no_content }
      format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name), status: :see_other}
    end
  end
end
# config/routes.rb
Rails.application.routes.draw do
  # ...
  devise_for :users, controllers: { sessions: 'sessions' }
  # ...
end

You just need add status: :see_other (303) to redirect_to method arguments to make it works correctly

@mattbrictson
Copy link
Contributor

I fixed this as follows:

<%= link_to("Log out", destroy_user_session_path, method: :delete, data: { turbo_method: :delete }) %>
module Users
  class SessionsController < Devise::SessionsController
    def destroy
      super do
        # Turbo requires redirects be :see_other (303); so override Devise default (302)
        return redirect_to after_sign_out_path_for(resource_name), status: :see_other
      end
    end
  end
end

@MattBudz
Copy link

MattBudz commented Feb 4, 2022

@mattbrictson you can also add notice: I18n.t('devise.sessions.signed_out') to show the successful sign out message

return redirect_to after_sign_out_path_for(resource_name), status: :see_other, notice: I18n.t('devise.sessions.signed_out')

@spaquet
Copy link

spaquet commented Feb 21, 2022

I just notice that when you are using a button to log out:
<%= button_to "Log Out", destroy_user_session_path, method: :delete %> the the redirect works as expected and the user is taken to the root_path.
However as soon as you are using a link things are not working (mostly due to the status issue mentioned above).

@spaquet
Copy link

spaquet commented Feb 21, 2022

Also, overriding the after_sign_out_path_for(resource_or_scope) method in the ApplicationController does not work when using a link... so there is no way to fix this issue without first fixing the status problem.

@francesco-loreti
Copy link

any news?

@spaquet
Copy link

spaquet commented May 27, 2022

nope...

@stevecondylios
Copy link

stevecondylios commented Jun 9, 2022

This worked for me initially:

<%= link_to "Logout", destroy_user_session_path, data: { turbo_method: :delete } %>

But after making these turbo changes (to fix an unrelated issue of some bootstrap javascripts not working) from:

// app/javascript/application.js
import "@hotwired/turbo-rails"

to this

// app/javascript/application.js
import { Turbo } from "@hotwired/turbo-rails"
Turbo.session.drive = false

then logout broke.

However, this worked:

<%= button_to "Log out", destroy_user_session_path, method: :delete, data: { turbo: false } %>

Hope it's useful.

@kluzny
Copy link

kluzny commented Jul 2, 2022

I ended up with this which makes a mini form:

<%= button_to "Logout", destroy_user_session_path, method: :delete, form: {  data: { turbo: :false } } %>

@sergiy-koshoviy
Copy link

It works for me
<%= button_to 'Logout', destroy_user_session_path, method: :delete, form: {turbolink: false} %>

@Genkilabs
Copy link

Looking at the patch by @mattbrictson I see it looks really similar to the PR here #5410

@danielricecodes
Copy link

danielricecodes commented Sep 3, 2022

Disabling Turbo is the wrong answer here. This issue is specific to an incorrect redirect being returned by Devise::SessionController.

My Selenium tests fail like this:

2) Authentication as a User allows a user to Log In and Out
     Failure/Error: raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}"
     
     ActionController::RoutingError:
       No route matches [DELETE] "/"

The Rails log will follow a pattern similar to this:

Started DELETE "/users/sign_out" for 127.0.0.1 at 2022-09-02 17:00:43 -0600
Processing by Devise::SessionsController#destroy as TURBO_STREAM
  User Load (1.6ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["id", 1], ["LIMIT", 1]]
Redirected to http://127.0.0.1:57460/
Completed 302 Found in 10ms (ActiveRecord: 1.6ms | Allocations: 1787)
Started DELETE "/" for 127.0.0.1 at 2022-09-02 17:00:43 -0600

The issue is that Turbo is attempting to render after_sign_out_path with the incorrect HTTP Verb (DELETE, in this case). You want your Devise controller to respond with HTTP 303 See Other. Solutions that override Devise's default behavior, such as @mattbrictson's, should be used.

I fixed this as follows:

<%= link_to("Log out", destroy_user_session_path, method: :delete, data: { turbo_method: :delete }) %>
module Users
  class SessionsController < Devise::SessionsController
    def destroy
      super do
        # Turbo requires redirects be :see_other (303); so override Devise default (302)
        return redirect_to after_sign_out_path_for(resource_name), status: :see_other
      end
    end
  end
end

Don't forget to update your routes, too:

  devise_for :users, controllers: {
    sessions: 'users/sessions'
  }

None of the other replies actually fix the OP's original issue.

@d33pjs
Copy link

d33pjs commented Sep 11, 2022

module Users
  class SessionsController < Devise::SessionsController
    def destroy
      super do
        # Turbo requires redirects be :see_other (303); so override Devise default (302)
        return redirect_to after_sign_out_path_for(resource_name), status: :see_other
      end
    end
  end
end

I'm sorry, can anyone - maybe @mattbrictson or @danielricecodes - tell me, where to put that?

Thanks in advance.

@olivier-thatch
Copy link

olivier-thatch commented Sep 15, 2022

This issue is not specific to Turbo, it's how the Fetch API behaves: when the HTTP status is 302, the browser will send another request to the redirect URL using the same HTTP method as the initial request. Turbo wraps the Fetch API, but I ran into the same issue in our app which doesn't use Turbo.

In our case we solved this by changing the HTTP method for the sign out route from DELETE to GET.

In config/initializers/devise.rb:

  # The default HTTP method used to sign out a resource. Default is :delete.
  config.sign_out_via = :get

Then you can use a regular link in your view, no Turbo/JS needed at all:

<%= link_to "Sign Out", destroy_user_session_path %>

Alternatively, you can keep using a DELETE route (preferable, since GET routes with side-effects are kind of icky) and use a form rather than a link:

<%= button_to "Sign Out", destroy_user_session_path, method: :delete %>

This also requires no Turbo/JS, though you'll probably want to sprinkle some CSS to make the button look like a regular link.

@florianfelsing
Copy link

florianfelsing commented Sep 21, 2022

@Havoc85

You need to generate or create an inherited Devise controller for sessions first. You can do that by running the following command (assuming that "user" is your Devise model):

rails generate devise:controllers users -c=sessions

Uncomment the destroy method in the new file that gets created and place the above code inside the method.

Don't forget to update your Devise routes for the changes to take effect.

@d33pjs
Copy link

d33pjs commented Sep 21, 2022

@Havoc85

You need to generate or create an inherited Devise controller for sessions first. You can do that by running the following command (assuming that "user" is your Devise model):

rails generate devise:controllers users -c=sessions

Uncomment the destroy method in the new file that gets created and place the above code inside the method.

Don't forget to update your Devise routes for the changes to take effect.

Thank you for that description! That helps a lot.

@Alertzero
Copy link

Alertzero commented Oct 23, 2022

@Havoc85

You need to generate or create an inherited Devise controller for sessions first. You can do that by running the following command (assuming that "user" is your Devise model):

rails generate devise:controllers users -c=sessions

Uncomment the destroy method in the new file that gets created and place the above code inside the method.

Don't forget to update your Devise routes for the changes to take effect.

I followed all the instruction and still face with the actionController::RoutingError (No route matches [GET] "/users/sign_out"): Not sure if there is anyway to find the root cause

***solved just because changing to the correct resource_name

@jx-bamboo
Copy link

This worked for me initially:

<%= link_to "Logout", destroy_user_session_path, data: { turbo_method: :delete } %>

But after making these turbo changes (to fix an unrelated issue of some bootstrap javascripts not working) from:

// app/javascript/application.js
import "@hotwired/turbo-rails"

to this

// app/javascript/application.js
import { Turbo } from "@hotwired/turbo-rails"
Turbo.session.drive = false

then logout broke.

However, this worked:

<%= button_to "Log out", destroy_user_session_path, method: :delete, data: { turbo: false } %>

Hope it's useful.

👍 Thanks

littleforest added a commit to ApprenticeshipStandardsDotOrg/ApprenticeshipStandardsDotOrg that referenced this issue Nov 21, 2022
littleforest added a commit to ApprenticeshipStandardsDotOrg/ApprenticeshipStandardsDotOrg that referenced this issue Nov 23, 2022
littleforest added a commit to ApprenticeshipStandardsDotOrg/ApprenticeshipStandardsDotOrg that referenced this issue Nov 28, 2022
@Alertzero
Copy link

false

Not a correct way because we want to use turbo

@revskill10
Copy link

My solution:

sessions_controller.rb

class Users::SessionsController < Devise::SessionsController
  def destroy
    super do
      # Turbo requires redirects be :see_other (303); so override Devise default (302)
      return redirect_to new_user_session_path, status: :see_other, notice: I18n.t("devise.sessions.signed_out")
    end
  end
end
<%= link_to "Sign out", "/users/sign_out", method: :delete, data: { turbo_method: :delete, turbo_confirm: 'Are you sure?' } %>

joeroe added a commit to xronos-ch/xronos.rails that referenced this issue Jan 11, 2023
This was related to the move to turbo. See: heartcombo/devise#5458

Refactored some of application.html.erb to a partial while I'm at it.
carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
@johnpitchko
Copy link

<%= button_to "Sign Out", destroy_user_session_path, method: :delete %> Did not work.

<%= button_to "Log out", destroy_user_session_path, method: :delete, data: { turbo: false } %> Did work; thank you @stevecondylios

However, I ended up using @revskill10 's solution as I wanted use a link_to rather than a button. Thanks everyone for your wisdom!

@carlosantoniodasilva
Copy link
Member

carlosantoniodasilva commented Feb 2, 2023

Hey, you're all welcome to try this new branch and see if that works for you out of the box: #5548. Please see the description on how to get things setup, and report back if you run into any issues. Thanks.

@jinyuanwong
Copy link

data: { turbo_method: :delete }

Using the first line of codes works perfectly for me! Thank you! I highly recommend everyone to have a try.

@marcbest
Copy link

marcbest commented Feb 9, 2023

Setting config.navigational_formats = ['*/*', :html, :turbo_stream] in devise.rb seemed to work for me

@carlosantoniodasilva
Copy link
Member

The main branch should contain all that's necessary for fully working with Turbo now, which should fix this. A new version will be released soon, but feel free to test it out from the main branch in the meantime, and report back on any issues. Thanks.

@manikantapamarthi
Copy link

You have to change from:

<%= link_to "Sign out", destroy_user_session_path, method: delete %>

to

<%= link_to "Sign out", destroy_user_session_path, data: { turbo_method: :delete" } %>

typo data: { turbo_method: :"delete" }

@rolandoalvarado
Copy link

I just started working on Rails 7 app and experienced same issue. My issue was login and logout works. It just renders the new page at the bottom of the login page instead of redirecting. I was able to fix by:

For login button: = f.submit('Login', class: 'btn', params: { prompt: 'login' }, data: { turbo: 'false' })
For Logout: = button_to('Log out', user_session_path(current_user), class: 'btn', method: :delete, data: { turbo: 'false' })

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

Successfully merging a pull request may close this issue.