SSO enforcement for Git and API access

Works by looking up browser sessions stored in Redis to see if the user
has any active SAML sessions for the group a resource belongs to.

When we are in a web request we use the current session.
Outside of web requests we check for background sessions.

ee/app/policies/ee/group_policy.rb

@@ -120,7 +120,7 @@ def sso_enforcement_prevents_access?
       return false unless subject.persisted?
       return false if user&.admin?
 
-      ::Gitlab::Auth::GroupSaml::SsoEnforcer.group_access_restricted?(subject)
+      ::Gitlab::Auth::GroupSaml::SsoEnforcer.group_access_restricted?(subject, user)
     end
   end
 end

ee/app/policies/ee/project_policy.rb

@@ -227,7 +227,7 @@ module ProjectPolicy
       end
 
       condition(:needs_new_sso_session) do
-        ::Gitlab::Auth::GroupSaml::SsoEnforcer.group_access_restricted?(subject.group)
+        ::Gitlab::Auth::GroupSaml::SsoEnforcer.group_access_restricted?(subject.group, @user)
       end
 
       condition(:ip_enforcement_prevents_access) do

ee/changelogs/unreleased/jej-enforce-sso-background-session-for-git-and-api.yml

+---
+title: SAML SSO enforcement applies to access over Git and API
+merge_request: 12594
+author:
+type: added

ee/lib/gitlab/auth/group_saml/background_sso_state.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Auth
+    module GroupSaml
+      class BackgroundSsoState
+        attr_reader :user
+
+        def initialize(user)
+          @user = user
+        end
+
+        def all
+          sessions = ActiveSession.list_sessions(user)
+          key = SsoState::SESSION_STORE_KEY
+          sessions.map { |session| session[key.to_s] }.compact
+        end
+
+        def most_recent(saml_provider_id)
+          all.map { |session| session[saml_provider_id] }.max
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/auth/group_saml/sso_enforcer.rb

@@ -11,26 +11,31 @@ def initialize(saml_provider)
         end
 
         def update_session
-          SsoState.new(saml_provider.id).update_active(DateTime.now)
+          sso_state.update_active(DateTime.now)
         end
 
-        def active_session?
-          SsoState.new(saml_provider.id).active?
+        def active_session?(user)
+          skip_background_session_check = ::Feature.disabled?(:enforced_sso_checks_background_session, saml_provider.group)
+          sso_state.active?(user, skip_background_session_check: skip_background_session_check)
         end
 
-        def access_restricted?
-          saml_enforced? && !active_session? && ::Feature.enabled?(:enforced_sso_requires_session, group)
+        def access_restricted?(user)
+          saml_enforced? && !active_session?(user) && ::Feature.enabled?(:enforced_sso_requires_session, group)
         end
 
-        def self.group_access_restricted?(group)
-          return false unless ::Feature.enabled?(:enforced_sso_requires_session, group)
+        def self.group_access_restricted?(group, user)
           return false unless group
+          return false unless ::Feature.enabled?(:enforced_sso_requires_session, group)
 
           saml_provider = group&.root_ancestor&.saml_provider
 
           return false unless saml_provider
 
-          new(saml_provider).access_restricted?
+          new(saml_provider).access_restricted?(user)
+        end
+
+        def sso_state
+          @sso_state ||= SsoState.new(saml_provider.id)
         end
 
         private

ee/lib/gitlab/auth/group_saml/sso_state.rb

@@ -12,8 +12,20 @@ def initialize(saml_provider_id)
           @saml_provider_id = saml_provider_id
         end
 
-        def active?
-          !session_available? || active_session_data[saml_provider_id]
+        def active?(user, skip_background_session_check: false)
+          if session_available?
+            session_active?
+          else
+            skip_background_session_check || background_sso_session?(user)
+          end
+        end
+
+        def session_active?
+          active_session_data[saml_provider_id]
+        end
+
+        def background_sso_session?(user)
+          BackgroundSsoState.new(user).most_recent(saml_provider_id)
         end
 
         def update_active(value)

