summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--GITLAB_PAGES_VERSION2
-rw-r--r--app/controllers/autocomplete_controller.rb5
-rw-r--r--app/finders/issuable_finder.rb20
-rw-r--r--app/models/hooks/web_hook_log.rb7
-rw-r--r--app/serializers/build_details_entity.rb2
-rw-r--r--config/gitlab_loose_foreign_keys.yml3
-rw-r--r--db/migrate/20220329130330_add_author_to_ci_subscriptions_projects.rb20
-rw-r--r--db/schema_migrations/202203291303301
-rw-r--r--db/structure.sql5
-rw-r--r--spec/controllers/autocomplete_controller_spec.rb77
-rw-r--r--spec/graphql/resolvers/issues_resolver_spec.rb11
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml1
-rw-r--r--spec/models/hooks/web_hook_log_spec.rb35
-rw-r--r--spec/serializers/build_details_entity_spec.rb18
-rw-r--r--spec/support/shared_examples/finders/issues_finder_shared_examples.rb71
15 files changed, 207 insertions, 71 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION
index 91951fd8ad7..30e298c7494 100644
--- a/GITLAB_PAGES_VERSION
+++ b/GITLAB_PAGES_VERSION
@@ -1 +1 @@
-1.61.0
+1.61.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/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 8ecf0c158e0..47b2a460e6f 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -483,10 +483,14 @@ class IssuableFinder
end
def by_crm_contact(items)
+ return items unless can_filter_by_crm_contact?
+
Issuables::CrmContactFilter.new(params: original_params).filter(items)
end
def by_crm_organization(items)
+ return items unless can_filter_by_crm_organization?
+
Issuables::CrmOrganizationFilter.new(params: original_params).filter(items)
end
@@ -499,4 +503,20 @@ class IssuableFinder
def feature_flag_scope
params.group || params.project
end
+
+ def can_filter_by_crm_contact?
+ current_user&.can?(:read_crm_contact, root_group)
+ end
+
+ def can_filter_by_crm_organization?
+ current_user&.can?(:read_crm_organization, root_group)
+ end
+
+ def root_group
+ strong_memoize(:root_group) do
+ base_group = params.group || params.project&.group
+
+ base_group&.root_ancestor
+ end
+ end
end
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/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb
index 5f72259f34a..dc7b5e95361 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: invalid_dependencies, punctuation: punctuation }
+ { invalid_dependencies: html_escape(invalid_dependencies), punctuation: punctuation }
end
def help_message(docs_url)
diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml
index 8a9f3f0da43..c5c2d0a61b9 100644
--- a/config/gitlab_loose_foreign_keys.yml
+++ b/config/gitlab_loose_foreign_keys.yml
@@ -144,6 +144,9 @@ ci_subscriptions_projects:
- table: projects
column: upstream_project_id
on_delete: async_delete
+ - table: users
+ column: author_id
+ on_delete: async_delete
ci_triggers:
- table: users
column: owner_id
diff --git a/db/migrate/20220329130330_add_author_to_ci_subscriptions_projects.rb b/db/migrate/20220329130330_add_author_to_ci_subscriptions_projects.rb
new file mode 100644
index 00000000000..b1d0ac64d42
--- /dev/null
+++ b/db/migrate/20220329130330_add_author_to_ci_subscriptions_projects.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class AddAuthorToCiSubscriptionsProjects < Gitlab::Database::Migration[2.0]
+ disable_ddl_transaction!
+
+ INDEX_NAME = 'index_ci_subscriptions_projects_author_id'
+
+ def up
+ unless column_exists?(:ci_subscriptions_projects, :author_id)
+ add_column :ci_subscriptions_projects, :author_id, :bigint
+ end
+
+ add_concurrent_index :ci_subscriptions_projects, :author_id, name: INDEX_NAME
+ end
+
+ def down
+ remove_concurrent_index_by_name :ci_subscriptions_projects, INDEX_NAME
+ remove_column :ci_subscriptions_projects, :author_id
+ end
+end
diff --git a/db/schema_migrations/20220329130330 b/db/schema_migrations/20220329130330
new file mode 100644
index 00000000000..367d43a89a2
--- /dev/null
+++ b/db/schema_migrations/20220329130330
@@ -0,0 +1 @@
+8726707f9f4bb8d256886c592b6a11ba8487de24f5340c837800f5ce0c32df9d \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 28082885574..cc1e6dfb288 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -13289,7 +13289,8 @@ ALTER SEQUENCE ci_stages_id_seq OWNED BY ci_stages.id;
CREATE TABLE ci_subscriptions_projects (
id bigint NOT NULL,
downstream_project_id bigint NOT NULL,
- upstream_project_id bigint NOT NULL
+ upstream_project_id bigint NOT NULL,
+ author_id bigint
);
CREATE SEQUENCE ci_subscriptions_projects_id_seq
@@ -27805,6 +27806,8 @@ CREATE INDEX index_ci_stages_on_pipeline_id_and_position ON ci_stages USING btre
CREATE INDEX index_ci_stages_on_project_id ON ci_stages USING btree (project_id);
+CREATE INDEX index_ci_subscriptions_projects_author_id ON ci_subscriptions_projects USING btree (author_id);
+
CREATE INDEX index_ci_subscriptions_projects_on_upstream_project_id ON ci_subscriptions_projects USING btree (upstream_project_id);
CREATE UNIQUE INDEX index_ci_subscriptions_projects_unique_subscription ON ci_subscriptions_projects USING btree (downstream_project_id, upstream_project_id);
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/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb
index a5b5a8e4f72..89e45810033 100644
--- a/spec/graphql/resolvers/issues_resolver_spec.rb
+++ b/spec/graphql/resolvers/issues_resolver_spec.rb
@@ -30,6 +30,9 @@ RSpec.describe Resolvers::IssuesResolver do
before_all do
project.add_developer(current_user)
project.add_reporter(reporter)
+
+ create(:crm_settings, group: group, enabled: true)
+
create(:label_link, label: label1, target: issue1)
create(:label_link, label: label1, target: issue2)
create(:label_link, label: label2, target: issue2)
@@ -399,6 +402,8 @@ RSpec.describe Resolvers::IssuesResolver do
let_it_be(:crm_issue3) { create(:issue, project: project) }
before_all do
+ group.add_developer(current_user)
+
create(:issue_customer_relations_contact, issue: crm_issue1, contact: contact1)
create(:issue_customer_relations_contact, issue: crm_issue2, contact: contact2)
create(:issue_customer_relations_contact, issue: crm_issue3, contact: contact3)
@@ -631,13 +636,13 @@ RSpec.describe Resolvers::IssuesResolver do
end
it 'finds a specific issue with iid', :request_store do
- result = batch_sync(max_queries: 7) { resolve_issues(iid: issue1.iid).to_a }
+ result = batch_sync(max_queries: 8) { resolve_issues(iid: issue1.iid).to_a }
expect(result).to contain_exactly(issue1)
end
it 'batches queries that only include IIDs', :request_store do
- result = batch_sync(max_queries: 7) do
+ result = batch_sync(max_queries: 8) do
[issue1, issue2]
.map { |issue| resolve_issues(iid: issue.iid.to_s) }
.flat_map(&:to_a)
@@ -647,7 +652,7 @@ RSpec.describe Resolvers::IssuesResolver do
end
it 'finds a specific issue with iids', :request_store do
- result = batch_sync(max_queries: 7) do
+ result = batch_sync(max_queries: 8) do
resolve_issues(iids: [issue1.iid]).to_a
end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index af910b08fae..8c1e60e78b0 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -572,7 +572,6 @@ project:
- remove_source_branch_after_merge
- deleting_user
- upstream_projects
-- downstream_projects
- upstream_project_subscriptions
- downstream_project_subscriptions
- service_desk_setting
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
diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb
index dd8238456aa..916798c669c 100644
--- a/spec/serializers/build_details_entity_spec.rb
+++ b/spec/serializers/build_details_entity_spec.rb
@@ -170,6 +170,24 @@ 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
diff --git a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb
index 9d8f37a3e64..049ead9fb89 100644
--- a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb
+++ b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb
@@ -914,42 +914,65 @@ RSpec.shared_examples 'issues or work items finder' do |factory, execute_context
end
end
- context 'filtering by crm contact' do
- let_it_be(:contact1) { create(:contact, group: group) }
- let_it_be(:contact2) { create(:contact, group: group) }
+ context 'crm filtering' do
+ let_it_be(:root_group) { create(:group) }
+ let_it_be(:group) { create(:group, parent: root_group) }
+ let_it_be(:project_crm) { create(:project, :public, group: group) }
+ let_it_be(:organization) { create(:organization, group: root_group) }
+ let_it_be(:contact1) { create(:contact, group: root_group, organization: organization) }
+ let_it_be(:contact2) { create(:contact, group: root_group, organization: organization) }
- let_it_be(:contact1_item1) { create(factory, project: project1) }
- let_it_be(:contact1_item2) { create(factory, project: project1) }
- let_it_be(:contact2_item1) { create(factory, project: project1) }
+ let_it_be(:contact1_item1) { create(factory, project: project_crm) }
+ let_it_be(:contact1_item2) { create(factory, project: project_crm) }
+ let_it_be(:contact2_item1) { create(factory, project: project_crm) }
+ let_it_be(:item_no_contact) { create(factory, project: project_crm) }
- let(:params) { { crm_contact_id: contact1.id } }
+ let_it_be(:all_project_issues) do
+ [contact1_item1, contact1_item2, contact2_item1, item_no_contact]
+ end
+
+ before do
+ create(:crm_settings, group: root_group, enabled: true)
- it 'returns for that contact' do
create(:issue_customer_relations_contact, issue: contact1_item1, contact: contact1)
create(:issue_customer_relations_contact, issue: contact1_item2, contact: contact1)
create(:issue_customer_relations_contact, issue: contact2_item1, contact: contact2)
-
- expect(items).to contain_exactly(contact1_item1, contact1_item2)
end
- end
- context 'filtering by crm organization' do
- let_it_be(:organization) { create(:organization, group: group) }
- let_it_be(:contact1) { create(:contact, group: group, organization: organization) }
- let_it_be(:contact2) { create(:contact, group: group, organization: organization) }
+ context 'filtering by crm contact' do
+ let(:params) { { project_id: project_crm.id, crm_contact_id: contact1.id } }
- let_it_be(:contact1_item1) { create(factory, project: project1) }
- let_it_be(:contact1_item2) { create(factory, project: project1) }
- let_it_be(:contact2_item1) { create(factory, project: project1) }
+ context 'when the user can read crm contacts' do
+ it 'returns for that contact' do
+ root_group.add_reporter(user)
- let(:params) { { crm_organization_id: organization.id } }
+ expect(items).to contain_exactly(contact1_item1, contact1_item2)
+ end
+ end
- it 'returns for that contact' do
- create(:issue_customer_relations_contact, issue: contact1_item1, contact: contact1)
- create(:issue_customer_relations_contact, issue: contact1_item2, contact: contact1)
- create(:issue_customer_relations_contact, issue: contact2_item1, contact: contact2)
+ context 'when the user can not read crm contacts' do
+ it 'does not filter by contact' do
+ expect(items).to match_array(all_project_issues)
+ end
+ end
+ end
+
+ context 'filtering by crm organization' do
+ let(:params) { { project_id: project_crm.id, crm_organization_id: organization.id } }
+
+ context 'when the user can read crm organization' do
+ it 'returns for that organization' do
+ root_group.add_reporter(user)
- expect(items).to contain_exactly(contact1_item1, contact1_item2, contact2_item1)
+ expect(items).to contain_exactly(contact1_item1, contact1_item2, contact2_item1)
+ end
+ end
+
+ context 'when the user can not read crm organization' do
+ it 'does not filter by organization' do
+ expect(items).to match_array(all_project_issues)
+ end
+ end
end
end
View Lorry Controller Status