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 e3ec4f603b9..e5f6f195fe5 100644
Binary files a/spec/fixtures/gitlab/import_export/lightweight_project_export.tar.gz and b/spec/fixtures/gitlab/import_export/lightweight_project_export.tar.gz differ
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