Merge branch 'dblessing-limit-project-tokens' into 'master'

Limit resource access tokens to paid GitLab.com plans

See merge request gitlab-org/gitlab!43199

app/helpers/projects_helper.rb

@@ -770,7 +770,7 @@ module ProjectsHelper
   def project_access_token_available?(project)
     return false if ::Gitlab.com?
 
-    ::Feature.enabled?(:resource_access_token, project, default_enabled: true)
+    can?(current_user, :admin_resource_access_tokens, project)
   end
 end
 

app/policies/group_policy.rb

@@ -56,6 +56,9 @@ class GroupPolicy < BasePolicy
     @user.is_a?(DeployToken) && @user.groups.include?(@subject) && @user.write_package_registry
   end
 
+  with_scope :subject
+  condition(:resource_access_token_available) { resource_access_token_available? }
+
   rule { design_management_enabled }.policy do
     enable :read_design_activity
   end
@@ -187,6 +190,10 @@ class GroupPolicy < BasePolicy
     enable :read_group
   end
 
+  rule { resource_access_token_available & can?(:admin_group) }.policy do
+    enable :admin_resource_access_tokens
+  end
+
   def access_level
     return GroupMember::NO_ACCESS if @user.nil?
     return GroupMember::NO_ACCESS unless user_is_user?
@@ -203,6 +210,14 @@ class GroupPolicy < BasePolicy
   def user_is_user?
     user.is_a?(User)
   end
+
+  def group
+    @subject
+  end
+
+  def resource_access_token_available?
+    true
+  end
 end
 
 GroupPolicy.prepend_if_ee('EE::GroupPolicy')

app/policies/project_policy.rb

@@ -104,6 +104,9 @@ class ProjectPolicy < BasePolicy
   with_scope :subject
   condition(:service_desk_enabled) { @subject.service_desk_enabled? }
 
+  with_scope :subject
+  condition(:resource_access_token_available) { resource_access_token_available? }
+
   # We aren't checking `:read_issue` or `:read_merge_request` in this case
   # because it could be possible for a user to see an issuable-iid
   # (`:read_issue_iid` or `:read_merge_request_iid`) but then wouldn't be
@@ -589,6 +592,10 @@ class ProjectPolicy < BasePolicy
     prevent :read_project
   end
 
+  rule { resource_access_token_available & can?(:admin_project) }.policy do
+    enable :admin_resource_access_tokens
+  end
+
   private
 
   def user_is_user?
@@ -663,6 +670,10 @@ class ProjectPolicy < BasePolicy
     end
   end
 
+  def resource_access_token_available?
+    true
+  end
+
   def project
     @subject
   end

app/services/resource_access_tokens/create_service.rb

@@ -32,20 +32,11 @@ module ResourceAccessTokens
     attr_reader :resource_type, :resource
 
     def feature_enabled?
-      return false if ::Gitlab.com?
-
-      ::Feature.enabled?(:resource_access_token, resource, default_enabled: true)
+      return true unless ::Gitlab.com?
     end
 
     def has_permission_to_create?
-      case resource_type
-      when 'project'
-        can?(current_user, :admin_project, resource)
-      when 'group'
-        can?(current_user, :admin_group, resource)
-      else
-        false
-      end
+      %w(project group).include?(resource_type) && can?(current_user, :admin_resource_access_tokens, resource)
     end
 
     def create_user

config/feature_flags/development/resource_access_token.yml → config/feature_flags/licensed/resource_access_token.yml

@@ -3,5 +3,5 @@ name: resource_access_token
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29622
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/235765
 group: group::access
-type: development
+type: licensed
 default_enabled: true

ee/app/models/ee/namespace.rb

@@ -156,6 +156,10 @@ module EE
       available_features[feature]
     end
 
+    def feature_available_non_trial?(feature)
+      feature_available?(feature.to_sym) && !trial_active?
+    end
+
     override :actual_plan
     def actual_plan
       strong_memoize(:actual_plan) do

ee/app/models/license.rb

@@ -35,6 +35,7 @@ class License < ApplicationRecord
     push_rules
     repository_mirrors
     repository_size_limit
+    resource_access_token
     seat_link
     send_emails_from_admin_area
     scoped_issue_board

ee/app/policies/ee/group_policy.rb

@@ -319,5 +319,13 @@ module EE
 
       ::Gitlab::Auth::GroupSaml::SsoEnforcer.group_access_restricted?(subject)
     end
+
+    # Available in Core for self-managed but only paid, non-trial for .com to prevent abuse
+    override :resource_access_token_available?
+    def resource_access_token_available?
+      return true unless ::Gitlab.com?
+
+      group.feature_available_non_trial?(:resource_access_token)
+    end
   end
 end

ee/app/policies/ee/project_policy.rb

@@ -350,5 +350,13 @@ module EE
 
       super
     end
+
+    # Available in Core for self-managed but only paid, non-trial for .com to prevent abuse
+    override :resource_access_token_available?
+    def resource_access_token_available?
+      return true unless ::Gitlab.com?
+
+      project.namespace.feature_available_non_trial?(:resource_access_token)
+    end
   end
 end

