Skip to content

Add thread safety #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add thread safety #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions lib/rls_rails/helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ module RLS
# This variable is very problematic and not relieable, since in a
# threaded environment of a connection pool this module is changed and
# race conditions can occur, de-syncing the module-status with the true db status.
@rls_status = {user_id: '', tenant_id: '', disabled: ''}

def self.disable!
return if RLS.status[:disable] === 'true' # do not use disabled? here since it may be blank

clear_query_cache
execute_sql("SET SESSION rls.disable = TRUE;")
@rls_status.merge!(disabled: 'true')
thread_rls_status.merge!(disabled: 'true')
debug_print "WARNING: ROW LEVEL SECURITY DISABLED!\n"
end

Expand All @@ -27,7 +25,7 @@ def self.enable!
clear_query_cache
debug_print "ROW LEVEL SECURITY ENABLED!\n"
execute_sql("SET SESSION rls.disable = FALSE;")
@rls_status.merge!(disabled: 'false')
thread_rls_status.merge!(disabled: 'false')
end

def self.enabled?
Expand All @@ -41,7 +39,7 @@ def self.set_tenant tenant
clear_query_cache
debug_print "Accessing database as #{tenant.name}\n"
execute_sql "SET SESSION rls.disable = FALSE; SET SESSION rls.tenant_id = #{tenant.id};"
@rls_status.merge!(tenant_id: tenant.id.to_s)
thread_rls_status.merge!(tenant_id: tenant.id.to_s)
end

def self.set_user user
Expand All @@ -51,7 +49,7 @@ def self.set_user user
clear_query_cache
debug_print "Accessing database as #{user.class}##{user.id}\n"
execute_sql "SET SESSION rls.disable = FALSE; SET SESSION rls.user_id = #{user.id};"
@rls_status.merge!(user_id: user.id.to_s)
thread_rls_status.merge!(user_id: user.id.to_s)
end

def self.current_tenant_id
Expand All @@ -71,7 +69,7 @@ def self.reset!
RESET rls.disable;
SQL
clear_query_cache
@rls_status.merge!(tenant_id: '', user_id: '', disabled: '')
thread_rls_status.merge!(tenant_id: '', user_id: '', disabled: '')
end

# Sets the RLS status to the given value in one go.
Expand All @@ -89,7 +87,7 @@ def self.status= status
SET SESSION rls.user_id = '#{user_id}';
SET SESSION rls.tenant_id = '#{tenant_id}';
SQL
@rls_status.merge!(tenant_id: tenant_id, user_id: user_id, disabled: disable)
thread_rls_status.merge!(tenant_id: tenant_id, user_id: user_id, disabled: disable)
end

# @return [Hash] Values of the current RLS sesssion
Expand Down Expand Up @@ -171,4 +169,8 @@ def self.execute_sql query
def self.debug_print s
print s if Railtie.config.rls_rails.verbose
end
end

def self.thread_rls_status
Thread.current["rls_status"] ||= { user_id: '', tenant_id: '', disabled: '' }
end
end
14 changes: 14 additions & 0 deletions lib/rls_rails/railtie.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ class Railtie < ::Rails::Railtie
ActiveRecord::Migration.include RLS::Statements
#ActiveRecord::Migration::CommandRecorder.include Scenic::CommandRecorder
#ActiveRecord::SchemaDumper.prepend Scenic::SchemaDumper

ActiveRecord::Base.connection.class.set_callback :checkout, :after do
# ensure the RLS-related session variables are reset when a
# thread checks out a connection
execute <<~SQL
RESET rls.user_id;
RESET rls.tenant_id;
RESET rls.disable;
SQL

clear_query_cache

RLS.thread_rls_status.merge!(tenant_id: '', user_id: '', disabled: '')
Comment on lines +26 to +34
Copy link
Member

@sbiastoch sbiastoch Apr 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to reuse here the existing RLS.reset! method (maybe with minor modifications) to not have this code duplicated here?

end
end
end

Expand Down