diff options
9 files changed, 36 insertions, 24 deletions
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 6f3c96fa654..be1e932a1ab 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -185,7 +185,7 @@ class Projects::BranchesController < Projects::ApplicationController # Here we get one more branch to indicate if there are more data we're not showing limit = @overview_max_branches + 1 - if Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: true) + if Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: :yaml) @active_branches = BranchesFinder.new(@repository, { per_page: limit, sort: sort_value_recently_updated }) .execute(gitaly_pagination: true).select(&:active?) diff --git a/app/services/auth/dependency_proxy_authentication_service.rb b/app/services/auth/dependency_proxy_authentication_service.rb index 1b8c16b7c79..fab42e0ebb6 100644 --- a/app/services/auth/dependency_proxy_authentication_service.rb +++ b/app/services/auth/dependency_proxy_authentication_service.rb @@ -8,7 +8,10 @@ module Auth def execute(authentication_abilities:) return error('dependency proxy not enabled', 404) unless ::Gitlab.config.dependency_proxy.enabled - return error('access forbidden', 403) unless current_user + + # Because app/controllers/concerns/dependency_proxy/auth.rb consumes this + # JWT only as `User.find`, we currently only allow User (not DeployToken, etc) + return error('access forbidden', 403) unless current_user.is_a?(User) { token: authorized_token.encoded } end diff --git a/app/services/projects/branches_by_mode_service.rb b/app/services/projects/branches_by_mode_service.rb index fb66bfa073b..22a09a56cd0 100644 --- a/app/services/projects/branches_by_mode_service.rb +++ b/app/services/projects/branches_by_mode_service.rb @@ -37,7 +37,7 @@ class Projects::BranchesByModeService def use_gitaly_pagination? return false if params[:page].present? || params[:search].present? - Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: true) + Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: :yaml) end def fetch_branches_via_offset_pagination diff --git a/changelogs/unreleased/security-10io-dependency-proxy-and-deploy-tokens.yml b/changelogs/unreleased/security-10io-dependency-proxy-and-deploy-tokens.yml new file mode 100644 index 00000000000..26137a42420 --- /dev/null +++ b/changelogs/unreleased/security-10io-dependency-proxy-and-deploy-tokens.yml @@ -0,0 +1,5 @@ +--- +title: Do not allow deploy tokens in the dependency proxy authentication service +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-322500-disable-gitaly-branch-pagination-ff-by-default.yml b/changelogs/unreleased/security-322500-disable-gitaly-branch-pagination-ff-by-default.yml new file mode 100644 index 00000000000..869cd7ab57c --- /dev/null +++ b/changelogs/unreleased/security-322500-disable-gitaly-branch-pagination-ff-by-default.yml @@ -0,0 +1,5 @@ +--- +title: Disable keyset pagination for branches by default +merge_request: +author: +type: security diff --git a/config/feature_flags/development/branch_list_keyset_pagination.yml b/config/feature_flags/development/branch_list_keyset_pagination.yml index 23b573e5004..12200292058 100644 --- a/config/feature_flags/development/branch_list_keyset_pagination.yml +++ b/config/feature_flags/development/branch_list_keyset_pagination.yml @@ -5,4 +5,4 @@ rollout_issue_url: milestone: '13.2' type: development group: group::source code -default_enabled: true +default_enabled: false diff --git a/lib/gitlab/pagination/gitaly_keyset_pager.rb b/lib/gitlab/pagination/gitaly_keyset_pager.rb index 1350168967e..b05891066ac 100644 --- a/lib/gitlab/pagination/gitaly_keyset_pager.rb +++ b/lib/gitlab/pagination/gitaly_keyset_pager.rb @@ -26,11 +26,11 @@ module Gitlab private def keyset_pagination_enabled? - Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: true) && params[:pagination] == 'keyset' + Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: :yaml) && params[:pagination] == 'keyset' end def paginate_first_page? - Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: true) && (params[:page].blank? || params[:page].to_i == 1) + Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: :yaml) && (params[:page].blank? || params[:page].to_i == 1) end def paginate_via_gitaly(finder) diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index e154e691d5f..3087e0e1abb 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -262,25 +262,21 @@ RSpec.describe JwtController do let(:credential_user) { group_deploy_token.username } let(:credential_password) { group_deploy_token.token } - it_behaves_like 'with valid credentials' + it_behaves_like 'returning response status', :forbidden end context 'with project deploy token' do let(:credential_user) { project_deploy_token.username } let(:credential_password) { project_deploy_token.token } - it_behaves_like 'with valid credentials' + it_behaves_like 'returning response status', :forbidden end context 'with invalid credentials' do let(:credential_user) { 'foo' } let(:credential_password) { 'bar' } - it 'returns unauthorized' do - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end + it_behaves_like 'returning response status', :unauthorized end end diff --git a/spec/services/auth/dependency_proxy_authentication_service_spec.rb b/spec/services/auth/dependency_proxy_authentication_service_spec.rb index ba50149f53a..1fd1677c7da 100644 --- a/spec/services/auth/dependency_proxy_authentication_service_spec.rb +++ b/spec/services/auth/dependency_proxy_authentication_service_spec.rb @@ -13,28 +13,31 @@ RSpec.describe Auth::DependencyProxyAuthenticationService do describe '#execute' do subject { service.execute(authentication_abilities: nil) } + shared_examples 'returning' do |status:, message:| + it "returns #{message}", :aggregate_failures do + expect(subject[:http_status]).to eq(status) + expect(subject[:message]).to eq(message) + end + end + context 'dependency proxy is not enabled' do before do stub_config(dependency_proxy: { enabled: false }) end - it 'returns not found' do - result = subject - - expect(result[:http_status]).to eq(404) - expect(result[:message]).to eq('dependency proxy not enabled') - end + it_behaves_like 'returning', status: 404, message: 'dependency proxy not enabled' end context 'without a user' do let(:user) { nil } - it 'returns forbidden' do - result = subject + it_behaves_like 'returning', status: 403, message: 'access forbidden' + end + + context 'with a deploy token as user' do + let_it_be(:user) { create(:deploy_token) } - expect(result[:http_status]).to eq(403) - expect(result[:message]).to eq('access forbidden') - end + it_behaves_like 'returning', status: 403, message: 'access forbidden' end context 'with a user' do |