diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-03 03:20:18 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-03-03 03:20:18 +0300 |
| commit | 475d5a7a176dcb87bd1fb8d55883ad2b3b2a7955 (patch) | |
| tree | 93a6467c8d82d26468ce3dcebef5a7838c5a974b /spec | |
| parent | bd091da6d5cb036cf3c58d4ba5671f931c8381e1 (diff) | |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
27 files changed, 816 insertions, 279 deletions
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index f3c7b501faa..35e5422d072 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -353,7 +353,16 @@ RSpec.describe Projects::ServicesController do it 'does not modify integration' do expect { put :update, params: project_params.merge(service: integration_params) } - .not_to change { project.prometheus_integration.reload.attributes } + .not_to change { prometheus_integration_as_data } + end + + def prometheus_integration_as_data + pi = project.prometheus_integration.reload + attrs = pi.attributes.except('encrypted_properties', + 'encrypted_properties_iv', + 'encrypted_properties_tmp') + + [attrs, pi.encrypted_properties_tmp] end end diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 526983a0d5f..5efa88a2a7d 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -114,16 +114,16 @@ RSpec.describe LabelsHelper do describe 'text_color_for_bg' do it 'uses light text on dark backgrounds' do - expect(text_color_for_bg('#222E2E')).to eq('#FFFFFF') + expect(text_color_for_bg('#222E2E')).to be_color('#FFFFFF') end it 'uses dark text on light backgrounds' do - expect(text_color_for_bg('#EEEEEE')).to eq('#333333') + expect(text_color_for_bg('#EEEEEE')).to be_color('#333333') end it 'supports RGB triplets' do - expect(text_color_for_bg('#FFF')).to eq '#333333' - expect(text_color_for_bg('#000')).to eq '#FFFFFF' + expect(text_color_for_bg('#FFF')).to be_color '#333333' + expect(text_color_for_bg('#000')).to be_color '#FFFFFF' end end diff --git a/spec/lib/banzai/filter/references/label_reference_filter_spec.rb b/spec/lib/banzai/filter/references/label_reference_filter_spec.rb index b18d68c8dd4..c342a831d62 100644 --- a/spec/lib/banzai/filter/references/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/label_reference_filter_spec.rb @@ -277,7 +277,7 @@ RSpec.describe Banzai::Filter::References::LabelReferenceFilter do end context 'References with html entities' do - let!(:label) { create(:label, name: '<html>', project: project) } + let!(:label) { create(:label, title: '<html>', project: project) } it 'links to a valid reference' do doc = reference_filter('See ~"<html>"') diff --git a/spec/lib/bulk_imports/common/pipelines/labels_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/labels_pipeline_spec.rb index 48db24def48..ac516418ce8 100644 --- a/spec/lib/bulk_imports/common/pipelines/labels_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/labels_pipeline_spec.rb @@ -43,7 +43,7 @@ RSpec.describe BulkImports::Common::Pipelines::LabelsPipeline do expect(label.title).to eq('Label 1') expect(label.description).to eq('Label 1') - expect(label.color).to eq('#6699cc') + expect(label.color).to be_color('#6699cc') expect(File.directory?(tmpdir)).to eq(false) end end diff --git a/spec/lib/gitlab/background_migration/encrypt_integration_properties_spec.rb b/spec/lib/gitlab/background_migration/encrypt_integration_properties_spec.rb new file mode 100644 index 00000000000..7334867e8fb --- /dev/null +++ b/spec/lib/gitlab/background_migration/encrypt_integration_properties_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::EncryptIntegrationProperties do + let(:integrations) do + table(:integrations) do |integrations| + integrations.send :attr_encrypted, :encrypted_properties_tmp, + attribute: :encrypted_properties, + mode: :per_attribute_iv, + key: ::Settings.attr_encrypted_db_key_base_32, + algorithm: 'aes-256-gcm', + marshal: true, + marshaler: ::Gitlab::Json, + encode: false, + encode_iv: false + end + end + + let!(:no_properties) { integrations.create! } + let!(:with_plaintext_1) { integrations.create!(properties: json_props(1)) } + let!(:with_plaintext_2) { integrations.create!(properties: json_props(2)) } + let!(:with_encrypted) do + x = integrations.new + x.properties = nil + x.encrypted_properties_tmp = some_props(3) + x.save! + x + end + + let(:start_id) { integrations.minimum(:id) } + let(:end_id) { integrations.maximum(:id) } + + it 'ensures all properties are encrypted', :aggregate_failures do + described_class.new.perform(start_id, end_id) + + props = integrations.all.to_h do |record| + [record.id, [Gitlab::Json.parse(record.properties), record.encrypted_properties_tmp]] + end + + expect(integrations.count).to eq(4) + + expect(props).to match( + no_properties.id => both(be_nil), + with_plaintext_1.id => both(eq some_props(1)), + with_plaintext_2.id => both(eq some_props(2)), + with_encrypted.id => match([be_nil, eq(some_props(3))]) + ) + end + + private + + def both(obj) + match [obj, obj] + end + + def some_props(id) + HashWithIndifferentAccess.new({ id: id, foo: 1, bar: true, baz: %w[a string array] }) + end + + def json_props(id) + some_props(id).to_json + end +end diff --git a/spec/lib/gitlab/ci/parsers/security/common_spec.rb b/spec/lib/gitlab/ci/parsers/security/common_spec.rb index f7d28d309e9..78c357eb14a 100644 --- a/spec/lib/gitlab/ci/parsers/security/common_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/common_spec.rb @@ -26,8 +26,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do allow(parser).to receive(:tracking_data).and_return(tracking_data) allow(parser).to receive(:create_flags).and_return(vulnerability_flags_data) end - - artifact.each_blob { |blob| described_class.parse!(blob, report, vulnerability_finding_signatures_enabled) } end describe 'schema validation' do @@ -40,13 +38,24 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do allow(validator_class).to receive(:new).and_call_original end - context 'when enforce_security_report_validation is enabled' do + context 'when show_report_validation_warnings is enabled' do before do - stub_feature_flags(enforce_security_report_validation: true) + stub_feature_flags(show_report_validation_warnings: true) end - context 'when the validate flag is set as `true`' do - let(:validate) { true } + context 'when the validate flag is set to `false`' do + let(:validate) { false } + let(:valid?) { false } + let(:errors) { ['foo'] } + + before do + allow_next_instance_of(validator_class) do |instance| + allow(instance).to receive(:valid?).and_return(valid?) + allow(instance).to receive(:errors).and_return(errors) + end + + allow(parser).to receive_messages(create_scanner: true, create_scan: true) + end it 'instantiates the validator with correct params' do parse_report @@ -54,26 +63,25 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do expect(validator_class).to have_received(:new).with(report.type, {}) end - context 'when the report data is valid according to the schema' do - let(:valid?) { true } - - before do - allow_next_instance_of(validator_class) do |instance| - allow(instance).to receive(:valid?).and_return(valid?) - allow(instance).to receive(:errors).and_return([]) - end - - allow(parser).to receive_messages(create_scanner: true, create_scan: true) + context 'when the report data is not valid according to the schema' do + it 'adds warnings to the report' do + expect { parse_report }.to change { report.warnings }.from([]).to([{ message: 'foo', type: 'Schema' }]) end - it 'does not add errors to the report' do - expect { parse_report }.not_to change { report.errors }.from([]) + it 'keeps the execution flow as normal' do + parse_report + + expect(parser).to have_received(:create_scanner) + expect(parser).to have_received(:create_scan) end + end - it 'adds the schema validation status to the report' do - parse_report + context 'when the report data is valid according to the schema' do + let(:valid?) { true } + let(:errors) { [] } - expect(report.schema_validation_status).to eq(:valid_schema) + it 'does not add warnings to the report' do + expect { parse_report }.not_to change { report.errors } end it 'keeps the execution flow as normal' do @@ -83,42 +91,62 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do expect(parser).to have_received(:create_scan) end end + end - context 'when the report data is not valid according to the schema' do - let(:valid?) { false } - - before do - allow_next_instance_of(validator_class) do |instance| - allow(instance).to receive(:valid?).and_return(valid?) - allow(instance).to receive(:errors).and_return(['foo']) - end + context 'when the validate flag is set to `true`' do + let(:validate) { true } + let(:valid?) { false } + let(:errors) { ['foo'] } - allow(parser).to receive_messages(create_scanner: true, create_scan: true) + before do + allow_next_instance_of(validator_class) do |instance| + allow(instance).to receive(:valid?).and_return(valid?) + allow(instance).to receive(:errors).and_return(errors) end + allow(parser).to receive_messages(create_scanner: true, create_scan: true) + end + + it 'instantiates the validator with correct params' do + parse_report + + expect(validator_class).to have_received(:new).with(report.type, {}) + end + + context 'when the report data is not valid according to the schema' do it 'adds errors to the report' do expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }]) end - it 'adds the schema validation status to the report' do + it 'does not try to create report entities' do parse_report - expect(report.schema_validation_status).to eq(:invalid_schema) + expect(parser).not_to have_received(:create_scanner) + expect(parser).not_to have_received(:create_scan) + end + end + + context 'when the report data is valid according to the schema' do + let(:valid?) { true } + let(:errors) { [] } + + it 'does not add errors to the report' do + expect { parse_report }.not_to change { report.errors }.from([]) end - it 'does not try to create report entities' do + it 'keeps the execution flow as normal' do parse_report - expect(parser).not_to have_received(:create_scanner) - expect(parser).not_to have_received(:create_scan) + expect(parser).to have_received(:create_scanner) + expect(parser).to have_received(:create_scan) end end end end - context 'when enforce_security_report_validation is disabled' do + context 'when show_report_validation_warnings is disabled' do before do - stub_feature_flags(enforce_security_report_validation: false) + stub_feature_flags(show_report_validation_warnings: false) end context 'when the validate flag is set as `false`' do @@ -181,277 +209,283 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do end end - describe 'parsing finding.name' do - let(:artifact) { build(:ci_job_artifact, :common_security_report_with_blank_names) } - - context 'when message is provided' do - it 'sets message from the report as a finding name' do - finding = report.findings.find { |x| x.compare_key == 'CVE-1020' } - expected_name = Gitlab::Json.parse(finding.raw_metadata)['message'] - - expect(finding.name).to eq(expected_name) - end + context 'report parsing' do + before do + artifact.each_blob { |blob| described_class.parse!(blob, report, vulnerability_finding_signatures_enabled) } end - context 'when message is not provided' do - context 'and name is provided' do - it 'sets name from the report as a name' do - finding = report.findings.find { |x| x.compare_key == 'CVE-1030' } - expected_name = Gitlab::Json.parse(finding.raw_metadata)['name'] + describe 'parsing finding.name' do + let(:artifact) { build(:ci_job_artifact, :common_security_report_with_blank_names) } + + context 'when message is provided' do + it 'sets message from the report as a finding name' do + finding = report.findings.find { |x| x.compare_key == 'CVE-1020' } + expected_name = Gitlab::Json.parse(finding.raw_metadata)['message'] expect(finding.name).to eq(expected_name) end end - context 'and name is not provided' do - context 'when CVE identifier exists' do - it 'combines identifier with location to create name' do - finding = report.findings.find { |x| x.compare_key == 'CVE-2017-11429' } - expect(finding.name).to eq("CVE-2017-11429 in yarn.lock") + context 'when message is not provided' do + context 'and name is provided' do + it 'sets name from the report as a name' do + finding = report.findings.find { |x| x.compare_key == 'CVE-1030' } + expected_name = Gitlab::Json.parse(finding.raw_metadata)['name'] + + expect(finding.name).to eq(expected_name) end end - context 'when CWE identifier exists' do - it 'combines identifier with location to create name' do - finding = report.findings.find { |x| x.compare_key == 'CWE-2017-11429' } - expect(finding.name).to eq("CWE-2017-11429 in yarn.lock") + context 'and name is not provided' do + context 'when CVE identifier exists' do + it 'combines identifier with location to create name' do + finding = report.findings.find { |x| x.compare_key == 'CVE-2017-11429' } + expect(finding.name).to eq("CVE-2017-11429 in yarn.lock") + end end - end - context 'when neither CVE nor CWE identifier exist' do - it 'combines identifier with location to create name' do - finding = report.findings.find { |x| x.compare_key == 'OTHER-2017-11429' } - expect(finding.name).to eq("other-2017-11429 in yarn.lock") + context 'when CWE identifier exists' do + it 'combines identifier with location to create name' do + finding = report.findings.find { |x| x.compare_key == 'CWE-2017-11429' } + expect(finding.name).to eq("CWE-2017-11429 in yarn.lock") + end + end + + context 'when neither CVE nor CWE identifier exist' do + it 'combines identifier with location to create name' do + finding = report.findings.find { |x| x.compare_key == 'OTHER-2017-11429' } + expect(finding.name).to eq("other-2017-11429 in yarn.lock") + end end end end end - end - describe 'parsing finding.details' do - context 'when details are provided' do - it 'sets details from the report' do - finding = report.findings.find { |x| x.compare_key == 'CVE-1020' } - expected_details = Gitlab::Json.parse(finding.raw_metadata)['details'] + describe 'parsing finding.details' do + context 'when details are provided' do + it 'sets details from the report' do + finding = report.findings.find { |x| x.compare_key == 'CVE-1020' } + expected_details = Gitlab::Json.parse(finding.raw_metadata)['details'] - expect(finding.details).to eq(expected_details) + expect(finding.details).to eq(expected_details) + end end - end - context 'when details are not provided' do - it 'sets empty hash' do - finding = report.findings.find { |x| x.compare_key == 'CVE-1030' } - expect(finding.details).to eq({}) + context 'when details are not provided' do + it 'sets empty hash' do + finding = report.findings.find { |x| x.compare_key == 'CVE-1030' } + expect(finding.details).to eq({}) + end end end - end - describe 'top-level scanner' do - it 'is the primary scanner' do - expect(report.primary_scanner.external_id).to eq('gemnasium') - expect(report.primary_scanner.name).to eq('Gemnasium') - expect(report.primary_scanner.vendor).to eq('GitLab') - expect(report.primary_scanner.version).to eq('2.18.0') - end + describe 'top-level scanner' do + it 'is the primary scanner' do + expect(report.primary_scanner.external_id).to eq('gemnasium') + expect(report.primary_scanner.name).to eq('Gemnasium') + expect(report.primary_scanner.vendor).to eq('GitLab') + expect(report.primary_scanner.version).to eq('2.18.0') + end - it 'returns nil report has no scanner' do - empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) - described_class.parse!({}.to_json, empty_report) + it 'returns nil report has no scanner' do + empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) + described_class.parse!({}.to_json, empty_report) - expect(empty_report.primary_scanner).to be_nil + expect(empty_report.primary_scanner).to be_nil + end end - end - describe 'parsing scanners' do - subject(:scanner) { report.findings.first.scanner } + describe 'parsing scanners' do + subject(:scanner) { report.findings.first.scanner } - context 'when vendor is not missing in scanner' do - it 'returns scanner with parsed vendor value' do - expect(scanner.vendor).to eq('GitLab') + context 'when vendor is not missing in scanner' do + it 'returns scanner with parsed vendor value' do + expect(scanner.vendor).to eq('GitLab') + end end end - end - describe 'parsing scan' do - it 'returns scan object for each finding' do - scans = report.findings.map(&:scan) + describe 'parsing scan' do + it 'returns scan object for each finding' do + scans = report.findings.map(&:scan) - expect(scans.map(&:status).all?('success')).to be(true) - expect(scans.map(&:start_time).all?('placeholder-value')).to be(true) - expect(scans.map(&:end_time).all?('placeholder-value')).to be(true) - expect(scans.size).to eq(3) - expect(scans.first).to be_a(::Gitlab::Ci::Reports::Security::Scan) - end + expect(scans.map(&:status).all?('success')).to be(true) + expect(scans.map(&:start_time).all?('placeholder-value')).to be(true) + expect(scans.map(&:end_time).all?('placeholder-value')).to be(true) + expect(scans.size).to eq(3) + expect(scans.first).to be_a(::Gitlab::Ci::Reports::Security::Scan) + end - it 'returns nil when scan is not a hash' do - empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) - described_class.parse!({}.to_json, empty_report) + it 'returns nil when scan is not a hash' do + empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) + described_class.parse!({}.to_json, empty_report) - expect(empty_report.scan).to be(nil) + expect(empty_report.scan).to be(nil) + end end - end - describe 'parsing schema version' do - it 'parses the version' do - expect(report.version).to eq('14.0.2') - end + describe 'parsing schema version' do + it 'parses the version' do + expect(report.version).to eq('14.0.2') + end - it 'returns nil when there is no version' do - empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) - described_class.parse!({}.to_json, empty_report) + it 'returns nil when there is no version' do + empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) + described_class.parse!({}.to_json, empty_report) - expect(empty_report.version).to be_nil + expect(empty_report.version).to be_nil + end end - end - describe 'parsing analyzer' do - it 'associates analyzer with report' do - expect(report.analyzer.id).to eq('common-analyzer') - expect(report.analyzer.name).to eq('Common Analyzer') - expect(report.analyzer.version).to eq('2.0.1') - expect(report.analyzer.vendor).to eq('Common') - end + describe 'parsing analyzer' do + it 'associates analyzer with report' do + expect(report.analyzer.id).to eq('common-analyzer') + expect(report.analyzer.name).to eq('Common Analyzer') + expect(report.analyzer.version).to eq('2.0.1') + expect(report.analyzer.vendor).to eq('Common') + end - it 'returns nil when analyzer data is not available' do - empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) - described_class.parse!({}.to_json, empty_report) + it 'returns nil when analyzer data is not available' do + empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) + described_class.parse!({}.to_json, empty_report) - expect(empty_report.analyzer).to be_nil + expect(empty_report.analyzer).to be_nil + end end - end - describe 'parsing flags' do - it 'returns flags object for each finding' do - flags = report.findings.first.flags + describe 'parsing flags' do + it 'returns flags object for each finding' do + flags = report.findings.first.flags - expect(flags).to contain_exactly( - have_attributes(type: 'flagged-as-likely-false-positive', origin: 'post analyzer X', description: 'static string to sink'), + expect(flags).to contain_exactly( + have_attributes(type: 'flagged-as-likely-false-positive', origin: 'post analyzer X', description: 'static string to sink'), have_attributes(type: 'flagged-as-likely-false-positive', origin: 'post analyzer Y', description: 'integer to sink') - ) + ) + end end - end - describe 'parsing links' do - it 'returns links object for each finding', :aggregate_failures do - links = report.findings.flat_map(&:links) + describe 'parsing links' do + it 'returns links object for each finding', :aggregate_failures do + links = report.findings.flat_map(&:links) - expect(links.map(&:url)).to match_array(['https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1020', 'https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1030']) - expect(links.map(&:name)).to match_array([nil, 'CVE-1030']) - expect(links.size).to eq(2) - expect(links.first).to be_a(::Gitlab::Ci::Reports::Security::Link) + expect(links.map(&:url)).to match_array(['https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1020', 'https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1030']) + expect(links.map(&:name)).to match_array([nil, 'CVE-1030']) + expect(links.size).to eq(2) + expect(links.first).to be_a(::Gitlab::Ci::Reports::Security::Link) + end end - end - describe 'parsing evidence' do - it 'returns evidence object for each finding', :aggregate_failures do - evidences = report.findings.map(&:evidence) + describe 'parsing evidence' do + it 'returns evidence object for each finding', :aggregate_failures do + evidences = report.findings.map(&:evidence) - expect(evidences.first.data).not_to be_empty - expect(evidences.first.data["summary"]).to match(/The Origin header was changed/) - expect(evidences.size).to eq(3) - expect(evidences.compact.size).to eq(2) - expect(evidences.first).to be_a(::Gitlab::Ci::Reports::Security::Evidence) + expect(evidences.first.data).not_to be_empty + expect(evidences.first.data["summary"]).to match(/The Origin header was changed/) + expect(evidences.size).to eq(3) + expect(evidences.compact.size).to eq(2) + expect(evidences.first).to be_a(::Gitlab::Ci::Reports::Security::Evidence) + end end - end - describe 'setting the uuid' do - let(:finding_uuids) { report.findings.map(&:uuid) } - let(:uuid_1) do - Security::VulnerabilityUUID.generate( - report_type: "sast", - primary_identifier_fingerprint: report.findings[0].identifiers.first.fingerprint, - location_fingerprint: location.fingerprint, - project_id: pipeline.project_id - ) - end + describe 'setting the uuid' do + let(:finding_uuids) { report.findings.map(&:uuid) } + let(:uuid_1) do + Security::VulnerabilityUUID.generate( + report_type: "sast", + primary_identifier_fingerprint: report.findings[0].identifiers.first.fingerprint, + location_fingerprint: location.fingerprint, + project_id: pipeline.project_id + ) + end - let(:uuid_2) do - Security::VulnerabilityUUID.generate( - report_type: "sast", - primary_identifier_fingerprint: report.findings[1].identifiers.first.fingerprint, - location_fingerprint: location.fingerprint, - project_id: pipeline.project_id - ) - end + let(:uuid_2) do + Security::VulnerabilityUUID.generate( + report_type: "sast", + primary_identifier_fingerprint: report.findings[1].identifiers.first.fingerprint, + location_fingerprint: location.fingerprint, + project_id: pipeline.project_id + ) + end - let(:expected_uuids) { [uuid_1, uuid_2, nil] } + let(:expected_uuids) { [uuid_1, uuid_2, nil] } - it 'sets the UUIDv5 for findings', :aggregate_failures do - allow_next_instance_of(Gitlab::Ci::Reports::Security::Report) do |report| - allow(report).to receive(:type).and_return('sast') + it 'sets the UUIDv5 for findings', :aggregate_failures do + allow_next_instance_of(Gitlab::Ci::Reports::Security::Report) do |report| + allow(report).to receive(:type).and_return('sast') - expect(finding_uuids).to match_array(expected_uuids) + expect(finding_uuids).to match_array(expected_uuids) + end end end - end - describe 'parsing tracking' do - let(:tracking_data) do - { + describe 'parsing tracking' do + let(:tracking_data) do + { 'type' => 'source', 'items' => [ - 'signatures' => [ - { 'algorithm' => 'hash', 'value' => 'hash_value' }, - { 'algorithm' => 'location', 'value' => 'location_value' }, - { 'algorithm' => 'scope_offset', 'value' => 'scope_offset_value' } - ] + 'signatures' => [ + { 'algorithm' => 'hash', 'value' => 'hash_value' }, + { 'algorithm' => 'location', 'value' => 'location_value' }, + { 'algorithm' => 'scope_offset', 'value' => 'scope_offset_value' } ] - } - end + ] + } + end - context 'with valid tracking information' do - it 'creates signatures for each algorithm' do - finding = report.findings.first - expect(finding.signatures.size).to eq(3) - expect(finding.signatures.map(&:algorithm_type).to_set).to eq(Set['hash', 'location', 'scope_offset']) + context 'with valid tracking information' do + it 'creates signatures for each algorithm' do + finding = report.findings.first + expect(finding.signatures.size).to eq(3) + expect(finding.signatures.map(&:algorithm_type).to_set).to eq(Set['hash', 'location', 'scope_offset']) + end end - end - context 'with invalid tracking information' do - let(:tracking_data) do - { + context 'with invalid tracking information' do + let(:tracking_data) do + { 'type' => 'source', 'items' => [ - 'signatures' => [ - { 'algorithm' => 'hash', 'value' => 'hash_value' }, - { 'algorithm' => 'location', 'value' => 'location_value' }, - { 'algorithm' => 'INVALID', 'value' => 'scope_offset_value' } - ] + 'signatures' => [ + { 'algorithm' => 'hash', 'value' => 'hash_value' }, + { 'algorithm' => 'location', 'value' => 'location_value' }, + { 'algorithm' => 'INVALID', 'value' => 'scope_offset_value' } ] - } - end + ] + } + end - it 'ignores invalid algorithm types' do - finding = report.findings.first - expect(finding.signatures.size).to eq(2) - expect(finding.signatures.map(&:algorithm_type).to_set).to eq(Set['hash', 'location']) + it 'ignores invalid algorithm types' do + finding = report.findings.first + expect(finding.signatures.size).to eq(2) + expect(finding.signatures.map(&:algorithm_type).to_set).to eq(Set['hash', 'location']) + end end - end - context 'with valid tracking information' do - it 'creates signatures for each signature algorithm' do - finding = report.findings.first - expect(finding.signatures.size).to eq(3) - expect(finding.signatures.map(&:algorithm_type)).to eq(%w[hash location scope_offset]) - - signatures = finding.signatures.index_by(&:algorithm_type) - expected_values = tracking_data['items'][0]['signatures'].index_by { |x| x['algorithm'] } - expect(signatures['hash'].signature_value).to eq(expected_values['hash']['value']) - expect(signatures['location'].signature_value).to eq(expected_values['location']['value']) - expect(signatures['scope_offset'].signature_value).to eq(expected_values['scope_offset']['value']) - end + context 'with valid tracking information' do + it 'creates signatures for each signature algorithm' do + finding = report.findings.first + expect(finding.signatures.size).to eq(3) + expect(finding.signatures.map(&:algorithm_type)).to eq(%w[hash location scope_offset]) + + signatures = finding.signatures.index_by(&:algorithm_type) + expected_values = tracking_data['items'][0]['signatures'].index_by { |x| x['algorithm'] } + expect(signatures['hash'].signature_value).to eq(expected_values['hash']['value']) + expect(signatures['location'].signature_value).to eq(expected_values['location']['value']) + expect(signatures['scope_offset'].signature_value).to eq(expected_values['scope_offset']['value']) + end - it 'sets the uuid according to the higest priority signature' do - finding = report.findings.first - highest_signature = finding.signatures.max_by(&:priority) + it 'sets the uuid according to the higest priority signature' do + finding = report.findings.first + highest_signature = finding.signatures.max_by(&:priority) - identifiers = if vulnerability_finding_signatures_enabled - "#{finding.report_type}-#{finding.primary_identifier.fingerprint}-#{highest_signature.signature_hex}-#{report.project_id}" - else - "#{finding.report_type}-#{finding.primary_identifier.fingerprint}-#{finding.location.fingerprint}-#{report.project_id}" - end + identifiers = if vulnerability_finding_signatures_enabled + "#{finding.report_type}-#{finding.primary_identifier.fingerprint}-#{highest_signature.signature_hex}-#{report.project_id}" + else + "#{finding.report_type}-#{finding.primary_identifier.fingerprint}-#{finding.location.fingerprint}-#{report.project_id}" + end - expect(finding.uuid).to eq(Gitlab::UUID.v5(identifiers)) + expect(finding.uuid).to eq(Gitlab::UUID.v5(identifiers)) + end end end end diff --git a/spec/lib/gitlab/ci/reports/security/report_spec.rb b/spec/lib/gitlab/ci/reports/security/report_spec.rb index a8b962ee970..4dc1eca3859 100644 --- a/spec/lib/gitlab/ci/reports/security/report_spec.rb +++ b/spec/lib/gitlab/ci/reports/security/report_spec.rb @@ -158,6 +158,16 @@ RSpec.describe Gitlab::Ci::Reports::Security::Report do end end + describe '#add_warning' do + context 'when the message is given' do + it 'adds a new warning to report' do + expect { report.add_warning('foo', 'bar') }.to change { report.warnings } + .from([]) + .to([{ type: 'foo', message: 'bar' }]) + end + end + end + describe 'errored?' do subject { report.errored? } diff --git a/spec/lib/gitlab/color_spec.rb b/spec/lib/gitlab/color_spec.rb new file mode 100644 index 00000000000..8b16e13fa4d --- /dev/null +++ b/spec/lib/gitlab/color_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Color do + describe ".of" do + described_class::Constants::COLOR_NAME_TO_HEX.each do |name, value| + it "parses #{name} to #{value}" do + expect(described_class.of(name)).to eq(value) + end + end + + it 'parses hex literals as colors' do + expect(described_class.of('#fff')).to eq(described_class.new('#fff')) + expect(described_class.of('#fefefe')).to eq(described_class.new('#fefefe')) + end + + it 'raises if the input is nil' do + expect { described_class.of(nil) }.to raise_error(ArgumentError) + end + + it 'returns an invalid color if the input is not valid' do + expect(described_class.of('unknown color')).not_to be_valid + end + end + + describe '#new' do + it 'handles nil values' do + expect(described_class.new(nil)).to eq(described_class.new(nil)) + end + + it 'strips input' do + expect(described_class.new(' abc ')).to eq(described_class.new('abc')) + end + end + + describe '#valid?' do + described_class::Constants::COLOR_NAME_TO_HEX.each_key do |name| + specify "#{name} is a valid color" do + expect(described_class.of(name)).to be_valid + end + end + + specify '#fff is a valid color' do + expect(described_class.new('#fff')).to be_valid + end + + specify '#ffffff is a valid color' do + expect(described_class.new('#ffffff')).to be_valid + end + + specify '#ABCDEF is a valid color' do + expect(described_class.new('#ABCDEF')).to be_valid + end + + specify '#123456 is a valid color' do + expect(described_class.new('#123456')).to be_valid + end + + specify '#1234567 is not a valid color' do + expect(described_class.new('#1234567')).not_to be_valid + end + + specify 'fff is not a valid color' do + expect(described_class.new('fff')).not_to be_valid + end + + specify '#deadbeaf is not a valid color' do + expect(described_class.new('#deadbeaf')).not_to be_valid + end + + specify '#a1b2c3 is a valid color' do + expect(described_class.new('#a1b2c3')).to be_valid + end + + specify 'nil is not a valid color' do + expect(described_class.new(nil)).not_to be_valid + end + end + + describe '#light?' do + specify '#fff is light' do + expect(described_class.new('#fff')).to be_light + end + + specify '#a7a7a7 is light' do + expect(described_class.new('#a7a7a7')).to be_light + end + + specify '#a6a7a7 is dark' do + expect(described_class.new('#a6a7a7')).not_to be_light + end + + specify '#000 is dark' do + expect(described_class.new('#000')).not_to be_light + end + + specify 'invalid colors are not light' do + expect(described_class.new('not-a-color')).not_to be_light + end + end + + describe '#contrast' do + context 'with light colors' do + it 'is dark' do + %w[#fff #fefefe #a7a7a7].each do |hex| + expect(described_class.new(hex)).to have_attributes( + contrast: described_class::Constants::DARK, + luminosity: :light + ) + end + end + end + + context 'with dark colors' do + it 'is light' do + %w[#000 #a6a7a7].each do |hex| + expect(described_class.new(hex)).to have_attributes( + contrast: described_class::Constants::LIGHT, + luminosity: :dark + ) + end + end + end + end + + describe 'as_json' do + it 'serializes correctly' do + expect(described_class.new('#f0f1f2').as_json).to eq('#f0f1f2') + end + end +end diff --git a/spec/lib/gitlab/database/type/color_spec.rb b/spec/lib/gitlab/database/type/color_spec.rb new file mode 100644 index 00000000000..84fd8d0bbce --- /dev/null +++ b/spec/lib/gitlab/database/type/color_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Database::Type::Color do + subject(:type) { described_class.new } + + let(:color) { ::Gitlab::Color.of('red') } + + it 'serializes by calling #to_s' do + expect(type.serialize(color)).to eq(color.to_s) + end + + it 'serializes nil to nil' do + expect(type.serialize(nil)).to be_nil + end + + it 'casts by calling Color::new' do + expect(type.cast('#fff')).to eq(::Gitlab::Color.new('#fff')) + end + + it 'accepts colors as arguments to cast' do + expect(type.cast(color)).to eq(color) + end + + it 'allows nil database values' do + expect(type.cast(nil)).to be_nil + end + + it 'tells us what is serializable' do + [nil, 'foo', color].each do |value| + expect(type.serializable?(value)).to be true + end + end + + it 'tells us what is not serializable' do + [0, 3.2, true, Time.current, { some: 'hash' }].each do |value| + expect(type.serializable?(value)).to be false + end + end +end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index ba6997adbf6..1c490f94e5f 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::Utils do delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string, :to_exclusive_sentence, :bytes_to_megabytes, - :append_path, :check_path_traversal!, :allowlisted?, :check_allowed_absolute_path!, :decode_path, :ms_to_round_sec, to: :described_class + :append_path, :check_path_traversal!, :allowlisted?, :check_allowed_absolute_path!, :decode_path, :ms_to_round_sec, :check_allowed_absolute_path_and_path_traversal!, to: :described_class describe '.check_path_traversal!' do it 'detects path traversal in string without any separators' do @@ -58,6 +58,65 @@ RSpec.describe Gitlab::Utils do end end + describe '.check_allowed_absolute_path_and_path_traversal!' do + let(:allowed_paths) { %w[/home/foo ./foo .test/foo ..test/foo dir/..foo.rb dir/.foo.rb] } + + it 'detects path traversal in string without any separators' do + expect { check_allowed_absolute_path_and_path_traversal!('.', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('..', allowed_paths) }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the start of the string' do + expect { check_allowed_absolute_path_and_path_traversal!('../foo', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('..\\foo', allowed_paths) }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the start of the string, even to just the subdirectory' do + expect { check_allowed_absolute_path_and_path_traversal!('../', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('..\\', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('/../', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('\\..\\', allowed_paths) }.to raise_error(/Invalid path/) + end + + it 'detects path traversal in the middle of the string' do + expect { check_allowed_absolute_path_and_path_traversal!('foo/../../bar', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\..\\bar', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\bar', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\../bar', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\..\\..\\..\\../bar', allowed_paths) }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string when slash-terminates' do + expect { check_allowed_absolute_path_and_path_traversal!('foo/../', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\', allowed_paths) }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string' do + expect { check_allowed_absolute_path_and_path_traversal!('foo/..', allowed_paths) }.to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\..', allowed_paths) }.to raise_error(/Invalid path/) + end + + it 'does not return errors for a safe string' do + expect(check_allowed_absolute_path_and_path_traversal!('./foo', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('.test/foo', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('..test/foo', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('dir/..foo.rb', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('dir/.foo.rb', allowed_paths)).to be_nil + end + + it 'raises error for a non-string' do + expect {check_allowed_absolute_path_and_path_traversal!(nil, allowed_paths)}.to raise_error(StandardError) + end + + it 'raises an exception if an absolute path is not allowed' do + expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError) + end + + it 'does nothing for an allowed absolute path' do + expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil + end + end + describe '.allowlisted?' do let(:allowed_paths) { ['/home/foo', '/foo/bar', '/etc/passwd']} diff --git a/spec/migrations/20220204194347_encrypt_integration_properties_spec.rb b/spec/migrations/20220204194347_encrypt_integration_properties_spec.rb new file mode 100644 index 00000000000..78e3b43ff76 --- /dev/null +++ b/spec/migrations/20220204194347_encrypt_integration_properties_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require_migration! + +RSpec.describe EncryptIntegrationProperties, :migration, schema: 20220204193000 do + subject(:migration) { described_class.new } + + let(:integrations) { table(:integrations) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 2) + end + + it 'correctly schedules background migrations', :aggregate_failures do + # update required + record1 = integrations.create!(properties: some_props) + record2 = integrations.create!(properties: some_props) + record3 = integrations.create!(properties: some_props) + record4 = integrations.create!(properties: nil) + record5 = integrations.create!(properties: nil) + + Sidekiq::Testing.fake! do + freeze_time do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_migration(record1.id, record2.id) + expect(described_class::MIGRATION).to be_scheduled_migration(record3.id, record4.id) + expect(described_class::MIGRATION).to be_scheduled_migration(record5.id, record5.id) + + expect(BackgroundMigrationWorker.jobs.size).to eq(3) + end + end + end + + def some_props + { iid: generate(:iid), url: generate(:url), username: generate(:username) }.to_json + end +end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index c0bb0ba636d..3d85bd245d9 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -843,4 +843,82 @@ RSpec.describe Integration do expect(subject.password_fields).to eq([]) end end + + describe 'encrypted_properties' do + let(:properties) { { foo: 1, bar: true } } + let(:db_props) { properties.stringify_keys } + let(:record) { create(:integration, :instance, properties: properties) } + + it 'contains the same data as properties' do + expect(record).to have_attributes( + properties: db_props, + encrypted_properties_tmp: db_props + ) + end + + it 'is persisted' do + encrypted_properties = described_class.id_in(record.id) + + expect(encrypted_properties).to contain_exactly have_attributes(encrypted_properties_tmp: db_props) + end + + it 'is updated when using prop_accessors' do + some_integration = Class.new(described_class) do + prop_accessor :foo + end + + record = some_integration.new + + record.foo = 'the foo' + + expect(record.encrypted_properties_tmp).to eq({ 'foo' => 'the foo' }) + end + + it 'saves correctly using insert_all' do + hash = record.to_integration_hash + hash[:project_id] = project + + expect do + described_class.insert_all([hash]) + end.to change(described_class, :count).by(1) + + expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props) + end + + it 'is part of the to_integration_hash' do + hash = record.to_integration_hash + + expect(hash).to include('encrypted_properties' => be_present, 'encrypted_properties_iv' => be_present) + expect(hash['encrypted_properties']).not_to eq(record.encrypted_properties) + expect(hash['encrypted_properties_iv']).not_to eq(record.encrypted_properties_iv) + + decrypted = described_class.decrypt(:encrypted_properties_tmp, + hash['encrypted_properties'], + { iv: hash['encrypted_properties_iv'] }) + + expect(decrypted).to eq db_props + end + + context 'when the properties are empty' do + let(:properties) { {} } + + it 'is part of the to_integration_hash' do + hash = record.to_integration_hash + + expect(hash).to include('encrypted_properties' => be_nil, 'encrypted_properties_iv' => be_nil) + end + + it 'saves correctly using insert_all' do + hash = record.to_integration_hash + hash[:project_id] = project + + expect do + described_class.insert_all([hash]) + end.to change(described_class, :count).by(1) + + expect(described_class.last).not_to eq record + expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props) + end + end + end end diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 14acaf11ca4..635c3596ff8 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -67,24 +67,21 @@ RSpec.describe Label do label = described_class.new(color: ' #abcdef ') label.valid? - expect(label.color).to eq('#abcdef') + expect(label.color).to be_color('#abcdef') end it 'uses default color if color is missing' do label = described_class.new(color: nil) - expect(label.color).to be(Label::DEFAULT_COLOR) + expect(label.color).to be_color(Label::DEFAULT_COLOR) end end describe '#text_color' do it 'uses default color if color is missing' do - expect(LabelsHelper).to receive(:text_color_for_bg).with(Label::DEFAULT_COLOR) - .and_return(spy) - label = described_class.new(color: nil) - label.text_color + expect(label.text_color).to eq(Label::DEFAULT_COLOR.contrast) end end diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb index 11738e3cba8..34533da53dd 100644 --- a/spec/requests/api/group_labels_spec.rb +++ b/spec/requests/api/group_labels_spec.rb @@ -140,7 +140,7 @@ RSpec.describe API::GroupLabels do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(group_label1.name) - expect(json_response['color']).to eq(group_label1.color) + expect(json_response['color']).to be_color(group_label1.color) expect(json_response['description']).to eq(group_label1.description) end end @@ -156,7 +156,7 @@ RSpec.describe API::GroupLabels do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(valid_new_label_title) - expect(json_response['color']).to eq('#FFAABB') + expect(json_response['color']).to be_color('#FFAABB') expect(json_response['description']).to eq('test') end @@ -169,7 +169,7 @@ RSpec.describe API::GroupLabels do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(valid_new_label_title) - expect(json_response['color']).to eq('#FFAABB') + expect(json_response['color']).to be_color('#FFAABB') expect(json_response['description']).to be_nil end @@ -276,7 +276,7 @@ RSpec.describe API::GroupLabels do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(valid_new_label_title) - expect(json_response['color']).to eq('#FFFFFF') + expect(json_response['color']).to be_color('#FFFFFF') expect(json_response['description']).to eq('test') end @@ -332,7 +332,7 @@ RSpec.describe API::GroupLabels do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(valid_new_label_title) - expect(json_response['color']).to eq('#FFFFFF') + expect(json_response['color']).to be_color('#FFFFFF') expect(json_response['description']).to eq('test') end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index db9d72245b3..48f2c45bd98 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -34,7 +34,7 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(valid_label_title_2) - expect(json_response['color']).to eq(label1.color) + expect(json_response['color']).to be_color(label1.color) end it "returns 200 if colors is changed (#{route_type} route)" do @@ -42,7 +42,7 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(label1.name) - expect(json_response['color']).to eq('#FFFFFF') + expect(json_response['color']).to be_color('#FFFFFF') end it "returns 200 if a priority is added (#{route_type} route)" do @@ -86,7 +86,7 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(valid_label_title_2) - expect(json_response['color']).to eq('#FFFFFF') + expect(json_response['color']).to be_color('#FFFFFF') expect(json_response['description']).to eq('test') end @@ -266,8 +266,8 @@ RSpec.describe API::Labels do 'open_merge_requests_count' => 0, 'name' => group_label.name, 'description' => nil, - 'color' => a_string_matching(/^#\h{6}$/), - 'text_color' => a_string_matching(/^#\h{6}$/), + 'color' => a_valid_color, + 'text_color' => a_valid_color, 'priority' => nil, 'subscribed' => false, 'is_project_label' => false) @@ -277,8 +277,8 @@ RSpec.describe API::Labels do 'open_merge_requests_count' => 1, 'name' => priority_label.name, 'description' => nil, - 'color' => a_string_matching(/^#\h{6}$/), - 'text_color' => a_string_matching(/^#\h{6}$/), + 'color' => a_valid_color, + 'text_color' => a_valid_color, 'priority' => 3, 'subscribed' => false, 'is_project_label' => true) @@ -336,7 +336,7 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(valid_label_title_2) - expect(json_response['color']).to eq('#FFAABB') + expect(json_response['color']).to be_color('#FFAABB') expect(json_response['description']).to eq('test') expect(json_response['priority']).to eq(2) end @@ -350,7 +350,7 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(valid_label_title_2) - expect(json_response['color']).to eq('#FFAABB') + expect(json_response['color']).to be_color('#FFAABB') expect(json_response['description']).to be_nil expect(json_response['priority']).to be_nil end @@ -365,7 +365,7 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(valid_label_title_2) - expect(json_response['color']).to eq('#FFAABB') + expect(json_response['color']).to be_color('#FFAABB') expect(json_response['description']).to be_nil expect(json_response['priority']).to eq(3) end @@ -552,7 +552,7 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(label1.name) - expect(json_response['color']).to eq(label1.color) + expect(json_response['color']).to be_color(label1.color.to_s) end context 'if group label already exists' do diff --git a/spec/serializers/label_serializer_spec.rb b/spec/serializers/label_serializer_spec.rb index 40249450f7f..05c74fca8a8 100644 --- a/spec/serializers/label_serializer_spec.rb +++ b/spec/serializers/label_serializer_spec.rb @@ -40,7 +40,7 @@ RSpec.describe LabelSerializer do expect(subject.keys).to eq([:id, :title, :color, :project_id, :text_color]) expect(subject[:id]).to eq(resource.id) expect(subject[:title]).to eq(resource.title) - expect(subject[:color]).to eq(resource.color) + expect(subject[:color]).to be_color(resource.color) expect(subject[:text_color]).to eq(resource.text_color) expect(subject[:project_id]).to eq(resource.project_id) end diff --git a/spec/services/bulk_create_integration_service_spec.rb b/spec/services/bulk_create_integration_service_spec.rb index 28871bd5482..68c5af33fd8 100644 --- a/spec/services/bulk_create_integration_service_spec.rb +++ b/spec/services/bulk_create_integration_service_spec.rb @@ -13,14 +13,23 @@ RSpec.describe BulkCreateIntegrationService do let_it_be(:excluded_project) { create(:project, group: excluded_group) } let(:instance_integration) { create(:jira_integration, :instance) } - let(:excluded_attributes) { %w[id project_id group_id inherit_from_id instance template created_at updated_at] } + let(:excluded_attributes) do + %w[ + id project_id group_id inherit_from_id instance template + created_at updated_at + encrypted_properties encrypted_properties_iv + ] + end shared_examples 'creates integration from batch ids' do + def attributes(record) + record.reload.attributes.except(*excluded_attributes) + end + it 'updates the inherited integrations' do described_class.new(integration, batch, association).execute - expect(created_integration.attributes.except(*excluded_attributes)) - .to eq(integration.reload.attributes.except(*excluded_attributes)) + expect(attributes(created_integration)).to eq attributes(integration) end context 'integration with data fields' do @@ -29,8 +38,8 @@ RSpec.describe BulkCreateIntegrationService do it 'updates the data fields from inherited integrations' do described_class.new(integration, batch, association).execute - expect(created_integration.reload.data_fields.attributes.except(*excluded_attributes)) - .to eq(integration.reload.data_fields.attributes.except(*excluded_attributes)) + expect(attributes(created_integration.data_fields)) + .to eq attributes(integration.data_fields) end end end diff --git a/spec/services/labels/create_service_spec.rb b/spec/services/labels/create_service_spec.rb index 7a31a5a7cae..02dec8ae690 100644 --- a/spec/services/labels/create_service_spec.rb +++ b/spec/services/labels/create_service_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Labels::CreateService do let(:unknown_color) { 'unknown' } let(:no_color) { '' } - let(:expected_saved_color) { hex_color } + let(:expected_saved_color) { ::Gitlab::Color.of(hex_color) } context 'in a project' do context 'with color in hex-code' do @@ -47,7 +47,6 @@ RSpec.describe Labels::CreateService do context 'with color surrounded by spaces' do it 'creates a label' do label = described_class.new(params_with(spaced_color)).execute(project: project) - expect(label).to be_persisted expect(label.color).to eq expected_saved_color end diff --git a/spec/services/labels/promote_service_spec.rb b/spec/services/labels/promote_service_spec.rb index 81c24b26c9f..a10aaa14030 100644 --- a/spec/services/labels/promote_service_spec.rb +++ b/spec/services/labels/promote_service_spec.rb @@ -202,7 +202,7 @@ RSpec.describe Labels::PromoteService do expect(new_label.title).to eq(promoted_label_name) expect(new_label.description).to eq(promoted_description) - expect(new_label.color).to eq(promoted_color) + expect(new_label.color).to be_color(promoted_color) end it_behaves_like 'promoting a project label to a group label' diff --git a/spec/services/labels/update_service_spec.rb b/spec/services/labels/update_service_spec.rb index af2403656af..abc456f75f9 100644 --- a/spec/services/labels/update_service_spec.rb +++ b/spec/services/labels/update_service_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Labels::UpdateService do let(:unknown_color) { 'unknown' } let(:no_color) { '' } - let(:expected_saved_color) { hex_color } + let(:expected_saved_color) { ::Gitlab::Color.of(hex_color) } before do @label = Labels::CreateService.new(title: 'Initial', color: '#000000').execute(project: project) diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 377acdd14cd..03f66256487 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -23,11 +23,11 @@ RSpec.describe Projects::CreateService, '#execute' do end it 'creates labels on project creation' do - created_label = project.labels.last - - expect(created_label.type).to eq('ProjectLabel') - expect(created_label.project_id).to eq(project.id) - expect(created_label.title).to eq('bug') + expect(project.labels).to include have_attributes( + type: eq('ProjectLabel'), + project_id: eq(project.id), + title: eq('bug') + ) end context 'using gitlab project import' do diff --git a/spec/services/security/merge_reports_service_spec.rb b/spec/services/security/merge_reports_service_spec.rb index 120ce12aa58..e61977297c5 100644 --- a/spec/services/security/merge_reports_service_spec.rb +++ b/spec/services/security/merge_reports_service_spec.rb @@ -153,7 +153,18 @@ RSpec.describe Security::MergeReportsService, '#execute' do report_2.add_error('zoo', 'baz') end - it { is_expected.to eq([{ type: 'foo', message: 'bar' }, { type: 'zoo', message: 'baz' }]) } + it { is_expected.to match_array([{ type: 'foo', message: 'bar' }, { type: 'zoo', message: 'baz' }]) } + end + + describe 'warnings on target report' do + subject { merged_report.warnings } + + before do + report_1.add_warning('foo', 'bar') + report_2.add_warning('zoo', 'baz') + end + + it { is_expected.to match_array([{ type: 'foo', message: 'bar' }, { type: 'zoo', message: 'baz' }]) } end it 'copies scanners into target report and eliminates duplicates' do diff --git a/spec/support/helpers/migrations_helpers.rb b/spec/support/helpers/migrations_helpers.rb index e7dd2d54aee..afa7ee84bda 100644 --- a/spec/support/helpers/migrations_helpers.rb +++ b/spec/support/helpers/migrations_helpers.rb @@ -13,6 +13,8 @@ module MigrationsHelpers def self.name table_name.singularize.camelcase end + + yield self if block_given? end end diff --git a/spec/support/matchers/be_color.rb b/spec/support/matchers/be_color.rb new file mode 100644 index 00000000000..8fe29d003f9 --- /dev/null +++ b/spec/support/matchers/be_color.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# Assert that this value is a valid color equal to the argument +# +# ``` +# expect(value).to be_color('#fff') +# ``` +RSpec::Matchers.define :be_color do |expected| + match do |actual| + next false unless actual.present? + + if expected + ::Gitlab::Color.of(actual) == ::Gitlab::Color.of(expected) + else + ::Gitlab::Color.of(actual).valid? + end + end +end + +RSpec::Matchers.alias_matcher :a_valid_color, :be_color diff --git a/spec/support/shared_examples/namespaces/traversal_scope_examples.rb b/spec/support/shared_examples/namespaces/traversal_scope_examples.rb index db2863b50a3..40c6d400dab 100644 --- a/spec/support/shared_examples/namespaces/traversal_scope_examples.rb +++ b/spec/support/shared_examples/namespaces/traversal_scope_examples.rb @@ -244,6 +244,16 @@ RSpec.shared_examples 'namespace traversal scopes' do it { is_expected.to contain_exactly(group_2, nested_group_2, deep_nested_group_2) } end + + context 'with nested query groups' do + let!(:nested_group_1b) { create(:group, parent: group_1) } + let!(:deep_nested_group_1b) { create(:group, parent: nested_group_1b) } + let(:group1_hierarchy) { [group_1, nested_group_1, deep_nested_group_1, nested_group_1b, deep_nested_group_1b] } + + subject { described_class.where(id: [group_1, nested_group_1]).self_and_descendants } + + it { is_expected.to match_array group1_hierarchy } + end end describe '.self_and_descendants' do diff --git a/spec/support/shared_examples/services/incident_shared_examples.rb b/spec/support/shared_examples/services/incident_shared_examples.rb index cc26cf87322..b533b095aac 100644 --- a/spec/support/shared_examples/services/incident_shared_examples.rb +++ b/spec/support/shared_examples/services/incident_shared_examples.rb @@ -70,7 +70,7 @@ RSpec.shared_examples 'incident management label service' do expect(execute).to be_success expect(execute.payload).to eq(label: label) expect(label.title).to eq(title) - expect(label.color).to eq(color) + expect(label.color).to be_color(color) expect(label.description).to eq(description) end end diff --git a/spec/validators/color_validator_spec.rb b/spec/validators/color_validator_spec.rb index 27acbe5e89d..9c1339caffb 100644 --- a/spec/validators/color_validator_spec.rb +++ b/spec/validators/color_validator_spec.rb @@ -23,7 +23,12 @@ RSpec.describe ColorValidator do '#ffff' | false '#000111222' | false 'invalid' | false + 'red' | false '000' | false + nil | true # use presence to validate non-nil + '' | false + Time.current | false + ::Gitlab::Color.of(:red) | true end with_them do @@ -41,4 +46,22 @@ RSpec.describe ColorValidator do Timeout.timeout(5.seconds) { subject.valid? } end.not_to raise_error end + + context 'when color must be present' do + subject do + Class.new do + include ActiveModel::Model + include ActiveModel::Validations + attr_accessor :color + + validates :color, color: true, presence: true + end.new + end + + it 'rejects nil' do + subject.color = nil + + expect(subject).not_to be_valid + end + end end |