Merge branch 'sfang-project-token-optional' into 'master'

Provide option to disable project access token creation [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57756

app/helpers/groups_helper.rb

@@ -220,6 +220,10 @@ module GroupsHelper
       !multiple_members?(group)
   end
 
+  def render_setting_to_allow_project_access_token_creation?(group)
+    group.root? && current_user.can?(:admin_setting_to_allow_project_access_token_creation, group)
+  end
+
   def show_thanks_for_purchase_banner?
     params.key?(:purchased_quantity) && params[:purchased_quantity].to_i > 0
   end

app/models/group.rb

@@ -119,7 +119,6 @@ class Group < Namespace
   end
 
   delegate :default_branch_name, to: :namespace_settings
-  delegate :resource_access_token_creation_allowed, :resource_access_token_creation_allowed=, :resource_access_token_creation_allowed?, to: :namespace_settings
 
   class << self
     def sort_by_attribute(method)

app/models/namespace_setting.rb

@@ -5,10 +5,11 @@ class NamespaceSetting < ApplicationRecord
 
   validate :default_branch_name_content
   validate :allow_mfa_for_group
+  validate :allow_resource_access_token_creation_for_group
 
   before_validation :normalize_default_branch_name
 
-  NAMESPACE_SETTINGS_PARAMS = [:default_branch_name, :delayed_project_removal].freeze
+  NAMESPACE_SETTINGS_PARAMS = [:default_branch_name, :delayed_project_removal, :resource_access_token_creation_allowed].freeze
 
   self.primary_key = :namespace_id
 
@@ -31,6 +32,12 @@ class NamespaceSetting < ApplicationRecord
       errors.add(:allow_mfa_for_subgroups, _('is not allowed since the group is not top-level group.'))
     end
   end
+
+  def allow_resource_access_token_creation_for_group
+    if namespace&.subgroup? && !resource_access_token_creation_allowed
+      errors.add(:resource_access_token_creation_allowed, _('is not allowed since the group is not top-level group.'))
+    end
+  end
 end
 
 NamespaceSetting.prepend_if_ee('EE::NamespaceSetting')

app/policies/group_policy.rb

@@ -217,6 +217,7 @@ class GroupPolicy < BasePolicy
   rule { can?(:admin_group) & resource_access_token_feature_available }.policy do
     enable :read_resource_access_tokens
     enable :destroy_resource_access_tokens
+    enable :admin_setting_to_allow_project_access_token_creation
   end
 
   rule { resource_access_token_creation_allowed & can?(:read_resource_access_tokens) }.policy do
@@ -253,7 +254,7 @@ class GroupPolicy < BasePolicy
   end
 
   def resource_access_token_creation_allowed?
-    group.resource_access_token_creation_allowed?
+    resource_access_token_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
   end
 end
 

app/policies/project_policy.rb

@@ -737,7 +737,7 @@ class ProjectPolicy < BasePolicy
 
     return true unless group # always enable for projects in personal namespaces
 
-    group.resource_access_token_creation_allowed
+    resource_access_token_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
   end
 
   def project

app/services/namespace_settings/update_service.rb

@@ -13,12 +13,25 @@ module NamespaceSettings
     end
 
     def execute
+      validate_resource_access_token_creation_allowed_param
+
       if group.namespace_settings
         group.namespace_settings.attributes = settings_params
       else
         group.build_namespace_settings(settings_params)
       end
     end
+
+    private
+
+    def validate_resource_access_token_creation_allowed_param
+      return if settings_params[:resource_access_token_creation_allowed].nil?
+
+      unless can?(current_user, :admin_group, group)
+        settings_params.delete(:resource_access_token_creation_allowed)
+        group.namespace_settings.errors.add(:resource_access_token_creation_allowed, _('can only be changed by a group admin.'))
+      end
+    end
   end
 end
 

app/views/groups/settings/_permissions.html.haml

@@ -30,6 +30,7 @@
           %span.d-block= s_('GroupSettings|Disable group mentions')
           %span.text-muted= s_('GroupSettings|This setting will prevent group members from being notified if the group is mentioned.')
 
+    = render 'groups/settings/project_access_token_creation', f: f, group: @group
     = render_if_exists 'groups/settings/delayed_project_removal', f: f, group: @group
     = render_if_exists 'groups/settings/ip_restriction', f: f, group: @group
     = render_if_exists 'groups/settings/allowed_email_domain', f: f, group: @group

app/views/groups/settings/_project_access_token_creation.html.haml

+- return unless render_setting_to_allow_project_access_token_creation?(group)
+
+.form-group.gl-mb-3
+  .form-check
+    = f.check_box :resource_access_token_creation_allowed, checked: group.namespace_settings.resource_access_token_creation_allowed?, class: 'form-check-input', data: { qa_selector: 'resource_access_token_creation_allowed_checkbox' }
+    = f.label :resource_access_token_creation_allowed, class: 'form-check-label' do
+      %span.gl-display-block= s_('GroupSettings|Allow project access token creation')
+      - project_access_tokens_link = help_page_path('user/project/settings/project_access_tokens')
+      - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: project_access_tokens_link }
+      %span.text-muted= s_('GroupSettings|Users can create %{link_start}project access tokens%{link_end} for projects in this group.').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }

