diff options
| author | Douwe Maan <douwe@gitlab.com> | 2017-09-05 12:13:46 +0000 |
|---|---|---|
| committer | Jose Ivan Vargas <jvargas@gitlab.com> | 2017-09-08 12:48:00 -0500 |
| commit | 320da597af68560d64663a3aedb3eec84bcf5f61 (patch) | |
| tree | bbe9e3d45a2b980d2933ff3cc0b3d315f56c616b | |
| parent | 74d579b9e02da27bb7692c4501d16667fdfac4e0 (diff) | |
| download | gitlab-ce-320da597af68560d64663a3aedb3eec84bcf5f61.tar.gz | |
Merge branch 'feature/gpg-verification-status' into 'master'
GPG signature must match the committer in order to be verified
See merge request !13771
40 files changed, 863 insertions, 282 deletions
diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 6d7c7e3c930..534eee9101a 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -248,7 +248,10 @@ $(function () { // Initialize popovers $body.popover({ selector: '[data-toggle="popover"]', - trigger: 'focus' + trigger: 'focus', + // set the viewport to the main content, excluding the navigation bar, so + // the navigation can't overlap the popover + viewport: '.page-with-sidebar' }); $('.trigger-submit').on('change', function () { return $(this).parents('form').submit(); diff --git a/app/models/commit.rb b/app/models/commit.rb index 71aa93d0979..96605c9168b 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -393,6 +393,6 @@ class Commit end def gpg_commit - @gpg_commit ||= Gitlab::Gpg::Commit.for_commit(self) + @gpg_commit ||= Gitlab::Gpg::Commit.new(self) end end diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 3df60ddc950..1633acd4fa9 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -56,7 +56,7 @@ class GpgKey < ActiveRecord::Base def verified_user_infos user_infos.select do |user_info| - user_info[:email] == user.email + user.verified_email?(user_info[:email]) end end @@ -64,13 +64,17 @@ class GpgKey < ActiveRecord::Base user_infos.map do |user_info| [ user_info[:email], - user_info[:email] == user.email + user.verified_email?(user_info[:email]) ] end.to_h end def verified? - emails_with_verified_status.any? { |_email, verified| verified } + emails_with_verified_status.values.any? + end + + def verified_and_belongs_to_email?(email) + emails_with_verified_status.fetch(email, false) end def update_invalid_gpg_signatures @@ -78,11 +82,14 @@ class GpgKey < ActiveRecord::Base end def revoke - GpgSignature.where(gpg_key: self, valid_signature: true).update_all( - gpg_key_id: nil, - valid_signature: false, - updated_at: Time.zone.now - ) + GpgSignature + .where(gpg_key: self) + .where.not(verification_status: GpgSignature.verification_statuses[:unknown_key]) + .update_all( + gpg_key_id: nil, + verification_status: GpgSignature.verification_statuses[:unknown_key], + updated_at: Time.zone.now + ) destroy end diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index 50fb35c77ec..454c90d5fc4 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -1,9 +1,21 @@ class GpgSignature < ActiveRecord::Base include ShaAttribute + include IgnorableColumn + + ignore_column :valid_signature sha_attribute :commit_sha sha_attribute :gpg_key_primary_keyid + enum verification_status: { + unverified: 0, + verified: 1, + same_user_different_email: 2, + other_user: 3, + unverified_key: 4, + unknown_key: 5 + } + belongs_to :project belongs_to :gpg_key @@ -20,6 +32,6 @@ class GpgSignature < ActiveRecord::Base end def gpg_commit - Gitlab::Gpg::Commit.new(project, commit_sha) + Gitlab::Gpg::Commit.new(commit) end end diff --git a/app/models/user.rb b/app/models/user.rb index d09e8478b69..eccb29f8566 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1045,6 +1045,10 @@ class User < ActiveRecord::Base ensure_rss_token! end + def verified_email?(email) + self.email == email + end + protected # override, from Devise::Validatable diff --git a/app/views/projects/commit/_invalid_signature_badge.html.haml b/app/views/projects/commit/_invalid_signature_badge.html.haml deleted file mode 100644 index 3a73aae9d95..00000000000 --- a/app/views/projects/commit/_invalid_signature_badge.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -- title = capture do - .gpg-popover-icon.invalid - = render 'shared/icons/icon_status_notfound_borderless.svg' - %div - This commit was signed with an <strong>unverified</strong> signature. - -- locals = { signature: signature, title: title, label: 'Unverified', css_classes: ['invalid'] } - -= render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_other_user_signature_badge.html.haml b/app/views/projects/commit/_other_user_signature_badge.html.haml new file mode 100644 index 00000000000..80eca96f7ce --- /dev/null +++ b/app/views/projects/commit/_other_user_signature_badge.html.haml @@ -0,0 +1,6 @@ +- title = capture do + This commit was signed with a different user's verified signature. + +- locals = { signature: signature, title: title, label: 'Unverified', css_class: 'invalid', icon: 'icon_status_notfound_borderless', show_user: true } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_same_user_different_email_signature_badge.html.haml b/app/views/projects/commit/_same_user_different_email_signature_badge.html.haml new file mode 100644 index 00000000000..e737de48e22 --- /dev/null +++ b/app/views/projects/commit/_same_user_different_email_signature_badge.html.haml @@ -0,0 +1,7 @@ +- title = capture do + This commit was signed with a verified signature, but the committer email + is <strong>not verified</strong> to belong to the same user. + +- locals = { signature: signature, title: title, label: 'Unverified', css_class: ['invalid'], icon: 'icon_status_notfound_borderless', show_user: true } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_signature.html.haml b/app/views/projects/commit/_signature.html.haml index 60fa52557ef..145bc629380 100644 --- a/app/views/projects/commit/_signature.html.haml +++ b/app/views/projects/commit/_signature.html.haml @@ -1,5 +1,2 @@ - if signature - - if signature.valid_signature? - = render partial: 'projects/commit/valid_signature_badge', locals: { signature: signature } - - else - = render partial: 'projects/commit/invalid_signature_badge', locals: { signature: signature } + = render partial: "projects/commit/#{signature.verification_status}_signature_badge", locals: { signature: signature } diff --git a/app/views/projects/commit/_signature_badge.html.haml b/app/views/projects/commit/_signature_badge.html.haml index a3783b31b86..edff018ba6d 100644 --- a/app/views/projects/commit/_signature_badge.html.haml +++ b/app/views/projects/commit/_signature_badge.html.haml @@ -1,18 +1,28 @@ -- css_classes = commit_signature_badge_classes(css_classes) +- signature = local_assigns.fetch(:signature) +- title = local_assigns.fetch(:title) +- label = local_assigns.fetch(:label) +- css_class = local_assigns.fetch(:css_class) +- icon = local_assigns.fetch(:icon) +- show_user = local_assigns.fetch(:show_user, false) + +- css_classes = commit_signature_badge_classes(css_class) - title = capture do .gpg-popover-status - = title + .gpg-popover-icon{ class: css_class } + = render "shared/icons/#{icon}.svg" + %div + = title - content = capture do - .clearfix - = content + - if show_user + .clearfix + = render partial: 'projects/commit/signature_badge_user', locals: { signature: signature } GPG Key ID: %span.monospace= signature.gpg_key_primary_keyid - - = link_to('Learn more about signing commits', help_page_path('user/project/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link') + = link_to('Learn more about signing commits', help_page_path('user/project/repository/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link') %button{ class: css_classes, data: { toggle: 'popover', html: 'true', placement: 'auto top', title: title, content: content } } = label diff --git a/app/views/projects/commit/_signature_badge_user.html.haml b/app/views/projects/commit/_signature_badge_user.html.haml new file mode 100644 index 00000000000..b20198e76db --- /dev/null +++ b/app/views/projects/commit/_signature_badge_user.html.haml @@ -0,0 +1,21 @@ +- gpg_key = signature.gpg_key +- user = gpg_key&.user +- user_name = signature.gpg_key_user_name +- user_email = signature.gpg_key_user_email + +- if user + = link_to user_path(user), class: 'gpg-popover-user-link' do + %div + = user_avatar_without_link(user: user, size: 32) + + %div + %strong= user.name + %div= user.to_reference +- else + = mail_to user_email do + %div + = user_avatar_without_link(user_name: user_name, user_email: user_email, size: 32) + + %div + %strong= user_name + %div= user_email diff --git a/app/views/projects/commit/_unknown_key_signature_badge.html.haml b/app/views/projects/commit/_unknown_key_signature_badge.html.haml new file mode 100644 index 00000000000..75c5cf57bcc --- /dev/null +++ b/app/views/projects/commit/_unknown_key_signature_badge.html.haml @@ -0,0 +1 @@ += render partial: 'projects/commit/unverified_signature_badge', locals: { signature: signature } diff --git a/app/views/projects/commit/_unverified_key_signature_badge.html.haml b/app/views/projects/commit/_unverified_key_signature_badge.html.haml new file mode 100644 index 00000000000..75c5cf57bcc --- /dev/null +++ b/app/views/projects/commit/_unverified_key_signature_badge.html.haml @@ -0,0 +1 @@ += render partial: 'projects/commit/unverified_signature_badge', locals: { signature: signature } diff --git a/app/views/projects/commit/_unverified_signature_badge.html.haml b/app/views/projects/commit/_unverified_signature_badge.html.haml new file mode 100644 index 00000000000..1af58027b83 --- /dev/null +++ b/app/views/projects/commit/_unverified_signature_badge.html.haml @@ -0,0 +1,6 @@ +- title = capture do + This commit was signed with an <strong>unverified</strong> signature. + +- locals = { signature: signature, title: title, label: 'Unverified', css_class: 'invalid', icon: 'icon_status_notfound_borderless' } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_valid_signature_badge.html.haml b/app/views/projects/commit/_valid_signature_badge.html.haml deleted file mode 100644 index db1a41bbf64..00000000000 --- a/app/views/projects/commit/_valid_signature_badge.html.haml +++ /dev/null @@ -1,32 +0,0 @@ -- title = capture do - .gpg-popover-icon.valid - = render 'shared/icons/icon_status_success_borderless.svg' - %div - This commit was signed with a <strong>verified</strong> signature. - -- content = capture do - - gpg_key = signature.gpg_key - - user = gpg_key&.user - - user_name = signature.gpg_key_user_name - - user_email = signature.gpg_key_user_email - - - if user - = link_to user_path(user), class: 'gpg-popover-user-link' do - %div - = user_avatar_without_link(user: user, size: 32) - - %div - %strong= gpg_key.user.name - %div @#{gpg_key.user.username} - - else - = mail_to user_email do - %div - = user_avatar_without_link(user_name: user_name, user_email: user_email, size: 32) - - %div - %strong= user_name - %div= user_email - -- locals = { signature: signature, title: title, content: content, label: 'Verified', css_classes: ['valid'] } - -= render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/views/projects/commit/_verified_signature_badge.html.haml b/app/views/projects/commit/_verified_signature_badge.html.haml new file mode 100644 index 00000000000..423beba2120 --- /dev/null +++ b/app/views/projects/commit/_verified_signature_badge.html.haml @@ -0,0 +1,7 @@ +- title = capture do + This commit was signed with a <strong>verified</strong> signature and the + committer email is verified to belong to the same user. + +- locals = { signature: signature, title: title, label: 'Verified', css_class: 'valid', icon: 'icon_status_success_borderless', show_user: true } + += render partial: 'projects/commit/signature_badge', locals: locals diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index f34dff2d656..9b5ff17aafa 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -6,7 +6,11 @@ class CreateGpgSignatureWorker project = Project.find_by(id: project_id) return unless project + commit = project.commit(commit_sha) + + return unless commit + # This calculates and caches the signature in the database - Gitlab::Gpg::Commit.new(project, commit_sha).signature + Gitlab::Gpg::Commit.new(commit).signature end end diff --git a/changelogs/unreleased/feature-gpg-verification-status.yml b/changelogs/unreleased/feature-gpg-verification-status.yml new file mode 100644 index 00000000000..7518fafcdb8 --- /dev/null +++ b/changelogs/unreleased/feature-gpg-verification-status.yml @@ -0,0 +1,6 @@ +--- +title: 'Update the GPG verification semantics: A GPG signature must additionally match + the committer in order to be verified' +merge_request: 13771 +author: Alexis Reigel +type: changed diff --git a/db/migrate/20170817123339_add_verification_status_to_gpg_signatures.rb b/db/migrate/20170817123339_add_verification_status_to_gpg_signatures.rb new file mode 100644 index 00000000000..128cd109f8d --- /dev/null +++ b/db/migrate/20170817123339_add_verification_status_to_gpg_signatures.rb @@ -0,0 +1,20 @@ +class AddVerificationStatusToGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + def up + # First we remove all signatures because we need to re-verify them all + # again anyway (because of the updated verification logic). + # + # This makes adding the column with default values faster + truncate(:gpg_signatures) + + add_column_with_default(:gpg_signatures, :verification_status, :smallint, default: 0) + end + + def down + remove_column(:gpg_signatures, :verification_status) + end +end diff --git a/db/post_migrate/20170830084744_destroy_gpg_signatures.rb b/db/post_migrate/20170830084744_destroy_gpg_signatures.rb new file mode 100644 index 00000000000..b04d36f6537 --- /dev/null +++ b/db/post_migrate/20170830084744_destroy_gpg_signatures.rb @@ -0,0 +1,10 @@ +class DestroyGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + def up + truncate(:gpg_signatures) + end + + def down + end +end diff --git a/db/post_migrate/20170831195038_remove_valid_signature_from_gpg_signatures.rb b/db/post_migrate/20170831195038_remove_valid_signature_from_gpg_signatures.rb new file mode 100644 index 00000000000..9b6745e33d9 --- /dev/null +++ b/db/post_migrate/20170831195038_remove_valid_signature_from_gpg_signatures.rb @@ -0,0 +1,11 @@ +class RemoveValidSignatureFromGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + def up + remove_column :gpg_signatures, :valid_signature + end + + def down + add_column :gpg_signatures, :valid_signature, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 3e2c407ddfc..71efe05c659 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170824162758) do +ActiveRecord::Schema.define(version: 20170831195038) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -596,11 +596,11 @@ ActiveRecord::Schema.define(version: 20170824162758) do t.datetime "updated_at", null: false t.integer "project_id" t.integer "gpg_key_id" - t.boolean "valid_signature" t.binary "commit_sha" t.binary "gpg_key_primary_keyid" t.text "gpg_key_user_name" t.text "gpg_key_user_email" + t.integer "verification_status", limit: 2, default: 0, null: false end add_index "gpg_signatures", ["commit_sha"], name: "index_gpg_signatures_on_commit_sha", unique: true, using: :btree diff --git a/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png b/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png Binary files differindex 33936a7d6d7..088ecfa6d89 100644 --- a/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png +++ b/doc/user/project/gpg_signed_commits/img/project_signed_and_unsigned_commits.png diff --git a/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png b/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png Binary files differindex 22565cf7c7e..4e3392406b1 100644 --- a/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png +++ b/doc/user/project/gpg_signed_commits/img/project_signed_commit_unverified_signature.png diff --git a/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png b/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png Binary files differindex 1778b2ddf2b..766970dee81 100644 --- a/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png +++ b/doc/user/project/gpg_signed_commits/img/project_signed_commit_verified_signature.png diff --git a/doc/user/project/repository/gpg_signed_commits/index.md b/doc/user/project/repository/gpg_signed_commits/index.md new file mode 100644 index 00000000000..afe8066d408 --- /dev/null +++ b/doc/user/project/repository/gpg_signed_commits/index.md @@ -0,0 +1,246 @@ +# Signing commits with GPG + +> [Introduced][ce-9546] in GitLab 9.5. + +GitLab can show whether a commit is verified or not when signed with a GPG key. +All you need to do is upload the public GPG key in your profile settings. + +GPG verified tags are not supported yet. + +## Getting started with GPG + +Here are a few guides to get you started with GPG: + +- [Git Tools - Signing Your Work](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work) +- [Managing OpenPGP Keys](https://riseup.net/en/security/message-security/openpgp/gpg-keys) +- [OpenPGP Best Practices](https://riseup.net/en/security/message-security/openpgp/best-practices) +- [Creating a new GPG key with subkeys](https://www.void.gr/kargig/blog/2013/12/02/creating-a-new-gpg-key-with-subkeys/) (advanced) + +## How GitLab handles GPG + +GitLab uses its own keyring to verify the GPG signature. It does not access any +public key server. + +In order to have a commit verified on GitLab the corresponding public key needs +to be uploaded to GitLab. For a signature to be verified three conditions need +to be met: + +1. The public key needs to be added your GitLab account +1. One of the emails in the GPG key matches your **primary** email +1. The committer's email matches the verified email from the gpg key + +## Generating a GPG key + +If you don't already have a GPG key, the following steps will help you get +started: + +1. [Install GPG](https://www.gnupg.org/download/index.html) for your operating system +1. Generate the private/public key pair with the following command: + + ```sh + gpg --full-gen-key + ``` + + This will spawn a series of questions. + +1. The first question is which algorithm can be used. Select the kind you want + or press <kbd>Enter</kbd> to choose the default (RSA and RSA): + + ``` + Please select what kind of key you want: + (1) RSA and RSA (default) + (2) DSA and Elgamal + (3) DSA (sign only) + (4) RSA (sign only) + Your selection? 1 + ``` + +1. The next question is key length. We recommend to choose the highest value + which is `4096`: + + ``` + RSA keys may be between 1024 and 4096 bits long. + What keysize do you want? (2048) 4096 + Requested keysize is 4096 bits + ``` +1. Next, you need to specify the validity period of your key. This is something + subjective, and you can use the default value which is to never expire: + + ``` + Please specify how long the key should be valid. + 0 = key does not expire + <n> = key expires in n days + <n>w = key expires in n weeks + <n>m = key expires in n months + <n>y = key expires in n years + Key is valid for? (0) 0 + Key does not expire at all + ``` + +1. Confirm that the answers you gave were correct by typing `y`: + + ``` + Is this correct? (y/N) y + ``` + +1. Enter you real name, the email address to be associated with this key (should + match the primary email address you use in GitLab) and an optional comment + (press <kbd>Enter</kbd> to skip): + + ``` + GnuPG needs to construct a user ID to identify your key. + + Real name: Mr. Robot + Email address: mr@robot.sh + Comment: + You selected this USER-ID: + "Mr. Robot <mr@robot.sh>" + + Change (N)ame, (C)omment, (E)mail or (O)kay/(Q)uit? O + ``` + +1. Pick a strong password when asked and type it twice to confirm. +1. Use the following command to list the private GPG key you just created: + + ``` + gpg --list-secret-keys mr@robot.sh + ``` + + Replace `mr@robot.sh` with the email address you entered above. + +1. Copy the GPG key ID that starts with `sec`. In the following example, that's + `0x30F2B65B9246B6CA`: + + ``` + sec rsa4096/0x30F2B65B9246B6CA 2017-08-18 [SC] + D5E4F29F3275DC0CDA8FFC8730F2B65B9246B6CA + uid [ultimate] Mr. Robot <mr@robot.sh> + ssb rsa4096/0xB7ABC0813E4028C0 2017-08-18 [E] + ``` + +1. Export the public key of that ID (replace your key ID from the previous step): + + ``` + gpg --armor --export 0x30F2B65B9246B6CA + ``` + +1. Finally, copy the public key and [add it in your profile settings](#adding-a-gpg-key-to-your-account) + +## Adding a GPG key to your account + +>**Note:** +Once you add a key, you cannot edit it, only remove it. In case the paste +didn't work, you'll have to remove the offending key and re-add it. + +You can add a GPG key in your profile's settings: + +1. On the upper right corner, click on your avatar and go to your **Settings**. + +  + +1. Navigate to the **GPG keys** tab and paste your _public_ key in the 'Key' + box. + +  + +1. Finally, click on **Add key** to add it to GitLab. You will be able to see + its fingerprint, the corresponding email address and creation date. + +  + +## Associating your GPG key with Git + +After you have [created your GPG key](#generating-a-gpg-key) and [added it to +your account](#adding-a-gpg-key-to-your-account), it's time to tell Git which +key to use. + +1. Use the following command to list the private GPG key you just created: + + ``` + gpg --list-secret-keys mr@robot.sh + ``` + + Replace `mr@robot.sh` with the email address you entered above. + +1. Copy the GPG key ID that starts with `sec`. In the following example, that's + `0x30F2B65B9246B6CA`: + + ``` + sec rsa4096/0x30F2B65B9246B6CA 2017-08-18 [SC] + D5E4F29F3275DC0CDA8FFC8730F2B65B9246B6CA + uid [ultimate] Mr. Robot <mr@robot.sh> + ssb rsa4096/0xB7ABC0813E4028C0 2017-08-18 [E] + ``` + +1. Tell Git to use that key to sign the commits: + + ``` + git config --global user.signingkey 0x30F2B65B9246B6CA + ``` + + Replace `0x30F2B65B9246B6CA` with your GPG key ID. + +## Signing commits + +After you have [created your GPG key](#generating-a-gpg-key) and [added it to +your account](#adding-a-gpg-key-to-your-account), you can start signing your +commits: + +1. Commit like you used to, the only difference is the addition of the `-S` flag: + + ``` + git commit -S -m "My commit msg" + ``` + +1. Enter the passphrase of your GPG key when asked. +1. Push to GitLab and check that your commits [are verified](#verifying-commits). + +If you don't want to type the `-S` flag every time you commit, you can tell Git +to sign your commits automatically: + +``` +git config --global commit.gpgsign true +``` + +## Verifying commits + +1. Within a project or [merge request](../../merge_requests/index.md), navigate to + the **Commits** tab. Signed commits will show a badge containing either + "Verified" or "Unverified", depending on the verification status of the GPG + signature. + +  + +1. By clicking on the GPG badge, details of the signature are displayed. + +  + +  + +## Revoking a GPG key + +Revoking a key **unverifies** already signed commits. Commits that were +verified by using this key will change to an unverified state. Future commits +will also stay unverified once you revoke this key. This action should be used +in case your key has been compromised. + +To revoke a GPG key: + +1. On the upper right corner, click on your avatar and go to your **Settings**. +1. Navigate to the **GPG keys** tab. +1. Click on **Revoke** besides the GPG key you want to delete. + +## Removing a GPG key + +Removing a key **does not unverify** already signed commits. Commits that were +verified by using this key will stay verified. Only unpushed commits will stay +unverified once you remove this key. To unverify already signed commits, you need +to [revoke the associated GPG key](#revoking-a-gpg-key) from your account. + +To remove a GPG key from your account: + +1. On the upper right corner, click on your avatar and go to your **Settings**. +1. Navigate to the **GPG keys** tab. +1. Click on the trash icon besides the GPG key you want to delete. + +[ce-9546]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9546 diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 45e9f9d65ae..025f826e65f 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -39,7 +39,7 @@ module Gitlab fingerprints = CurrentKeyChain.fingerprints_from_key(key) GPGME::Key.find(:public, fingerprints).flat_map do |raw_key| - raw_key.uids.map { |uid| { name: uid.name, email: uid.email } } + raw_key.uids.map { |uid| { name: uid.name, email: uid.email.downcase } } end end end diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 606c7576f70..86bd9f5b125 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -1,17 +1,12 @@ module Gitlab module Gpg class Commit - def self.for_commit(commit) - new(commit.project, commit.sha) - end - - def initialize(project, sha) - @project = project - @sha = sha + def initialize(commit) + @commit = commit @signature_text, @signed_text = begin - Rugged::Commit.extract_signature(project.repository.rugged, sha) + Rugged::Commit.extract_signature(@commit.project.repository.rugged, @commit.sha) rescue Rugged::OdbError nil end @@ -26,7 +21,7 @@ module Gitlab return @signature if @signature - cached_signature = GpgSignature.find_by(commit_sha: @sha) + cached_signature = GpgSignature.find_by(commit_sha: @commit.sha) return @signature = cached_signature if cached_signature.present? @signature = create_cached_signature! @@ -73,20 +68,31 @@ module Gitlab def attributes(gpg_key) user_infos = user_infos(gpg_key) + verification_status = verification_status(gpg_key) { - commit_sha: @sha, - project: @project, + commit_sha: @commit.sha, + project: @commit.project, gpg_key: gpg_key, gpg_key_primary_keyid: gpg_key&.primary_keyid || verified_signature.fingerprint, gpg_key_user_name: user_infos[:name], gpg_key_user_email: user_infos[:email], - valid_signature: gpg_signature_valid_signature_value(gpg_key) + verification_status: verification_status } end - def gpg_signature_valid_signature_value(gpg_key) - !!(gpg_key && gpg_key.verified? && verified_signature.valid?) + def verification_status(gpg_key) + return :unknown_key unless gpg_key + return :unverified_key unless gpg_key.verified? + return :unverified unless verified_signature.valid? + + if gpg_key.verified_and_belongs_to_email?(@commit.committer_email) + :verified + elsif gpg_key.user.all_emails.include?(@commit.committer_email) + :same_user_different_email + else + :other_user + end end def user_infos(gpg_key) diff --git a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb index a525ee7a9ee..e085eab26c9 100644 --- a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb +++ b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb @@ -8,7 +8,7 @@ module Gitlab def run GpgSignature .select(:id, :commit_sha, :project_id) - .where('gpg_key_id IS NULL OR valid_signature = ?', false) + .where('gpg_key_id IS NULL OR verification_status <> ?', GpgSignature.verification_statuses[:verified]) .where(gpg_key_primary_keyid: @gpg_key.primary_keyid) .find_each { |sig| sig.gpg_commit.update_signature!(sig) } end diff --git a/spec/factories/gpg_signature.rb b/spec/factories/gpg_signature.rb index a5aeffbe12d..c0beecf0bea 100644 --- a/spec/factories/gpg_signature.rb +++ b/spec/factories/gpg_signature.rb @@ -6,6 +6,6 @@ FactoryGirl.define do project gpg_key gpg_key_primary_keyid { gpg_key.primary_keyid } - valid_signature true + verification_status :verified end end diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 0c9fcc60d30..479fb713297 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -203,105 +203,4 @@ describe 'Commits' do end end end - - describe 'GPG signed commits', :js do - it 'changes from unverified to verified when the user changes his email to match the gpg key' do - user = create :user, email: 'unrelated.user@example.org' - project.team << [user, :master] - - Sidekiq::Testing.inline! do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - sign_in(user) - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).not_to have_content 'Verified' - end - - # user changes his email which makes the gpg key verified - Sidekiq::Testing.inline! do - user.skip_reconfirmation! - user.update_attributes!(email: GpgHelpers::User1.emails.first) - end - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).to have_content 'Verified' - end - end - - it 'changes from unverified to verified when the user adds the missing gpg key' do - user = create :user, email: GpgHelpers::User1.emails.first - project.team << [user, :master] - - sign_in(user) - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).not_to have_content 'Verified' - end - - # user adds the gpg key which makes the signature valid - Sidekiq::Testing.inline! do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - visit project_commits_path(project, :'signed-commits') - - within '#commits-list' do - expect(page).to have_content 'Unverified' - expect(page).to have_content 'Verified' - end - end - - it 'shows popover badges' do - gpg_user = create :user, email: GpgHelpers::User1.emails.first, username: 'nannie.bernhard', name: 'Nannie Bernhard' - Sidekiq::Testing.inline! do - create :gpg_key, key: GpgHelpers::User1.public_key, user: gpg_user - end - - user = create :user - project.team << [user, :master] - - sign_in(user) - visit project_commits_path(project, :'signed-commits') - - # unverified signature - click_on 'Unverified', match: :first - within '.popover' do - expect(page).to have_content 'This commit was signed with an unverified signature.' - expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" - end - - # verified and the gpg user has a gitlab profile - click_on 'Verified', match: :first - within '.popover' do - expect(page).to have_content 'This commit was signed with a verified signature.' - expect(page).to have_content 'Nannie Bernhard' - expect(page).to have_content '@nannie.bernhard' - expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" - end - - # verified and the gpg user's profile doesn't exist anymore - gpg_user.destroy! - - visit project_commits_path(project, :'signed-commits') - - click_on 'Verified', match: :first - within '.popover' do - expect(page).to have_content 'This commit was signed with a verified signature.' - expect(page).to have_content 'Nannie Bernhard' - expect(page).to have_content 'nannie.bernhard@example.com' - expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" - end - end - end end diff --git a/spec/features/profiles/gpg_keys_spec.rb b/spec/features/profiles/gpg_keys_spec.rb index 6edc482b47e..623e4f341c5 100644 --- a/spec/features/profiles/gpg_keys_spec.rb +++ b/spec/features/profiles/gpg_keys_spec.rb @@ -42,7 +42,7 @@ feature 'Profile > GPG Keys' do scenario 'User revokes a key via the key index' do gpg_key = create :gpg_key, user: user, key: GpgHelpers::User2.public_key - gpg_signature = create :gpg_signature, gpg_key: gpg_key, valid_signature: true + gpg_signature = create :gpg_signature, gpg_key: gpg_key, verification_status: :verified visit profile_gpg_keys_path @@ -51,7 +51,7 @@ feature 'Profile > GPG Keys' do expect(page).to have_content('Your GPG keys (0)') expect(gpg_signature.reload).to have_attributes( - valid_signature: false, + verification_status: 'unknown_key', gpg_key: nil ) end diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb new file mode 100644 index 00000000000..8efa5b58141 --- /dev/null +++ b/spec/features/signed_commits_spec.rb @@ -0,0 +1,179 @@ +require 'spec_helper' + +describe 'GPG signed commits', :js do + let(:project) { create(:project, :repository) } + + it 'changes from unverified to verified when the user changes his email to match the gpg key' do + user = create :user, email: 'unrelated.user@example.org' + project.team << [user, :master] + + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + sign_in(user) + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).not_to have_content 'Verified' + end + + # user changes his email which makes the gpg key verified + Sidekiq::Testing.inline! do + user.skip_reconfirmation! + user.update_attributes!(email: GpgHelpers::User1.emails.first) + end + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).to have_content 'Verified' + end + end + + it 'changes from unverified to verified when the user adds the missing gpg key' do + user = create :user, email: GpgHelpers::User1.emails.first + project.team << [user, :master] + + sign_in(user) + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).not_to have_content 'Verified' + end + + # user adds the gpg key which makes the signature valid + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + visit project_commits_path(project, :'signed-commits') + + within '#commits-list' do + expect(page).to have_content 'Unverified' + expect(page).to have_content 'Verified' + end + end + + context 'shows popover badges' do + let(:user_1) do + create :user, email: GpgHelpers::User1.emails.first, username: 'nannie.bernhard', name: 'Nannie Bernhard' + end + + let(:user_1_key) do + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user_1 + end + end + + let(:user_2) do + create(:user, email: GpgHelpers::User2.emails.first, username: 'bette.cartwright', name: 'Bette Cartwright').tap do |user| + # secondary, unverified email + create :email, user: user, email: GpgHelpers::User2.emails.last + end + end + + let(:user_2_key) do + Sidekiq::Testing.inline! do + create :gpg_key, key: GpgHelpers::User2.public_key, user: user_2 + end + end + + before do + user = create :user + project.team << [user, :master] + + sign_in(user) + end + + it 'unverified signature' do + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed commit by bette cartwright')) do + click_on 'Unverified' + within '.popover' do + expect(page).to have_content 'This commit was signed with an unverified signature.' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" + end + end + end + + it 'unverified signature: user email does not match the committer email, but is the same user' do + user_2_key + + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed and authored commit by bette cartwright, different email')) do + click_on 'Unverified' + within '.popover' do + expect(page).to have_content 'This commit was signed with a verified signature, but the committer email is not verified to belong to the same user.' + expect(page).to have_content 'Bette Cartwright' + expect(page).to have_content '@bette.cartwright' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" + end + end + end + + it 'unverified signature: user email does not match the committer email' do + user_2_key + + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed commit by bette cartwright')) do + click_on 'Unverified' + within '.popover' do + expect(page).to have_content "This commit was signed with a different user's verified signature." + expect(page).to have_content 'Bette Cartwright' + expect(page).to have_content '@bette.cartwright' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User2.primary_keyid}" + end + end + end + + it 'verified and the gpg user has a gitlab profile' do + user_1_key + + visit project_commits_path(project, :'signed-commits') + + within(find('.commit', text: 'signed and authored commit by nannie bernhard')) do + click_on 'Verified' + within '.popover' do + expect(page).to have_content 'This commit was signed with a verified signature and the committer email is verified to belong to the same user.' + expect(page).to have_content 'Nannie Bernhard' + expect(page).to have_content '@nannie.bernhard' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" + end + end + end + + it "verified and the gpg user's profile doesn't exist anymore" do + user_1_key + + visit project_commits_path(project, :'signed-commits') + + # wait for the signature to get generated + within(find('.commit', text: 'signed and authored commit by nannie bernhard')) do + expect(page).to have_content 'Verified' + end + + user_1.destroy! + + refresh + + within(find('.commit', text: 'signed and authored commit by nannie bernhard')) do + click_on 'Verified' + within '.popover' do + expect(page).to have_content 'This commit was signed with a verified signature and the committer email is verified to belong to the same user.' + expect(page).to have_content 'Nannie Bernhard' + expect(page).to have_content 'nannie.bernhard@example.com' + expect(page).to have_content "GPG Key ID: #{GpgHelpers::User1.primary_keyid}" + end + end + end + end +end diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index e521fcc6dc1..b07462e4978 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -2,45 +2,9 @@ require 'rails_helper' describe Gitlab::Gpg::Commit do describe '#signature' do - let!(:project) { create :project, :repository, path: 'sample-project' } - let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } - - context 'unsigned commit' do - it 'returns nil' do - expect(described_class.new(project, commit_sha).signature).to be_nil - end - end - - context 'known and verified public key' do - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: create(:user, email: GpgHelpers::User1.emails.first) - end - - before do - allow(Rugged::Commit).to receive(:extract_signature) - .with(Rugged::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) - end - - it 'returns a valid signature' do - expect(described_class.new(project, commit_sha).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - valid_signature: true - ) - end - + shared_examples 'returns the cached signature on second call' do it 'returns the cached signature on second call' do - gpg_commit = described_class.new(project, commit_sha) + gpg_commit = described_class.new(commit) expect(gpg_commit).to receive(:using_keychain).and_call_original gpg_commit.signature @@ -51,11 +15,140 @@ describe Gitlab::Gpg::Commit do end end - context 'known but unverified public key' do - let!(:gpg_key) { create :gpg_key, key: GpgHelpers::User1.public_key } + let!(:project) { create :project, :repository, path: 'sample-project' } + let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } - before do - allow(Rugged::Commit).to receive(:extract_signature) + context 'unsigned commit' do + let!(:commit) { create :commit, project: project, sha: commit_sha } + + it 'returns nil' do + expect(described_class.new(commit).signature).to be_nil + end + end + + context 'known key' do + context 'user matches the key uid' do + context 'user email matches the email committer' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } + + let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) + .with(Rugged::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns a valid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'verified' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + + context 'user email does not match the committer email, but is the same user' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } + + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, user: user, email: GpgHelpers::User2.emails.first + end + end + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) + .with(Rugged::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'same_user_different_email' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + + context 'user email does not match the committer email' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } + + let(:user) { create(:user, email: GpgHelpers::User1.emails.first) } + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) + .with(Rugged::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'other_user' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + end + + context 'user does not match the key uid' do + let!(:commit) { create :commit, project: project, sha: commit_sha } + + let(:user) { create(:user, email: GpgHelpers::User2.emails.first) } + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) .with(Rugged::Repository, commit_sha) .and_return( [ @@ -63,33 +156,27 @@ describe Gitlab::Gpg::Commit do GpgHelpers::User1.signed_commit_base_data ] ) - end - - it 'returns an invalid signature' do - expect(described_class.new(project, commit_sha).signature).to have_attributes( - commit_sha: commit_sha, - project: project, - gpg_key: gpg_key, - gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - gpg_key_user_name: GpgHelpers::User1.names.first, - gpg_key_user_email: GpgHelpers::User1.emails.first, - valid_signature: false - ) - end - - it 'returns the cached signature on second call' do - gpg_commit = described_class.new(project, commit_sha) - - expect(gpg_commit).to receive(:using_keychain).and_call_original - gpg_commit.signature + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'unverified_key' + ) + end - # consecutive call - expect(gpg_commit).not_to receive(:using_keychain).and_call_original - gpg_commit.signature + it_behaves_like 'returns the cached signature on second call' end end - context 'unknown public key' do + context 'unknown key' do + let!(:commit) { create :commit, project: project, sha: commit_sha } + before do allow(Rugged::Commit).to receive(:extract_signature) .with(Rugged::Repository, commit_sha) @@ -102,27 +189,18 @@ describe Gitlab::Gpg::Commit do end it 'returns an invalid signature' do - expect(described_class.new(project, commit_sha).signature).to have_attributes( + expect(described_class.new(commit).signature).to have_attributes( commit_sha: commit_sha, project: project, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, gpg_key_user_name: nil, gpg_key_user_email: nil, - valid_signature: false + verification_status: 'unknown_key' ) end - it 'returns the cached signature on second call' do - gpg_commit = described_class.new(project, commit_sha) - - expect(gpg_commit).to receive(:using_keychain).and_call_original - gpg_commit.signature - - # consecutive call - expect(gpg_commit).not_to receive(:using_keychain).and_call_original - gpg_commit.signature - end + it_behaves_like 'returns the cached signature on second call' end end end 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 4de4419de27..b9fd4d02156 100644 --- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb +++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb @@ -4,8 +4,29 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do describe '#run' do let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } let!(:project) { create :project, :repository, path: 'sample-project' } + let!(:raw_commit) do + raw_commit = double( + :raw_commit, + signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], + sha: commit_sha, + committer_email: GpgHelpers::User1.emails.first + ) + + allow(raw_commit).to receive :save! + + raw_commit + end + + let!(:commit) do + create :commit, git_commit: raw_commit, project: project + end before do + allow_any_instance_of(Project).to receive(:commit).and_return(commit) + allow(Rugged::Commit).to receive(:extract_signature) .with(Rugged::Repository, commit_sha) .and_return( @@ -25,7 +46,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' end it 'assigns the gpg key to the signature when the missing gpg key is added' do @@ -39,7 +60,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end @@ -54,7 +75,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end end @@ -68,7 +89,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unknown_key' end it 'updates the signature to being valid when the missing gpg key is added' do @@ -82,7 +103,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end @@ -97,7 +118,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unknown_key' ) end end @@ -115,7 +136,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: nil, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unknown_key' end it 'updates the signature to being valid when the user updates the email address' do @@ -123,7 +144,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do key: GpgHelpers::User1.public_key, user: user - expect(invalid_gpg_signature.reload.valid_signature).to be_falsey + expect(invalid_gpg_signature.reload.verification_status).to eq 'unverified_key' # InvalidGpgSignatureUpdater is called by the after_update hook user.update_attributes!(email: GpgHelpers::User1.emails.first) @@ -133,7 +154,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: true + verification_status: 'verified' ) end @@ -147,7 +168,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unverified_key' ) # InvalidGpgSignatureUpdater is called by the after_update hook @@ -158,7 +179,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do commit_sha: commit_sha, gpg_key: gpg_key, gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, - valid_signature: false + verification_status: 'unverified_key' ) end end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index 30ad033b204..11a2aea1915 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -42,6 +42,21 @@ describe Gitlab::Gpg do described_class.user_infos_from_key('bogus') ).to eq [] end + + it 'downcases the email' do + public_key = double(:key) + fingerprints = double(:fingerprints) + uid = double(:uid, name: 'Nannie Bernhard', email: 'NANNIE.BERNHARD@EXAMPLE.COM') + raw_key = double(:raw_key, uids: [uid]) + allow(Gitlab::Gpg::CurrentKeyChain).to receive(:fingerprints_from_key).with(public_key).and_return(fingerprints) + allow(GPGME::Key).to receive(:find).with(:public, anything).and_return([raw_key]) + + user_infos = described_class.user_infos_from_key(public_key) + expect(user_infos).to eq([{ + name: 'Nannie Bernhard', + email: 'nannie.bernhard@example.com' + }]) + end end describe '.current_home_dir' do diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index e48f20bf53b..9c99c3e5c08 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -99,14 +99,14 @@ describe GpgKey do end describe '#verified?' do - it 'returns true one of the email addresses in the key belongs to the user' do + it 'returns true if one of the email addresses in the key belongs to the user' do user = create :user, email: 'bette.cartwright@example.com' gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user expect(gpg_key.verified?).to be_truthy end - it 'returns false if one of the email addresses in the key does not belong to the user' do + it 'returns false if none of the email addresses in the key does not belong to the user' do user = create :user, email: 'someone.else@example.com' gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user @@ -114,6 +114,32 @@ describe GpgKey do end end + describe 'verified_and_belongs_to_email?' do + it 'returns false if none of the email addresses in the key does not belong to the user' do + user = create :user, email: 'someone.else@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + + expect(gpg_key.verified?).to be_falsey + expect(gpg_key.verified_and_belongs_to_email?('someone.else@example.com')).to be_falsey + end + + it 'returns false if one of the email addresses in the key belongs to the user and does not match the provided email' do + user = create :user, email: 'bette.cartwright@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + + expect(gpg_key.verified?).to be_truthy + expect(gpg_key.verified_and_belongs_to_email?('bette.cartwright@example.net')).to be_falsey + end + + it 'returns true if one of the email addresses in the key belongs to the user and matches the provided email' do + user = create :user, email: 'bette.cartwright@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + + expect(gpg_key.verified?).to be_truthy + expect(gpg_key.verified_and_belongs_to_email?('bette.cartwright@example.com')).to be_truthy + end + end + describe 'notification', :mailer do let(:user) { create(:user) } @@ -129,15 +155,15 @@ describe GpgKey do describe '#revoke' do it 'invalidates all associated gpg signatures and destroys the key' do gpg_key = create :gpg_key - gpg_signature = create :gpg_signature, valid_signature: true, gpg_key: gpg_key + gpg_signature = create :gpg_signature, verification_status: :verified, gpg_key: gpg_key unrelated_gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key - unrelated_gpg_signature = create :gpg_signature, valid_signature: true, gpg_key: unrelated_gpg_key + unrelated_gpg_signature = create :gpg_signature, verification_status: :verified, gpg_key: unrelated_gpg_key gpg_key.revoke expect(gpg_signature.reload).to have_attributes( - valid_signature: false, + verification_status: 'unknown_key', gpg_key: nil ) @@ -145,7 +171,7 @@ describe GpgKey do # unrelated signature is left untouched expect(unrelated_gpg_signature.reload).to have_attributes( - valid_signature: true, + verification_status: 'verified', gpg_key: unrelated_gpg_key ) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e25ffa4d228..3bfdc8e0d17 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2074,4 +2074,18 @@ describe User do end end end + + describe '#verified_email?' do + it 'returns true when the email is the primary email' do + user = build :user, email: 'email@example.com' + + expect(user.verified_email?('email@example.com')).to be true + end + + it 'returns false when the email is not the primary email' do + user = build :user, email: 'email@example.com' + + expect(user.verified_email?('other_email@example.com')).to be false + end + end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 1e39f80699c..290ded3ff7e 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -5,7 +5,7 @@ module TestEnv # When developing the seed repository, comment out the branch you will modify. BRANCH_SHA = { - 'signed-commits' => '5d4a1cb', + 'signed-commits' => '2d1096e', 'not-merged-branch' => 'b83d6e3', 'branch-merged' => '498214d', 'empty-branch' => '7efb185', diff --git a/spec/workers/create_gpg_signature_worker_spec.rb b/spec/workers/create_gpg_signature_worker_spec.rb index 54978baca88..aa6c347d738 100644 --- a/spec/workers/create_gpg_signature_worker_spec.rb +++ b/spec/workers/create_gpg_signature_worker_spec.rb @@ -7,9 +7,14 @@ describe CreateGpgSignatureWorker do let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } it 'calls Gitlab::Gpg::Commit#signature' do - expect(Gitlab::Gpg::Commit).to receive(:new).with(project, commit_sha).and_call_original + commit = instance_double(Commit) + gpg_commit = instance_double(Gitlab::Gpg::Commit) - expect_any_instance_of(Gitlab::Gpg::Commit).to receive(:signature) + allow(Project).to receive(:find_by).with(id: project.id).and_return(project) + allow(project).to receive(:commit).with(commit_sha).and_return(commit) + + expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit) + expect(gpg_commit).to receive(:signature) described_class.new.perform(commit_sha, project.id) end |
