From 247f9bceb96f7c5c5cb3152158439d018ea52abe Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Tue, 2 Apr 2024 19:49:55 +0530
Subject: [PATCH 01/11] Disable personal access tokens for enterprise users

Allow admins of enterprise groups to disable personal access token
generation for the enterprise users. Token which are created before
disabling personal access tokens will be invalid.

Changelog: fixed
EE: true
---
 .../personal_access_tokens_controller.rb      |  4 ++
 .../groups/settings/_permissions.html.haml    |  1 +
 ...nal_access_tokens_to_namespace_settings.rb | 10 ++++
 db/schema_migrations/20240402120744           |  1 +
 db/structure.sql                              |  1 +
 ee/app/controllers/ee/groups_controller.rb    |  1 +
 ee/app/models/ee/group.rb                     |  5 ++
 ee/app/models/ee/namespace.rb                 |  2 +
 .../_personal_access_tokens.html.haml         | 10 ++++
 ...erprise_disable_personal_access_tokens.yml |  9 ++++
 ee/lib/ee/gitlab/auth/auth_finders.rb         | 10 ++++
 .../user_settings/menus/access_tokens_menu.rb | 20 ++++++++
 .../lib/ee/gitlab/auth/auth_finders_spec.rb   | 46 ++++++++++++++++++-
 .../menus/access_tokens_menu_spec.rb          | 35 ++++++++++++++
 .../user_settings/menus/access_tokens_menu.rb |  7 ++-
 locale/gitlab.pot                             |  9 ++++
 .../menus/access_tokens_menu_spec.rb          |  2 +-
 17 files changed, 169 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb
 create mode 100644 db/schema_migrations/20240402120744
 create mode 100644 ee/app/views/groups/settings/_personal_access_tokens.html.haml
 create mode 100644 ee/config/feature_flags/gitlab_com_derisk/enterprise_disable_personal_access_tokens.yml
 create mode 100644 ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb
 create mode 100644 ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb

diff --git a/app/controllers/user_settings/personal_access_tokens_controller.rb b/app/controllers/user_settings/personal_access_tokens_controller.rb
index a8e9a328c2606f..e33ca19331172f 100644
--- a/app/controllers/user_settings/personal_access_tokens_controller.rb
+++ b/app/controllers/user_settings/personal_access_tokens_controller.rb
@@ -71,6 +71,10 @@ def represent(tokens)
 
     def check_personal_access_tokens_enabled
       render_404 if Gitlab::CurrentSettings.personal_access_tokens_disabled?
+
+      if current_user.enterprise_user? && current_user.user_detail.enterprise_group.disable_personal_access_tokens?
+        render_404
+      end
     end
   end
 end
diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml
index 804eb4f6554c89..f84e2ef9220ec8 100644
--- a/app/views/groups/settings/_permissions.html.haml
+++ b/app/views/groups/settings/_permissions.html.haml
@@ -50,6 +50,7 @@
     = render 'groups/settings/two_factor_auth', f: f, group: @group
     = render_if_exists 'groups/personal_access_token_expiration_policy', f: f, group: @group
     = render 'groups/settings/membership', f: f, group: @group
+    = render_if_exists 'groups/settings/personal_access_tokens', f: f, group: @group
 
     %h5= _('Customer relations')
     .form-group.gl-mb-3
diff --git a/db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb b/db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb
new file mode 100644
index 00000000000000..2b8d7c117b3c24
--- /dev/null
+++ b/db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class AddDisablePersonalAccessTokensToNamespaceSettings < Gitlab::Database::Migration[2.2]
+  enable_lock_retries!
+  milestone '16.11'
+
+  def change
+    add_column :namespace_settings, :disable_personal_access_tokens, :boolean, default: false, null: false
+  end
+end
diff --git a/db/schema_migrations/20240402120744 b/db/schema_migrations/20240402120744
new file mode 100644
index 00000000000000..5583a96610de06
--- /dev/null
+++ b/db/schema_migrations/20240402120744
@@ -0,0 +1 @@
+bd1e2bdc4fda7c033a3407516450f15b45bfab4d6600a7bc940d10ca38f3e491
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index d3873e787420ff..9a79731504bb23 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -11882,6 +11882,7 @@ CREATE TABLE namespace_settings (
     lock_math_rendering_limits_enabled boolean DEFAULT false NOT NULL,
     duo_features_enabled boolean,
     lock_duo_features_enabled boolean DEFAULT false NOT NULL,
+    disable_personal_access_tokens boolean DEFAULT false NOT NULL,
     CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)),
     CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)),
     CONSTRAINT namespace_settings_unique_project_download_limit_allowlist_size CHECK ((cardinality(unique_project_download_limit_allowlist) <= 100))
