diff options
65 files changed, 1412 insertions, 617 deletions
diff --git a/app/graphql/types/notes/diff_position_input_type.rb b/app/graphql/types/notes/diff_position_input_type.rb index ad9fe028dfd..ccde4188f29 100644 --- a/app/graphql/types/notes/diff_position_input_type.rb +++ b/app/graphql/types/notes/diff_position_input_type.rb @@ -6,9 +6,9 @@ module Types graphql_name 'DiffPositionInput' argument :new_line, GraphQL::Types::Int, required: false, - description: copy_field_description(Types::Notes::DiffPositionType, :new_line) + description: "#{copy_field_description(Types::Notes::DiffPositionType, :new_line)} Please see the [REST API Documentation](https://docs.gitlab.com/ee/api/discussions.html#create-a-new-thread-in-the-merge-request-diff) for more information on how to use this field." argument :old_line, GraphQL::Types::Int, required: false, - description: copy_field_description(Types::Notes::DiffPositionType, :old_line) + description: "#{copy_field_description(Types::Notes::DiffPositionType, :old_line)} Please see the [REST API Documentation](https://docs.gitlab.com/ee/api/discussions.html#create-a-new-thread-in-the-merge-request-diff) for more information on how to use this field." end end end diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 2150729cb2a..877785c9eaf 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -61,7 +61,7 @@ module LabelsHelper render_label_text( label.name, suffix: suffix, - css_class: "gl-label-text #{text_color_class_for_bg(label.color)}", + css_class: "gl-label-text #{label.text_color_class}", bg_color: label.color ) end @@ -114,30 +114,8 @@ module LabelsHelper end end - def text_color_class_for_bg(bg_color) - if light_color?(bg_color) - 'gl-label-text-dark' - else - 'gl-label-text-light' - end - end - def text_color_for_bg(bg_color) - if light_color?(bg_color) - '#333333' - else - '#FFFFFF' - end - end - - def light_color?(color) - if color.length == 4 - r, g, b = color[1, 4].scan(/./).map { |v| (v * 2).hex } - else - r, g, b = color[1, 7].scan(/.{2}/).map(&:hex) - end - - (r + g + b) > 500 + ::Gitlab::Color.of(bg_color).contrast end def labels_filter_path_with_defaults(only_group_labels: false, include_ancestor_groups: true, include_descendant_groups: false) diff --git a/app/models/integration.rb b/app/models/integration.rb index e64fff2d811..fd78649e372 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -49,6 +49,16 @@ class Integration < ApplicationRecord serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize + 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 + alias_attribute :type, :type_new default_value_for :active, false @@ -67,6 +77,8 @@ class Integration < ApplicationRecord default_value_for :wiki_page_events, true after_initialize :initialize_properties + after_initialize :copy_properties_to_encrypted_properties + before_save :copy_properties_to_encrypted_properties after_commit :reset_updated_properties @@ -123,8 +135,10 @@ class Integration < ApplicationRecord def #{arg}=(value) self.properties ||= {} + self.encrypted_properties_tmp = properties updated_properties['#{arg}'] = #{arg} unless #{arg}_changed? self.properties['#{arg}'] = value + self.encrypted_properties_tmp['#{arg}'] = value end def #{arg}_changed? @@ -354,6 +368,12 @@ class Integration < ApplicationRecord self.properties = {} if has_attribute?(:properties) && properties.nil? end + def copy_properties_to_encrypted_properties + self.encrypted_properties_tmp = properties + rescue ActiveModel::MissingAttributeError + # ignore - in a record built from using a restricted select list + end + def title # implement inside child end @@ -394,7 +414,21 @@ class Integration < ApplicationRecord # return a hash of columns => values suitable for passing to insert_all def to_integration_hash column = self.class.attribute_aliases.fetch('type', 'type') - as_json(except: %w[id instance project_id group_id]).merge(column => type) + copy_properties_to_encrypted_properties + + as_json(except: %w[id instance project_id group_id encrypted_properties_tmp]) + .merge(column => type) + .merge(reencrypt_properties) + end + + def reencrypt_properties + unless properties.nil? || properties.empty? + alg = self.class.encrypted_attributes[:encrypted_properties_tmp][:algorithm] + iv = generate_iv(alg) + ep = self.class.encrypt(:encrypted_properties_tmp, properties, { iv: iv }) + end + + { 'encrypted_properties' => ep, 'encrypted_properties_iv' => iv } end def to_data_fields_hash diff --git a/app/models/label.rb b/app/models/label.rb index 0ebbb5b9bd3..b08b5bd63d5 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -12,8 +12,9 @@ class Label < ApplicationRecord cache_markdown_field :description, pipeline: :single_line - DEFAULT_COLOR = '#6699cc' + DEFAULT_COLOR = ::Gitlab::Color.of('#6699cc') + attribute :color, ::Gitlab::Database::Type::Color.new default_value_for :color, DEFAULT_COLOR has_many :lists, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -22,9 +23,9 @@ class Label < ApplicationRecord has_many :issues, through: :label_links, source: :target, source_type: 'Issue' has_many :merge_requests, through: :label_links, source: :target, source_type: 'MergeRequest' - before_validation :strip_whitespace_from_title_and_color + before_validation :strip_whitespace_from_title - validates :color, color: true, allow_blank: false + validates :color, color: true, presence: true # Don't allow ',' for label titles validates :title, presence: true, format: { with: /\A[^,]+\z/ } @@ -212,7 +213,7 @@ class Label < ApplicationRecord end def text_color - LabelsHelper.text_color_for_bg(self.color) + color.contrast end def title=(value) @@ -285,8 +286,8 @@ class Label < ApplicationRecord CGI.unescapeHTML(Sanitize.clean(value.to_s)) end - def strip_whitespace_from_title_and_color - %w(color title).each { |attr| self[attr] = self[attr]&.strip } + def strip_whitespace_from_title + self[:title] = title&.strip end end diff --git a/app/models/namespaces/traversal/linear_scopes.rb b/app/models/namespaces/traversal/linear_scopes.rb index 09d69a5f77a..0cac4c9143a 100644 --- a/app/models/namespaces/traversal/linear_scopes.rb +++ b/app/models/namespaces/traversal/linear_scopes.rb @@ -126,36 +126,26 @@ module Namespaces end def self_and_descendants_with_comparison_operators(include_self: true) - base = all.select( - :traversal_ids, - 'LEAD (namespaces.traversal_ids, 1) OVER (ORDER BY namespaces.traversal_ids ASC) next_traversal_ids' - ) + base = all.select(:traversal_ids) base_cte = Gitlab::SQL::CTE.new(:descendants_base_cte, base) namespaces = Arel::Table.new(:namespaces) # Bound the search space to ourselves (optional) and descendants. # - # WHERE (base_cte.next_traversal_ids IS NULL OR base_cte.next_traversal_ids > namespaces.traversal_ids) - # AND next_traversal_ids_sibling(base_cte.traversal_ids) > namespaces.traversal_ids + # WHERE next_traversal_ids_sibling(base_cte.traversal_ids) > namespaces.traversal_ids records = unscoped + .distinct + .with(base_cte.to_arel) .from([base_cte.table, namespaces]) - .where(base_cte.table[:next_traversal_ids].eq(nil).or(base_cte.table[:next_traversal_ids].gt(namespaces[:traversal_ids]))) .where(next_sibling_func(base_cte.table[:traversal_ids]).gt(namespaces[:traversal_ids])) # AND base_cte.traversal_ids <= namespaces.traversal_ids - records = if include_self - records.where(base_cte.table[:traversal_ids].lteq(namespaces[:traversal_ids])) - else - records.where(base_cte.table[:traversal_ids].lt(namespaces[:traversal_ids])) - end - - records_cte = Gitlab::SQL::CTE.new(:descendants_cte, records) - - unscoped - .unscope(where: [:type]) - .with(base_cte.to_arel, records_cte.to_arel) - .from(records_cte.alias_to(namespaces)) + if include_self + records.where(base_cte.table[:traversal_ids].lteq(namespaces[:traversal_ids])) + else + records.where(base_cte.table[:traversal_ids].lt(namespaces[:traversal_ids])) + end end def next_sibling_func(*args) diff --git a/app/presenters/label_presenter.rb b/app/presenters/label_presenter.rb index 8d604f9a0f6..6929bf79fdf 100644 --- a/app/presenters/label_presenter.rb +++ b/app/presenters/label_presenter.rb @@ -14,6 +14,10 @@ class LabelPresenter < Gitlab::View::Presenter::Delegated end end + def text_color_class + "gl-label-text-#{label.color.contrast.luminosity}" + end + def destroy_path case label when GroupLabel then group_label_path(label.group, label) diff --git a/app/serializers/analytics/cycle_analytics/stage_entity.rb b/app/serializers/analytics/cycle_analytics/stage_entity.rb index cfbf6f60e38..ac19998e90e 100644 --- a/app/serializers/analytics/cycle_analytics/stage_entity.rb +++ b/app/serializers/analytics/cycle_analytics/stage_entity.rb @@ -57,7 +57,8 @@ module Analytics def html_description(event) options = {} if event.label_based? - options[:label_html] = render_label(event.label, link: '', small: true, tooltip: true) + label = event.label.present + options[:label_html] = render_label(label, link: '', small: true, tooltip: true) end content_tag(:p) { event.html_description(options).html_safe } diff --git a/app/serializers/label_entity.rb b/app/serializers/label_entity.rb index e586d7f8407..5785715390f 100644 --- a/app/serializers/label_entity.rb +++ b/app/serializers/label_entity.rb @@ -4,7 +4,9 @@ class LabelEntity < Grape::Entity expose :id expose :title - expose :color + expose :color do |label| + label.color.to_s + end expose :description expose :group_id expose :project_id diff --git a/app/services/labels/base_service.rb b/app/services/labels/base_service.rb index ead7f2ea607..f694e6d47a0 100644 --- a/app/services/labels/base_service.rb +++ b/app/services/labels/base_service.rb @@ -2,162 +2,8 @@ module Labels class BaseService < ::BaseService - COLOR_NAME_TO_HEX = { - black: '#000000', - silver: '#C0C0C0', - gray: '#808080', - white: '#FFFFFF', - maroon: '#800000', - red: '#FF0000', - purple: '#800080', - fuchsia: '#FF00FF', - green: '#008000', - lime: '#00FF00', - olive: '#808000', - yellow: '#FFFF00', - navy: '#000080', - blue: '#0000FF', - teal: '#008080', - aqua: '#00FFFF', - orange: '#FFA500', - aliceblue: '#F0F8FF', - antiquewhite: '#FAEBD7', - aquamarine: '#7FFFD4', - azure: '#F0FFFF', - beige: '#F5F5DC', - bisque: '#FFE4C4', - blanchedalmond: '#FFEBCD', - blueviolet: '#8A2BE2', - brown: '#A52A2A', - burlywood: '#DEB887', - cadetblue: '#5F9EA0', - chartreuse: '#7FFF00', - chocolate: '#D2691E', - coral: '#FF7F50', - cornflowerblue: '#6495ED', - cornsilk: '#FFF8DC', - crimson: '#DC143C', - darkblue: '#00008B', - darkcyan: '#008B8B', - darkgoldenrod: '#B8860B', - darkgray: '#A9A9A9', - darkgreen: '#006400', - darkgrey: '#A9A9A9', - darkkhaki: '#BDB76B', - darkmagenta: '#8B008B', - darkolivegreen: '#556B2F', - darkorange: '#FF8C00', - darkorchid: '#9932CC', - darkred: '#8B0000', - darksalmon: '#E9967A', - darkseagreen: '#8FBC8F', - darkslateblue: '#483D8B', - darkslategray: '#2F4F4F', - darkslategrey: '#2F4F4F', - darkturquoise: '#00CED1', - darkviolet: '#9400D3', - deeppink: '#FF1493', - deepskyblue: '#00BFFF', - dimgray: '#696969', - dimgrey: '#696969', - dodgerblue: '#1E90FF', - firebrick: '#B22222', - floralwhite: '#FFFAF0', - forestgreen: '#228B22', - gainsboro: '#DCDCDC', - ghostwhite: '#F8F8FF', - gold: '#FFD700', - goldenrod: '#DAA520', - greenyellow: '#ADFF2F', - grey: '#808080', - honeydew: '#F0FFF0', - hotpink: '#FF69B4', - indianred: '#CD5C5C', - indigo: '#4B0082', - ivory: '#FFFFF0', - khaki: '#F0E68C', - lavender: '#E6E6FA', - lavenderblush: '#FFF0F5', - lawngreen: '#7CFC00', - lemonchiffon: '#FFFACD', - lightblue: '#ADD8E6', - lightcoral: '#F08080', - lightcyan: '#E0FFFF', - lightgoldenrodyellow: '#FAFAD2', - lightgray: '#D3D3D3', - lightgreen: '#90EE90', - lightgrey: '#D3D3D3', - lightpink: '#FFB6C1', - lightsalmon: '#FFA07A', - lightseagreen: '#20B2AA', - lightskyblue: '#87CEFA', - lightslategray: '#778899', - lightslategrey: '#778899', - lightsteelblue: '#B0C4DE', - lightyellow: '#FFFFE0', - limegreen: '#32CD32', - linen: '#FAF0E6', - mediumaquamarine: '#66CDAA', - mediumblue: '#0000CD', - mediumorchid: '#BA55D3', - mediumpurple: '#9370DB', - mediumseagreen: '#3CB371', - mediumslateblue: '#7B68EE', - mediumspringgreen: '#00FA9A', - mediumturquoise: '#48D1CC', - mediumvioletred: '#C71585', - midnightblue: '#191970', - mintcream: '#F5FFFA', - mistyrose: '#FFE4E1', - moccasin: '#FFE4B5', - navajowhite: '#FFDEAD', - oldlace: '#FDF5E6', - olivedrab: '#6B8E23', - orangered: '#FF4500', - orchid: '#DA70D6', - palegoldenrod: '#EEE8AA', - palegreen: '#98FB98', - paleturquoise: '#AFEEEE', - palevioletred: '#DB7093', - papayawhip: '#FFEFD5', - peachpuff: '#FFDAB9', - peru: '#CD853F', - pink: '#FFC0CB', - plum: '#DDA0DD', - powderblue: '#B0E0E6', - rosybrown: '#BC8F8F', - royalblue: '#4169E1', - saddlebrown: '#8B4513', - salmon: '#FA8072', - sandybrown: '#F4A460', - seagreen: '#2E8B57', - seashell: '#FFF5EE', - sienna: '#A0522D', - skyblue: '#87CEEB', - slateblue: '#6A5ACD', - slategray: '#708090', - slategrey: '#708090', - snow: '#FFFAFA', - springgreen: '#00FF7F', - steelblue: '#4682B4', - tan: '#D2B48C', - thistle: '#D8BFD8', - tomato: '#FF6347', - turquoise: '#40E0D0', - violet: '#EE82EE', - wheat: '#F5DEB3', - whitesmoke: '#F5F5F5', - yellowgreen: '#9ACD32', - rebeccapurple: '#663399' - }.freeze - def convert_color_name_to_hex - color = params[:color] - color_name = color.strip.downcase - - return color if color_name.start_with?('#') - - COLOR_NAME_TO_HEX[color_name.to_sym] || color + ::Gitlab::Color.of(params[:color]) end end end diff --git a/app/services/security/merge_reports_service.rb b/app/services/security/merge_reports_service.rb index 5f6f98a3c39..a982ec7efe2 100644 --- a/app/services/security/merge_reports_service.rb +++ b/app/services/security/merge_reports_service.rb @@ -21,7 +21,10 @@ module Security source_reports.first.type, source_reports.first.pipeline, source_reports.first.created_at - ).tap { |report| report.errors = source_reports.flat_map(&:errors) } + ).tap do |report| + report.errors = source_reports.flat_map(&:errors) + report.warnings = source_reports.flat_map(&:warnings) + end end def copy_resources_to_target_report diff --git a/app/validators/color_validator.rb b/app/validators/color_validator.rb index 974dfbbf394..d108e4c5426 100644 --- a/app/validators/color_validator.rb +++ b/app/validators/color_validator.rb @@ -12,11 +12,13 @@ # end # class ColorValidator < ActiveModel::EachValidator - PATTERN = /\A\#(?:[0-9A-Fa-f]{3}){1,2}\Z/.freeze - def validate_each(record, attribute, value) - unless value =~ PATTERN - record.errors.add(attribute, "must be a valid color code") + case value + when NilClass then return + when ::Gitlab::Color then return if value.valid? + when ::String then return if ::Gitlab::Color.new(value).valid? end + + record.errors.add(attribute, "must be a valid color code") end end diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml index aae6212f964..67f24383b17 100644 --- a/app/views/profiles/two_factor_auths/show.html.haml +++ b/app/views/profiles/two_factor_auths/show.html.haml @@ -113,7 +113,7 @@ %span.gl-text-gray-500 = _("no name set") %td= registration[:created_at].to_date.to_s(:medium) - %td= link_to _('Delete'), registration[:delete_path], method: :delete, class: "gl-button btn btn-danger float-right", data: { confirm: _('Are you sure you want to delete this device? This action cannot be undone.') } + %td= link_to _('Delete'), registration[:delete_path], method: :delete, class: "gl-button btn btn-danger float-right", data: { confirm: _('Are you sure you want to delete this device? This action cannot be undone.'), confirm_btn_variant: "danger" }, aria: { label: _('Delete') } - else .settings-message.text-center diff --git a/config/feature_flags/development/enforce_security_report_validation.yml b/config/feature_flags/development/show_report_validation_warnings.yml index ada5863b4d7..b184deecc0b 100644 --- a/config/feature_flags/development/enforce_security_report_validation.yml +++ b/config/feature_flags/development/show_report_validation_warnings.yml @@ -1,8 +1,8 @@ --- -name: enforce_security_report_validation -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79798 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351000 -milestone: '14.8' +name: show_report_validation_warnings +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80930 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353125 +milestone: '14.9' type: development group: group::threat insights default_enabled: false diff --git a/db/migrate/20220204193000_add_integrations_encrypted_properties.rb b/db/migrate/20220204193000_add_integrations_encrypted_properties.rb new file mode 100644 index 00000000000..7df88b68657 --- /dev/null +++ b/db/migrate/20220204193000_add_integrations_encrypted_properties.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddIntegrationsEncryptedProperties < Gitlab::Database::Migration[1.0] + def change + add_column :integrations, :encrypted_properties, :binary + add_column :integrations, :encrypted_properties_iv, :binary + end +end diff --git a/db/post_migrate/20220204194347_encrypt_integration_properties.rb b/db/post_migrate/20220204194347_encrypt_integration_properties.rb new file mode 100644 index 00000000000..82dd3a05e1d --- /dev/null +++ b/db/post_migrate/20220204194347_encrypt_integration_properties.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class EncryptIntegrationProperties < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + MIGRATION = 'EncryptIntegrationProperties' + BATCH_SIZE = 1_000 + INTERVAL = 2.minutes.to_i + + def up + queue_background_migration_jobs_by_range_at_intervals( + define_batchable_model('integrations').all, + MIGRATION, + INTERVAL, + track_jobs: true, + batch_size: BATCH_SIZE + ) + end + + def down + # this migration is not reversible + end +end diff --git a/db/schema_migrations/20220204193000 b/db/schema_migrations/20220204193000 new file mode 100644 index 00000000000..f0d16b9671c --- /dev/null +++ b/db/schema_migrations/20220204193000 @@ -0,0 +1 @@ +9d98618a1e9fd0474c45ac54420fc64a1d90ad77f36be594337e5b117fccdadb
\ No newline at end of file diff --git a/db/schema_migrations/20220204194347 b/db/schema_migrations/20220204194347 new file mode 100644 index 00000000000..d506497e036 --- /dev/null +++ b/db/schema_migrations/20220204194347 @@ -0,0 +1 @@ +1593e935601ae1f2ab788109687bb40bad026f3f425339a39c8d13d3e4c7e306
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8437f9bb2b5..357e332ee39 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16093,6 +16093,8 @@ CREATE TABLE integrations ( type_new text, vulnerability_events boolean DEFAULT false NOT NULL, archive_trace_events boolean DEFAULT false NOT NULL, + encrypted_properties bytea, + encrypted_properties_iv bytea, CONSTRAINT check_a948a0aa7e CHECK ((char_length(type_new) <= 255)) ); diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 33c45443f69..36c4465add5 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -15255,6 +15255,7 @@ Represents the security scan information. | ---- | ---- | ----------- | | <a id="scanerrors"></a>`errors` | [`[String!]!`](#string) | List of errors. | | <a id="scanname"></a>`name` | [`String!`](#string) | Name of the scan. | +| <a id="scanwarnings"></a>`warnings` | [`[String!]!`](#string) | List of warnings. | ### `ScanExecutionPolicy` @@ -19814,8 +19815,8 @@ Input type for DastSiteProfile authentication. | ---- | ---- | ----------- | | <a id="diffpositioninputbasesha"></a>`baseSha` | [`String`](#string) | Merge base of the branch the comment was made on. | | <a id="diffpositioninputheadsha"></a>`headSha` | [`String!`](#string) | SHA of the HEAD at the time the comment was made. | -| <a id="diffpositioninputnewline"></a>`newLine` | [`Int`](#int) | Line on HEAD SHA that was changed. | -| <a id="diffpositioninputoldline"></a>`oldLine` | [`Int`](#int) | Line on start SHA that was changed. | +| <a id="diffpositioninputnewline"></a>`newLine` | [`Int`](#int) | Line on HEAD SHA that was changed. Please see the [REST API Documentation](https://docs.gitlab.com/ee/api/discussions.html#create-a-new-thread-in-the-merge-request-diff) for more information on how to use this field. | +| <a id="diffpositioninputoldline"></a>`oldLine` | [`Int`](#int) | Line on start SHA that was changed. Please see the [REST API Documentation](https://docs.gitlab.com/ee/api/discussions.html#create-a-new-thread-in-the-merge-request-diff) for more information on how to use this field. | | <a id="diffpositioninputpaths"></a>`paths` | [`DiffPathsInput!`](#diffpathsinput) | The paths of the file that was changed. Both of the properties of this input are optional, but at least one of them is required. | | <a id="diffpositioninputstartsha"></a>`startSha` | [`String!`](#string) | SHA of the branch being compared against. | diff --git a/doc/development/secure_coding_guidelines.md b/doc/development/secure_coding_guidelines.md index 8d77467d1b5..bc876667f8d 100644 --- a/doc/development/secure_coding_guidelines.md +++ b/doc/development/secure_coding_guidelines.md @@ -460,8 +460,7 @@ parameter when using `check_allowed_absolute_path!()`. To use a combination of both checks, follow the example below: ```ruby -path = Gitlab::Utils.check_path_traversal!(path) -Gitlab::Utils.check_allowed_absolute_path!(path, path_allowlist) +Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) ``` In the REST API, we have the [`FilePath`](https://gitlab.com/gitlab-org/security/gitlab/-/blob/master/lib/api/validations/validators/file_path.rb) diff --git a/doc/security/two_factor_authentication.md b/doc/security/two_factor_authentication.md index d0842ddb103..cab9f6a957e 100644 --- a/doc/security/two_factor_authentication.md +++ b/doc/security/two_factor_authentication.md @@ -33,10 +33,10 @@ To enable 2FA for all users: If you want 2FA enforcement to take effect during the next sign-in attempt, change the grace period to `0`. -## Disable 2FA enforcement through rails console +## Disable 2FA enforcement through Rails console -Using the [rails console](../administration/operations/rails_console.md), enforcing 2FA for -all user can be disabled. Connect to the rails console and run: +Using the [Rails console](../administration/operations/rails_console.md), enforcing 2FA for +all user can be disabled. Connect to the Rails console and run: ```ruby Gitlab::CurrentSettings.update!('require_two_factor_authentication': false) @@ -108,13 +108,10 @@ reactivate 2FA from scratch if they want to use it again. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/270554) in GitLab 13.7. > - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/299088) from GitLab Free to GitLab Premium in 13.9. -> - It's [deployed behind a feature flag](../user/feature_flags.md), disabled by default. -> - It's disabled on GitLab.com. -> - It's not recommended for production use. -> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-2fa-for-git-operations). +> - It's deployed behind a feature flag, disabled by default. -WARNING: -This feature might not be available to you. Check the **version history** note above for details. +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../administration/feature_flags.md) named `two_factor_for_cli`. On GitLab.com, this feature is not available. The feature is not ready for production use. This feature flag also affects [session duration for Git Operations when 2FA is enabled](../user/admin_area/settings/account_and_limit_settings.md#customize-session-duration-for-git-operations-when-2fa-is-enabled). Two-factor authentication can be enforced for Git over SSH operations. However, we recommend using [ED25519_SK](../ssh/index.md#ed25519_sk-ssh-keys) or [ECDSA_SK](../ssh/index.md#ecdsa_sk-ssh-keys) SSH keys instead. @@ -135,30 +132,6 @@ After the OTP is verified, Git over SSH operations can be used for a session dur Once an OTP is verified, anyone can run Git over SSH with that private SSH key for the configured [session duration](../user/admin_area/settings/account_and_limit_settings.md#customize-session-duration-for-git-operations-when-2fa-is-enabled). -### Enable or disable 2FA for Git operations - -2FA for Git operations is under development and not -ready for production use. It is deployed behind a feature flag that is -**disabled by default**. [GitLab administrators with access to the GitLab Rails console](../administration/feature_flags.md) -can enable it. - -To enable it: - -```ruby -Feature.enable(:two_factor_for_cli) -``` - -To disable it: - -```ruby -Feature.disable(:two_factor_for_cli) -``` - -The feature flag affects these features: - -- [Two-factor Authentication (2FA) for Git over SSH operations](#2fa-for-git-over-ssh-operations). -- [Customize session duration for Git Operations when 2FA is enabled](../user/admin_area/settings/account_and_limit_settings.md#customize-session-duration-for-git-operations-when-2fa-is-enabled). - <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/user/admin_area/settings/account_and_limit_settings.md b/doc/user/admin_area/settings/account_and_limit_settings.md index 1d982196228..2f30298644a 100644 --- a/doc/user/admin_area/settings/account_and_limit_settings.md +++ b/doc/user/admin_area/settings/account_and_limit_settings.md @@ -178,14 +178,9 @@ nginx['client_max_body_size'] = "200m" > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/296669) in GitLab 13.9. > - It's deployed behind a feature flag, disabled by default. -> - It's disabled on GitLab.com. -> - It's not recommended for production use. -> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](../../../security/two_factor_authentication.md#enable-or-disable-2fa-for-git-operations). -NOTE: -This feature is under development and not ready for production use. It is deployed -behind a feature flag that is **disabled by default**. To use it in GitLab -self-managed instances, ask a GitLab administrator to [enable it](../../../security/two_factor_authentication.md#enable-or-disable-2fa-for-git-operations). +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `two_factor_for_cli`. On GitLab.com, this feature is not available. This feature is not ready for production use. This feature flag also affects [2FA for Git over SSH operations](../../../security/two_factor_authentication.md#2fa-for-git-over-ssh-operations). GitLab administrators can choose to customize the session duration (in minutes) for Git operations when 2FA is enabled. The default is 15 and this can be set to a value between 1 and 10080. diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 2e686e1d70b..93447274805 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -33,13 +33,14 @@ usernames. A GitLab administrator can configure the GitLab instance to ## Project members permissions -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/219299) in GitLab 14.8, personal namespace owners appear with Owner role in new projects in their namespace. Introduced [with a flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`. Disabled by default. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/219299) in GitLab 14.8, personal namespace owners appear with Owner role in new projects in their namespace. Introduced [with a flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`. Disabled by default. +> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/351919) in GitLab 14.9. FLAG: On self-managed GitLab, personal namespace owners appearing with the Owner role in new projects in their namespace is disabled. To make it available, ask an administrator to [enable the feature flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`. The feature is not ready for production use. -On GitLab.com, this feature is not available. +On GitLab.com, this feature is available. A user's role determines what permissions they have on a project. The Owner role provides all permissions but is available only: diff --git a/lib/api/entities/label_basic.rb b/lib/api/entities/label_basic.rb index ed52688638e..7c846180558 100644 --- a/lib/api/entities/label_basic.rb +++ b/lib/api/entities/label_basic.rb @@ -3,7 +3,11 @@ module API module Entities class LabelBasic < Grape::Entity - expose :id, :name, :color, :description, :description_html, :text_color + expose :id, :name, :description, :description_html, :text_color + + expose :color do |label, options| + label.color.to_s + end end end end diff --git a/lib/api/validations/validators/file_path.rb b/lib/api/validations/validators/file_path.rb index 246c445658f..268ddc29d4e 100644 --- a/lib/api/validations/validators/file_path.rb +++ b/lib/api/validations/validators/file_path.rb @@ -8,8 +8,7 @@ module API options = @option.is_a?(Hash) ? @option : {} path_allowlist = options.fetch(:allowlist, []) path = params[attr_name] - path = Gitlab::Utils.check_path_traversal!(path) - Gitlab::Utils.check_allowed_absolute_path!(path, path_allowlist) + Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) rescue StandardError raise Grape::Exceptions::Validation.new( params: [@scope.full_name(attr_name)], diff --git a/lib/gitlab/background_migration/encrypt_integration_properties.rb b/lib/gitlab/background_migration/encrypt_integration_properties.rb new file mode 100644 index 00000000000..3843356af69 --- /dev/null +++ b/lib/gitlab/background_migration/encrypt_integration_properties.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Migrates the integration.properties column from plaintext to encrypted text. + class EncryptIntegrationProperties + # The Integration model, with just the relevant bits. + class Integration < ActiveRecord::Base + include EachBatch + + ALGORITHM = 'aes-256-gcm' + + self.table_name = 'integrations' + self.inheritance_column = :_type_disabled + + scope :with_properties, -> { where.not(properties: nil) } + scope :not_already_encrypted, -> { where(encrypted_properties: nil) } + scope :for_batch, ->(range) { where(id: range) } + + attr_encrypted :encrypted_properties_tmp, + attribute: :encrypted_properties, + mode: :per_attribute_iv, + key: ::Settings.attr_encrypted_db_key_base_32, + algorithm: ALGORITHM, + marshal: true, + marshaler: ::Gitlab::Json, + encode: false, + encode_iv: false + + # See 'Integration#reencrypt_properties' + def encrypt_properties + data = ::Gitlab::Json.parse(properties) + iv = generate_iv(ALGORITHM) + ep = self.class.encrypt(:encrypted_properties_tmp, data, { iv: iv }) + + [ep, iv] + end + end + + def perform(start_id, stop_id) + batch_query = Integration.with_properties.not_already_encrypted.for_batch(start_id..stop_id) + encrypt_batch(batch_query) + mark_job_as_succeeded(start_id, stop_id) + end + + private + + def mark_job_as_succeeded(*arguments) + Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( + self.class.name.demodulize, + arguments + ) + end + + # represent binary string as a PSQL binary literal: + # https://www.postgresql.org/docs/9.4/datatype-binary.html + def bytea(value) + "'\\x#{value.unpack1('H*')}'::bytea" + end + + def encrypt_batch(batch_query) + values = batch_query.select(:id, :properties).map do |record| + encrypted_properties, encrypted_properties_iv = record.encrypt_properties + "(#{record.id}, #{bytea(encrypted_properties)}, #{bytea(encrypted_properties_iv)})" + end + + return if values.empty? + + Integration.connection.execute(<<~SQL.squish) + WITH cte(cte_id, cte_encrypted_properties, cte_encrypted_properties_iv) + AS #{::Gitlab::Database::AsWithMaterialized.materialized_if_supported} ( + SELECT * + FROM (VALUES #{values.join(',')}) AS t (id, encrypted_properties, encrypted_properties_iv) + ) + UPDATE #{Integration.table_name} + SET encrypted_properties = cte_encrypted_properties + , encrypted_properties_iv = cte_encrypted_properties_iv + FROM cte + WHERE cte_id = id + SQL + end + end + end +end diff --git a/lib/gitlab/ci/parsers/security/common.rb b/lib/gitlab/ci/parsers/security/common.rb index f57959f9547..ff8641c014d 100644 --- a/lib/gitlab/ci/parsers/security/common.rb +++ b/lib/gitlab/ci/parsers/security/common.rb @@ -42,14 +42,19 @@ module Gitlab attr_reader :json_data, :report, :validate def valid? - if Feature.enabled?(:enforce_security_report_validation) - if !validate || schema_validator.valid? - report.schema_validation_status = :valid_schema - true + if Feature.enabled?(:show_report_validation_warnings) + # We want validation to happen regardless of VALIDATE_SCHEMA CI variable + schema_validation_passed = schema_validator.valid? + + if validate + schema_validator.errors.each { |error| report.add_error('Schema', error) } unless schema_validation_passed + + schema_validation_passed else - report.schema_validation_status = :invalid_schema - schema_validator.errors.each { |error| report.add_error('Schema', error) } - false + # We treat all schema validation errors as warnings + schema_validator.errors.each { |error| report.add_warning('Schema', error) } + + true end else return true if !validate || schema_validator.valid? diff --git a/lib/gitlab/ci/reports/security/report.rb b/lib/gitlab/ci/reports/security/report.rb index fbf8c81ac36..8c528056d0c 100644 --- a/lib/gitlab/ci/reports/security/report.rb +++ b/lib/gitlab/ci/reports/security/report.rb @@ -6,7 +6,7 @@ module Gitlab module Security class Report attr_reader :created_at, :type, :pipeline, :findings, :scanners, :identifiers - attr_accessor :scan, :scanned_resources, :errors, :analyzer, :version, :schema_validation_status + attr_accessor :scan, :scanned_resources, :errors, :analyzer, :version, :schema_validation_status, :warnings delegate :project_id, to: :pipeline @@ -19,6 +19,7 @@ module Gitlab @identifiers = {} @scanned_resources = [] @errors = [] + @warnings = [] end def commit_sha @@ -29,6 +30,10 @@ module Gitlab errors << { type: type, message: message } end + def add_warning(type, message) + warnings << { type: type, message: message } + end + def errored? errors.present? end diff --git a/lib/gitlab/color.rb b/lib/gitlab/color.rb new file mode 100644 index 00000000000..e0caabb0ec6 --- /dev/null +++ b/lib/gitlab/color.rb @@ -0,0 +1,222 @@ +# frozen_string_literal: true + +module Gitlab + class Color + PATTERN = /\A\#(?:[0-9A-Fa-f]{3}){1,2}\Z/.freeze + + def initialize(value) + @value = value&.strip&.freeze + end + + module Constants + DARK = Color.new('#333333') + LIGHT = Color.new('#FFFFFF') + + COLOR_NAME_TO_HEX = { + black: '#000000', + silver: '#C0C0C0', + gray: '#808080', + white: '#FFFFFF', + maroon: '#800000', + red: '#FF0000', + purple: '#800080', + fuchsia: '#FF00FF', + green: '#008000', + lime: '#00FF00', + olive: '#808000', + yellow: '#FFFF00', + navy: '#000080', + blue: '#0000FF', + teal: '#008080', + aqua: '#00FFFF', + orange: '#FFA500', + aliceblue: '#F0F8FF', + antiquewhite: '#FAEBD7', + aquamarine: '#7FFFD4', + azure: '#F0FFFF', + beige: '#F5F5DC', + bisque: '#FFE4C4', + blanchedalmond: '#FFEBCD', + blueviolet: '#8A2BE2', + brown: '#A52A2A', + burlywood: '#DEB887', + cadetblue: '#5F9EA0', + chartreuse: '#7FFF00', + chocolate: '#D2691E', + coral: '#FF7F50', + cornflowerblue: '#6495ED', + cornsilk: '#FFF8DC', + crimson: '#DC143C', + darkblue: '#00008B', + darkcyan: '#008B8B', + darkgoldenrod: '#B8860B', + darkgray: '#A9A9A9', + darkgreen: '#006400', + darkgrey: '#A9A9A9', + darkkhaki: '#BDB76B', + darkmagenta: '#8B008B', + darkolivegreen: '#556B2F', + darkorange: '#FF8C00', + darkorchid: '#9932CC', + darkred: '#8B0000', + darksalmon: '#E9967A', + darkseagreen: '#8FBC8F', + darkslateblue: '#483D8B', + darkslategray: '#2F4F4F', + darkslategrey: '#2F4F4F', + darkturquoise: '#00CED1', + darkviolet: '#9400D3', + deeppink: '#FF1493', + deepskyblue: '#00BFFF', + dimgray: '#696969', + dimgrey: '#696969', + dodgerblue: '#1E90FF', + firebrick: '#B22222', + floralwhite: '#FFFAF0', + forestgreen: '#228B22', + gainsboro: '#DCDCDC', + ghostwhite: '#F8F8FF', + gold: '#FFD700', + goldenrod: '#DAA520', + greenyellow: '#ADFF2F', + grey: '#808080', + honeydew: '#F0FFF0', + hotpink: '#FF69B4', + indianred: '#CD5C5C', + indigo: '#4B0082', + ivory: '#FFFFF0', + khaki: '#F0E68C', + lavender: '#E6E6FA', + lavenderblush: '#FFF0F5', + lawngreen: '#7CFC00', + lemonchiffon: '#FFFACD', + lightblue: '#ADD8E6', + lightcoral: '#F08080', + lightcyan: '#E0FFFF', + lightgoldenrodyellow: '#FAFAD2', + lightgray: '#D3D3D3', + lightgreen: '#90EE90', + lightgrey: '#D3D3D3', + lightpink: '#FFB6C1', + lightsalmon: '#FFA07A', + lightseagreen: '#20B2AA', + lightskyblue: '#87CEFA', + lightslategray: '#778899', + lightslategrey: '#778899', + lightsteelblue: '#B0C4DE', + lightyellow: '#FFFFE0', + limegreen: '#32CD32', + linen: '#FAF0E6', + mediumaquamarine: '#66CDAA', + mediumblue: '#0000CD', + mediumorchid: '#BA55D3', + mediumpurple: '#9370DB', + mediumseagreen: '#3CB371', + mediumslateblue: '#7B68EE', + mediumspringgreen: '#00FA9A', + mediumturquoise: '#48D1CC', + mediumvioletred: '#C71585', + midnightblue: '#191970', + mintcream: '#F5FFFA', + mistyrose: '#FFE4E1', + moccasin: '#FFE4B5', + navajowhite: '#FFDEAD', + oldlace: '#FDF5E6', + olivedrab: '#6B8E23', + orangered: '#FF4500', + orchid: '#DA70D6', + palegoldenrod: '#EEE8AA', + palegreen: '#98FB98', + paleturquoise: '#AFEEEE', + palevioletred: '#DB7093', + papayawhip: '#FFEFD5', + peachpuff: '#FFDAB9', + peru: '#CD853F', + pink: '#FFC0CB', + plum: '#DDA0DD', + powderblue: '#B0E0E6', + rosybrown: '#BC8F8F', + royalblue: '#4169E1', + saddlebrown: '#8B4513', + salmon: '#FA8072', + sandybrown: '#F4A460', + seagreen: '#2E8B57', + seashell: '#FFF5EE', + sienna: '#A0522D', + skyblue: '#87CEEB', + slateblue: '#6A5ACD', + slategray: '#708090', + slategrey: '#708090', + snow: '#FFFAFA', + springgreen: '#00FF7F', + steelblue: '#4682B4', + tan: '#D2B48C', + thistle: '#D8BFD8', + tomato: '#FF6347', + turquoise: '#40E0D0', + violet: '#EE82EE', + wheat: '#F5DEB3', + whitesmoke: '#F5F5F5', + yellowgreen: '#9ACD32', + rebeccapurple: '#663399' + }.stringify_keys.transform_values { Color.new(_1) }.freeze + end + + def self.of(color) + raise ArgumentError, 'No color spec' unless color + return color if color.is_a?(self) + + color = color.to_s.strip + Constants::COLOR_NAME_TO_HEX[color.downcase] || new(color) + end + + def to_s + @value.to_s + end + + def as_json(_options = nil) + to_s + end + + def eql(other) + return false unless other.is_a?(self.class) + + to_s == other.to_s + end + alias_method :==, :eql + + def valid? + PATTERN.match?(@value) + end + + def light? + valid? && rgb.sum > 500 + end + + def luminosity + return :light if light? + + :dark + end + + def contrast + return Constants::DARK if light? + + Constants::LIGHT + end + + private + + def rgb + return [] unless valid? + + @rgb ||= begin + if @value.length == 4 + @value[1, 4].scan(/./).map { |v| (v * 2).hex } + else + @value[1, 7].scan(/.{2}/).map(&:hex) + end + end + end + end +end diff --git a/lib/gitlab/database/type/color.rb b/lib/gitlab/database/type/color.rb new file mode 100644 index 00000000000..32ea33a42f3 --- /dev/null +++ b/lib/gitlab/database/type/color.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Type + class Color < ActiveModel::Type::Value + def serialize(value) + value.to_s if value + end + + def serializable?(value) + value.nil? || value.is_a?(::String) || value.is_a?(::Gitlab::Color) + end + + def cast_value(value) + ::Gitlab::Color.new(value.to_s) + end + end + end + end +end diff --git a/lib/gitlab/json.rb b/lib/gitlab/json.rb index 368b621bdfb..9824b46554f 100644 --- a/lib/gitlab/json.rb +++ b/lib/gitlab/json.rb @@ -16,6 +16,9 @@ module Gitlab # @return [Boolean, String, Array, Hash] # @raise [JSON::ParserError] raised if parsing fails def parse(string, opts = {}) + # Parse nil as nil + return if string.nil? + # First we should ensure this really is a string, not some other # type which purports to be a string. This handles some legacy # usage of the JSON class. @@ -30,6 +33,7 @@ module Gitlab end alias_method :parse!, :parse + alias_method :load, :parse # Restricted method for converting a Ruby object to JSON. If you # need to pass options to this, you should use `.generate` instead, @@ -67,6 +71,14 @@ module Gitlab ::JSON.pretty_generate(object, opts) end + # The standard parser error we should be returning. Defined in a method + # so we can potentially override it later. + # + # @return [JSON::ParserError] + def parser_error + ::JSON::ParserError + end + private # Convert JSON string into Ruby through toggleable adapters. @@ -134,14 +146,6 @@ module Gitlab opts end - # The standard parser error we should be returning. Defined in a method - # so we can potentially override it later. - # - # @return [JSON::ParserError] - def parser_error - ::JSON::ParserError - end - # @param [Nil, Boolean] an extracted :legacy_mode key from the opts hash # @return [Boolean] def legacy_mode_enabled?(arg_value) diff --git a/lib/gitlab/performance_bar/stats.rb b/lib/gitlab/performance_bar/stats.rb index cf524e69454..8743772eef6 100644 --- a/lib/gitlab/performance_bar/stats.rb +++ b/lib/gitlab/performance_bar/stats.rb @@ -25,8 +25,8 @@ module Gitlab log_queries(id, data, 'active-record') log_queries(id, data, 'gitaly') log_queries(id, data, 'redis') - rescue StandardError => err - logger.error(message: "failed to process request id #{id}: #{err.message}") + rescue StandardError => e + logger.error(message: "failed to process request id #{id}: #{e.message}") end private @@ -34,6 +34,8 @@ module Gitlab def request(id) # Peek gem stores request data under peek:requests:request_id key json_data = @redis.get("peek:requests:#{id}") + raise "No data for #{id}" if json_data.nil? + Gitlab::Json.parse(json_data) end diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index 608545baf74..a337fcc43c4 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -37,6 +37,13 @@ module Gitlab raise StandardError, "path #{path} is not allowed" end + def check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) + traversal_path = check_path_traversal!(path) + raise StandardError, "path is not a string!" unless traversal_path.is_a?(String) + + check_allowed_absolute_path!(traversal_path, path_allowlist) + end + def decode_path(encoded_path) decoded = CGI.unescape(encoded_path) if decoded != CGI.unescape(decoded) diff --git a/qa/qa/resource/group_base.rb b/qa/qa/resource/group_base.rb index f34911e901b..db6272ff749 100644 --- a/qa/qa/resource/group_base.rb +++ b/qa/qa/resource/group_base.rb @@ -65,7 +65,7 @@ module QA end def marked_for_deletion? - !parse_body(api_get_from("#{api_get_path}"))[:marked_for_deletion_on].nil? + !parse_body(api_get_from(api_get_path.to_s))[:marked_for_deletion_on].nil? end # Get group badges @@ -84,22 +84,6 @@ module QA end end - # Get group members - # - # @return [Array<QA::Resource::User>] - def members - parse_body(api_get_from("#{api_get_path}/members")).map do |member| - User.init do |resource| - resource.api_client = api_client - resource.id = member[:id] - resource.name = member[:name] - resource.username = member[:username] - resource.email = member[:email] - resource.access_level = member[:access_level] - end - end - end - # API get path # # @return [String] diff --git a/qa/qa/resource/reusable_collection.rb b/qa/qa/resource/reusable_collection.rb index 1566641cb7d..27d34b16e2f 100644 --- a/qa/qa/resource/reusable_collection.rb +++ b/qa/qa/resource/reusable_collection.rb @@ -36,6 +36,10 @@ module QA next QA::Runtime::Logger.debug("#{resource.class.name} reused as :#{reuse_as} has already been removed.") unless resource.exists? next if resource.respond_to?(:marked_for_deletion?) && resource.marked_for_deletion? + if resource.reload!.api_resource[:marked_for_deletion_on].present? + next QA::Runtime::Logger.debug("#{resource.class.name} reused as :#{reuse_as} is already scheduled to be removed.") + end + resource.method(:remove_via_api!).super_method.call end end diff --git a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_group_spec.rb b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_group_spec.rb index 0f0e6f011d2..99a78f15b1f 100644 --- a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_group_spec.rb +++ b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_group_spec.rb @@ -147,39 +147,6 @@ module QA end end end - - context 'with group members' do - let(:member) do - Resource::User.fabricate_via_api! do |usr| - usr.api_client = admin_api_client - usr.hard_delete_on_api_removal = true - end - end - - before do - member.set_public_email - source_group.add_member(member, Resource::Members::AccessLevel::DEVELOPER) - - imported_group # trigger import - end - - after do - member.remove_via_api! - end - - it( - 'adds members for imported group', - testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347609' - ) do - expect { imported_group.import_status }.to eventually_eq('finished').within(import_wait_duration) - - imported_member = imported_group.reload!.members.find { |usr| usr.username == member.username } - aggregate_failures do - expect(imported_member).not_to be_nil - expect(imported_member.access_level).to eq(Resource::Members::AccessLevel::DEVELOPER) - end - end - end end end end diff --git a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_members_spec.rb b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_members_spec.rb new file mode 100644 index 00000000000..6bc7fd65db1 --- /dev/null +++ b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_members_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require_relative 'gitlab_project_migration_common' + +module QA + RSpec.describe 'Manage' do + describe 'Gitlab migration' do + include_context 'with gitlab project migration' + + let(:member) do + Resource::User.fabricate_via_api! do |usr| + usr.api_client = admin_api_client + usr.hard_delete_on_api_removal = true + end + end + + let(:imported_group_member) do + imported_group.reload!.list_members.find { |usr| usr['username'] == member.username } + end + + let(:imported_project_member) do + imported_project.reload!.list_members.find { |usr| usr['username'] == member.username } + end + + before do + member.set_public_email + end + + after do + member.remove_via_api! + end + + context 'with group member' do + before do + source_group.add_member(member, Resource::Members::AccessLevel::DEVELOPER) + end + + it 'member retains indirect membership in imported project' do + expect_import_finished + + aggregate_failures do + expect(imported_project_member).to be_nil + expect(imported_group_member&.fetch('access_level')).to eq( + Resource::Members::AccessLevel::DEVELOPER + ) + end + end + end + + context 'with project member' do + before do + source_project.add_member(member, Resource::Members::AccessLevel::DEVELOPER) + end + + it 'member retains direct membership in imported project' do + expect_import_finished + + aggregate_failures do + expect(imported_group_member).to be_nil + expect(imported_project_member&.fetch('access_level')).to eq( + Resource::Members::AccessLevel::DEVELOPER + ) + end + end + end + end + end +end diff --git a/qa/spec/resource/reusable_collection_spec.rb b/qa/spec/resource/reusable_collection_spec.rb index 9116462b396..cb2df6931d0 100644 --- a/qa/spec/resource/reusable_collection_spec.rb +++ b/qa/spec/resource/reusable_collection_spec.rb @@ -20,6 +20,10 @@ RSpec.describe QA::Resource::ReusableCollection do end def exists?() end + + def reload! + Struct.new(:api_resource).new({ marked_for_deletion_on: false }) + end end end @@ -88,8 +92,22 @@ RSpec.describe QA::Resource::ReusableCollection do it 'removes each instance of each resource class' do described_class.remove_all_via_api! - expect(a_resource_instance.removed).to be true - expect(another_resource_instance.removed).to be true + expect(a_resource_instance.removed).to be_truthy + expect(another_resource_instance.removed).to be_truthy + end + + context 'when a resource is marked for deletion' do + before do + marked_for_deletion = Struct.new(:api_resource).new({ marked_for_deletion_on: true }) + + allow(a_resource_instance).to receive(:reload!).and_return(marked_for_deletion) + allow(another_resource_instance).to receive(:reload!).and_return(marked_for_deletion) + end + + it 'does not remove the resource' do + expect(a_resource_instance.removed).to be_falsey + expect(another_resource_instance.removed).to be_falsy + end end end 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 |