app/views/projects/settings/access_tokens/index.html.haml

@@ -9,10 +9,19 @@
     %h4.gl-mt-0
       = page_title
     %p
+    - if current_user.can?(:create_resource_access_tokens, @project)
       = _('You can generate an access token scoped to this project for each application to use the GitLab API.')
-    -# Commented out until https://gitlab.com/gitlab-org/gitlab/-/issues/219551 is fixed
-    -# %p
-    -#   = _('You can also use project access tokens to authenticate against Git over HTTP.')
+      -# Commented out until https://gitlab.com/gitlab-org/gitlab/-/issues/219551 is fixed
+      -# %p
+      -#   = _('You can also use project access tokens to authenticate against Git over HTTP.')
+    - else
+      = _('Project access token creation is disabled in this group. You can still use and manage existing tokens.')
+      %p
+      - if current_user.can?(:admin_group, @project.group)
+        - group_settings_link = edit_group_path(@project.group)
+        - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: group_settings_link }
+        = _('You can enable project access token creation in %{link_start}group settings%{link_end}.').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
+
 
   .col-lg-8
     - if @new_project_access_token

config/feature_flags/development/resource_access_token_feature.yml

----
-name: resource_access_token_feature
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29622
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/235765
-milestone: '13.0'
-type: development
-group: group::access
-default_enabled: true

doc/user/project/settings/project_access_tokens.md

@@ -78,3 +78,9 @@ the following table.
 | `write_registry`   | Allows write-access (push) to [container registry](../../packages/container_registry/index.md). |
 | `read_repository`  | Allows read-only access (pull) to the repository. |
 | `write_repository` | Allows read-write access (pull, push) to the repository. |
+
+## Enable or disable project access token creation
+
+You may enable or disable project access token creation for all projects in a group in **Group > Settings > General > Permissions, LFS, 2FA > Allow project access token creation**.
+Even when creation is disabled, you can still use and revoke existing project access tokens.
+This setting is only available on root-level groups.

ee/app/policies/ee/group_policy.rb

@@ -393,11 +393,9 @@ module EE
     # Available in Core for self-managed but only paid, non-trial for .com to prevent abuse
     override :resource_access_token_feature_available?
     def resource_access_token_feature_available?
-      value_from_super = super
+      return super unless ::Gitlab.com?
 
-      return value_from_super unless ::Gitlab.com?
-
-      value_from_super && group.feature_available_non_trial?(:resource_access_token)
+      group.feature_available_non_trial?(:resource_access_token)
     end
   end
 end

ee/app/policies/ee/project_policy.rb

@@ -425,14 +425,11 @@ module EE
     # Available in Core for self-managed but only paid, non-trial for .com to prevent abuse
     override :resource_access_token_feature_available?
     def resource_access_token_feature_available?