ee/changelogs/unreleased/dblessing-limit-project-tokens.yml

+---
+title: Limit Project Access Tokens/Bots to paid groups in Gitlab.com
+merge_request: 43199
+author:
+type: changed

ee/spec/controllers/projects/settings/access_tokens_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::Settings::AccessTokensController do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+
+  before_all do
+    project.add_maintainer(user)
+  end
+
+  before do
+    sign_in(user)
+  end
+
+  shared_examples 'feature unavailable' do
+    context 'on GitLab.com' do
+      before do
+        allow(Gitlab).to receive(:com?).and_return(true)
+      end
+
+      context 'with a free plan' do
+        it { is_expected.to have_gitlab_http_status(:not_found) }
+      end
+
+      context 'with a paid group plan' do
+        let_it_be(:group) { create(:group_with_plan, plan: :bronze_plan) }
+        let_it_be(:project) { create(:project, group: group) }
+
+        before do
+          project.add_developer(user)
+        end
+
+        it { is_expected.to have_gitlab_http_status(:not_found) }
+      end
+    end
+  end
+
+  describe '#index' do
+    subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
+
+    it_behaves_like 'feature unavailable'
+  end
+
+  describe '#create', :clean_gitlab_redis_shared_state do
+    subject { post :create, params: { namespace_id: project.namespace, project_id: project }.merge(project_access_token: access_token_params) }
+
+    let_it_be(:access_token_params) { {} }
+
+    it_behaves_like 'feature unavailable'
+  end
+
+  describe '#revoke' do
+    let_it_be(:bot_user) { create(:user, :project_bot) }
+    let_it_be(:project_access_token) { create(:personal_access_token, user: bot_user) }
+
+    subject { put :revoke, params: { namespace_id: project.namespace, project_id: project, id: project_access_token } }
+
+    before_all do
+      project.add_maintainer(bot_user)
+    end
+
+    it_behaves_like 'feature unavailable'
+  end
+end

ee/spec/models/ee/namespace_spec.rb

@@ -305,14 +305,12 @@ RSpec.describe Namespace do
     end
   end
 
-  describe '#feature_available?' do
+  shared_examples 'feature available' do
     let(:hosted_plan) { create(:bronze_plan) }
     let(:group) { create(:group) }
     let(:licensed_feature) { :epics }
     let(:feature) { licensed_feature }
 
-    subject { group.feature_available?(feature) }
-
     before do
       create(:gitlab_subscription, namespace: group, hosted_plan: hosted_plan)
 
@@ -385,6 +383,32 @@ RSpec.describe Namespace do
     end
   end
 
+  describe '#feature_available?' do
+    subject { group.feature_available?(feature) }
+
+    it_behaves_like 'feature available'
+  end
+
+  describe '#feature_available_non_trial?' do
+    subject { group.feature_available_non_trial?(feature) }
+
+    it_behaves_like 'feature available'
+
+    context 'when the group has an active trial' do
+      let(:hosted_plan) { create(:bronze_plan) }
+      let(:group) { create(:group) }
+      let(:feature) { :resource_access_token }
+
+      before do
+        create(:gitlab_subscription, :active_trial, namespace: group, hosted_plan: hosted_plan)
+        stub_licensed_features(feature => true)
+        stub_ee_application_setting(should_check_namespace_plan: true)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#actual_limits' do
     subject { namespace.actual_limits }
 

ee/spec/policies/group_policy_spec.rb

@@ -1111,4 +1111,36 @@ RSpec.describe GroupPolicy do
   it_behaves_like 'update namespace limit policy'
 
   include_examples 'analytics report embedding'
+
+  context 'group access tokens' do
+    it_behaves_like 'GitLab.com Core resource access tokens'
+
+    context 'on GitLab.com paid' do
+      let_it_be(:group) { create(:group_with_plan, plan: :bronze_plan) }
+
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+      end
+
+      context 'with owner' do
+        let(:current_user) { owner }
+
+        before do
+          group.add_owner(owner)
+        end
+
+        it { is_expected.to be_allowed(:admin_resource_access_tokens) }
+      end
+
+      context 'with developer' do
+        let(:current_user) { developer }
+
+        before do
+          group.add_developer(developer)
+        end
+
+        it { is_expected.not_to be_allowed(:admin_resource_access_tokens)}
+      end
+    end
+  end
 end

ee/spec/policies/project_policy_spec.rb

@@ -1378,4 +1378,37 @@ RSpec.describe ProjectPolicy do
   end
 
   include_examples 'analytics report embedding'
