diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-02-25 22:18:22 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-02-25 22:18:22 +0000 |
commit | a7c021d1ebdd162d670a7945f759438eb16f9ab7 (patch) | |
tree | 93e2c6b649d4f6b4f683688a9ee346d353f3a4c8 | |
parent | 3a958c4937d67378e1dc22d38a6e2f55112fd173 (diff) | |
parent | c5048370a67df387d3940433641906ddabb15ce6 (diff) | |
download | gitlab-ce-a7c021d1ebdd162d670a7945f759438eb16f9ab7.tar.gz |
Merge remote-tracking branch 'dev/14-6-stable' into 14-6-stable
42 files changed, 653 insertions, 72 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c356e303f7..39ae46d244d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.6.5 (2022-02-25) + +### Security (8 changes) + +- [Limit commands_changes to certain keys](gitlab-org/security/gitlab@138c437f2819d62ce4750fb84399d8868c844b01) ([merge request](gitlab-org/security/gitlab!2227)) +- [Add runners_token prefix to Group and Project](gitlab-org/security/gitlab@682d4e9b63d3d36901638edc75c1b265460d42dc) ([merge request](gitlab-org/security/gitlab!2250)) +- [Anonymous user can enumerate all users through GraphQL endpoint](gitlab-org/security/gitlab@2b00a8036b291d3ad5de551a5e13c2a0a39d0234) ([merge request](gitlab-org/security/gitlab!2102)) +- [Check for unsafe characters in email addresses before sending](gitlab-org/security/gitlab@6bc653b3dadefb3d2c80823786d43e6b7f8c4620) ([merge request](gitlab-org/security/gitlab!2208)) +- [Warn when snippet contains unretrievable files](gitlab-org/security/gitlab@f9ae9515ec98ab934f4aa3a35af0aca806bbe21d) ([merge request](gitlab-org/security/gitlab!2203)) +- [Prevent DOS when rendering math markdown](gitlab-org/security/gitlab@fd6d496df6f4b5eb3da0b851f9ff8ebb1d68d3f2) ([merge request](gitlab-org/security/gitlab!2201)) +- [Check permission when creating members through service](gitlab-org/security/gitlab@948e5103285de2a6cdb5152ff2c13ae4db2f4cda) ([merge request](gitlab-org/security/gitlab!2211)) +- [Reset password field on page load](gitlab-org/security/gitlab@1417b463f2771a4b17e068dea9de3aa6c4540962) ([merge request](gitlab-org/security/gitlab!2194)) + ## 14.6.4 (2022-02-03) No changes. diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 7d6296b6cb9..5ac2cb57085 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -14.6.4
\ No newline at end of file +14.6.5
\ No newline at end of file @@ -1 +1 @@ -14.6.4
\ No newline at end of file +14.6.5
\ No newline at end of file diff --git a/app/assets/javascripts/mirrors/mirror_repos.js b/app/assets/javascripts/mirrors/mirror_repos.js index e59da18fb77..5bf08be1ead 100644 --- a/app/assets/javascripts/mirrors/mirror_repos.js +++ b/app/assets/javascripts/mirrors/mirror_repos.js @@ -6,6 +6,8 @@ import { __ } from '~/locale'; import { hide } from '~/tooltips'; import SSHMirror from './ssh_mirror'; +const PASSWORD_FIELD_SELECTOR = '.js-mirror-password-field'; + export default class MirrorRepos { constructor(container) { this.$container = $(container); @@ -27,7 +29,6 @@ export default class MirrorRepos { this.$passwordGroup = $('.js-password-group', this.$container); this.$password = $('.js-password', this.$passwordGroup); this.$authMethod = $('.js-auth-method', this.$form); - this.$keepDivergentRefsInput.on('change', () => this.updateKeepDivergentRefs()); this.$authMethod.on('change', () => this.togglePassword()); this.$password.on('input.updateUrl', () => this.debouncedUpdateUrl()); @@ -35,6 +36,13 @@ export default class MirrorRepos { this.initMirrorSSH(); this.updateProtectedBranches(); this.updateKeepDivergentRefs(); + MirrorRepos.resetPasswordField(); + } + + static resetPasswordField() { + if (document.querySelector(PASSWORD_FIELD_SELECTOR)) { + document.querySelector(PASSWORD_FIELD_SELECTOR).value = ''; + } } initMirrorSSH() { diff --git a/app/assets/javascripts/snippets/components/show.vue b/app/assets/javascripts/snippets/components/show.vue index 35d88d5ec8e..ee8b00c1f5d 100644 --- a/app/assets/javascripts/snippets/components/show.vue +++ b/app/assets/javascripts/snippets/components/show.vue @@ -1,5 +1,5 @@ <script> -import { GlLoadingIcon } from '@gitlab/ui'; +import { GlAlert, GlLoadingIcon } from '@gitlab/ui'; import eventHub from '~/blob/components/eventhub'; import { SNIPPET_MARK_VIEW_APP_START, @@ -23,6 +23,7 @@ export default { EmbedDropdown, SnippetHeader, SnippetTitle, + GlAlert, GlLoadingIcon, SnippetBlob, CloneDropdownButton, @@ -35,6 +36,9 @@ export default { canBeCloned() { return Boolean(this.snippet.sshUrlToRepo || this.snippet.httpUrlToRepo); }, + hasUnretrievableBlobs() { + return this.snippet.hasUnretrievableBlobs; + }, }, beforeCreate() { performanceMarkAndMeasure({ mark: SNIPPET_MARK_VIEW_APP_START }); @@ -66,6 +70,13 @@ export default { data-qa-selector="clone_button" /> </div> + <gl-alert v-if="hasUnretrievableBlobs" variant="danger" class="gl-mb-3" :dismissible="false"> + {{ + __( + 'WARNING: This snippet contains hidden files which might be used to mask malicious behavior. Exercise caution if cloning and executing code from this snippet.', + ) + }} + </gl-alert> <snippet-blob v-for="blob in blobs" :key="blob.path" diff --git a/app/assets/javascripts/snippets/mixins/snippets.js b/app/assets/javascripts/snippets/mixins/snippets.js index b72befef56b..0b3cca4e53a 100644 --- a/app/assets/javascripts/snippets/mixins/snippets.js +++ b/app/assets/javascripts/snippets/mixins/snippets.js @@ -17,6 +17,7 @@ export const getSnippetMixin = { // Set `snippet.blobs` since some child components are coupled to this. if (!isEmpty(res)) { + res.hasUnretrievableBlobs = res.blobs?.hasUnretrievableBlobs || false; // It's possible for us to not get any blobs in a response. // In this case, we should default to current blobs. res.blobs = res.blobs ? res.blobs.nodes : blobsDefault; diff --git a/app/graphql/queries/snippet/snippet.query.graphql b/app/graphql/queries/snippet/snippet.query.graphql index 24b268ec853..5c0c7ebaa1b 100644 --- a/app/graphql/queries/snippet/snippet.query.graphql +++ b/app/graphql/queries/snippet/snippet.query.graphql @@ -15,6 +15,7 @@ query GetSnippetQuery($ids: [SnippetID!]) { sshUrlToRepo blobs { __typename + hasUnretrievableBlobs nodes { __typename binary diff --git a/app/graphql/queries/snippet/snippet_blob_content.query.graphql b/app/graphql/queries/snippet/snippet_blob_content.query.graphql index 005f42ff726..4459a5e4316 100644 --- a/app/graphql/queries/snippet/snippet_blob_content.query.graphql +++ b/app/graphql/queries/snippet/snippet_blob_content.query.graphql @@ -12,6 +12,7 @@ query SnippetBlobContent($ids: [ID!], $rich: Boolean!, $paths: [String!]) { richData @include(if: $rich) plainData @skip(if: $rich) } + hasUnretrievableBlobs } } } diff --git a/app/graphql/resolvers/snippets/blobs_resolver.rb b/app/graphql/resolvers/snippets/blobs_resolver.rb index cbbc65d7263..29716ce1394 100644 --- a/app/graphql/resolvers/snippets/blobs_resolver.rb +++ b/app/graphql/resolvers/snippets/blobs_resolver.rb @@ -19,18 +19,18 @@ module Resolvers def resolve(paths: []) return [snippet.blob] if snippet.empty_repo? - if paths.empty? - snippet.blobs - else - snippet.repository.blobs_at(transformed_blob_paths(paths)) - end - end - - private - - def transformed_blob_paths(paths) - ref = snippet.default_branch - paths.map { |path| [ref, path] } + paths = snippet.all_files if paths.empty? + blobs = snippet.blobs(paths) + + # TODO: Some blobs, e.g. those with non-utf8 filenames, are returned as nil from the + # repository. We need to provide a flag to notify the user of this until we come up with a + # way to retrieve and display these blobs. We will be exploring a more holistic solution for + # this general problem of making all blobs retrievable as part + # of https://gitlab.com/gitlab-org/gitlab/-/issues/323082, at which point this attribute may + # be removed. + context[:unretrievable_blobs?] = blobs.size < paths.size + + blobs end end end diff --git a/app/graphql/resolvers/users_resolver.rb b/app/graphql/resolvers/users_resolver.rb index c6de3dba41a..1424c14083d 100644 --- a/app/graphql/resolvers/users_resolver.rb +++ b/app/graphql/resolvers/users_resolver.rb @@ -29,7 +29,7 @@ module Resolvers description: 'Return only admin users.' def resolve(ids: nil, usernames: nil, sort: nil, search: nil, admins: nil) - authorize! + authorize!(usernames) ::UsersFinder.new(context[:current_user], finder_params(ids, usernames, sort, search, admins)).execute end @@ -46,8 +46,11 @@ module Resolvers super end - def authorize! - Ability.allowed?(context[:current_user], :read_users_list) || raise_resource_not_available_error! + def authorize!(usernames) + authorized = Ability.allowed?(context[:current_user], :read_users_list) + authorized &&= usernames.present? if context[:current_user].blank? + + raise_resource_not_available_error! unless authorized end private diff --git a/app/graphql/types/snippets/blob_connection_type.rb b/app/graphql/types/snippets/blob_connection_type.rb new file mode 100644 index 00000000000..15d26af7374 --- /dev/null +++ b/app/graphql/types/snippets/blob_connection_type.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Types + module Snippets + # rubocop: disable Graphql/AuthorizeTypes + class BlobConnectionType < GraphQL::Types::Relay::BaseConnection + field :has_unretrievable_blobs, GraphQL::Types::Boolean, null: false, + description: 'Indicates if the snippet has unretrievable blobs.', + resolver_method: :unretrievable_blobs? + + def unretrievable_blobs? + !!context[:unretrievable_blobs?] + end + end + end +end diff --git a/app/graphql/types/snippets/blob_type.rb b/app/graphql/types/snippets/blob_type.rb index 2b9b76a6194..80702c71f63 100644 --- a/app/graphql/types/snippets/blob_type.rb +++ b/app/graphql/types/snippets/blob_type.rb @@ -8,6 +8,8 @@ module Types description 'Represents the snippet blob' present_using SnippetBlobPresenter + connection_type_class(Types::Snippets::BlobConnectionType) + field :rich_data, GraphQL::Types::String, description: 'Blob highlighted data.', null: true diff --git a/app/models/concerns/token_authenticatable_strategies/encrypted.rb b/app/models/concerns/token_authenticatable_strategies/encrypted.rb index 50a2613bb10..e957d09fbc6 100644 --- a/app/models/concerns/token_authenticatable_strategies/encrypted.rb +++ b/app/models/concerns/token_authenticatable_strategies/encrypted.rb @@ -5,16 +5,18 @@ module TokenAuthenticatableStrategies def find_token_authenticatable(token, unscoped = false) return if token.blank? - if required? - find_by_encrypted_token(token, unscoped) - elsif optional? - find_by_encrypted_token(token, unscoped) || - find_by_plaintext_token(token, unscoped) - elsif migrating? - find_by_plaintext_token(token, unscoped) - else - raise ArgumentError, _("Unknown encryption strategy: %{encrypted_strategy}!") % { encrypted_strategy: encrypted_strategy } - end + instance = if required? + find_by_encrypted_token(token, unscoped) + elsif optional? + find_by_encrypted_token(token, unscoped) || + find_by_plaintext_token(token, unscoped) + elsif migrating? + find_by_plaintext_token(token, unscoped) + else + raise ArgumentError, _("Unknown encryption strategy: %{encrypted_strategy}!") % { encrypted_strategy: encrypted_strategy } + end + + instance if instance && matches_prefix?(instance, token) end def ensure_token(instance) @@ -41,9 +43,7 @@ module TokenAuthenticatableStrategies def get_token(instance) return insecure_strategy.get_token(instance) if migrating? - encrypted_token = instance.read_attribute(encrypted_field) - token = EncryptionHelper.decrypt_token(encrypted_token) - token || (insecure_strategy.get_token(instance) if optional?) + get_encrypted_token(instance) end def set_token(instance, token) @@ -69,6 +69,12 @@ module TokenAuthenticatableStrategies protected + def get_encrypted_token(instance) + encrypted_token = instance.read_attribute(encrypted_field) + token = EncryptionHelper.decrypt_token(encrypted_token) + token || (insecure_strategy.get_token(instance) if optional?) + end + def encrypted_strategy value = options[:encrypted] value = value.call if value.is_a?(Proc) @@ -95,14 +101,22 @@ module TokenAuthenticatableStrategies .new(klass, token_field, options) end + def matches_prefix?(instance, token) + prefix = options[:prefix] + prefix = prefix.call(instance) if prefix.is_a?(Proc) + prefix = '' unless prefix.is_a?(String) + + token.start_with?(prefix) + end + def token_set?(instance) - raw_token = instance.read_attribute(encrypted_field) + token = get_encrypted_token(instance) unless required? - raw_token ||= insecure_strategy.get_token(instance) + token ||= insecure_strategy.get_token(instance) end - raw_token.present? + token.present? && matches_prefix?(instance, token) end def encrypted_field diff --git a/app/models/group.rb b/app/models/group.rb index f51782785f9..4d637d0c445 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -18,6 +18,13 @@ class Group < Namespace include EachBatch include BulkMemberAccessLoad + extend ::Gitlab::Utils::Override + + # Prefix for runners_token which can be used to invalidate existing tokens. + # The value chosen here is GR (for Gitlab Runner) combined with the rotation + # date (20220225) decimal to hex encoded. + RUNNERS_TOKEN_PREFIX = 'GR1348941' + def self.sti_name 'Group' end @@ -108,7 +115,9 @@ class Group < Namespace message: Gitlab::Regex.group_name_regex_message }, if: :name_changed? - add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required } + add_authentication_token_field :runners_token, + encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required }, + prefix: ->(instance) { instance.runners_token_prefix } after_create :post_create_hook after_destroy :post_destroy_hook @@ -652,6 +661,15 @@ class Group < Namespace ensure_runners_token! end + def runners_token_prefix + Feature.enabled?(:groups_runners_token_prefix, self, default_enabled: :yaml) ? RUNNERS_TOKEN_PREFIX : '' + end + + override :format_runners_token + def format_runners_token(token) + "#{runners_token_prefix}#{token}" + end + def project_creation_level super || ::Gitlab::CurrentSettings.default_project_creation end diff --git a/app/models/note.rb b/app/models/note.rb index a143c21c0f9..9b95476cb15 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -46,7 +46,7 @@ class Note < ApplicationRecord attr_accessor :user_visible_reference_count # Attribute used to store the attributes that have been changed by quick actions. - attr_accessor :commands_changes + attr_writer :commands_changes # Attribute used to determine whether keep_around_commits will be skipped for diff notes. attr_accessor :skip_keep_around_commits @@ -612,6 +612,41 @@ class Note < ApplicationRecord change_position.line_range["end"] || change_position.line_range["start"] end + def commands_changes + @commands_changes&.slice( + :due_date, + :label_ids, + :remove_label_ids, + :add_label_ids, + :canonical_issue_id, + :clone_with_notes, + :confidential, + :create_merge_request, + :add_contacts, + :remove_contacts, + :assignee_ids, + :milestone_id, + :time_estimate, + :spend_time, + :discussion_locked, + :merge, + :rebase, + :wip_event, + :target_branch, + :reviewer_ids, + :health_status, + :promote_to_epic, + :weight, + :emoji_award, + :todo_event, + :subscription_event, + :state_event, + :title, + :tag_message, + :tag_name + ) + end + private def system_note_viewable_by?(user) diff --git a/app/models/project.rb b/app/models/project.rb index a751e8adeb0..1a4a0b6f105 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -73,6 +73,11 @@ class Project < ApplicationRecord GL_REPOSITORY_TYPES = [Gitlab::GlRepository::PROJECT, Gitlab::GlRepository::WIKI, Gitlab::GlRepository::DESIGN].freeze + # Prefix for runners_token which can be used to invalidate existing tokens. + # The value chosen here is GR (for Gitlab Runner) combined with the rotation + # date (20220225) decimal to hex encoded. + RUNNERS_TOKEN_PREFIX = 'GR1348941' + cache_markdown_field :description, pipeline: :description default_value_for :packages_enabled, true @@ -93,7 +98,9 @@ class Project < ApplicationRecord default_value_for :autoclose_referenced_issues, true default_value_for(:ci_config_path) { Gitlab::CurrentSettings.default_ci_config_path } - add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:projects_tokens_optional_encryption, default_enabled: true) ? :optional : :required } + add_authentication_token_field :runners_token, + encrypted: -> { Feature.enabled?(:projects_tokens_optional_encryption, default_enabled: true) ? :optional : :required }, + prefix: ->(instance) { instance.runners_token_prefix } before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? } @@ -1843,6 +1850,15 @@ class Project < ApplicationRecord ensure_runners_token! end + def runners_token_prefix + Feature.enabled?(:projects_runners_token_prefix, self, default_enabled: :yaml) ? RUNNERS_TOKEN_PREFIX : '' + end + + override :format_runners_token + def format_runners_token(token) + "#{runners_token_prefix}#{token}" + end + def pages_deployed? pages_metadatum&.deployed? end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 6a8123b3c08..b04fca64c87 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -237,15 +237,19 @@ class Snippet < ApplicationRecord end end + def all_files + list_files(default_branch) + end + def blob @blob ||= Blob.decorate(SnippetBlob.new(self), self) end - def blobs + def blobs(paths = []) return [] unless repository_exists? - files = list_files(default_branch) - items = files.map { |file| [default_branch, file] } + paths = all_files if paths.empty? + items = paths.map { |path| [default_branch, path] } repository.blobs_at(items).compact end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index acd00d0d1ec..88d3a3a987a 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -19,6 +19,8 @@ module Members end def execute + raise Gitlab::Access::AccessDeniedError unless can?(current_user, create_member_permission(source), source) + validate_invite_source! validate_invitable! @@ -144,6 +146,17 @@ module Members def formatted_errors errors.to_sentence end + + def create_member_permission(source) + case source + when Group + :admin_group_member + when Project + :admin_project_member + else + raise "Unknown source type: #{source.class}!" + end + end end end diff --git a/app/views/projects/mirrors/_authentication_method.html.haml b/app/views/projects/mirrors/_authentication_method.html.haml index 5f31ec4087e..d248ffd33df 100644 --- a/app/views/projects/mirrors/_authentication_method.html.haml +++ b/app/views/projects/mirrors/_authentication_method.html.haml @@ -13,4 +13,4 @@ .form-group .well-password-auth.collapse.js-well-password-auth = f.label :password, _("Password"), class: "label-bold" - = f.password_field :password, class: 'form-control gl-form-input qa-password', autocomplete: 'new-password' + = f.password_field :password, class: 'form-control gl-form-input qa-password js-mirror-password-field', autocomplete: 'off' diff --git a/config/feature_flags/development/groups_runners_token_prefix.yml b/config/feature_flags/development/groups_runners_token_prefix.yml new file mode 100644 index 00000000000..87b87266673 --- /dev/null +++ b/config/feature_flags/development/groups_runners_token_prefix.yml @@ -0,0 +1,8 @@ +--- +name: groups_runners_token_prefix +introduced_by_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353805 +milestone: '14.9' +type: development +group: group::database +default_enabled: true diff --git a/config/feature_flags/development/projects_runners_token_prefix.yml b/config/feature_flags/development/projects_runners_token_prefix.yml new file mode 100644 index 00000000000..5dd21d115f6 --- /dev/null +++ b/config/feature_flags/development/projects_runners_token_prefix.yml @@ -0,0 +1,8 @@ +--- +name: projects_runners_token_prefix +introduced_by_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353805 +milestone: '14.9' +type: development +group: group::database +default_enabled: true diff --git a/config/initializers/action_mailer_hooks.rb b/config/initializers/action_mailer_hooks.rb index 46d5e387d9d..fb09ed34bf6 100644 --- a/config/initializers/action_mailer_hooks.rb +++ b/config/initializers/action_mailer_hooks.rb @@ -8,6 +8,7 @@ end ActionMailer::Base.register_interceptors( ::Gitlab::Email::Hook::AdditionalHeadersInterceptor, ::Gitlab::Email::Hook::EmailTemplateInterceptor, + ::Gitlab::Email::Hook::ValidateAddressesInterceptor, ::Gitlab::Email::Hook::DeliveryMetricsObserver ) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index bce60bb6cbf..7b2cb372cf9 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -7519,6 +7519,7 @@ The connection type for [`SnippetBlob`](#snippetblob). | Name | Type | Description | | ---- | ---- | ----------- | | <a id="snippetblobconnectionedges"></a>`edges` | [`[SnippetBlobEdge]`](#snippetblobedge) | A list of edges. | +| <a id="snippetblobconnectionhasunretrievableblobs"></a>`hasUnretrievableBlobs` | [`Boolean!`](#boolean) | Indicates if the snippet has unretrievable blobs. | | <a id="snippetblobconnectionnodes"></a>`nodes` | [`[SnippetBlob]`](#snippetblob) | A list of nodes. | | <a id="snippetblobconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. | diff --git a/lib/api/users.rb b/lib/api/users.rb index ce0a0e9b502..191a580ac3e 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -105,9 +105,6 @@ module API params.except!(:created_after, :created_before, :order_by, :sort, :two_factor, :without_projects) end - users = UsersFinder.new(current_user, params).execute - users = reorder_users(users) - authorized = can?(current_user, :read_users_list) # When `current_user` is not present, require that the `username` @@ -119,6 +116,9 @@ module API forbidden!("Not authorized to access /api/v4/users") unless authorized + users = UsersFinder.new(current_user, params).execute + users = reorder_users(users) + entity = current_user&.admin? ? Entities::UserWithAdmin : Entities::UserBasic users = users.preload(:identities, :u2f_registrations) if entity == Entities::UserWithAdmin users = users.preload(:identities, :webauthn_registrations) if entity == Entities::UserWithAdmin diff --git a/lib/banzai/filter/math_filter.rb b/lib/banzai/filter/math_filter.rb index 6859d67c9d8..0ac506776be 100644 --- a/lib/banzai/filter/math_filter.rb +++ b/lib/banzai/filter/math_filter.rb @@ -25,7 +25,14 @@ module Banzai DOLLAR_SIGN = '$' + # Limit to how many nodes can be marked as math elements. + # Prevents timeouts for large notes. + # For more information check: https://gitlab.com/gitlab-org/gitlab/-/issues/341832 + RENDER_NODES_LIMIT = 50 + def call + nodes_count = 0 + doc.xpath(XPATH_CODE).each do |code| closing = code.next opening = code.previous @@ -41,6 +48,9 @@ module Banzai code[STYLE_ATTRIBUTE] = 'inline' closing.content = closing.content[1..] opening.content = opening.content[0..-2] + + nodes_count += 1 + break if nodes_count >= RENDER_NODES_LIMIT end end diff --git a/lib/gitlab/email/hook/validate_addresses_interceptor.rb b/lib/gitlab/email/hook/validate_addresses_interceptor.rb new file mode 100644 index 00000000000..e63f047e63d --- /dev/null +++ b/lib/gitlab/email/hook/validate_addresses_interceptor.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Email + module Hook + # Check for unsafe characters in the envelope-from and -to addresses. + # These are passed directly as arguments to sendmail and are liable to shell injection attacks: + # https://github.com/mikel/mail/blob/2.7.1/lib/mail/network/delivery_methods/sendmail.rb#L53-L58 + class ValidateAddressesInterceptor + UNSAFE_CHARACTERS = /(\\|[^[:print:]])/.freeze + + def self.delivering_email(message) + addresses = Array(message.smtp_envelope_from) + Array(message.smtp_envelope_to) + + addresses.each do |address| + next unless address.match?(UNSAFE_CHARACTERS) + + Gitlab::AuthLogger.info( + message: 'Skipping email with unsafe characters in address', + address: address, + subject: message.subject + ) + + message.perform_deliveries = false + + break + end + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9846d3a5238..cb3f43c72d5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -39328,6 +39328,9 @@ msgstr "" msgid "WARNING:" msgstr "" +msgid "WARNING: This snippet contains hidden files which might be used to mask malicious behavior. Exercise caution if cloning and executing code from this snippet." +msgstr "" + msgid "Wait for the file to load to copy its contents" msgstr "" diff --git a/spec/frontend/snippets/components/show_spec.js b/spec/frontend/snippets/components/show_spec.js index af61f4ea54f..c73bf8f80a2 100644 --- a/spec/frontend/snippets/components/show_spec.js +++ b/spec/frontend/snippets/components/show_spec.js @@ -1,4 +1,4 @@ -import { GlLoadingIcon } from '@gitlab/ui'; +import { GlLoadingIcon, GlAlert } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import { Blob, BinaryBlob } from 'jest/blob/components/mock_data'; import EmbedDropdown from '~/snippets/components/embed_dropdown.vue'; @@ -106,6 +106,23 @@ describe('Snippet view app', () => { }); }); + describe('hasUnretrievableBlobs alert rendering', () => { + it.each` + hasUnretrievableBlobs | condition | isRendered + ${false} | ${'not render'} | ${false} + ${true} | ${'render'} | ${true} + `('does $condition gl-alert by default', ({ hasUnretrievableBlobs, isRendered }) => { + createComponent({ + data: { + snippet: { + hasUnretrievableBlobs, + }, + }, + }); + expect(wrapper.findComponent(GlAlert).exists()).toBe(isRendered); + }); + }); + describe('Clone button rendering', () => { it.each` httpUrlToRepo | sshUrlToRepo | shouldRender | isRendered diff --git a/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb b/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb index ebe286769cf..b5db7ac5748 100644 --- a/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb +++ b/spec/graphql/resolvers/snippets/blobs_resolver_spec.rb @@ -13,11 +13,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do let_it_be(:current_user) { create(:user) } let_it_be(:snippet) { create(:personal_snippet, :private, :repository, author: current_user) } + let(:query_context) { {} } + context 'when user is not authorized' do let(:other_user) { create(:user) } it 'redacts the field' do expect(resolve_blobs(snippet, user: other_user)).to be_nil + expect(query_context[:unretrievable_blobs?]).to eq(false) end end @@ -28,6 +31,7 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do expect(result).to match_array(snippet.list_files.map do |file| have_attributes(path: file) end) + expect(query_context[:unretrievable_blobs?]).to eq(false) end end @@ -37,12 +41,14 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do path = 'CHANGELOG' expect(resolve_blobs(snippet, paths: [path])).to contain_exactly(have_attributes(path: path)) + expect(query_context[:unretrievable_blobs?]).to eq(false) end end context 'the argument does not match anything' do it 'returns an empty result' do expect(resolve_blobs(snippet, paths: ['does not exist'])).to be_empty + expect(query_context[:unretrievable_blobs?]).to eq(true) end end @@ -53,12 +59,15 @@ RSpec.describe Resolvers::Snippets::BlobsResolver do expect(resolve_blobs(snippet, paths: paths)).to match_array(paths.map do |file| have_attributes(path: file) end) + expect(query_context[:unretrievable_blobs?]).to eq(false) end end end end - def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths }) - resolve(described_class, args: args, ctx: { current_user: user }, obj: snippet) + def resolve_blobs(snippet, user: current_user, paths: [], args: { paths: paths }, has_unretrievable_blobs: false) + query_context[:current_user] = user + query_context[:unretrievable_blobs?] = has_unretrievable_blobs + resolve(described_class, args: args, ctx: query_context, obj: snippet) end end diff --git a/spec/graphql/resolvers/users_resolver_spec.rb b/spec/graphql/resolvers/users_resolver_spec.rb index 031d7c99eef..29947c33430 100644 --- a/spec/graphql/resolvers/users_resolver_spec.rb +++ b/spec/graphql/resolvers/users_resolver_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Resolvers::UsersResolver do let_it_be(:user1) { create(:user, name: "SomePerson") } let_it_be(:user2) { create(:user, username: "someone123784") } + let_it_be(:current_user) { create(:user) } specify do expect(described_class).to have_nullable_graphql_type(Types::UserType.connection_type) @@ -14,14 +15,14 @@ RSpec.describe Resolvers::UsersResolver do describe '#resolve' do it 'raises an error when read_users_list is not authorized' do - expect(Ability).to receive(:allowed?).with(nil, :read_users_list).and_return(false) + expect(Ability).to receive(:allowed?).with(current_user, :read_users_list).and_return(false) expect { resolve_users }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) end context 'when no arguments are passed' do it 'returns all users' do - expect(resolve_users).to contain_exactly(user1, user2) + expect(resolve_users).to contain_exactly(user1, user2, current_user) end end @@ -65,9 +66,21 @@ RSpec.describe Resolvers::UsersResolver do expect(resolve_users( args: { search: "someperson" } )).to contain_exactly(user1) end end + + context 'with anonymous access' do + let_it_be(:current_user) { nil } + + it 'prohibits search without usernames passed' do + expect { resolve_users }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + + it 'allows to search by username' do + expect(resolve_users(args: { usernames: [user1.username] })).to contain_exactly(user1) + end + end end def resolve_users(args: {}, ctx: {}) - resolve(described_class, args: args, ctx: ctx) + resolve(described_class, args: args, ctx: { current_user: current_user }.merge(ctx)) end end diff --git a/spec/lib/banzai/filter/math_filter_spec.rb b/spec/lib/banzai/filter/math_filter_spec.rb index 6d22fa3a001..128f8532d39 100644 --- a/spec/lib/banzai/filter/math_filter_spec.rb +++ b/spec/lib/banzai/filter/math_filter_spec.rb @@ -126,4 +126,12 @@ RSpec.describe Banzai::Filter::MathFilter do expect(before.to_s).to eq '$' expect(after.to_s).to eq '$' end + + it 'limits how many elements can be marked as math' do + stub_const('Banzai::Filter::MathFilter::RENDER_NODES_LIMIT', 2) + + doc = filter('$<code>2+2</code>$ + $<code>3+3</code>$ + $<code>4+4</code>$') + + expect(doc.search('.js-render-math').count).to eq(2) + end end diff --git a/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb b/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb new file mode 100644 index 00000000000..a3f0158db40 --- /dev/null +++ b/spec/lib/gitlab/email/hook/validate_addresses_interceptor_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Email::Hook::ValidateAddressesInterceptor do + describe 'UNSAFE_CHARACTERS' do + subject { described_class::UNSAFE_CHARACTERS } + + it { is_expected.to match('\\') } + it { is_expected.to match("\x00") } + it { is_expected.to match("\x01") } + it { is_expected.not_to match('') } + it { is_expected.not_to match('user@example.com') } + it { is_expected.not_to match('foo-123+bar_456@example.com') } + end + + describe '.delivering_email' do + let(:mail) do + ActionMailer::Base.mail(to: 'test@mail.com', from: 'info@mail.com', subject: 'title', body: 'hello') + end + + let(:unsafe_email) { "evil+\x01$HOME@example.com" } + + it 'sends emails to normal addresses' do + expect(Gitlab::AuthLogger).not_to receive(:info) + expect { mail.deliver_now }.to change(ActionMailer::Base.deliveries, :count) + end + + [:from, :to, :cc, :bcc].each do |header| + it "does not send emails if the #{header.inspect} header contains unsafe characters" do + mail[header] = unsafe_email + + expect(Gitlab::AuthLogger).to receive(:info).with( + message: 'Skipping email with unsafe characters in address', + address: unsafe_email, + subject: mail.subject + ) + + expect { mail.deliver_now }.not_to change(ActionMailer::Base.deliveries, :count) + end + end + + [:reply_to].each do |header| + it "sends emails if the #{header.inspect} header contains unsafe characters" do + mail[header] = unsafe_email + + expect(Gitlab::AuthLogger).not_to receive(:info) + expect { mail.deliver_now }.to change(ActionMailer::Base.deliveries, :count) + end + end + end +end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 4bdb3e0a32a..e59eaa6dd80 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -290,3 +290,106 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do end end end + +RSpec.shared_examples 'prefixed token rotation' do + describe "ensure_runners_token" do + subject { instance.ensure_runners_token } + + context 'token is not set' do + it 'generates a new token' do + expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/) + expect(instance).not_to be_persisted + end + end + + context 'token is set, but does not match the prefix' do + before do + instance.set_runners_token('abcdef') + end + + it 'generates a new token' do + expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/) + expect(instance).not_to be_persisted + end + + context 'feature flag is disabled' do + before do + flag = "#{described_class.name.downcase.pluralize}_runners_token_prefix" + stub_feature_flags(flag => false) + end + + it 'leaves the token unchanged' do + expect { subject }.not_to change(instance, :runners_token) + expect(instance).not_to be_persisted + end + end + end + + context 'token is set and matches prefix' do + before do + instance.set_runners_token(instance.class::RUNNERS_TOKEN_PREFIX + '-abcdef') + end + + it 'leaves the token unchanged' do + expect { subject }.not_to change(instance, :runners_token) + expect(instance).not_to be_persisted + end + end + end + + describe 'ensure_runners_token!' do + subject { instance.ensure_runners_token! } + + context 'token is not set' do + it 'generates a new token' do + expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/) + expect(instance).to be_persisted + end + end + + context 'token is set, but does not match the prefix' do + before do + instance.set_runners_token('abcdef') + end + + it 'generates a new token' do + expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/) + expect(instance).to be_persisted + end + + context 'feature flag is disabled' do + before do + flag = "#{described_class.name.downcase.pluralize}_runners_token_prefix" + stub_feature_flags(flag => false) + end + + it 'leaves the token unchanged' do + expect { subject }.not_to change(instance, :runners_token) + end + end + end + + context 'token is set and matches prefix' do + before do + instance.set_runners_token(instance.class::RUNNERS_TOKEN_PREFIX + '-abcdef') + instance.save! + end + + it 'leaves the token unchanged' do + expect { subject }.not_to change(instance, :runners_token) + end + end + end +end + +RSpec.describe Project, 'TokenAuthenticatable' do + let(:instance) { build(:project, runners_token: nil) } + + it_behaves_like 'prefixed token rotation' +end + +RSpec.describe Group, 'TokenAuthenticatable' do + let(:instance) { build(:group, runners_token: nil) } + + it_behaves_like 'prefixed token rotation' +end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index b311e302a31..46ef04adc9a 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -30,6 +30,21 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do expect(subject.find_token_authenticatable('my-value')) .to eq 'encrypted resource' end + + context 'when a prefix is required' do + let(:options) { { encrypted: :required, prefix: 'GR1348941' } } + + it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:where) + .and_return(model) + allow(model).to receive(:find_by) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) + .and_return('encrypted resource') + + expect(subject.find_token_authenticatable('my-value')) + .to be_nil + end + end end context 'when encryption is optional' do @@ -56,6 +71,21 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do expect(subject.find_token_authenticatable('my-value')) .to eq 'plaintext resource' end + + context 'when a prefix is required' do + let(:options) { { encrypted: :optional, prefix: 'GR1348941' } } + + it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:where) + .and_return(model) + allow(model).to receive(:find_by) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) + .and_return('encrypted resource') + + expect(subject.find_token_authenticatable('my-value')) + .to be_nil + end + end end context 'when encryption is migrating' do @@ -78,6 +108,21 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do expect(subject.find_token_authenticatable('my-value')) .to be_nil end + + context 'when a prefix is required' do + let(:options) { { encrypted: :migrating, prefix: 'GR1348941' } } + + it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:where) + .and_return(model) + allow(model).to receive(:find_by) + .with('some_field' => 'my-value') + .and_return('cleartext resource') + + expect(subject.find_token_authenticatable('my-value')) + .to be_nil + end + end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index fed4ee3f3a4..29a3bca4421 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2775,4 +2775,12 @@ RSpec.describe Group do end end end + + describe '#runners_token' do + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'it has a prefixable runners_token', :groups_runners_token_prefix + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 9d9cca0678a..530732420e6 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1624,4 +1624,14 @@ RSpec.describe Note do end end end + + describe '#commands_changes' do + let(:note) { build(:note) } + + it 'only returns allowed keys' do + note.commands_changes = { emoji_award: {}, time_estimate: {}, spend_time: {}, target_project: build(:project) } + + expect(note.commands_changes.keys).to contain_exactly(:emoji_award, :time_estimate, :spend_time) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4e38bf7d3e3..0b4e50a376d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -765,8 +765,8 @@ RSpec.describe Project, factory_default: :keep do end it 'does not set an random token if one provided' do - project = FactoryBot.create(:project, runners_token: 'my-token') - expect(project.runners_token).to eq('my-token') + project = FactoryBot.create(:project, runners_token: "#{Project::RUNNERS_TOKEN_PREFIX}my-token") + expect(project.runners_token).to eq("#{Project::RUNNERS_TOKEN_PREFIX}my-token") end end @@ -7551,6 +7551,14 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#runners_token' do + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'it has a prefixable runners_token', :projects_runners_token_prefix + end + private def finish_job(export_job) diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 5d4a78bb15f..92e4bc7d1a9 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Snippet do + include FakeBlobHelpers + describe 'modules' do subject { described_class } @@ -526,6 +528,21 @@ RSpec.describe Snippet do end end + describe '#all_files' do + let(:snippet) { create(:snippet, :repository) } + let(:files) { double(:files) } + + subject(:all_files) { snippet.all_files } + + before do + allow(snippet.repository).to receive(:ls_files).with(snippet.default_branch).and_return(files) + end + + it 'lists files from the repository with the default branch' do + expect(all_files).to eq(files) + end + end + describe '#blobs' do context 'when repository does not exist' do let(:snippet) { create(:snippet) } @@ -552,6 +569,23 @@ RSpec.describe Snippet do end end end + + context 'when some blobs are not retrievable from repository' do + let(:snippet) { create(:snippet, :repository) } + let(:container) { double(:container) } + let(:retrievable_filename) { 'retrievable_file'} + let(:unretrievable_filename) { 'unretrievable_file'} + + before do + allow(snippet).to receive(:list_files).and_return([retrievable_filename, unretrievable_filename]) + blob = fake_blob(path: retrievable_filename, container: container) + allow(snippet.repository).to receive(:blobs_at).and_return([blob, nil]) + end + + it 'does not include unretrievable blobs' do + expect(snippet.blobs.map(&:name)).to contain_exactly(retrievable_filename) + end + end end describe '#to_json' do diff --git a/spec/requests/api/graphql/users_spec.rb b/spec/requests/api/graphql/users_spec.rb index 67cd35ee545..fe824834a2c 100644 --- a/spec/requests/api/graphql/users_spec.rb +++ b/spec/requests/api/graphql/users_spec.rb @@ -5,11 +5,13 @@ require 'spec_helper' RSpec.describe 'Users' do include GraphqlHelpers - let_it_be(:current_user) { create(:user, created_at: 1.day.ago) } + let_it_be(:user0) { create(:user, created_at: 1.day.ago) } let_it_be(:user1) { create(:user, created_at: 2.days.ago) } let_it_be(:user2) { create(:user, created_at: 3.days.ago) } let_it_be(:user3) { create(:user, created_at: 4.days.ago) } + let(:current_user) { user0 } + describe '.users' do shared_examples 'a working users query' do it_behaves_like 'a working graphql query' do @@ -19,7 +21,7 @@ RSpec.describe 'Users' do end it 'includes a list of users' do - post_graphql(query) + post_graphql(query, current_user: current_user) expect(graphql_data.dig('users', 'nodes')).not_to be_empty end @@ -47,7 +49,7 @@ RSpec.describe 'Users' do let_it_be(:query) { graphql_query_for(:users, { ids: user1.to_global_id.to_s, usernames: user1.username }, 'nodes { id }') } it 'displays an error' do - post_graphql(query) + post_graphql(query, current_user: current_user) expect(graphql_errors).to include( a_hash_including('message' => a_string_matching(%r{Provide either a list of usernames or ids})) @@ -66,14 +68,14 @@ RSpec.describe 'Users' do it_behaves_like 'a working users query' - it 'includes all non-admin users', :aggregate_failures do - post_graphql(query) + it 'includes all users', :aggregate_failures do + post_query expect(graphql_data.dig('users', 'nodes')).to include( + { "id" => user0.to_global_id.to_s }, { "id" => user1.to_global_id.to_s }, { "id" => user2.to_global_id.to_s }, { "id" => user3.to_global_id.to_s }, - { "id" => current_user.to_global_id.to_s }, { "id" => admin.to_global_id.to_s }, { "id" => another_admin.to_global_id.to_s } ) @@ -81,10 +83,12 @@ RSpec.describe 'Users' do end context 'when current user is an admin' do + let(:current_user) { admin } + it_behaves_like 'a working users query' it 'includes only admins', :aggregate_failures do - post_graphql(query, current_user: admin) + post_graphql(query, current_user: current_user) expect(graphql_data.dig('users', 'nodes')).to include( { "id" => another_admin.to_global_id.to_s }, @@ -92,10 +96,10 @@ RSpec.describe 'Users' do ) expect(graphql_data.dig('users', 'nodes')).not_to include( + { "id" => user0.to_global_id.to_s }, { "id" => user1.to_global_id.to_s }, { "id" => user2.to_global_id.to_s }, - { "id" => user3.to_global_id.to_s }, - { "id" => current_user.to_global_id.to_s } + { "id" => user3.to_global_id.to_s } ) end end @@ -110,7 +114,7 @@ RSpec.describe 'Users' do end context 'when sorting by created_at' do - let_it_be(:ascending_users) { [user3, user2, user1, current_user].map { |u| global_id_of(u) } } + let_it_be(:ascending_users) { [user3, user2, user1, user0].map { |u| global_id_of(u) } } context 'when ascending' do it_behaves_like 'sorted paginated query' do diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index d4f8b841c96..3c28aed6cac 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -233,11 +233,9 @@ RSpec.describe API::Notes do subject { post api(request_path, user), params: { body: request_body } } context 'a command only note' do - let(:assignee) { create(:user) } - let(:request_body) { "/assign #{assignee.to_reference}" } + let(:request_body) { "/spend 1h" } before do - project.add_developer(assignee) project.add_developer(user) end @@ -256,7 +254,7 @@ RSpec.describe API::Notes do end it 'applies the commands' do - expect { subject }.to change { merge_request.reset.assignees } + expect { subject }.to change { merge_request.reset.total_time_spent } end it 'reports the changes' do @@ -264,9 +262,9 @@ RSpec.describe API::Notes do expect(json_response).to include( 'commands_changes' => include( - 'assignee_ids' => [Integer] + 'spend_time' => include('duration' => 3600) ), - 'summary' => include("Assigned #{assignee.to_reference}.") + 'summary' => include('Added 1h spent time.') ) end end diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb index 13f56fe7458..00ad7e9f02b 100644 --- a/spec/services/members/create_service_spec.rb +++ b/spec/services/members/create_service_spec.rb @@ -11,19 +11,37 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ let(:additional_params) { { invite_source: '_invite_source_' } } let(:params) { { user_ids: user_ids, access_level: access_level }.merge(additional_params) } + let(:current_user) { user } - subject(:execute_service) { described_class.new(user, params.merge({ source: source })).execute } + subject(:execute_service) { described_class.new(current_user, params.merge({ source: source })).execute } before do - if source.is_a?(Project) + case source + when Project source.add_maintainer(user) OnboardingProgress.onboard(source.namespace) - else + when Group source.add_owner(user) OnboardingProgress.onboard(source) end end + context 'when the current user does not have permission to create members' do + let(:current_user) { create(:user) } + + it 'raises a Gitlab::Access::AccessDeniedError' do + expect { execute_service }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end + + context 'when passing an invalid source' do + let_it_be(:source) { Object.new } + + it 'raises a RuntimeError' do + expect { execute_service }.to raise_error(RuntimeError, 'Unknown source type: Object!') + end + end + context 'when passing valid parameters' do it 'adds a user to members' do expect(execute_service[:status]).to eq(:success) diff --git a/spec/support/shared_examples/models/runners_token_prefix_shared_examples.rb b/spec/support/shared_examples/models/runners_token_prefix_shared_examples.rb new file mode 100644 index 00000000000..bbce67ae7b9 --- /dev/null +++ b/spec/support/shared_examples/models/runners_token_prefix_shared_examples.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'it has a prefixable runners_token' do |feature_flag| + context 'feature flag enabled' do + before do + stub_feature_flags(feature_flag => [subject]) + end + + describe '#runners_token' do + it 'has a runners_token_prefix' do + expect(subject.runners_token_prefix).not_to be_empty + end + + it 'starts with the runners_token_prefix' do + expect(subject.runners_token).to start_with(subject.runners_token_prefix) + end + end + end + + context 'feature flag disabled' do + before do + stub_feature_flags(feature_flag => false) + end + + describe '#runners_token' do + it 'does not have a runners_token_prefix' do + expect(subject.runners_token_prefix).to be_empty + end + + it 'starts with the runners_token_prefix' do + expect(subject.runners_token).to start_with(subject.runners_token_prefix) + end + end + end +end |