diff --git a/ee/app/controllers/ee/groups_controller.rb b/ee/app/controllers/ee/groups_controller.rb
index 4e5b33a24f89ac..1f0d486eb4b4d0 100644
--- a/ee/app/controllers/ee/groups_controller.rb
+++ b/ee/app/controllers/ee/groups_controller.rb
@@ -97,6 +97,7 @@ def group_params_ee
         params_ee << { value_stream_dashboard_aggregation_attributes: [:enabled] } if can?(current_user, :modify_value_stream_dashboard_settings, current_group)
         params_ee << :experiment_features_enabled if experiment_settings_allowed?
         params_ee.push(%i[duo_features_enabled lock_duo_features_enabled]) if licensed_ai_features_available?
+        params_ee << :disable_personal_access_tokens
       end + security_policies_toggle_params
     end
 
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 1b98ae375a849e..7432887b17d001 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -1005,6 +1005,11 @@ def code_suggestions_purchased?
       ::GitlabSubscriptions::AddOnPurchase.active.for_gitlab_duo_pro.exists?(namespace_id: id)
     end
 
+    # Disable personal access tokens for enterprise users of this group
+    def disable_personal_access_tokens?
+      ::Feature.enabled?(:enterprise_disable_personal_access_tokens, self) && root? && namespace_settings.disable_personal_access_tokens?
+    end
+
     private
 
     def active_project_tokens_of_root_ancestor
diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb
index 201e7792ea58db..8fd8b93075678a 100644
--- a/ee/app/models/ee/namespace.rb
+++ b/ee/app/models/ee/namespace.rb
@@ -168,6 +168,8 @@ module Namespace
 
       delegate :security_policy_management_project, to: :security_orchestration_policy_configuration, allow_nil: true
 
+      delegate :disable_personal_access_tokens=, to: :namespace_settings
+
       before_create :sync_membership_lock_with_parent
 
       # Changing the plan or other details may invalidate this cache
diff --git a/ee/app/views/groups/settings/_personal_access_tokens.html.haml b/ee/app/views/groups/settings/_personal_access_tokens.html.haml
new file mode 100644
index 00000000000000..d5345109c39876
--- /dev/null
+++ b/ee/app/views/groups/settings/_personal_access_tokens.html.haml
@@ -0,0 +1,10 @@
+- return unless group.root? && group.licensed_feature_available?(:disable_personal_access_tokens)
+
+%h5= _('Personal access tokens')
+
+.form-group.gl-mb-3
+  = f.gitlab_ui_checkbox_component :disable_personal_access_tokens, checkbox_options: { checked: group.disable_personal_access_tokens? } do |c|
+    - c.with_label do
+      = s_('GroupSettings|Disable Personal access tokens')
+    - c.with_help_text do
+      = s_("GroupSettings|If enabled, enterprise user accounts will not be able to use personal access tokens.")
diff --git a/ee/config/feature_flags/gitlab_com_derisk/enterprise_disable_personal_access_tokens.yml b/ee/config/feature_flags/gitlab_com_derisk/enterprise_disable_personal_access_tokens.yml
new file mode 100644
index 00000000000000..e09e1b4570053a
--- /dev/null
+++ b/ee/config/feature_flags/gitlab_com_derisk/enterprise_disable_personal_access_tokens.yml
@@ -0,0 +1,9 @@
+---
+name: enterprise_disable_personal_access_tokens
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/369504
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148415
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/455064
+milestone: '16.11'
+group: group::authentication
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/ee/lib/ee/gitlab/auth/auth_finders.rb b/ee/lib/ee/gitlab/auth/auth_finders.rb
index 31fe3a8152c03a..a146ac5b143ea4 100644
--- a/ee/lib/ee/gitlab/auth/auth_finders.rb
+++ b/ee/lib/ee/gitlab/auth/auth_finders.rb
@@ -37,6 +37,16 @@ def find_oauth_access_token
           super
         end
 
+        override :find_user_from_access_token
+        def find_user_from_access_token
+          user = super
+          if user && user.enterprise_user? && user.user_detail.enterprise_group.disable_personal_access_tokens?
+            raise ::Gitlab::Auth::UnauthorizedError
+          end
+
+          user
+        end
+
         def scim_request?
           current_request.path.starts_with?("/api/scim/")
         end
