From a5f5dde09f72743021ca1f5aa2560aaa227f69b6 Mon Sep 17 00:00:00 2001
From: Andrew Evans <aevans@gitlab.com>
Date: Wed, 3 Apr 2024 08:44:43 +0000
Subject: [PATCH] Add API endpoint to return current SSO session expiry time
This change will add an API endpoint on EE to return the expiry time of
the user's current Group SAML SSO sessions. This way the frontend can be
more responsive if a tab is left open overnight or over the weekend, and
the client no longer has an active SSO session.
Changelog: added
EE: true
---
.../active_sessions_controller.rb | 2 +
.../active_sessions_controller.rb | 26 +++++++
ee/config/routes/user_settings.rb | 9 +++
ee/lib/gitlab/auth/group_saml/sso_enforcer.rb | 9 +++
ee/lib/gitlab/auth/group_saml/sso_state.rb | 4 ++
.../active_sessions_controller_spec.rb | 72 +++++++++++++++++++
.../auth/group_saml/sso_enforcer_spec.rb | 51 +++++++++++++
.../gitlab/auth/group_saml/sso_state_spec.rb | 19 +++++
ee/spec/routing/user_settings_routing_spec.rb | 23 ++++++
lib/gitlab/namespaced_session_store.rb | 8 +++
.../gitlab/namespaced_session_store_spec.rb | 72 +++++++++++++++++++
11 files changed, 295 insertions(+)
create mode 100644 ee/app/controllers/ee/user_settings/active_sessions_controller.rb
create mode 100644 ee/config/routes/user_settings.rb
create mode 100644 ee/spec/controllers/user_settings/active_sessions_controller_spec.rb
create mode 100644 ee/spec/routing/user_settings_routing_spec.rb
diff --git a/app/controllers/user_settings/active_sessions_controller.rb b/app/controllers/user_settings/active_sessions_controller.rb
index bfc969d0ff8a8..34359eb008084 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 0000000000000..0b4d3ae4d0075
--- /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 0000000000000..5152dbd9cc479
--- /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 4480588a7b054..c43c325fa18d4 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 2a8e410dfd68f..a33f73b0e564a 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 0000000000000..e2d3d6262ccba
--- /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 984598649bfe2..95be2308a0ad8 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 f22e50c68f866..d80ce672aca2c 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 0000000000000..261f861fec716
--- /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 957e8fe9b9f58..5f1f684f81bae 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 4e9b35e68593b..07e2b9c0cea3b 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) }
--
GitLab