From d83b0295084a6d6cd71c153e6f7256ec34bcb7f6 Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Mon, 16 Jun 2025 17:07:39 -0400 Subject: [PATCH 1/3] Configure web based commit signing with project update endpoint - Adds an optional parameter ro the project update api endpoint - Checks for feature availability before updating the project EE: true Changelog: added --- doc/api/openapi/openapi_v2.yaml | 7 ++ ee/app/services/ee/projects/update_service.rb | 7 +- ee/lib/ee/api/entities/project.rb | 4 + ee/lib/ee/api/helpers/projects_helpers.rb | 6 +- ee/spec/lib/ee/api/entities/project_spec.rb | 25 ++++++ ee/spec/requests/api/projects_spec.rb | 47 +++++++++++ .../services/projects/update_service_spec.rb | 80 +++++++++++-------- 7 files changed, 141 insertions(+), 35 deletions(-) diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index 7903acd71db190..6bcecee3e92662 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -45493,6 +45493,8 @@ definitions: type: string auto_duo_code_review_enabled: type: string + web_based_commit_signing_enabled: + type: string description: API_Entities_Project model API_Entities_LicenseBasic: type: object @@ -61152,6 +61154,8 @@ definitions: type: string auto_duo_code_review_enabled: type: string + web_based_commit_signing_enabled: + type: string permissions: type: object properties: @@ -61688,6 +61692,9 @@ definitions: ci_restrict_pipeline_cancellation_role: type: string description: Roles allowed to cancel pipelines and jobs. + web_based_commit_signing_enabled: + type: boolean + description: Enable web based commit signing for this project description: Update an existing project postApiV4ProjectsIdShare: type: object diff --git a/ee/app/services/ee/projects/update_service.rb b/ee/app/services/ee/projects/update_service.rb index b87f5314d78380..6e62fc66cea419 100644 --- a/ee/app/services/ee/projects/update_service.rb +++ b/ee/app/services/ee/projects/update_service.rb @@ -75,8 +75,11 @@ def assign_repository_size_limit def validate_web_based_commit_signing_enabled return unless params.key?(:web_based_commit_signing_enabled) - params.delete(:web_based_commit_signing_enabled) unless - ::Feature.enabled?(:use_web_based_commit_signing_enabled, project) && !namespace_settings_enabled? + return if ::Gitlab::Saas.feature_available?(:repositories_web_based_commit_signing) && + ::Feature.enabled?(:use_web_based_commit_signing_enabled, project) && + !namespace_settings_enabled? + + params.delete(:web_based_commit_signing_enabled) end def namespace_settings_enabled? diff --git a/ee/lib/ee/api/entities/project.rb b/ee/lib/ee/api/entities/project.rb index 022ec45d29055b..7f6e5a7063996e 100644 --- a/ee/lib/ee/api/entities/project.rb +++ b/ee/lib/ee/api/entities/project.rb @@ -65,6 +65,10 @@ def preload_relation(projects_relation, options = {}) expose :allow_pipeline_trigger_approve_deployment, documentation: { type: 'boolean' }, if: ->(project, _) { project.feature_available?(:protected_environments) } expose :prevent_merge_without_jira_issue, if: ->(project, _) { project.feature_available?(:jira_issue_association_enforcement) } expose :auto_duo_code_review_enabled, if: ->(project, _) { project.namespace.has_active_add_on_purchase?(:duo_enterprise) } + expose :web_based_commit_signing_enabled, if: ->(project, options) do + ::Gitlab::Saas.feature_available?(:repositories_web_based_commit_signing) && + Ability.allowed?(options[:current_user], :admin_project, project) + end end end end diff --git a/ee/lib/ee/api/helpers/projects_helpers.rb b/ee/lib/ee/api/helpers/projects_helpers.rb index 9bc2e392d6b220..6bc39bfe87c207 100644 --- a/ee/lib/ee/api/helpers/projects_helpers.rb +++ b/ee/lib/ee/api/helpers/projects_helpers.rb @@ -47,6 +47,9 @@ module ProjectsHelpers optional :merge_trains_enabled, type: Grape::API::Boolean, desc: 'Enable merge trains.' optional :merge_trains_skip_train_allowed, type: Grape::API::Boolean, desc: 'Allow merge train merge requests to be merged without waiting for pipelines to finish.' optional :ci_restrict_pipeline_cancellation_role, type: String, desc: 'Roles allowed to cancel pipelines and jobs.' + optional :web_based_commit_signing_enabled, + type: ::Grape::API::Boolean, + desc: 'Enable web based commit signing for this project' end params :share_project_params_ee do @@ -79,7 +82,8 @@ def update_params_at_least_one_of :merge_trains_skip_train_allowed, :requirements_access_level, :prevent_merge_without_jira_issue, - :ci_restrict_pipeline_cancellation_role + :ci_restrict_pipeline_cancellation_role, + :web_based_commit_signing_enabled ] end end diff --git a/ee/spec/lib/ee/api/entities/project_spec.rb b/ee/spec/lib/ee/api/entities/project_spec.rb index 1bda5cd81a59eb..d624c79b041720 100644 --- a/ee/spec/lib/ee/api/entities/project_spec.rb +++ b/ee/spec/lib/ee/api/entities/project_spec.rb @@ -8,6 +8,7 @@ let(:options) { {} } let(:developer) { create(:user, developer_of: project) } let(:guest) { create(:user, guest_of: project) } + let(:maintainer) { create(:user, maintainer_of: project) } let(:entity) do ::API::Entities::Project.new(project, options) @@ -153,4 +154,28 @@ def mock_available end end end + + describe 'web_based_commit_signing_enabled' do + context 'and the repositories_web_based_commit_signing feature is available' do + before do + stub_saas_features(repositories_web_based_commit_signing: true) + end + + context 'and user is a maintainer' do + let(:options) { { current_user: maintainer } } + + it 'returns expected data' do + expect(subject.keys).to include( + :web_based_commit_signing_enabled + ) + end + end + end + + it 'not web_based_commit_signing_enabled' do + expect(subject.keys).not_to include( + :web_based_commit_signing_enabled + ) + end + end end diff --git a/ee/spec/requests/api/projects_spec.rb b/ee/spec/requests/api/projects_spec.rb index b80f1456ef46ea..c5ae203fd3f068 100644 --- a/ee/spec/requests/api/projects_spec.rb +++ b/ee/spec/requests/api/projects_spec.rb @@ -1951,6 +1951,53 @@ end end end + + context 'updating web_based_commit_signing_enabled' do + using RSpec::Parameterized::TableSyntax + + let(:project_params) { { web_based_commit_signing_enabled: true } } + + shared_examples_for 'does not update the value' do + it { expect { subject }.not_to change { project.reload.web_based_commit_signing_enabled }.from(false) } + end + + context 'when authenticated as maintainer' do + before do + project.add_maintainer(user) + end + + it 'does not render the value' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.keys).not_to include('web_based_commit_signing_enabled') + end + + it_behaves_like 'does not update the value' + + context 'and the feature is available' do + before do + stub_saas_features(repositories_web_based_commit_signing: true) + end + + context 'and the feature is not enabled' do + before do + stub_feature_flags(use_web_based_commit_signing_enabled: false) + end + + it_behaves_like 'does not update the value' + end + + context 'and the feature is enabled' do + it 'updates the attribute as expected' do + expect { subject }.to change { project.reload.web_based_commit_signing_enabled }.from(false).to(true) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['web_based_commit_signing_enabled']).to eq(true) + end + end + end + end + end end describe 'DELETE /projects/:id' do diff --git a/ee/spec/services/projects/update_service_spec.rb b/ee/spec/services/projects/update_service_spec.rb index 67e7ab553ecaaf..18fcc7a963f67a 100644 --- a/ee/spec/services/projects/update_service_spec.rb +++ b/ee/spec/services/projects/update_service_spec.rb @@ -896,55 +896,71 @@ def expect_on_service(service, instance) let_it_be(:group) { create(:group) } let_it_be_with_reload(:project) { create(:project, :repository, creator: user, group: group) } - context 'when web_based_commit_signing_enabled is not present' do + context 'when the repositories_web_based_commit_signing feature is not available' do + before do + stub_saas_features(repositories_web_based_commit_signing: false) + end + it_behaves_like 'ignores web_based_commit_signing_enabled param', - param: {}, + param: { web_based_commit_signing_enabled: true }, expected: false end - context 'when group setting web_based_commit_signing_enabled is enabled' do - before_all do - group.namespace_settings.update!(web_based_commit_signing_enabled: true) - project.project_setting.update!(web_based_commit_signing_enabled: true) + context 'when the repositories_web_based_commit_signing feature not available' do + before do + stub_saas_features(repositories_web_based_commit_signing: true) end - it_behaves_like 'ignores web_based_commit_signing_enabled param', - param: { web_based_commit_signing_enabled: false }, - expected: true - end + context 'when web_based_commit_signing_enabled is not present' do + it_behaves_like 'ignores web_based_commit_signing_enabled param', + param: {}, + expected: false + end - context 'when group setting web_based_commit_signing_enabled is not enabled' do - it 'updates web_based_commit_signing_enabled to the project' do - params = { web_based_commit_signing_enabled: true } - result = update_project(project, user, params) + context 'when group setting web_based_commit_signing_enabled is enabled' do + before_all do + group.namespace_settings.update!(web_based_commit_signing_enabled: true) + project.project_setting.update!(web_based_commit_signing_enabled: true) + end - expect(result).to eq(status: :success) + it_behaves_like 'ignores web_based_commit_signing_enabled param', + param: { web_based_commit_signing_enabled: false }, + expected: true + end + + context 'when group setting web_based_commit_signing_enabled is not enabled' do + it 'updates web_based_commit_signing_enabled to the project' do + params = { web_based_commit_signing_enabled: true } + result = update_project(project, user, params) + + expect(result).to eq(status: :success) - expect(project.web_based_commit_signing_enabled).to be(true) + expect(project.web_based_commit_signing_enabled).to be(true) + end end - end - context 'when project does not belong to a group' do - let_it_be(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } + context 'when project does not belong to a group' do + let_it_be(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } - it 'updates web_based_commit_signing_enabled to the project' do - params = { web_based_commit_signing_enabled: true } - result = update_project(project, user, params) + it 'updates web_based_commit_signing_enabled to the project' do + params = { web_based_commit_signing_enabled: true } + result = update_project(project, user, params) - expect(result).to eq(status: :success) + expect(result).to eq(status: :success) - expect(project.web_based_commit_signing_enabled).to be(true) + expect(project.web_based_commit_signing_enabled).to be(true) + end end - end - context 'when use_web_based_commit_signing_enabled FF is disabled' do - before do - stub_feature_flags(use_web_based_commit_signing_enabled: false) - end + context 'when use_web_based_commit_signing_enabled FF is disabled' do + before do + stub_feature_flags(use_web_based_commit_signing_enabled: false) + end - it_behaves_like 'ignores web_based_commit_signing_enabled param', - param: { web_based_commit_signing_enabled: true }, - expected: false + it_behaves_like 'ignores web_based_commit_signing_enabled param', + param: { web_based_commit_signing_enabled: true }, + expected: false + end end end -- GitLab From 38af02065e134e5387dd2037c975f2c3ae219372 Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Thu, 19 Jun 2025 14:46:54 -0400 Subject: [PATCH 2/3] Refactor tests and rephrase context --- ee/spec/lib/ee/api/entities/project_spec.rb | 24 +++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/ee/spec/lib/ee/api/entities/project_spec.rb b/ee/spec/lib/ee/api/entities/project_spec.rb index d624c79b041720..e0f4ca632da7a9 100644 --- a/ee/spec/lib/ee/api/entities/project_spec.rb +++ b/ee/spec/lib/ee/api/entities/project_spec.rb @@ -156,10 +156,22 @@ def mock_available end describe 'web_based_commit_signing_enabled' do - context 'and the repositories_web_based_commit_signing feature is available' do - before do - stub_saas_features(repositories_web_based_commit_signing: true) + before do + stub_saas_features(repositories_web_based_commit_signing: repositories_web_based_commit_signing) + end + + context 'and the repositories_web_based_commit_signing feature is not available' do + let(:repositories_web_based_commit_signing) { false } + + it 'does not serialize web_based_commit_signing_enabled' do + expect(subject.keys).not_to include( + :web_based_commit_signing_enabled + ) end + end + + context 'and the repositories_web_based_commit_signing feature is available' do + let(:repositories_web_based_commit_signing) { true } context 'and user is a maintainer' do let(:options) { { current_user: maintainer } } @@ -171,11 +183,5 @@ def mock_available end end end - - it 'not web_based_commit_signing_enabled' do - expect(subject.keys).not_to include( - :web_based_commit_signing_enabled - ) - end end end -- GitLab From 60674797bbcf26b63042962c1cd3656561b1dcfc Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Fri, 20 Jun 2025 10:39:36 -0400 Subject: [PATCH 3/3] Refactor tests and fix typo --- ee/spec/lib/ee/api/entities/project_spec.rb | 10 ++++++++++ ee/spec/requests/api/projects_spec.rb | 14 ++++++++------ ee/spec/services/projects/update_service_spec.rb | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/ee/spec/lib/ee/api/entities/project_spec.rb b/ee/spec/lib/ee/api/entities/project_spec.rb index e0f4ca632da7a9..9a413a5d55986e 100644 --- a/ee/spec/lib/ee/api/entities/project_spec.rb +++ b/ee/spec/lib/ee/api/entities/project_spec.rb @@ -173,6 +173,16 @@ def mock_available context 'and the repositories_web_based_commit_signing feature is available' do let(:repositories_web_based_commit_signing) { true } + context 'and user does not have permission for the attribute' do + let(:options) { { current_user: developer } } + + it 'does not serialize web_based_commit_signing_enabled' do + expect(subject.keys).not_to include( + :web_based_commit_signing_enabled + ) + end + end + context 'and user is a maintainer' do let(:options) { { current_user: maintainer } } diff --git a/ee/spec/requests/api/projects_spec.rb b/ee/spec/requests/api/projects_spec.rb index c5ae203fd3f068..53d4ce4a9b3a6f 100644 --- a/ee/spec/requests/api/projects_spec.rb +++ b/ee/spec/requests/api/projects_spec.rb @@ -1966,14 +1966,16 @@ project.add_maintainer(user) end - it 'does not render the value' do - subject + context 'and the feature is not available' do + it 'does not render the value' do + subject - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.keys).not_to include('web_based_commit_signing_enabled') - end + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.keys).not_to include('web_based_commit_signing_enabled') + end - it_behaves_like 'does not update the value' + it_behaves_like 'does not update the value' + end context 'and the feature is available' do before do diff --git a/ee/spec/services/projects/update_service_spec.rb b/ee/spec/services/projects/update_service_spec.rb index 18fcc7a963f67a..5891752152de7c 100644 --- a/ee/spec/services/projects/update_service_spec.rb +++ b/ee/spec/services/projects/update_service_spec.rb @@ -906,7 +906,7 @@ def expect_on_service(service, instance) expected: false end - context 'when the repositories_web_based_commit_signing feature not available' do + context 'when the repositories_web_based_commit_signing feature is available' do before do stub_saas_features(repositories_web_based_commit_signing: true) end -- GitLab