+
+  context 'project access tokens' do
+    it_behaves_like 'GitLab.com Core resource access tokens'
+
+    context 'on GitLab.com paid' do
+      let_it_be(:group) { create(:group_with_plan, plan: :bronze_plan) }
+      let_it_be(:project) { create(:project, group: group) }
+
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+      end
+
+      context 'with maintainer' do
+        let(:current_user) { maintainer }
+
+        before do
+          project.add_maintainer(maintainer)
+        end
+
+        it { is_expected.to be_allowed(:admin_resource_access_tokens) }
+      end
+
+      context 'with developer' do
+        let(:current_user) { developer }
+
+        before do
+          project.add_developer(developer)
+        end
+
+        it { is_expected.not_to be_allowed(:admin_resource_access_tokens)}
+      end
+    end
+  end
 end

spec/controllers/projects/settings/access_tokens_controller_spec.rb

@@ -14,28 +14,21 @@ RSpec.describe Projects::Settings::AccessTokensController do
     sign_in(user)
   end
 
-  shared_examples 'feature unavailability' do
-    context 'when flag is disabled' do
-      before do
-        stub_feature_flags(resource_access_token: false)
-      end
+  shared_examples 'feature unavailable' do
+    let_it_be(:project) { create(:project) }
 
-      it { is_expected.to have_gitlab_http_status(:not_found) }
+    before do
+      allow(Gitlab).to receive(:com?).and_return(false)
+      project.add_developer(user)
     end
 
-    context 'when environment is Gitlab.com' do
-      before do
-        allow(Gitlab).to receive(:com?).and_return(true)
-      end
-
-      it { is_expected.to have_gitlab_http_status(:not_found) }
-    end
+    it { is_expected.to have_gitlab_http_status(:not_found) }
   end
 
   describe '#index' do
     subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
 
-    it_behaves_like 'feature unavailability'
+    it_behaves_like 'feature unavailable'
 
     context 'when feature is available' do
       let_it_be(:bot_user) { create(:user, :project_bot) }
@@ -84,7 +77,7 @@ RSpec.describe Projects::Settings::AccessTokensController do
 
     let_it_be(:access_token_params) { {} }
 
-    it_behaves_like 'feature unavailability'
+    it_behaves_like 'feature unavailable'
 
     context 'when feature is available' do
       let_it_be(:access_token_params) { { name: 'Nerd bot', scopes: ["api"], expires_at: 1.month.since.to_date } }
@@ -148,7 +141,7 @@ RSpec.describe Projects::Settings::AccessTokensController do
       project.add_maintainer(bot_user)
     end
 
-    it_behaves_like 'feature unavailability'
+    it_behaves_like 'feature unavailable'
 
     context 'when feature is available' do
       before do
@@ -185,6 +178,5 @@ RSpec.describe Projects::Settings::AccessTokensController do
 
   def enable_feature
     allow(Gitlab).to receive(:com?).and_return(false)
-    stub_feature_flags(resource_access_token: true)
   end
 end

spec/policies/group_policy_spec.rb

@@ -880,4 +880,6 @@ RSpec.describe GroupPolicy do
       it { is_expected.to be_disallowed(:destroy_package) }
     end
   end
+
+  it_behaves_like 'Self-managed Core resource access tokens'
 end

spec/policies/project_policy_spec.rb

@@ -941,4 +941,6 @@ RSpec.describe ProjectPolicy do
       end
     end
   end
+
+  it_behaves_like 'Self-managed Core resource access tokens'
 end

spec/services/resource_access_tokens/create_service_spec.rb

@@ -24,16 +24,6 @@ RSpec.describe ResourceAccessTokens::CreateService do
       end
     end
 
-    shared_examples 'fails when flag is disabled' do
-      before do
-        stub_feature_flags(resource_access_token: false)
-      end
-
-      it 'returns nil' do
-        expect(subject).to be nil
-      end
-    end
-
     shared_examples 'fails on gitlab.com' do
       before do
         allow(Gitlab).to receive(:com?) { true }
@@ -181,7 +171,6 @@ RSpec.describe ResourceAccessTokens::CreateService do
       let_it_be(:resource) { project }
 
       it_behaves_like 'fails when user does not have the permission to create a Resource Bot'
-      it_behaves_like 'fails when flag is disabled'
       it_behaves_like 'fails on gitlab.com'
 
       context 'user with valid permission' do

spec/support/shared_examples/policies/resource_access_token_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'Self-managed Core resource access tokens' do
+  before do
+    allow(::Gitlab).to receive(:com?).and_return(false)
+  end
+
+  context 'with owner' do
+    let(:current_user) { owner }
+
+    it { is_expected.to be_allowed(:admin_resource_access_tokens) }
+  end
+
+  context 'with developer' do
+    let(:current_user) { developer }
+
+    it { is_expected.not_to be_allowed(:admin_resource_access_tokens) }
+  end
+end
+
+RSpec.shared_examples 'GitLab.com Core resource access tokens' do
+  before do
+    allow(::Gitlab).to receive(:com?).and_return(true)
+    stub_ee_application_setting(should_check_namespace_plan: true)
+  end
+
+  context 'with owner' do
+    let(:current_user) { owner }
+
+    it { is_expected.not_to be_allowed(:admin_resource_access_tokens) }
+  end
+end