diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-05-27 17:21:48 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-05-27 17:21:48 +0000 |
commit | 73c7aa95bd5cb05532c313ceced360c4ba059779 (patch) | |
tree | c819b81c796005349e62b54907cd223ae443c366 | |
parent | 2d5e8ce6e55f1ab6f776921a7a38590329da12bf (diff) | |
parent | 34fb412ff1a7b0dd0b5a3cfe3f6d48c091fa701e (diff) | |
download | gitlab-ce-73c7aa95bd5cb05532c313ceced360c4ba059779.tar.gz |
Merge remote-tracking branch 'dev/12-9-stable' into 12-9-stable
57 files changed, 878 insertions, 312 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 546e66fe451..0adc7ae8cb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 12.9.8 (2020-05-27) + +### Security (13 changes) + +- Hide EKS secret key in admin integrations settings. +- Added data integrity check before updating a deploy key. +- Display only verified emails on notifications and profile page. +- Disable caching on repo/blobs/[sha]/raw endpoint. +- Require confirmed email address for GitLab OAuth authentication. +- Kubernetes cluster details page no longer exposes Service Token. +- Fix confirming unverified emails with soft email confirmation flow enabled. +- Disallow user to control PUT request using mermaid markdown in issue description. +- Check forked project permissions before allowing fork. +- Limit memory footprint of a command that generates ZIP artifacts metadata. +- Fix file enuming using Group Import. +- Prevent XSS in the monitoring dashboard. +- Use `gsub` instead of the Ruby `%` operator to perform variable substitution in Prometheus proxy API. + + ## 12.9.7 (2020-05-13) ### Added (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c03876d0152..2f0635c0444 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -12.9.7 +12.9.8 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 2465b5f7b85..ae1f60d7f6e 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.25.2 +8.25.3 @@ -1 +1 @@ -12.9.7 +12.9.8 diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index e20c87ed8a0..07f53ff9273 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -98,7 +98,6 @@ export default class Clusters { }); this.installApplication = this.installApplication.bind(this); - this.showToken = this.showToken.bind(this); this.errorContainer = document.querySelector('.js-cluster-error'); this.successContainer = document.querySelector('.js-cluster-success'); @@ -109,7 +108,6 @@ export default class Clusters { ); this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason'); this.successApplicationContainer = document.querySelector('.js-cluster-application-notice'); - this.showTokenButton = document.querySelector('.js-show-cluster-token'); this.tokenField = document.querySelector('.js-cluster-token'); this.ingressDomainHelpText = document.querySelector('.js-ingress-domain-help-text'); this.ingressDomainSnippet = @@ -248,7 +246,6 @@ export default class Clusters { } addListeners() { - if (this.showTokenButton) this.showTokenButton.addEventListener('click', this.showToken); eventHub.$on('installApplication', this.installApplication); eventHub.$on('updateApplication', data => this.updateApplication(data)); eventHub.$on('saveKnativeDomain', data => this.saveKnativeDomain(data)); @@ -263,7 +260,6 @@ export default class Clusters { } removeListeners() { - if (this.showTokenButton) this.showTokenButton.removeEventListener('click', this.showToken); eventHub.$off('installApplication', this.installApplication); eventHub.$off('updateApplication', this.updateApplication); eventHub.$off('saveKnativeDomain'); @@ -326,18 +322,6 @@ export default class Clusters { } } - showToken() { - const type = this.tokenField.getAttribute('type'); - - if (type === 'password') { - this.tokenField.setAttribute('type', 'text'); - this.showTokenButton.textContent = s__('ClusterIntegration|Hide'); - } else { - this.tokenField.setAttribute('type', 'password'); - this.showTokenButton.textContent = s__('ClusterIntegration|Show'); - } - } - hideAll() { this.errorContainer.classList.add('hidden'); this.successContainer.classList.add('hidden'); diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index 9136a47d542..0e20c96df8c 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -101,7 +101,11 @@ export default class Issue { this.disableCloseReopenButton($button); - const url = $button.attr('href'); + const url = $button.data('close-reopen-url'); + if (!url) { + return; + } + return axios .put(url) .then(({ data }) => { diff --git a/app/assets/javascripts/monitoring/components/duplicate_dashboard_form.vue b/app/assets/javascripts/monitoring/components/duplicate_dashboard_form.vue index e678957c1e5..e030041a1ac 100644 --- a/app/assets/javascripts/monitoring/components/duplicate_dashboard_form.vue +++ b/app/assets/javascripts/monitoring/components/duplicate_dashboard_form.vue @@ -1,6 +1,7 @@ <script> import { __, s__, sprintf } from '~/locale'; import { GlFormGroup, GlFormInput, GlFormRadioGroup, GlFormTextarea } from '@gitlab/ui'; +import { escape as esc } from 'lodash'; const defaultFileName = dashboard => dashboard.path.split('/').reverse()[0]; @@ -42,7 +43,7 @@ export default { html: sprintf( __('Commit to %{branchName} branch'), { - branchName: `<strong>${this.defaultBranch}</strong>`, + branchName: `<strong>${esc(this.defaultBranch)}</strong>`, }, false, ), diff --git a/app/assets/javascripts/profile/profile.js b/app/assets/javascripts/profile/profile.js index 8dd37aee7e1..21cc27cb1ce 100644 --- a/app/assets/javascripts/profile/profile.js +++ b/app/assets/javascripts/profile/profile.js @@ -40,7 +40,9 @@ export default class Profile { bindEvents() { $('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm); $('.js-group-notification-email').on('change', this.submitForm); - $('#user_notification_email').on('change', this.submitForm); + $('#user_notification_email').on('select2-selecting', event => { + setTimeout(this.submitForm.bind(event.currentTarget)); + }); $('#user_notified_of_own_activity').on('change', this.submitForm); this.form.on('submit', this.onSubmitForm); } diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 2192bcc96ee..096f7ac84f3 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -182,7 +182,9 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController params[:application_setting][:import_sources]&.delete("") params[:application_setting][:restricted_visibility_levels]&.delete("") - params[:application_setting].delete(:elasticsearch_aws_secret_access_key) if params[:application_setting][:elasticsearch_aws_secret_access_key].blank? + + remove_blank_params_for!(:elasticsearch_aws_secret_access_key, :eks_secret_access_key) + # TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204) params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file] params.delete(:domain_blacklist_raw) if params[:domain_blacklist] @@ -249,6 +251,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController render action end + def remove_blank_params_for!(*keys) + params[:application_setting].delete_if { |setting, value| setting.to_sym.in?(keys) && value.blank? } + end + # overridden in EE def valid_setting_panels VALID_SETTING_PANELS diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 1cf9046e30f..4ab02005b45 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -53,10 +53,16 @@ module MembershipActions end def request_access - membershipable.request_access(current_user) + access_requester = membershipable.request_access(current_user) - redirect_to polymorphic_path(membershipable), - notice: _('Your request for access has been queued for review.') + if access_requester.persisted? + redirect_to polymorphic_path(membershipable), + notice: _('Your request for access has been queued for review.') + else + redirect_to polymorphic_path(membershipable), + alert: _("Your request for access could not be processed: %{error_meesage}") % + { error_meesage: access_requester.errors.full_messages.to_sentence } + end end def approve_access_request diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 2a4e659c5b9..f6ad2bf5312 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -4,6 +4,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController include Gitlab::Experimentation::ControllerConcern include InitializesCurrentUserMode + before_action :verify_confirmed_email!, only: [:new] + layout 'profile' # Overridden from Doorkeeper::AuthorizationsController to @@ -21,4 +23,13 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController render "doorkeeper/authorizations/error" end end + + private + + def verify_confirmed_email! + return if current_user&.confirmed? + + pre_auth.error = :unconfirmed_email + render "doorkeeper/authorizations/error" + end end diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index f43e9f2bd19..971c84a5a20 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -36,6 +36,8 @@ class Projects::DeployKeysController < Projects::ApplicationController end def update + access_denied! unless deploy_key + if deploy_key.update(update_params) flash[:notice] = _('Deploy key was successfully updated.') redirect_to_ci_cd_settings @@ -84,10 +86,12 @@ class Projects::DeployKeysController < Projects::ApplicationController end def update_params - permitted_params = [deploy_keys_projects_attributes: [:id, :can_push]] + permitted_params = [deploy_keys_projects_attributes: [:can_push]] permitted_params << :title if can?(current_user, :update_deploy_key, deploy_key) - params.require(:deploy_key).permit(*permitted_params) + key_update_params = params.require(:deploy_key).permit(*permitted_params) + key_update_params.dig(:deploy_keys_projects_attributes, '0')&.merge!(id: deploy_keys_project.id) + key_update_params end def authorize_update_deploy_key! diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 38bd95e6a20..c8c1f47c182 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -14,6 +14,7 @@ class NotificationSetting < ApplicationRecord validates :user_id, uniqueness: { scope: [:source_type, :source_id], message: "already exists in source", allow_nil: true } + validate :owns_notification_email, if: :notification_email_changed? scope :for_groups, -> { where(source_type: 'Namespace') } @@ -97,6 +98,13 @@ class NotificationSetting < ApplicationRecord def event_enabled?(event) respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend end + + def owns_notification_email + return if user.temp_oauth_email? + return if notification_email.empty? + + errors.add(:notification_email, _("is not an email you own")) unless user.verified_emails.include?(notification_email) + end end NotificationSetting.prepend_if_ee('EE::NotificationSetting') diff --git a/app/models/user.rb b/app/models/user.rb index 0c7dfac5776..d0c32ea37cc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -221,9 +221,10 @@ class User < ApplicationRecord if previous_changes.key?('email') # Grab previous_email here since previous_changes changes after # #update_emails_with_primary_email and #update_notification_email are called + previous_confirmed_at = previous_changes.key?('confirmed_at') ? previous_changes['confirmed_at'][0] : confirmed_at previous_email = previous_changes[:email][0] - update_emails_with_primary_email(previous_email) + update_emails_with_primary_email(previous_confirmed_at, previous_email) update_invalid_gpg_signatures if previous_email == notification_email @@ -746,15 +747,15 @@ class User < ApplicationRecord end def owns_notification_email - return if temp_oauth_email? + return if new_record? || temp_oauth_email? - errors.add(:notification_email, _("is not an email you own")) unless all_emails.include?(notification_email) + errors.add(:notification_email, _("is not an email you own")) unless verified_emails.include?(notification_email) end def owns_public_email return if public_email.blank? - errors.add(:public_email, _("is not an email you own")) unless all_emails.include?(public_email) + errors.add(:public_email, _("is not an email you own")) unless verified_emails.include?(public_email) end def owns_commit_email @@ -802,13 +803,15 @@ class User < ApplicationRecord # By using an `after_commit` instead of `after_update`, we avoid the recursive callback # scenario, though it then requires us to use the `previous_changes` hash # rubocop: disable CodeReuse/ServiceClass - def update_emails_with_primary_email(previous_email) + def update_emails_with_primary_email(previous_confirmed_at, previous_email) primary_email_record = emails.find_by(email: email) Emails::DestroyService.new(self, user: self).execute(primary_email_record) if primary_email_record # the original primary email was confirmed, and we want that to carry over. We don't # have access to the original confirmation values at this point, so just set confirmed_at - Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: confirmed_at) + Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: previous_confirmed_at) + + update_columns(confirmed_at: primary_email_record.confirmed_at) if primary_email_record&.confirmed_at end # rubocop: enable CodeReuse/ServiceClass @@ -1200,18 +1203,20 @@ class User < ApplicationRecord all_emails end - def all_public_emails - all_emails(include_private_email: false) - end - - def verified_emails + def verified_emails(include_private_email: true) verified_emails = [] verified_emails << email if primary_email_verified? - verified_emails << private_commit_email + verified_emails << private_commit_email if include_private_email verified_emails.concat(emails.confirmed.pluck(:email)) verified_emails end + def public_verified_emails + emails = verified_emails(include_private_email: false) + emails << email unless temp_oauth_email? + emails.uniq + end + def any_email?(check_email) downcased = check_email.downcase diff --git a/app/services/clusters/update_service.rb b/app/services/clusters/update_service.rb index 8cb77040b14..e3b7983c130 100644 --- a/app/services/clusters/update_service.rb +++ b/app/services/clusters/update_service.rb @@ -10,6 +10,12 @@ module Clusters def execute(cluster) if validate_params(cluster) + token = params.dig(:platform_kubernetes_attributes, :token) + + if token.blank? + params[:platform_kubernetes_attributes]&.delete(:token) + end + cluster.update(params) else false diff --git a/app/services/prometheus/proxy_variable_substitution_service.rb b/app/services/prometheus/proxy_variable_substitution_service.rb index b34afaf80b8..c4cc9a85733 100644 --- a/app/services/prometheus/proxy_variable_substitution_service.rb +++ b/app/services/prometheus/proxy_variable_substitution_service.rb @@ -4,6 +4,16 @@ module Prometheus class ProxyVariableSubstitutionService < BaseService include Stepable + VARIABLE_INTERPOLATION_REGEX = / + %{ # Variable needs to be wrapped in these chars. + \s* # Allow whitespace before and after the variable name. + (?<variable> # Named capture. + \w+ # Match one or more word characters. + ) + \s* + } + /x.freeze + steps :validate_variables, :add_params_to_result, :substitute_ruby_variables, @@ -35,6 +45,14 @@ module Prometheus success(result) end + def substitute_ruby_variables(result) + return success(result) unless query(result) + + result[:params][:query] = gsub(query(result), full_context) + + success(result) + end + def substitute_liquid_variables(result) return success(result) unless query(result) @@ -46,26 +64,20 @@ module Prometheus error(e.message) end - def substitute_ruby_variables(result) - return success(result) unless query(result) - - # The % operator doesn't replace variables if the hash contains string - # keys. - result[:params][:query] = query(result) % predefined_context.symbolize_keys - - success(result) - rescue TypeError, ArgumentError => exception - log_error(exception.message) - Gitlab::ErrorTracking.track_exception(exception, { - template_string: query(result), - variables: predefined_context - }) - - error(_('Malformed string')) + def gsub(string, context) + # Search for variables of the form `%{variable}` in the string and replace + # them with their value. + string.gsub(VARIABLE_INTERPOLATION_REGEX) do |match| + # Replace with the value of the variable, or if there is no such variable, + # replace the invalid variable with itself. So, + # `up{instance="%{invalid_variable}"}` will remain + # `up{instance="%{invalid_variable}"}` after substitution. + context.fetch($~[:variable], match) + end end def predefined_context - @predefined_context ||= Gitlab::Prometheus::QueryVariables.call(@environment) + Gitlab::Prometheus::QueryVariables.call(@environment).stringify_keys end def full_context diff --git a/app/views/admin/application_settings/_eks.html.haml b/app/views/admin/application_settings/_eks.html.haml index b1f7ed76281..d959b4f9b43 100644 --- a/app/views/admin/application_settings/_eks.html.haml +++ b/app/views/admin/application_settings/_eks.html.haml @@ -26,6 +26,6 @@ = f.text_field :eks_access_key_id, class: 'form-control' .form-group = f.label :eks_secret_access_key, 'Secret access key', class: 'label-bold' - = f.password_field :eks_secret_access_key, value: @application_setting.eks_secret_access_key, class: 'form-control' + = f.password_field :eks_secret_access_key, autocomplete: 'off', class: 'form-control' = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/clusters/clusters/_provider_details_form.html.haml b/app/views/clusters/clusters/_provider_details_form.html.haml index dd7d6182e3c..fcb5d4402d6 100644 --- a/app/views/clusters/clusters/_provider_details_form.html.haml +++ b/app/views/clusters/clusters/_provider_details_form.html.haml @@ -25,16 +25,10 @@ label: s_('ClusterIntegration|CA Certificate'), label_class: 'label-bold', input_group_class: 'gl-field-error-anchor', append: copy_ca_cert_btn - - show_token_btn = (platform_field.button s_('ClusterIntegration|Show'), - type: 'button', class: 'js-show-cluster-token btn btn-default') - - copy_token_btn = clipboard_button(text: platform.token, title: s_('ClusterIntegration|Copy Service Token'), - class: 'input-group-text btn-default') if cluster.read_only_kubernetes_platform_fields? - - = platform_field.text_field :token, type: 'password', class: 'js-select-on-focus js-cluster-token', - required: true, title: s_('ClusterIntegration|Service token is required.'), - readonly: cluster.read_only_kubernetes_platform_fields?, - label: s_('ClusterIntegration|Service Token'), label_class: 'label-bold', - input_group_class: 'gl-field-error-anchor', append: show_token_btn + copy_token_btn + = platform_field.password_field :token, type: 'password', class: 'js-select-on-focus js-cluster-token', + readonly: cluster.read_only_kubernetes_platform_fields?, autocomplete: 'new-password', + label: s_('ClusterIntegration|Enter new Service Token'), label_class: 'label-bold', + input_group_class: 'gl-field-error-anchor' = platform_field.form_group :authorization_type do = platform_field.check_box :authorization_type, { disabled: true, label: s_('ClusterIntegration|RBAC-enabled cluster'), diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml index beda6f05f88..c05d42a5846 100644 --- a/app/views/profiles/_email_settings.html.haml +++ b/app/views/profiles/_email_settings.html.haml @@ -5,7 +5,7 @@ - help_text = email_change_disabled ? s_("Your account uses dedicated credentials for the \"%{group_name}\" group and can only be updated through SSO.") % { group_name: @user.managing_group.name } : read_only_help_text = form.text_field :email, required: true, class: 'input-lg', value: (@user.email unless @user.temp_oauth_email?), help: help_text.html_safe, readonly: readonly || email_change_disabled -= form.select :public_email, options_for_select(@user.all_public_emails, selected: @user.public_email), += form.select :public_email, options_for_select(@user.public_verified_emails, selected: @user.public_email), { help: s_("Profiles|This email will be displayed on your public profile"), include_blank: s_("Profiles|Do not show on profile") }, control_class: 'select2 input-lg', disabled: email_change_disabled - commit_email_link_url = help_page_path('user/profile/index', anchor: 'commit-email', target: '_blank') diff --git a/app/views/profiles/notifications/_email_settings.html.haml b/app/views/profiles/notifications/_email_settings.html.haml index d2c62d3d006..7ac3ef9b141 100644 --- a/app/views/profiles/notifications/_email_settings.html.haml +++ b/app/views/profiles/notifications/_email_settings.html.haml @@ -1,6 +1,6 @@ - form = local_assigns.fetch(:form) .form-group = form.label :notification_email, class: "label-bold" - = form.select :notification_email, @user.all_public_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil) + = form.select :notification_email, @user.public_verified_emails, { include_blank: false }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil) .help-block = local_assigns.fetch(:help_text, nil) diff --git a/app/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml index 5be086948e7..a25cd78fb0b 100644 --- a/app/views/profiles/notifications/_group_settings.html.haml +++ b/app/views/profiles/notifications/_group_settings.html.haml @@ -13,4 +13,4 @@ .table-section.section-30 = form_for setting, url: profile_notifications_group_path(group), method: :put, html: { class: 'update-notifications' } do |f| - = f.select :notification_email, @user.all_public_emails, { include_blank: 'Global notification email' }, class: 'select2 js-group-notification-email' + = f.select :notification_email, @user.public_verified_emails, { include_blank: 'Global notification email' }, class: 'select2 js-group-notification-email' diff --git a/app/views/projects/deploy_keys/edit.html.haml b/app/views/projects/deploy_keys/edit.html.haml index 3e7872ebc1c..0ce93eef369 100644 --- a/app/views/projects/deploy_keys/edit.html.haml +++ b/app/views/projects/deploy_keys/edit.html.haml @@ -1,9 +1,9 @@ - page_title 'Edit Deploy Key' -%h3.page-title Edit Deploy Key +%h3.page-title= _('Edit Deploy Key') %hr %div - = form_for [@project.namespace.becomes(Namespace), @project, @deploy_key], html: { class: 'js-requires-input' } do |f| + = form_for [@project.namespace.becomes(Namespace), @project, @deploy_key], include_id: false, html: { class: 'js-requires-input' } do |f| = render partial: 'shared/deploy_keys/form', locals: { form: f, deploy_key: @deploy_key } .form-actions = f.submit 'Save changes', class: 'btn-success btn' diff --git a/app/views/shared/issuable/_close_reopen_report_toggle.html.haml b/app/views/shared/issuable/_close_reopen_report_toggle.html.haml index 0d59c9304b4..db01eb24a79 100644 --- a/app/views/shared/issuable/_close_reopen_report_toggle.html.haml +++ b/app/views/shared/issuable/_close_reopen_report_toggle.html.haml @@ -8,7 +8,7 @@ .float-left.btn-group.prepend-left-10.issuable-close-dropdown.droplab-dropdown.js-issuable-close-dropdown = link_to "#{display_button_action} #{display_issuable_type}", close_reopen_issuable_path(issuable), - method: button_method, class: "#{button_class} btn-#{button_action}", title: "#{display_button_action} #{display_issuable_type}" + method: button_method, class: "#{button_class} btn-#{button_action}", title: "#{display_button_action} #{display_issuable_type}", data: { 'close-reopen-url': close_reopen_issuable_path(issuable) } = button_tag type: 'button', class: "#{toggle_class} btn-#{button_action}-color", data: { 'dropdown-trigger' => '#issuable-close-menu' }, 'aria-label' => 'Toggle dropdown' do diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 258d8a99986..86703e50f42 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -36,6 +36,7 @@ en: access_denied: 'The resource owner or authorization server denied the request.' invalid_scope: 'The requested scope is invalid, unknown, or malformed.' server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.' + unconfirmed_email: 'Verify the email address in your account profile before you sign in.' temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.' #configuration error messages diff --git a/doc/api/projects.md b/doc/api/projects.md index a00bd442872..a748dd35016 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1181,7 +1181,7 @@ PUT /projects/:id | `approvals_before_merge` | integer | no | **(STARTER)** How many approvers should approve merge request by default | | `external_authorization_classification_label` | string | no | **(PREMIUM)** The classification label for the project | | `mirror` | boolean | no | **(STARTER)** Enables pull mirroring in a project | -| `mirror_user_id` | integer | no | **(STARTER)** User responsible for all the activity surrounding a pull mirror event | +| `mirror_user_id` | integer | no | **(STARTER)** User responsible for all the activity surrounding a pull mirror event. Can only be set by admins. | | `mirror_trigger_builds` | boolean | no | **(STARTER)** Pull mirroring triggers builds | | `only_mirror_protected_branches` | boolean | no | **(STARTER)** Only mirror protected branches | | `mirror_overwrites_diverged_branches` | boolean | no | **(STARTER)** Pull mirror overwrites diverged branches | diff --git a/lib/api/group_import.rb b/lib/api/group_import.rb index ed52506de14..ec51c2f44c3 100644 --- a/lib/api/group_import.rb +++ b/lib/api/group_import.rb @@ -4,6 +4,8 @@ module API class GroupImport < Grape::API MAXIMUM_FILE_SIZE = 50.megabytes.freeze + helpers Helpers::FileUploadHelpers + helpers do def parent_group find_group!(params[:parent_id]) if params[:parent_id].present? @@ -48,29 +50,20 @@ module API params do requires :path, type: String, desc: 'Group path' requires :name, type: String, desc: 'Group name' + requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The group export file to be imported' optional :parent_id, type: Integer, desc: "The ID of the parent group that the group will be imported into. Defaults to the current user's namespace." - optional 'file.path', type: String, desc: 'Path to locally stored body (generated by Workhorse)' - optional 'file.name', type: String, desc: 'Real filename as send in Content-Disposition (generated by Workhorse)' - optional 'file.type', type: String, desc: 'Real content type as send in Content-Type (generated by Workhorse)' - optional 'file.size', type: Integer, desc: 'Real size of file (generated by Workhorse)' - optional 'file.md5', type: String, desc: 'MD5 checksum of the file (generated by Workhorse)' - optional 'file.sha1', type: String, desc: 'SHA1 checksum of the file (generated by Workhorse)' - optional 'file.sha256', type: String, desc: 'SHA256 checksum of the file (generated by Workhorse)' end post 'import' do authorize_create_group! require_gitlab_workhorse! - - uploaded_file = UploadedFile.from_params(params, :file, ImportExportUploader.workhorse_local_upload_path) - - bad_request!('Unable to process group import file') unless uploaded_file + validate_file! group_params = { path: params[:path], name: params[:name], parent_id: params[:parent_id], visibility_level: closest_allowed_visibility_level, - import_export_upload: ImportExportUpload.new(import_file: uploaded_file) + import_export_upload: ImportExportUpload.new(import_file: params[:file]) } group = ::Groups::CreateService.new(current_user, group_params).execute diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 3717e25d997..a605160209e 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -441,6 +441,8 @@ module API not_found!("Source Project") unless fork_from_project + authorize! :fork_project, fork_from_project + result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project) if result diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 0b2df85f61f..bf4f08ce390 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -6,6 +6,8 @@ module API class Repositories < Grape::API include PaginationParams + helpers ::API::Helpers::HeadersHelpers + before { authorize! :download_code, user_project } params do @@ -67,6 +69,8 @@ module API get ':id/repository/blobs/:sha/raw' do assign_blob_vars! + no_cache_headers + send_git_blob @repo, @blob end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 53d9de6dfc7..4b34a838e44 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4192,9 +4192,6 @@ msgstr "" msgid "ClusterIntegration|Copy Kubernetes cluster name" msgstr "" -msgid "ClusterIntegration|Copy Service Token" -msgstr "" - msgid "ClusterIntegration|Could not load IAM roles" msgstr "" @@ -4273,6 +4270,9 @@ msgstr "" msgid "ClusterIntegration|Enabled stack" msgstr "" +msgid "ClusterIntegration|Enter new Service Token" +msgstr "" + msgid "ClusterIntegration|Enter the details for your Amazon EKS Kubernetes cluster" msgstr "" @@ -4345,9 +4345,6 @@ msgstr "" msgid "ClusterIntegration|Helm streamlines installing and managing Kubernetes applications. Tiller runs inside of your Kubernetes Cluster, and manages releases of your charts." msgstr "" -msgid "ClusterIntegration|Hide" -msgstr "" - msgid "ClusterIntegration|If you are setting up multiple clusters and are using Auto DevOps, %{help_link_start}read this first%{help_link_end}." msgstr "" @@ -4711,9 +4708,6 @@ msgstr "" msgid "ClusterIntegration|Set a prefix for your namespaces. If not set, defaults to your project path. If modified, existing environments will use their current namespaces until the cluster cache is cleared." msgstr "" -msgid "ClusterIntegration|Show" -msgstr "" - msgid "ClusterIntegration|Something went wrong on our end." msgstr "" @@ -12071,9 +12065,6 @@ msgstr "" msgid "Makes this issue confidential." msgstr "" -msgid "Malformed string" -msgstr "" - msgid "Manage" msgstr "" @@ -20537,9 +20528,6 @@ msgstr "" msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches." msgstr "" -msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches. Upon creation or when reassigning you can only assign yourself to be the mirror user." -msgstr "" - msgid "This variable can not be masked" msgstr "" @@ -23146,6 +23134,9 @@ msgstr "" msgid "You will be removed from existing projects/groups" msgstr "" +msgid "You will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches." +msgstr "" + msgid "You will lose all changes you've made to this file. This action cannot be undone." msgstr "" @@ -23377,6 +23368,9 @@ msgstr "" msgid "Your projects" msgstr "" +msgid "Your request for access could not be processed: %{error_meesage}" +msgstr "" + msgid "Your request for access has been queued for review." msgstr "" @@ -23773,6 +23767,9 @@ msgstr "" msgid "email '%{email}' does not match the allowed domain of '%{email_domain}'" msgstr "" +msgid "email '%{email}' is not a verified email." +msgstr "" + msgid "enabled" msgstr "" diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 05c4743ed7f..3ea5392524d 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -135,6 +135,46 @@ describe Admin::ApplicationSettingsController do end end + describe 'PATCH #integrations' do + before do + stub_feature_flags(instance_level_integrations: false) + sign_in(admin) + end + + describe 'EKS integration' do + let(:application_setting) { ApplicationSetting.current } + let(:settings_params) do + { + eks_integration_enabled: '1', + eks_account_id: '123456789012', + eks_access_key_id: 'dummy access key', + eks_secret_access_key: 'dummy secret key' + } + end + + it 'updates EKS settings' do + patch :integrations, params: { application_setting: settings_params } + + expect(application_setting.eks_integration_enabled).to be_truthy + expect(application_setting.eks_account_id).to eq '123456789012' + expect(application_setting.eks_access_key_id).to eq 'dummy access key' + expect(application_setting.eks_secret_access_key).to eq 'dummy secret key' + end + + context 'secret access key is blank' do + let(:settings_params) { { eks_integration_enabled: '0', eks_secret_access_key: '' } } + + it 'does not update the secret key' do + application_setting.update!(eks_secret_access_key: 'dummy secret key') + + patch :integrations, params: { application_setting: settings_params } + + expect(application_setting.reload.eks_secret_access_key).to eq 'dummy secret key' + end + end + end + end + describe 'PUT #reset_registration_token' do before do sign_in(admin) diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 1b4bebd9707..f975502ca4e 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' describe Oauth::AuthorizationsController do - let(:user) { create(:user) } let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') } let(:params) do { @@ -19,53 +18,68 @@ describe Oauth::AuthorizationsController do end describe 'GET #new' do - context 'without valid params' do - it 'returns 200 code and renders error view' do - get :new + context 'when the user is confirmed' do + let(:user) { create(:user) } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template('doorkeeper/authorizations/error') + context 'without valid params' do + it 'returns 200 code and renders error view' do + get :new + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/error') + end end - end - context 'with valid params' do - render_views + context 'with valid params' do + render_views - it 'returns 200 code and renders view' do - get :new, params: params + it 'returns 200 code and renders view' do + get :new, params: params - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template('doorkeeper/authorizations/new') - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/new') + end - it 'deletes session.user_return_to and redirects when skip authorization' do - application.update(trusted: true) - request.session['user_return_to'] = 'http://example.com' + it 'deletes session.user_return_to and redirects when skip authorization' do + application.update(trusted: true) + request.session['user_return_to'] = 'http://example.com' - get :new, params: params + get :new, params: params - expect(request.session['user_return_to']).to be_nil - expect(response).to have_gitlab_http_status(:found) - end + expect(request.session['user_return_to']).to be_nil + expect(response).to have_gitlab_http_status(:found) + end - context 'when there is already an access token for the application' do - context 'when the request scope matches any of the created token scopes' do - before do - scopes = Doorkeeper::OAuth::Scopes.from_string('api') + context 'when there is already an access token for the application' do + context 'when the request scope matches any of the created token scopes' do + before do + scopes = Doorkeeper::OAuth::Scopes.from_string('api') - allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes) + allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes) - create :oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes - end + create :oauth_access_token, application: application, resource_owner_id: user.id, scopes: scopes + end - it 'authorizes the request and redirects' do - get :new, params: params + it 'authorizes the request and redirects' do + get :new, params: params - expect(request.session['user_return_to']).to be_nil - expect(response).to have_gitlab_http_status(:found) + expect(request.session['user_return_to']).to be_nil + expect(response).to have_gitlab_http_status(:found) + end end end end end + + context 'when the user is unconfirmed' do + let(:user) { create(:user, confirmed_at: nil) } + + it 'returns 200 and renders error view' do + get :new, params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/error') + end + end end end diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb index 47d6f11fecf..343f29ef687 100644 --- a/spec/controllers/profiles/notifications_controller_spec.rb +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' describe Profiles::NotificationsController do let(:user) do create(:user) do |user| - user.emails.create(email: 'original@example.com') - user.emails.create(email: 'new@example.com') + user.emails.create(email: 'original@example.com', confirmed_at: Time.current) + user.emails.create(email: 'new@example.com', confirmed_at: Time.current) user.notification_email = 'original@example.com' user.save! end diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index a97f9ebf36b..66a0c735de5 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -256,7 +256,7 @@ describe Projects::DeployKeysController do end def deploy_key_params(title, can_push) - deploy_keys_projects_attributes = { '0' => { id: deploy_keys_project, can_push: can_push } } + deploy_keys_projects_attributes = { '0' => { can_push: can_push } } { deploy_key: { title: title, deploy_keys_projects_attributes: deploy_keys_projects_attributes } } end @@ -300,6 +300,42 @@ describe Projects::DeployKeysController do expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) end end + + context 'when a different deploy key id param is injected' do + let(:extra_params) { deploy_key_params('updated title', '1') } + let(:hacked_params) do + extra_params.reverse_merge(id: other_deploy_key_id, + namespace_id: project.namespace, + project_id: project) + end + + subject { put :update, params: hacked_params } + + context 'and that deploy key id exists' do + let(:other_project) { create(:project) } + let(:other_deploy_key) do + key = create(:deploy_key) + project.deploy_keys << key + key + end + + let(:other_deploy_key_id) { other_deploy_key.id } + + it 'does not update the can_push attribute' do + expect { subject }.not_to change { deploy_key.deploy_keys_project_for(project).can_push } + end + end + + context 'and that deploy key id does not exist' do + let(:other_deploy_key_id) { 9999 } + + it 'returns 404' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end context 'with admin as project maintainer' do diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 34f6da682b6..aacd1154e85 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -40,6 +40,10 @@ FactoryBot.define do after(:build) { |user, _| user.block! } end + trait :unconfirmed do + confirmed_at { nil } + end + trait :with_avatar do avatar { fixture_file_upload('spec/fixtures/dk.png') } end diff --git a/spec/features/groups/clusters/user_spec.rb b/spec/features/groups/clusters/user_spec.rb index e9ef66e31a2..a29afba99e4 100644 --- a/spec/features/groups/clusters/user_spec.rb +++ b/spec/features/groups/clusters/user_spec.rb @@ -39,7 +39,7 @@ describe 'User Cluster', :js do expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) .to have_content('http://example.com') expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value) - .to have_content('my-token') + .to be_empty end end diff --git a/spec/features/oauth_provider_authorize_spec.rb b/spec/features/oauth_provider_authorize_spec.rb new file mode 100644 index 00000000000..284fe3b0af9 --- /dev/null +++ b/spec/features/oauth_provider_authorize_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'OAuth Provider' do + describe 'Standard OAuth Authorization' do + let(:application) { create(:oauth_application, scopes: 'read_user') } + + before do + sign_in(user) + + visit oauth_authorization_path(client_id: application.uid, + redirect_uri: application.redirect_uri.split.first, + response_type: 'code', + state: 'my_state', + scope: 'read_user') + end + + it_behaves_like 'Secure OAuth Authorizations' + end +end diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb index 79676927fa2..5c82d848563 100644 --- a/spec/features/projects/clusters/user_spec.rb +++ b/spec/features/projects/clusters/user_spec.rb @@ -46,7 +46,7 @@ describe 'User Cluster', :js do expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) .to have_content('http://example.com') expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value) - .to have_content('my-token') + .to be_empty end it 'user sees RBAC is enabled by default' do diff --git a/spec/frontend/clusters/clusters_bundle_spec.js b/spec/frontend/clusters/clusters_bundle_spec.js index d7c648bcd20..9d0ed423759 100644 --- a/spec/frontend/clusters/clusters_bundle_spec.js +++ b/spec/frontend/clusters/clusters_bundle_spec.js @@ -82,28 +82,6 @@ describe('Clusters', () => { }); }); - describe('showToken', () => { - it('should update token field type', () => { - cluster.showTokenButton.click(); - - expect(cluster.tokenField.getAttribute('type')).toEqual('text'); - - cluster.showTokenButton.click(); - - expect(cluster.tokenField.getAttribute('type')).toEqual('password'); - }); - - it('should update show token button text', () => { - cluster.showTokenButton.click(); - - expect(cluster.showTokenButton.textContent).toEqual('Hide'); - - cluster.showTokenButton.click(); - - expect(cluster.showTokenButton.textContent).toEqual('Show'); - }); - }); - describe('checkForNewInstalls', () => { const INITIAL_APP_MAP = { helm: { status: null, title: 'Helm Tiller' }, diff --git a/spec/frontend/fixtures/static/issue_with_mermaid_graph.html b/spec/frontend/fixtures/static/issue_with_mermaid_graph.html new file mode 100644 index 00000000000..4b60842a655 --- /dev/null +++ b/spec/frontend/fixtures/static/issue_with_mermaid_graph.html @@ -0,0 +1,82 @@ +<div class="description" updated-at=""> + <div class="md issue-realtime-trigger-pulse"> + <svg + id="mermaid-1587752414912" + width="100%" + xmlns="http://www.w3.org/2000/svg" + style="max-width: 185.35000610351562px;" + viewBox="0 0 185.35000610351562 50.5" + class="mermaid" + > + <g transform="translate(0, 0)"> + <g class="output"> + <g class="clusters"></g> + <g class="edgePaths"></g> + <g class="edgeLabels"></g> + <g class="nodes"> + <g + class="node js-issuable-actions btn-close clickable" + style="opacity: 1;" + id="A" + transform="translate(92.67500305175781,25.25)" + title="click to PUT" + > + <a + class="js-issuable-actions btn-close clickable" + href="https://invalid" + rel="noopener" + > + <rect + rx="0" + ry="0" + x="-84.67500305175781" + y="-17.25" + width="169.35000610351562" + height="34.5" + class="label-container" + ></rect> + <g class="label" transform="translate(0,0)"> + <g transform="translate(-74.67500305175781,-7.25)"> + <text style=""> + <tspan xml:space="preserve" dy="1em" x="1">Click to send a PUT request</tspan> + </text> + </g> + </g> + </a> + </g> + </g> + </g> + </g> + <text class="source" display="none"> + Click to send a PUT request + </text> + </svg> + </div> + <textarea + data-update-url="/h5bp/html5-boilerplate/-/issues/35.json" + dir="auto" + class="hidden js-task-list-field" + ></textarea> + <div class="modal-open recaptcha-modal js-recaptcha-modal" style="display: none;"> + <div role="dialog" tabindex="-1" class="modal d-block"> + <div role="document" class="modal-dialog"> + <div class="modal-content"> + <div class="modal-header"> + <h4 class="modal-title float-left">Please solve the reCAPTCHA</h4> + <button type="button" data-dismiss="modal" aria-label="Close" class="close float-right"> + <span aria-hidden="true">×</span> + </button> + </div> + <div class="modal-body"> + <div> + <p>We want to be sure it is you, please confirm you are not a robot.</p> + <div></div> + </div> + </div> + <!----> + </div> + </div> + </div> + <div class="modal-backdrop fade show"></div> + </div> +</div> diff --git a/spec/frontend/issue_spec.js b/spec/frontend/issue_spec.js index 586bd7f8529..24020daf728 100644 --- a/spec/frontend/issue_spec.js +++ b/spec/frontend/issue_spec.js @@ -18,6 +18,7 @@ describe('Issue', () => { preloadFixtures('issues/closed-issue.html'); preloadFixtures('issues/issue-with-task-list.html'); preloadFixtures('issues/open-issue.html'); + preloadFixtures('static/issue_with_mermaid_graph.html'); function expectErrorMessage() { const $flashMessage = $('div.flash-alert'); @@ -228,4 +229,30 @@ describe('Issue', () => { }); }); }); + + describe('when not displaying blocked warning', () => { + describe('when clicking a mermaid graph inside an issue description', () => { + let mock; + let spy; + + beforeEach(() => { + loadFixtures('static/issue_with_mermaid_graph.html'); + mock = new MockAdapter(axios); + spy = jest.spyOn(axios, 'put'); + }); + + afterEach(() => { + mock.restore(); + jest.clearAllMocks(); + }); + + it('does not make a PUT request', () => { + Issue.prototype.initIssueBtnEventListeners(); + + $('svg a.js-issuable-actions').trigger('click'); + + expect(spy).not.toHaveBeenCalled(); + }); + }); + }); }); diff --git a/spec/frontend/monitoring/components/duplicate_dashboard_form_spec.js b/spec/frontend/monitoring/components/duplicate_dashboard_form_spec.js index 10fd58f749d..61d5f7a99d3 100644 --- a/spec/frontend/monitoring/components/duplicate_dashboard_form_spec.js +++ b/spec/frontend/monitoring/components/duplicate_dashboard_form_spec.js @@ -3,9 +3,17 @@ import DuplicateDashboardForm from '~/monitoring/components/duplicate_dashboard_ import { dashboardGitResponse } from '../mock_data'; -describe('DuplicateDashboardForm', () => { - let wrapper; +let wrapper; + +const createMountedWrapper = (props = {}) => { + // Use `mount` to render native input elements + wrapper = mount(DuplicateDashboardForm, { + propsData: { ...props }, + sync: false, + }); +}; +describe('DuplicateDashboardForm', () => { const defaultBranch = 'master'; const findByRef = ref => wrapper.find({ ref }); @@ -20,14 +28,7 @@ describe('DuplicateDashboardForm', () => { }; beforeEach(() => { - // Use `mount` to render native input elements - wrapper = mount(DuplicateDashboardForm, { - propsData: { - dashboard: dashboardGitResponse[0], - defaultBranch, - }, - sync: false, - }); + createMountedWrapper({ dashboard: dashboardGitResponse[0], defaultBranch }); }); it('renders correctly', () => { @@ -144,3 +145,18 @@ describe('DuplicateDashboardForm', () => { }); }); }); + +describe('DuplicateDashboardForm escapes elements', () => { + const branchToEscape = "<img/src='x'onerror=alert(document.domain)>"; + + beforeEach(() => { + createMountedWrapper({ dashboard: dashboardGitResponse[0], defaultBranch: branchToEscape }); + }); + + it('should escape branch name data', () => { + const branchOptionHtml = wrapper.vm.branchOptions[0].html; + const escapedBranch = '<img/src='x'onerror=alert(document.domain)>'; + + expect(branchOptionHtml).toEqual(expect.stringContaining(escapedBranch)); + }); +}); diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 576ac880fca..589b12599f6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -108,6 +108,11 @@ describe Group do let(:group_notification_email) { 'user+group@example.com' } let(:subgroup_notification_email) { 'user+subgroup@example.com' } + before do + create(:email, :confirmed, user: user, email: group_notification_email) + create(:email, :confirmed, user: user, email: subgroup_notification_email) + end + subject { subgroup.notification_email_for(user) } context 'when both group notification emails are set' do diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 9ab9ae494ec..67738eaec20 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -48,6 +48,33 @@ RSpec.describe NotificationSetting do expect(notification_setting.reopen_merge_request).to eq(false) end end + + context 'notification_email' do + let_it_be(:user) { create(:user) } + subject { described_class.new(source_id: 1, source_type: 'Project', user_id: user.id) } + + it 'allows to change email to verified one' do + email = create(:email, :confirmed, user: user) + + subject.update(notification_email: email.email) + + expect(subject).to be_valid + end + + it 'does not allow to change email to not verified one' do + email = create(:email, user: user) + + subject.update(notification_email: email.email) + + expect(subject).to be_invalid + end + + it 'allows to change email to empty one' do + subject.update(notification_email: '') + + expect(subject).to be_valid + end + end end describe '#for_projects' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 849494e7cd4..44a6ec476f3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -235,7 +235,7 @@ describe User, :do_not_mock_admin_mode do end it_behaves_like 'an object with email-formated attributes', :public_email, :notification_email do - subject { build(:user).tap { |user| user.emails << build(:email, email: email_value) } } + subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } } end describe '#commit_email' do @@ -504,6 +504,32 @@ describe User, :do_not_mock_admin_mode do user = build(:user, email: "temp-email-for-oauth@example.com") expect(user).to be_valid end + + it 'does not accept not verified emails' do + email = create(:email) + user = email.user + user.update(notification_email: email.email) + + expect(user).to be_invalid + end + end + + context 'owns_public_email' do + it 'accepts verified emails' do + email = create(:email, :confirmed, email: 'test@test.com') + user = email.user + user.update(public_email: email.email) + + expect(user).to be_valid + end + + it 'does not accept not verified emails' do + email = create(:email) + user = email.user + user.update(public_email: email.email) + + expect(user).to be_invalid + end end context 'set_commit_email' do @@ -853,6 +879,108 @@ describe User, :do_not_mock_admin_mode do expect(@user.emails.count).to eq 1 expect(@user.emails.first.confirmed_at).not_to eq nil end + + context 'when the first email was unconfirmed and the second email gets confirmed' do + let(:user) { create(:user, :unconfirmed, email: 'should-be-unconfirmed@test.com') } + + before do + user.update!(email: 'should-be-confirmed@test.com') + user.confirm + end + + it 'updates user.email' do + expect(user.email).to eq('should-be-confirmed@test.com') + end + + it 'confirms user.email' do + expect(user).to be_confirmed + end + + it 'keeps the unconfirmed email unconfirmed' do + email = user.emails.first + + expect(email.email).to eq('should-be-unconfirmed@test.com') + expect(email).not_to be_confirmed + end + + it 'has only one email association' do + expect(user.emails.size).to be(1) + end + end + end + + context 'when an existing email record is set as primary' do + let(:user) { create(:user, email: 'confirmed@test.com') } + + context 'when it is unconfirmed' do + let(:originally_unconfirmed_email) { 'should-stay-unconfirmed@test.com' } + + before do + user.emails << create(:email, email: originally_unconfirmed_email, confirmed_at: nil) + + user.update!(email: originally_unconfirmed_email) + end + + it 'keeps the user confirmed' do + expect(user).to be_confirmed + end + + it 'keeps the original email' do + expect(user.email).to eq('confirmed@test.com') + end + + context 'when the email gets confirmed' do + before do + user.confirm + end + + it 'keeps the user confirmed' do + expect(user).to be_confirmed + end + + it 'updates the email' do + expect(user.email).to eq(originally_unconfirmed_email) + end + end + end + + context 'when it is confirmed' do + let!(:old_confirmed_email) { user.email } + let(:confirmed_email) { 'already-confirmed@test.com' } + + before do + user.emails << create(:email, :confirmed, email: confirmed_email) + + user.update!(email: confirmed_email) + end + + it 'keeps the user confirmed' do + expect(user).to be_confirmed + end + + it 'updates the email' do + expect(user.email).to eq(confirmed_email) + end + + it 'moves the old email' do + email = user.reload.emails.first + + expect(email.email).to eq(old_confirmed_email) + expect(email).to be_confirmed + end + end + end + + context 'when unconfirmed user deletes a confirmed additional email' do + let(:user) { create(:user, :unconfirmed) } + + before do + user.emails << create(:email, :confirmed) + end + + it 'does not affect the confirmed status' do + expect { user.emails.confirmed.destroy_all }.not_to change { user.confirmed? } # rubocop: disable Cop/DestroyAll + end end describe '#update_notification_email' do @@ -2052,6 +2180,31 @@ describe User, :do_not_mock_admin_mode do end end + describe '#public_verified_emails' do + let(:user) { create(:user) } + + it 'returns only confirmed public emails' do + email_confirmed = create :email, user: user, confirmed_at: Time.current + create :email, user: user + + expect(user.public_verified_emails).to contain_exactly( + user.email, + email_confirmed.email + ) + end + + it 'returns confirmed public emails plus main user email when user is not confirmed' do + user = create(:user, confirmed_at: nil) + email_confirmed = create :email, user: user, confirmed_at: Time.current + create :email, user: user + + expect(user.public_verified_emails).to contain_exactly( + user.email, + email_confirmed.email + ) + end + end + describe '#verified_email?' do let(:user) { create(:user) } @@ -4089,9 +4242,10 @@ describe User, :do_not_mock_admin_mode do context 'when an ancestor has a level other than Global' do let(:ancestor) { create(:group) } let(:group) { create(:group, parent: ancestor) } + let(:email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } before do - create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: 'ancestor@example.com') + create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: email.email) end it 'has the same level set' do @@ -4116,10 +4270,12 @@ describe User, :do_not_mock_admin_mode do let(:grand_ancestor) { create(:group) } let(:ancestor) { create(:group, parent: grand_ancestor) } let(:group) { create(:group, parent: ancestor) } + let(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } + let(:grand_email) { create(:email, :confirmed, email: 'grand@example.com', user: user) } before do - create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: 'grand@example.com') - create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: 'ancestor@example.com') + create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: grand_email.email) + create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: ancestor_email.email) end it 'has the same email set' do @@ -4157,7 +4313,7 @@ describe User, :do_not_mock_admin_mode do context 'when group has notification email set' do it 'returns group notification email' do group_notification_email = 'user+group@example.com' - + create(:email, :confirmed, user: user, email: group_notification_email) create(:notification_setting, user: user, source: group, notification_email: group_notification_email) is_expected.to eq(group_notification_email) diff --git a/spec/requests/api/group_import_spec.rb b/spec/requests/api/group_import_spec.rb index 3f85428aac2..1150d177f7a 100644 --- a/spec/requests/api/group_import_spec.rb +++ b/spec/requests/api/group_import_spec.rb @@ -11,7 +11,7 @@ describe API::GroupImport do let(:file) { File.join('spec', 'fixtures', 'group_export.tar.gz') } let(:export_path) { "#{Dir.tmpdir}/group_export_spec" } let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } before do allow_next_instance_of(Gitlab::ImportExport) do |import_export| @@ -35,7 +35,7 @@ describe API::GroupImport do } end - subject { post api('/groups/import', user), params: params, headers: workhorse_header } + subject { upload_archive(file_upload, workhorse_headers, params) } shared_examples 'when all params are correct' do context 'when user is authorized to create new group' do @@ -151,7 +151,7 @@ describe API::GroupImport do params[:file] = file_upload expect do - post api('/groups/import', user), params: params, headers: workhorse_header + upload_archive(file_upload, workhorse_headers, params) end.not_to change { Group.count }.from(1) expect(response).to have_gitlab_http_status(:bad_request) @@ -171,7 +171,7 @@ describe API::GroupImport do context 'without a file from workhorse' do it 'rejects the request' do - subject + upload_archive(nil, workhorse_headers, params) expect(response).to have_gitlab_http_status(:bad_request) end @@ -179,7 +179,7 @@ describe API::GroupImport do context 'without a workhorse header' do it 'rejects request without a workhorse header' do - post api('/groups/import', user), params: params + upload_archive(file_upload, {}, params) expect(response).to have_gitlab_http_status(:forbidden) end @@ -189,9 +189,7 @@ describe API::GroupImport do let(:params) do { path: 'test-import-group', - name: 'test-import-group', - 'file.path' => file_upload.path, - 'file.name' => file_upload.original_filename + name: 'test-import-group' } end @@ -229,9 +227,7 @@ describe API::GroupImport do { path: 'test-import-group', name: 'test-import-group', - file: fog_file, - 'file.remote_id' => file_name, - 'file.size' => fog_file.size + file: fog_file } end @@ -245,10 +241,21 @@ describe API::GroupImport do include_examples 'when some params are missing' end end + + def upload_archive(file, headers = {}, params = {}) + workhorse_finalize( + api('/groups/import', user), + method: :post, + file_key: :file, + params: params.merge(file: file), + headers: headers, + send_rewritten_field: true + ) + end end describe 'POST /groups/import/authorize' do - subject { post api('/groups/import/authorize', user), headers: workhorse_header } + subject { post api('/groups/import/authorize', user), headers: workhorse_headers } it 'authorizes importing group with workhorse header' do subject @@ -258,7 +265,7 @@ describe API::GroupImport do end it 'rejects requests that bypassed gitlab-workhorse' do - workhorse_header.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) + workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) subject diff --git a/spec/requests/api/notification_settings_spec.rb b/spec/requests/api/notification_settings_spec.rb index cbdab2f53a6..2dfde4c8ec9 100644 --- a/spec/requests/api/notification_settings_spec.rb +++ b/spec/requests/api/notification_settings_spec.rb @@ -19,7 +19,7 @@ describe API::NotificationSettings do end describe "PUT /notification_settings" do - let(:email) { create(:email, user: user) } + let(:email) { create(:email, :confirmed, user: user) } it "updates global notification settings for the current user" do put api("/notification_settings", user), params: { level: 'watch', notification_email: email.email } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 83f678ad2cb..8d0b9f88274 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1874,6 +1874,17 @@ describe API::Projects do expect(project_fork_target).to be_forked end + it 'fails without permission from forked_from project' do + project_fork_source.project_feature.update_attribute(:forking_access_level, ProjectFeature::PRIVATE) + + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(project_fork_target.forked_from_project).to be_nil + expect(project_fork_target.fork_network_member).not_to be_present + expect(project_fork_target).not_to be_forked + end + it 'denies project to be forked from a private project' do post api("/projects/#{project_fork_target.id}/fork/#{private_project_fork_source.id}", user) diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index b503c923037..e455108ae4c 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -177,6 +177,12 @@ describe API::Repositories do expect(headers['Content-Disposition']).to eq 'inline' end + it_behaves_like 'uncached response' do + before do + get api(route, current_user) + end + end + context 'when sha does not exist' do it_behaves_like '404 response' do let(:request) { get api(route.sub(sample_blob.oid, '123456'), current_user) } diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index d7c08484dc4..e5802743844 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -9,15 +9,11 @@ describe 'OpenID Connect requests' do name: 'Alice', username: 'alice', email: 'private@example.com', - emails: [public_email], - public_email: public_email.email, website_url: 'https://example.com', avatar: fixture_file_upload('spec/fixtures/dk.png') ) end - let(:public_email) { build :email, email: 'public@example.com' } - let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id } let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id } @@ -37,7 +33,7 @@ describe 'OpenID Connect requests' do 'name' => 'Alice', 'nickname' => 'alice', 'email' => 'public@example.com', - 'email_verified' => false, + 'email_verified' => true, 'website' => 'https://example.com', 'profile' => 'http://localhost/alice', 'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png", @@ -62,6 +58,11 @@ describe 'OpenID Connect requests' do get '/oauth/userinfo', params: {}, headers: { 'Authorization' => "Bearer #{access_token.token}" } end + before do + email = create(:email, :confirmed, email: 'public@example.com', user: user) + user.update!(public_email: email.email) + end + context 'Application without OpenID scope' do let(:application) { create :oauth_application, scopes: 'api' } @@ -123,7 +124,7 @@ describe 'OpenID Connect requests' do end it 'has false in email_verified claim' do - expect(json_response['email_verified']).to eq(false) + expect(json_response['email_verified']).to eq(true) end end diff --git a/spec/requests/profiles/notifications_controller_spec.rb b/spec/requests/profiles/notifications_controller_spec.rb index 41349d6c12d..0b2741677ab 100644 --- a/spec/requests/profiles/notifications_controller_spec.rb +++ b/spec/requests/profiles/notifications_controller_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' describe 'view user notifications' do let(:user) do create(:user) do |user| - user.emails.create(email: 'original@example.com') - user.emails.create(email: 'new@example.com') + user.emails.create(email: 'original@example.com', confirmed_at: Time.current) + user.emails.create(email: 'new@example.com', confirmed_at: Time.current) user.notification_email = 'original@example.com' user.save! end diff --git a/spec/services/clusters/update_service_spec.rb b/spec/services/clusters/update_service_spec.rb index d487edd8850..5a7726eded8 100644 --- a/spec/services/clusters/update_service_spec.rb +++ b/spec/services/clusters/update_service_spec.rb @@ -47,6 +47,39 @@ describe Clusters::UpdateService do expect(cluster.platform.namespace).to eq('custom-namespace') end end + + context 'when service token is empty' do + let(:params) do + { + platform_kubernetes_attributes: { + token: '' + } + } + end + + it 'does not update the token' do + current_token = cluster.platform.token + is_expected.to eq(true) + cluster.platform.reload + + expect(cluster.platform.token).to eq(current_token) + end + end + + context 'when service token is not empty' do + let(:params) do + { + platform_kubernetes_attributes: { + token: 'new secret token' + } + } + end + + it 'updates the token' do + is_expected.to eq(true) + expect(cluster.platform.token).to eq('new secret token') + end + end end context 'when invalid params' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 96906b4ca3c..762999e7877 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2413,6 +2413,8 @@ describe NotificationService, :mailer do group = create(:group) project.update(group: group) + + create(:email, :confirmed, user: u_custom_notification_enabled, email: group_notification_email) create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email) end @@ -2447,6 +2449,7 @@ describe NotificationService, :mailer do group = create(:group) project.update(group: group) + create(:email, :confirmed, user: u_member, email: group_notification_email) create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email) end @@ -2540,6 +2543,7 @@ describe NotificationService, :mailer do group = create(:group) project.update(group: group) + create(:email, :confirmed, user: u_member, email: group_notification_email) create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email) end diff --git a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb index 9978c631366..7faac1b4fc6 100644 --- a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb +++ b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb @@ -6,19 +6,46 @@ describe Prometheus::ProxyVariableSubstitutionService do describe '#execute' do let_it_be(:environment) { create(:environment) } - let(:params_keys) { { query: 'up{environment="%{ci_environment_slug}"}' } } + let(:params_keys) { { query: "up{environment=\"#{w('ci_environment_slug')}\"}" } } let(:params) { ActionController::Parameters.new(params_keys).permit! } let(:result) { subject.execute } subject { described_class.new(environment, params) } - shared_examples 'success' do + # Default implementation of the w method. The `success` shared example overrides + # this implementation to test the Ruby and Liquid syntaxes. + # This method wraps the given variable name in the variable interpolation + # syntax. Using this method along with the `success` shared example allows + # a spec to test both syntaxes. + def w(variable_name) + "%{#{variable_name}}" + end + + shared_examples 'replaces variables with values' do it 'replaces variables with values' do expect(result[:status]).to eq(:success) expect(result[:params][:query]).to eq(expected_query) end end + shared_examples 'success' do + context 'with Ruby syntax `${}`' do + it_behaves_like 'replaces variables with values' + + def w(variable_name) + "%{#{variable_name}}" + end + end + + context 'with Liquid syntax `{{}}`' do + it_behaves_like 'replaces variables with values' + + def w(variable_name) + "{{#{variable_name}}}" + end + end + end + shared_examples 'error' do |message| it 'returns error' do expect(result[:status]).to eq(:error) @@ -39,11 +66,13 @@ describe Prometheus::ProxyVariableSubstitutionService do end context 'with predefined variables' do - let(:params_keys) { { query: 'up{%{environment_filter}}' } } + # Liquid replaces the opening brace of the query as well, if there is no space + # between `up{` and the rest of the string. + let(:params_keys) { { query: "up{ #{w('environment_filter')}}" } } it_behaves_like 'success' do let(:expected_query) do - %Q[up{container_name!="POD",environment="#{environment.slug}"}] + %Q[up{ container_name!="POD",environment="#{environment.slug}"}] end end @@ -54,26 +83,16 @@ describe Prometheus::ProxyVariableSubstitutionService do let(:expected_query) { nil } end end + end - context 'with liquid format' do - let(:params_keys) do - { query: 'up{environment="{{ci_environment_slug}}"}' } - end - - it_behaves_like 'success' do - let(:expected_query) { %Q[up{environment="#{environment.slug}"}] } - end + context 'with ruby and liquid formats' do + let(:params_keys) do + { query: 'up{%{environment_filter},env2="{{ci_environment_slug}}"}' } end - context 'with ruby and liquid formats' do - let(:params_keys) do - { query: 'up{%{environment_filter},env2="{{ci_environment_slug}}"}' } - end - - it_behaves_like 'success' do - let(:expected_query) do - %Q[up{container_name!="POD",environment="#{environment.slug}",env2="#{environment.slug}"}] - end + it_behaves_like 'success' do + let(:expected_query) do + %Q[up{container_name!="POD",environment="#{environment.slug}",env2="#{environment.slug}"}] end end end @@ -83,7 +102,7 @@ describe Prometheus::ProxyVariableSubstitutionService do let(:params_keys) do { - query: 'up{pod_name="{{pod_name}}"}', + query: "up{pod_name=\"#{w('pod_name')}\"}", variables: ['pod_name', pod_name] } end @@ -92,24 +111,10 @@ describe Prometheus::ProxyVariableSubstitutionService do let(:expected_query) { %q[up{pod_name="pod1"}] } end - context 'with ruby variable interpolation format' do - let(:params_keys) do - { - query: 'up{pod_name="%{pod_name}"}', - variables: ['pod_name', pod_name] - } - end - - it_behaves_like 'success' do - # Custom variables cannot be used with the Ruby interpolation format. - let(:expected_query) { "up{pod_name=\"%{pod_name}\"}" } - end - end - context 'with predefined variables in variables parameter' do let(:params_keys) do { - query: 'up{pod_name="{{pod_name}}",env="{{ci_environment_slug}}"}', + query: "up{pod_name=\"#{w('pod_name')}\",env=\"#{w('ci_environment_slug')}\"}", variables: ['pod_name', pod_name, 'ci_environment_slug', 'custom_value'] } end @@ -124,7 +129,7 @@ describe Prometheus::ProxyVariableSubstitutionService do context 'with invalid variables parameter' do let(:params_keys) do { - query: 'up{pod_name="{{pod_name}}"}', + query: "up{pod_name=\"#{w('pod_name')}\"}", variables: ['a'] } end @@ -136,68 +141,66 @@ describe Prometheus::ProxyVariableSubstitutionService do context 'with nil variables' do let(:params_keys) do { - query: 'up{pod_name="{{pod_name}}"}', + query: "up{pod_name=\"%{pod_name}\"}", variables: nil } end - it_behaves_like 'success' do - let(:expected_query) { 'up{pod_name=""}' } + it_behaves_like 'replaces variables with values' do + let(:expected_query) { "up{pod_name=\"%{pod_name}\"}" } end end + end - context 'with ruby and liquid variables' do + context 'gsub variable substitution tolerance for weirdness' do + context 'with whitespace around variable' do let(:params_keys) do { - query: 'up{env1="%{ruby_variable}",env2="{{ liquid_variable }}"}', - variables: %w(ruby_variable value liquid_variable env_slug) + query: 'up{' \ + "env1=#{w(' ci_environment_slug')}," \ + "env2=#{w('ci_environment_slug ')}," \ + "#{w(' environment_filter ')}" \ + '}' } end it_behaves_like 'success' do - # It should replace only liquid variables with their values - let(:expected_query) { %q[up{env1="%{ruby_variable}",env2="env_slug"}] } + let(:expected_query) do + 'up{' \ + "env1=#{environment.slug}," \ + "env2=#{environment.slug}," \ + "container_name!=\"POD\",environment=\"#{environment.slug}\"" \ + '}' + end end end - end - - context 'with liquid tags and ruby format variables' do - let(:params_keys) do - { - query: 'up{ {% if true %}env1="%{ci_environment_slug}",' \ - 'env2="{{ci_environment_slug}}"{% endif %} }' - } - end - # The following spec will fail and should be changed to a 'success' spec - # once we remove support for the Ruby interpolation format. - # https://gitlab.com/gitlab-org/gitlab/issues/37990 - # - # Liquid tags `{% %}` cannot be used currently because the Ruby `%` - # operator raises an error when it encounters a Liquid `{% %}` tag in the - # string. - # - # Once we remove support for the Ruby format, users can start using - # Liquid tags. - - it_behaves_like 'error', 'Malformed string' - end + context 'with liquid tags and ruby format variables' do + let(:params_keys) do + { + query: 'up{ {% if true %}env1="%{ci_environment_slug}",' \ + 'env2="{{ci_environment_slug}}"{% endif %} }' + } + end - context 'ruby template rendering' do - let(:params_keys) do - { query: 'up{env=%{ci_environment_slug},%{environment_filter}}' } + it_behaves_like 'replaces variables with values' do + let(:expected_query) { "up{ env1=\"#{environment.slug}\",env2=\"#{environment.slug}\" }" } + end end - it_behaves_like 'success' do - let(:expected_query) do - "up{env=#{environment.slug},container_name!=\"POD\"," \ - "environment=\"#{environment.slug}\"}" + context 'with empty variables' do + let(:params_keys) do + { query: "up{env1=%{},env2=%{ }}" } + end + + it_behaves_like 'replaces variables with values' do + let(:expected_query) { "up{env1=%{},env2=%{ }}" } end end context 'with multiple occurrences of variable in string' do let(:params_keys) do - { query: 'up{env1=%{ci_environment_slug},env2=%{ci_environment_slug}}' } + { query: "up{env1=#{w('ci_environment_slug')},env2=#{w('ci_environment_slug')}}" } end it_behaves_like 'success' do @@ -207,7 +210,7 @@ describe Prometheus::ProxyVariableSubstitutionService do context 'with multiple variables in string' do let(:params_keys) do - { query: 'up{env=%{ci_environment_slug},%{environment_filter}}' } + { query: "up{env=#{w('ci_environment_slug')},#{w('environment_filter')}}" } end it_behaves_like 'success' do @@ -219,54 +222,24 @@ describe Prometheus::ProxyVariableSubstitutionService do end context 'with unknown variables in string' do - let(:params_keys) { { query: 'up{env=%{env_slug}}' } } + let(:params_keys) { { query: "up{env=#{w('env_slug')}}" } } - it_behaves_like 'success' do - let(:expected_query) { 'up{env=%{env_slug}}' } - end - end - - # This spec is needed if there are multiple keys in the context provided - # by `Gitlab::Prometheus::QueryVariables.call(environment)` which is - # passed to the Ruby `%` operator. - # If the number of keys in the context is one, there is no need for - # this spec. - context 'with extra variables in context' do - let(:params_keys) { { query: 'up{env=%{ci_environment_slug}}' } } - - it_behaves_like 'success' do - let(:expected_query) { "up{env=#{environment.slug}}" } - end - - it 'has more than one variable in context' do - expect(Gitlab::Prometheus::QueryVariables.call(environment).size).to be > 1 + it_behaves_like 'replaces variables with values' do + let(:expected_query) { "up{env=%{env_slug}}" } end end # The ruby % operator will not replace known variables if there are unknown # variables also in the string. It doesn't raise an error # (though the `sprintf` and `format` methods do). + # Fortunately, we do not use the % operator anymore. context 'with unknown and known variables in string' do let(:params_keys) do - { query: 'up{env=%{ci_environment_slug},other_env=%{env_slug}}' } + { query: "up{env=%{ci_environment_slug},other_env=%{env_slug}}" } end - it_behaves_like 'success' do - let(:expected_query) { 'up{env=%{ci_environment_slug},other_env=%{env_slug}}' } - end - end - - context 'when rendering raises error' do - context 'when TypeError is raised' do - let(:params_keys) { { query: '{% a %}' } } - - it_behaves_like 'error', 'Malformed string' - end - - context 'when ArgumentError is raised' do - let(:params_keys) { { query: '%<' } } - - it_behaves_like 'error', 'Malformed string' + it_behaves_like 'replaces variables with values' do + let(:expected_query) { "up{env=#{environment.slug},other_env=#{w('env_slug')}}" } end end end diff --git a/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb b/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb new file mode 100644 index 00000000000..028e075c87a --- /dev/null +++ b/spec/support/shared_examples/features/secure_oauth_authorizations_shared_examples.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'Secure OAuth Authorizations' do + context 'when user is confirmed' do + let(:user) { create(:user) } + + it 'asks the user to authorize the application' do + expect(page).to have_text "Authorize #{application.name} to use your account?" + end + end + + context 'when user is unconfirmed' do + let(:user) { create(:user, confirmed_at: nil) } + + it 'displays an error' do + expect(page).to have_text I18n.t('doorkeeper.errors.messages.unconfirmed_email') + end + end +end diff --git a/spec/support/shared_examples/mailers/notify_shared_examples.rb b/spec/support/shared_examples/mailers/notify_shared_examples.rb index 45987059123..1f5803b90a0 100644 --- a/spec/support/shared_examples/mailers/notify_shared_examples.rb +++ b/spec/support/shared_examples/mailers/notify_shared_examples.rb @@ -28,6 +28,7 @@ RSpec.shared_examples 'an email sent to a user' do it 'is sent to user\'s group notification email' do group_notification_email = 'user+group@example.com' + create(:email, :confirmed, user: recipient, email: group_notification_email) create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email) expect(subject).to deliver_to(group_notification_email) diff --git a/spec/support/shared_examples/uncached_response_shared_examples.rb b/spec/support/shared_examples/uncached_response_shared_examples.rb new file mode 100644 index 00000000000..3997017ff35 --- /dev/null +++ b/spec/support/shared_examples/uncached_response_shared_examples.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true +# +# Pairs with lib/gitlab/no_cache_headers.rb +# + +RSpec.shared_examples 'uncached response' do + it 'defines an uncached header response' do + expect(response.headers["Cache-Control"]).to include("no-store", "no-cache") + expect(response.headers["Pragma"]).to eq("no-cache") + expect(response.headers["Expires"]).to eq("Fri, 01 Jan 1990 00:00:00 GMT") + end +end diff --git a/spec/views/admin/application_settings/_eks.html.haml_spec.rb b/spec/views/admin/application_settings/_eks.html.haml_spec.rb new file mode 100644 index 00000000000..52434557d3a --- /dev/null +++ b/spec/views/admin/application_settings/_eks.html.haml_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'admin/application_settings/_eks' do + let_it_be(:admin) { create(:admin) } + let(:page) { Capybara::Node::Simple.new(rendered) } + + before do + assign(:application_setting, application_setting) + allow(view).to receive(:current_user) { admin } + allow(view).to receive(:expanded) { true } + end + + shared_examples 'EKS secret access key input' do + it 'renders an empty password field' do + render + expect(rendered).to have_field('Secret access key', type: 'password') + expect(page.find_field('Secret access key').value).to be_blank + end + end + + context 'when eks_secret_access_key is not set' do + let(:application_setting) { build(:application_setting) } + + include_examples 'EKS secret access key input' + end + + context 'when eks_secret_access_key is set' do + let(:application_setting) { build(:application_setting, eks_secret_access_key: 'eks_secret_access_key') } + + include_examples 'EKS secret access key input' + end +end |