diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml
index 8ea807003407f1cd74d341734fd20d3b1fbeb02f..b1fa63dbf5601623801d473a350bfbbca38b13b6 100644
--- a/app/views/groups/settings/_permissions.html.haml
+++ b/app/views/groups/settings/_permissions.html.haml
@@ -44,6 +44,7 @@
= render 'groups/settings/subgroup_creation_level', f: f, group: @group
= render_if_exists 'groups/settings/prevent_forking', f: f, group: @group
= render_if_exists 'groups/settings/service_access_tokens_expiration_enforced', f: f, group: @group
+ = render_if_exists 'groups/settings/enforce_ssh_certificates', f: f, group: @group
= 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
diff --git a/config/feature_flags/development/enforce_ssh_certificates_via_settings.yml b/config/feature_flags/development/enforce_ssh_certificates_via_settings.yml
new file mode 100644
index 0000000000000000000000000000000000000000..e721c2afe8c8a0949186e59dc525e4d8bc1bad44
--- /dev/null
+++ b/config/feature_flags/development/enforce_ssh_certificates_via_settings.yml
@@ -0,0 +1,8 @@
+---
+name: enforce_ssh_certificates_via_settings
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/136498
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/426235
+milestone: '16.7'
+type: development
+group: group::source code
+default_enabled: false
diff --git a/db/migrate/20231109165512_add_enforce_ssh_certificates_to_namespace_settings.rb b/db/migrate/20231109165512_add_enforce_ssh_certificates_to_namespace_settings.rb
new file mode 100644
index 0000000000000000000000000000000000000000..98c4de0cb6a57e6df4922d19ca05d58a48356f3e
--- /dev/null
+++ b/db/migrate/20231109165512_add_enforce_ssh_certificates_to_namespace_settings.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class AddEnforceSshCertificatesToNamespaceSettings < Gitlab::Database::Migration[2.2]
+ enable_lock_retries!
+
+ milestone '16.7'
+
+ def change
+ add_column :namespace_settings, :enforce_ssh_certificates, :boolean, default: false, null: false
+ end
+end
diff --git a/db/schema_migrations/20231109165512 b/db/schema_migrations/20231109165512
new file mode 100644
index 0000000000000000000000000000000000000000..1e3a229c9d1c30f334a763a884ffb28435c10937
--- /dev/null
+++ b/db/schema_migrations/20231109165512
@@ -0,0 +1 @@
+2d3abd070d856db04eea298bbbe82681ca01912e19f978de876fce68ed2ada26
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 5ea1c69abc56322049be6bc6855f3b1ab35f7278..6116bb5688f8150a45a8caa799a81301d497bb9f 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -19472,6 +19472,7 @@ CREATE TABLE namespace_settings (
service_access_tokens_expiration_enforced boolean DEFAULT true NOT NULL,
product_analytics_enabled boolean DEFAULT false NOT NULL,
allow_merge_without_pipeline boolean DEFAULT false NOT NULL,
+ enforce_ssh_certificates 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 786e1d630b9210efc9891db09dcc76524453ff98..4b4d96f485c1f684b541aae1915c64bec6044d92 100644
--- a/ee/app/controllers/ee/groups_controller.rb
+++ b/ee/app/controllers/ee/groups_controller.rb
@@ -98,6 +98,7 @@ def group_params_ee
params_ee << :max_personal_access_token_lifetime if current_group&.personal_access_token_expiration_policy_available?
params_ee << :prevent_forking_outside_group if can_change_prevent_forking?(current_user, current_group)
params_ee << :service_access_tokens_expiration_enforced if can_change_service_access_tokens_expiration?(current_user, current_group)
+ params_ee << :enforce_ssh_certificates if current_group&.ssh_certificates_available?
params_ee << :code_suggestions if ai_assist_ui_enabled?
params_ee << { value_stream_dashboard_aggregation_attributes: [:enabled] } if can?(current_user, :modify_value_stream_dashboard_settings, current_group)
params_ee << :product_analytics_enabled
diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb
index b1948f719050953139cd70a9479cbc3fb16c6c15..c770ef222bec53db57febf924f1b2d18b60dfb1b 100644
--- a/ee/app/models/ee/namespace.rb
+++ b/ee/app/models/ee/namespace.rb
@@ -114,6 +114,7 @@ module Namespace
:additional_purchased_storage_ends_on, :additional_purchased_storage_ends_on=,
:temporary_storage_increase_ends_on, :temporary_storage_increase_ends_on=,
to: :namespace_limit, allow_nil: true
+ delegate :enforce_ssh_certificates=, to: :namespace_settings
# `eligible_additional_purchased_storage_size` uses a FF to start checking `additional_purchased_storage_ends_on`
# if the FF is enabled before returning `additional_purchased_storage_size`
@@ -548,6 +549,14 @@ def domain_verification_available?
::Gitlab.com? && root? && licensed_feature_available?(:domain_verification)
end
+ def enforce_ssh_certificates?
+ root? && namespace_settings.enforce_ssh_certificates?
+ end
+
+ def ssh_certificates_available?
+ root? && licensed_feature_available?(:ssh_certificates)
+ end
+
def custom_roles_enabled?
root_ancestor.licensed_feature_available?(:custom_roles)
end
diff --git a/ee/app/views/groups/settings/_enforce_ssh_certificates.html.haml b/ee/app/views/groups/settings/_enforce_ssh_certificates.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..74fb495b01e70aca009ab1b835b56207977ef2e2
--- /dev/null
+++ b/ee/app/views/groups/settings/_enforce_ssh_certificates.html.haml
@@ -0,0 +1,7 @@
+- if group.ssh_certificates_available?
+ %h5= s_('GroupSettings|Enforce SSH Certificates')
+
+ .form-group.gl-mb-3
+ = f.gitlab_ui_checkbox_component :enforce_ssh_certificates,
+ s_('GroupSettings|Enforce SSH Certificates'),
+ checkbox_options: { disabled: !can?(current_user, :admin_group, group), checked: group.enforce_ssh_certificates? }
diff --git a/ee/lib/ee/gitlab/git_access_project.rb b/ee/lib/ee/gitlab/git_access_project.rb
index 7adcac2635d187978f043211c98fbd10d5c7440b..adc816afcd2ccc49f9e7cfbb0e7ca85635e71593 100644
--- a/ee/lib/ee/gitlab/git_access_project.rb
+++ b/ee/lib/ee/gitlab/git_access_project.rb
@@ -34,14 +34,22 @@ def allowed_access_namespace?
# It may happen, when a user authenticates via SSH certificate and tries accessing to personal namespace
return allowed_namespace_path.blank? unless namespace&.licensed_feature_available?(:ssh_certificates)
- root_namespace = namespace.root_ancestor
-
- # When allowed_namespace_path is not specified, it's checked whether SSH certificates are not enforced
- return true if allowed_namespace_path.blank? && ::Feature.disabled?(:enforce_ssh_certificates, root_namespace)
- return root_namespace.enabled_git_access_protocol != 'ssh_certificates' if allowed_namespace_path.blank?
+ # When allowed_namespace_path is not specified, it's checked whether SSH certificates are enforced
+ return !enforced_ssh_certificates? if allowed_namespace_path.blank?
allowed_namespace = ::Namespace.find_by_full_path(allowed_namespace_path)
- allowed_namespace.present? && root_namespace.id == allowed_namespace.id
+ allowed_namespace.present? && namespace.root_ancestor.id == allowed_namespace.id
+ end
+
+ # Verify that enabled_git_access_protocol is ssh_certificates and the
+ # actor is either User or Key
+ # Deploy Keys are allowed anyway
+ def enforced_ssh_certificates?
+ return false if ::Feature.disabled?(:enforce_ssh_certificates_via_settings, namespace.root_ancestor)
+ return false unless namespace.root_ancestor.enforce_ssh_certificates?
+ return false unless actor.is_a?(User) || actor.instance_of?(::Key)
+
+ user.human?
end
end
end
diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb
index 9def58b82a85f8e0740fc010c818d22945277679..6c7c6ff2c30f335567bff2e99f00d49ff36730c1 100644
--- a/ee/spec/models/ee/namespace_spec.rb
+++ b/ee/spec/models/ee/namespace_spec.rb
@@ -1971,6 +1971,60 @@
end
end
+ describe '#enforce_ssh_certificates?' do
+ let(:namespace) { create(:group) }
+
+ before do
+ namespace.enforce_ssh_certificates = true
+ end
+
+ it 'delegates the field to namespace settings' do
+ expect(namespace.namespace_settings).to receive(:enforce_ssh_certificates?).and_call_original
+
+ expect(namespace.enforce_ssh_certificates?).to eq(true)
+ end
+
+ context 'with a subgroup' do
+ let(:namespace) { create(:group, :nested) }
+
+ it 'returns false' do
+ expect(namespace.enforce_ssh_certificates?).to eq(false)
+ end
+ end
+ end
+
+ describe '#ssh_certificates_available?' do
+ let(:namespace) { create(:group) }
+
+ context 'when the feature is not licensed' do
+ before do
+ stub_licensed_features(ssh_certificates: false)
+ end
+
+ it 'is not available' do
+ expect(namespace.ssh_certificates_available?).to eq(false)
+ end
+ end
+
+ context 'when the feature is licensed' do
+ before do
+ stub_licensed_features(ssh_certificates: true)
+ end
+
+ it 'is available' do
+ expect(namespace.ssh_certificates_available?).to eq(true)
+ end
+
+ context 'with a subgroup' do
+ let(:subgroup) { create(:group, :nested) }
+
+ it 'is not available' do
+ expect(subgroup.ssh_certificates_available?).to eq(false)
+ end
+ end
+ end
+ end
+
describe '#custom_roles_enabled?', feature_category: :system_access do
let_it_be(:namespace) { create(:group) }
diff --git a/ee/spec/requests/api/internal/base_spec.rb b/ee/spec/requests/api/internal/base_spec.rb
index a913d7a0be95c4d133c7ebf97aaf80437bcf0d6a..bd2711aecae19cc704a3487c7109037763e0facc 100644
--- a/ee/spec/requests/api/internal/base_spec.rb
+++ b/ee/spec/requests/api/internal/base_spec.rb
@@ -525,20 +525,26 @@ def check_access_by_alias(alias_name)
end
context 'when authenticated via an SSH certificate' do
- let_it_be(:root_group) { create(:group) }
+ let_it_be_with_refind(:root_group) { create(:group) }
let_it_be(:group) { create(:group, parent: root_group) }
let_it_be(:project) { create(:project, :public, :repository, group: group) }
- def check_allowed(namespace_path)
+ let(:namespace_path) { nil }
+
+ let(:params) do
+ {
+ action: "git-upload-pack",
+ user_id: user.id,
+ project: project.full_path,
+ protocol: 'ssh',
+ namespace_path: namespace_path
+ }
+ end
+
+ def check_allowed
post(
api("/internal/allowed"),
- params: {
- action: "git-upload-pack",
- user_id: user.id,
- project: project.full_path,
- protocol: 'ssh_certificates',
- namespace_path: namespace_path
- },
+ params: params,
headers: gitlab_shell_internal_api_request_header
)
end
@@ -548,32 +554,67 @@ def check_allowed(namespace_path)
end
context 'when group is not specified' do
- it 'is successful' do
- check_allowed(nil)
+ it 'returns success response' do
+ check_allowed
expect(response).to have_gitlab_http_status(:ok)
end
context 'when auth via SSH certificates is enforced' do
- it 'is forbidden' do
- root_group.namespace_settings.enabled_git_access_protocol = 'ssh_certificates'
+ before do
+ root_group.namespace_settings.enforce_ssh_certificates = true
root_group.save!
+ end
- check_allowed(nil)
+ it 'returns an unauthorized error response' do
+ check_allowed
expect(response).to have_gitlab_http_status(:unauthorized)
expect(json_response['status']).to eq(false)
expect(json_response['message']).to eq('You are not allowed to access projects in this namespace.')
end
- context 'when enforce_ssh_certificates feature flag is disabled' do
- it 'is successful' do
- root_group.namespace_settings.enabled_git_access_protocol = 'ssh_certificates'
- root_group.save!
+ context 'when enforce_ssh_certificates_via_settings feature flag is disabled' do
+ before do
+ stub_feature_flags(enforce_ssh_certificates_via_settings: false)
+ end
- stub_feature_flags(enforce_ssh_certificates: false)
+ it 'returns success response' do
+ check_allowed
- check_allowed(nil)
+ expect(response).to have_gitlab_http_status(:ok)
+ end
+ end
+
+ context 'when service account is used' do
+ let(:params) do
+ {
+ action: "git-upload-pack",
+ project: project.full_path,
+ namespace_path: namespace_path,
+ user_id: create(:user, :service_account).id
+ }
+ end
+
+ it 'returns success response' do
+ check_allowed
+
+ expect(response).to have_gitlab_http_status(:ok)
+ end
+ end
+
+ context 'when deploy key is used' do
+ let(:params) do
+ {
+ action: "git-upload-pack",
+ project: project.full_path,
+ namespace_path: namespace_path,
+ key_id: create(:deploy_key).id
+ }
+ end
+
+ it 'returns success response' do
+ check_allowed
expect(response).to have_gitlab_http_status(:ok)
end
@@ -582,8 +623,10 @@ def check_allowed(namespace_path)
end
context 'when non-root group is specified' do
- it 'is forbidden' do
- check_allowed(group.full_path)
+ let(:namespace_path) { group.full_path }
+
+ it 'returns an unauthorized error response' do
+ check_allowed
expect(response).to have_gitlab_http_status(:unauthorized)
expect(json_response['status']).to eq(false)
@@ -592,17 +635,19 @@ def check_allowed(namespace_path)
end
context 'when root group is specified' do
+ let(:namespace_path) { root_group.full_path }
+
it 'is successful' do
- check_allowed(root_group.full_path)
+ check_allowed
expect(response).to have_gitlab_http_status(:ok)
end
context 'when ssh_certificates licensed feature is not available' do
- it 'is forbidden' do
+ it 'returns an unauthorized error response' do
stub_licensed_features(ssh_certificates: false)
- check_allowed(root_group.full_path)
+ check_allowed
expect(response).to have_gitlab_http_status(:unauthorized)
end
@@ -611,8 +656,8 @@ def check_allowed(namespace_path)
context 'when personal project is accessed' do
let_it_be(:project) { create(:project, :public, :repository, namespace: user.namespace) }
- it 'is forbidden' do
- check_allowed(root_group.full_path)
+ it 'returns an unauthorized error response' do
+ check_allowed
expect(response).to have_gitlab_http_status(:unauthorized)
end
diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb
index faadd8fc4a2a9bccc3f27a8cea5722720dfeec05..949eaf546add52ba8f654d3a9dfe4048b096eb4e 100644
--- a/ee/spec/requests/groups_controller_spec.rb
+++ b/ee/spec/requests/groups_controller_spec.rb
@@ -333,6 +333,35 @@
end
end
end
+
+ context 'setting enforce_ssh_certificates' do
+ let(:params) { { group: { enforce_ssh_certificates: true } } }
+
+ it 'does not change the column' do
+ expect { subject }.not_to change { group.reload.enforce_ssh_certificates? }
+ expect(response).to have_gitlab_http_status(:found)
+ end
+
+ context 'when ssh_certificates licensed feature is available' do
+ before do
+ stub_licensed_features(ssh_certificates: true)
+ end
+
+ it 'successfully changes the column' do
+ expect { subject }.to change { group.reload.enforce_ssh_certificates? }
+ expect(response).to have_gitlab_http_status(:found)
+ end
+
+ context 'when a group is not a top-level group' do
+ let(:group) { create(:group, :nested) }
+
+ it 'does not change the column' do
+ expect { subject }.not_to change { group.reload.enforce_ssh_certificates? }
+ expect(response).to have_gitlab_http_status(:found)
+ end
+ end
+ end
+ end
end
describe 'PUT #transfer', :saas do
diff --git a/ee/spec/views/groups/settings/_enforce_ssh_certificates.html.haml_spec.rb b/ee/spec/views/groups/settings/_enforce_ssh_certificates.html.haml_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..51884c94df32a08e025a3dd1d9962938ff182e85
--- /dev/null
+++ b/ee/spec/views/groups/settings/_enforce_ssh_certificates.html.haml_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'groups/settings/_enforce_ssh_certificates.html.haml', feature_category: :source_code_management do
+ let_it_be(:group) { build_stubbed(:group, namespace_settings: build_stubbed(:namespace_settings)) }
+
+ before do
+ allow(view).to receive(:group).and_return(group)
+ end
+
+ context 'when ssh certificates feature is unavailable' do
+ it 'does not render enforce SSH certificates settings' do
+ render
+
+ expect(rendered).to be_empty
+ end
+ end
+
+ context 'when ssh certificates feature is available' do
+ it 'renders enforce SSH certificates settings' do
+ form = instance_double('Gitlab::FormBuilders::GitlabUiFormBuilder')
+ allow(view).to receive(:can?).and_return(true)
+ allow(view).to receive(:f).and_return(form)
+ allow(group).to receive(:ssh_certificates_available?).and_return(true)
+
+ expect(form).to receive(:gitlab_ui_checkbox_component).with(
+ :enforce_ssh_certificates, "Enforce SSH Certificates", anything
+ )
+
+ render
+
+ expect(rendered).to render_template('groups/settings/_enforce_ssh_certificates')
+ expect(rendered).to have_content('Enforce SSH Certificates')
+ end
+ end
+end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5c827b241365d25d222a1724ba9e69c28623cb5f..3fcd83490f7f59e6172736ae26cb6fb487c26314 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -23266,6 +23266,9 @@ msgstr ""
msgid "GroupSettings|Enabling these features is your acceptance of the %{link_start}GitLab Testing Agreement%{link_end}."
msgstr ""
+msgid "GroupSettings|Enforce SSH Certificates"
+msgstr ""
+
msgid "GroupSettings|Experiment and Beta features"
msgstr ""