Require session with smartcard login for Git access

We want the ability to restrict access for Git activity when smartcard
authentication is used. Git does not support smartcards yet but we can
check if the user has a valid browser session where smartcard
authentication was used.

config/gitlab.yml.example

@@ -664,6 +664,9 @@ production: &base
     # Port where the client side certificate is requested by the webserver (NGINX/Apache)
     # client_certificate_required_port: 3444
 
+    # Browser session with smartcard sign-in is required for Git access
+    # required_for_git_access: false
+
   ## Kerberos settings
   kerberos:
     # Allow the HTTP Negotiate authentication method for Git clients

config/initializers/1_settings.rb

@@ -76,6 +76,7 @@ Gitlab.ee do
   Settings['smartcard'] ||= Settingslogic.new({})
   Settings.smartcard['enabled'] = false if Settings.smartcard['enabled'].nil?
   Settings.smartcard['client_certificate_required_port'] = 3444 if Settings.smartcard['client_certificate_required_port'].nil?
+  Settings.smartcard['required_for_git_access'] = false if Settings.smartcard['required_for_git_access'].nil?
 end
 
 Settings['omniauth'] ||= Settingslogic.new({})

ee/app/controllers/smartcard_controller.rb

@@ -28,6 +28,7 @@ class SmartcardController < ApplicationController
       return
     end
 
+    store_active_session
     log_audit_event(user, with: certificate.auth_method)
     sign_in_and_redirect(user)
   end
@@ -43,6 +44,10 @@ class SmartcardController < ApplicationController
     end
   end
 
+  def store_active_session
+    Gitlab::Auth::Smartcard::SessionEnforcer.new.update_session
+  end
+
   def log_audit_event(user, options = {})
     AuditEventService.new(user, user, options).for_authentication.security_event
   end

ee/changelogs/unreleased/ee-6990-enforce_smartcard_session_for_git.yml

+---
+title: Require session with smartcard login for Git access
+merge_request: 14368
+author:
+type: added

ee/lib/ee/gitlab/git_access.rb

@@ -11,6 +11,7 @@ module EE
       override :check
       def check(cmd, changes)
         check_geo_license!
+        check_smartcard_access!
 
         super
       end
@@ -64,6 +65,18 @@ module EE
         end
       end
 
+      def check_smartcard_access!
+        unless can_access_without_new_smartcard_login?
+          raise ::Gitlab::GitAccess::UnauthorizedError, 'Project requires smartcard login. Please login to GitLab using a smartcard.'
+        end
+      end
+
+      def can_access_without_new_smartcard_login?
+        return true unless user
+
+        !::Gitlab::Auth::Smartcard::SessionEnforcer.new.access_restricted?(user)
+      end
+
       def geo?
         actor == :geo
       end

ee/lib/gitlab/auth/smartcard.rb

@@ -8,6 +8,10 @@ module Gitlab
       def enabled?
         ::License.feature_available?(:smartcard_auth) && ::Gitlab.config.smartcard.enabled
       end
+
+      def required_for_git_access?
+        self.enabled? && ::Gitlab.config.smartcard.required_for_git_access
+      end
     end
   end
 end

ee/lib/gitlab/auth/smartcard/session.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Auth
+    module Smartcard
+      class Session
+        SESSION_STORE_KEY = :smartcard_signins
+
+        def active?(user)
+          sessions = ActiveSession.list_sessions(user)
+          sessions.any? do |session|
+            Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY, session.with_indifferent_access )['last_signin_at']
+          end
+        end
+
+        def update_active(value)
+          current_session_data['last_signin_at'] = value
+        end
+
+        private
+
+        def current_session_data
+          Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY)
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/auth/smartcard/session_enforcer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Auth
+    module Smartcard
+      class SessionEnforcer
+        def update_session
+          session.update_active(DateTime.now)
+        end
+
+        def access_restricted?(user)
+          return false unless ::Gitlab::Auth::Smartcard.required_for_git_access?
+
+          !active_session?(user)
+        end
+
+        private
+
+        def session
+          @session ||= Smartcard::Session.new
+        end
+
+        def active_session?(user)
+          session.active?(user)
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/auth/smartcard/session_enforcer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Auth::Smartcard::SessionEnforcer do
+  describe '#update_session' do
+    let(:session) { {} }
+
+    around do |example|
+      Gitlab::Session.with_session(session) do
+        example.run
+      end
+    end
+
+    it 'stores the time of last sign-in in session' do
+      expect { subject.update_session }.to change { session[:smartcard_signins] }
+      expect(session[:smartcard_signins]).to have_key('last_signin_at')
+      expect(session[:smartcard_signins]['last_signin_at']).not_to be_nil
+    end
+  end
+
+  describe '#access_restricted?' do
+    let(:user) { create(:user) }
+
+    subject { described_class.new.access_restricted?(user) }
+
+    before do
+      stub_licensed_features(smartcard_auth: true)
+      stub_smartcard_setting(enabled: true, required_for_git_access: true)
+    end
+
+    context 'with a smartcard session', :clean_gitlab_redis_shared_state do
+      let(:session_id) { '42' }
+      let(:stored_session) do
+        { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
+      end
+
+      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_falsey }
+    end
+
+    context 'without any session' do
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with the setting off' do
+      before do
+        stub_smartcard_setting(required_for_git_access: false)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'with smartcard auth disabled' do
+      before do
+        stub_smartcard_setting(enabled: false)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'without a license' do
+      before do
+        stub_licensed_features(smartcard_auth: false)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+  end
+end

