Merge branch '296669-ui-support-customizable-timeouts-for-git-cli-2fa' into 'master'

Support setting customizable timeouts for git CLI 2FA via UI & API

See merge request gitlab-org/gitlab!52769

app/views/admin/application_settings/_account_and_limit.html.haml

@@ -29,6 +29,7 @@
       = f.number_field :session_expire_delay, class: 'form-control gl-form-input', title: _('Maximum duration of a session.'), data: { toggle: 'tooltip', container: 'body' }
       %span.form-text.text-muted#session_expire_delay_help_block= _('GitLab restart is required to apply changes.')
 
+    = render_if_exists 'admin/application_settings/git_two_factor_session_expiry', form: f
     = render_if_exists 'admin/application_settings/personal_access_token_expiration_policy', form: f
     = render_if_exists 'admin/application_settings/enforce_pat_expiration', form: f
     = render_if_exists 'admin/application_settings/enforce_ssh_key_expiration', form: f

doc/api/settings.md

@@ -282,6 +282,7 @@ listed in the descriptions of the relevant settings.
 | `first_day_of_week`                      | integer          | no                                   | Start day of the week for calendar views and date pickers. Valid values are `0` (default) for Sunday, `1` for Monday, and `6` for Saturday. |
 | `geo_node_allowed_ips`                   | string           | yes                                  | **(PREMIUM)** Comma-separated list of IPs and CIDRs of allowed secondary nodes. For example, `1.1.1.1, 2.2.2.0/24`. |
 | `geo_status_timeout`                     | integer          | no                                   | **(PREMIUM)** The amount of seconds after which a request to get a secondary node status times out. |
+| `git_two_factor_session_expiry`          | integer          | no                                   | **(PREMIUM)** Maximum duration (in minutes) of a session for Git operations when 2FA is enabled. |
 | `gitaly_timeout_default`                 | integer          | no                                   | Default Gitaly timeout, in seconds. This timeout is not enforced for Git fetch/push operations or Sidekiq jobs. Set to `0` to disable timeouts. |
 | `gitaly_timeout_fast`                    | integer          | no                                   | Gitaly fast operation timeout, in seconds. Some Gitaly operations are expected to be fast. If they exceed this threshold, there may be a problem with a storage shard and 'failing fast' can help maintain the stability of the GitLab instance. Set to `0` to disable timeouts. |
 | `gitaly_timeout_medium`                  | integer          | no                                   | Medium Gitaly timeout, in seconds. This should be a value between the Fast and the Default timeout. Set to `0` to disable timeouts. |

doc/user/admin_area/settings/account_and_limit_settings.md

