diff --git a/app/controllers/user_settings/active_sessions_controller.rb b/app/controllers/user_settings/active_sessions_controller.rb index bfc969d0ff8a809607ad305f5c27ac5e2cf813f4..34359eb0080844a67fab96c7cee198886d6b29c5 100644 --- a/app/controllers/user_settings/active_sessions_controller.rb +++ b/app/controllers/user_settings/active_sessions_controller.rb @@ -20,3 +20,5 @@ def destroy end end end + +UserSettings::ActiveSessionsController.prepend_mod diff --git a/ee/app/controllers/ee/user_settings/active_sessions_controller.rb b/ee/app/controllers/ee/user_settings/active_sessions_controller.rb new file mode 100644 index 0000000000000000000000000000000000000000..0b4d3ae4d0075bf79dcf9adb7ae52c279310b98d --- /dev/null +++ b/ee/app/controllers/ee/user_settings/active_sessions_controller.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module EE + module UserSettings + module ActiveSessionsController + # dotcom-focused endpoint to return time remaining on existing SAML sessions + # since we want only session data for current device / browser, this endpoint must be + # in a regular app controller, not in the Grape API. client-side JS does not have access to + # _gitlab_session_abc123 cookie + def saml + session_info = ::Gitlab::Auth::GroupSaml::SsoEnforcer.sessions_time_remaining_for_expiry + + session_info = session_info.map do |item| + time_remaining = item[:time_remaining].in_milliseconds.to_i + time_remaining = 0 if time_remaining <= 0 + + { + provider_id: item[:provider_id], + time_remaining_ms: time_remaining + } + end + render json: session_info + end + end + end +end diff --git a/ee/config/routes/user_settings.rb b/ee/config/routes/user_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..5152dbd9cc479da38082011893034df661e73ecf --- /dev/null +++ b/ee/config/routes/user_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +namespace :user_settings do + resources :active_sessions, only: [] do + collection do + get :saml, format: :json + end + end +end diff --git a/ee/lib/gitlab/auth/group_saml/sso_enforcer.rb b/ee/lib/gitlab/auth/group_saml/sso_enforcer.rb index 4480588a7b0541ecdfd3388afbc7c236c472441b..c43c325fa18d40923a31a4d0e2d27719e3545184 100644 --- a/ee/lib/gitlab/auth/group_saml/sso_enforcer.rb +++ b/ee/lib/gitlab/auth/group_saml/sso_enforcer.rb @@ -7,6 +7,15 @@ class SsoEnforcer DEFAULT_SESSION_TIMEOUT = 1.day class << self + def sessions_time_remaining_for_expiry + SsoState.active_saml_sessions.map do |id, last_sign_in_at| + expires_at = last_sign_in_at + DEFAULT_SESSION_TIMEOUT + # expires_at is DateTime; convert to Time; Time - Time yields a Float + time_remaining_for_expiry = expires_at.to_time - Time.current + { provider_id: id, time_remaining: time_remaining_for_expiry } + end + end + def access_restricted?(user:, resource:, session_timeout: DEFAULT_SESSION_TIMEOUT) group = resource.is_a?(::Group) ? resource : resource.group diff --git a/ee/lib/gitlab/auth/group_saml/sso_state.rb b/ee/lib/gitlab/auth/group_saml/sso_state.rb index 2a8e410dfd68f72a9416ce79e9b619055f17257e..a33f73b0e564aec71b0d1612a3460be0a13d152a 100644 --- a/ee/lib/gitlab/auth/group_saml/sso_state.rb +++ b/ee/lib/gitlab/auth/group_saml/sso_state.rb @@ -6,6 +6,10 @@ module GroupSaml class SsoState SESSION_STORE_KEY = :active_group_sso_sign_ins + def self.active_saml_sessions + Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY).to_h + end + attr_reader :saml_provider_id def initialize(saml_provider_id) diff --git a/ee/spec/controllers/user_settings/active_sessions_controller_spec.rb b/ee/spec/controllers/user_settings/active_sessions_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e2d3d6262ccba96b72478d97b5eb7d84e083a9b4 --- /dev/null +++ b/ee/spec/controllers/user_settings/active_sessions_controller_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require('spec_helper') + +RSpec.describe UserSettings::ActiveSessionsController, feature_category: :system_access do + let_it_be(:user) { create(:user) } + + describe '/saml' do + subject(:get_saml) { get :saml } + + let(:saml_provider) { create(:saml_provider) } + let(:other_saml_provider) { create(:saml_provider) } + let(:first_saml_sign_in) { Time.utc(2024, 2, 2, 1, 44) } + let(:json_response) { Gitlab::Json.parse(response.body).map(&:with_indifferent_access) } + + around do |ex| + travel_to(first_saml_sign_in + 6.hours) { ex.run } + end + + it 'responds with 404' do + get_saml + + expect(response).to redirect_to(new_user_session_path) + end + + context 'with signed-in user' do + before do + sign_in(user) + end + + context 'with no current SAML sessions' do + it 'responds with empty array' do + get_saml + + expect(json_response).to eq([]) + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'with SAML sign-in session data' do + # mimic the actions of ::Gitlab::Auth::GroupSaml::SsoState.update_active, but + # ensure it is hooked into the controller test session rather than initializing its + # own thread-local session + before do + key = ::Gitlab::Auth::GroupSaml::SsoState::SESSION_STORE_KEY + store = Gitlab::NamespacedSessionStore.new(key, session) + store[saml_provider.id] = first_saml_sign_in + store[other_saml_provider.id] = (first_saml_sign_in + 4.hours) + end + + it 'responds with JSON of current SAML sessions' do + get_saml + + expect(json_response).to match_array( + [ + { + provider_id: saml_provider.id, + time_remaining_ms: 18.hours.in_milliseconds + }, + { + provider_id: other_saml_provider.id, + time_remaining_ms: 22.hours.in_milliseconds + } + ] + ) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end +end diff --git a/ee/spec/lib/gitlab/auth/group_saml/sso_enforcer_spec.rb b/ee/spec/lib/gitlab/auth/group_saml/sso_enforcer_spec.rb index 984598649bfe212f2097dcb76a501fb43f019616..95be2308a0ad8dddb3551f3742e204d1f0b70d23 100644 --- a/ee/spec/lib/gitlab/auth/group_saml/sso_enforcer_spec.rb +++ b/ee/spec/lib/gitlab/auth/group_saml/sso_enforcer_spec.rb @@ -504,4 +504,55 @@ expect { described_class.access_restricted_groups(groups) }.not_to exceed_all_query_limit(control) end end + + describe '.sessions_time_remaining_for_expiry' do + subject(:sessions_time_remaining_for_expiry) { described_class.sessions_time_remaining_for_expiry } + + it 'returns data for existing sessions' do + freeze_time do + described_class.new(saml_provider).update_session + + expect(sessions_time_remaining_for_expiry).to match_array( + [ + { + provider_id: saml_provider.id, + time_remaining: described_class::DEFAULT_SESSION_TIMEOUT + } + ] + ) + end + end + + it 'returns empty array when no session data exists' do + expect(sessions_time_remaining_for_expiry).to eq([]) + end + + it 'returns calculated data when sessions have been around for some time' do + other_saml_provider = build_stubbed(:saml_provider) + frozen_time = Time.utc(2024, 2, 2, 1, 44) + + travel_to(frozen_time) do + described_class.new(saml_provider).update_session + end + + travel_to(frozen_time + 4.hours) do + described_class.new(other_saml_provider).update_session + end + + travel_to(frozen_time + 6.hours) do + expect(sessions_time_remaining_for_expiry).to match_array( + [ + { + provider_id: saml_provider.id, + time_remaining: 18.hours.to_f + }, + { + provider_id: other_saml_provider.id, + time_remaining: 22.hours.to_f + } + ] + ) + end + end + end end diff --git a/ee/spec/lib/gitlab/auth/group_saml/sso_state_spec.rb b/ee/spec/lib/gitlab/auth/group_saml/sso_state_spec.rb index f22e50c68f866867ed9cf76c54c59971b390e6f3..d80ce672aca2c5b511e361b6833a132f13f8658d 100644 --- a/ee/spec/lib/gitlab/auth/group_saml/sso_state_spec.rb +++ b/ee/spec/lib/gitlab/auth/group_saml/sso_state_spec.rb @@ -7,6 +7,25 @@ subject(:sso_state) { described_class.new(saml_provider_id) } + describe '.active_saml_sessions' do + subject(:active_saml_sessions) { described_class.active_saml_sessions } + + context 'when session data is stored' do + let(:session_data) do + { + 27 => (Time.current - 1.day), + 99 => (Time.current - 12.hours) + } + end + + around do |ex| + Gitlab::Session.with_session(described_class::SESSION_STORE_KEY => session_data) { ex.run } + end + + it { is_expected.to match(session_data) } + end + end + describe '#update_active' do it 'updates the current sign in state' do Gitlab::Session.with_session({}) do diff --git a/ee/spec/routing/user_settings_routing_spec.rb b/ee/spec/routing/user_settings_routing_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..261f861fec716444020e7fc52aa7db70e2ae9537 --- /dev/null +++ b/ee/spec/routing/user_settings_routing_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'EE-specific user_settings routing', feature_category: :system_access do + describe '/-/user_settings/active_sessions/saml.json' do + subject { get('/-/user_settings/active_sessions/saml.json') } + + it { is_expected.to route_to(controller: 'user_settings/active_sessions', action: 'saml', format: 'json') } + end + + describe '/-/user_settings/active_sessions/saml' do + subject { get('/-/user_settings/active_sessions/saml') } + + it { is_expected.to route_to('user_settings/active_sessions#saml') } + end + + describe '/-/user_settings/active_sessions/saml.html' do + subject { get('/-/user_settings/active_sessions/saml.html') } + + it { is_expected.not_to route_to('user_settings/active_sessions#saml') } + end +end diff --git a/lib/gitlab/namespaced_session_store.rb b/lib/gitlab/namespaced_session_store.rb index 957e8fe9b9f582c3bda835f8c77d7e47fff73f1e..5f1f684f81bae082bc1da0c086e8ffac53d5a64b 100644 --- a/lib/gitlab/namespaced_session_store.rb +++ b/lib/gitlab/namespaced_session_store.rb @@ -2,6 +2,8 @@ module Gitlab class NamespacedSessionStore + include Enumerable + def initialize(key, session = Session.current) @namespace_key = key @session = session @@ -11,6 +13,12 @@ def initiated? !session.nil? end + def each(&block) + return unless session + + session.fetch(@namespace_key, {}).each(&block) + end + def [](key) return unless session diff --git a/spec/lib/gitlab/namespaced_session_store_spec.rb b/spec/lib/gitlab/namespaced_session_store_spec.rb index 4e9b35e68593b84303ecf43d4bca9d400e3ecfd2..07e2b9c0cea3ba4dc15ac282e7c2a388c9ef88f4 100644 --- a/spec/lib/gitlab/namespaced_session_store_spec.rb +++ b/spec/lib/gitlab/namespaced_session_store_spec.rb @@ -5,6 +5,78 @@ RSpec.describe Gitlab::NamespacedSessionStore do let(:key) { :some_key } + describe 'Enumerable methods' do + subject(:instance) { described_class.new(key) } + + context 'with data in the session' do + around do |ex| + Gitlab::Session.with_session(key => { a: 1, b: 2 }) { ex.run } + end + + it 'passes .each call to storage hash' do + keys = [] + values = [] + # rubocop:disable Lint/UnreachableLoop -- false positive + instance.each do |key, val| + keys << key + values << val + end + # rubocop:enable Lint/UnreachableLoop + + expect(keys).to match_array([:a, :b]) + expect(values).to match_array([1, 2]) + end + + it 'passes .map to storage hash' do + expect(instance.map { |item| item }).to match_array([[:a, 1], [:b, 2]]) + end + + it 'converts into a basic hash upon request' do + expect(instance.to_h).to match(a: 1, b: 2) + end + end + + context 'with no data in session' do + subject(:iterator) do + instance.each do # rubocop:disable Lint/UnreachableLoop -- no clearer way to write this + raise 'This code should not be reachable' + end + end + + around do |ex| + Gitlab::Session.with_session(another_key: { a: 1, b: 2 }) { ex.run } + end + + it 'does not iterate when session is not initialized' do + expect { iterator }.not_to raise_error + end + + it 'converts to empty hash with .to_h' do + expect(instance.to_h).to eq({}) + end + end + + context 'with empty data in session' do + subject(:iterator) do + instance.each do # rubocop:disable Lint/UnreachableLoop -- no clearer way to write this + raise 'This code should not be reachable' + end + end + + around do |ex| + Gitlab::Session.with_session(key => {}) { ex.run } + end + + it 'does not raise error' do + expect { iterator }.not_to raise_error + end + + it 'converts to empty hash with .to_h' do + expect(instance.to_h).to eq({}) + end + end + end + context 'current session' do subject { described_class.new(key) }