diff --git a/ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb b/ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb
new file mode 100644
index 00000000000000..60e225f0e1c997
--- /dev/null
+++ b/ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module EE
+  module Sidebars
+    module UserSettings
+      module Menus
+        module AccessTokensMenu
+          extend ::Gitlab::Utils::Override
+
+          override :render?
+          def render?
+            return false if context.current_user&.user_detail&.enterprise_group&.disable_personal_access_tokens?
+
+            super
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
index 77e0693d2c8448..7eedfe1a8da2e6 100644
--- a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
+++ b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Auth::AuthFinders do
+RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do
   include described_class
   include ::EE::GeoHelpers
 
@@ -140,7 +140,7 @@
         expect(find_user_from_access_token).to eq user
       end
 
-      context 'when personal access tokens are disabled' do
+      context 'when personal access tokens are disabled in instance level' do
         before do
           stub_licensed_features(disable_personal_access_tokens: true)
           stub_application_setting(disable_personal_access_tokens: true)
@@ -150,6 +150,48 @@
           expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
         end
       end
+
+      context 'when personal access tokens are disabled for the group for enterprise users' do
+        let(:user) { create(:enterprise_user) }
+
+        before do
+          user.user_detail.enterprise_group.update!(disable_personal_access_tokens: true)
+        end
+
+        it 'raised unauthorized error' do
+          expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+        end
+      end
+
+      context 'when personal access tokens are disabled for the group for service accounts' do
+        let(:user) { create(:service_account, provisioned_by_group: create(:group)) }
+
+        before do
+          user.provisioned_by_group.update!(disable_personal_access_tokens: true)
+        end
+
+        it 'returns user' do
+          expect(find_user_from_access_token).to eq user
+        end
+      end
+
+      context 'when the feature flag enterprise_disable_personal_access_tokens is disabled' do
+        before do
+          stub_feature_flags(enterprise_disable_personal_access_tokens: false)
+        end
+
+        context 'when personal access tokens are disabled for the group for enterprise users' do
+          let(:user) { create(:enterprise_user) }
+
+          before do
+            user.user_detail.enterprise_group.update!(disable_personal_access_tokens: true)
+          end
+
+          it 'returns user' do
+            expect(find_user_from_access_token).to eq user
+          end
+        end
+      end
     end
   end
 
diff --git a/ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb b/ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb
new file mode 100644
index 00000000000000..abeaa247dc5bcc
--- /dev/null
+++ b/ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::UserSettings::Menus::AccessTokensMenu, feature_category: :system_access do
+  subject(:sidebar_item) { described_class.new(context) }
+
+  let_it_be(:user) { build(:user) }
+
+  context 'when personal access tokens are disabled for enterprise users' do
+    let(:user) { build(:enterprise_user) }
+
+    before do
+      allow(user.user_detail.enterprise_group).to receive(:disable_personal_access_tokens?).and_return(true)
+    end
+
+    context 'when user is logged in' do
+      let(:context) { Sidebars::Context.new(current_user: user, container: nil) }
+
+      it 'does not render' do
+        expect(sidebar_item.render?).to be false
+      end
+    end
+
+    context 'when user is not logged in' do
+      let(:context) { Sidebars::Context.new(current_user: nil, container: nil) }
+
+      subject { described_class.new(context) }
+
+      it 'does not render' do
+        expect(sidebar_item.render?).to be false
+      end
+    end
+  end
+end
diff --git a/lib/sidebars/user_settings/menus/access_tokens_menu.rb b/lib/sidebars/user_settings/menus/access_tokens_menu.rb
index cb3f319ddde5fe..3f875eaa08ba3a 100644
--- a/lib/sidebars/user_settings/menus/access_tokens_menu.rb
+++ b/lib/sidebars/user_settings/menus/access_tokens_menu.rb
@@ -21,7 +21,10 @@ def sprite_icon
 
         override :render?
         def render?
-          !!context.current_user && !Gitlab::CurrentSettings.personal_access_tokens_disabled?
+          return false unless context.current_user
+          return false if Gitlab::CurrentSettings.personal_access_tokens_disabled?
+
+          true
         end
 
         override :active_routes
@@ -37,3 +40,5 @@ def extra_container_html_options
     end
   end
 end
+
+Sidebars::UserSettings::Menus::AccessTokensMenu.prepend_mod
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f20d7363e4aa44..79c39acab6ce6a 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24806,6 +24806,9 @@ msgstr ""
 msgid "GroupSettings|Default to Auto DevOps pipeline for all projects within this group"
 msgstr ""
 
+msgid "GroupSettings|Disable Personal access tokens"
+msgstr ""
+
 msgid "GroupSettings|Duo features"
 msgstr ""
 
