From 5ccd9f39098fbeabef912e2fa0300542feb31cb6 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 2 Jul 2024 12:52:39 -0700
Subject: [PATCH] Add require_personal_access_token_expiry application setting

---
 app/helpers/access_tokens_helper.rb                  |  2 ++
 app/helpers/application_settings_helper.rb           |  4 +++-
 app/models/application_setting_implementation.rb     |  3 ++-
 app/models/personal_access_token.rb                  |  2 +-
 .../personal_access_tokens/create_service.rb         |  6 +++++-
 .../resource_access_tokens/create_service.rb         |  8 +++++++-
 .../_account_and_limit.html.haml                     |  5 +++++
 ...add_require_pat_expiry_to_application_settings.rb |  9 +++++++++
 db/schema_migrations/20240702181131                  |  1 +
 db/structure.sql                                     |  1 +
 .../service_account_token_validator.rb               |  4 +++-
 locale/gitlab.pot                                    |  6 ++++++
 spec/helpers/access_tokens_helper_spec.rb            | 12 +++++++++++-
 13 files changed, 56 insertions(+), 7 deletions(-)
 create mode 100644 db/migrate/20240702181131_add_require_pat_expiry_to_application_settings.rb
 create mode 100644 db/schema_migrations/20240702181131

diff --git a/app/helpers/access_tokens_helper.rb b/app/helpers/access_tokens_helper.rb
index f05c764bfb40e..66b5256a18b18 100644
--- a/app/helpers/access_tokens_helper.rb
+++ b/app/helpers/access_tokens_helper.rb
@@ -36,6 +36,8 @@ def tokens_app_data
   end
 
   def expires_at_field_data
+    return {} unless Gitlab::CurrentSettings.require_personal_access_token_expiry?
+
     {
       min_date: 1.day.from_now.iso8601
     }
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 5ba028a3b0324..e14186b6929bd 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -8,6 +8,7 @@ module ApplicationSettingsHelper
     :password_authentication_enabled_for_web?,
     :akismet_enabled?,
     :spam_check_endpoint_enabled?,
+    :require_personal_access_token_expiry?,
     to: :'Gitlab::CurrentSettings.current_application_settings'
 
   def user_oauth_applications?
@@ -526,7 +527,8 @@ def visible_attributes
       :downstream_pipeline_trigger_limit_per_project_user_sha,
       :asciidoc_max_includes,
       :ai_action_api_rate_limit,
-      :code_suggestions_api_rate_limit
+      :code_suggestions_api_rate_limit,
+      :require_personal_access_token_expiry
     ].tap do |settings|
       unless Gitlab.com?
         settings << :deactivate_dormant_users
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index 2d14747edff4e..a7243dbab653f 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -289,7 +289,8 @@ def defaults # rubocop:disable Metrics/AbcSize
         user_starred_projects_api_limit: 100,
         nuget_skip_metadata_url_validation: false,
         ai_action_api_rate_limit: 160,
-        code_suggestions_api_rate_limit: 60
+        code_suggestions_api_rate_limit: 60,
+        require_personal_access_token_expiry: true
       }.tap do |hsh|
         hsh.merge!(non_production_defaults) unless Rails.env.production?
       end
diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index 4135b398b0982..9c98ac21b7bff 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -104,7 +104,7 @@ def prefix_from_application_current_settings
   end
 
   def allow_expires_at_to_be_empty?
-    false
+    !Gitlab::CurrentSettings.require_personal_access_token_expiry?
   end
 
   def expires_at_before_instance_max_expiry_date
diff --git a/app/services/personal_access_tokens/create_service.rb b/app/services/personal_access_tokens/create_service.rb
index 095cfadf02c80..a8bc922e8a288 100644
--- a/app/services/personal_access_tokens/create_service.rb
+++ b/app/services/personal_access_tokens/create_service.rb
@@ -41,7 +41,11 @@ def personal_access_token_params
     end
 
     def pat_expiration
-      params[:expires_at].presence || max_expiry_date
+      return params[:expires_at] if params[:expires_at].present?
+
+      return max_expiry_date if Gitlab::CurrentSettings.require_personal_access_token_expiry?
+
+      nil
     end
 
     def max_expiry_date
diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb
index 9e195184f47a6..d6931d997cee3 100644
--- a/app/services/resource_access_tokens/create_service.rb
+++ b/app/services/resource_access_tokens/create_service.rb
@@ -117,7 +117,13 @@ def create_membership(resource, user, access_level)
     end
 
     def pat_expiration
-      params[:expires_at].presence || PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now
+      return params[:expires_at] if params[:expires_at].present?
+
+      if Gitlab::CurrentSettings.require_personal_access_token_expiry?
+        return PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now
+      end
+
+      nil
     end
 
     def log_event(token)
diff --git a/app/views/admin/application_settings/_account_and_limit.html.haml b/app/views/admin/application_settings/_account_and_limit.html.haml
index 8708d2afc0e6b..33137daa3c4ca 100644
--- a/app/views/admin/application_settings/_account_and_limit.html.haml
+++ b/app/views/admin/application_settings/_account_and_limit.html.haml
@@ -29,6 +29,11 @@
       = f.gitlab_ui_checkbox_component :remember_me_enabled, _('Allow users to extend their session'), help_text: _("Users can select 'Remember me' on sign-in to keep their session active beyond the session duration. %{link_start}Learn more%{link_end}.").html_safe % { link_start: remember_me_help_link_start, link_end: '</a>'.html_safe }
 
     = render_if_exists 'admin/application_settings/git_two_factor_session_expiry', form: f
