From 4e86ca506d67f27cb4ca93b47efc53adc1c237a6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 4 Nov 2021 00:12:36 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/email.rb | 12 +- app/models/user.rb | 62 ++-- app/services/emails/destroy_service.rb | 2 + .../error_tracking_event_payload.json | 4 +- app/views/profiles/emails/index.html.haml | 2 +- ...imary_email_to_emails_if_user_confirmed.rb | 30 ++ ...rge_request_diff_commit_users_migration.rb | 55 ++++ db/schema_migrations/20211004120135 | 1 + db/schema_migrations/20211028155449 | 1 + doc/api/graphql/reference/index.md | 18 +- doc/ci/pipelines/pipeline_artifacts.md | 20 +- doc/ci/yaml/index.md | 297 +++--------------- doc/topics/index.md | 25 +- doc/user/group/saml_sso/index.md | 5 +- doc/user/permissions.md | 3 +- doc/user/project/releases/index.md | 163 +++++++++- doc/user/project/releases/release_cli.md | 76 +++++ lib/api/error_tracking/collector.rb | 14 + .../collector/payload_validator.rb | 13 + ...imary_email_to_emails_if_user_confirmed.rb | 58 ++++ .../fix_merge_request_diff_commit_users.rb | 129 ++++++++ .../database/background_migration_job.rb | 1 + .../import_export/base/object_builder.rb | 12 +- .../import_export/project/import_export.yml | 4 - .../import_export/project/object_builder.rb | 40 ++- .../import_export/project/relation_factory.rb | 4 +- lib/gitlab/spamcheck/client.rb | 2 + qa/qa.rb | 3 +- .../scenario/test/integration/registry_tls.rb | 13 + .../container_registry_omnibus_spec.rb | 145 ++++++--- .../dependency_proxy/dependency_proxy_spec.rb | 12 +- .../oauth/authorizations_controller_spec.rb | 7 +- spec/features/profiles/emails_spec.rb | 17 +- .../profiles/user_manages_emails_spec.rb | 15 +- spec/features/signed_commits_spec.rb | 1 + .../lightweight_project_export.tar.gz | Bin 3647 -> 3758 bytes spec/helpers/users_helper_spec.rb | 2 +- .../collector/payload_validator_spec.rb | 49 +++ ..._email_to_emails_if_user_confirmed_spec.rb | 49 +++ ...ix_merge_request_diff_commit_users_spec.rb | 286 +++++++++++++++++ .../gpg/invalid_gpg_signature_updater_spec.rb | 2 + .../project/object_builder_spec.rb | 114 +++++++ spec/lib/gitlab/spamcheck/client_spec.rb | 2 +- spec/lib/gitlab/x509/signature_spec.rb | 104 +++--- ...equest_diff_commit_users_migration_spec.rb | 63 ++++ ..._email_to_emails_if_user_confirmed_spec.rb | 31 ++ spec/models/email_spec.rb | 11 +- spec/models/user_spec.rb | 59 ++-- .../api/error_tracking/collector_spec.rb | 15 + spec/requests/api/users_spec.rb | 6 +- spec/requests/users_controller_spec.rb | 2 +- spec/services/emails/create_service_spec.rb | 5 +- spec/services/emails/destroy_service_spec.rb | 10 + 53 files changed, 1584 insertions(+), 492 deletions(-) create mode 100644 db/post_migrate/20211004120135_schedule_add_primary_email_to_emails_if_user_confirmed.rb create mode 100644 db/post_migrate/20211028155449_schedule_fix_merge_request_diff_commit_users_migration.rb create mode 100644 db/schema_migrations/20211004120135 create mode 100644 db/schema_migrations/20211028155449 create mode 100644 doc/user/project/releases/release_cli.md create mode 100644 lib/error_tracking/collector/payload_validator.rb create mode 100644 lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed.rb create mode 100644 lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb create mode 100644 qa/qa/scenario/test/integration/registry_tls.rb create mode 100644 spec/lib/error_tracking/collector/payload_validator_spec.rb create mode 100644 spec/lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed_spec.rb create mode 100644 spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb create mode 100644 spec/migrations/20211028155449_schedule_fix_merge_request_diff_commit_users_migration_spec.rb create mode 100644 spec/migrations/schedule_add_primary_email_to_emails_if_user_confirmed_spec.rb diff --git a/app/models/email.rb b/app/models/email.rb index 0140f784842..676e79406e9 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -29,7 +29,7 @@ class Email < ApplicationRecord end def unique_email - self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email) + self.errors.add(:email, 'has already been taken') if primary_email_of_another_user? end def validate_email_format @@ -40,4 +40,14 @@ class Email < ApplicationRecord def update_invalid_gpg_signatures user.update_invalid_gpg_signatures if confirmed? end + + def user_primary_email? + email.casecmp?(user.email) + end + + private + + def primary_email_of_another_user? + User.where(email: email).where.not(id: user_id).exists? + end end diff --git a/app/models/user.rb b/app/models/user.rb index b42a47cec0a..886c30bd28e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -274,14 +274,21 @@ class User < ApplicationRecord after_update :username_changed_hook, if: :saved_change_to_username? after_destroy :post_destroy_hook after_destroy :remove_key_cache + after_create :add_primary_email_to_emails!, if: :confirmed? after_commit(on: :update) do 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 + # Add the old primary email to Emails if not added already - this should be removed + # after the background migration for MR https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70872/ has completed, + # as the primary email is now added to Emails upon confirmation + # Issue to remove that: https://gitlab.com/gitlab-org/gitlab/-/issues/344134 previous_confirmed_at = previous_changes.key?('confirmed_at') ? previous_changes['confirmed_at'][0] : confirmed_at previous_email = previous_changes[:email][0] + if previous_confirmed_at && !emails.exists?(email: previous_email) + # rubocop: disable CodeReuse/ServiceClass + Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: previous_confirmed_at) + # rubocop: enable CodeReuse/ServiceClass + end - update_emails_with_primary_email(previous_confirmed_at, previous_email) update_invalid_gpg_signatures end end @@ -935,6 +942,8 @@ class User < ApplicationRecord end def unique_email + return if errors.added?(:email, _('has already been taken')) + if !emails.exists?(email: email) && Email.exists?(email: email) errors.add(:email, _('has already been taken')) end @@ -963,24 +972,6 @@ class User < ApplicationRecord skip_reconfirmation! if emails.confirmed.where(email: self.email).any? end - # Note: the use of the Emails services will cause `saves` on the user object, running - # through the callbacks again and can have side effects, such as the `previous_changes` - # hash and `_was` variables getting munged. - # 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_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: previous_confirmed_at) - - update_columns(confirmed_at: primary_email_record.confirmed_at) if primary_email_record&.confirmed_at - end - # rubocop: enable CodeReuse/ServiceClass - def update_invalid_gpg_signatures gpg_keys.each(&:update_invalid_gpg_signatures) end @@ -1389,7 +1380,7 @@ class User < ApplicationRecord all_emails << email unless temp_oauth_email? all_emails << private_commit_email if include_private_email all_emails.concat(emails.map(&:email)) - all_emails + all_emails.uniq end def verified_emails(include_private_email: true) @@ -1397,7 +1388,7 @@ class User < ApplicationRecord verified_emails << email if primary_email_verified? verified_emails << private_commit_email if include_private_email verified_emails.concat(emails.confirmed.pluck(:email)) - verified_emails + verified_emails.uniq end def public_verified_emails @@ -1978,6 +1969,25 @@ class User < ApplicationRecord ci_job_token_scope.present? end + # override from Devise::Confirmable + # + # Add the primary email to user.emails (or confirm it if it was already + # present) when the primary email is confirmed. + def confirm(*args) + saved = super(*args) + return false unless saved + + email_to_confirm = self.emails.find_by(email: self.email) + + if email_to_confirm.present? + email_to_confirm.confirm(*args) + else + add_primary_email_to_emails! + end + + saved + end + protected # override, from Devise::Validatable @@ -2018,6 +2028,12 @@ class User < ApplicationRecord 'en' end + # rubocop: disable CodeReuse/ServiceClass + def add_primary_email_to_emails! + Emails::CreateService.new(self, user: self, email: self.email).execute(confirmed_at: self.confirmed_at) + end + # rubocop: enable CodeReuse/ServiceClass + def notification_email_verified return if notification_email.blank? || temp_oauth_email? diff --git a/app/services/emails/destroy_service.rb b/app/services/emails/destroy_service.rb index d10833e66cb..d211c3470b2 100644 --- a/app/services/emails/destroy_service.rb +++ b/app/services/emails/destroy_service.rb @@ -3,6 +3,8 @@ module Emails class DestroyService < ::Emails::BaseService def execute(email) + raise StandardError, 'Cannot delete primary email' if email.user_primary_email? + email.destroy && update_secondary_emails!(email.email) end diff --git a/app/validators/json_schemas/error_tracking_event_payload.json b/app/validators/json_schemas/error_tracking_event_payload.json index b50dd0063e1..73ff71043ce 100644 --- a/app/validators/json_schemas/error_tracking_event_payload.json +++ b/app/validators/json_schemas/error_tracking_event_payload.json @@ -1,7 +1,7 @@ { "description": "Error tracking event payload", "type": "object", - "required": [], + "required": ["exception"], "properties": { "environment": { "type": "string" @@ -14,7 +14,7 @@ }, "exception": { "type": "object", - "required": [], + "required": ["values"], "properties": { "values": { "type": "array", diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index 35bdfbb1c29..0cfe65994da 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -44,7 +44,7 @@ %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Public email') - if @primary_email === current_user.notification_email_or_default %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Default notification email') - - @emails.each do |email| + - @emails.reject(&:user_primary_email?).each do |email| %li{ data: { qa_selector: 'email_row_content' } } = render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? } %span.float-right diff --git a/db/post_migrate/20211004120135_schedule_add_primary_email_to_emails_if_user_confirmed.rb b/db/post_migrate/20211004120135_schedule_add_primary_email_to_emails_if_user_confirmed.rb new file mode 100644 index 00000000000..d7b213b384a --- /dev/null +++ b/db/post_migrate/20211004120135_schedule_add_primary_email_to_emails_if_user_confirmed.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class ScheduleAddPrimaryEmailToEmailsIfUserConfirmed < Gitlab::Database::Migration[1.0] + INTERVAL = 2.minutes.to_i + BATCH_SIZE = 10_000 + MIGRATION = 'AddPrimaryEmailToEmailsIfUserConfirmed' + + disable_ddl_transaction! + + class User < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'users' + self.inheritance_column = :_type_disabled + end + + def up + queue_background_migration_jobs_by_range_at_intervals( + User, + MIGRATION, + INTERVAL, + batch_size: BATCH_SIZE, + track_jobs: true + ) + end + + def down + # intentionally blank + end +end diff --git a/db/post_migrate/20211028155449_schedule_fix_merge_request_diff_commit_users_migration.rb b/db/post_migrate/20211028155449_schedule_fix_merge_request_diff_commit_users_migration.rb new file mode 100644 index 00000000000..659cb7b76b2 --- /dev/null +++ b/db/post_migrate/20211028155449_schedule_fix_merge_request_diff_commit_users_migration.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class ScheduleFixMergeRequestDiffCommitUsersMigration < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + MIGRATION_CLASS = 'FixMergeRequestDiffCommitUsers' + + class Project < ApplicationRecord + include EachBatch + + self.table_name = 'projects' + end + + def up + # This is the day on which we merged + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63669. Since the + # deploy of this MR we may have imported projects using the old format, but + # after their merge_request_diff_id range had been migrated by Sidekiq. As a + # result, there may be rows without a committer_id or commit_author_id + # field. + date = '2021-07-07 00:00:00' + + transaction do + Project.each_batch(of: 10_000) do |batch| + time = Time.now.utc + rows = batch + .where('created_at >= ?', date) + .where(import_type: 'gitlab_project') + .pluck(:id) + .map do |id| + Gitlab::Database::BackgroundMigrationJob.new( + class_name: MIGRATION_CLASS, + arguments: [id], + created_at: time, + updated_at: time + ) + end + + Gitlab::Database::BackgroundMigrationJob + .bulk_insert!(rows, validate: false) + end + end + + job = Gitlab::Database::BackgroundMigrationJob + .for_migration_class(MIGRATION_CLASS) + .pending + .first + + migrate_in(2.minutes, MIGRATION_CLASS, job.arguments) if job + end + + def down + # no-op + end +end diff --git a/db/schema_migrations/20211004120135 b/db/schema_migrations/20211004120135 new file mode 100644 index 00000000000..e03dc56e002 --- /dev/null +++ b/db/schema_migrations/20211004120135 @@ -0,0 +1 @@ +9cefd32c003a68752f257973a983f77215b02011b7ca792de06c0e92c2462745 \ No newline at end of file diff --git a/db/schema_migrations/20211028155449 b/db/schema_migrations/20211028155449 new file mode 100644 index 00000000000..00b1c4e14dd --- /dev/null +++ b/db/schema_migrations/20211028155449 @@ -0,0 +1 @@ +385c540b1f80c31a5ba009ae3507d2b855a737389bcb75c07a8872f26f8a9b44 \ No newline at end of file diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 924ced90321..4b09907b9a8 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -12646,7 +12646,6 @@ Represents vulnerability finding of a security report on the pipeline. | `containerRegistryEnabled` | [`Boolean`](#boolean) | Indicates if Container Registry is enabled for the current user. | | `containerRepositoriesCount` | [`Int!`](#int) | Number of container repositories in the project. | | `createdAt` | [`Time`](#time) | Timestamp of the project creation. | -| `dastProfiles` | [`DastProfileConnection`](#dastprofileconnection) | DAST Profiles associated with the project. (see [Connections](#connections)) | | `dastScannerProfiles` | [`DastScannerProfileConnection`](#dastscannerprofileconnection) | DAST scanner profiles associated with the project. (see [Connections](#connections)) | | `dastSiteProfiles` | [`DastSiteProfileConnection`](#dastsiteprofileconnection) | DAST Site Profiles associated with the project. (see [Connections](#connections)) | | `description` | [`String`](#string) | Short description of the project. | @@ -12884,8 +12883,25 @@ Returns [`DastProfile`](#dastprofile). | Name | Type | Description | | ---- | ---- | ----------- | +| `hasDastProfileSchedule` | [`Boolean`](#boolean) | Filter DAST Profiles by whether or not they have a schedule. Will be ignored if `dast_view_scans` feature flag is disabled. | | `id` | [`DastProfileID!`](#dastprofileid) | ID of the DAST Profile. | +##### `Project.dastProfiles` + +DAST Profiles associated with the project. + +Returns [`DastProfileConnection`](#dastprofileconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#connection-pagination-arguments): +`before: String`, `after: String`, `first: Int`, `last: Int`. + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `hasDastProfileSchedule` | [`Boolean`](#boolean) | Filter DAST Profiles by whether or not they have a schedule. Will be ignored if `dast_view_scans` feature flag is disabled. | + ##### `Project.dastSiteProfile` DAST Site Profile associated with the project. diff --git a/doc/ci/pipelines/pipeline_artifacts.md b/doc/ci/pipelines/pipeline_artifacts.md index 55555571f97..b174b6af9f9 100644 --- a/doc/ci/pipelines/pipeline_artifacts.md +++ b/doc/ci/pipelines/pipeline_artifacts.md @@ -7,20 +7,24 @@ type: reference, howto # Pipeline artifacts **(FREE)** -Pipeline artifacts are files created by GitLab after a pipeline finishes. These are different than [job artifacts](job_artifacts.md) because they are not explicitly managed by the `.gitlab-ci.yml` definitions. +Pipeline artifacts are files created by GitLab after a pipeline finishes. Pipeline artifacts are +different to [job artifacts](job_artifacts.md) because they are not explicitly managed by +`.gitlab-ci.yml` definitions. -Pipeline artifacts are used by the [test coverage visualization feature](../../user/project/merge_requests/test_coverage_visualization.md) to collect coverage information. It uses the [`artifacts: reports`](../yaml/index.md#artifactsreports) CI/CD keyword. +Pipeline artifacts are used by the [test coverage visualization feature](../../user/project/merge_requests/test_coverage_visualization.md) +to collect coverage information. It uses the [`artifacts: reports`](../yaml/index.md#artifactsreports) CI/CD keyword. ## Storage -Pipeline artifacts are saved to disk or object storage. They count towards a project's [storage usage quota](../../user/usage_quotas.md#storage-usage-quota). The **Artifacts** on the Usage Quotas page is the sum of all job artifacts and pipeline artifacts. +Pipeline artifacts are saved to disk or object storage. They count towards a project's [storage usage quota](../../user/usage_quotas.md#storage-usage-quota). +The **Artifacts** on the Usage Quotas page is the sum of all job artifacts and pipeline artifacts. ## When pipeline artifacts are deleted -Pipeline artifacts are deleted either: +Pipeline artifacts from: -- Seven days after creation. -- After another pipeline runs successfully, if they are from the most recent successful - pipeline. +- The latest pipeline are kept forever. +- Pipelines superseded by a newer pipeline are deleted seven days after their creation date. -This deletion may take up to two days. +It can take up to two days for GitLab to delete pipeline artifacts from when they are due to be +deleted. diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 22d69cafa83..bd1b96bbc9b 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -2872,10 +2872,9 @@ they expire and are deleted. The `expire_in` setting does not affect: - Artifacts from the latest job, unless keeping the latest job artifacts is: - [Disabled at the project level](../pipelines/job_artifacts.md#keep-artifacts-from-most-recent-successful-jobs). - [Disabled instance-wide](../../user/admin_area/settings/continuous_integration.md#keep-the-latest-artifacts-for-all-jobs-in-the-latest-successful-pipelines). -- [Pipeline artifacts](../pipelines/pipeline_artifacts.md). It's not possible to specify an - expiration date for these: - - Pipeline artifacts from the latest pipeline are kept forever. - - Other pipeline artifacts are erased after one week. +- [Pipeline artifacts](../pipelines/pipeline_artifacts.md). You can't specify an expiration date for + pipeline artifacts. See [When pipeline artifacts are deleted](../pipelines/pipeline_artifacts.md#when-pipeline-artifacts-are-deleted) + for more information. The value of `expire_in` is an elapsed time in seconds, unless a unit is provided. Valid values include: @@ -4039,10 +4038,20 @@ finishes. > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/19298) in GitLab 13.2. Use `release` to create a [release](../../user/project/releases/index.md). -Requires the [`release-cli`](https://gitlab.com/gitlab-org/release-cli/-/tree/master/docs) -to be available in your GitLab Runner Docker or shell executor. -These keywords are supported: +The release job must have access to [`release-cli`](https://gitlab.com/gitlab-org/release-cli/-/tree/master/docs) +when it runs. When using a runner with: + +- The [Docker executor](https://docs.gitlab.com/runner/executors/docker.html), use + [`image:`](#image) and a Docker image that includes `release-cli`. For example, + this image from the GitLab.com Container registry: `registry.gitlab.com/gitlab-org/release-cli:latest` + +- The [Shell executor](https://docs.gitlab.com/runner/executors/shell.html), the server + where the runner is registered must [have `release-cli` installed](../../user/project/releases/release_cli.md). + +**Keyword type**: Job keyword. You can use it only as part of a job. + +**Possible inputs**: The `release:` subkeys: - [`tag_name`](#releasetag_name) - [`description`](#releasedescription) @@ -4052,149 +4061,44 @@ These keywords are supported: - [`released_at`](#releasereleased_at) (optional) - [`assets:links`](#releaseassetslinks) (optional) -The release is created only if the job processes without error. If the Rails API -returns an error during release creation, the `release` job fails. +**Example of `release` keyword**: -#### `release-cli` Docker image - -You must specify the Docker image to use for the `release-cli`: - -```yaml -image: registry.gitlab.com/gitlab-org/release-cli:latest -``` - -#### `release-cli` for shell executors - -> - [Introduced](https://gitlab.com/gitlab-org/release-cli/-/issues/21) in GitLab 13.8. -> - [Changed](https://gitlab.com/gitlab-org/release-cli/-/merge_requests/108): the `release-cli` binaries are also -[available in the Package Registry](https://gitlab.com/jaime/release-cli/-/packages) -starting from GitLab 14.2. - -For GitLab Runner shell executors, you can download and install the `release-cli` manually for your [supported OS and architecture](https://release-cli-downloads.s3.amazonaws.com/latest/index.html). -Once installed, the `release` keyword should be available to you. - -**Install on Unix/Linux** - -1. Download the binary for your system from S3, in the following example for amd64 systems: - - ```shell - curl --location --output /usr/local/bin/release-cli "https://release-cli-downloads.s3.amazonaws.com/latest/release-cli-linux-amd64" + ```yaml + release_job: + stage: release + image: registry.gitlab.com/gitlab-org/release-cli:latest + rules: + - if: $CI_COMMIT_TAG # Run this job when a tag is created manually + script: + - echo 'Running the release job.' + release: + name: 'Release $CI_COMMIT_TAG' + description: 'Release created using the release-cli.' ``` -Or from the GitLab package registry: +This example creates a release: - ```shell - curl --location --output /usr/local/bin/release-cli "https://gitlab.com/api/v4/projects/gitlab-org%2Frelease-cli/packages/generic/release-cli/latest/release-cli-darwin-amd64" - ``` +- When you push a Git tag. +- When you add a Git tag in the UI at **Repository > Tags**. -1. Give it permissions to execute: +**Additional details**: - ```shell - sudo chmod +x /usr/local/bin/release-cli - ``` +- All jobs except [trigger](#trigger) jobs must have the `script` keyword. A `release` + job can use the output from script commands, but you can use a placeholder script if + the script is not needed: -1. Verify `release-cli` is available: - - ```shell - $ release-cli -v - - release-cli version 0.6.0 - ``` - -**Install on Windows PowerShell** - -1. Create a folder somewhere in your system, for example `C:\GitLab\Release-CLI\bin` - - ```shell - New-Item -Path 'C:\GitLab\Release-CLI\bin' -ItemType Directory - ``` - -1. Download the executable file: - - ```shell - PS C:\> Invoke-WebRequest -Uri "https://release-cli-downloads.s3.amazonaws.com/latest/release-cli-windows-amd64.exe" -OutFile "C:\GitLab\Release-CLI\bin\release-cli.exe" - - Directory: C:\GitLab\Release-CLI - Mode LastWriteTime Length Name - ---- ------------- ------ ---- - d----- 3/16/2021 4:17 AM bin - - ``` - -1. Add the directory to your `$env:PATH`: - - ```shell - $env:PATH += ";C:\GitLab\Release-CLI\bin" - ``` - -1. Verify `release-cli` is available: - - ```shell - PS C:\> release-cli -v - - release-cli version 0.6.0 - ``` - -#### Use a custom SSL CA certificate authority - -You can use the `ADDITIONAL_CA_CERT_BUNDLE` CI/CD variable to configure a custom SSL CA certificate authority, -which is used to verify the peer when the `release-cli` creates a release through the API using HTTPS with custom certificates. -The `ADDITIONAL_CA_CERT_BUNDLE` value should contain the -[text representation of the X.509 PEM public-key certificate](https://tools.ietf.org/html/rfc7468#section-5.1) -or the `path/to/file` containing the certificate authority. -For example, to configure this value in the `.gitlab-ci.yml` file, use the following: - -```yaml -release: - variables: - ADDITIONAL_CA_CERT_BUNDLE: | - -----BEGIN CERTIFICATE----- - MIIGqTCCBJGgAwIBAgIQI7AVxxVwg2kch4d56XNdDjANBgkqhkiG9w0BAQsFADCB - ... - jWgmPqF3vUbZE0EyScetPJquRFRKIesyJuBFMAs= - -----END CERTIFICATE----- + ```yaml script: - - echo "Create release" - release: - name: 'My awesome release' - tag_name: '$CI_COMMIT_TAG' -``` + - echo 'release job' + ``` -The `ADDITIONAL_CA_CERT_BUNDLE` value can also be configured as a -[custom variable in the UI](../variables/index.md#custom-cicd-variables), -either as a `file`, which requires the path to the certificate, or as a variable, -which requires the text representation of the certificate. + An [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/223856) exists to remove this requirement in an upcoming version of GitLab. -#### `script` +**Related topics**: -All jobs except [trigger](#trigger) jobs must have the `script` keyword. A `release` -job can use the output from script commands, but you can use a placeholder script if -the script is not needed: - -```yaml -script: - - echo 'release job' -``` - -An [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/223856) exists to remove this requirement in an upcoming version of GitLab. - -A pipeline can have multiple `release` jobs, for example: - -```yaml -ios-release: - script: - - echo 'iOS release job' - release: - tag_name: v1.0.0-ios - description: 'iOS release v1.0.0' - -android-release: - script: - - echo 'Android release job' - release: - tag_name: v1.0.0-android - description: 'Android release v1.0.0' -``` +- [CI/CD example of the `release` keyword](../../user/project/releases/index.md#cicd-example-of-the-release-keyword). +- [Create multiple releases in a single pipeline](../../user/project/releases/index.md#create-multiple-releases-in-a-single-pipeline). +- [Use a custom SSL CA certificate authority](../../user/project/releases/index.md#use-a-custom-ssl-ca-certificate-authority). #### `release:tag_name` @@ -4238,11 +4142,8 @@ The release name. If omitted, it is populated with the value of `release: tag_na Specifies the long description of the release. You can also specify a file that contains the description. -##### Read description from a file - -> [Introduced](https://gitlab.com/gitlab-org/release-cli/-/merge_requests/67) in GitLab 13.7. - -You can specify a file in `$CI_PROJECT_DIR` that contains the description. The file must be relative +In [GitLab 13.7 and later]((https://gitlab.com/gitlab-org/release-cli/-/merge_requests/67)), +you can specify a file in `$CI_PROJECT_DIR` that contains the description. The file must be relative to the project directory (`$CI_PROJECT_DIR`), and if the file is a symbolic link it can't reside outside of `$CI_PROJECT_DIR`. The `./path/to/file` and filename can't contain spaces. @@ -4291,114 +4192,6 @@ assets: link_type: 'other' # optional ``` -#### Complete example for `release` - -If you combine the previous examples for `release`, you get two options, depending on how you generate the -tags. You can't use these options together, so choose one: - -- To create a release when you push a Git tag, or when you add a Git tag - in the UI by going to **Repository > Tags**: - - ```yaml - release_job: - stage: release - image: registry.gitlab.com/gitlab-org/release-cli:latest - rules: - - if: $CI_COMMIT_TAG # Run this job when a tag is created manually - script: - - echo 'running release_job' - release: - name: 'Release $CI_COMMIT_TAG' - description: 'Created using the release-cli $EXTRA_DESCRIPTION' # $EXTRA_DESCRIPTION must be defined - tag_name: '$CI_COMMIT_TAG' # elsewhere in the pipeline. - ref: '$CI_COMMIT_TAG' - milestones: - - 'm1' - - 'm2' - - 'm3' - released_at: '2020-07-15T08:00:00Z' # Optional, is auto generated if not defined, or can use a variable. - assets: # Optional, multiple asset links - links: - - name: 'asset1' - url: 'https://example.com/assets/1' - - name: 'asset2' - url: 'https://example.com/assets/2' - filepath: '/pretty/url/1' # optional - link_type: 'other' # optional - ``` - -- To create a release automatically when commits are pushed or merged to the default branch, - using a new Git tag that is defined with variables: - - NOTE: - Environment variables set in `before_script` or `script` are not available for expanding - in the same job. Read more about - [potentially making variables available for expanding](https://gitlab.com/gitlab-org/gitlab-runner/-/issues/6400). - - ```yaml - prepare_job: - stage: prepare # This stage must run before the release stage - rules: - - if: $CI_COMMIT_TAG - when: never # Do not run this job when a tag is created manually - - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH # Run this job when commits are pushed or merged to the default branch - script: - - echo "EXTRA_DESCRIPTION=some message" >> variables.env # Generate the EXTRA_DESCRIPTION and TAG environment variables - - echo "TAG=v$(cat VERSION)" >> variables.env # and append to the variables.env file - artifacts: - reports: - dotenv: variables.env # Use artifacts:reports:dotenv to expose the variables to other jobs - - release_job: - stage: release - image: registry.gitlab.com/gitlab-org/release-cli:latest - needs: - - job: prepare_job - artifacts: true - rules: - - if: $CI_COMMIT_TAG - when: never # Do not run this job when a tag is created manually - - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH # Run this job when commits are pushed or merged to the default branch - script: - - echo 'running release_job for $TAG' - release: - name: 'Release $TAG' - description: 'Created using the release-cli $EXTRA_DESCRIPTION' # $EXTRA_DESCRIPTION and the $TAG - tag_name: '$TAG' # variables must be defined elsewhere - ref: '$CI_COMMIT_SHA' # in the pipeline. For example, in the - milestones: # prepare_job - - 'm1' - - 'm2' - - 'm3' - released_at: '2020-07-15T08:00:00Z' # Optional, is auto generated if not defined, or can use a variable. - assets: - links: - - name: 'asset1' - url: 'https://example.com/assets/1' - - name: 'asset2' - url: 'https://example.com/assets/2' - filepath: '/pretty/url/1' # optional - link_type: 'other' # optional - ``` - -#### Release assets as Generic packages - -You can use [Generic packages](../../user/packages/generic_packages/) to host your release assets. -For a complete example, see the [Release assets as Generic packages](https://gitlab.com/gitlab-org/release-cli/-/tree/master/docs/examples/release-assets-as-generic-package/) -project. - -#### `release-cli` command line - -The entries under the `release` node are transformed into a `bash` command line and sent -to the Docker container, which contains the [release-cli](https://gitlab.com/gitlab-org/release-cli). -You can also call the `release-cli` directly from a `script` entry. - -For example, if you use the YAML described previously: - -```shell -release-cli create --name "Release $CI_COMMIT_SHA" --description "Created using the release-cli $EXTRA_DESCRIPTION" --tag-name "v${MAJOR}.${MINOR}.${REVISION}" --ref "$CI_COMMIT_SHA" --released-at "2020-07-15T08:00:00Z" --milestone "m1" --milestone "m2" --milestone "m3" --assets-link "{\"name\":\"asset1\",\"url\":\"https://example.com/assets/1\",\"link_type\":\"other\"} -``` - ### `secrets` > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33014) in GitLab 13.4. diff --git a/doc/topics/index.md b/doc/topics/index.md index fde420c64f6..6d2c839430b 100644 --- a/doc/topics/index.md +++ b/doc/topics/index.md @@ -1,24 +1,9 @@ --- -stage: none -group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +redirect_to: '../index.md' +remove_date: '2022-02-03' --- -# Topics +This document was moved to [another location](../index.md). -Welcome to Topics! We have organized our content resources into topics -to get you started on areas of your interest. Each topic page -consists of an index listing all related content. It guides -you through better understanding GitLab concepts -through our regular docs, and, when available, through articles (guides, -tutorials, technical overviews, blog posts) and videos. - -- [Auto DevOps](autodevops/index.md) -- [Authentication](authentication/index.md) -- [Continuous Integration (GitLab CI/CD)](../ci/index.md) -- [Cron](cron/index.md) -- [Git](git/index.md) -- [GitLab Flow](gitlab_flow.md) -- [GitLab Installation](../install/index.md) -- [GitLab Pages](../user/project/pages/index.md) -- [Offline GitLab](offline/index.md) + + diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index a5d4d9bd965..25cc07b7f75 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -245,8 +245,9 @@ On subsequent visits, you should be able to go [sign in to GitLab.com with SAML] > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/263661) in GitLab 13.7. GitLab allows setting certain user attributes based on values from the SAML response. -This affects users created on first sign-in via Group SAML. Existing users' -attributes are not affected regardless of the values sent in the SAML response. +Existing users will have these attributes updated if the user was originally +provisioned by the group. Users are provisioned by the group when the account was +created via [SCIM](scim_setup.md) or by first sign-in with SAML SSO for GitLab.com groups. #### Supported user attributes diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 60333d1ab6a..a0f83aca80c 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -486,12 +486,11 @@ which visibility level you select on project settings. GitLab CI/CD permissions rely on the role the user has in GitLab: -- Administrator - Maintainer - Developer - Guest/Reporter -The Administrator role can perform any action on GitLab CI/CD in scope of the GitLab +GitLab administrators can perform any action on GitLab CI/CD in scope of the GitLab instance and project. | Action | Guest, Reporter | Developer |Maintainer| Administrator | diff --git a/doc/user/project/releases/index.md b/doc/user/project/releases/index.md index 9e4d621dbc6..d16c6cb24f1 100644 --- a/doc/user/project/releases/index.md +++ b/doc/user/project/releases/index.md @@ -78,17 +78,172 @@ To create a new release through the GitLab UI: [release notes](#release-notes-description), or [assets links](#links). 1. Click **Create release**. -### Create release from GitLab CI +## Create a release by using a CI/CD job > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/19298) in GitLab 12.7. -You can [create a release directly from the GitLab CI pipeline](../../../ci/yaml/index.md#release) -by using a `release` node in the job definition. +You can create a release directly as part of the GitLab CI/CD pipeline +by using [the `release` keyword](../../../ci/yaml/index.md#release) in the job definition. The release is created only if the job processes without error. If the Rails API returns an error during release creation, the release job fails. -### Upcoming releases +### CI/CD example of the `release` keyword + +To create a release when you push a Git tag, or when you add a Git tag +in the UI by going to **Repository > Tags**: + +```yaml +release_job: + stage: release + image: registry.gitlab.com/gitlab-org/release-cli:latest + rules: + - if: $CI_COMMIT_TAG # Run this job when a tag is created manually + script: + - echo 'running release_job' + release: + name: 'Release $CI_COMMIT_TAG' + description: 'Created using the release-cli $EXTRA_DESCRIPTION' # $EXTRA_DESCRIPTION must be defined + tag_name: '$CI_COMMIT_TAG' # elsewhere in the pipeline. + ref: '$CI_COMMIT_TAG' + milestones: + - 'm1' + - 'm2' + - 'm3' + released_at: '2020-07-15T08:00:00Z' # Optional, is auto generated if not defined, or can use a variable. + assets: # Optional, multiple asset links + links: + - name: 'asset1' + url: 'https://example.com/assets/1' + - name: 'asset2' + url: 'https://example.com/assets/2' + filepath: '/pretty/url/1' # optional + link_type: 'other' # optional +``` + +To create a release automatically when commits are pushed or merged to the default branch, +using a new Git tag that is defined with variables: + +```yaml +prepare_job: + stage: prepare # This stage must run before the release stage + rules: + - if: $CI_COMMIT_TAG + when: never # Do not run this job when a tag is created manually + - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH # Run this job when commits are pushed or merged to the default branch + script: + - echo "EXTRA_DESCRIPTION=some message" >> variables.env # Generate the EXTRA_DESCRIPTION and TAG environment variables + - echo "TAG=v$(cat VERSION)" >> variables.env # and append to the variables.env file + artifacts: + reports: + dotenv: variables.env # Use artifacts:reports:dotenv to expose the variables to other jobs + +release_job: + stage: release + image: registry.gitlab.com/gitlab-org/release-cli:latest + needs: + - job: prepare_job + artifacts: true + rules: + - if: $CI_COMMIT_TAG + when: never # Do not run this job when a tag is created manually + - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH # Run this job when commits are pushed or merged to the default branch + script: + - echo 'running release_job for $TAG' + release: + name: 'Release $TAG' + description: 'Created using the release-cli $EXTRA_DESCRIPTION' # $EXTRA_DESCRIPTION and the $TAG + tag_name: '$TAG' # variables must be defined elsewhere + ref: '$CI_COMMIT_SHA' # in the pipeline. For example, in the + milestones: # prepare_job + - 'm1' + - 'm2' + - 'm3' + released_at: '2020-07-15T08:00:00Z' # Optional, is auto generated if not defined, or can use a variable. + assets: + links: + - name: 'asset1' + url: 'https://example.com/assets/1' + - name: 'asset2' + url: 'https://example.com/assets/2' + filepath: '/pretty/url/1' # optional + link_type: 'other' # optional +``` + +NOTE: +Environment variables set in `before_script` or `script` are not available for expanding +in the same job. Read more about +[potentially making variables available for expanding](https://gitlab.com/gitlab-org/gitlab-runner/-/issues/6400). + +### Use a custom SSL CA certificate authority + +You can use the `ADDITIONAL_CA_CERT_BUNDLE` CI/CD variable to configure a custom SSL CA certificate authority, +which is used to verify the peer when the `release-cli` creates a release through the API using HTTPS with custom certificates. +The `ADDITIONAL_CA_CERT_BUNDLE` value should contain the +[text representation of the X.509 PEM public-key certificate](https://tools.ietf.org/html/rfc7468#section-5.1) +or the `path/to/file` containing the certificate authority. +For example, to configure this value in the `.gitlab-ci.yml` file, use the following: + +```yaml +release: + variables: + ADDITIONAL_CA_CERT_BUNDLE: | + -----BEGIN CERTIFICATE----- + MIIGqTCCBJGgAwIBAgIQI7AVxxVwg2kch4d56XNdDjANBgkqhkiG9w0BAQsFADCB + ... + jWgmPqF3vUbZE0EyScetPJquRFRKIesyJuBFMAs= + -----END CERTIFICATE----- + script: + - echo "Create release" + release: + name: 'My awesome release' + tag_name: '$CI_COMMIT_TAG' +``` + +The `ADDITIONAL_CA_CERT_BUNDLE` value can also be configured as a +[custom variable in the UI](../../../ci/variables/index.md#custom-cicd-variables), +either as a `file`, which requires the path to the certificate, or as a variable, +which requires the text representation of the certificate. + +### `release-cli` command line + +The entries under the `release` node are transformed into a `bash` command line and sent +to the Docker container, which contains the [release-cli](https://gitlab.com/gitlab-org/release-cli). +You can also call the `release-cli` directly from a `script` entry. + +For example, if you use the YAML described previously: + +```shell +release-cli create --name "Release $CI_COMMIT_SHA" --description "Created using the release-cli $EXTRA_DESCRIPTION" --tag-name "v${MAJOR}.${MINOR}.${REVISION}" --ref "$CI_COMMIT_SHA" --released-at "2020-07-15T08:00:00Z" --milestone "m1" --milestone "m2" --milestone "m3" --assets-link "{\"name\":\"asset1\",\"url\":\"https://example.com/assets/1\",\"link_type\":\"other\"} +``` + +### Create multiple releases in a single pipeline + +A pipeline can have multiple `release` jobs, for example: + +```yaml +ios-release: + script: + - echo 'iOS release job' + release: + tag_name: v1.0.0-ios + description: 'iOS release v1.0.0' + +android-release: + script: + - echo 'Android release job' + release: + tag_name: v1.0.0-android + description: 'Android release v1.0.0' +``` + +### Release assets as Generic packages + +You can use [Generic packages](../../packages/generic_packages/index.md) to host your release assets. +For a complete example, see the [Release assets as Generic packages](https://gitlab.com/gitlab-org/release-cli/-/tree/master/docs/examples/release-assets-as-generic-package/) +project. + +## Upcoming releases > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/38105) in GitLab 12.1. diff --git a/doc/user/project/releases/release_cli.md b/doc/user/project/releases/release_cli.md new file mode 100644 index 00000000000..c23ceaa1aba --- /dev/null +++ b/doc/user/project/releases/release_cli.md @@ -0,0 +1,76 @@ +--- +type: reference, howto +stage: Release +group: Release +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Install the `release-cli` for the Shell executor + +> - [Introduced](https://gitlab.com/gitlab-org/release-cli/-/issues/21) in GitLab 13.8. +> - [Changed](https://gitlab.com/gitlab-org/release-cli/-/merge_requests/108) in GitLab 14.2, the `release-cli` binaries are also [available in the Package Registry](https://gitlab.com/jaime/release-cli/-/packages). + +When you use a runner with the Shell executor, you can download and install +the `release-cli` manually for your [supported OS and architecture](https://release-cli-downloads.s3.amazonaws.com/latest/index.html). +Once installed, [the `release` keyword](../../../ci/yaml/index.md#release) is available to use in your CI/CD jobs. + +## Install on Unix/Linux + +1. Download the binary for your system from S3, in the following example for amd64 systems: + + ```shell + curl --location --output /usr/local/bin/release-cli "https://release-cli-downloads.s3.amazonaws.com/latest/release-cli-linux-amd64" + ``` + + Or from the GitLab Package Registry: + + ```shell + curl --location --output /usr/local/bin/release-cli "https://gitlab.com/api/v4/projects/gitlab-org%2Frelease-cli/packages/generic/release-cli/latest/release-cli-darwin-amd64" + ``` + +1. Give it permissions to execute: + + ```shell + sudo chmod +x /usr/local/bin/release-cli + ``` + +1. Verify `release-cli` is available: + + ```shell + $ release-cli -v + + release-cli version 0.6.0 + ``` + +## Install on Windows PowerShell + +1. Create a folder somewhere in your system, for example `C:\GitLab\Release-CLI\bin` + + ```shell + New-Item -Path 'C:\GitLab\Release-CLI\bin' -ItemType Directory + ``` + +1. Download the executable file: + + ```shell + PS C:\> Invoke-WebRequest -Uri "https://release-cli-downloads.s3.amazonaws.com/latest/release-cli-windows-amd64.exe" -OutFile "C:\GitLab\Release-CLI\bin\release-cli.exe" + + Directory: C:\GitLab\Release-CLI + Mode LastWriteTime Length Name + ---- ------------- ------ ---- + d----- 3/16/2021 4:17 AM bin + ``` + +1. Add the directory to your `$env:PATH`: + + ```shell + $env:PATH += ";C:\GitLab\Release-CLI\bin" + ``` + +1. Verify `release-cli` is available: + + ```shell + PS C:\> release-cli -v + + release-cli version 0.6.0 + ``` diff --git a/lib/api/error_tracking/collector.rb b/lib/api/error_tracking/collector.rb index 142de3d35b0..13fda356257 100644 --- a/lib/api/error_tracking/collector.rb +++ b/lib/api/error_tracking/collector.rb @@ -12,6 +12,10 @@ module API content_type :txt, 'text/plain' default_format :envelope + rescue_from ActiveRecord::RecordInvalid do |e| + render_api_error!(e.message, 400) + end + before do not_found!('Project') unless project not_found! unless feature_enabled? @@ -50,6 +54,12 @@ module API bad_request!('Failed to parse sentry request') end end + + def validate_payload(payload) + unless ::ErrorTracking::Collector::PayloadValidator.new.valid?(payload) + bad_request!('Unsupported sentry payload') + end + end end desc 'Submit error tracking event to the project as envelope' do @@ -88,6 +98,8 @@ module API # We don't have use for transaction request yet, # so we record only event one. if type == 'event' + validate_payload(parsed_request[:event]) + ::ErrorTracking::CollectErrorService .new(project, nil, event: parsed_request[:event]) .execute @@ -125,6 +137,8 @@ module API bad_request!('Failed to parse sentry request') end + validate_payload(parsed_body) + ::ErrorTracking::CollectErrorService .new(project, nil, event: parsed_body) .execute diff --git a/lib/error_tracking/collector/payload_validator.rb b/lib/error_tracking/collector/payload_validator.rb new file mode 100644 index 00000000000..aae19a3635a --- /dev/null +++ b/lib/error_tracking/collector/payload_validator.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module ErrorTracking + module Collector + class PayloadValidator + PAYLOAD_SCHEMA_PATH = Rails.root.join('app', 'validators', 'json_schemas', 'error_tracking_event_payload.json').to_s + + def valid?(payload) + JSONSchemer.schema(Pathname.new(PAYLOAD_SCHEMA_PATH)).valid?(payload) + end + end + end +end diff --git a/lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed.rb b/lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed.rb new file mode 100644 index 00000000000..b39c0953fb1 --- /dev/null +++ b/lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Add user primary email to emails table if confirmed + class AddPrimaryEmailToEmailsIfUserConfirmed + INNER_BATCH_SIZE = 1_000 + + # Stubbed class to access the User table + class User < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'users' + self.inheritance_column = :_type_disabled + + scope :confirmed, -> { where.not(confirmed_at: nil) } + + has_many :emails + end + + # Stubbed class to access the Emails table + class Email < ActiveRecord::Base + self.table_name = 'emails' + self.inheritance_column = :_type_disabled + + belongs_to :user + end + + def perform(start_id, end_id) + User.confirmed.where(id: start_id..end_id).select(:id, :email, :confirmed_at).each_batch(of: INNER_BATCH_SIZE) do |users| + current_time = Time.now.utc + + attributes = users.map do |user| + { + user_id: user.id, + email: user.email, + confirmed_at: user.confirmed_at, + created_at: current_time, + updated_at: current_time + } + end + + Email.insert_all(attributes) + end + mark_job_as_succeeded(start_id, end_id) + end + + private + + def mark_job_as_succeeded(*arguments) + Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( + 'AddPrimaryEmailToEmailsIfUserConfirmed', + arguments + ) + end + end + end +end diff --git a/lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb b/lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb new file mode 100644 index 00000000000..1c04b5b025e --- /dev/null +++ b/lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Background migration for fixing merge_request_diff_commit rows that don't + # have committer/author details due to + # https://gitlab.com/gitlab-org/gitlab/-/issues/344080. + # + # This migration acts on a single project and corrects its data. Because + # this process needs Git/Gitaly access, and duplicating all that code is far + # too much, this migration relies on global models such as Project, + # MergeRequest, etc. + class FixMergeRequestDiffCommitUsers + def initialize + @commits = {} + @users = {} + end + + def perform(project_id) + if (project = ::Project.find_by_id(project_id)) + process(project) + end + + ::Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( + 'FixMergeRequestDiffCommitUsers', + [project_id] + ) + + schedule_next_job + end + + def process(project) + # Loading everything using one big query may result in timeouts (e.g. + # for projects the size of gitlab-org/gitlab). So instead we query + # data on a per merge request basis. + project.merge_requests.each_batch do |mrs| + ::MergeRequestDiffCommit + .select([ + :merge_request_diff_id, + :relative_order, + :sha, + :committer_id, + :commit_author_id + ]) + .joins(merge_request_diff: :merge_request) + .where(merge_requests: { id: mrs.select(:id) }) + .where('commit_author_id IS NULL OR committer_id IS NULL') + .each do |commit| + update_commit(project, commit) + end + end + end + + # rubocop: disable Metrics/AbcSize + def update_commit(project, row) + commit = find_commit(project, row.sha) + updates = [] + + unless row.commit_author_id + author_id = find_or_create_user(commit, :author_name, :author_email) + + updates << [arel_table[:commit_author_id], author_id] if author_id + end + + unless row.committer_id + committer_id = + find_or_create_user(commit, :committer_name, :committer_email) + + updates << [arel_table[:committer_id], committer_id] if committer_id + end + + return if updates.empty? + + update = Arel::UpdateManager + .new + .table(MergeRequestDiffCommit.arel_table) + .where(matches_row(row)) + .set(updates) + .to_sql + + MergeRequestDiffCommit.connection.execute(update) + end + # rubocop: enable Metrics/AbcSize + + def schedule_next_job + job = Database::BackgroundMigrationJob + .for_migration_class('FixMergeRequestDiffCommitUsers') + .pending + .first + + return unless job + + BackgroundMigrationWorker.perform_in( + 2.minutes, + 'FixMergeRequestDiffCommitUsers', + job.arguments + ) + end + + def find_commit(project, sha) + @commits[sha] ||= (project.commit(sha)&.to_hash || {}) + end + + def find_or_create_user(commit, name_field, email_field) + name = commit[name_field] + email = commit[email_field] + + return unless name && email + + @users[[name, email]] ||= + MergeRequest::DiffCommitUser.find_or_create(name, email).id + end + + def matches_row(row) + primary_key = Arel::Nodes::Grouping + .new([arel_table[:merge_request_diff_id], arel_table[:relative_order]]) + + primary_val = Arel::Nodes::Grouping + .new([row.merge_request_diff_id, row.relative_order]) + + primary_key.eq(primary_val) + end + + def arel_table + MergeRequestDiffCommit.arel_table + end + end + end +end diff --git a/lib/gitlab/database/background_migration_job.rb b/lib/gitlab/database/background_migration_job.rb index 1121793917b..c046571a111 100644 --- a/lib/gitlab/database/background_migration_job.rb +++ b/lib/gitlab/database/background_migration_job.rb @@ -4,6 +4,7 @@ module Gitlab module Database class BackgroundMigrationJob < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord include EachBatch + include BulkInsertSafe self.table_name = :background_migration_jobs diff --git a/lib/gitlab/import_export/base/object_builder.rb b/lib/gitlab/import_export/base/object_builder.rb index 48836729ff6..5e9c8292c1e 100644 --- a/lib/gitlab/import_export/base/object_builder.rb +++ b/lib/gitlab/import_export/base/object_builder.rb @@ -47,16 +47,16 @@ module Gitlab attributes end + def find_with_cache(key = cache_key) + return yield unless lru_cache && key + + lru_cache[key] ||= yield + end + private attr_reader :klass, :attributes, :lru_cache, :cache_key - def find_with_cache - return yield unless lru_cache && cache_key - - lru_cache[cache_key] ||= yield - end - def cache_from_request_store Gitlab::SafeRequestStore[:lru_cache] ||= LruRedux::Cache.new(LRU_CACHE_SIZE) end diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 71a8d747963..bbcf802daf6 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -401,10 +401,6 @@ excluded_attributes: - :verification_checksum - :verification_failure merge_request_diff_commits: - - :author_name - - :author_email - - :committer_name - - :committer_email - :merge_request_diff_id - :commit_author_id - :committer_id diff --git a/lib/gitlab/import_export/project/object_builder.rb b/lib/gitlab/import_export/project/object_builder.rb index b03dceba303..f7598ba1337 100644 --- a/lib/gitlab/import_export/project/object_builder.rb +++ b/lib/gitlab/import_export/project/object_builder.rb @@ -29,6 +29,7 @@ module Gitlab def find return if epic? && group.nil? return find_diff_commit_user if diff_commit_user? + return find_diff_commit if diff_commit? super end @@ -83,9 +84,38 @@ module Gitlab end def find_diff_commit_user - find_with_cache do - MergeRequest::DiffCommitUser - .find_or_create(@attributes['name'], @attributes['email']) + find_or_create_diff_commit_user(@attributes['name'], @attributes['email']) + end + + def find_diff_commit + row = @attributes.dup + + # Diff commits come in two formats: + # + # 1. The old format where author/committer details are separate fields + # 2. The new format where author/committer details are nested objects, + # and pre-processed by `find_diff_commit_user`. + # + # The code here ensures we support both the old and new format. + aname = row.delete('author_name') + amail = row.delete('author_email') + cname = row.delete('committer_name') + cmail = row.delete('committer_email') + author = row.delete('commit_author') + committer = row.delete('committer') + + row['commit_author'] = author || + find_or_create_diff_commit_user(aname, amail) + + row['committer'] = committer || + find_or_create_diff_commit_user(cname, cmail) + + MergeRequestDiffCommit.new(row) + end + + def find_or_create_diff_commit_user(name, email) + find_with_cache([MergeRequest::DiffCommitUser, name, email]) do + MergeRequest::DiffCommitUser.find_or_create(name, email) end end @@ -113,6 +143,10 @@ module Gitlab klass == MergeRequest::DiffCommitUser end + def diff_commit? + klass == MergeRequestDiffCommit + end + # If an existing group milestone used the IID # claim the IID back and set the group milestone to use one available # This is necessary to fix situations like the following: diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb index 888a5a10f2c..d84db92fe69 100644 --- a/lib/gitlab/import_export/project/relation_factory.rb +++ b/lib/gitlab/import_export/project/relation_factory.rb @@ -33,7 +33,8 @@ module Gitlab links: 'Releases::Link', metrics_setting: 'ProjectMetricsSetting', commit_author: 'MergeRequest::DiffCommitUser', - committer: 'MergeRequest::DiffCommitUser' }.freeze + committer: 'MergeRequest::DiffCommitUser', + merge_request_diff_commits: 'MergeRequestDiffCommit' }.freeze BUILD_MODELS = %i[Ci::Build commit_status].freeze @@ -59,6 +60,7 @@ module Gitlab external_pull_requests DesignManagement::Design MergeRequest::DiffCommitUser + MergeRequestDiffCommit ].freeze def create diff --git a/lib/gitlab/spamcheck/client.rb b/lib/gitlab/spamcheck/client.rb index b8c07c0c316..925ca44dfc9 100644 --- a/lib/gitlab/spamcheck/client.rb +++ b/lib/gitlab/spamcheck/client.rb @@ -69,6 +69,8 @@ module Gitlab user_pb.emails << build_email(user.email, user.confirmed?) user.emails.each do |email| + next if email.user_primary_email? + user_pb.emails << build_email(email.email, email.confirmed?) end diff --git a/qa/qa.rb b/qa/qa.rb index cc83efb90e8..1cdf6bc89ca 100644 --- a/qa/qa.rb +++ b/qa/qa.rb @@ -50,7 +50,8 @@ module QA "user_gpg" => "UserGPG", "smtp" => "SMTP", "otp" => "OTP", - "jira_api" => "JiraAPI" + "jira_api" => "JiraAPI", + "registry_tls" => "RegistryTLS" ) loader.setup diff --git a/qa/qa/scenario/test/integration/registry_tls.rb b/qa/qa/scenario/test/integration/registry_tls.rb new file mode 100644 index 00000000000..4e9d6b6ea97 --- /dev/null +++ b/qa/qa/scenario/test/integration/registry_tls.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module QA + module Scenario + module Test + module Integration + class RegistryTLS < Test::Instance::All + tags :registry_tls + end + end + end + end +end diff --git a/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_omnibus_spec.rb b/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_omnibus_spec.rb index 3d02c2884a2..125867bc694 100644 --- a/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_omnibus_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_omnibus_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :registry, :orchestrated, only: { pipeline: :main } do + RSpec.describe 'Package', :orchestrated do describe 'Self-managed Container Registry' do let(:project) do Resource::Project.fabricate_via_api! do |project| @@ -28,59 +28,112 @@ module QA runner.remove_via_api! end - it "pushes image and deletes tag", testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1911' do - Resource::Repository::Commit.fabricate_via_api! do |commit| - commit.project = project - commit.commit_message = 'Add .gitlab-ci.yml' - commit.add_files([{ - file_path: '.gitlab-ci.yml', - content: - <<~YAML - build: - image: docker:19.03.12 - stage: build - services: - - name: docker:19.03.12-dind - command: - - /bin/sh - - -c - - | - apk add --no-cache openssl - true | openssl s_client -showcerts -connect gitlab.test:5050 > /usr/local/share/ca-certificates/gitlab.test.crt - update-ca-certificates - dockerd-entrypoint.sh || exit - variables: - IMAGE_TAG: $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_SLUG - script: - - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD gitlab.test:5050 - - docker build -t $IMAGE_TAG . - - docker push $IMAGE_TAG - tags: - - "runner-for-#{project.name}" - YAML - }]) + context 'when tls is enabled' do + it "pushes image and deletes tag", :registry_tls, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1911' do + Resource::Repository::Commit.fabricate_via_api! do |commit| + commit.project = project + commit.commit_message = 'Add .gitlab-ci.yml' + commit.add_files([{ + file_path: '.gitlab-ci.yml', + content: + <<~YAML + build: + image: docker:19.03.12 + stage: build + services: + - name: docker:19.03.12-dind + command: + - /bin/sh + - -c + - | + apk add --no-cache openssl + true | openssl s_client -showcerts -connect gitlab.test:5050 > /usr/local/share/ca-certificates/gitlab.test.crt + update-ca-certificates + dockerd-entrypoint.sh || exit + variables: + IMAGE_TAG: $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_SLUG + script: + - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD gitlab.test:5050 + - docker build -t $IMAGE_TAG . + - docker push $IMAGE_TAG + tags: + - "runner-for-#{project.name}" + YAML + }]) + end + + Flow::Pipeline.visit_latest_pipeline + + Page::Project::Pipeline::Show.perform do |pipeline| + pipeline.click_job('build') + end + + Page::Project::Job::Show.perform do |job| + expect(job).to be_successful(timeout: 800) + end + + Page::Project::Menu.perform(&:go_to_container_registry) + + Page::Project::Registry::Show.perform do |registry| + expect(registry).to have_registry_repository(project.path_with_namespace) + + registry.click_on_image(project.path_with_namespace) + expect(registry).to have_tag('master') + + registry.click_delete + expect(registry).not_to have_tag('master') + end end + end - Flow::Pipeline.visit_latest_pipeline + context "when tls is disabled" do + it "pushes image and deletes tag", :registry, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/2378' do + Resource::Repository::Commit.fabricate_via_api! do |commit| + commit.project = project + commit.commit_message = 'Add .gitlab-ci.yml' + commit.add_files([{ + file_path: '.gitlab-ci.yml', + content: + <<~YAML + build: + image: docker:19.03.12 + stage: build + services: + - name: docker:19.03.12-dind + command: ["--insecure-registry=gitlab.test:5050"] + variables: + IMAGE_TAG: $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_SLUG + script: + - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD gitlab.test:5050 + - docker build -t $IMAGE_TAG . + - docker push $IMAGE_TAG + tags: + - "runner-for-#{project.name}" + YAML + }]) + end - Page::Project::Pipeline::Show.perform do |pipeline| - pipeline.click_job('build') - end + Flow::Pipeline.visit_latest_pipeline - Page::Project::Job::Show.perform do |job| - expect(job).to be_successful(timeout: 800) - end + Page::Project::Pipeline::Show.perform do |pipeline| + pipeline.click_job('build') + end - Page::Project::Menu.perform(&:go_to_container_registry) + Page::Project::Job::Show.perform do |job| + expect(job).to be_successful(timeout: 800) + end - Page::Project::Registry::Show.perform do |registry| - expect(registry).to have_registry_repository(project.path_with_namespace) + Page::Project::Menu.perform(&:go_to_container_registry) - registry.click_on_image(project.path_with_namespace) - expect(registry).to have_tag('master') + Page::Project::Registry::Show.perform do |registry| + expect(registry).to have_registry_repository(project.path_with_namespace) - registry.click_delete - expect(registry).not_to have_tag('master') + registry.click_on_image(project.path_with_namespace) + expect(registry).to have_tag('master') + + registry.click_delete + expect(registry).not_to have_tag('master') + end end end end diff --git a/qa/qa/specs/features/browser_ui/5_package/dependency_proxy/dependency_proxy_spec.rb b/qa/qa/specs/features/browser_ui/5_package/dependency_proxy/dependency_proxy_spec.rb index ffdd17e5c7c..41e5a99657c 100644 --- a/qa/qa/specs/features/browser_ui/5_package/dependency_proxy/dependency_proxy_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/dependency_proxy/dependency_proxy_spec.rb @@ -57,17 +57,11 @@ module QA image: "#{docker_client_version}" services: - name: "#{docker_client_version}-dind" - command: - - /bin/sh - - -c - - | - apk add --no-cache openssl - true | openssl s_client -showcerts -connect gitlab.test:5050 > /usr/local/share/ca-certificates/gitlab.test.crt - update-ca-certificates - dockerd-entrypoint.sh || exit + command: ["--insecure-registry=gitlab.test:80"] before_script: - apk add curl jq grep - - docker login -u "$CI_DEPENDENCY_PROXY_USER" -p "$CI_DEPENDENCY_PROXY_PASSWORD" "$CI_DEPENDENCY_PROXY_SERVER" + - echo $CI_DEPENDENCY_PROXY_SERVER + - docker login -u "$CI_DEPENDENCY_PROXY_USER" -p "$CI_DEPENDENCY_PROXY_PASSWORD" gitlab.test:80 script: - docker pull #{dependency_proxy_url}/#{image_sha} - TOKEN=$(curl "https://auth.docker.io/token?service=registry.docker.io&scope=repository:ratelimitpreview/test:pull" | jq --raw-output .token) diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 0e25f6a96d7..98cc8d83e0c 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -3,8 +3,7 @@ require 'spec_helper' RSpec.describe Oauth::AuthorizationsController do - let(:user) { create(:user, confirmed_at: confirmed_at) } - let(:confirmed_at) { 1.hour.ago } + let(:user) { create(:user) } let!(:application) { create(:oauth_application, scopes: 'api read_user', redirect_uri: 'http://example.com') } let(:params) do { @@ -40,7 +39,7 @@ RSpec.describe Oauth::AuthorizationsController do end context 'when the user is unconfirmed' do - let(:confirmed_at) { nil } + let(:user) { create(:user, :unconfirmed) } it 'returns 200 and renders error view' do subject @@ -73,8 +72,6 @@ RSpec.describe Oauth::AuthorizationsController do include_examples "Implicit grant can't be used in confidential application" context 'when the user is confirmed' do - let(:confirmed_at) { 1.hour.ago } - context 'when there is already an access token for the application with a matching scope' do before do scopes = Doorkeeper::OAuth::Scopes.from_string('api') diff --git a/spec/features/profiles/emails_spec.rb b/spec/features/profiles/emails_spec.rb index 6b6f628e2d5..8f05de60be9 100644 --- a/spec/features/profiles/emails_spec.rb +++ b/spec/features/profiles/emails_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe 'Profile > Emails' do let(:user) { create(:user) } + let(:other_user) { create(:user) } before do sign_in(user) @@ -23,13 +24,23 @@ RSpec.describe 'Profile > Emails' do expect(page).to have_content('Resend confirmation email') end - it 'does not add a duplicate email' do + it 'does not add an email that is the primary email of another user' do + fill_in('Email', with: other_user.email) + click_button('Add email address') + + email = user.emails.find_by(email: other_user.email) + expect(email).to be_nil + expect(page).to have_content('Email has already been taken') + end + + it 'adds an email that is the primary email of the same user' do fill_in('Email', with: user.email) click_button('Add email address') email = user.emails.find_by(email: user.email) - expect(email).to be_nil - expect(page).to have_content('Email has already been taken') + expect(email).to be_present + expect(page).to have_content("#{user.email} Verified") + expect(page).not_to have_content("#{user.email} Unverified") end it 'does not add an invalid email' do diff --git a/spec/features/profiles/user_manages_emails_spec.rb b/spec/features/profiles/user_manages_emails_spec.rb index 373c4f565f2..b037d5048aa 100644 --- a/spec/features/profiles/user_manages_emails_spec.rb +++ b/spec/features/profiles/user_manages_emails_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe 'User manages emails' do let(:user) { create(:user) } + let(:other_user) { create(:user) } before do sign_in(user) @@ -11,7 +12,7 @@ RSpec.describe 'User manages emails' do visit(profile_emails_path) end - it "shows user's emails" do + it "shows user's emails", :aggregate_failures do expect(page).to have_content(user.email) user.emails.each do |email| @@ -19,7 +20,7 @@ RSpec.describe 'User manages emails' do end end - it 'adds an email' do + it 'adds an email', :aggregate_failures do fill_in('email_email', with: 'my@email.com') click_button('Add') @@ -34,21 +35,21 @@ RSpec.describe 'User manages emails' do end end - it 'does not add a duplicate email' do - fill_in('email_email', with: user.email) + it 'does not add an email that is the primary email of another user', :aggregate_failures do + fill_in('email_email', with: other_user.email) click_button('Add') - email = user.emails.find_by(email: user.email) + email = user.emails.find_by(email: other_user.email) expect(email).to be_nil - expect(page).to have_content(user.email) + expect(page).to have_content('Email has already been taken') user.emails.each do |email| expect(page).to have_content(email.email) end end - it 'removes an email' do + it 'removes an email', :aggregate_failures do fill_in('email_email', with: 'my@email.com') click_button('Add') diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb index d679e4dbb99..70b53f23508 100644 --- a/spec/features/signed_commits_spec.rb +++ b/spec/features/signed_commits_spec.rb @@ -11,6 +11,7 @@ RSpec.describe 'GPG signed commits' do perform_enqueued_jobs do create :gpg_key, key: GpgHelpers::User1.public_key, user: user + user.reload # necessary to reload the association with gpg_keys end visit project_commit_path(project, ref) diff --git a/spec/fixtures/gitlab/import_export/lightweight_project_export.tar.gz b/spec/fixtures/gitlab/import_export/lightweight_project_export.tar.gz index e3ec4f603b9d05d4872dd5613aafd07457471f7b..e5f6f195fe5ae3f01300e0436a775ad490dd9d3a 100644 GIT binary patch literal 3758 zcmV;f4pH$RiwFn-v3g+u18iw$XmodFX=iA3UvP47YGq?|UuAf3Z*p`lbYXG;?HX&0 z9aVXOw7?cyG*E;nOs8o=mwn9hzD%?&T?&*wEYJ#U9nZ|1x%YJE&J6R|2X#^SAqp6S zKO~SE@r5=71r0F>K~NHCNF``62+`7n1PK~bK%v3ccji6!-rer4yKM;fB%8Z)&UeoF z&iDSlbB0Gu!yB^IJy`R-;XR&h^(}bgIIbi~4E-&a>S;3^%dK; zF5(`?)_=K0|An$x?&<%DXxy+cES+p5dhMjO;StBRsq-IJ14}pYV(f7Y{pUgRTm8Q* zm3sPrBHC(ZLSX8|XCn6ZOI2AAd5PC`O|FXNQpG4C&JZw%%Q}xMWmQBX;t^t8Y>{f3CkNYD(eiglh>QN`f3iX_V#DpfQUn5versEL)r)afDBMV8j+a^rD~ zogw7ffb__GXLgBTevA3eoc}2t(zkmDsOE&`(PKCdv`<74b`}Q51`+v=y)Aw$5 z^q(81N4|6al8-LAW7&&lBB{p5w8d1Zxm&$GK$9sJbS{&4*pzVLX?(BDdcA2_i#i94#SaIpo%TKQyymx8e zi)%h|&tm|8_0DGxT=o3LYqu@A>&TMPqu1>}u;$4AbC(<)T{AXz;r?p%X#e4(qt&&y zkM-%dtoT;P9i|?xKl1j@r_LOy4_x#7KXmE$%MNefx1#Trx6dL^oHF+geGk34Y}+^X z{7U}wQ!g(+@3qrk{+x2~wl@!yzk9OPK5d!(U!2^JHZF1i^p`KoT>h7O0$JjOKF}Gl zB#WHH@qI8V@h95;Punih#vKfn%NmtFGwS#1w-VGbd0@qAB4NFIm6@PYU}vwnu#1omR94*2WbBkr2BG1I@O4 zL;!D!81Z~}x)uMB5)q0KnTVk{#K9)aQoKOZFs56^h%^uoU<$Rv2RXs+1ko_?&Kqx& zt%#utc3s;|uz)jAAce|~OTv{S5vF410M828C9~p_Dj+pFNU08_0D;cYBi(b4t$Khu z$b~{IJrm26{&2PiSgM;4)n`Qmf%1q!wCK}BDphBycgQ%2)rM`4 zE51FBty$DM+AY=6S)dZ4Xm%4AMh$4&3<^tt=MLaIX}h3Ac0#xeqzPY&X9q5DB#Igp zWcUeDGaZ6M0>y6F%MxIgAXa#JkoAdg0tmZ3@^;KsAt|UfnUs6%YPlkb>o-!$L=Xy{ zToM4n#UW1+u!|KqzBIm#K}=&W@JJDm`N z=-Z!ma>ysRgUAD`eL{uyhTU}KDgV;sJeP%yoq_%)m=DjlL*b68Qh^XBBg&QpfZ;n3 zpy(iSu*}d9voSXapkO@ooSmJEc2bQi&4+e*IPleUaTaMlI3wsF6iGfDaQ(v8tt13b zX89Qb$|X@Mx1Ti@=y+9Fw2q^}h+qqSo_K?py(wwgcJ%3qDgF7G-IS$~UrSHJOVtXO z@pCJlWUinPAwP>}>Q1~RX@26J!4;(?he^A%B+N^eR;P0Zp2VcyfW73<;~7K;_7Q3@ z60KC*&z?ZHntF^n=m6+x7g(6dAo=E`DO5=234vH#$H zz;XF@x;%B(SSJZp+ItHJ%FMY!5z9Gr)AkBW#2tm%q2wn#)C0$)Hof)0z}u!B_hJX4 zizS<%y>pJr=?QZD_NG1XUx}*)NvDxYGNGd$XNna^e=eyL8eg;yB;j685z?;N6p41S zYDq-AQL5_LK%y+lLP_L#Q8zeItzdx@s(eY-MVS&m5`fvahJB(WBk7+Yc(Tn8$3hq~ zl9JUzP0Hc*dXrc*AV(0QvNJgq3r%(ymxS?X&Tf~>QZ*UMM=NcL)>$zYT72RF@k16l z%~1AG83-fGh#4*BQ4lKB|0jlrMLM;iQMQ!hQZe_)d7@SZ2jE${wE-Vu=0t?1xlG-{ zNw|BHZ8stG$29AUyI@a%D10OVK&pqHMH~nFF%Sd*7lI8Vu_g#jS(}KM zEq#SFFYcq~b^I2S9Ljc~4^KIeyLtH_NJma82yrf)Nd~1xBF8|G7#q+ju%GMj6aqoQ z!{Cl$!o}v*wQZjbrJLDAOJNik)FE7Bc`du3` zh1oH>e&d$W;ii5J(=67+>U{L8LbYEKs`=C>4d5aw(C&a1Ei_~)YFt3mvq-A-NJ-4y zR`u)XyrVq|6^^LOK`v@~l!=B-Dc31Ehjn@C2BR614Q$O(g^GYuJ<0DC2PR$vrLK3b zvSC31-eD&@-|MVat?7PxK^x=g^SvVrW!!g(<|W1=((raC8}(D~107KjHY^CV zFeNu^&`7OO7&nMZ<5@`Z?CB_l0(sGc(BtR8A_dE5YYU+0dAJVZ5${e3M z>j|lNKy1^opZKj(+}0b)5-q=Gi8pi+*ofA6y~0p5*@Hy_%TF_p4k?0nL~N~t1j1UXm~Es$#Ryn_2E_~6RNtp#iaYgNQ?s-bgLRphH2(n_iz z2r{p$Dj4{xj5xVm)fG_@*~QkEy=)j5bnSwo%5vA!j?dfLrXF9FvH4ii0=4PTog6L^ z30nV;7oluO$q2KfZ*iuh#psbHBF%BzOx{3}RFPFqmpNI}B~BH=e5xf`<7D`ibZlsX z!RwuDW}f(IOy?HDw~S!1(N9T{og)9;_2FaP?b2|ECMq^~-4w&xEnsU-7b|r>=u6Tc z%{EQ2M7a#8JrG>zh^#H6=tkNB7c&luSU!>d!eZS-UNjtO4 zc1?ZNn8nWQ?ijqvTo=LX;@Gv8+vj3H*~Oh(@n&O=T6$>dHIjd9L&4K0Cl2hCuj|pB zAY7?OrP$rM+}P6~c@NB7-PRAiLR@Lc<(JhCbdEJTEe+0H3!h(3kcXr3uVPQqdQDQQ zQsNgbtq2YJ!#|YH40RvXTW$pm zKs9i25}1@ayryVgGfmvu8-(Yhby}_QHq^)4N zS9k4OMbF5J{X7Q-58`9rZo=B2z-r*<6t#){1`cQuHm>dENvtBhp1{)AgAn&(Kfd7E z8*{Vct4-KawUMj083b%i5ZAVvS@c=3J~RC%`vq>V^;)m>TCeq5uk~85^;)m>TCeq5 Yuk~85^;)m>+TyhT0Xomkf&fqe0BSF4?EnA( literal 3647 zcmV-F4#4priwFQ1g9Kjy1MM4Yj2%^ZfwaIDTQpFFC`_kmLYIBa^FG!@+tRf_>B9o8 zz~Xpj=FGjPJ9lQ7$3Ccw!Vgiv82llD)QB&%At-2wK?s79Ktn1)gF%RvCL~DEm;wq7 zzP>Z>nR|D8Z_{l<*h!kZbIy0p`QEQN9UgI9dpFkn;oY8X^)1N5aa=``82VeS$l)(1 zMDO%3nU^Jo7pol4bG!%u94`rCwU6Nz2A$#)cs_Cg73BJXYtIb_V1_ZzJ`piY`uhQS z_OLqkG?zF&u`PC#We3?w;t`dY#Gg@2Jc&(qlpAD`t2N0a*4a@5nI0ZwTi9)2#l<%U z*z-Nb$1TT1K4wR)z%<*YU92JxE7(G+ncD<8pna3@x1wUhcCivJ3lDU$hkYdjur0+2 zye2#9yFvT$h5-QPOxpp(^8la?JFkgctSfF{S=jZmRngvRKulfH>`CmR@Q?NmtD)%F#1w-VGbd0@qAB4LFWJCEo)70a zwnu#1ol&$V)+Q7Kkr2BG1I@O4L;!D!81Z~}ri_2ci3r7rO~gN zfY#`sr8=+z6gtO`^vvD1>H+GY7YecTOf6IX18q&)B{v~z&x!^LT56?pKqXAk>=p>jr>rQips)mF?f|}1 zwhKmNCydKLTJWZLcHn|UR2SseRFL5(OwDWvhy;#ZzlS9tEJ3XC@*wLI-vkhLdlc=M zD??ULYci?y*wu1P64!5}oQWV5y0{_$go{I&AP^TTaD07;=Z6IOYQ8=y38SJs4DS9q zSQj{YS8$mxO7$q`fFoKPt{pfb2GP4erA9ujBhgo1M$l96jFF*Cs#T zhh(7zqGm_;uz?!~+*#05nha{Tp84u9I_J2YQjn9kH{(J4Nm$&WVbQWPFm~skS&AD`6-|%2o@tsD#(+En?Ax96^f8&gM)kblG8E66T{hzg?|L z^<*lat+W|hXTvz&>Jul3AF|A8mWqe!K$uxZ!YH{%L8#IApEw>C>C}ft)lyDM#o8km ziOK>FAhPu813tvUi5N|5nTCavboVCPZb9mgY1J1G!I=P2cu5j~)DAt1I1ct>AV>f( z@{%nRCbe)&g=LwYigm6^#UIwcaaTm5{xH&+gsJgskuJ6bUOoxZk&`MyTnlHK zLAjCWF)$>?25bua=Q=!%K#}k;grk^ov3qrG+h;>L{c75SFzdAA{pk z*<=$lL-F6~-2B;3qJDV=j?M3yP$;7 zrkdEArwbJUrFN1(D^5(jCQ3u^d~L(9(b&F*Sd5!F;hl`Hi+Csk^4MXuIV39J-JT2OC|p59&ec_suX9MNsDdEKysoO?;OjEt^=cLlX>M@8mP{%ujPVw~)SN42zw9%8HDN{Cm&D$F|#} z;|^U^?C^SNhP8XZa!nT-bw24!${)=(P4Gmy3aC91Jl+{uTSd`}wgWC^oD{KqqWy)< zddbE^)i_pEO6eHiiR+HI9%n&~)FqiOyBC~GYxW%kyO!}E7`dE`TGVnD0@B-80)+Lu=D36(4wrlFE z#x3Ax$23vqdPox&#)Fi;^A=OL9=>iBzah-iwGMr%M)KEMD0q6g!-13X9W;6pBnFMB z>3Ku03iUK7lLIr?A@xK58`t!?O0M34&#~>KO}P2H&fR6-c{rMBD)toZj3iAVCB92i zzN(gQ-sbhAohBurroG>LyzwtJyo#zoWk}=j2Nd&ntMJ+5VG#*LCq}e}+d)!yB?ysuSJ<-2tEf{&)Ds@Se;gOwVJFdEX+tvj<w(x+lh>e z4R3@tGrf_hy6^9{L04FsT*B$`;0cVKAp}K=rH^#GvPT5-M|8Wgdnzs)TfectfBX7N zH-ivPg?~%;E}!1}ojW%7|AsrK@7?;?zc$Q_eE0q(A6;_CvKv=Vf93Ai9$9()Rq8wc zUitVfzqnxO{kn1Hs_D~KeyLU8eQ?uPU%hYrpT&XOUfuG*+HZYq8NPWXx$Xh!f;;(_ zc0Ts2HzscV0{_q*&wk_Wd!kpsv-*?aZ<(eI6=I_g&*}Ut(;brR{zwDBGmjRA-4(A2#~meee4<=ci{GU;N>d``^1#(l%`V z_=^2o?>wb(#qSP2i0=N=zW46hbdhuD)BV4FusZYZ$S0oje&Fr>K8;Ogi9sY`!Y zc4+(F6@9O~eHMA*l=)BSd+5z&+rGK`*YaPUdU^SIubuw#=amDuy}7^oy_0SBi_hY- R_$)qs{|By&xtst%002xb9bo_f diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index 480b1e2a0de..2b55319c70c 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -383,7 +383,7 @@ RSpec.describe UsersHelper do end context 'when `user.unconfirmed_email` is set' do - let(:user) { create(:user, unconfirmed_email: 'foo@bar.com') } + let(:user) { create(:user, :unconfirmed, unconfirmed_email: 'foo@bar.com') } it 'sets `modal_attributes.messageHtml` correctly' do expect(Gitlab::Json.parse(confirm_user_data[:modal_attributes])['messageHtml']).to eq('This user has an unconfirmed email address (foo@bar.com). You may force a confirmation.') diff --git a/spec/lib/error_tracking/collector/payload_validator_spec.rb b/spec/lib/error_tracking/collector/payload_validator_spec.rb new file mode 100644 index 00000000000..852cf9eac6c --- /dev/null +++ b/spec/lib/error_tracking/collector/payload_validator_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ErrorTracking::Collector::PayloadValidator do + describe '#valid?' do + RSpec.shared_examples 'valid payload' do + it 'returns true' do + expect(described_class.new.valid?(payload)).to be_truthy + end + end + + RSpec.shared_examples 'invalid payload' do + it 'returns false' do + expect(described_class.new.valid?(payload)).to be_falsey + end + end + + context 'ruby payload' do + let(:payload) { Gitlab::Json.parse(fixture_file('error_tracking/parsed_event.json')) } + + it_behaves_like 'valid payload' + end + + context 'python payload' do + let(:payload) { Gitlab::Json.parse(fixture_file('error_tracking/python_event.json')) } + + it_behaves_like 'valid payload' + end + + context 'browser payload' do + let(:payload) { Gitlab::Json.parse(fixture_file('error_tracking/browser_event.json')) } + + it_behaves_like 'valid payload' + end + + context 'empty payload' do + let(:payload) { '' } + + it_behaves_like 'invalid payload' + end + + context 'invalid payload' do + let(:payload) { { 'foo' => 'bar' } } + + it_behaves_like 'invalid payload' + end + end +end diff --git a/spec/lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed_spec.rb b/spec/lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed_spec.rb new file mode 100644 index 00000000000..b50a55a9e41 --- /dev/null +++ b/spec/lib/gitlab/background_migration/add_primary_email_to_emails_if_user_confirmed_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::AddPrimaryEmailToEmailsIfUserConfirmed do + let(:users) { table(:users) } + let(:emails) { table(:emails) } + + let!(:unconfirmed_user) { users.create!(name: 'unconfirmed', email: 'unconfirmed@example.com', confirmed_at: nil, projects_limit: 100) } + let!(:confirmed_user_1) { users.create!(name: 'confirmed-1', email: 'confirmed-1@example.com', confirmed_at: 1.day.ago, projects_limit: 100) } + let!(:confirmed_user_2) { users.create!(name: 'confirmed-2', email: 'confirmed-2@example.com', confirmed_at: 1.day.ago, projects_limit: 100) } + let!(:email) { emails.create!(user_id: confirmed_user_1.id, email: 'confirmed-1@example.com', confirmed_at: 1.day.ago) } + + let(:perform) { described_class.new.perform(users.first.id, users.last.id) } + + it 'adds the primary email of confirmed users to Emails, unless already added', :aggregate_failures do + expect(emails.where(email: [unconfirmed_user.email, confirmed_user_2.email])).to be_empty + + expect { perform }.not_to raise_error + + expect(emails.where(email: unconfirmed_user.email).count).to eq(0) + expect(emails.where(email: confirmed_user_1.email, user_id: confirmed_user_1.id).count).to eq(1) + expect(emails.where(email: confirmed_user_2.email, user_id: confirmed_user_2.id).count).to eq(1) + + email_2 = emails.find_by(email: confirmed_user_2.email, user_id: confirmed_user_2.id) + expect(email_2.confirmed_at).to eq(confirmed_user_2.reload.confirmed_at) + end + + it 'sets timestamps on the created Emails' do + perform + + email_2 = emails.find_by(email: confirmed_user_2.email, user_id: confirmed_user_2.id) + + expect(email_2.created_at).not_to be_nil + expect(email_2.updated_at).not_to be_nil + end + + context 'when a range of IDs is specified' do + let!(:confirmed_user_3) { users.create!(name: 'confirmed-3', email: 'confirmed-3@example.com', confirmed_at: 1.hour.ago, projects_limit: 100) } + let!(:confirmed_user_4) { users.create!(name: 'confirmed-4', email: 'confirmed-4@example.com', confirmed_at: 1.hour.ago, projects_limit: 100) } + + it 'only acts on the specified range of IDs', :aggregate_failures do + expect do + described_class.new.perform(confirmed_user_2.id, confirmed_user_3.id) + end.to change { Email.count }.by(2) + expect(emails.where(email: confirmed_user_4.email).count).to eq(0) + end + end +end diff --git a/spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb b/spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb new file mode 100644 index 00000000000..ffed0b517f9 --- /dev/null +++ b/spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb @@ -0,0 +1,286 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# The underlying migration relies on the global models (e.g. Project). This +# means we also need to use FactoryBot factories to ensure everything is +# operating using the same types. If we use `table()` and similar methods we +# would have to duplicate a lot of logic just for these tests. +# +# rubocop: disable RSpec/FactoriesInMigrationSpecs +RSpec.describe Gitlab::BackgroundMigration::FixMergeRequestDiffCommitUsers do + let(:migration) { described_class.new } + + describe '#perform' do + context 'when the project exists' do + it 'processes the project' do + project = create(:project) + + expect(migration).to receive(:process).with(project) + expect(migration).to receive(:schedule_next_job) + + migration.perform(project.id) + end + + it 'marks the background job as finished' do + project = create(:project) + + Gitlab::Database::BackgroundMigrationJob.create!( + class_name: 'FixMergeRequestDiffCommitUsers', + arguments: [project.id] + ) + + migration.perform(project.id) + + job = Gitlab::Database::BackgroundMigrationJob + .find_by(class_name: 'FixMergeRequestDiffCommitUsers') + + expect(job.status).to eq('succeeded') + end + end + + context 'when the project does not exist' do + it 'does nothing' do + expect(migration).not_to receive(:process) + expect(migration).to receive(:schedule_next_job) + + migration.perform(-1) + end + end + end + + describe '#update_commit' do + let(:project) { create(:project, :repository) } + let(:mr) do + create( + :merge_request_with_diffs, + source_project: project, + target_project: project + ) + end + + let(:diff) { mr.merge_request_diffs.first } + let(:commit) { project.commit } + + def update_row(migration, project, diff, row) + migration.update_commit(project, row) + + diff + .merge_request_diff_commits + .find_by(sha: row.sha, relative_order: row.relative_order) + end + + it 'populates missing commit authors' do + commit_row = create( + :merge_request_diff_commit, + merge_request_diff: diff, + sha: commit.sha, + relative_order: 9000 + ) + + updated = update_row(migration, project, diff, commit_row) + + expect(updated.commit_author.name).to eq(commit.to_hash[:author_name]) + expect(updated.commit_author.email).to eq(commit.to_hash[:author_email]) + end + + it 'populates missing committers' do + commit_row = create( + :merge_request_diff_commit, + merge_request_diff: diff, + sha: commit.sha, + relative_order: 9000 + ) + + updated = update_row(migration, project, diff, commit_row) + + expect(updated.committer.name).to eq(commit.to_hash[:committer_name]) + expect(updated.committer.email).to eq(commit.to_hash[:committer_email]) + end + + it 'leaves existing commit authors as-is' do + user = create(:merge_request_diff_commit_user) + commit_row = create( + :merge_request_diff_commit, + merge_request_diff: diff, + sha: commit.sha, + relative_order: 9000, + commit_author: user + ) + + updated = update_row(migration, project, diff, commit_row) + + expect(updated.commit_author).to eq(user) + end + + it 'leaves existing committers as-is' do + user = create(:merge_request_diff_commit_user) + commit_row = create( + :merge_request_diff_commit, + merge_request_diff: diff, + sha: commit.sha, + relative_order: 9000, + committer: user + ) + + updated = update_row(migration, project, diff, commit_row) + + expect(updated.committer).to eq(user) + end + + it 'does nothing when both the author and committer are present' do + user = create(:merge_request_diff_commit_user) + commit_row = create( + :merge_request_diff_commit, + merge_request_diff: diff, + sha: commit.sha, + relative_order: 9000, + committer: user, + commit_author: user + ) + + recorder = ActiveRecord::QueryRecorder.new do + migration.update_commit(project, commit_row) + end + + expect(recorder.count).to be_zero + end + + it 'does nothing if the commit does not exist in Git' do + user = create(:merge_request_diff_commit_user) + commit_row = create( + :merge_request_diff_commit, + merge_request_diff: diff, + sha: 'kittens', + relative_order: 9000, + committer: user, + commit_author: user + ) + + recorder = ActiveRecord::QueryRecorder.new do + migration.update_commit(project, commit_row) + end + + expect(recorder.count).to be_zero + end + + it 'does nothing when the committer/author are missing in the Git commit' do + user = create(:merge_request_diff_commit_user) + commit_row = create( + :merge_request_diff_commit, + merge_request_diff: diff, + sha: commit.sha, + relative_order: 9000, + committer: user, + commit_author: user + ) + + allow(migration).to receive(:find_or_create_user).and_return(nil) + + recorder = ActiveRecord::QueryRecorder.new do + migration.update_commit(project, commit_row) + end + + expect(recorder.count).to be_zero + end + end + + describe '#schedule_next_job' do + it 'schedules the next background migration' do + Gitlab::Database::BackgroundMigrationJob + .create!(class_name: 'FixMergeRequestDiffCommitUsers', arguments: [42]) + + expect(BackgroundMigrationWorker) + .to receive(:perform_in) + .with(2.minutes, 'FixMergeRequestDiffCommitUsers', [42]) + + migration.schedule_next_job + end + + it 'does nothing when there are no jobs' do + expect(BackgroundMigrationWorker) + .not_to receive(:perform_in) + + migration.schedule_next_job + end + end + + describe '#find_commit' do + let(:project) { create(:project, :repository) } + + it 'finds a commit using Git' do + commit = project.commit + found = migration.find_commit(project, commit.sha) + + expect(found).to eq(commit.to_hash) + end + + it 'caches the results' do + commit = project.commit + + migration.find_commit(project, commit.sha) + + expect { migration.find_commit(project, commit.sha) } + .not_to change { Gitlab::GitalyClient.get_request_count } + end + + it 'returns an empty hash if the commit does not exist' do + expect(migration.find_commit(project, 'kittens')).to eq({}) + end + end + + describe '#find_or_create_user' do + let(:project) { create(:project, :repository) } + + it 'creates missing users' do + commit = project.commit.to_hash + id = migration.find_or_create_user(commit, :author_name, :author_email) + + expect(MergeRequest::DiffCommitUser.count).to eq(1) + + created = MergeRequest::DiffCommitUser.first + + expect(created.name).to eq(commit[:author_name]) + expect(created.email).to eq(commit[:author_email]) + expect(created.id).to eq(id) + end + + it 'returns users that already exist' do + commit = project.commit.to_hash + user1 = migration.find_or_create_user(commit, :author_name, :author_email) + user2 = migration.find_or_create_user(commit, :author_name, :author_email) + + expect(user1).to eq(user2) + end + + it 'caches the results' do + commit = project.commit.to_hash + + migration.find_or_create_user(commit, :author_name, :author_email) + + recorder = ActiveRecord::QueryRecorder.new do + migration.find_or_create_user(commit, :author_name, :author_email) + end + + expect(recorder.count).to be_zero + end + + it 'returns nil if the commit details are missing' do + id = migration.find_or_create_user({}, :author_name, :author_email) + + expect(id).to be_nil + end + end + + describe '#matches_row' do + it 'returns the query matches for the composite primary key' do + row = double(:commit, merge_request_diff_id: 4, relative_order: 5) + arel = migration.matches_row(row) + + expect(arel.to_sql).to eq( + '("merge_request_diff_commits"."merge_request_diff_id", "merge_request_diff_commits"."relative_order") = (4, 5)' + ) + end + end +end +# rubocop: enable RSpec/FactoriesInMigrationSpecs diff --git a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb index c1516a48b80..771f6e1ec46 100644 --- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb +++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb @@ -140,6 +140,8 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do key: GpgHelpers::User1.public_key, user: user + user.reload # necessary to reload the association with gpg_keys + expect(invalid_gpg_signature.reload.verification_status).to eq 'unverified_key' # InvalidGpgSignatureUpdater is called by the after_update hook diff --git a/spec/lib/gitlab/import_export/project/object_builder_spec.rb b/spec/lib/gitlab/import_export/project/object_builder_spec.rb index 4c9f9f7c690..42598047500 100644 --- a/spec/lib/gitlab/import_export/project/object_builder_spec.rb +++ b/spec/lib/gitlab/import_export/project/object_builder_spec.rb @@ -176,4 +176,118 @@ RSpec.describe Gitlab::ImportExport::Project::ObjectBuilder do expect(found.email).to eq('alice@example.com') end end + + context 'merge request diff commits' do + context 'when the "committer" object is present' do + it 'uses this object as the committer' do + user = MergeRequest::DiffCommitUser + .find_or_create('Alice', 'alice@example.com') + + commit = described_class.build( + MergeRequestDiffCommit, + { + 'committer' => user, + 'committer_name' => 'Bla', + 'committer_email' => 'bla@example.com', + 'author_name' => 'Bla', + 'author_email' => 'bla@example.com' + } + ) + + expect(commit.committer).to eq(user) + end + end + + context 'when the "committer" object is missing' do + it 'creates one from the committer name and Email' do + commit = described_class.build( + MergeRequestDiffCommit, + { + 'committer_name' => 'Alice', + 'committer_email' => 'alice@example.com', + 'author_name' => 'Alice', + 'author_email' => 'alice@example.com' + } + ) + + expect(commit.committer.name).to eq('Alice') + expect(commit.committer.email).to eq('alice@example.com') + end + end + + context 'when the "commit_author" object is present' do + it 'uses this object as the author' do + user = MergeRequest::DiffCommitUser + .find_or_create('Alice', 'alice@example.com') + + commit = described_class.build( + MergeRequestDiffCommit, + { + 'committer_name' => 'Alice', + 'committer_email' => 'alice@example.com', + 'commit_author' => user, + 'author_name' => 'Bla', + 'author_email' => 'bla@example.com' + } + ) + + expect(commit.commit_author).to eq(user) + end + end + + context 'when the "commit_author" object is missing' do + it 'creates one from the author name and Email' do + commit = described_class.build( + MergeRequestDiffCommit, + { + 'committer_name' => 'Alice', + 'committer_email' => 'alice@example.com', + 'author_name' => 'Alice', + 'author_email' => 'alice@example.com' + } + ) + + expect(commit.commit_author.name).to eq('Alice') + expect(commit.commit_author.email).to eq('alice@example.com') + end + end + end + + describe '#find_or_create_diff_commit_user' do + context 'when the user already exists' do + it 'returns the existing user' do + user = MergeRequest::DiffCommitUser + .find_or_create('Alice', 'alice@example.com') + + found = described_class + .new(MergeRequestDiffCommit, {}) + .send(:find_or_create_diff_commit_user, user.name, user.email) + + expect(found).to eq(user) + end + end + + context 'when the user does not exist' do + it 'creates the user' do + found = described_class + .new(MergeRequestDiffCommit, {}) + .send(:find_or_create_diff_commit_user, 'Alice', 'alice@example.com') + + expect(found.name).to eq('Alice') + expect(found.email).to eq('alice@example.com') + end + end + + it 'caches the results' do + builder = described_class.new(MergeRequestDiffCommit, {}) + + builder.send(:find_or_create_diff_commit_user, 'Alice', 'alice@example.com') + + record = ActiveRecord::QueryRecorder.new do + builder.send(:find_or_create_diff_commit_user, 'Alice', 'alice@example.com') + end + + expect(record.count).to eq(1) + end + end end diff --git a/spec/lib/gitlab/spamcheck/client_spec.rb b/spec/lib/gitlab/spamcheck/client_spec.rb index 15e963fe423..e542ce455bb 100644 --- a/spec/lib/gitlab/spamcheck/client_spec.rb +++ b/spec/lib/gitlab/spamcheck/client_spec.rb @@ -82,7 +82,7 @@ RSpec.describe Gitlab::Spamcheck::Client do end end - describe '#build_user_proto_buf', :aggregate_failures do + describe '#build_user_protobuf', :aggregate_failures do it 'builds the expected protobuf object' do user_pb = described_class.new.send(:build_user_protobuf, user) expect(user_pb.username).to eq user.username diff --git a/spec/lib/gitlab/x509/signature_spec.rb b/spec/lib/gitlab/x509/signature_spec.rb index 7ba15faf910..0e34d5393d6 100644 --- a/spec/lib/gitlab/x509/signature_spec.rb +++ b/spec/lib/gitlab/x509/signature_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Gitlab::X509::Signature do end shared_examples "a verified signature" do - let_it_be(:user) { create(:user, email: X509Helpers::User1.certificate_email) } + let!(:user) { create(:user, email: X509Helpers::User1.certificate_email) } subject(:signature) do described_class.new( @@ -30,10 +30,12 @@ RSpec.describe Gitlab::X509::Signature do expect(signature.verification_status).to eq(:verified) end - it "returns an unverified signature if the email matches but isn't confirmed" do - user.update!(confirmed_at: nil) + context "if the email matches but isn't confirmed" do + let!(:user) { create(:user, :unconfirmed, email: X509Helpers::User1.certificate_email) } - expect(signature.verification_status).to eq(:unverified) + it "returns an unverified signature" do + expect(signature.verification_status).to eq(:unverified) + end end it 'returns an unverified signature if email does not match' do @@ -297,7 +299,7 @@ RSpec.describe Gitlab::X509::Signature do end context 'verified signature' do - let_it_be(:user) { create(:user, email: X509Helpers::User1.certificate_email) } + let_it_be(:user) { create(:user, :unconfirmed, email: X509Helpers::User1.certificate_email) } subject(:signature) do described_class.new( @@ -316,52 +318,56 @@ RSpec.describe Gitlab::X509::Signature do allow(OpenSSL::X509::Store).to receive(:new).and_return(store) end - it 'returns a verified signature if email does match' do - expect(signature.x509_certificate).to have_attributes(certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) - expect(signature.verified_signature).to be_truthy - expect(signature.verification_status).to eq(:verified) + context 'when user email is confirmed' do + before_all do + user.confirm + end + + it 'returns a verified signature if email does match', :ggregate_failures do + expect(signature.x509_certificate).to have_attributes(certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + expect(signature.verified_signature).to be_truthy + expect(signature.verification_status).to eq(:verified) + end + + it 'returns an unverified signature if email does not match', :aggregate_failures do + signature = described_class.new( + X509Helpers::User1.signed_tag_signature, + X509Helpers::User1.signed_tag_base_data, + "gitlab@example.com", + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate).to have_attributes(certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + expect(signature.verified_signature).to be_truthy + expect(signature.verification_status).to eq(:unverified) + end + + it 'returns an unverified signature if email does match and time is wrong', :aggregate_failures do + signature = described_class.new( + X509Helpers::User1.signed_tag_signature, + X509Helpers::User1.signed_tag_base_data, + X509Helpers::User1.certificate_email, + Time.new(2020, 2, 22) + ) + + expect(signature.x509_certificate).to have_attributes(certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + expect(signature.verified_signature).to be_falsey + expect(signature.verification_status).to eq(:unverified) + end + + it 'returns an unverified signature if certificate is revoked' do + expect(signature.verification_status).to eq(:verified) + + signature.x509_certificate.revoked! + + expect(signature.verification_status).to eq(:unverified) + end end - it "returns an unverified signature if the email matches but isn't confirmed" do - user.update!(confirmed_at: nil) - - expect(signature.verification_status).to eq(:unverified) - end - - it 'returns an unverified signature if email does not match' do - signature = described_class.new( - X509Helpers::User1.signed_tag_signature, - X509Helpers::User1.signed_tag_base_data, - "gitlab@example.com", - X509Helpers::User1.signed_commit_time - ) - - expect(signature.x509_certificate).to have_attributes(certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) - expect(signature.verified_signature).to be_truthy - expect(signature.verification_status).to eq(:unverified) - end - - it 'returns an unverified signature if email does match and time is wrong' do - signature = described_class.new( - X509Helpers::User1.signed_tag_signature, - X509Helpers::User1.signed_tag_base_data, - X509Helpers::User1.certificate_email, - Time.new(2020, 2, 22) - ) - - expect(signature.x509_certificate).to have_attributes(certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) - expect(signature.verified_signature).to be_falsey - expect(signature.verification_status).to eq(:unverified) - end - - it 'returns an unverified signature if certificate is revoked' do - expect(signature.verification_status).to eq(:verified) - - signature.x509_certificate.revoked! - + it 'returns an unverified signature if the email matches but is not confirmed' do expect(signature.verification_status).to eq(:unverified) end end diff --git a/spec/migrations/20211028155449_schedule_fix_merge_request_diff_commit_users_migration_spec.rb b/spec/migrations/20211028155449_schedule_fix_merge_request_diff_commit_users_migration_spec.rb new file mode 100644 index 00000000000..6511f554436 --- /dev/null +++ b/spec/migrations/20211028155449_schedule_fix_merge_request_diff_commit_users_migration_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! 'schedule_fix_merge_request_diff_commit_users_migration' + +RSpec.describe ScheduleFixMergeRequestDiffCommitUsersMigration, :migration do + let(:migration) { described_class.new } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:namespace) { namespaces.create!(name: 'foo', path: 'foo') } + + describe '#up' do + it 'does nothing when there are no projects to correct' do + migration.up + + expect(Gitlab::Database::BackgroundMigrationJob.count).to be_zero + end + + it 'schedules imported projects created after July' do + project = projects.create!( + namespace_id: namespace.id, + import_type: 'gitlab_project', + created_at: '2021-08-01' + ) + + expect(migration) + .to receive(:migrate_in) + .with(2.minutes, 'FixMergeRequestDiffCommitUsers', [project.id]) + + migration.up + + expect(Gitlab::Database::BackgroundMigrationJob.count).to eq(1) + + job = Gitlab::Database::BackgroundMigrationJob.first + + expect(job.class_name).to eq('FixMergeRequestDiffCommitUsers') + expect(job.arguments).to eq([project.id]) + end + + it 'ignores projects imported before July' do + projects.create!( + namespace_id: namespace.id, + import_type: 'gitlab_project', + created_at: '2020-08-01' + ) + + migration.up + + expect(Gitlab::Database::BackgroundMigrationJob.count).to be_zero + end + + it 'ignores projects that are not imported' do + projects.create!( + namespace_id: namespace.id, + created_at: '2021-08-01' + ) + + migration.up + + expect(Gitlab::Database::BackgroundMigrationJob.count).to be_zero + end + end +end diff --git a/spec/migrations/schedule_add_primary_email_to_emails_if_user_confirmed_spec.rb b/spec/migrations/schedule_add_primary_email_to_emails_if_user_confirmed_spec.rb new file mode 100644 index 00000000000..c66ac1bd7e9 --- /dev/null +++ b/spec/migrations/schedule_add_primary_email_to_emails_if_user_confirmed_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe ScheduleAddPrimaryEmailToEmailsIfUserConfirmed, :sidekiq do + let(:migration) { described_class.new } + let(:users) { table(:users) } + + let!(:user_1) { users.create!(name: 'confirmed-user-1', email: 'confirmed-1@example.com', confirmed_at: 1.day.ago, projects_limit: 100) } + let!(:user_2) { users.create!(name: 'confirmed-user-2', email: 'confirmed-2@example.com', confirmed_at: 1.day.ago, projects_limit: 100) } + let!(:user_3) { users.create!(name: 'confirmed-user-3', email: 'confirmed-3@example.com', confirmed_at: 1.day.ago, projects_limit: 100) } + let!(:user_4) { users.create!(name: 'confirmed-user-4', email: 'confirmed-4@example.com', confirmed_at: 1.day.ago, projects_limit: 100) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 2) + stub_const("#{described_class.name}::INTERVAL", 2.minutes.to_i) + end + + it 'schedules addition of primary email to emails in delayed batches' do + Sidekiq::Testing.fake! do + freeze_time do + migration.up + + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, user_1.id, user_2.id) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, user_3.id, user_4.id) + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index 2b09ee5c190..59299a507e4 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -13,6 +13,15 @@ RSpec.describe Email do it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email do subject { build(:email) } end + + context 'when the email conflicts with the primary email of a different user' do + let(:user) { create(:user) } + let(:email) { build(:email, email: user.email) } + + it 'is invalid' do + expect(email).to be_invalid + end + end end it 'normalize email value' do @@ -33,7 +42,7 @@ RSpec.describe Email do end describe 'scopes' do - let(:user) { create(:user) } + let(:user) { create(:user, :unconfirmed) } it 'scopes confirmed emails' do create(:email, :confirmed, user: user) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cad9ad81a32..bd357858dcf 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1123,7 +1123,7 @@ RSpec.describe User do end describe 'after commit hook' do - describe '#update_emails_with_primary_email' do + describe 'when the primary email is updated' do before do @user = create(:user, email: 'primary@example.com').tap do |user| user.skip_reconfirmation! @@ -1132,13 +1132,7 @@ RSpec.describe User do @user.reload end - it 'gets called when email updated' do - expect(@user).to receive(:update_emails_with_primary_email) - - @user.update!(email: 'new_primary@example.com') - end - - it 'adds old primary to secondary emails when secondary is a new email' do + it 'keeps old primary to secondary emails when secondary is a new email' do @user.update!(email: 'new_primary@example.com') @user.reload @@ -1146,22 +1140,6 @@ RSpec.describe User do expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com']) end - it 'adds old primary to secondary emails if secondary is becoming a primary' do - @user.update!(email: @secondary.email) - @user.reload - - expect(@user.emails.count).to eq 1 - expect(@user.emails.first.email).to eq 'primary@example.com' - end - - it 'transfers old confirmation values into new secondary' do - @user.update!(email: @secondary.email) - @user.reload - - 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') } @@ -1178,11 +1156,8 @@ RSpec.describe User 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 + it 'does not add unconfirmed email to secondary' do + expect(user.emails.map(&:email)).not_to include('should-be-unconfirmed@test.com') end it 'has only one email association' do @@ -1244,7 +1219,7 @@ RSpec.describe User do expect(user.email).to eq(confirmed_email) end - it 'moves the old email' do + it 'keeps the old email' do email = user.reload.emails.first expect(email.email).to eq(old_confirmed_email) @@ -1499,7 +1474,7 @@ RSpec.describe User do allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) end - let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') } + let(:user) { create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com') } it 'returns unconfirmed' do expect(user.confirmed?).to be_falsey @@ -1509,6 +1484,22 @@ RSpec.describe User do user.confirm expect(user.confirmed?).to be_truthy end + + it 'adds the confirmed primary email to emails' do + expect(user.emails.confirmed.map(&:email)).not_to include(user.email) + + user.confirm + + expect(user.emails.confirmed.map(&:email)).to include(user.email) + end + end + + context 'if the user is created with confirmed_at set to a time' do + let!(:user) { create(:user, email: 'test@gitlab.com', confirmed_at: Time.now.utc) } + + it 'adds the confirmed primary email to emails upon creation' do + expect(user.emails.confirmed.map(&:email)).to include(user.email) + end end describe '#to_reference' do @@ -2216,7 +2207,7 @@ RSpec.describe User do end context 'primary email not confirmed' do - let(:user) { create(:user, confirmed_at: nil) } + let(:user) { create(:user, :unconfirmed) } let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } it 'finds user respecting the confirmed flag' do @@ -2231,7 +2222,7 @@ RSpec.describe User do end it 'returns nil when user is not confirmed' do - user = create(:user, email: 'foo@example.com', confirmed_at: nil) + user = create(:user, :unconfirmed, email: 'foo@example.com') expect(described_class.find_by_any_email(user.email, confirmed: false)).to eq(user) expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil @@ -6011,7 +6002,7 @@ RSpec.describe User do subject { user.confirmation_required_on_sign_in? } context 'when user is confirmed' do - let(:user) { build_stubbed(:user) } + let(:user) { create(:user) } it 'is falsey' do expect(user.confirmed?).to be_truthy diff --git a/spec/requests/api/error_tracking/collector_spec.rb b/spec/requests/api/error_tracking/collector_spec.rb index 57c87811c9e..21e2849fef0 100644 --- a/spec/requests/api/error_tracking/collector_spec.rb +++ b/spec/requests/api/error_tracking/collector_spec.rb @@ -136,6 +136,21 @@ RSpec.describe API::ErrorTracking::Collector do it_behaves_like 'bad request' end + context 'body with string instead of json' do + let(:params) { '"********"' } + + it_behaves_like 'bad request' + end + + context 'collector fails with validation error' do + before do + allow(::ErrorTracking::CollectErrorService) + .to receive(:new).and_raise(ActiveRecord::RecordInvalid) + end + + it_behaves_like 'bad request' + end + context 'gzip body' do let(:headers) do { diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 50e5025fd47..b93df2f3bae 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1906,7 +1906,8 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.first['email']).to eq(email.email) + expect(json_response.first['email']).to eq(user.email) + expect(json_response.second['email']).to eq(email.email) end it "returns a 404 for invalid ID" do @@ -2488,7 +2489,8 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.first["email"]).to eq(email.email) + expect(json_response.first['email']).to eq(user.email) + expect(json_response.second['email']).to eq(email.email) end context "scopes" do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index accacd705e7..701a73761fd 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -305,7 +305,7 @@ RSpec.describe UsersController do context 'user with keys' do let!(:gpg_key) { create(:gpg_key, user: user) } - let!(:another_gpg_key) { create(:another_gpg_key, user: user) } + let!(:another_gpg_key) { create(:another_gpg_key, user: user.reload) } shared_examples_for 'renders all verified GPG keys' do it 'renders all verified keys separated with a new line with text/plain content type' do diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 1396a1fce30..2fabf4ae66a 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Emails::CreateService do - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let(:opts) { { email: 'new@email.com', user: user } } subject(:service) { described_class.new(user, opts) } @@ -22,7 +23,7 @@ RSpec.describe Emails::CreateService do it 'has the right user association' do service.execute - expect(user.emails).to eq(Email.where(opts)) + expect(user.emails).to include(Email.find_by(opts)) end end end diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index f8407be41e7..7dcf367016e 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -15,5 +15,15 @@ RSpec.describe Emails::DestroyService do expect(user.emails).not_to include(email) expect(response).to be true end + + context 'when it corresponds to the user primary email' do + let(:email) { user.emails.find_by!(email: user.email) } + + it 'does not remove the email and raises an exception' do + expect { service.execute(email) }.to raise_error(StandardError, 'Cannot delete primary email') + + expect(user.emails).to include(email) + end + end end end