@@ -24854,6 +24857,9 @@ msgstr ""
 msgid "GroupSettings|How do I manage group SSH certificates?"
 msgstr ""
 
+msgid "GroupSettings|If enabled, enterprise user accounts will not be able to use personal access tokens."
+msgstr ""
+
 msgid "GroupSettings|If enabled, individual user accounts will be able to use only issued SSH certificates for Git access. It doesn't apply to service accounts, deploy keys, and other types of internal accounts."
 msgstr ""
 
@@ -36882,6 +36888,9 @@ msgstr ""
 msgid "Personal access token"
 msgstr ""
 
+msgid "Personal access tokens"
+msgstr ""
+
 msgid "Personal projects"
 msgstr ""
 
diff --git a/spec/lib/sidebars/user_settings/menus/access_tokens_menu_spec.rb b/spec/lib/sidebars/user_settings/menus/access_tokens_menu_spec.rb
index eebd089ad3f946..ad96c1e4324e5a 100644
--- a/spec/lib/sidebars/user_settings/menus/access_tokens_menu_spec.rb
+++ b/spec/lib/sidebars/user_settings/menus/access_tokens_menu_spec.rb
@@ -14,7 +14,7 @@
 
     let_it_be(:user) { build(:user) }
 
-    context 'when personal access tokens are disabled' do
+    context 'when personal access tokens are disabled in the instance' do
       before do
         allow(::Gitlab::CurrentSettings).to receive_messages(personal_access_tokens_disabled?: true)
       end
-- 
GitLab


From 3d650bec202d38ef80b6aa87981c26e54f5988f1 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Tue, 9 Apr 2024 16:13:59 +0000
Subject: [PATCH 02/11] Apply 1 suggestion(s) to 1 file(s)

---
 ee/lib/ee/gitlab/auth/auth_finders.rb | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ee/lib/ee/gitlab/auth/auth_finders.rb b/ee/lib/ee/gitlab/auth/auth_finders.rb
index a146ac5b143ea4..3bfa4ef398843b 100644
--- a/ee/lib/ee/gitlab/auth/auth_finders.rb
+++ b/ee/lib/ee/gitlab/auth/auth_finders.rb
@@ -40,11 +40,12 @@ def find_oauth_access_token
         override :find_user_from_access_token
         def find_user_from_access_token
           user = super
-          if user && user.enterprise_user? && user.user_detail.enterprise_group.disable_personal_access_tokens?
-            raise ::Gitlab::Auth::UnauthorizedError
-          end
 
-          user
+          disable_personal_access_tokens = user&.enterprise_user? &&          
+            user.user_detail.enterprise_group.disable_personal_access_tokens?
+          return user unless disable_personal_access_tokens
+
+          raise ::Gitlab::Auth::UnauthorizedError
         end
 
         def scim_request?
-- 
GitLab


From 137968e35447847d67cd8b675672e45c6b8a4e97 Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Wed, 10 Apr 2024 23:10:06 +0530
Subject: [PATCH 03/11] Fix CI

---
 .../personal_access_tokens_controller.rb       |  6 ++----
 .../personal_access_tokens_controller.rb       | 18 ++++++++++++++++++
 ee/app/models/ee/group.rb                      |  2 ++
 ee/app/models/ee/namespace.rb                  |  2 --
 ee/app/models/ee/user.rb                       |  6 +++++-
 ee/lib/ee/gitlab/auth/auth_finders.rb          |  2 +-
 ee/spec/requests/api/epics_spec.rb             |  4 ++++
 7 files changed, 32 insertions(+), 8 deletions(-)
 create mode 100644 ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb

diff --git a/app/controllers/user_settings/personal_access_tokens_controller.rb b/app/controllers/user_settings/personal_access_tokens_controller.rb
index e33ca19331172f..0bb71966b2141d 100644
--- a/app/controllers/user_settings/personal_access_tokens_controller.rb
+++ b/app/controllers/user_settings/personal_access_tokens_controller.rb
@@ -71,10 +71,8 @@ def represent(tokens)
 
     def check_personal_access_tokens_enabled
       render_404 if Gitlab::CurrentSettings.personal_access_tokens_disabled?
-
-      if current_user.enterprise_user? && current_user.user_detail.enterprise_group.disable_personal_access_tokens?
-        render_404
-      end
     end
   end
 end
