From f9b752cff4ef386ac09c574166e1f9e3b8015a6c Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Wed, 26 Jan 2022 16:21:50 +0100 Subject: [PATCH 1/2] Enforce authentication This adds a `before_action` to `ApplicationController` (with carefully selected exceptions in a few places) and fixes abilities to actually restrict access to signed in users. --- app/controllers/application_controller.rb | 12 ++++++++++++ app/controllers/page_controller.rb | 2 ++ app/controllers/sessions_controller.rb | 2 ++ app/controllers/users_controller.rb | 7 +++++++ app/models/ability.rb | 2 +- 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b2ccc321..abddac0f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,6 +2,8 @@ class ApplicationController < ActionController::Base rescue_from Hdm::Error, with: :display_error_page rescue_from CanCan::AccessDenied, with: :access_denied + before_action :authentication_required + helper_method :current_user private @@ -16,6 +18,16 @@ def current_user end end + def authentication_required + unless current_user + if User.none? + redirect_to new_user_path, notice: 'Please create an admin user first.' + else + redirect_to login_path + end + end + end + def load_environments @environments = Environment.all @environment = Environment.find(params[:environment_id]) diff --git a/app/controllers/page_controller.rb b/app/controllers/page_controller.rb index 07e93568..bb20900f 100644 --- a/app/controllers/page_controller.rb +++ b/app/controllers/page_controller.rb @@ -1,4 +1,6 @@ class PageController < ApplicationController + skip_before_action :authentication_required + add_breadcrumb "Home", :root_path def index diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a79d513e..c816c8a9 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,6 @@ class SessionsController < ApplicationController + skip_before_action :authentication_required + add_breadcrumb "Home", :root_path def new diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9788f955..6e18e630 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,4 +1,7 @@ class UsersController < ApplicationController + skip_before_action :authentication_required, only: [:new, :create] + before_action :conditional_authentication, only: [:new, :create] + load_and_authorize_resource add_breadcrumb "Home", :root_path @@ -84,4 +87,8 @@ def user_params end end end + + def conditional_authentication + authentication_required if User.exists? + end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 1d406714..d3359d5f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -34,7 +34,7 @@ def initialize(user) if User.none? can :create, User else - user ||= User.new # guest user (not logged in) + return unless user.present? if user.admin? if User.admins.count > 1 From 7fd8e293465c96785183bb2908c25be190cf2f03 Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Mon, 31 Jan 2022 11:13:04 +0100 Subject: [PATCH 2/2] Add test to ensure authentication requirements. --- .../required_authentication_test.rb | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 test/integration/required_authentication_test.rb diff --git a/test/integration/required_authentication_test.rb b/test/integration/required_authentication_test.rb new file mode 100644 index 00000000..3639b210 --- /dev/null +++ b/test/integration/required_authentication_test.rb @@ -0,0 +1,52 @@ +require "test_helper" + +class RequiredAuthenticationTest < ActionDispatch::IntegrationTest + + test "authentication requirements for environments" do + authentication_required_for :get, environments_path + end + + test "authentication requiremens for nodes" do + authentication_required_for :get, environment_nodes_path("development") + end + + test "authentication requirements for keys" do + authentication_required_for :get, + environment_node_keys_path("development", "testhost") + authentication_required_for :get, + environment_node_key_path("development", "testhost", "hdm::integer") + authentication_required_for :patch, + environment_node_key_path("development", "testhost", "hdm::integer") + authentication_required_for :delete, + environment_node_key_path("development", "testhost", "hdm::integer") + end + + test "authentication requirements for decrypted values" do + authentication_required_for :post, + environment_node_decrypted_values_path("development", "testhost") + end + + test "authentication requirements for encrypted values" do + authentication_required_for :post, + environment_node_encrypted_values_path("development", "testhost") + end + + test "authentication requirements for users" do + user = FactoryBot.create(:user, admin: true) + + authentication_required_for :get, users_path + authentication_required_for :get, user_path(user) + authentication_required_for :get, new_user_path + authentication_required_for :post, users_path + authentication_required_for :get, edit_user_path(user) + authentication_required_for :patch, user_path(user) + authentication_required_for :delete, user_path(user) + end + + private + + def authentication_required_for(method, path) + send(method, path) + assert_redirected_to login_path + end +end