From 223fcec836887fd94cf1b3175fcd23133bac3615 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Tue, 19 Sep 2023 17:58:45 +0200 Subject: [PATCH 1/3] Disambiguate authentication and auhtorization --- .../{auth_adapters => authentication_adapters}/backend.rb | 2 +- admin/app/controllers/solidus_admin/base_controller.rb | 4 ++-- .../controller_helpers/{auth.rb => authentication.rb} | 2 +- admin/lib/solidus_admin/preview.rb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename admin/app/controllers/solidus_admin/{auth_adapters => authentication_adapters}/backend.rb (94%) rename admin/app/controllers/solidus_admin/controller_helpers/{auth.rb => authentication.rb} (93%) diff --git a/admin/app/controllers/solidus_admin/auth_adapters/backend.rb b/admin/app/controllers/solidus_admin/authentication_adapters/backend.rb similarity index 94% rename from admin/app/controllers/solidus_admin/auth_adapters/backend.rb rename to admin/app/controllers/solidus_admin/authentication_adapters/backend.rb index 6469570cc53..d59d889bf72 100644 --- a/admin/app/controllers/solidus_admin/auth_adapters/backend.rb +++ b/admin/app/controllers/solidus_admin/authentication_adapters/backend.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module SolidusAdmin::AuthAdapters::Backend +module SolidusAdmin::AuthenticationAdapters::Backend extend ActiveSupport::Concern included do diff --git a/admin/app/controllers/solidus_admin/base_controller.rb b/admin/app/controllers/solidus_admin/base_controller.rb index e59f7a2ed04..10c4c9eb592 100644 --- a/admin/app/controllers/solidus_admin/base_controller.rb +++ b/admin/app/controllers/solidus_admin/base_controller.rb @@ -8,10 +8,10 @@ class BaseController < ApplicationController include Spree::Core::ControllerHelpers::Store include GearedPagination::Controller - include SolidusAdmin::ControllerHelpers::Auth + include SolidusAdmin::ControllerHelpers::Authentication include SolidusAdmin::ControllerHelpers::Locale include SolidusAdmin::ComponentsHelper - include SolidusAdmin::AuthAdapters::Backend if defined?(Spree::Backend) + include SolidusAdmin::AuthenticationAdapters::Backend if defined?(Spree::Backend) layout 'solidus_admin/application' helper 'solidus_admin/components' diff --git a/admin/app/controllers/solidus_admin/controller_helpers/auth.rb b/admin/app/controllers/solidus_admin/controller_helpers/authentication.rb similarity index 93% rename from admin/app/controllers/solidus_admin/controller_helpers/auth.rb rename to admin/app/controllers/solidus_admin/controller_helpers/authentication.rb index 60d251149d2..4afc7bfd0c6 100644 --- a/admin/app/controllers/solidus_admin/controller_helpers/auth.rb +++ b/admin/app/controllers/solidus_admin/controller_helpers/authentication.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module SolidusAdmin::ControllerHelpers::Auth +module SolidusAdmin::ControllerHelpers::Authentication extend ActiveSupport::Concern included do diff --git a/admin/lib/solidus_admin/preview.rb b/admin/lib/solidus_admin/preview.rb index 2b80cb9771d..8c22d089ede 100644 --- a/admin/lib/solidus_admin/preview.rb +++ b/admin/lib/solidus_admin/preview.rb @@ -27,7 +27,7 @@ module ControllerHelper extend ActiveSupport::Concern included do - include SolidusAdmin::ControllerHelpers::Auth + include SolidusAdmin::ControllerHelpers::Authentication helper ActionView::Helpers helper SolidusAdmin::ComponentsHelper helper_method :current_component From 5621c39b91a4aa0290061b5746d11a954a891efe Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Wed, 20 Sep 2023 14:38:42 +0200 Subject: [PATCH 2/3] Ensure logout uses the correct HTTP method --- .../solidus_admin/sidebar/account_nav/component.html.erb | 2 +- .../solidus_admin/sidebar/account_nav/component_spec.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/admin/app/components/solidus_admin/sidebar/account_nav/component.html.erb b/admin/app/components/solidus_admin/sidebar/account_nav/component.html.erb index 0082c3ee4a4..fd4a4d3f359 100644 --- a/admin/app/components/solidus_admin/sidebar/account_nav/component.html.erb +++ b/admin/app/components/solidus_admin/sidebar/account_nav/component.html.erb @@ -58,7 +58,7 @@ <% end %>
  • - <%= link_to @logout_path, method: @logout_method, class: 'flex gap-2 items-center px-2' do %> + <%= button_to @logout_path, method: @logout_method, class: 'flex gap-2 items-center px-2' do %> <%= icon_tag("logout-box-line", class: "w-5 h-5 fill-current shrink") %> <%= t('.logout') %> <% end %> diff --git a/admin/spec/components/solidus_admin/sidebar/account_nav/component_spec.rb b/admin/spec/components/solidus_admin/sidebar/account_nav/component_spec.rb index 0b76d1fdc11..5a1cb4e5fe4 100644 --- a/admin/spec/components/solidus_admin/sidebar/account_nav/component_spec.rb +++ b/admin/spec/components/solidus_admin/sidebar/account_nav/component_spec.rb @@ -22,8 +22,10 @@ # Links are hidden within a
    element expect(page).to have_link("Account", href: "/admin/account", visible: :any) - expect(page).to have_link("Logout", href: "/admin/logout", visible: :any) - expect(page.find_link("Logout", visible: :any)["data-method"]).to eq("delete") + within('form[action="/admin/logout"]') do + expect(page).to have_button("Logout", visible: :any) + expect(page).to have_css('input[type="hidden"][name="_method"][value="delete"]') + end end end end From 3dbae4917c7955c60944df72a6ce2c9c9d4bd95d Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Wed, 20 Sep 2023 15:50:42 +0200 Subject: [PATCH 3/3] Move authorization checks outside of the backend "auth" adapter Those are not dependent on the authentication system. --- .../solidus_admin/accounts_controller.rb | 2 ++ .../authentication_adapters/backend.rb | 15 ++-------- .../solidus_admin/base_controller.rb | 1 + .../controller_helpers/authorization.rb | 29 +++++++++++++++++++ 4 files changed, 34 insertions(+), 13 deletions(-) create mode 100644 admin/app/controllers/solidus_admin/controller_helpers/authorization.rb diff --git a/admin/app/controllers/solidus_admin/accounts_controller.rb b/admin/app/controllers/solidus_admin/accounts_controller.rb index cd3ddaa884f..b43f84c08b2 100644 --- a/admin/app/controllers/solidus_admin/accounts_controller.rb +++ b/admin/app/controllers/solidus_admin/accounts_controller.rb @@ -2,6 +2,8 @@ module SolidusAdmin class AccountsController < SolidusAdmin::BaseController + skip_before_action :authorize_solidus_admin_user! + def show redirect_to spree.edit_admin_user_path(current_solidus_admin_user) end diff --git a/admin/app/controllers/solidus_admin/authentication_adapters/backend.rb b/admin/app/controllers/solidus_admin/authentication_adapters/backend.rb index d59d889bf72..02234657433 100644 --- a/admin/app/controllers/solidus_admin/authentication_adapters/backend.rb +++ b/admin/app/controllers/solidus_admin/authentication_adapters/backend.rb @@ -11,20 +11,9 @@ module SolidusAdmin::AuthenticationAdapters::Backend private def authenticate_solidus_backend_user! - if respond_to?(:model_class, true) && model_class - record = model_class - else - record = controller_name.to_sym - end - authorize! :admin, record - authorize! action_name.to_sym, record - rescue CanCan::AccessDenied - instance_exec(&Spree::Admin::BaseController.unauthorized_redirect) - end + return if spree_current_user - # Needs to be overriden so that we use Spree's Ability rather than anyone else's. - def current_ability - @current_ability ||= Spree::Ability.new(spree_current_user) + instance_exec(&Spree::Admin::BaseController.unauthorized_redirect) end def store_location diff --git a/admin/app/controllers/solidus_admin/base_controller.rb b/admin/app/controllers/solidus_admin/base_controller.rb index 10c4c9eb592..457bc8243be 100644 --- a/admin/app/controllers/solidus_admin/base_controller.rb +++ b/admin/app/controllers/solidus_admin/base_controller.rb @@ -9,6 +9,7 @@ class BaseController < ApplicationController include GearedPagination::Controller include SolidusAdmin::ControllerHelpers::Authentication + include SolidusAdmin::ControllerHelpers::Authorization include SolidusAdmin::ControllerHelpers::Locale include SolidusAdmin::ComponentsHelper include SolidusAdmin::AuthenticationAdapters::Backend if defined?(Spree::Backend) diff --git a/admin/app/controllers/solidus_admin/controller_helpers/authorization.rb b/admin/app/controllers/solidus_admin/controller_helpers/authorization.rb new file mode 100644 index 00000000000..c0a1d970dce --- /dev/null +++ b/admin/app/controllers/solidus_admin/controller_helpers/authorization.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module SolidusAdmin::ControllerHelpers::Authorization + extend ActiveSupport::Concern + + included do + before_action :authorize_solidus_admin_user! + end + + private + + def current_ability + @current_ability ||= Spree::Ability.new(current_solidus_admin_user) + end + + def authorize_solidus_admin_user! + subject = authorization_subject + + authorize! :admin, subject + authorize! action_name, subject + end + + def authorization_subject + "Spree::#{controller_name.classify}".constantize + rescue NameError + raise NotImplementedError, "Couldn't infer the model class from the controller name, " \ + "please implement `#{self.class}#authorization_subject`." + end +end