+
+UserSettings::PersonalAccessTokensController.prepend_mod
diff --git a/ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb b/ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb
new file mode 100644
index 00000000000000..1a83a3f4e7929b
--- /dev/null
+++ b/ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module EE
+  module UserSettings
+    module PersonalAccessTokensController
+      extend ::Gitlab::Utils::Override
+
+      override :check_personal_access_tokens_enabled
+      def check_personal_access_tokens_enabled
+        super
+
+        if current_user.enterprise_user? && current_user.user_detail.enterprise_group.disable_personal_access_tokens?
+          render_404
+        end
+      end
+    end
+  end
+end
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 7432887b17d001..89536cbb5e10bf 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -97,6 +97,8 @@ module Group
       delegate :experiment_settings_allowed?, to: :namespace_settings
       delegate :user_cap_enabled?, to: :namespace_settings
 
+      delegate :disable_personal_access_tokens=, to: :namespace_settings
+
       delegate :wiki_access_level, :wiki_access_level=, to: :group_feature, allow_nil: true
 
       # Use +checked_file_template_project+ instead, which implements important
diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb
index 8fd8b93075678a..201e7792ea58db 100644
--- a/ee/app/models/ee/namespace.rb
+++ b/ee/app/models/ee/namespace.rb
@@ -168,8 +168,6 @@ module Namespace
 
       delegate :security_policy_management_project, to: :security_orchestration_policy_configuration, allow_nil: true
 
-      delegate :disable_personal_access_tokens=, to: :namespace_settings
-
       before_create :sync_membership_lock_with_parent
 
       # Changing the plan or other details may invalidate this cache
diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index 6b4f21cf1a7fab..4da0f6575ba417 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -529,7 +529,11 @@ def enterprise_user_of_group?(group)
     end
 
     def enterprise_user?
-      user_detail.enterprise_group.present?
+      if association(:user_detail).loaded?
+        user_detail.enterprise_group.present?
+      else
+        ::User.joins(user_detail: :enterprise_group).exists?(id: id)
+      end
     end
 
     def gitlab_employee?
diff --git a/ee/lib/ee/gitlab/auth/auth_finders.rb b/ee/lib/ee/gitlab/auth/auth_finders.rb
index 3bfa4ef398843b..cbc9a937578eaf 100644
--- a/ee/lib/ee/gitlab/auth/auth_finders.rb
+++ b/ee/lib/ee/gitlab/auth/auth_finders.rb
@@ -41,7 +41,7 @@ def find_oauth_access_token
         def find_user_from_access_token
           user = super
 
-          disable_personal_access_tokens = user&.enterprise_user? &&          
+          disable_personal_access_tokens = user&.enterprise_user? &&
             user.user_detail.enterprise_group.disable_personal_access_tokens?
           return user unless disable_personal_access_tokens
 
diff --git a/ee/spec/requests/api/epics_spec.rb b/ee/spec/requests/api/epics_spec.rb
index d06feb40057dfb..dde9f37a4ccc1e 100644
--- a/ee/spec/requests/api/epics_spec.rb
+++ b/ee/spec/requests/api/epics_spec.rb
@@ -928,6 +928,10 @@
       end
 
       it_behaves_like 'PUT request permissions for admin mode' do
+        before do
+          allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(101)
+        end
+
         let(:path) { url }
       end
 
-- 
GitLab


From 39e32a54784abdd2dc8af1a22e9302a287f25a4d Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Wed, 10 Apr 2024 23:54:49 +0530
Subject: [PATCH 04/11] Fix CI from master

---
 ee/spec/requests/api/epics_spec.rb | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/ee/spec/requests/api/epics_spec.rb b/ee/spec/requests/api/epics_spec.rb
index dde9f37a4ccc1e..7904bff500ded3 100644
--- a/ee/spec/requests/api/epics_spec.rb
+++ b/ee/spec/requests/api/epics_spec.rb
@@ -673,6 +673,7 @@
 
       context 'when the request is correct' do
         before do
+          allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(101)
           post api(url, user), params: params
         end
 
@@ -822,6 +823,7 @@
 
       context 'with rate limiter', :freeze_time, :clean_gitlab_redis_rate_limiting do
         before do
+          allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(101)
           stub_application_setting(issues_create_limit: 1)
         end
 
@@ -873,7 +875,7 @@
       it 'creates a new epic with labels param as array' do
         # TODO: remove threshold after epic-work item sync
         # issue: https://gitlab.com/gitlab-org/gitlab/-/issues/438295
-        allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(135)
+        allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(136)
         params[:labels] = ['label1', 'label2', 'foo, bar', '&,?']
 
         post api(url, user), params: params
@@ -924,14 +926,10 @@
         stub_licensed_features(epics: true, subepics: true)
         # TODO: reduce threshold after epic-work item sync
         # issue: https://gitlab.com/gitlab-org/gitlab/-/issues/438295
