Skip to content

Commit

Permalink
745 move authenticate user method (#788)
Browse files Browse the repository at this point in the history
* add: authenticate_user! as a before action in application controller instead of calling it within multiple controllers

* add: skip_before_action authenticate_user! for controllers that don't need user authentication

* remove: commented out 'skip_before_action' from controllers

* fix failing tests: after adding authentication

these tests were failing as these controller tests required authentication

* add: config for devise
configuration for setting up custom failures through warden in the devise.rb

* add custom failure file

this custom failure module redirects a user to the sign-in page if they arent no authenticated

* update: failure app

This failure app is mostly reproduced from Devises FailureApp class.
The 5th line ```opts[:script_name] = nil``` was preventing the OrganizationMiddleware from generating the correct route with the Organization slug. This version allows us to keep the full functionality of the scoped_url method within Devises FailureApp and generate the route

 Co-authored-by: Paul DobbinSchmaltz <[email protected]>

* add more context to why this authentication failure app is needed.

* update: rails standard fix
  • Loading branch information
Gabe-Torres authored and ErinClaudio committed Jun 27, 2024
1 parent 7dfb98c commit d2cdc9d
Show file tree
Hide file tree
Showing 18 changed files with 82 additions and 21 deletions.
1 change: 1 addition & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class ApplicationController < ActionController::Base
before_action :authenticate_user!
verify_authorized unless: :devise_controller?

before_action :set_current_user
Expand Down
1 change: 1 addition & 0 deletions app/controllers/contacts_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class ContactsController < ApplicationController
skip_before_action :authenticate_user!
skip_verify_authorized only: %i[new create]

def new
Expand Down
1 change: 1 addition & 0 deletions app/controllers/errors_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class ErrorsController < ApplicationController
skip_before_action :authenticate_user!
skip_verify_authorized

def not_found
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/organizations/adoptable_pets_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
class Organizations::AdoptablePetsController < Organizations::BaseController
include ::Pagy::Backend

skip_before_action :authenticate_user!
skip_verify_authorized only: %i[index]
before_action :set_likes, only: %i[index show], if: -> { current_user&.adopter_foster_account }
helper_method :get_animals
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
class Organizations::AdopterFosterer::AdopterApplicationsController < Organizations::BaseController
before_action :authenticate_user!
before_action :set_application, only: %i[update]
layout "adopter_foster_dashboard"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
class Organizations::AdopterFosterer::ProfilesController < Organizations::BaseController
before_action :authenticate_user!
before_action :authorize_with!, only: %i[new create]
before_action :set_profile, only: %i[show edit update]

Expand Down
1 change: 1 addition & 0 deletions app/controllers/organizations/faq_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class Organizations::FaqController < Organizations::BaseController
skip_before_action :authenticate_user!
skip_verify_authorized only: %i[index]

def index
Expand Down
1 change: 1 addition & 0 deletions app/controllers/organizations/home_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class Organizations::HomeController < Organizations::BaseController
skip_before_action :authenticate_user!
skip_verify_authorized only: %i[index]

def index
Expand Down
1 change: 1 addition & 0 deletions app/controllers/root_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class RootController < ApplicationController
skip_before_action :authenticate_user!
skip_verify_authorized only: %i[index]

def index
Expand Down
1 change: 1 addition & 0 deletions app/controllers/static_pages_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class StaticPagesController < ApplicationController
skip_before_action :authenticate_user!
skip_verify_authorized only: %i[about_us cookie_policy donate faq partners
privacy_policy terms_and_conditions]

Expand Down
6 changes: 6 additions & 0 deletions config/initializers/devise.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require "authentication_failure_app"

# Assuming you have not yet modified this file, each configuration option below
# is set to its default value. Note that some are commented out while others
# are not: uncommented lines are intended to protect your configuration from
Expand Down Expand Up @@ -363,4 +365,8 @@

config.responder.error_status = :unprocessable_entity
config.responder.redirect_status = :see_other

config.warden do |manager|
manager.failure_app = AuthenticationFailureApp
end
end
17 changes: 0 additions & 17 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 55 additions & 0 deletions lib/authentication_failure_app.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
class AuthenticationFailureApp < Devise::FailureApp
# This method is used to generate the URL for the redirect after a user fails to authenticate,
# for example, when an unauthenticated user tries to access an authenticated route within an organization's scope.
# The issue was that if a user failed to authenticate while in the context of an organization,
# the organization's slug should be in the URL. So, after failing, the user would be redirected
# to the sign-in page, but the organization's scope would be lost in the URL,
# and it would fall back to the root URL instead of the organization's sign-in URL.
# This fixes that to ensure the organization's slug is present in the URL.
#
# This code is 99% reproduced from the Devise gem's Devise::FailureApp class
# at: lib/devise/failure_app.rb. The only exception is that we comment out the
# 5th line:
# opts[:script_name] = nil
#
# This line was causing the`env["SCRIPT_NAME"]` value that's set by
# OrganizationMiddleware to not be used when generating the route in the
# statement `context.send(route, opts)`, which wipes out the current
# Organization context on authentication failure.
#
# If Devise is upgraded, may need to compare this method across versions for
# any needed updates.
def scope_url
opts = {}

# Initialize script_name with nil to prevent infinite loops in
# authenticated mounted engines in rails 4.2 and 5.0
# opts[:script_name] = nil

route = route(scope)

opts[:format] = request_format unless skip_format?

router_name = Devise.mappings[scope].router_name || Devise.available_router_name
context = send(router_name)

if relative_url_root?
opts[:script_name] = relative_url_root

# We need to add the rootpath to `script_name` manually for applications that use a Rails
# version lower than 5.1. Otherwise, it is going to generate a wrong path for Engines
# that use Devise. Remove it when the support of Rails 5.0 is dropped.
elsif root_path_defined?(context) && !rails_51_and_up?
rootpath = context.routes.url_helpers.root_path
opts[:script_name] = rootpath.chomp("/") if rootpath.length > 1
end

if context.respond_to?(route)
context.send(route, opts)
elsif respond_to?(:root_url)
root_url(opts)
else
"/"
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
class Organizations::Staff::FosterersControllerTest < ActionDispatch::IntegrationTest
setup do
@organization = ActsAsTenant.current_tenant
@adopter_foster_account = create(:adopter_foster_account)
sign_in @adopter_foster_account.user
end

context "authorization" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
class Organizations::Staff::FostersControllerTest < ActionDispatch::IntegrationTest
context "authorization" do
include ActionPolicy::TestHelper

context "context only action" do
setup do
@organization = ActsAsTenant.current_tenant
@adopter_foster_account = create(:adopter_foster_account)
sign_in @adopter_foster_account.user
end

context "#new" do
Expand Down Expand Up @@ -66,6 +67,7 @@ class Organizations::Staff::FostersControllerTest < ActionDispatch::IntegrationT
context "existing record actions" do
setup do
@foster = create(:foster)
sign_in @foster.user
end

context "#edit" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class Organizations::Staff::StaffControllerTest < ActionDispatch::IntegrationTes
setup do
@organization = ActsAsTenant.current_tenant
@staff = create(:staff_account)
sign_in @staff.user
end

context "authorization" do
Expand Down
4 changes: 3 additions & 1 deletion test/controllers/states_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

class StatesControllerTest < ActionDispatch::IntegrationTest
test "should return turbo stream with the states in it" do
create(:adopter, :with_profile)
adopter = create(:adopter, :with_profile)
sign_in adopter

name = "adopter[address_attributes][state]"
target = "adopter_foster_profile_location_attributes_province_state"

Expand Down
3 changes: 3 additions & 0 deletions test/integration/adoption_application_reviews_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class AdoptionApplicationReviewsTest < ActionDispatch::IntegrationTest

context "non-staff" do
should "not see any applications" do
@user = create(:user)
sign_in @user

get staff_adoption_application_reviews_path

assert_response :redirect
Expand Down

0 comments on commit d2cdc9d

Please sign in to comment.