+    %h5
+      = _('Personal/project/group access token expiration')
+    .form-group
+      = f.gitlab_ui_checkbox_component :require_personal_access_token_expiry, _('Require expiration date'),
+        help_text: _('When enabled, a user will be required to enter in an expiration date when creating an access token. Changes will not affect existing token expiration dates.')
     = render_if_exists 'admin/application_settings/personal_access_token_expiration_policy', form: f
     = render_if_exists 'admin/application_settings/service_access_tokens_expiration_enforced', form: f
     = render_if_exists 'admin/application_settings/ssh_key_expiration_policy', form: f
diff --git a/db/migrate/20240702181131_add_require_pat_expiry_to_application_settings.rb b/db/migrate/20240702181131_add_require_pat_expiry_to_application_settings.rb
new file mode 100644
index 0000000000000..5ecc89fa2172e
--- /dev/null
+++ b/db/migrate/20240702181131_add_require_pat_expiry_to_application_settings.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddRequirePatExpiryToApplicationSettings < Gitlab::Database::Migration[2.2]
+  milestone '17.2'
+
+  def change
+    add_column :application_settings, :require_personal_access_token_expiry, :boolean, default: true, null: false
+  end
+end
diff --git a/db/schema_migrations/20240702181131 b/db/schema_migrations/20240702181131
new file mode 100644
index 0000000000000..4b8ba284bd507
--- /dev/null
+++ b/db/schema_migrations/20240702181131
@@ -0,0 +1 @@
+ce07813b40454e38bdcc467c3c313573f1944848974f39d4ceea2c299714c6ac
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 5d6744e3c0c54..e74005aa661cb 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -5769,6 +5769,7 @@ CREATE TABLE application_settings (
     code_creation jsonb DEFAULT '{}'::jsonb NOT NULL,
     code_suggestions_api_rate_limit integer DEFAULT 60 NOT NULL,
     ai_action_api_rate_limit integer DEFAULT 160 NOT NULL,
+    require_personal_access_token_expiry boolean DEFAULT true NOT NULL,
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
     CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
diff --git a/ee/lib/ee/gitlab/personal_access_tokens/service_account_token_validator.rb b/ee/lib/ee/gitlab/personal_access_tokens/service_account_token_validator.rb
index 494a9b0689565..df560eb83a8e6 100644
--- a/ee/lib/ee/gitlab/personal_access_tokens/service_account_token_validator.rb
+++ b/ee/lib/ee/gitlab/personal_access_tokens/service_account_token_validator.rb
@@ -11,7 +11,9 @@ def initialize(service_account_user)
         attr_accessor :service_account_user
 
         def expiry_enforced?
-          return true unless License.feature_available?(:service_accounts) && service_account_user.service_account?
+          unless License.feature_available?(:service_accounts) && service_account_user.service_account?
+            return ::Gitlab::CurrentSettings.require_personal_access_token_expiry?
+          end
 
           if saas?
             return true unless service_account_scoped_to_group
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 061390a3cb11e..797d041a30cb7 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -38378,6 +38378,9 @@ msgstr ""
 msgid "Personal projects limit:"
 msgstr ""
 
+msgid "Personal/project/group access token expiration"
+msgstr ""
+
 msgid "PersonalProject|Learn to move a project to a group"
 msgstr ""
 
@@ -59811,6 +59814,9 @@ msgstr ""
 msgid "When enabled, SSH keys with no expiry date or an invalid expiration date are no longer accepted. Leave blank for no limit."
 msgstr ""
 
+msgid "When enabled, a user will be required to enter in an expiration date when creating an access token. Changes will not affect existing token expiration dates."
+msgstr ""
+
 msgid "When enabled, cleanup policies execute faster but put more load on Redis."
 msgstr ""
 
diff --git a/spec/helpers/access_tokens_helper_spec.rb b/spec/helpers/access_tokens_helper_spec.rb
index 576045b22d4bc..a013521d54357 100644
--- a/spec/helpers/access_tokens_helper_spec.rb
+++ b/spec/helpers/access_tokens_helper_spec.rb
@@ -2,7 +2,7 @@
 
 require "spec_helper"
 
-RSpec.describe AccessTokensHelper do
+RSpec.describe AccessTokensHelper, feature_category: :system_access do
   describe "#scope_description" do
     using RSpec::Parameterized::TableSyntax
 
@@ -72,6 +72,16 @@
         min_date: 1.day.from_now.iso8601
       })
     end
+
+    context 'when require_personal_access_token_expiry is true' do
+      before do
+        stub_application_setting(require_personal_access_token_expiry: false)
+      end
+
+      it 'returns an empty hash' do
+        expect(helper.expires_at_field_data).to eq({})
+      end
+    end
   end
 
   describe '#show_token_expiration_banner?' do
-- 
GitLab