-      value_from_super = super
+      return super unless ::Gitlab.com?
 
-      return value_from_super unless ::Gitlab.com?
+      namespace = project.namespace
 
-      group = project.namespace
-
-      ::Feature.enabled?(:resource_access_token_feature, group, default_enabled: true) &&
-        group.feature_available_non_trial?(:resource_access_token)
+      namespace.feature_available_non_trial?(:resource_access_token)
     end
   end
 end

ee/changelogs/unreleased/sfang-project-token-optional.yml

+---
+title: Provide option to disable project access token creation
+merge_request: 57756
+author:
+type: added

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -109,6 +109,58 @@ RSpec.describe GroupsHelper do
     end
   end
 
+  describe '#render_setting_to_allow_project_access_token_creation?' do
+    context 'with self-managed' do
+      let_it_be(:parent) { create(:group) }
+      let_it_be(:group) { create(:group, parent: parent) }
+
+      before do
+        parent.add_owner(owner)
+        group.add_owner(owner)
+      end
+
+      it 'returns true if group is root' do
+        expect(helper.render_setting_to_allow_project_access_token_creation?(parent)).to be_truthy
+      end
+
+      it 'returns false if group is subgroup' do
+        expect(helper.render_setting_to_allow_project_access_token_creation?(group)).to be_falsey
+      end
+    end
+
+    context 'on .com' do
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+        stub_ee_application_setting(should_check_namespace_plan: true)
+      end
+
+      context 'with a free plan' do
+        let_it_be(:group) { create(:group) }
+
+        it 'returns false' do
+          expect(helper.render_setting_to_allow_project_access_token_creation?(group)).to be_falsey
+        end
+      end
+
+      context 'with a paid plan' do
+        let_it_be(:parent) { create(:group_with_plan, plan: :bronze_plan) }
+        let_it_be(:group) { create(:group, parent: parent) }
+
+        before do
+          parent.add_owner(owner)
+        end
+
+        it 'returns true if group is root' do
+          expect(helper.render_setting_to_allow_project_access_token_creation?(parent)).to be_truthy
+        end
+
+        it 'returns false if group is subgroup' do
+          expect(helper.render_setting_to_allow_project_access_token_creation?(group)).to be_falsey
+        end
+      end
+    end
+  end
+
   describe '#permanent_deletion_date' do
     let(:date) { 2.days.from_now }
 

ee/spec/policies/group_policy_spec.rb

@@ -1395,6 +1395,19 @@ RSpec.describe GroupPolicy do
 
             it { is_expected.not_to be_allowed(:create_resource_access_tokens) }
           end
+
+          context 'when parent group has resource access token creation disabled' do
+            let(:parent) { create(:group_with_plan, plan: :bronze_plan) }
+            let(:group) { create(:group, parent: parent) }
+
+            before do
+              parent.namespace_settings.update_column(:resource_access_token_creation_allowed, false)
+            end
+
+            context 'cannot create resource access tokens' do
+              it { is_expected.not_to be_allowed(:create_resource_access_tokens) }
+            end
+          end
         end
 
         context 'read resource access tokens' do

ee/spec/policies/project_policy_spec.rb

@@ -1587,16 +1587,34 @@ RSpec.describe ProjectPolicy do
         context 'create resource access tokens' do
           it { is_expected.to be_allowed(:create_resource_access_tokens) }
 
-          context 'when resource access token creation is not allowed' do
-            let(:group) { create(:group) }
-            let(:project) { create(:project, group: group) }
+          context 'with a personal namespace project' do
+            let(:namespace) { create(:namespace_with_plan, plan: :bronze_plan) }
+            let(:project) { create(:project, namespace: namespace) }
+
+            it { is_expected.to be_allowed(:create_resource_access_tokens) }
+          end
 
