From 763dae9e6b0848fb2066f458c64905c6e374dfe7 Mon Sep 17 00:00:00 2001
From: ghinfey <ghinfey@gitlab.com>
Date: Tue, 18 Feb 2025 14:13:42 +0000
Subject: [PATCH] Add unverified_author_email to grahql endpoint
Add unverified_author_email to the graphql verification
status field.
Update front end to consume new field to match how
templating currently handles it.
Changelog: fixed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181805
---
.../commit/components/signature_badge.vue | 2 +-
app/assets/javascripts/commit/constants.js | 14 +++++++++++
.../types/commit_signature_interface.rb | 3 ++-
app/models/concerns/commit_signature.rb | 13 ++++++----
app/models/concerns/enums/commit_signature.rb | 3 ++-
.../projects/commit/_signature.html.haml | 2 +-
doc/api/graphql/reference/_index.md | 1 +
lib/gitlab/x509/signature.rb | 1 -
.../commit/components/signature_badge_spec.js | 3 +++
.../gpg/invalid_gpg_signature_updater_spec.rb | 3 ++-
spec/lib/gitlab/ssh/commit_spec.rb | 4 ++++
.../commit_signatures/gpg_signature_spec.rb | 24 +++++++++----------
.../commit_signatures/ssh_signature_spec.rb | 20 ++++++++--------
.../x509_commit_signature_spec.rb | 14 +++++------
14 files changed, 67 insertions(+), 40 deletions(-)
diff --git a/app/assets/javascripts/commit/components/signature_badge.vue b/app/assets/javascripts/commit/components/signature_badge.vue
index 0ec828fa2082..70b89d6ca06f 100644
--- a/app/assets/javascripts/commit/components/signature_badge.vue
+++ b/app/assets/javascripts/commit/components/signature_badge.vue
@@ -51,7 +51,7 @@ export default {
class="gl-border-0 gl-bg-transparent gl-p-0 gl-outline-none"
:aria-label="statusConfig.label"
>
- <gl-badge :variant="statusConfig.variant">
+ <gl-badge :icon="statusConfig.icon" :variant="statusConfig.variant">
{{ statusConfig.label }}
</gl-badge>
</button>
diff --git a/app/assets/javascripts/commit/constants.js b/app/assets/javascripts/commit/constants.js
index 417a0e2b6d00..fb68c118cfcb 100644
--- a/app/assets/javascripts/commit/constants.js
+++ b/app/assets/javascripts/commit/constants.js
@@ -12,6 +12,7 @@ export const verificationStatuses = {
MULTIPLE_SIGNATURES: 'MULTIPLE_SIGNATURES',
REVOKED_KEY: 'REVOKED_KEY',
VERIFIED_SYSTEM: 'VERIFIED_SYSTEM',
+ UNVERIFIED_AUTHOR_EMAIL: 'UNVERIFIED_AUTHOR_EMAIL',
};
export const signatureTypes = {
@@ -29,6 +30,13 @@ const UNVERIFIED_CONFIG = {
description: __('This commit was signed with an unverified signature.'),
};
+export const REVERIFIED_CONFIG = {
+ variant: 'warning',
+ icon: 'warning',
+ label: __('Verified'),
+ title: __('Verified commit with unverified email'),
+};
+
export const VERIFIED_CONFIG = {
variant: 'success',
label: __('Verified'),
@@ -42,6 +50,12 @@ export const statusConfig = {
'This commit was signed with a verified signature and the committer email was verified to belong to the same user.',
),
},
+ [verificationStatuses.UNVERIFIED_AUTHOR_EMAIL]: {
+ ...REVERIFIED_CONFIG,
+ description: __(
+ 'This commit was previously signed with a verified signature and verified committer email address. However the committer email address is no longer verified to the same user.',
+ ),
+ },
[verificationStatuses.VERIFIED_SYSTEM]: {
...VERIFIED_CONFIG,
description: __(
diff --git a/app/graphql/types/commit_signature_interface.rb b/app/graphql/types/commit_signature_interface.rb
index 87ed6ea4ff46..df138fdb1290 100644
--- a/app/graphql/types/commit_signature_interface.rb
+++ b/app/graphql/types/commit_signature_interface.rb
@@ -10,7 +10,8 @@ module CommitSignatureInterface
field :verification_status, CommitSignatures::VerificationStatusEnum,
null: true,
- description: 'Indicates verification status of the associated key or certificate.'
+ description: 'Indicates verification status of the associated key or certificate.',
+ calls_gitaly: true
field :commit_sha, GraphQL::Types::String,
null: true,
diff --git a/app/models/concerns/commit_signature.rb b/app/models/concerns/commit_signature.rb
index e4260edae409..31216b1fc9d0 100644
--- a/app/models/concerns/commit_signature.rb
+++ b/app/models/concerns/commit_signature.rb
@@ -42,13 +42,16 @@ def signed_by_user
raise NoMethodError, 'must implement `signed_by_user` method'
end
- def reverified_status
- return verification_status unless Feature.enabled?(:check_for_mailmapped_commit_emails, project)
- return verification_status unless verified? || verified_system?
+ # If commit is persisted as verified, check that commit email is still correct.
+ def verification_status
+ persisted_status = read_attribute(:verification_status)
+ return persisted_status unless Feature.enabled?(:check_for_mailmapped_commit_emails, project)
+ return persisted_status unless verified? || verified_system?
+ return persisted_status unless commit
- return 'unverified_author_email' if emails_for_verification&.exclude?(commit&.author_email)
+ return 'unverified_author_email' if emails_for_verification&.exclude?(commit.author_email)
- verification_status
+ persisted_status
end
private
diff --git a/app/models/concerns/enums/commit_signature.rb b/app/models/concerns/enums/commit_signature.rb
index 92625af58ef8..0c6b28bbef96 100644
--- a/app/models/concerns/enums/commit_signature.rb
+++ b/app/models/concerns/enums/commit_signature.rb
@@ -11,7 +11,8 @@ class CommitSignature
unknown_key: 5,
multiple_signatures: 6,
revoked_key: 7,
- verified_system: 8
+ verified_system: 8,
+ unverified_author_email: 9
# EE adds more values in ee/app/models/concerns/ee/enums/commit_signature.rb
}.freeze
diff --git a/app/views/projects/commit/_signature.html.haml b/app/views/projects/commit/_signature.html.haml
index c21387758dc6..e384a4724f95 100644
--- a/app/views/projects/commit/_signature.html.haml
+++ b/app/views/projects/commit/_signature.html.haml
@@ -1,5 +1,5 @@
- if signature
- uri = "projects/commit/#{'x509/' if signature.x509?}"
- = render partial: "#{uri}#{signature.reverified_status}_signature_badge", locals: { signature: signature }
+ = render partial: "#{uri}#{signature.verification_status}_signature_badge", locals: { signature: signature }
- else
= render partial: 'projects/commit/unverified_signature_badge', locals: { signature: nil }
diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md
index d406a5e02b43..4796e64a7a81 100644
--- a/doc/api/graphql/reference/_index.md
+++ b/doc/api/graphql/reference/_index.md
@@ -43917,6 +43917,7 @@ Verification status of a GPG, X.509 or SSH signature for a commit.
| <a id="verificationstatussame_user_different_email"></a>`SAME_USER_DIFFERENT_EMAIL` | same_user_different_email verification status. |
| <a id="verificationstatusunknown_key"></a>`UNKNOWN_KEY` | unknown_key verification status. |
| <a id="verificationstatusunverified"></a>`UNVERIFIED` | unverified verification status. |
+| <a id="verificationstatusunverified_author_email"></a>`UNVERIFIED_AUTHOR_EMAIL` | unverified_author_email verification status. |
| <a id="verificationstatusunverified_key"></a>`UNVERIFIED_KEY` | unverified_key verification status. |
| <a id="verificationstatusverified"></a>`VERIFIED` | verified verification status. |
| <a id="verificationstatusverified_ca"></a>`VERIFIED_CA` | verified_ca verification status. |
diff --git a/lib/gitlab/x509/signature.rb b/lib/gitlab/x509/signature.rb
index 1f96b95716bc..0f9d40440a28 100644
--- a/lib/gitlab/x509/signature.rb
+++ b/lib/gitlab/x509/signature.rb
@@ -52,7 +52,6 @@ def verification_status
:unverified
end
- alias_method :reverified_status, :verification_status
private
diff --git a/spec/frontend/commit/components/signature_badge_spec.js b/spec/frontend/commit/components/signature_badge_spec.js
index 4e8ad8e12f12..ae5f879f470d 100644
--- a/spec/frontend/commit/components/signature_badge_spec.js
+++ b/spec/frontend/commit/components/signature_badge_spec.js
@@ -44,9 +44,12 @@ describe('Commit signature', () => {
${signatureTypes.GPG} | ${verificationStatuses.OTHER_USER}
${signatureTypes.GPG} | ${verificationStatuses.SAME_USER_DIFFERENT_EMAIL}
${signatureTypes.GPG} | ${verificationStatuses.MULTIPLE_SIGNATURES}
+ ${signatureTypes.GPG} | ${verificationStatuses.UNVERIFIED_AUTHOR_EMAIL}
${signatureTypes.X509} | ${verificationStatuses.VERIFIED}
+ ${signatureTypes.X509} | ${verificationStatuses.UNVERIFIED_AUTHOR_EMAIL}
${signatureTypes.SSH} | ${verificationStatuses.VERIFIED}
${signatureTypes.SSH} | ${verificationStatuses.REVOKED_KEY}
+ ${signatureTypes.SSH} | ${verificationStatuses.UNVERIFIED_AUTHOR_EMAIL}
`(
'For a specified `$signatureType` and `$verificationStatus` it renders component correctly',
({ signatureType, verificationStatus }) => {
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 db88e99970c2..5c3410056fbb 100644
--- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
+++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
@@ -19,7 +19,8 @@
:raw_commit,
signature: signature,
sha: commit_sha,
- committer_email: committer_email
+ committer_email: committer_email,
+ author_email: committer_email
)
allow(raw_commit).to receive :save!
diff --git a/spec/lib/gitlab/ssh/commit_spec.rb b/spec/lib/gitlab/ssh/commit_spec.rb
index 25aa92526301..79393c7f8bd5 100644
--- a/spec/lib/gitlab/ssh/commit_spec.rb
+++ b/spec/lib/gitlab/ssh/commit_spec.rb
@@ -26,6 +26,10 @@
.with(Gitlab::Git::Repository, commit.sha)
.and_return(signature_data)
+ allow_next_instance_of(Commit) do |instance|
+ allow(instance).to receive(:author_email).and_return(user_author.email)
+ end
+
allow(verifier).to receive_messages({
verification_status: verification_status,
signed_by_key: signed_by_key,
diff --git a/spec/models/commit_signatures/gpg_signature_spec.rb b/spec/models/commit_signatures/gpg_signature_spec.rb
index 5f27212ac33c..a338964ca681 100644
--- a/spec/models/commit_signatures/gpg_signature_spec.rb
+++ b/spec/models/commit_signatures/gpg_signature_spec.rb
@@ -116,7 +116,7 @@
end
end
- describe '#reverified_status' do
+ describe '#verification_status' do
let(:verification_status) { :verified }
let(:signature) do
create(:gpg_signature, commit_sha: commit_sha, gpg_key: gpg_key, project: project,
@@ -128,16 +128,16 @@
end
# verified is used for user signed gpg commits.
- context 'when verification_status is verified' do
- it 'returns existing verification status' do
- expect(signature.reverified_status).to eq('verified')
+ context 'when persisted verification_status is verified' do
+ it 'returns persisted verification status' do
+ expect(signature.verification_status).to eq('verified')
end
context 'when commit author does not match the gpg_key author' do
let(:commit_author) { create(:user) }
it 'returns unverified_author_email' do
- expect(signature.reverified_status).to eq('unverified_author_email')
+ expect(signature.verification_status).to eq('unverified_author_email')
end
context 'when check_for_mailmapped_commit_emails feature flag is disabled' do
@@ -146,33 +146,33 @@
end
it 'verification status is unmodified' do
- expect(signature.reverified_status).to eq('verified')
+ expect(signature.verification_status).to eq('verified')
end
end
end
end
- context 'when verification_status not verified' do
+ context 'when persisted verification_status not verified' do
let(:verification_status) { :unverified }
it 'returns the signature verification status' do
- expect(signature.reverified_status).to eq('unverified')
+ expect(signature.verification_status).to eq('unverified')
end
end
# verified_system is used for ui signed commits.
- context 'when verification_status is verified_system' do
+ context 'when persisted verification_status is verified_system' do
let(:verification_status) { :verified_system }
it 'returns existing verification status' do
- expect(signature.reverified_status).to eq('verified_system')
+ expect(signature.verification_status).to eq('verified_system')
end
context 'when commit author does not match the gpg_key author' do
let(:commit_author) { create(:user) }
- it 'returns existing verification status' do
- expect(signature.reverified_status).to eq('unverified_author_email')
+ it 'returns unverified_author_email' do
+ expect(signature.verification_status).to eq('unverified_author_email')
end
end
end
diff --git a/spec/models/commit_signatures/ssh_signature_spec.rb b/spec/models/commit_signatures/ssh_signature_spec.rb
index 55e0ef20aee7..e6061d1703da 100644
--- a/spec/models/commit_signatures/ssh_signature_spec.rb
+++ b/spec/models/commit_signatures/ssh_signature_spec.rb
@@ -76,21 +76,21 @@
end
end
- describe '#reverified_status' do
+ describe '#verification_status' do
before do
allow(signature.project).to receive(:commit).with(commit_sha).and_return(commit)
end
- context 'when verification_status is verified' do
+ context 'when persisted verification_status is verified' do
it 'returns verified' do
- expect(signature.reverified_status).to eq('verified')
+ expect(signature.verification_status).to eq('verified')
end
context 'and the author email does not belong to the signed by user' do
let(:user) { create(:user) }
it 'returns unverified_author_email' do
- expect(signature.reverified_status).to eq('unverified_author_email')
+ expect(signature.verification_status).to eq('unverified_author_email')
end
context 'when check_for_mailmapped_commit_emails feature flag is disabled' do
@@ -99,17 +99,17 @@
end
it 'verification status is unmodified' do
- expect(signature.reverified_status).to eq('verified')
+ expect(signature.verification_status).to eq('verified')
end
end
end
end
- context 'when verification_status not verified' do
+ context 'when persisted verification_status not verified' do
let(:signature) { create(:ssh_signature, verification_status: 'unverified') }
it 'returns the signature verification status' do
- expect(signature.reverified_status).to eq('unverified')
+ expect(signature.verification_status).to eq('unverified')
end
end
@@ -117,14 +117,14 @@
let(:verification_status) { :verified_system }
it 'returns the signature verification status' do
- expect(signature.reverified_status).to eq('verified_system')
+ expect(signature.verification_status).to eq('verified_system')
end
context 'and the author email does not belong to the signed by user' do
let(:user) { create(:user) }
it 'returns unverified_author_email' do
- expect(signature.reverified_status).to eq('unverified_author_email')
+ expect(signature.verification_status).to eq('unverified_author_email')
end
context 'when check_for_mailmapped_commit_emails feature flag is disabled' do
@@ -133,7 +133,7 @@
end
it 'verification status is unmodified' do
- expect(signature.reverified_status).to eq('verified_system')
+ expect(signature.verification_status).to eq('verified_system')
end
end
end
diff --git a/spec/models/commit_signatures/x509_commit_signature_spec.rb b/spec/models/commit_signatures/x509_commit_signature_spec.rb
index 24a49e5bdb82..b287c344747c 100644
--- a/spec/models/commit_signatures/x509_commit_signature_spec.rb
+++ b/spec/models/commit_signatures/x509_commit_signature_spec.rb
@@ -9,7 +9,7 @@
let_it_be(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:commit) { create(:commit, project: project, sha: commit_sha) }
- let_it_be(:x509_certificate) { create(:x509_certificate) }
+ let_it_be(:x509_certificate) { create(:x509_certificate, email: 'r.meier@siemens.com') }
let_it_be(:verification_status) { "verified" }
let(:attributes) do
@@ -49,16 +49,16 @@
end
end
- describe '#reverified_status' do
+ describe '#verification_status' do
let_it_be(:matching_email) { 'r.meier@siemens.com' }
- subject(:reverified_status) { described_class.safe_create!(attributes).reverified_status }
+ subject(:signature) { described_class.safe_create!(attributes) }
context 'when the commit email matches the x509 certificate emails' do
let_it_be(:x509_certificate) { create(:x509_certificate, email: matching_email) }
it 'returns verified' do
- expect(reverified_status).to eq('verified')
+ expect(signature.verification_status).to eq('verified')
end
end
@@ -71,7 +71,7 @@
end
it 'returns verified' do
- expect(reverified_status).to eq('verified')
+ expect(signature.verification_status).to eq('verified')
end
end
@@ -84,7 +84,7 @@
end
it 'returns unverified_author_email' do
- expect(reverified_status).to eq('unverified_author_email')
+ expect(signature.verification_status).to eq('unverified_author_email')
end
end
@@ -94,7 +94,7 @@
end
it 'verification status is unmodified' do
- expect(reverified_status).to eq('verified')
+ expect(signature.verification_status).to eq('verified')
end
end
end
--
GitLab