diff options
-rw-r--r-- | CHANGELOG.md | 23 | ||||
-rw-r--r-- | GITALY_SERVER_VERSION | 2 | ||||
-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-- | spec/controllers/autocomplete_controller_spec.rb | 77 | ||||
-rw-r--r-- | spec/models/hooks/web_hook_log_spec.rb | 35 |
7 files changed, 109 insertions, 42 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index d81818121d5..9a5bcb26dea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,29 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.1.4 (2022-07-28) + +### Security (18 changes) + +- [Security datadog integration leaking](gitlab-org/security/gitlab@1aadbf61796ff95b4716fb8ef335c5a4dbdf8c6a) ([merge request](gitlab-org/security/gitlab!2594)) +- [Prevent users who cannot admin a public project from viewing deploy keys](gitlab-org/security/gitlab@a69bec0e58f4ee0df0774dcde1ce1817d41daa7c) ([merge request](gitlab-org/security/gitlab!2641)) +- [Add additional condition to accept invitation](gitlab-org/security/gitlab@b3480d20512d61b761da2a8772ec2bfe32182169) ([merge request](gitlab-org/security/gitlab!2655)) +- [Update GITLAB_PAGES_VERSION](gitlab-org/security/gitlab@6161c0445e4908cdc152bee4be8b9d223df9f1d0) ([merge request](gitlab-org/security/gitlab!2584)) +- [Add html_escape to build_details_entity](gitlab-org/security/gitlab@c0a82385320e144b55e400eff96f39aa56af33ef) ([merge request](gitlab-org/security/gitlab!2610)) +- [Check permissions when filtering by contact or organization](gitlab-org/security/gitlab@5b8f2da656ec7839614d4a49f35778ed00b31b2b) ([merge request](gitlab-org/security/gitlab!2645)) +- [Use author to run subscribed pipeline](gitlab-org/security/gitlab@982917161408cfe4c3191f352bb1d390dfa3c9fd) ([merge request](gitlab-org/security/gitlab!2558)) +- [Remove prohibited branches after project import](gitlab-org/security/gitlab@48864bf7539176eb385f5ce792c8e369a2b79fe6) ([merge request](gitlab-org/security/gitlab!2589)) +- [Remove feature flag `ci_yaml_limit_size`](gitlab-org/security/gitlab@b532b82ef2a0ef5b045932a56ffc06281df7a1d8) ([merge request](gitlab-org/security/gitlab!2630)) +- [Maintainer can change the visibility of Project and Group](gitlab-org/security/gitlab@9556769a3a2fe2c030eb55e87b1465638d315358) ([merge request](gitlab-org/security/gitlab!2618)) +- [Do not link unverified secondary emails with any users](gitlab-org/security/gitlab@331b1d6b590b01c3d45332a6c8e09def249c627e) ([merge request](gitlab-org/security/gitlab!2627)) +- [Forbid exchanging access token for ROP flow to users required 2FA setup](gitlab-org/security/gitlab@15bc29c4ab878365356e59c0c8d2e0f361b71e70) ([merge request](gitlab-org/security/gitlab!2621)) +- [Remove todos from confidential notes when user loses access](gitlab-org/security/gitlab@9672296987190cc4b08b1f32f3c45086880b06bc) ([merge request](gitlab-org/security/gitlab!2608)) +- [Remove group_bot_user and group_access_token after group delete](gitlab-org/security/gitlab@9b1a5e7101e1addee8c3429f4937980020c5f6b3) ([merge request](gitlab-org/security/gitlab!2634)) +- [Protect integration secrets](gitlab-org/security/gitlab@4920ac9b10e237cca5d279cbd3cf60daa829a71b) ([merge request](gitlab-org/security/gitlab!2585)) +- [Protect Grafana and Sentry integrations](gitlab-org/security/gitlab@a5d33edb1c557e9bbd0a1a79c81e851c1d740e37) ([merge request](gitlab-org/security/gitlab!2576)) +- [Fix IDOR in Jira issue show action](gitlab-org/security/gitlab@8b62e0c09c33c44f508266d7b8db4892105124ac) ([merge request](gitlab-org/security/gitlab!2648)) +- [Limit proxied requests to Grafana API](gitlab-org/security/gitlab@4580e61bc22b04eda13e293c5868a26a1a6ee571) ([merge request](gitlab-org/security/gitlab!2597)) + ## 15.1.3 (2022-07-19) ### Added (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index a9e1108673e..5f456472367 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -15.1.3
\ No newline at end of file +15.1.4
\ No newline at end of file diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index bb120e876c6..5b1ab2700c8 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.59.0 +1.59.1 diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 32d1ddf920e..6d1ffc1f2e8 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -5,6 +5,7 @@ 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] @@ -69,6 +70,10 @@ 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 2f03b3591cf..24e5f193a32 100644 --- a/app/models/hooks/web_hook_log.rb +++ b/app/models/hooks/web_hook_log.rb @@ -22,6 +22,7 @@ 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) @@ -52,4 +53,10 @@ 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/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index e874df62cd7..70e58124d21 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -378,63 +378,74 @@ RSpec.describe AutocompleteController do end context 'GET deploy_keys_with_owners' do - let!(:deploy_key) { create(:deploy_key, user: user) } - let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) } + 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 context 'unauthorized user' do it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) expect(response).to have_gitlab_http_status(:redirect) end end - context 'when the user who can read the project is logged in' do + context 'when the user is logged in' do before do sign_in(user) end - context 'and they cannot read the project' do + context 'with a non-existing project' do it 'returns a not found response' do - 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 }) + get(:deploy_keys_with_owners, params: { project_id: 9999 }) expect(response).to have_gitlab_http_status(:not_found) end end - it 'renders the deploy key in a json payload, with its owner' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + 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 }) - 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) - expect(json_response.first['deploy_keys_projects']).to be_nil - end + expect(response).to have_gitlab_http_status(:forbidden) + end + end - context 'with an unknown project' do - it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: 9999 }) + context 'when user can admin project' do + before do + public_project.add_maintainer(user) + end - expect(response).to have_gitlab_http_status(:not_found) - end - 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 '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 + 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) + expect(json_response.first['deploy_keys_projects']).to be_nil + 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 - it 'returns a payload without owner' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + it 'returns a payload without owner' do + get(:deploy_keys_with_owners, params: { project_id: public_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 - expect(json_response.first['deploy_keys_projects']).to be_nil + 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 + expect(json_response.first['deploy_keys_projects']).to be_nil + end + end end end end diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index e1fea3318f6..8ff8a1c3865 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -30,15 +30,12 @@ RSpec.describe WebHookLog do end describe '#save' do - let(:web_hook_log) { build(:web_hook_log, url: url) } - let(:url) { 'http://example.com' } - - subject { web_hook_log.save! } + context 'with basic auth credentials' do + let(:web_hook_log) { build(:web_hook_log, url: 'http://test:123@example.com') } - it { is_expected.to eq(true) } + subject { web_hook_log.save! } - context 'with basic auth credentials' do - let(:url) { 'http://test:123@example.com'} + it { is_expected.to eq(true) } it 'obfuscates the basic auth credentials' do subject @@ -46,6 +43,30 @@ 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 '.delete_batch_for' do |