+          context 'when resource access token creation is not allowed' do
             before do
               group.namespace_settings.update_column(:resource_access_token_creation_allowed, false)
             end
 
             it { is_expected.not_to be_allowed(:create_resource_access_tokens) }
           end
+
+          context 'when parent group has resource access token creation disabled' do
+            let(:parent) { create(:group_with_plan, plan: :bronze_plan) }
+            let(:group) { create(:group, parent: parent) }
+            let(:project) { create(:project, group: group) }
+
+            before do
+              parent.namespace_settings.update_column(:resource_access_token_creation_allowed, false)
+            end
+
+            context 'cannot create resource access tokens' do
+              it { is_expected.not_to be_allowed(:create_resource_access_tokens) }
+            end
+          end
         end
 
         context 'read resource access tokens' do

locale/gitlab.pot

@@ -15137,6 +15137,9 @@ msgstr ""
 msgid "GroupSelect|Select a group"
 msgstr ""
 
+msgid "GroupSettings|Allow project access token creation"
+msgstr ""
+
 msgid "GroupSettings|Apply integration settings to all Projects"
 msgstr ""
 
@@ -15251,6 +15254,9 @@ msgstr ""
 msgid "GroupSettings|Transfer group"
 msgstr ""
 
+msgid "GroupSettings|Users can create %{link_start}project access tokens%{link_end} for projects in this group."
+msgstr ""
+
 msgid "GroupSettings|What are badges?"
 msgstr ""
 
@@ -23957,6 +23963,9 @@ msgstr ""
 msgid "Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group."
 msgstr ""
 
+msgid "Project access token creation is disabled in this group. You can still use and manage existing tokens."
+msgstr ""
+
 msgid "Project already deleted"
 msgstr ""
 
@@ -34977,6 +34986,9 @@ msgstr ""
 msgid "You can easily contribute to them by requesting to join these groups."
 msgstr ""
 
+msgid "You can enable project access token creation in %{link_start}group settings%{link_end}."
+msgstr ""
+
 msgid "You can filter by 'days to merge' by clicking on the columns in the chart."
 msgstr ""
 
@@ -35792,6 +35804,9 @@ msgstr ""
 msgid "can contain only letters of the Base64 alphabet (RFC4648) with the addition of '@', ':' and '.'"
 msgstr ""
 
+msgid "can only be changed by a group admin."
+msgstr ""
+
 msgid "can't be enabled because signed commits are required for this project"
 msgstr ""
 

spec/controllers/groups_controller_spec.rb

@@ -614,6 +614,43 @@ RSpec.describe GroupsController, factory_default: :keep do
     end
   end
 
+  context "updating :resource_access_token_creation_allowed" do
+    subject do
+      put :update,
+        params: {
+          id: group.to_param,
+          group: { resource_access_token_creation_allowed: false }
+        }
+    end
+
+    context 'when user is a group owner' do
+      before do
+        group.add_owner(user)
+        sign_in(user)
+      end
+
+      it "updates the attribute" do
+        expect { subject }
+            .to change { group.namespace_settings.reload.resource_access_token_creation_allowed }
+            .from(true)
+            .to(false)
+
+        expect(response).to have_gitlab_http_status(:found)
+      end
+    end
+
+    context 'when not a group owner' do
+      before do
+        group.add_developer(user)
+        sign_in(user)
+      end
+
+      it "does not update the attribute" do
+        expect { subject }.not_to change { group.namespace_settings.reload.resource_access_token_creation_allowed }
+      end
+    end
+  end
+
   describe '#ensure_canonical_path' do
     before do
       sign_in(user)

spec/controllers/projects/settings/access_tokens_controller_spec.rb

@@ -4,7 +4,8 @@ require('spec_helper')
 
 RSpec.describe Projects::Settings::AccessTokensController do
   let_it_be(:user) { create(:user) }