-        allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(147)
+        allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(148)
       end
 
       it_behaves_like 'PUT request permissions for admin mode' do
-        before do
-          allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(101)
-        end
-
         let(:path) { url }
       end
 
@@ -971,7 +969,7 @@
         before do
           # TODO: reduce threshold after epic-work item sync
           # issue: https://gitlab.com/gitlab-org/gitlab/-/issues/438295
-          allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(174)
+          allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(175)
 
           group.add_developer(user)
         end
-- 
GitLab


From 3f49096fa990d5597972d0276395e814a0ab5082 Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Thu, 11 Apr 2024 06:16:25 +0530
Subject: [PATCH 05/11] Simplify the enterprise_user check

---
 ee/app/models/ee/user.rb | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index 4da0f6575ba417..8a228066d9afc9 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -529,11 +529,7 @@ def enterprise_user_of_group?(group)
     end
 
     def enterprise_user?
-      if association(:user_detail).loaded?
-        user_detail.enterprise_group.present?
-      else
-        ::User.joins(user_detail: :enterprise_group).exists?(id: id)
-      end
+      user_detail.enterprise_group_id.present?
     end
 
     def gitlab_employee?
-- 
GitLab


From f4a47d5a80200fd22611548d9a29f84314726451 Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Thu, 11 Apr 2024 11:34:46 +0530
Subject: [PATCH 06/11] Increase threshold for query recorder

---
 spec/requests/api/ci/jobs_spec.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb
index 9e1203bc720988..9393ddb15ec57a 100644
--- a/spec/requests/api/ci/jobs_spec.rb
+++ b/spec/requests/api/ci/jobs_spec.rb
@@ -821,6 +821,7 @@ def go
     let(:job) { create(:ci_build, :canceled, pipeline: pipeline) }
 
     before do
+      allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(102)
       post api("/projects/#{project.id}/jobs/#{job.id}/retry", api_user)
     end
 
-- 
GitLab


From 9a6736fca6e471b75b721ceaefef6efd784606e4 Mon Sep 17 00:00:00 2001
From: Jon Glassman <jglassman@gitlab.com>
Date: Thu, 11 Apr 2024 08:04:40 +0000
Subject: [PATCH 07/11] Apply 2 suggestion(s) to 2 file(s)

---
 ee/app/views/groups/settings/_personal_access_tokens.html.haml | 2 +-
 locale/gitlab.pot                                              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/views/groups/settings/_personal_access_tokens.html.haml b/ee/app/views/groups/settings/_personal_access_tokens.html.haml
index d5345109c39876..421d7182282a3b 100644
--- a/ee/app/views/groups/settings/_personal_access_tokens.html.haml
+++ b/ee/app/views/groups/settings/_personal_access_tokens.html.haml
@@ -5,6 +5,6 @@
 .form-group.gl-mb-3
   = f.gitlab_ui_checkbox_component :disable_personal_access_tokens, checkbox_options: { checked: group.disable_personal_access_tokens? } do |c|
     - c.with_label do
-      = s_('GroupSettings|Disable Personal access tokens')
+      = s_('GroupSettings|Disable personal access tokens')
     - c.with_help_text do
       = s_("GroupSettings|If enabled, enterprise user accounts will not be able to use personal access tokens.")
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 79c39acab6ce6a..7caa927d401a3e 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24806,7 +24806,7 @@ msgstr ""
 msgid "GroupSettings|Default to Auto DevOps pipeline for all projects within this group"
 msgstr ""
 
-msgid "GroupSettings|Disable Personal access tokens"
+msgid "GroupSettings|Disable personal access tokens"
 msgstr ""
 
 msgid "GroupSettings|Duo features"
-- 
GitLab


From a57f790d0c7f230e4b33693fd6e95c63dffb3c29 Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Thu, 11 Apr 2024 13:36:35 +0530
Subject: [PATCH 08/11] Review suggestions

---
 ..._add_disable_personal_access_tokens_to_namespace_settings.rb | 1 -
 ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb                 | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb b/db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb
index 2b8d7c117b3c24..ea6e7ca13ada10 100644
--- a/db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb
+++ b/db/migrate/20240402120744_add_disable_personal_access_tokens_to_namespace_settings.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 class AddDisablePersonalAccessTokensToNamespaceSettings < Gitlab::Database::Migration[2.2]
-  enable_lock_retries!
   milestone '16.11'
 
   def change
diff --git a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
index 7eedfe1a8da2e6..534f3749107545 100644
--- a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
+++ b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
@@ -140,7 +140,7 @@
         expect(find_user_from_access_token).to eq user
       end
 
