diff --git a/app/assets/javascripts/commit/components/signature_badge.vue b/app/assets/javascripts/commit/components/signature_badge.vue index 0ec828fa20829739f4d4bfb2c66663e92be83836..70b89d6ca06ff6bc703dc88f00c912788be76ed9 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 417a0e2b6d0090b777ec2b98fa95306af12d69ee..fb68c118cfcb8dc4452e79dce010d535297ef2e0 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 87ed6ea4ff468d5e68f1d739ef6f53cdf28b7c93..df138fdb129059da140d6f51b66e71e2007dbf2e 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 e4260edae409b1ea578aad20209fb5d1d9069389..31216b1fc9d07165135b7a3cda507eaaf7acb6d4 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 92625af58ef887aef731f94630e04436c2c3fb50..0c6b28bbef9623c293075d9aba39e2ac0600c9a6 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 c21387758dc689fbb5b44ccaa057cb710aaae11a..e384a4724f956124bf91cf05c260a8fc944b6bb6 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 d406a5e02b4341f9bae3481bc53a7cc432881546..4796e64a7a81f7206d12010b3a6fc58c7bfcaebd 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 1f96b95716bcc74a713bebf6e5da847f7eaf7720..0f9d40440a28a93f0f32b2919905f64a9e4b8f69 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 4e8ad8e12f1279ae2d9217cbb0ceb3655262c043..ae5f879f470df173fcd90ed7ff894aecbbb6cfd9 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 db88e99970c2e52ddeb498803f4db77225bb951a..5c3410056fbb7fc16162c7fbdc0ae4f61ec42376 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 25aa925263011cd6aea107b8033ff0ff99db0ae1..79393c7f8bd50c3cbee7c02b2f9d7c7bf78c356c 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 5f27212ac33c551011a39871371ada2c6e63450b..a338964ca681087b88b4046702b6e76dae8199b3 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 55e0ef20aee7a5f71069e03386f8607eba008e73..e6061d1703dab6827cff5b4ecdd97d3a4b0b74a7 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 24a49e5bdb824943110f2cbe96ce4cc80c4a91e5..b287c344747cfb969e50d24003e74dce5770aa20 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