From b791d8e0216bc70e4328f42ffad5c05adbafe7f6 Mon Sep 17 00:00:00 2001 From: Benjamin Ortiz Date: Thu, 13 Jun 2024 11:57:55 -0700 Subject: [PATCH] configuration to disable api key support (#123) ## Problem A user cannot use all the features of the library without also using the API Key auth features. ## Solution ```ruby Stitches.configure do |config| config.disable_api_key_support = true end ``` --- lib/stitches/api_key.rb | 2 + lib/stitches/configuration.rb | 3 +- spec/api_key_middleware_spec.rb | 482 +++++++++++++++++--------------- spec/configuration_spec.rb | 4 + 4 files changed, 265 insertions(+), 226 deletions(-) diff --git a/lib/stitches/api_key.rb b/lib/stitches/api_key.rb index 071c18a..71dde40 100644 --- a/lib/stitches/api_key.rb +++ b/lib/stitches/api_key.rb @@ -23,6 +23,8 @@ class ApiKey < Stitches::AllowlistMiddleware protected def do_call(env) + return @app.call(env) if Stitches.configuration.disable_api_key_support + authorization = env["HTTP_AUTHORIZATION"] if authorization if authorization =~ /#{configuration.custom_http_auth_scheme}\s+key=(.*)\s*$/ diff --git a/lib/stitches/configuration.rb b/lib/stitches/configuration.rb index 2669caf..e3fb56f 100644 --- a/lib/stitches/configuration.rb +++ b/lib/stitches/configuration.rb @@ -17,9 +17,10 @@ def reset_to_defaults! @max_cache_size = NonNullInteger.new("max_cache_size", 0) @disabled_key_leniency_in_seconds = ActiveSupport::Duration.days(3) @disabled_key_leniency_error_log_threshold_in_seconds = ActiveSupport::Duration.days(2) + @disable_api_key_support = false end - attr_accessor :disabled_key_leniency_in_seconds, :disabled_key_leniency_error_log_threshold_in_seconds + attr_accessor :disabled_key_leniency_in_seconds, :disabled_key_leniency_error_log_threshold_in_seconds, :disable_api_key_support # A RegExp that allows URLS around the mime type and api key requirements. # nil means that ever request must have a proper mime type and api key. diff --git a/spec/api_key_middleware_spec.rb b/spec/api_key_middleware_spec.rb index 65b8933..d469c9b 100644 --- a/spec/api_key_middleware_spec.rb +++ b/spec/api_key_middleware_spec.rb @@ -12,13 +12,6 @@ def warn(message) let(:auth_header) { "MyAwesomeInternalScheme key=#{uuid}" } let(:allowlist) { nil } - before do - Stitches.configuration.reset_to_defaults! - Stitches.configuration.custom_http_auth_scheme = 'MyAwesomeInternalScheme' - Stitches.configuration.allowlist_regexp = allowlist if allowlist - Stitches::ApiClientAccessWrapper.clear_api_cache - end - def execute_call(auth: auth_header) headers = { "Accept" => "application/json; version=1" @@ -34,342 +27,381 @@ def expect_unauthorized expect(response.headers["WWW-Authenticate"]).to eq("MyAwesomeInternalScheme realm=FakeApp") end - context "with modern schema" do - let(:api_client_enabled) { true } - let(:disabled_at) { nil } - let!(:api_client) { - uuid = SecureRandom.uuid - ApiClient.create(name: "MyApiClient", key: SecureRandom.uuid, enabled: false, created_at: 20.days.ago, disabled_at: 15.days.ago) - ApiClient.create(name: "MyApiClient", key: uuid, enabled: api_client_enabled, created_at: 10.days.ago, disabled_at: disabled_at) - } - - context "when path is not on allowlist" do - context "when api_client is valid" do - it "executes the correct controller" do - execute_call - - expect(response.body).to include "Hello" - end - - it "saves the api_client information used" do - execute_call - - expect(response.body).to include "MyApiClient" - expect(response.body).to include "#{api_client.id}" - end + context "disabled api key support" do + before do + Stitches.configuration.reset_to_defaults! + Stitches.configuration.custom_http_auth_scheme = 'MyAwesomeInternalScheme' + Stitches.configuration.disable_api_key_support = true + end - context "caching is enabled" do - before do - allow(ApiClient).to receive(:find_by).and_call_original + let(:uuid) { SecureRandom.uuid } - Stitches.configure do |config| - config.max_cache_ttl = 5 - config.max_cache_size = 10 - end - end + it "executes the correct controller" do + execute_call - it "only gets the the api_client information once" do - execute_call - execute_call - - expect(response.body).to include "#{api_client.id}" - expect(ApiClient).to have_received(:find_by).once - end - end - end - - context "when api client key does not match" do - let(:uuid) { SecureRandom.uuid } # random uuid + expect(response.body).to include "Hello" + end - it "rejects request" do - execute_call + it "does not look up information from the database" do + execute_call - expect_unauthorized - end - end + expect(response.body).to include "NameNotFound" + expect(response.body).to include "IdNotFound" + end - context "when api client key not enabled" do - let(:api_client_enabled) { false } + it "indicates a successful response" do + execute_call - context "when disabled_at is not set" do - it "rejects request" do - execute_call + expect(response.status).to eq 200 + end + end - expect_unauthorized - end - end + context "enabled api key support" do + before do + Stitches.configuration.reset_to_defaults! + Stitches.configuration.custom_http_auth_scheme = 'MyAwesomeInternalScheme' + Stitches.configuration.allowlist_regexp = allowlist if allowlist + Stitches.configuration.disable_api_key_support = false + Stitches::ApiClientAccessWrapper.clear_api_cache + end - context "when disabled_at is set to a time older than three days ago" do - let(:disabled_at) { 4.day.ago } + context "with modern schema" do + let(:api_client_enabled) { true } + let(:disabled_at) { nil } + let!(:api_client) { + uuid = SecureRandom.uuid + ApiClient.create(name: "MyApiClient", key: SecureRandom.uuid, enabled: false, created_at: 20.days.ago, disabled_at: 15.days.ago) + ApiClient.create(name: "MyApiClient", key: uuid, enabled: api_client_enabled, created_at: 10.days.ago, disabled_at: disabled_at) + } - it "does not allow the call" do + context "when path is not on allowlist" do + context "when api_client is valid" do + it "executes the correct controller" do execute_call - expect_unauthorized - + expect(response.body).to include "Hello" end - end - context "when disabled_at is set to a recent time" do - let(:disabled_at) { 1.day.ago } - - it "allows the call" do + it "saves the api_client information used" do execute_call - expect(response.body).to include "Hello" expect(response.body).to include "MyApiClient" expect(response.body).to include "#{api_client.id}" end - it "warns about the disabled key to log writer when available" do - stub_const("StitchFix::Logger::LogWriter", FakeLogger.new) - allow(StitchFix::Logger::LogWriter).to receive(:warn) + context "caching is enabled" do + before do + allow(ApiClient).to receive(:find_by).and_call_original - execute_call + Stitches.configure do |config| + config.max_cache_ttl = 5 + config.max_cache_size = 10 + end + end - expect(StitchFix::Logger::LogWriter).to have_received(:warn).once + it "only gets the the api_client information once" do + execute_call + execute_call + + expect(response.body).to include "#{api_client.id}" + expect(ApiClient).to have_received(:find_by).once + end end + end - it "warns about the disabled key to the Rails.logger" do - allow(Rails.logger).to receive(:warn) - allow(Rails.logger).to receive(:error) + context "when api client key does not match" do + let(:uuid) { SecureRandom.uuid } # random uuid + it "rejects request" do execute_call - expect(Rails.logger).to have_received(:warn).once - expect(Rails.logger).not_to have_received(:error) + expect_unauthorized end end - context "when disabled_at is set to a dangerously long time" do - let(:disabled_at) { 52.hours.ago } + context "when api client key not enabled" do + let(:api_client_enabled) { false } - it "allows the call" do - execute_call + context "when disabled_at is not set" do + it "rejects request" do + execute_call - expect(response.body).to include "Hello" - expect(response.body).to include "MyApiClient" - expect(response.body).to include "#{api_client.id}" + expect_unauthorized + end end - it "logs error about the disabled key to log writer when available" do - stub_const("StitchFix::Logger::LogWriter", FakeLogger.new) - allow(StitchFix::Logger::LogWriter).to receive(:error) + context "when disabled_at is set to a time older than three days ago" do + let(:disabled_at) { 4.day.ago } - execute_call + it "does not allow the call" do + execute_call - expect(StitchFix::Logger::LogWriter).to have_received(:error).once - end + expect_unauthorized - it "logs error about the disabled key to the Rails.logger" do - allow(Rails.logger).to receive(:warn) - allow(Rails.logger).to receive(:error) do |message1| - expect(message1).not_to include uuid end + end - execute_call + context "when disabled_at is set to a recent time" do + let(:disabled_at) { 1.day.ago } - expect(Rails.logger).to have_received(:error).once - expect(Rails.logger).not_to have_received(:warn) - end - end + it "allows the call" do + execute_call - context "when disabled_at is set to an unacceptably long time" do - let(:disabled_at) { 5.days.ago } + expect(response.body).to include "Hello" + expect(response.body).to include "MyApiClient" + expect(response.body).to include "#{api_client.id}" + end - it "forbids the call" do - execute_call + it "warns about the disabled key to log writer when available" do + stub_const("StitchFix::Logger::LogWriter", FakeLogger.new) + allow(StitchFix::Logger::LogWriter).to receive(:warn) - expect_unauthorized - end + execute_call - it "logs error about the disabled key to log writer when available" do - stub_const("StitchFix::Logger::LogWriter", FakeLogger.new) - allow(StitchFix::Logger::LogWriter).to receive(:error) + expect(StitchFix::Logger::LogWriter).to have_received(:warn).once + end - execute_call + it "warns about the disabled key to the Rails.logger" do + allow(Rails.logger).to receive(:warn) + allow(Rails.logger).to receive(:error) + + execute_call - expect(StitchFix::Logger::LogWriter).to have_received(:error).once + expect(Rails.logger).to have_received(:warn).once + expect(Rails.logger).not_to have_received(:error) + end end - it "logs error about the disabled key to the Rails.logger" do - allow(Rails.logger).to receive(:warn) - allow(Rails.logger).to receive(:error) + context "when disabled_at is set to a dangerously long time" do + let(:disabled_at) { 52.hours.ago } + + it "allows the call" do + execute_call - execute_call + expect(response.body).to include "Hello" + expect(response.body).to include "MyApiClient" + expect(response.body).to include "#{api_client.id}" + end - expect(Rails.logger).to have_received(:error).once - expect(Rails.logger).not_to have_received(:warn) - end - end + it "logs error about the disabled key to log writer when available" do + stub_const("StitchFix::Logger::LogWriter", FakeLogger.new) + allow(StitchFix::Logger::LogWriter).to receive(:error) - context "custom leniency is set" do - before do - Stitches.configuration.disabled_key_leniency_in_seconds = 100 - Stitches.configuration.disabled_key_leniency_error_log_threshold_in_seconds = 50 - end + execute_call - context "when disabled_at is set to an unacceptably long time" do - let(:disabled_at) { 101.seconds.ago } + expect(StitchFix::Logger::LogWriter).to have_received(:error).once + end - it "forbids the call" do + it "logs error about the disabled key to the Rails.logger" do + allow(Rails.logger).to receive(:warn) allow(Rails.logger).to receive(:error) do |message1| expect(message1).not_to include uuid end execute_call - expect_unauthorized expect(Rails.logger).to have_received(:error).once + expect(Rails.logger).not_to have_received(:warn) end end - context "when disabled_at is set to a dangerously long time" do - let(:disabled_at) { 75.seconds.ago } + context "when disabled_at is set to an unacceptably long time" do + let(:disabled_at) { 5.days.ago } - it "allows the call" do + it "forbids the call" do + execute_call + + expect_unauthorized + end + + it "logs error about the disabled key to log writer when available" do + stub_const("StitchFix::Logger::LogWriter", FakeLogger.new) + allow(StitchFix::Logger::LogWriter).to receive(:error) + + execute_call + + expect(StitchFix::Logger::LogWriter).to have_received(:error).once + end + + it "logs error about the disabled key to the Rails.logger" do + allow(Rails.logger).to receive(:warn) allow(Rails.logger).to receive(:error) execute_call - expect(response.body).to include "Hello" expect(Rails.logger).to have_received(:error).once + expect(Rails.logger).not_to have_received(:warn) end end - context "when disabled_at is set to a short time ago" do - let(:disabled_at) { 25.seconds.ago } + context "custom leniency is set" do + before do + Stitches.configuration.disabled_key_leniency_in_seconds = 100 + Stitches.configuration.disabled_key_leniency_error_log_threshold_in_seconds = 50 + end - it "allows the call" do - allow(Rails.logger).to receive(:warn) do |message1| - expect(message1).not_to include uuid + context "when disabled_at is set to an unacceptably long time" do + let(:disabled_at) { 101.seconds.ago } + + it "forbids the call" do + allow(Rails.logger).to receive(:error) do |message1| + expect(message1).not_to include uuid + end + + execute_call + + expect_unauthorized + expect(Rails.logger).to have_received(:error).once end + end - execute_call + context "when disabled_at is set to a dangerously long time" do + let(:disabled_at) { 75.seconds.ago } - expect(response.body).to include "Hello" - expect(Rails.logger).to have_received(:warn).once + it "allows the call" do + allow(Rails.logger).to receive(:error) + + execute_call + + expect(response.body).to include "Hello" + expect(Rails.logger).to have_received(:error).once + end + end + + context "when disabled_at is set to a short time ago" do + let(:disabled_at) { 25.seconds.ago } + + it "allows the call" do + allow(Rails.logger).to receive(:warn) do |message1| + expect(message1).not_to include uuid + end + + execute_call + + expect(response.body).to include "Hello" + expect(Rails.logger).to have_received(:warn).once + end end end end - end - context "when authorization header is missing" do - it "rejects request" do - execute_call(auth: nil) + context "when authorization header is missing" do + it "rejects request" do + execute_call(auth: nil) - expect_unauthorized + expect_unauthorized + end + end + + context "when scheme does not match" do + it "rejects request" do + execute_call(auth: "OtherScheme key=#{uuid}") + + expect_unauthorized + end end end - context "when scheme does not match" do - it "rejects request" do - execute_call(auth: "OtherScheme key=#{uuid}") + context "when path is on allowlist" do + let(:allowlist) { /.*hello.*/ } - expect_unauthorized + context "when api_client is valid" do + it "executes the correct controller" do + execute_call + + expect(response.body).to include "Hello" + end + + it "does not save the api_client information used" do + execute_call + + expect(response.body).to include "NameNotFound" + expect(response.body).to include "IdNotFound" + end + end + + context "when api client key does not match" do + let(:uuid) { SecureRandom.uuid } # random uuid + + it "executes the correct controller" do + execute_call + + expect(response.body).to include "Hello" + end end end end - context "when path is on allowlist" do - let(:allowlist) { /.*hello.*/ } + context "when schema is old and missing disabled_at field" do + around(:each) do |example| + load 'fake_app/db/schema_missing_disabled_at.rb' + ApiClient.reset_column_information + example.run + load 'fake_app/db/schema_modern.rb' + ApiClient.reset_column_information + end context "when api_client is valid" do + let!(:api_client) { + uuid = SecureRandom.uuid + ApiClient.create(name: "MyApiClient", key: uuid, created_at: Time.now(), enabled: true) + } + it "executes the correct controller" do execute_call expect(response.body).to include "Hello" end - it "does not save the api_client information used" do + it "saves the api_client information used" do execute_call - expect(response.body).to include "NameNotFound" - expect(response.body).to include "IdNotFound" + expect(response.body).to include "MyApiClient" + expect(response.body).to include "#{api_client.id}" end end - context "when api client key does not match" do - let(:uuid) { SecureRandom.uuid } # random uuid + context "when api_client is not enabled" do + let!(:api_client) { + uuid = SecureRandom.uuid + ApiClient.create(name: "MyApiClient", key: uuid, created_at: Time.now(), enabled: false) + } - it "executes the correct controller" do + it "rejects request" do execute_call - expect(response.body).to include "Hello" + expect_unauthorized end end end - end - - context "when schema is old and missing disabled_at field" do - around(:each) do |example| - load 'fake_app/db/schema_missing_disabled_at.rb' - ApiClient.reset_column_information - example.run - load 'fake_app/db/schema_modern.rb' - ApiClient.reset_column_information - end - - context "when api_client is valid" do - let!(:api_client) { - uuid = SecureRandom.uuid - ApiClient.create(name: "MyApiClient", key: uuid, created_at: Time.now(), enabled: true) - } - - it "executes the correct controller" do - execute_call - expect(response.body).to include "Hello" + context "when schema is old and missing enabled field" do + around(:each) do |example| + load 'fake_app/db/schema_missing_enabled.rb' + ApiClient.reset_column_information + example.run + load 'fake_app/db/schema_modern.rb' + ApiClient.reset_column_information end - it "saves the api_client information used" do - execute_call - - expect(response.body).to include "MyApiClient" - expect(response.body).to include "#{api_client.id}" - end - end - - context "when api_client is not enabled" do let!(:api_client) { uuid = SecureRandom.uuid - ApiClient.create(name: "MyApiClient", key: uuid, created_at: Time.now(), enabled: false) + ApiClient.create(name: "MyApiClient", key: uuid, created_at: Time.now()) } - it "rejects request" do - execute_call - - expect_unauthorized - end - end - end - - context "when schema is old and missing enabled field" do - around(:each) do |example| - load 'fake_app/db/schema_missing_enabled.rb' - ApiClient.reset_column_information - example.run - load 'fake_app/db/schema_modern.rb' - ApiClient.reset_column_information - end - - let!(:api_client) { - uuid = SecureRandom.uuid - ApiClient.create(name: "MyApiClient", key: uuid, created_at: Time.now()) - } - - context "when api_client is valid" do - it "executes the correct controller" do - execute_call + context "when api_client is valid" do + it "executes the correct controller" do + execute_call - expect(response.body).to include "Hello" - end + expect(response.body).to include "Hello" + end - it "saves the api_client information used" do - execute_call + it "saves the api_client information used" do + execute_call - expect(response.body).to include "MyApiClient" - expect(response.body).to include "#{api_client.id}" + expect(response.body).to include "MyApiClient" + expect(response.body).to include "#{api_client.id}" + end end end end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 951fdb5..d75e65a 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -44,6 +44,10 @@ expect(Stitches.configuration.max_cache_size).to eq(0) end + it "sets a default of false for disable_api_key_support" do + expect(Stitches.configuration.disable_api_key_support).to be false + end + it "blows up if you try to use custom_http_auth_scheme without having set it" do expect { Stitches.configuration.custom_http_auth_scheme