-      context 'when personal access tokens are disabled in instance level' do
+      context 'when personal access tokens are disabled on instance level' do
         before do
           stub_licensed_features(disable_personal_access_tokens: true)
           stub_application_setting(disable_personal_access_tokens: true)
-- 
GitLab


From cbe9605e9bb11baece3531ada9e7af88ab0c7ff2 Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Thu, 11 Apr 2024 13:43:22 +0530
Subject: [PATCH 09/11] Changes suggested on review

---
 .../ee/user_settings/personal_access_tokens_controller.rb   | 6 +++---
 ee/app/models/ee/user.rb                                    | 6 ++++--
 ee/lib/ee/gitlab/auth/auth_finders.rb                       | 2 +-
 .../ee/sidebars/user_settings/menus/access_tokens_menu.rb   | 2 +-
 ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb             | 4 ++--
 .../sidebars/user_settings/menus/access_tokens_menu_spec.rb | 2 +-
 6 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb b/ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb
index 1a83a3f4e7929b..d6a1ea92da6c80 100644
--- a/ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb
+++ b/ee/app/controllers/ee/user_settings/personal_access_tokens_controller.rb
@@ -9,9 +9,9 @@ module PersonalAccessTokensController
       def check_personal_access_tokens_enabled
         super
 
-        if current_user.enterprise_user? && current_user.user_detail.enterprise_group.disable_personal_access_tokens?
-          render_404
-        end
+        return unless current_user.enterprise_user? && current_user.enterprise_group.disable_personal_access_tokens?
+
+        render_404
       end
     end
   end
diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index 8a228066d9afc9..28cf4f78541c73 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -59,6 +59,8 @@ module User
         :onboarding_status_registration_type, :onboarding_status_registration_type=,
         :onboarding_status_email_opt_in, :onboarding_status_email_opt_in=, :onboarding_status, :onboarding_status=,
         :onboarding_status_initial_registration_type, :onboarding_status_initial_registration_type=,
+        :enterprise_group, :enterprise_group=,
+        :enterprise_group_id, :enterprise_group_id=,
         to: :user_detail, allow_nil: true
 
       delegate :enabled_zoekt?, :enabled_zoekt, :enabled_zoekt=,
@@ -525,11 +527,11 @@ def authorized_by_provisioning_group?(group)
     end
 
     def enterprise_user_of_group?(group)
-      user_detail.enterprise_group_id == group.id
+      enterprise_group_id == group.id
     end
 
     def enterprise_user?
-      user_detail.enterprise_group_id.present?
+      enterprise_group_id.present?
     end
 
     def gitlab_employee?
diff --git a/ee/lib/ee/gitlab/auth/auth_finders.rb b/ee/lib/ee/gitlab/auth/auth_finders.rb
index cbc9a937578eaf..17ddfad5d7cb5c 100644
--- a/ee/lib/ee/gitlab/auth/auth_finders.rb
+++ b/ee/lib/ee/gitlab/auth/auth_finders.rb
@@ -42,7 +42,7 @@ def find_user_from_access_token
           user = super
 
           disable_personal_access_tokens = user&.enterprise_user? &&
-            user.user_detail.enterprise_group.disable_personal_access_tokens?
+            user.enterprise_group.disable_personal_access_tokens?
           return user unless disable_personal_access_tokens
 
           raise ::Gitlab::Auth::UnauthorizedError
diff --git a/ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb b/ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb
index 60e225f0e1c997..194e9ceddba46d 100644
--- a/ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb
+++ b/ee/lib/ee/sidebars/user_settings/menus/access_tokens_menu.rb
@@ -9,7 +9,7 @@ module AccessTokensMenu
 
           override :render?
           def render?
-            return false if context.current_user&.user_detail&.enterprise_group&.disable_personal_access_tokens?
+            return false if context.current_user&.enterprise_group&.disable_personal_access_tokens?
 
             super
           end
diff --git a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
index 534f3749107545..34211d00230f09 100644
--- a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
+++ b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
@@ -155,7 +155,7 @@
         let(:user) { create(:enterprise_user) }
 
         before do
-          user.user_detail.enterprise_group.update!(disable_personal_access_tokens: true)
+          user.enterprise_group.update!(disable_personal_access_tokens: true)
         end
 
         it 'raised unauthorized error' do
@@ -184,7 +184,7 @@
           let(:user) { create(:enterprise_user) }
 
           before do
-            user.user_detail.enterprise_group.update!(disable_personal_access_tokens: true)
+            user.enterprise_group.update!(disable_personal_access_tokens: true)
           end
 
           it 'returns user' do
