diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-27 22:06:03 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-27 22:06:03 +0300 |
commit | 9f8db4b0e551fe6c658b89c98db8e408e709aad6 (patch) | |
tree | 6e0f85e4ff21e8dd107187ce885beeb46bb912bc | |
parent | 42c5548470596a37ed1a071e3bb72af2c9c35c0e (diff) |
Add latest changes from gitlab-org/security/gitlab@15-0-stable-ee
-rw-r--r-- | GITLAB_PAGES_VERSION | 2 | ||||
-rw-r--r-- | app/controllers/autocomplete_controller.rb | 5 | ||||
-rw-r--r-- | app/models/hooks/web_hook_log.rb | 7 | ||||
-rw-r--r-- | app/serializers/build_details_entity.rb | 2 | ||||
-rw-r--r-- | spec/controllers/autocomplete_controller_spec.rb | 73 | ||||
-rw-r--r-- | spec/models/hooks/web_hook_log_spec.rb | 35 | ||||
-rw-r--r-- | spec/serializers/build_details_entity_spec.rb | 18 |
7 files changed, 40 insertions, 102 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 69478d187bd..79f82f6b8e0 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.58.1 +1.58.0 diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 22d7ccbd069..f84d2ed320d 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -5,7 +5,6 @@ class AutocompleteController < ApplicationController skip_before_action :authenticate_user!, only: [:users, :award_emojis, :merge_request_target_branches] before_action :check_search_rate_limit!, only: [:users, :projects] - before_action :authorize_admin_project, only: :deploy_keys_with_owners feature_category :users, [:users, :user] feature_category :projects, [:projects] @@ -68,10 +67,6 @@ class AutocompleteController < ApplicationController private - def authorize_admin_project - render_403 unless Ability.allowed?(current_user, :admin_project, project) - end - def project @project ||= Autocomplete::ProjectFinder .new(current_user, params) diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb index 04d6d1ebd5c..8c0565e4a38 100644 --- a/app/models/hooks/web_hook_log.rb +++ b/app/models/hooks/web_hook_log.rb @@ -20,7 +20,6 @@ class WebHookLog < ApplicationRecord validates :web_hook, presence: true before_save :obfuscate_basic_auth - before_save :redact_author_email def self.recent where('created_at >= ?', 2.days.ago.beginning_of_day) @@ -40,10 +39,4 @@ class WebHookLog < ApplicationRecord def obfuscate_basic_auth self.url = safe_url end - - def redact_author_email - return unless self.request_data.dig('commit', 'author', 'email').present? - - self.request_data['commit']['author']['email'] = _('[REDACTED]') - end end diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index dc7b5e95361..5f72259f34a 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -151,7 +151,7 @@ class BuildDetailsEntity < Ci::JobEntity # We do not return the invalid_dependencies for all scenarios see https://gitlab.com/gitlab-org/gitlab/-/issues/287772#note_914406387 punctuation = invalid_dependencies.empty? ? '.' : ': ' _("This job could not start because it could not retrieve the needed artifacts%{punctuation}%{invalid_dependencies}") % - { invalid_dependencies: html_escape(invalid_dependencies), punctuation: punctuation } + { invalid_dependencies: invalid_dependencies, punctuation: punctuation } end def help_message(docs_url) diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 1df685e3e5a..0a809e80fcd 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -378,72 +378,61 @@ RSpec.describe AutocompleteController do end context 'GET deploy_keys_with_owners' do - let_it_be(:public_project) { create(:project, :public) } - let_it_be(:user) { create(:user) } - let_it_be(:deploy_key) { create(:deploy_key, user: user) } - let_it_be(:deploy_keys_project) do - create(:deploy_keys_project, :write_access, project: public_project, deploy_key: deploy_key) - end + let!(:deploy_key) { create(:deploy_key, user: user) } + let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) } context 'unauthorized user' do it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: public_project.id }) + get(:deploy_keys_with_owners, params: { project_id: project.id }) expect(response).to have_gitlab_http_status(:redirect) end end - context 'when the user is logged in' do + context 'when the user who can read the project is logged in' do before do sign_in(user) end - context 'with a non-existing project' do + context 'and they cannot read the project' do it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: 9999 }) + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false) + + get(:deploy_keys_with_owners, params: { project_id: project.id }) expect(response).to have_gitlab_http_status(:not_found) end end - context 'with an existing project' do - context 'when user cannot admin project' do - it 'returns a forbidden response' do - get(:deploy_keys_with_owners, params: { project_id: public_project.id }) + it 'renders the deploy key in a json payload, with its owner' do + get(:deploy_keys_with_owners, params: { project_id: project.id }) - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user can admin project' do - before do - public_project.add_maintainer(user) - end + expect(json_response.count).to eq(1) + expect(json_response.first['title']).to eq(deploy_key.title) + expect(json_response.first['owner']['id']).to eq(deploy_key.user.id) + end - context 'and user can read owner of key' do - it 'renders the deploy keys in a json payload, with owner' do - get(:deploy_keys_with_owners, params: { project_id: public_project.id }) + context 'with an unknown project' do + it 'returns a not found response' do + get(:deploy_keys_with_owners, params: { project_id: 9999 }) - expect(json_response.count).to eq(1) - expect(json_response.first['title']).to eq(deploy_key.title) - expect(json_response.first['owner']['id']).to eq(deploy_key.user.id) - end - end + expect(response).to have_gitlab_http_status(:not_found) + end + end - context 'and user cannot read owner of key' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false) - end + context 'and the user cannot read the owner of the key' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false) + end - it 'returns a payload without owner' do - get(:deploy_keys_with_owners, params: { project_id: public_project.id }) + it 'returns a payload without owner' do + get(:deploy_keys_with_owners, params: { project_id: project.id }) - expect(json_response.count).to eq(1) - expect(json_response.first['title']).to eq(deploy_key.title) - expect(json_response.first['owner']).to be_nil - end - end + expect(json_response.count).to eq(1) + expect(json_response.first['title']).to eq(deploy_key.title) + expect(json_response.first['owner']).to be_nil end end end diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index 25df569c461..9cfbb14e087 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -30,12 +30,15 @@ RSpec.describe WebHookLog do end describe '#save' do - context 'with basic auth credentials' do - let(:web_hook_log) { build(:web_hook_log, url: 'http://test:123@example.com') } + let(:web_hook_log) { build(:web_hook_log, url: url) } + let(:url) { 'http://example.com' } + + subject { web_hook_log.save! } - subject { web_hook_log.save! } + it { is_expected.to eq(true) } - it { is_expected.to eq(true) } + context 'with basic auth credentials' do + let(:url) { 'http://test:123@example.com'} it 'obfuscates the basic auth credentials' do subject @@ -43,30 +46,6 @@ RSpec.describe WebHookLog do expect(web_hook_log.url).to eq('http://*****:*****@example.com') end end - - context 'with author email' do - let(:author) { create(:user) } - let(:web_hook_log) { create(:web_hook_log, request_data: data) } - let(:data) do - { - commit: { - author: { - name: author.name, - email: author.email - } - } - }.deep_stringify_keys - end - - it "redacts author's email" do - expect(web_hook_log.request_data['commit']).to match a_hash_including( - 'author' => { - 'name' => author.name, - 'email' => _('[REDACTED]') - } - ) - end - end end describe '#success?' do diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index 916798c669c..dd8238456aa 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -170,24 +170,6 @@ RSpec.describe BuildDetailsEntity do expect(message).to include('could not retrieve the needed artifacts.') end end - - context 'when dependency contains invalid dependency names' do - invalid_name = 'XSS<a href=# data-disable-with="<img src=x onerror=alert(document.domain)>">' - let!(:test1) { create(:ci_build, :success, :expired, pipeline: pipeline, name: invalid_name, stage_idx: 0) } - let!(:build) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 1, options: { dependencies: [invalid_name] }) } - - before do - build.pipeline.unlocked! - build.drop!(:missing_dependency_failure) - end - - it { is_expected.to include(failure_reason: 'missing_dependency_failure') } - - it 'escapes the invalid dependency names' do - escaped_name = html_escape(invalid_name) - expect(message).to include(escaped_name) - end - end end context 'when a build has environment with latest deployment' do |