ee/spec/lib/gitlab/auth/group_saml/background_sso_state_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Auth::GroupSaml::BackgroundSsoState do
+  let(:user) { double }
+
+  subject { described_class.new(user) }
+
+  describe '#all' do
+    it 'returns SAML data from sessions' do
+      saml_data = { 7 => double }
+      sessions = [{ 'active_group_sso_sign_ins' => saml_data }]
+      allow(ActiveSession).to receive(:list_sessions).and_return(sessions)
+
+      expect(subject.all).to eq([saml_data])
+    end
+
+    it 'skips sessions which do not have SAML data' do
+      sessions = [{ a: 1 }, { b: 2 }]
+      allow(ActiveSession).to receive(:list_sessions).and_return(sessions)
+
+      expect(subject.all).to eq([])
+    end
+  end
+
+  describe '#most_recent' do
+    let(:saml_datetime) { double }
+    let(:saml_provider_id) { 7 }
+    let(:saml_data) { { saml_provider_id => saml_datetime } }
+    let(:session_with_saml) { { 'active_group_sso_sign_ins' => saml_data } }
+
+    it 'returns the date a saml provider was used to sign in' do
+      allow(ActiveSession).to receive(:list_sessions).and_return([session_with_saml])
+
+      expect(subject.most_recent(saml_provider_id)).to eq(saml_datetime)
+    end
+
+    it 'looks up the most recent SAML session for a provider' do
+      latest_date = 1.day.ago
+      oldest_date = 1.month.ago
+
+      saml_sessions = [{ saml_provider_id => oldest_date }, { saml_provider_id => latest_date }]
+      allow(subject).to receive(:all).and_return(saml_sessions)
+
+      expect(subject.most_recent(saml_provider_id)).to eq(latest_date)
+    end
+
+    it 'returns nil when the saml provider has no sessions' do
+      allow(ActiveSession).to receive(:list_sessions).and_return([session_with_saml])
+
+      expect(subject.most_recent(6)).to eq(nil)
+    end
+
+    it 'returns nil when there was no saml data' do
+      sessions = [{ a: 1 }, { b: 2 }]
+      allow(ActiveSession).to receive(:list_sessions).and_return(sessions)
+
+      expect(subject.most_recent(saml_provider_id)).to eq(nil)
+    end
+  end
+end

ee/spec/lib/gitlab/auth/group_saml/sso_enforcer_spec.rb

@@ -29,29 +29,51 @@
   end
 
   describe '#active_session?' do
+    let(:user) { double }
+
     it 'returns false if nothing has been stored' do
-      expect(subject).not_to be_active_session
+      expect(subject).not_to be_active_session(user)
     end
 
     it 'returns true if a sign in has been recorded' do
       subject.update_session
 
-      expect(subject).to be_active_session
+      expect(subject).to be_active_session(user)
+    end
+
+    context 'without an active session' do
+      let(:session) { nil }
+
+      it 'checks background session' do
+        expect(subject.sso_state).to receive(:background_sso_session?).with(user)
+
+        subject.active_session?(user)
+      end
+
+      it 'skips background session check when that feature is not enabled' do
+        stub_feature_flags(enforced_sso_checks_background_session: false)
+
+        expect(subject.sso_state).not_to receive(:background_sso_session?)
+
+        subject.active_session?(user)
+      end
     end
   end
 
   describe '#allows_access?' do
+    let(:user) { double }
+
     it 'allows access when saml_provider is nil' do
       subject = described_class.new(nil)
 
-      expect(subject).not_to be_access_restricted
+      expect(subject).not_to be_access_restricted(user)
     end
 
     context 'when sso enforcement is disabled' do
       let(:saml_provider) { build_stubbed(:saml_provider, enforced_sso: false) }
 
       it 'allows access when sso enforcement is disabled' do
-        expect(subject).not_to be_access_restricted
+        expect(subject).not_to be_access_restricted(user)
       end
     end
 
@@ -59,24 +81,24 @@
       let(:saml_provider) { build_stubbed(:saml_provider, enforced_sso: true, enabled: false) }
 
       it 'allows access when saml_provider is disabled' do
-        expect(subject).not_to be_access_restricted
+        expect(subject).not_to be_access_restricted(user)
       end
     end
 
     it 'allows access when the sso enforcement feature is disabled' do
       stub_feature_flags(enforced_sso: { enabled: false, thing: saml_provider.group })
 
