From 99a78b874f73bae4535d2b3221b12b39ab5ce885 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Wed, 7 Sep 2022 11:17:45 -0500 Subject: [PATCH] Disable creation of all types of personal tokens with FIPS enabled When FIPS is enabled, disable the creation of all types of personal access tokens including user PATs, group access tokens, project access tokens, and impersonation tokens. Changelog: added EE: true --- .../personal_access_tokens_controller.rb | 6 ++++ app/policies/project_policy.rb | 2 +- .../layouts/nav/sidebar/_profile.html.haml | 23 +++++++------- ee/app/helpers/ee/users_helper.rb | 5 ++++ ee/app/policies/ee/group_policy.rb | 1 + ee/app/policies/ee/project_policy.rb | 1 + ee/app/policies/ee/user_policy.rb | 7 +++++ ee/spec/helpers/users_helper_spec.rb | 28 +++++++++++++++++ ee/spec/policies/group_policy_spec.rb | 2 ++ ee/spec/policies/project_policy_spec.rb | 2 ++ ee/spec/policies/user_policy_spec.rb | 12 ++++++++ .../impersonation_tokens_controller_spec.rb | 30 +++++++++++++++++++ .../resource_access_token_shared_examples.rb | 19 ++++++++++++ .../personal_access_tokens_controller_spec.rb | 16 ++++++++++ .../impersonation_tokens_controller_spec.rb | 12 ++++++++ .../nav/sidebar/_profile.html.haml_spec.rb | 16 ++++++++++ 16 files changed, 170 insertions(+), 12 deletions(-) create mode 100644 ee/spec/requests/admin/impersonation_tokens_controller_spec.rb create mode 100644 ee/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb diff --git a/app/controllers/profiles/personal_access_tokens_controller.rb b/app/controllers/profiles/personal_access_tokens_controller.rb index 8ed67c26f1943d..4cf26d3e1e2f7b 100644 --- a/app/controllers/profiles/personal_access_tokens_controller.rb +++ b/app/controllers/profiles/personal_access_tokens_controller.rb @@ -3,6 +3,8 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController feature_category :authentication_and_authorization + before_action :check_personal_access_tokens_enabled + def index set_index_vars scopes = params[:scopes].split(',').map(&:squish).select(&:present?).map(&:to_sym) unless params[:scopes].nil? @@ -83,4 +85,8 @@ def add_pagination_headers(relation) def page (params[:page] || 1).to_i end + + def check_personal_access_tokens_enabled + render_404 if Gitlab::CurrentSettings.personal_access_tokens_disabled? + end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 4f15b5f6ded26e..63c1fc9f3ddaf0 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -794,7 +794,7 @@ class ProjectPolicy < BasePolicy rule { project_bot }.enable :project_bot_access - rule { can?(:read_all_resources) }.enable :read_resource_access_tokens + rule { can?(:read_all_resources) & resource_access_token_feature_available }.enable :read_resource_access_tokens rule { can?(:admin_project) & resource_access_token_feature_available }.policy do enable :read_resource_access_tokens diff --git a/app/views/layouts/nav/sidebar/_profile.html.haml b/app/views/layouts/nav/sidebar/_profile.html.haml index a1393615e691b1..0e3327935ca305 100644 --- a/app/views/layouts/nav/sidebar/_profile.html.haml +++ b/app/views/layouts/nav/sidebar/_profile.html.haml @@ -51,17 +51,18 @@ = link_to profile_chat_names_path do %strong.fly-out-top-item-name = _('Chat') - = nav_link(controller: :personal_access_tokens) do - = link_to profile_personal_access_tokens_path do - .nav-icon-container - = sprite_icon('token') - %span.nav-item-name - = _('Access Tokens') - %ul.sidebar-sub-level-items.is-fly-out-only - = nav_link(controller: :personal_access_tokens, html_options: { class: "fly-out-top-item" } ) do - = link_to profile_personal_access_tokens_path do - %strong.fly-out-top-item-name - = _('Access Tokens') + - unless Gitlab::CurrentSettings.personal_access_tokens_disabled? + = nav_link(controller: :personal_access_tokens) do + = link_to profile_personal_access_tokens_path do + .nav-icon-container + = sprite_icon('token') + %span.nav-item-name + = _('Access Tokens') + %ul.sidebar-sub-level-items.is-fly-out-only + = nav_link(controller: :personal_access_tokens, html_options: { class: "fly-out-top-item" } ) do + = link_to profile_personal_access_tokens_path do + %strong.fly-out-top-item-name + = _('Access Tokens') = nav_link(controller: :emails) do = link_to profile_emails_path, data: { qa_selector: 'profile_emails_link' } do .nav-icon-container diff --git a/ee/app/helpers/ee/users_helper.rb b/ee/app/helpers/ee/users_helper.rb index 6ba11bdd339836..664bc5a73451b9 100644 --- a/ee/app/helpers/ee/users_helper.rb +++ b/ee/app/helpers/ee/users_helper.rb @@ -12,6 +12,11 @@ def display_public_email?(user) !::Feature.enabled?(:hide_public_email_on_profile, user.provisioned_by_group) end + override :impersonation_enabled? + def impersonation_enabled? + super && !::Gitlab::CurrentSettings.personal_access_tokens_disabled? + end + def users_sentence(users, link_class: nil) users.map { |user| link_to(user.name, user, class: link_class) }.to_sentence.html_safe end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index f78f895008f388..7e665303640a50 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -543,6 +543,7 @@ def sso_session_prevents_access? # 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? + return false if ::Gitlab::CurrentSettings.personal_access_tokens_disabled? return super unless ::Gitlab.com? group.feature_available_non_trial?(:resource_access_token) diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 5607224348290b..55862aea7cf7e4 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -496,6 +496,7 @@ def lookup_access_level! # 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? + return false if ::Gitlab::CurrentSettings.personal_access_tokens_disabled? return super unless ::Gitlab.com? namespace = project.namespace diff --git a/ee/app/policies/ee/user_policy.rb b/ee/app/policies/ee/user_policy.rb index 3fba0fc4ef1134..19225fbc2e940c 100644 --- a/ee/app/policies/ee/user_policy.rb +++ b/ee/app/policies/ee/user_policy.rb @@ -14,11 +14,18 @@ module UserPolicy @subject.can_remove_self? end + desc "Personal access tokens are disabled" + condition(:personal_access_tokens_disabled, scope: :global, score: 0) do + ::Gitlab::CurrentSettings.personal_access_tokens_disabled? + end + rule { can?(:update_user) }.enable :update_name rule { updating_name_disabled_for_users & ~admin }.prevent :update_name rule { user_is_self & ~can_remove_self }.prevent :destroy_user + + rule { personal_access_tokens_disabled }.prevent :create_user_personal_access_token end end end diff --git a/ee/spec/helpers/users_helper_spec.rb b/ee/spec/helpers/users_helper_spec.rb index d35a213bfd043b..6f08c3400877d7 100644 --- a/ee/spec/helpers/users_helper_spec.rb +++ b/ee/spec/helpers/users_helper_spec.rb @@ -143,4 +143,32 @@ it { is_expected.to be false } end end + + describe '#impersonation_enabled?' do + subject { helper.impersonation_enabled? } + + context 'when impersonation is enabled' do + before do + stub_config_setting(impersonation_enabled: true) + end + + it { is_expected.to eq(true) } + + context 'when personal access tokens are disabled' do + before do + stub_ee_application_setting(personal_access_tokens_disabled?: true) + end + + it { is_expected.to eq(false) } + end + end + + context 'when impersonation is disabled' do + before do + stub_config_setting(impersonation_enabled: false) + end + + it { is_expected.to eq(false) } + end + end end diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index f1e116c38c1bbf..4aeb75a7599461 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -1623,6 +1623,8 @@ def set_access_level(access_level) group.add_owner(owner) end + it_behaves_like 'GitLab.com Paid plan resource access tokens' + context 'create resource access tokens' do it { is_expected.to be_allowed(:create_resource_access_tokens) } diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 5c1c900c05e79a..bcbc2d4ee6b07f 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -1830,6 +1830,8 @@ project.add_maintainer(maintainer) end + it_behaves_like 'GitLab.com Paid plan resource access tokens' + context 'create resource access tokens' do it { is_expected.to be_allowed(:create_resource_access_tokens) } diff --git a/ee/spec/policies/user_policy_spec.rb b/ee/spec/policies/user_policy_spec.rb index bb7307627ae2ea..617c97170fab54 100644 --- a/ee/spec/policies/user_policy_spec.rb +++ b/ee/spec/policies/user_policy_spec.rb @@ -138,4 +138,16 @@ def policy end end end + + describe ':create_user_personal_access_token' do + subject { described_class.new(current_user, current_user) } + + context 'when personal access tokens are disabled' do + before do + stub_ee_application_setting(personal_access_tokens_disabled?: true) + end + + it { is_expected.to be_disallowed(:create_user_personal_access_token) } + end + end end diff --git a/ee/spec/requests/admin/impersonation_tokens_controller_spec.rb b/ee/spec/requests/admin/impersonation_tokens_controller_spec.rb new file mode 100644 index 00000000000000..b84e0d296c4d8a --- /dev/null +++ b/ee/spec/requests/admin/impersonation_tokens_controller_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::ImpersonationTokensController, :enable_admin_mode do + let(:admin) { create(:admin) } + let!(:user) { create(:user) } + + before do + sign_in(admin) + end + + context 'when impersonation is enabled' do + before do + stub_config_setting(impersonation_enabled: true) + end + + context 'when personal access tokens are disabled' do + before do + stub_ee_application_setting(personal_access_tokens_disabled?: true) + end + + it 'responds with a 404' do + get admin_user_impersonation_tokens_path(user_id: user.username) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/ee/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb b/ee/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb new file mode 100644 index 00000000000000..d2a538cd2d45b7 --- /dev/null +++ b/ee/spec/support/shared_examples/policies/resource_access_token_shared_examples.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'GitLab.com Paid plan resource access tokens' do + context 'on SaaS', :saas do + it { is_expected.to be_allowed(:create_resource_access_tokens) } + it { is_expected.to be_allowed(:read_resource_access_tokens) } + it { is_expected.to be_allowed(:destroy_resource_access_tokens) } + + context 'when personal access tokens are disabled' do + before do + stub_ee_application_setting(personal_access_tokens_disabled?: true) + end + + it { is_expected.not_to be_allowed(:create_resource_access_tokens) } + it { is_expected.not_to be_allowed(:read_resource_access_tokens) } + it { is_expected.not_to be_allowed(:destroy_resource_access_tokens) } + end + end +end diff --git a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb index 160af8cf3f0b88..8dee0490fd6210 100644 --- a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb @@ -36,6 +36,14 @@ def created_token expect(created_token.expires_at).to eq(expires_at) end + it 'does not allow creation when personal access tokens are disabled' do + allow(::Gitlab::CurrentSettings).to receive_messages(personal_access_tokens_disabled?: true) + + post :create, params: { personal_access_token: token_attributes } + + expect(response).to have_gitlab_http_status(:not_found) + end + it_behaves_like "#create access token" do let(:url) { :create } end @@ -70,6 +78,14 @@ def created_token ) end + it 'returns 404 when personal access tokens are disabled' do + allow(::Gitlab::CurrentSettings).to receive_messages(personal_access_tokens_disabled?: true) + + get :index + + expect(response).to have_gitlab_http_status(:not_found) + end + context "access_token_pagination feature flag is enabled" do before do stub_feature_flags(access_token_pagination: true) diff --git a/spec/requests/admin/impersonation_tokens_controller_spec.rb b/spec/requests/admin/impersonation_tokens_controller_spec.rb index 2017a512bced91..ee0e12ad0c072e 100644 --- a/spec/requests/admin/impersonation_tokens_controller_spec.rb +++ b/spec/requests/admin/impersonation_tokens_controller_spec.rb @@ -10,6 +10,18 @@ sign_in(admin) end + context 'when impersonation is enabled' do + before do + stub_config_setting(impersonation_enabled: true) + end + + it 'responds ok' do + get admin_user_impersonation_tokens_path(user_id: user.username) + + expect(response).to have_gitlab_http_status(:ok) + end + end + context "when impersonation is disabled" do before do stub_config_setting(impersonation_enabled: false) diff --git a/spec/views/layouts/nav/sidebar/_profile.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_profile.html.haml_spec.rb index 3d28be68b25fb0..f5a0a7a935c12d 100644 --- a/spec/views/layouts/nav/sidebar/_profile.html.haml_spec.rb +++ b/spec/views/layouts/nav/sidebar/_profile.html.haml_spec.rb @@ -11,4 +11,20 @@ it_behaves_like 'has nav sidebar' it_behaves_like 'sidebar includes snowplow attributes', 'render', 'user_side_navigation', 'user_side_navigation' + + it 'has a link to access tokens' do + render + + expect(rendered).to have_link(_('Access Tokens'), href: profile_personal_access_tokens_path) + end + + context 'when personal access tokens are disabled' do + it 'does not have a link to access tokens' do + allow(::Gitlab::CurrentSettings).to receive_messages(personal_access_tokens_disabled?: true) + + render + + expect(rendered).not_to have_link(_('Access Tokens'), href: profile_personal_access_tokens_path) + end + end end -- GitLab