@@ -131,6 +131,23 @@ add the line below to `/etc/gitlab/gitlab.rb` before increasing the max attachme
 nginx['client_max_body_size'] = "200m"
 ```
 
+## Customize session duration for Git Operations when 2FA is enabled **(PREMIUM)**
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/296669) in GitLab 13.9.
+> - It's deployed behind a feature flag, disabled by default.
+> - It's disabled on GitLab.com.
+> - It's not recommended for production use.
+> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](../../../security/two_factor_authentication.md#enable-or-disable-two-factor-authentication-2fa-for-git-operations)
+
+GitLab administrators can choose to customize the session duration (in minutes) for Git operations when 2FA is enabled. The default is 15 and this can be set to a value between 1 and 10080.
+
+To set a limit on how long these sessions are valid:
+
+1. Navigate to **Admin Area > Settings > General**.
+1. Expand the **Account and limit** section.
+1. Fill in the **Session duration for Git operations when 2FA is enabled (minutes)** field.
+1. Click **Save changes**.
+
 ## Limiting lifetime of personal access tokens **(ULTIMATE SELF)**
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/3649) in GitLab Ultimate 12.6.

ee/app/controllers/ee/admin/application_settings_controller.rb

@@ -49,6 +49,10 @@ module EE
           end
         end
 
+        if License.feature_available?(:git_two_factor_enforcement) && ::Feature.enabled?(:two_factor_for_cli)
+          attrs << :git_two_factor_session_expiry
+        end
+
         if License.feature_available?(:admin_merge_request_approvers_rules)
           attrs += EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes
         end

ee/app/helpers/ee/application_settings_helper.rb

@@ -111,6 +111,7 @@ module EE
        %i[
         email_additional_text
         file_template_project_id
+        git_two_factor_session_expiry
         group_owners_can_manage_default_branch_protection
         default_project_deletion_protection
         deletion_adjourned_period

ee/app/models/ee/application_setting.rb

@@ -118,6 +118,10 @@ module EE
                 },
                 if: proc { License.current&.restricted_user_count? }
 
+      validates :git_two_factor_session_expiry,
+                presence: true,
+                numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: 10080 }
+
       after_commit :update_personal_access_tokens_lifetime, if: :saved_change_to_max_personal_access_token_lifetime?
       after_commit :resume_elasticsearch_indexing
     end
@@ -151,6 +155,7 @@ module EE
           enforce_namespace_storage_limit: false,
           enforce_pat_expiration: true,
           geo_node_allowed_ips: '0.0.0.0/0, ::/0',
+          git_two_factor_session_expiry: 15,
           lock_memberships_to_ldap: false,
           max_personal_access_token_lifetime: nil,
           mirror_capacity_threshold: Settings.gitlab['mirror_capacity_threshold'],

ee/app/models/license.rb

@@ -82,6 +82,7 @@ class License < ApplicationRecord
     file_locks
     geo
     generic_alert_fingerprinting
+    git_two_factor_enforcement
     github_project_service_integration
     group_allowed_email_domains
     group_coverage_reports

ee/app/views/admin/application_settings/_git_two_factor_session_expiry.html.haml

+- return unless License.feature_available?(:git_two_factor_enforcement)
+- return unless Feature.enabled?(:two_factor_for_cli)
+
+.form-group
+  = form.label :git_two_factor_session_expiry, s_('AdminSettings|Session duration for Git operations when 2FA is enabled (minutes)')
+  = form.number_field :git_two_factor_session_expiry, class: 'form-control', title: s_('AdminSettings|Maximum duration of a session for Git operations when 2FA is enabled.'), data: { toggle: 'tooltip', container: 'body' }

ee/changelogs/unreleased/296669-ui-support-customizable-timeouts-for-git-cli-2fa.yml

+---
+title: Support setting customizable timeouts for git CLI 2FA via UI & API
+merge_request: 52769
+author:
+type: added

ee/lib/ee/api/entities/application_setting.rb

@@ -22,6 +22,7 @@ module EE
           expose :group_owners_can_manage_default_branch_protection, if: ->(_instance, _opts) { ::License.feature_available?(:default_branch_protection_restriction_in_groups) }
           expose :maintenance_mode, if: ->(_instance, _opts) { ::Gitlab::Geo.license_allows? && ::Feature.enabled?(:maintenance_mode) }
           expose :maintenance_mode_message, if: ->(_instance, _opts) { ::Gitlab::Geo.license_allows? && ::Feature.enabled?(:maintenance_mode) }
+          expose :git_two_factor_session_expiry, if: ->(_instance, _opts) { License.feature_available?(:git_two_factor_enforcement) && ::Feature.enabled?(:two_factor_for_cli) }
         end
       end
     end

ee/lib/ee/api/helpers/settings_helpers.rb

@@ -51,6 +51,7 @@ module EE
             optional :group_owners_can_manage_default_branch_protection, type: Grape::API::Boolean, desc: 'Allow owners to manage default branch protection in groups'
             optional :maintenance_mode, type: Grape::API::Boolean, desc: 'When instance is in maintenance mode, non-admin users can sign in with read-only access and make read-only API requests'
             optional :maintenance_mode_message, type: String, desc: 'Message displayed when instance is in maintenance mode'
+            optional :git_two_factor_session_expiry, type: Integer, desc: 'Maximum duration (in minutes) of a session for Git operations when 2FA is enabled'
           end
         end
 

ee/lib/ee/api/settings.rb

@@ -9,6 +9,7 @@ module EE
         helpers do
           extend ::Gitlab::Utils::Override
 
+          # rubocop:disable Metrics/CyclomaticComplexity
           override :filter_attributes_using_license
           def filter_attributes_using_license(attrs)
             unless ::License.feature_available?(:repository_mirrors)
@@ -47,12 +48,17 @@ module EE
               attrs = attrs.except(:group_owners_can_manage_default_branch_protection)
             end
 
+            unless License.feature_available?(:git_two_factor_enforcement) && ::Feature.enabled?(:two_factor_for_cli)
+              attrs = attrs.except(:git_two_factor_session_expiry)
+            end
+
             unless ::Gitlab::Geo.license_allows? && ::Feature.enabled?(:maintenance_mode)
               attrs = attrs.except(:maintenance_mode, :maintenance_mode_message)
             end
 
             attrs
           end
+          # rubocop:enable Metrics/CyclomaticComplexity
         end
       end
     end

ee/spec/controllers/admin/application_settings_controller_spec.rb

@@ -104,6 +104,17 @@ RSpec.describe Admin::ApplicationSettingsController do
       it_behaves_like 'settings for licensed features'
     end
 
+    context 'updating `git_two_factor_session_expiry` setting' do
+      before do
+        stub_feature_flags(two_factor_for_cli: true)
+      end
+
+      let(:settings) { { git_two_factor_session_expiry: 10 } }
+      let(:feature) { :git_two_factor_enforcement }
+
+      it_behaves_like 'settings for licensed features'
+    end
+
     context 'updating maintenance mode setting' do
       before do
         stub_feature_flags(maintenance_mode: true)

ee/spec/models/application_setting_spec.rb

@@ -93,6 +93,17 @@ RSpec.describe ApplicationSetting do
     it { is_expected.not_to allow_value(-1).for(:new_user_signups_cap) }
     it { is_expected.not_to allow_value(2.5).for(:new_user_signups_cap) }
 
+    it { is_expected.to allow_value(1).for(:git_two_factor_session_expiry) }
+    it { is_expected.to allow_value(10).for(:git_two_factor_session_expiry) }
+    it { is_expected.to allow_value(10079).for(:git_two_factor_session_expiry) }
+    it { is_expected.to allow_value(10080).for(:git_two_factor_session_expiry) }
+    it { is_expected.not_to allow_value(nil).for(:git_two_factor_session_expiry) }
+    it { is_expected.not_to allow_value("value").for(:git_two_factor_session_expiry) }
+    it { is_expected.not_to allow_value(2.5).for(:git_two_factor_session_expiry) }
+    it { is_expected.not_to allow_value(-5).for(:git_two_factor_session_expiry) }
+    it { is_expected.not_to allow_value(0).for(:git_two_factor_session_expiry) }
+    it { is_expected.not_to allow_value(10081).for(:git_two_factor_session_expiry) }
+
     describe 'when additional email text is enabled' do
       before do
         stub_licensed_features(email_additional_text: true)

ee/spec/requests/api/settings_spec.rb

@@ -169,6 +169,17 @@ RSpec.describe API::Settings, 'EE Settings' do
     it_behaves_like 'settings for licensed features'
   end
 
+  context 'git_two_factor_session_expiry setting' do
+    before do
+      stub_feature_flags(two_factor_for_cli: true)
+    end
+
+    let(:settings) { { git_two_factor_session_expiry: 10 } }
+    let(:feature) { :git_two_factor_enforcement }
+
+    it_behaves_like 'settings for licensed features'
+  end
+
   context 'default project deletion protection' do
     let(:settings) { { default_project_deletion_protection: true } }
     let(:feature) { :default_project_deletion_protection }

lib/gitlab/auth/otp/session_enforcer.rb

@@ -5,7 +5,6 @@ module Gitlab
     module Otp
       class SessionEnforcer
         OTP_SESSIONS_NAMESPACE = 'session:otp'
-        DEFAULT_EXPIRATION = 15.minutes.to_i
 
         def initialize(key)
           @key = key
@@ -13,7 +12,7 @@ module Gitlab
 
         def update_session
           Gitlab::Redis::SharedState.with do |redis|
-            redis.setex(key_name, DEFAULT_EXPIRATION, true)
+            redis.setex(key_name, session_expiry_in_seconds, true)
           end
         end
 
@@ -30,6 +29,10 @@ module Gitlab
         def key_name
           @key_name ||= "#{OTP_SESSIONS_NAMESPACE}:#{key.id}"
         end
+
+        def session_expiry_in_seconds
+          Gitlab::CurrentSettings.git_two_factor_session_expiry.minutes.to_i
+        end
       end
     end
   end

locale/gitlab.pot

@@ -2058,6 +2058,9 @@ msgstr ""
 msgid "AdminSettings|Integrations configured here will automatically apply to all projects on this instance."
 msgstr ""
 
+msgid "AdminSettings|Maximum duration of a session for Git operations when 2FA is enabled."
+msgstr ""
+
 msgid "AdminSettings|Moved to integrations"
 msgstr ""
 
@@ -2082,6 +2085,9 @@ msgstr ""
 msgid "AdminSettings|Service template allows you to set default values for integrations"
 msgstr ""
 
+msgid "AdminSettings|Session duration for Git operations when 2FA is enabled (minutes)"
+msgstr ""
+
 msgid "AdminSettings|Set an instance-wide auto included %{link_start}pipeline configuration%{link_end}. This pipeline configuration will be run after the project's own configuration."
 msgstr ""
 

spec/lib/gitlab/auth/otp/session_enforcer_spec.rb

@@ -9,11 +9,12 @@ RSpec.describe Gitlab::Auth::Otp::SessionEnforcer, :clean_gitlab_redis_shared_st
     it 'registers a session in Redis' do
       redis = double(:redis)
       expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis)
+      session_expiry_in_seconds = Gitlab::CurrentSettings.git_two_factor_session_expiry.minutes.to_i
 
       expect(redis).to(
         receive(:setex)
           .with("#{described_class::OTP_SESSIONS_NAMESPACE}:#{key.id}",
-                described_class::DEFAULT_EXPIRATION,
+                session_expiry_in_seconds,
                 true)
           .once)
 

spec/lib/gitlab/git_access_spec.rb

@@ -411,6 +411,59 @@ RSpec.describe Gitlab::GitAccess do
           expect { pull_access_check }.not_to raise_error
         end
       end
+
+      context 'based on the duration set by the `git_two_factor_session_expiry` setting' do
+        let_it_be(:git_two_factor_session_expiry) { 20 }
+        let_it_be(:redis_key_expiry_at) { git_two_factor_session_expiry.minutes.from_now }
+
+        before do
+          stub_application_setting(git_two_factor_session_expiry: git_two_factor_session_expiry)
+        end
+
+        def value_of_key
+          key_expired = Time.current > redis_key_expiry_at
+          return if key_expired
+
+          true
+        end
+
+        def stub_redis
+          redis = double(:redis)
+          expect(Gitlab::Redis::SharedState).to receive(:with).at_most(:twice).and_yield(redis)
+
+          expect(redis).to(
+            receive(:get)
+              .with("#{Gitlab::Auth::Otp::SessionEnforcer::OTP_SESSIONS_NAMESPACE}:#{key.id}"))
+              .at_most(:twice)
+              .and_return(value_of_key)
+        end
+
+        context 'at a time before the stipulated expiry' do
+          it 'allows push and pull access' do
+            travel_to(10.minutes.from_now) do
+              stub_redis
+
+              aggregate_failures do
+                expect { push_access_check }.not_to raise_error
+                expect { pull_access_check }.not_to raise_error
+              end
+            end
+          end
+        end
+
+        context 'at a time after the stipulated expiry' do
+          it 'does not allow push and pull access' do
+            travel_to(30.minutes.from_now) do
+              stub_redis
+
+              aggregate_failures do
+                expect { push_access_check }.to raise_error
+                expect { pull_access_check }.to raise_error
+              end
+            end
+          end
+        end
+      end
     end
 
     context 'without OTP session' do