diff --git a/ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb b/ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb
index abeaa247dc5bcc..49568baf78b464 100644
--- a/ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb
+++ b/ee/spec/lib/ee/sidebars/user_settings/menus/access_tokens_menu_spec.rb
@@ -11,7 +11,7 @@
     let(:user) { build(:enterprise_user) }
 
     before do
-      allow(user.user_detail.enterprise_group).to receive(:disable_personal_access_tokens?).and_return(true)
+      allow(user.enterprise_group).to receive(:disable_personal_access_tokens?).and_return(true)
     end
 
     context 'when user is logged in' do
-- 
GitLab


From 74233fc7002cf7181e20aa2afb655c6321e1102f Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Thu, 11 Apr 2024 15:47:43 +0530
Subject: [PATCH 10/11] Handle loose primary key

---
 ee/app/models/ee/user.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index 28cf4f78541c73..e2f295dc593103 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -531,7 +531,8 @@ def enterprise_user_of_group?(group)
     end
 
     def enterprise_user?
-      enterprise_group_id.present?
+      # NOTE: Double check is added since enterprise_group_id is a lose foreign key and this is a high traffic method
+      enterprise_group_id.present? && enterprise_group.present?
     end
 
     def gitlab_employee?
-- 
GitLab


From c941f3b41e31df51a6b6ad88e82dc78aa8a3241e Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Thu, 11 Apr 2024 20:59:28 +0530
Subject: [PATCH 11/11] changes as per review

---
 ee/app/models/ee/group.rb                     |  5 ++-
 ee/app/models/ee/user.rb                      |  1 +
 .../lib/ee/gitlab/auth/auth_finders_spec.rb   |  1 +
 .../personal_access_tokens_controller_spec.rb | 37 +++++++++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 ee/spec/requests/ee/user_settings/personal_access_tokens_controller_spec.rb

diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index 89536cbb5e10bf..e341f8b3f01fc7 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -1009,7 +1009,10 @@ def code_suggestions_purchased?
 
     # Disable personal access tokens for enterprise users of this group
     def disable_personal_access_tokens?
-      ::Feature.enabled?(:enterprise_disable_personal_access_tokens, self) && root? && namespace_settings.disable_personal_access_tokens?
+      ::Feature.enabled?(:enterprise_disable_personal_access_tokens, self) &&
+        root? &&
+        licensed_feature_available?(:disable_personal_access_tokens) &&
+        namespace_settings.disable_personal_access_tokens?
     end
 
     private
diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index e2f295dc593103..ec5e78b8083206 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -532,6 +532,7 @@ def enterprise_user_of_group?(group)
 
     def enterprise_user?
       # NOTE: Double check is added since enterprise_group_id is a lose foreign key and this is a high traffic method
+      # This would make sure that we don't fire a query in most cases on gitlab.com, as we have more normal users than enterprise users.
       enterprise_group_id.present? && enterprise_group.present?
     end
 
diff --git a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
index 34211d00230f09..46928847cfc2ea 100644
--- a/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
+++ b/ee/spec/lib/ee/gitlab/auth/auth_finders_spec.rb
@@ -156,6 +156,7 @@
 
         before do
           user.enterprise_group.update!(disable_personal_access_tokens: true)
+          stub_licensed_features(disable_personal_access_tokens: true)
         end
 
         it 'raised unauthorized error' do
diff --git a/ee/spec/requests/ee/user_settings/personal_access_tokens_controller_spec.rb b/ee/spec/requests/ee/user_settings/personal_access_tokens_controller_spec.rb
new file mode 100644
index 00000000000000..2337dabc93d182
--- /dev/null
+++ b/ee/spec/requests/ee/user_settings/personal_access_tokens_controller_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe UserSettings::PersonalAccessTokensController, type: :request, feature_category: :shared do
+  let(:user) { create(:enterprise_user, :with_namespace) }
+
+  before do
+    sign_in(user)
+  end
+
+  subject(:make_request) { get '/-/user_settings/personal_access_tokens' }
+
+  context "when personal access tokens are disabled for enterprise users" do
+    before do
+      allow(user.enterprise_group).to receive(:disable_personal_access_tokens?).and_return(true)
+      stub_licensed_features(disable_personal_access_tokens: true)
+    end
+
+    it 'returns not found' do
+      make_request
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  context "when personal access tokens are not disabled for enterprise users" do
+    before do
+      allow(user.enterprise_group).to receive(:disable_personal_access_tokens?).and_return(false)
+      stub_licensed_features(disable_personal_access_tokens: true)
+    end
+
+    it 'render page' do
+      make_request
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+  end
+end
-- 
GitLab