-  let_it_be(:project) { create(:project) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
   let_it_be(:bot_user) { create(:user, :project_bot) }
 
   before_all do
@@ -40,6 +41,26 @@ RSpec.describe Projects::Settings::AccessTokensController do
 
     it_behaves_like 'feature unavailable'
     it_behaves_like 'project access tokens available #create'
+
+    context 'when project access token creation is disabled' do
+      before do
+        group.namespace_settings.update_column(:resource_access_token_creation_allowed, false)
+      end
+
+      it { is_expected.to have_gitlab_http_status(:not_found) }
+
+      it 'does not create the token' do
+        expect { subject }.not_to change { PersonalAccessToken.count }
+      end
+
+      it 'does not add the project bot as a member' do
+        expect { subject }.not_to change { Member.count }
+      end
+
+      it 'does not create the project bot user' do
+        expect { subject }.not_to change { User.count }
+      end
+    end
   end
 
   describe '#revoke', :sidekiq_inline do

spec/features/projects/settings/access_tokens_spec.rb

@@ -34,6 +34,18 @@ RSpec.describe 'Project > Settings > Access Tokens', :js do
     find('#created-personal-access-token').value
   end
 
+  context 'when user is not a project maintainer' do
+    before do
+      project.add_developer(user)
+    end
+
+    it 'does not show project access token page' do
+      visit project_settings_access_tokens_path(project)
+
+      expect(page).to have_content("Page Not Found")
+    end
+  end
+
   describe 'token creation' do
     it 'allows creation of a project access token' do
       name = 'My project access token'
@@ -67,7 +79,54 @@ RSpec.describe 'Project > Settings > Access Tokens', :js do
       it 'does not show project access token creation form' do
         visit project_settings_access_tokens_path(project)
 
-        expect(page).not_to have_selector('.new_project_access_token')
+        expect(page).not_to have_selector('#new_project_access_token')
+      end
+
+      it 'shows project access token creation disabled text' do
+        visit project_settings_access_tokens_path(project)
+
+        expect(page).to have_text('Project access token creation is disabled in this group. You can still use and manage existing tokens.')
+      end
+
+      context 'with a project in a personal namespace' do
+        let(:personal_project) { create(:project) }
+
+        before do
+          personal_project.add_maintainer(user)
+        end
+
+        it 'shows project access token creation form and text' do
+          visit project_settings_access_tokens_path(personal_project)
+
+          expect(page).to have_selector('#new_project_access_token')
+          expect(page).to have_text('You can generate an access token scoped to this project for each application to use the GitLab API.')
+        end
+      end
+
+      context 'group settings link' do
+        context 'when user is not a group owner' do
+          before do
+            group.add_developer(user)
+          end
+
+          it 'does not show group settings link' do
+            visit project_settings_access_tokens_path(project)
+
+            expect(page).not_to have_link('group settings', href: edit_group_path(group))
+          end
+        end
+
+        context 'when user is a group owner' do
+          before do
+            group.add_owner(user)
+          end
+
+          it 'shows group settings link' do
+            visit project_settings_access_tokens_path(project)
+
+            expect(page).to have_link('group settings', href: edit_group_path(group))
+          end
+        end
       end
     end
   end
@@ -102,5 +161,19 @@ RSpec.describe 'Project > Settings > Access Tokens', :js do
       expect(page).to have_selector('.settings-message')
       expect(no_project_access_tokens_message).to have_text(no_active_tokens_text)
     end
+
+    context 'when resource access token creation is not allowed' do
+      before do
+        group.namespace_settings.update_column(:resource_access_token_creation_allowed, false)
+      end
+
+      it 'allows revocation of an active token' do
+        visit project_settings_access_tokens_path(project)
+        accept_confirm { click_on 'Revoke' }
+
+        expect(page).to have_selector('.settings-message')
+        expect(no_project_access_tokens_message).to have_text(no_active_tokens_text)
+      end
+    end
   end
 end

spec/helpers/groups_helper_spec.rb

@@ -461,6 +461,26 @@ RSpec.describe GroupsHelper do
     end
   end
 
+  describe '#render_setting_to_allow_project_access_token_creation?' do
+    let_it_be(:current_user) { create(:user) }
+    let_it_be(:parent) { create(:group) }
+    let_it_be(:group) { create(:group, parent: parent) }
+
+    before do
+      allow(helper).to receive(:current_user) { current_user }
+      parent.add_owner(current_user)
+      group.add_owner(current_user)
+    end
+
+    it 'returns true if group is root' do
+      expect(helper.render_setting_to_allow_project_access_token_creation?(parent)).to be_truthy
+    end
+
+    it 'returns false if group is subgroup' do
+      expect(helper.render_setting_to_allow_project_access_token_creation?(group)).to be_falsy
+    end
+  end
+
   describe '#group_open_issues_count' do
     let_it_be(:current_user) { create(:user) }
     let_it_be(:group) { create(:group, :public) }

spec/models/namespace_setting_spec.rb

@@ -66,5 +66,36 @@ RSpec.describe NamespaceSetting, type: :model do
         end
       end
     end
+
+    describe '#allow_resource_access_token_creation_for_group' do
+      let(:settings) { group.namespace_settings }
+
+      context 'group is top-level group' do
+        let(:group) { create(:group) }
+
+        it 'is valid' do
+          settings.resource_access_token_creation_allowed = false
+
+          expect(settings).to be_valid
+        end
+      end
+
+      context 'group is a subgroup' do
+        let(:group) { create(:group, parent: create(:group)) }
+
+        it 'is invalid when resource access token creation is not enabled' do
+          settings.resource_access_token_creation_allowed = false
+
+          expect(settings).to be_invalid
+          expect(group.namespace_settings.errors.messages[:resource_access_token_creation_allowed]).to include("is not allowed since the group is not top-level group.")
+        end
+
+        it 'is valid when resource access tokens are enabled' do
+          settings.resource_access_token_creation_allowed = true
+
+          expect(settings).to be_valid
+        end
+      end
+    end
   end
 end

spec/services/namespace_settings/update_service_spec.rb

@@ -44,5 +44,36 @@ RSpec.describe NamespaceSettings::UpdateService do
           .from(nil).to(example_branch_name)
       end
     end
+
+    context "updating :resource_access_token_creation_allowed" do
+      let(:settings) { { resource_access_token_creation_allowed: false } }
+
+      context 'when user is a group owner' do
+        before do
+          group.add_owner(user)
+        end
+
+        it "changes settings" do
+          expect { service.execute }
+            .to change { group.namespace_settings.resource_access_token_creation_allowed }
+            .from(true).to(false)
+        end
+      end
+
+      context 'when user is not a group owner' do
+        before do
+          group.add_developer(user)
+        end
+
+        it "does not change settings" do
+          expect { service.execute }.not_to change { group.namespace_settings.resource_access_token_creation_allowed }
+        end
+
+        it 'returns the group owner error' do
+          service.execute
+          expect(group.namespace_settings.errors.messages[:resource_access_token_creation_allowed]).to include('can only be changed by a group admin.')
+        end
+      end
+    end
   end
 end

spec/support/shared_examples/policies/resource_access_token_shared_examples.rb

@@ -21,6 +21,29 @@ RSpec.shared_examples 'Self-managed Core resource access tokens' do
 
         it { is_expected.not_to be_allowed(:create_resource_access_tokens) }
       end
+
+      context 'when parent group has project access token creation disabled' do
+        let(:parent) { create(:group) }
+        let(:group) { create(:group, parent: parent) }
+        let(:project) { create(:project, group: group) }
+
+        before do
+          parent.namespace_settings.update_column(:resource_access_token_creation_allowed, false)
+        end
+
+        it { is_expected.not_to be_allowed(:create_resource_access_tokens) }
+      end
+
+      context 'with a personal namespace project' do
+        let(:namespace) { create(:namespace) }
+        let(:project) { create(:project, namespace: namespace) }
+
+        before do
+          project.add_maintainer(current_user)
+        end
+
+        it { is_expected.to be_allowed(:create_resource_access_tokens) }
+      end
     end
 
     context 'read resource access tokens' do