diff --git a/.rubocop_todo/gitlab/feature_flag_without_actor.yml b/.rubocop_todo/gitlab/feature_flag_without_actor.yml index 51b554e67becc13e63d7f24d66b1f8dfdc0be0ce..e023eddee5381d7c3fb63bc5461bd9a23d1514f3 100644 --- a/.rubocop_todo/gitlab/feature_flag_without_actor.yml +++ b/.rubocop_todo/gitlab/feature_flag_without_actor.yml @@ -77,7 +77,6 @@ Gitlab/FeatureFlagWithoutActor: - 'ee/app/views/projects/on_demand_scans/index.html.haml' - 'ee/app/views/projects/settings/merge_requests/_merge_trains_settings.html.haml' - 'ee/lib/api/code_suggestions.rb' - - 'ee/lib/api/internal/suggested_reviewers.rb' - 'ee/lib/ee/api/entities/application_setting.rb' - 'ee/lib/ee/api/geo.rb' - 'ee/lib/ee/api/internal/base.rb' diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml index fcaad442bc7dbbf277c15c1cc1b9e899b8f86399..7e30bdaf3278294408ef77b50e39e4f336ab8a22 100644 --- a/.rubocop_todo/rspec/named_subject.yml +++ b/.rubocop_todo/rspec/named_subject.yml @@ -649,7 +649,6 @@ RSpec/NamedSubject: - 'ee/spec/requests/api/internal/app_sec/dast/site_validations_spec.rb' - 'ee/spec/requests/api/internal/base_spec.rb' - 'ee/spec/requests/api/internal/kubernetes_spec.rb' - - 'ee/spec/requests/api/internal/suggested_reviewers_spec.rb' - 'ee/spec/requests/api/issues_spec.rb' - 'ee/spec/requests/api/members_spec.rb' - 'ee/spec/requests/api/merge_trains_spec.rb' diff --git a/app/assets/javascripts/sidebar/components/assignees/constants.js b/app/assets/javascripts/sidebar/components/assignees/constants.js index 01267dab435d016ead31fa860446ff1fb040a58e..7914ba31e36715a2f17b1195ed12d59aa4cc2a05 100644 --- a/app/assets/javascripts/sidebar/components/assignees/constants.js +++ b/app/assets/javascripts/sidebar/components/assignees/constants.js @@ -13,7 +13,6 @@ export const userTypes = { automation_bot: 'AUTOMATION_BOT', security_policy_bot: 'SECURITY_POLICY_BOT', admin_bot: 'ADMIN_BOT', - suggested_reviewers_bot: 'SUGGESTED_REVIEWERS_BOT', service_account: 'SERVICE_ACCOUNT', llm_bot: 'LLM_BOT', placeholder: 'PLACEHOLDER', diff --git a/app/models/concerns/has_user_type.rb b/app/models/concerns/has_user_type.rb index e001997c62df3398a85b830fdb909e5ff4dc0184..6a8347deec3d70fb001c98bc7218db160f7ecb10 100644 --- a/app/models/concerns/has_user_type.rb +++ b/app/models/concerns/has_user_type.rb @@ -16,7 +16,6 @@ module HasUserType automation_bot: 9, security_policy_bot: 10, admin_bot: 11, - suggested_reviewers_bot: 12, service_account: 13, # 14: Deprecated llm_bot type (removed) placeholder: 15, @@ -34,7 +33,6 @@ module HasUserType automation_bot security_policy_bot admin_bot - suggested_reviewers_bot service_account duo_code_review_bot ].freeze diff --git a/config/feature_flags/ops/suggested_reviewers_internal_api.yml b/config/feature_flags/ops/suggested_reviewers_internal_api.yml deleted file mode 100644 index f19beefa58a5ffa4360a44a281e07cce0621fcfa..0000000000000000000000000000000000000000 --- a/config/feature_flags/ops/suggested_reviewers_internal_api.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: suggested_reviewers_internal_api -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106648 -rollout_issue_url: -milestone: '15.7' -type: ops -group: group::ai framework -default_enabled: true diff --git a/db/post_migrate/20250919100000_remove_suggested_reviewers_bot_user_type.rb b/db/post_migrate/20250919100000_remove_suggested_reviewers_bot_user_type.rb new file mode 100644 index 0000000000000000000000000000000000000000..631dccdadbae7a5f618a185b6f7bcae125e5b637 --- /dev/null +++ b/db/post_migrate/20250919100000_remove_suggested_reviewers_bot_user_type.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class RemoveSuggestedReviewersBotUserType < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_main + + milestone '18.5' + + def up + # Remove any existing users with suggested_reviewers_bot user_type + User.where(user_type: 12).delete_all + end + + def down + # This is irreversible as we've deleted the users + # The enum value 12 for suggested_reviewers_bot can be re-added + # in the application code if needed + end +end diff --git a/db/schema_migrations/20250919100000 b/db/schema_migrations/20250919100000 new file mode 100644 index 0000000000000000000000000000000000000000..9d0688e5afa6de293bdce5bd8ae4234add9925a9 --- /dev/null +++ b/db/schema_migrations/20250919100000 @@ -0,0 +1 @@ +154fe4fdb2393d2edbf7f20c38bc8235b9c3ff40be4aa942b7fdc4c8dc83fad2 \ No newline at end of file diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 2a776e60b2ab1d7337705a13a9b47d66239b2705..83b833fadd377742e07ae62eee65afa4872715c5 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -50316,7 +50316,6 @@ Possible types of user. | `SECURITY_POLICY_BOT` | Security policy bot. | | `SERVICE_ACCOUNT` | Service account. | | `SERVICE_USER` | Service user. | -| `SUGGESTED_REVIEWERS_BOT` | Suggested reviewers bot. | | `SUPPORT_BOT` | Support bot. | | `VISUAL_REVIEW_BOT` | Visual review bot. | diff --git a/ee/app/policies/ee/base_policy.rb b/ee/app/policies/ee/base_policy.rb index 05c2b1a6dbe08736d68ba8a2cfebf76cd6401602..574cfa75ce8cb9ed0845791c11cbc2ebd58972a4 100644 --- a/ee/app/policies/ee/base_policy.rb +++ b/ee/app/policies/ee/base_policy.rb @@ -16,10 +16,6 @@ module BasePolicy with_scope :user condition(:visual_review_bot, score: 0) { @user&.visual_review_bot? } - desc "User is suggested reviewers bot" - with_scope :user - condition(:suggested_reviewers_bot, score: 0) { @user&.suggested_reviewers_bot? } - with_scope :global condition(:license_block) { License.block_changes? } diff --git a/ee/app/policies/ee/policy_actor.rb b/ee/app/policies/ee/policy_actor.rb index 6052c5269dbfadfa5ef1874c052e069b429f0035..98e40ef22cbc0544ce1a1557ca6659e810372573 100644 --- a/ee/app/policies/ee/policy_actor.rb +++ b/ee/app/policies/ee/policy_actor.rb @@ -10,10 +10,6 @@ def visual_review_bot? false end - def suggested_reviewers_bot? - false - end - def group_sso?(_) false end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index eb8c148bc9b0f92618290d277a1496e5a7af376e..db76c23f4fd91a1703a6fd9cee3bfeb875dcfafa 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -1076,11 +1076,6 @@ module ProjectPolicy enable :create_key_result end - rule { suggested_reviewers_bot & suggested_reviewers_available & resource_access_token_feature_available & resource_access_token_creation_allowed }.policy do - enable :admin_project_member - enable :create_resource_access_tokens - end - rule { custom_role_enables_manage_project_access_tokens & resource_access_token_feature_available & resource_access_token_creation_allowed }.policy do enable :read_resource_access_tokens enable :create_resource_access_tokens diff --git a/ee/lib/api/internal/suggested_reviewers.rb b/ee/lib/api/internal/suggested_reviewers.rb deleted file mode 100644 index 1ede1c3a9a0a8869954a87cb65e9ef5c1001aa8c..0000000000000000000000000000000000000000 --- a/ee/lib/api/internal/suggested_reviewers.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -module API - # Suggested Reviewers Internal API - module Internal - class SuggestedReviewers < ::API::Base - feature_category :code_review_workflow - - before do - check_feature_enabled - authenticate_gitlab_suggested_reviewers_request! - end - - helpers do - def check_feature_enabled - not_found! unless Feature.enabled?(:suggested_reviewers_internal_api, type: :ops) - not_found! if ::Feature.enabled?(:hide_suggested_reviewers, type: :beta) - end - - def authenticate_gitlab_suggested_reviewers_request! - return if Gitlab::AppliedMl::SuggestedReviewers.verify_api_request(headers) - - render_api_error!('Suggested Reviewers JWT authentication invalid', :unauthorized) - end - - def create_access_token(project) - token_params = { - name: 'Suggested reviewers token', - access_level: Gitlab::Access::REPORTER, - scopes: [Gitlab::Auth::READ_API_SCOPE], - expires_at: 1.day.from_now - } - - ::ResourceAccessTokens::CreateService.new( - ::Users::Internal.suggested_reviewers_bot, - project, - token_params - ).execute - end - end - - namespace 'internal' do - namespace 'suggested_reviewers' do - resource :tokens do - desc 'Create a project access token' do - detail 'Creates a new access token for a project.' - tags 'project_access_tokens' - success Entities::ResourceAccessTokenWithToken - failure [ - { code: 400, message: 'Bad request' }, - { code: 401, message: 'Unauthorized' }, - { code: 404, message: 'Not found' } - ] - end - params do - requires :project_id, type: Integer, desc: 'The ID of the project' - end - post do - ::Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/503887', - new_threshold: 111) - - project = find_project(params[:project_id]) - not_found! unless project&.can_suggest_reviewers? - - token_response = create_access_token(project) - if token_response.success? - present( - token_response.payload[:access_token], - with: Entities::ResourceAccessTokenWithToken, - resource: project - ) - else - bad_request!(token_response.message) - end - end - end - end - end - end - end -end diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index fe998573ce3a2d6b40ef06e913209e266c2805fe..5d90bc709c930c3b90e32a2c7ec7e8e9542c831e 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -96,7 +96,6 @@ module API mount ::API::Internal::AppSec::Dast::SiteValidations mount ::API::Internal::Search::Zoekt - mount ::API::Internal::SuggestedReviewers mount ::API::Internal::Ai::XRay::Scan mount ::API::Internal::Observability diff --git a/ee/lib/ee/users/internal.rb b/ee/lib/ee/users/internal.rb index 4ca636d18572799ecae742ecd263a3ed71977398..960fd4d19ea4ed0bd6ff635e816d8e9bb15d21bc 100644 --- a/ee/lib/ee/users/internal.rb +++ b/ee/lib/ee/users/internal.rb @@ -9,7 +9,7 @@ module Internal extend Forwardable # Delegate to an instance method of the class - def_delegators :new, :visual_review_bot, :suggested_reviewers_bot + def_delegators :new, :visual_review_bot end # rubocop:disable CodeReuse/ActiveRecord -- Need to instantiate a record here @@ -23,18 +23,6 @@ def visual_review_bot u.private_profile = true end end - - def suggested_reviewers_bot - email_pattern = "suggested-reviewers-bot%s@#{Settings.gitlab.host}" - - unique_internal( - ::User.where(user_type: :suggested_reviewers_bot), 'suggested-reviewers-bot', email_pattern) do |u| - u.bio = 'The GitLab suggested reviewers bot used for suggested reviewers' - u.name = 'GitLab Suggested Reviewers Bot' - u.confirmed_at = Time.zone.now - u.private_profile = true - end - end # rubocop:enable CodeReuse/ActiveRecord end end diff --git a/ee/spec/lib/ee/users/internal_spec.rb b/ee/spec/lib/ee/users/internal_spec.rb index 026d76fef9fb996c5d1ac4a176576df1e7600b7a..8c1ebbe47f71f7d497e5c8dff8267d615d5a0c87 100644 --- a/ee/spec/lib/ee/users/internal_spec.rb +++ b/ee/spec/lib/ee/users/internal_spec.rb @@ -26,6 +26,4 @@ end.not_to change { User.count } end end - - it_behaves_like 'bot users', :suggested_reviewers_bot end diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 17f1c46555d4941fc98ca04bbc0cc941421d1ad5..91abef483d190f9c362f135d54aa2551cd74a564 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3303,57 +3303,6 @@ def create_member_role(member, abilities = member_role_abilities) end end - describe 'permissions for suggested reviewers bot', :saas do - let(:permissions) { [:admin_project_member, :create_resource_access_tokens] } - let(:namespace) { build_stubbed(:namespace) } - let(:project) { build_stubbed(:project, namespace: namespace) } - - context 'when user is suggested_reviewers_bot' do - let(:current_user) { Users::Internal.suggested_reviewers_bot } - - where(:suggested_reviewers_available, :token_creation_allowed, :allowed) do - false | false | false - false | true | false - true | false | false - true | true | true - end - - with_them do - before do - allow(project).to receive(:can_suggest_reviewers?).and_return(suggested_reviewers_available) - - allow(::Gitlab::CurrentSettings) - .to receive(:personal_access_tokens_disabled?) - .and_return(!token_creation_allowed) - end - - it 'always allows permissions except when feature disabled' do - if allowed - expect_allowed(*permissions) - else - expect_disallowed(*permissions) - end - end - end - end - - context 'when user is not suggested_reviewers_bot' do - let(:current_user) { developer } - - before do - allow(project).to receive(:can_suggest_reviewers?).and_return(true) - - allow(::Gitlab::CurrentSettings) - .to receive(:personal_access_tokens_disabled?) - .and_return(false) - end - - it 'does not allow permissions' do - expect_disallowed(*permissions) - end - end - end - describe 'read_runners' do context 'with auditor' do let(:current_user) { auditor } diff --git a/ee/spec/requests/api/internal/suggested_reviewers_spec.rb b/ee/spec/requests/api/internal/suggested_reviewers_spec.rb deleted file mode 100644 index 014b3dd896af23eac2b032eab5710f48f0b8eba4..0000000000000000000000000000000000000000 --- a/ee/spec/requests/api/internal/suggested_reviewers_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe API::Internal::SuggestedReviewers, feature_category: :code_review_workflow do - describe 'POST /internal/suggested_reviewers/tokens' do - let_it_be(:organization) { create(:organization) } - let_it_be_with_reload(:project) { create(:project) } - let_it_be(:secret) do - SecureRandom.random_bytes(Gitlab::AppliedMl::SuggestedReviewers::SECRET_LENGTH) - end - - let(:url) { '/internal/suggested_reviewers/tokens' } - let(:params) { { project_id: project.id } } - let(:headers) { {} } - - subject do - post api(url), params: params, headers: headers - end - - before do - allow(Gitlab::AppliedMl::SuggestedReviewers).to receive(:secret).and_return(secret) - end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(suggested_reviewers_internal_api: false) - end - - it 'returns 404' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when hide_suggested_reviewers is enabled' do - it 'returns 404' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when feature flag is enabled' do - before do - stub_feature_flags(suggested_reviewers_internal_api: true) - stub_feature_flags(hide_suggested_reviewers: false) - end - - context 'when authentication header is not set', :aggregate_failures do - it 'returns 401' do - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(json_response).to eq('message' => 'Suggested Reviewers JWT authentication invalid') - end - end - - context 'when authentication header is set' do - let(:headers) do - jwt_token = JWT.encode( - { 'iss' => Gitlab::AppliedMl::SuggestedReviewers::JWT_ISSUER, 'iat' => 1.minute.ago.to_i }, - secret, 'HS256' - ) - - { Gitlab::AppliedMl::SuggestedReviewers::INTERNAL_API_REQUEST_HEADER => jwt_token } - end - - context 'when project is not allowed to suggest reviewers' do - before do - stub_licensed_features(suggested_reviewers: false) - end - - it 'returns 404' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when project is allowed to suggest reviewers', :saas do - before do - stub_licensed_features(suggested_reviewers: true) - project.project_setting.update!(suggested_reviewers_enabled: true) - end - - context 'when create token service fails' do - let(:service) { instance_spy(ResourceAccessTokens::CreateService) } - let(:error_response) do - ServiceResponse.error(message: 'something went wrong') - end - - before do - allow(service).to receive(:execute).and_return(error_response) - allow(ResourceAccessTokens::CreateService).to receive(:new).and_return(service) - end - - it 'returns 400', :aggregate_failures do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to eq('message' => '400 Bad request - something went wrong') - end - end - - context 'when create token service succeeds' do - it 'returns 200', [:freeze_time, :aggregate_failures] do - expires_at = 1.day.from_now.to_date.to_s - - subject - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to include( - 'name' => 'Suggested reviewers token', - 'access_level' => Gitlab::Access::REPORTER, - 'scopes' => ['read_api'], - 'expires_at' => expires_at, - 'token' => kind_of(String) - ) - end - end - end - end - end - end -end diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index 0cc95751461951d9855232fdcc19bb25d2322057..058c029c16d11a9f19a55ffe7206c7179f5b0154 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -13,7 +13,7 @@ specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot - visual_review_bot migration_bot automation_bot security_policy_bot admin_bot suggested_reviewers_bot + visual_review_bot migration_bot automation_bot security_policy_bot admin_bot service_account placeholder duo_code_review_bot import_user]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) diff --git a/tooling/danger/internal_users.rb b/tooling/danger/internal_users.rb index 431bd4a66170ec2bb0f14640a68ca1f4f554ba71..73027b132010a7049bf145675133135587384ec3 100644 --- a/tooling/danger/internal_users.rb +++ b/tooling/danger/internal_users.rb @@ -40,7 +40,6 @@ module InternalUsers automation_bot security_policy_bot admin_bot - suggested_reviewers_bot placeholder duo_code_review_bot import_user