-      expect(subject).not_to be_access_restricted
+      expect(subject).not_to be_access_restricted(user)
     end
 
     it 'prevents access when sso enforcement active but there is no session' do
-      expect(subject).to be_access_restricted
+      expect(subject).to be_access_restricted(user)
     end
 
     it 'allows access when sso is enforced but a saml session is active' do
       subject.update_session
 
-      expect(subject).not_to be_access_restricted
+      expect(subject).not_to be_access_restricted(user)
     end
   end
 end

ee/spec/lib/gitlab/auth/group_saml/sso_state_spec.rb

@@ -3,6 +3,7 @@
 require 'spec_helper'
 
 describe Gitlab::Auth::GroupSaml::SsoState do
+  let(:user) { double(:user, id: 2) }
   let(:saml_provider_id) { 10 }
 
   subject { described_class.new(saml_provider_id) }
@@ -18,13 +19,58 @@
     end
   end
 
-  describe '#active?' do
+  describe '#session_active?' do
     it 'gets the current sign in state' do
       current_state = double
 
       Gitlab::Session.with_session(active_group_sso_sign_ins: { saml_provider_id => current_state }) do
-        expect(subject.active?).to eq current_state
+        expect(subject.session_active?).to eq current_state
       end
     end
   end
+
+  describe '#active?' do
+    let(:current_sign_in) { double }
+    let(:sign_ins) { { saml_provider_id => current_sign_in } }
+
+    context 'with an active session' do
+      around do |example|
+        Gitlab::Session.with_session(active_group_sso_sign_ins: sign_ins) do
+          example.run
+        end
+      end
+
+      it 'uses the active session' do
+        expect(subject.active?(user)).to eq(current_sign_in)
+      end
+    end
+
+    context 'without an active session' do
+      it 'uses redis background sessions' do
+        expect(subject).to receive(:background_sso_session?).with(user)
+
+        subject.active?(user)
+      end
+    end
+  end
+
+  describe '#background_sso_session?', :clean_gitlab_redis_shared_state do
+    context 'with a valid background redis session' do
+      let(:session_id) { '6919a6f1bb119dd7396fadc38fd18d0d' }
+      let(:stored_session) { { 'active_group_sso_sign_ins' => { saml_provider_id => 1.day.ago } } }
+
+      before do
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.set("session:gitlab:#{session_id}", Marshal.dump(stored_session))
+          redis.sadd("session:lookup:user:gitlab:#{user.id}", [session_id])
+        end
+      end
+
+      it { is_expected.to be_background_sso_session(user) }
+    end
+
+    context 'without a valid background sesssion' do
+      it { is_expected.not_to be_background_sso_session(user) }
+    end
+  end
 end

ee/spec/policies/group_policy_spec.rb

@@ -80,8 +80,20 @@
       end
 
       context 'when there is no global session or sso state' do
-        it "allows access because we haven't yet restricted all use cases" do
-          is_expected.to be_allowed(:read_group)
+        context 'with background SAML session' do
+          before do
+            allow_any_instance_of(Gitlab::Auth::GroupSaml::SsoState).to receive(:background_sso_session?).and_return(true)
+          end
+
+          it 'allows access' do
+            is_expected.to be_allowed(:read_group)
+          end
+        end
+
+        context 'without background SAML session' do
+          it 'prevents access' do
+            is_expected.not_to be_allowed(:read_group)
+          end
         end
       end
     end

ee/spec/policies/project_policy_spec.rb

@@ -257,8 +257,20 @@
       end
 
       context 'when there is no global session or sso state' do
-        it "allows access because we haven't yet restricted all use cases" do
-          is_expected.to be_allowed(:read_project)
+        context 'with background SAML session' do
+          before do
+            allow_any_instance_of(Gitlab::Auth::GroupSaml::SsoState).to receive(:background_sso_session?).and_return(true)
+          end
+
+          it 'allows access' do
+            is_expected.to be_allowed(:read_project)
+          end
+        end
+
+        context 'without background SAML session' do
+          it 'prevents access' do
+            is_expected.not_to be_allowed(:read_project)
+          end
         end
       end
     end