Skip to content

feat: Add GET route for sign out on static error pages #5189

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.

Sign up for GitHub

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

Open
Gabe-Torres wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: Add GET route for sign out on static error pages #5189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
769a1f8
feat: Add GET route for sign out on static error pages
Gabe-Torres May 9, 2025
a7522b6
fix: Rename sign_out method to avoid Devise conflict
Gabe-Torres May 9, 2025
fa8e19d
Merge branch 'main' into 5101_static_error_pages
Gabe-Torres Jun 8, 2025
3336cfb
Merge branch 'main' into 5101_static_error_pages
awwaiid Aug 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/controllers/users/sessions_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ def create
UsersRole.set_last_role_for(current_user, @role)
end

# GET /resource/sign_out
def sign_out_error_page
Copy link
Collaborator

@dorner dorner Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused... doesn't Devise already generate a sign_out route? Is there a reason you can't reuse that?

Copy link
Contributor Author

@Gabe-Torres Gabe-Torres Jul 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for the custom route is that the static error pages (403, 404, 422, 500) are served as plain HTML files and can only make GET requests. Devise's standard sign-out route requires a DELETE request. I believe another way to reuse Devise's included sign-out is to make JavaScript scripts

Copy link
Collaborator

@awwaiid awwaiid Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree and confirm -- upsettingly devise does use the DELETE which is actually sent as a POST with a method. The other way to do this would be to make them form submits, but that seems wrong also. I like it this way as a new endpoint.

sign_out(current_user) if user_signed_in?
redirect_to root_path, notice: "Signed out successfully"
end

# DELETE /resource/sign_out
# def destroy
# super
Expand Down
4 changes: 4 additions & 0 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ def set_up_flipper
omniauth_callbacks: 'users/omniauth_callbacks'
}

devise_scope :user do
get 'users/sign_out', to: 'users/sessions#sign_out_error_page', as: :user_sign_out
end

#
# Mount web interface to see delayed job status and queue length.
# Visible only to logged in users with the `super_admin` role
Expand Down
2 changes: 1 addition & 1 deletion public/403.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<div class="navbar-custom-menu">
<ul class="nav navbar-nav">
<li class="user user-menu">
<a rel="nofollow" class=" btn btn-danger btn-md " data-method="delete" href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
<a rel="nofollow" class=" btn btn-danger btn-md " href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
</li>
</ul>
</div>
Expand Down
2 changes: 1 addition & 1 deletion public/404.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<div class="navbar-custom-menu">
<ul class="nav navbar-nav">
<li class="user user-menu">
<a rel="nofollow" class=" btn btn-danger btn-md " data-method="delete" href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
<a rel="nofollow" class=" btn btn-danger btn-md " href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
</li>
</ul>
</div>
Expand Down
2 changes: 1 addition & 1 deletion public/422.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<div class="navbar-custom-menu">
<ul class="nav navbar-nav">
<li class="user user-menu">
<a rel="nofollow" class=" btn btn-danger btn-md " data-method="delete" href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
<a rel="nofollow" class=" btn btn-danger btn-md " href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
</li>
</ul>
</div>
Expand Down
2 changes: 1 addition & 1 deletion public/500.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<div class="navbar-custom-menu">
<ul class="nav navbar-nav">
<li class="user user-menu">
<a rel="nofollow" class=" btn btn-danger btn-md " data-method="delete" href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
<a rel="nofollow" class=" btn btn-danger btn-md " href="/users/sign_out"><i class="fa fa-sign-out"></i> Log out</a>
</li>
</ul>
</div>
Expand Down
49 changes: 49 additions & 0 deletions spec/features/error_pages_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
require "rails_helper"

RSpec.describe "Error Pages", type: :feature do
let(:user) { create(:user) }

before do
sign_in(user)
end

context "on 403 error page" do
it "allows user to log out" do
visit "/403"
expect(page).to have_link("Log out")

click_link "Log out"
expect(page).to have_content("Signed out successfully")
end
end

context "on 404 error page" do
it "allows user to log out" do
visit "/404"
expect(page).to have_link("Log out")

click_link "Log out"
expect(page).to have_content("Signed out successfully")
end
end

context "on 422 error page" do
it "allows user to log out" do
visit "/422"
expect(page).to have_link("Log out")

click_link "Log out"
expect(page).to have_content("Signed out successfully")
end
end

context "on 500 error page" do
it "allows user to log out" do
visit "/500"
expect(page).to have_link("Log out")

click_link "Log out"
expect(page).to have_content("Signed out successfully")
end
end
end
Loading