diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-30 22:43:18 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-30 22:43:18 +0300 |
commit | 01ab84cac0d67be0e81d9c31216408dffc0ce369 (patch) | |
tree | 439efb51b3818bd985b805fb8e1de9cd915badd1 | |
parent | 4432289851dcfc0bc030323f581866103fd12f66 (diff) |
Add latest changes from gitlab-org/security/gitlab@16-2-stable-ee
18 files changed, 174 insertions, 64 deletions
diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb index 4c2bd2a9d42..278d306301a 100644 --- a/app/controllers/projects/refs_controller.rb +++ b/app/controllers/projects/refs_controller.rb @@ -15,6 +15,8 @@ class Projects::RefsController < Projects::ApplicationController urgency :low, [:switch, :logs_tree] def switch + Gitlab::PathTraversal.check_path_traversal!(@id) + respond_to do |format| format.html do new_path = @@ -40,6 +42,8 @@ class Projects::RefsController < Projects::ApplicationController redirect_to new_path end end + rescue Gitlab::PathTraversal::PathTraversalAttackError + head :bad_request end def logs_tree diff --git a/app/graphql/resolvers/group_issues_resolver.rb b/app/graphql/resolvers/group_issues_resolver.rb index 43f01395896..7bbc662c6c8 100644 --- a/app/graphql/resolvers/group_issues_resolver.rb +++ b/app/graphql/resolvers/group_issues_resolver.rb @@ -9,6 +9,11 @@ module Resolvers include GroupIssuableResolver + before_connection_authorization do |nodes, _| + projects = nodes.map(&:project) + ActiveRecord::Associations::Preloader.new(records: projects, associations: :namespace).call + end + def ready?(**args) if args.dig(:not, :release_tag).present? raise ::Gitlab::Graphql::Errors::ArgumentError, 'releaseTag filter is not allowed when parent is a group.' diff --git a/app/graphql/resolvers/issues_resolver.rb b/app/graphql/resolvers/issues_resolver.rb index 17e3e159a5b..589366ba26d 100644 --- a/app/graphql/resolvers/issues_resolver.rb +++ b/app/graphql/resolvers/issues_resolver.rb @@ -23,6 +23,7 @@ module Resolvers projects = nodes.map(&:project) ::Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects, current_user).execute ::Preloaders::GroupPolicyPreloader.new(projects.filter_map(&:group), current_user).execute + ActiveRecord::Associations::Preloader.new(records: projects, associations: :namespace).call end def ready?(**args) diff --git a/doc/administration/audit_event_streaming/index.md b/doc/administration/audit_event_streaming/index.md index 622d29fa9a7..b92f85dd7ed 100644 --- a/doc/administration/audit_event_streaming/index.md +++ b/doc/administration/audit_event_streaming/index.md @@ -92,7 +92,7 @@ To add Google Cloud Logging streaming destinations to a top-level group: 1. Select **Secure > Audit events**. 1. On the main area, select **Streams** tab. 1. Select **Add streaming destination** and select **Google Cloud Logging** to show the section for adding destinations. -1. Enter the Google Project ID, Google Client Email, Log ID, and Google Private Key to add. +1. Enter the Google project ID, Google client email, log ID, and Google private key to add. 1. Select **Add** to add the new streaming destination. ## List streaming destinations @@ -200,7 +200,8 @@ To update the streaming destinations for an instance: ### Google Cloud Logging streaming -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124384) in GitLab 16.2. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124384) in GitLab 16.2. +> - Button to add private key [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/419675) in GitLab 16.3. Prerequisites: @@ -212,7 +213,8 @@ To update Google Cloud Logging streaming destinations to a top-level group: 1. Select **Secure > Audit events**. 1. On the main area, select **Streams** tab. 1. Select the Google Cloud Logging stream to expand. -1. Enter the Google Project ID, Google Client Email, Log ID, and Google Private Key to update. +1. Enter the Google project ID, Google client email, and log ID to update. +1. Select **Add a new private key** and enter a Google private key to update the private key. 1. Select **Save** to update the streaming destination. ## Delete streaming destinations diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 425a2b7e980..67075efbfb9 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -15920,7 +15920,6 @@ Stores Google Cloud Logging configurations associated with IAM service accounts, | <a id="googlecloudloggingconfigurationtypegroup"></a>`group` | [`Group!`](#group) | Group the configuration belongs to. | | <a id="googlecloudloggingconfigurationtypeid"></a>`id` | [`ID!`](#id) | ID of the configuration. | | <a id="googlecloudloggingconfigurationtypelogidname"></a>`logIdName` | [`String!`](#string) | Log ID. | -| <a id="googlecloudloggingconfigurationtypeprivatekey"></a>`privateKey` | [`String!`](#string) | Private key. | ### `GpgSignature` diff --git a/lib/api/ml/mlflow/api_helpers.rb b/lib/api/ml/mlflow/api_helpers.rb index 7f4a895235c..cfe4cc6b5ae 100644 --- a/lib/api/ml/mlflow/api_helpers.rb +++ b/lib/api/ml/mlflow/api_helpers.rb @@ -4,6 +4,14 @@ module API module Ml module Mlflow module ApiHelpers + def check_api_read! + not_found! unless can?(current_user, :read_model_experiments, user_project) + end + + def check_api_write! + unauthorized! unless can?(current_user, :write_model_experiments, user_project) + end + def resource_not_found! render_structured_api_error!({ error_code: 'RESOURCE_DOES_NOT_EXIST' }, 404) end diff --git a/lib/api/ml/mlflow/entrypoint.rb b/lib/api/ml/mlflow/entrypoint.rb index 048234eccd1..d8474946a75 100644 --- a/lib/api/ml/mlflow/entrypoint.rb +++ b/lib/api/ml/mlflow/entrypoint.rb @@ -26,7 +26,8 @@ module API authenticate! - not_found! unless can?(current_user, :read_model_experiments, user_project) + check_api_read! + check_api_write! unless request.get? || request.head? end rescue_from ActiveRecord::ActiveRecordError do |e| diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 468f284f136..7198917a097 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -689,6 +689,7 @@ module API desc 'Mark this project as forked from another' do success code: 201, model: Entities::Project failure [ + { code: 401, message: 'Unauthorized' }, { code: 403, message: 'Unauthenticated' }, { code: 404, message: 'Not found' } ] @@ -706,7 +707,11 @@ module API authorize! :fork_project, fork_from_project - result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project) + service = ::Projects::ForkService.new(fork_from_project, current_user) + + unauthorized!('Target Namespace') unless service.valid_fork_target?(user_project.namespace) + + result = service.execute(user_project) if result present_project user_project.reset, with: Entities::Project, current_user: current_user diff --git a/lib/gitlab/pagination/gitaly_keyset_pager.rb b/lib/gitlab/pagination/gitaly_keyset_pager.rb index 6235874132f..82d6fc64d89 100644 --- a/lib/gitlab/pagination/gitaly_keyset_pager.rb +++ b/lib/gitlab/pagination/gitaly_keyset_pager.rb @@ -15,7 +15,7 @@ module Gitlab # It is expected that the given finder will respond to `execute` method with `gitaly_pagination:` option # and supports pagination via gitaly. def paginate(finder) - return finder.execute(gitaly_pagination: false) if no_pagination? + return finder.execute(gitaly_pagination: false) if no_pagination?(finder) return paginate_via_gitaly(finder) if keyset_pagination_enabled?(finder) return paginate_first_page_via_gitaly(finder) if paginate_first_page?(finder) @@ -28,8 +28,8 @@ module Gitlab private - def no_pagination? - params[:pagination] == 'none' + def no_pagination?(finder) + params[:pagination] == 'none' && finder.is_a?(::Repositories::TreeFinder) end def keyset_pagination_enabled?(finder) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 50694b8fc5c..8b07d6daa3e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6575,6 +6575,9 @@ msgstr "" msgid "AuditStreams|Active" msgstr "" +msgid "AuditStreams|Add a new private key" +msgstr "" + msgid "AuditStreams|Add an HTTP endpoint to manage audit logs in third-party systems." msgstr "" @@ -6689,6 +6692,9 @@ msgstr "" msgid "AuditStreams|This is great for keeping everything one place." msgstr "" +msgid "AuditStreams|Use the Google Cloud console to view the private key. To change the private key, replace it with a new private key." +msgstr "" + msgid "AuditStreams|Value" msgstr "" diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index 0b1d0b75de7..7ea0e678a41 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -12,40 +12,70 @@ RSpec.describe Projects::RefsController, feature_category: :source_code_manageme end describe 'GET #switch' do - using RSpec::Parameterized::TableSyntax + context 'with normal parameters' do + using RSpec::Parameterized::TableSyntax - let(:id) { 'master' } - let(:params) do - { destination: destination, namespace_id: project.namespace.to_param, project_id: project, id: id, - ref_type: ref_type } - end + let(:id) { 'master' } + let(:id_and_path) { "#{id}/#{path}" } + + let(:params) do + { destination: destination, namespace_id: project.namespace.to_param, project_id: project, id: id, + ref_type: ref_type, path: path } + end + + subject { get :switch, params: params } + + where(:destination, :ref_type, :path, :redirected_to) do + 'tree' | nil | nil | lazy { project_tree_path(project, id) } + 'tree' | 'heads' | nil | lazy { project_tree_path(project, id) } + 'tree' | nil | 'foo/bar' | lazy { project_tree_path(project, id_and_path) } + 'blob' | nil | nil | lazy { project_blob_path(project, id) } + 'blob' | 'heads' | nil | lazy { project_blob_path(project, id) } + 'blob' | nil | 'foo/bar' | lazy { project_blob_path(project, id_and_path) } + 'graph' | nil | nil | lazy { project_network_path(project, id) } + 'graph' | 'heads' | nil | lazy { project_network_path(project, id, ref_type: 'heads') } + 'graph' | nil | 'foo/bar' | lazy { project_network_path(project, id_and_path) } + 'graphs' | nil | nil | lazy { project_graph_path(project, id) } + 'graphs' | 'heads' | nil | lazy { project_graph_path(project, id, ref_type: 'heads') } + 'graphs' | nil | 'foo/bar' | lazy { project_graph_path(project, id_and_path) } + 'find_file' | nil | nil | lazy { project_find_file_path(project, id) } + 'find_file' | 'heads' | nil | lazy { project_find_file_path(project, id) } + 'find_file' | nil | 'foo/bar' | lazy { project_find_file_path(project, id_and_path) } + 'graphs_commits' | nil | nil | lazy { commits_project_graph_path(project, id) } + 'graphs_commits' | 'heads' | nil | lazy { commits_project_graph_path(project, id) } + 'graphs_commits' | nil | 'foo/bar' | lazy { commits_project_graph_path(project, id_and_path) } + 'badges' | nil | nil | lazy { project_settings_ci_cd_path(project, ref: id) } + 'badges' | 'heads' | nil | lazy { project_settings_ci_cd_path(project, ref: id) } + 'badges' | nil | 'foo/bar' | lazy { project_settings_ci_cd_path(project, ref: id_and_path) } + 'commits' | nil | nil | lazy { project_commits_path(project, id) } + 'commits' | 'heads' | nil | lazy { project_commits_path(project, id, ref_type: 'heads') } + 'commits' | nil | 'foo/bar' | lazy { project_commits_path(project, id_and_path) } + nil | nil | nil | lazy { project_commits_path(project, id) } + nil | 'heads' | nil | lazy { project_commits_path(project, id, ref_type: 'heads') } + nil | nil | 'foo/bar' | lazy { project_commits_path(project, id_and_path) } + end - subject { get :switch, params: params } - - where(:destination, :ref_type, :redirected_to) do - 'tree' | nil | lazy { project_tree_path(project, id) } - 'tree' | 'heads' | lazy { project_tree_path(project, id) } - 'blob' | nil | lazy { project_blob_path(project, id) } - 'blob' | 'heads' | lazy { project_blob_path(project, id) } - 'graph' | nil | lazy { project_network_path(project, id) } - 'graph' | 'heads' | lazy { project_network_path(project, id, ref_type: 'heads') } - 'graphs' | nil | lazy { project_graph_path(project, id) } - 'graphs' | 'heads' | lazy { project_graph_path(project, id, ref_type: 'heads') } - 'find_file' | nil | lazy { project_find_file_path(project, id) } - 'find_file' | 'heads' | lazy { project_find_file_path(project, id) } - 'graphs_commits' | nil | lazy { commits_project_graph_path(project, id) } - 'graphs_commits' | 'heads' | lazy { commits_project_graph_path(project, id) } - 'badges' | nil | lazy { project_settings_ci_cd_path(project, ref: id) } - 'badges' | 'heads' | lazy { project_settings_ci_cd_path(project, ref: id) } - 'commits' | nil | lazy { project_commits_path(project, id) } - 'commits' | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') } - nil | nil | lazy { project_commits_path(project, id) } - nil | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') } + with_them do + it 'redirects to destination' do + expect(subject).to redirect_to(redirected_to) + end + end end - with_them do - it 'redirects to destination' do - expect(subject).to redirect_to(redirected_to) + context 'with bad path parameter' do + it 'returns 400 bad request' do + params = { + destination: 'tree', + namespace_id: project.namespace.to_param, + project_id: project, + id: 'master', + ref_type: nil, + path: '../bad_path_redirect' + } + + get :switch, params: params + + expect(response).to have_gitlab_http_status(:bad_request) end end end diff --git a/spec/helpers/nav/new_dropdown_helper_spec.rb b/spec/helpers/nav/new_dropdown_helper_spec.rb index 26dadd3b4f1..4ec120d152b 100644 --- a/spec/helpers/nav/new_dropdown_helper_spec.rb +++ b/spec/helpers/nav/new_dropdown_helper_spec.rb @@ -22,6 +22,7 @@ RSpec.describe Nav::NewDropdownHelper, feature_category: :navigation do allow(helper).to receive(:can?).and_return(false) allow(user).to receive(:can_create_project?) { with_can_create_project } allow(user).to receive(:can_create_group?) { with_can_create_group } + allow(user).to receive(:can?).and_call_original allow(user).to receive(:can?).with(:create_snippet) { with_can_create_snippet } end diff --git a/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb b/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb index 2e87c582040..cb3f1fe86dc 100644 --- a/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb +++ b/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb @@ -127,17 +127,27 @@ RSpec.describe Gitlab::Pagination::GitalyKeysetPager do end end - context 'with "none" pagination option' do + context "with 'none' pagination option" do let(:expected_result) { double(:result) } let(:query) { { pagination: 'none' } } - it 'uses offset pagination' do - expect(finder).to receive(:execute).with(gitaly_pagination: false).and_return(expected_result) - expect(Kaminari).not_to receive(:paginate_array) - expect(Gitlab::Pagination::OffsetPagination).not_to receive(:new) + context "with a finder that is not a TreeFinder" do + it_behaves_like 'offset pagination' + end + + context "with a finder that is a TreeFinder" do + before do + allow(finder).to receive(:is_a?).with(::Repositories::TreeFinder).and_return(true) + end - actual_result = pager.paginate(finder) - expect(actual_result).to eq(expected_result) + it "doesn't uses offset pagination" do + expect(finder).to receive(:execute).with(gitaly_pagination: false).and_return(expected_result) + expect(Kaminari).not_to receive(:paginate_array) + expect(Gitlab::Pagination::OffsetPagination).not_to receive(:new) + + actual_result = pager.paginate(finder) + expect(actual_result).to eq(expected_result) + end end end end diff --git a/spec/requests/api/ml/mlflow/experiments_spec.rb b/spec/requests/api/ml/mlflow/experiments_spec.rb index fc2e814752c..409b4529699 100644 --- a/spec/requests/api/ml/mlflow/experiments_spec.rb +++ b/spec/requests/api/ml/mlflow/experiments_spec.rb @@ -179,7 +179,7 @@ RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' end end @@ -203,7 +203,7 @@ RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] end end diff --git a/spec/requests/api/ml/mlflow/runs_spec.rb b/spec/requests/api/ml/mlflow/runs_spec.rb index a85fe4d867a..88fd1bada1c 100644 --- a/spec/requests/api/ml/mlflow/runs_spec.rb +++ b/spec/requests/api/ml/mlflow/runs_spec.rb @@ -124,7 +124,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' end end @@ -204,7 +204,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' end end @@ -222,7 +222,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do describe 'Error Cases' do it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value, :timestamp] end @@ -247,7 +247,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] end @@ -272,7 +272,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] end @@ -342,7 +342,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index f5d1bbbc7e8..ef0959689e3 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3270,16 +3270,41 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and project_fork_target.add_maintainer(user) end - it 'allows project to be forked from an existing project' do - expect(project_fork_target).not_to be_forked + context 'and user is a reporter of target group' do + let_it_be_with_reload(:target_group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) } + let_it_be_with_reload(:project_fork_target) { create(:project, namespace: target_group) } - post api(path, user) - project_fork_target.reload + before do + target_group.add_reporter(user) + end - expect(response).to have_gitlab_http_status(:created) - expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id) - expect(project_fork_target.fork_network_member).to be_present - expect(project_fork_target).to be_forked + it 'fails as target namespace is unauthorized' do + post api(path, user) + + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['message']).to eq "401 Unauthorized - Target Namespace" + end + end + + context 'and user is a developer of target group' do + let_it_be_with_reload(:target_group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) } + let_it_be_with_reload(:project_fork_target) { create(:project, namespace: target_group) } + + before do + target_group.add_developer(user) + end + + it 'allows project to be forked from an existing project' do + expect(project_fork_target).not_to be_forked + + post api(path, user) + project_fork_target.reload + + expect(response).to have_gitlab_http_status(:created) + expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id) + expect(project_fork_target.fork_network_member).to be_present + expect(project_fork_target).to be_forked + end end it 'fails without permission from forked_from project' do diff --git a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb index cbd0ffbab21..f2052f4202d 100644 --- a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb +++ b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb @@ -254,7 +254,7 @@ RSpec.shared_examples 'a redacted search results' do end context 'with :with_api_entity_associations' do - it_behaves_like "redaction limits N+1 queries", limit: 14 + it_behaves_like "redaction limits N+1 queries", limit: 15 end end diff --git a/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb index f2c38d70508..00e50b07909 100644 --- a/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb @@ -8,12 +8,25 @@ RSpec.shared_examples 'MLflow|Not Found - Resource Does Not Exist' do end end -RSpec.shared_examples 'MLflow|Requires api scope' do +RSpec.shared_examples 'MLflow|Requires api scope and write permission' do context 'when user has access but token has wrong scope' do let(:access_token) { tokens[:read] } it { is_expected.to have_gitlab_http_status(:forbidden) } end + + context 'when user has access but is not allowed to write' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?) + .with(current_user, :write_model_experiments, project) + .and_return(false) + end + + it "is Unauthorized" do + is_expected.to have_gitlab_http_status(:unauthorized) + end + end end RSpec.shared_examples 'MLflow|Requires read_api scope' do |