diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-12 03:08:51 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-12 03:08:51 +0300 |
commit | 98a00b024553a603e16380b93fc3f89a169bf438 (patch) | |
tree | 4a6dade546a6857bffc21df2126b113528d746d9 | |
parent | c53e365d68ee800702befb15adfdfac708d5de6f (diff) |
Add latest changes from gitlab-org/gitlab@master
46 files changed, 1163 insertions, 109 deletions
diff --git a/.gitignore b/.gitignore index b6f5a8f514a..9aff9469db3 100644 --- a/.gitignore +++ b/.gitignore @@ -110,6 +110,7 @@ tags.temp jest-snapshot-test-match.json jest-test-report.json jest-snapshot-test-report.json +/go.work.sum # Vite Ruby /public/vite* diff --git a/.gitlab/issue_templates/Pipeline Execution Refinement Spike.md b/.gitlab/issue_templates/Pipeline Execution Refinement Spike.md index 38877ad777a..d456d7bdb5b 100644 --- a/.gitlab/issue_templates/Pipeline Execution Refinement Spike.md +++ b/.gitlab/issue_templates/Pipeline Execution Refinement Spike.md @@ -1,4 +1,3 @@ -< ## Author Checklist (_NOTE: This section can be removed when the issue is ready for creation_) - [ ] Set title to "Spike: Refine "<original issue title>" @@ -6,9 +5,9 @@ - [ ] Update the weight slash command to an appropriate value based on the timebox size - [ ] Add `~frontend`, `~backend`, or `~documentation` labels as appropriate - [ ] Update the expected outcomes if the default ones don't capture everything that should be done in this spike. +- [ ] Run `/copy_metadata #<original issue>` to copy over labels /blocks #<original issue> -/copy_metadata #<original issue> /label ~"spike" ~"group::pipeline execution" ~"section::ci" ~"devops::verify" ~"workflow::ready for development" ~"cicd::planning" /weight ## @@ -17,17 +16,12 @@ ## Timebox Expectations -<How much time should be spent on this spike> +_How much time should be spent on this spike_ ## Expected Outcomes -- [ ] Possible cause(s) are determined +- [ ] Possible cause(s) are determined - [ ] Technical proposal added to #<original issue> - [ ] Weight added to #<original issue> - ## Links/References - - - - diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eb032de45f..fa03f476320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 16.7.2 (2024-01-10) + +### Fixed (1 change) + +- [Add missing ci_sources_pipelines indexes for self-host](gitlab-org/security/gitlab@1e0c4e44228a3ec8013b70e5ef33ac3184f6cb4c) ([merge request](gitlab-org/security/gitlab!3809)) + +### Changed (1 change) + +- [Enable Apollo Boards by default](gitlab-org/security/gitlab@dc1e1e8368fce9ca8c407d439877980e0426b356) ([merge request](gitlab-org/security/gitlab!3809)) + +### Security (4 changes) + +- [Consider older commits when resetting codeowner approvals](gitlab-org/security/gitlab@d20f815258ac8f7195a04aeab760645993354485) ([merge request](gitlab-org/security/gitlab!3764)) +- [Password reset followups](gitlab-org/security/gitlab@48154de65e174b93d70bc561c7a0c8b0815d367f) ([merge request](gitlab-org/security/gitlab!3777)) +- [Add verification layer for BaseSlash commands](gitlab-org/security/gitlab@f972a67468aa2da9530188930da2fb5225eb1aad) ([merge request](gitlab-org/security/gitlab!3763)) +- [Ensure workspaces are created under same root namespace as agent](gitlab-org/security/gitlab@ca7e81b8ce6c2140820c9ce21aa75af1967a2fb5) ([merge request](gitlab-org/security/gitlab!3759)) + ## 16.7.0 (2023-12-20) ### Added (199 changes) @@ -905,6 +922,20 @@ entry. - [Move export buttons next to each other](gitlab-org/gitlab@106bea7a6246cd153cf66d133936a09d46369ae3) ([merge request](gitlab-org/gitlab!137461)) +## 16.6.4 (2024-01-10) + +### Fixed (1 change) + +- [Add missing ci_sources_pipelines indexes for self-host](gitlab-org/security/gitlab@3c1411f3e5843e0d52315237b8fcc7d33c7dff2e) ([merge request](gitlab-org/security/gitlab!3810)) + +### Security (5 changes) + +- [Consider older commits when resetting codeowner approvals](gitlab-org/security/gitlab@950b435ea484bcff31bd549d11738ff54fe21ccf) ([merge request](gitlab-org/security/gitlab!3765)) +- [Password reset followups](gitlab-org/security/gitlab@db6b903bac7435dc2484bb4b97715d05a81d45c6) ([merge request](gitlab-org/security/gitlab!3778)) +- [Add verification layer for BaseSlash commands](gitlab-org/security/gitlab@0db0cb8673d406e0f464fd9f6b95e46c3b4f9695) ([merge request](gitlab-org/security/gitlab!3757)) +- [Ensure workspaces are created under same root namespace as agent](gitlab-org/security/gitlab@9d9a33b8201ccd5734ec56b149841193f4611741) ([merge request](gitlab-org/security/gitlab!3760)) +- [Sync security with canonical for 16.6 stable branch](gitlab-org/security/gitlab@e1ec9524344f469375e40145e78d3068462bed05) ([merge request](gitlab-org/security/gitlab!3782)) + ## 16.6.2 (2023-12-13) ### Fixed (1 change) @@ -1490,6 +1521,19 @@ entry. - [Remove pubsub migration helper for actioncable](gitlab-org/gitlab@763ca1305db6f1c9cf6700b8497494a81926d742) ([merge request](gitlab-org/gitlab!133066)) - [Use partitioned table for CommitStatus](gitlab-org/gitlab@063826e042778995fae13928a2fb5de2c8855b45) ([merge request](gitlab-org/gitlab!134489)) +## 16.5.6 (2024-01-11) + +### Fixed (1 change) + +- [Add missing ci_sources_pipelines indexes for self-host](gitlab-org/security/gitlab@bcc8c8f2b4c3fec3288cba0d3e69d22ebc519dfb) ([merge request](gitlab-org/security/gitlab!3811)) + +### Security (4 changes) + +- [Consider older commits when resetting codeowner approvals](gitlab-org/security/gitlab@6a7a49ba29793e1a3454f56dee47b584938c6c46) ([merge request](gitlab-org/security/gitlab!3766)) +- [Password reset followups](gitlab-org/security/gitlab@0043a2acd829accf42e8bb61dbf5aaa2a2bb3e22) ([merge request](gitlab-org/security/gitlab!3779)) +- [Add verification layer for BaseSlash commands](gitlab-org/security/gitlab@d8293ba365c85465f5fe02ac2edc6d22714a9946) ([merge request](gitlab-org/security/gitlab!3758)) +- [Ensure workspaces are created under same root namespace as agent](gitlab-org/security/gitlab@59391fdf8a15807c28e68136279e348bd29d572a) ([merge request](gitlab-org/security/gitlab!3761)) + ## 16.5.4 (2023-12-13) ### Fixed (1 change) @@ -2218,6 +2262,13 @@ entry. - [Alias read_namespace to access_namespace and move usages to new ability](gitlab-org/gitlab@61cdb4127143162a9bf9182f9c3c2d8421ee447f) by @Taucher2003 ([merge request](gitlab-org/gitlab!126625)) - [Remove `custom_roles_on_groups` feature flag](gitlab-org/gitlab@ddb4b4399b8bb82793410005c5778a002ae409b9) ([merge request](gitlab-org/gitlab!132187)) **GitLab Enterprise Edition** +## 16.4.5 (2024-01-11) + +### Security (2 changes) + +- [Fix clickouse-server version in CI](gitlab-org/security/gitlab@71878185c35a1025d00eebc6dea54688fb93b585) ([merge request](gitlab-org/security/gitlab!3808)) +- [User password reset accepts multiple email addresses](gitlab-org/security/gitlab@394e91005d4a7b3df36be077aecf8a36acdee0f3) ([merge request](gitlab-org/security/gitlab!3795)) + ## 16.4.4 (2023-12-13) ### Security (8 changes) @@ -3021,6 +3072,13 @@ entry. - [Convert design_user_mentions.note_id to bigint for self-managed](gitlab-org/gitlab@08219da99fc356fecc4e9965fe1891baca4d10ff) ([merge request](gitlab-org/gitlab!129111)) - [Migrate etag cache store from SharedState to Cache](gitlab-org/gitlab@6476298fcdcf77206fa768bcca6bd1e3c7994936) ([merge request](gitlab-org/gitlab!129050)) +## 16.3.7 (2024-01-11) + +### Security (2 changes) + +- [Fix clickouse-server version in CI](gitlab-org/security/gitlab@114a80eae87becf50ff7df80ae632409c08ea79e) ([merge request](gitlab-org/security/gitlab!3807)) +- [User password reset accepts multiple email addresses](gitlab-org/security/gitlab@d65f79dcccc4df9bea87a8a172719961b010cf4f) ([merge request](gitlab-org/security/gitlab!3796)) + ## 16.3.6 (2023-10-30) ### Security (9 changes) @@ -3890,6 +3948,13 @@ entry. - [Fix test pollution in count_deployments_metric_spec](gitlab-org/gitlab@610e6a033fe9b20aabc237b18837cddf150d4d1b) ([merge request](gitlab-org/gitlab!126808)) - [Update BulkImports::PipelineBatchWorker resource boundary](gitlab-org/gitlab@7d2477d81bcc2d035be26587802706f7098b6e44) ([merge request](gitlab-org/gitlab!126696)) +## 16.2.9 (2024-01-11) + +### Security (2 changes) + +- [Fix clickouse-server version in CI](gitlab-org/security/gitlab@2cbdec30b45c350c246fb1298d3060028433e993) ([merge request](gitlab-org/security/gitlab!3806)) +- [User password reset accepts multiple email addresses](gitlab-org/security/gitlab@7b7ac0b352c269e8b8865b6d1dc39e31fd0be9cf) ([merge request](gitlab-org/security/gitlab!3797)) + ## 16.2.8 (2023-09-28) ### Security (16 changes) @@ -4712,6 +4777,12 @@ No changes. - [Add schema_version in the commits index mapping](gitlab-org/gitlab@e75b94903b69e1e1588e251217926882875555a8) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123435)) **GitLab Enterprise Edition** - [Allow to set labels for Redis calls](gitlab-org/gitlab@8ccfff9e2d250eb22afaa7d0243e707b536a5436) ([merge request](gitlab-org/gitlab!122340)) +## 16.1.6 (2024-01-11) + +### Security (1 change) + +- [User password reset accepts multiple email addresses](gitlab-org/security/gitlab@44deb06e2c8e1c1b9424fc89dec4f60c8806711a) ([merge request](gitlab-org/security/gitlab!3798)) + ## 16.1.5 (2023-08-31) ### Fixed (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index f6d051647ed..e6960b59d9d 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -73920c7baa9ace55ba51d555e309897fd331c1b4 +7a5df4f668644c2fefd7fda1ff6050eb8b68933b diff --git a/app/controllers/projects/integrations/slash_commands_controller.rb b/app/controllers/projects/integrations/slash_commands_controller.rb new file mode 100644 index 00000000000..891a7c1a749 --- /dev/null +++ b/app/controllers/projects/integrations/slash_commands_controller.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +module Projects + module Integrations + class SlashCommandsController < Projects::ApplicationController + before_action :authenticate_user! + + feature_category :integrations + + def show + @redirect_url = integration_redirect_url + + unless valid_request? + @error = s_("Integrations|The slash command verification request has expired. Please run the command again.") + return + end + + return if valid_user? || @redirect_url.blank? + + @error = s_("Integrations|The slash command request is invalid.") + end + + def confirm + if valid_request? && valid_user? + Gitlab::SlashCommands::VerifyRequest.new(integration, chat_user, request_params[:response_url]).approve! + redirect_to request_params[:redirect_url] + else + @error = s_("Integrations|The slash command request is invalid.") + render :show + end + end + + private + + def request_params + params.permit(:integration, :team, :channel, :response_url, :command_id, :redirect_url) + end + + def cached_params + @cached_params ||= Rails.cache.fetch(cache_key) + end + + def cache_key + @cache_key ||= Kernel.format(::Integrations::BaseSlashCommands::CACHE_KEY, secret: request_params[:command_id]) + end + + def integration + integration = request_params[:integration] + + case integration + when 'slack_slash_commands' + project.slack_slash_commands_integration + when 'mattermost_slash_commands' + project.mattermost_slash_commands_integration + end + end + + def integration_redirect_url + return unless integration + + team, channel, url = request_params.values_at(:team, :channel, :response_url) + + integration.redirect_url(team, channel, url) + end + + def valid_request? + cached_params.present? + end + + def valid_user? + return false unless chat_user + + current_user == chat_user.user + end + + def chat_user + @chat_user ||= ChatNames::FindUserService.new(cached_params[:team_id], cached_params[:user_id]).execute + end + end + end +end diff --git a/app/models/chat_name.rb b/app/models/chat_name.rb index 38e6273bf20..b413da01f24 100644 --- a/app/models/chat_name.rb +++ b/app/models/chat_name.rb @@ -11,6 +11,13 @@ class ChatName < ApplicationRecord validates :chat_id, uniqueness: { scope: :team_id } + attr_encrypted :token, + mode: :per_attribute_iv, + algorithm: 'aes-256-gcm', + key: Settings.attr_encrypted_db_key_base_32, + encode: false, + encode_iv: false + # Updates the "last_used_timestamp" but only if it wasn't already updated # recently. # diff --git a/app/models/concerns/recoverable_by_any_email.rb b/app/models/concerns/recoverable_by_any_email.rb index c946e7e78c6..7bd908597c9 100644 --- a/app/models/concerns/recoverable_by_any_email.rb +++ b/app/models/concerns/recoverable_by_any_email.rb @@ -1,37 +1,34 @@ # frozen_string_literal: true -# Concern that overrides the Devise methods -# to send reset password instructions to any verified user email +# Concern that overrides the Devise methods to allow reset password instructions +# to be sent to any users' confirmed secondary emails. +# See https://github.com/heartcombo/devise/blob/main/lib/devise/models/recoverable.rb module RecoverableByAnyEmail extend ActiveSupport::Concern class_methods do def send_reset_password_instructions(attributes = {}) - email = attributes.delete(:email) - super unless email + return super unless attributes[:email] - recoverable = by_email_with_errors(email) - recoverable.send_reset_password_instructions(to: email) if recoverable&.persisted? - recoverable - end + email = Email.confirmed.find_by(email: attributes[:email].to_s) + return super unless email - private + recoverable = email.user - def by_email_with_errors(email) - record = find_by_any_email(email, confirmed: true) || new - record.errors.add(:email, :invalid) unless record.persisted? - record + recoverable.send_reset_password_instructions(to: email.email) + recoverable end end def send_reset_password_instructions(opts = {}) token = set_reset_password_token + send_reset_password_instructions_notification(token, opts) token end - private + protected def send_reset_password_instructions_notification(token, opts = {}) send_devise_notification(:reset_password_instructions, token, opts) diff --git a/app/models/integrations/base_slash_commands.rb b/app/models/integrations/base_slash_commands.rb index 58821e5fb4e..f477263303f 100644 --- a/app/models/integrations/base_slash_commands.rb +++ b/app/models/integrations/base_slash_commands.rb @@ -4,6 +4,9 @@ # This class is not meant to be used directly, but only to inherrit from. module Integrations class BaseSlashCommands < Integration + CACHE_KEY = "slash-command-requests:%{secret}" + CACHE_EXPIRATION_TIME = 3.minutes + attribute :category, default: 'chat' def valid_token?(token) @@ -26,32 +29,44 @@ module Integrations chat_user = find_chat_user(params) user = chat_user&.user - if user - unless user.can?(:use_slash_commands) - return Gitlab::SlashCommands::Presenters::Access.new.deactivated if user.deactivated? + return unknown_user_message(params) unless user + + unless user.can?(:use_slash_commands) + return Gitlab::SlashCommands::Presenters::Access.new.deactivated if user.deactivated? - return Gitlab::SlashCommands::Presenters::Access.new.access_denied(project) - end + return Gitlab::SlashCommands::Presenters::Access.new.access_denied(project) + end + if Gitlab::SlashCommands::VerifyRequest.new(self, chat_user).valid? Gitlab::SlashCommands::Command.new(project, chat_user, params).execute else - url = authorize_chat_name_url(params) - Gitlab::SlashCommands::Presenters::Access.new(url).authorize + command_id = cache_slash_commands_request!(params) + Gitlab::SlashCommands::Presenters::Access.new.confirm(confirmation_url(command_id, params)) end end private - # rubocop: disable CodeReuse/ServiceClass def find_chat_user(params) - ChatNames::FindUserService.new(params[:team_id], params[:user_id]).execute + ChatNames::FindUserService.new(params[:team_id], params[:user_id]).execute # rubocop: disable CodeReuse/ServiceClass end - # rubocop: enable CodeReuse/ServiceClass - # rubocop: disable CodeReuse/ServiceClass def authorize_chat_name_url(params) - ChatNames::AuthorizeUserService.new(params).execute + ChatNames::AuthorizeUserService.new(params).execute # rubocop: disable CodeReuse/ServiceClass + end + + def unknown_user_message(params) + url = authorize_chat_name_url(params) + Gitlab::SlashCommands::Presenters::Access.new(url).authorize + end + + def cache_slash_commands_request!(params) + secret = SecureRandom.uuid + Kernel.format(CACHE_KEY, secret: secret).tap do |cache_key| + Rails.cache.write(cache_key, params, expires_in: CACHE_EXPIRATION_TIME) + end + + secret end - # rubocop: enable CodeReuse/ServiceClass end end diff --git a/app/models/integrations/mattermost_slash_commands.rb b/app/models/integrations/mattermost_slash_commands.rb index 3a1f7329699..dcbda8d1ed0 100644 --- a/app/models/integrations/mattermost_slash_commands.rb +++ b/app/models/integrations/mattermost_slash_commands.rb @@ -4,6 +4,8 @@ module Integrations class MattermostSlashCommands < BaseSlashCommands include Ci::TriggersHelper + MATTERMOST_URL = '%{ORIGIN}/%{TEAM}/channels/%{CHANNEL}' + field :token, type: :password, description: -> { _('The Mattermost token.') }, @@ -43,6 +45,21 @@ module Integrations [[], e.message] end + def redirect_url(team, channel, url) + return if Gitlab::UrlBlocker.blocked_url?(url, schemes: %w[http https], enforce_sanitization: true) + + origin = Addressable::URI.parse(url).origin + format(MATTERMOST_URL, ORIGIN: origin, TEAM: team, CHANNEL: channel) + end + + def confirmation_url(command_id, params) + team, channel, response_url = params.values_at(:team_domain, :channel_name, :response_url) + + Rails.application.routes.url_helpers.project_integrations_slash_commands_url( + project, command_id: command_id, integration: to_param, team: team, channel: channel, response_url: response_url + ) + end + private def command(params) diff --git a/app/models/integrations/slack_slash_commands.rb b/app/models/integrations/slack_slash_commands.rb index c5ea6f22951..401ef4fb9fc 100644 --- a/app/models/integrations/slack_slash_commands.rb +++ b/app/models/integrations/slack_slash_commands.rb @@ -4,6 +4,8 @@ module Integrations class SlackSlashCommands < BaseSlashCommands include Ci::TriggersHelper + SLACK_REDIRECT_URL = 'slack://channel?team=%{TEAM}&id=%{CHANNEL}' + field :token, type: :password, non_empty_password_title: -> { s_('ProjectService|Enter new token') }, @@ -29,6 +31,18 @@ module Integrations end end + def redirect_url(team, channel, _url) + Kernel.format(SLACK_REDIRECT_URL, TEAM: team, CHANNEL: channel) + end + + def confirmation_url(command_id, params) + team, channel, response_url = params.values_at(:team_id, :channel_id, :response_url) + + Rails.application.routes.url_helpers.project_integrations_slash_commands_url( + project, command_id: command_id, integration: to_param, team: team, channel: channel, response_url: response_url + ) + end + private def format(text) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b67d6d20f41..ae68a36c8d2 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1159,6 +1159,10 @@ class MergeRequest < ApplicationRecord end end + def previous_diff + merge_request_diffs.order(id: :desc).offset(1).take + end + def version_params_for(diff_refs) if diff = merge_request_diff_for(diff_refs) { diff_id: diff.id } diff --git a/app/views/devise/passwords/new.html.haml b/app/views/devise/passwords/new.html.haml index 77a37a7c5c0..536d4c9fd4b 100644 --- a/app/views/devise/passwords/new.html.haml +++ b/app/views/devise/passwords/new.html.haml @@ -7,7 +7,7 @@ = f.label :email, _('Email') = f.email_field :email, class: "form-control gl-form-input", required: true, autocomplete: 'off', value: params[:user_email], autofocus: true, title: _('Please provide a valid email address.') .form-text.text-muted - = _('Requires a verified GitLab email address.') + = _('Requires your primary or verified secondary GitLab email address.') - if recaptcha_enabled? .gl-mb-5 diff --git a/app/views/projects/integrations/slash_commands/show.html.haml b/app/views/projects/integrations/slash_commands/show.html.haml new file mode 100644 index 00000000000..4f91f3fcbd2 --- /dev/null +++ b/app/views/projects/integrations/slash_commands/show.html.haml @@ -0,0 +1,30 @@ +- breadcrumb_title s_('Integrations|Base slash commands') +- page_title s_('Integrations|Base slash commands') +- integration_name = params[:integration].titleize +%main{ role: 'main' } + .gl-max-w-80.gl-mx-auto.gl-mt-6 + = render Pajamas::CardComponent.new do |c| + - c.with_header do + %h4.gl-m-0= sprintf(s_('Integrations|Authorize %{integration_name} (%{user}) to use your account?'), { user: current_user.username, integration_name: integration_name }) + - c.with_body do + %p + = sprintf(s_('Integrations|An application called %{integration_name} is requesting access to your GitLab account.'), { integration_name: integration_name }) + %p + = _('This application will be able to:') + %ul + %li= s_('SlackIntegration|Perform deployments.') + %li= s_('SlackIntegration|Run ChatOps jobs.') + %h4.gl-mb-0 + = @error + - c.with_footer do + .gl-display-flex + - if @error.nil? + = form_tag confirm_project_integrations_slash_commands_path(@project), method: :post do + = hidden_field_tag :command_id, params[:command_id] + = hidden_field_tag :response_url, params[:response_url] + = hidden_field_tag :integration, params[:integration] + = hidden_field_tag :redirect_url, @redirect_url + = render Pajamas::ButtonComponent.new(type: :submit, variant: :danger) do + = _('Authorize') + = render Pajamas::ButtonComponent.new(variant: :confirm, href: @redirect_url, button_options: { class: 'gl-ml-3' }) do + = s_('Integrations|Go back to your workspace') diff --git a/config/routes/project.rb b/config/routes/project.rb index 41a67d315f6..6c77bf64755 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -444,6 +444,12 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end end + namespace :integrations do + resource :slash_commands, only: [:show] do + post :confirm + end + end + get :planning_hierarchy resources :badges, only: [] do diff --git a/db/migrate/20231123160255_add_token_to_chat_names.rb b/db/migrate/20231123160255_add_token_to_chat_names.rb new file mode 100644 index 00000000000..f35a6b812f3 --- /dev/null +++ b/db/migrate/20231123160255_add_token_to_chat_names.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class AddTokenToChatNames < Gitlab::Database::Migration[2.2] + enable_lock_retries! + + milestone '16.7' + + def change + add_column :chat_names, :encrypted_token, :binary + add_column :chat_names, :encrypted_token_iv, :binary + end +end diff --git a/db/post_migrate/20240107154747_sent_notifications_self_install_finalize_bbm.rb b/db/post_migrate/20240107154747_sent_notifications_self_install_finalize_bbm.rb new file mode 100644 index 00000000000..78efdf36d64 --- /dev/null +++ b/db/post_migrate/20240107154747_sent_notifications_self_install_finalize_bbm.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class SentNotificationsSelfInstallFinalizeBbm < Gitlab::Database::Migration[2.2] + include Gitlab::Database::MigrationHelpers::ConvertToBigint + + milestone '16.8' + + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + TABLE_NAME = 'sent_notifications' + + def up + ensure_batched_background_migration_is_finished( + job_class_name: 'CopyColumnUsingBackgroundMigrationJob', + table_name: TABLE_NAME, + column_name: 'id', + job_arguments: [['id'], ['id_convert_to_bigint']] + ) + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20240107154805_sent_notifications_self_install_id_swap.rb b/db/post_migrate/20240107154805_sent_notifications_self_install_id_swap.rb new file mode 100644 index 00000000000..41e330ba7a0 --- /dev/null +++ b/db/post_migrate/20240107154805_sent_notifications_self_install_id_swap.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class SentNotificationsSelfInstallIdSwap < Gitlab::Database::Migration[2.2] + include Gitlab::Database::MigrationHelpers::ConvertToBigint + + milestone '16.8' + + disable_ddl_transaction! + + TABLE_NAME = :sent_notifications + COLUMN_NAME = :id_convert_to_bigint + INDEX_NAME = :index_sent_notifications_on_id_convert_to_bigint + + def up + return if com_or_dev_or_test_but_not_jh? + return if columns_swapped?(TABLE_NAME, :id) + return if temp_column_removed?(TABLE_NAME, :id) + + swap + end + + def down + return if com_or_dev_or_test_but_not_jh? + return unless columns_swapped?(TABLE_NAME, :id) + return if temp_column_removed?(TABLE_NAME, :id) + + swap + end + + def swap + add_concurrent_index TABLE_NAME, COLUMN_NAME, unique: true, name: INDEX_NAME + + with_lock_retries(raise_on_exhaustion: true) do + execute "LOCK TABLE #{TABLE_NAME} IN ACCESS EXCLUSIVE MODE" + + # Swap Columns + temp_name = quote_column_name(:id_tmp) + id_name = quote_column_name(:id) + id_convert_to_bigint_name = quote_column_name(COLUMN_NAME) + execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN #{id_name} TO #{temp_name}" + execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN #{id_convert_to_bigint_name} TO #{id_name}" + execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN #{temp_name} TO #{id_convert_to_bigint_name}" + + # Reset trigger + function_name = Gitlab::Database::UnidirectionalCopyTrigger.on_table(TABLE_NAME, connection: connection) + .name(:id, :id_convert_to_bigint) + execute "ALTER FUNCTION #{quote_table_name(function_name)} RESET ALL" + + execute "ALTER SEQUENCE #{TABLE_NAME}_id_seq OWNED BY #{TABLE_NAME}.id" + change_column_default TABLE_NAME, :id, -> { "nextval('#{TABLE_NAME}_id_seq'::regclass)" } + change_column_default TABLE_NAME, :id_convert_to_bigint, 0 + + execute "ALTER TABLE #{TABLE_NAME} DROP CONSTRAINT #{TABLE_NAME}_pkey CASCADE" + rename_index TABLE_NAME, INDEX_NAME, "#{TABLE_NAME}_pkey" + execute "ALTER TABLE #{TABLE_NAME} ADD CONSTRAINT #{TABLE_NAME}_pkey PRIMARY KEY USING INDEX #{TABLE_NAME}_pkey" + end + end +end diff --git a/db/schema_migrations/20231123160255 b/db/schema_migrations/20231123160255 new file mode 100644 index 00000000000..f158c3d99f1 --- /dev/null +++ b/db/schema_migrations/20231123160255 @@ -0,0 +1 @@ +05dab8240da25338ef7b7f06414f53eac6c584b39158bb80e22f635c216b8276
\ No newline at end of file diff --git a/db/schema_migrations/20240107154747 b/db/schema_migrations/20240107154747 new file mode 100644 index 00000000000..303b6bd9e73 --- /dev/null +++ b/db/schema_migrations/20240107154747 @@ -0,0 +1 @@ +066ff822fdc28dc62946b40c0b954d351c9bc55e83d5993bd2b35c4cecfe61f6
\ No newline at end of file diff --git a/db/schema_migrations/20240107154805 b/db/schema_migrations/20240107154805 new file mode 100644 index 00000000000..11470faaa54 --- /dev/null +++ b/db/schema_migrations/20240107154805 @@ -0,0 +1 @@ +bae4fbcf7cc5217663a630deed0adc509f3386921a84fffa06adef603a9bf378
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 913c244b820..21a040bbd85 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14020,7 +14020,9 @@ CREATE TABLE chat_names ( chat_name character varying, last_used_at timestamp without time zone, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + encrypted_token bytea, + encrypted_token_iv bytea ); CREATE SEQUENCE chat_names_id_seq diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 767ceb154a7..22cb7531a87 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -16855,6 +16855,7 @@ The currently authenticated GitLab user. | <a id="currentusercreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp of when the user was created. | | <a id="currentuserdiscord"></a>`discord` | [`String`](#string) | Discord ID of the user. | | <a id="currentuserduochatavailable"></a>`duoChatAvailable` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in 16.8. This feature is an Experiment. It can be changed or removed at any time. User access to AI chat feature. | +| <a id="currentuserduocodesuggestionsavailable"></a>`duoCodeSuggestionsAvailable` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in 16.8. This feature is an Experiment. It can be changed or removed at any time. User access to code suggestions feature. | | <a id="currentuseremail"></a>`email` **{warning-solid}** | [`String`](#string) | **Deprecated** in 13.7. This was renamed. Use: [`User.publicEmail`](#userpublicemail). | | <a id="currentuseremails"></a>`emails` | [`EmailConnection`](#emailconnection) | User's email addresses. (see [Connections](#connections)) | | <a id="currentusergitpodenabled"></a>`gitpodEnabled` | [`Boolean`](#boolean) | Whether Gitpod is enabled at the user level. | diff --git a/doc/api/namespaces.md b/doc/api/namespaces.md index 4e285cde975..6d5e15934e9 100644 --- a/doc/api/namespaces.md +++ b/doc/api/namespaces.md @@ -18,6 +18,8 @@ You might also want to view documentation for: ## List namespaces +> `top_level_only` [introduced](https://gitlab.com/gitlab-org/customers-gitlab-com/-/issues/7600) in GitLab 16.8. + Get a list of the namespaces of the authenticated user. If the user is an administrator, a list of all namespaces in the GitLab instance is shown. @@ -25,12 +27,14 @@ administrator, a list of all namespaces in the GitLab instance is shown. GET /namespaces GET /namespaces?search=foobar GET /namespaces?owned_only=true +GET /namespaces?top_level_only=true ``` -| Attribute | Type | Required | Description | -| ------------ | ------- | -------- | ----------- | -| `search` | string | no | Returns a list of namespaces the user is authorized to view based on the search criteria | -| `owned_only` | boolean | no | In GitLab 14.2 and later, returns a list of owned namespaces only | +| Attribute | Type | Required | Description | +| ---------------- | ------- | -------- | ----------- | +| `search` | string | no | Returns a list of namespaces the user is authorized to view based on the search criteria | +| `owned_only` | boolean | no | In GitLab 14.2 and later, returns a list of owned namespaces only | +| `top_level_only` | boolean | no | In GitLab 16.8 and later, returns a list of top level namespaces only | Example request: diff --git a/doc/development/ai_features/index.md b/doc/development/ai_features/index.md index 87c82f85a5a..da652cc2c44 100644 --- a/doc/development/ai_features/index.md +++ b/doc/development/ai_features/index.md @@ -187,7 +187,7 @@ Therefore, a different setup is required from the [SaaS-only AI features](#test- ``` 1. Run `poetry run ai_gateway`. - 1. Visit [OpenAPI playground](`http://0.0.0.0:5052/docs`), try an endpoint (e.g. `/v1/chat/agent`) and make sure you get a successful response. + 1. Visit [OpenAPI playground](http://0.0.0.0:5052/docs), try an endpoint (e.g. `/v1/chat/agent`) and make sure you get a successful response. If something went wrong, check `modelgateway_debug.log` if it contains error information. 1. Setup GitLab Development Kit (GDK): 1. [Install it](https://gitlab.com/gitlab-org/gitlab-development-kit#installation). diff --git a/doc/user/application_security/container_scanning/index.md b/doc/user/application_security/container_scanning/index.md index 9cb27be9d08..5be9e169078 100644 --- a/doc/user/application_security/container_scanning/index.md +++ b/doc/user/application_security/container_scanning/index.md @@ -379,6 +379,9 @@ The following options are available: | [Grype](https://github.com/anchore/grype) | `registry.gitlab.com/security-products/container-scanning/grype:6` | | Trivy | `registry.gitlab.com/security-products/container-scanning/trivy:6` | +WARNING: +Do not use the `:latest` tag when selecting the scanner image. + ### Setting the default branch image > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/338877) in GitLab 14.5. diff --git a/doc/user/compliance/license_list.md b/doc/user/compliance/license_list.md index 43d76845ffa..3bfc5612db9 100644 --- a/doc/user/compliance/license_list.md +++ b/doc/user/compliance/license_list.md @@ -4,9 +4,13 @@ group: Threat Insights info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# License list **(ULTIMATE ALL)** +<!--- start_remove The following content will be removed on remove_date: '2024-08-15' --> -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13582) in GitLab 12.7. +# License list (deprecated) **(ULTIMATE ALL)** + +WARNING: +This feature was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/436100) in GitLab 16.8 +and is planned for removal in 17.0. Use the [Dependency List](../application_security/dependency_list/index.md) instead. The License list allows you to see your project's licenses and key details about them. @@ -32,3 +36,5 @@ The licenses are displayed, where: - **Policy Violation:** The license has a [license policy](license_approval_policies.md) marked as **Deny**. ![License List](img/license_list_v13_0.png) + +<!--- end_remove --> diff --git a/go.work b/go.work new file mode 100644 index 00000000000..61be32bc68d --- /dev/null +++ b/go.work @@ -0,0 +1,3 @@ +go 1.20 + +use ./workhorse diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb index 750dc7fc2a1..17425c288fc 100644 --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -34,6 +34,7 @@ module API params do optional :search, type: String, desc: 'Returns a list of namespaces the user is authorized to view based on the search criteria' optional :owned_only, type: Boolean, desc: 'In GitLab 14.2 and later, returns a list of owned namespaces only' + optional :top_level_only, type: Boolean, default: false, desc: 'Only include top level namespaces' use :pagination use :optional_list_params_ee @@ -43,6 +44,8 @@ module API namespaces = current_user.admin ? Namespace.all : current_user.namespaces(owned_only: owned_only) + namespaces = namespaces.top_most if params[:top_level_only] + namespaces = namespaces.without_project_namespaces.include_route namespaces = namespaces.include_gitlab_subscription_with_hosted_plan if Gitlab.ee? diff --git a/lib/gitlab/slash_commands/presenters/access.rb b/lib/gitlab/slash_commands/presenters/access.rb index e098762f290..56a960c9bbf 100644 --- a/lib/gitlab/slash_commands/presenters/access.rb +++ b/lib/gitlab/slash_commands/presenters/access.rb @@ -42,6 +42,17 @@ module Gitlab ephemeral_response(text: message) end + + def confirm(url) + text = [ + _("To ensure the highest security standards, we verify the source of all slash commands."), + Kernel.format(_("Please confirm the request by accessing %{url} through a web browser."), + url: "<#{url}|this link>"), + _("Upon successful validation, you're granted access to slash commands.") + ].join("\n\n") + + ephemeral_response(text: text) + end end end end diff --git a/lib/gitlab/slash_commands/verify_request.rb b/lib/gitlab/slash_commands/verify_request.rb new file mode 100644 index 00000000000..41f71064573 --- /dev/null +++ b/lib/gitlab/slash_commands/verify_request.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Gitlab + module SlashCommands + class VerifyRequest + attr_accessor :integration, :chat_name, :response_url + + def initialize(integration, chat_name, response_url = nil) + @integration = integration + @chat_name = chat_name + @response_url = response_url + end + + def approve! + update_token! + update_source_message + end + + def valid? + return false if integration.token.nil? || chat_name.token.nil? + + ActiveSupport::SecurityUtils.secure_compare(integration.token, chat_name.token) + end + + private + + def update_token! + chat_name.update!(token: integration.token) + end + + def update_source_message + request_body = Gitlab::Json.dump(verified_request_body) + + Gitlab::HTTP.post(response_url, body: request_body, headers: headers) + end + + def verified_request_body + { + 'replace_original' => 'true', + 'text' => _("You've successfully verified! You now have access to slash commands. " \ + "Thanks for helping ensure security!") + } + end + + def headers + { 'Content-Type' => 'application/json' } + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 473a6abb519..788ab63458a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25988,6 +25988,9 @@ msgstr "" msgid "Integrations|All projects inheriting these settings will also be reset." msgstr "" +msgid "Integrations|An application called %{integration_name} is requesting access to your GitLab account." +msgstr "" + msgid "Integrations|An application called %{integration_name} is requesting access to your GitLab account. This application was created by GitLab Inc." msgstr "" @@ -26000,6 +26003,9 @@ msgstr "" msgid "Integrations|Authorize %{integration_name} (%{user}) to use your account?" msgstr "" +msgid "Integrations|Base slash commands" +msgstr "" + msgid "Integrations|Branches for which notifications are to be sent" msgstr "" @@ -26072,6 +26078,9 @@ msgstr "" msgid "Integrations|GitLab for Slack app must be reinstalled to enable notifications" msgstr "" +msgid "Integrations|Go back to your workspace" +msgstr "" + msgid "Integrations|Group-level integration management" msgstr "" @@ -26144,6 +26153,12 @@ msgstr "" msgid "Integrations|Standard" msgstr "" +msgid "Integrations|The slash command request is invalid." +msgstr "" + +msgid "Integrations|The slash command verification request has expired. Please run the command again." +msgstr "" + msgid "Integrations|There are no projects using custom settings" msgstr "" @@ -36426,6 +36441,9 @@ msgstr "" msgid "Please complete your profile with email address" msgstr "" +msgid "Please confirm the request by accessing %{url} through a web browser." +msgstr "" + msgid "Please confirm your email address" msgstr "" @@ -41359,15 +41377,15 @@ msgid_plural "Requires %{count} approvals from %{names}." msgstr[0] "" msgstr[1] "" -msgid "Requires a verified GitLab email address." -msgstr "" - msgid "Requires you to deploy or set up cloud-hosted Sentry." msgstr "" msgid "Requires your primary GitLab email address. If you want to confirm a secondary email address, go to %{emails_link_start}Emails%{emails_link_end}" msgstr "" +msgid "Requires your primary or verified secondary GitLab email address." +msgstr "" + msgid "Resend" msgstr "" @@ -51108,6 +51126,9 @@ msgstr "" msgid "To ensure no loss of access to personal content, only use this account for matters related to %{group_name}." msgstr "" +msgid "To ensure the highest security standards, we verify the source of all slash commands." +msgstr "" + msgid "To find the state of this project's repository at the time of any of these versions, check out %{link_start}the tags%{link_end}" msgstr "" @@ -52639,6 +52660,9 @@ msgstr "" msgid "Uploading: %{progress}" msgstr "" +msgid "Upon successful validation, you're granted access to slash commands." +msgstr "" + msgid "Upstream" msgstr "" @@ -56842,6 +56866,9 @@ msgid_plural "You've successfully purchased the %{plan} plan subscription for %{ msgstr[0] "" msgstr[1] "" +msgid "You've successfully verified! You now have access to slash commands. Thanks for helping ensure security!" +msgstr "" + msgid "YouTube" msgstr "" diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index aad946acad4..cff84da7382 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PasswordsController do +RSpec.describe PasswordsController, feature_category: :system_access do include DeviseHelpers before do @@ -109,8 +109,9 @@ RSpec.describe PasswordsController do describe '#create' do let(:user) { create(:user) } + let(:email) { user.email } - subject(:perform_request) { post(:create, params: { user: { email: user.email } }) } + subject(:perform_request) { post(:create, params: { user: { email: email } }) } context 'when reCAPTCHA is disabled' do before do @@ -161,5 +162,131 @@ RSpec.describe PasswordsController do expect(flash[:notice]).to include 'If your email address exists in our database, you will receive a password recovery link at your email address in a few minutes.' end end + + context "sending 'Reset password instructions' email" do + include EmailHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:user_confirmed_primary_email) { user.email } + let_it_be(:user_confirmed_secondary_email) { create(:email, :confirmed, user: user, email: 'confirmed-secondary-email@example.com').email } + let_it_be(:user_unconfirmed_secondary_email) { create(:email, user: user, email: 'unconfirmed-secondary-email@example.com').email } + let_it_be(:unknown_email) { 'attacker@example.com' } + let_it_be(:invalid_email) { 'invalid_email' } + let_it_be(:sql_injection_email) { 'sql-injection-email@example.com OR 1=1' } + let_it_be(:another_user_confirmed_primary_email) { create(:user).email } + let_it_be(:another_user_unconfirmed_primary_email) { create(:user, :unconfirmed).email } + + before do + reset_delivered_emails! + + perform_request + + perform_enqueued_jobs + end + + context "when email param matches user's confirmed primary email" do + let(:email) { user_confirmed_primary_email } + + it 'sends email to the primary email only' do + expect_only_one_email_to_be_sent(subject: 'Reset password instructions', to: [user_confirmed_primary_email]) + end + end + + context "when email param matches user's unconfirmed primary email" do + let(:email) { another_user_unconfirmed_primary_email } + + # By default 'devise' gem allows password reset by unconfirmed primary email. + # When user account with unconfirmed primary email that means it is unconfirmed. + # + # Password reset by unconfirmed primary email is very helpful from + # security perspective. Example: + # Malicious person creates user account on GitLab with someone's email. + # If the email owner confirms the email for newly created account, the malicious person will be able + # to sign in into the account by password they provided during account signup. + # The malicious person could set up 2FA to the user account, after that + # te email owner would not able to get access to that user account even + # after performing password reset. + # To deal with that case safely the email owner should reset password + # for the user account first. That will make sure that after the user account + # is confirmed the malicious person is not be able to sign in with + # the password they provided during the account signup. Then email owner + # could sign into the account, they will see a prompt to confirm the account email + # to proceed. They can safely confirm the email and take over the account. + # That is one of the reasons why password reset by unconfirmed primary email should be allowed. + it 'sends email to the primary email only' do + expect_only_one_email_to_be_sent(subject: 'Reset password instructions', to: [another_user_unconfirmed_primary_email]) + end + end + + context "when email param matches user's confirmed secondary email" do + let(:email) { user_confirmed_secondary_email } + + it 'sends email to the confirmed secondary email only' do + expect_only_one_email_to_be_sent(subject: 'Reset password instructions', to: [user_confirmed_secondary_email]) + end + end + + # While unconfirmed primary emails are linked with users accounts, + # unconfirmed secondary emails should not be linked with any users till they are confirmed + # See https://gitlab.com/gitlab-org/gitlab/-/issues/356665 + # + # In https://gitlab.com/gitlab-org/gitlab/-/issues/367823, it is considerd + # to prevent reserving emails on Gitlab by unconfirmed secondary emails. + # As per this issue, there might be cases that there are multiple users + # with the same unconfirmed secondary emails. It would be impossible to identify for + # what user account password reset is requested if password reset were allowed + # by unconfirmed secondary emails. + # Also note that it is not possible to request email confirmation for + # unconfirmed secondary emails without having access to the user account. + context "when email param matches user's unconfirmed secondary email" do + let(:email) { user_unconfirmed_secondary_email } + + it 'does not send email to anyone' do + should_not_email_anyone + end + end + + context 'when email param is unknown email' do + let(:email) { unknown_email } + + it 'does not send email to anyone' do + should_not_email_anyone + end + end + + context 'when email param is invalid email' do + let(:email) { invalid_email } + + it 'does not send email to anyone' do + should_not_email_anyone + end + end + + context 'when email param with attempt to cause SQL injection' do + let(:email) { sql_injection_email } + + it 'does not send email to anyone' do + should_not_email_anyone + end + end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/436084 + context 'when email param with multiple emails' do + let(:email) do + [ + user_confirmed_primary_email, + user_confirmed_secondary_email, + user_unconfirmed_secondary_email, + unknown_email, + another_user_confirmed_primary_email, + another_user_unconfirmed_primary_email + ] + end + + it 'does not send email to anyone' do + should_not_email_anyone + end + end + end end end diff --git a/spec/factories/chat_names.rb b/spec/factories/chat_names.rb index c872694ee64..ee29f0fa0e4 100644 --- a/spec/factories/chat_names.rb +++ b/spec/factories/chat_names.rb @@ -7,6 +7,8 @@ FactoryBot.define do team_id { 'T0001' } team_domain { 'Awesome Team' } + token { SecureRandom.uuid } + sequence(:chat_id) { |n| "U#{n}" } chat_name { generate(:username) } end diff --git a/spec/lib/gitlab/slash_commands/presenters/access_spec.rb b/spec/lib/gitlab/slash_commands/presenters/access_spec.rb index 3af0ae03256..d83871d67a2 100644 --- a/spec/lib/gitlab/slash_commands/presenters/access_spec.rb +++ b/spec/lib/gitlab/slash_commands/presenters/access_spec.rb @@ -76,4 +76,19 @@ RSpec.describe Gitlab::SlashCommands::Presenters::Access do end end end + + describe '#confirm' do + let(:url) { 'https://example.com/api' } + + subject { described_class.new.confirm(url) } + + it { is_expected.to be_a(Hash) } + + it 'tells the user to confirm the request' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to match( + "Please confirm the request by accessing <#{url}|this link> through a web browser" + ) + end + end end diff --git a/spec/lib/gitlab/slash_commands/verify_request_spec.rb b/spec/lib/gitlab/slash_commands/verify_request_spec.rb new file mode 100644 index 00000000000..df6052ded93 --- /dev/null +++ b/spec/lib/gitlab/slash_commands/verify_request_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::SlashCommands::VerifyRequest, feature_category: :integrations do + let_it_be(:integration) { create(:slack_slash_commands_integration) } + let_it_be(:chat_name) { create(:chat_name) } + let(:response_url) { 'http://www.example.com/' } + + subject(:verification) { described_class.new(integration, chat_name, response_url) } + + describe '#approve!' do + before do + stub_request(:post, "http://www.example.com/").to_return(status: 200, body: 'ok') + end + + it 'updates the token' do + expect { verification.approve! }.to change { chat_name.reload.token }.to(integration.token) + end + + it 'updates the ephemeral message' do + expect(Gitlab::HTTP).to receive(:post).with( + response_url, a_hash_including(body: an_instance_of(String), headers: an_instance_of(Hash)) + ).once + + verification.approve! + end + end + + describe '#valid?' do + it 'compares tokens' do + expect(ActiveSupport::SecurityUtils).to receive(:secure_compare).with(integration.token, chat_name.token) + verification.valid? + end + end +end diff --git a/spec/mailers/devise_mailer_spec.rb b/spec/mailers/devise_mailer_spec.rb index 0a6d38996b7..af3a9457527 100644 --- a/spec/mailers/devise_mailer_spec.rb +++ b/spec/mailers/devise_mailer_spec.rb @@ -103,10 +103,10 @@ RSpec.describe DeviseMailer, feature_category: :user_management do describe '#reset_password_instructions' do let_it_be(:user) { create(:user) } - let(:params) { {} } + let(:opts) { {} } subject do - described_class.reset_password_instructions(user, 'faketoken', params) + described_class.reset_password_instructions(user, 'faketoken', opts) end it_behaves_like 'an email sent from GitLab' @@ -139,9 +139,9 @@ RSpec.describe DeviseMailer, feature_category: :user_management do is_expected.to have_header 'X-Mailgun-Suppressions-Bypass', 'true' end - context 'with email in params' do + context 'with email in opts' do let(:email) { 'example@example.com' } - let(:params) { { to: email } } + let(:opts) { { to: email } } it 'is sent to the specified email' do is_expected.to deliver_to email diff --git a/spec/migrations/sent_notifications_self_install_id_swap_spec.rb b/spec/migrations/sent_notifications_self_install_id_swap_spec.rb new file mode 100644 index 00000000000..db66b72d2ec --- /dev/null +++ b/spec/migrations/sent_notifications_self_install_id_swap_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SentNotificationsSelfInstallIdSwap, feature_category: :database do + let(:connection) { described_class.new.connection } + + describe '#up' do + before do + # rubocop: disable RSpec/AnyInstanceOf -- This mixin is only used for migrations, it's okay to use this + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(dot_com?) + # rubocop: enable RSpec/AnyInstanceOf + end + + context 'when we are NOT GitLab.com, dev, or test' do + let(:dot_com?) { false } + + context 'when sent_notifications.id is not a bigint' do + around do |example| + connection.execute('ALTER TABLE sent_notifications ALTER COLUMN id TYPE integer') + example.run + connection.execute('ALTER TABLE sent_notifications ALTER COLUMN id TYPE bigint') + end + + context 'when id_convert_to_bigint exists' do + around do |example| + connection.execute('ALTER TABLE sent_notifications ADD COLUMN IF NOT EXISTS id_convert_to_bigint bigint') + Gitlab::Database::UnidirectionalCopyTrigger.on_table(:sent_notifications, connection: connection).create( + :id, :id_convert_to_bigint) + example.run + connection.execute('ALTER TABLE sent_notifications DROP COLUMN id_convert_to_bigint') + end + + it 'swaps the integer and bigint columns' do + sent_notifications = table(:sent_notifications) + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + sent_notifications.reset_column_information + expect(sent_notifications.columns.find { |c| c.name == 'id' }.sql_type).to eq('integer') + expect(sent_notifications.columns.find do |c| + c.name == 'id_convert_to_bigint' + end.sql_type).to eq('bigint') + } + + migration.after -> { + sent_notifications.reset_column_information + expect(sent_notifications.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint') + expect(sent_notifications.columns.find do |c| + c.name == 'id_convert_to_bigint' + end.sql_type).to eq('integer') + } + end + end + end + end + end + end + + context 'when any other condition' do + let(:dot_com?) { true } + + it 'does not do anything' do + sent_notifications = table(:sent_notifications) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + expect(sent_notifications.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint') + } + + migration.after -> { + expect(sent_notifications.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint') + } + end + end + end + end + end +end diff --git a/spec/models/concerns/recoverable_by_any_email_spec.rb b/spec/models/concerns/recoverable_by_any_email_spec.rb index 1e701f145be..ba0bb99effb 100644 --- a/spec/models/concerns/recoverable_by_any_email_spec.rb +++ b/spec/models/concerns/recoverable_by_any_email_spec.rb @@ -4,79 +4,140 @@ require 'spec_helper' RSpec.describe RecoverableByAnyEmail, feature_category: :system_access do describe '.send_reset_password_instructions' do - let_it_be(:user) { create(:user, email: 'test@example.com') } - let_it_be(:verified_email) { create(:email, :confirmed, user: user) } - let_it_be(:unverified_email) { create(:email, user: user) } + include EmailHelpers subject(:send_reset_password_instructions) do User.send_reset_password_instructions(email: email) end - shared_examples 'sends the password reset email' do + let_it_be(:user) { create(:user) } + let_it_be(:user_confirmed_primary_email) { user.email } + + let_it_be(:user_confirmed_secondary_email) do + create(:email, :confirmed, user: user, email: 'confirmed-secondary-email@example.com').email + end + + let_it_be(:user_unconfirmed_secondary_email) do + create(:email, user: user, email: 'unconfirmed-secondary-email@example.com').email + end + + let_it_be(:unknown_email) { 'attacker@example.com' } + let_it_be(:invalid_email) { 'invalid_email' } + let_it_be(:sql_injection_email) { 'sql-injection-email@example.com OR 1=1' } + + let_it_be(:another_user_confirmed_primary_email) { create(:user).email } + + let_it_be(:another_user) { create(:user, :unconfirmed) } + let_it_be(:another_user_unconfirmed_primary_email) { another_user.email } + + shared_examples "sends 'Reset password instructions' email" do it 'finds the user' do - expect(send_reset_password_instructions).to eq(user) + expect(send_reset_password_instructions).to eq(expected_user) end it 'sends the email' do + reset_delivered_emails! + expect { send_reset_password_instructions }.to have_enqueued_mail(DeviseMailer, :reset_password_instructions) + + perform_enqueued_jobs + + expect_only_one_email_to_be_sent(subject: 'Reset password instructions', to: [email]) end end - shared_examples 'does not send the password reset email' do + shared_examples "does not send 'Reset password instructions' email" do + # If user is not found, returns a new user with errors. + # See https://github.com/heartcombo/devise/blob/main/lib/devise/models/recoverable.rb it 'does not find the user' do - expect(subject.id).to be_nil - expect(subject.errors).not_to be_empty + expect(send_reset_password_instructions).to be_instance_of User + expect(send_reset_password_instructions).to be_new_record + expect(send_reset_password_instructions.errors).not_to be_empty end - it 'does not send any email' do - subject + it 'does not send email to anyone' do + reset_delivered_emails! + + expect { send_reset_password_instructions } + .not_to have_enqueued_mail(DeviseMailer, :reset_password_instructions) + + perform_enqueued_jobs - expect { subject }.not_to have_enqueued_mail(DeviseMailer, :reset_password_instructions) + should_not_email_anyone end end - context 'with user primary email' do - let(:email) { user.email } + context "when email param matches user's confirmed primary email" do + let(:expected_user) { user } + let(:email) { user_confirmed_primary_email } - it_behaves_like 'sends the password reset email' + it_behaves_like "sends 'Reset password instructions' email" end - context 'with user verified email' do - let(:email) { verified_email.email } + context "when email param matches user's unconfirmed primary email" do + let(:expected_user) { another_user } + let(:email) { another_user_unconfirmed_primary_email } - it_behaves_like 'sends the password reset email' + it_behaves_like "sends 'Reset password instructions' email" end - context 'with user unverified email' do - let(:email) { unverified_email.email } + context "when email param matches user's confirmed secondary email" do + let(:expected_user) { user } + let(:email) { user_confirmed_secondary_email } - it_behaves_like 'does not send the password reset email' + it_behaves_like "sends 'Reset password instructions' email" end - end - describe '#send_reset_password_instructions' do - let_it_be(:user) { create(:user) } - let_it_be(:opts) { { email: 'random@email.com' } } - let_it_be(:token) { 'passwordresettoken' } + context "when email param matches user's unconfirmed secondary email" do + let(:email) { user_unconfirmed_secondary_email } - before do - allow(user).to receive(:set_reset_password_token).and_return(token) + it_behaves_like "does not send 'Reset password instructions' email" end - subject { user.send_reset_password_instructions(opts) } + context 'when email param is unknown email' do + let(:email) { unknown_email } - it 'sends the email' do - expect { subject }.to have_enqueued_mail(DeviseMailer, :reset_password_instructions) + it_behaves_like "does not send 'Reset password instructions' email" end - it 'calls send_reset_password_instructions_notification with correct arguments' do - expect(user).to receive(:send_reset_password_instructions_notification).with(token, opts) + context 'when email param is invalid email' do + let(:email) { invalid_email } - subject + it_behaves_like "does not send 'Reset password instructions' email" end - it 'returns the generated token' do - expect(subject).to eq(token) + context 'when email param with attempt to cause SQL injection' do + let(:email) { sql_injection_email } + + it_behaves_like "does not send 'Reset password instructions' email" + end + + context 'when email param is nil' do + let(:email) { nil } + + it_behaves_like "does not send 'Reset password instructions' email" + end + + context 'when email param is empty string' do + let(:email) { '' } + + it_behaves_like "does not send 'Reset password instructions' email" + end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/436084 + context 'when email param with multiple emails' do + let(:email) do + [ + user_confirmed_primary_email, + user_confirmed_secondary_email, + user_unconfirmed_secondary_email, + unknown_email, + another_user_confirmed_primary_email, + another_user_unconfirmed_primary_email + ] + end + + it_behaves_like "does not send 'Reset password instructions' email" end end end diff --git a/spec/models/integrations/mattermost_slash_commands_spec.rb b/spec/models/integrations/mattermost_slash_commands_spec.rb index 3dee8737067..43316e164ed 100644 --- a/spec/models/integrations/mattermost_slash_commands_spec.rb +++ b/spec/models/integrations/mattermost_slash_commands_spec.rb @@ -125,5 +125,39 @@ RSpec.describe Integrations::MattermostSlashCommands, feature_category: :integra end end end + + describe '#redirect_url' do + let(:url) { 'http://www.mattermost.com/hooks' } + + subject { integration.redirect_url('team', 'channel', url) } + + it { is_expected.to eq("http://www.mattermost.com/team/channels/channel") } + + context 'with invalid URL scheme' do + let(:url) { 'javascript://www.mattermost.com/hooks' } + + it { is_expected.to be_nil } + end + + context 'with unsafe URL' do + let(:url) { "https://replaceme.com/'><script>alert(document.cookie)</script>" } + + it { is_expected.to be_nil } + end + end + + describe '#confirmation_url' do + let(:params) do + { + team_domain: 'gitlab', + channel_name: 'test-channel', + response_url: 'http://mattermost.gitlab.com/hooks/commands/my123command' + } + end + + subject { integration.confirmation_url('command-id', params) } + + it { is_expected.to be_present } + end end end diff --git a/spec/models/integrations/slack_slash_commands_spec.rb b/spec/models/integrations/slack_slash_commands_spec.rb index f373fc2a2de..5d6f214a5d5 100644 --- a/spec/models/integrations/slack_slash_commands_spec.rb +++ b/spec/models/integrations/slack_slash_commands_spec.rb @@ -40,4 +40,27 @@ RSpec.describe Integrations::SlackSlashCommands, feature_category: :integrations end end end + + describe '#redirect_url' do + let(:integration) { build(:slack_slash_commands_integration) } + + subject { integration.redirect_url('team', 'channel', 'www.example.com') } + + it { is_expected.to eq('slack://channel?team=team&id=channel') } + end + + describe '#confirmation_url' do + let(:integration) { build(:slack_slash_commands_integration) } + let(:params) do + { + team_id: 'T123456', + channel_id: 'C654321', + response_url: 'https://hooks.slack.com/services/T123456/C654321' + } + end + + subject { integration.confirmation_url('command-id', params) } + + it { is_expected.to be_present } + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c30bbb3a134..797ab5be235 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -6181,4 +6181,34 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it { is_expected.to eq(false) } end + + describe '#previous_diff' do + let(:merge_request) { create(:merge_request, :skip_diff_creation) } + + subject { merge_request.previous_diff } + + context 'when there is are no merge_request_diffs' do + it { is_expected.to be_nil } + end + + context 'when there is one merge request_diff' do + let(:merge_request) { create(:merge_request) } + + it { is_expected.to be_nil } + end + + context 'when there are multiple merge_request_diffs' do + let(:oldest_merge_request_diff) { create(:merge_request_diff, merge_request: merge_request) } + let(:second_to_last_merge_request_diff) { create(:merge_request_diff, merge_request: merge_request) } + let(:most_recent_merge_request_diff) { create(:merge_request_diff, merge_request: merge_request) } + + before do + oldest_merge_request_diff + second_to_last_merge_request_diff + most_recent_merge_request_diff + end + + it { is_expected.to eq(second_to_last_merge_request_diff) } + end + end end diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb index 5fd41013b25..2320b3be0c1 100644 --- a/spec/requests/api/namespaces_spec.rb +++ b/spec/requests/api/namespaces_spec.rb @@ -109,6 +109,19 @@ RSpec.describe API::Namespaces, :aggregate_failures, feature_category: :groups_a expect(json_response.map { |resource| resource['id'] }).to match_array([user.namespace_id, group2.id]) end end + + context 'with top_level_only param' do + it 'returns only top level groups' do + group1.add_owner(user) + group2.add_owner(user) + + get api("/namespaces?top_level_only=true", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.map { |resource| resource['id'] }).to match_array([user.namespace_id, group1.id]) + end + end end end diff --git a/spec/requests/projects/integrations/slash_commands_controller_spec.rb b/spec/requests/projects/integrations/slash_commands_controller_spec.rb new file mode 100644 index 00000000000..3d61f882bdf --- /dev/null +++ b/spec/requests/projects/integrations/slash_commands_controller_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Projects::Integrations::SlashCommandsController, feature_category: :integrations do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user, developer_projects: [project]) } + let_it_be(:chat_name) { create(:chat_name, user: user) } + + let(:params) do + { + command_id: 'command-id', + integration: 'mattermost_slash_commands', + team: 1, + channel: 2, + response_url: 'http://www.example.com' + } + end + + before do + create(:mattermost_slash_commands_integration, project: project) + end + + describe 'GET #show' do + context 'when user is signed in' do + before do + sign_in(user) + end + + context 'when request is invalid' do + it 'renders the "show" template with expired message' do + get project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + expect(response.body).to include( + 'The slash command verification request has expired. Please run the command again.' + ) + end + end + + context 'when request is valid', :use_clean_rails_memory_store_caching do + before do + Rails.cache.write( + "slash-command-requests:#{params[:command_id]}", { team_id: chat_name.team_id, user_id: chat_name.chat_id } + ) + stub_request(:post, "http://www.example.com/").to_return(status: 200, body: 'ok') + end + + context 'when user is valid' do + it 'renders the "show" template with authorize button' do + get project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + expect(response.body).to include('Authorize') + end + end + + context 'when user is invalid' do + let(:chat_name) { create(:chat_name) } + + it 'renders the "show" template' do + get project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + expect(response.body).to include('The slash command request is invalid.') + end + end + end + end + + context 'when user is not signed in' do + it 'redirects with a status of 302' do + get project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:redirect) + end + end + end + + describe 'POST #confirm' do + let(:params) { super().merge(redirect_url: 'http://www.example.com') } + + context 'when user is signed in' do + before do + sign_in(user) + end + + context 'when request is invalid' do + it 'renders the "show" template' do + post confirm_project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + expect(response.body).to include('The slash command request is invalid.') + end + end + + context 'when request is valid', :use_clean_rails_memory_store_caching do + before do + Rails.cache.write( + "slash-command-requests:#{params[:command_id]}", { team_id: chat_name.team_id, user_id: chat_name.chat_id } + ) + stub_request(:post, "http://www.example.com/").to_return(status: 200, body: 'ok') + end + + context 'when user is valid' do + it 'redirects back to the integration' do + post confirm_project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:redirect) + end + end + + context 'when user is invalid' do + let(:chat_name) { create(:chat_name) } + + it 'renders the "show" template' do + post confirm_project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + expect(response.body).to include('The slash command request is invalid.') + end + end + end + end + + context 'when user is not signed in' do + it 'redirects with a status of 302' do + post confirm_project_integrations_slash_commands_path(project), params: params + + expect(response).to have_gitlab_http_status(:redirect) + end + end + end +end diff --git a/spec/support/helpers/email_helpers.rb b/spec/support/helpers/email_helpers.rb index 3583889d305..5d06eb64e0b 100644 --- a/spec/support/helpers/email_helpers.rb +++ b/spec/support/helpers/email_helpers.rb @@ -14,6 +14,18 @@ module EmailHelpers ActiveJob::Base.queue_adapter.enqueued_jobs.clear end + def expect_only_one_email_to_be_sent(subject:, to:) + count_of_sent_emails = ActionMailer::Base.deliveries.count + expect(count_of_sent_emails).to eq(1), "Expected only one email to be sent, but #{count_of_sent_emails} emails were sent instead" + + return unless count_of_sent_emails == 1 + + message = ActionMailer::Base.deliveries.first + + expect(message.subject).to eq(subject), "Expected '#{subject}' email to be sent, but '#{message.subject}' email was sent instead" + expect(message.to).to match_array(to), "Expected the email to be sent to #{to}, but it was sent to #{message.to} instead" + end + def should_only_email(*users, kind: :to) recipients = email_recipients(kind: kind) diff --git a/spec/support/helpers/user_with_namespace_shim.yml b/spec/support/helpers/user_with_namespace_shim.yml index 3e5556b6d58..7b3dc099cf9 100644 --- a/spec/support/helpers/user_with_namespace_shim.yml +++ b/spec/support/helpers/user_with_namespace_shim.yml @@ -43,6 +43,7 @@ - ee/spec/features/issues/user_sees_empty_state_spec.rb - ee/spec/features/issues/user_uses_quick_actions_spec.rb - ee/spec/features/issues/user_views_issues_spec.rb +- ee/spec/features/merge_request/code_owner_approvals_reset_after_merging_to_source_branch_spec.rb - ee/spec/features/merge_request/draft_comments_spec.rb - ee/spec/features/merge_request/user_approves_with_password_spec.rb - ee/spec/features/merge_request/user_approves_with_saml_auth_spec.rb diff --git a/spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb b/spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb index ff3cc1841b4..90bb75d402f 100644 --- a/spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb +++ b/spec/support/shared_examples/models/integrations/base_slash_commands_shared_examples.rb @@ -91,38 +91,72 @@ RSpec.shared_examples Integrations::BaseSlashCommands do described_class.create!(project: project, properties: { token: 'token' }) end - it 'triggers the command' do - expect_any_instance_of(Gitlab::SlashCommands::Command).to receive(:execute) + context 'with verified request' do + before do + allow_next_instance_of(::Gitlab::SlashCommands::VerifyRequest) do |instance| + allow(instance).to receive(:valid?).and_return(true) + end + end - subject.trigger(params) - end + it 'triggers the command' do + expect_any_instance_of(Gitlab::SlashCommands::Command).to receive(:execute) + + subject.trigger(params) + end - shared_examples_for 'blocks command execution' do - it do - expect_any_instance_of(Gitlab::SlashCommands::Command).not_to receive(:execute) + shared_examples_for 'blocks command execution' do + it do + expect_any_instance_of(Gitlab::SlashCommands::Command).not_to receive(:execute) - result = subject.trigger(params) - expect(result[:text]).to match(error_message) + result = subject.trigger(params) + expect(result[:text]).to match(error_message) + end end - end - context 'when user is blocked' do - before do - chat_name.user.block + context 'when user is blocked' do + before do + chat_name.user.block + end + + it_behaves_like 'blocks command execution' do + let(:error_message) { 'you do not have access to the GitLab project' } + end end - it_behaves_like 'blocks command execution' do - let(:error_message) { 'you do not have access to the GitLab project' } + context 'when user is deactivated' do + before do + chat_name.user.deactivate + end + + it_behaves_like 'blocks command execution' do + let(:error_message) { "your #{Gitlab.config.gitlab.url} account needs to be reactivated" } + end end end - context 'when user is deactivated' do + context 'with unverified request' do before do - chat_name.user.deactivate + allow_next_instance_of(::Gitlab::SlashCommands::VerifyRequest) do |instance| + allow(instance).to receive(:valid?).and_return(false) + end end - it_behaves_like 'blocks command execution' do - let(:error_message) { "your #{Gitlab.config.gitlab.url} account needs to be reactivated" } + let(:params) do + { + team_domain: 'http://domain.tld', + channel_name: 'channel-test', + team_id: chat_name.team_id, + user_id: chat_name.chat_id, + response_url: 'http://domain.tld/commands', + token: 'token' + } + end + + it 'caches the slash command params and returns confirmation message' do + expect(Rails.cache).to receive(:write).with(an_instance_of(String), params, { expires_in: 3.minutes }) + expect_any_instance_of(Gitlab::SlashCommands::Presenters::Access).to receive(:confirm) + + subject.trigger(params) end end end |