diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4697a8e225c..80b917cf6d2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1612,7 +1612,8 @@ def set_global_session_data session[:miq_compare] = @compare.nil? ? (@keep_compare ? session[:miq_compare] : nil) : Marshal.dump(@compare) session[:miq_compressed] = @compressed unless @compressed.nil? session[:miq_exists_mode] = @exists_mode unless @exists_mode.nil? - session[:last_trans_time] = Time.now + # Update user activity time in separate memcache key + SessionActivityService.update_last_transaction_time(current_user.id) if current_user # Set timelines hash, if it is in the session for the running controller set_tl_session_data diff --git a/app/services/privilege_checker_service.rb b/app/services/privilege_checker_service.rb index 28af2b9fdf3..388b94323a5 100644 --- a/app/services/privilege_checker_service.rb +++ b/app/services/privilege_checker_service.rb @@ -1,10 +1,10 @@ class PrivilegeCheckerService def valid_session?(session, current_user) - user_signed_in?(current_user) && session_active?(session) && server_ready?(current_user) + user_signed_in?(current_user) && session_active?(current_user.id) && server_ready?(current_user) end def user_session_timed_out?(session, current_user) - user_signed_in?(current_user) && !session_active?(session) + user_signed_in?(current_user) && !session_active?(current_user.id) end private @@ -13,8 +13,9 @@ def user_signed_in?(current_user) !!current_user end - def session_active?(session) - Time.current - (session[:last_trans_time] || Time.current) <= ::Settings.session.timeout + def session_active?(user_id) + return false unless user_id + SessionActivityService.session_active?(user_id) end def server_ready?(current_user) diff --git a/app/services/session_activity_service.rb b/app/services/session_activity_service.rb new file mode 100644 index 00000000000..d904b5634dc --- /dev/null +++ b/app/services/session_activity_service.rb @@ -0,0 +1,78 @@ +class SessionActivityService + # Key prefix for storing session activity data in memcache + SESSION_ACTIVITY_PREFIX = "user_activity:".freeze + + # Class variables to cache the Dalli client instance + @@dalli_client = nil + @@dalli_client_mutex = Mutex.new + + # Get the last transaction time for a user + # @param user_id [String, Integer] The user ID + # @return [Time, nil] The last transaction time or nil if not found + def self.get_last_transaction_time(user_id) + return nil unless user_id + + value = dalli_client.get(activity_key(user_id)) + value ? Time.parse(value) : nil + end + + # Update the last transaction time for a user + # @param user_id [String, Integer] The user ID + # @param time [Time] The time to set (defaults to current time) + def self.update_last_transaction_time(user_id, time = Time.current) + return unless user_id + + dalli_client.set(activity_key(user_id), time.iso8601, ttl) + end + + # Check if a user session is active based on last transaction time + # @param user_id [String, Integer] The user ID + # @return [Boolean] True if the session is active, false otherwise + def self.session_active?(user_id) + last_time = get_last_transaction_time(user_id) + return false unless last_time + + Time.current - last_time <= ::Settings.session.timeout + end + + # Generate the key for storing user activity data + # @param user_id [String, Integer] The user ID + # @return [String] The key for memcache + def self.activity_key(user_id) + "#{SESSION_ACTIVITY_PREFIX}#{user_id}" + end + + # Get a cached Dalli client for interacting with memcache + # Thread-safe implementation using a mutex to ensure only one client is created + # @return [Dalli::Client] A configured Dalli client + def self.dalli_client + # Return the cached client if it exists + return @@dalli_client if @@dalli_client + + # Use a mutex to ensure thread safety when creating the client + @@dalli_client_mutex.synchronize do + # Check again inside the mutex in case another thread created the client + return @@dalli_client if @@dalli_client + + require 'dalli' + options = { + :namespace => "MIQ:USER_ACTIVITY", + :expires_in => ttl + } + + @@dalli_client = MiqMemcached.client(options) + end + + @@dalli_client + end + + # Get the TTL for user activity data + # @return [Integer] The TTL in seconds + def self.ttl + # Set TTL to double the session timeout to ensure we keep the data + # long enough for proper timeout detection + (::Settings.session.timeout * 2).to_i + end + + private_class_method :activity_key, :dalli_client, :ttl +end diff --git a/spec/services/privilege_checker_service_spec.rb b/spec/services/privilege_checker_service_spec.rb index 60cde28a2b7..291e1a2ba4a 100644 --- a/spec/services/privilege_checker_service_spec.rb +++ b/spec/services/privilege_checker_service_spec.rb @@ -1,93 +1,74 @@ describe PrivilegeCheckerService do - let(:privilege_checker) { described_class.new } + let(:service) { described_class.new } + let(:user) { double("User", :id => 123, :super_admin_user? => false) } + let(:session) { {} } + let(:server) { double("MiqServer", :logon_status => :ready) } - describe "#valid_session?" do - shared_examples_for "PrivilegeCheckerService#valid_session? that returns false" do - it "returns false" do - expect(privilege_checker.valid_session?(session, user)).to be_falsey - end - end + before do + allow(MiqServer).to receive(:my_server).and_return(server) + end - let(:session) do - { - :last_trans_time => last_trans_time - } - end + describe '#valid_session?' do + context 'when user is signed in' do + context 'when session is active' do + before do + allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(true) + end - context "when the user is signed out" do - let(:user) { nil } - let(:last_trans_time) { nil } + it 'returns true when server is ready' do + expect(service.valid_session?(session, user)).to be true + end - it_behaves_like "PrivilegeCheckerService#valid_session? that returns false" - end + it 'returns true when user is super admin' do + allow(user).to receive(:super_admin_user?).and_return(true) + allow(server).to receive(:logon_status).and_return(:not_ready) - context "when the user is signed in" do - let(:user) { FactoryBot.create(:user) } + expect(service.valid_session?(session, user)).to be true + end - context "when the session is timed out" do - let(:last_trans_time) { 2.hours.ago } + it 'returns false when server is not ready' do + allow(server).to receive(:logon_status).and_return(:not_ready) - it_behaves_like "PrivilegeCheckerService#valid_session? that returns false" + expect(service.valid_session?(session, user)).to be false + end end - context "when the session has not timed out" do - let(:last_trans_time) { Time.current } - let(:server) { double("MiqServer", :logon_status => logon_status) } - + context 'when session is not active' do before do - allow(MiqServer).to receive(:my_server).and_return(server) + allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(false) end - context "when the server is not ready" do - let(:logon_status) { :not_ready } - - it_behaves_like "PrivilegeCheckerService#valid_session? that returns false" - end - - context "when the server is ready" do - let(:logon_status) { :ready } - - it "returns true" do - expect(privilege_checker.valid_session?(session, user)).to be_truthy - end + it 'returns false' do + expect(service.valid_session?(session, user)).to be false end end end - end - describe "#user_session_timed_out?" do - let(:session) do - { - :last_trans_time => last_trans_time - } + context 'when user is not signed in' do + it 'returns false' do + expect(service.valid_session?(session, nil)).to be false + end end + end - context "when a user exists" do - let(:user) { FactoryBot.create(:user) } - - context "when the session is timed out" do - let(:last_trans_time) { 2.hours.ago } + describe '#user_session_timed_out?' do + context 'when user is signed in' do + it 'returns true when session is not active' do + allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(false) - it "returns true" do - expect(privilege_checker.user_session_timed_out?(session, user)).to be_truthy - end + expect(service.user_session_timed_out?(session, user)).to be true end - context "when the session has not timed out" do - let(:last_trans_time) { Time.current } + it 'returns false when session is active' do + allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(true) - it "returns false" do - expect(privilege_checker.user_session_timed_out?(session, user)).to be_falsey - end + expect(service.user_session_timed_out?(session, user)).to be false end end - context "when a user does not exist" do - let(:user) { nil } - let(:last_trans_time) { nil } - - it "returns false" do - expect(privilege_checker.user_session_timed_out?(session, user)).to be_falsey + context 'when user is not signed in' do + it 'returns false' do + expect(service.user_session_timed_out?(session, nil)).to be false end end end diff --git a/spec/services/session_activity_service_spec.rb b/spec/services/session_activity_service_spec.rb new file mode 100644 index 00000000000..c8c0519ba54 --- /dev/null +++ b/spec/services/session_activity_service_spec.rb @@ -0,0 +1,88 @@ +describe SessionActivityService do + let(:user_id) { 123 } + let(:current_time) { Time.zone.local(2025, 10, 30, 14, 30, 0) } + let(:mock_client) { double("Dalli::Client") } + + before do + Timecop.freeze(current_time) + # Reset the cached client before each test + SessionActivityService.instance_variable_set(:@dalli_client, nil) + # Mock the Dalli client to avoid actual memcache calls in tests + allow(MiqMemcached).to receive(:client).and_return(mock_client) + end + + after do + Timecop.return + end + + describe '.update_last_transaction_time' do + it 'stores the current time for a user' do + expect(mock_client).to receive(:set).with( + "user_activity:#{user_id}", + current_time.iso8601, + (::Settings.session.timeout * 2).to_i + ) + + described_class.update_last_transaction_time(user_id) + end + + it 'does nothing when user_id is nil' do + expect(mock_client).not_to receive(:set) + described_class.update_last_transaction_time(nil) + end + end + + describe '.get_last_transaction_time' do + it 'retrieves the stored time for a user' do + stored_time = current_time - 5.minutes + expect(mock_client).to receive(:get).with("user_activity:#{user_id}").and_return(stored_time.iso8601) + + result = described_class.get_last_transaction_time(user_id) + expect(result).to be_within(1.second).of(stored_time) + end + + it 'returns nil when no time is stored' do + expect(mock_client).to receive(:get).with("user_activity:#{user_id}").and_return(nil) + + result = described_class.get_last_transaction_time(user_id) + expect(result).to be_nil + end + + it 'returns nil when user_id is nil' do + expect(mock_client).not_to receive(:get) + + result = described_class.get_last_transaction_time(nil) + expect(result).to be_nil + end + end + + describe '.session_active?' do + context 'when session is active' do + it 'returns true when last activity is within timeout period' do + stored_time = current_time - (::Settings.session.timeout / 2) + allow(described_class).to receive(:get_last_transaction_time).with(user_id).and_return(stored_time) + + expect(described_class.session_active?(user_id)).to be true + end + end + + context 'when session is inactive' do + it 'returns false when last activity is beyond timeout period' do + stored_time = current_time - (::Settings.session.timeout + 1.minute) + allow(described_class).to receive(:get_last_transaction_time).with(user_id).and_return(stored_time) + + expect(described_class.session_active?(user_id)).to be false + end + + it 'returns false when no activity is recorded' do + allow(described_class).to receive(:get_last_transaction_time).with(user_id).and_return(nil) + + expect(described_class.session_active?(user_id)).to be false + end + + it 'returns false when user_id is nil' do + expect(described_class.session_active?(nil)).to be false + end + end + end +end