From f151e2383c4dc1e15acd3cbe5cd3b58042bd02dc Mon Sep 17 00:00:00 2001 From: Ofri Chen Date: Wed, 27 Aug 2025 15:43:08 +0300 Subject: [PATCH 1/3] log multitenant violations --- lib/multitenant.rb | 75 +++++++++++++++-- spec/multitenant_spec.rb | 172 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 241 insertions(+), 6 deletions(-) diff --git a/lib/multitenant.rb b/lib/multitenant.rb index f045841..93a9331 100644 --- a/lib/multitenant.rb +++ b/lib/multitenant.rb @@ -9,13 +9,32 @@ class << self CURRENT_TENANT = 'Multitenant.current_tenant'.freeze ALLOW_DANGEROUS = 'Multitenant.allow_dangerous_cross_tenants'.freeze EXTRA_TENANT_IDS = 'Multitenant.extra_tenant_ids'.freeze + ALLOW_NEXT_TENANT_OPERATION = 'Multitenant.allow_next_tenant_operation'.freeze + + @@multitenant_violation_log_sample_rate = 0 + + def multitenant_violation_log_sample_rate=(sample_rate) + @@multitenant_violation_log_sample_rate = sample_rate + end + + def multitenant_violation_log_sample_rate + @@multitenant_violation_log_sample_rate + end + + def allow_next_tenant_operation + Thread.current[ALLOW_NEXT_TENANT_OPERATION] = true + end def current_tenant Thread.current[CURRENT_TENANT] end def current_tenant=(value) - Thread.current[CURRENT_TENANT] = value + if current_tenant != value && value != nil + log_multitenant_violation_if_needed('current_tenant_set') + end + + _set_current_tenant(value) end def allow_dangerous_cross_tenants @@ -23,7 +42,11 @@ def allow_dangerous_cross_tenants end def allow_dangerous_cross_tenants=(value) - Thread.current[ALLOW_DANGEROUS] = value + if value && !allow_dangerous_cross_tenants + log_multitenant_violation_if_needed('allow_dangerous_cross_tenants_set') + end + + _set_allow_dangerous_cross_tenants(value) end def extra_tenant_ids @@ -37,24 +60,64 @@ def extra_tenant_ids=(value) # execute a block scoped to the current tenant # unsets the current tenant after execution def with_tenant(tenant, options = {}, &block) + if current_tenant != tenant && tenant != nil + log_multitenant_violation_if_needed('with_tenant') + end + previous_tenant = Multitenant.current_tenant - Multitenant.current_tenant = tenant + _set_current_tenant(tenant) + previous_extra_tenant_ids = Multitenant.extra_tenant_ids Multitenant.extra_tenant_ids = options[:extra_tenant_ids] if options[:extra_tenant_ids] yield ensure - Multitenant.current_tenant = previous_tenant + _set_current_tenant(previous_tenant) Multitenant.extra_tenant_ids = previous_extra_tenant_ids end def dangerous_cross_tenants(&block) + if !allow_dangerous_cross_tenants + log_multitenant_violation_if_needed('dangerous_cross_tenants') + end + previous_value = Multitenant.allow_dangerous_cross_tenants - Multitenant.allow_dangerous_cross_tenants = true + _set_allow_dangerous_cross_tenants(true) + Multitenant.with_tenant(nil) do yield end ensure - Multitenant.allow_dangerous_cross_tenants = previous_value + _set_allow_dangerous_cross_tenants(previous_value) + end + + private + + def _set_current_tenant(value) + Thread.current[CURRENT_TENANT] = value + end + + def _set_allow_dangerous_cross_tenants(value) + Thread.current[ALLOW_DANGEROUS] = value + end + + def log_multitenant_violation_if_needed(kind) + if Thread.current[ALLOW_NEXT_TENANT_OPERATION] + Thread.current[ALLOW_NEXT_TENANT_OPERATION] = false + return + end + + return unless Random.rand < Multitenant.multitenant_violation_log_sample_rate + + $logger.warn( + tag: 'multitenant_violation', + message: 'multitenant usage outside allowed contexts', + kind: kind, + ) + rescue => e + begin + $logger.error(tag: 'multitenant_violation', message: 'error while logging multitenant violation', error: e.message) + rescue + end end end diff --git a/spec/multitenant_spec.rb b/spec/multitenant_spec.rb index ca746a9..599ccee 100644 --- a/spec/multitenant_spec.rb +++ b/spec/multitenant_spec.rb @@ -218,4 +218,176 @@ class Item < ActiveRecord::Base @user.company_id.should == @company.id end end + + describe "logging" do + let(:mock_logger) { double('logger') } + + before do + $logger = mock_logger + Multitenant.multitenant_violation_log_sample_rate = 1.0 + mock_logger.stub(:respond_to?).with(:warn).and_return(true) + mock_logger.stub(:respond_to?).with(:error).and_return(true) + end + + after do + Multitenant.multitenant_violation_log_sample_rate = 0 + $logger = nil + end + + it "current_tenant= logs violations when changing tenant" do + tenant = Company.create! :name => 'test' + + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'current_tenant_set' + ) + Multitenant.current_tenant = tenant + end + + it "current_tenant= skips logging when allow_next_tenant_operation is called" do + tenant = Company.create! :name => 'test' + Multitenant.allow_next_tenant_operation + + mock_logger.should_not_receive(:warn) + Multitenant.current_tenant = tenant + end + + it "current_tenant= logs normally after skip flag is consumed" do + tenant1 = Company.create! :name => 'test1' + tenant2 = Company.create! :name => 'test2' + + # Use skip flag first to consume it + Multitenant.allow_next_tenant_operation + Multitenant.current_tenant = tenant1 + + # Next call should log normally (proving flag was consumed) + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'current_tenant_set' + ) + Multitenant.current_tenant = tenant2 + end + + it "allow_dangerous_cross_tenants= logs violations when enabling dangerous mode" do + # Ensure initial state is false so the logging condition will be met + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'allow_dangerous_cross_tenants_set' + ) + Multitenant.allow_dangerous_cross_tenants = true + end + + it "allow_dangerous_cross_tenants= skips logging when allow_next_tenant_operation is called" do + # Ensure initial state is false so the logging condition will be met + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + Multitenant.allow_next_tenant_operation + + mock_logger.should_not_receive(:warn) + Multitenant.allow_dangerous_cross_tenants = true + end + + it "allow_dangerous_cross_tenants= logs normally after skip flag is consumed" do + # Ensure initial state is false + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + + # Use skip flag first to consume it + Multitenant.allow_next_tenant_operation + Multitenant.allow_dangerous_cross_tenants = true + + # Reset state and next call should log normally + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'allow_dangerous_cross_tenants_set' + ) + Multitenant.allow_dangerous_cross_tenants = true + end + + it "with_tenant logs violations when switching tenant" do + current_tenant = Company.create! :name => 'current' + new_tenant = Company.create! :name => 'new' + Thread.current['Multitenant.current_tenant'] = current_tenant + + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'with_tenant' + ) + Multitenant.with_tenant(new_tenant) { } + end + + it "with_tenant skips logging when allow_next_tenant_operation is called" do + current_tenant = Company.create! :name => 'current' + new_tenant = Company.create! :name => 'new' + Thread.current['Multitenant.current_tenant'] = current_tenant + Multitenant.allow_next_tenant_operation + + mock_logger.should_not_receive(:warn) + Multitenant.with_tenant(new_tenant) { } + end + + it "with_tenant logs normally after skip flag is consumed" do + current_tenant = Company.create! :name => 'current' + new_tenant1 = Company.create! :name => 'new1' + new_tenant2 = Company.create! :name => 'new2' + Thread.current['Multitenant.current_tenant'] = current_tenant + + # Use skip flag first to consume it + Multitenant.allow_next_tenant_operation + Multitenant.with_tenant(new_tenant1) { } + + # Next call should log normally + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'with_tenant' + ) + Multitenant.with_tenant(new_tenant2) { } + end + + it "dangerous_cross_tenants logs violations when entering dangerous mode" do + # Ensure allow_dangerous_cross_tenants is false so logging condition is met + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'dangerous_cross_tenants' + ) + Multitenant.dangerous_cross_tenants { } + end + + it "dangerous_cross_tenants skips logging when allow_next_tenant_operation is called" do + # Ensure allow_dangerous_cross_tenants is false so logging condition is met + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + Multitenant.allow_next_tenant_operation + + mock_logger.should_not_receive(:warn) + Multitenant.dangerous_cross_tenants { } + end + + it "dangerous_cross_tenants logs normally after skip flag is consumed" do + # Ensure allow_dangerous_cross_tenants is false so logging condition is met + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + + # Use skip flag first to consume it + Multitenant.allow_next_tenant_operation + Multitenant.dangerous_cross_tenants { } + + # Reset state and next call should log normally + Thread.current['Multitenant.allow_dangerous_cross_tenants'] = false + mock_logger.should_receive(:warn).with( + :tag => 'multitenant_violation', + :message => 'multitenant usage outside allowed contexts', + :kind => 'dangerous_cross_tenants' + ) + Multitenant.dangerous_cross_tenants { } + end + end end From c252557dd0eee7c019897f6f4ec0fb565090e360 Mon Sep 17 00:00:00 2001 From: Ofri Chen Date: Wed, 27 Aug 2025 17:47:25 +0300 Subject: [PATCH 2/3] renamed set_multitenant_violation_log_sample_rate --- lib/multitenant.rb | 2 +- spec/multitenant_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/multitenant.rb b/lib/multitenant.rb index 93a9331..5684733 100644 --- a/lib/multitenant.rb +++ b/lib/multitenant.rb @@ -13,7 +13,7 @@ class << self @@multitenant_violation_log_sample_rate = 0 - def multitenant_violation_log_sample_rate=(sample_rate) + def set_multitenant_violation_log_sample_rate(sample_rate) @@multitenant_violation_log_sample_rate = sample_rate end diff --git a/spec/multitenant_spec.rb b/spec/multitenant_spec.rb index 599ccee..0544088 100644 --- a/spec/multitenant_spec.rb +++ b/spec/multitenant_spec.rb @@ -224,13 +224,13 @@ class Item < ActiveRecord::Base before do $logger = mock_logger - Multitenant.multitenant_violation_log_sample_rate = 1.0 + Multitenant.set_multitenant_violation_log_sample_rate(1.0) mock_logger.stub(:respond_to?).with(:warn).and_return(true) mock_logger.stub(:respond_to?).with(:error).and_return(true) end after do - Multitenant.multitenant_violation_log_sample_rate = 0 + Multitenant.set_multitenant_violation_log_sample_rate(0) $logger = nil end From c0d8179ded6435fe3fac51030f05f7f01f7d95ba Mon Sep 17 00:00:00 2001 From: Ofri Chen Date: Wed, 27 Aug 2025 18:08:21 +0300 Subject: [PATCH 3/3] try add caller to the log --- lib/multitenant.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/multitenant.rb b/lib/multitenant.rb index 5684733..0526458 100644 --- a/lib/multitenant.rb +++ b/lib/multitenant.rb @@ -108,10 +108,13 @@ def log_multitenant_violation_if_needed(kind) return unless Random.rand < Multitenant.multitenant_violation_log_sample_rate + caller_frame = caller(2, 1).first rescue 'unknown' + $logger.warn( tag: 'multitenant_violation', message: 'multitenant usage outside allowed contexts', kind: kind, + caller: caller_frame ) rescue => e begin