ee/spec/lib/gitlab/auth/smartcard/session_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Auth::Smartcard::Session do
+  describe '#active?' do
+    let(:user) { create(:user) }
+
+    subject { described_class.new.active?(user) }
+
+    context 'with a smartcard session', :clean_gitlab_redis_shared_state do
+      let(:session_id) { '42' }
+      let(:stored_session) do
+        { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
+      end
+
+      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_truthy }
+    end
+
+    context 'without any session' do
+      it { is_expected.to be_falsey }
+    end
+  end
+
+  describe '#update_active' do
+    let(:now) { Time.now }
+
+    around do |example|
+      Gitlab::Session.with_session({}) do
+        example.run
+      end
+    end
+
+    it 'stores the time of last sign-in' do
+      subject.update_active(now)
+
+      expect(Gitlab::Session.current[:smartcard_signins]).to eq({ 'last_signin_at' => now })
+    end
+  end
+end

ee/spec/lib/gitlab/git_access_spec.rb

@@ -613,6 +613,61 @@ describe Gitlab::GitAccess do
     end
   end
 
+  describe '#check_smartcard_access!' do
+    before do
+      stub_licensed_features(smartcard_auth: true)
+      stub_smartcard_setting(enabled: true, required_for_git_access: true)
+
+      project.add_developer(user)
+    end
+
+    context 'user with a smartcard session', :clean_gitlab_redis_shared_state do
+      let(:session_id) { '42' }
+      let(:stored_session) do
+        { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
+      end
+
+      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 'allows pull changes' do
+        expect { pull_changes }.not_to raise_error
+      end
+
+      it 'allows push changes' do
+        expect { push_changes }.not_to raise_error
+      end
+    end
+
+    context 'user without a smartcard session' do
+      it 'does not allow pull changes' do
+        expect { pull_changes }.to raise_error(Gitlab::GitAccess::UnauthorizedError)
+      end
+
+      it 'does not allow push changes' do
+        expect { push_changes }.to raise_error(Gitlab::GitAccess::UnauthorizedError)
+      end
+    end
+
+    context 'with the setting off' do
+      before do
+        stub_smartcard_setting(required_for_git_access: false)
+      end
+
+      it 'allows pull changes' do
+        expect { pull_changes }.not_to raise_error
+      end
+
+      it 'allows push changes' do
+        expect { push_changes }.not_to raise_error
+      end
+    end
+  end
+
   private
 
   def access

ee/spec/requests/api/internal_spec.rb

@@ -80,5 +80,68 @@ describe API::Internal do
         end
       end
     end
+
+    context 'smartcard session required' do
+      set(:project) { create(:project, :repository, :wiki_repo) }
+
+      subject do
+        post(
+          api("/internal/allowed"),
+          params: { key_id: key.id,
+                    project: project.full_path,
+                    gl_repository: "project-#{project.id}",
+                    action: 'git-upload-pack',
+                    secret_token: secret_token,
+                    protocol: 'ssh' })
+      end
+
+      before do
+        stub_licensed_features(smartcard_auth: true)
+        stub_smartcard_setting(enabled: true, required_for_git_access: true)
+
+        project.add_developer(user)
+      end
+
+      context 'user with a smartcard session', :clean_gitlab_redis_shared_state do
+        let(:session_id) { '42' }
+        let(:stored_session) do
+          { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
+        end
+
+        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 "allows access" do
+          subject
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'user without a smartcard session' do
+        it "does not allow access" do
+          subject
+
+          expect(response).to have_gitlab_http_status(401)
+          expect(json_response['message']).to eql('Project requires smartcard login. Please login to GitLab using a smartcard.')
+        end
+      end
+
+      context 'with the setting off' do
+        before do
+          stub_smartcard_setting(required_for_git_access: false)
+        end
+
+        it "allows access" do
+          subject
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+    end
   end
 end

ee/spec/requests/projects/git_http_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::GitHttpController, type: :request do
+  include GitHttpHelpers
+
+  describe 'GET #info_refs' do
+    set(:user) { create(:user) }
+    set(:project) { create(:project, :repository, :private) }
+    let(:path) { "#{project.full_path}.git" }
+    let(:env) { { user: user.username, password: user.password } }
+
+    context 'smartcard session required' do
+      subject { clone_get(path, env) }
+
+      before do
+        stub_licensed_features(smartcard_auth: true)
+        stub_smartcard_setting(enabled: true, required_for_git_access: true)
+
+        project.add_developer(user)
+      end
+
+      context 'user with a smartcard session', :clean_gitlab_redis_shared_state do
+        let(:session_id) { '42' }
+        let(:stored_session) do
+          { 'smartcard_signins' => { 'last_signin_at' => 5.minutes.ago } }
+        end
+
+        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 "allows access" do
+          subject
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'user without a smartcard session' do
+        it "does not allow access" do
+          subject
+
+          expect(response).to have_gitlab_http_status(403)
+          expect(response.body).to eq('Project requires smartcard login. Please login to GitLab using a smartcard.')
+        end
+      end
+
+      context 'with the setting off' do
+        before do
+          stub_smartcard_setting(required_for_git_access: false)
+        end
+
+        it "allows access" do
+          subject
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+    end
+  end
+end

ee/spec/requests/smartcard_controller_spec.rb

@@ -8,6 +8,7 @@ describe SmartcardController, type: :request do
   let(:certificate_headers) { { 'X-SSL-CLIENT-CERTIFICATE': 'certificate' } }
   let(:openssl_certificate_store) { instance_double(OpenSSL::X509::Store) }
   let(:audit_event_service) { instance_double(AuditEventService) }
+  let(:session_enforcer) { instance_double(Gitlab::Auth::Smartcard::SessionEnforcer) }
 
   shared_examples 'a client certificate authentication' do |auth_method|
     context 'with smartcard_auth enabled' do
@@ -33,6 +34,14 @@ describe SmartcardController, type: :request do
         subject
       end
 
+      it 'stores active session' do
+        expect(::Gitlab::Auth::Smartcard::SessionEnforcer).to(
+          receive(:new).and_return(session_enforcer))
+        expect(session_enforcer).to receive(:update_session)
+
+        subject
+      end
+
       context 'user does not exist' do
         context 'signup allowed' do
           it 'creates user' do

ee/spec/support/helpers/ee/stub_configuration.rb

@@ -29,5 +29,9 @@ module EE
     def stub_geo_setting(messages)
       allow(::Gitlab.config.geo).to receive_messages(to_settings(messages))
     end
+
+    def stub_smartcard_setting(messages)
+      allow(::Gitlab.config.smartcard).to receive_messages(to_settings(messages))
+    end
   end
 end

lib/gitlab/namespaced_session_store.rb

@@ -4,19 +4,24 @@ module Gitlab
   class NamespacedSessionStore
     delegate :[], :[]=, to: :store
 
-    def initialize(key)
+    def initialize(key, session = Session.current)
       @key = key
+      @session = session
     end
 
     def initiated?
-      !Session.current.nil?
+      !session.nil?
     end
 
     def store
-      return unless Session.current
+      return unless session
 
-      Session.current[@key] ||= {}
-      Session.current[@key]
+      session[@key] ||= {}
+      session[@key]
     end
+
+    private
+
+    attr_reader :session
   end
 end

spec/lib/gitlab/namespaced_session_store_spec.rb

@@ -4,19 +4,33 @@ require 'spec_helper'
 
 describe Gitlab::NamespacedSessionStore do
   let(:key) { :some_key }
-  subject { described_class.new(key) }
 
-  it 'stores data under the specified key' do
-    Gitlab::Session.with_session({}) do
-      subject[:new_data] = 123
+  context 'current session' do
+    subject { described_class.new(key) }
 
-      expect(Thread.current[:session_storage][key]).to eq(new_data: 123)
+    it 'stores data under the specified key' do
+      Gitlab::Session.with_session({}) do
+        subject[:new_data] = 123
+
+        expect(Thread.current[:session_storage][key]).to eq(new_data: 123)
+      end
+    end
+
+    it 'retrieves data from the given key' do
+      Thread.current[:session_storage] = { key => { existing_data: 123 } }
+
+      expect(subject[:existing_data]).to eq 123
     end
   end
 
-  it 'retrieves data from the given key' do
-    Thread.current[:session_storage] = { key => { existing_data: 123 } }
+  context 'passed in session' do
+    let(:data) { { 'data' => 42 } }
+    let(:session) { { 'some_key' => data } }
+
+    subject { described_class.new(key, session.with_indifferent_access) }
 
-    expect(subject[:existing_data]).to eq 123
+    it 'retrieves data from the given key' do
+      expect(subject['data']).to eq 42
+    end
   end
 end

spec/support/helpers/git_http_helpers.rb

 module GitHttpHelpers
+  include WorkhorseHelpers
+
   def clone_get(project, options = {})
     get "/#{project}/info/refs", params: { service: 'git-upload-pack' }, headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
   end