From 9040ba3cf8aa5479e76b57480dcb94a117161ad7 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 12 Dec 2024 16:53:16 +0000 Subject: [PATCH 01/11] Ads BasicAuthentication concern, implementation borrowed from the mission_control-jobs gem --- .../active_insights/application_controller.rb | 2 ++ .../active_insights/basic_authentication.rb | 34 +++++++++++++++++++ lib/active_insights.rb | 4 ++- 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 app/controllers/concerns/active_insights/basic_authentication.rb diff --git a/app/controllers/active_insights/application_controller.rb b/app/controllers/active_insights/application_controller.rb index 6a9bacb..ed8115b 100644 --- a/app/controllers/active_insights/application_controller.rb +++ b/app/controllers/active_insights/application_controller.rb @@ -2,6 +2,8 @@ module ActiveInsights class ApplicationController < ActionController::Base + include ActiveInsights::BasicAuthentication + protect_from_forgery with: :exception around_action :setup_time_zone diff --git a/app/controllers/concerns/active_insights/basic_authentication.rb b/app/controllers/concerns/active_insights/basic_authentication.rb new file mode 100644 index 0000000..e1c3b44 --- /dev/null +++ b/app/controllers/concerns/active_insights/basic_authentication.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module ActiveInsights::BasicAuthentication + extend ActiveSupport::Concern + + included do + before_action :authenticate_by_http_basic + end + + def authenticate_by_http_basic + return unless http_basic_authentication_enabled? + + if http_basic_authentication_configured? + http_basic_authenticate_or_request_with(**http_basic_authentication_credentials) + else + head :unauthorized + end + end + + def http_basic_authentication_enabled? + ActiveInsights.http_basic_auth_enabled + end + + def http_basic_authentication_configured? + http_basic_authentication_credentials.values.all?(&:present?) + end + + def http_basic_authentication_credentials + { + name: ActiveInsights.http_basic_auth_user, + password: ActiveInsights.http_basic_auth_password + }.transform_values(&:presence) + end +end diff --git a/lib/active_insights.rb b/lib/active_insights.rb index 328912a..c93e34b 100644 --- a/lib/active_insights.rb +++ b/lib/active_insights.rb @@ -7,7 +7,9 @@ require "active_insights/engine" module ActiveInsights - mattr_accessor :connects_to, :ignored_endpoints, :enabled + mattr_accessor :connects_to, :ignored_endpoints, :enabled, + :http_basic_auth_enabled, :http_basic_auth_user, + :http_basic_auth_password class << self def ignored_endpoint?(payload) From b7fee7f65343985440156737f9977f67bdef2e45 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 12 Dec 2024 16:56:27 +0000 Subject: [PATCH 02/11] Rubocop fixes --- .devcontainer/devcontainer.json | 12 ++++++++++++ .../concerns/active_insights/basic_authentication.rb | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 .devcontainer/devcontainer.json diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..7413838 --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,12 @@ +{ + "image": "ruby:3.3.5", + "postCreateCommand": "bundle install", + "forwardPorts": [9292], + "customizations": { + "vscode": { + "extensions": [ + "Shopify.ruby-extensions-pack", + ] + } + } +} diff --git a/app/controllers/concerns/active_insights/basic_authentication.rb b/app/controllers/concerns/active_insights/basic_authentication.rb index e1c3b44..bf9e573 100644 --- a/app/controllers/concerns/active_insights/basic_authentication.rb +++ b/app/controllers/concerns/active_insights/basic_authentication.rb @@ -11,7 +11,7 @@ def authenticate_by_http_basic return unless http_basic_authentication_enabled? if http_basic_authentication_configured? - http_basic_authenticate_or_request_with(**http_basic_authentication_credentials) + http_basic_authenticate_or_request_with(**http_basic_auth_credentials) else head :unauthorized end @@ -22,10 +22,10 @@ def http_basic_authentication_enabled? end def http_basic_authentication_configured? - http_basic_authentication_credentials.values.all?(&:present?) + http_basic_auth_credentials.values.all?(&:present?) end - def http_basic_authentication_credentials + def http_basic_auth_credentials { name: ActiveInsights.http_basic_auth_user, password: ActiveInsights.http_basic_auth_password From 10196acec5910d9765985c231c039ae0cd6abe41 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 12 Dec 2024 17:09:23 +0000 Subject: [PATCH 03/11] Adds tests for auth settings --- test/active_insights_test.rb | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/active_insights_test.rb b/test/active_insights_test.rb index 46e96a7..9347bdc 100644 --- a/test/active_insights_test.rb +++ b/test/active_insights_test.rb @@ -2,8 +2,38 @@ require "test_helper" +puts ActiveInsights.http_basic_auth_enabled.inspect + class ActiveInsightsTest < ActiveSupport::TestCase + teardown do + ActiveInsights.http_basic_auth_enabled = nil + ActiveInsights.http_basic_auth_user = nil + ActiveInsights.http_basic_auth_password = nil + end + test "it has a version number" do assert ActiveInsights::VERSION end + + test "it does not enabled http basic auth by default" do + assert_not ActiveInsights.http_basic_auth_enabled + end + + test "it has an accessor to set whether http basic auth is enabled" do + ActiveInsights.http_basic_auth_enabled = true + + assert ActiveInsights.http_basic_auth_enabled + end + + test "it has an accessor to set http basic auth user" do + ActiveInsights.http_basic_auth_user = "johndoe" + + assert_equal "johndoe", ActiveInsights.http_basic_auth_user + end + + test "it has an accessor to set http basic auth password" do + ActiveInsights.http_basic_auth_password = "secret" + + assert_equal "secret", ActiveInsights.http_basic_auth_password + end end From 8186532c8bcc7571c700c052a67f6432c89f2bc2 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 12 Dec 2024 17:20:58 +0000 Subject: [PATCH 04/11] Adds tests for BasicAuthentication concern --- .../basic_authentication_test.rb | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 test/controllers/concerns/active_insights/basic_authentication_test.rb diff --git a/test/controllers/concerns/active_insights/basic_authentication_test.rb b/test/controllers/concerns/active_insights/basic_authentication_test.rb new file mode 100644 index 0000000..7762844 --- /dev/null +++ b/test/controllers/concerns/active_insights/basic_authentication_test.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +# +require "test_helper" + +class ActiveInsights::BasicAuthenticationTest < ActionController::TestCase + + class TestController < ActionController::Base + include ActiveInsights::BasicAuthentication + + def index + render plain: "Hello from TestController" + end + end + + setup do + @controller = TestController.new + @original_routes = Rails.application.routes + + Rails.application.routes.draw do + get "test", to: "active_insights/basic_authentication_test/test#index" + end + end + + teardown do + Rails.application.routes_reloader.reload! + ActiveInsights.http_basic_auth_enabled = nil + ActiveInsights.http_basic_auth_user = nil + ActiveInsights.http_basic_auth_password = nil + end + + test "it does not require http basic auth by default" do + get :index + + assert_response :success + end + + test "it requires http basic auth when enabled" do + ActiveInsights.http_basic_auth_enabled = true + + get :index + + assert_response :unauthorized + end + + test "when enabled and proper credentials are included, it allows access" do + ActiveInsights.http_basic_auth_enabled = true + ActiveInsights.http_basic_auth_user = "johndoe" + ActiveInsights.http_basic_auth_password = "secret" + @request.env["HTTP_AUTHORIZATION"] = + ActionController::HttpAuthentication::Basic.encode_credentials("johndoe", + "secret") + + get :index + + assert_response :success + end +end From 91abb7d1a75793ab1a2e882779cf97f30387d3e9 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 12 Dec 2024 17:32:44 +0000 Subject: [PATCH 05/11] README updates --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 2d01ab8..5747f66 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,14 @@ If you are using Sprockets, add the ActiveInsights css file to manifest.js: //= link active_insights/application.css ``` +To use HTTP basic authentication, enable it and define a user and password: + +```ruby +config.active_insights.http_basic_auth_enabled = true +config.active_insights.http_basic_auth_user = ENV["BASIC_AUTH_USER"] +config.active_insights.http_basic_auth_password = ENV["BASIC_AUTH_PASSWORD"] +``` + ## Development After checking out the repo, run `bin/setup` to install dependencies. Then, run From 10e797d622f0ffac827df4021ec80f1bdb045033 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 12 Dec 2024 18:54:04 +0000 Subject: [PATCH 06/11] Fix Rubocop stuff --- .../active_insights/basic_authentication_test.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/controllers/concerns/active_insights/basic_authentication_test.rb b/test/controllers/concerns/active_insights/basic_authentication_test.rb index 7762844..ae47d33 100644 --- a/test/controllers/concerns/active_insights/basic_authentication_test.rb +++ b/test/controllers/concerns/active_insights/basic_authentication_test.rb @@ -1,9 +1,8 @@ # frozen_string_literal: true -# require "test_helper" -class ActiveInsights::BasicAuthenticationTest < ActionController::TestCase +class ActiveInsights::BasicAuthenticationTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base include ActiveInsights::BasicAuthentication @@ -30,7 +29,7 @@ def index end test "it does not require http basic auth by default" do - get :index + get "/test" assert_response :success end @@ -38,7 +37,7 @@ def index test "it requires http basic auth when enabled" do ActiveInsights.http_basic_auth_enabled = true - get :index + get "/test" assert_response :unauthorized end @@ -47,11 +46,11 @@ def index ActiveInsights.http_basic_auth_enabled = true ActiveInsights.http_basic_auth_user = "johndoe" ActiveInsights.http_basic_auth_password = "secret" - @request.env["HTTP_AUTHORIZATION"] = - ActionController::HttpAuthentication::Basic.encode_credentials("johndoe", - "secret") + auth = ActionController::HttpAuthentication::Basic.encode_credentials( + "johndoe", "secret" + ) - get :index + get "/test", headers: { "HTTP_AUTHORIZATION" => auth } assert_response :success end From 79715c2bdb424dd2edff2fe09600ae9fc97d6b26 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Thu, 12 Dec 2024 23:42:37 +0000 Subject: [PATCH 07/11] Increments version --- lib/active_insights/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_insights/version.rb b/lib/active_insights/version.rb index 5c37645..419406e 100644 --- a/lib/active_insights/version.rb +++ b/lib/active_insights/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module ActiveInsights - VERSION = "1.3.1" + VERSION = "1.4.0" end From 50c1b6a416e1f6fbcb5a10d4c63f3d553997bcae Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Sat, 14 Dec 2024 23:52:25 +0000 Subject: [PATCH 08/11] Remove debugging --- test/active_insights_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/active_insights_test.rb b/test/active_insights_test.rb index 9347bdc..b24f78b 100644 --- a/test/active_insights_test.rb +++ b/test/active_insights_test.rb @@ -2,8 +2,6 @@ require "test_helper" -puts ActiveInsights.http_basic_auth_enabled.inspect - class ActiveInsightsTest < ActiveSupport::TestCase teardown do ActiveInsights.http_basic_auth_enabled = nil From 7691f6c38536397ef7b7f8efd277ba42f5b840c2 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Sat, 14 Dec 2024 23:52:30 +0000 Subject: [PATCH 09/11] Remove devcontainer --- .devcontainer/devcontainer.json | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 .devcontainer/devcontainer.json diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json deleted file mode 100644 index 7413838..0000000 --- a/.devcontainer/devcontainer.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "image": "ruby:3.3.5", - "postCreateCommand": "bundle install", - "forwardPorts": [9292], - "customizations": { - "vscode": { - "extensions": [ - "Shopify.ruby-extensions-pack", - ] - } - } -} From 9dcfb65bc0fccd38a29b9c5722cdf89b6edbeec0 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Sun, 15 Dec 2024 00:02:32 +0000 Subject: [PATCH 10/11] Removes test controller --- .../basic_authentication_test.rb | 80 ++++++++++--------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/test/controllers/concerns/active_insights/basic_authentication_test.rb b/test/controllers/concerns/active_insights/basic_authentication_test.rb index ae47d33..45cf912 100644 --- a/test/controllers/concerns/active_insights/basic_authentication_test.rb +++ b/test/controllers/concerns/active_insights/basic_authentication_test.rb @@ -3,55 +3,57 @@ require "test_helper" class ActiveInsights::BasicAuthenticationTest < ActionDispatch::IntegrationTest + test "it does not require http basic auth by default" do + get active_insights.requests_path - class TestController < ActionController::Base - include ActiveInsights::BasicAuthentication - - def index - render plain: "Hello from TestController" - end + assert_response :success end - setup do - @controller = TestController.new - @original_routes = Rails.application.routes - - Rails.application.routes.draw do - get "test", to: "active_insights/basic_authentication_test/test#index" + test "it requires http basic auth when enabled" do + with_http_basic_auth do + get active_insights.requests_path + assert_response :unauthorized end end - teardown do - Rails.application.routes_reloader.reload! - ActiveInsights.http_basic_auth_enabled = nil - ActiveInsights.http_basic_auth_user = nil - ActiveInsights.http_basic_auth_password = nil + test "allows access with correct credentials" do + with_http_basic_auth(user: "dev", password: "secret") do + get active_insights.requests_path, headers: auth_headers("dev", "secret") + assert_response :success + end end - test "it does not require http basic auth by default" do - get "/test" - - assert_response :success + test "disallows access with incorrect credentials" do + with_http_basic_auth(user: "dev", password: "secret") do + get active_insights.requests_path, headers: auth_headers("dev", "wrong") + assert_response :unauthorized + end end - test "it requires http basic auth when enabled" do - ActiveInsights.http_basic_auth_enabled = true - - get "/test" - - assert_response :unauthorized + private + + # rubocop:disable Metrics/MethodLength + def with_http_basic_auth(enabled: true, user: nil, password: nil) + previous_enabled, ActiveInsights.http_basic_auth_enabled = + ActiveInsights.http_basic_auth_enabled, enabled + previous_user, ActiveInsights.http_basic_auth_user = + ActiveInsights.http_basic_auth_user, user + previous_password, ActiveInsights.http_basic_auth_password = + ActiveInsights.http_basic_auth_password, password + yield + ensure + ActiveInsights.http_basic_auth_enabled = previous_enabled + ActiveInsights.http_basic_auth_user = previous_user + ActiveInsights.http_basic_auth_password = previous_password end - - test "when enabled and proper credentials are included, it allows access" do - ActiveInsights.http_basic_auth_enabled = true - ActiveInsights.http_basic_auth_user = "johndoe" - ActiveInsights.http_basic_auth_password = "secret" - auth = ActionController::HttpAuthentication::Basic.encode_credentials( - "johndoe", "secret" - ) - - get "/test", headers: { "HTTP_AUTHORIZATION" => auth } - - assert_response :success + # rubocop:enable Metrics/MethodLength + + def auth_headers(user, password) + { + Authorization: + ActionController::HttpAuthentication::Basic.encode_credentials( + user, password + ) + } end end From 65e0038f05634af02a5f1cf44dbda69785c68498 Mon Sep 17 00:00:00 2001 From: Rob Cameron Date: Sun, 15 Dec 2024 00:12:24 +0000 Subject: [PATCH 11/11] Formatting --- .../concerns/active_insights/basic_authentication_test.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/controllers/concerns/active_insights/basic_authentication_test.rb b/test/controllers/concerns/active_insights/basic_authentication_test.rb index 45cf912..5420e41 100644 --- a/test/controllers/concerns/active_insights/basic_authentication_test.rb +++ b/test/controllers/concerns/active_insights/basic_authentication_test.rb @@ -12,6 +12,7 @@ class ActiveInsights::BasicAuthenticationTest < ActionDispatch::IntegrationTest test "it requires http basic auth when enabled" do with_http_basic_auth do get active_insights.requests_path + assert_response :unauthorized end end @@ -19,6 +20,7 @@ class ActiveInsights::BasicAuthenticationTest < ActionDispatch::IntegrationTest test "allows access with correct credentials" do with_http_basic_auth(user: "dev", password: "secret") do get active_insights.requests_path, headers: auth_headers("dev", "secret") + assert_response :success end end @@ -26,6 +28,7 @@ class ActiveInsights::BasicAuthenticationTest < ActionDispatch::IntegrationTest test "disallows access with incorrect credentials" do with_http_basic_auth(user: "dev", password: "secret") do get active_insights.requests_path, headers: auth_headers("dev", "wrong") + assert_response :unauthorized end end